From 0edded5ef7976975b1b971222a34c221169f277d Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 19 Oct 2026 06:42:29 +0000 Subject: [PATCH 1/8] fix(github): resolve credentials per repository across pull request operations --- apps/server/src/assets/AssetAccess.test.ts | 28 +- apps/server/src/assets/GitHubMediaFetch.ts | 22 +- .../pullRequest/PullRequestService.test.ts | 322 +++++++++++++++++- .../src/pullRequest/PullRequestService.ts | 209 +++++++++--- .../sourceControl/GitHubWorkspaceAccount.ts | 29 ++ .../SourceControlProviderRegistry.test.ts | 4 +- .../src/state/pullRequestRouting.ts | 8 +- .../src/state/pullRequests.test.ts | 3 +- packages/contracts/src/pullRequest.ts | 15 +- .../src/server/PullRequestProvider.ts | 21 +- .../src/client/definition.ts | 40 +++ .../src/server/GitHubCredentials.test.ts | 61 +++- .../src/server/GitHubCredentials.ts | 65 +++- .../src/server/GitHubPullRequestApi.test.ts | 2 + .../server/GitHubPullRequestProvider.test.ts | 49 ++- .../src/server/GitHubPullRequestProvider.ts | 84 ++++- .../src/server/GitHubRequestPolicy.ts | 70 +++- .../src/server/GitHubRest.test.ts | 1 + .../src/server/GitHubRest.ts | 3 +- .../GitHubSourceControlProvider.test.ts | 1 + .../src/server/GitHubSourceControlProvider.ts | 151 ++++---- .../src/server/gitHubGraphQlClient.test.ts | 86 +++++ .../src/server/gitHubGraphQlClient.ts | 9 +- .../src/server/githubStackActions.test.ts | 1 + .../src/server/githubStackRebase.ts | 2 +- 25 files changed, 1091 insertions(+), 195 deletions(-) create mode 100644 apps/server/src/sourceControl/GitHubWorkspaceAccount.ts diff --git a/apps/server/src/assets/AssetAccess.test.ts b/apps/server/src/assets/AssetAccess.test.ts index a327121fd6198..4f2f493652282 100644 --- a/apps/server/src/assets/AssetAccess.test.ts +++ b/apps/server/src/assets/AssetAccess.test.ts @@ -28,6 +28,7 @@ import * as ServerSecretStore from "../auth/ServerSecretStore.ts"; import * as ServerConfig from "../config.ts"; import * as Orchestrator from "../orchestration-v2/Orchestrator.ts"; import * as ProjectFaviconResolver from "../project/ProjectFaviconResolver.ts"; +import * as RepositoryIdentityResolver from "../project/RepositoryIdentityResolver.ts"; import * as T3ProjectFileLoader from "../project/T3ProjectFileLoader.ts"; import * as WorkspacePaths from "../workspace/WorkspacePaths.ts"; import { assetFileResponse } from "../http.ts"; @@ -195,17 +196,22 @@ describe("AssetAccess", () => { expect(authorizations).toEqual([undefined, "Bearer signed-in", "Bearer signed-in"]); }).pipe( Effect.provide( - Layer.mock(GitHubCredentials.GitHubCredentials)({ - get: (host) => - ++lookups === 1 - ? Effect.fail(new GitHubCredentials.GitHubNotSignedInError({ host })) - : Effect.succeed({ - host, - token: Redacted.make("signed-in"), - source: "gh" as const, - fingerprint: "fingerprint", - }), - }), + Layer.mergeAll( + Layer.mock(RepositoryIdentityResolver.RepositoryIdentityResolver)({ + resolve: () => Effect.succeed(null), + }), + Layer.mock(GitHubCredentials.GitHubCredentials)({ + get: (host) => + ++lookups === 1 + ? Effect.fail(new GitHubCredentials.GitHubNotSignedInError({ host })) + : Effect.succeed({ + host, + token: Redacted.make("signed-in"), + source: "gh" as const, + fingerprint: "fingerprint", + }), + }), + ), ), Effect.provideService( HttpClient.HttpClient, diff --git a/apps/server/src/assets/GitHubMediaFetch.ts b/apps/server/src/assets/GitHubMediaFetch.ts index 668f91b3d78ad..ffda0861b777a 100644 --- a/apps/server/src/assets/GitHubMediaFetch.ts +++ b/apps/server/src/assets/GitHubMediaFetch.ts @@ -14,6 +14,8 @@ import { import * as GitHubCredentials from "@t3tools/source-control-github/server/GitHubCredentials"; +import { workspaceGitHubCredential } from "../sourceControl/GitHubWorkspaceAccount.ts"; + /** * Everything a redirect leads to — the presigned object stores GitHub hands assets off to — * authorizes with its own signature, and some of them reject a request that also carries a @@ -50,11 +52,19 @@ const SVG_CONTENT_TYPE = "image/svg+xml"; // body cannot run script against this origin. const SVG_CONTENT_SECURITY_POLICY = "default-src 'none'; style-src 'unsafe-inline'; sandbox"; -const githubToken = GitHubCredentials.GitHubCredentials.pipe( - Effect.flatMap((credentials) => credentials.get("github.com")), - Effect.map((credential) => credential.token), - Effect.orElseSucceed(() => null), -); +/** The github.com credential the workspace's repository is read with; null still loads public media. */ +const githubToken = (cwd: string) => + workspaceGitHubCredential(cwd).pipe( + Effect.flatMap((credential) => + credential?.host === "github.com" + ? Effect.succeed(credential) + : GitHubCredentials.GitHubCredentials.pipe( + Effect.flatMap((credentials) => credentials.get("github.com")), + ), + ), + Effect.map((credential) => credential.token), + Effect.orElseSucceed(() => null), + ); const fetchFollowingRedirects = Effect.fn("GitHubMediaFetch.fetchFollowingRedirects")(function* ( url: string, @@ -92,7 +102,7 @@ export const githubMediaResponse = Effect.fn("GitHubMediaFetch.githubMediaRespon requestHeaders: Record, ) { // Both media hosts are served by github.com's account, which is the host `gh` stores it under. - const token = yield* githubToken; + const token = yield* githubToken(asset.cwd); const forwarded: Record = {}; for (const name of FORWARDED_REQUEST_HEADERS) { const value = requestHeaders[name]; diff --git a/apps/server/src/pullRequest/PullRequestService.test.ts b/apps/server/src/pullRequest/PullRequestService.test.ts index 21d947dcced8b..1c335f35c4c4f 100644 --- a/apps/server/src/pullRequest/PullRequestService.test.ts +++ b/apps/server/src/pullRequest/PullRequestService.test.ts @@ -15,6 +15,7 @@ import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; import * as Redacted from "effect/Redacted"; import * as Stream from "effect/Stream"; +import * as Queue from "effect/Queue"; import { HttpClient, HttpClientResponse } from "effect/http"; import * as TestClock from "effect/testing/TestClock"; import type { @@ -525,6 +526,7 @@ function fakeProvider( function makeService(input: { readonly settings?: ServerSettings; + readonly settingsChanges?: Stream.Stream; readonly projects: ReadonlyArray; readonly providers: ReadonlyArray; readonly resolveHandle?: SourceControlProviderRegistry.SourceControlProviderRegistry["Service"]["resolveHandle"]; @@ -537,6 +539,7 @@ function makeService(input: { Layer.mergeAll( Layer.mock(ServerSettingsService.ServerSettingsService)({ getSettings: Effect.succeed(input.settings ?? DEFAULT_SERVER_SETTINGS), + subscribeChanges: Effect.succeed(input.settingsChanges ?? Stream.empty), }), Layer.succeed( PullRequestProviderRegistry.PullRequestProviderRegistry, @@ -1422,7 +1425,12 @@ it.effect("routing verifies the current account on the requested host without ca providers: [ fakeProvider(SourceControlProviderKind.make("github"), { getRoutingIdentity: (input) => { - assert.deepStrictEqual(input, { cwd: "/a", host: "github.example.test" }); + // The repository names whose account to verify, on a host with one per owner. + assert.deepStrictEqual(input, { + cwd: "/a", + host: "github.example.test", + repository: "acme/web", + }); return Effect.succeed({ viewer, accountId: viewer === "first-account" ? "123" : "456", @@ -1672,13 +1680,14 @@ it.effect("keeps branch deletion on the credential that passed source repository const credentials = Layer.succeed( GitHubCredentials.GitHubCredentials, GitHubCredentials.GitHubCredentials.of({ - get: (host) => + get: (host, repository) => Effect.sync(() => ({ host, - token: Redacted.make(activeToken), + token: Redacted.make(repository === "acme/web" ? activeToken : "host-account"), source: "gh" as const, fingerprint: `${host}:${activeToken}`, })), + accountFor: () => Effect.succeed(undefined), invalidate: () => Effect.void, }), ); @@ -1752,7 +1761,7 @@ it.effect("keeps branch deletion on the credential that passed source repository Effect.provide( GitHubPullRequestApi.layer.pipe( // GraphQL reads go over `http` too, through the same credential and quota as REST. - Layer.provide( + Layer.provideMerge( GitHubRest.layer.pipe( Layer.provideMerge(Layer.mergeAll(credentials, http)), Layer.provideMerge(GitHubQuota.layer), @@ -3970,6 +3979,311 @@ it.effect("shares one cold viewer lookup across distinct concurrent lists", () = }), ); +it.effect("reads each repository as the account its scope names", () => + Effect.gen(function* () { + const viewerAsked: Array = []; + const searches: Array<{ readonly viewer: string; readonly repositories: string }> = []; + const statsAsked: Array> = []; + // Two tokens belong to the personal login; they must still use separate batches. + const accountOf = (repository: string | undefined) => + repository === "acme/web" || repository === "acme-labs/api" ? "work" : "personal"; + const service = yield* makeService({ + projects: [ + project({ id: "p1", title: "web", workspaceRoot: "/a", repository: "acme/web" }), + project({ id: "p2", title: "api", workspaceRoot: "/b", repository: "acme-labs/api" }), + project({ id: "p3", title: "docs", workspaceRoot: "/c", repository: "acme/docs" }), + project({ id: "p4", title: "blog", workspaceRoot: "/d", repository: "me/blog" }), + ], + providers: [ + fakeProvider(SourceControlProviderKind.make("github"), { + // The host's own account is the empty scope. + credentialScope: ({ repository }) => + Effect.succeed( + repository === "acme/docs" + ? "docs-token" + : accountOf(repository) === "work" + ? "work" + : "", + ), + getViewer: (input) => + Effect.sync(() => { + viewerAsked.push(input.repository); + return accountOf(input.repository); + }), + listChangeRequestsAcross: (input) => + Effect.sync(() => { + searches.push({ + viewer: input.viewer, + repositories: input.repositories.toSorted().join(","), + }); + return { + items: input.repositories.map((repository, index) => ({ + ...batchedChangeRequest(index + 1, repository, "2026-07-02T00:00:00Z"), + author: { login: accountOf(repository), name: null, avatarUrl: null }, + })), + truncated: false, + }; + }), + listChangeRequestStats: (input) => + Effect.sync(() => { + statsAsked.push(input.changeRequests.map((entry) => entry.repository)); + return []; + }), + }), + ], + }); + + const result = yield* service.list({ state: "open", involvement: "all" }); + assert.strictEqual(viewerAsked.length, 3); + // Each account's repositories share one search, whoever owns them. + assert.deepStrictEqual( + searches.toSorted((left, right) => left.viewer.localeCompare(right.viewer)), + [ + { viewer: "personal", repositories: "acme/docs" }, + { viewer: "personal", repositories: "me/blog" }, + { viewer: "work", repositories: "acme-labs/api,acme/web" }, + ], + ); + // Every row names who it was read as, so each is "authored" by its own account. + assert.deepStrictEqual( + result.entries + .map((entry) => `${entry.repository} ${entry.viewer} ${entry.author?.login}`) + .toSorted(), + [ + "acme-labs/api work work", + "acme/docs personal personal", + "acme/web work work", + "me/blog personal personal", + ], + ); + assert.strictEqual(result.providers.length, 1); + assert.strictEqual(result.providers[0]?.configured, true); + + yield* service.listStats({ + refs: result.entries.map(({ projectId, repository, number }) => ({ + projectId, + repository, + number, + })), + }); + assert.deepStrictEqual(statsAsked.map((repositories) => repositories.toSorted()).toSorted(), [ + ["acme-labs/api", "acme/web"], + ["acme/docs"], + ["me/blog"], + ]); + }), +); + +it.effect("reads an open pull request again once an account choice changes", () => + Effect.gen(function* () { + let viewers = 0; + let previews = 0; + let summaries = 0; + // The account the repository resolves to, as GitHub reads it from the same settings. + let account = ""; + const changes = yield* Queue.unbounded(); + const service = yield* makeService({ + settingsChanges: Stream.fromQueue(changes), + projects: [project({ id: "p1", title: "web", workspaceRoot: "/a", repository: "acme/web" })], + providers: [ + fakeProvider(SourceControlProviderKind.make("github"), { + credentialScope: () => Effect.sync(() => account), + getViewer: () => + Effect.sync(() => { + viewers++; + return account || "personal"; + }), + getChangeRequestPreview: () => + Effect.sync(() => { + previews += 1; + return { ...changeRequest(1, "2026-07-02T00:00:00Z"), title: account || "personal" }; + }), + getChangeRequestSummary: () => + Effect.sync(() => { + summaries += 1; + return { ...changeRequest(1, "2026-07-02T00:00:00Z"), title: account || "personal" }; + }), + }), + ], + }); + const ref = { projectId: "p1" as ProjectId, repository: "acme/web", number: 1 }; + const read = Effect.suspend(() => + Effect.all([ + service.list({ state: "open", involvement: "all" }), + service.preview(ref), + service.summary(ref, { recoverTransientFailure: false }), + ]), + ); + + yield* read; + yield* read; + assert.deepStrictEqual([viewers, previews, summaries], [1, 1, 1]); + const refreshed = yield* service.subscribeRefreshes.pipe( + Stream.runHead, + Effect.forkChild({ startImmediately: true }), + ); + + account = "work"; + yield* Queue.offer(changes, { + ...DEFAULT_SERVER_SETTINGS, + sourceControlHosts: { + github: { hosts: { "github.com": { enabled: true, owners: { acme: "work" } } } }, + }, + }); + yield* Fiber.join(refreshed); + const [, preview, summary] = yield* read; + assert.strictEqual(preview.title, "work"); + assert.strictEqual(summary.title, "work"); + assert.deepStrictEqual([viewers, previews, summaries], [2, 2, 2]); + }), +); + +it.effect("leaves only the repositories of an account that cannot be read unavailable", () => + Effect.gen(function* () { + const service = yield* makeService({ + projects: [ + project({ id: "p1", title: "web", workspaceRoot: "/a", repository: "acme/web" }), + project({ id: "p2", title: "blog", workspaceRoot: "/b", repository: "me/blog" }), + ], + providers: [ + fakeProvider(SourceControlProviderKind.make("github"), { + credentialScope: ({ repository }) => + Effect.succeed(repository.startsWith("acme/") ? "work" : ""), + getViewer: (input) => + input.repository?.startsWith("acme/") + ? Effect.fail( + new PullRequestProviderError({ + provider: SourceControlProviderKind.make("github"), + operation: "getViewer", + reason: "unauthenticated", + detail: "No GitHub credential for work.", + }), + ) + : Effect.succeed("personal"), + listChangeRequestsAcross: (input) => + Effect.succeed({ + items: input.repositories.map((repository) => + batchedChangeRequest(1, repository, "2026-07-02T00:00:00Z"), + ), + truncated: false, + }), + }), + ], + }); + + const result = yield* service.list({ state: "open", involvement: "all" }); + assert.deepStrictEqual( + result.entries.map((entry) => entry.repository), + ["me/blog"], + ); + assert.deepStrictEqual( + result.errors.map((error) => error.message), + ["acme/web could not be read."], + ); + assert.strictEqual(result.providers[0]?.configured, true); + }), +); + +it.effect("rechecks an empty search after switching the repository account", () => + Effect.gen(function* () { + let account = "personal"; + const changes = yield* Queue.unbounded(); + const service = yield* makeService({ + settingsChanges: Stream.fromQueue(changes), + projects: [project({ id: "p1", title: "web", workspaceRoot: "/a", repository: "acme/web" })], + providers: [ + fakeProvider(SourceControlProviderKind.make("github"), { + credentialScope: () => Effect.sync(() => account), + getViewer: () => Effect.sync(() => account), + listChangeRequestsAcross: () => + Effect.sync(() => ({ + items: + account === "personal" + ? [batchedChangeRequest(1, "acme/web", "2026-07-02T00:00:00Z")] + : [], + truncated: false, + })), + listChangeRequests: () => + Effect.succeed({ + items: [changeRequest(2, "2026-07-02T00:00:00Z")], + truncated: false, + continues: false, + }), + }), + ], + }); + const list = Effect.suspend(() => service.list({ state: "open", involvement: "all" })); + assert.deepStrictEqual( + (yield* list).entries.map((entry) => entry.number), + [1], + ); + const refreshed = yield* service.subscribeRefreshes.pipe( + Stream.runHead, + Effect.forkChild({ startImmediately: true }), + ); + account = "work"; + yield* Queue.offer(changes, { + ...DEFAULT_SERVER_SETTINGS, + sourceControlHosts: { + github: { hosts: { "github.com": { enabled: true, owners: { acme: "work" } } } }, + }, + }); + yield* Fiber.join(refreshed); + assert.deepStrictEqual( + (yield* list).entries.map((entry) => [entry.number, entry.viewer]), + [[2, "work"]], + ); + }), +); + +it.effect("bounds simultaneous credential lookups for a large project list", () => + Effect.gen(function* () { + let active = 0; + let peak = 0; + const projects = Array.from({ length: 40 }, (_, index) => + project({ + id: `p${index}`, + title: `Project ${index}`, + workspaceRoot: `/w/${index}`, + repository: `owner-${index}/web`, + }), + ); + const pending = yield* Queue.unbounded>(); + yield* Stream.fromQueue(pending).pipe( + Stream.runForEach((ready) => Deferred.succeed(ready, undefined)), + Effect.forkScoped, + ); + const service = yield* makeService({ + projects, + providers: [ + fakeProvider(SourceControlProviderKind.make("github"), { + credentialScope: () => + Effect.gen(function* () { + active++; + peak = Math.max(peak, active); + const ready = yield* Deferred.make(); + yield* Queue.offer(pending, ready); + yield* Deferred.await(ready); + active--; + return "account"; + }), + listChangeRequestsAcross: ({ repositories }) => + Effect.succeed({ + items: repositories.map((repository) => + batchedChangeRequest(1, repository, "2026-07-02T00:00:00Z"), + ), + truncated: false, + }), + }), + ], + }); + const result = yield* service.list({ state: "open", involvement: "all", limit: 100 }); + assert.strictEqual(result.entries.length, projects.length); + assert.isBelow(peak, projects.length); + assert.strictEqual(active, 0); + }), +); + it.effect("uses five host reads for the normal indexed-repository page workflow", () => Effect.gen(function* () { let viewerCalls = 0; diff --git a/apps/server/src/pullRequest/PullRequestService.ts b/apps/server/src/pullRequest/PullRequestService.ts index 328e32526c1ea..2c73b9ca4c890 100644 --- a/apps/server/src/pullRequest/PullRequestService.ts +++ b/apps/server/src/pullRequest/PullRequestService.ts @@ -309,6 +309,11 @@ export interface SupportedProject { */ readonly remote: string; readonly repositoryKey: string | null; + /** + * Which credential the repository is read with (the provider's `credentialScope`), empty for a + * provider with one account per host. Repositories with one share a viewer and batched reads. + */ + readonly credentialScope: string; } interface WorkspaceProjects { @@ -479,6 +484,7 @@ function withRateLimitBackoff( ? {} : { mergeMessageRewrite: api.mergeMessageRewrite }), ...(api.repositoryKey === undefined ? {} : { repositoryKey: api.repositoryKey }), + ...(api.credentialScope === undefined ? {} : { credentialScope: api.credentialScope }), getViewer: options?.viewerAllowsPause === true ? interactive("getViewer", api.getViewer) @@ -664,6 +670,14 @@ export const make = Effect.gen(function* () { ).pipe(Effect.map((resolved) => new Map(resolved))); }; + const withCredentialScope = ( + project: Omit, + ): Effect.Effect => + ( + project.api.credentialScope?.({ host: project.host, repository: project.repository }) ?? + Effect.succeed("") + ).pipe(Effect.map((credentialScope) => ({ ...project, credentialScope }))); + const listWorkspaceProjects = ( filter: Pick, ): Effect.Effect => @@ -699,7 +713,7 @@ export const make = Effect.gen(function* () { ), ), Effect.map(({ refinedProviders, projects }) => { - const supported: SupportedProject[] = []; + const supported: Array> = []; const unimplemented = new Map< string, { kind: SourceControlProviderKind; projectCount: number } @@ -759,6 +773,11 @@ export const make = Effect.gen(function* () { } return { supported, unimplemented, viewerRoots }; }), + Effect.flatMap(({ supported, ...rest }) => + Effect.forEach(supported, withCredentialScope, { + concurrency: REPOSITORY_CONCURRENCY, + }).pipe(Effect.map((scoped) => ({ ...rest, supported: scoped }))), + ), ); /** @@ -822,7 +841,7 @@ export const make = Effect.gen(function* () { ), ); if (Option.isSome(selected)) { - return { + return yield* withCredentialScope({ cursorKey: listCursorKey(host, repository), project: selected.value, api: withRateLimitBackoff(api, host, rateLimits), @@ -830,22 +849,20 @@ export const make = Effect.gen(function* () { host, remote: normalizeGitRemoteUrl(`https://${host}/${repository}`), repositoryKey: null, - }; + }); } } return yield* new PullRequestUnavailableError({ reason: "provider-unsupported" }); }); } - return Effect.succeed( - route.repositoryKey !== null || - route.repository.toLowerCase() === repository.toLowerCase() - ? route - : { - ...route, - repository, - remote: normalizeGitRemoteUrl(`https://${host}/${repository}`), - }, - ); + return route.repositoryKey !== null || + route.repository.toLowerCase() === repository.toLowerCase() + ? Effect.succeed(route) + : withCredentialScope({ + ...route, + repository, + remote: normalizeGitRemoteUrl(`https://${host}/${repository}`), + }); }), ); }), @@ -907,16 +924,23 @@ export const make = Effect.gen(function* () { type ResolvedViewer = { readonly host: string; + /** The provider's `credentialScope`, empty for a provider with one account per host. */ + readonly scope: string; readonly kind: SourceControlProviderKind; readonly viewer: string | null; readonly error: PullRequestProviderError | null; }; + // Every grouping of repositories goes through this key: a group is read with one credential, so + // it can share a viewer, a batched search and a line-count read without mixing two accounts. + const credentialKey = (host: string, scope: string) => `${host}\n${scope}`; const viewersByHost = new Map(); const viewerFlights = yield* Cache.makeWith( (key: string): Effect.Effect => { - const [host, kind, roots] = JSON.parse(key) as [ + const [host, kind, scope, repository, roots] = JSON.parse(key) as [ string, SourceControlProviderKind, + string, + string | null, ReadonlyArray, ]; const registered = registry.get(kind); @@ -924,19 +948,27 @@ export const make = Effect.gen(function* () { return Effect.die(new Error(`Missing pull request provider: ${kind}`)); } const api = withRateLimitBackoff(registered, host, rateLimits, { viewerAllowsPause: true }); - return Effect.firstSuccessOf(roots.map((cwd) => api.getViewer({ cwd, host }))).pipe( + return Effect.firstSuccessOf( + roots.map((cwd) => + api.getViewer({ cwd, host, ...(repository === null ? {} : { repository }) }), + ), + ).pipe( Effect.map((viewer) => ({ host, + scope, 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.map(Clock.currentTimeMillis, (at) => + viewersByHost.set(credentialKey(host, scope), { at, result }), + ), ), Effect.catch((error) => Effect.succeed({ host, + scope, kind, viewer: null, error, @@ -959,20 +991,32 @@ export const make = Effect.gen(function* () { options?: { readonly allowPaused: boolean }, ) => Effect.forEach( - [...new Set(projects.map(({ host }) => host))], - (host) => + [...new Set(projects.map((project) => credentialKey(project.host, project.credentialScope)))], + (accountKey) => Effect.flatMap(Clock.currentTimeMillis, (now): Effect.Effect => { - const held = viewersByHost.get(host); + const held = viewersByHost.get(accountKey); 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; + const forAccount = projects.filter( + (project) => credentialKey(project.host, project.credentialScope) === accountKey, + ); + const { host, api } = forAccount[0]!; + const scope = forAccount[0]!.credentialScope; // 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); - const key = JSON.stringify([host, api.kind, [...new Set(roots)].sort()]); + viewerRoots.get(host) ?? forAccount.map(({ project }) => project.workspaceRoot); + // A scoped provider names the account of the repository it is asked about. + const repository = + scope === "" ? null : forAccount.map((project) => project.repository).toSorted()[0]!; + const key = JSON.stringify([ + host, + api.kind, + scope, + repository, + [...new Set(roots)].sort(), + ]); if (options?.allowPaused === true) return Cache.get(viewerFlights, key); // The pause is checked here rather than inside the lookup, so that it holds back the // callers nobody is waiting on without splitting the flight they share with a press. @@ -983,6 +1027,7 @@ export const make = Effect.gen(function* () { Effect.catch((error) => Effect.succeed({ host, + scope, kind: api.kind, viewer: null, error: new PullRequestProviderError({ @@ -1036,6 +1081,7 @@ export const make = Effect.gen(function* () { const viewer = input.viewer.toLowerCase(); return { ...(input.item.stack === undefined ? {} : { stack: input.item.stack }), + viewer: input.viewer, provider: input.project.api.kind, host: input.project.host, projectId: input.project.project.id, @@ -1072,8 +1118,8 @@ export const make = Effect.gen(function* () { // authored/reviewing searches for that same repository are therefore real empty answers, not // a reason to issue the two-command per-repository fallback again. const searchVisibleAt = new Map(); - const searchVisibilityKey = (host: string, repository: string) => - `${host}\n${repository.trim().toLowerCase()}`; + const searchVisibilityKey = (host: string, repository: string, scope: string) => + `${host}\n${repository.trim().toLowerCase()}\n${scope}`; const listUncached: PullRequestService["Service"]["list"] = (input) => Effect.gen(function* () { @@ -1090,23 +1136,34 @@ export const make = Effect.gen(function* () { } const viewerResults = yield* resolveViewers(projects, viewerRoots); + const viewerByAccount = new Map(); + // The first account read on each host, for a client that predates rows carrying their own + // viewer. Rows read as another account on the host name theirs. const viewers: Record = {}; for (const result of viewerResults) { - if (result.viewer !== null) viewers[result.host] = result.viewer; + if (result.viewer === null) continue; + viewerByAccount.set(credentialKey(result.host, result.scope), result.viewer); + viewers[result.host] ??= result.viewer; } + const viewerFor = (project: SupportedProject): string | undefined => + viewerByAccount.get(credentialKey(project.host, project.credentialScope)); const providers: ReadonlyArray = [ - ...viewerResults.map((result) => { - const capabilities = projects.find((project) => project.host === result.host)?.api - .capabilities; + ...[...new Set(viewerResults.map((result) => result.host))].map((host) => { + const forHost = viewerResults.filter((result) => result.host === host); + const configured = forHost.some((result) => result.viewer !== null); + const error = configured + ? null + : (forHost.find((result) => result.error !== null)?.error ?? null); + const capabilities = projects.find((project) => project.host === host)?.api.capabilities; return { - host: result.host, - kind: result.kind, + host, + kind: forHost[0]!.kind, searchesOnHost: capabilities?.search ?? false, actions: capabilities?.actions ?? [], - projectCount: projectCounts.get(result.host) ?? 1, - configured: result.viewer !== null, - detail: result.error === null ? null : providerDetail(result.error), + projectCount: projectCounts.get(host) ?? 1, + configured, + detail: error === null ? null : providerDetail(error), }; }), ...[...unimplemented].map(([host, { kind, projectCount }]) => ({ @@ -1124,9 +1181,9 @@ export const make = Effect.gen(function* () { continuation === null ? projects : projects.filter(({ cursorKey }) => continuation.has(cursorKey)); - const readable = selected.filter(({ host }) => viewers[host] !== undefined); + const readable = selected.filter((project) => viewerFor(project) !== undefined); const unreadable = selected - .filter(({ host }) => viewers[host] === undefined) + .filter((project) => viewerFor(project) === undefined) .map(({ project, repository }) => ({ projectId: project.id, projectTitle: project.title, @@ -1134,7 +1191,12 @@ export const make = Effect.gen(function* () { })); if (readable.length === 0) { const errors = viewerResults.flatMap((result) => - result.error === null || !selected.some(({ host }) => host === result.host) + result.error === null || + !selected.some( + (project) => + credentialKey(project.host, project.credentialScope) === + credentialKey(result.host, result.scope), + ) ? [] : [result.error], ); @@ -1159,7 +1221,7 @@ export const make = Effect.gen(function* () { const readRepository = (project: SupportedProject): Effect.Effect => { { - const viewer = viewers[project.host]!; + const viewer = viewerFor(project)!; const key = project.cursorKey; const cursor = cursorOf(project); return project.api @@ -1231,7 +1293,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 = viewerFor(first)!; const cursor = cursorOf(first); return readAcross({ cwd: first.project.workspaceRoot, @@ -1261,7 +1323,10 @@ export const make = Effect.gen(function* () { const held = rows.get(key); if (held === undefined) rows.set(key, [item]); else held.push(item); - searchVisibleAt.set(searchVisibilityKey(first.host, item.repository), now); + searchVisibleAt.set( + searchVisibilityKey(first.host, item.repository, first.credentialScope), + now, + ); } const boundary = page.items.reduce( (oldest, item) => @@ -1278,7 +1343,7 @@ export const make = Effect.gen(function* () { // believed. Only on its first slice: after that it has a boundary to carry on // from, and silence past one means the rows are older rather than absent. const lastVisible = searchVisibleAt.get( - searchVisibilityKey(project.host, project.repository), + searchVisibilityKey(project.host, project.repository, project.credentialScope), ); const searchIsKnownVisible = !page.truncated && @@ -1328,7 +1393,7 @@ export const make = Effect.gen(function* () { separate.push(project); continue; } - const key = `${project.host}\n${cursorOf(project)?.updatedBefore ?? ""}`; + const key = `${credentialKey(project.host, project.credentialScope)}\n${cursorOf(project)?.updatedBefore ?? ""}`; const group = together.get(key); if (group === undefined) together.set(key, [project]); else group.push(project); @@ -1383,10 +1448,14 @@ export const make = Effect.gen(function* () { if (project === undefined || api?.getRoutingIdentity === undefined) { return yield* new PullRequestUnavailableError({ reason: "provider-unsupported" }); } + // The repository names whose account to verify, where a host has one per owner. An older + // client leaves it out and is answered for the host's own account, which is what it compares. + const repository = input.repository?.trim(); const identity = yield* api .getRoutingIdentity({ cwd: project.project.workspaceRoot, host, + ...(repository ? { repository } : {}), }) .pipe(Effect.mapError(toPullRequestError("routeIdentity"))); return { ...identity, host, provider: "github" as const }; @@ -1413,7 +1482,11 @@ export const make = Effect.gen(function* () { } const result = yield* api .withVerifiedCredential( - { cwd: project.project.workspaceRoot, host: project.host }, + { + cwd: project.project.workspaceRoot, + host: project.host, + repository: project.repository, + }, (identity) => identity.accountId === input.expectedAccountId ? operation.pipe(Effect.provideService(routingCredential, identity), Effect.result) @@ -1433,6 +1506,7 @@ export const make = Effect.gen(function* () { .getRoutingIdentity({ cwd: project.project.workspaceRoot, host: project.host, + repository: project.repository, }) .pipe(Effect.mapError(toPullRequestError("routeIdentity"))); if (!identity.viewer.trim() || !identity.accountId.trim()) { @@ -1920,7 +1994,11 @@ export const make = Effect.gen(function* () { return input.action === "delete-branch" && project.api.withVerifiedCredential !== undefined ? project.api .withVerifiedCredential( - { cwd: project.project.workspaceRoot, host: project.host }, + { + cwd: project.project.workspaceRoot, + host: project.host, + repository: project.repository, + }, () => operation.pipe(Effect.result), ) .pipe( @@ -2367,14 +2445,15 @@ export const make = Effect.gen(function* () { } wanted.set(`${project.project.id} ${ref.number}`, { project, number: ref.number }); } - const byHost = new Map>(); + const byAccount = 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 key = credentialKey(entry.project.host, entry.project.credentialScope); + const held = byAccount.get(key); + if (held === undefined) byAccount.set(key, [entry]); else held.push(entry); } const stats = yield* Effect.forEach( - [...byHost.values()], + [...byAccount.values()], (entries) => { const first = entries[0]!; const readStats = first.project.api.listChangeRequestStats; @@ -2638,6 +2717,7 @@ export const make = Effect.gen(function* () { project.repository.toLowerCase(), project.project.id, project.project.workspaceRoot, + project.credentialScope, String(input.number), input.expectedAccountId ?? "", input[credentialNamespace] ?? "", @@ -3241,10 +3321,10 @@ export const make = Effect.gen(function* () { Effect.gen(function* () { const ref = yield* canonicalRef(input); const credential = yield* routingCredential; + const fingerprint = + credential?.credentialFingerprint ?? (yield* requireProject(ref)).credentialScope; return yield* read( - credential === null - ? ref - : { ...ref, [credentialNamespace]: credential.credentialFingerprint }, + fingerprint === "" ? ref : { ...ref, [credentialNamespace]: fingerprint }, ...args, ); }); @@ -3334,6 +3414,33 @@ export const make = Effect.gen(function* () { ), ); + // Account choices decide who reads which repository, so a change to them drops everything read + // as the previous account at once: held logins, listings, and every pull request's reads. + const forgetAccountReads = Effect.suspend(() => { + projectEpochFloor = ++epochCounter; + projectEpochs.clear(); + mergedEpochs.clear(); + refEpochs.clear(); + return invalidate({}, { notifyReaders: true }); + }); + const sourceControlHostsOf = (settings: { readonly sourceControlHosts: unknown }) => + JSON.stringify(settings.sourceControlHosts); + const settingsChanges = yield* serverSettings.subscribeChanges; + let sourceControlHosts = yield* serverSettings.getSettings.pipe( + Effect.map(sourceControlHostsOf), + Effect.orElseSucceed(() => null), + ); + yield* settingsChanges.pipe( + Stream.map(sourceControlHostsOf), + Stream.filter((next) => { + const changed = next !== sourceControlHosts; + sourceControlHosts = next; + return changed; + }), + Stream.runForEach(() => forgetAccountReads), + Effect.forkIn(scope), + ); + return PullRequestService.of({ routing, routingIdentity, diff --git a/apps/server/src/sourceControl/GitHubWorkspaceAccount.ts b/apps/server/src/sourceControl/GitHubWorkspaceAccount.ts new file mode 100644 index 0000000000000..3415b0e2cb12d --- /dev/null +++ b/apps/server/src/sourceControl/GitHubWorkspaceAccount.ts @@ -0,0 +1,29 @@ +import * as Effect from "effect/Effect"; + +import * as GitHubCredentials from "@t3tools/source-control-github/server/GitHubCredentials"; +import { gitHubApiHostForRemote } from "@t3tools/source-control-github/server/gitHubRepositoryResolution"; + +import * as RepositoryIdentityResolver from "../project/RepositoryIdentityResolver.ts"; + +const workspaceGitHubRepository = Effect.fn("GitHubWorkspaceAccount.repository")(function* ( + cwd: string, +) { + const resolver = yield* RepositoryIdentityResolver.RepositoryIdentityResolver; + const identity = yield* resolver.resolve(cwd); + if (identity === null) return null; + const host = gitHubApiHostForRemote(identity.locator.remoteUrl); + const owner = identity.owner?.trim(); + const name = identity.name?.trim(); + return host === null || !owner || !name ? null : { host, repository: `${owner}/${name}` }; +}); + +export const workspaceGitHubCredential = Effect.fn("GitHubWorkspaceAccount.credential")(function* ( + cwd: string, +) { + const workspace = yield* workspaceGitHubRepository(cwd); + if (workspace === null) return null; + const credentials = yield* GitHubCredentials.GitHubCredentials; + return yield* credentials + .get(workspace.host, workspace.repository) + .pipe(Effect.orElseSucceed(() => null)); +}); diff --git a/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts b/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts index dc5a6505170bd..d92601146415e 100644 --- a/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts +++ b/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts @@ -138,7 +138,9 @@ function makeRegistry(input: { : ServerSettings.ServerSettingsService.layerTest(), Layer.mock(GitHubPullRequestApi.GitHubPullRequestApi)({}), // The GitHub provider's GraphQL reads go through these; no test here makes one. - Layer.mock(GitHubRequestPolicy.GitHubRequestPolicy)({}), + Layer.mock(GitHubRequestPolicy.GitHubRequestPolicy)({ + withRepositoryAccount: (_input, effect) => effect, + }), Layer.succeed( HttpClient.HttpClient, HttpClient.make(() => Effect.die(new Error("Unexpected GitHub GraphQL read"))), diff --git a/packages/client-runtime/src/state/pullRequestRouting.ts b/packages/client-runtime/src/state/pullRequestRouting.ts index b95b3d1ec7f47..20b3de6dd2307 100644 --- a/packages/client-runtime/src/state/pullRequestRouting.ts +++ b/packages/client-runtime/src/state/pullRequestRouting.ts @@ -409,8 +409,10 @@ export function createPullRequestRouter() { if (!(yield* routingAllowed(registry, origin.target.environmentId, id, writes.has(tag)))) return yield* visit(index + 1); // An older server would discard expectedAccountId. Verify it implements the guard first. + // The repository picks the account on a host that signs in per repository owner. const alternateProbe = request(WS_METHODS.pullRequestsRoutingIdentity, { host: identity.host, + repository: ref.repository, }).pipe( Effect.timeout("2 seconds"), Effect.catchCause((cause) => @@ -421,7 +423,11 @@ export function createPullRequestRouter() { .run( id, reads.has(tag) - ? shareMetadataRead(pendingIdentities, identity.host.toLowerCase(), alternateProbe) + ? shareMetadataRead( + pendingIdentities, + encodeKey([identity.host.toLowerCase(), ref.repository.toLowerCase()]), + alternateProbe, + ) : alternateProbe, ) .pipe( diff --git a/packages/client-runtime/src/state/pullRequests.test.ts b/packages/client-runtime/src/state/pullRequests.test.ts index 2bb7fdd82485a..d05d9586b307e 100644 --- a/packages/client-runtime/src/state/pullRequests.test.ts +++ b/packages/client-runtime/src/state/pullRequests.test.ts @@ -110,7 +110,8 @@ for (const scenario of [ input: unknown, ) => Effect.gen(function* () { - if (local) expect(input).toEqual({ host: "github.com" }); + // The repository picks the account on a host that signs in per owner. + if (local) expect(input).toEqual({ host: "github.com", repository: "acme/web" }); calls.push(`${name}:identity`); if ( local && diff --git a/packages/contracts/src/pullRequest.ts b/packages/contracts/src/pullRequest.ts index 2415c59f056e4..2a92e7e6e23d6 100644 --- a/packages/contracts/src/pullRequest.ts +++ b/packages/contracts/src/pullRequest.ts @@ -315,6 +315,12 @@ export type PullRequestStackMembership = typeof PullRequestStackMembership.Type; export const PullRequestListEntry = Schema.Struct({ stack: Schema.optional(PullRequestStackMembership), + /** + * The login this row was read as, which is what "authored" compares its author with. Absent + * from a server that predates it, where `viewers` names the host's one account. Two rows on one + * host can differ: GitHub reads each repository owner as the account Settings choose for it. + */ + viewer: Schema.optional(TrimmedNonEmptyString), provider: SourceControlProviderKind, host: TrimmedNonEmptyString, projectId: ProjectId, @@ -384,7 +390,8 @@ export type PullRequestListProjectError = typeof PullRequestListProjectError.Typ export const PullRequestListResult = Schema.Struct({ /** - * A host that could not be read is absent rather than present-and-undefined, because an + * The signed-in account per host, for rows without a `viewer` of their own; a host read as + * several accounts names the first. 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. */ viewers: Schema.Record(TrimmedNonEmptyString, TrimmedNonEmptyString), @@ -413,6 +420,12 @@ export type PullRequestRef = typeof PullRequestRef.Type; export const PullRequestRoutingIdentityInput = Schema.Struct({ host: TrimmedNonEmptyString, + /** + * The `owner/name` the routed request is about, for a host that signs in per repository owner. + * Absent asks for the host's own account; a server that predates it ignores it, answers for + * that account, and so is only chosen when the two accounts match. + */ + repository: Schema.optional(TrimmedNonEmptyString), }); export type PullRequestRoutingIdentityInput = typeof PullRequestRoutingIdentityInput.Type; diff --git a/packages/source-control-core/src/server/PullRequestProvider.ts b/packages/source-control-core/src/server/PullRequestProvider.ts index 882bc73c11534..f5a5d7dda9045 100644 --- a/packages/source-control-core/src/server/PullRequestProvider.ts +++ b/packages/source-control-core/src/server/PullRequestProvider.ts @@ -243,8 +243,12 @@ export interface ProviderRepositoryRef { } export interface PullRequestProviderApi { + /** + * `repository` names the repository the operation is about, for a host whose account depends + * on it (see `credentialScope`); absent asks for the host's own account. + */ readonly withVerifiedCredential?: ( - input: { readonly cwd: string; readonly host: string }, + input: { readonly cwd: string; readonly host: string; readonly repository?: string }, use: (identity: { readonly accountId: string; readonly viewer: string; @@ -254,6 +258,7 @@ export interface PullRequestProviderApi { readonly getRoutingIdentity?: (input: { readonly cwd: string; readonly host: string; + readonly repository?: string; }) => Effect.Effect< { readonly accountId: string; readonly viewer: string }, PullRequestProviderError @@ -270,9 +275,21 @@ export interface PullRequestProviderApi { */ readonly repositoryKey?: (input: { readonly canonicalKey: string }) => string; + /** + * Which credential a repository is read with, for a host whose settings choose one per + * repository rather than once. Repositories with the same scope share a viewer and may share + * one batched read. Absent means one account for the whole host. + */ + readonly credentialScope?: (input: { + readonly host: string; + readonly repository: string; + }) => Effect.Effect; + + /** `repository` picks the account for a host with a `credentialScope`. */ readonly getViewer: (input: { readonly cwd: string; readonly host?: string; + readonly repository?: string; }) => Effect.Effect; readonly listChangeRequests: ( @@ -302,6 +319,7 @@ export interface PullRequestProviderApi { * the answer: the newest `limit` rows across every repository named, which is exactly the * rows a page ordered by update shows. `cursor` is one boundary for all of them, so a caller * with repositories standing at different boundaries asks in groups rather than in one call. + * Every repository named reads as the same viewer, which grouping by `credentialScope` ensures. */ readonly listChangeRequestsAcross?: (input: { readonly cwd: string; @@ -316,6 +334,7 @@ export interface PullRequestProviderApi { readonly filters?: PullRequestListFilters | undefined; }) => Effect.Effect; + /** One call reads as one account, like `listChangeRequestsAcross`. */ readonly listChangeRequestStats?: (input: { readonly cwd: string; readonly host: string; diff --git a/packages/source-control-github/src/client/definition.ts b/packages/source-control-github/src/client/definition.ts index 66c1cd003968e..3062a74076427 100644 --- a/packages/source-control-github/src/client/definition.ts +++ b/packages/source-control-github/src/client/definition.ts @@ -9,6 +9,7 @@ import { } from "@t3tools/contracts"; import * as Effect from "effect/Effect"; import * as Schema from "effect/Schema"; +import * as SchemaTransformation from "effect/SchemaTransformation"; import { defineSourceControlClient, isChangeRequestInProjectRepository, @@ -22,12 +23,51 @@ const CHANGE_REQUEST_REFERENCE = const KIND = SourceControlProviderKind.make("github"); +/** + * An owner, or an `owner/name` repository, lowercased on decode: GitHub treats `PumpCade` and + * `pumpcade` as one. + */ +const GitHubRuleKey = TrimmedNonEmptyString.pipe( + Schema.decodeTo(Schema.String, SchemaTransformation.toLowerCase()), +); + export const GitHubHostChoice = Schema.Struct({ account: Schema.optionalKey(TrimmedNonEmptyString), enabled: Schema.Boolean.pipe(Schema.withDecodingDefault(Effect.succeed(true))), + /** The login that reads each owner's repositories, overriding `account` for them. */ + owners: Schema.optionalKey(Schema.Record(GitHubRuleKey, TrimmedNonEmptyString)), + /** The login that reads one `owner/name` repository, overriding its owner's. */ + repositories: Schema.optionalKey(Schema.Record(GitHubRuleKey, TrimmedNonEmptyString)), }); export type GitHubHostChoice = typeof GitHubHostChoice.Type; +/** + * The logins Settings choose for a repository on a host, given as `owner/name` or just its owner, + * most specific first: the repository's own rule, its owner's, then the host's pinned account. + * None leaves the choice to `gh`; a login `gh` no longer holds gives way to the next. + */ +export function gitHubAccountsFor( + choice: GitHubHostChoice | undefined, + repository: string | null | undefined, +): ReadonlyArray { + const key = repository?.trim().toLowerCase(); + const owner = key?.split("/")[0]; + const chosen = [ + key && key.includes("/") ? choice?.repositories?.[key] : undefined, + owner ? choice?.owners?.[owner] : undefined, + choice?.account, + ].filter((account): account is string => account !== undefined); + return [...new Set(chosen)]; +} + +/** The most specific login Settings choose for a repository on a host, if any. */ +export function gitHubAccountFor( + choice: GitHubHostChoice | undefined, + repository: string | null | undefined, +): string | undefined { + return gitHubAccountsFor(choice, repository)[0]; +} + export const settings = makeProviderSettingsSchema({ hosts: Schema.Record(GitHubHost, GitHubHostChoice).pipe( Schema.withDecodingDefault(Effect.succeed({})), diff --git a/packages/source-control-github/src/server/GitHubCredentials.test.ts b/packages/source-control-github/src/server/GitHubCredentials.test.ts index aa4f67ce3c5b6..742f70e528526 100644 --- a/packages/source-control-github/src/server/GitHubCredentials.test.ts +++ b/packages/source-control-github/src/server/GitHubCredentials.test.ts @@ -18,15 +18,20 @@ const TOKEN_VARIABLES = [ "GITHUB_ENTERPRISE_TOKEN", ] as const; -const hostSettings = ( - hosts: Record, -) => +type HostChoice = { + readonly account?: string; + readonly enabled?: boolean; + readonly owners?: Record; + readonly repositories?: Record; +}; + +const hostSettings = (hosts: Record) => Object.fromEntries( Object.entries(hosts).map(([host, choice]) => [host, { enabled: true, ...choice }]), ); function harness( - hosts: Record = {}, + hosts: Record = {}, signedOut: ReadonlyArray = [], tokens: Record = {}, ) { @@ -109,10 +114,16 @@ describe("GitHubCredentials", () => { 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" } }); + const { layer, calls } = harness({ + "github.com": { + account: "personal", + owners: { acme: "work" }, + repositories: { "acme/web": "docs" }, + }, + }); return Effect.gen(function* () { const credentials = yield* GitHubCredentials.GitHubCredentials; - const credential = yield* credentials.get("github.com"); + const credential = yield* credentials.get("github.com", "acme/web"); expect(Redacted.value(credential.token)).toBe("from-env"); expect(credential.source).toBe("env"); expect(calls).toEqual([]); @@ -146,6 +157,36 @@ describe("GitHubCredentials", () => { }).pipe(Effect.provide(layer)); }); + it.effect( + "resolves repository, owner, host and active credentials in order, skipping stale rules", + () => { + const { layer } = harness( + { + "github.com": { + account: "personal", + owners: { acme: "work", departed: "gone" }, + repositories: { "acme/handbook": "docs", "acme/legacy": "gone" }, + }, + "ghe.example.test": { account: "gone", owners: { acme: "gone" } }, + }, + ["gone"], + ); + return Effect.gen(function* () { + const credentials = yield* GitHubCredentials.GitHubCredentials; + const tokenFor = (host: string, repository: string) => + credentials + .get(host, repository) + .pipe(Effect.map((credential) => Redacted.value(credential.token))); + expect(yield* tokenFor("GitHub.com", "Acme/Handbook")).toBe("token-for-docs"); + expect(yield* tokenFor("github.com", "acme/web")).toBe("token-for-work"); + expect(yield* tokenFor("github.com", "acme/legacy")).toBe("token-for-work"); + expect(yield* tokenFor("github.com", "departed/web")).toBe("token-for-personal"); + expect(yield* tokenFor("github.com", "other/web")).toBe("token-for-personal"); + expect(yield* tokenFor("ghe.example.test", "acme/web")).toBe("active-token"); + }).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* () { @@ -160,10 +201,14 @@ describe("GitHubCredentials", () => { 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" }); + const { layer, calls } = harness( + { "github.com": { owners: { acme: "work" }, repositories: { "acme/web": "docs" } } }, + [], + { "github.com": "saved-token" }, + ); return Effect.gen(function* () { const credentials = yield* GitHubCredentials.GitHubCredentials; - const credential = yield* credentials.get("github.com"); + const credential = yield* credentials.get("github.com", "acme/web"); expect(Redacted.value(credential.token)).toBe("saved-token"); expect(credential.source).toBe("settings"); expect(calls).toEqual([]); diff --git a/packages/source-control-github/src/server/GitHubCredentials.ts b/packages/source-control-github/src/server/GitHubCredentials.ts index b2d0b7d0ea34b..75716e2c216f8 100644 --- a/packages/source-control-github/src/server/GitHubCredentials.ts +++ b/packages/source-control-github/src/server/GitHubCredentials.ts @@ -18,6 +18,7 @@ import { readSourceControlHostSettings } from "@t3tools/source-control-core/clie import type { ServerSettings } from "@t3tools/contracts"; import * as GitHubClient from "../client/definition.ts"; +import { gitHubAccountFor, gitHubAccountsFor } from "../client/definition.ts"; const TOKEN_TTL = Duration.minutes(5); const MISSING_TTL = Duration.seconds(10); @@ -30,6 +31,11 @@ export interface GitHubCredential { readonly token: Redacted.Redacted; readonly source: GitHubCredentialSource; readonly fingerprint: string; + /** + * The login Settings picked and `gh` handed over, set only when one was chosen and held. Absent + * for gh's active login, an environment token, or one saved in Settings. + */ + readonly account?: string; } export class GitHubCliMissingError extends Schema.TaggedError()( @@ -79,9 +85,18 @@ export type GitHubCredentialUnavailableError = export class GitHubCredentials extends Context.Service< GitHubCredentials, { + /** + * The token for a repository on `host`, given as `owner/name` or just its owner. Without one, + * or for one Settings have no rule for, it is the host's account. + */ readonly get: ( host: string, + repository?: string | null, ) => Effect.Effect; + readonly accountFor: ( + host: string, + repository: string | null, + ) => Effect.Effect; readonly invalidate: (host: string) => Effect.Effect; } >()("@t3tools/source-control-github/server/GitHubCredentials") {} @@ -91,7 +106,7 @@ function normalizeHost(host: string): string { } /** Hosts gh treats as GitHub.com-like for `GH_TOKEN`: github.com and GHE.com data residency. */ -function isGitHubDotCom(host: string): boolean { +export function isGitHubDotCom(host: string): boolean { return host === "github.com" || host.endsWith(".ghe.com"); } @@ -183,25 +198,33 @@ export const make = Effect.gen(function* () { Effect.orElseSucceed(() => null), ); - const cacheKey = (host: string, account: string | undefined) => - account === undefined ? host : `${host}\u0000${account}`; + const cacheKey = (host: string, accounts: ReadonlyArray) => + [host, ...accounts].join("\u0000"); const lookup = Effect.fn("GitHubCredentials.lookup")(function* (key: string) { - const [host = key, choice] = key.split("\u0000"); + const [host = key, ...accounts] = key.split("\u0000"); const fromEnv = environmentToken(host, environment); - const token = - fromEnv ?? - (yield* fromGh(host, choice).pipe( - Effect.catchTags({ - GitHubNotSignedInError: (error) => - choice === undefined ? Effect.fail(error) : fromGh(host, undefined), - }), - )); + let token = fromEnv; + let account: string | undefined; + if (token === null) { + for (const choice of accounts) { + const selected = yield* fromGh(host, choice).pipe( + Effect.catchTags({ GitHubNotSignedInError: () => Effect.succeed(null) }), + ); + if (selected !== null) { + token = selected; + account = choice; + break; + } + } + token ??= yield* fromGh(host, undefined); + } return { host, token: Redacted.make(token), source: fromEnv !== null ? "env" : "gh", fingerprint: yield* fingerprintOf(host, token), + ...(account === undefined ? {} : { account }), } satisfies GitHubCredential; }); @@ -218,7 +241,7 @@ export const make = Effect.gen(function* () { }); return GitHubCredentials.of({ - get: Effect.fn("GitHubCredentials.get")(function* (rawHost) { + get: Effect.fn("GitHubCredentials.get")(function* (rawHost, repository) { const host = normalizeHost(rawHost); const choice = yield* hostChoice(host); if (choice?.enabled === false) { @@ -233,12 +256,22 @@ export const make = Effect.gen(function* () { fingerprint: yield* fingerprintOf(host, saved), } satisfies GitHubCredential; } - return yield* Cache.get(cache, cacheKey(host, choice?.account)); + return yield* Cache.get(cache, cacheKey(host, gitHubAccountsFor(choice, repository))); }), + accountFor: (rawHost, repository) => + hostChoice(normalizeHost(rawHost)).pipe( + Effect.map((choice) => gitHubAccountFor(choice, repository)), + ), invalidate: (rawHost) => { const host = normalizeHost(rawHost); - return hostChoice(host).pipe( - Effect.flatMap((choice) => Cache.invalidate(cache, cacheKey(host, choice?.account))), + return Cache.keys(cache).pipe( + Effect.flatMap((keys) => + Effect.forEach( + [...keys].filter((key) => key === host || key.startsWith(`${host}\u0000`)), + (key) => Cache.invalidate(cache, key), + { discard: true }, + ), + ), ); }, }); diff --git a/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts b/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts index 705be8fb556bb..ea873ce30e29a 100644 --- a/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts +++ b/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts @@ -169,6 +169,7 @@ const layer = it.layer( Layer.succeed(GitHubCredentials.GitHubCredentials, { get: (host) => Effect.map(mockedCredential(host), (held) => ({ ...held, host, source: "gh" as const })), + accountFor: () => Effect.succeed(undefined), invalidate: () => Effect.void, }), ), @@ -633,6 +634,7 @@ it.effect( source: "gh" as const, fingerprint: `${host}:${activeToken.length}${activeToken.at(-1)}`, })), + accountFor: () => Effect.succeed(undefined), invalidate: () => Effect.void, }), ); diff --git a/packages/source-control-github/src/server/GitHubPullRequestProvider.test.ts b/packages/source-control-github/src/server/GitHubPullRequestProvider.test.ts index 7073c288f06ab..bab0a7cbc12f5 100644 --- a/packages/source-control-github/src/server/GitHubPullRequestProvider.test.ts +++ b/packages/source-control-github/src/server/GitHubPullRequestProvider.test.ts @@ -13,6 +13,19 @@ import type { GitHubPullRequestCore } from "./gitHubPullRequestJson.ts"; import { gitHubViewerPermissions, loginAvatarUrl, make } from "./GitHubPullRequestProvider.ts"; import type { GitHubReviewThreadComments } from "./gitHubPullRequestJson.ts"; +const mockGitHubPullRequestApi = Layer.mock(GitHubPullRequestApi.GitHubPullRequestApi); +/** + * The adapter's API, with a policy that runs every call as it is: these tests are about how the + * API's answers map, not about whose account reads them. + */ +const mockApi = (api: Parameters[0]) => + Layer.merge( + mockGitHubPullRequestApi(api), + Layer.mock(GitHubRequestPolicy.GitHubRequestPolicy)({ + withRepositoryAccount: (_input, effect) => effect, + }), + ); + const coreFields = { reviewRequestTeams: [], checksTruncated: false, @@ -32,7 +45,7 @@ it.effect("maps credential verification failures without relabeling operation fa let operations = 0; const provider = yield* make.pipe( Effect.provide( - Layer.mock(GitHubPullRequestApi.GitHubPullRequestApi)({ + mockApi({ revalidateChecks: (_input, read) => read, withVerifiedCredential: (_input, use) => verificationFails @@ -69,7 +82,7 @@ it.effect("advertises standalone branch deletion for source repository writers", let action: string | undefined; const provider = yield* make.pipe( Effect.provide( - Layer.mock(GitHubPullRequestApi.GitHubPullRequestApi)({ + mockApi({ runPullRequestAction: (input) => Effect.sync(() => { action = input.action; @@ -118,7 +131,7 @@ it.effect("refreshes checks without permissions or comparison reads", () => ); const provider = yield* make.pipe( Effect.provide( - Layer.mock(GitHubPullRequestApi.GitHubPullRequestApi)({ + mockApi({ revalidateChecks: (_input, read) => read, getPullRequestDetail: () => Effect.sync(() => { @@ -161,7 +174,7 @@ it.effect("uses one narrow read for a linked pull request summary", () => let summaryReads = 0; const provider = yield* make.pipe( Effect.provide( - Layer.mock(GitHubPullRequestApi.GitHubPullRequestApi)({ + mockApi({ revalidateChecks: (_input, read) => read, getPullRequestSummary: () => Effect.sync(() => { @@ -210,7 +223,7 @@ it.effect("declares host-native stacks and passes the one the CLI reads through" }; const provider = yield* make.pipe( Effect.provide( - Layer.mock(GitHubPullRequestApi.GitHubPullRequestApi)({ + mockApi({ revalidateChecks: (_input, read) => read, getPullRequestStack: (input) => Effect.succeed(input.number === 7 ? stack : null), }), @@ -230,7 +243,7 @@ it.effect("reports a failed stack read against its own operation", () => Effect.gen(function* () { const provider = yield* make.pipe( Effect.provide( - Layer.mock(GitHubPullRequestApi.GitHubPullRequestApi)({ + mockApi({ revalidateChecks: (_input, read) => read, getPullRequestStack: () => Effect.fail( @@ -379,7 +392,7 @@ describe("gitHubViewerPermissions", () => { } }).pipe( Effect.provide( - Layer.mock(GitHubPullRequestApi.GitHubPullRequestApi)({ + mockApi({ revalidateChecks: (_input, read) => read, getPullRequestDetail: () => Effect.succeed({ @@ -473,7 +486,7 @@ describe("gitHubViewerPermissions", () => { ]); }).pipe( Effect.provide( - Layer.mock(GitHubPullRequestApi.GitHubPullRequestApi)({ + mockApi({ revalidateChecks: (_input, read) => read, getPullRequestDetail: () => Effect.succeed({ @@ -577,7 +590,7 @@ it.effect( Effect.gen(function* () { const provider = yield* make.pipe( Effect.provide( - Layer.mock(GitHubPullRequestApi.GitHubPullRequestApi)({ + mockApi({ getPullRequestDetail: () => Effect.succeed({ ...openDetail, comparison: { behindBy: 2, viewerCanUpdate: true } }), listWorkflowRunsRequiringApproval: () => @@ -618,7 +631,7 @@ it.effect("does not classify same-repository gates as fork workflow approvals", expect(detail.checks).toEqual([]); }).pipe( Effect.provide( - Layer.mock(GitHubPullRequestApi.GitHubPullRequestApi)({ + mockApi({ revalidateChecks: (_input, read) => read, getPullRequestDetail: () => Effect.succeed({ ...openDetail, isCrossRepository: false }), listWorkflowRunsRequiringApproval: () => @@ -657,7 +670,7 @@ it.effect("keeps an unsafe workflow approval scope visible as unknown", () => ]); }).pipe( Effect.provide( - Layer.mock(GitHubPullRequestApi.GitHubPullRequestApi)({ + mockApi({ revalidateChecks: (_input, read) => read, getPullRequestDetail: () => Effect.succeed(openDetail), listWorkflowRunsRequiringApproval: () => @@ -699,7 +712,7 @@ it.effect("propagates workflow discovery rate limits", () => expect(error.reason).toBe("rate-limited"); }).pipe( Effect.provide( - Layer.mock(GitHubPullRequestApi.GitHubPullRequestApi)({ + mockApi({ revalidateChecks: (_input, read) => read, getPullRequestDetail: () => Effect.succeed(openDetail), listWorkflowRunsRequiringApproval: () => @@ -748,7 +761,7 @@ describe("getViewerPermissions", () => { expect(permissions.actions).not.toContain("update-branch"); }).pipe( Effect.provide( - Layer.mock(GitHubPullRequestApi.GitHubPullRequestApi)({ + mockApi({ revalidateChecks: (_input, read) => read, getPullRequestDetail: () => Effect.die("Unexpected detail read"), getViewerAccess: () => Effect.sync(() => (accessReads++, access)), @@ -761,7 +774,7 @@ describe("getViewerPermissions", () => { detail: Effect.Effect, onDetail: (allowReserve: boolean) => void = () => {}, ) => - Layer.mock(GitHubPullRequestApi.GitHubPullRequestApi)({ + mockApi({ revalidateChecks: (_input, read) => read, getPullRequestDetail: () => SourceControlRateLimit.Interactive.pipe( @@ -824,7 +837,7 @@ describe("getViewerPermissions", () => { expect(permissions.actions).not.toContain("update-branch"); }).pipe( Effect.provide( - Layer.mock(GitHubPullRequestApi.GitHubPullRequestApi)({ + mockApi({ revalidateChecks: (_input, read) => read, getPullRequestDetail: () => Effect.fail( @@ -903,7 +916,7 @@ describe("getChangeRequest commits", () => { }; const layerWith = (commits: GitHubReviewThreadComments["commits"]) => - Layer.mock(GitHubPullRequestApi.GitHubPullRequestApi)({ + mockApi({ revalidateChecks: (_input, read) => read, getPullRequestActivity: () => Effect.succeed({ @@ -990,7 +1003,7 @@ describe("getChangeRequestActivity dismissed reviews", () => { viewer: { canUpdate: true, didAuthor: false }, }; const layerFor = (body: string) => - Layer.mock(GitHubPullRequestApi.GitHubPullRequestApi)({ + mockApi({ revalidateChecks: (_input, read) => read, getPullRequestActivity: () => Effect.succeed({ author: null, comments: [dismissedReview(body)], commits: [] }), @@ -1074,7 +1087,7 @@ describe("editing", () => { ]); }).pipe( Effect.provide( - Layer.mock(GitHubPullRequestApi.GitHubPullRequestApi)({ + mockApi({ revalidateChecks: (_input, read) => read, updatePullRequest: (input) => Effect.sync(() => void rewrites.push(input)), updateComment: (input) => Effect.sync(() => void rewrites.push(input)), diff --git a/packages/source-control-github/src/server/GitHubPullRequestProvider.ts b/packages/source-control-github/src/server/GitHubPullRequestProvider.ts index 7ab10f2639490..bd35bf00f5ae4 100644 --- a/packages/source-control-github/src/server/GitHubPullRequestProvider.ts +++ b/packages/source-control-github/src/server/GitHubPullRequestProvider.ts @@ -11,6 +11,7 @@ import type { import * as SourceControlRateLimit from "@t3tools/source-control-core/server/SourceControlRateLimit"; import * as GitHubPullRequestApi from "./GitHubPullRequestApi.ts"; +import * as GitHubRequestPolicy from "./GitHubRequestPolicy.ts"; import { PullRequestProviderError, type PullRequestProviderFailure, @@ -172,6 +173,7 @@ const rendersEmpty = (body: string): boolean => export const make = Effect.gen(function* () { const cli = yield* GitHubPullRequestApi.GitHubPullRequestApi; + const policy = yield* GitHubRequestPolicy.GitHubRequestPolicy; const fail = (operation: string) => (error: GitHubPullRequestApi.GitHubPullRequestApiError) => new PullRequestProviderError({ @@ -233,20 +235,35 @@ export const make = Effect.gen(function* () { ), ); - const provider: PullRequestProviderApi = { + const asRepository = (input: { + readonly host: string; + readonly repository?: string | undefined; + }) => ({ host: input.host, repository: input.repository?.trim() || null }); + + const provider = { kind: SourceControlProviderKind.make("github"), capabilities: CAPABILITIES, mergeMessageRewrite: removeAgentCredits, + // Repositories read with one credential share a viewer and batched reads. + credentialScope: ({ host, repository }) => policy.credentialScope(host, repository), getRoutingIdentity: (input) => - cli.getRoutingIdentity(input).pipe(Effect.mapError(fail("routeIdentity"))), + policy + .withRepositoryAccount(asRepository(input), cli.getRoutingIdentity(input)) + .pipe(Effect.mapError(fail("routeIdentity"))), withVerifiedCredential: (input, use) => - cli - .withVerifiedCredential(input, (identity) => use(identity).pipe(Effect.result)) + policy + .withRepositoryAccount( + asRepository(input), + cli.withVerifiedCredential(input, (identity) => use(identity).pipe(Effect.result)), + ) .pipe(Effect.mapError(fail("routeIdentity")), Effect.flatMap(Effect.fromResult)), getViewer: (input) => - cli - .getViewerLogin({ cwd: input.cwd, host: input.host ?? "github.com" }) + policy + .withRepositoryAccount( + asRepository({ host: input.host ?? "github.com", repository: input.repository }), + cli.getViewerLogin({ cwd: input.cwd, host: input.host ?? "github.com" }), + ) .pipe(Effect.mapError(fail("getViewer"))), listChangeRequests: (input) => @@ -631,7 +648,58 @@ export const make = Effect.gen(function* () { resolved: input.resolved, }) .pipe(Effect.mapError(fail("setThreadResolution"))), - }; + } satisfies PullRequestProviderApi; + + // Batched callers group by credentialScope, so every repository shares the first one's token. + const owned = + ( + call: (input: I) => Effect.Effect, + repositoryOf: (input: I) => string | undefined, + ) => + (input: I) => + policy.withRepositoryAccount( + asRepository({ host: input.host, repository: repositoryOf(input) }), + call(input), + ); + const inRepository = ( + call: (input: I) => Effect.Effect, + ) => owned(call, (input) => input.repository); - return provider; + return { + ...provider, + listChangeRequests: inRepository(provider.listChangeRequests), + listChangeRequestsAcross: owned( + provider.listChangeRequestsAcross, + (input) => input.repositories[0], + ), + listChangeRequestStats: owned( + provider.listChangeRequestStats, + (input) => input.changeRequests[0]?.repository, + ), + getChangeRequestSummary: inRepository(provider.getChangeRequestSummary), + getChangeRequestStack: inRepository(provider.getChangeRequestStack), + getChangeRequestWatchFingerprint: inRepository(provider.getChangeRequestWatchFingerprint), + getChangeRequestPreview: inRepository(provider.getChangeRequestPreview), + getChangeRequestChecks: inRepository(provider.getChangeRequestChecks), + getChangeRequest: inRepository(provider.getChangeRequest), + getChangeRequestActivity: inRepository(provider.getChangeRequestActivity), + getReviewThreadComments: inRepository(provider.getReviewThreadComments), + getViewerPermissions: inRepository(provider.getViewerPermissions), + getDiff: inRepository(provider.getDiff), + getDiffFileContents: inRepository(provider.getDiffFileContents), + getFilesViewed: inRepository(provider.getFilesViewed), + setFilesViewed: inRepository(provider.setFilesViewed), + listReviewerCandidates: inRepository(provider.listReviewerCandidates), + setReviewerRequest: inRepository(provider.setReviewerRequest), + listLabelCandidates: inRepository(provider.listLabelCandidates), + setLabels: inRepository(provider.setLabels), + runAction: inRepository(provider.runAction), + updateChangeRequest: inRepository(provider.updateChangeRequest), + comment: inRepository(provider.comment), + updateComment: inRepository(provider.updateComment), + submitReview: inRepository(provider.submitReview), + replyToThread: inRepository(provider.replyToThread), + setReaction: inRepository(provider.setReaction), + setThreadResolution: inRepository(provider.setThreadResolution), + } satisfies PullRequestProviderApi; }); diff --git a/packages/source-control-github/src/server/GitHubRequestPolicy.ts b/packages/source-control-github/src/server/GitHubRequestPolicy.ts index 831ba010b9c84..055ff5306971f 100644 --- a/packages/source-control-github/src/server/GitHubRequestPolicy.ts +++ b/packages/source-control-github/src/server/GitHubRequestPolicy.ts @@ -32,6 +32,19 @@ export const PinnedGitHubCredential = Context.Reference<{ defaultValue: () => null, }); +/** + * Which repository the requests inside are about, set by `withRepositoryAccount`: the credential + * is the one Settings choose for `repository` (`owner/name`, or just an owner), and `credential` + * is the fingerprint of the one it is actually read with, which GraphQL batches split by so one + * batch never mixes two credentials' lookups. + */ +export const GitHubCredentialRepository = Context.Reference<{ + readonly repository: string; + readonly credential: string | undefined; +} | null>("@t3tools/source-control-github/server/GitHubRequestPolicy/GitHubCredentialRepository", { + defaultValue: () => null, +}); + export class GitHubApiRequestError extends Schema.TaggedError()( "GitHubApiRequestError", { host: Schema.String, operation: Schema.String, cause: Schema.Defect() }, @@ -224,13 +237,30 @@ export class GitHubRequestPolicy extends Context.Service< input: GitHubRequest, send: (headers: Readonly>) => Effect.Effect, E, R>, ) => Effect.Effect; - /** The credential a request to `host` would carry right now. */ + /** The credential a request to `host` about `repository` would carry right now. */ readonly credential: ( host: string, + repository?: string | null, ) => Effect.Effect< { readonly token: Redacted.Redacted; readonly fingerprint: string }, GitHubApiError >; + /** + * Runs `effect` as the account Settings choose for `repository` (`owner/name`, or just an + * owner) on `host`, so every request inside it carries that account's credential. A pinned + * credential, such as a routed operation's verified one, still wins, and so does a repository + * set further out: a checkout of a fork's pull request reads the fork as the account of the + * repository it was opened against. + */ + readonly withRepositoryAccount: ( + input: { readonly host: string; readonly repository: string | null }, + effect: Effect.Effect, + ) => Effect.Effect; + /** + * The fingerprint of the credential `repository` is read with on `host`, after any chosen + * login `gh` no longer holds gave way; empty when there is none, which the read then reports. + */ + readonly credentialScope: (host: string, repository: string) => Effect.Effect; } >()("@t3tools/source-control-github/server/GitHubRequestPolicy") {} @@ -243,7 +273,7 @@ export const make = Effect.gen(function* () { const credential: GitHubRequestPolicy["Service"]["credential"] = Effect.fn( "GitHubRest.credential", - )(function* (host) { + )(function* (host, repository) { const normalized = normalizeHost(host); const pinned = yield* PinnedGitHubCredential; if (pinned !== null) { @@ -256,10 +286,37 @@ export const make = Effect.gen(function* () { } return { token: pinned.token, fingerprint: pinned.credentialFingerprint }; } - const held = yield* credentials.get(normalized); + const held = yield* credentials.get( + normalized, + repository ?? (yield* GitHubCredentialRepository)?.repository ?? null, + ); return { token: held.token, fingerprint: held.fingerprint }; }); + const credentialScope: GitHubRequestPolicy["Service"]["credentialScope"] = (host, repository) => + credential(host, repository).pipe( + Effect.map((credential) => credential.fingerprint), + Effect.orElseSucceed(() => ""), + ); + + const withRepositoryAccount: GitHubRequestPolicy["Service"]["withRepositoryAccount"] = ( + input, + effect, + ) => + Effect.gen(function* () { + const repository = input.repository; + if (repository === null || (yield* GitHubCredentialRepository) !== null) { + return yield* effect; + } + const credential = yield* credentialScope(input.host, repository); + return yield* effect.pipe( + Effect.provideService(GitHubCredentialRepository, { + repository, + credential: credential === "" ? undefined : credential, + }), + ); + }); + const request: GitHubRequestPolicy["Service"]["request"] = ( input: GitHubRequest, send: (headers: Readonly>) => Effect.Effect, E, R>, @@ -351,7 +408,12 @@ export const make = Effect.gen(function* () { .pipe(Effect.provideService(SourceControlRateLimit.CredentialScope, scope || fingerprint)); }); - return GitHubRequestPolicy.of({ request, credential }); + return GitHubRequestPolicy.of({ + request, + credential, + withRepositoryAccount, + credentialScope, + }); }); export const layer = Layer.effect(GitHubRequestPolicy, make); diff --git a/packages/source-control-github/src/server/GitHubRest.test.ts b/packages/source-control-github/src/server/GitHubRest.test.ts index c8f0779a854d5..9881a6f779514 100644 --- a/packages/source-control-github/src/server/GitHubRest.test.ts +++ b/packages/source-control-github/src/server/GitHubRest.test.ts @@ -38,6 +38,7 @@ function harness(respond: (request: HttpClientRequest.HttpClientRequest) => Resp fingerprint: `${host}:${tokens[0]}`, }; }), + accountFor: () => Effect.succeed(undefined), invalidate: () => Effect.sync(() => { invalidations++; diff --git a/packages/source-control-github/src/server/GitHubRest.ts b/packages/source-control-github/src/server/GitHubRest.ts index 6ed65eebabf77..3ffbde31bf75f 100644 --- a/packages/source-control-github/src/server/GitHubRest.ts +++ b/packages/source-control-github/src/server/GitHubRest.ts @@ -48,9 +48,10 @@ export class GitHubRest extends Context.Service< GitHubRest, { readonly rest: (input: GitHubRestInput) => Effect.Effect; - /** The credential a request to `host` would carry right now. */ + /** The credential a request to `host` about `repository` would carry right now. */ readonly credential: ( host: string, + repository?: string | null, ) => Effect.Effect< { readonly token: Redacted.Redacted; readonly fingerprint: string }, GitHubApiError diff --git a/packages/source-control-github/src/server/GitHubSourceControlProvider.test.ts b/packages/source-control-github/src/server/GitHubSourceControlProvider.test.ts index d60ae1641f47a..b478d1fbe1512 100644 --- a/packages/source-control-github/src/server/GitHubSourceControlProvider.test.ts +++ b/packages/source-control-github/src/server/GitHubSourceControlProvider.test.ts @@ -233,6 +233,7 @@ function harness(input: { source: "gh" as const, fingerprint: `${host}:token`, })), + accountFor: () => Effect.succeed(undefined), invalidate: () => Effect.void, }), GitHubQuota.layer, diff --git a/packages/source-control-github/src/server/GitHubSourceControlProvider.ts b/packages/source-control-github/src/server/GitHubSourceControlProvider.ts index 8c8aa06cd9a02..5b8b98812b69f 100644 --- a/packages/source-control-github/src/server/GitHubSourceControlProvider.ts +++ b/packages/source-control-github/src/server/GitHubSourceControlProvider.ts @@ -450,6 +450,7 @@ const contextHost = (context: SourceControlProvider.SourceControlProviderContext export const make = Effect.gen(function* () { const api = yield* GitHubRest.GitHubRest; + const policy = yield* GitHubRequestPolicy.GitHubRequestPolicy; const sourceControlHost = yield* SourceControlHost.SourceControlHost; const process = sourceControlHost.process; const environment = yield* HostProcess.Environment; @@ -531,6 +532,16 @@ export const make = Effect.gen(function* () { const rest = (input: GitHubRest.GitHubRestInput, notFoundDetail?: string) => api.rest(input).pipe(Effect.mapError((error) => fromGitHubApiError(error, notFoundDetail))); + /** Runs `effect` as the account Settings choose for the repository. */ + const asAccountOf = ( + locator: Pick, + effect: Effect.Effect, + ) => + policy.withRepositoryAccount( + { host: locator.host, repository: `${locator.owner}/${locator.name}` }, + effect, + ); + /** * Head lookups to one host under one credential share aliased documents: the resolver batches by * the host's endpoint and `batchKey`. Each lookup has its own `repository` root, so lookups on @@ -562,25 +573,28 @@ export const make = Effect.gen(function* () { const limit = Math.min(Math.max(Math.trunc(input.limit), 1), 100); const ownerMatch = /^([^:/\s]+):(.+)$/u.exec(input.headSelector.trim()); const headRefName = ownerMatch?.[2] ?? input.headSelector.trim(); - const result = yield* graphqlRead( - { - host: locator.host, - operation: "listPullRequestsByHead", - allowReserve: input.allowReserve, - // Up to 50 heads of 100 rows each. A default branch such as `main` can match a hundred - // fork pull requests, so the usual cap would cut the answer short. - maxResponseBytes: HEAD_LOOKUP_MAX_RESPONSE_BYTES, - }, - "GitHub returned an invalid change request list.", - GraphQLResolver.request( + const result = yield* asAccountOf( + locator, + graphqlRead( { - owner: locator.owner, - name: locator.name, - head: headRefName, - states: GRAPHQL_STATES[input.state], - first: ownerMatch ? OWNER_HEAD_SCAN_LIMIT : limit, + host: locator.host, + operation: "listPullRequestsByHead", + allowReserve: input.allowReserve, + // Up to 50 heads of 100 rows each. A default branch such as `main` can match a hundred + // fork pull requests, so the usual cap would cut the answer short. + maxResponseBytes: HEAD_LOOKUP_MAX_RESPONSE_BYTES, }, - input.allowReserve ? interactiveHeadResolver : backgroundHeadResolver, + "GitHub returned an invalid change request list.", + GraphQLResolver.request( + { + owner: locator.owner, + name: locator.name, + head: headRefName, + states: GRAPHQL_STATES[input.state], + first: ownerMatch ? OWNER_HEAD_SCAN_LIMIT : limit, + }, + input.allowReserve ? interactiveHeadResolver : backgroundHeadResolver, + ), ), ); if (result.repository === null) return yield* notFound("The repository could not be read."); @@ -623,14 +637,17 @@ export const make = Effect.gen(function* () { } const locator = parsed.kind === "url" ? parsed.locator : yield* resolveRepository(input); const decodeDetail = "GitHub returned an invalid pull request."; - const result = yield* graphqlRead( - { host: locator.host, operation: "getPullRequest", allowReserve: true }, - decodeDetail, - client.PullRequestByNumber({ - owner: locator.owner, - name: locator.name, - number: parsed.number, - }), + const result = yield* asAccountOf( + locator, + graphqlRead( + { host: locator.host, operation: "getPullRequest", allowReserve: true }, + decodeDetail, + client.PullRequestByNumber({ + owner: locator.owner, + name: locator.name, + number: parsed.number, + }), + ), ); const node = result.repository?.pullRequest; if (node == null) return yield* notFound("The pull request does not exist."); @@ -644,14 +661,17 @@ export const make = Effect.gen(function* () { const readRepository = Effect.fn("GitHubSourceControlProvider.readRepository")(function* ( locator: GitHubRepositoryLocator, ) { - const response = yield* rest( - { - host: locator.host, - operation: "getRepository", - path: `repos/${encodeURIComponent(locator.owner)}/${encodeURIComponent(locator.name)}`, - allowReserve: true, - }, - REPOSITORY_NOT_FOUND, + const response = yield* asAccountOf( + locator, + rest( + { + host: locator.host, + operation: "getRepository", + path: `repos/${encodeURIComponent(locator.owner)}/${encodeURIComponent(locator.name)}`, + allowReserve: true, + }, + REPOSITORY_NOT_FOUND, + ), ); const decoded = decodeRawRepository(response.body); if (Result.isFailure(decoded)) { @@ -746,7 +766,11 @@ export const make = Effect.gen(function* () { const known = remoteFor(headNameWithOwner); if (known !== null) return known; const [owner, name] = headNameWithOwner.split("/"); - const fork = yield* readRepository({ host: base.host, owner: owner!, name: name! }); + // A private fork is visible to whoever can see the repository it was opened against. + const fork = yield* asAccountOf( + base, + readRepository({ host: base.host, owner: owner!, name: name! }), + ); const originUrl = yield* git.readConfigValue(input.cwd, "remote.origin.url"); return yield* git.ensureRemote({ cwd: input.cwd, @@ -900,7 +924,10 @@ export const make = Effect.gen(function* () { input.url.pathname, ); if (!match) return undefined; - return readLinkSubject(input, `repos/${match[1]}/${match[2]}/issues/${match[3]}`); + return asAccountOf( + { host: input.url.host, owner: match[1]!, name: match[2]! }, + readLinkSubject(input, `repos/${match[1]}/${match[2]}/issues/${match[3]}`), + ); }, listChangeRequests: (input) => (input.state === "open" ? Effect.succeed(true) : SourceControlRateLimit.Interactive).pipe( @@ -937,22 +964,25 @@ export const make = Effect.gen(function* () { failure("The pull request description could not be read.", cause), ), ); - yield* rest({ - host: locator.host, - operation: "createPullRequest", - method: "POST", - allowReserve: true, - path: `repos/${encodeURIComponent(locator.owner)}/${encodeURIComponent(locator.name)}/pulls`, - // `owner:branch` is how the REST API takes a fork's head, the same as `gh --head`. - // gh allows maintainer edits unless told otherwise; the API's default is not documented. - body: { - base: input.baseRefName, - head: input.headSelector, - title: input.title, - body, - maintainer_can_modify: true, - }, - }); + yield* asAccountOf( + locator, + rest({ + host: locator.host, + operation: "createPullRequest", + method: "POST", + allowReserve: true, + path: `repos/${encodeURIComponent(locator.owner)}/${encodeURIComponent(locator.name)}/pulls`, + // `owner:branch` is how the REST API takes a fork's head, the same as `gh --head`. + // gh allows maintainer edits unless told otherwise; the API's default is not documented. + body: { + base: input.baseRefName, + head: input.headSelector, + title: input.title, + body, + maintainer_can_modify: true, + }, + }), + ); }).pipe( Effect.mapError( providerError("createChangeRequest", input.cwd, { reference: input.headSelector }), @@ -977,12 +1007,12 @@ export const make = Effect.gen(function* () { providerError("getRepositoryCloneUrls", input.cwd, { repository: input.repository }), ), ), - createRepository: (input) => - Effect.gen(function* () { - const locator = parseGitHubRepositorySelector( - input.repository, - (environment.GH_HOST ?? "github.com").toLowerCase(), - ); + createRepository: (input) => { + const locator = parseGitHubRepositorySelector( + input.repository, + (environment.GH_HOST ?? "github.com").toLowerCase(), + ); + const create = Effect.gen(function* () { const viewer = locator === null ? null : yield* readViewerLogin(locator.host); const owner = locator?.owner ?? viewer; const name = locator?.name ?? input.repository.trim(); @@ -1006,11 +1036,14 @@ export const make = Effect.gen(function* () { return yield* failure("GitHub returned an invalid repository.", decoded.failure); } return repositoryCloneUrls(decoded.success); - }).pipe( + }); + // An organization's repository is created by the account Settings choose for it. + return (locator === null ? create : asAccountOf(locator, create)).pipe( Effect.mapError( providerError("createRepository", input.cwd, { repository: input.repository }), ), - ), + ); + }, getDefaultBranch: (input) => Effect.gen(function* () { const locator = yield* resolveRepository({ diff --git a/packages/source-control-github/src/server/gitHubGraphQlClient.test.ts b/packages/source-control-github/src/server/gitHubGraphQlClient.test.ts index 785ae1ac98bae..dd0c4a557e93c 100644 --- a/packages/source-control-github/src/server/gitHubGraphQlClient.test.ts +++ b/packages/source-control-github/src/server/gitHubGraphQlClient.test.ts @@ -12,6 +12,7 @@ import * as GitHubRest from "./GitHubRest.ts"; import * as GitHubCredentials from "./GitHubCredentials.ts"; import * as GitHubGraphQlClient from "./gitHubGraphQlClient.ts"; import * as GitHubQuota from "./GitHubQuota.ts"; +import * as GitHubRequestPolicy from "./GitHubRequestPolicy.ts"; import { PullRequestBatchesGraphQLGroup } from "./pullRequestBatches.graphql.ts"; const NOW = Date.parse("2026-10-05T12:00:00.000Z"); @@ -28,6 +29,7 @@ function harness(respond: (request: HttpClientRequest.HttpClientRequest) => Resp source: "gh" as const, fingerprint: `${host}:secret`, }), + accountFor: () => Effect.succeed(undefined), invalidate: () => Effect.sync(() => { invalidations++; @@ -108,6 +110,90 @@ describe("GitHubGraphQlAuth", () => { }, ); + it.effect( + "batches a repository's lookups only with lookups read with the same credential", + () => { + const requests: Array = []; + const layer = GitHubRest.layer.pipe( + Layer.provideMerge( + Layer.mergeAll( + Layer.succeed(GitHubCredentials.GitHubCredentials, { + get: (host, repository) => + Effect.succeed({ + host, + token: Redacted.make(repository === "acme/web" ? "work-token" : "personal-token"), + source: "gh" as const, + fingerprint: `${host}:${repository === "acme/web" ? "work" : "personal"}`, + }), + // Every repository names a login gh no longer holds, and each falls back to the + // credential above: batches follow that credential, not the name. + accountFor: () => Effect.succeed("gone"), + invalidate: () => Effect.void, + }), + Layer.succeed( + HttpClient.HttpClient, + HttpClient.make((request) => { + requests.push(request); + return Effect.succeed( + HttpClientResponse.fromWeb( + request, + json({ + data: { + _0_repository: { pullRequest: stats }, + _1_repository: { pullRequest: stats }, + }, + }), + ), + ); + }), + ), + GitHubQuota.layer, + SourceControlRateLimit.layer, + ), + ), + ); + return Effect.gen(function* () { + const policy = yield* GitHubRequestPolicy.GitHubRequestPolicy; + const client = yield* GraphQLClient.make( + PullRequestBatchesGraphQLGroup.middleware(GitHubGraphQlClient.GitHubGraphQlAuth), + ).pipe(Effect.provide(GitHubGraphQlClient.layer)); + const resolver = GraphQLResolver.aliased(client.PullRequestStats, { + batchKey: GitHubGraphQlClient.batchKey, + }); + const lookup = (owner: string, number: number) => + policy.withRepositoryAccount( + { host: "github.com", repository: `${owner}/web` }, + GraphQLResolver.request({ owner, name: "web", number }, resolver).pipe( + Effect.provideService(GitHubGraphQlClient.GitHubGraphQlTarget, { + host: "github.com", + operation: "stats", + }), + ), + ); + yield* Effect.all([lookup("acme", 1), lookup("someone", 2), lookup("other", 3)], { + concurrency: "unbounded", + }); + expect(requests.map((request) => request.headers["authorization"]).toSorted()).toEqual([ + "Bearer personal-token", + "Bearer work-token", + ]); + const bodyFor = (token: string) => { + const body = requests.find( + (request) => request.headers["authorization"] === `Bearer ${token}`, + )?.body; + expect(body?._tag).toBe("Uint8Array"); + return body?._tag === "Uint8Array" ? new TextDecoder().decode(body.body) : ""; + }; + expect(bodyFor("work-token")).toContain('"acme"'); + expect(bodyFor("work-token")).not.toContain('"someone"'); + expect(bodyFor("work-token")).not.toContain('"other"'); + expect(bodyFor("personal-token")).toContain('"someone"'); + expect(bodyFor("personal-token")).toContain('"other"'); + expect(bodyFor("personal-token")).not.toContain('"acme"'); + }).pipe(Effect.provide(layer)); + }, + ); + it.effect("pauses the host on a RATE_LIMITED answer sent with HTTP 200", () => { const reset = Math.floor(NOW / 1000) + 600; const { layer, requests } = harness(() => diff --git a/packages/source-control-github/src/server/gitHubGraphQlClient.ts b/packages/source-control-github/src/server/gitHubGraphQlClient.ts index f1f6ee8944e0b..2925e26104790 100644 --- a/packages/source-control-github/src/server/gitHubGraphQlClient.ts +++ b/packages/source-control-github/src/server/gitHubGraphQlClient.ts @@ -217,14 +217,17 @@ const layerProtocol = GraphQLProtocol.layerHttp({ export const layer = Layer.merge(GitHubGraphQlAuth.layer, layerProtocol); /** - * What decides which lookups to one host may share a batched request: the credential and - * rate-limit scope it will be sent under. Read from the context the lookup was made in. Resolvers - * already keep hosts apart, since they batch by the transport's URL too. + * What decides which lookups to one host may share a batched request: the credential, the + * credential the repositories are read with, and the rate-limit scope it will be sent under. + * Read from the context the lookup was made in, which is the context a batch then runs in. + * Resolvers already keep hosts apart, since they batch by the transport's URL too. */ export const batchKey = (context: Context.Context) => JSON.stringify([ Context.getOrElse(context, GitHubRequestPolicy.PinnedGitHubCredential, () => null) ?.credentialFingerprint ?? null, + Context.getOrElse(context, GitHubRequestPolicy.GitHubCredentialRepository, () => null) + ?.credential ?? null, Context.getOrElse(context, SourceControlRateLimit.CredentialScope, () => ""), ]); diff --git a/packages/source-control-github/src/server/githubStackActions.test.ts b/packages/source-control-github/src/server/githubStackActions.test.ts index 44cb6446d3833..0d17bf4d0f2bd 100644 --- a/packages/source-control-github/src/server/githubStackActions.test.ts +++ b/packages/source-control-github/src/server/githubStackActions.test.ts @@ -95,6 +95,7 @@ const runGitHubStackAction = (send: Send, input: Parameters Effect.succeed(undefined), invalidate: () => Effect.void, }), GitHubQuota.layer, diff --git a/packages/source-control-github/src/server/githubStackRebase.ts b/packages/source-control-github/src/server/githubStackRebase.ts index 2b854dec12e40..a097bd14bb066 100644 --- a/packages/source-control-github/src/server/githubStackRebase.ts +++ b/packages/source-control-github/src/server/githubStackRebase.ts @@ -45,7 +45,7 @@ export const cascadeRebaseStack = Effect.fn("cascadeRebaseStack")(function* (inp const api = yield* GitHubRest.GitHubRest; const { process } = yield* SourceControlHost.SourceControlHost; const fileSystem = yield* FileSystem.FileSystem; - const { token } = yield* api.credential(input.host); + const { token } = yield* api.credential(input.host, input.repository); const first = input.layers[0]; if (first === undefined) return 0; const directory = yield* fileSystem.makeTempDirectoryScoped({ prefix: "t3-stack-rebase-" }).pipe( From 0f05bf1a8d27fd1853549bf14171adc1055267e9 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 19 Oct 2026 07:21:47 +0000 Subject: [PATCH 2/8] fix(github): resolve repository credentials across pull request operations --- .../src/server/GitHubRequestPolicy.ts | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/packages/source-control-github/src/server/GitHubRequestPolicy.ts b/packages/source-control-github/src/server/GitHubRequestPolicy.ts index 055ff5306971f..10048615a6596 100644 --- a/packages/source-control-github/src/server/GitHubRequestPolicy.ts +++ b/packages/source-control-github/src/server/GitHubRequestPolicy.ts @@ -39,8 +39,10 @@ export const PinnedGitHubCredential = Context.Reference<{ * batch never mixes two credentials' lookups. */ export const GitHubCredentialRepository = Context.Reference<{ + readonly host: string; readonly repository: string; readonly credential: string | undefined; + readonly token: Redacted.Redacted | undefined; } | null>("@t3tools/source-control-github/server/GitHubRequestPolicy/GitHubCredentialRepository", { defaultValue: () => null, }); @@ -286,9 +288,17 @@ export const make = Effect.gen(function* () { } return { token: pinned.token, fingerprint: pinned.credentialFingerprint }; } + const selected = yield* GitHubCredentialRepository; + if ( + selected?.host === normalized && + selected.token !== undefined && + selected.credential !== undefined + ) { + return { token: selected.token, fingerprint: selected.credential }; + } const held = yield* credentials.get( normalized, - repository ?? (yield* GitHubCredentialRepository)?.repository ?? null, + repository ?? (selected?.host === normalized ? selected.repository : null), ); return { token: held.token, fingerprint: held.fingerprint }; }); @@ -305,14 +315,17 @@ export const make = Effect.gen(function* () { ) => Effect.gen(function* () { const repository = input.repository; - if (repository === null || (yield* GitHubCredentialRepository) !== null) { + const host = normalizeHost(input.host); + if (repository === null || (yield* GitHubCredentialRepository)?.host === host) { return yield* effect; } - const credential = yield* credentialScope(input.host, repository); + const held = yield* credential(host, repository).pipe(Effect.orElseSucceed(() => null)); return yield* effect.pipe( Effect.provideService(GitHubCredentialRepository, { + host, repository, - credential: credential === "" ? undefined : credential, + credential: held?.fingerprint, + token: held?.token, }), ); }); From 8999d61d28a5d6fd12813a1f6b38fb2e8b60f4d1 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 19 Oct 2026 08:25:38 +0000 Subject: [PATCH 3/8] fix(github): ignore inherited account rules --- packages/source-control-github/src/client/definition.ts | 8 ++++++-- .../src/server/GitHubCredentials.test.ts | 2 ++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/source-control-github/src/client/definition.ts b/packages/source-control-github/src/client/definition.ts index 3062a74076427..3305d8a8ac3ee 100644 --- a/packages/source-control-github/src/client/definition.ts +++ b/packages/source-control-github/src/client/definition.ts @@ -53,8 +53,12 @@ export function gitHubAccountsFor( const key = repository?.trim().toLowerCase(); const owner = key?.split("/")[0]; const chosen = [ - key && key.includes("/") ? choice?.repositories?.[key] : undefined, - owner ? choice?.owners?.[owner] : undefined, + key && key.includes("/") && choice?.repositories && Object.hasOwn(choice.repositories, key) + ? choice.repositories[key] + : undefined, + owner && choice?.owners && Object.hasOwn(choice.owners, owner) + ? choice.owners[owner] + : undefined, choice?.account, ].filter((account): account is string => account !== undefined); return [...new Set(chosen)]; diff --git a/packages/source-control-github/src/server/GitHubCredentials.test.ts b/packages/source-control-github/src/server/GitHubCredentials.test.ts index 742f70e528526..2f422e3517d36 100644 --- a/packages/source-control-github/src/server/GitHubCredentials.test.ts +++ b/packages/source-control-github/src/server/GitHubCredentials.test.ts @@ -182,6 +182,8 @@ describe("GitHubCredentials", () => { expect(yield* tokenFor("github.com", "acme/legacy")).toBe("token-for-work"); expect(yield* tokenFor("github.com", "departed/web")).toBe("token-for-personal"); expect(yield* tokenFor("github.com", "other/web")).toBe("token-for-personal"); + expect(yield* tokenFor("github.com", "constructor/repo")).toBe("token-for-personal"); + expect(yield* tokenFor("github.com", "toString/repo")).toBe("token-for-personal"); expect(yield* tokenFor("ghe.example.test", "acme/web")).toBe("active-token"); }).pipe(Effect.provide(layer)); }, From 18d7abe4173c30f3321a1b6542ed8bf20ef74ecf Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 19 Oct 2026 09:00:08 +0000 Subject: [PATCH 4/8] test(github): preserve pull request media account scope --- apps/server/src/assets/AssetAccess.test.ts | 71 ++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/apps/server/src/assets/AssetAccess.test.ts b/apps/server/src/assets/AssetAccess.test.ts index 4f2f493652282..ea70ea7c424b5 100644 --- a/apps/server/src/assets/AssetAccess.test.ts +++ b/apps/server/src/assets/AssetAccess.test.ts @@ -4,6 +4,7 @@ import * as NodeHttpPlatform from "@effect/platform-node/NodeHttpPlatform"; import * as NodeFSP from "node:fs/promises"; import { AssetAccessError, + AssetResource, AssetPreviewTypeValidationError, ThreadId, TurnItemId, @@ -21,12 +22,14 @@ import * as Path from "effect/Path"; import * as PlatformError from "effect/PlatformError"; import * as Schema from "effect/Schema"; import * as TestClock from "effect/testing/TestClock"; +import * as ChildProcessSpawner from "effect/process/ChildProcessSpawner"; import { HttpClient, HttpClientResponse, HttpServerResponse } from "effect/http"; import { vi } from "vite-plus/test"; import * as ServerSecretStore from "../auth/ServerSecretStore.ts"; import * as ServerConfig from "../config.ts"; import * as Orchestrator from "../orchestration-v2/Orchestrator.ts"; +import * as ProcessRunner from "../processRunner.ts"; import * as ProjectFaviconResolver from "../project/ProjectFaviconResolver.ts"; import * as RepositoryIdentityResolver from "../project/RepositoryIdentityResolver.ts"; import * as T3ProjectFileLoader from "../project/T3ProjectFileLoader.ts"; @@ -48,6 +51,7 @@ vi.mock("node:fs/promises", async (importOriginal) => { const layerConfig = ServerConfig.ServerConfig.layerTest(process.cwd(), { prefix: "t3-asset-access-test-", }); +const decodeAssetResource = Schema.decodeUnknownSync(AssetResource); const screenshotPng = new Uint8Array(24); screenshotPng.set([ 0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0, 0, 0, 13, 0x49, 0x48, 0x44, 0x52, @@ -1514,6 +1518,73 @@ describe("AssetAccess", () => { const encoded = yield* Schema.encodeEffect(Schema.fromJsonString(AssetAccessError))(error); expect(encoded).not.toContain(url); } + + const authorizations: Array = []; + const mediaHttpClient = HttpClient.make((request) => { + authorizations.push(request.headers.authorization); + return Effect.succeed( + HttpClientResponse.fromWeb( + request, + new Response(null, { headers: { "content-type": "image/png" } }), + ), + ); + }); + const processLayer = Layer.mock(ProcessRunner.ProcessRunner)({ + run: ({ args }) => + Effect.succeed({ + stdout: args.includes("rev-parse") + ? "/repo\n" + : args.includes("remote") + ? "origin\thttps://github.com/workspace/app.git (fetch)\n" + : "", + stderr: "", + code: ChildProcessSpawner.ExitCode(0), + timedOut: false, + stdoutTruncated: false, + stderrTruncated: false, + stdoutInvalidUtf8: false, + stderrInvalidUtf8: false, + }), + }); + const accounts = Layer.mergeAll( + Layer.effect( + RepositoryIdentityResolver.RepositoryIdentityResolver, + RepositoryIdentityResolver.make(), + ).pipe(Layer.provide(processLayer)), + Layer.mock(GitHubCredentials.GitHubCredentials)({ + get: (host, repository) => + repository === "dependency/library" || repository === "workspace/app" + ? Effect.succeed({ + host, + token: Redacted.make( + repository === "dependency/library" ? "dependency-token" : "workspace-token", + ), + source: "gh" as const, + fingerprint: repository, + }) + : Effect.fail(new GitHubCredentials.GitHubNotSignedInError({ host })), + }), + ); + + for (const repository of [undefined, "dependency/library"]) { + const resource = decodeAssetResource({ + _tag: "github-media", + cwd: "/repo", + url: "https://github.com/user-attachments/assets/1a1842fb-6383-492f-873c-57aa0033fa6c", + ...(repository === undefined ? {} : { repository }), + }); + const issued = yield* issueAssetUrl({ resource }); + const asset = yield* resolve(issued.relativeUrl); + if (asset?.kind !== "github-media") throw new Error("Expected signed GitHub media"); + if (repository !== undefined) expect.soft(asset).toMatchObject({ repository }); + const response = yield* githubMediaResponse(asset, {}).pipe( + Effect.provide(accounts), + Effect.provideService(HttpClient.HttpClient, mediaHttpClient), + Effect.provideService(HostProcess.Environment, {}), + ); + expect(response.status).toBe(200); + } + expect(authorizations).toEqual(["Bearer workspace-token", "Bearer dependency-token"]); }).pipe(Effect.provide(layerTest)), ); }); From 10696f737c95102c0cd610eea056046ba9b596a7 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 19 Oct 2026 09:11:19 +0000 Subject: [PATCH 5/8] fix(github): align workspace and pull request media credentials --- apps/server/src/assets/AssetAccess.test.ts | 2 +- apps/server/src/assets/AssetAccess.ts | 6 ++ apps/server/src/assets/GitHubMediaFetch.ts | 23 +++++-- .../src/project/RepositoryIdentityResolver.ts | 61 ++++++++++++++++++- .../sourceControl/GitHubWorkspaceAccount.ts | 11 ++-- packages/contracts/src/assets.ts | 3 +- .../src/server/GitHubSourceControlProvider.ts | 9 +-- .../src/server/gitHubRepositoryResolution.ts | 8 +++ 8 files changed, 101 insertions(+), 22 deletions(-) diff --git a/apps/server/src/assets/AssetAccess.test.ts b/apps/server/src/assets/AssetAccess.test.ts index ea70ea7c424b5..053142865a76a 100644 --- a/apps/server/src/assets/AssetAccess.test.ts +++ b/apps/server/src/assets/AssetAccess.test.ts @@ -202,7 +202,7 @@ describe("AssetAccess", () => { Effect.provide( Layer.mergeAll( Layer.mock(RepositoryIdentityResolver.RepositoryIdentityResolver)({ - resolve: () => Effect.succeed(null), + resolveGitHubRepository: () => Effect.succeed(null), }), Layer.mock(GitHubCredentials.GitHubCredentials)({ get: (host) => diff --git a/apps/server/src/assets/AssetAccess.ts b/apps/server/src/assets/AssetAccess.ts index 24224bdd7a772..45578ad213db0 100644 --- a/apps/server/src/assets/AssetAccess.ts +++ b/apps/server/src/assets/AssetAccess.ts @@ -159,6 +159,7 @@ const AssetClaimsSchema = Schema.Union([ Schema.Struct({ version: Schema.Literal(1), kind: Schema.Literal("github-media"), + repository: Schema.optional(Schema.String), /** Already narrowed to a GitHub media host at mint time; the signature is what keeps it there. */ url: Schema.String, cwd: Schema.String, @@ -188,6 +189,7 @@ export type ResolvedAsset = } | { readonly kind: "github-media"; + readonly repository?: string; readonly url: string; readonly cwd: string; readonly expiresAt: number; @@ -775,6 +777,9 @@ export const issueAssetUrl = Effect.fn("AssetAccess.issueAssetUrl")(function* (i kind: "github-media", url: fetchUrl, cwd: input.resource.cwd, + ...(input.resource.repository === undefined + ? {} + : { repository: input.resource.repository }), expiresAt, }; fileName = githubMediaFileName(fetchUrl); @@ -892,6 +897,7 @@ export const resolveAsset = Effect.fn("AssetAccess.resolveAsset")(function* ( kind: "github-media", url: claims.url, cwd: claims.cwd, + ...(claims.repository === undefined ? {} : { repository: claims.repository }), expiresAt: claims.expiresAt, } satisfies ResolvedAsset; } diff --git a/apps/server/src/assets/GitHubMediaFetch.ts b/apps/server/src/assets/GitHubMediaFetch.ts index ffda0861b777a..d8c1168fbd766 100644 --- a/apps/server/src/assets/GitHubMediaFetch.ts +++ b/apps/server/src/assets/GitHubMediaFetch.ts @@ -13,6 +13,7 @@ import { } from "effect/http"; import * as GitHubCredentials from "@t3tools/source-control-github/server/GitHubCredentials"; +import { parseGitHubRepositorySelector } from "@t3tools/source-control-github/server/gitHubRepositoryResolution"; import { workspaceGitHubCredential } from "../sourceControl/GitHubWorkspaceAccount.ts"; @@ -52,9 +53,16 @@ const SVG_CONTENT_TYPE = "image/svg+xml"; // body cannot run script against this origin. const SVG_CONTENT_SECURITY_POLICY = "default-src 'none'; style-src 'unsafe-inline'; sandbox"; -/** The github.com credential the workspace's repository is read with; null still loads public media. */ -const githubToken = (cwd: string) => - workspaceGitHubCredential(cwd).pipe( +/** Explicit PR repositories override the checkout; null still loads public media. */ +const githubToken = (cwd: string, repository?: string) => + Effect.gen(function* () { + if (repository === undefined) return yield* workspaceGitHubCredential(cwd); + const locator = parseGitHubRepositorySelector(repository, "github.com"); + // These media URLs are only supported on github.com, regardless of the PR's host. + if (locator === null || locator.host !== "github.com") return null; + const credentials = yield* GitHubCredentials.GitHubCredentials; + return yield* credentials.get(locator.host, `${locator.owner}/${locator.name}`); + }).pipe( Effect.flatMap((credential) => credential?.host === "github.com" ? Effect.succeed(credential) @@ -98,11 +106,16 @@ const fetchFollowingRedirects = Effect.fn("GitHubMediaFetch.fetchFollowingRedire }); export const githubMediaResponse = Effect.fn("GitHubMediaFetch.githubMediaResponse")(function* ( - asset: { readonly url: string; readonly cwd: string; readonly expiresAt: number }, + asset: { + readonly url: string; + readonly cwd: string; + readonly repository?: string; + readonly expiresAt: number; + }, requestHeaders: Record, ) { // Both media hosts are served by github.com's account, which is the host `gh` stores it under. - const token = yield* githubToken(asset.cwd); + const token = yield* githubToken(asset.cwd, asset.repository); const forwarded: Record = {}; for (const name of FORWARDED_REQUEST_HEADERS) { const value = requestHeaders[name]; diff --git a/apps/server/src/project/RepositoryIdentityResolver.ts b/apps/server/src/project/RepositoryIdentityResolver.ts index 80d5b39823abc..c0b1e5db9b887 100644 --- a/apps/server/src/project/RepositoryIdentityResolver.ts +++ b/apps/server/src/project/RepositoryIdentityResolver.ts @@ -4,6 +4,11 @@ import { detectSourceControlProviderFromGitRemoteUrl, normalizeGitRemoteUrl, } from "@t3tools/shared/git"; +import { + gitHubRepositoryEnvironment, + resolveGitHubRepository, + type GitHubRepositoryLocator, +} from "@t3tools/source-control-github/server/gitHubRepositoryResolution"; import * as Cache from "effect/Cache"; import * as Context from "effect/Context"; import * as Duration from "effect/Duration"; @@ -34,6 +39,10 @@ export interface RepositoryIdentityResolverOptions { export class RepositoryIdentityResolver extends Context.Service< RepositoryIdentityResolver, { + readonly resolveGitHubRepository: ( + cwd: string, + options?: { readonly refresh?: boolean }, + ) => Effect.Effect; readonly resolve: ( cwd: string, options?: { readonly refresh?: boolean }, @@ -188,6 +197,7 @@ export const make = Effect.fn("RepositoryIdentityResolver.make")(function* ( options: RepositoryIdentityResolverOptions = {}, ) { const processRunner = yield* ProcessRunner.ProcessRunner; + const platform = yield* HostProcess.Platform; const cacheCapacity = options.cacheCapacity ?? DEFAULT_REPOSITORY_IDENTITY_CACHE_CAPACITY; const refine = options.refine ?? Effect.succeed; const timeToLive = (exit: Exit.Exit) => @@ -219,17 +229,64 @@ export const make = Effect.fn("RepositoryIdentityResolver.make")(function* ( { capacity: cacheCapacity, timeToLive }, ); + // GitHub's effective target can differ from the upstream identity used to group projects. + const gitHubRemotesCache = yield* Cache.makeWith( + (rootPath: string) => + Effect.all( + [ + ["remote", "-v"], + ["config", "--get-regexp", "^remote\\..*\\.gh-resolved$"], + ].map((args) => + processRunner.run({ + command: "git", + args: ["-C", rootPath, ...args], + env: explicitBareRepositoryEnv(process.env, platform), + timeoutBehavior: "timedOutResult", + }), + ), + ).pipe( + Effect.map(([remotes, resolved]) => ({ + remotes: remotes?.code === 0 ? remotes.stdout : "", + resolved: resolved?.code === 0 ? resolved.stdout : "", + })), + Effect.orElseSucceed(() => ({ remotes: "", resolved: "" })), + ), + { + capacity: cacheCapacity, + timeToLive: () => options.positiveCacheTtl ?? DEFAULT_POSITIVE_CACHE_TTL, + }, + ); + + const resolveGitHub: RepositoryIdentityResolver["Service"]["resolveGitHubRepository"] = + Effect.fnUntraced(function* (cwd, resolveOptions) { + const environment = yield* HostProcess.Environment; + const { defaultHost, locator: override } = gitHubRepositoryEnvironment(environment); + if (override !== null) return override; + if (resolveOptions?.refresh) yield* Cache.invalidate(repositoryRootCache, cwd); + const rootPath = yield* Cache.get(repositoryRootCache, cwd); + if (rootPath === null) return null; + if (resolveOptions?.refresh) yield* Cache.invalidate(gitHubRemotesCache, rootPath); + return resolveGitHubRepository({ + ...(yield* Cache.get(gitHubRemotesCache, rootPath)), + hostHint: undefined, + defaultHost, + }).locator; + }); + const resolve: RepositoryIdentityResolver["Service"]["resolve"] = Effect.fnUntraced( function* (cwd, options) { if (options?.refresh) yield* Cache.invalidate(repositoryRootCache, cwd); const cacheKey = yield* Cache.get(repositoryRootCache, cwd); if (cacheKey === null) return null; - if (options?.refresh) yield* Cache.invalidate(repositoryIdentityCache, cacheKey); + if (options?.refresh) { + yield* Cache.invalidate(repositoryIdentityCache, cacheKey); + yield* Cache.invalidate(gitHubRemotesCache, cacheKey); + } return yield* Cache.get(repositoryIdentityCache, cacheKey); }, ); - return RepositoryIdentityResolver.of({ resolve }); + return RepositoryIdentityResolver.of({ resolve, resolveGitHubRepository: resolveGitHub }); }); export const layer = Layer.effect(RepositoryIdentityResolver, make()).pipe( diff --git a/apps/server/src/sourceControl/GitHubWorkspaceAccount.ts b/apps/server/src/sourceControl/GitHubWorkspaceAccount.ts index 3415b0e2cb12d..ae38de0bb0022 100644 --- a/apps/server/src/sourceControl/GitHubWorkspaceAccount.ts +++ b/apps/server/src/sourceControl/GitHubWorkspaceAccount.ts @@ -1,7 +1,6 @@ import * as Effect from "effect/Effect"; import * as GitHubCredentials from "@t3tools/source-control-github/server/GitHubCredentials"; -import { gitHubApiHostForRemote } from "@t3tools/source-control-github/server/gitHubRepositoryResolution"; import * as RepositoryIdentityResolver from "../project/RepositoryIdentityResolver.ts"; @@ -9,12 +8,10 @@ const workspaceGitHubRepository = Effect.fn("GitHubWorkspaceAccount.repository") cwd: string, ) { const resolver = yield* RepositoryIdentityResolver.RepositoryIdentityResolver; - const identity = yield* resolver.resolve(cwd); - if (identity === null) return null; - const host = gitHubApiHostForRemote(identity.locator.remoteUrl); - const owner = identity.owner?.trim(); - const name = identity.name?.trim(); - return host === null || !owner || !name ? null : { host, repository: `${owner}/${name}` }; + const repository = yield* resolver.resolveGitHubRepository(cwd); + return repository === null + ? null + : { host: repository.host, repository: `${repository.owner}/${repository.name}` }; }); export const workspaceGitHubCredential = Effect.fn("GitHubWorkspaceAccount.credential")(function* ( diff --git a/packages/contracts/src/assets.ts b/packages/contracts/src/assets.ts index 6737e01d47b7b..001296c4ae6bd 100644 --- a/packages/contracts/src/assets.ts +++ b/packages/contracts/src/assets.ts @@ -49,8 +49,9 @@ export const AssetResource = Schema.Union([ }), // An upload a pull request body points at on GitHub. A private repository serves these only // to a request that carries a credential, which the client has none of, so the server fetches - // them with the `gh` credential the repository at `cwd` authenticates with. + // them with the repository credential, falling back to the checkout at `cwd`. Schema.TaggedStruct("github-media", { + repository: Schema.optional(TrimmedNonEmptyString.check(Schema.isMaxLength(2048))), cwd: TrimmedNonEmptyString.check(Schema.isMaxLength(ASSET_PATH_MAX_LENGTH)), url: TrimmedNonEmptyString.check(Schema.isMaxLength(2048)), }), diff --git a/packages/source-control-github/src/server/GitHubSourceControlProvider.ts b/packages/source-control-github/src/server/GitHubSourceControlProvider.ts index 5b8b98812b69f..e426452902484 100644 --- a/packages/source-control-github/src/server/GitHubSourceControlProvider.ts +++ b/packages/source-control-github/src/server/GitHubSourceControlProvider.ts @@ -36,6 +36,7 @@ import { } from "./gitHubPullRequests.ts"; import { gitHubApiHostForRemote, + gitHubRepositoryEnvironment, parseFetchRemotes, parseGitHubRepositorySelector, parsePullRequestReference, @@ -473,13 +474,9 @@ export const make = Effect.gen(function* () { readonly host?: string | undefined; readonly context?: SourceControlProvider.SourceControlProviderContext | undefined; }) { - const envRepository = environment.GH_REPO?.trim(); const hostHint = input.host ?? contextHost(input.context); - const defaultHost = (hostHint ?? environment.GH_HOST ?? "github.com").toLowerCase(); - if (envRepository) { - const locator = parseGitHubRepositorySelector(envRepository, defaultHost); - if (locator !== null) return locator; - } + const { defaultHost, locator: override } = gitHubRepositoryEnvironment(environment, hostHint); + if (override !== null) return override; const [remotes, resolved] = yield* Effect.all([ gitRead(input.cwd, ["remote", "-v"]), gitRead(input.cwd, ["config", "--get-regexp", "^remote\\..*\\.gh-resolved$"]), diff --git a/packages/source-control-github/src/server/gitHubRepositoryResolution.ts b/packages/source-control-github/src/server/gitHubRepositoryResolution.ts index bd36758605e16..18c4c8b92f3e0 100644 --- a/packages/source-control-github/src/server/gitHubRepositoryResolution.ts +++ b/packages/source-control-github/src/server/gitHubRepositoryResolution.ts @@ -131,6 +131,14 @@ export function parseGitHubRepositorySelector( return null; } +export function gitHubRepositoryEnvironment(environment: NodeJS.ProcessEnv, hostHint?: string) { + const defaultHost = (hostHint ?? environment.GH_HOST ?? "github.com").toLowerCase(); + return { + defaultHost, + locator: parseGitHubRepositorySelector(environment.GH_REPO?.trim() ?? "", defaultHost), + }; +} + export function parsePullRequestReference( reference: string, ): From a99c162383ea5f0d05a78f50ff86a84d107f942f Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 19 Oct 2026 09:19:32 +0000 Subject: [PATCH 6/8] fix(github): preserve resolver cache and fixture contracts --- .../ThreadPullRequestService.test.ts | 2 ++ .../src/project/ManagedProjectFolders.test.ts | 2 +- .../project/ProjectEnrichmentService.test.ts | 18 ++++++------ .../project/ProjectService.deletion.test.ts | 2 +- .../server/src/project/ProjectService.test.ts | 8 +++--- .../src/project/RepositoryIdentityResolver.ts | 28 +++++++++---------- 6 files changed, 31 insertions(+), 29 deletions(-) diff --git a/apps/server/src/orchestration-v2/ThreadPullRequestService.test.ts b/apps/server/src/orchestration-v2/ThreadPullRequestService.test.ts index e47191c3fbc02..e94b0a0f618fd 100644 --- a/apps/server/src/orchestration-v2/ThreadPullRequestService.test.ts +++ b/apps/server/src/orchestration-v2/ThreadPullRequestService.test.ts @@ -47,6 +47,7 @@ describe("ThreadPullRequestServiceV2 project guard", () => { const result = yield* ThreadPullRequestService.resolveProjectForPullRequestDiscovery( project, { + resolveGitHubRepository: () => Effect.succeed(null), resolve: (root) => { resolvedRoot = root; return Effect.succeed({ @@ -84,6 +85,7 @@ describe("ThreadPullRequestServiceV2 project guard", () => { }; const refreshes: Array = []; const resolver = { + resolveGitHubRepository: () => Effect.succeed(null), resolve: (_root: string, options?: { readonly refresh?: boolean }) => { refreshes.push(options?.refresh); return Effect.succeed(null); diff --git a/apps/server/src/project/ManagedProjectFolders.test.ts b/apps/server/src/project/ManagedProjectFolders.test.ts index 453cf4431bdc7..1d6f46c11d160 100644 --- a/apps/server/src/project/ManagedProjectFolders.test.ts +++ b/apps/server/src/project/ManagedProjectFolders.test.ts @@ -43,7 +43,7 @@ const layerGitWorkflow = (vcsProcess?: Layer.Layer) => const layerEnrichment = ProjectEnrichmentService.layer.pipe( Layer.provide( Layer.mergeAll( - Layer.succeed(RepositoryIdentityResolver.RepositoryIdentityResolver, { + Layer.mock(RepositoryIdentityResolver.RepositoryIdentityResolver)({ resolve: () => Effect.succeed(null), }), Layer.succeed(ProjectFaviconResolver.ProjectFaviconResolver, { diff --git a/apps/server/src/project/ProjectEnrichmentService.test.ts b/apps/server/src/project/ProjectEnrichmentService.test.ts index 7a0238159a747..e181ad993a523 100644 --- a/apps/server/src/project/ProjectEnrichmentService.test.ts +++ b/apps/server/src/project/ProjectEnrichmentService.test.ts @@ -51,7 +51,7 @@ const layer = ( it.effect("preserves either enrichment field when the other resolver fails", () => Effect.gen(function* () { const layerMetadata = Layer.merge( - Layer.succeed(RepositoryIdentityResolver.RepositoryIdentityResolver, { + Layer.mock(RepositoryIdentityResolver.RepositoryIdentityResolver)({ resolve: (workspaceRoot) => workspaceRoot === "/repo-fails" ? Effect.die("repository resolver failed") @@ -105,7 +105,7 @@ it.effect("does not warn about favicons for workspace roots that no longer exist warnings.push(message); }); const metadataLayer = Layer.merge( - Layer.succeed(RepositoryIdentityResolver.RepositoryIdentityResolver, { + Layer.mock(RepositoryIdentityResolver.RepositoryIdentityResolver)({ resolve: (workspaceRoot) => Effect.succeed(identity(workspaceRoot)), }), Layer.succeed(ProjectFaviconResolver.ProjectFaviconResolver, { @@ -154,7 +154,7 @@ it.effect("publishes repository completion while favicon enrichment is still pen Effect.gen(function* () { const releaseFavicon = yield* Deferred.make(); const layerMetadata = Layer.merge( - Layer.succeed(RepositoryIdentityResolver.RepositoryIdentityResolver, { + Layer.mock(RepositoryIdentityResolver.RepositoryIdentityResolver)({ resolve: (workspaceRoot) => Effect.succeed(identity(workspaceRoot)), }), Layer.succeed(ProjectFaviconResolver.ProjectFaviconResolver, { @@ -182,7 +182,7 @@ it.effect("keeps repository workers available when every favicon worker is hung" const faviconWorkersStarted = yield* Deferred.make(); const faviconStarts = yield* Ref.make(0); const layerMetadata = Layer.merge( - Layer.succeed(RepositoryIdentityResolver.RepositoryIdentityResolver, { + Layer.mock(RepositoryIdentityResolver.RepositoryIdentityResolver)({ resolve: (workspaceRoot) => Effect.succeed(identity(workspaceRoot)), }), Layer.succeed(ProjectFaviconResolver.ProjectFaviconResolver, { @@ -229,7 +229,7 @@ it.effect("getAvailable returns immediately while repository identity is still u const repositoryStarted = yield* Deferred.make(); const releaseRepository = yield* Deferred.make(); const layerMetadata = Layer.merge( - Layer.succeed(RepositoryIdentityResolver.RepositoryIdentityResolver, { + Layer.mock(RepositoryIdentityResolver.RepositoryIdentityResolver)({ resolve: (workspaceRoot) => Deferred.succeed(repositoryStarted, undefined).pipe( Effect.andThen(Deferred.await(releaseRepository)), @@ -270,7 +270,7 @@ it.effect( const repositoryStarted = yield* Deferred.make(); const releaseRepository = yield* Deferred.make(); const layerMetadata = Layer.merge( - Layer.succeed(RepositoryIdentityResolver.RepositoryIdentityResolver, { + Layer.mock(RepositoryIdentityResolver.RepositoryIdentityResolver)({ resolve: (workspaceRoot) => { if (workspaceRoot === "/no-remote") { return Effect.succeed(null); @@ -332,7 +332,7 @@ it.effect("deduplicates requests, bounds pending work, and reloads invalidated r const repositoryCalls = yield* Ref.make>([]); const version = yield* Ref.make(1); const layerMetadata = Layer.merge( - Layer.succeed(RepositoryIdentityResolver.RepositoryIdentityResolver, { + Layer.mock(RepositoryIdentityResolver.RepositoryIdentityResolver)({ resolve: (workspaceRoot) => Effect.gen(function* () { yield* Ref.update(repositoryCalls, (calls) => [...calls, workspaceRoot]); @@ -399,7 +399,7 @@ it.effect("rescans a favicon only after 15 minutes", () => Effect.gen(function* () { const faviconScans = yield* Ref.make(0); const layerMetadata = Layer.merge( - Layer.succeed(RepositoryIdentityResolver.RepositoryIdentityResolver, { + Layer.mock(RepositoryIdentityResolver.RepositoryIdentityResolver)({ resolve: (workspaceRoot) => Effect.succeed(identity(workspaceRoot)), }), Layer.succeed(ProjectFaviconResolver.ProjectFaviconResolver, { @@ -444,7 +444,7 @@ it.effect( const remoteVersion = yield* Ref.make(0); const faviconScans = yield* Ref.make(0); const layerMetadata = Layer.merge( - Layer.succeed(RepositoryIdentityResolver.RepositoryIdentityResolver, { + Layer.mock(RepositoryIdentityResolver.RepositoryIdentityResolver)({ resolve: (workspaceRoot) => Ref.get(remoteVersion).pipe( Effect.map((version) => (version === 0 ? null : identity(workspaceRoot, version))), diff --git a/apps/server/src/project/ProjectService.deletion.test.ts b/apps/server/src/project/ProjectService.deletion.test.ts index a35e58224baa2..6cb4bff595bde 100644 --- a/apps/server/src/project/ProjectService.deletion.test.ts +++ b/apps/server/src/project/ProjectService.deletion.test.ts @@ -54,7 +54,7 @@ const layerServices = Layer.mergeAll( ProjectEnrichmentService.layer.pipe( Layer.provide( Layer.merge( - Layer.succeed(RepositoryIdentityResolver.RepositoryIdentityResolver, { + Layer.mock(RepositoryIdentityResolver.RepositoryIdentityResolver)({ resolve: () => Effect.succeed(null), }), Layer.succeed(ProjectFaviconResolver.ProjectFaviconResolver, { diff --git a/apps/server/src/project/ProjectService.test.ts b/apps/server/src/project/ProjectService.test.ts index 67e76bfeafbc8..c323d1b051f27 100644 --- a/apps/server/src/project/ProjectService.test.ts +++ b/apps/server/src/project/ProjectService.test.ts @@ -33,7 +33,7 @@ const layerWorkspacePaths = Layer.succeed(WorkspacePaths.WorkspacePaths, { }); const layerMetadata = Layer.merge( - Layer.succeed(RepositoryIdentityResolver.RepositoryIdentityResolver, { + Layer.mock(RepositoryIdentityResolver.RepositoryIdentityResolver)({ resolve: (workspaceRoot) => Effect.succeed({ canonicalKey: `github.com/t3tools/${workspaceRoot.split("/").at(-1)}`, @@ -495,7 +495,7 @@ it.effect( const faviconCalls = yield* Ref.make(0); const layerSlowMetadata = Layer.merge( - Layer.succeed(RepositoryIdentityResolver.RepositoryIdentityResolver, { + Layer.mock(RepositoryIdentityResolver.RepositoryIdentityResolver)({ resolve: (workspaceRoot) => Effect.gen(function* () { yield* Ref.update(repositoryCalls, (count) => count + 1); @@ -593,7 +593,7 @@ it.effect( it.effect("keeps project snapshots available when optional metadata enrichment fails", () => Effect.gen(function* () { const layerFailingMetadata = Layer.merge( - Layer.succeed(RepositoryIdentityResolver.RepositoryIdentityResolver, { + Layer.mock(RepositoryIdentityResolver.RepositoryIdentityResolver)({ resolve: () => Effect.succeed(null), }), Layer.succeed(ProjectFaviconResolver.ProjectFaviconResolver, { @@ -632,7 +632,7 @@ it.effect("invalidates workspace-derived metadata when a project moves", () => Effect.gen(function* () { const metadataVersion = yield* Ref.make(1); const layerVersionedMetadata = Layer.merge( - Layer.succeed(RepositoryIdentityResolver.RepositoryIdentityResolver, { + Layer.mock(RepositoryIdentityResolver.RepositoryIdentityResolver)({ resolve: (workspaceRoot) => Ref.get(metadataVersion).pipe( Effect.map((version) => ({ diff --git a/apps/server/src/project/RepositoryIdentityResolver.ts b/apps/server/src/project/RepositoryIdentityResolver.ts index c0b1e5db9b887..501c451815260 100644 --- a/apps/server/src/project/RepositoryIdentityResolver.ts +++ b/apps/server/src/project/RepositoryIdentityResolver.ts @@ -245,16 +245,17 @@ export const make = Effect.fn("RepositoryIdentityResolver.make")(function* ( }), ), ).pipe( - Effect.map(([remotes, resolved]) => ({ - remotes: remotes?.code === 0 ? remotes.stdout : "", - resolved: resolved?.code === 0 ? resolved.stdout : "", - })), - Effect.orElseSucceed(() => ({ remotes: "", resolved: "" })), + Effect.map(([remotes, resolved]) => + remotes?.code === 0 && remotes.stdout.trim().length > 0 + ? { + remotes: remotes.stdout, + resolved: resolved?.code === 0 ? resolved.stdout : "", + } + : null, + ), + Effect.orElseSucceed(() => null), ), - { - capacity: cacheCapacity, - timeToLive: () => options.positiveCacheTtl ?? DEFAULT_POSITIVE_CACHE_TTL, - }, + { capacity: cacheCapacity, timeToLive }, ); const resolveGitHub: RepositoryIdentityResolver["Service"]["resolveGitHubRepository"] = @@ -266,11 +267,10 @@ export const make = Effect.fn("RepositoryIdentityResolver.make")(function* ( const rootPath = yield* Cache.get(repositoryRootCache, cwd); if (rootPath === null) return null; if (resolveOptions?.refresh) yield* Cache.invalidate(gitHubRemotesCache, rootPath); - return resolveGitHubRepository({ - ...(yield* Cache.get(gitHubRemotesCache, rootPath)), - hostHint: undefined, - defaultHost, - }).locator; + const remotes = yield* Cache.get(gitHubRemotesCache, rootPath); + return remotes === null + ? null + : resolveGitHubRepository({ ...remotes, hostHint: undefined, defaultHost }).locator; }); const resolve: RepositoryIdentityResolver["Service"]["resolve"] = Effect.fnUntraced( From d1c944afa2ea0ce1458254f053b8b06d1d098cbf Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 19 Oct 2026 10:19:25 +0000 Subject: [PATCH 7/8] test(github): cover link lookup deadline during account selection --- .../GitHubSourceControlProvider.test.ts | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/packages/source-control-github/src/server/GitHubSourceControlProvider.test.ts b/packages/source-control-github/src/server/GitHubSourceControlProvider.test.ts index b478d1fbe1512..402b7cb697534 100644 --- a/packages/source-control-github/src/server/GitHubSourceControlProvider.test.ts +++ b/packages/source-control-github/src/server/GitHubSourceControlProvider.test.ts @@ -2,6 +2,7 @@ import { SourceControlProviderKind } from "@t3tools/contracts"; import * as HostProcess from "@t3tools/shared/HostProcess"; import { assert, describe, it } from "@effect/vitest"; import * as NodeServices from "@effect/platform-node/NodeServices"; +import * as Deferred from "effect/Deferred"; import * as Effect from "effect/Effect"; import * as Fiber from "effect/Fiber"; import * as FileSystem from "effect/FileSystem"; @@ -1069,6 +1070,45 @@ it.effect.each(["read", "decode"] as const)( }), ); +it.effect("times out linked subjects while account resolution is blocked", () => + Effect.gen(function* () { + const started = yield* Deferred.make(); + const released = yield* Deferred.make(); + let restAttempts = 0; + const { layer } = harness({ + remotes: "", + credential: () => + Deferred.succeed(started, undefined).pipe( + Effect.andThen(Effect.never), + Effect.onInterrupt(() => Deferred.succeed(released, undefined)), + ), + api: { + rest: () => + Effect.sync(() => { + restAttempts++; + return restResponse({ title: "Pairing expiry", body: "Preserve remote access" }); + }), + }, + }); + const provider = yield* GitHubSourceControlProvider.make.pipe(Effect.provide(layer)); + const lookup = provider.resolveLink?.({ + cwd: "/repo", + url: new URL("https://github.com/owner/repo/issues/42"), + }); + assert.ok(lookup); + const pending = yield* lookup.pipe(Effect.flip, Effect.forkChild); + yield* Deferred.await(started); + yield* TestClock.adjust("3 seconds"); + assert.strictEqual(restAttempts, 0); + assert.isDefined(pending.pollUnsafe(), "resolveLink must finish within three seconds"); + const error = yield* Fiber.join(pending); + assert.strictEqual(error._tag, "SourceControlProviderError"); + assert.strictEqual(error.operation, "resolveLink"); + assert.strictEqual(error.cwd, "/repo"); + assert.isTrue(yield* Deferred.isDone(released)); + }), +); + const multiAccountStatus = (extra: ReadonlyArray> = []) => processResult( JSON.stringify({ From d236f45cf0817dd13809f5be92f58fac3fc60493 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 19 Oct 2026 10:22:31 +0000 Subject: [PATCH 8/8] fix(github): bound complete link lookups to three seconds --- .../src/server/GitHubSourceControlProvider.ts | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/packages/source-control-github/src/server/GitHubSourceControlProvider.ts b/packages/source-control-github/src/server/GitHubSourceControlProvider.ts index e426452902484..1a6c2a86d701c 100644 --- a/packages/source-control-github/src/server/GitHubSourceControlProvider.ts +++ b/packages/source-control-github/src/server/GitHubSourceControlProvider.ts @@ -874,7 +874,6 @@ export const make = Effect.gen(function* () { maxResponseBytes: 1_000_000, }) .pipe( - Effect.timeout("3 seconds"), Effect.mapError( (cause) => new SourceControlProviderError({ @@ -924,6 +923,20 @@ export const make = Effect.gen(function* () { return asAccountOf( { host: input.url.host, owner: match[1]!, name: match[2]! }, readLinkSubject(input, `repos/${match[1]}/${match[2]}/issues/${match[3]}`), + ).pipe( + Effect.timeout("3 seconds"), + Effect.catchTags({ + TimeoutError: (cause) => + Effect.fail( + new SourceControlProviderError({ + provider: SourceControlProviderKind.make("github"), + operation: "resolveLink", + cwd: input.cwd, + detail: "The linked subject could not be read.", + cause, + }), + ), + }), ); }, listChangeRequests: (input) =>