diff --git a/apps/server/scripts/evaluate-thread-titles.ts b/apps/server/scripts/evaluate-thread-titles.ts index 4e0691fe3a06d22ec0950efafc1657f272ff6f40..414bd99d07459bcc736088cfc2c63dfe70bcc9b7 100644 --- a/apps/server/scripts/evaluate-thread-titles.ts +++ b/apps/server/scripts/evaluate-thread-titles.ts @@ -157,9 +157,11 @@ await Effect.runPromise( ForgejoCli.layer, AzureDevOpsCli.layer, // No saved credentials here; Bitbucket falls back to T3CODE_BITBUCKET_* variables. - BitbucketApi.layer.pipe(Layer.provide(ServerSettings.layerTest())), + BitbucketApi.layer, ), ), + // Default settings: no saved Bitbucket token, gh's own GitHub account choice. + Layer.provide(ServerSettings.layerTest()), Layer.provide(VcsDriverRegistry.layer.pipe(Layer.provide(VcsProjectConfig.layer))), Layer.provide(GitVcsDriver.layer), Layer.provide(VcsProcess.layer), diff --git a/apps/server/scripts/measure-pr-preview.ts b/apps/server/scripts/measure-pr-preview.ts index ae291cd17cc69884cd0f05ead65c24232b03cb1b..98fc82b5398227ca614aea382f3cdabd5474b6a5 100644 --- a/apps/server/scripts/measure-pr-preview.ts +++ b/apps/server/scripts/measure-pr-preview.ts @@ -14,6 +14,7 @@ import * as GitHubPullRequestCli from "../src/pullRequest/GitHubPullRequestCli.t import * as GitHubPullRequestProvider from "../src/pullRequest/GitHubPullRequestProvider.ts"; import * as GitHubApi from "../src/sourceControl/GitHubApi.ts"; import * as GitHubCredentials from "../src/sourceControl/GitHubCredentials.ts"; +import * as ServerSettings from "../src/serverSettings.ts"; import * as GitHubGraphQlBudget from "../src/sourceControl/githubGraphQlBudget.ts"; import * as SourceControlRateLimit from "../src/sourceControl/SourceControlRateLimit.ts"; import * as VcsProcess from "../src/vcs/VcsProcess.ts"; @@ -64,7 +65,7 @@ const measuredApi = Layer.effect( }); }), ).pipe( - Layer.provide(GitHubCredentials.layer), + Layer.provide(GitHubCredentials.layer.pipe(Layer.provide(ServerSettings.layerTest()))), Layer.provide(GitHubGraphQlBudget.layer), Layer.provide(SourceControlRateLimit.layer), Layer.provide(FetchHttpClient.layer), diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index 42fdc87afe57a29e30e922835cedff92da0633a3..8f743c65fa31639542102daf2ac91e97df2b7b50 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -4553,7 +4553,7 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { Effect.flip, Effect.map((error) => error.message), ); - expect(errorMessage).toContain("GitHub CLI (`gh`) is required"); + expect(errorMessage).toContain("No GitHub credential on the server"); }), ); diff --git a/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts b/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts index 33f0df8e8bcbaeec348d454439b08d07e9cab301..d9579f317d95b13c315545b2cf8ea21f75871a99 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts @@ -18,7 +18,6 @@ import * as GitHubGraphQlBudget from "../sourceControl/githubGraphQlBudget.ts"; import * as SourceControlRateLimit from "../sourceControl/SourceControlRateLimit.ts"; import * as GitHubPullRequestCli from "./GitHubPullRequestCli.ts"; import { KnownWorkflowRuns } from "./gitHubConditionalChecks.ts"; -import { BASE_COMPARISON_GRAPHQL_QUERY } from "./gitHubPullRequestJson.ts"; const encodeJson = Schema.encodeSync(Schema.fromJsonString(Schema.Unknown)); @@ -4041,45 +4040,6 @@ layer("GitHubPullRequestCli.layer", (it) => { }), ); - it.effect("sends the base comparison's head as a variable, not inside the document", () => - Effect.gen(function* () { - mockedExecute.mockReturnValue( - Effect.succeed( - output( - encodeJson({ - data: { - repository: { - pullRequest: { - viewerCanUpdateBranch: true, - baseRef: { compare: { behindBy: 4 } }, - }, - }, - }, - }), - ), - ), - ); - const cli = yield* GitHubPullRequestCli.GitHubPullRequestCli; - - const comparison = yield* cli.getPullRequestBaseComparison({ - cwd: "/w", - repository: "acme/web", - host: "github.com", - number: 7, - headRef: "fork:feat/page", - }); - - expect(varsAt(0)).toEqual({ - owner: "acme", - name: "web", - number: 7, - headRef: "fork:feat/page", - }); - expect(comparison).toEqual({ behindBy: 4, viewerCanUpdate: true }); - expect(queryAt(0)).toContain(BASE_COMPARISON_GRAPHQL_QUERY.slice(0, -2)); - }), - ); - it.effect("stops GraphQL reads at the protected reserve until reset", () => Effect.gen(function* () { mockedExecute.mockReturnValue( @@ -4088,10 +4048,11 @@ layer("GitHubPullRequestCli.layer", (it) => { encodeJson({ data: { repository: { - pullRequest: { - viewerCanUpdateBranch: true, - baseRef: { compare: { behindBy: 4 } }, - }, + mergeCommitAllowed: true, + squashMergeAllowed: false, + rebaseMergeAllowed: true, + viewerPermission: "READ", + pullRequest: { viewerCanUpdate: true, viewerDidAuthor: true }, }, rateLimit: { cost: 1, @@ -4110,13 +4071,12 @@ layer("GitHubPullRequestCli.layer", (it) => { repository: "acme/web", host: "github.com", number: 7, - headRef: "fork:feat/page", } as const; - yield* cli.getPullRequestBaseComparison(input); + yield* cli.getViewerAccess(input); expect(queryAt(0)).toContain("rateLimit { cost limit remaining resetAt }"); - const error = yield* Effect.flip(cli.getPullRequestBaseComparison(input)); + const error = yield* Effect.flip(cli.getViewerAccess(input)); assert.strictEqual(error._tag, "SourceControlRateLimitPausedError"); if (error._tag !== "SourceControlRateLimitPausedError") return; @@ -4136,10 +4096,11 @@ layer("GitHubPullRequestCli.layer", (it) => { encodeJson({ data: { repository: { - pullRequest: { - viewerCanUpdateBranch: true, - baseRef: { compare: { behindBy: 4 } }, - }, + mergeCommitAllowed: true, + squashMergeAllowed: false, + rebaseMergeAllowed: true, + viewerPermission: "READ", + pullRequest: { viewerCanUpdate: true, viewerDidAuthor: true }, }, rateLimit: { cost: 1, @@ -4171,12 +4132,11 @@ layer("GitHubPullRequestCli.layer", (it) => { ); const cli = yield* GitHubPullRequestCli.GitHubPullRequestCli; - yield* cli.getPullRequestBaseComparison({ + yield* cli.getViewerAccess({ cwd: "/w", repository: "acme/web", host: "github.com", number: 7, - headRef: "fork:feat/page", }); const access = yield* cli.getViewerAccess({ cwd: "/w", diff --git a/apps/server/src/pullRequest/GitHubPullRequestCli.ts b/apps/server/src/pullRequest/GitHubPullRequestCli.ts index d393db9fa78b8f47f440d896ab5a6fb592f7b2a8..9e98286dc82125c7d6f2502185e972dcb24cf902 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestCli.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestCli.ts @@ -94,8 +94,6 @@ import { decodePullRequestStackMembershipsJson, pullRequestSearchGraphQlQuery, PULL_REQUEST_SEARCH_MAX_ROWS, - BASE_COMPARISON_GRAPHQL_QUERY, - decodeBaseComparisonJson, PULL_REQUEST_FILES_VIEWED_GRAPHQL_QUERY, PULL_REQUEST_NODE_ID_GRAPHQL_QUERY, REACTION_SUBJECT_PULL_REQUEST_GRAPHQL_QUERY, @@ -115,7 +113,6 @@ import { UPDATE_REVIEW_COMMENT_GRAPHQL_MUTATION, VIEWER_PERMISSIONS_GRAPHQL_QUERY, decodeViewerPermissionsJson, - type GitHubBaseComparison, type GitHubPullRequestActivity, type GitHubPullRequestActivityPage, type GitHubWorkflowRunPage, @@ -146,7 +143,7 @@ export class GitHubPullRequestReadError extends Schema.TaggedError Effect.Effect; - /** - * How far the branch trails its base, and whether this viewer may update it. Its own read - * because the comparison needs the head ref the detail answers with — a fork's branch is not - * addressable in the base repository by name alone. - */ - readonly getPullRequestBaseComparison: (input: { - readonly cwd: string; - readonly repository: string; - readonly host: string; - readonly number: number; - /** Qualified `owner:branch`, which is the only form a fork's head resolves under. */ - readonly headRef: string; - /** Manual action checks may use the quota held back from automatic reads. */ - readonly allowReserve?: boolean | undefined; - }) => Effect.Effect; - readonly getPullRequestActivity: (input: { readonly cwd: string; readonly repository: string; @@ -2170,19 +2151,6 @@ export const make = Effect.gen(function* () { ); }, - getPullRequestBaseComparison: (input) => { - const { owner, name } = parseRepositorySelector(input.repository); - return graphqlRead({ - cwd: input.cwd, - host: input.host, - operation: "getPullRequestBaseComparison", - ...(input.allowReserve === true ? { allowReserve: true } : {}), - variables: { owner, name, number: input.number, headRef: input.headRef }, - query: BASE_COMPARISON_GRAPHQL_QUERY, - decode: decodeBaseComparisonJson, - }); - }, - getPullRequestActivity: (input) => Effect.gen(function* () { const { owner, name } = parseRepositorySelector(input.repository); diff --git a/apps/server/src/pullRequest/GitHubPullRequestProvider.test.ts b/apps/server/src/pullRequest/GitHubPullRequestProvider.test.ts index 6905b382688008599be9a4a180bdc7de8775cec3..88079ebb94c7f8ae31e36f906eaaace6d96f2661 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestProvider.test.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestProvider.test.ts @@ -7,7 +7,9 @@ import type { PullRequestReaction } from "@t3tools/contracts"; import { decodePullRequestDetailJson } from "./gitHubPullRequestJson.ts"; import * as GitHubApi from "../sourceControl/GitHubApi.ts"; +import * as GitHubCli from "../sourceControl/GitHubCli.ts"; import * as GitHubPullRequestCli from "./GitHubPullRequestCli.ts"; +import type { GitHubPullRequestCore } from "./gitHubPullRequestJson.ts"; import { gitHubViewerPermissions, loginAvatarUrl, make } from "./GitHubPullRequestProvider.ts"; import type { GitHubReviewThreadComments } from "./gitHubPullRequestJson.ts"; @@ -486,8 +488,6 @@ describe("gitHubViewerPermissions", () => { url: "https://github.com/acme/web/actions/runs/123", }, ]), - getPullRequestBaseComparison: () => - Effect.succeed({ behindBy: 0, viewerCanUpdate: true }), getViewerAccess: () => Effect.succeed({ canWrite: true, @@ -585,7 +585,6 @@ it.effect("does not classify same-repository gates as fork workflow approvals", Layer.mock(GitHubPullRequestCli.GitHubPullRequestCli)({ revalidateChecks: (_input, read) => read, getPullRequestDetail: () => Effect.succeed({ ...openDetail, isCrossRepository: false }), - getPullRequestBaseComparison: () => Effect.succeed({ behindBy: 0, viewerCanUpdate: true }), listWorkflowRunsRequiringApproval: () => Effect.die("same-repository pull requests must not probe fork workflow approvals"), getViewerAccess: () => @@ -625,7 +624,6 @@ it.effect("keeps an unsafe workflow approval scope visible as unknown", () => Layer.mock(GitHubPullRequestCli.GitHubPullRequestCli)({ revalidateChecks: (_input, read) => read, getPullRequestDetail: () => Effect.succeed(openDetail), - getPullRequestBaseComparison: () => Effect.succeed({ behindBy: 0, viewerCanUpdate: true }), listWorkflowRunsRequiringApproval: () => Effect.fail( new GitHubPullRequestCli.GitHubWorkflowApprovalRefusedError({ @@ -669,7 +667,6 @@ it.effect("propagates workflow discovery rate limits", () => Layer.mock(GitHubPullRequestCli.GitHubPullRequestCli)({ revalidateChecks: (_input, read) => read, getPullRequestDetail: () => Effect.succeed(openDetail), - getPullRequestBaseComparison: () => Effect.succeed({ behindBy: 0, viewerCanUpdate: true }), listWorkflowRunsRequiringApproval: () => Effect.fail( new GitHubApi.GitHubApiRateLimitError({ @@ -691,6 +688,14 @@ it.effect("propagates workflow discovery rate limits", () => ); describe("getViewerPermissions", () => { + const access = { + canWrite: true, + canTriage: true, + canUpdate: true, + didAuthor: false, + mergeCapabilities: { merge: true, squash: true, rebase: true }, + }; + it.effect("checks fresh access without reading branch details for unrelated operations", () => { let accessReads = 0; return Effect.gen(function* () { @@ -711,44 +716,27 @@ describe("getViewerPermissions", () => { Layer.mock(GitHubPullRequestCli.GitHubPullRequestCli)({ revalidateChecks: (_input, read) => read, getPullRequestDetail: () => Effect.die("Unexpected detail read"), - getPullRequestBaseComparison: () => Effect.die("Unexpected comparison read"), - getViewerAccess: () => - Effect.sync(() => { - accessReads++; - return { - canWrite: true, - canTriage: true, - canUpdate: true, - didAuthor: false, - mergeCapabilities: { merge: true, squash: true, rebase: true }, - }; - }), + getViewerAccess: () => Effect.sync(() => (accessReads++, access)), }), ), ); }); - const layerWithComparison = ( - comparison: Effect.Effect<{ - readonly behindBy: number | null; - readonly viewerCanUpdate: boolean; - }>, + const layerWithDetail = ( + detail: Effect.Effect, + onDetail: (allowReserve: boolean) => void = () => {}, ) => Layer.mock(GitHubPullRequestCli.GitHubPullRequestCli)({ revalidateChecks: (_input, read) => read, - getPullRequestDetail: () => Effect.succeed(openDetail), - getPullRequestBaseComparison: () => comparison, - getViewerAccess: () => - Effect.succeed({ - canWrite: true, - canTriage: true, - canUpdate: true, - didAuthor: false, - mergeCapabilities: { merge: true, squash: true, rebase: true }, - }), + getPullRequestDetail: () => + GitHubCli.AllowGitHubReserve.pipe( + Effect.tap((allowReserve) => Effect.sync(() => onDetail(allowReserve))), + Effect.flatMap(() => detail), + ), + getViewerAccess: () => Effect.die("Unexpected access read"), }); - it.effect("offers update-branch when the comparison grants it", () => + it.effect("answers access and update-branch from the one detail read", () => Effect.gen(function* () { const provider = yield* make; const permissions = yield* provider.getViewerPermissions({ @@ -758,16 +746,20 @@ describe("getViewerPermissions", () => { number: 7, }); + expect(permissions.actions).toContain("merge"); expect(permissions.actions).toContain("update-branch"); expect(permissions.updateMethods).toEqual(["merge", "rebase"]); }).pipe( - Effect.provide(layerWithComparison(Effect.succeed({ behindBy: 3, viewerCanUpdate: true }))), + Effect.provide( + layerWithDetail( + Effect.succeed({ ...openDetail, comparison: { behindBy: 3, viewerCanUpdate: true } }), + ), + ), ), ); it.effect("uses the GraphQL reserve for manual permission checks", () => { - let viewerAllowReserve: boolean | undefined; - let comparisonAllowReserve: boolean | undefined; + let allowed: boolean | undefined; return Effect.gen(function* () { const provider = yield* make; yield* provider.getViewerPermissions({ @@ -776,36 +768,45 @@ describe("getViewerPermissions", () => { host: "github.com", number: 7, }); + expect(allowed).toBe(true); + }).pipe( + Effect.provide( + layerWithDetail(Effect.succeed(openDetail), (allowReserve) => (allowed = allowReserve)), + ), + ); + }); - expect(viewerAllowReserve).toBe(true); - expect(comparisonAllowReserve).toBe(true); + it.effect("still answers from the light access read when the detail read fails", () => + Effect.gen(function* () { + const provider = yield* make; + const permissions = yield* provider.getViewerPermissions({ + cwd: "/w", + repository: "acme/web", + host: "github.com", + number: 7, + }); + expect(permissions.actions).toContain("merge"); + expect(permissions.actions).not.toContain("update-branch"); }).pipe( Effect.provide( Layer.mock(GitHubPullRequestCli.GitHubPullRequestCli)({ revalidateChecks: (_input, read) => read, - getPullRequestDetail: () => Effect.succeed(openDetail), - getPullRequestBaseComparison: (input) => - Effect.sync(() => { - comparisonAllowReserve = input.allowReserve; - return { behindBy: 3, viewerCanUpdate: true }; - }), - getViewerAccess: (input) => - Effect.sync(() => { - viewerAllowReserve = input.allowReserve; - return { - canWrite: true, - canTriage: true, - canUpdate: true, - didAuthor: false, - mergeCapabilities: { merge: true, squash: true, rebase: true }, - }; - }), + getPullRequestDetail: () => + Effect.fail( + new GitHubPullRequestCli.GitHubPullRequestReadError({ + command: "gh", + cwd: "/w", + operation: "getPullRequestDetail", + cause: new Error("head changed"), + }), + ), + getViewerAccess: () => Effect.succeed(access), }), ), - ); - }); + ), + ); - it.effect("withholds update-branch when the comparison cannot be read", () => + it.effect("withholds update-branch where GitHub could not compare the branch", () => Effect.gen(function* () { const provider = yield* make; const permissions = yield* provider.getViewerPermissions({ @@ -817,33 +818,8 @@ describe("getViewerPermissions", () => { expect(permissions.actions).not.toContain("update-branch"); expect(permissions.updateMethods).toBeUndefined(); - // The rest of the answer survives a comparison nobody could make. expect(permissions.actions).toContain("merge"); - }).pipe( - Effect.provide( - Layer.mock(GitHubPullRequestCli.GitHubPullRequestCli)({ - revalidateChecks: (_input, read) => read, - getPullRequestDetail: () => Effect.succeed(openDetail), - getPullRequestBaseComparison: () => - Effect.fail( - new GitHubPullRequestCli.GitHubPullRequestReadError({ - command: "gh", - cwd: "/w", - operation: "getPullRequestBaseComparison", - cause: new Error("unreadable"), - }), - ), - getViewerAccess: () => - Effect.succeed({ - canWrite: true, - canTriage: true, - canUpdate: true, - didAuthor: false, - mergeCapabilities: { merge: true, squash: true, rebase: true }, - }), - }), - ), - ), + }).pipe(Effect.provide(layerWithDetail(Effect.succeed({ ...openDetail, comparison: null })))), ); }); diff --git a/apps/server/src/pullRequest/GitHubPullRequestProvider.ts b/apps/server/src/pullRequest/GitHubPullRequestProvider.ts index 940b04715c84ea3dcde9f5ac3f14ed6a75f2d2b4..74c5852a48a49eb331fd92d93334e3fb78895ac5 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestProvider.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestProvider.ts @@ -7,6 +7,7 @@ import type { PullRequestViewerPermissions, } from "@t3tools/contracts"; +import * as GitHubCli from "../sourceControl/GitHubCli.ts"; import * as GitHubPullRequestCli from "./GitHubPullRequestCli.ts"; import { PullRequestProviderError, @@ -109,6 +110,7 @@ export function gitHubProviderFailure( case "GitHubCliMissingError": return { reason: "missing-tool" }; case "GitHubNotSignedInError": + case "GitHubHostDisabledError": case "GitHubApiAuthenticationError": return { reason: "unauthenticated" }; case "GitHubApiRateLimitError": @@ -513,38 +515,34 @@ export const make = Effect.gen(function* () { getReviewThreadComments: (input) => cli.getReviewThreadComments(input).pipe(Effect.mapError(fail("getReviewThreadComments"))), - getViewerPermissions: (input) => - Effect.all( - [ - cli.getViewerAccess({ ...input, allowReserve: true }), - // Whether this viewer may update the branch is only on the comparison, and the - // comparison only resolves through the head ref the detail carries. A failure here - // withholds that one action rather than the whole answer, the way the detail path - // leaves the banner unknown. - input.includeUpdateBranch === false - ? Effect.succeed(false) - : cli.getPullRequestDetail(input).pipe( - Effect.flatMap((pullRequest) => - pullRequest.state !== "open" || pullRequest.headRepositoryOwner === null - ? Effect.succeed(false) - : cli - .getPullRequestBaseComparison({ - ...input, - headRef: `${pullRequest.headRepositoryOwner}:${pullRequest.headBranch}`, - allowReserve: true, - }) - .pipe(Effect.map((comparison) => comparison.viewerCanUpdate === true)), - ), - Effect.orElseSucceed(() => false), - ), - ], - { concurrency: 2 }, - ).pipe( + getViewerPermissions: (input) => { + const lightAccess = cli + .getViewerAccess({ ...input, allowReserve: true }) + .pipe(Effect.map((access) => gitHubViewerPermissions(access))); + if (input.includeUpdateBranch === false) { + return lightAccess.pipe(Effect.mapError(fail("getViewerPermissions"))); + } + // The core detail already carries the viewer's access, the merge settings, and the base + // comparison, so one read usually answers what used to take three. When that heavier read + // fails, the light access read still answers, withholding only update-branch. + return cli.getPullRequestDetail(input).pipe( + Effect.provideService(GitHubCli.AllowGitHubReserve, true), + Effect.map((pullRequest) => + gitHubViewerPermissions({ + ...pullRequest.viewerAccess, + canUpdateBranch: + pullRequest.state === "open" && pullRequest.comparison?.viewerCanUpdate === true, + }), + ), + Effect.catchIf( + (error) => + error._tag !== "GitHubApiRateLimitError" && + error._tag !== "SourceControlRateLimitPausedError", + () => lightAccess, + ), Effect.mapError(fail("getViewerPermissions")), - Effect.map(([access, canUpdateBranch]) => - gitHubViewerPermissions({ ...access, canUpdateBranch }), - ), - ), + ); + }, getDiff: (input) => cli.getPullRequestDiff(input).pipe(Effect.mapError(fail("getDiff"))), diff --git a/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts b/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts index 2e008fe4c6365ab531e6ec0039e81128b1cf68e5..3105634dc0875deb78ef6d2ba9a5ce74bfee60c3 100644 --- a/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts +++ b/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts @@ -11,7 +11,6 @@ import { decodePullRequestStackMembershipsJson, buildReviewerRequest, buildSetFilesViewedGraphQlMutation, - decodeBaseComparisonJson, decodePullRequestActivityJson, decodePullRequestDetailJson, decodePullRequestFilesJson, @@ -1696,48 +1695,6 @@ describe("decodePullRequestFilesJson", () => { }); }); -describe("how far a branch trails its base", () => { - const comparison = (pullRequest: unknown) => - JSON.stringify({ data: { repository: { pullRequest } } }); - - it("reads the commit count and whether this viewer may move the branch", () => { - const decoded = expectSuccess( - decodeBaseComparisonJson( - comparison({ viewerCanUpdateBranch: true, baseRef: { compare: { behindBy: 12 } } }), - ), - ); - expect(decoded).toEqual({ behindBy: 12, viewerCanUpdate: true }); - }); - - it("reads a current branch as nothing to do", () => { - expect( - expectSuccess( - decodeBaseComparisonJson( - comparison({ viewerCanUpdateBranch: false, baseRef: { compare: { behindBy: 0 } } }), - ), - ), - ).toEqual({ behindBy: 0, viewerCanUpdate: false }); - }); - - it("answers unknown where the head could not be compared", () => { - // A pull request from a fork whose repository is gone, which GitHub answers with a null - // comparison beside a perfectly good pull request. - expect( - expectSuccess( - decodeBaseComparisonJson(comparison({ viewerCanUpdateBranch: true, baseRef: null })), - ).behindBy, - ).toBeNull(); - expect(expectSuccess(decodeBaseComparisonJson(comparison(null)))).toEqual({ - behindBy: null, - viewerCanUpdate: false, - }); - }); - - it("refuses a body that is not the answer to this question", () => { - expect(Result.isSuccess(decodeBaseComparisonJson("{"))).toBe(false); - }); -}); - describe("decodePullRequestFilesViewedJson", () => { const page = ( nodes: ReadonlyArray, diff --git a/apps/server/src/pullRequest/gitHubPullRequestJson.ts b/apps/server/src/pullRequest/gitHubPullRequestJson.ts index 1409e92a1fadf809b683214dcdcb0a5313365a1e..ce89cf6fe44a27842b4ec0263dae1baf7c1e8b8a 100644 --- a/apps/server/src/pullRequest/gitHubPullRequestJson.ts +++ b/apps/server/src/pullRequest/gitHubPullRequestJson.ts @@ -2892,75 +2892,12 @@ function toCanTriage(viewerPermission: string | null | undefined): boolean { * need `read:org`, which a repository-scoped token need not carry — and a query GitHub refuses * fails whole, taking the people down with the teams. */ -/** - * Where the branch stands against its base, and whether this viewer may move it. - * - * `mergeStateStatus` is not the answer: GitHub only reports BEHIND where the repository requires - * branches to be up to date before merging, so on every other repository a stale branch reads as - * CLEAN or BLOCKED like any other. The comparison counts the commits instead, which is the same - * number GitHub's own "out-of-date" banner shows. - * - * `headRef` is qualified `owner:branch` because a pull request from a fork has no branch of that - * name in the base repository, and an unqualified name is simply not found there. - */ -export const BASE_COMPARISON_GRAPHQL_QUERY = `query($owner: String!, $name: String!, $number: Int!, $headRef: String!) { - repository(owner: $owner, name: $name) { - pullRequest(number: $number) { - viewerCanUpdateBranch - baseRef { - compare(headRef: $headRef) { - behindBy - } - } - } - } -}`; - -const RawBaseComparisonSchema = Schema.Struct({ - data: Schema.Struct({ - repository: Schema.NullOr( - Schema.Struct({ - pullRequest: Schema.NullOr( - Schema.Struct({ - viewerCanUpdateBranch: Schema.optional(Schema.NullOr(Schema.Boolean)), - /** Null where the head repository is gone, which is a comparison nobody can make. */ - baseRef: Schema.optional( - Schema.NullOr( - Schema.Struct({ - compare: Schema.optional( - Schema.NullOr(Schema.Struct({ behindBy: Schema.Number })), - ), - }), - ), - ), - }), - ), - }), - ), - }), -}); - -const decodeBaseComparison = decodeJsonResult(RawBaseComparisonSchema); - export interface GitHubBaseComparison { /** Null where the host could not compare, which the page reads as "unknown". */ readonly behindBy: number | null; readonly viewerCanUpdate: boolean; } -export function decodeBaseComparisonJson( - raw: string, -): Result.Result { - const decoded = decodeBaseComparison(raw); - if (!Result.isSuccess(decoded)) return Result.fail(decoded.failure); - const pullRequest = decoded.success.data.repository?.pullRequest; - const behindBy = pullRequest?.baseRef?.compare?.behindBy; - return Result.succeed({ - behindBy: typeof behindBy === "number" && behindBy >= 0 ? behindBy : null, - viewerCanUpdate: pullRequest?.viewerCanUpdateBranch === true, - }); -} - export const REVIEWER_CANDIDATES_GRAPHQL_QUERY = `query($owner: String!, $name: String!, $number: Int!) { repository(owner: $owner, name: $name) { assignableUsers(first: ${GRAPHQL_PAGE_SIZE}) { diff --git a/apps/server/src/serverSettings.test.ts b/apps/server/src/serverSettings.test.ts index 4376d5d7ea69b9c07f99f8922d15921c6c884249..59f188d7d1b585c47cae3a59c11ef2de4199fe85 100644 --- a/apps/server/src/serverSettings.test.ts +++ b/apps/server/src/serverSettings.test.ts @@ -1588,6 +1588,54 @@ it.layer(NodeServices.layer)("server settings", (it) => { }).pipe(Effect.provide(layerServerSettingsWithSecrets())), ); + it.effect( + "keeps GitHub tokens per host in the secret store and tells clients only that one is set", + () => + Effect.gen(function* () { + const serverSettings = yield* ServerSettingsModule.ServerSettingsService; + const secrets = yield* ServerSecretStore.ServerSecretStore; + const serverConfig = yield* ServerConfig.ServerConfig; + const fileSystem = yield* FileSystem.FileSystem; + + const saved = yield* serverSettings.updateSettings({ + github: { tokens: { "GitHub.com": "ghp_dotcom", "ghe.acme.test": "ghp_ghe" } }, + }); + assert.deepEqual(saved.github.tokens, { + "github.com": "ghp_dotcom", + "ghe.acme.test": "ghp_ghe", + }); + const raw = yield* fileSystem.readFileString(serverConfig.settingsPath); + assert.notInclude(raw, "ghp_dotcom"); + assert.notInclude(raw, "ghp_ghe"); + + const forClient = ServerSettingsModule.redactServerSettingsForClient(saved).github; + assert.notInclude(forClient.tokens["github.com"]!, "ghp_dotcom"); + assert.isAbove(forClient.tokens["github.com"]!.length, 0); + + // Echoing the redacted values back keeps them; host and account changes leave tokens alone. + yield* serverSettings.updateSettings({ github: { tokens: forClient.tokens } }); + yield* serverSettings.updateSettings({ + github: { hosts: { "github.com": { enabled: true, account: "work" } } }, + }); + assert.deepEqual((yield* serverSettings.getSettings).github.tokens, { + "github.com": "ghp_dotcom", + "ghe.acme.test": "ghp_ghe", + }); + + // An empty token removes that host's token and nothing else. + const cleared = yield* serverSettings.updateSettings({ + github: { tokens: { "github.com": "" } }, + }); + assert.equal(cleared.github.tokens["github.com"] ?? "", ""); + assert.equal(cleared.github.tokens["ghe.acme.test"], "ghp_ghe"); + const remaining = yield* Effect.forEach(["github.com", "ghe.acme.test"], (host) => + secrets.get(`github-token-${Buffer.from(host, "utf8").toString("base64url")}`), + ); + assert.isTrue(Option.isNone(remaining[0]!)); + assert.isTrue(Option.isSome(remaining[1]!)); + }).pipe(Effect.provide(layerServerSettingsWithSecrets())), + ); + it.effect("removes a Bitbucket secret once its token is cleared by hand in settings.json", () => Effect.gen(function* () { const serverConfig = yield* ServerConfig.ServerConfig; diff --git a/apps/server/src/serverSettings.ts b/apps/server/src/serverSettings.ts index 6452c8d775a2936799f2ff173decf83de63dfe3c..7cbcd895ac140f5979a3d578ff7718f3efb9c3af 100644 --- a/apps/server/src/serverSettings.ts +++ b/apps/server/src/serverSettings.ts @@ -161,6 +161,11 @@ const BITBUCKET_SECRET_NAMES = { } as const; const BITBUCKET_SECRET_FIELDS = ["accessToken", "apiToken"] as const; +/** Hosts are case-insensitive; a patch can arrive before decoding lowercased its keys. */ +function gitHubTokenSecretName(host: string): string { + return `github-token-${Buffer.from(host.trim().toLowerCase(), "utf8").toString("base64url")}`; +} + const redactSecret = (value: string) => (value.length > 0 ? SECRET_REDACTED : ""); function redactProviderEnvironmentVariable( @@ -204,7 +209,13 @@ export function redactServerSettingsForClient(settings: ServerSettings): ServerS accessToken: redactSecret(settings.bitbucket.accessToken), apiToken: redactSecret(settings.bitbucket.apiToken), }; - return { ...settings, providerInstances, usageLimitSources, bitbucket }; + const github = { + ...settings.github, + tokens: Object.fromEntries( + Object.entries(settings.github.tokens).map(([host, token]) => [host, redactSecret(token)]), + ), + }; + return { ...settings, providerInstances, usageLimitSources, bitbucket, github }; } export function applyProviderInstanceMutation( @@ -707,7 +718,24 @@ const make = Effect.gen(function* () { bitbucket[field] = SECRET_REDACTED; moved = true; } - return moved ? { ...settings, bitbucket } : settings; + const tokens = { ...settings.github.tokens }; + for (const [host, value] of Object.entries(tokens)) { + if (value.length === 0 || value === SECRET_REDACTED) continue; + const stored = yield* secretStore + .set(gitHubTokenSecretName(host), textEncoder.encode(value)) + .pipe( + Effect.as(true), + Effect.catch(() => + Effect.logWarning("failed to move a GitHub token into the secret store", { + host, + }).pipe(Effect.as(false)), + ), + ); + if (!stored) continue; + tokens[host] = SECRET_REDACTED; + moved = true; + } + return moved ? { ...settings, bitbucket, github: { ...settings.github, tokens } } : settings; }); const loadSettingsFromDisk = Effect.gen(function* () { @@ -882,11 +910,27 @@ const make = Effect.gen(function* () { ); bitbucket[field] = Option.isSome(secret) ? textDecoder.decode(secret.value) : ""; } + const tokens: Record = {}; + for (const [host, value] of Object.entries(settings.github.tokens)) { + if (value !== SECRET_REDACTED) { + tokens[host] = value; + continue; + } + const secret = yield* secretStore + .get(gitHubTokenSecretName(host)) + .pipe( + Effect.mapError( + (cause) => new ServerSettingsError({ settingsPath, operation: "read-secret", cause }), + ), + ); + tokens[host] = Option.isSome(secret) ? textDecoder.decode(secret.value) : ""; + } return { ...settings, providerInstances: providerInstances as ServerSettings["providerInstances"], usageLimitSources: usageLimitSources as ServerSettings["usageLimitSources"], bitbucket, + github: { ...settings.github, tokens }, }; }); @@ -1047,12 +1091,46 @@ const make = Effect.gen(function* () { bitbucket[field] = SECRET_REDACTED; } + const tokens: Record = {}; + for (const [rawHost, raw] of Object.entries(next.github.tokens)) { + const host = rawHost.trim().toLowerCase(); + let value = raw; + if (value === SECRET_REDACTED) { + // The marker keeps what is saved; a hand-edited plaintext token moves into the store. + const inline = current.github.tokens[host]; + if (inline === undefined || inline === SECRET_REDACTED || inline.length === 0) { + tokens[host] = SECRET_REDACTED; + continue; + } + value = inline; + } + const secretName = gitHubTokenSecretName(host); + if (value.length === 0) { + changes.push({ kind: "remove", secretName, operation: "remove-secret" }); + continue; + } + changes.push({ kind: "write", secretName, value: textEncoder.encode(value) }); + tokens[host] = SECRET_REDACTED; + } + const nextHosts = new Set( + Object.keys(next.github.tokens).map((host) => host.trim().toLowerCase()), + ); + for (const host of Object.keys(current.github.tokens)) { + if (nextHosts.has(host.trim().toLowerCase())) continue; + changes.push({ + kind: "remove", + secretName: gitHubTokenSecretName(host), + operation: "remove-stale-secret", + }); + } + return { settings: { ...next, providerInstances: providerInstances as ServerSettings["providerInstances"], usageLimitSources: usageLimitSources as ServerSettings["usageLimitSources"], bitbucket, + github: { ...next.github, tokens }, }, changes, }; diff --git a/apps/server/src/sourceControl/GitHubApi.test.ts b/apps/server/src/sourceControl/GitHubApi.test.ts index 85fe9a2dcfbe184837cffb9b8bddef72d996ebe5..a5c0f82be44311b9cee5d1114c477a66769e0cc8 100644 --- a/apps/server/src/sourceControl/GitHubApi.test.ts +++ b/apps/server/src/sourceControl/GitHubApi.test.ts @@ -15,6 +15,7 @@ import * as GitHubCredentials from "./GitHubCredentials.ts"; import * as GitHubGraphQlBudget from "./githubGraphQlBudget.ts"; import * as SourceControlRateLimit from "./SourceControlRateLimit.ts"; import * as VcsProcess from "../vcs/VcsProcess.ts"; +import * as ServerSettings from "../serverSettings.ts"; const NOW = Date.parse("2026-10-05T12:00:00.000Z"); @@ -416,6 +417,7 @@ describe("GitHubCredentials", () => { GitHubCredentials.layer.pipe( Layer.provide(Layer.mock(VcsProcess.VcsProcess)({ run })), Layer.provide(NodeServices.layer), + Layer.provide(ServerSettings.layerTest()), ); it.effect("fails with GitHubCliMissingError when gh is not on PATH", () => diff --git a/apps/server/src/sourceControl/GitHubCli.ts b/apps/server/src/sourceControl/GitHubCli.ts index 95501d3c163a9245b0be07fd49dd3fa8f0ae3b5e..f4463be6de831cfaddd8b7042c56c40d51994595 100644 --- a/apps/server/src/sourceControl/GitHubCli.ts +++ b/apps/server/src/sourceControl/GitHubCli.ts @@ -44,7 +44,7 @@ export class GitHubCliUnavailableError extends Schema.TaggedError = {}, + signedOut: ReadonlyArray = [], + tokens: Record = {}, +) { + const calls: Array> = []; + const process = Layer.mock(VcsProcess.VcsProcess)({ + run: (input) => + Effect.sync(() => { + calls.push(input.args); + const user = input.args[input.args.indexOf("--user") + 1]; + if (input.args.includes("--user") && user !== undefined && signedOut.includes(user)) { + return { + exitCode: ChildProcessSpawner.ExitCode(0), + stdout: "", + stderr: "", + stdoutTruncated: false, + stderrTruncated: false, + }; + } + return { + exitCode: ChildProcessSpawner.ExitCode(0), + stdout: input.args.includes("--user") ? `token-for-${user}\n` : "active-token\n", + stderr: "", + stdoutTruncated: false, + stderrTruncated: false, + }; + }), + }); + const layer = GitHubCredentials.layer.pipe( + Layer.provideMerge( + ServerSettings.ServerSettingsService.layerTest({ github: { hosts, tokens } }), + ), + Layer.provide(process), + Layer.provide(NodeServices.layer), + ); + return { layer, calls }; +} + +describe("GitHubCredentials", () => { + beforeEach(() => { + for (const name of TOKEN_VARIABLES) vi.stubEnv(name, ""); + }); + afterEach(() => { + vi.unstubAllEnvs(); + }); + + it.effect("asks gh for the active login when Settings pin nothing", () => { + const { layer, calls } = harness(); + return Effect.gen(function* () { + const credentials = yield* GitHubCredentials.GitHubCredentials; + const credential = yield* credentials.get("GitHub.com"); + expect(Redacted.value(credential.token)).toBe("active-token"); + expect(calls).toEqual([["auth", "token", "--hostname", "github.com"]]); + }).pipe(Effect.provide(layer)); + }); + + it.effect("passes --user for an account pinned in Settings", () => { + const { layer, calls } = harness({ "github.com": { account: "work" } }); + return Effect.gen(function* () { + const credentials = yield* GitHubCredentials.GitHubCredentials; + const credential = yield* credentials.get("github.com"); + expect(Redacted.value(credential.token)).toBe("token-for-work"); + expect(calls).toEqual([["auth", "token", "--hostname", "github.com", "--user", "work"]]); + }).pipe(Effect.provide(layer)); + }); + + it.effect("fails a host turned off in Settings without asking gh", () => { + const { layer, calls } = harness({ "github.com": { enabled: false } }); + return Effect.gen(function* () { + const credentials = yield* GitHubCredentials.GitHubCredentials; + const error = yield* Effect.flip(credentials.get("github.com")); + expect(error._tag).toBe("GitHubHostDisabledError"); + expect(error.message).toBe( + "GitHub host github.com is turned off in Settings → Source Control.", + ); + expect(calls).toEqual([]); + }).pipe(Effect.provide(layer)); + }); + + it.effect("lets an environment token win over a pinned account, as gh does", () => { + vi.stubEnv("GH_TOKEN", "from-env"); + const { layer, calls } = harness({ "github.com": { account: "work" } }); + return Effect.gen(function* () { + const credentials = yield* GitHubCredentials.GitHubCredentials; + const credential = yield* credentials.get("github.com"); + expect(Redacted.value(credential.token)).toBe("from-env"); + expect(credential.source).toBe("env"); + expect(calls).toEqual([]); + }).pipe(Effect.provide(layer)); + }); + + it.effect("picks up a changed account on the next request without a restart", () => { + const { layer, calls } = harness(); + return Effect.gen(function* () { + const credentials = yield* GitHubCredentials.GitHubCredentials; + const settings = yield* ServerSettings.ServerSettingsService; + expect(Redacted.value((yield* credentials.get("github.com")).token)).toBe("active-token"); + + yield* settings.updateSettings({ + github: { hosts: { "github.com": { account: "work", enabled: true } } }, + }); + expect(Redacted.value((yield* credentials.get("github.com")).token)).toBe("token-for-work"); + + yield* settings.updateSettings({ github: { hosts: { "github.com": { enabled: false } } } }); + expect((yield* Effect.flip(credentials.get("github.com")))._tag).toBe( + "GitHubHostDisabledError", + ); + + yield* settings.updateSettings({ github: { hosts: {} } }); + expect(Redacted.value((yield* credentials.get("github.com")).token)).toBe("active-token"); + // The unpinned token stayed cached; only the newly pinned account cost a gh call. + expect(calls).toHaveLength(2); + }).pipe(Effect.provide(layer)); + }); + + it.effect("falls back to the active login when the pinned one is no longer signed in", () => { + const { layer, calls } = harness({ "github.com": { account: "gone" } }, ["gone"]); + return Effect.gen(function* () { + const credentials = yield* GitHubCredentials.GitHubCredentials; + expect(Redacted.value((yield* credentials.get("github.com")).token)).toBe("active-token"); + expect(calls).toEqual([ + ["auth", "token", "--hostname", "github.com", "--user", "gone"], + ["auth", "token", "--hostname", "github.com"], + ]); + }).pipe(Effect.provide(layer)); + }); + + it.effect("uses a token saved in Settings before GH_TOKEN and gh", () => { + vi.stubEnv("GH_TOKEN", "env-token"); + const { layer, calls } = harness({}, [], { "github.com": "saved-token" }); + return Effect.gen(function* () { + const credentials = yield* GitHubCredentials.GitHubCredentials; + const credential = yield* credentials.get("github.com"); + expect(Redacted.value(credential.token)).toBe("saved-token"); + expect(credential.source).toBe("settings"); + expect(calls).toEqual([]); + }).pipe(Effect.provide(layer)); + }); + + it.effect("keeps a host turned off even with a token saved in Settings", () => { + const { layer } = harness({ "github.com": { enabled: false } }, [], { + "github.com": "saved-token", + }); + return Effect.gen(function* () { + const credentials = yield* GitHubCredentials.GitHubCredentials; + expect((yield* Effect.flip(credentials.get("github.com")))._tag).toBe( + "GitHubHostDisabledError", + ); + }).pipe(Effect.provide(layer)); + }); +}); diff --git a/apps/server/src/sourceControl/GitHubCredentials.ts b/apps/server/src/sourceControl/GitHubCredentials.ts index 7207373bcf313357129012ba135286274b82e358..fb043d809c6846ada3409d045dd8ee43d656bea6 100644 --- a/apps/server/src/sourceControl/GitHubCredentials.ts +++ b/apps/server/src/sourceControl/GitHubCredentials.ts @@ -13,6 +13,7 @@ import * as Schema from "effect/Schema"; import { HostProcessEnvironment, HostProcessWorkingDirectory } from "@t3tools/shared/hostProcess"; +import * as ServerSettings from "../serverSettings.ts"; import * as VcsProcess from "../vcs/VcsProcess.ts"; /** How long a token is reused before `gh` is asked again, so a `gh auth switch` applies soon. */ @@ -20,7 +21,7 @@ const TOKEN_TTL = Duration.minutes(5); /** No credential is retried sooner, so a fresh `gh auth login` takes effect on the next read. */ const MISSING_TTL = Duration.seconds(10); -export const GitHubCredentialSource = Schema.Literals(["env", "gh"]); +export const GitHubCredentialSource = Schema.Literals(["settings", "env", "gh"]); export type GitHubCredentialSource = typeof GitHubCredentialSource.Type; export interface GitHubCredential { @@ -41,13 +42,25 @@ export class GitHubCliMissingError extends Schema.TaggedError()( "GitHubNotSignedInError", + { host: Schema.String, account: Schema.optional(Schema.String) }, +) { + override get message(): string { + return this.account === undefined + ? `No GitHub credential for ${this.host}: run \`gh auth login --hostname ${this.host}\`.` + : `No GitHub credential for ${this.account} on ${this.host}: run \`gh auth login --hostname ${this.host}\` for that account or pick another in Settings → Source Control.`; + } +} + +/** The user turned the host off in Settings. */ +export class GitHubHostDisabledError extends Schema.TaggedError()( + "GitHubHostDisabledError", { host: Schema.String }, ) { override get message(): string { - return `No GitHub credential for ${this.host}: run \`gh auth login --hostname ${this.host}\`.`; + return `GitHub host ${this.host} is turned off in Settings → Source Control.`; } } @@ -65,8 +78,13 @@ export class GitHubCliFailedError extends Schema.TaggedError + const fromGh = (host: string, account: string | undefined) => process .run({ operation: "GitHubCredentials.get", command: "gh", - args: ["auth", "token", "--hostname", host], + args: [ + "auth", + "token", + "--hostname", + host, + ...(account === undefined ? [] : ["--user", account]), + ], cwd: workingDirectory, // Never let gh print the token into a debug log. env: { GH_DEBUG: "", GH_PROMPT_DISABLED: "1" }, @@ -146,19 +171,48 @@ export const make = Effect.gen(function* () { ? new GitHubCliMissingError({ host }) : // gh exits non-zero with "no oauth token" when it has no login for the host. error._tag === "VcsProcessExitError" - ? new GitHubNotSignedInError({ host }) + ? new GitHubNotSignedInError({ host, ...(account === undefined ? {} : { account }) }) : new GitHubCliFailedError({ host, cause: error }), ), Effect.map((output) => output.stdout.trim()), Effect.filterOrFail( (token) => token !== "", - () => new GitHubNotSignedInError({ host }), + () => new GitHubNotSignedInError({ host, ...(account === undefined ? {} : { account }) }), ), ); - const lookup = Effect.fn("GitHubCredentials.lookup")(function* (host: string) { + /** The Settings choice for a host; unreadable settings fall back to gh's own choice. */ + const hostChoice = (host: string) => + serverSettings.getSettings.pipe( + Effect.map((settings) => settings.github.hosts[host]), + Effect.orElseSucceed(() => undefined), + ); + + /** A token saved in Settings for the host, read fresh so a saved or removed one applies at once. */ + const savedToken = (host: string) => + serverSettings.getSettings.pipe( + Effect.map((settings) => settings.github.tokens[host]?.trim() || null), + Effect.orElseSucceed(() => null), + ); + + /** Cache key: the host plus its pinned account, so a changed pin misses the cache. */ + const cacheKey = (host: string, account: string | undefined) => + account === undefined ? host : `${host}\u0000${account}`; + + const lookup = Effect.fn("GitHubCredentials.lookup")(function* (key: string) { + const [host = key, choice] = key.split("\u0000"); + // An environment token wins over a pinned account, exactly as it does in gh. const fromEnv = environmentToken(host, environment); - const token = fromEnv ?? (yield* fromGh(host)); + // A pinned login gh no longer holds (logged out, expired) falls back to the active one, + // which is what discovery reports as the account in use. + const token = + fromEnv ?? + (yield* fromGh(host, choice).pipe( + Effect.catchTags({ + GitHubNotSignedInError: (error) => + choice === undefined ? Effect.fail(error) : fromGh(host, undefined), + }), + )); return { host, token: Redacted.make(token), @@ -181,8 +235,31 @@ export const make = Effect.gen(function* () { }); return GitHubCredentials.of({ - get: (host) => Cache.get(cache, normalizeHost(host)), - invalidate: (host) => Cache.invalidate(cache, normalizeHost(host)), + get: Effect.fn("GitHubCredentials.get")(function* (rawHost) { + const host = normalizeHost(rawHost); + const choice = yield* hostChoice(host); + if (choice?.enabled === false) { + return yield* new GitHubHostDisabledError({ host }); + } + // A token saved in Settings is the most deliberate choice, so it comes before the + // environment and gh. It is read from the secret store each time, so it needs no cache. + const saved = yield* savedToken(host); + if (saved !== null) { + return { + host, + token: Redacted.make(saved), + source: "settings", + fingerprint: yield* fingerprintOf(host, saved), + } satisfies GitHubCredential; + } + return yield* Cache.get(cache, cacheKey(host, choice?.account)); + }), + invalidate: (rawHost) => { + const host = normalizeHost(rawHost); + return hostChoice(host).pipe( + Effect.flatMap((choice) => Cache.invalidate(cache, cacheKey(host, choice?.account))), + ); + }, }); }); diff --git a/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts b/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts index 6376e2d868e552087041cf9b12d896019e4cab13..ea6624aef89242fba2766d6c42535ac8cc060385 100644 --- a/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts +++ b/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts @@ -329,6 +329,7 @@ it("parses GitHub auth status accounts by host and active state", () => { authenticated: true, active: true, error: null, + environmentVariable: null, }, { host: "github.com", @@ -336,6 +337,7 @@ it("parses GitHub auth status accounts by host and active state", () => { authenticated: false, active: false, error: null, + environmentVariable: null, }, { host: "github.example.test", @@ -343,6 +345,7 @@ it("parses GitHub auth status accounts by host and active state", () => { authenticated: true, active: false, error: null, + environmentVariable: null, }, ], ); @@ -466,3 +469,75 @@ it.effect.each(["read", "decode"] as const)( else assert.propertyVal(error.cause, "_tag", "SchemaError"); }), ); + +const multiAccountStatus = (extra: ReadonlyArray> = []) => + processResult( + JSON.stringify({ + hosts: { + "github.com": [ + { state: "success", active: true, host: "github.com", login: "personal" }, + { state: "success", active: false, host: "github.com", login: "work" }, + ...extra, + ], + "ghe.acme.test": [ + { state: "error", active: true, host: "ghe.acme.test", login: "jm", error: "expired" }, + ], + }, + }), + ); + +it("reports every gh login and leads with the account Settings pin", () => { + const auth = GitHubSourceControlProvider.parseGitHubAuth(multiAccountStatus(), { + hosts: { "github.com": { account: "work", enabled: true } }, + tokens: {}, + }); + assert.deepStrictEqual(auth.account, Option.some("work")); + assert.deepStrictEqual(auth.accounts, [ + { host: "github.com", account: "personal", active: true, authenticated: true }, + { host: "github.com", account: "work", active: false, authenticated: true }, + { host: "ghe.acme.test", account: "jm", active: true, authenticated: false, error: "expired" }, + ]); +}); + +it("falls back to gh's active login when the pinned account is gone", () => { + const auth = GitHubSourceControlProvider.parseGitHubAuth(multiAccountStatus(), { + hosts: { "github.com": { account: "former-job", enabled: true } }, + tokens: {}, + }); + assert.deepStrictEqual(auth.account, Option.some("personal")); +}); + +it("reports unauthenticated when Settings turn off every signed-in host", () => { + const auth = GitHubSourceControlProvider.parseGitHubAuth(multiAccountStatus(), { + hosts: { "github.com": { enabled: false } }, + tokens: {}, + }); + assert.strictEqual(auth.status, "unauthenticated"); + assert.deepStrictEqual( + auth.detail, + Option.some("Every GitHub host gh is signed in to is turned off in Settings → Source Control."), + ); +}); + +it("names the environment token that overrides the Settings choice", () => { + const auth = GitHubSourceControlProvider.parseGitHubAuth( + multiAccountStatus([ + { + state: "success", + active: false, + host: "github.com", + login: "bot", + tokenSource: "GH_TOKEN", + }, + ]), + { hosts: { "github.com": { account: "work", enabled: true } }, tokens: {} }, + ); + assert.deepStrictEqual(auth.account, Option.some("bot")); + assert.deepStrictEqual( + auth.detail, + Option.some( + "Using GH_TOKEN from the server environment; it overrides the account chosen in Settings.", + ), + ); + assert.strictEqual(auth.accounts?.[2]?.environmentVariable, "GH_TOKEN"); +}); diff --git a/apps/server/src/sourceControl/GitHubSourceControlProvider.ts b/apps/server/src/sourceControl/GitHubSourceControlProvider.ts index 8e183e8b9ace2afd6aa81f7ba7ca030e94d9de96..0b868ba3f4394945e7917970b62ac3679a00c215 100644 --- a/apps/server/src/sourceControl/GitHubSourceControlProvider.ts +++ b/apps/server/src/sourceControl/GitHubSourceControlProvider.ts @@ -4,16 +4,24 @@ import * as Effect from "effect/Effect"; import * as Option from "effect/Option"; import * as Result from "effect/Result"; import { + DEFAULT_SERVER_SETTINGS, SourceControlProviderError, type ChangeRequest, + type GitHubSettings, type SourceControlProviderDiscoveryItem, } from "@t3tools/contracts"; import { HostProcessEnvironment } from "@t3tools/shared/hostProcess"; +import * as ServerSettings from "../serverSettings.ts"; import * as GitHubApi from "./GitHubApi.ts"; import * as GitHubCli from "./GitHubCli.ts"; -import { findAuthenticatedGitHubAccount, parseGitHubAuthStatus } from "./gitHubAuthStatus.ts"; +import { + effectiveGitHubAccount, + findAuthenticatedGitHubAccount, + parseGitHubAuthStatus, + type GitHubAuthStatusAccount, +} from "./gitHubAuthStatus.ts"; import * as SourceControlProvider from "./SourceControlProvider.ts"; import { combinedAuthOutput, @@ -60,29 +68,78 @@ function toChangeRequest(summary: GitHubCli.GitHubPullRequestSummary): ChangeReq }; } -function parseGitHubAuth(input: SourceControlAuthProbeInput) { +function authAccounts(accounts: ReadonlyArray) { + return accounts.map((entry) => ({ + host: entry.host, + account: entry.account, + active: entry.active, + authenticated: entry.authenticated, + ...(entry.error === null ? {} : { error: entry.error }), + ...(entry.environmentVariable === null + ? {} + : { environmentVariable: entry.environmentVariable }), + })); +} + +/** + * Reads `gh auth status --json hosts`. The headline account is the one GitHub requests will + * use: Settings can pin a login per host or turn a host off, and an environment token beats both. + */ +export function parseGitHubAuth( + input: SourceControlAuthProbeInput, + settings: GitHubSettings = DEFAULT_SERVER_SETTINGS.github, +) { const output = combinedAuthOutput(input); const authStatus = parseGitHubAuthStatus(input.stdout); - const authenticatedAccount = findAuthenticatedGitHubAccount(authStatus.accounts); - const host = authenticatedAccount?.host; + const hosts = [...new Set(authStatus.accounts.map((entry) => entry.host))]; + const fallback = findAuthenticatedGitHubAccount(authStatus.accounts); + // Lead with the host gh would pick, unless Settings turned it off. + const orderedHosts = fallback + ? [fallback.host, ...hosts.filter((host) => host !== fallback.host)] + : hosts; + const chosen = orderedHosts + .map((host) => effectiveGitHubAccount(host, authStatus.accounts, settings)) + .find((entry) => entry !== undefined); + const accounts = authStatus.parsed ? { accounts: authAccounts(authStatus.accounts) } : {}; - if (authenticatedAccount) { - return providerAuth({ - status: "authenticated", - account: authenticatedAccount.account, - host, - }); + if (chosen) { + return { + ...providerAuth({ + status: "authenticated", + account: chosen.account, + host: chosen.host, + detail: + chosen.environmentVariable === null + ? undefined + : `Using ${chosen.environmentVariable} from the server environment; it overrides the account chosen in Settings.`, + }), + ...accounts, + }; + } + + if (fallback) { + return { + ...providerAuth({ + status: "unauthenticated", + host: fallback.host, + detail: "Every GitHub host gh is signed in to is turned off in Settings → Source Control.", + }), + ...accounts, + }; } const failedAccount = authStatus.accounts.find((entry) => entry.active) ?? authStatus.accounts[0]; if (authStatus.parsed) { - return providerAuth({ - status: "unauthenticated", - host: failedAccount?.host, - detail: - failedAccount?.error ?? - "Run `gh auth login` to authenticate GitHub CLI with an active account.", - }); + return { + ...providerAuth({ + status: "unauthenticated", + host: failedAccount?.host, + detail: + failedAccount?.error ?? + "Run `gh auth login` to authenticate GitHub CLI with an active account.", + }), + ...accounts, + }; } // gh gained `auth status --json` in 2.81.0. Older versions reject the flag and exit @@ -98,14 +155,12 @@ function parseGitHubAuth(input: SourceControlAuthProbeInput) { if (input.exitCode !== 0) { return providerAuth({ status: "unauthenticated", - host, detail: firstSafeAuthLine(output) ?? "Run `gh auth login` to authenticate GitHub CLI.", }); } return providerAuth({ status: "unknown", - host, detail: firstSafeAuthLine(output) ?? "GitHub CLI auth status could not be parsed.", }); } @@ -132,51 +187,75 @@ function environmentTokenVariable(environment: NodeJS.ProcessEnv): string | null } /** - * GitHub is usable with a token from the environment or with `gh` to hand one over. An - * environment token is checked against the API, since `gh auth status` may not know it. + * GitHub is usable with a token saved in Settings, one from the environment, or `gh` to hand one + * over. Reads the + * GitHub settings on every probe, so a saved account choice shows on rescan. An environment + * token is checked against the API, since `gh auth status` may not know it. */ export const makeDiscovery = Effect.gen(function* () { const api = yield* GitHubApi.GitHubApi; const process = yield* VcsProcess.VcsProcess; const environment = yield* HostProcessEnvironment; + const serverSettings = yield* ServerSettings.ServerSettingsService; + return { type: "managed-cli", kind: discovery.kind, label: discovery.label, installHint: discovery.installHint, probe: Effect.fn("GitHubSourceControlProvider.discovery")(function* (cwd: string) { - const cli = yield* probeSourceControlProvider({ cwd, process, spec: discovery }); + const settings = yield* serverSettings.getSettings.pipe( + Effect.map((current) => current.github), + Effect.orElseSucceed(() => DEFAULT_SERVER_SETTINGS.github), + ); + const cli = yield* probeSourceControlProvider({ + cwd, + process, + spec: { ...discovery, parseAuth: (input) => parseGitHubAuth(input, settings) }, + }); + // A token saved in Settings wins over the environment, which wins over gh. + const savedToken = (settings.tokens["github.com"] ?? "").trim() !== ""; const variable = environmentTokenVariable(environment); - if (variable === null) return cli; + const tokenSource = savedToken ? "the token saved in Settings" : variable; + // A host turned off in Settings stays off even with a token. + if (tokenSource === null || settings.hosts["github.com"]?.enabled === false) return cli; const viewer = yield* api .rest({ host: "github.com", operation: "discovery", path: "user" }) .pipe(Effect.result); const login = Result.isSuccess(viewer) ? Option.getOrUndefined(decodeViewer(viewer.success.body))?.login : undefined; + // The per-host logins stay, so the account picker still lists every host gh knows. + const accounts = cli.auth.accounts === undefined ? {} : { accounts: cli.auth.accounts }; return { ...cli, status: "available" as const, - auth: - login !== undefined + auth: { + ...(login !== undefined ? providerAuth({ status: "authenticated", account: login, host: "github.com", - detail: `Using the token in ${variable} from the server environment.`, + detail: savedToken + ? "Using the token saved in Settings; it overrides GH_TOKEN and the gh login." + : `Using ${variable} from the server environment; it overrides the account chosen in Settings.`, }) : Result.isFailure(viewer) && viewer.failure._tag !== "GitHubApiAuthenticationError" ? // Only a refusal says the token is bad; a network error or a pause says nothing. providerAuth({ status: "unknown", host: "github.com", - detail: `Could not check the token in ${variable}: ${viewer.failure.message}`, + detail: `Could not check ${savedToken ? tokenSource : `the token in ${tokenSource}`}: ${viewer.failure.message}`, }) : providerAuth({ status: "unauthenticated", host: "github.com", - detail: `GitHub refused the token in ${variable}. Replace it, or unset it to use \`gh auth login\`.`, - }), + detail: savedToken + ? "GitHub refused the token saved in Settings. Replace or remove it in Settings → Source Control." + : `GitHub refused the token in ${tokenSource}. Replace it, or unset it to use \`gh auth login\`.`, + })), + ...accounts, + }, } satisfies SourceControlProviderDiscoveryItem; }), refineUnknownRemote: () => Effect.succeed(null), diff --git a/apps/server/src/sourceControl/SourceControlDiscovery.test.ts b/apps/server/src/sourceControl/SourceControlDiscovery.test.ts index 672963fc9ec43ba73e1b1a96313e556b2db68cb1..54de8cc14fde7864ce80a8e67f0b1c72be682a34 100644 --- a/apps/server/src/sourceControl/SourceControlDiscovery.test.ts +++ b/apps/server/src/sourceControl/SourceControlDiscovery.test.ts @@ -11,6 +11,7 @@ import { ChildProcessSpawner } from "effect/process"; import { FetchHttpClient, HttpClient, HttpClientResponse } from "effect/http"; import { VcsProcessSpawnError } from "@t3tools/contracts"; +import * as ServerSettings from "../serverSettings.ts"; import * as ServerConfig from "../config.ts"; import * as VcsDriverRegistry from "../vcs/VcsDriverRegistry.ts"; import * as VcsProcess from "../vcs/VcsProcess.ts"; @@ -38,6 +39,7 @@ const layerSourceControlProviderRegistryTest = (input: { }).pipe(Layer.provide(NodeServices.layer)), Layer.mock(AzureDevOpsCli.AzureDevOpsCli)({}), Layer.mock(BitbucketApi.BitbucketApi)(input.bitbucket), + ServerSettings.ServerSettingsService.layerTest(), Layer.mock(GitHubCli.GitHubCli)({}), Layer.mock(GitHubApi.GitHubApi)({}), Layer.mock(GitLabCli.GitLabCli)({}), diff --git a/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts b/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts index 39dbcaba04ffad2b4e055783c5b481b514dc23eb..08751017b35133f03171ba34e8be2dd391da3f15 100644 --- a/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts +++ b/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts @@ -7,6 +7,7 @@ import * as Option from "effect/Option"; import { ChildProcessSpawner } from "effect/process"; import { VcsRepositoryDetectionError } from "@t3tools/contracts"; +import * as ServerSettings from "../serverSettings.ts"; import * as ServerConfig from "../config.ts"; import type * as VcsDriver from "../vcs/VcsDriver.ts"; import * as VcsDriverRegistry from "../vcs/VcsDriverRegistry.ts"; @@ -96,6 +97,7 @@ function makeRegistry(input: { layerProcess, Layer.mock(AzureDevOpsCli.AzureDevOpsCli)({}), Layer.mock(BitbucketApi.BitbucketApi)({}), + ServerSettings.ServerSettingsService.layerTest(), Layer.mock(GitHubCli.GitHubCli)(input.github ?? {}), Layer.mock(GitHubApi.GitHubApi)(input.githubApi ?? {}), Layer.mock(GitLabCli.GitLabCli)(input.gitlab ?? {}), diff --git a/apps/server/src/sourceControl/gitHubAuthStatus.ts b/apps/server/src/sourceControl/gitHubAuthStatus.ts index d58909c560c2b7c85c7674277e3c87e2e79e416c..7321ead2d4ec1f83bba46fb207ad3d75f68cf4af 100644 --- a/apps/server/src/sourceControl/gitHubAuthStatus.ts +++ b/apps/server/src/sourceControl/gitHubAuthStatus.ts @@ -1,3 +1,4 @@ +import type { GitHubSettings } from "@t3tools/contracts"; import * as Option from "effect/Option"; import * as Schema from "effect/Schema"; @@ -7,6 +8,7 @@ const GitHubAuthStatusAccountSchema = Schema.Struct({ active: Schema.Boolean, host: Schema.String, login: Schema.String, + tokenSource: Schema.optional(Schema.String), }); const GitHubAuthStatusSchema = Schema.Struct({ @@ -23,6 +25,8 @@ export interface GitHubAuthStatusAccount { readonly authenticated: boolean; readonly active: boolean; readonly error: string | null; + /** The variable (GH_TOKEN and kin) the login came from; it overrides every stored login. */ + readonly environmentVariable: string | null; } export interface GitHubAuthStatus { @@ -30,6 +34,8 @@ export interface GitHubAuthStatus { readonly accounts: ReadonlyArray; } +const ENVIRONMENT_TOKEN_SOURCE = /^(?:GH|GITHUB)_(?:ENTERPRISE_)?TOKEN$/u; + function nonEmptyString(value: string): string | null { const trimmed = value.trim(); return trimmed.length > 0 ? trimmed : null; @@ -54,6 +60,9 @@ export function parseGitHubAuthStatus(text: string): GitHubAuthStatus { authenticated: account.state === "success", active: account.active, error: account.error?.trim() || null, + environmentVariable: ENVIRONMENT_TOKEN_SOURCE.test(account.tokenSource ?? "") + ? (account.tokenSource ?? null) + : null, }, ]; }), @@ -70,3 +79,25 @@ export function findAuthenticatedGitHubAccount( accounts.find((account) => account.authenticated) ); } + +/** + * The login GitHub requests for a host will use, honoring Settings: an environment token + * first (gh's own precedence), then the pinned account, then gh's active login. + * Returns undefined when the host is turned off or has no usable login. + */ +export function effectiveGitHubAccount( + host: string, + accounts: ReadonlyArray, + settings: GitHubSettings, +): GitHubAuthStatusAccount | undefined { + const choice = settings.hosts[host]; + if (choice?.enabled === false) return undefined; + const usable = accounts.filter((account) => account.host === host && account.authenticated); + return ( + usable.find((account) => account.environmentVariable !== null) ?? + (choice?.account === undefined + ? undefined + : usable.find((account) => account.account === choice.account)) ?? + findAuthenticatedGitHubAccount(usable) + ); +} diff --git a/apps/web/src/components/settings/GitHubAccountSettings.logic.test.ts b/apps/web/src/components/settings/GitHubAccountSettings.logic.test.ts new file mode 100644 index 0000000000000000000000000000000000000000..a1a895297da28660a47e0e2cf8df1c2554897ea6 --- /dev/null +++ b/apps/web/src/components/settings/GitHubAccountSettings.logic.test.ts @@ -0,0 +1,55 @@ +import { describe, expect, it } from "vite-plus/test"; + +import { groupGitHubAccounts, nextGitHubHosts } from "./GitHubAccountSettings.logic"; + +describe("groupGitHubAccounts", () => { + it("groups logins by host, keeping broken and environment logins out of the picker", () => { + expect( + groupGitHubAccounts([ + { host: "github.com", account: "personal", active: true, authenticated: true }, + { host: "github.com", account: "work", active: false, authenticated: true }, + { host: "github.com", account: "old", active: false, authenticated: false, error: "bad" }, + { + host: "github.com", + account: "bot", + active: false, + authenticated: true, + environmentVariable: "GH_TOKEN", + }, + { host: "ghe.acme.test", account: "jm", active: false, authenticated: true }, + ]), + ).toEqual([ + { + host: "github.com", + activeAccount: "personal", + selectable: ["personal", "work"], + broken: [ + { host: "github.com", account: "old", active: false, authenticated: false, error: "bad" }, + ], + environmentVariable: "GH_TOKEN", + }, + { + host: "ghe.acme.test", + activeAccount: "jm", + selectable: ["jm"], + broken: [], + environmentVariable: null, + }, + ]); + }); +}); + +describe("nextGitHubHosts", () => { + it("pins an account, keeps other hosts, and drops a host back on gh's defaults", () => { + const other = { "ghe.acme.test": { enabled: false } }; + const pinned = nextGitHubHosts(other, "github.com", { account: "work" }); + expect(pinned).toEqual({ ...other, "github.com": { enabled: true, account: "work" } }); + + const disabled = nextGitHubHosts(pinned, "github.com", { enabled: false }); + expect(disabled["github.com"]).toEqual({ enabled: false, account: "work" }); + + expect(nextGitHubHosts(disabled, "github.com", { enabled: true, account: null })).toEqual( + other, + ); + }); +}); diff --git a/apps/web/src/components/settings/GitHubAccountSettings.logic.ts b/apps/web/src/components/settings/GitHubAccountSettings.logic.ts new file mode 100644 index 0000000000000000000000000000000000000000..466372f7699d808d7339e2d7cd3e3db44bd44e2c --- /dev/null +++ b/apps/web/src/components/settings/GitHubAccountSettings.logic.ts @@ -0,0 +1,52 @@ +import type { GitHubSettings, SourceControlProviderAuth } from "@t3tools/contracts"; + +export type GitHubDiscoveredAccount = NonNullable[number]; + +export interface GitHubHostGroup { + readonly host: string; + /** gh's active stored login, the one used when Settings pin nothing. */ + readonly activeAccount: string | null; + /** Stored logins that work and can be pinned with `gh auth token --user`. */ + readonly selectable: ReadonlyArray; + /** Logins gh holds but cannot use; shown with their error. */ + readonly broken: ReadonlyArray; + /** Set when GH_TOKEN or a sibling overrides every stored login for this host. */ + readonly environmentVariable: string | null; +} + +/** Groups gh's logins by host, in the order gh reported the hosts. */ +export function groupGitHubAccounts( + accounts: ReadonlyArray, +): ReadonlyArray { + const hosts = [...new Set(accounts.map((entry) => entry.host))]; + return hosts.map((host) => { + const entries = accounts.filter((entry) => entry.host === host); + const stored = entries.filter((entry) => entry.environmentVariable === undefined); + const selectable = stored.filter((entry) => entry.authenticated); + return { + host, + activeAccount: (selectable.find((entry) => entry.active) ?? selectable[0])?.account ?? null, + selectable: selectable.map((entry) => entry.account), + broken: stored.filter((entry) => !entry.authenticated), + environmentVariable: + entries.find((entry) => entry.environmentVariable)?.environmentVariable ?? null, + }; + }); +} + +/** + * The full `hosts` map after one host changes. Hosts left on gh's defaults (enabled, + * no pinned account) are dropped so settings only hold real choices. + */ +export function nextGitHubHosts( + current: GitHubSettings["hosts"], + host: string, + change: { readonly enabled?: boolean; readonly account?: string | null }, +): GitHubSettings["hosts"] { + const previous = current[host]; + const enabled = change.enabled ?? previous?.enabled ?? true; + const account = change.account === undefined ? previous?.account : (change.account ?? undefined); + const { [host]: _replaced, ...rest } = current; + if (enabled && account === undefined) return rest; + return { ...rest, [host]: { enabled, ...(account === undefined ? {} : { account }) } }; +} diff --git a/apps/web/src/components/settings/GitHubAccountSettings.tsx b/apps/web/src/components/settings/GitHubAccountSettings.tsx new file mode 100644 index 0000000000000000000000000000000000000000..98e7289b260fe1c4d3bf540e91caa065a8c4d3f6 --- /dev/null +++ b/apps/web/src/components/settings/GitHubAccountSettings.tsx @@ -0,0 +1,224 @@ +import type { EnvironmentId, SourceControlProviderAuth } from "@t3tools/contracts"; +import { EyeIcon, EyeOffIcon } from "lucide-react"; +import { useState } from "react"; + +import { useEnvironmentSettings } from "../../hooks/useSettings"; +import { serverEnvironment } from "../../state/server"; +import { useAtomCommand } from "../../state/use-atom-command"; +import { Button } from "../ui/button"; +import { Select, SelectItem, SelectPopup, SelectTrigger, SelectValue } from "../ui/select"; +import { Switch } from "../ui/switch"; +import { groupGitHubAccounts, nextGitHubHosts } from "./GitHubAccountSettings.logic"; +import { redactedPlaceholder } from "./RedactedSensitiveText"; + +/** Sentinel select value for "follow gh's active login"; logins never contain spaces. */ +const ACTIVE_ACCOUNT = "active gh account"; + +/** + * A login, blurred like RedactedSensitiveText until the panel reveals it. Plain text, not a + * button, so it can sit inside select options; one panel toggle reveals every login. + */ +function RedactedLogin(props: { + readonly account: string; + readonly revealed: boolean; + /** Distinguishes hidden logins from each other for a screen reader, e.g. "Account 2". */ + readonly label?: string; +}) { + return props.revealed ? ( + {props.account} + ) : ( + + + {redactedPlaceholder(props.account)} + + {props.label ?? "Hidden account"} + + ); +} + +/** + * Per-host GitHub choices for one environment: turn a host off, or pin which of the + * logins `gh` holds is used instead of its active one. Changes save immediately. + */ +export function GitHubAccountSettings({ + environmentId, + auth, + onSaved, +}: { + readonly environmentId: EnvironmentId; + readonly auth: SourceControlProviderAuth; + readonly onSaved: () => void; +}) { + const hosts = useEnvironmentSettings(environmentId, (settings) => settings.github.hosts); + const updateSettings = useAtomCommand(serverEnvironment.updateSettings, { + label: "save GitHub account settings", + }); + const [saving, setSaving] = useState(false); + const [revealed, setRevealed] = useState(false); + const groups = groupGitHubAccounts(auth.accounts ?? []); + + const save = async ( + host: string, + change: { readonly enabled?: boolean; readonly account?: string | null }, + ) => { + setSaving(true); + try { + const result = await updateSettings({ + environmentId, + input: { patch: { github: { hosts: nextGitHubHosts(hosts, host, change) } } }, + }); + if (result._tag === "Success") onSaved(); + } finally { + setSaving(false); + } + }; + + if (groups.length === 0) { + return ( +

+ Sign in with gh auth login on + the server host, then rescan to choose accounts here. +

+ ); + } + + return ( +
+
+

+ Choose which gh login each + GitHub host uses, or turn a host off. +

+ +
+ {groups.map((group) => { + const choice = hosts[group.host]; + const enabled = choice?.enabled ?? true; + const stalePin = + choice?.account !== undefined && !group.selectable.includes(choice.account); + const pinned = choice?.account !== undefined && !stalePin ? choice.account : ACTIVE_ACCOUNT; + return ( +
+
+
+ {group.host} + {group.selectable.length === 1 && group.selectable[0] !== undefined ? ( +

+ Signed in as + +

+ ) : null} +
+ void save(group.host, { enabled: checked })} + /> +
+ {group.selectable.length > 1 ? ( +
+ Account +
+ +
+
+ ) : null} + {stalePin ? ( +
+

+ The chosen login is no longer signed in, so the active gh login is used. +

+ +
+ ) : null} + {group.broken.map((entry) => ( +

+ + can't be used: {entry.error ?? "gh reports this login as invalid."} +

+ ))} + {group.environmentVariable ? ( +

+ {group.environmentVariable} is set on the server, so it overrides the account chosen + here until it is unset. +

+ ) : null} +
+ ); + })} +
+ ); +} diff --git a/apps/web/src/components/settings/GitHubTokenSettings.tsx b/apps/web/src/components/settings/GitHubTokenSettings.tsx new file mode 100644 index 0000000000000000000000000000000000000000..80ad552f15b26f4eb6836e8518eab7fef5c1bf74 --- /dev/null +++ b/apps/web/src/components/settings/GitHubTokenSettings.tsx @@ -0,0 +1,138 @@ +import type { EnvironmentId } from "@t3tools/contracts"; +import { ExternalLinkIcon } from "lucide-react"; +import { useState } from "react"; + +import { useEnvironmentSettings } from "../../hooks/useSettings"; +import { serverEnvironment } from "../../state/server"; +import { useAtomCommand } from "../../state/use-atom-command"; +import { Button, InlineButton } from "../ui/button"; +import { Input } from "../ui/input"; +import { Label } from "../ui/label"; + +const DEFAULT_HOST = "github.com"; + +/** Where to create a token for a host: GitHub's own page, or the same path on an Enterprise host. */ +function newTokenUrl(host: string): string { + return `https://${host}/settings/personal-access-tokens/new`; +} + +/** + * A GitHub token for one host, kept in the server's secret store. It is used before + * `GH_TOKEN` and the `gh` login, so it also works on a server without `gh` installed. + * Write-only: the saved token is never shown, only whether one is set. + */ +export function GitHubTokenSettings({ + environmentId, + onSaved, +}: { + readonly environmentId: EnvironmentId; + readonly onSaved: () => void; +}) { + const tokens = useEnvironmentSettings(environmentId, (settings) => settings.github.tokens); + const updateSettings = useAtomCommand(serverEnvironment.updateSettings, { + label: "save GitHub token", + }); + const [host, setHost] = useState(DEFAULT_HOST); + const [draft, setDraft] = useState(""); + const [saving, setSaving] = useState(false); + const normalizedHost = host.trim().toLowerCase(); + const isSaved = (tokens[normalizedHost] ?? "").length > 0; + const savedHosts = Object.entries(tokens) + .filter(([, value]) => value.length > 0) + .map(([saved]) => saved); + + const save = async (target: string, token: string) => { + setSaving(true); + try { + const result = await updateSettings({ + environmentId, + input: { patch: { github: { tokens: { [target]: token } } } }, + }); + if (result._tag === "Success") { + setDraft(""); + onSaved(); + } + } finally { + setSaving(false); + } + }; + + return ( +
{ + event.preventDefault(); + if (normalizedHost && draft.trim()) void save(normalizedHost, draft.trim()); + }} + > + {/* Locked while saving: a successful save clears the draft, which would drop edits made mid-request. */} +
+

+ A token saved here is used before{" "} + GH_TOKEN and the{" "} + gh login, so GitHub works + without the GitHub CLI. Give it read and write access to pull requests and contents.{" "} + + } + > + Create a token + + +

+
+
+ + setHost(event.target.value)} + /> +
+
+ + setDraft(event.target.value)} + /> +
+
+
+

+ {savedHosts.length === 0 + ? "No token saved; the server uses GH_TOKEN or the gh login." + : `Saved for ${savedHosts.join(", ")}.`} +

+
+ {isSaved ? ( + + ) : null} + +
+
+
+
+ ); +} diff --git a/apps/web/src/components/settings/RedactedSensitiveText.tsx b/apps/web/src/components/settings/RedactedSensitiveText.tsx index 4ae2276b8e63e55e787bf3a3862f7dc2e84ab9b5..46f0897db34261f0e1811edb0c3772e76f4ac446 100644 --- a/apps/web/src/components/settings/RedactedSensitiveText.tsx +++ b/apps/web/src/components/settings/RedactedSensitiveText.tsx @@ -5,7 +5,7 @@ import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; const REDACTED_TEXT_ALPHABET = "abcdefghjkmnpqrstuvwxyz23456789"; -function redactedPlaceholder(value: string): string { +export function redactedPlaceholder(value: string): string { let state = 0x811c9dc5; for (let index = 0; index < value.length; index += 1) { state ^= value.charCodeAt(index); diff --git a/apps/web/src/components/settings/SourceControlSettings.tsx b/apps/web/src/components/settings/SourceControlSettings.tsx index 9fba0edc7d44ff30d336508e4788df2f4bc5399d..978d7a57105d025d8f96cc0dc0bfcc36f83faf3b 100644 --- a/apps/web/src/components/settings/SourceControlSettings.tsx +++ b/apps/web/src/components/settings/SourceControlSettings.tsx @@ -57,6 +57,8 @@ import { type Icon, } from "../Icons"; import { BitbucketCredentialsSettings } from "./BitbucketCredentialsSettings"; +import { GitHubAccountSettings } from "./GitHubAccountSettings"; +import { GitHubTokenSettings } from "./GitHubTokenSettings"; import { RedactedSensitiveText } from "./RedactedSensitiveText"; import { SourceControlWritingSettingsSection } from "./SourceControlWritingSettings"; import { @@ -222,6 +224,9 @@ function itemSummary({ if (auth) { if (auth.status === "authenticated") { + // The server names the account its requests use, Settings choice included, and + // says when an environment token overrides it. + const authDetail = optionLabel(auth.detail); return ( <> Authenticated @@ -231,6 +236,7 @@ function itemSummary({ ) : null} + {authDetail ? · {authDetail} : null} ); } @@ -241,6 +247,11 @@ function itemSummary({ return Available. {item.installHint}; } + // Signed in, but every login is turned off here: the fix is the switch below, not the CLI. + if (auth.status === "unauthenticated" && auth.accounts?.some((entry) => entry.authenticated)) { + return {optionLabel(auth.detail) ?? `Every ${item.label} host is turned off.`}; + } + if (auth.status === "unauthenticated") { return ( @@ -283,7 +294,8 @@ function DiscoveryItemRow({ if ( (item.kind === "git" && searchTargetId === searchableSetting("git-fetch-interval").id) || (item.kind === "bitbucket" && - searchTargetId === searchableSetting("bitbucket-credentials").id) + searchTargetId === searchableSetting("bitbucket-credentials").id) || + (item.kind === "github" && searchTargetId === searchableSetting("github-accounts").id) ) { setIsExpanded(true); } @@ -603,6 +615,25 @@ export function SourceControlSettingsPanel() { onSaved={handleScan} /> + ) : item.kind === "github" ? ( + +
+ {/* Shown even without gh: a saved token is how GitHub works without the CLI. */} + + {item.status === "available" ? ( + + ) : null} +
+ ) : undefined} ))} diff --git a/apps/web/src/components/settings/settingsSearch.ts b/apps/web/src/components/settings/settingsSearch.ts index 4797c9b64ddeed73af835e2f96638a0ef0cd9846..18b2fe5a30a774145e3aaddf41061b37cb371b69 100644 --- a/apps/web/src/components/settings/settingsSearch.ts +++ b/apps/web/src/components/settings/settingsSearch.ts @@ -751,6 +751,16 @@ export const SETTINGS_SEARCH_ITEMS = [ environmentOnly: true, scope: "project-defaults", }, + { + id: "github-accounts", + title: "GitHub accounts and token", + to: "/settings/source-control", + searchTerms: [ + "github gh account login user host enterprise ghes switch multiple accounts disable sign in token personal access token pat api key credential", + ], + environmentOnly: true, + scope: "environment-defaults", + }, { id: "bitbucket-credentials", title: "Bitbucket credentials", diff --git a/docs/user/source-control.md b/docs/user/source-control.md index d5ffeb0b47787cfda841755c3b4023f6d04cd7ee..03c360ff3a10bffee6c2edda53f0182d2eb3bea2 100644 --- a/docs/user/source-control.md +++ b/docs/user/source-control.md @@ -11,11 +11,18 @@ and choose **Rescan**. ### GitHub -Install [GitHub CLI](https://cli.github.com/) 2.81.0 or newer, then sign in: +T3 Code talks to GitHub's API directly and only needs a token. Any of these works, in this +order of precedence: -```bash -gh auth login -``` +1. A token saved in **Settings → Source Control → GitHub**. It is kept in the server's secret + store, and works without the GitHub CLI. +2. `GH_TOKEN` (`GH_ENTERPRISE_TOKEN` with `GH_HOST` for GitHub Enterprise Server) in the + server's environment. +3. [GitHub CLI](https://cli.github.com/) 2.81.0 or newer, signed in with `gh auth login`. + +If `gh` is signed in to several accounts or hosts, expand **GitHub** in the same place to pick +the account each host uses or turn a host off. A saved token or `GH_TOKEN` takes precedence +over that choice; a host turned off stays off either way. ### Forgejo and Gitea @@ -136,7 +143,7 @@ environment clears its permission. GitHub review details, linked PR status, and permitted review actions can then use another connected environment signed in to the same GitHub account. Each needs a project on that host. A connected local environment is preferred for actions and can answer slow or failed reads. -Browsers and mobile clients need a paired environment to use its GitHub CLI credentials. +Browsers and mobile clients need a paired environment to use its GitHub credentials. Credentials stay on their machines. Previously verified credentials remain usable for routing for ten minutes during a GitHub outage; new credentials must be verified first. An action with an uncertain result is never automatically retried elsewhere. Listings, diffs, and checkout or @@ -164,7 +171,7 @@ does not show its diff, so marks are made and read on web and desktop. - **Not authenticated:** run the provider's login command on the server, then rescan. For Bitbucket, check the credentials saved in Settings → Source Control, or confirm the running server received the environment variables. -- **GitHub sign-in cannot be verified:** update GitHub CLI to at least 2.81.0. +- **GitHub sign-in cannot be verified:** update GitHub CLI to at least 2.81.0, or save a token in Settings → Source Control. - **Push fails despite a connected account:** check the Git remote's credentials. SSH and HTTPS remotes can require separate setup from the hosting provider's API access. - **A review cannot load:** open it on the host website while resolving connectivity, permissions, diff --git a/packages/contracts/src/pullRequest.ts b/packages/contracts/src/pullRequest.ts index 68cdfc43e40f9eb064971392b5a94604acda8314..2cd0e45cc8e034d5d3c1444dc765bf1fec1b9e04 100644 --- a/packages/contracts/src/pullRequest.ts +++ b/packages/contracts/src/pullRequest.ts @@ -1264,16 +1264,17 @@ export type PullRequestUnavailableReason = typeof PullRequestUnavailableReason.T /** * What each host needs before it can be read, so a failure names the fix rather than the - * symptom. Bitbucket is credentials on the server rather than a signed-in CLI, which is why - * these are whole sentences instead of a tool name to interpolate. + * symptom. The reason names keep their `cli-` prefix for wire compatibility; for GitHub and + * Bitbucket they mean "no credential" and "a refused credential", not a missing tool. */ const PROVIDER_REQUIREMENT: Partial< Record > = { github: { missing: - "GitHub CLI (`gh`) is required to browse change requests on this host. Install it from https://cli.github.com/ and reload.", - unauthenticated: "GitHub CLI is not authenticated. Run `gh auth login` and retry.", + "No GitHub credential on the server. Set GH_TOKEN, or install the GitHub CLI (https://cli.github.com/) and run `gh auth login`.", + unauthenticated: + "GitHub has no working credential for this host. Run `gh auth login`, or check the account and hosts in Settings → Source Control.", }, forgejo: { missing: diff --git a/packages/contracts/src/settings.test.ts b/packages/contracts/src/settings.test.ts index 20fc7b7ddd38dcb6f84861f40fd8f3bad7e0a5d9..419db85883bcb6de8b3500541dea4f69d5630881 100644 --- a/packages/contracts/src/settings.test.ts +++ b/packages/contracts/src/settings.test.ts @@ -987,6 +987,13 @@ describe("ServerSettingsPatch.providerInstances", () => { }); describe("ServerSettingsPatch string normalization", () => { + it("lowercases GitHub hosts and defaults them to enabled", () => { + const patch = decodeServerSettingsPatch({ + github: { hosts: { " GitHub.com ": { account: " work " } } }, + }); + expect(patch.github?.hosts).toEqual({ "github.com": { account: "work", enabled: true } }); + }); + it("trims string settings while decoding patches", () => { const patch = decodeServerSettingsPatch({ addProjectBaseDirectory: " ~/Development ", diff --git a/packages/contracts/src/settings.ts b/packages/contracts/src/settings.ts index 7e73079a58d0cdb8535dce0b99731b89307c4573..7984ca20b57aba97387a4b785ab62eb283bf4108 100644 --- a/packages/contracts/src/settings.ts +++ b/packages/contracts/src/settings.ts @@ -1013,6 +1013,37 @@ export const BitbucketSettings = Schema.Struct({ }); export type BitbucketSettings = typeof BitbucketSettings.Type; +/** + * Per-host choices for the GitHub CLI's logins. `account` pins one of the logins + * `gh` holds for the host instead of its active one; a disabled host gets no + * credential at all. A token saved here wins over `GH_TOKEN` and friends, which win over `gh`. + */ +/** A GitHub host name, lowercased on decode so `GitHub.com` and `github.com` are one entry. */ +export const GitHubHost = TrimmedNonEmptyString.pipe( + Schema.decodeTo(Schema.String, SchemaTransformation.toLowerCase()), +); + +export const GitHubHostSettings = Schema.Struct({ + account: Schema.optionalKey(TrimmedNonEmptyString), + enabled: Schema.Boolean.pipe(Schema.withDecodingDefault(Effect.succeed(true))), +}); +export type GitHubHostSettings = typeof GitHubHostSettings.Type; + +export const GitHubSettings = Schema.Struct({ + /** Keyed by lowercased host, for example `github.com`. */ + hosts: Schema.Record(GitHubHost, GitHubHostSettings).pipe( + Schema.withDecodingDefault(Effect.succeed({})), + ), + /** + * A token per host, used before `GH_TOKEN` and `gh`. The server keeps each one in its secret + * store; settings and clients only ever see a redaction marker for a saved token. + */ + tokens: Schema.Record(GitHubHost, TrimmedString).pipe( + Schema.withDecodingDefault(Effect.succeed({})), + ), +}); +export type GitHubSettings = typeof GitHubSettings.Type; + export const ObservabilitySettings = Schema.Struct({ otlpTracesUrl: TrimmedString.pipe(Schema.withDecodingDefault(Effect.succeed(""))), otlpMetricsUrl: TrimmedString.pipe(Schema.withDecodingDefault(Effect.succeed(""))), @@ -1423,6 +1454,7 @@ export const ServerSettings = Schema.Struct({ ), observability: ObservabilitySettings.pipe(Schema.withDecodingDefault(Effect.succeed({}))), bitbucket: BitbucketSettings.pipe(Schema.withDecodingDefault(Effect.succeed({}))), + github: GitHubSettings.pipe(Schema.withDecodingDefault(Effect.succeed({}))), // Keyed by a user-chosen id so a source keeps its rows across edits. Entries // this build cannot decode round-trip untouched, as provider instances do. usageLimitSources: Schema.Record(UsageLimitSourceId, UsageLimitSourceConfig).pipe( @@ -1714,6 +1746,16 @@ export const ServerSettingsPatch = Schema.Struct({ apiToken: Schema.optionalKey(TrimmedString), }), ), + /** + * `hosts` replaces the whole map, so an omitted host or account clears it. `tokens` merges per + * host: an empty token removes that host's token, the redaction marker keeps it. + */ + github: Schema.optionalKey( + Schema.Struct({ + hosts: Schema.optionalKey(Schema.Record(GitHubHost, GitHubHostSettings)), + tokens: Schema.optionalKey(Schema.Record(GitHubHost, TrimmedString)), + }), + ), providers: Schema.optionalKey( Schema.Struct({ codex: Schema.optionalKey(CodexSettingsPatch), diff --git a/packages/contracts/src/sourceControl.ts b/packages/contracts/src/sourceControl.ts index b0d2bd4a75bdf52b05bff8463a576fc2d00475fe..f9717b45c4c319fcc9cf6c6882c5f7e95f066651 100644 --- a/packages/contracts/src/sourceControl.ts +++ b/packages/contracts/src/sourceControl.ts @@ -123,6 +123,20 @@ export const SourceControlProviderAuth = Schema.Struct({ account: Schema.Option(TrimmedNonEmptyString), host: Schema.Option(TrimmedNonEmptyString), detail: Schema.Option(TrimmedNonEmptyString), + /** Every login the provider CLI holds, across hosts. Only GitHub reports these today. */ + accounts: Schema.optionalKey( + Schema.Array( + Schema.Struct({ + host: TrimmedNonEmptyString, + account: TrimmedNonEmptyString, + active: Schema.Boolean, + authenticated: Schema.Boolean, + error: Schema.optionalKey(TrimmedNonEmptyString), + /** Set when the login comes from a token variable such as `GH_TOKEN`, which wins over Settings. */ + environmentVariable: Schema.optionalKey(TrimmedNonEmptyString), + }), + ), + ), }); export type SourceControlProviderAuth = typeof SourceControlProviderAuth.Type; diff --git a/packages/shared/src/serverSettings.test.ts b/packages/shared/src/serverSettings.test.ts index fb25119cadcea3637434d58a554b9df141684a63..f7305812e74fc13aa706017aa2997a0265e2cc93 100644 --- a/packages/shared/src/serverSettings.test.ts +++ b/packages/shared/src/serverSettings.test.ts @@ -41,6 +41,17 @@ describe("serverSettings helpers", () => { logsAfterDays: 30, }); }); + it("replaces GitHub host choices so a cleared account pin does not survive", () => { + const pinned = applyServerSettingsPatch(DEFAULT_SERVER_SETTINGS, { + github: { hosts: { "github.com": { account: "work", enabled: true } } }, + }); + expect(pinned.github.hosts).toEqual({ "github.com": { account: "work", enabled: true } }); + expect( + applyServerSettingsPatch(pinned, { + github: { hosts: { "github.com": { enabled: false } } }, + }).github.hosts, + ).toEqual({ "github.com": { enabled: false } }); + }); it("replaces SSH host lists when saving, editing, and removing hosts", () => { const host = { id: "mini", label: "Mac mini", target: "mini" }; const saved = applyServerSettingsPatch(DEFAULT_SERVER_SETTINGS, { deviceHosts: [host] }); diff --git a/packages/shared/src/serverSettings.ts b/packages/shared/src/serverSettings.ts index aabc3612c98808d719ee30e02c01d33e8705147f..5e66b0ebe3325f75e9c06cec01325fd2a5602816 100644 --- a/packages/shared/src/serverSettings.ts +++ b/packages/shared/src/serverSettings.ts @@ -378,6 +378,10 @@ export function applyServerSettingsPatch( ], } : {}), + // Host replacement: deepMerge would keep a cleared account pin. + ...(patch.github?.hosts !== undefined + ? { github: { ...next.github, hosts: patch.github.hosts } } + : {}), ...(projectSettingsOverridesPatch !== undefined ? { projectSettingsOverrides: Object.fromEntries(