From 3d09c4c4655fa0cf042fc76edf5b34636c514cc6 Mon Sep 17 00:00:00 2001 From: Dominic Vonk Date: Mon, 10 Aug 2026 20:13:46 +0000 Subject: [PATCH 01/11] feat(github): support multiple accounts --- .../pullRequest/GitHubPullRequestCli.test.ts | 27 +- .../src/pullRequest/GitHubPullRequestCli.ts | 81 +++++- .../pullRequest/GitHubPullRequestProvider.ts | 10 +- .../src/pullRequest/PullRequestProvider.ts | 7 + .../pullRequest/PullRequestService.test.ts | 36 +++ .../src/pullRequest/PullRequestService.ts | 253 ++++++++++++----- apps/server/src/serverSettings.ts | 2 + .../src/sourceControl/GitHubCli.test.ts | 114 +++++++- apps/server/src/sourceControl/GitHubCli.ts | 263 +++++++++++++++--- .../GitHubSourceControlProvider.test.ts | 45 +++ .../GitHubSourceControlProvider.ts | 64 +++-- .../SourceControlProviderDiscovery.ts | 3 + .../src/sourceControl/gitHubAuthStatus.ts | 4 + .../pullRequest/pullRequestList.logic.test.ts | 24 ++ .../pullRequest/pullRequestList.logic.ts | 4 +- .../settings/SourceControlSettings.tsx | 234 +++++++++++++++- docs/user/source-control.md | 6 + packages/contracts/src/pullRequest.ts | 8 +- packages/contracts/src/settings.test.ts | 17 ++ packages/contracts/src/settings.ts | 25 ++ packages/contracts/src/sourceControl.ts | 9 + packages/shared/src/serverSettings.ts | 4 + 22 files changed, 1085 insertions(+), 155 deletions(-) diff --git a/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts b/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts index bf7e8951afe2..26ac0cf7ba98 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts @@ -1298,12 +1298,37 @@ layer("GitHubPullRequestCli.layer", (it) => { }), ); + it.effect("uses the requested host token when looking up the viewer", () => + Effect.gen(function* () { + mockedExecute.mockReturnValueOnce(Effect.succeed(output("octocat"))); + const cli = yield* GitHubPullRequestCli.GitHubPullRequestCli; + + const viewer = yield* cli.getViewerLogin({ + cwd: "/w", + host: "github.acme.test", + repository: "acme/web", + }); + + assert.strictEqual(viewer, "octocat"); + expect(callAt(0).args).toEqual([ + "api", + "user", + "--hostname", + "github.acme.test", + "--jq", + ".login", + ]); + }), + ); + it.effect("fails when the authenticated account has no login", () => Effect.gen(function* () { mockedExecute.mockReturnValueOnce(Effect.succeed(output(" "))); const cli = yield* GitHubPullRequestCli.GitHubPullRequestCli; - const error = yield* Effect.flip(cli.getViewerLogin({ cwd: "/w" })); + const error = yield* Effect.flip( + cli.getViewerLogin({ cwd: "/w", host: "github.com", repository: "acme/web" }), + ); assert.strictEqual(error._tag, "GitHubViewerLoginUnavailableError"); }), diff --git a/apps/server/src/pullRequest/GitHubPullRequestCli.ts b/apps/server/src/pullRequest/GitHubPullRequestCli.ts index 392fac8564f9..d1be48c6ff24 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestCli.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestCli.ts @@ -275,8 +275,16 @@ export interface GitHubPullRequestDiffSlice { export class GitHubPullRequestCli extends Context.Service< GitHubPullRequestCli, { + readonly getAuthScope: (input: { + readonly cwd: string; + readonly host: string; + readonly repository: string; + }) => Effect.Effect; + readonly getViewerLogin: (input: { readonly cwd: string; + readonly host: string; + readonly repository: string; }) => Effect.Effect; readonly listPullRequests: (input: { @@ -658,12 +666,15 @@ export const make = Effect.gen(function* () { const graphql = (input: { readonly cwd: string; readonly host: string; + readonly repository: string; readonly query: string; readonly variables: Readonly>; }) => github .execute({ cwd: input.cwd, + host: input.host, + repository: input.repository, args: ["api", "graphql", "--hostname", input.host, "--input", "-"], stdin: encodeGraphQlRequestJson({ query: input.query, variables: input.variables }), }) @@ -673,6 +684,8 @@ export const make = Effect.gen(function* () { const graphqlRead = (input: { readonly cwd: string; readonly host: string; + readonly repositories?: ReadonlyArray; + readonly repository?: string; readonly operation: string; /** Variables as `-f` flags, for values this module composed itself. */ readonly variables?: ReadonlyArray; @@ -690,6 +703,9 @@ export const make = Effect.gen(function* () { input.privateVariables === undefined ? { cwd: input.cwd, + host: input.host, + ...(input.repositories === undefined ? {} : { repositories: input.repositories }), + ...(input.repository === undefined ? {} : { repository: input.repository }), args: [ "api", "graphql", @@ -702,6 +718,9 @@ export const make = Effect.gen(function* () { } : { cwd: input.cwd, + host: input.host, + ...(input.repositories === undefined ? {} : { repositories: input.repositories }), + ...(input.repository === undefined ? {} : { repository: input.repository }), args: ["api", "graphql", "--hostname", input.host, "--input", "-"], stdin: encodeGraphQlRequestJson({ query: input.query, @@ -748,6 +767,8 @@ export const make = Effect.gen(function* () { return github .execute({ cwd: input.cwd, + host: input.host, + repository: input.repository, args: [ "api", "--hostname", @@ -810,6 +831,8 @@ export const make = Effect.gen(function* () { const { owner, name } = parseRepositorySelector(input.repository); const refsResult = yield* github.execute({ cwd: input.cwd, + host: input.host, + repository: input.repository, args: [ "api", "--hostname", @@ -850,6 +873,8 @@ export const make = Effect.gen(function* () { github .execute({ cwd: input.cwd, + host: input.host, + repository: input.repository, args: [ "api", "--hostname", @@ -892,15 +917,27 @@ export const make = Effect.gen(function* () { }); return GitHubPullRequestCli.of({ + getAuthScope: (input) => + github.getAuthScope?.(input) ?? Effect.succeed(`active:${input.host.toLowerCase()}`), + getViewerLogin: (input) => - github.execute({ cwd: input.cwd, args: ["api", "user", "--jq", ".login"] }).pipe( - Effect.flatMap((result) => { - const login = result.stdout.trim(); - return login.length > 0 - ? Effect.succeed(login) - : Effect.fail(new GitHubViewerLoginUnavailableError({ command: "gh", cwd: input.cwd })); - }), - ), + github + .execute({ + cwd: input.cwd, + host: input.host, + repository: input.repository, + args: ["api", "user", "--hostname", input.host, "--jq", ".login"], + }) + .pipe( + Effect.flatMap((result) => { + const login = result.stdout.trim(); + return login.length > 0 + ? Effect.succeed(login) + : Effect.fail( + new GitHubViewerLoginUnavailableError({ command: "gh", cwd: input.cwd }), + ); + }), + ), listPullRequests: (input) => { const fallbackMaxRows = Math.max(input.limit + 1, PULL_REQUEST_FALLBACK_MAX_ROWS); @@ -911,6 +948,8 @@ export const make = Effect.gen(function* () { github .execute({ cwd: input.cwd, + host: input.host, + repository: input.repository, args: [ "pr", "list", @@ -1005,6 +1044,7 @@ export const make = Effect.gen(function* () { return graphqlRead({ cwd: input.cwd, host: input.host, + repositories: input.repositories, operation: "searchPullRequests", // The reader's own words are in the query, so it travels over stdin rather than in argv. privateVariables: { q: query }, @@ -1040,6 +1080,7 @@ export const make = Effect.gen(function* () { return graphqlRead({ cwd: input.cwd, host: input.host, + repositories: chunk.map(({ repository }) => repository), operation: "listPullRequestStats", query, decode: decodePullRequestStatsJson, @@ -1060,6 +1101,8 @@ export const make = Effect.gen(function* () { github .execute({ cwd: input.cwd, + host: input.host, + repository: input.repository, args: [ "pr", "view", @@ -1089,6 +1132,8 @@ export const make = Effect.gen(function* () { github .execute({ cwd: input.cwd, + host: input.host, + repository: input.repository, args: [ "pr", "view", @@ -1142,6 +1187,8 @@ export const make = Effect.gen(function* () { return github .execute({ cwd: input.cwd, + host: input.host, + repository: input.repository, args: ["pr", "diff", String(input.number), ...repositoryArgs(input), "--color", "never"], maxOutputBytes: DIFF_MAX_OUTPUT_BYTES, timeoutMs: DIFF_TIMEOUT_MS, @@ -1180,6 +1227,7 @@ export const make = Effect.gen(function* () { graphqlRead({ cwd: input.cwd, host: input.host, + repository: input.repository, operation: "listReviewThreadComments", variables: [ ["-f", `owner=${owner}`], @@ -1203,6 +1251,7 @@ export const make = Effect.gen(function* () { graphqlRead({ cwd: input.cwd, host: input.host, + repository: input.repository, operation: "listReviewThreadComments", variables: [["-f", `threadId=${threadId}`], cursorVariable(cursor)], query: REVIEW_THREAD_COMMENTS_GRAPHQL_QUERY, @@ -1284,6 +1333,8 @@ export const make = Effect.gen(function* () { return github .execute({ cwd: input.cwd, + host: input.host, + repository: input.repository, args: [ "api", "graphql", @@ -1315,6 +1366,8 @@ export const make = Effect.gen(function* () { github .execute({ cwd: input.cwd, + host: input.host, + repository: input.repository, args: [ "repo", "view", @@ -1344,6 +1397,7 @@ export const make = Effect.gen(function* () { return graphqlRead({ cwd: input.cwd, host: input.host, + repository: input.repository, operation: "getViewerAccess", variables: [ ["-f", `owner=${owner}`], @@ -1360,6 +1414,7 @@ export const make = Effect.gen(function* () { return graphqlRead({ cwd: input.cwd, host: input.host, + repository: input.repository, operation: "listReviewerCandidates", variables: [ ["-f", `owner=${owner}`], @@ -1376,6 +1431,8 @@ export const make = Effect.gen(function* () { return github .execute({ cwd: input.cwd, + host: input.host, + repository: input.repository, // Posting to a login GitHub has already been asked about is what a re-request is, so // there is nothing to say here about somebody who has reviewed once already. The body // travels over stdin for the reason every other one does: argv is visible in process @@ -1400,6 +1457,8 @@ export const make = Effect.gen(function* () { return github .execute({ cwd: input.cwd, + host: input.host, + repository: input.repository, args: ["pr", subcommand!, String(input.number), ...repositoryArgs(input), ...flags], }) .pipe(Effect.asVoid); @@ -1409,6 +1468,8 @@ export const make = Effect.gen(function* () { github .execute({ cwd: input.cwd, + host: input.host, + repository: input.repository, // The body travels over stdin: argv is visible in process listings and is echoed // back inside process-runner failure messages. args: [ @@ -1428,6 +1489,8 @@ export const make = Effect.gen(function* () { return github .execute({ cwd: input.cwd, + host: input.host, + repository: input.repository, // The whole review is one request, so nothing is visible to anyone else until the // verdict is sent. The payload travels over stdin for the same reason a comment // body does: argv is visible in process listings and echoed back in failures. @@ -1454,6 +1517,7 @@ export const make = Effect.gen(function* () { graphql({ cwd: input.cwd, host: input.host, + repository: input.repository, query: REVIEW_THREAD_REPLY_GRAPHQL_MUTATION, variables: { threadId: input.threadId, body: input.body }, }), @@ -1462,6 +1526,7 @@ export const make = Effect.gen(function* () { graphql({ cwd: input.cwd, host: input.host, + repository: input.repository, query: input.resolved ? RESOLVE_REVIEW_THREAD_GRAPHQL_MUTATION : UNRESOLVE_REVIEW_THREAD_GRAPHQL_MUTATION, diff --git a/apps/server/src/pullRequest/GitHubPullRequestProvider.ts b/apps/server/src/pullRequest/GitHubPullRequestProvider.ts index b77c20a541c5..d5ca7feec91f 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestProvider.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestProvider.ts @@ -114,8 +114,16 @@ export const make = Effect.gen(function* () { kind: "github", capabilities: CAPABILITIES, + getAuthScope: (input) => cli.getAuthScope(input).pipe(Effect.mapError(fail("getAuthScope"))), + getViewer: (input) => - cli.getViewerLogin({ cwd: input.cwd }).pipe(Effect.mapError(fail("getViewer"))), + cli + .getViewerLogin({ + cwd: input.cwd, + host: input.host, + repository: input.repository, + }) + .pipe(Effect.mapError(fail("getViewer"))), listChangeRequests: (input) => cli diff --git a/apps/server/src/pullRequest/PullRequestProvider.ts b/apps/server/src/pullRequest/PullRequestProvider.ts index 34ec28b41069..c73cf774d384 100644 --- a/apps/server/src/pullRequest/PullRequestProvider.ts +++ b/apps/server/src/pullRequest/PullRequestProvider.ts @@ -196,8 +196,15 @@ export interface PullRequestProviderApi { /** The signed-in account, which is what involvement filtering compares against. */ readonly getViewer: (input: { readonly cwd: string; + readonly host: string; + readonly repository: string; }) => Effect.Effect; + /** Credential identity used to keep same-host repository batches account-safe. */ + readonly getAuthScope?: ( + input: ProviderRepositoryRef, + ) => Effect.Effect; + readonly listChangeRequests: ( input: ProviderRepositoryRef & { readonly state: PullRequestListState; diff --git a/apps/server/src/pullRequest/PullRequestService.test.ts b/apps/server/src/pullRequest/PullRequestService.test.ts index c46808aa2d82..58b9aa33a6c6 100644 --- a/apps/server/src/pullRequest/PullRequestService.test.ts +++ b/apps/server/src/pullRequest/PullRequestService.test.ts @@ -1050,6 +1050,42 @@ it.effect("keeps two hosts of one provider kind as two accounts", () => }), ); +it.effect("keeps owner-specific accounts separate on the same GitHub host", () => + Effect.gen(function* () { + const viewersUsed = new Map(); + const service = yield* makeService({ + projects: [ + project({ id: "p1", title: "acme", workspaceRoot: "/acme", repository: "acme/web" }), + project({ id: "p2", title: "personal", workspaceRoot: "/me", repository: "bilal/site" }), + ], + providers: [ + fakeProvider("github", { + getAuthScope: (input) => Effect.succeed(input.repository.split("/")[0]!), + getViewer: (input) => + Effect.succeed(input.repository.startsWith("acme/") ? "work-user" : "bilal"), + listChangeRequests: (input) => { + viewersUsed.set(input.repository, input.viewer); + return Effect.succeed({ + items: [changeRequest(1, "2026-07-02T00:00:00Z")], + truncated: false, + continues: true, + }); + }, + }), + ], + }); + + const result = yield* service.list({ state: "open" }); + + assert.deepStrictEqual(Object.fromEntries(viewersUsed), { + "acme/web": "work-user", + "bilal/site": "bilal", + }); + assert.strictEqual(result.viewers["github.com acme/web"], "work-user"); + assert.strictEqual(result.viewers["github.com bilal/site"], "bilal"); + }), +); + it.effect("reports repositories on a host that could not be read", () => Effect.gen(function* () { const service = yield* makeService({ diff --git a/apps/server/src/pullRequest/PullRequestService.ts b/apps/server/src/pullRequest/PullRequestService.ts index 8652e4b9c9fc..fac623d57b6e 100644 --- a/apps/server/src/pullRequest/PullRequestService.ts +++ b/apps/server/src/pullRequest/PullRequestService.ts @@ -203,12 +203,7 @@ interface WorkspaceProjects { string, { readonly kind: SourceControlProviderKind; readonly projectCount: number } >; - /** - * Every checkout on a host, including the ones the listing de-duplicated away. Asking who is - * signed in is a question about the host rather than about a repository, and any checkout can - * answer it — so a broken worktree is not allowed to take the host down with it just because - * it happened to be the one the listing kept. - */ + /** Every checkout of a repository, including ones the listing de-duplicated away. */ readonly viewerRoots: ReadonlyMap>; } @@ -397,8 +392,9 @@ export const make = Effect.gen(function* () { // Recorded before the de-duplication below, so the viewer lookup keeps the alternates // the listing is about to drop. if (api !== null) { - const roots = viewerRoots.get(host); - if (roots === undefined) viewerRoots.set(host, [project.workspaceRoot]); + const viewerKey = listCursorKey(host, repository); + const roots = viewerRoots.get(viewerKey); + if (roots === undefined) viewerRoots.set(viewerKey, [project.workspaceRoot]); else if (!roots.includes(project.workspaceRoot)) roots.push(project.workspaceRoot); } const key = listCursorKey(host, repository); @@ -478,17 +474,12 @@ export const make = Effect.gen(function* () { return Effect.succeed(decoded); }; - /** - * One viewer lookup per host, tried across that host's workspaces so a single broken checkout - * cannot hide every healthy repository on it. Per host and not per provider kind: two GitHub - * hosts are two accounts, and the wrong login would misattribute every review request. - * - * Its failure doubles as the answer to "is this host set up", which is what the provider - * switcher shows. - */ + /** One viewer lookup per credential scope, which may differ by repository on the same host. */ type ResolvedViewer = { readonly host: string; readonly kind: SourceControlProviderKind; + readonly scope: string; + readonly projects: ReadonlyArray; readonly viewer: string | null; readonly error: PullRequestProviderError | null; }; @@ -497,41 +488,121 @@ export const make = Effect.gen(function* () { // per read, three reads per page. Only a success is believed for a while: a failure is the // "is this host set up" answer the provider switcher shows, and holding it would keep saying // signed-out after the reader has signed in. - const viewersByHost = new Map(); + const viewersByScope = new Map< + string, + { readonly at: number; readonly result: Omit } + >(); const resolveViewers = ( projects: ReadonlyArray, viewerRoots: WorkspaceProjects["viewerRoots"], ) => - Effect.forEach( - [...new Set(projects.map(({ host }) => host))], - (host) => - Effect.flatMap(Clock.currentTimeMillis, (now): Effect.Effect => { - const held = viewersByHost.get(host); - if (held !== undefined && now - held.at <= Duration.toMillis(VIEWER_CACHE_TTL)) { - return Effect.succeed(held.result); - } - const forHost = projects.filter((project) => project.host === host); - const api = forHost[0]!.api; - // Every checkout on the host, not just the ones that survived de-duplication: one - // unreadable worktree would otherwise report the whole host as signed out. - const roots = - viewerRoots.get(host) ?? forHost.map(({ project }) => project.workspaceRoot); - return Effect.firstSuccessOf(roots.map((cwd) => api.getViewer({ cwd }))).pipe( - Effect.map((viewer) => ({ - host, - kind: api.kind, - viewer: viewer as string | null, - error: null as PullRequestProviderError | null, - })), - Effect.tap((result) => - Effect.map(Clock.currentTimeMillis, (at) => viewersByHost.set(host, { at, result })), - ), - Effect.catch((error) => Effect.succeed({ host, kind: api.kind, viewer: null, error })), - ); - }), - { concurrency: REPOSITORY_CONCURRENCY }, - ); + Effect.gen(function* () { + const scoped = yield* Effect.forEach( + projects, + (project) => + ( + project.api.getAuthScope?.({ + cwd: project.project.workspaceRoot, + host: project.host, + repository: project.repository, + }) ?? Effect.succeed(`host:${project.host}`) + ).pipe( + Effect.match({ + onFailure: (error) => ({ + project, + scope: `unavailable:${listCursorKey(project.host, project.repository)}`, + error, + }), + onSuccess: (scope) => ({ project, scope, error: null }), + }), + ), + { concurrency: REPOSITORY_CONCURRENCY }, + ); + const groups = new Map< + string, + { + scope: string; + projects: SupportedProject[]; + error: PullRequestProviderError | null; + } + >(); + for (const entry of scoped) { + const key = `${entry.project.host}\n${entry.scope}`; + const held = groups.get(key); + if (held === undefined) { + groups.set(key, { + scope: entry.scope, + projects: [entry.project], + error: entry.error, + }); + } else held.projects.push(entry.project); + } + + return yield* Effect.forEach( + [...groups.entries()], + ([cacheKey, group]) => + Effect.flatMap(Clock.currentTimeMillis, (now): Effect.Effect => { + const held = viewersByScope.get(cacheKey); + if (held !== undefined && now - held.at <= Duration.toMillis(VIEWER_CACHE_TTL)) { + return Effect.succeed({ ...held.result, projects: group.projects }); + } + const first = group.projects[0]!; + const api = first.api; + if (group.error !== null) { + return Effect.succeed({ + host: first.host, + kind: api.kind, + scope: group.scope, + projects: group.projects, + viewer: null, + error: group.error, + }); + } + return Effect.firstSuccessOf( + group.projects.flatMap((project) => + ( + viewerRoots.get(listCursorKey(project.host, project.repository)) ?? [ + project.project.workspaceRoot, + ] + ).map((cwd) => + api.getViewer({ + cwd, + host: project.host, + repository: project.repository, + }), + ), + ), + ).pipe( + Effect.map((viewer) => ({ + host: first.host, + kind: api.kind, + scope: group.scope, + projects: group.projects, + viewer: viewer as string | null, + error: null as PullRequestProviderError | null, + })), + Effect.tap((result) => + Effect.map(Clock.currentTimeMillis, (at) => { + const { projects: _projects, ...cached } = result; + viewersByScope.set(cacheKey, { at, result: cached }); + }), + ), + Effect.catch((error) => + Effect.succeed({ + host: first.host, + kind: api.kind, + scope: group.scope, + projects: group.projects, + viewer: null, + error, + }), + ), + ); + }), + { concurrency: REPOSITORY_CONCURRENCY }, + ); + }); const toEntry = (input: { readonly project: SupportedProject; @@ -584,23 +655,45 @@ export const make = Effect.gen(function* () { const viewerResults = yield* resolveViewers(projects, viewerRoots); const viewers: Record = {}; + const scopes = new Map(); + const viewerScopeCounts = new Map(); for (const result of viewerResults) { - if (result.viewer !== null) viewers[result.host] = result.viewer; + viewerScopeCounts.set(result.host, (viewerScopeCounts.get(result.host) ?? 0) + 1); } + for (const result of viewerResults) { + const hostHasMultipleScopes = (viewerScopeCounts.get(result.host) ?? 0) > 1; + for (const project of result.projects) { + const key = listCursorKey(project.host, project.repository); + scopes.set(key, result.scope); + if (result.viewer !== null && hostHasMultipleScopes) viewers[key] = result.viewer; + } + if (result.viewer !== null && viewers[result.host] === undefined) { + viewers[result.host] = result.viewer; + } + } + + const viewerOf = (project: SupportedProject): string | undefined => + (viewerScopeCounts.get(project.host) ?? 0) > 1 + ? viewers[listCursorKey(project.host, project.repository)] + : viewers[project.host]; - // One summary per host, which is what the viewer lookup already answers for: two GitHub - // hosts sign in separately, so collapsing them by kind would report one as the other. + // One summary per host even where its repositories use several credentials: the provider + // switcher is host-shaped, while repository-specific failures remain in `errors` below. const providers: ReadonlyArray = [ - ...viewerResults.map((result) => ({ - host: result.host, - kind: result.kind, - searchesOnHost: - projects.find((project) => project.host === result.host)?.api.capabilities.search ?? - false, - projectCount: projectCounts.get(result.host) ?? 1, - configured: result.viewer !== null, - detail: result.error === null ? null : providerDetail(result.error), - })), + ...[...new Set(projects.map(({ host }) => host))].map((host) => { + const results = viewerResults.filter((result) => result.host === host); + const configured = results.some((result) => result.viewer !== null); + const error = results.find((result) => result.error !== null)?.error ?? null; + return { + host, + kind: results[0]!.kind, + searchesOnHost: + projects.find((project) => project.host === host)?.api.capabilities.search ?? false, + projectCount: projectCounts.get(host) ?? 1, + configured, + detail: configured || error === null ? null : providerDetail(error), + }; + }), ...[...unimplemented].map(([host, { kind, projectCount }]) => ({ host, kind, @@ -621,11 +714,11 @@ export const make = Effect.gen(function* () { : projects.filter(({ host, repository }) => continuation.has(listCursorKey(host, repository)), ); - const readable = selected.filter(({ host }) => viewers[host] !== undefined); + const readable = selected.filter((project) => viewerOf(project) !== undefined); // A host that could not be read still has projects, and they are absent from the list. // Reporting them keeps "N repositories were unavailable" honest instead of dropping them. const unreadable = selected - .filter(({ host }) => viewers[host] === undefined) + .filter((project) => viewerOf(project) === undefined) .map(({ project, repository }) => ({ projectId: project.id, projectTitle: project.title, @@ -641,7 +734,7 @@ export const make = Effect.gen(function* () { // nothing has asked for nothing, and a host it never mentioned being signed out is no // reason to refuse it. const errors = viewerResults.flatMap((result) => - result.error === null || !selected.some(({ host }) => host === result.host) + result.error === null || !result.projects.some((project) => selected.includes(project)) ? [] : [result.error], ); @@ -670,7 +763,7 @@ export const make = Effect.gen(function* () { */ const readRepository = (project: SupportedProject): Effect.Effect => { { - const viewer = viewers[project.host]!; + const viewer = viewerOf(project)!; const key = listCursorKey(project.host, project.repository); const cursor = cursorOf(project); return project.api @@ -755,7 +848,7 @@ export const make = Effect.gen(function* () { const separately = () => Effect.forEach(chunk, readRepository, { concurrency: REPOSITORY_CONCURRENCY }); if (readAcross === undefined) return separately(); - const viewer = viewers[first.host]!; + const viewer = viewerOf(first)!; const cursor = cursorOf(first); return readAcross({ cwd: first.project.workspaceRoot, @@ -837,7 +930,7 @@ export const make = Effect.gen(function* () { separate.push(project); continue; } - const key = `${project.host}\n${cursorOf(project)?.updatedBefore ?? ""}`; + const key = `${project.host}\n${scopes.get(listCursorKey(project.host, project.repository)) ?? ""}\n${cursorOf(project)?.updatedBefore ?? ""}`; const group = together.get(key); if (group === undefined) together.set(key, [project]); else group.push(project); @@ -1336,14 +1429,32 @@ export const make = Effect.gen(function* () { } wanted.set(`${project.project.id} ${ref.number}`, { project, number: ref.number }); } - const byHost = new Map>(); - for (const entry of wanted.values()) { - const held = byHost.get(entry.project.host); - if (held === undefined) byHost.set(entry.project.host, [entry]); + const scoped = yield* Effect.forEach( + [...wanted.values()], + (entry) => + ( + entry.project.api.getAuthScope?.({ + cwd: entry.project.project.workspaceRoot, + host: entry.project.host, + repository: entry.project.repository, + }) ?? Effect.succeed(`host:${entry.project.host}`) + ).pipe( + Effect.map((scope) => ({ ...entry, scope })), + Effect.orElseSucceed(() => null), + ), + { concurrency: REPOSITORY_CONCURRENCY }, + ); + type ScopedStat = Exclude<(typeof scoped)[number], null>; + const byScope = new Map>(); + for (const entry of scoped) { + if (entry === null) continue; + const key = `${entry.project.host}\n${entry.scope}`; + const held = byScope.get(key); + if (held === undefined) byScope.set(key, [entry]); else held.push(entry); } const stats = yield* Effect.forEach( - [...byHost.values()], + [...byScope.values()], (entries) => { const first = entries[0]!; const readStats = first.project.api.listChangeRequestStats; @@ -1625,7 +1736,7 @@ export const make = Effect.gen(function* () { listingsEpoch = ++epochCounter; // A whole-workspace refresh is the reader asking to be re-answered from the hosts, // and that includes who the hosts say they are. - viewersByHost.clear(); + viewersByScope.clear(); return; } bumpRefEpoch(input.reference); diff --git a/apps/server/src/serverSettings.ts b/apps/server/src/serverSettings.ts index 2798faf6f006..aa5e67fb9204 100644 --- a/apps/server/src/serverSettings.ts +++ b/apps/server/src/serverSettings.ts @@ -212,6 +212,8 @@ const ATOMIC_SETTINGS_KEYS: ReadonlySet = new Set([ "backgroundActivity", "automaticGitFetchInterval", "providerHealthRefreshInterval", + "githubDefaultAccounts", + "githubAccountOverrides", "sourceControlWriterModelSelection", "textGenerationModelSelection", ]); diff --git a/apps/server/src/sourceControl/GitHubCli.test.ts b/apps/server/src/sourceControl/GitHubCli.test.ts index 5daf7676d60c..4dbf60755ce0 100644 --- a/apps/server/src/sourceControl/GitHubCli.test.ts +++ b/apps/server/src/sourceControl/GitHubCli.test.ts @@ -5,6 +5,7 @@ import * as PlatformError from "effect/PlatformError"; import { ChildProcessSpawner } from "effect/unstable/process"; import { VcsProcessExitError, VcsProcessSpawnError } from "@t3tools/contracts"; +import { ServerSettingsService } from "../serverSettings.ts"; import * as VcsProcess from "../vcs/VcsProcess.ts"; import * as GitHubCli from "./GitHubCli.ts"; @@ -18,19 +19,118 @@ const processOutput = (stdout: string): VcsProcess.VcsProcessOutput => ({ const mockRun = vi.fn(); -const layer = GitHubCli.layer.pipe( - Layer.provide( - Layer.mock(VcsProcess.VcsProcess)({ - run: mockRun, - }), - ), -); +const layerWithSettings = (overrides: Parameters[0] = {}) => + GitHubCli.layer.pipe( + Layer.provide(ServerSettingsService.layerTest(overrides)), + Layer.provide( + Layer.mock(VcsProcess.VcsProcess)({ + run: mockRun, + }), + ), + ); + +const layer = layerWithSettings(); + +const originalGitHubToken = process.env.GITHUB_TOKEN; afterEach(() => { + if (originalGitHubToken === undefined) delete process.env.GITHUB_TOKEN; + else process.env.GITHUB_TOKEN = originalGitHubToken; mockRun.mockReset(); }); describe("GitHubCli.layer", () => { + it.effect("uses the GitHub CLI active account when no selection is configured", () => + Effect.gen(function* () { + mockRun.mockReturnValueOnce(Effect.succeed(processOutput("ok"))); + const gh = yield* GitHubCli.GitHubCli; + + yield* gh.execute({ + cwd: "/repo", + host: "github.com", + repository: "acme/widget", + args: ["api", "user"], + }); + + expect(mockRun).toHaveBeenCalledTimes(1); + expect(mockRun.mock.calls[0]?.[0]).not.toHaveProperty("env"); + }).pipe(Effect.provide(layer)), + ); + + it.effect("selects an environment-backed GitHub credential by token source", () => { + process.env.GITHUB_TOKEN = "environment-token"; + mockRun.mockReturnValueOnce(Effect.succeed(processOutput("ok"))); + const selectedLayer = layerWithSettings({ + githubDefaultAccounts: { + "github.com": { + host: "github.com", + login: "DominicVonk", + tokenSource: "GITHUB_TOKEN", + }, + }, + }); + + return Effect.gen(function* () { + const gh = yield* GitHubCli.GitHubCli; + yield* gh.execute({ + cwd: "/repo", + host: "github.com", + repository: "acme/widget", + args: ["api", "user"], + }); + + expect(mockRun).toHaveBeenCalledWith({ + operation: "GitHubCli.execute", + command: "gh", + args: ["api", "user"], + cwd: "/repo", + env: { GH_TOKEN: "environment-token" }, + timeoutMs: 30_000, + }); + }).pipe(Effect.provide(selectedLayer)); + }); + + it.effect("selects a keyring credential for a GitHub owner override", () => { + mockRun + .mockReturnValueOnce(Effect.succeed(processOutput("keyring-token\n"))) + .mockReturnValueOnce(Effect.succeed(processOutput("ok"))); + const selectedLayer = layerWithSettings({ + githubAccountOverrides: { + "github.com/acme": { + host: "github.com", + login: "work-user", + tokenSource: "keyring", + }, + }, + }); + + return Effect.gen(function* () { + const gh = yield* GitHubCli.GitHubCli; + yield* gh.execute({ + cwd: "/repo", + host: "github.com", + repository: "acme/widget", + args: ["api", "user"], + }); + + expect(mockRun).toHaveBeenNthCalledWith(1, { + operation: "GitHubCli.authToken", + command: "gh", + args: ["auth", "token", "--hostname", "github.com", "--user", "work-user"], + cwd: "/repo", + timeoutMs: 30_000, + }); + expect(mockRun).toHaveBeenNthCalledWith(2, { + operation: "GitHubCli.execute", + command: "gh", + args: ["api", "user"], + cwd: "/repo", + env: { GH_TOKEN: "keyring-token" }, + timeoutMs: 30_000, + }); + }).pipe(Effect.provide(selectedLayer)); + }); + it("does not classify a missing cwd as an unavailable gh executable", () => { const context = { command: "gh", cwd: "/repo" } as const; const missingCwd = new VcsProcessSpawnError({ diff --git a/apps/server/src/sourceControl/GitHubCli.ts b/apps/server/src/sourceControl/GitHubCli.ts index a705b0fb0b3e..6bdf51751d2b 100644 --- a/apps/server/src/sourceControl/GitHubCli.ts +++ b/apps/server/src/sourceControl/GitHubCli.ts @@ -6,11 +6,13 @@ import * as Result from "effect/Result"; import * as Schema from "effect/Schema"; import { + type GitHubAccountSelection, TrimmedNonEmptyString, type SourceControlRepositoryVisibility, type VcsError, } from "@t3tools/contracts"; +import { ServerSettingsService } from "../serverSettings.ts"; import * as VcsProcess from "../vcs/VcsProcess.ts"; import { decodeGitHubPullRequestJson, @@ -196,57 +198,104 @@ export interface GitHubRepositoryCloneUrls { readonly sshUrl: string; } +interface GitHubAuthTarget { + readonly host?: string; + readonly repositories?: ReadonlyArray; + readonly repository?: string; +} + +const GITHUB_TOKEN_ENV_SOURCES = new Set([ + "GH_TOKEN", + "GITHUB_TOKEN", + "GH_ENTERPRISE_TOKEN", + "GITHUB_ENTERPRISE_TOKEN", +]); + +function selectionKey(selection: GitHubAccountSelection): string { + return `${selection.host.toLowerCase()}\n${selection.login.toLowerCase()}\n${selection.tokenSource}`; +} + +function definedAuthTarget(input: GitHubAuthTarget): GitHubAuthTarget { + return { + ...(input.host === undefined ? {} : { host: input.host }), + ...(input.repositories === undefined ? {} : { repositories: input.repositories }), + ...(input.repository === undefined ? {} : { repository: input.repository }), + }; +} + export class GitHubCli extends Context.Service< GitHubCli, { readonly execute: (input: { readonly cwd: string; readonly args: ReadonlyArray; + readonly host?: string; + readonly repositories?: ReadonlyArray; + readonly repository?: string; readonly timeoutMs?: number; /** Piped to the child's stdin, for payloads that must never appear in argv. */ readonly stdin?: string; readonly maxOutputBytes?: number; }) => Effect.Effect; - readonly listOpenPullRequests: (input: { - readonly cwd: string; - readonly headSelector: string; - readonly limit?: number; - }) => Effect.Effect, GitHubCliError>; - - readonly getPullRequest: (input: { - readonly cwd: string; - readonly reference: string; - }) => Effect.Effect; - - readonly getRepositoryCloneUrls: (input: { - readonly cwd: string; - readonly repository: string; - }) => Effect.Effect; - - readonly createRepository: (input: { - readonly cwd: string; - readonly repository: string; - readonly visibility: SourceControlRepositoryVisibility; - }) => Effect.Effect; - - readonly createPullRequest: (input: { - readonly cwd: string; - readonly baseBranch: string; - readonly headSelector: string; - readonly title: string; - readonly bodyFile: string; - }) => Effect.Effect; - - readonly getDefaultBranch: (input: { - readonly cwd: string; - }) => Effect.Effect; - - readonly checkoutPullRequest: (input: { - readonly cwd: string; - readonly reference: string; - readonly force?: boolean; - }) => Effect.Effect; + /** Stable, non-secret identity for the credential that will serve this target. */ + readonly getAuthScope?: ( + input: GitHubAuthTarget & { readonly cwd: string }, + ) => Effect.Effect; + + readonly listOpenPullRequests: ( + input: { + readonly cwd: string; + readonly headSelector: string; + readonly limit?: number; + } & GitHubAuthTarget, + ) => Effect.Effect, GitHubCliError>; + + readonly getPullRequest: ( + input: { + readonly cwd: string; + readonly reference: string; + } & GitHubAuthTarget, + ) => Effect.Effect; + + readonly getRepositoryCloneUrls: ( + input: { + readonly cwd: string; + readonly repository: string; + } & GitHubAuthTarget, + ) => Effect.Effect; + + readonly createRepository: ( + input: { + readonly cwd: string; + readonly repository: string; + readonly visibility: SourceControlRepositoryVisibility; + } & GitHubAuthTarget, + ) => Effect.Effect; + + readonly createPullRequest: ( + input: { + readonly cwd: string; + readonly baseBranch: string; + readonly headSelector: string; + readonly title: string; + readonly bodyFile: string; + } & GitHubAuthTarget, + ) => Effect.Effect; + + readonly getDefaultBranch: ( + input: { + readonly cwd: string; + } & GitHubAuthTarget, + ) => Effect.Effect; + + readonly checkoutPullRequest: ( + input: { + readonly cwd: string; + readonly reference: string; + readonly force?: boolean; + } & GitHubAuthTarget, + ) => Effect.Effect; } >()("t3/sourceControl/GitHubCli") {} @@ -307,26 +356,150 @@ function deriveRepositoryCloneUrlsFromCreateOutput( } export const make = Effect.gen(function* () { - const process = yield* VcsProcess.VcsProcess; + const vcsProcess = yield* VcsProcess.VcsProcess; + const settings = yield* ServerSettingsService; + const tokens = new Map(); + + const accountSelection = Effect.fn("GitHubCli.accountSelection")(function* ( + input: GitHubAuthTarget & { readonly cwd: string }, + ): Effect.fn.Return { + const host = (input.host ?? "github.com").toLowerCase(); + const current = yield* settings.getSettings.pipe( + Effect.mapError( + (cause) => new GitHubCliCommandError({ command: "gh", cwd: input.cwd, cause }), + ), + ); + const defaults = new Map( + Object.entries(current.githubDefaultAccounts).map(([accountHost, account]) => [ + accountHost.toLowerCase(), + account, + ]), + ); + const overrides = new Map( + Object.entries(current.githubAccountOverrides).map(([ownerKey, account]) => [ + ownerKey.toLowerCase(), + account, + ]), + ); + const defaultAccount = defaults.get(host); + const repositories = input.repositories ?? (input.repository ? [input.repository] : []); + const accounts = + repositories.length === 0 + ? [defaultAccount] + : repositories.map((repository) => { + const owner = repository.trim().split("/")[0]?.toLowerCase(); + return owner === undefined + ? defaultAccount + : (overrides.get(`${host}/${owner}`) ?? defaultAccount); + }); + const byKey = new Map( + accounts.map((account) => [ + account === undefined ? `active:${host}` : selectionKey(account), + account, + ]), + ); + if (byKey.size > 1) { + return yield* new GitHubCliCommandError({ + command: "gh", + cwd: input.cwd, + cause: new Error("Repositories require different GitHub accounts."), + }); + } + const account = byKey.values().next().value; + if (account !== undefined && account.host.toLowerCase() !== host) { + return yield* new GitHubCliAuthenticationError({ + command: "gh", + cwd: input.cwd, + cause: new Error("The selected GitHub account belongs to a different host."), + }); + } + return account; + }); - const execute: GitHubCli["Service"]["execute"] = (input) => - process + const tokenFor = Effect.fn("GitHubCli.tokenFor")(function* (input: { + readonly account: GitHubAccountSelection; + readonly cwd: string; + }) { + if (GITHUB_TOKEN_ENV_SOURCES.has(input.account.tokenSource)) { + const token = process.env[input.account.tokenSource]?.trim(); + if (token !== undefined && token.length > 0) return token; + return yield* new GitHubCliAuthenticationError({ + command: "gh", + cwd: input.cwd, + cause: new Error("The selected GitHub token environment variable is unavailable."), + }); + } + + const key = selectionKey(input.account); + const cached = tokens.get(key); + if (cached !== undefined) return cached; + + const output = yield* vcsProcess + .run({ + operation: "GitHubCli.authToken", + command: "gh", + args: ["auth", "token", "--hostname", input.account.host, "--user", input.account.login], + cwd: input.cwd, + timeoutMs: DEFAULT_TIMEOUT_MS, + }) + .pipe( + Effect.mapError( + (cause) => new GitHubCliAuthenticationError({ command: "gh", cwd: input.cwd, cause }), + ), + ); + const token = output.stdout.trim(); + if (token.length === 0) { + return yield* new GitHubCliAuthenticationError({ + command: "gh", + cwd: input.cwd, + cause: new Error("GitHub CLI returned an empty account token."), + }); + } + tokens.set(key, token); + return token; + }); + + const run = (input: Parameters[0], env?: NodeJS.ProcessEnv) => + vcsProcess .run({ operation: "GitHubCli.execute", command: "gh", args: input.args, cwd: input.cwd, + ...(env !== undefined ? { env } : {}), timeoutMs: input.timeoutMs ?? DEFAULT_TIMEOUT_MS, ...(input.stdin !== undefined ? { stdin: input.stdin } : {}), ...(input.maxOutputBytes !== undefined ? { maxOutputBytes: input.maxOutputBytes } : {}), }) .pipe(Effect.mapError((error) => fromVcsError({ command: "gh", cwd: input.cwd }, error))); + const execute: GitHubCli["Service"]["execute"] = Effect.fn("GitHubCli.execute")( + function* (input) { + const account = yield* accountSelection(input); + if (account === undefined) return yield* run(input); + + const host = input.host ?? "github.com"; + const token = yield* tokenFor({ account, cwd: input.cwd }); + const tokenVariable = + host === "github.com" || host.endsWith(".ghe.com") ? "GH_TOKEN" : "GH_ENTERPRISE_TOKEN"; + return yield* run(input, { [tokenVariable]: token }); + }, + ); + return GitHubCli.of({ execute, + getAuthScope: (input) => + accountSelection(input).pipe( + Effect.map((account) => + account === undefined + ? `active:${(input.host ?? "github.com").toLowerCase()}` + : selectionKey(account), + ), + ), listOpenPullRequests: (input) => execute({ cwd: input.cwd, + ...definedAuthTarget(input), args: [ "pr", "list", @@ -366,6 +539,7 @@ export const make = Effect.gen(function* () { getPullRequest: (input) => execute({ cwd: input.cwd, + ...definedAuthTarget(input), args: [ "pr", "view", @@ -398,6 +572,7 @@ export const make = Effect.gen(function* () { getRepositoryCloneUrls: (input) => execute({ cwd: input.cwd, + ...definedAuthTarget(input), args: ["repo", "view", input.repository, "--json", "nameWithOwner,url,sshUrl"], }).pipe( Effect.map((result) => result.stdout.trim()), @@ -418,6 +593,7 @@ export const make = Effect.gen(function* () { createRepository: (input) => execute({ cwd: input.cwd, + ...definedAuthTarget(input), args: ["repo", "create", input.repository, `--${input.visibility}`], }).pipe( Effect.map((result) => @@ -427,6 +603,7 @@ export const make = Effect.gen(function* () { createPullRequest: (input) => execute({ cwd: input.cwd, + ...definedAuthTarget(input), args: [ "pr", "create", @@ -443,6 +620,7 @@ export const make = Effect.gen(function* () { getDefaultBranch: (input) => execute({ cwd: input.cwd, + ...definedAuthTarget(input), args: ["repo", "view", "--json", "defaultBranchRef", "--jq", ".defaultBranchRef.name"], }).pipe( Effect.map((value) => { @@ -453,6 +631,7 @@ export const make = Effect.gen(function* () { checkoutPullRequest: (input) => execute({ cwd: input.cwd, + ...definedAuthTarget(input), args: ["pr", "checkout", input.reference, ...(input.force ? ["--force"] : [])], }).pipe(Effect.asVoid), }); diff --git a/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts b/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts index 9e8a68295667..ef74b5803496 100644 --- a/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts +++ b/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts @@ -327,6 +327,7 @@ it("parses GitHub auth status accounts by host and active state", () => { account: "active-user", authenticated: true, active: true, + tokenSource: "keyring", error: null, }, { @@ -334,6 +335,7 @@ it("parses GitHub auth status accounts by host and active state", () => { account: "stale-user", authenticated: false, active: false, + tokenSource: "keyring", error: null, }, { @@ -341,12 +343,55 @@ it("parses GitHub auth status accounts by host and active state", () => { account: "enterprise-user", authenticated: true, active: false, + tokenSource: "keyring", error: null, }, ], ); }); +it("keeps duplicate GitHub logins distinct by token source", () => { + const auth = GitHubSourceControlProvider.discovery.parseAuth( + processResult( + JSON.stringify({ + hosts: { + "github.com": [ + { + state: "success", + active: true, + host: "github.com", + login: "DominicVonk", + tokenSource: "GITHUB_TOKEN", + }, + { + state: "success", + active: false, + host: "github.com", + login: "DominicVonk", + tokenSource: "keyring", + }, + ], + }, + }), + ), + ); + + assert.deepStrictEqual(auth.githubAccounts, [ + { + host: "github.com", + login: "DominicVonk", + tokenSource: "GITHUB_TOKEN", + active: true, + }, + { + host: "github.com", + login: "DominicVonk", + tokenSource: "keyring", + active: false, + }, + ]); +}); + it("reports unauthenticated when GitHub JSON has accounts but none are valid", () => { const auth = GitHubSourceControlProvider.discovery.parseAuth( processResult( diff --git a/apps/server/src/sourceControl/GitHubSourceControlProvider.ts b/apps/server/src/sourceControl/GitHubSourceControlProvider.ts index b5d5d3a55f8f..9d55866c4a77 100644 --- a/apps/server/src/sourceControl/GitHubSourceControlProvider.ts +++ b/apps/server/src/sourceControl/GitHubSourceControlProvider.ts @@ -7,6 +7,7 @@ import { type ChangeRequest, type ChangeRequestState, } from "@t3tools/contracts"; +import { normalizeGitRemoteUrl } from "@t3tools/shared/git"; import * as GitHubCli from "./GitHubCli.ts"; import { findAuthenticatedGitHubAccount, parseGitHubAuthStatus } from "./gitHubAuthStatus.ts"; @@ -20,6 +21,12 @@ import { type SourceControlCliDiscoverySpec, } from "./SourceControlProviderDiscovery.ts"; +function authTarget(context: SourceControlProvider.SourceControlProviderContext | undefined) { + if (context === undefined) return {}; + const [host, ...path] = normalizeGitRemoteUrl(context.remoteUrl).split("/"); + return host && path.length >= 2 ? { host, repository: path.join("/") } : {}; +} + function toChangeRequest(summary: GitHubCli.GitHubPullRequestSummary): ChangeRequest { return { provider: "github", @@ -53,6 +60,18 @@ function parseGitHubAuth(input: SourceControlAuthProbeInput) { status: "authenticated", account: authenticatedAccount.account, host, + githubAccounts: authStatus.accounts.flatMap((account) => + account.authenticated && account.tokenSource !== null + ? [ + { + host: account.host, + login: account.account, + tokenSource: account.tokenSource, + active: account.active, + }, + ] + : [], + ), }); } @@ -103,6 +122,7 @@ export const make = Effect.gen(function* () { return github .listOpenPullRequests({ cwd: input.cwd, + ...authTarget(input.context), headSelector: input.headSelector, ...(input.limit !== undefined ? { limit: input.limit } : {}), }) @@ -129,6 +149,7 @@ export const make = Effect.gen(function* () { return github .execute({ cwd: input.cwd, + ...authTarget(input.context), args: [ "pr", "list", @@ -188,7 +209,7 @@ export const make = Effect.gen(function* () { kind: "github", listChangeRequests, getChangeRequest: (input) => - github.getPullRequest(input).pipe( + github.getPullRequest({ ...input, ...authTarget(input.context) }).pipe( Effect.map(toChangeRequest), Effect.mapError( (error) => @@ -209,6 +230,7 @@ export const make = Effect.gen(function* () { github .createPullRequest({ cwd: input.cwd, + ...authTarget(input.context), baseBranch: input.baseRefName, headSelector: input.headSelector, title: input.title, @@ -231,22 +253,28 @@ export const make = Effect.gen(function* () { ), ), getRepositoryCloneUrls: (input) => - github.getRepositoryCloneUrls(input).pipe( - Effect.mapError( - (error) => - new SourceControlProviderError({ - provider: "github", - operation: "getRepositoryCloneUrls", - command: error.command, - cwd: input.cwd, - repository: SourceControlProvider.transportSafeSourceControlErrorValue( - input.repository, - ), - detail: error.detail, - cause: error, - }), + github + .getRepositoryCloneUrls({ + ...input, + ...authTarget(input.context), + repository: input.repository, + }) + .pipe( + Effect.mapError( + (error) => + new SourceControlProviderError({ + provider: "github", + operation: "getRepositoryCloneUrls", + command: error.command, + cwd: input.cwd, + repository: SourceControlProvider.transportSafeSourceControlErrorValue( + input.repository, + ), + detail: error.detail, + cause: error, + }), + ), ), - ), createRepository: (input) => github.createRepository(input).pipe( Effect.mapError( @@ -265,7 +293,7 @@ export const make = Effect.gen(function* () { ), ), getDefaultBranch: (input) => - github.getDefaultBranch(input).pipe( + github.getDefaultBranch({ ...input, ...authTarget(input.context) }).pipe( Effect.mapError( (error) => new SourceControlProviderError({ @@ -279,7 +307,7 @@ export const make = Effect.gen(function* () { ), ), checkoutChangeRequest: (input) => - github.checkoutPullRequest(input).pipe( + github.checkoutPullRequest({ ...input, ...authTarget(input.context) }).pipe( Effect.mapError( (error) => new SourceControlProviderError({ diff --git a/apps/server/src/sourceControl/SourceControlProviderDiscovery.ts b/apps/server/src/sourceControl/SourceControlProviderDiscovery.ts index e3a6bd1fb205..f01e7f69a2a7 100644 --- a/apps/server/src/sourceControl/SourceControlProviderDiscovery.ts +++ b/apps/server/src/sourceControl/SourceControlProviderDiscovery.ts @@ -3,6 +3,7 @@ import type { SourceControlProviderDiscoveryItem, SourceControlProviderInfo, SourceControlProviderKind, + GitHubAuthAccount, } from "@t3tools/contracts"; import * as Effect from "effect/Effect"; import * as Option from "effect/Option"; @@ -97,12 +98,14 @@ export function providerAuth(input: { readonly account?: string | undefined; readonly host?: string | undefined; readonly detail?: string | undefined; + readonly githubAccounts?: ReadonlyArray | undefined; }): SourceControlProviderAuth { return { status: input.status, account: authAccount(input.account), host: authHost(input.host), detail: authDetail(input.detail), + ...(input.githubAccounts === undefined ? {} : { githubAccounts: input.githubAccounts }), }; } diff --git a/apps/server/src/sourceControl/gitHubAuthStatus.ts b/apps/server/src/sourceControl/gitHubAuthStatus.ts index d58909c560c2..030d75d02142 100644 --- a/apps/server/src/sourceControl/gitHubAuthStatus.ts +++ b/apps/server/src/sourceControl/gitHubAuthStatus.ts @@ -7,6 +7,7 @@ const GitHubAuthStatusAccountSchema = Schema.Struct({ active: Schema.Boolean, host: Schema.String, login: Schema.String, + tokenSource: Schema.optional(Schema.String), }); const GitHubAuthStatusSchema = Schema.Struct({ @@ -22,6 +23,7 @@ export interface GitHubAuthStatusAccount { readonly account: string; readonly authenticated: boolean; readonly active: boolean; + readonly tokenSource: string | null; readonly error: string | null; } @@ -53,6 +55,8 @@ export function parseGitHubAuthStatus(text: string): GitHubAuthStatus { account: login, authenticated: account.state === "success", active: account.active, + tokenSource: + account.tokenSource === undefined ? null : nonEmptyString(account.tokenSource), error: account.error?.trim() || null, }, ]; diff --git a/apps/web/src/components/pullRequest/pullRequestList.logic.test.ts b/apps/web/src/components/pullRequest/pullRequestList.logic.test.ts index 11f5f86d4200..f2712a150644 100644 --- a/apps/web/src/components/pullRequest/pullRequestList.logic.test.ts +++ b/apps/web/src/components/pullRequest/pullRequestList.logic.test.ts @@ -102,6 +102,30 @@ describe("pull request involvement filtering", () => { filterPullRequestsByInvolvement(mixed, VIEWERS, "authored").map((item) => item.number), ).toEqual([1]); }); + + it("uses repository-specific viewers for owner account overrides", () => { + const mixed = [ + entry({ + number: 1, + repository: "acme/web", + author: { login: "work-user", name: null, avatarUrl: null }, + }), + entry({ + number: 2, + repository: "bilal/site", + author: { login: "Bilal", name: null, avatarUrl: null }, + }), + ]; + const viewers = { + "github.com": "work-user", + "github.com acme/web": "work-user", + "github.com bilal/site": "Bilal", + }; + + expect( + filterPullRequestsByInvolvement(mixed, viewers, "authored").map((item) => item.number), + ).toEqual([1, 2]); + }); }); describe("pull request grouping", () => { diff --git a/apps/web/src/components/pullRequest/pullRequestList.logic.ts b/apps/web/src/components/pullRequest/pullRequestList.logic.ts index 059004447646..eb337c0033fc 100644 --- a/apps/web/src/components/pullRequest/pullRequestList.logic.ts +++ b/apps/web/src/components/pullRequest/pullRequestList.logic.ts @@ -31,7 +31,9 @@ function normalize(value: string | null | undefined): string | null { * about the others. */ function isAuthoredByViewer(entry: PullRequestListEntry, viewers: PullRequestViewers): boolean { - const viewer = normalize(viewers[entry.host]); + const viewer = normalize( + viewers[`${entry.host} ${entry.repository.toLowerCase()}`] ?? viewers[entry.host], + ); return viewer !== null && normalize(entry.author?.login) === viewer; } diff --git a/apps/web/src/components/settings/SourceControlSettings.tsx b/apps/web/src/components/settings/SourceControlSettings.tsx index 10b54f6d7afa..3cf4f92f3485 100644 --- a/apps/web/src/components/settings/SourceControlSettings.tsx +++ b/apps/web/src/components/settings/SourceControlSettings.tsx @@ -1,9 +1,18 @@ -import { ChevronDownIcon, GitPullRequestIcon, InfoIcon, RefreshCwIcon } from "lucide-react"; +import { + ChevronDownIcon, + GitPullRequestIcon, + InfoIcon, + PlusIcon, + RefreshCwIcon, + Trash2Icon, +} from "lucide-react"; import * as Duration from "effect/Duration"; import * as Option from "effect/Option"; import { useState, type ReactNode } from "react"; import type { BackgroundActivitySettings, + GitHubAccountSelection, + GitHubAuthAccount, SourceControlProviderKind, SourceControlDiscoveryResult, SourceControlProviderAuth, @@ -34,6 +43,7 @@ import { EmptyTitle, } from "../ui/empty"; import { Skeleton } from "../ui/skeleton"; +import { Input } from "../ui/input"; import { NumberField, NumberFieldDecrement, @@ -42,6 +52,7 @@ import { NumberFieldInput, } from "../ui/number-field"; import { Switch } from "../ui/switch"; +import { Select, SelectItem, SelectPopup, SelectTrigger, SelectValue } from "../ui/select"; import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; import { AzureDevOpsIcon, @@ -426,6 +437,220 @@ function GitFetchIntervalSettings() { ); } +function githubAccountKey(account: GitHubAccountSelection): string { + return `${account.host}\n${account.login}\n${account.tokenSource}`; +} + +function githubAccountLabel(account: GitHubAccountSelection): string { + return `${account.login} · ${account.tokenSource}`; +} + +function githubAccountSelection(account: GitHubAuthAccount): GitHubAccountSelection { + return { + host: account.host, + login: account.login, + tokenSource: account.tokenSource, + }; +} + +function hasMultipleGitHubAccountsOnHost(accounts: ReadonlyArray): boolean { + return accounts.some( + (account, index) => + accounts.findIndex( + (candidate) => candidate.host.toLowerCase() === account.host.toLowerCase(), + ) !== index, + ); +} + +function GitHubAccountSelect({ + accounts, + value, + label, + onChange, +}: { + readonly accounts: ReadonlyArray; + readonly value: GitHubAccountSelection; + readonly label: string; + readonly onChange: (account: GitHubAccountSelection) => void; +}) { + const selections = accounts.map(githubAccountSelection); + const selected = + selections.find((account) => githubAccountKey(account) === githubAccountKey(value)) ?? value; + return ( + + ); +} + +function GitHubAccountSettings({ + accounts, +}: { + readonly accounts: ReadonlyArray; +}) { + const settings = usePrimarySettings(); + const updateSettings = useUpdatePrimarySettings(); + const [owner, setOwner] = useState(""); + const uniqueAccounts = [ + ...new Map(accounts.map((account) => [githubAccountKey(account), account])).values(), + ]; + const accountsByHost = new Map(); + for (const account of uniqueAccounts) { + const host = account.host.toLowerCase(); + const held = accountsByHost.get(host); + if (held === undefined) accountsByHost.set(host, [account]); + else held.push(account); + } + const selectableHosts = [...accountsByHost.entries()].filter( + ([, hostAccounts]) => hostAccounts.length > 1, + ); + const selectableAccounts = selectableHosts.flatMap(([, hostAccounts]) => hostAccounts); + const initialAccount = + selectableAccounts.find((account) => account.active) ?? selectableAccounts[0]; + const [accountKey, setAccountKey] = useState( + initialAccount === undefined ? "" : githubAccountKey(initialAccount), + ); + + if (selectableAccounts.length === 0) return null; + const normalizedOwner = owner.trim().replace(/^\/+|\/+$/g, ""); + const overrideAccount = + selectableAccounts.find((entry) => githubAccountKey(entry) === accountKey) ?? + selectableAccounts[0]!; + const canAddOverride = /^[A-Za-z0-9_.-]+$/.test(normalizedOwner); + + const addOverride = () => { + if (!canAddOverride) return; + updateSettings({ + githubAccountOverrides: { + ...settings.githubAccountOverrides, + [`${overrideAccount.host.toLowerCase()}/${normalizedOwner.toLowerCase()}`]: + githubAccountSelection(overrideAccount), + }, + }); + setOwner(""); + }; + + return ( +
+ {selectableHosts.map(([host, hostAccounts]) => { + const active = hostAccounts.find((account) => account.active) ?? hostAccounts[0]!; + const selected = settings.githubDefaultAccounts[host] ?? githubAccountSelection(active); + return ( +
+
+
Default account
+
{host}
+
+ + updateSettings({ + githubDefaultAccounts: { + ...settings.githubDefaultAccounts, + [host]: account, + }, + }) + } + /> +
+ ); + })} + +
+
+
Organization or user overrides
+
+ Use a different signed-in account for repositories owned by this organization or user. +
+
+ {Object.entries(settings.githubAccountOverrides).map(([ownerKey, account]) => { + const hostAccounts = accountsByHost.get(account.host.toLowerCase()) ?? []; + return ( +
+ {ownerKey} + + updateSettings({ + githubAccountOverrides: { + ...settings.githubAccountOverrides, + [ownerKey]: nextAccount, + }, + }) + } + /> + +
+ ); + })} +
{ + event.preventDefault(); + addOverride(); + }} + > + + setAccountKey(githubAccountKey(account))} + /> + + +
+
+ ); +} + function SourceControlSectionSkeleton({ title, headerAction, @@ -574,7 +799,12 @@ export function SourceControlSettingsPanel() { headerAction={hasVersionControlSystems ? null : scanButton} > {result.sourceControlProviders.map((item) => ( - + + {item.kind === "github" && + hasMultipleGitHubAccountsOnHost(item.auth.githubAccounts ?? []) ? ( + + ) : undefined} + ))} ) : null} diff --git a/docs/user/source-control.md b/docs/user/source-control.md index dc4a802329c8..b61ded794f3c 100644 --- a/docs/user/source-control.md +++ b/docs/user/source-control.md @@ -68,6 +68,12 @@ Run a quick **Rescan** after setting up a new machine or changing credentials. ``` 3. Open **Settings → Source Control** in T3 Code and verify GitHub shows as authenticated +When GitHub CLI has more than one authenticated account for the same host, expand GitHub in +**Settings → Source Control** to choose the default account. You can also assign a different +signed-in account to repositories owned by a specific organization or user. T3 Code uses the +credentials already managed by GitHub CLI or its token environment variables; token values are +not stored in T3 Code settings or sent to clients. + You can now clone, publish, and create pull requests. ### For GitLab diff --git a/packages/contracts/src/pullRequest.ts b/packages/contracts/src/pullRequest.ts index 707ec540d15b..07da0d581b1d 100644 --- a/packages/contracts/src/pullRequest.ts +++ b/packages/contracts/src/pullRequest.ts @@ -387,10 +387,10 @@ export type PullRequestListProjectError = typeof PullRequestListProjectError.Typ export const PullRequestListResult = Schema.Struct({ /** - * The signed-in account per host, which is what involvement filtering compares. Keyed by - * host rather than by provider kind: two GitHub hosts are two accounts. A host that could - * not be read is absent rather than present-and-undefined, because an open-keyed record - * cannot carry an optional value through the JSON codec. + * The signed-in account per host, with `host owner/repository` entries where one host uses + * several credentials. A target that could not be read is absent rather than + * present-and-undefined, because an open-keyed record cannot carry an optional value through + * the JSON codec. */ viewers: Schema.Record(TrimmedNonEmptyString, TrimmedNonEmptyString), providers: Schema.Array(PullRequestProviderSummary), diff --git a/packages/contracts/src/settings.test.ts b/packages/contracts/src/settings.test.ts index 46705837afa4..a631b85d3d41 100644 --- a/packages/contracts/src/settings.test.ts +++ b/packages/contracts/src/settings.test.ts @@ -114,6 +114,23 @@ describe("ServerSettings.providerInstances (slice-2 invariant)", () => { it("defaults to an empty record so legacy configs without the key still decode", () => { expect(DEFAULT_SERVER_SETTINGS.providerInstances).toEqual({}); + expect(DEFAULT_SERVER_SETTINGS.githubDefaultAccounts).toEqual({}); + expect(DEFAULT_SERVER_SETTINGS.githubAccountOverrides).toEqual({}); + }); + + it("accepts GitHub account defaults and owner overrides without token values", () => { + const account = { + host: "github.com", + login: "octocat", + tokenSource: "keyring", + }; + const decoded = decodeServerSettingsPatch({ + githubDefaultAccounts: { "github.com": account }, + githubAccountOverrides: { "github.com/acme": account }, + }); + + expect(decoded.githubDefaultAccounts).toEqual({ "github.com": account }); + expect(decoded.githubAccountOverrides).toEqual({ "github.com/acme": account }); }); it("decodes a fully empty config (legacy on-disk shape) without complaint", () => { diff --git a/packages/contracts/src/settings.ts b/packages/contracts/src/settings.ts index 388205649c85..b3b2d5dd62d8 100644 --- a/packages/contracts/src/settings.ts +++ b/packages/contracts/src/settings.ts @@ -495,6 +495,25 @@ export const SourceControlWritingStyleSettings = Schema.Struct({ }); export type SourceControlWritingStyleSettings = typeof SourceControlWritingStyleSettings.Type; +export const GitHubAccountSelection = Schema.Struct({ + host: TrimmedNonEmptyString.check(Schema.isMaxLength(255)), + login: TrimmedNonEmptyString.check(Schema.isMaxLength(100)), + tokenSource: TrimmedNonEmptyString.check(Schema.isMaxLength(500)), +}); +export type GitHubAccountSelection = typeof GitHubAccountSelection.Type; + +export const GitHubDefaultAccounts = Schema.Record( + TrimmedNonEmptyString.check(Schema.isMaxLength(255)), + GitHubAccountSelection, +); +export type GitHubDefaultAccounts = typeof GitHubDefaultAccounts.Type; + +export const GitHubAccountOverrides = Schema.Record( + TrimmedNonEmptyString.check(Schema.isMaxLength(356)), + GitHubAccountSelection, +); +export type GitHubAccountOverrides = typeof GitHubAccountOverrides.Type; + export const DEFAULT_AUTOMATIC_GIT_FETCH_INTERVAL = Duration.seconds(30); export const DEFAULT_PROVIDER_HEALTH_REFRESH_INTERVAL = Duration.minutes(5); @@ -588,6 +607,10 @@ export const ServerSettings = Schema.Struct({ sourceControlWriterModelSelection: Schema.NullOr(ModelSelection).pipe( Schema.withDecodingDefault(Effect.succeed(null)), ), + githubDefaultAccounts: GitHubDefaultAccounts.pipe(Schema.withDecodingDefault(Effect.succeed({}))), + githubAccountOverrides: GitHubAccountOverrides.pipe( + Schema.withDecodingDefault(Effect.succeed({})), + ), // Legacy single-instance-per-driver settings. Continues to be the source // of truth until `providerInstances` (below) lands per-driver migration @@ -731,6 +754,8 @@ export const ServerSettingsPatch = Schema.Struct({ }), ), sourceControlWriterModelSelection: Schema.optionalKey(Schema.NullOr(ModelSelection)), + githubDefaultAccounts: Schema.optionalKey(GitHubDefaultAccounts), + githubAccountOverrides: Schema.optionalKey(GitHubAccountOverrides), observability: Schema.optionalKey( Schema.Struct({ otlpTracesUrl: Schema.optionalKey(TrimmedString), diff --git a/packages/contracts/src/sourceControl.ts b/packages/contracts/src/sourceControl.ts index 104aadd9161f..9c7db87874fb 100644 --- a/packages/contracts/src/sourceControl.ts +++ b/packages/contracts/src/sourceControl.ts @@ -113,11 +113,20 @@ export const SourceControlProviderAuthStatus = Schema.Literals([ ]); export type SourceControlProviderAuthStatus = typeof SourceControlProviderAuthStatus.Type; +export const GitHubAuthAccount = Schema.Struct({ + host: TrimmedNonEmptyString, + login: TrimmedNonEmptyString, + tokenSource: TrimmedNonEmptyString, + active: Schema.Boolean, +}); +export type GitHubAuthAccount = typeof GitHubAuthAccount.Type; + export const SourceControlProviderAuth = Schema.Struct({ status: SourceControlProviderAuthStatus, account: Schema.Option(TrimmedNonEmptyString), host: Schema.Option(TrimmedNonEmptyString), detail: Schema.Option(TrimmedNonEmptyString), + githubAccounts: Schema.optionalKey(Schema.Array(GitHubAuthAccount)), }); export type SourceControlProviderAuth = typeof SourceControlProviderAuth.Type; diff --git a/packages/shared/src/serverSettings.ts b/packages/shared/src/serverSettings.ts index 21d819a1c9ea..d433cfcf684d 100644 --- a/packages/shared/src/serverSettings.ts +++ b/packages/shared/src/serverSettings.ts @@ -131,6 +131,8 @@ export function applyServerSettingsPatch( providerHealthRefreshInterval, backgroundActivityProfile, backgroundActivity, + githubDefaultAccounts, + githubAccountOverrides, ...patchForMerge } = patch; const currentBackgroundActivity = normalizeServerBackgroundActivitySettings(current); @@ -190,6 +192,8 @@ export function applyServerSettingsPatch( ...(patch.sourceControlWriterModelSelection !== undefined ? { sourceControlWriterModelSelection: patch.sourceControlWriterModelSelection } : {}), + ...(githubDefaultAccounts !== undefined ? { githubDefaultAccounts } : {}), + ...(githubAccountOverrides !== undefined ? { githubAccountOverrides } : {}), ...(automaticGitFetchInterval !== undefined ? { automaticGitFetchInterval } : {}), ...(providerHealthRefreshInterval !== undefined ? { providerHealthRefreshInterval } : {}), }; From de85a7b1d342ecce96d221f2cf2fe2e770e948a6 Mon Sep 17 00:00:00 2001 From: Dominic Vonk Date: Mon, 10 Aug 2026 20:28:56 +0000 Subject: [PATCH 02/11] fix(github): address multi-account review --- .../pullRequest/GitHubPullRequestProvider.ts | 6 +- .../src/sourceControl/GitHubCli.test.ts | 234 +++++++++++++++++- apps/server/src/sourceControl/GitHubCli.ts | 122 +++++++-- .../GitHubSourceControlProvider.test.ts | 37 +++ .../GitHubSourceControlProvider.ts | 9 +- .../settings/SourceControlSettings.tsx | 2 +- 6 files changed, 387 insertions(+), 23 deletions(-) diff --git a/apps/server/src/pullRequest/GitHubPullRequestProvider.ts b/apps/server/src/pullRequest/GitHubPullRequestProvider.ts index d5ca7feec91f..95786342e33d 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestProvider.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestProvider.ts @@ -67,7 +67,11 @@ function reasonFor( error: GitHubPullRequestCli.GitHubPullRequestCliError, ): PullRequestProviderError["reason"] { if (error._tag === "GitHubCliUnavailableError") return "missing-tool"; - if (error._tag === "GitHubCliAuthenticationError") return "unauthenticated"; + if ( + error._tag === "GitHubCliAuthenticationError" || + error._tag === "GitHubAccountTokenUnavailableError" + ) + return "unauthenticated"; return "failed"; } diff --git a/apps/server/src/sourceControl/GitHubCli.test.ts b/apps/server/src/sourceControl/GitHubCli.test.ts index 4dbf60755ce0..bdfc61c49596 100644 --- a/apps/server/src/sourceControl/GitHubCli.test.ts +++ b/apps/server/src/sourceControl/GitHubCli.test.ts @@ -3,9 +3,9 @@ import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; import * as PlatformError from "effect/PlatformError"; import { ChildProcessSpawner } from "effect/unstable/process"; -import { VcsProcessExitError, VcsProcessSpawnError } from "@t3tools/contracts"; +import { ServerSettingsError, VcsProcessExitError, VcsProcessSpawnError } from "@t3tools/contracts"; -import { ServerSettingsService } from "../serverSettings.ts"; +import * as ServerSettings from "../serverSettings.ts"; import * as VcsProcess from "../vcs/VcsProcess.ts"; import * as GitHubCli from "./GitHubCli.ts"; @@ -19,9 +19,9 @@ const processOutput = (stdout: string): VcsProcess.VcsProcessOutput => ({ const mockRun = vi.fn(); -const layerWithSettings = (overrides: Parameters[0] = {}) => +const layerWithSettings = (overrides: Parameters[0] = {}) => GitHubCli.layer.pipe( - Layer.provide(ServerSettingsService.layerTest(overrides)), + Layer.provide(ServerSettings.layerTest(overrides)), Layer.provide( Layer.mock(VcsProcess.VcsProcess)({ run: mockRun, @@ -131,6 +131,232 @@ describe("GitHubCli.layer", () => { }).pipe(Effect.provide(selectedLayer)); }); + it.effect("reads keyring credentials again after token rotation", () => { + mockRun + .mockReturnValueOnce(Effect.succeed(processOutput("first-token\n"))) + .mockReturnValueOnce(Effect.succeed(processOutput("ok"))) + .mockReturnValueOnce(Effect.succeed(processOutput("rotated-token\n"))) + .mockReturnValueOnce(Effect.succeed(processOutput("ok"))); + const selectedLayer = layerWithSettings({ + githubDefaultAccounts: { + "github.com": { + host: "github.com", + login: "work-user", + tokenSource: "keyring", + }, + }, + }); + + return Effect.gen(function* () { + const gh = yield* GitHubCli.GitHubCli; + const input = { + cwd: "/repo", + host: "github.com", + repository: "acme/widget", + args: ["api", "user"], + } as const; + + yield* gh.execute(input); + yield* gh.execute(input); + + expect(mockRun).toHaveBeenNthCalledWith(2, { + operation: "GitHubCli.execute", + command: "gh", + args: ["api", "user"], + cwd: "/repo", + env: { GH_TOKEN: "first-token" }, + timeoutMs: 30_000, + }); + expect(mockRun).toHaveBeenNthCalledWith(4, { + operation: "GitHubCli.execute", + command: "gh", + args: ["api", "user"], + cwd: "/repo", + env: { GH_TOKEN: "rotated-token" }, + timeoutMs: 30_000, + }); + }).pipe(Effect.provide(selectedLayer)); + }); + + it.effect("normalizes host casing before selecting the token environment variable", () => { + process.env.GITHUB_TOKEN = "environment-token"; + mockRun.mockReturnValueOnce(Effect.succeed(processOutput("ok"))); + const selectedLayer = layerWithSettings({ + githubDefaultAccounts: { + "github.com": { + host: "GitHub.com", + login: "work-user", + tokenSource: "GITHUB_TOKEN", + }, + }, + }); + + return Effect.gen(function* () { + const gh = yield* GitHubCli.GitHubCli; + yield* gh.execute({ + cwd: "/repo", + host: "GitHub.com", + repository: "acme/widget", + args: ["api", "user"], + }); + + expect(mockRun).toHaveBeenCalledWith({ + operation: "GitHubCli.execute", + command: "gh", + args: ["api", "user"], + cwd: "/repo", + env: { GH_TOKEN: "environment-token" }, + timeoutMs: 30_000, + }); + }).pipe(Effect.provide(selectedLayer)); + }); + + it.effect("reports conflicting repository account selections structurally", () => { + const selectedLayer = layerWithSettings({ + githubDefaultAccounts: { + "github.com": { host: "github.com", login: "personal", tokenSource: "keyring" }, + }, + githubAccountOverrides: { + "github.com/acme": { host: "github.com", login: "work", tokenSource: "keyring" }, + }, + }); + + return Effect.gen(function* () { + const gh = yield* GitHubCli.GitHubCli; + const error = yield* gh.getAuthScope!({ + cwd: "/repo", + host: "github.com", + repositories: ["personal/widget", "acme/widget"], + }).pipe(Effect.flip); + + assert.equal(error._tag, "GitHubAccountSelectionConflictError"); + if (error._tag !== "GitHubAccountSelectionConflictError") return; + assert.equal(error.host, "github.com"); + assert.deepStrictEqual(error.repositories, ["personal/widget", "acme/widget"]); + assert.notProperty(error, "cause"); + }).pipe(Effect.provide(selectedLayer)); + }); + + it.effect("reports account host mismatches structurally", () => { + const selectedLayer = layerWithSettings({ + githubDefaultAccounts: { + "github.com": { + host: "github.example.test", + login: "enterprise-user", + tokenSource: "keyring", + }, + }, + }); + + return Effect.gen(function* () { + const gh = yield* GitHubCli.GitHubCli; + const error = yield* gh.getAuthScope!({ + cwd: "/repo", + host: "github.com", + repository: "acme/widget", + }).pipe(Effect.flip); + + assert.equal(error._tag, "GitHubAccountHostMismatchError"); + if (error._tag !== "GitHubAccountHostMismatchError") return; + assert.equal(error.host, "github.com"); + assert.equal(error.accountHost, "github.example.test"); + assert.equal(error.login, "enterprise-user"); + assert.notProperty(error, "cause"); + }).pipe(Effect.provide(selectedLayer)); + }); + + it.effect("reports unavailable token sources structurally", () => { + delete process.env.GITHUB_TOKEN; + const selectedLayer = layerWithSettings({ + githubDefaultAccounts: { + "github.com": { + host: "github.com", + login: "environment-user", + tokenSource: "GITHUB_TOKEN", + }, + }, + }); + + return Effect.gen(function* () { + const gh = yield* GitHubCli.GitHubCli; + const error = yield* gh + .execute({ + cwd: "/repo", + host: "github.com", + repository: "acme/widget", + args: ["api", "user"], + }) + .pipe(Effect.flip); + + assert.equal(error._tag, "GitHubAccountTokenUnavailableError"); + if (error._tag !== "GitHubAccountTokenUnavailableError") return; + assert.equal(error.reason, "env-missing"); + assert.equal(error.tokenSource, "GITHUB_TOKEN"); + assert.notProperty(error, "cause"); + }).pipe(Effect.provide(selectedLayer)); + }); + + it.effect("reports empty keyring token output structurally", () => { + mockRun.mockReturnValueOnce(Effect.succeed(processOutput("\n"))); + const selectedLayer = layerWithSettings({ + githubDefaultAccounts: { + "github.com": { host: "github.com", login: "work-user", tokenSource: "keyring" }, + }, + }); + + return Effect.gen(function* () { + const gh = yield* GitHubCli.GitHubCli; + const error = yield* gh + .execute({ + cwd: "/repo", + host: "github.com", + repository: "acme/widget", + args: ["api", "user"], + }) + .pipe(Effect.flip); + + assert.equal(error._tag, "GitHubAccountTokenUnavailableError"); + if (error._tag !== "GitHubAccountTokenUnavailableError") return; + assert.equal(error.reason, "empty-output"); + assert.equal(error.login, "work-user"); + assert.notProperty(error, "cause"); + }).pipe(Effect.provide(selectedLayer)); + }); + + it.effect("reports settings read failures at the account-selection stage", () => { + const cause = new ServerSettingsError({ + settingsPath: "/settings.json", + operation: "read-file", + cause: new Error("unavailable"), + }); + const selectedLayer = GitHubCli.layer.pipe( + Layer.provide( + Layer.mock(ServerSettings.ServerSettingsService)({ + getSettings: Effect.fail(cause), + }), + ), + Layer.provide( + Layer.mock(VcsProcess.VcsProcess)({ + run: mockRun, + }), + ), + ); + + return Effect.gen(function* () { + const gh = yield* GitHubCli.GitHubCli; + const error = yield* gh.getAuthScope!({ + cwd: "/repo", + host: "github.com", + repository: "acme/widget", + }).pipe(Effect.flip); + + assert.equal(error._tag, "GitHubAccountSettingsUnavailableError"); + if (error._tag !== "GitHubAccountSettingsUnavailableError") return; + assert.equal(error.host, "github.com"); + assert.strictEqual(error.cause, cause); + }).pipe(Effect.provide(selectedLayer)); + }); + it("does not classify a missing cwd as an unavailable gh executable", () => { const context = { command: "gh", cwd: "/repo" } as const; const missingCwd = new VcsProcessSpawnError({ diff --git a/apps/server/src/sourceControl/GitHubCli.ts b/apps/server/src/sourceControl/GitHubCli.ts index 6bdf51751d2b..d085b1de8a9d 100644 --- a/apps/server/src/sourceControl/GitHubCli.ts +++ b/apps/server/src/sourceControl/GitHubCli.ts @@ -79,6 +79,83 @@ export class GitHubCliCommandError extends Schema.TaggedErrorClass()( + "GitHubAccountSettingsUnavailableError", + { + command: Schema.Literal("gh"), + cwd: Schema.String, + host: Schema.String, + cause: Schema.Defect(), + }, +) { + get detail(): string { + return `GitHub account settings could not be loaded for ${this.host}.`; + } + + override get message(): string { + return `GitHub CLI failed in accountSelection: ${this.detail}`; + } +} + +export class GitHubAccountSelectionConflictError extends Schema.TaggedErrorClass()( + "GitHubAccountSelectionConflictError", + { + command: Schema.Literal("gh"), + cwd: Schema.String, + host: Schema.String, + repositories: Schema.Array(Schema.String), + }, +) { + get detail(): string { + return `Repositories on ${this.host} require different GitHub accounts: ${this.repositories.join(", ")}.`; + } + + override get message(): string { + return `GitHub CLI failed in accountSelection: ${this.detail}`; + } +} + +export class GitHubAccountHostMismatchError extends Schema.TaggedErrorClass()( + "GitHubAccountHostMismatchError", + { + command: Schema.Literal("gh"), + cwd: Schema.String, + host: Schema.String, + accountHost: Schema.String, + login: Schema.String, + }, +) { + get detail(): string { + return `GitHub account ${this.login} belongs to ${this.accountHost}, not ${this.host}.`; + } + + override get message(): string { + return `GitHub CLI failed in accountSelection: ${this.detail}`; + } +} + +export class GitHubAccountTokenUnavailableError extends Schema.TaggedErrorClass()( + "GitHubAccountTokenUnavailableError", + { + command: Schema.Literal("gh"), + cwd: Schema.String, + host: Schema.String, + login: Schema.String, + tokenSource: Schema.String, + reason: Schema.Literals(["env-missing", "empty-output"]), + }, +) { + get detail(): string { + return this.reason === "env-missing" + ? `GitHub token source ${this.tokenSource} is unavailable for ${this.login} on ${this.host}.` + : `GitHub CLI returned no token for ${this.login} on ${this.host}.`; + } + + override get message(): string { + return `GitHub CLI failed in tokenFor: ${this.detail}`; + } +} + const gitHubCliDecodeFields = { command: Schema.Literal("gh"), cwd: Schema.String, @@ -142,6 +219,10 @@ export const GitHubCliError = Schema.Union([ GitHubCliAuthenticationError, GitHubPullRequestNotFoundError, GitHubCliCommandError, + GitHubAccountSettingsUnavailableError, + GitHubAccountSelectionConflictError, + GitHubAccountHostMismatchError, + GitHubAccountTokenUnavailableError, GitHubPullRequestListDecodeError, GitHubChangeRequestListDecodeError, GitHubPullRequestDecodeError, @@ -358,7 +439,6 @@ function deriveRepositoryCloneUrlsFromCreateOutput( export const make = Effect.gen(function* () { const vcsProcess = yield* VcsProcess.VcsProcess; const settings = yield* ServerSettingsService; - const tokens = new Map(); const accountSelection = Effect.fn("GitHubCli.accountSelection")(function* ( input: GitHubAuthTarget & { readonly cwd: string }, @@ -366,7 +446,13 @@ export const make = Effect.gen(function* () { const host = (input.host ?? "github.com").toLowerCase(); const current = yield* settings.getSettings.pipe( Effect.mapError( - (cause) => new GitHubCliCommandError({ command: "gh", cwd: input.cwd, cause }), + (cause) => + new GitHubAccountSettingsUnavailableError({ + command: "gh", + cwd: input.cwd, + host, + cause, + }), ), ); const defaults = new Map( @@ -399,18 +485,21 @@ export const make = Effect.gen(function* () { ]), ); if (byKey.size > 1) { - return yield* new GitHubCliCommandError({ + return yield* new GitHubAccountSelectionConflictError({ command: "gh", cwd: input.cwd, - cause: new Error("Repositories require different GitHub accounts."), + host, + repositories, }); } const account = byKey.values().next().value; if (account !== undefined && account.host.toLowerCase() !== host) { - return yield* new GitHubCliAuthenticationError({ + return yield* new GitHubAccountHostMismatchError({ command: "gh", cwd: input.cwd, - cause: new Error("The selected GitHub account belongs to a different host."), + host, + accountHost: account.host, + login: account.login, }); } return account; @@ -423,17 +512,16 @@ export const make = Effect.gen(function* () { if (GITHUB_TOKEN_ENV_SOURCES.has(input.account.tokenSource)) { const token = process.env[input.account.tokenSource]?.trim(); if (token !== undefined && token.length > 0) return token; - return yield* new GitHubCliAuthenticationError({ + return yield* new GitHubAccountTokenUnavailableError({ command: "gh", cwd: input.cwd, - cause: new Error("The selected GitHub token environment variable is unavailable."), + host: input.account.host, + login: input.account.login, + tokenSource: input.account.tokenSource, + reason: "env-missing", }); } - const key = selectionKey(input.account); - const cached = tokens.get(key); - if (cached !== undefined) return cached; - const output = yield* vcsProcess .run({ operation: "GitHubCli.authToken", @@ -449,13 +537,15 @@ export const make = Effect.gen(function* () { ); const token = output.stdout.trim(); if (token.length === 0) { - return yield* new GitHubCliAuthenticationError({ + return yield* new GitHubAccountTokenUnavailableError({ command: "gh", cwd: input.cwd, - cause: new Error("GitHub CLI returned an empty account token."), + host: input.account.host, + login: input.account.login, + tokenSource: input.account.tokenSource, + reason: "empty-output", }); } - tokens.set(key, token); return token; }); @@ -478,7 +568,7 @@ export const make = Effect.gen(function* () { const account = yield* accountSelection(input); if (account === undefined) return yield* run(input); - const host = input.host ?? "github.com"; + const host = (input.host ?? "github.com").toLowerCase(); const token = yield* tokenFor({ account, cwd: input.cwd }); const tokenVariable = host === "github.com" || host.endsWith(".ghe.com") ? "GH_TOKEN" : "GH_ENTERPRISE_TOKEN"; diff --git a/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts b/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts index ef74b5803496..12ae740b3972 100644 --- a/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts +++ b/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts @@ -178,6 +178,43 @@ it.effect("treats empty non-open change request listing output as no results", ( }), ); +it.effect("preserves explicit ports in GitHub Enterprise auth targets", () => + Effect.gen(function* () { + let listInput: Parameters[0] | null = + null; + const provider = yield* makeProvider({ + listOpenPullRequests: (input) => { + listInput = input; + return Effect.succeed([]); + }, + }); + + yield* provider.listChangeRequests({ + cwd: "/repo", + context: { + provider: { + kind: "github", + name: "GitHub Enterprise", + baseUrl: "https://github.example:8443", + }, + remoteName: "origin", + remoteUrl: "https://github.example:8443/owner/repo.git", + }, + headSelector: "feature/accounts", + state: "open", + limit: 10, + }); + + assert.deepStrictEqual(listInput, { + cwd: "/repo", + host: "github.example:8443", + repository: "owner/repo", + headSelector: "feature/accounts", + limit: 10, + }); + }), +); + it.effect("creates GitHub PRs through provider-neutral input names", () => Effect.gen(function* () { let createInput: Parameters[0] | null = diff --git a/apps/server/src/sourceControl/GitHubSourceControlProvider.ts b/apps/server/src/sourceControl/GitHubSourceControlProvider.ts index 9d55866c4a77..5ab57a3b191e 100644 --- a/apps/server/src/sourceControl/GitHubSourceControlProvider.ts +++ b/apps/server/src/sourceControl/GitHubSourceControlProvider.ts @@ -23,7 +23,14 @@ import { function authTarget(context: SourceControlProvider.SourceControlProviderContext | undefined) { if (context === undefined) return {}; - const [host, ...path] = normalizeGitRemoteUrl(context.remoteUrl).split("/"); + const [normalizedHost, ...path] = normalizeGitRemoteUrl(context.remoteUrl).split("/"); + let host = normalizedHost; + try { + const remote = new URL(context.remoteUrl); + host = remote.host.toLowerCase(); + } catch { + // SCP-style remotes have no explicit port and already use the normalized host. + } return host && path.length >= 2 ? { host, repository: path.join("/") } : {}; } diff --git a/apps/web/src/components/settings/SourceControlSettings.tsx b/apps/web/src/components/settings/SourceControlSettings.tsx index 3cf4f92f3485..7f3d0933cba2 100644 --- a/apps/web/src/components/settings/SourceControlSettings.tsx +++ b/apps/web/src/components/settings/SourceControlSettings.tsx @@ -442,7 +442,7 @@ function githubAccountKey(account: GitHubAccountSelection): string { } function githubAccountLabel(account: GitHubAccountSelection): string { - return `${account.login} · ${account.tokenSource}`; + return `${account.login} · ${account.host} · ${account.tokenSource}`; } function githubAccountSelection(account: GitHubAuthAccount): GitHubAccountSelection { From c6e2f67a2ed8f821400d05fe591b3a2300306537 Mon Sep 17 00:00:00 2001 From: Dominic Vonk Date: Mon, 10 Aug 2026 21:21:40 +0000 Subject: [PATCH 03/11] fix(github): preserve token lookup failures --- .../src/sourceControl/GitHubCli.test.ts | 35 +++++++++++++++++++ apps/server/src/sourceControl/GitHubCli.ts | 6 +--- 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/apps/server/src/sourceControl/GitHubCli.test.ts b/apps/server/src/sourceControl/GitHubCli.test.ts index bdfc61c49596..ec646b2b4f94 100644 --- a/apps/server/src/sourceControl/GitHubCli.test.ts +++ b/apps/server/src/sourceControl/GitHubCli.test.ts @@ -178,6 +178,41 @@ describe("GitHubCli.layer", () => { }).pipe(Effect.provide(selectedLayer)); }); + it.effect("preserves missing-tool classification while reading a keyring token", () => { + const cause = new VcsProcessSpawnError({ + operation: "GitHubCli.authToken", + command: "gh", + cwd: "/repo", + cause: PlatformError.systemError({ + _tag: "NotFound", + module: "ChildProcess", + method: "spawn", + description: "gh missing", + }), + }); + mockRun.mockReturnValueOnce(Effect.fail(cause)); + const selectedLayer = layerWithSettings({ + githubDefaultAccounts: { + "github.com": { host: "github.com", login: "work-user", tokenSource: "keyring" }, + }, + }); + + return Effect.gen(function* () { + const gh = yield* GitHubCli.GitHubCli; + const error = yield* gh + .execute({ + cwd: "/repo", + host: "github.com", + repository: "acme/widget", + args: ["api", "user"], + }) + .pipe(Effect.flip); + + assert.equal(error._tag, "GitHubCliUnavailableError"); + assert.strictEqual(error.cause, cause); + }).pipe(Effect.provide(selectedLayer)); + }); + it.effect("normalizes host casing before selecting the token environment variable", () => { process.env.GITHUB_TOKEN = "environment-token"; mockRun.mockReturnValueOnce(Effect.succeed(processOutput("ok"))); diff --git a/apps/server/src/sourceControl/GitHubCli.ts b/apps/server/src/sourceControl/GitHubCli.ts index d085b1de8a9d..7a590f4d896e 100644 --- a/apps/server/src/sourceControl/GitHubCli.ts +++ b/apps/server/src/sourceControl/GitHubCli.ts @@ -530,11 +530,7 @@ export const make = Effect.gen(function* () { cwd: input.cwd, timeoutMs: DEFAULT_TIMEOUT_MS, }) - .pipe( - Effect.mapError( - (cause) => new GitHubCliAuthenticationError({ command: "gh", cwd: input.cwd, cause }), - ), - ); + .pipe(Effect.mapError((error) => fromVcsError({ command: "gh", cwd: input.cwd }, error))); const token = output.stdout.trim(); if (token.length === 0) { return yield* new GitHubAccountTokenUnavailableError({ From 8dd34bafbf306585d00b8b01809cb4079b795b9e Mon Sep 17 00:00:00 2001 From: Dominic Vonk Date: Mon, 10 Aug 2026 21:42:13 +0000 Subject: [PATCH 04/11] fix(github): normalize auth target ports --- .../GitHubSourceControlProvider.test.ts | 55 ++++++++++--------- .../GitHubSourceControlProvider.ts | 9 +-- 2 files changed, 31 insertions(+), 33 deletions(-) diff --git a/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts b/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts index 12ae740b3972..bd37a55b671a 100644 --- a/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts +++ b/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts @@ -178,40 +178,45 @@ it.effect("treats empty non-open change request listing output as no results", ( }), ); -it.effect("preserves explicit ports in GitHub Enterprise auth targets", () => +it.effect("drops transport ports from GitHub Enterprise auth targets", () => Effect.gen(function* () { - let listInput: Parameters[0] | null = - null; + const listInputs: Array[0]> = + []; const provider = yield* makeProvider({ listOpenPullRequests: (input) => { - listInput = input; + listInputs.push(input); return Effect.succeed([]); }, }); - yield* provider.listChangeRequests({ - cwd: "/repo", - context: { - provider: { - kind: "github", - name: "GitHub Enterprise", - baseUrl: "https://github.example:8443", + for (const remoteUrl of [ + "https://github.example:8443/owner/repo.git", + "ssh://git@github.example:2222/owner/repo.git", + ]) { + yield* provider.listChangeRequests({ + cwd: "/repo", + context: { + provider: { + kind: "github", + name: "GitHub Enterprise", + baseUrl: "https://github.example", + }, + remoteName: "origin", + remoteUrl, }, - remoteName: "origin", - remoteUrl: "https://github.example:8443/owner/repo.git", - }, - headSelector: "feature/accounts", - state: "open", - limit: 10, - }); + headSelector: "feature/accounts", + state: "open", + limit: 10, + }); + } - assert.deepStrictEqual(listInput, { - cwd: "/repo", - host: "github.example:8443", - repository: "owner/repo", - headSelector: "feature/accounts", - limit: 10, - }); + assert.deepStrictEqual( + listInputs.map(({ host, repository }) => ({ host, repository })), + [ + { host: "github.example", repository: "owner/repo" }, + { host: "github.example", repository: "owner/repo" }, + ], + ); }), ); diff --git a/apps/server/src/sourceControl/GitHubSourceControlProvider.ts b/apps/server/src/sourceControl/GitHubSourceControlProvider.ts index 5ab57a3b191e..9d55866c4a77 100644 --- a/apps/server/src/sourceControl/GitHubSourceControlProvider.ts +++ b/apps/server/src/sourceControl/GitHubSourceControlProvider.ts @@ -23,14 +23,7 @@ import { function authTarget(context: SourceControlProvider.SourceControlProviderContext | undefined) { if (context === undefined) return {}; - const [normalizedHost, ...path] = normalizeGitRemoteUrl(context.remoteUrl).split("/"); - let host = normalizedHost; - try { - const remote = new URL(context.remoteUrl); - host = remote.host.toLowerCase(); - } catch { - // SCP-style remotes have no explicit port and already use the normalized host. - } + const [host, ...path] = normalizeGitRemoteUrl(context.remoteUrl).split("/"); return host && path.length >= 2 ? { host, repository: path.join("/") } : {}; } From bafdd1ab0f07ead70fa5d90e33aa1d5c5a3195f8 Mon Sep 17 00:00:00 2001 From: Dominic Vonk Date: Mon, 10 Aug 2026 22:03:35 +0000 Subject: [PATCH 05/11] style(server): use settings module namespace --- apps/server/src/sourceControl/GitHubCli.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/server/src/sourceControl/GitHubCli.ts b/apps/server/src/sourceControl/GitHubCli.ts index 7a590f4d896e..2b87acb50484 100644 --- a/apps/server/src/sourceControl/GitHubCli.ts +++ b/apps/server/src/sourceControl/GitHubCli.ts @@ -12,7 +12,7 @@ import { type VcsError, } from "@t3tools/contracts"; -import { ServerSettingsService } from "../serverSettings.ts"; +import * as ServerSettings from "../serverSettings.ts"; import * as VcsProcess from "../vcs/VcsProcess.ts"; import { decodeGitHubPullRequestJson, @@ -438,7 +438,7 @@ function deriveRepositoryCloneUrlsFromCreateOutput( export const make = Effect.gen(function* () { const vcsProcess = yield* VcsProcess.VcsProcess; - const settings = yield* ServerSettingsService; + const settings = yield* ServerSettings.ServerSettingsService; const accountSelection = Effect.fn("GitHubCli.accountSelection")(function* ( input: GitHubAuthTarget & { readonly cwd: string }, From 683c1630c97368f2b307fc150fb046751733c28f Mon Sep 17 00:00:00 2001 From: Dominic Vonk Date: Mon, 10 Aug 2026 22:17:36 +0000 Subject: [PATCH 06/11] fix(web): expose stale GitHub routing reset --- .../settings/SourceControlSettings.tsx | 37 +++++++++++++------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/apps/web/src/components/settings/SourceControlSettings.tsx b/apps/web/src/components/settings/SourceControlSettings.tsx index 7f3d0933cba2..3b81f8d5e0e2 100644 --- a/apps/web/src/components/settings/SourceControlSettings.tsx +++ b/apps/web/src/components/settings/SourceControlSettings.tsx @@ -453,15 +453,6 @@ function githubAccountSelection(account: GitHubAuthAccount): GitHubAccountSelect }; } -function hasMultipleGitHubAccountsOnHost(accounts: ReadonlyArray): boolean { - return accounts.some( - (account, index) => - accounts.findIndex( - (candidate) => candidate.host.toLowerCase() === account.host.toLowerCase(), - ) !== index, - ); -} - function GitHubAccountSelect({ accounts, value, @@ -529,8 +520,31 @@ function GitHubAccountSettings({ const [accountKey, setAccountKey] = useState( initialAccount === undefined ? "" : githubAccountKey(initialAccount), ); + const hasSavedRouting = + Object.keys(settings.githubDefaultAccounts).length > 0 || + Object.keys(settings.githubAccountOverrides).length > 0; - if (selectableAccounts.length === 0) return null; + if (selectableAccounts.length === 0) { + if (!hasSavedRouting) return null; + return ( +
+
+
Saved account routing
+
+ Multiple signed-in accounts are no longer available. Clear saved routing to use the + active GitHub account. +
+
+ +
+ ); + } const normalizedOwner = owner.trim().replace(/^\/+|\/+$/g, ""); const overrideAccount = selectableAccounts.find((entry) => githubAccountKey(entry) === accountKey) ?? @@ -800,8 +814,7 @@ export function SourceControlSettingsPanel() { > {result.sourceControlProviders.map((item) => ( - {item.kind === "github" && - hasMultipleGitHubAccountsOnHost(item.auth.githubAccounts ?? []) ? ( + {item.kind === "github" ? ( ) : undefined} From a8311272bdfab83eda142dbb0d4362838641d9ac Mon Sep 17 00:00:00 2001 From: Dominic Vonk Date: Mon, 10 Aug 2026 22:24:47 +0000 Subject: [PATCH 07/11] fix(web): keep GitHub routing cleanup visible --- .../settings/SourceControlSettings.tsx | 40 ++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/apps/web/src/components/settings/SourceControlSettings.tsx b/apps/web/src/components/settings/SourceControlSettings.tsx index 3b81f8d5e0e2..1fe91c420e77 100644 --- a/apps/web/src/components/settings/SourceControlSettings.tsx +++ b/apps/web/src/components/settings/SourceControlSettings.tsx @@ -453,6 +453,15 @@ function githubAccountSelection(account: GitHubAuthAccount): GitHubAccountSelect }; } +function hasMultipleGitHubAccountsOnHost(accounts: ReadonlyArray): boolean { + return accounts.some( + (account, index) => + accounts.findIndex( + (candidate) => candidate.host.toLowerCase() === account.host.toLowerCase(), + ) !== index, + ); +} + function GitHubAccountSelect({ accounts, value, @@ -514,6 +523,10 @@ function GitHubAccountSettings({ const selectableHosts = [...accountsByHost.entries()].filter( ([, hostAccounts]) => hostAccounts.length > 1, ); + const selectableHostKeys = new Set(selectableHosts.map(([host]) => host)); + const hiddenDefaultHosts = Object.keys(settings.githubDefaultAccounts).filter( + (host) => !selectableHostKeys.has(host.toLowerCase()), + ); const selectableAccounts = selectableHosts.flatMap(([, hostAccounts]) => hostAccounts); const initialAccount = selectableAccounts.find((account) => account.active) ?? selectableAccounts[0]; @@ -660,6 +673,27 @@ function GitHubAccountSettings({ Add + {hiddenDefaultHosts.length > 0 ? ( +
+
+
Unavailable saved defaults
+
+ Saved defaults for {hiddenDefaultHosts.join(", ")} are no longer selectable. +
+
+ +
+ ) : null} ); @@ -748,6 +782,7 @@ function EmptySourceControlDiscovery({ export function SourceControlSettingsPanel() { const environmentId = usePrimaryEnvironment()?.environmentId ?? null; + const settings = usePrimarySettings(); const discovery = useEnvironmentQuery( environmentId === null ? null @@ -814,7 +849,10 @@ export function SourceControlSettingsPanel() { > {result.sourceControlProviders.map((item) => ( - {item.kind === "github" ? ( + {item.kind === "github" && + (hasMultipleGitHubAccountsOnHost(item.auth.githubAccounts ?? []) || + Object.keys(settings.githubDefaultAccounts).length > 0 || + Object.keys(settings.githubAccountOverrides).length > 0) ? ( ) : undefined} From fb7da77a8e2b2e5522a43919ae8a583e757213bd Mon Sep 17 00:00:00 2001 From: Dominic Vonk Date: Mon, 10 Aug 2026 22:41:01 +0000 Subject: [PATCH 08/11] refactor(web): clarify GitHub account routing --- .../settings/SourceControlSettings.tsx | 38 ++++++++++++++----- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/apps/web/src/components/settings/SourceControlSettings.tsx b/apps/web/src/components/settings/SourceControlSettings.tsx index 1fe91c420e77..05b3c49bd15f 100644 --- a/apps/web/src/components/settings/SourceControlSettings.tsx +++ b/apps/web/src/components/settings/SourceControlSettings.tsx @@ -441,8 +441,16 @@ function githubAccountKey(account: GitHubAccountSelection): string { return `${account.host}\n${account.login}\n${account.tokenSource}`; } -function githubAccountLabel(account: GitHubAccountSelection): string { - return `${account.login} · ${account.host} · ${account.tokenSource}`; +function githubTokenSourceLabel(tokenSource: string): string { + return tokenSource === "keyring" ? "GitHub CLI" : tokenSource; +} + +function githubAccountLabel(account: GitHubAccountSelection, includeHost: boolean): string { + return [ + account.login, + ...(includeHost ? [account.host] : []), + githubTokenSourceLabel(account.tokenSource), + ].join(" · "); } function githubAccountSelection(account: GitHubAuthAccount): GitHubAccountSelection { @@ -466,11 +474,13 @@ function GitHubAccountSelect({ accounts, value, label, + includeHost = false, onChange, }: { readonly accounts: ReadonlyArray; readonly value: GitHubAccountSelection; readonly label: string; + readonly includeHost?: boolean; readonly onChange: (account: GitHubAccountSelection) => void; }) { const selections = accounts.map(githubAccountSelection); @@ -485,7 +495,7 @@ function GitHubAccountSelect({ }} > - {githubAccountLabel(selected)} + {githubAccountLabel(selected, includeHost)} {selections.map((account) => ( @@ -494,7 +504,7 @@ function GitHubAccountSelect({ hideIndicator value={githubAccountKey(account)} > - {githubAccountLabel(account)} + {githubAccountLabel(account, includeHost)} ))} @@ -578,6 +588,13 @@ function GitHubAccountSettings({ return (
+
+
GitHub accounts
+
+ Choose the signed-in account T3 Code uses. Defaults apply to every repository on a host; + owner overrides take precedence. +
+
{selectableHosts.map(([host, hostAccounts]) => { const active = hostAccounts.find((account) => account.active) ?? hostAccounts[0]!; const selected = settings.githubDefaultAccounts[host] ?? githubAccountSelection(active); @@ -587,8 +604,10 @@ function GitHubAccountSettings({ className="flex flex-col gap-2 sm:flex-row sm:items-center sm:justify-between" >
-
Default account
-
{host}
+
Default for {host}
+
+ Used unless a repository owner override matches. +
-
Organization or user overrides
+
Repository owner overrides
- Use a different signed-in account for repositories owned by this organization or user. + Use another account for every repository owned by an organization or user.
{Object.entries(settings.githubAccountOverrides).map(([ownerKey, account]) => { @@ -658,7 +677,7 @@ function GitHubAccountSettings({ size="sm" value={owner} onValueChange={setOwner} - placeholder="Organization or user" + placeholder="Organization or user, e.g. acme-corp" aria-label="GitHub organization or user" className="flex-1" /> @@ -666,6 +685,7 @@ function GitHubAccountSettings({ accounts={selectableAccounts} value={githubAccountSelection(overrideAccount)} label="GitHub account for new override" + includeHost={selectableHosts.length > 1} onChange={(account) => setAccountKey(githubAccountKey(account))} /> +
+ ); + } + + const normalizedOwner = owner.trim().replace(/^\/+|\/+$/g, ""); + const overrideAccount = + selectableAccounts.find((entry) => accountKey(entry) === accountKeyValue) ?? + selectableAccounts[0]!; + const canAddOverride = /^[A-Za-z0-9_.-]+$/.test(normalizedOwner); + + const addOverride = () => { + if (!canAddOverride) return; + updateSettings({ + githubAccountOverrides: { + ...settings.githubAccountOverrides, + [`${overrideAccount.host.toLowerCase()}/${normalizedOwner.toLowerCase()}`]: + accountSelection(overrideAccount), + }, + }); + setOwner(""); + }; + + return ( +
+
+
GitHub accounts
+
+ Choose the signed-in account T3 Code uses. Defaults apply to every repository on a host; + owner overrides take precedence. +
+
+ {selectableHosts.map(([host, hostAccounts]) => { + const active = hostAccounts.find((account) => account.active) ?? hostAccounts[0]!; + const selected = settings.githubDefaultAccounts[host] ?? accountSelection(active); + return ( +
+
+
Default for {host}
+
+ Used unless a repository owner override matches. +
+
+ + updateSettings({ + githubDefaultAccounts: { + ...settings.githubDefaultAccounts, + [host]: account, + }, + }) + } + /> +
+ ); + })} + +
+
+
Repository owner overrides
+
+ Use another account for every repository owned by an organization or user. +
+
+ {Object.entries(settings.githubAccountOverrides).map(([ownerKey, account]) => { + const hostAccounts = accountsByHost.get(account.host.toLowerCase()) ?? []; + return ( +
+ {ownerKey} + + updateSettings({ + githubAccountOverrides: { + ...settings.githubAccountOverrides, + [ownerKey]: nextAccount, + }, + }) + } + /> + +
+ ); + })} +
{ + event.preventDefault(); + addOverride(); + }} + > + + 1} + onChange={(account) => setAccountKeyValue(accountKey(account))} + /> + + + {hiddenDefaultHosts.length > 0 ? ( +
+
+
Unavailable saved defaults
+
+ Saved defaults for {hiddenDefaultHosts.join(", ")} are no longer selectable. +
+
+ +
+ ) : null} +
+
+ ); +} diff --git a/apps/web/src/components/settings/SourceControlSettings.tsx b/apps/web/src/components/settings/SourceControlSettings.tsx index 05b3c49bd15f..9b61f96e1ce3 100644 --- a/apps/web/src/components/settings/SourceControlSettings.tsx +++ b/apps/web/src/components/settings/SourceControlSettings.tsx @@ -1,18 +1,9 @@ -import { - ChevronDownIcon, - GitPullRequestIcon, - InfoIcon, - PlusIcon, - RefreshCwIcon, - Trash2Icon, -} from "lucide-react"; +import { ChevronDownIcon, GitPullRequestIcon, InfoIcon, RefreshCwIcon } from "lucide-react"; import * as Duration from "effect/Duration"; import * as Option from "effect/Option"; import { useState, type ReactNode } from "react"; import type { BackgroundActivitySettings, - GitHubAccountSelection, - GitHubAuthAccount, SourceControlProviderKind, SourceControlDiscoveryResult, SourceControlProviderAuth, @@ -43,7 +34,6 @@ import { EmptyTitle, } from "../ui/empty"; import { Skeleton } from "../ui/skeleton"; -import { Input } from "../ui/input"; import { NumberField, NumberFieldDecrement, @@ -52,7 +42,6 @@ import { NumberFieldInput, } from "../ui/number-field"; import { Switch } from "../ui/switch"; -import { Select, SelectItem, SelectPopup, SelectTrigger, SelectValue } from "../ui/select"; import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; import { AzureDevOpsIcon, @@ -63,6 +52,7 @@ import { JujutsuIcon, type Icon, } from "../Icons"; +import { GitHubAccountSettings, hasMultipleGitHubAccountsOnHost } from "./GitHubAccountSettings"; import { RedactedSensitiveText } from "./RedactedSensitiveText"; import { SourceControlWritingSettingsSection } from "./SourceControlWritingSettings"; import { SettingResetButton, SettingsPageContainer, SettingsSection } from "./settingsLayout"; @@ -437,288 +427,6 @@ function GitFetchIntervalSettings() { ); } -function githubAccountKey(account: GitHubAccountSelection): string { - return `${account.host}\n${account.login}\n${account.tokenSource}`; -} - -function githubTokenSourceLabel(tokenSource: string): string { - return tokenSource === "keyring" ? "GitHub CLI" : tokenSource; -} - -function githubAccountLabel(account: GitHubAccountSelection, includeHost: boolean): string { - return [ - account.login, - ...(includeHost ? [account.host] : []), - githubTokenSourceLabel(account.tokenSource), - ].join(" · "); -} - -function githubAccountSelection(account: GitHubAuthAccount): GitHubAccountSelection { - return { - host: account.host, - login: account.login, - tokenSource: account.tokenSource, - }; -} - -function hasMultipleGitHubAccountsOnHost(accounts: ReadonlyArray): boolean { - return accounts.some( - (account, index) => - accounts.findIndex( - (candidate) => candidate.host.toLowerCase() === account.host.toLowerCase(), - ) !== index, - ); -} - -function GitHubAccountSelect({ - accounts, - value, - label, - includeHost = false, - onChange, -}: { - readonly accounts: ReadonlyArray; - readonly value: GitHubAccountSelection; - readonly label: string; - readonly includeHost?: boolean; - readonly onChange: (account: GitHubAccountSelection) => void; -}) { - const selections = accounts.map(githubAccountSelection); - const selected = - selections.find((account) => githubAccountKey(account) === githubAccountKey(value)) ?? value; - return ( - - ); -} - -function GitHubAccountSettings({ - accounts, -}: { - readonly accounts: ReadonlyArray; -}) { - const settings = usePrimarySettings(); - const updateSettings = useUpdatePrimarySettings(); - const [owner, setOwner] = useState(""); - const uniqueAccounts = [ - ...new Map(accounts.map((account) => [githubAccountKey(account), account])).values(), - ]; - const accountsByHost = new Map(); - for (const account of uniqueAccounts) { - const host = account.host.toLowerCase(); - const held = accountsByHost.get(host); - if (held === undefined) accountsByHost.set(host, [account]); - else held.push(account); - } - const selectableHosts = [...accountsByHost.entries()].filter( - ([, hostAccounts]) => hostAccounts.length > 1, - ); - const selectableHostKeys = new Set(selectableHosts.map(([host]) => host)); - const hiddenDefaultHosts = Object.keys(settings.githubDefaultAccounts).filter( - (host) => !selectableHostKeys.has(host.toLowerCase()), - ); - const selectableAccounts = selectableHosts.flatMap(([, hostAccounts]) => hostAccounts); - const initialAccount = - selectableAccounts.find((account) => account.active) ?? selectableAccounts[0]; - const [accountKey, setAccountKey] = useState( - initialAccount === undefined ? "" : githubAccountKey(initialAccount), - ); - const hasSavedRouting = - Object.keys(settings.githubDefaultAccounts).length > 0 || - Object.keys(settings.githubAccountOverrides).length > 0; - - if (selectableAccounts.length === 0) { - if (!hasSavedRouting) return null; - return ( -
-
-
Saved account routing
-
- Multiple signed-in accounts are no longer available. Clear saved routing to use the - active GitHub account. -
-
- -
- ); - } - const normalizedOwner = owner.trim().replace(/^\/+|\/+$/g, ""); - const overrideAccount = - selectableAccounts.find((entry) => githubAccountKey(entry) === accountKey) ?? - selectableAccounts[0]!; - const canAddOverride = /^[A-Za-z0-9_.-]+$/.test(normalizedOwner); - - const addOverride = () => { - if (!canAddOverride) return; - updateSettings({ - githubAccountOverrides: { - ...settings.githubAccountOverrides, - [`${overrideAccount.host.toLowerCase()}/${normalizedOwner.toLowerCase()}`]: - githubAccountSelection(overrideAccount), - }, - }); - setOwner(""); - }; - - return ( -
-
-
GitHub accounts
-
- Choose the signed-in account T3 Code uses. Defaults apply to every repository on a host; - owner overrides take precedence. -
-
- {selectableHosts.map(([host, hostAccounts]) => { - const active = hostAccounts.find((account) => account.active) ?? hostAccounts[0]!; - const selected = settings.githubDefaultAccounts[host] ?? githubAccountSelection(active); - return ( -
-
-
Default for {host}
-
- Used unless a repository owner override matches. -
-
- - updateSettings({ - githubDefaultAccounts: { - ...settings.githubDefaultAccounts, - [host]: account, - }, - }) - } - /> -
- ); - })} - -
-
-
Repository owner overrides
-
- Use another account for every repository owned by an organization or user. -
-
- {Object.entries(settings.githubAccountOverrides).map(([ownerKey, account]) => { - const hostAccounts = accountsByHost.get(account.host.toLowerCase()) ?? []; - return ( -
- {ownerKey} - - updateSettings({ - githubAccountOverrides: { - ...settings.githubAccountOverrides, - [ownerKey]: nextAccount, - }, - }) - } - /> - -
- ); - })} -
{ - event.preventDefault(); - addOverride(); - }} - > - - 1} - onChange={(account) => setAccountKey(githubAccountKey(account))} - /> - - - {hiddenDefaultHosts.length > 0 ? ( -
-
-
Unavailable saved defaults
-
- Saved defaults for {hiddenDefaultHosts.join(", ")} are no longer selectable. -
-
- -
- ) : null} -
-
- ); -} - function SourceControlSectionSkeleton({ title, headerAction, From 74095d6531798b43446cde40431471751e04bd8c Mon Sep 17 00:00:00 2001 From: Dominic Vonk Date: Mon, 10 Aug 2026 23:29:13 +0000 Subject: [PATCH 10/11] fix(github): preserve account routing in GraphQL reads --- .../pullRequest/GitHubPullRequestCli.test.ts | 1 + .../src/pullRequest/GitHubPullRequestCli.ts | 13 +- .../pullRequest/GitHubPullRequestProvider.ts | 3 +- .../src/sourceControl/GitHubCli.test.ts | 26 ++- apps/server/src/sourceControl/GitHubCli.ts | 149 +++++++++++++----- .../src/sourceControl/GitHubCredentials.ts | 36 ++--- 6 files changed, 138 insertions(+), 90 deletions(-) diff --git a/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts b/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts index 26ac0cf7ba98..3ba34cbb5d62 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts @@ -941,6 +941,7 @@ layer("GitHubPullRequestCli.layer", (it) => { }); const args = callAt(0).args; + expect(callAt(0).repositories).toEqual(["acme/web"]); expect(args).toContain("--hostname"); expect(args).toContain("github.acme.dev"); expect(args).toContain("owner=acme"); diff --git a/apps/server/src/pullRequest/GitHubPullRequestCli.ts b/apps/server/src/pullRequest/GitHubPullRequestCli.ts index 165031b5f9ad..09fa838aeedf 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestCli.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestCli.ts @@ -697,15 +697,16 @@ export const make = Effect.gen(function* () { readonly privateVariables?: Readonly>; readonly query: string; readonly decode: (raw: string) => Result.Result; - }): Effect.Effect => - github + }): Effect.Effect => { + const repositories = + input.repositories ?? (input.repository === undefined ? undefined : [input.repository]); + return github .execute( input.privateVariables === undefined ? { cwd: input.cwd, host: input.host, - ...(input.repositories === undefined ? {} : { repositories: input.repositories }), - ...(input.repository === undefined ? {} : { repository: input.repository }), + ...(repositories === undefined ? {} : { repositories }), args: [ "api", "graphql", @@ -719,8 +720,7 @@ export const make = Effect.gen(function* () { : { cwd: input.cwd, host: input.host, - ...(input.repositories === undefined ? {} : { repositories: input.repositories }), - ...(input.repository === undefined ? {} : { repository: input.repository }), + ...(repositories === undefined ? {} : { repositories }), args: ["api", "graphql", "--hostname", input.host, "--input", "-"], stdin: encodeGraphQlRequestJson({ query: input.query, @@ -743,6 +743,7 @@ export const make = Effect.gen(function* () { ); }), ); + }; /** * One page of the patch, read from the files API. GitHub refuses `pr diff` outright past 300 diff --git a/apps/server/src/pullRequest/GitHubPullRequestProvider.ts b/apps/server/src/pullRequest/GitHubPullRequestProvider.ts index 56837149d3d0..d91bb42fc8dd 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestProvider.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestProvider.ts @@ -69,7 +69,8 @@ function reasonFor( if (error._tag === "GitHubCliUnavailableError") return "missing-tool"; if ( error._tag === "GitHubCliAuthenticationError" || - (error._tag === "GitHubCredentialError" && error.reason._tag === "TokenUnavailable") + error._tag === "GitHubTokenEnvironmentUnavailableError" || + error._tag === "GitHubTokenOutputEmptyError" ) return "unauthenticated"; return "failed"; diff --git a/apps/server/src/sourceControl/GitHubCli.test.ts b/apps/server/src/sourceControl/GitHubCli.test.ts index 65710557521f..a26a73fd486d 100644 --- a/apps/server/src/sourceControl/GitHubCli.test.ts +++ b/apps/server/src/sourceControl/GitHubCli.test.ts @@ -269,12 +269,9 @@ describe("GitHubCli.layer", () => { }) .pipe(Effect.flip); - assert.equal(error._tag, "GitHubCredentialError"); - if (error._tag !== "GitHubCredentialError") return; - assert.equal(error.reason._tag, "TokenUnavailable"); - if (error.reason._tag !== "TokenUnavailable") return; - assert.equal(error.reason.kind, "env-missing"); - assert.equal(error.reason.tokenSource, "GITHUB_TOKEN"); + assert.equal(error._tag, "GitHubTokenEnvironmentUnavailableError"); + if (error._tag !== "GitHubTokenEnvironmentUnavailableError") return; + assert.equal(error.tokenSource, "GITHUB_TOKEN"); }).pipe(Effect.provide(selectedLayer)); }); @@ -297,12 +294,9 @@ describe("GitHubCli.layer", () => { }) .pipe(Effect.flip); - assert.equal(error._tag, "GitHubCredentialError"); - if (error._tag !== "GitHubCredentialError") return; - assert.equal(error.reason._tag, "TokenUnavailable"); - if (error.reason._tag !== "TokenUnavailable") return; - assert.equal(error.reason.kind, "empty-output"); - assert.equal(error.reason.login, "work-user"); + assert.equal(error._tag, "GitHubTokenOutputEmptyError"); + if (error._tag !== "GitHubTokenOutputEmptyError") return; + assert.equal(error.login, "work-user"); }).pipe(Effect.provide(selectedLayer)); }); @@ -335,12 +329,10 @@ describe("GitHubCli.layer", () => { }) .pipe(Effect.flip); - assert.equal(error._tag, "GitHubCredentialError"); - if (error._tag !== "GitHubCredentialError") return; + assert.equal(error._tag, "GitHubAccountSettingsUnavailableError"); + if (error._tag !== "GitHubAccountSettingsUnavailableError") return; assert.equal(error.host, "github.com"); - assert.equal(error.reason._tag, "SettingsUnavailable"); - if (error.reason._tag !== "SettingsUnavailable") return; - assert.strictEqual(error.reason.cause, cause); + assert.strictEqual(error.cause, cause); }).pipe(Effect.provide(selectedLayer)); }); diff --git a/apps/server/src/sourceControl/GitHubCli.ts b/apps/server/src/sourceControl/GitHubCli.ts index 323d33412829..814f7c23018a 100644 --- a/apps/server/src/sourceControl/GitHubCli.ts +++ b/apps/server/src/sourceControl/GitHubCli.ts @@ -80,28 +80,88 @@ export class GitHubCliCommandError extends Schema.TaggedErrorClass()( - "GitHubCredentialError", +const gitHubCredentialFields = { + command: Schema.Literal("gh"), + cwd: Schema.String, + host: Schema.String, +} as const; + +export class GitHubAccountSettingsUnavailableError extends Schema.TaggedErrorClass()( + "GitHubAccountSettingsUnavailableError", { - command: Schema.Literal("gh"), - cwd: Schema.String, - host: Schema.String, - reason: GitHubCredentials.GitHubCredentialReason, + ...gitHubCredentialFields, + cause: Schema.Defect(), }, ) { get detail(): string { - switch (this.reason._tag) { - case "SettingsUnavailable": - return `GitHub account settings could not be loaded for ${this.host}.`; - case "SelectionConflict": - return `Repositories on ${this.host} require different GitHub accounts: ${this.reason.repositories.join(", ")}.`; - case "HostMismatch": - return `GitHub account ${this.reason.login} belongs to ${this.reason.accountHost}, not ${this.host}.`; - case "TokenUnavailable": - return this.reason.kind === "env-missing" - ? `GitHub token source ${this.reason.tokenSource} is unavailable for ${this.reason.login} on ${this.host}.` - : `GitHub CLI returned no token for ${this.reason.login} on ${this.host}.`; - } + return `GitHub account settings could not be loaded for ${this.host}.`; + } + + override get message(): string { + return `GitHub CLI credential selection failed: ${this.detail}`; + } +} + +export class GitHubAccountSelectionConflictError extends Schema.TaggedErrorClass()( + "GitHubAccountSelectionConflictError", + { + ...gitHubCredentialFields, + repositories: Schema.Array(Schema.String), + }, +) { + get detail(): string { + return `Repositories on ${this.host} require different GitHub accounts: ${this.repositories.join(", ")}.`; + } + + override get message(): string { + return `GitHub CLI credential selection failed: ${this.detail}`; + } +} + +export class GitHubAccountHostMismatchError extends Schema.TaggedErrorClass()( + "GitHubAccountHostMismatchError", + { + ...gitHubCredentialFields, + accountHost: Schema.String, + login: Schema.String, + }, +) { + get detail(): string { + return `GitHub account ${this.login} belongs to ${this.accountHost}, not ${this.host}.`; + } + + override get message(): string { + return `GitHub CLI credential selection failed: ${this.detail}`; + } +} + +export class GitHubTokenEnvironmentUnavailableError extends Schema.TaggedErrorClass()( + "GitHubTokenEnvironmentUnavailableError", + { + ...gitHubCredentialFields, + login: Schema.String, + tokenSource: Schema.String, + }, +) { + get detail(): string { + return `GitHub token source ${this.tokenSource} is unavailable for ${this.login} on ${this.host}.`; + } + + override get message(): string { + return `GitHub CLI credential selection failed: ${this.detail}`; + } +} + +export class GitHubTokenOutputEmptyError extends Schema.TaggedErrorClass()( + "GitHubTokenOutputEmptyError", + { + ...gitHubCredentialFields, + login: Schema.String, + tokenSource: Schema.String, + }, +) { + get detail(): string { + return `GitHub CLI returned no token for ${this.login} on ${this.host}.`; } override get message(): string { @@ -172,7 +232,11 @@ export const GitHubCliError = Schema.Union([ GitHubCliAuthenticationError, GitHubPullRequestNotFoundError, GitHubCliCommandError, - GitHubCredentialError, + GitHubAccountSettingsUnavailableError, + GitHubAccountSelectionConflictError, + GitHubAccountHostMismatchError, + GitHubTokenEnvironmentUnavailableError, + GitHubTokenOutputEmptyError, GitHubPullRequestListDecodeError, GitHubChangeRequestListDecodeError, GitHubPullRequestDecodeError, @@ -387,22 +451,33 @@ export const make = Effect.gen(function* () { const current = yield* settings.getSettings.pipe( Effect.mapError( (cause) => - new GitHubCredentialError({ + new GitHubAccountSettingsUnavailableError({ command: "gh", cwd: input.cwd, host, - reason: { _tag: "SettingsUnavailable", cause }, + cause, }), ), ); const selected = GitHubCredentials.selectCredentialRoute(current, input); if (Result.isFailure(selected)) { - return yield* new GitHubCredentialError({ - command: "gh", - cwd: input.cwd, - host, - reason: selected.failure, - }); + switch (selected.failure._tag) { + case "SelectionConflict": + return yield* new GitHubAccountSelectionConflictError({ + command: "gh", + cwd: input.cwd, + host, + repositories: [...selected.failure.repositories], + }); + case "HostMismatch": + return yield* new GitHubAccountHostMismatchError({ + command: "gh", + cwd: input.cwd, + host, + accountHost: selected.failure.accountHost, + login: selected.failure.login, + }); + } } return selected.success; }); @@ -414,16 +489,12 @@ export const make = Effect.gen(function* () { if (GITHUB_TOKEN_ENV_SOURCES.has(input.account.tokenSource)) { const token = process.env[input.account.tokenSource]?.trim(); if (token !== undefined && token.length > 0) return token; - return yield* new GitHubCredentialError({ + return yield* new GitHubTokenEnvironmentUnavailableError({ command: "gh", cwd: input.cwd, host: input.account.host, - reason: { - _tag: "TokenUnavailable", - login: input.account.login, - tokenSource: input.account.tokenSource, - kind: "env-missing", - }, + login: input.account.login, + tokenSource: input.account.tokenSource, }); } @@ -438,16 +509,12 @@ export const make = Effect.gen(function* () { .pipe(Effect.mapError((error) => fromVcsError({ command: "gh", cwd: input.cwd }, error))); const token = output.stdout.trim(); if (token.length === 0) { - return yield* new GitHubCredentialError({ + return yield* new GitHubTokenOutputEmptyError({ command: "gh", cwd: input.cwd, host: input.account.host, - reason: { - _tag: "TokenUnavailable", - login: input.account.login, - tokenSource: input.account.tokenSource, - kind: "empty-output", - }, + login: input.account.login, + tokenSource: input.account.tokenSource, }); } return token; diff --git a/apps/server/src/sourceControl/GitHubCredentials.ts b/apps/server/src/sourceControl/GitHubCredentials.ts index 165edccfa0e9..9a6e81e2d13d 100644 --- a/apps/server/src/sourceControl/GitHubCredentials.ts +++ b/apps/server/src/sourceControl/GitHubCredentials.ts @@ -1,5 +1,4 @@ import * as Result from "effect/Result"; -import * as Schema from "effect/Schema"; import type { GitHubAccountSelection, ServerSettings } from "@t3tools/contracts"; @@ -14,29 +13,16 @@ export interface GitHubCredentialRoute { readonly account: GitHubAccountSelection | undefined; } -export const GitHubCredentialReason = Schema.Union([ - Schema.TaggedStruct("SettingsUnavailable", { - cause: Schema.Defect(), - }), - Schema.TaggedStruct("SelectionConflict", { - repositories: Schema.Array(Schema.String), - }), - Schema.TaggedStruct("HostMismatch", { - accountHost: Schema.String, - login: Schema.String, - }), - Schema.TaggedStruct("TokenUnavailable", { - login: Schema.String, - tokenSource: Schema.String, - kind: Schema.Literals(["env-missing", "empty-output"]), - }), -]); -export type GitHubCredentialReason = typeof GitHubCredentialReason.Type; - -type GitHubCredentialRoutingReason = Extract< - GitHubCredentialReason, - { readonly _tag: "SelectionConflict" | "HostMismatch" } ->; +export type GitHubCredentialRoutingError = + | { + readonly _tag: "SelectionConflict"; + readonly repositories: ReadonlyArray; + } + | { + readonly _tag: "HostMismatch"; + readonly accountHost: string; + readonly login: string; + }; type GitHubCredentialSettings = Pick< ServerSettings, @@ -51,7 +37,7 @@ export function accountKey(account: GitHubAccountSelection): string { export function selectCredentialRoute( settings: GitHubCredentialSettings, target: GitHubCredentialTarget, -): Result.Result { +): Result.Result { const host = (target.host ?? "github.com").toLowerCase(); const defaults = new Map( Object.entries(settings.githubDefaultAccounts).map(([accountHost, account]) => [ From f84749e945909291e1b5e79d4dd5cab1c01eb086 Mon Sep 17 00:00:00 2001 From: Dominic Vonk Date: Tue, 11 Aug 2026 19:09:59 +0000 Subject: [PATCH 11/11] refactor(github): scope account routing by host --- .../src/pullRequest/GitHubPullRequestCli.ts | 44 ++--- .../src/pullRequest/PullRequestService.ts | 23 ++- apps/server/src/serverSettings.ts | 3 +- .../src/sourceControl/GitHubCli.test.ts | 50 +++--- apps/server/src/sourceControl/GitHubCli.ts | 73 ++++----- .../sourceControl/GitHubCredentials.test.ts | 46 +++--- .../src/sourceControl/GitHubCredentials.ts | 58 +++---- .../settings/GitHubAccountSettings.tsx | 153 +++++++++++------- .../settings/SourceControlSettings.tsx | 3 +- packages/contracts/src/settings.test.ts | 14 +- packages/contracts/src/settings.ts | 27 ++-- packages/shared/src/serverSettings.ts | 6 +- 12 files changed, 243 insertions(+), 257 deletions(-) diff --git a/apps/server/src/pullRequest/GitHubPullRequestCli.ts b/apps/server/src/pullRequest/GitHubPullRequestCli.ts index 09fa838aeedf..cc56fca17316 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestCli.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestCli.ts @@ -681,32 +681,34 @@ export const make = Effect.gen(function* () { .pipe(Effect.asVoid); /** A GraphQL read whose answer is decoded, reporting a failure against the read that made it. */ - const graphqlRead = (input: { - readonly cwd: string; - readonly host: string; - readonly repositories?: ReadonlyArray; - readonly repository?: string; - readonly operation: string; - /** Variables as `-f` flags, for values this module composed itself. */ - readonly variables?: ReadonlyArray; - /** - * Variables carrying words the reader typed. Document and variables travel over stdin - * together, because argv is visible in process listings and is echoed back inside a - * process-runner failure message. - */ - readonly privateVariables?: Readonly>; - readonly query: string; - readonly decode: (raw: string) => Result.Result; - }): Effect.Effect => { - const repositories = - input.repositories ?? (input.repository === undefined ? undefined : [input.repository]); + const graphqlRead = ( + input: { + readonly cwd: string; + readonly host: string; + readonly operation: string; + /** Variables as `-f` flags, for values this module composed itself. */ + readonly variables?: ReadonlyArray; + /** + * Variables carrying words the reader typed. Document and variables travel over stdin + * together, because argv is visible in process listings and is echoed back inside a + * process-runner failure message. + */ + readonly privateVariables?: Readonly>; + readonly query: string; + readonly decode: (raw: string) => Result.Result; + } & ( + | { readonly repository: string; readonly repositories?: never } + | { readonly repository?: never; readonly repositories: ReadonlyArray } + ), + ): Effect.Effect => { + const repositories = input.repositories ?? [input.repository]; return github .execute( input.privateVariables === undefined ? { cwd: input.cwd, host: input.host, - ...(repositories === undefined ? {} : { repositories }), + repositories, args: [ "api", "graphql", @@ -720,7 +722,7 @@ export const make = Effect.gen(function* () { : { cwd: input.cwd, host: input.host, - ...(repositories === undefined ? {} : { repositories }), + repositories, args: ["api", "graphql", "--hostname", input.host, "--input", "-"], stdin: encodeGraphQlRequestJson({ query: input.query, diff --git a/apps/server/src/pullRequest/PullRequestService.ts b/apps/server/src/pullRequest/PullRequestService.ts index 4c0ebfad16b7..cc77177c39aa 100644 --- a/apps/server/src/pullRequest/PullRequestService.ts +++ b/apps/server/src/pullRequest/PullRequestService.ts @@ -563,6 +563,13 @@ export const make = Effect.gen(function* () { { readonly at: number; readonly result: Omit } >(); + const getBatchKey = (project: SupportedProject) => + project.api.getBatchKey?.({ + cwd: project.project.workspaceRoot, + host: project.host, + repository: project.repository, + }) ?? Effect.succeed(`host:${project.host}`); + const resolveViewers = ( projects: ReadonlyArray, viewerRoots: WorkspaceProjects["viewerRoots"], @@ -571,13 +578,7 @@ export const make = Effect.gen(function* () { const keyed = yield* Effect.forEach( projects, (project) => - ( - project.api.getBatchKey?.({ - cwd: project.project.workspaceRoot, - host: project.host, - repository: project.repository, - }) ?? Effect.succeed(`host:${project.host}`) - ).pipe( + getBatchKey(project).pipe( Effect.match({ onFailure: (error) => ({ project, @@ -1502,13 +1503,7 @@ export const make = Effect.gen(function* () { const keyed = yield* Effect.forEach( [...wanted.values()], (entry) => - ( - entry.project.api.getBatchKey?.({ - cwd: entry.project.project.workspaceRoot, - host: entry.project.host, - repository: entry.project.repository, - }) ?? Effect.succeed(`host:${entry.project.host}`) - ).pipe( + getBatchKey(entry.project).pipe( Effect.map((batchKey) => ({ ...entry, batchKey })), Effect.orElseSucceed(() => null), ), diff --git a/apps/server/src/serverSettings.ts b/apps/server/src/serverSettings.ts index aa5e67fb9204..b3326b09b7c6 100644 --- a/apps/server/src/serverSettings.ts +++ b/apps/server/src/serverSettings.ts @@ -212,8 +212,7 @@ const ATOMIC_SETTINGS_KEYS: ReadonlySet = new Set([ "backgroundActivity", "automaticGitFetchInterval", "providerHealthRefreshInterval", - "githubDefaultAccounts", - "githubAccountOverrides", + "githubAccountRouting", "sourceControlWriterModelSelection", "textGenerationModelSelection", ]); diff --git a/apps/server/src/sourceControl/GitHubCli.test.ts b/apps/server/src/sourceControl/GitHubCli.test.ts index a26a73fd486d..9d46654705b3 100644 --- a/apps/server/src/sourceControl/GitHubCli.test.ts +++ b/apps/server/src/sourceControl/GitHubCli.test.ts @@ -61,11 +61,10 @@ describe("GitHubCli.layer", () => { process.env.GITHUB_TOKEN = "environment-token"; mockRun.mockReturnValueOnce(Effect.succeed(processOutput("ok"))); const selectedLayer = layerWithSettings({ - githubDefaultAccounts: { + githubAccountRouting: { "github.com": { - host: "github.com", - login: "DominicVonk", - tokenSource: "GITHUB_TOKEN", + defaultAccount: { login: "DominicVonk", tokenSource: "GITHUB_TOKEN" }, + ownerOverrides: {}, }, }, }); @@ -95,11 +94,9 @@ describe("GitHubCli.layer", () => { .mockReturnValueOnce(Effect.succeed(processOutput("keyring-token\n"))) .mockReturnValueOnce(Effect.succeed(processOutput("ok"))); const selectedLayer = layerWithSettings({ - githubAccountOverrides: { - "github.com/acme": { - host: "github.com", - login: "work-user", - tokenSource: "keyring", + githubAccountRouting: { + "github.com": { + ownerOverrides: { acme: { login: "work-user", tokenSource: "keyring" } }, }, }, }); @@ -138,11 +135,10 @@ describe("GitHubCli.layer", () => { .mockReturnValueOnce(Effect.succeed(processOutput("rotated-token\n"))) .mockReturnValueOnce(Effect.succeed(processOutput("ok"))); const selectedLayer = layerWithSettings({ - githubDefaultAccounts: { + githubAccountRouting: { "github.com": { - host: "github.com", - login: "work-user", - tokenSource: "keyring", + defaultAccount: { login: "work-user", tokenSource: "keyring" }, + ownerOverrides: {}, }, }, }); @@ -192,8 +188,11 @@ describe("GitHubCli.layer", () => { }); mockRun.mockReturnValueOnce(Effect.fail(cause)); const selectedLayer = layerWithSettings({ - githubDefaultAccounts: { - "github.com": { host: "github.com", login: "work-user", tokenSource: "keyring" }, + githubAccountRouting: { + "github.com": { + defaultAccount: { login: "work-user", tokenSource: "keyring" }, + ownerOverrides: {}, + }, }, }); @@ -217,11 +216,10 @@ describe("GitHubCli.layer", () => { process.env.GITHUB_TOKEN = "environment-token"; mockRun.mockReturnValueOnce(Effect.succeed(processOutput("ok"))); const selectedLayer = layerWithSettings({ - githubDefaultAccounts: { + githubAccountRouting: { "github.com": { - host: "GitHub.com", - login: "work-user", - tokenSource: "GITHUB_TOKEN", + defaultAccount: { login: "work-user", tokenSource: "GITHUB_TOKEN" }, + ownerOverrides: {}, }, }, }); @@ -249,11 +247,10 @@ describe("GitHubCli.layer", () => { it.effect("reports unavailable token sources structurally", () => { delete process.env.GITHUB_TOKEN; const selectedLayer = layerWithSettings({ - githubDefaultAccounts: { + githubAccountRouting: { "github.com": { - host: "github.com", - login: "environment-user", - tokenSource: "GITHUB_TOKEN", + defaultAccount: { login: "environment-user", tokenSource: "GITHUB_TOKEN" }, + ownerOverrides: {}, }, }, }); @@ -278,8 +275,11 @@ describe("GitHubCli.layer", () => { it.effect("reports empty keyring token output structurally", () => { mockRun.mockReturnValueOnce(Effect.succeed(processOutput("\n"))); const selectedLayer = layerWithSettings({ - githubDefaultAccounts: { - "github.com": { host: "github.com", login: "work-user", tokenSource: "keyring" }, + githubAccountRouting: { + "github.com": { + defaultAccount: { login: "work-user", tokenSource: "keyring" }, + ownerOverrides: {}, + }, }, }); diff --git a/apps/server/src/sourceControl/GitHubCli.ts b/apps/server/src/sourceControl/GitHubCli.ts index 814f7c23018a..dcbeb0819f06 100644 --- a/apps/server/src/sourceControl/GitHubCli.ts +++ b/apps/server/src/sourceControl/GitHubCli.ts @@ -118,23 +118,6 @@ export class GitHubAccountSelectionConflictError extends Schema.TaggedErrorClass } } -export class GitHubAccountHostMismatchError extends Schema.TaggedErrorClass()( - "GitHubAccountHostMismatchError", - { - ...gitHubCredentialFields, - accountHost: Schema.String, - login: Schema.String, - }, -) { - get detail(): string { - return `GitHub account ${this.login} belongs to ${this.accountHost}, not ${this.host}.`; - } - - override get message(): string { - return `GitHub CLI credential selection failed: ${this.detail}`; - } -} - export class GitHubTokenEnvironmentUnavailableError extends Schema.TaggedErrorClass()( "GitHubTokenEnvironmentUnavailableError", { @@ -234,7 +217,6 @@ export const GitHubCliError = Schema.Union([ GitHubCliCommandError, GitHubAccountSettingsUnavailableError, GitHubAccountSelectionConflictError, - GitHubAccountHostMismatchError, GitHubTokenEnvironmentUnavailableError, GitHubTokenOutputEmptyError, GitHubPullRequestListDecodeError, @@ -303,25 +285,35 @@ const GITHUB_TOKEN_ENV_SOURCES = new Set([ ]); function definedAuthTarget(input: GitHubAuthTarget): GitHubAuthTarget { + if (input.repositories === undefined) return {}; return { ...(input.host === undefined ? {} : { host: input.host }), - ...(input.repositories === undefined ? {} : { repositories: input.repositories }), + repositories: input.repositories, }; } +function repositoryAuthTarget(input: { + readonly host?: string | undefined; + readonly repository: string; +}): GitHubAuthTarget { + return input.host === undefined + ? { repositories: [input.repository] } + : { host: input.host, repositories: [input.repository] }; +} + export class GitHubCli extends Context.Service< GitHubCli, { - readonly execute: (input: { - readonly cwd: string; - readonly args: ReadonlyArray; - readonly host?: string; - readonly repositories?: ReadonlyArray; - readonly timeoutMs?: number; - /** Piped to the child's stdin, for payloads that must never appear in argv. */ - readonly stdin?: string; - readonly maxOutputBytes?: number; - }) => Effect.Effect; + readonly execute: ( + input: { + readonly cwd: string; + readonly args: ReadonlyArray; + readonly timeoutMs?: number; + /** Piped to the child's stdin, for payloads that must never appear in argv. */ + readonly stdin?: string; + readonly maxOutputBytes?: number; + } & GitHubAuthTarget, + ) => Effect.Effect; /** Stable, non-secret identity for the credential that will serve this target. */ readonly getBatchKey: ( @@ -469,20 +461,13 @@ export const make = Effect.gen(function* () { host, repositories: [...selected.failure.repositories], }); - case "HostMismatch": - return yield* new GitHubAccountHostMismatchError({ - command: "gh", - cwd: input.cwd, - host, - accountHost: selected.failure.accountHost, - login: selected.failure.login, - }); } } return selected.success; }); const tokenFor = Effect.fn("GitHubCli.tokenFor")(function* (input: { + readonly host: string; readonly account: GitHubAccountSelection; readonly cwd: string; }) { @@ -492,7 +477,7 @@ export const make = Effect.gen(function* () { return yield* new GitHubTokenEnvironmentUnavailableError({ command: "gh", cwd: input.cwd, - host: input.account.host, + host: input.host, login: input.account.login, tokenSource: input.account.tokenSource, }); @@ -502,7 +487,7 @@ export const make = Effect.gen(function* () { .run({ operation: "GitHubCli.authToken", command: "gh", - args: ["auth", "token", "--hostname", input.account.host, "--user", input.account.login], + args: ["auth", "token", "--hostname", input.host, "--user", input.account.login], cwd: input.cwd, timeoutMs: DEFAULT_TIMEOUT_MS, }) @@ -512,7 +497,7 @@ export const make = Effect.gen(function* () { return yield* new GitHubTokenOutputEmptyError({ command: "gh", cwd: input.cwd, - host: input.account.host, + host: input.host, login: input.account.login, tokenSource: input.account.tokenSource, }); @@ -539,7 +524,7 @@ export const make = Effect.gen(function* () { const route = yield* credentialRoute(input); if (route.account === undefined) return yield* run(input); - const token = yield* tokenFor({ account: route.account, cwd: input.cwd }); + const token = yield* tokenFor({ host: route.host, account: route.account, cwd: input.cwd }); const tokenVariable = route.host === "github.com" || route.host.endsWith(".ghe.com") ? "GH_TOKEN" @@ -627,8 +612,7 @@ export const make = Effect.gen(function* () { getRepositoryCloneUrls: (input) => execute({ cwd: input.cwd, - ...definedAuthTarget(input), - repositories: [input.repository], + ...repositoryAuthTarget(input), args: ["repo", "view", input.repository, "--json", "nameWithOwner,url,sshUrl"], }).pipe( Effect.map((result) => result.stdout.trim()), @@ -649,8 +633,7 @@ export const make = Effect.gen(function* () { createRepository: (input) => execute({ cwd: input.cwd, - ...definedAuthTarget(input), - repositories: [input.repository], + ...repositoryAuthTarget(input), args: ["repo", "create", input.repository, `--${input.visibility}`], }).pipe( Effect.map((result) => diff --git a/apps/server/src/sourceControl/GitHubCredentials.test.ts b/apps/server/src/sourceControl/GitHubCredentials.test.ts index a492f16f10d8..1157acd1e17a 100644 --- a/apps/server/src/sourceControl/GitHubCredentials.test.ts +++ b/apps/server/src/sourceControl/GitHubCredentials.test.ts @@ -8,7 +8,10 @@ import { selectCredentialRoute } from "./GitHubCredentials.ts"; describe("selectCredentialRoute", () => { it("uses the active account when no routing is saved", () => { assert.deepStrictEqual( - selectCredentialRoute(DEFAULT_SERVER_SETTINGS, { host: "GitHub.com" }), + selectCredentialRoute(DEFAULT_SERVER_SETTINGS, { + host: "GitHub.com", + repositories: [], + }), Result.succeed({ host: "github.com", key: "active:github.com", account: undefined }), ); }); @@ -16,11 +19,11 @@ describe("selectCredentialRoute", () => { it("selects defaults and owner overrides", () => { const settings = { ...DEFAULT_SERVER_SETTINGS, - githubDefaultAccounts: { - "github.com": { host: "github.com", login: "personal", tokenSource: "keyring" }, - }, - githubAccountOverrides: { - "github.com/acme": { host: "github.com", login: "work", tokenSource: "keyring" }, + githubAccountRouting: { + "github.com": { + defaultAccount: { login: "personal", tokenSource: "keyring" }, + ownerOverrides: { acme: { login: "work", tokenSource: "keyring" } }, + }, }, }; @@ -37,11 +40,11 @@ describe("selectCredentialRoute", () => { const selected = selectCredentialRoute( { ...DEFAULT_SERVER_SETTINGS, - githubDefaultAccounts: { - "github.com": { host: "github.com", login: "personal", tokenSource: "keyring" }, - }, - githubAccountOverrides: { - "github.com/acme": { host: "github.com", login: "work", tokenSource: "keyring" }, + githubAccountRouting: { + "github.com": { + defaultAccount: { login: "personal", tokenSource: "keyring" }, + ownerOverrides: { acme: { login: "work", tokenSource: "keyring" } }, + }, }, }, { @@ -57,26 +60,23 @@ describe("selectCredentialRoute", () => { }); }); - it("rejects an account saved under another host", () => { + it("scopes the same owner independently per host", () => { const selected = selectCredentialRoute( { ...DEFAULT_SERVER_SETTINGS, - githubDefaultAccounts: { + githubAccountRouting: { "github.com": { - host: "github.example.test", - login: "enterprise-user", - tokenSource: "keyring", + ownerOverrides: { acme: { login: "public-user", tokenSource: "keyring" } }, + }, + "github.example.test": { + ownerOverrides: { acme: { login: "enterprise-user", tokenSource: "keyring" } }, }, }, }, - { host: "github.com", repositories: ["acme/widget"] }, + { host: "github.example.test", repositories: ["acme/widget"] }, ); - assert(Result.isFailure(selected)); - assert.deepStrictEqual(selected.failure, { - _tag: "HostMismatch", - accountHost: "github.example.test", - login: "enterprise-user", - }); + assert(Result.isSuccess(selected)); + assert.equal(selected.success.account?.login, "enterprise-user"); }); }); diff --git a/apps/server/src/sourceControl/GitHubCredentials.ts b/apps/server/src/sourceControl/GitHubCredentials.ts index 9a6e81e2d13d..75f9a5be9bea 100644 --- a/apps/server/src/sourceControl/GitHubCredentials.ts +++ b/apps/server/src/sourceControl/GitHubCredentials.ts @@ -2,10 +2,9 @@ import * as Result from "effect/Result"; import type { GitHubAccountSelection, ServerSettings } from "@t3tools/contracts"; -export interface GitHubCredentialTarget { - readonly host?: string; - readonly repositories?: ReadonlyArray; -} +export type GitHubCredentialTarget = + | { readonly host?: undefined; readonly repositories?: undefined } + | { readonly host?: string; readonly repositories: ReadonlyArray }; export interface GitHubCredentialRoute { readonly host: string; @@ -13,24 +12,15 @@ export interface GitHubCredentialRoute { readonly account: GitHubAccountSelection | undefined; } -export type GitHubCredentialRoutingError = - | { - readonly _tag: "SelectionConflict"; - readonly repositories: ReadonlyArray; - } - | { - readonly _tag: "HostMismatch"; - readonly accountHost: string; - readonly login: string; - }; +export interface GitHubCredentialRoutingError { + readonly _tag: "SelectionConflict"; + readonly repositories: ReadonlyArray; +} -type GitHubCredentialSettings = Pick< - ServerSettings, - "githubDefaultAccounts" | "githubAccountOverrides" ->; +type GitHubCredentialSettings = Pick; -export function accountKey(account: GitHubAccountSelection): string { - return `${account.host.toLowerCase()}\n${account.login.toLowerCase()}\n${account.tokenSource}`; +function accountKey(host: string, account: GitHubAccountSelection): string { + return `${host.toLowerCase()}\n${account.login.toLowerCase()}\n${account.tokenSource}`; } /** Selects an account without reading credentials or performing any other effects. */ @@ -39,32 +29,27 @@ export function selectCredentialRoute( target: GitHubCredentialTarget, ): Result.Result { const host = (target.host ?? "github.com").toLowerCase(); - const defaults = new Map( - Object.entries(settings.githubDefaultAccounts).map(([accountHost, account]) => [ - accountHost.toLowerCase(), - account, - ]), - ); + const routing = Object.entries(settings.githubAccountRouting).find( + ([accountHost]) => accountHost.toLowerCase() === host, + )?.[1]; const overrides = new Map( - Object.entries(settings.githubAccountOverrides).map(([ownerKey, account]) => [ - ownerKey.toLowerCase(), + Object.entries(routing?.ownerOverrides ?? {}).map(([owner, account]) => [ + owner.toLowerCase(), account, ]), ); - const defaultAccount = defaults.get(host); + const defaultAccount = routing?.defaultAccount; const repositories = target.repositories ?? []; const accounts = repositories.length === 0 ? [defaultAccount] : repositories.map((repository) => { const owner = repository.trim().split("/")[0]?.toLowerCase(); - return owner === undefined - ? defaultAccount - : (overrides.get(`${host}/${owner}`) ?? defaultAccount); + return owner === undefined ? defaultAccount : (overrides.get(owner) ?? defaultAccount); }); const byKey = new Map( accounts.map((account) => [ - account === undefined ? `active:${host}` : accountKey(account), + account === undefined ? `active:${host}` : accountKey(host, account), account, ]), ); @@ -72,12 +57,5 @@ export function selectCredentialRoute( return Result.fail({ _tag: "SelectionConflict", repositories: [...repositories] }); } const [key, account] = byKey.entries().next().value ?? [`active:${host}`, undefined]; - if (account !== undefined && account.host.toLowerCase() !== host) { - return Result.fail({ - _tag: "HostMismatch", - accountHost: account.host, - login: account.login, - }); - } return Result.succeed({ host, key, account }); } diff --git a/apps/web/src/components/settings/GitHubAccountSettings.tsx b/apps/web/src/components/settings/GitHubAccountSettings.tsx index e59159d47477..832085fd1397 100644 --- a/apps/web/src/components/settings/GitHubAccountSettings.tsx +++ b/apps/web/src/components/settings/GitHubAccountSettings.tsx @@ -1,21 +1,27 @@ import { PlusIcon, Trash2Icon } from "lucide-react"; import { useState } from "react"; -import type { GitHubAccountSelection, GitHubAuthAccount } from "@t3tools/contracts"; +import type { + GitHubAccountRouting, + GitHubAccountSelection, + GitHubAuthAccount, +} from "@t3tools/contracts"; import { usePrimarySettings, useUpdatePrimarySettings } from "../../hooks/useSettings"; import { Button } from "../ui/button"; import { Input } from "../ui/input"; import { Select, SelectItem, SelectPopup, SelectTrigger, SelectValue } from "../ui/select"; -function accountKey(account: GitHubAccountSelection): string { - return `${account.host}\n${account.login}\n${account.tokenSource}`; +type RoutedAccount = GitHubAccountSelection & { readonly host: string }; + +function accountKey(account: RoutedAccount): string { + return `${account.host.toLowerCase()}\n${account.login.toLowerCase()}\n${account.tokenSource}`; } function tokenSourceLabel(tokenSource: string): string { return tokenSource === "keyring" ? "GitHub CLI" : tokenSource; } -function accountLabel(account: GitHubAccountSelection, includeHost: boolean): string { +function accountLabel(account: RoutedAccount, includeHost: boolean): string { return [ account.login, ...(includeHost ? [account.host] : []), @@ -23,12 +29,12 @@ function accountLabel(account: GitHubAccountSelection, includeHost: boolean): st ].join(" · "); } -function accountSelection(account: GitHubAuthAccount): GitHubAccountSelection { - return { - host: account.host, - login: account.login, - tokenSource: account.tokenSource, - }; +function accountSelection(account: RoutedAccount): GitHubAccountSelection { + return { login: account.login, tokenSource: account.tokenSource }; +} + +function routedAccount(host: string, account: GitHubAccountSelection): RoutedAccount { + return { host, ...account }; } export function hasMultipleGitHubAccountsOnHost( @@ -49,19 +55,18 @@ function GitHubAccountSelect({ includeHost = false, onChange, }: { - readonly accounts: ReadonlyArray; - readonly value: GitHubAccountSelection; + readonly accounts: ReadonlyArray; + readonly value: RoutedAccount; readonly label: string; readonly includeHost?: boolean; - readonly onChange: (account: GitHubAccountSelection) => void; + readonly onChange: (account: RoutedAccount) => void; }) { - const selections = accounts.map(accountSelection); - const selected = selections.find((account) => accountKey(account) === accountKey(value)) ?? value; + const selected = accounts.find((account) => accountKey(account) === accountKey(value)) ?? value; return (