diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index 7cc252d3bf28..5a30611af55c 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -493,6 +493,8 @@ function createGitHubCliWithFakeGh(scenario: FakeGhScenario = {}): { return { service: { execute, + getBatchKey: (input) => + Effect.succeed(`active:${(input.host ?? "github.com").toLowerCase()}`), listOpenPullRequests: (input) => execute({ cwd: input.cwd, diff --git a/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts b/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts index bf7e8951afe2..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"); @@ -1298,12 +1299,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..cc56fca17316 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 getBatchKey: (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,38 +666,49 @@ 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, + repositories: [input.repository], args: ["api", "graphql", "--hostname", input.host, "--input", "-"], stdin: encodeGraphQlRequestJson({ query: input.query, variables: input.variables }), }) .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 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 => - github + 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, args: [ "api", "graphql", @@ -702,6 +721,8 @@ export const make = Effect.gen(function* () { } : { cwd: input.cwd, + host: input.host, + repositories, args: ["api", "graphql", "--hostname", input.host, "--input", "-"], stdin: encodeGraphQlRequestJson({ query: input.query, @@ -724,6 +745,7 @@ export const make = Effect.gen(function* () { ); }), ); + }; /** * One page of the patch, read from the files API. GitHub refuses `pr diff` outright past 300 @@ -748,6 +770,8 @@ export const make = Effect.gen(function* () { return github .execute({ cwd: input.cwd, + host: input.host, + repositories: [input.repository], args: [ "api", "--hostname", @@ -810,6 +834,8 @@ export const make = Effect.gen(function* () { const { owner, name } = parseRepositorySelector(input.repository); const refsResult = yield* github.execute({ cwd: input.cwd, + host: input.host, + repositories: [input.repository], args: [ "api", "--hostname", @@ -850,6 +876,8 @@ export const make = Effect.gen(function* () { github .execute({ cwd: input.cwd, + host: input.host, + repositories: [input.repository], args: [ "api", "--hostname", @@ -892,15 +920,31 @@ export const make = Effect.gen(function* () { }); return GitHubPullRequestCli.of({ + getBatchKey: (input) => + github.getBatchKey({ + cwd: input.cwd, + host: input.host, + repositories: [input.repository], + }), + 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, + repositories: [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 +955,8 @@ export const make = Effect.gen(function* () { github .execute({ cwd: input.cwd, + host: input.host, + repositories: [input.repository], args: [ "pr", "list", @@ -1005,6 +1051,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 +1087,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 +1108,8 @@ export const make = Effect.gen(function* () { github .execute({ cwd: input.cwd, + host: input.host, + repositories: [input.repository], args: [ "pr", "view", @@ -1089,6 +1139,8 @@ export const make = Effect.gen(function* () { github .execute({ cwd: input.cwd, + host: input.host, + repositories: [input.repository], args: [ "pr", "view", @@ -1142,6 +1194,8 @@ export const make = Effect.gen(function* () { return github .execute({ cwd: input.cwd, + host: input.host, + repositories: [input.repository], args: ["pr", "diff", String(input.number), ...repositoryArgs(input), "--color", "never"], maxOutputBytes: DIFF_MAX_OUTPUT_BYTES, timeoutMs: DIFF_TIMEOUT_MS, @@ -1180,6 +1234,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 +1258,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 +1340,8 @@ export const make = Effect.gen(function* () { return github .execute({ cwd: input.cwd, + host: input.host, + repositories: [input.repository], args: [ "api", "graphql", @@ -1315,6 +1373,8 @@ export const make = Effect.gen(function* () { github .execute({ cwd: input.cwd, + host: input.host, + repositories: [input.repository], args: [ "repo", "view", @@ -1344,6 +1404,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 +1421,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 +1438,8 @@ export const make = Effect.gen(function* () { return github .execute({ cwd: input.cwd, + host: input.host, + repositories: [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 +1464,8 @@ export const make = Effect.gen(function* () { return github .execute({ cwd: input.cwd, + host: input.host, + repositories: [input.repository], args: ["pr", subcommand!, String(input.number), ...repositoryArgs(input), ...flags], }) .pipe(Effect.asVoid); @@ -1409,6 +1475,8 @@ export const make = Effect.gen(function* () { github .execute({ cwd: input.cwd, + host: input.host, + repositories: [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 +1496,8 @@ export const make = Effect.gen(function* () { return github .execute({ cwd: input.cwd, + host: input.host, + repositories: [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 +1524,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 +1533,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..d91bb42fc8dd 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestProvider.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestProvider.ts @@ -67,7 +67,12 @@ 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 === "GitHubTokenEnvironmentUnavailableError" || + error._tag === "GitHubTokenOutputEmptyError" + ) + return "unauthenticated"; return "failed"; } @@ -114,8 +119,16 @@ export const make = Effect.gen(function* () { kind: "github", capabilities: CAPABILITIES, + getBatchKey: (input) => cli.getBatchKey(input).pipe(Effect.mapError(fail("getBatchKey"))), + 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..bcf651f140f6 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; + /** Opaque identity used to keep only compatible repositories in the same provider batch. */ + readonly getBatchKey?: ( + 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 77a7118d961a..f74ea9a4e0c8 100644 --- a/apps/server/src/pullRequest/PullRequestService.test.ts +++ b/apps/server/src/pullRequest/PullRequestService.test.ts @@ -1160,6 +1160,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", { + getBatchKey: (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 adff1f83729b..cc77177c39aa 100644 --- a/apps/server/src/pullRequest/PullRequestService.ts +++ b/apps/server/src/pullRequest/PullRequestService.ts @@ -206,12 +206,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>; } @@ -467,8 +462,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); @@ -548,17 +544,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 provider batch key, which may differ by repository on the same host. */ type ResolvedViewer = { readonly host: string; readonly kind: SourceControlProviderKind; + readonly batchKey: string; + readonly projects: ReadonlyArray; readonly viewer: string | null; readonly error: PullRequestProviderError | null; }; @@ -567,41 +558,122 @@ 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 viewersByBatchKey = new Map< + string, + { 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"], ) => - 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 keyed = yield* Effect.forEach( + projects, + (project) => + getBatchKey(project).pipe( + Effect.match({ + onFailure: (error) => ({ + project, + batchKey: `unavailable:${listCursorKey(project.host, project.repository)}`, + error, + }), + onSuccess: (batchKey) => ({ project, batchKey, error: null }), + }), + ), + { concurrency: REPOSITORY_CONCURRENCY }, + ); + const groups = new Map< + string, + { + batchKey: string; + projects: SupportedProject[]; + error: PullRequestProviderError | null; + } + >(); + for (const entry of keyed) { + const key = `${entry.project.host}\n${entry.batchKey}`; + const held = groups.get(key); + if (held === undefined) { + groups.set(key, { + batchKey: entry.batchKey, + 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 = viewersByBatchKey.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, + batchKey: group.batchKey, + 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, + batchKey: group.batchKey, + 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; + viewersByBatchKey.set(cacheKey, { at, result: cached }); + }), + ), + Effect.catch((error) => + Effect.succeed({ + host: first.host, + kind: api.kind, + batchKey: group.batchKey, + projects: group.projects, + viewer: null, + error, + }), + ), + ); + }), + { concurrency: REPOSITORY_CONCURRENCY }, + ); + }); const toEntry = (input: { readonly project: SupportedProject; @@ -654,23 +726,45 @@ export const make = Effect.gen(function* () { const viewerResults = yield* resolveViewers(projects, viewerRoots); const viewers: Record = {}; + const batchKeys = new Map(); + const viewerBatchCounts = new Map(); + for (const result of viewerResults) { + viewerBatchCounts.set(result.host, (viewerBatchCounts.get(result.host) ?? 0) + 1); + } for (const result of viewerResults) { - if (result.viewer !== null) viewers[result.host] = result.viewer; + const hostHasMultipleBatches = (viewerBatchCounts.get(result.host) ?? 0) > 1; + for (const project of result.projects) { + const key = listCursorKey(project.host, project.repository); + batchKeys.set(key, result.batchKey); + if (result.viewer !== null && hostHasMultipleBatches) viewers[key] = result.viewer; + } + if (result.viewer !== null && viewers[result.host] === undefined) { + viewers[result.host] = result.viewer; + } } - // 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. + const viewerOf = (project: SupportedProject): string | undefined => + (viewerBatchCounts.get(project.host) ?? 0) > 1 + ? viewers[listCursorKey(project.host, project.repository)] + : viewers[project.host]; + + // 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, @@ -691,11 +785,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, @@ -711,7 +805,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], ); @@ -740,7 +834,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 @@ -825,7 +919,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, @@ -907,7 +1001,7 @@ export const make = Effect.gen(function* () { separate.push(project); continue; } - const key = `${project.host}\n${cursorOf(project)?.updatedBefore ?? ""}`; + const key = `${project.host}\n${batchKeys.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); @@ -1406,14 +1500,26 @@ 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 keyed = yield* Effect.forEach( + [...wanted.values()], + (entry) => + getBatchKey(entry.project).pipe( + Effect.map((batchKey) => ({ ...entry, batchKey })), + Effect.orElseSucceed(() => null), + ), + { concurrency: REPOSITORY_CONCURRENCY }, + ); + type BatchedStat = Exclude<(typeof keyed)[number], null>; + const byBatchKey = new Map>(); + for (const entry of keyed) { + if (entry === null) continue; + const key = `${entry.project.host}\n${entry.batchKey}`; + const held = byBatchKey.get(key); + if (held === undefined) byBatchKey.set(key, [entry]); else held.push(entry); } const stats = yield* Effect.forEach( - [...byHost.values()], + [...byBatchKey.values()], (entries) => { const first = entries[0]!; const readStats = first.project.api.listChangeRequestStats; @@ -1695,7 +1801,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(); + viewersByBatchKey.clear(); return; } bumpRefEpoch(input.reference); diff --git a/apps/server/src/serverSettings.ts b/apps/server/src/serverSettings.ts index 2798faf6f006..b3326b09b7c6 100644 --- a/apps/server/src/serverSettings.ts +++ b/apps/server/src/serverSettings.ts @@ -212,6 +212,7 @@ const ATOMIC_SETTINGS_KEYS: ReadonlySet = new Set([ "backgroundActivity", "automaticGitFetchInterval", "providerHealthRefreshInterval", + "githubAccountRouting", "sourceControlWriterModelSelection", "textGenerationModelSelection", ]); diff --git a/apps/server/src/sourceControl/GitHubCli.test.ts b/apps/server/src/sourceControl/GitHubCli.test.ts index 5daf7676d60c..9d46654705b3 100644 --- a/apps/server/src/sourceControl/GitHubCli.test.ts +++ b/apps/server/src/sourceControl/GitHubCli.test.ts @@ -3,8 +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 * as ServerSettings from "../serverSettings.ts"; import * as VcsProcess from "../vcs/VcsProcess.ts"; import * as GitHubCli from "./GitHubCli.ts"; @@ -18,19 +19,323 @@ 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(ServerSettings.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", + repositories: ["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({ + githubAccountRouting: { + "github.com": { + defaultAccount: { login: "DominicVonk", tokenSource: "GITHUB_TOKEN" }, + ownerOverrides: {}, + }, + }, + }); + + return Effect.gen(function* () { + const gh = yield* GitHubCli.GitHubCli; + yield* gh.execute({ + cwd: "/repo", + host: "github.com", + repositories: ["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({ + githubAccountRouting: { + "github.com": { + ownerOverrides: { acme: { login: "work-user", tokenSource: "keyring" } }, + }, + }, + }); + + return Effect.gen(function* () { + const gh = yield* GitHubCli.GitHubCli; + yield* gh.execute({ + cwd: "/repo", + host: "github.com", + repositories: ["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.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({ + githubAccountRouting: { + "github.com": { + defaultAccount: { login: "work-user", tokenSource: "keyring" }, + ownerOverrides: {}, + }, + }, + }); + + return Effect.gen(function* () { + const gh = yield* GitHubCli.GitHubCli; + const input = { + cwd: "/repo", + host: "github.com", + repositories: ["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("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({ + githubAccountRouting: { + "github.com": { + defaultAccount: { login: "work-user", tokenSource: "keyring" }, + ownerOverrides: {}, + }, + }, + }); + + return Effect.gen(function* () { + const gh = yield* GitHubCli.GitHubCli; + const error = yield* gh + .execute({ + cwd: "/repo", + host: "github.com", + repositories: ["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"))); + const selectedLayer = layerWithSettings({ + githubAccountRouting: { + "github.com": { + defaultAccount: { login: "work-user", tokenSource: "GITHUB_TOKEN" }, + ownerOverrides: {}, + }, + }, + }); + + return Effect.gen(function* () { + const gh = yield* GitHubCli.GitHubCli; + yield* gh.execute({ + cwd: "/repo", + host: "GitHub.com", + repositories: ["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 unavailable token sources structurally", () => { + delete process.env.GITHUB_TOKEN; + const selectedLayer = layerWithSettings({ + githubAccountRouting: { + "github.com": { + defaultAccount: { login: "environment-user", tokenSource: "GITHUB_TOKEN" }, + ownerOverrides: {}, + }, + }, + }); + + return Effect.gen(function* () { + const gh = yield* GitHubCli.GitHubCli; + const error = yield* gh + .execute({ + cwd: "/repo", + host: "github.com", + repositories: ["acme/widget"], + args: ["api", "user"], + }) + .pipe(Effect.flip); + + assert.equal(error._tag, "GitHubTokenEnvironmentUnavailableError"); + if (error._tag !== "GitHubTokenEnvironmentUnavailableError") return; + assert.equal(error.tokenSource, "GITHUB_TOKEN"); + }).pipe(Effect.provide(selectedLayer)); + }); + + it.effect("reports empty keyring token output structurally", () => { + mockRun.mockReturnValueOnce(Effect.succeed(processOutput("\n"))); + const selectedLayer = layerWithSettings({ + githubAccountRouting: { + "github.com": { + defaultAccount: { login: "work-user", tokenSource: "keyring" }, + ownerOverrides: {}, + }, + }, + }); + + return Effect.gen(function* () { + const gh = yield* GitHubCli.GitHubCli; + const error = yield* gh + .execute({ + cwd: "/repo", + host: "github.com", + repositories: ["acme/widget"], + args: ["api", "user"], + }) + .pipe(Effect.flip); + + assert.equal(error._tag, "GitHubTokenOutputEmptyError"); + if (error._tag !== "GitHubTokenOutputEmptyError") return; + assert.equal(error.login, "work-user"); + }).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 + .getBatchKey({ + cwd: "/repo", + host: "github.com", + repositories: ["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 a705b0fb0b3e..dcbeb0819f06 100644 --- a/apps/server/src/sourceControl/GitHubCli.ts +++ b/apps/server/src/sourceControl/GitHubCli.ts @@ -6,12 +6,15 @@ import * as Result from "effect/Result"; import * as Schema from "effect/Schema"; import { + type GitHubAccountSelection, TrimmedNonEmptyString, type SourceControlRepositoryVisibility, type VcsError, } from "@t3tools/contracts"; +import * as ServerSettings from "../serverSettings.ts"; import * as VcsProcess from "../vcs/VcsProcess.ts"; +import * as GitHubCredentials from "./GitHubCredentials.ts"; import { decodeGitHubPullRequestJson, decodeGitHubPullRequestListJson, @@ -77,6 +80,78 @@ export class GitHubCliCommandError extends Schema.TaggedErrorClass()( + "GitHubAccountSettingsUnavailableError", + { + ...gitHubCredentialFields, + cause: Schema.Defect(), + }, +) { + get detail(): string { + 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 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 { + return `GitHub CLI credential selection failed: ${this.detail}`; + } +} + const gitHubCliDecodeFields = { command: Schema.Literal("gh"), cwd: Schema.String, @@ -140,6 +215,10 @@ export const GitHubCliError = Schema.Union([ GitHubCliAuthenticationError, GitHubPullRequestNotFoundError, GitHubCliCommandError, + GitHubAccountSettingsUnavailableError, + GitHubAccountSelectionConflictError, + GitHubTokenEnvironmentUnavailableError, + GitHubTokenOutputEmptyError, GitHubPullRequestListDecodeError, GitHubChangeRequestListDecodeError, GitHubPullRequestDecodeError, @@ -196,57 +275,104 @@ export interface GitHubRepositoryCloneUrls { readonly sshUrl: string; } +type GitHubAuthTarget = GitHubCredentials.GitHubCredentialTarget; + +const GITHUB_TOKEN_ENV_SOURCES = new Set([ + "GH_TOKEN", + "GITHUB_TOKEN", + "GH_ENTERPRISE_TOKEN", + "GITHUB_ENTERPRISE_TOKEN", +]); + +function definedAuthTarget(input: GitHubAuthTarget): GitHubAuthTarget { + if (input.repositories === undefined) return {}; + return { + ...(input.host === undefined ? {} : { host: input.host }), + 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 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; + 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: ( + 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 +433,113 @@ function deriveRepositoryCloneUrlsFromCreateOutput( } export const make = Effect.gen(function* () { - const process = yield* VcsProcess.VcsProcess; + const vcsProcess = yield* VcsProcess.VcsProcess; + const settings = yield* ServerSettings.ServerSettingsService; + + const credentialRoute = Effect.fn("GitHubCli.credentialRoute")(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 GitHubAccountSettingsUnavailableError({ + command: "gh", + cwd: input.cwd, + host, + cause, + }), + ), + ); + const selected = GitHubCredentials.selectCredentialRoute(current, input); + if (Result.isFailure(selected)) { + switch (selected.failure._tag) { + case "SelectionConflict": + return yield* new GitHubAccountSelectionConflictError({ + command: "gh", + cwd: input.cwd, + host, + repositories: [...selected.failure.repositories], + }); + } + } + return selected.success; + }); + + const tokenFor = Effect.fn("GitHubCli.tokenFor")(function* (input: { + readonly host: string; + 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 GitHubTokenEnvironmentUnavailableError({ + command: "gh", + cwd: input.cwd, + host: input.host, + login: input.account.login, + tokenSource: input.account.tokenSource, + }); + } + + const output = yield* vcsProcess + .run({ + operation: "GitHubCli.authToken", + command: "gh", + args: ["auth", "token", "--hostname", input.host, "--user", input.account.login], + cwd: input.cwd, + timeoutMs: DEFAULT_TIMEOUT_MS, + }) + .pipe(Effect.mapError((error) => fromVcsError({ command: "gh", cwd: input.cwd }, error))); + const token = output.stdout.trim(); + if (token.length === 0) { + return yield* new GitHubTokenOutputEmptyError({ + command: "gh", + cwd: input.cwd, + host: input.host, + login: input.account.login, + tokenSource: input.account.tokenSource, + }); + } + return token; + }); - const execute: GitHubCli["Service"]["execute"] = (input) => - process + 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 route = yield* credentialRoute(input); + if (route.account === undefined) return yield* run(input); + + 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" + : "GH_ENTERPRISE_TOKEN"; + return yield* run(input, { [tokenVariable]: token }); + }, + ); + return GitHubCli.of({ execute, + getBatchKey: (input) => credentialRoute(input).pipe(Effect.map((route) => route.key)), listOpenPullRequests: (input) => execute({ cwd: input.cwd, + ...definedAuthTarget(input), args: [ "pr", "list", @@ -366,6 +579,7 @@ export const make = Effect.gen(function* () { getPullRequest: (input) => execute({ cwd: input.cwd, + ...definedAuthTarget(input), args: [ "pr", "view", @@ -398,6 +612,7 @@ export const make = Effect.gen(function* () { getRepositoryCloneUrls: (input) => execute({ cwd: input.cwd, + ...repositoryAuthTarget(input), args: ["repo", "view", input.repository, "--json", "nameWithOwner,url,sshUrl"], }).pipe( Effect.map((result) => result.stdout.trim()), @@ -418,6 +633,7 @@ export const make = Effect.gen(function* () { createRepository: (input) => execute({ cwd: input.cwd, + ...repositoryAuthTarget(input), args: ["repo", "create", input.repository, `--${input.visibility}`], }).pipe( Effect.map((result) => @@ -427,6 +643,7 @@ export const make = Effect.gen(function* () { createPullRequest: (input) => execute({ cwd: input.cwd, + ...definedAuthTarget(input), args: [ "pr", "create", @@ -443,6 +660,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 +671,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/GitHubCredentials.test.ts b/apps/server/src/sourceControl/GitHubCredentials.test.ts new file mode 100644 index 000000000000..1157acd1e17a --- /dev/null +++ b/apps/server/src/sourceControl/GitHubCredentials.test.ts @@ -0,0 +1,82 @@ +import { assert, describe, it } from "@effect/vitest"; +import * as Result from "effect/Result"; + +import { DEFAULT_SERVER_SETTINGS } from "@t3tools/contracts"; + +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", + repositories: [], + }), + Result.succeed({ host: "github.com", key: "active:github.com", account: undefined }), + ); + }); + + it("selects defaults and owner overrides", () => { + const settings = { + ...DEFAULT_SERVER_SETTINGS, + githubAccountRouting: { + "github.com": { + defaultAccount: { login: "personal", tokenSource: "keyring" }, + ownerOverrides: { acme: { login: "work", tokenSource: "keyring" } }, + }, + }, + }; + + const selected = selectCredentialRoute(settings, { + host: "github.com", + repositories: ["acme/widget"], + }); + + assert(Result.isSuccess(selected)); + assert.equal(selected.success.account?.login, "work"); + }); + + it("rejects a batch that needs different accounts", () => { + const selected = selectCredentialRoute( + { + ...DEFAULT_SERVER_SETTINGS, + githubAccountRouting: { + "github.com": { + defaultAccount: { login: "personal", tokenSource: "keyring" }, + ownerOverrides: { acme: { login: "work", tokenSource: "keyring" } }, + }, + }, + }, + { + host: "github.com", + repositories: ["personal/widget", "acme/widget"], + }, + ); + + assert(Result.isFailure(selected)); + assert.deepStrictEqual(selected.failure, { + _tag: "SelectionConflict", + repositories: ["personal/widget", "acme/widget"], + }); + }); + + it("scopes the same owner independently per host", () => { + const selected = selectCredentialRoute( + { + ...DEFAULT_SERVER_SETTINGS, + githubAccountRouting: { + "github.com": { + ownerOverrides: { acme: { login: "public-user", tokenSource: "keyring" } }, + }, + "github.example.test": { + ownerOverrides: { acme: { login: "enterprise-user", tokenSource: "keyring" } }, + }, + }, + }, + { host: "github.example.test", repositories: ["acme/widget"] }, + ); + + 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 new file mode 100644 index 000000000000..75f9a5be9bea --- /dev/null +++ b/apps/server/src/sourceControl/GitHubCredentials.ts @@ -0,0 +1,61 @@ +import * as Result from "effect/Result"; + +import type { GitHubAccountSelection, ServerSettings } from "@t3tools/contracts"; + +export type GitHubCredentialTarget = + | { readonly host?: undefined; readonly repositories?: undefined } + | { readonly host?: string; readonly repositories: ReadonlyArray }; + +export interface GitHubCredentialRoute { + readonly host: string; + readonly key: string; + readonly account: GitHubAccountSelection | undefined; +} + +export interface GitHubCredentialRoutingError { + readonly _tag: "SelectionConflict"; + readonly repositories: ReadonlyArray; +} + +type GitHubCredentialSettings = Pick; + +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. */ +export function selectCredentialRoute( + settings: GitHubCredentialSettings, + target: GitHubCredentialTarget, +): Result.Result { + const host = (target.host ?? "github.com").toLowerCase(); + const routing = Object.entries(settings.githubAccountRouting).find( + ([accountHost]) => accountHost.toLowerCase() === host, + )?.[1]; + const overrides = new Map( + Object.entries(routing?.ownerOverrides ?? {}).map(([owner, account]) => [ + owner.toLowerCase(), + account, + ]), + ); + 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(owner) ?? defaultAccount); + }); + const byKey = new Map( + accounts.map((account) => [ + account === undefined ? `active:${host}` : accountKey(host, account), + account, + ]), + ); + if (byKey.size > 1) { + return Result.fail({ _tag: "SelectionConflict", repositories: [...repositories] }); + } + const [key, account] = byKey.entries().next().value ?? [`active:${host}`, undefined]; + return Result.succeed({ host, key, account }); +} diff --git a/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts b/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts index 9e8a68295667..4ec72524abd1 100644 --- a/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts +++ b/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts @@ -178,6 +178,48 @@ it.effect("treats empty non-open change request listing output as no results", ( }), ); +it.effect("drops transport ports from GitHub Enterprise auth targets", () => + Effect.gen(function* () { + const listInputs: Array[0]> = + []; + const provider = yield* makeProvider({ + listOpenPullRequests: (input) => { + listInputs.push(input); + return Effect.succeed([]); + }, + }); + + 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, + }, + headSelector: "feature/accounts", + state: "open", + limit: 10, + }); + } + + assert.deepStrictEqual( + listInputs.map(({ host, repositories }) => ({ host, repositories })), + [ + { host: "github.example", repositories: ["owner/repo"] }, + { host: "github.example", repositories: ["owner/repo"] }, + ], + ); + }), +); + it.effect("creates GitHub PRs through provider-neutral input names", () => Effect.gen(function* () { let createInput: Parameters[0] | null = @@ -327,6 +369,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 +377,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 +385,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..3028e7bde6ca 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, repositories: [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/GitHubAccountSettings.tsx b/apps/web/src/components/settings/GitHubAccountSettings.tsx new file mode 100644 index 000000000000..832085fd1397 --- /dev/null +++ b/apps/web/src/components/settings/GitHubAccountSettings.tsx @@ -0,0 +1,325 @@ +import { PlusIcon, Trash2Icon } from "lucide-react"; +import { useState } from "react"; +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"; + +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: RoutedAccount, includeHost: boolean): string { + return [ + account.login, + ...(includeHost ? [account.host] : []), + tokenSourceLabel(account.tokenSource), + ].join(" ยท "); +} + +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( + 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: RoutedAccount; + readonly label: string; + readonly includeHost?: boolean; + readonly onChange: (account: RoutedAccount) => void; +}) { + const selected = accounts.find((account) => accountKey(account) === accountKey(value)) ?? value; + return ( + + ); +} + +export function GitHubAccountSettings({ + accounts, +}: { + readonly accounts: ReadonlyArray; +}) { + const settings = usePrimarySettings(); + const updateSettings = useUpdatePrimarySettings(); + const routing = settings.githubAccountRouting; + const [owner, setOwner] = useState(""); + const uniqueAccounts = [ + ...new Map(accounts.map((account) => [accountKey(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.entries(routing) + .filter( + ([host, route]) => + route.defaultAccount !== undefined && !selectableHostKeys.has(host.toLowerCase()), + ) + .map(([host]) => host); + const selectableAccounts = selectableHosts.flatMap(([, hostAccounts]) => hostAccounts); + const initialAccount = + selectableAccounts.find((account) => account.active) ?? selectableAccounts[0]; + const [accountKeyValue, setAccountKeyValue] = useState( + initialAccount === undefined ? "" : accountKey(initialAccount), + ); + const hasSavedRouting = Object.keys(routing).length > 0; + + const updateHostRouting = ( + host: string, + update: (current: GitHubAccountRouting[string]) => GitHubAccountRouting[string], + ) => { + const hostKey = host.toLowerCase(); + const nextRouting = { ...routing }; + const next = update(routing[hostKey] ?? { ownerOverrides: {} }); + if (next.defaultAccount === undefined && Object.keys(next.ownerOverrides).length === 0) { + delete nextRouting[hostKey]; + } else { + nextRouting[hostKey] = next; + } + updateSettings({ githubAccountRouting: nextRouting }); + }; + + 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) => accountKey(entry) === accountKeyValue) ?? + selectableAccounts[0]!; + const canAddOverride = /^[A-Za-z0-9_.-]+$/.test(normalizedOwner); + + const addOverride = () => { + if (!canAddOverride) return; + updateHostRouting(overrideAccount.host, (current) => ({ + ...current, + ownerOverrides: { + ...current.ownerOverrides, + [normalizedOwner.toLowerCase()]: accountSelection(overrideAccount), + }, + })); + setOwner(""); + }; + + const savedOverrides = Object.entries(routing).flatMap(([host, route]) => + Object.entries(route.ownerOverrides).map(([ownerKey, account]) => ({ + host, + owner: ownerKey, + account, + })), + ); + + 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 = routedAccount(host, routing[host]?.defaultAccount ?? active); + return ( +
+
+
Default for {host}
+
+ Used unless a repository owner override matches. +
+
+ + updateHostRouting(host, (current) => ({ + ...current, + defaultAccount: accountSelection(account), + })) + } + /> +
+ ); + })} + +
+
+
Repository owner overrides
+
+ Use another account for every repository owned by an organization or user. +
+
+ {savedOverrides.map(({ host, owner: ownerKey, account }) => { + const hostAccounts = accountsByHost.get(host.toLowerCase()) ?? []; + const routeKey = `${host}/${ownerKey}`; + return ( +
+ {routeKey} + + updateHostRouting(host, (current) => ({ + ...current, + ownerOverrides: { + ...current.ownerOverrides, + [ownerKey]: accountSelection(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 10b54f6d7afa..41c310ebc34c 100644 --- a/apps/web/src/components/settings/SourceControlSettings.tsx +++ b/apps/web/src/components/settings/SourceControlSettings.tsx @@ -52,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"; @@ -509,6 +510,7 @@ function EmptySourceControlDiscovery({ export function SourceControlSettingsPanel() { const environmentId = usePrimaryEnvironment()?.environmentId ?? null; + const settings = usePrimarySettings(); const discovery = useEnvironmentQuery( environmentId === null ? null @@ -574,7 +576,13 @@ export function SourceControlSettingsPanel() { headerAction={hasVersionControlSystems ? null : scanButton} > {result.sourceControlProviders.map((item) => ( - + + {item.kind === "github" && + (hasMultipleGitHubAccountsOnHost(item.auth.githubAccounts ?? []) || + Object.keys(settings.githubAccountRouting).length > 0) ? ( + + ) : 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..f26f79237c20 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.githubAccountRouting).toEqual({}); + }); + + it("accepts GitHub account defaults and owner overrides without token values", () => { + const account = { + login: "octocat", + tokenSource: "keyring", + }; + const decoded = decodeServerSettingsPatch({ + githubAccountRouting: { + "github.com": { defaultAccount: account, ownerOverrides: { acme: account } }, + }, + }); + + expect(decoded.githubAccountRouting).toEqual({ + "github.com": { defaultAccount: account, ownerOverrides: { 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..f325982cedb5 100644 --- a/packages/contracts/src/settings.ts +++ b/packages/contracts/src/settings.ts @@ -495,6 +495,24 @@ export const SourceControlWritingStyleSettings = Schema.Struct({ }); export type SourceControlWritingStyleSettings = typeof SourceControlWritingStyleSettings.Type; +export const GitHubAccountSelection = Schema.Struct({ + login: TrimmedNonEmptyString.check(Schema.isMaxLength(100)), + tokenSource: TrimmedNonEmptyString.check(Schema.isMaxLength(500)), +}); +export type GitHubAccountSelection = typeof GitHubAccountSelection.Type; + +export const GitHubAccountRouting = Schema.Record( + TrimmedNonEmptyString.check(Schema.isMaxLength(255)), + Schema.Struct({ + defaultAccount: Schema.optionalKey(GitHubAccountSelection), + ownerOverrides: Schema.Record( + TrimmedNonEmptyString.check(Schema.isMaxLength(100)), + GitHubAccountSelection, + ).pipe(Schema.withDecodingDefault(Effect.succeed({}))), + }), +); +export type GitHubAccountRouting = typeof GitHubAccountRouting.Type; + export const DEFAULT_AUTOMATIC_GIT_FETCH_INTERVAL = Duration.seconds(30); export const DEFAULT_PROVIDER_HEALTH_REFRESH_INTERVAL = Duration.minutes(5); @@ -588,6 +606,7 @@ export const ServerSettings = Schema.Struct({ sourceControlWriterModelSelection: Schema.NullOr(ModelSelection).pipe( Schema.withDecodingDefault(Effect.succeed(null)), ), + githubAccountRouting: GitHubAccountRouting.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 +750,7 @@ export const ServerSettingsPatch = Schema.Struct({ }), ), sourceControlWriterModelSelection: Schema.optionalKey(Schema.NullOr(ModelSelection)), + githubAccountRouting: Schema.optionalKey(GitHubAccountRouting), 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..4be34afe46ac 100644 --- a/packages/shared/src/serverSettings.ts +++ b/packages/shared/src/serverSettings.ts @@ -131,6 +131,7 @@ export function applyServerSettingsPatch( providerHealthRefreshInterval, backgroundActivityProfile, backgroundActivity, + githubAccountRouting, ...patchForMerge } = patch; const currentBackgroundActivity = normalizeServerBackgroundActivitySettings(current); @@ -190,6 +191,7 @@ export function applyServerSettingsPatch( ...(patch.sourceControlWriterModelSelection !== undefined ? { sourceControlWriterModelSelection: patch.sourceControlWriterModelSelection } : {}), + ...(githubAccountRouting !== undefined ? { githubAccountRouting } : {}), ...(automaticGitFetchInterval !== undefined ? { automaticGitFetchInterval } : {}), ...(providerHealthRefreshInterval !== undefined ? { providerHealthRefreshInterval } : {}), };