diff --git a/web/src/lib/api/pullCompose.ts b/web/src/lib/api/pullCompose.ts index 74cef72fa..349cb13c2 100644 --- a/web/src/lib/api/pullCompose.ts +++ b/web/src/lib/api/pullCompose.ts @@ -1,4 +1,17 @@ -import type { BranchEntry } from "./repo"; +import type { Did } from "@atcute/lexicons/syntax"; +import type { RepoInfo } from "$lib/components/repo/types"; +import { createBobbinClient } from "./client"; +import { branches as listBranches, gitTarget, type GitServiceConfig } from "./gitclient"; +import { listCommits, toMirrorCommitSummary } from "./gitmirror"; +import { listRepos } from "./records"; +import { + repoNameOf, + sortBranches, + toBranchSummary, + type BranchEntry, + type BranchSummary, + type CommitSummary +} from "./repo"; export type PullSource = "patch" | "branch" | "fork"; @@ -67,3 +80,143 @@ export const composeQuery = (selection: ComposeSelection): string => { if (selection.source === "fork" && selection.fork) query.set("fork", selection.fork); return query.toString(); }; + +const BRANCH_LIMIT = 500; +const COMMIT_LIMIT = 100; + +// one of the viewer's forks, as the fork picker needs it +export interface ForkOption { + uri: string; + repoDid: string; + name: string; + knot: string; +} + +export interface ComposeData { + commits: CommitSummary[]; + commitsError: string; + // `owner/repo` of the source repo, for the browse-at-commit links + sourceRepoPath: string; + branches: BranchSummary[]; + sourceBranches: BranchEntry[]; + forkBranches: BranchEntry[]; + forks: ForkOption[]; + source: PullSource; + sourceBranch: string; + targetBranch: string; + fork: string; + patch: string; + sourceRepoDid: string; + sourceKnot: string; + showDetails: boolean; + prefillError: string; +} + +// the route load calls this for the active account, the component calls it +// again client-side when the pick diverges since only the fork lane depends +// on the identity +export const loadCompose = async (args: { + config: GitServiceConfig; + repo: RepoInfo; + viewer: { did: string; handle: string } | null; + params: URLSearchParams; + fetch: typeof globalThis.fetch; +}): Promise => { + const { config, repo, viewer, params, fetch: fetchFn } = args; + const repoDid = repo.repoDid; + if (!repoDid) throw new Error("This repository has not been indexed yet"); + + const ctx = createBobbinClient({ serviceUrl: config.bobbinUrl, fetch: fetchFn }); + const source: PullSource = parseSource(params.get("source")) ?? "branch"; + + const [branchList, forkList] = await Promise.all([ + listBranches(gitTarget(config, repo, fetchFn), BRANCH_LIMIT) + .then((page) => page.branches ?? []) + .catch(() => [] as BranchEntry[]), + viewer + ? listRepos(ctx, viewer.did, { limit: 100 }) + .then((page) => + page.items + // `source` is any uri: an at-uri means a fork, anything else is + // an import, same test the repo layout's resolveSource makes + .filter( + (item) => + item.value.source?.startsWith("at://") && item.value.repoDid + ) + .map((item): ForkOption => ({ + uri: item.uri, + repoDid: item.value.repoDid as string, + name: repoNameOf(item), + knot: item.value.knot + })) + ) + .catch(() => [] as ForkOption[]) + : Promise.resolve([] as ForkOption[]) + ]); + + let fork = params.get("fork") ?? ""; + if (source === "fork" && !fork && forkList.length === 1) fork = forkList[0].repoDid; + const forkRepo = forkList.find((entry) => entry.repoDid === fork); + + let prefillError = ""; + let forkBranches: BranchEntry[] = []; + if (source === "fork" && forkRepo) { + try { + const page = await listBranches( + gitTarget(config, { uri: forkRepo.uri, repoDid: forkRepo.repoDid }, fetchFn), + BRANCH_LIMIT + ); + forkBranches = sortBranchesByRecency(page.branches ?? []); + } catch (cause) { + prefillError = cause instanceof Error ? cause.message : String(cause); + } + } + + const sourceBranches = sourceBranchChoices(branchList); + const targetBranch = defaultTargetBranch(branchList, params.get("targetBranch") ?? ""); + const sourceBranch = defaultSourceBranch( + source, + params.get("sourceBranch") ?? "", + sourceBranches, + forkBranches + ); + + const sourceRepoDid = fork || repoDid; + const hasComparison = Boolean(sourceRepoDid && sourceBranch && targetBranch); + + let commits: CommitSummary[] = []; + let commitsError = ""; + if (hasComparison && source !== "patch") { + try { + const page = await listCommits(ctx, { + repo: sourceRepoDid as Did, + ranges: [`${targetBranch}..${sourceBranch}`], + limit: COMMIT_LIMIT + }); + commits = page.commits.map(toMirrorCommitSummary); + } catch (cause) { + commitsError = cause instanceof Error ? cause.message : String(cause); + } + } + + return { + commits, + commitsError, + sourceRepoPath: forkRepo + ? `${viewer?.handle ?? repo.ownerHandle}/${forkRepo.name}` + : `${repo.ownerHandle}/${repo.name}`, + branches: sortBranches(branchList.map(toBranchSummary)), + sourceBranches, + forkBranches, + forks: forkList, + source, + sourceBranch, + targetBranch, + fork, + patch: params.get("patch") ?? "", + sourceRepoDid, + sourceKnot: forkRepo?.knot ?? repo.knot, + showDetails: hasComparison, + prefillError + }; +}; diff --git a/web/src/lib/auth.svelte.ts b/web/src/lib/auth.svelte.ts index 2120955ad..ea64139d4 100644 --- a/web/src/lib/auth.svelte.ts +++ b/web/src/lib/auth.svelte.ts @@ -19,7 +19,7 @@ import { listStoredSessions } from "@atcute/oauth-browser-client"; import { getContext } from "svelte"; -import { SvelteURL, SvelteURLSearchParams } from "svelte/reactivity"; +import { SvelteMap, SvelteURL, SvelteURLSearchParams } from "svelte/reactivity"; import oauthMetadata from "./oauth-client-metadata.json"; import { type AuthAccount, @@ -94,12 +94,14 @@ export interface Auth { readonly authenticating: boolean; readonly currentUser: CurrentUser | null; readonly accounts: AuthAccount[]; + hasAccount(did: string): boolean; bobbinUrl: string; refresh(): Promise; signIn(identifier: string, returnTo?: string): Promise; addAccount(identifier: string, returnTo?: string): Promise; completeSignIn(): Promise; - switchAccount(did: Did): Promise; + switchAccount(did: Did, returnTo?: string): Promise; + agentFor(did: Did): Promise; removeAccount(did: Did): Promise; signOut(): Promise; signOutAll(): Promise; @@ -204,6 +206,7 @@ export const createAuth = ( : { kind: "logged-out" } ); let accounts = $state([]); + const otherAgents = new SvelteMap(); // merge atcute's stored sessions with persisted account metadata. const syncAccounts = () => { @@ -253,12 +256,19 @@ export const createAuth = ( const adoptSession = (session: OAuthSession) => { const nextAgent = new OAuthUserAgent(session); const did = nextAgent.sub as Did; + otherAgents.delete(did); state = { kind: "profile-loading", agent: nextAgent, did }; const known = loadAccounts().find((account) => account.did === did); persistActive(did, known?.handle ?? did); void hydrateProfile(did, nextAgent); }; + const forgetSession = (did: Did) => { + otherAgents.delete(did); + deleteStoredSession(did); + saveAccounts(dropAccount(loadAccounts(), did)); + }; + const activate = async (did: Did): Promise => { try { const session = await getSession(did); @@ -266,8 +276,7 @@ export const createAuth = ( return true; } catch (cause) { if (isDeadSessionError(cause)) { - deleteStoredSession(did); - saveAccounts(dropAccount(loadAccounts(), did)); + forgetSession(did); return false; } state = failureState(errorMessage(cause)); @@ -275,6 +284,43 @@ export const createAuth = ( } }; + // allowStale would let a dead session through, the write then fails with the raw pds error + const agentFor = async (did: Did): Promise => { + configure(); + const active = currentAgent(); + const cached = active?.sub === did ? active : (otherAgents.get(did) ?? null); + try { + if (cached) { + await cached.getSession(); + return cached; + } + const agent = new OAuthUserAgent(await getSession(did)); + otherAgents.set(did, agent); + return agent; + } catch (cause) { + if (!isDeadSessionError(cause)) throw cause; + const handle = accounts.find((account) => account.did === did)?.handle ?? did; + if (currentDid() === did) { + await removeAccount(did); + } else { + forgetSession(did); + syncAccounts(); + } + if (browser) { + try { + await signIn(did, location.pathname + location.search); + // the page is navigating away, stay pending so the form keeps its spinner + return new Promise(() => {}); + } catch { + // fall through to the inline error + } + } + throw new Error(`${handle}'s session expired. Sign in again to act as them.`, { + cause + }); + } + }; + const refresh = async () => { if (!browser) return; configure(); @@ -353,19 +399,22 @@ export const createAuth = ( } }; - const switchAccount = async (did: Did) => { - if (!browser) return; + const switchAccount = async (did: Did, returnTo = "/") => { + if (!browser) return false; configure(); - if (!(await activate(did))) syncAccounts(); + if (await activate(did)) return true; + syncAccounts(); + await signIn(did, returnTo); + return false; }; const removeAccount = async (did: Did) => { if (!browser) return; const wasActive = currentDid() === did; try { - const activeAgent = currentAgent(); - if (wasActive && activeAgent) { - await activeAgent.signOut(); + const agent = wasActive ? currentAgent() : (otherAgents.get(did) ?? null); + if (agent) { + await agent.signOut(); } else { deleteStoredSession(did); } @@ -373,6 +422,7 @@ export const createAuth = ( deleteStoredSession(did); } + otherAgents.delete(did); saveAccounts(dropAccount(loadAccounts(), did)); accounts = reconcileAccounts(listStoredSessions(), loadAccounts()); @@ -394,12 +444,9 @@ export const createAuth = ( }; const signOutAll = async () => { - try { - const activeAgent = currentAgent(); - if (activeAgent) await activeAgent.signOut(); - } catch { - // remove local session state below. - } + const agents = [currentAgent(), ...otherAgents.values()].filter((agent) => agent !== null); + await Promise.allSettled(agents.map((agent) => agent.signOut())); + otherAgents.clear(); if (browser) { for (const did of listStoredSessions()) deleteStoredSession(did); } @@ -447,6 +494,8 @@ export const createAuth = ( get accounts() { return accounts; }, + hasAccount: (did: string) => accounts.some((account) => account.did === did), + agentFor, refresh, signIn, addAccount: signIn, diff --git a/web/src/lib/auth.test.ts b/web/src/lib/auth.test.ts new file mode 100644 index 000000000..39b93b75e --- /dev/null +++ b/web/src/lib/auth.test.ts @@ -0,0 +1,110 @@ +import type { Did } from "@atcute/lexicons/syntax"; +import type { Session } from "@atcute/oauth-browser-client"; +import { + OAuthResponseError, + OAuthUserAgent, + TokenRefreshError, + deleteStoredSession, + getSession +} from "@atcute/oauth-browser-client"; +import { type Mock, beforeEach, describe, expect, it, vi } from "vitest"; +import { createAuth } from "./auth.svelte"; + +interface MockAgent { + sub: string; + getSession: Mock; +} + +vi.mock("@atcute/oauth-browser-client", async (importOriginal) => { + const mod = await importOriginal>(); + class MockOAuthUserAgent { + static instances: MockOAuthUserAgent[] = []; + readonly sub: string; + getSession = vi.fn(async () => this.session); + signOut = vi.fn(async () => {}); + constructor(readonly session: { info: { sub: string } }) { + this.sub = session.info.sub; + MockOAuthUserAgent.instances.push(this); + } + } + return { + ...mod, + OAuthUserAgent: MockOAuthUserAgent, + configureOAuth: vi.fn(), + createAuthorizationUrl: vi.fn(), + finalizeAuthorization: vi.fn(), + getSession: vi.fn(), + listStoredSessions: vi.fn(() => []), + deleteStoredSession: vi.fn() + }; +}); + +// the vi.mock above swaps in a class that records its instances +const MockedUserAgent = OAuthUserAgent as unknown as { instances: MockAgent[] }; + +const mockedGetSession = vi.mocked(getSession); +const mockedDeleteStoredSession = vi.mocked(deleteStoredSession); + +const alice = "did:plc:alice" as Did; +const liveSession = (did: Did): Session => ({ info: { sub: did } }) as unknown as Session; + +beforeEach(() => { + vi.clearAllMocks(); + MockedUserAgent.instances.length = 0; +}); + +describe("agentFor", () => { + it("returns a live agent and reuses it", async () => { + mockedGetSession.mockResolvedValue(liveSession(alice)); + const auth = createAuth("http://127.0.0.1:1", null); + + const agent = await auth.agentFor(alice); + expect(agent.sub).toBe(alice); + expect(await auth.agentFor(alice)).toBe(agent); + expect(mockedGetSession).toHaveBeenCalledTimes(1); + expect(mockedGetSession.mock.calls[0]).toEqual([alice]); + expect(MockedUserAgent.instances[0].getSession).toHaveBeenCalledTimes(1); + }); + + it("prunes a revoked session and throws a friendly error", async () => { + mockedGetSession.mockRejectedValue(new TokenRefreshError(alice, "session was revoked")); + const auth = createAuth("http://127.0.0.1:1", null); + + await expect(auth.agentFor(alice)).rejects.toThrow(/session expired/); + expect(mockedDeleteStoredSession).toHaveBeenCalledWith(alice); + }); + + it("treats an invalid_token response from the token endpoint as dead", async () => { + mockedGetSession.mockRejectedValue( + new OAuthResponseError(new Response(null, { status: 400 }), { + error: "invalid_token", + error_description: '"exp" claim timestamp check failed' + }) + ); + const auth = createAuth("http://127.0.0.1:1", null); + + await expect(auth.agentFor(alice)).rejects.toThrow(/session expired/); + expect(mockedDeleteStoredSession).toHaveBeenCalledWith(alice); + }); + + it("keeps the session on transient failures", async () => { + const cause = new TypeError("fetch failed"); + mockedGetSession.mockRejectedValue(cause); + const auth = createAuth("http://127.0.0.1:1", null); + + await expect(auth.agentFor(alice)).rejects.toBe(cause); + expect(mockedDeleteStoredSession).not.toHaveBeenCalled(); + }); + + it("prunes a cached agent whose session died since it was minted", async () => { + mockedGetSession.mockResolvedValue(liveSession(alice)); + const auth = createAuth("http://127.0.0.1:1", null); + await auth.agentFor(alice); + + MockedUserAgent.instances[0].getSession.mockRejectedValue( + new TokenRefreshError(alice, "session was revoked") + ); + await expect(auth.agentFor(alice)).rejects.toThrow(/session expired/); + expect(mockedDeleteStoredSession).toHaveBeenCalledWith(alice); + }); +}); diff --git a/web/src/lib/components/auth/AccountSelector.stories.svelte b/web/src/lib/components/auth/AccountSelector.stories.svelte new file mode 100644 index 000000000..b3ec48796 --- /dev/null +++ b/web/src/lib/components/auth/AccountSelector.stories.svelte @@ -0,0 +1,46 @@ + + + + + + + + + + + + + { + const canvas = within(canvasElement); + await userEvent.click(canvas.getByRole("button", { name: /comment as alice/i })); + await userEvent.click(await canvas.findByRole("menuitem", { name: /bob/i })); + await waitFor(() => + expect(canvas.getByRole("button", { name: /comment as bob/i })).toBeVisible() + ); + }} +> + + diff --git a/web/src/lib/components/auth/AccountSelector.svelte b/web/src/lib/components/auth/AccountSelector.svelte new file mode 100644 index 000000000..f2cac9bf2 --- /dev/null +++ b/web/src/lib/components/auth/AccountSelector.svelte @@ -0,0 +1,65 @@ + + + + +{#if selected && others.length > 0} + + {#snippet trigger()} + + + + {/snippet} + +
{label}
+ {#each others as account (account.did)} + (value = account.did)}> + + + {/each} +
+{/if} diff --git a/web/src/lib/components/comment/CommentBox.svelte b/web/src/lib/components/comment/CommentBox.svelte index 48a2e62ea..fdd101ba2 100644 --- a/web/src/lib/components/comment/CommentBox.svelte +++ b/web/src/lib/components/comment/CommentBox.svelte @@ -67,7 +67,9 @@
-
+
{authorHandle}
{@render editor()} diff --git a/web/src/lib/components/comment/CommentCard.svelte b/web/src/lib/components/comment/CommentCard.svelte index ebea0fee0..e3c360084 100644 --- a/web/src/lib/components/comment/CommentCard.svelte +++ b/web/src/lib/components/comment/CommentCard.svelte @@ -1,5 +1,6 @@ + + + {#snippet template(args)} + + + + {/snippet} + + + { + const canvas = within(canvasElement); + const trigger = canvas.getByRole("button", { name: /open as alice/i }); + await expect(trigger).toBeInTheDocument(); + await userEvent.click(trigger); + await userEvent.click(await canvas.findByRole("menuitem", { name: /bob/i })); + // the story's bobbin is fake, the diverged refetch fails and the fork + // lane falls back to its empty state + await canvas.findByText(/select a fork first/i, {}, { timeout: 5000 }); + await expect(canvas.getByRole("button", { name: /open as bob/i })).toBeInTheDocument(); + }} +> + {#snippet template(args)} + + + + {/snippet} + diff --git a/web/src/lib/components/repo/pulls/PullCompose.svelte b/web/src/lib/components/repo/pulls/PullCompose.svelte index 02a5795db..63d8ad048 100644 --- a/web/src/lib/components/repo/pulls/PullCompose.svelte +++ b/web/src/lib/components/repo/pulls/PullCompose.svelte @@ -1,8 +1,10 @@ + + + + + + { + const canvas = within(canvasElement); + await userEvent.click(canvas.getByRole("button", { name: /alice/i })); + await userEvent.click(await canvas.findByRole("menuitem", { name: /bob/i })); + await waitFor(() => expect(args.onSwitchAccount).toHaveBeenCalledWith("did:plc:bob")); + }} +/> diff --git a/web/src/lib/components/shell/Topbar.svelte b/web/src/lib/components/shell/Topbar.svelte index 08e18b1b2..adb93bba1 100644 --- a/web/src/lib/components/shell/Topbar.svelte +++ b/web/src/lib/components/shell/Topbar.svelte @@ -15,7 +15,10 @@ link: "flex items-center text-foreground-default transition-colors hover:text-foreground-muted", identity: "flex items-center", ball: "size-8 rounded-full bg-background-inset md:size-5", - bar: "hidden h-4 w-24 rounded-sm bg-background-inset md:block" + bar: "hidden h-4 w-24 rounded-sm bg-background-inset md:block", + switchHeader: "px-2 py-1.5 typography-paragraph-small text-foreground-muted", + addBall: + "flex size-4.25 shrink-0 items-center justify-center rounded-full border border-border-default bg-background-canvas" }, variants: { variant: { @@ -61,20 +64,33 @@ import Logo from "../ui/Logo.svelte"; import User from "../ui/User.svelte"; import TopbarSearch from "./TopbarSearch.svelte"; - import type { CurrentUser } from "$lib/auth.svelte"; + import type { AuthAccount, CurrentUser } from "$lib/auth.svelte"; import Separator from "../ui/Separator.svelte"; interface Props { variant: NonNullable; onSignOut?: () => void | Promise; + onSwitchAccount?: (did: AuthAccount["did"]) => void | Promise; user: CurrentUser | null; + accounts?: readonly AuthAccount[]; + addAccountHref?: string; loading?: boolean; } - let { variant, onSignOut, user, loading = false }: Props = $props(); + let { + variant, + onSignOut, + onSwitchAccount, + user, + accounts = [], + addAccountHref = "/login", + loading = false + }: Props = $props(); const appPath = (path: string) => resolve(path as "/"); + const others = $derived(accounts.filter((account) => account.did !== user?.did)); + const style = $derived(topbar({ variant })); @@ -124,8 +140,12 @@
@@ -156,7 +176,11 @@
{#if loading} -
+
@@ -174,7 +198,28 @@ {#snippet trigger()} {/snippet} - Profile + {#if others.length > 0} +
Switch account
+ {#each others as account (account.did)} + onSwitchAccount?.(account.did)}> + + + {/each} + {/if} + + + + Add account + + + Profile Repositories @@ -182,8 +227,9 @@ Strings Settings - - Logout + Logout {:else}