diff --git a/apps/mobile/src/connection/storage.ts b/apps/mobile/src/connection/storage.ts index 053946b2d10fc47844f3ebfeca735720a0b751c2..7bf2884e81d984f7e9f5220050d0ea7a44ec1f7b 100644 --- a/apps/mobile/src/connection/storage.ts +++ b/apps/mobile/src/connection/storage.ts @@ -14,6 +14,8 @@ import { ConnectionTransientError, CredentialStore, ProfileStore, + GitHubRoutingPermissions, + makeGitHubRoutingPermissions, } from "@t3tools/client-runtime/connection"; import * as Context from "effect/Context"; import * as Effect from "effect/Effect"; @@ -39,6 +41,11 @@ function targetPersistenceError( export const connectionStorageLayer = Layer.effectContext( Effect.gen(function* () { const catalog = yield* CatalogStore.make(); + const githubRoutingPermissions = yield* makeGitHubRoutingPermissions({ + read: catalog.read.pipe(Effect.map((document) => document.githubRoutingPermissions ?? [])), + write: (githubRoutingPermissions) => + catalog.update((document) => ({ ...document, githubRoutingPermissions })), + }); const targetStore = ConnectionTargetStore.of({ list: catalog.read.pipe( @@ -138,6 +145,7 @@ export const connectionStorageLayer = Layer.effectContext( })), }); return Context.make(ConnectionTargetStore, targetStore).pipe( + Context.add(GitHubRoutingPermissions, githubRoutingPermissions), Context.add(ConnectionRegistrationStore, registrationStore), Context.add(ProfileStore.ConnectionProfileStore, profileStore), Context.add(CredentialStore.ConnectionCredentialStore, credentialStore), diff --git a/apps/mobile/src/features/connection/ConnectionsRouteScreen.tsx b/apps/mobile/src/features/connection/ConnectionsRouteScreen.tsx index ab6207f186ef16ff87a14f77dfdd32379586858d..bfab389a3a6479d916fe07cf4e19f3e2044b71e9 100644 --- a/apps/mobile/src/features/connection/ConnectionsRouteScreen.tsx +++ b/apps/mobile/src/features/connection/ConnectionsRouteScreen.tsx @@ -11,6 +11,7 @@ import { AppText as Text } from "../../components/AppText"; import { cn } from "../../lib/cn"; import { useRemoteConnections } from "../../state/use-remote-environment-registry"; import { ConnectionEnvironmentRow } from "./ConnectionEnvironmentRow"; +import { GitHubRoutingSettings } from "./GitHubRoutingSettings"; export function ConnectionsRouteScreen() { const { @@ -97,6 +98,7 @@ export function ConnectionsRouteScreen() { )} + ); diff --git a/apps/mobile/src/features/connection/GitHubRoutingSettings.tsx b/apps/mobile/src/features/connection/GitHubRoutingSettings.tsx new file mode 100644 index 0000000000000000000000000000000000000000..764a7d1a0d554280acc7702626137a8cd0d7789e --- /dev/null +++ b/apps/mobile/src/features/connection/GitHubRoutingSettings.tsx @@ -0,0 +1,126 @@ +import { useAtomValue } from "@effect/atom-react"; +import { + connectionCatalogDisplayUrl, + gitHubRoutingConnectionKey, + gitHubRoutingPermissionFor, + type GitHubRoutingPermission, +} from "@t3tools/client-runtime/connection"; +import { useState } from "react"; +import { Alert, Pressable, View } from "react-native"; + +import { AppText as Text } from "../../components/AppText"; +import { SymbolView } from "../../components/AppSymbol"; +import { environmentCatalog } from "../../connection/catalog"; +import { useAtomCommand } from "../../state/use-atom-command"; +import { SettingsSection } from "../settings/components/SettingsSection"; + +const options: ReadonlyArray<{ + value: GitHubRoutingPermission; + label: string; + description: string; +}> = [ + { value: "off", label: "Off", description: "Keep GitHub requests on this environment." }, + { + value: "read", + label: "Read PRs", + description: "Share PR data with other enabled environments.", + }, + { + value: "read-write", + label: "Read and act", + description: "Actions may use broader GitHub permissions than the original environment.", + }, +]; + +export function GitHubRoutingSettings() { + const catalog = useAtomValue(environmentCatalog.catalogValueAtom); + const permissions = useAtomValue(environmentCatalog.githubRoutingPermissionsValueAtom); + const update = useAtomCommand(environmentCatalog.setGitHubRoutingPermission); + const [expanded, setExpanded] = useState(null); + const [saving, setSaving] = useState(false); + if (catalog.entries.size === 0) return null; + + return ( + + + {[...catalog.entries.values()].map((entry, index) => { + const environmentId = entry.target.environmentId; + const selected = gitHubRoutingPermissionFor(entry, permissions); + const disabled = !catalog.isReady || saving || gitHubRoutingConnectionKey(entry) === null; + return ( + + setExpanded(expanded === environmentId ? null : environmentId)} + > + + + {entry.target.label} + + + {connectionCatalogDisplayUrl(entry) ?? "T3 Connect"} + + + + {options.find((option) => option.value === selected)?.label} + + + + {expanded === environmentId + ? options.map((option) => ( + { + setSaving(true); + void update({ environmentId, permission: option.value }).then((result) => { + setSaving(false); + if (result._tag === "Failure") + Alert.alert( + "Could not save GitHub routing permission", + "Try again before leaving this screen.", + ); + }); + }} + > + + {option.label} + + {option.description} + + + {selected === option.value ? ( + + ) : null} + + )) + : null} + + ); + })} + + + Choose environments you trust to share PR data and use each other's GitHub access. Enable + both environments. This applies only to this client. + + + ); +} diff --git a/apps/mobile/src/features/settings/SettingsEnvironmentsRouteScreen.tsx b/apps/mobile/src/features/settings/SettingsEnvironmentsRouteScreen.tsx index 9b8363c235db3d4736cf1d8ecfc4707862f184d5..653107d22bb393bfd10499bb4fa99968d8cc9db8 100644 --- a/apps/mobile/src/features/settings/SettingsEnvironmentsRouteScreen.tsx +++ b/apps/mobile/src/features/settings/SettingsEnvironmentsRouteScreen.tsx @@ -10,6 +10,7 @@ import { AppText as Text } from "../../components/AppText"; import { AndroidScreenHeader } from "../../components/AndroidScreenHeader"; import { CloudEnvironmentRows } from "../connection/CloudEnvironmentRows"; import { ConnectionEnvironmentRow } from "../connection/ConnectionEnvironmentRow"; +import { GitHubRoutingSettings } from "../connection/GitHubRoutingSettings"; import { splitEnvironmentSections } from "../connection/environmentSections"; import { cn } from "../../lib/cn"; import { useUniwindTheme } from "../../lib/useUniwindTheme"; @@ -174,6 +175,7 @@ export function SettingsEnvironmentsRouteScreen() { } : {})} /> + ); diff --git a/apps/server/src/auth/RpcAuthorization.ts b/apps/server/src/auth/RpcAuthorization.ts index d7d1be455fa1295d1951c23a41d09cd477eab3b9..2e2bdea73fc72b9b496b1a19a9997280752ddd8f 100644 --- a/apps/server/src/auth/RpcAuthorization.ts +++ b/apps/server/src/auth/RpcAuthorization.ts @@ -68,6 +68,8 @@ export const RPC_REQUIRED_SCOPES = { [WS_METHODS.pullRequestsList]: AuthOrchestrationReadScope, [WS_METHODS.pullRequestsListStats]: AuthOrchestrationReadScope, [WS_METHODS.pullRequestsSummary]: AuthOrchestrationReadScope, + [WS_METHODS.pullRequestsRouting]: AuthOrchestrationReadScope, + [WS_METHODS.pullRequestsRoutingIdentity]: AuthOrchestrationReadScope, [WS_METHODS.pullRequestsStack]: AuthOrchestrationReadScope, [WS_METHODS.pullRequestsLinkedThreads]: AuthOrchestrationReadScope, [WS_METHODS.pullRequestsDetail]: AuthOrchestrationReadScope, diff --git a/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts b/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts index 6ef485627afbc7649cff1d02d8905ab3da9d5399..a62d1ed219709943739fd5f13649734eca5d75ba 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts @@ -1,11 +1,14 @@ import { afterEach, assert, expect, it, vi } from "@effect/vitest"; import * as Effect from "effect/Effect"; +import * as Deferred from "effect/Deferred"; +import * as Fiber from "effect/Fiber"; import * as Layer from "effect/Layer"; import * as Schema from "effect/Schema"; import * as TestClock from "effect/testing/TestClock"; import { ChildProcessSpawner } from "effect/unstable/process"; import * as GitHubCli from "../sourceControl/GitHubCli.ts"; +import * as VcsProcess from "../vcs/VcsProcess.ts"; import * as GitHubGraphQlBudget from "../sourceControl/githubGraphQlBudget.ts"; import * as GitHubPullRequestCli from "./GitHubPullRequestCli.ts"; import { BASE_COMPARISON_GRAPHQL_QUERY } from "./gitHubPullRequestJson.ts"; @@ -192,7 +195,118 @@ afterEach(() => { mockedGetPullRequest.mockReset(); }); +it.effect( + "keeps a verified credential through an auth switch and separates token fingerprints", + () => + Effect.gen(function* () { + let activeToken = "broad-credential"; + const commands: VcsProcess.VcsProcessInput[] = []; + const github = yield* GitHubCli.make.pipe( + Effect.provideService(VcsProcess.VcsProcess, { + run: (input) => + Effect.sync(() => { + commands.push(input); + if (input.args[0] === "auth") return output(activeToken); + if (input.args[0] === "api") return output('{"id":123,"login":"same-account"}'); + return output(""); + }), + }), + ); + const cli = yield* GitHubPullRequestCli.make.pipe( + Effect.provideService(GitHubCli.GitHubCli, github), + Effect.provide(GitHubGraphQlBudget.layer), + ); + const input = { cwd: "/repo", host: "github.com" }; + const first = yield* cli.withVerifiedCredential(input, (identity) => + Effect.gen(function* () { + activeToken = "restricted-credential"; + expect(yield* cli.getViewerLogin(input)).toBe("same-account"); + yield* cli.commentOnPullRequest({ + ...input, + repository: "owner/repo", + number: 1, + body: "comment", + }); + return identity; + }), + ); + const second = yield* cli.withVerifiedCredential(input, Effect.succeed); + expect(first.accountId).toBe(second.accountId); + expect(first.credentialFingerprint).not.toBe(second.credentialFingerprint); + expect(encodeJson([first, second])).not.toContain("broad-credential"); + expect(encodeJson([first, second])).not.toContain("restricted-credential"); + expect(commands.find((command) => command.args[0] === "pr")?.env).toMatchObject({ + GH_TOKEN: "broad-credential", + GITHUB_TOKEN: "broad-credential", + GH_DEBUG: "", + }); + expect( + commands + .filter((command) => command.args[0] === "api") + .map((command) => command.env?.GH_TOKEN), + ).toEqual(["broad-credential", "restricted-credential"]); + expect(yield* cli.getRoutingIdentity(input)).toEqual({ + accountId: "123", + viewer: "same-account", + }); + }), +); + layer("GitHubPullRequestCli.layer", (it) => { + it.effect("coalesces concurrent identity verification for the same host and credential", () => + Effect.gen(function* () { + mockedExecute.mockImplementation((input) => + input.args[0] === "auth" + ? Effect.succeed(output("shared-credential")) + : Effect.yieldNow.pipe(Effect.as(output('{"id":123,"login":"viewer"}'))), + ); + const cli = yield* GitHubPullRequestCli.GitHubPullRequestCli; + const results = yield* Effect.all( + Array.from({ length: 4 }, () => + cli.getRoutingIdentity({ cwd: "/w", host: "github.identity-flight.test" }), + ), + { concurrency: 4 }, + ); + expect(results).toEqual( + Array.from({ length: 4 }, () => ({ accountId: "123", viewer: "viewer" })), + ); + expect(mockedExecute.mock.calls.filter(([input]) => input.args[0] === "api")).toHaveLength(1); + }), + ); + + it.effect( + "lets another identity reader continue when the first verification is interrupted", + () => + Effect.gen(function* () { + const firstStarted = yield* Deferred.make(); + const secondStarted = yield* Deferred.make(); + let tokens = 0; + let verifications = 0; + mockedExecute.mockImplementation((input) => + Effect.gen(function* () { + if (input.args[0] === "auth") { + if (++tokens === 2) yield* Deferred.succeed(secondStarted, undefined); + return output("cancel-credential"); + } + if (++verifications === 1) { + yield* Deferred.succeed(firstStarted, undefined); + return yield* Effect.never; + } + return output('{"id":123,"login":"viewer"}'); + }), + ); + const cli = yield* GitHubPullRequestCli.GitHubPullRequestCli; + const input = { cwd: "/w", host: "github.identity-cancel.test" }; + const first = yield* cli.getRoutingIdentity(input).pipe(Effect.forkChild); + yield* Deferred.await(firstStarted); + const second = yield* cli.getRoutingIdentity(input).pipe(Effect.forkChild); + yield* Deferred.await(secondStarted); + yield* Fiber.interrupt(first); + expect(yield* Fiber.join(second)).toEqual({ accountId: "123", viewer: "viewer" }); + expect(verifications).toBe(2); + }), + ); + it.effect("reads linked pull request status with the overview fields in one request", () => Effect.gen(function* () { mockedExecute.mockReturnValueOnce( @@ -2458,12 +2572,79 @@ layer("GitHubPullRequestCli.layer", (it) => { mockedExecute.mockReturnValueOnce(Effect.succeed(output(" "))); const cli = yield* GitHubPullRequestCli.GitHubPullRequestCli; - const error = yield* Effect.flip(cli.getViewerLogin({ cwd: "/w" })); + const error = yield* Effect.flip(cli.getViewerLogin({ cwd: "/w", host: "github.com" })); assert.strictEqual(error._tag, "GitHubViewerLoginUnavailableError"); }), ); + it.effect("looks up the authenticated account on the requested enterprise host", () => + Effect.gen(function* () { + mockedExecute + .mockReturnValueOnce(Effect.succeed(output("enterprise-test-credential"))) + .mockReturnValueOnce(Effect.succeed(output('{"id":456,"login":"enterprise-user"}'))); + const cli = yield* GitHubPullRequestCli.GitHubPullRequestCli; + + const login = yield* cli.getViewerLogin({ cwd: "/w", host: "github.acme.com" }); + + expect(login).toBe("enterprise-user"); + expect(callAt(0).args).toEqual(["auth", "token", "--hostname", "github.acme.com"]); + expect(callAt(1).args).toEqual(["api", "user", "--hostname", "github.acme.com"]); + }), + ); + + it.effect("reuses verified credentials offline and refuses an unverified replacement", () => + Effect.gen(function* () { + const cli = yield* GitHubPullRequestCli.GitHubPullRequestCli; + const input = { cwd: "/w", host: "github.identity-cache.test" }; + mockedExecute + .mockReturnValueOnce(Effect.succeed(output("test-credential-a"))) + .mockReturnValueOnce(Effect.succeed(output('{"id":123,"login":"maria-rcks"}'))); + expect(yield* cli.getRoutingIdentity(input)).toEqual({ + accountId: "123", + viewer: "maria-rcks", + }); + expect(callAt(1).env).toMatchObject({ + GH_ENTERPRISE_TOKEN: "test-credential-a", + GH_DEBUG: "", + }); + + mockedExecute.mockReturnValueOnce(Effect.succeed(output("test-credential-a"))); + expect(yield* cli.getRoutingIdentity(input)).toEqual({ + accountId: "123", + viewer: "maria-rcks", + }); + expect(mockedExecute).toHaveBeenCalledTimes(3); + + mockedExecute + .mockReturnValueOnce(Effect.succeed(output("test-credential-b"))) + .mockReturnValueOnce( + Effect.fail( + new GitHubCli.GitHubCliCommandError({ + command: "gh", + cwd: "/w", + cause: new Error("upstream failed with test-credential-b"), + }), + ), + ); + const failure = yield* cli.getRoutingIdentity(input).pipe(Effect.flip); + expect(failure._tag).toBe("GitHubViewerLoginUnavailableError"); + expect(String(failure)).not.toContain("test-credential-b"); + expect(callAt(4).env).toMatchObject({ + GH_ENTERPRISE_TOKEN: "test-credential-b", + GH_DEBUG: "", + }); + + mockedExecute + .mockReturnValueOnce(Effect.succeed(output("test-credential-b"))) + .mockReturnValueOnce(Effect.succeed(output('{"id":456,"login":"maria-rcks"}'))); + expect(yield* cli.getRoutingIdentity(input)).toEqual({ + accountId: "456", + viewer: "maria-rcks", + }); + }), + ); + it.effect("sends a whole review as one request body over stdin", () => Effect.gen(function* () { mockedExecute.mockReturnValue(Effect.succeed(output("{}"))); diff --git a/apps/server/src/pullRequest/GitHubPullRequestCli.ts b/apps/server/src/pullRequest/GitHubPullRequestCli.ts index 5e1950a6eeb049dae2850e47a12740fc0e2eb9c3..2558f6695f1ab16625c344304a5463f982531a98 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestCli.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestCli.ts @@ -1,11 +1,17 @@ import { runGitHubStackAction, type GitHubStackActionError } from "./githubStackActions.ts"; import * as Context from "effect/Context"; +import * as Clock from "effect/Clock"; +import * as NodeCrypto from "node:crypto"; import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; +import * as Redacted from "effect/Redacted"; import * as Result from "effect/Result"; import * as Schema from "effect/Schema"; +import * as Semaphore from "effect/Semaphore"; import { resolvePullRequestAuthorFilter, + PositiveInt, + TrimmedNonEmptyString, type PullRequestAction, type PullRequestStackHead, type PullRequestActor, @@ -413,8 +419,24 @@ export interface GitHubPullRequestDiffSlice { export class GitHubPullRequestCli extends Context.Service< GitHubPullRequestCli, { + readonly withVerifiedCredential: ( + input: { readonly cwd: string; readonly host: string }, + use: (identity: { + readonly accountId: string; + readonly viewer: string; + readonly credentialFingerprint: string; + }) => Effect.Effect, + ) => Effect.Effect; + readonly getRoutingIdentity: (input: { + readonly cwd: string; + readonly host: string; + }) => Effect.Effect< + { readonly accountId: string; readonly viewer: string }, + GitHubPullRequestCliError + >; readonly getViewerLogin: (input: { readonly cwd: string; + readonly host: string; }) => Effect.Effect; readonly listPullRequests: (input: { @@ -991,6 +1013,111 @@ function actionArgs( export const make = Effect.gen(function* () { const github = yield* GitHubCli.GitHubCli; const graphQlBudget = yield* GitHubGraphQlBudget.GitHubGraphQlBudget; + const routingIdentities = new Map< + string, + { + at: number; + value: { accountId: string; viewer: string }; + } + >(); + const identityLocks = new Map(); + const decodeRoutingIdentity = Schema.decodeUnknownEffect( + Schema.fromJsonString( + Schema.Struct({ + id: PositiveInt, + login: TrimmedNonEmptyString, + }), + ), + ); + const captureVerifiedCredential = Effect.fn("GitHubPullRequestCli.captureVerifiedCredential")( + function* (input: { readonly cwd: string; readonly host: string }) { + const unavailable = () => + new GitHubViewerLoginUnavailableError({ command: "gh", cwd: input.cwd }); + const host = input.host.toLowerCase(); + const pinned = yield* GitHubCli.PinnedGitHubCredential; + if (pinned !== null && pinned.host !== host) return yield* unavailable(); + // Only the digest is retained. Never attach credential lookup output to an error. + const token = + pinned !== null + ? Redacted.value(pinned.token) + : (yield* github + .execute({ + cwd: input.cwd, + args: ["auth", "token", "--hostname", host], + env: { GH_DEBUG: "" }, + }) + .pipe(Effect.mapError(unavailable))).stdout.trim(); + if (!token) return yield* unavailable(); + const key = `${host}:${NodeCrypto.createHash("sha256").update(token).digest("hex")}`; + const credential = { host, token: Redacted.make(token), credentialFingerprint: key }; + // A cold page may ask several times. Wait per credential and check again after the + // first verification; cancellation releases the next waiter without losing its request. + return yield* Effect.acquireUseRelease( + Effect.sync(() => { + const lock = identityLocks.get(key) ?? { gate: Semaphore.makeUnsafe(1), users: 0 }; + lock.users++; + identityLocks.set(key, lock); + return lock; + }), + (lock) => + lock.gate.withPermit( + Effect.gen(function* () { + const now = yield* Clock.currentTimeMillis; + const cached = routingIdentities.get(key); + if (cached !== undefined && now - cached.at < 10 * 60_000) + return { ...credential, ...cached.value }; + // Pin this read so an auth switch cannot poison its cache entry. + const response = yield* github + .execute({ + cwd: input.cwd, + args: ["api", "user", "--hostname", host], + env: { + GH_HOST: host, + GH_TOKEN: token, + GITHUB_TOKEN: token, + GH_ENTERPRISE_TOKEN: token, + GITHUB_ENTERPRISE_TOKEN: token, + GH_DEBUG: "", + }, + }) + .pipe(Effect.mapError(unavailable)); + const identity = yield* decodeRoutingIdentity(response.stdout).pipe( + Effect.mapError(unavailable), + ); + const value = { accountId: String(identity.id), viewer: identity.login }; + if (routingIdentities.size >= 128) + routingIdentities.delete(routingIdentities.keys().next().value!); + routingIdentities.set(key, { at: now, value }); + return { ...credential, ...value }; + }), + ), + (lock) => + Effect.sync(() => { + lock.users--; + if (lock.users === 0) identityLocks.delete(key); + }), + ); + }, + ); + const withVerifiedCredential: GitHubPullRequestCli["Service"]["withVerifiedCredential"] = ( + input, + use, + ) => + captureVerifiedCredential(input).pipe( + Effect.flatMap(({ host, token, accountId, viewer, credentialFingerprint }) => + use({ accountId, viewer, credentialFingerprint }).pipe( + Effect.provideService(GitHubCli.PinnedGitHubCredential, { + host, + token, + credentialFingerprint, + }), + ), + ), + ); + const getRoutingIdentity: GitHubPullRequestCli["Service"]["getRoutingIdentity"] = (input) => + captureVerifiedCredential(input).pipe( + Effect.map(({ accountId, viewer }) => ({ accountId, viewer })), + ); /** * The pull request's own node id, which is what a mutation against the pull request itself is @@ -1468,15 +1595,10 @@ export const make = Effect.gen(function* () { ); return GitHubPullRequestCli.of({ + withVerifiedCredential, + getRoutingIdentity, getViewerLogin: (input) => - github.execute({ cwd: input.cwd, args: ["api", "user", "--jq", ".login"] }).pipe( - Effect.flatMap((result) => { - const login = result.stdout.trim(); - return login.length > 0 - ? Effect.succeed(login) - : Effect.fail(new GitHubViewerLoginUnavailableError({ command: "gh", cwd: input.cwd })); - }), - ), + getRoutingIdentity(input).pipe(Effect.map((identity) => identity.viewer)), listPullRequests: (input) => { const fallbackMaxRows = Math.max(input.limit + 1, PULL_REQUEST_FALLBACK_MAX_ROWS); diff --git a/apps/server/src/pullRequest/GitHubPullRequestProvider.test.ts b/apps/server/src/pullRequest/GitHubPullRequestProvider.test.ts index fe5032d70d5cd884955ed5f9e7f4d610121f1085..c2571d91d5636e1ca337bb46f1dc20f224550f09 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestProvider.test.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestProvider.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "@effect/vitest"; import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; +import * as Redacted from "effect/Redacted"; import type { PullRequestReaction } from "@t3tools/contracts"; import * as GitHubCli from "../sourceControl/GitHubCli.ts"; @@ -8,6 +9,44 @@ import * as GitHubPullRequestCli from "./GitHubPullRequestCli.ts"; import { gitHubViewerPermissions, loginAvatarUrl, make } from "./GitHubPullRequestProvider.ts"; import type { GitHubReviewThreadComments } from "./gitHubPullRequestJson.ts"; +it.effect("maps credential verification failures without relabeling operation failures", () => + Effect.gen(function* () { + let verificationFails = true; + let operations = 0; + const provider = yield* make.pipe( + Effect.provide( + Layer.mock(GitHubPullRequestCli.GitHubPullRequestCli)({ + withVerifiedCredential: (_input, use) => + verificationFails + ? Effect.fail( + new GitHubPullRequestCli.GitHubViewerLoginUnavailableError({ + command: "gh", + cwd: "/w", + }), + ) + : use({ accountId: "123", viewer: "viewer", credentialFingerprint: "fingerprint" }), + }), + ), + ); + const verify = provider.withVerifiedCredential; + if (verify === undefined) + return yield* Effect.die("credential verification was not implemented"); + const input = { cwd: "/w", host: "github.com" }; + const operation = () => + Effect.sync(() => { + operations++; + }).pipe(Effect.andThen(Effect.fail("operation-failed"))); + expect(yield* verify(input, operation).pipe(Effect.flip)).toMatchObject({ + _tag: "PullRequestProviderError", + operation: "routeIdentity", + }); + expect(operations).toBe(0); + verificationFails = false; + expect(yield* verify(input, operation).pipe(Effect.flip)).toBe("operation-failed"); + expect(operations).toBe(1); + }), +); + it.effect("uses one narrow read for a linked pull request summary", () => Effect.gen(function* () { let summaryReads = 0; @@ -215,6 +254,23 @@ describe("gitHubViewerPermissions", () => { description: "GitHub could not determine whether workflows are awaiting approval.", url: null, }); + for (const fingerprint of ["broad", "restricted", "broad"]) { + const scoped = yield* provider + .getChangeRequest({ + cwd: "/w", + repository: "acme/web", + host: "github.com", + number: 7, + }) + .pipe( + Effect.provideService(GitHubCli.PinnedGitHubCredential, { + host: "github.com", + token: Redacted.make("credential"), + credentialFingerprint: fingerprint, + }), + ); + expect(scoped.mergeCapabilities.squash).toBe(fingerprint !== "restricted"); + } }).pipe( Effect.provide( Layer.mock(GitHubPullRequestCli.GitHubPullRequestCli)({ @@ -250,10 +306,16 @@ describe("gitHubViewerPermissions", () => { commits: [], }), getRepositoryAccess: () => - Effect.succeed({ - canWrite: false, - mergeCapabilities: { merge: true, squash: true, rebase: true }, - }), + GitHubCli.PinnedGitHubCredential.pipe( + Effect.map((credential) => ({ + canWrite: false, + mergeCapabilities: { + merge: true, + squash: credential?.credentialFingerprint !== "restricted", + rebase: true, + }, + })), + ), getViewerAccess: () => Effect.succeed({ canWrite: false, @@ -513,6 +575,36 @@ it.effect("propagates workflow discovery rate limits", () => ); describe("getViewerPermissions", () => { + it.effect("checks fresh access without reading branch details for unrelated operations", () => { + let accessReads = 0; + return Effect.gen(function* () { + const provider = yield* make; + const permissions = yield* provider.getViewerPermissions({ + cwd: "/w", + repository: "acme/web", + host: "github.com", + number: 7, + includeUpdateBranch: false, + }); + + expect(accessReads).toBe(1); + expect(permissions.actions).toContain("merge"); + expect(permissions.actions).not.toContain("update-branch"); + }).pipe( + Effect.provide( + Layer.mock(GitHubPullRequestCli.GitHubPullRequestCli)({ + getPullRequestDetail: () => Effect.die("Unexpected detail read"), + getPullRequestBaseComparison: () => Effect.die("Unexpected comparison read"), + getViewerAccess: () => + Effect.sync(() => { + accessReads++; + return { canWrite: true, canTriage: true, canUpdate: true, didAuthor: false }; + }), + }), + ), + ); + }); + const layerWithComparison = ( comparison: Effect.Effect<{ readonly behindBy: number | null; diff --git a/apps/server/src/pullRequest/GitHubPullRequestProvider.ts b/apps/server/src/pullRequest/GitHubPullRequestProvider.ts index 66a247efb90e23e8360fcc027eddc5fa1ef4993e..f8019ca36b4fff4ff5efd066b9864637a66befd5 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestProvider.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestProvider.ts @@ -11,6 +11,7 @@ import type { } from "@t3tools/contracts"; import * as GitHubPullRequestCli from "./GitHubPullRequestCli.ts"; +import { PinnedGitHubCredential } from "../sourceControl/GitHubCli.ts"; import { PullRequestProviderError, type PullRequestProviderFailure, @@ -198,7 +199,20 @@ export const make = Effect.gen(function* () { readonly cwd: string; readonly repository: string; readonly host: string; - }) => Cache.get(repositoryAccessCache, JSON.stringify([input.cwd, input.repository, input.host])); + }) => + PinnedGitHubCredential.pipe( + Effect.flatMap((credential) => + Cache.get( + repositoryAccessCache, + JSON.stringify([ + input.cwd, + input.repository, + input.host, + credential?.credentialFingerprint ?? null, + ]), + ), + ), + ); const fail = (operation: string) => (error: GitHubPullRequestCli.GitHubPullRequestCliError) => new PullRequestProviderError({ @@ -212,9 +226,17 @@ export const make = Effect.gen(function* () { const provider: PullRequestProviderApi = { kind: "github", capabilities: CAPABILITIES, + getRoutingIdentity: (input) => + cli.getRoutingIdentity(input).pipe(Effect.mapError(fail("routeIdentity"))), + withVerifiedCredential: (input, use) => + 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 }).pipe(Effect.mapError(fail("getViewer"))), + cli + .getViewerLogin({ cwd: input.cwd, host: input.host ?? "github.com" }) + .pipe(Effect.mapError(fail("getViewer"))), listChangeRequests: (input) => cli @@ -494,20 +516,22 @@ export const make = Effect.gen(function* () { // comparison only resolves through the head ref the detail carries. A failure here // withholds that one action rather than the whole answer, the way the detail path // leaves the banner unknown. - cli.getPullRequestDetail(input).pipe( - Effect.flatMap((pullRequest) => - pullRequest.state !== "open" || pullRequest.headRepositoryOwner === null - ? Effect.succeed(false) - : cli - .getPullRequestBaseComparison({ - ...input, - headRef: `${pullRequest.headRepositoryOwner}:${pullRequest.headBranch}`, - allowReserve: true, - }) - .pipe(Effect.map((comparison) => comparison.viewerCanUpdate === true)), - ), - Effect.orElseSucceed(() => false), - ), + input.includeUpdateBranch === false + ? Effect.succeed(false) + : cli.getPullRequestDetail(input).pipe( + Effect.flatMap((pullRequest) => + pullRequest.state !== "open" || pullRequest.headRepositoryOwner === null + ? Effect.succeed(false) + : cli + .getPullRequestBaseComparison({ + ...input, + headRef: `${pullRequest.headRepositoryOwner}:${pullRequest.headBranch}`, + allowReserve: true, + }) + .pipe(Effect.map((comparison) => comparison.viewerCanUpdate === true)), + ), + Effect.orElseSucceed(() => false), + ), ], { concurrency: 2 }, ).pipe( diff --git a/apps/server/src/pullRequest/PullRequestProvider.ts b/apps/server/src/pullRequest/PullRequestProvider.ts index ba8d13908b5ee4c1ddd8152f978fdfedc986c01a..405eb6ae9e6cd1dac82b61a9a6663a04f5fc078f 100644 --- a/apps/server/src/pullRequest/PullRequestProvider.ts +++ b/apps/server/src/pullRequest/PullRequestProvider.ts @@ -276,6 +276,21 @@ export interface ProviderRepositoryRef { * failing at call time. */ export interface PullRequestProviderApi { + readonly withVerifiedCredential?: ( + input: { readonly cwd: string; readonly host: string }, + use: (identity: { + readonly accountId: string; + readonly viewer: string; + readonly credentialFingerprint: string; + }) => Effect.Effect, + ) => Effect.Effect; + readonly getRoutingIdentity?: (input: { + readonly cwd: string; + readonly host: string; + }) => Effect.Effect< + { readonly accountId: string; readonly viewer: string }, + PullRequestProviderError + >; readonly kind: SourceControlProviderKind; readonly capabilities: PullRequestCapabilities; @@ -398,7 +413,11 @@ export interface PullRequestProviderApi { * is no request at all. */ readonly getViewerPermissions: ( - input: ProviderRepositoryRef & { readonly number: number }, + input: ProviderRepositoryRef & { + readonly number: number; + /** Skip branch comparison when checking permission for an unrelated operation. */ + readonly includeUpdateBranch?: boolean; + }, ) => Effect.Effect; /** diff --git a/apps/server/src/pullRequest/PullRequestService.test.ts b/apps/server/src/pullRequest/PullRequestService.test.ts index da554ffc40012aa8c530eda271665f7542a6a5c8..d354fd36d3170961ca6ef4e763216c3d735df51a 100644 --- a/apps/server/src/pullRequest/PullRequestService.test.ts +++ b/apps/server/src/pullRequest/PullRequestService.test.ts @@ -16,6 +16,7 @@ import type { PullRequestReviewerCapabilities, SourceControlProviderKind, } from "@t3tools/contracts"; +import { PullRequestOperationError } from "@t3tools/contracts"; import * as ProjectionSnapshotQuery from "../orchestration/Services/ProjectionSnapshotQuery.ts"; import * as SourceControlProviderRegistry from "../sourceControl/SourceControlProviderRegistry.ts"; @@ -974,6 +975,51 @@ it.effect("tries another workspace on the same host for the viewer", () => }), ); +it.effect("routing verifies the current account on the requested host without caching it", () => + Effect.gen(function* () { + let viewer = "first-account"; + const service = yield* makeService({ + projects: [ + project({ + id: "p1", + title: "web", + workspaceRoot: "/a", + repository: "acme/web", + host: "github.example.test", + }), + ], + providers: [ + fakeProvider("github", { + getRoutingIdentity: (input) => { + assert.deepStrictEqual(input, { cwd: "/a", host: "github.example.test" }); + return Effect.succeed({ + viewer, + accountId: viewer === "first-account" ? "123" : "456", + }); + }, + }), + ], + }); + const ref = { projectId: "p1" as ProjectId, repository: "acme/web", number: 1 }; + assert.deepStrictEqual(yield* service.routing(ref), { + host: "github.example.test", + provider: "github", + viewer: "first-account", + accountId: "123", + projectTitle: "web", + workspaceRoot: "/a", + }); + viewer = "second-account"; + assert.strictEqual((yield* service.routing(ref)).viewer, "second-account"); + viewer = " "; + const failure = yield* service.routing(ref).pipe(Effect.flip); + assert.strictEqual(failure._tag, "PullRequestOperationError"); + if (failure._tag === "PullRequestOperationError") { + assert.strictEqual(failure.operation, "routeIdentity"); + } + }), +); + it.effect("refuses an action the host never claimed it could run", () => Effect.gen(function* () { let ran = false; @@ -3707,9 +3753,7 @@ it.effect("shares linked summaries and reuses them for display without asking th yield* TestClock.adjust("61 seconds"); failing = true; - const strict = yield* Effect.flip( - service.summary(reference, { recoverTransientFailure: false }), - ); + const strict = yield* Effect.flip(service.summary({ ...reference, allowStale: false })); assert.strictEqual(strict._tag, "PullRequestOperationError"); const stale = yield* service.summary(reference); @@ -3723,6 +3767,147 @@ it.effect("shares linked summaries and reuses them for display without asking th }), ); +it.effect("keeps routed summaries and details separate when the GitHub account changes", () => + Effect.gen(function* () { + for (const operation of ["summary", "detail"] as const) { + let failing = false; + let calls = 0; + const read = () => + Effect.suspend(() => { + calls += 1; + return failing + ? Effect.fail(requestFailed) + : Effect.succeed(hostedChangeRequest("account A content")); + }); + const service = yield* makeService({ + projects: [ + project({ id: "p1", title: "web", workspaceRoot: "/a", repository: "acme/web" }), + ], + providers: [ + fakeProvider("github", { getChangeRequestSummary: read, getChangeRequest: read }), + ], + }); + const reference = { projectId: "p1" as ProjectId, repository: "acme/web", number: 1 }; + yield* service[operation]({ ...reference, expectedAccountId: "101" }); + failing = true; + + for (const allowStale of [false, true]) { + const error = yield* Effect.flip( + service[operation]({ ...reference, expectedAccountId: "202", allowStale }), + ); + assert.strictEqual(error._tag, "PullRequestOperationError"); + } + assert.strictEqual(calls, 3); + } + }), +); + +it.effect("isolates routed caches for two credentials belonging to the same account", () => + Effect.gen(function* () { + for (const operation of ["summary", "detail"] as const) { + let credential = "broad"; + let calls = 0; + const read = () => + Effect.suspend(() => { + calls += 1; + return credential === "broad" + ? Effect.succeed(hostedChangeRequest("private content")) + : Effect.fail(requestFailed); + }); + const service = yield* makeService({ + projects: [ + project({ id: "p1", title: "web", workspaceRoot: "/a", repository: "acme/web" }), + ], + providers: [ + fakeProvider("github", { + withVerifiedCredential: (_, use) => + Effect.suspend(() => + use({ + accountId: "101", + viewer: "octocat", + credentialFingerprint: credential, + }), + ), + getChangeRequest: read, + getChangeRequestSummary: read, + }), + ], + }); + const reference = { + projectId: "p1" as ProjectId, + repository: "acme/web", + number: 1, + host: "github.com", + expectedAccountId: "101", + }; + yield* service.withRoutingCredential(reference, service[operation](reference)); + credential = "restricted"; + for (const allowStale of [false, true]) { + const error = yield* Effect.flip( + service.withRoutingCredential( + reference, + service[operation]({ ...reference, allowStale }), + ), + ); + assert.strictEqual(error._tag, "PullRequestOperationError"); + } + assert.strictEqual(calls, 3); + } + }), +); + +it.effect("rejects mismatched routing credentials before use and preserves action errors", () => + Effect.gen(function* () { + let operations = 0; + const service = yield* makeService({ + projects: [project({ id: "p1", title: "web", workspaceRoot: "/a", repository: "acme/web" })], + providers: [ + fakeProvider("github", { + getRoutingIdentity: () => Effect.succeed({ accountId: "101", viewer: "octocat" }), + withVerifiedCredential: (_, use) => + use({ + accountId: "101", + viewer: "octocat", + credentialFingerprint: "credential-a", + }), + }), + ], + }); + const reference = { + projectId: "p1" as ProjectId, + repository: "acme/web", + number: 1, + host: "github.com", + expectedAccountId: "202", + }; + const actionError = new PullRequestOperationError({ + operation: "runAction", + detail: "ambiguous", + }); + const operation = Effect.sync(() => { + operations += 1; + }).pipe(Effect.andThen(Effect.fail(actionError))); + const rejected = yield* Effect.flip(service.withRoutingCredential(reference, operation)); + assert.strictEqual(rejected._tag, "PullRequestOperationError"); + if (rejected._tag === "PullRequestOperationError") + assert.strictEqual(rejected.operation, "routeIdentity"); + assert.strictEqual(operations, 0); + assert.strictEqual( + yield* Effect.flip( + service.withRoutingCredential({ ...reference, expectedAccountId: "101" }, operation), + ), + actionError, + ); + assert.strictEqual(operations, 1); + assert.deepStrictEqual(yield* service.routingIdentity({ host: "github.com" }), { + accountId: "101", + viewer: "octocat", + host: "github.com", + provider: "github", + }); + }), +); + it.effect("answers a known pull request immediately while the host refreshes", () => Effect.gen(function* () { const gate = yield* Deferred.make(); @@ -3895,6 +4080,13 @@ it.effect("does not let a stale detail reopen overwrite a fresher linked summary assert.strictEqual(display.title, "merged title"); assert.strictEqual(display.state, "merged"); assert.strictEqual(detailCalls, 2); + + summaryTitle = "updated after merge"; + yield* TestClock.adjust("61 seconds"); + assert.strictEqual((yield* service.summary(reference)).title, "merged title"); + const refreshed = yield* service.summary({ ...reference, allowStale: false }); + assert.strictEqual(refreshed.title, "updated after merge"); + assert.strictEqual(refreshed.state, "merged"); }), ); @@ -3978,6 +4170,8 @@ it.effect("keeps recent detail on a transient refresh failure but not after inva yield* service.detail(reference); yield* TestClock.adjust("16 seconds"); failing = true; + const strict = yield* Effect.flip(service.detail({ ...reference, allowStale: false })); + assert.strictEqual(strict._tag, "PullRequestOperationError"); const stale = yield* service.detail(reference); assert.strictEqual(stale.body, "last good body"); diff --git a/apps/server/src/pullRequest/PullRequestService.ts b/apps/server/src/pullRequest/PullRequestService.ts index 005acfa0f5c07182a0338c7c8a70ff4ed8ed4234..e4bd807cbefd7623d8012ecce71ea9acaeae9576 100644 --- a/apps/server/src/pullRequest/PullRequestService.ts +++ b/apps/server/src/pullRequest/PullRequestService.ts @@ -46,6 +46,9 @@ import { type PullRequestProviderSummary, type PullRequestReactionInput, type PullRequestRef, + type PullRequestRoutingResult, + type PullRequestRoutingIdentityInput, + type PullRequestRoutingIdentityResult, type PullRequestReviewVerdict, type PullRequestReviewerCandidateList, type PullRequestReviewerRequestInput, @@ -138,6 +141,14 @@ const VIEWER_CACHE_CAPACITY = 32; export type PullRequestError = PullRequestUnavailableError | PullRequestOperationError; +const routingCredential = Context.Reference<{ + readonly credentialFingerprint: string; + readonly viewer: string; +} | null>("t3/PullRequestService/routingCredential", { defaultValue: () => null }); +// Internal only: the client cannot choose its cache's credential namespace. +const credentialNamespace = Symbol("pullRequestCredentialNamespace"); +type CredentialRef = PullRequestRef & { readonly [credentialNamespace]?: string }; + export class PullRequestService extends Context.Service< PullRequestService, { @@ -147,6 +158,16 @@ export class PullRequestService extends Context.Service< readonly listStats: ( input: PullRequestListStatsInput, ) => Effect.Effect; + readonly routing: ( + input: PullRequestRef, + ) => Effect.Effect; + readonly routingIdentity: ( + input: PullRequestRoutingIdentityInput, + ) => Effect.Effect; + readonly withRoutingCredential: ( + input: PullRequestRef, + operation: Effect.Effect, + ) => Effect.Effect; readonly summary: ( input: PullRequestRef, options?: { readonly recoverTransientFailure?: boolean }, @@ -473,6 +494,10 @@ function withRateLimitBackoff( kind: api.kind, capabilities: api.capabilities, getViewer: wrap("getViewer", api.getViewer), + ...(api.getRoutingIdentity === undefined ? {} : { getRoutingIdentity: api.getRoutingIdentity }), + ...(api.withVerifiedCredential === undefined + ? {} + : { withVerifiedCredential: api.withVerifiedCredential }), listChangeRequests: wrap("listChangeRequests", api.listChangeRequests), ...(api.listChangeRequestsAcross === undefined ? {} @@ -776,13 +801,19 @@ export const make = Effect.gen(function* () { * handed to a provider on the client's word. Read freshly for that reason, rather than taken * from whatever the detail said when the page loaded. */ - const viewerPermissionsOf = (project: SupportedProject, ref: PullRequestRef, operation: string) => + const viewerPermissionsOf = ( + project: SupportedProject, + ref: PullRequestRef, + operation: string, + includeUpdateBranch = false, + ) => project.api .getViewerPermissions({ cwd: project.project.workspaceRoot, repository: project.repository, host: project.host, number: ref.number, + includeUpdateBranch, }) .pipe(Effect.mapError(toPullRequestError(operation))); @@ -1309,7 +1340,92 @@ export const make = Effect.gen(function* () { * and a host that cannot say leaves it null rather than failing the read it decorates. */ const viewerOf = (project: SupportedProject): Effect.Effect => - resolveViewers([project], new Map()).pipe(Effect.map(([resolved]) => resolved?.viewer ?? null)); + routingCredential.pipe( + Effect.flatMap((credential) => + credential !== null + ? Effect.succeed(credential.viewer) + : resolveViewers([project], new Map()).pipe( + Effect.map(([resolved]) => resolved?.viewer ?? null), + ), + ), + ); + + const routingIdentity: PullRequestService["Service"]["routingIdentity"] = Effect.fn( + "PullRequestService.routingIdentity", + )(function* (input) { + const host = input.host.toLowerCase(); + const { supported } = yield* listWorkspaceProjects({ host }); + const project = supported.find((candidate) => candidate.api.kind === "github"); + const api = registry.get("github"); + if (project === undefined || api?.getRoutingIdentity === undefined) { + return yield* new PullRequestUnavailableError({ reason: "provider-unsupported" }); + } + const identity = yield* api + .getRoutingIdentity({ + cwd: project.project.workspaceRoot, + host, + }) + .pipe(Effect.mapError(toPullRequestError("routeIdentity"))); + return { ...identity, host, provider: "github" as const }; + }); + + const withRoutingCredential: PullRequestService["Service"]["withRoutingCredential"] = ( + input, + operation, + ) => + Effect.gen(function* () { + if (input.expectedAccountId === undefined) return yield* operation; + const rejected = () => + new PullRequestOperationError({ + operation: "routeIdentity", + detail: "The GitHub account could not be verified before starting the operation.", + }); + const project = yield* requireProject(input).pipe(Effect.mapError(rejected)); + const api = project.api.kind === "github" ? registry.get("github") : null; + if ( + api?.withVerifiedCredential === undefined || + input.host?.toLowerCase() !== project.host.toLowerCase() + ) { + return yield* rejected(); + } + const result = yield* api + .withVerifiedCredential( + { cwd: project.project.workspaceRoot, host: project.host }, + (identity) => + identity.accountId === input.expectedAccountId + ? operation.pipe(Effect.provideService(routingCredential, identity), Effect.result) + : Effect.fail(rejected()), + ) + .pipe(Effect.catchTag("PullRequestProviderError", () => Effect.fail(rejected()))); + return yield* Effect.fromResult(result); + }); + + const routing = Effect.fn("PullRequestService.routing")(function* (input: PullRequestRef) { + const project = yield* requireProject(input); + const api = project.api.kind === "github" ? registry.get("github") : null; + if (api?.getRoutingIdentity === undefined) { + return yield* new PullRequestUnavailableError({ reason: "provider-unsupported" }); + } + const identity = yield* api + .getRoutingIdentity({ + cwd: project.project.workspaceRoot, + host: project.host, + }) + .pipe(Effect.mapError(toPullRequestError("routeIdentity"))); + if (!identity.viewer.trim() || !identity.accountId.trim()) { + return yield* new PullRequestOperationError({ + operation: "routeIdentity", + detail: "The signed-in account could not be verified.", + }); + } + return { + host: project.host, + provider: project.api.kind, + ...identity, + projectTitle: project.project.title, + workspaceRoot: project.project.workspaceRoot, + }; + }); const summaryUncached: PullRequestService["Service"]["summary"] = (input) => requireProject(input).pipe( @@ -1620,7 +1736,12 @@ export const make = Effect.gen(function* () { // What the host can do and what this account may ask of it are two questions, and both // have to say yes. The second is asked last, because it costs a request and the checks // above do not. - return viewerPermissionsOf(project, input, "runAction").pipe( + return viewerPermissionsOf( + project, + input, + "runAction", + input.action === "update-branch", + ).pipe( Effect.flatMap((viewer): Effect.Effect => { const stackRebase = input.stackNumber !== undefined && input.action === "update-branch"; if ( @@ -2193,6 +2314,12 @@ export const make = Effect.gen(function* () { const context = yield* Effect.context(); const runFork = Effect.runForkWith(context); + const revalidate = (read: Effect.Effect) => + Effect.context().pipe( + Effect.flatMap((caller) => + Effect.sync(() => runFork(Effect.ignore(read).pipe(Effect.provideContext(caller)))), + ), + ); /** * The diff is not live-polled and is expensive enough to keep its stale-while-revalidate path. @@ -2218,7 +2345,7 @@ export const make = Effect.gen(function* () { // Run as its own fiber rather than a child: the caller is answered and gone before the // refresh lands. The read still coalesces on the cache key, so ten stale reads in one // window cost one host request — and a failed refresh costs nothing but the retry. - return Effect.sync(() => runFork(Effect.ignore(recorded))).pipe(Effect.as(snapshot.value)); + return revalidate(recorded).pipe(Effect.as(snapshot.value)); }); }; })(); @@ -2276,9 +2403,7 @@ export const make = Effect.gen(function* () { const snapshot = held.get(key); if (snapshot === undefined) return read(key, effect); if (mode === "reuse") return Effect.succeed(snapshot.value); - return Effect.sync(() => runFork(Effect.ignore(read(key, effect)))).pipe( - Effect.as(snapshot.value), - ); + return revalidate(read(key, effect)).pipe(Effect.as(snapshot.value)); }; return { peek: (key: string) => held.get(key)?.value, read, record, serveHeld }; }; @@ -2300,25 +2425,25 @@ export const make = Effect.gen(function* () { Math.max(turnRefreshEpoch, refEpochs.get(refScope(ref)) ?? 0); // Keys carry the reference back out of the cache loader, so the slot layout is shared with // `refOfCacheKey` rather than read positionally at every loader. - const refCacheKey = (ref: PullRequestRef) => + const refCacheKey = (ref: CredentialRef) => JSON.stringify([ refEpoch(ref), ref.projectId, ref.host?.toLowerCase() ?? null, ref.repository.toLowerCase(), ref.number, + ref.expectedAccountId ?? null, + ref[credentialNamespace] ?? null, ]); const refOfCacheKey = (key: string): PullRequestRef => { - const [, projectId, host, repository, number] = JSON.parse(key) as [ - number, - string, - string | null, - string, - number, - ]; + const [, projectId, host, repository, number, expectedAccountId, fingerprint] = JSON.parse( + key, + ) as [number, string, string | null, string, number, string | null, string | null]; return { projectId, ...(host === null ? {} : { host }), + ...(expectedAccountId === null ? {} : { expectedAccountId }), + ...(fingerprint === null ? {} : { [credentialNamespace]: fingerprint }), repository, number, } as PullRequestRef; @@ -2365,7 +2490,7 @@ export const make = Effect.gen(function* () { }; const persistedRead = Effect.fn("PullRequestService.persistedRead")(function* ( - input: PullRequestRef, + input: CredentialRef, operation: string, codec: Schema.Codec, read: Effect.Effect, @@ -2379,6 +2504,8 @@ export const make = Effect.gen(function* () { project.project.id, project.project.workspaceRoot, String(input.number), + input.expectedAccountId ?? "", + input[credentialNamespace] ?? "", ] .map(encodeURIComponent) .join(":"); @@ -2409,6 +2536,7 @@ export const make = Effect.gen(function* () { const cached = persistedRead(input, "summary", summaryCodec, summaryUncached(input)); const held = lastGoodSummary.peek(key); return held !== undefined && + input.allowStale !== false && (options?.recoverTransientFailure !== false || held.state === "merged") ? Effect.succeed(held) : cached.pipe( @@ -2566,18 +2694,17 @@ export const make = Effect.gen(function* () { // `serveHeld` returns immediately. Skip the write when that read is older // than a later strict summary — display reuse would otherwise keep the // regression and never ask the host again. - return lastGoodDetail.serveHeld( - key, - Cache.get(detailCache, key).pipe( - Effect.tap((value) => { - const summary = summaryFromDetail(value, lastGoodSummary.peek(key)); - return shouldReplaceHeldSummary(key, summary) - ? lastGoodSummary.record(key, summary) - : Effect.void; - }), - ), - "revalidate", + const read = Cache.get(detailCache, key).pipe( + Effect.tap((value) => { + const summary = summaryFromDetail(value, lastGoodSummary.peek(key)); + return shouldReplaceHeldSummary(key, summary) + ? lastGoodSummary.record(key, summary) + : Effect.void; + }), ); + return input.allowStale === false + ? read.pipe(Effect.tap((value) => lastGoodDetail.record(key, value))) + : lastGoodDetail.serveHeld(key, read, "revalidate"); }; const activityCache = yield* Cache.makeWith( @@ -2758,11 +2885,33 @@ export const make = Effect.gen(function* () { } }); + const credentialCached = + , A, E>( + read: (input: I, ...args: Args) => Effect.Effect, + ) => + (input: I, ...args: Args) => + routingCredential.pipe( + Effect.flatMap((credential) => + read( + credential === null + ? input + : { + ...input, + [credentialNamespace]: credential.credentialFingerprint, + }, + ...args, + ), + ), + ); + return PullRequestService.of({ + routing, + routingIdentity, + withRoutingCredential, list, listStats, - summary, - stack, + summary: credentialCached(summary), + stack: credentialCached(stack), subscribeMerges: PubSub.subscribe(mergedPullRequests).pipe( Effect.map((subscription) => Stream.fromSubscription(subscription)), ), @@ -2770,8 +2919,8 @@ export const make = Effect.gen(function* () { Stream.filter((revision) => revision > 0), ), refreshAfterTurn, - detail, - activity, + detail: credentialCached(detail), + activity: credentialCached(activity), threadComments, diff, diffFileContents, diff --git a/apps/server/src/sourceControl/GitHubCli.test.ts b/apps/server/src/sourceControl/GitHubCli.test.ts index f72259b677ebfc4edc85796659540204bf0f3b40..d3fe840fa64c0b2e581067943604229c2979f625 100644 --- a/apps/server/src/sourceControl/GitHubCli.test.ts +++ b/apps/server/src/sourceControl/GitHubCli.test.ts @@ -1,13 +1,18 @@ import { assert, it, afterEach, describe, expect, vi } from "@effect/vitest"; +import * as Cache from "effect/Cache"; import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; import * as PlatformError from "effect/PlatformError"; +import * as Redacted from "effect/Redacted"; +import * as Schema from "effect/Schema"; import { ChildProcessSpawner } from "effect/unstable/process"; import { VcsProcessExitError, VcsProcessSpawnError } from "@t3tools/contracts"; import * as VcsProcess from "../vcs/VcsProcess.ts"; import * as GitHubCli from "./GitHubCli.ts"; +const encodeGitHubCliError = Schema.encodeEffect(Schema.fromJsonString(GitHubCli.GitHubCliError)); + const processOutput = (stdout: string): VcsProcess.VcsProcessOutput => ({ exitCode: ChildProcessSpawner.ExitCode(0), stdout, @@ -31,6 +36,111 @@ afterEach(() => { }); describe("GitHubCli.layer", () => { + it.effect("pins concurrent cached commands to their own verified credentials", () => + Effect.gen(function* () { + mockRun.mockImplementation((input) => + Effect.succeed(processOutput(input.env?.GH_TOKEN ?? "ambient")), + ); + const gh = yield* GitHubCli.GitHubCli; + // Constructed outside either request, like the PR service's read caches. + const cache = yield* Cache.make({ + lookup: (host: string) => + gh.execute({ + cwd: "/repo", + args: ["api", "user", "--hostname", host], + env: { GH_DEBUG: "api", GH_TOKEN: "changed-after-verification" }, + }), + capacity: 2, + timeToLive: "1 minute", + }); + const results = yield* Effect.all( + ["github.com", "github.example.test"].map((host, index) => + Cache.get(cache, host).pipe( + Effect.provideService(GitHubCli.PinnedGitHubCredential, { + host, + token: Redacted.make(`credential-${index}`), + credentialFingerprint: `fingerprint-${index}`, + }), + ), + ), + { concurrency: 2 }, + ); + expect(results.map((result) => result.stdout)).toEqual(["credential-0", "credential-1"]); + for (const [input] of mockRun.mock.calls) { + expect(input.env).toMatchObject({ + GH_HOST: input.args[3], + GH_DEBUG: "", + GH_TOKEN: input.env?.GITHUB_TOKEN, + GH_ENTERPRISE_TOKEN: input.env?.GH_TOKEN, + GITHUB_ENTERPRISE_TOKEN: input.env?.GH_TOKEN, + }); + } + expect((yield* gh.execute({ cwd: "/repo", args: ["api", "user"] })).stdout).toBe("ambient"); + }).pipe(Effect.provide(layer)), + ); + + it.effect("refuses other or implicit hosts before exposing a scoped credential to gh", () => + Effect.gen(function* () { + const gh = yield* GitHubCli.GitHubCli; + for (const args of [ + ["api", "user", "--hostname", "other.example.test"], + ["api", "user", "--hostname=other.example.test"], + ["pr", "view", "1", "--repo", "other.example.test/owner/repo"], + ["repo", "view", "other.example.test/owner/repo", "--json", "name"], + ["api", "https://other.example.test/user", "--hostname", "github.com"], + ["api", "user"], + ]) { + const failure = yield* gh.execute({ cwd: "/repo", args }).pipe( + Effect.provideService(GitHubCli.PinnedGitHubCredential, { + host: "github.com", + token: Redacted.make("secret-credential"), + credentialFingerprint: "fingerprint", + }), + Effect.flip, + ); + expect(failure._tag).toBe("GitHubCliCommandError"); + expect(yield* encodeGitHubCliError(failure)).not.toContain("secret-credential"); + } + expect(mockRun).not.toHaveBeenCalled(); + }).pipe(Effect.provide(layer)), + ); + + it.effect("pins repository-targeted writes on enterprise hosts", () => + Effect.gen(function* () { + mockRun.mockReturnValue(Effect.succeed(processOutput(""))); + const gh = yield* GitHubCli.GitHubCli; + yield* gh + .execute({ + cwd: "/repo", + args: ["pr", "merge", "1", "--repo", "github.example.test/owner/repo"], + }) + .pipe( + Effect.provideService(GitHubCli.PinnedGitHubCredential, { + host: "github.example.test", + token: Redacted.make("enterprise-credential"), + credentialFingerprint: "fingerprint", + }), + ); + yield* gh + .execute({ + cwd: "/repo", + args: ["repo", "view", "github.example.test/owner/repo", "--json", "name"], + }) + .pipe( + Effect.provideService(GitHubCli.PinnedGitHubCredential, { + host: "github.example.test", + token: Redacted.make("enterprise-credential"), + credentialFingerprint: "fingerprint", + }), + ); + expect(mockRun.mock.calls[0]?.[0].env).toMatchObject({ + GH_HOST: "github.example.test", + GH_ENTERPRISE_TOKEN: "enterprise-credential", + GH_DEBUG: "", + }); + }).pipe(Effect.provide(layer)), + ); + it("does not classify a missing cwd as an unavailable gh executable", () => { const context = { command: "gh", cwd: "/repo" } as const; const missingCwd = new VcsProcessSpawnError({ diff --git a/apps/server/src/sourceControl/GitHubCli.ts b/apps/server/src/sourceControl/GitHubCli.ts index cd8782c87d9e6d104ddcb575139d2fe3520139a9..30b0e4a09231c463879283f3cb83b6fd461f2e45 100644 --- a/apps/server/src/sourceControl/GitHubCli.ts +++ b/apps/server/src/sourceControl/GitHubCli.ts @@ -4,6 +4,7 @@ import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; import * as PlatformError from "effect/PlatformError"; +import * as Redacted from "effect/Redacted"; import * as Result from "effect/Result"; import * as Schema from "effect/Schema"; @@ -22,6 +23,40 @@ import { const DEFAULT_TIMEOUT_MS = 30_000; +/** Server-local credential scope; never put its value in RPC payloads or cache keys. */ +export const PinnedGitHubCredential = Context.Reference<{ + readonly host: string; + readonly token: Redacted.Redacted; + readonly credentialFingerprint: string; +} | null>("t3/sourceControl/PinnedGitHubCredential", { defaultValue: () => null }); + +function targetsVerifiedHost(args: ReadonlyArray, host: string): boolean { + const hosts: Array = []; + const repositoryHost = (repository: string | undefined) => { + if (repository === undefined) return null; + if (/^https?:\/\//i.test(repository)) { + try { + return new URL(repository).host.toLowerCase(); + } catch { + return null; + } + } + const parts = repository.split("/"); + return parts.length === 3 ? parts[0]!.toLowerCase() : null; + }; + if (args[0] === "repo" && args[1] === "view") hosts.push(repositoryHost(args[2])); + for (let index = 0; index < args.length; index++) { + const arg = args[index]!; + if (arg === "--hostname") hosts.push(args[++index]?.toLowerCase() ?? null); + else if (arg.startsWith("--hostname=")) hosts.push(arg.slice(11).toLowerCase()); + else if (arg === "--repo" || arg === "-R") hosts.push(repositoryHost(args[++index])); + else if (arg.startsWith("--repo=")) hosts.push(repositoryHost(arg.slice(7))); + else if (arg.startsWith("-R")) hosts.push(repositoryHost(arg.slice(2))); + else if (/^https?:\/\//i.test(arg)) hosts.push(repositoryHost(arg)); + } + return hosts.length > 0 && hosts.every((target) => target === host); +} + const gitHubCliFailureFields = { command: Schema.Literal("gh"), cwd: Schema.String, @@ -237,6 +272,7 @@ export class GitHubCli extends Context.Service< readonly timeoutMs?: number; /** Piped to the child's stdin, for payloads that must never appear in argv. */ readonly stdin?: string; + readonly env?: NodeJS.ProcessEnv; readonly maxOutputBytes?: number; }) => Effect.Effect; @@ -342,18 +378,43 @@ function deriveRepositoryCloneUrlsFromCreateOutput( export const make = Effect.gen(function* () { const process = yield* VcsProcess.VcsProcess; - const execute: GitHubCli["Service"]["execute"] = (input) => - process - .run({ - operation: "GitHubCli.execute", - command: "gh", - args: input.args, - cwd: input.cwd, - timeoutMs: input.timeoutMs ?? DEFAULT_TIMEOUT_MS, - ...(input.stdin !== undefined ? { stdin: input.stdin } : {}), - ...(input.maxOutputBytes !== undefined ? { maxOutputBytes: input.maxOutputBytes } : {}), - }) - .pipe(Effect.mapError((error) => fromVcsError({ command: "gh", cwd: input.cwd }, error))); + const execute: GitHubCli["Service"]["execute"] = Effect.fn("GitHubCli.execute")( + function* (input) { + const credential = yield* PinnedGitHubCredential; + if (credential !== null && !targetsVerifiedHost(input.args, credential.host)) { + return yield* new GitHubCliCommandError({ + command: "gh", + cwd: input.cwd, + cause: new Error("The GitHub command does not target the verified credential's host."), + }); + } + const token = credential === null ? undefined : Redacted.value(credential.token); + const env = + credential === null + ? input.env + : { + ...input.env, + GH_HOST: credential.host, + GH_TOKEN: token, + GITHUB_TOKEN: token, + GH_ENTERPRISE_TOKEN: token, + GITHUB_ENTERPRISE_TOKEN: token, + GH_DEBUG: "", + }; + return yield* process + .run({ + operation: "GitHubCli.execute", + command: "gh", + args: input.args, + cwd: input.cwd, + timeoutMs: input.timeoutMs ?? DEFAULT_TIMEOUT_MS, + ...(input.stdin !== undefined ? { stdin: input.stdin } : {}), + ...(env !== undefined ? { env } : {}), + ...(input.maxOutputBytes !== undefined ? { maxOutputBytes: input.maxOutputBytes } : {}), + }) + .pipe(Effect.mapError((error) => fromVcsError({ command: "gh", cwd: input.cwd }, error))); + }, + ); return GitHubCli.of({ execute, diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index 9ac919ea41bb22147a7fe4a8ea07b77697a5ee3e..a4898ce5ef194b9b9711d424c2b49edda6cc713a 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -630,6 +630,7 @@ const makeWsRpcLayer = ( const sourceControlRepositories = yield* SourceControlRepositoryService.SourceControlRepositoryService; const pullRequests = yield* PullRequestService.PullRequestService; + const withPullRequestViewer = pullRequests.withRoutingCredential; const pullRequestSync = yield* PullRequestSyncReactor.PullRequestSyncReactor; const bootstrapCredentials = yield* PairingGrantStore.PairingGrantStore; const sessions = yield* SessionStore.SessionStore; @@ -2156,14 +2157,34 @@ const makeWsRpcLayer = ( observeRpcEffect(WS_METHODS.pullRequestsListStats, pullRequests.listStats(input), { "rpc.aggregate": "pull-requests", }), + [WS_METHODS.pullRequestsRoutingIdentity]: (input) => + observeRpcEffect( + WS_METHODS.pullRequestsRoutingIdentity, + pullRequests.routingIdentity(input), + { + "rpc.aggregate": "pull-requests", + }, + ), + [WS_METHODS.pullRequestsRouting]: (input) => + observeRpcEffect(WS_METHODS.pullRequestsRouting, pullRequests.routing(input), { + "rpc.aggregate": "pull-requests", + }), [WS_METHODS.pullRequestsSummary]: (input) => - observeRpcEffect(WS_METHODS.pullRequestsSummary, pullRequests.summary(input), { - "rpc.aggregate": "pull-requests", - }), + observeRpcEffect( + WS_METHODS.pullRequestsSummary, + withPullRequestViewer(input, pullRequests.summary(input)), + { + "rpc.aggregate": "pull-requests", + }, + ), [WS_METHODS.pullRequestsStack]: (input) => - observeRpcEffect(WS_METHODS.pullRequestsStack, pullRequests.stack(input), { - "rpc.aggregate": "pull-requests", - }), + observeRpcEffect( + WS_METHODS.pullRequestsStack, + withPullRequestViewer(input, pullRequests.stack(input)), + { + "rpc.aggregate": "pull-requests", + }, + ), [WS_METHODS.pullRequestsLinkedThreads]: (input) => observeRpcEffect( WS_METHODS.pullRequestsLinkedThreads, @@ -2179,17 +2200,25 @@ const makeWsRpcLayer = ( { "rpc.aggregate": "pull-requests" }, ), [WS_METHODS.pullRequestsDetail]: (input) => - observeRpcEffect(WS_METHODS.pullRequestsDetail, pullRequests.detail(input), { - "rpc.aggregate": "pull-requests", - }), + observeRpcEffect( + WS_METHODS.pullRequestsDetail, + withPullRequestViewer(input, pullRequests.detail(input)), + { + "rpc.aggregate": "pull-requests", + }, + ), [WS_METHODS.pullRequestsActivity]: (input) => - observeRpcEffect(WS_METHODS.pullRequestsActivity, pullRequests.activity(input), { - "rpc.aggregate": "pull-requests", - }), + observeRpcEffect( + WS_METHODS.pullRequestsActivity, + withPullRequestViewer(input, pullRequests.activity(input)), + { + "rpc.aggregate": "pull-requests", + }, + ), [WS_METHODS.pullRequestsThreadComments]: (input) => observeRpcEffect( WS_METHODS.pullRequestsThreadComments, - pullRequests.threadComments(input), + withPullRequestViewer(input, pullRequests.threadComments(input)), { "rpc.aggregate": "pull-requests", }, @@ -2197,61 +2226,75 @@ const makeWsRpcLayer = ( [WS_METHODS.pullRequestsDiffFileContents]: (input) => observeRpcEffect( WS_METHODS.pullRequestsDiffFileContents, - pullRequests.diffFileContents(input), + withPullRequestViewer(input, pullRequests.diffFileContents(input)), { "rpc.aggregate": "pull-requests" }, ), [WS_METHODS.pullRequestsRunAction]: (input) => observeRpcEffect( WS_METHODS.pullRequestsRunAction, - pullRequests - .runAction(input) - .pipe( - Effect.tap(() => - resolvePullRequestSyncKey(input).pipe( - Effect.flatMap((key) => - key === null ? Effect.void : pullRequestSync.requestSync(key), - ), + withPullRequestViewer(input, pullRequests.runAction(input)).pipe( + Effect.tap(() => + resolvePullRequestSyncKey(input).pipe( + Effect.flatMap((key) => + key === null ? Effect.void : pullRequestSync.requestSync(key), ), ), ), + ), { "rpc.aggregate": "pull-requests" }, ), [WS_METHODS.pullRequestsUpdate]: (input) => - observeRpcEffect(WS_METHODS.pullRequestsUpdate, pullRequests.update(input), { - "rpc.aggregate": "pull-requests", - }), + observeRpcEffect( + WS_METHODS.pullRequestsUpdate, + withPullRequestViewer(input, pullRequests.update(input)), + { + "rpc.aggregate": "pull-requests", + }, + ), [WS_METHODS.pullRequestsComment]: (input) => - observeRpcEffect(WS_METHODS.pullRequestsComment, pullRequests.comment(input), { - "rpc.aggregate": "pull-requests", - }), + observeRpcEffect( + WS_METHODS.pullRequestsComment, + withPullRequestViewer(input, pullRequests.comment(input)), + { + "rpc.aggregate": "pull-requests", + }, + ), [WS_METHODS.pullRequestsUpdateComment]: (input) => observeRpcEffect( WS_METHODS.pullRequestsUpdateComment, - pullRequests.updateComment(input), + withPullRequestViewer(input, pullRequests.updateComment(input)), { "rpc.aggregate": "pull-requests", }, ), [WS_METHODS.pullRequestsSubmitReview]: (input) => - observeRpcEffect(WS_METHODS.pullRequestsSubmitReview, pullRequests.submitReview(input), { - "rpc.aggregate": "pull-requests", - }), + observeRpcEffect( + WS_METHODS.pullRequestsSubmitReview, + withPullRequestViewer(input, pullRequests.submitReview(input)), + { + "rpc.aggregate": "pull-requests", + }, + ), [WS_METHODS.pullRequestsReplyToThread]: (input) => observeRpcEffect( WS_METHODS.pullRequestsReplyToThread, - pullRequests.replyToThread(input), + withPullRequestViewer(input, pullRequests.replyToThread(input)), { "rpc.aggregate": "pull-requests" }, ), [WS_METHODS.pullRequestsSetThreadResolution]: (input) => observeRpcEffect( WS_METHODS.pullRequestsSetThreadResolution, - pullRequests.setThreadResolution(input), + withPullRequestViewer(input, pullRequests.setThreadResolution(input)), { "rpc.aggregate": "pull-requests" }, ), [WS_METHODS.pullRequestsSetReaction]: (input) => - observeRpcEffect(WS_METHODS.pullRequestsSetReaction, pullRequests.setReaction(input), { - "rpc.aggregate": "pull-requests", - }), + observeRpcEffect( + WS_METHODS.pullRequestsSetReaction, + withPullRequestViewer(input, pullRequests.setReaction(input)), + { + "rpc.aggregate": "pull-requests", + }, + ), [WS_METHODS.pullRequestsInvalidate]: (input) => observeRpcEffect( WS_METHODS.pullRequestsInvalidate, @@ -2279,25 +2322,29 @@ const makeWsRpcLayer = ( [WS_METHODS.pullRequestsReviewerCandidates]: (input) => observeRpcEffect( WS_METHODS.pullRequestsReviewerCandidates, - pullRequests.reviewerCandidates(input), + withPullRequestViewer(input, pullRequests.reviewerCandidates(input)), { "rpc.aggregate": "pull-requests" }, ), [WS_METHODS.pullRequestsRequestReviewers]: (input) => observeRpcEffect( WS_METHODS.pullRequestsRequestReviewers, - pullRequests.requestReviewers(input), + withPullRequestViewer(input, pullRequests.requestReviewers(input)), { "rpc.aggregate": "pull-requests" }, ), [WS_METHODS.pullRequestsLabelCandidates]: (input) => observeRpcEffect( WS_METHODS.pullRequestsLabelCandidates, - pullRequests.labelCandidates(input), + withPullRequestViewer(input, pullRequests.labelCandidates(input)), { "rpc.aggregate": "pull-requests" }, ), [WS_METHODS.pullRequestsSetLabels]: (input) => - observeRpcEffect(WS_METHODS.pullRequestsSetLabels, pullRequests.setLabels(input), { - "rpc.aggregate": "pull-requests", - }), + observeRpcEffect( + WS_METHODS.pullRequestsSetLabels, + withPullRequestViewer(input, pullRequests.setLabels(input)), + { + "rpc.aggregate": "pull-requests", + }, + ), [WS_METHODS.sourceControlLookupRepository]: (input) => observeRpcEffect( WS_METHODS.sourceControlLookupRepository, diff --git a/apps/web/src/components/settings/ConnectionsSettings.tsx b/apps/web/src/components/settings/ConnectionsSettings.tsx index 602be1cc3b517c41d9548177c44a790f050c329d..e8f1232b19bcfbb3ef4b3dcca2389e47892a720f 100644 --- a/apps/web/src/components/settings/ConnectionsSettings.tsx +++ b/apps/web/src/components/settings/ConnectionsSettings.tsx @@ -68,6 +68,7 @@ import { import { searchableSetting } from "./settingsSearch"; import { EnvironmentIconPicker } from "./EnvironmentIconPicker"; import { LoadBalancingSettings } from "./LoadBalancingSettings"; +import { GitHubRoutingSettings } from "./GitHubRoutingSettings"; import { Input } from "../ui/input"; import { CommandShortcut } from "../ui/command"; import { @@ -3714,6 +3715,7 @@ export function ConnectionsSettings() { savedEnvironments={savedEnvironments} /> + ); diff --git a/apps/web/src/components/settings/GitHubRoutingSettings.tsx b/apps/web/src/components/settings/GitHubRoutingSettings.tsx new file mode 100644 index 0000000000000000000000000000000000000000..8f83817294efc90affdcef34bc5c43065e5c9507 --- /dev/null +++ b/apps/web/src/components/settings/GitHubRoutingSettings.tsx @@ -0,0 +1,86 @@ +import { useAtomValue } from "@effect/atom-react"; +import { + gitHubRoutingConnectionKey, + gitHubRoutingPermissionFor, + type GitHubRoutingPermission, +} from "@t3tools/client-runtime/connection"; +import { useState } from "react"; + +import { environmentCatalog } from "~/connection/catalog"; +import type { EnvironmentPresentation } from "~/state/environments"; +import { useAtomCommand } from "~/state/use-atom-command"; +import { Select, SelectItem, SelectPopup, SelectTrigger, SelectValue } from "../ui/select"; +import { toastManager } from "../ui/toast"; +import { SettingsRow, SettingsSection } from "./settingsLayout"; +import { searchableSetting } from "./settingsSearch"; + +const options: ReadonlyArray<{ value: GitHubRoutingPermission; label: string }> = [ + { value: "off", label: "Off" }, + { value: "read", label: "Read PRs" }, + { value: "read-write", label: "Read and act" }, +]; + +export function GitHubRoutingSettings({ + environments, +}: { + readonly environments: ReadonlyArray; +}) { + const permissions = useAtomValue(environmentCatalog.githubRoutingPermissionsValueAtom); + const catalog = useAtomValue(environmentCatalog.catalogValueAtom); + const update = useAtomCommand(environmentCatalog.setGitHubRoutingPermission); + const [saving, setSaving] = useState(false); + + return ( + + + {environments.map((environment) => ( + { + if (permission === null) return; + setSaving(true); + void update({ environmentId: environment.environmentId, permission }).then( + (result) => { + setSaving(false); + if (result._tag === "Failure") + toastManager.add({ + type: "error", + title: "Could not save GitHub routing permission", + }); + }, + ); + }} + > + + + + + {options.map(({ value, label }) => ( + + {label} + + ))} + + + } + /> + ))} + + ); +} diff --git a/apps/web/src/components/settings/settingsSearch.ts b/apps/web/src/components/settings/settingsSearch.ts index 9a8af52b90f53b8944b700ef83dc3c6f506a105e..fefadbaaa9e8af602d56e172635513331941a58b 100644 --- a/apps/web/src/components/settings/settingsSearch.ts +++ b/apps/web/src/components/settings/settingsSearch.ts @@ -688,6 +688,12 @@ export const SETTINGS_SEARCH_ITEMS = [ "automatic machine environment resources cpu memory capacity preference weight shared projects", ], }, + { + id: "github-routing", + title: "GitHub routing", + to: "/settings/connections", + searchTerms: ["pull request trusted environments shared credentials permissions read actions"], + }, { id: "archive", title: "Archived threads", diff --git a/apps/web/src/connection/storage.test.ts b/apps/web/src/connection/storage.test.ts index 71b3815dcbae21c92e9b5fec80b515d9b4c51507..773d20c3b8825c50aad3b8c4b3116611d4a1617f 100644 --- a/apps/web/src/connection/storage.test.ts +++ b/apps/web/src/connection/storage.test.ts @@ -1,11 +1,22 @@ -import { ConnectionTransientError } from "@t3tools/client-runtime/connection"; +import { + ConnectionTransientError, + PrimaryConnectionTarget, +} from "@t3tools/client-runtime/connection"; +import { EnvironmentId } from "@t3tools/contracts"; import { ConnectionCatalogDocument } from "@t3tools/client-runtime/platform"; import { describe, expect, it } from "@effect/vitest"; import * as Effect from "effect/Effect"; +import * as Deferred from "effect/Deferred"; +import * as Option from "effect/Option"; import * as Schema from "effect/Schema"; +import * as Stream from "effect/Stream"; import { afterEach, vi } from "vite-plus/test"; -import { makeCatalogBackend, makeCatalogStore } from "./storage"; +import { + makeBrowserGitHubRoutingPermissions, + makeCatalogBackend, + makeCatalogStore, +} from "./storage"; const emptyCatalog = { schemaVersion: 1, @@ -16,6 +27,7 @@ const emptyCatalog = { disabledEnvironmentIds: [], } as const; const decodeCatalog = Schema.decodeUnknownSync(Schema.fromJsonString(ConnectionCatalogDocument)); +const encodeCatalog = Schema.encodeSync(Schema.fromJsonString(ConnectionCatalogDocument)); afterEach(() => { vi.unstubAllGlobals(); @@ -76,3 +88,90 @@ describe("makeCatalogBackend", () => { }), ); }); + +describe("browser GitHub routing permissions", () => { + it.effect("revokes across runtimes before storage events and resists stale catalog writes", () => + Effect.gen(function* () { + const values = new Map(); + const localStorage: Storage = { + get length() { + return values.size; + }, + key: (index) => [...values.keys()][index] ?? null, + getItem: (key) => values.get(key) ?? null, + setItem: (key, value) => { + values.set(key, value); + }, + removeItem: (key) => { + values.delete(key); + }, + clear: () => { + values.clear(); + }, + }; + const firstBrowser = Object.assign(new EventTarget(), { localStorage }); + const secondBrowser = Object.assign(new EventTarget(), { localStorage }); + const first = makeBrowserGitHubRoutingPermissions(firstBrowser); + const second = makeBrowserGitHubRoutingPermissions(secondBrowser); + const entry = { + target: new PrimaryConnectionTarget({ + environmentId: EnvironmentId.make("first"), + label: "First", + httpBaseUrl: "http://localhost:3000", + wsBaseUrl: "ws://localhost:3000", + }), + profile: Option.none(), + enabled: true, + }; + const other = { + ...entry, + target: new PrimaryConnectionTarget({ + ...entry.target, + environmentId: EnvironmentId.make("second"), + }), + }; + expect(yield* first.get(entry)).toBe("off"); + yield* first.set(entry, "read-write"); + expect(yield* second.get(entry)).toBe("read-write"); + const oldPermissions = Option.getOrThrow(yield* Stream.runHead(first.changes)); + const staleCatalog = yield* makeCatalogStore({ + read: Effect.succeed( + encodeCatalog({ ...emptyCatalog, githubRoutingPermissions: oldPermissions }), + ), + write: () => Effect.void, + }); + yield* staleCatalog.read; + const listening = yield* Deferred.make(); + const revoked = yield* Deferred.make(); + yield* second.changes.pipe( + Stream.runForEach((permissions) => + Deferred.succeed(permissions.length > 0 ? listening : revoked, undefined), + ), + Effect.forkChild, + ); + yield* Deferred.await(listening); + + yield* first.set(entry, "off"); + expect(yield* second.get(entry)).toBe("off"); + secondBrowser.dispatchEvent(Object.assign(new Event("storage"), { key: null })); + yield* Deferred.await(revoked); + yield* second.set(other, "read"); + yield* staleCatalog.update((document) => ({ ...document, accountId: "updated" })); + expect(yield* second.get(entry)).toBe("off"); + expect(yield* first.get(other)).toBe("read"); + expect(yield* makeBrowserGitHubRoutingPermissions(firstBrowser).get(entry)).toBe("off"); + + yield* first.set(entry, "read-write"); + yield* second.forget(entry.target.environmentId); + expect(yield* first.get(entry)).toBe("off"); + expect(yield* first.get(other)).toBe("read"); + vi.spyOn(localStorage, "setItem").mockImplementation(() => { + throw new Error("Storage unavailable"); + }); + expect(yield* first.set(entry, "read-write").pipe(Effect.flip)).toBeInstanceOf( + ConnectionTransientError, + ); + expect(yield* second.get(entry)).toBe("off"); + }).pipe(Effect.scoped), + ); +}); diff --git a/apps/web/src/connection/storage.ts b/apps/web/src/connection/storage.ts index a1653d50c02662f658d8fd89e5b08fe526b3ded4..2fbf39470264d1a7c0aad14e464526c9930a8cdd 100644 --- a/apps/web/src/connection/storage.ts +++ b/apps/web/src/connection/storage.ts @@ -16,8 +16,13 @@ import { import { TokenStore } from "@t3tools/client-runtime/authorization"; import { ConnectionTransientError, + ConnectionBlockedError, CredentialStore, ProfileStore, + GitHubRoutingPermissions, + StoredGitHubRoutingPermission, + gitHubRoutingConnectionKey, + gitHubRoutingPermissionFor, } from "@t3tools/client-runtime/connection"; import { EnvironmentId, @@ -32,8 +37,10 @@ import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; import * as Ref from "effect/Ref"; +import * as Queue from "effect/Queue"; import * as Schema from "effect/Schema"; import * as Semaphore from "effect/Semaphore"; +import * as Stream from "effect/Stream"; import { projectFaviconCache } from "../assets/projectFaviconCache"; const DATABASE_NAME = "t3code:connection-runtime"; @@ -368,12 +375,114 @@ export const makeCatalogStore = Effect.fn("web.connectionStorage.makeCatalogStor return { read, update } satisfies CatalogStore; }); +const GITHUB_ROUTING_KEY_PREFIX = "t3code:github-routing:"; +const GITHUB_ROUTING_CHANGED = "t3code:github-routing-changed"; +const isStoredGitHubRoutingPermission = Schema.is(StoredGitHubRoutingPermission); +const encodeStoredGitHubRoutingPermission = Schema.encodeSync( + Schema.fromJsonString(StoredGitHubRoutingPermission), +); + +/** Each grant has its own key so stale tabs and unrelated catalog saves cannot restore trust. */ +export function makeBrowserGitHubRoutingPermissions( + browser: Pick & EventTarget = window, +) { + const read = (key: string): StoredGitHubRoutingPermission | null => { + try { + const raw = browser.localStorage.getItem(key); + const value: unknown = raw === null ? null : JSON.parse(raw); + return isStoredGitHubRoutingPermission(value) && + key === `${GITHUB_ROUTING_KEY_PREFIX}${value.environmentId}` + ? value + : null; + } catch { + return null; + } + }; + const readAll = (): ReadonlyArray => { + try { + const values: StoredGitHubRoutingPermission[] = []; + const storage = browser.localStorage; + for (let index = 0; index < storage.length; index++) { + const key = storage.key(index); + if (key?.startsWith(GITHUB_ROUTING_KEY_PREFIX)) { + const value = read(key); + if (value !== null) values.push(value); + } + } + return values; + } catch { + return []; + } + }; + const write = (environmentId: EnvironmentId, value: StoredGitHubRoutingPermission | null) => + Effect.try({ + try: () => { + const key = `${GITHUB_ROUTING_KEY_PREFIX}${environmentId}`; + if (value === null) browser.localStorage.removeItem(key); + else browser.localStorage.setItem(key, encodeStoredGitHubRoutingPermission(value)); + browser.dispatchEvent(new Event(GITHUB_ROUTING_CHANGED)); + }, + catch: (cause) => catalogError("save GitHub routing permissions in", cause), + }); + return GitHubRoutingPermissions.of({ + get: (entry) => + Effect.sync(() => { + const value = read(`${GITHUB_ROUTING_KEY_PREFIX}${entry.target.environmentId}`); + return gitHubRoutingPermissionFor(entry, value === null ? [] : [value]); + }), + changes: Stream.callback>((queue) => + Effect.acquireRelease( + Effect.sync(() => { + const listener = (event: Event) => { + if (event.type === "storage") { + const key = (event as StorageEvent).key; + if (key !== null && !key?.startsWith(GITHUB_ROUTING_KEY_PREFIX)) return; + } + Queue.offerUnsafe(queue, readAll()); + }; + browser.addEventListener("storage", listener); + browser.addEventListener(GITHUB_ROUTING_CHANGED, listener); + Queue.offerUnsafe(queue, readAll()); + return listener; + }), + (listener) => + Effect.sync(() => { + browser.removeEventListener("storage", listener); + browser.removeEventListener(GITHUB_ROUTING_CHANGED, listener); + }), + ).pipe(Effect.asVoid), + ), + set: (entry, permission) => { + const connectionKey = gitHubRoutingConnectionKey(entry); + if (connectionKey === null) + return Effect.fail( + new ConnectionBlockedError({ + reason: "configuration", + detail: "This environment does not have a saved connection endpoint.", + }), + ); + return write( + entry.target.environmentId, + permission === "off" + ? null + : { + environmentId: entry.target.environmentId, + connectionKey, + permission, + }, + ); + }, + forget: (environmentId) => write(environmentId, null), + }); +} + export const connectionStorageLayer = Layer.effectContext( Effect.gen(function* () { const database = yield* Effect.acquireRelease(openDatabase(), (database) => Effect.sync(() => database.close()), ); const catalog = yield* makeCatalogStore(makeCatalogBackend(database)); + const githubRoutingPermissions = makeBrowserGitHubRoutingPermissions(); const targetStore = ConnectionTargetStore.of({ list: catalog.read.pipe( @@ -670,6 +779,7 @@ export const connectionStorageLayer = Layer.effectContext( }); return Context.make(ConnectionTargetStore, targetStore).pipe( + Context.add(GitHubRoutingPermissions, githubRoutingPermissions), Context.add(ConnectionRegistrationStore, registrationStore), Context.add(ProfileStore.ConnectionProfileStore, profileStore), Context.add(CredentialStore.ConnectionCredentialStore, credentialStore), diff --git a/docs/user/source-control.md b/docs/user/source-control.md index b6b4f89af1040ca73ea779ee16b03056b8318b54..d95444b4ae053b71e77159b163ee734f01a6543b 100644 --- a/docs/user/source-control.md +++ b/docs/user/source-control.md @@ -99,6 +99,22 @@ GitLab calls these merge requests. GitHub, GitLab, and Azure DevOps support auto-merge while checks are outstanding. GitHub also supports approving waiting fork workflows and opening a revert pull request for a merged change. +GitHub routing is off by default. In Settings → Connections (Environments on mobile), choose +**Read PRs** or **Read and act** for each environment you trust to share GitHub access. +Enable both the original environment and the environment answering its requests on this client. +**Read and act** can use broader GitHub permissions than the original environment's credential; +only enable it for environments you control and trust. Changing a saved endpoint or removing an +environment clears its permission. + +GitHub review details, linked PR status, and permitted review actions can then use another +connected environment signed in to the same GitHub account. Each needs a project on that host. +A connected local environment is preferred for actions and can answer slow or failed reads. +Browsers and mobile clients need a paired environment to use its GitHub CLI credentials. +Credentials stay on their machines. Previously verified credentials remain usable for routing +for ten minutes during a GitHub outage; new credentials must be verified first. An action with +an uncertain result is never automatically retried elsewhere. Listings, diffs, and checkout or +PR creation from Git actions continue to use the project's environment. + For Azure DevOps, use the host website to view diffs or change comments. Bitbucket does not support reopening a declined pull request. diff --git a/packages/client-runtime/src/connection/githubRoutingPermissions.ts b/packages/client-runtime/src/connection/githubRoutingPermissions.ts new file mode 100644 index 0000000000000000000000000000000000000000..fb9ddb1dbd46d0104b445fb08c735175d4181922 --- /dev/null +++ b/packages/client-runtime/src/connection/githubRoutingPermissions.ts @@ -0,0 +1,147 @@ +import { EnvironmentId } from "@t3tools/contracts"; +import * as Context from "effect/Context"; +import * as Effect from "effect/Effect"; +import * as Option from "effect/Option"; +import * as Schema from "effect/Schema"; +import * as Semaphore from "effect/Semaphore"; +import * as Stream from "effect/Stream"; +import * as SubscriptionRef from "effect/SubscriptionRef"; + +import type { ConnectionCatalogEntry } from "./catalog.ts"; +import { ConnectionBlockedError, type ConnectionAttemptError } from "./model.ts"; + +export const GitHubRoutingPermission = Schema.Literals(["off", "read", "read-write"]); +export type GitHubRoutingPermission = typeof GitHubRoutingPermission.Type; +export const StoredGitHubRoutingPermission = Schema.Struct({ + environmentId: EnvironmentId, + connectionKey: Schema.String, + permission: GitHubRoutingPermission, +}); +export type StoredGitHubRoutingPermission = typeof StoredGitHubRoutingPermission.Type; + +/** Trust belongs to the saved endpoint, never to an environment id advertised by a server alone. */ +export function gitHubRoutingConnectionKey(entry: ConnectionCatalogEntry): string | null { + const target = entry.target; + if (target._tag === "RelayConnectionTarget") + return JSON.stringify([target._tag, target.environmentId]); + const profile = Option.getOrNull(entry.profile); + if (target._tag === "SshConnectionTarget") { + if (profile?._tag !== "SshConnectionProfile") return null; + const { alias, hostname, username, port } = profile.target; + return JSON.stringify([target._tag, target.environmentId, alias, hostname, username, port]); + } + const baseUrls = + target._tag === "PrimaryConnectionTarget" + ? [target.httpBaseUrl, target.wsBaseUrl] + : profile?._tag === "BearerConnectionProfile" + ? [profile.httpBaseUrl, profile.wsBaseUrl] + : null; + if (baseUrls === null) return null; + try { + const urls = baseUrls.map((baseUrl) => new URL(baseUrl)); + if ( + !["http:", "https:"].includes(urls[0]!.protocol) || + !["ws:", "wss:"].includes(urls[1]!.protocol) || + urls.some((url) => url.username || url.password) + ) + return null; + return JSON.stringify([ + target._tag, + target.environmentId, + ...urls.map((url) => url.href.replace(/\/+$/, "")), + ]); + } catch { + return null; + } +} + +export function gitHubRoutingPermissionFor( + entry: ConnectionCatalogEntry, + permissions: ReadonlyArray, +): GitHubRoutingPermission { + const key = gitHubRoutingConnectionKey(entry); + return key === null + ? "off" + : (permissions.find((permission) => permission.connectionKey === key)?.permission ?? "off"); +} + +export class GitHubRoutingPermissions extends Context.Reference<{ + readonly get: (entry: ConnectionCatalogEntry) => Effect.Effect; + readonly changes: Stream.Stream>; + readonly set: ( + entry: ConnectionCatalogEntry, + permission: GitHubRoutingPermission, + ) => Effect.Effect; + readonly forget: (environmentId: EnvironmentId) => Effect.Effect; +}>("@t3tools/client-runtime/connection/GitHubRoutingPermissions", { + defaultValue: () => ({ + get: () => Effect.succeed("off"), + changes: Stream.succeed([]), + set: () => + Effect.fail( + new ConnectionBlockedError({ + reason: "unsupported", + detail: "GitHub routing preferences are unavailable on this client.", + }), + ), + forget: () => Effect.void, + }), +}) {} + +export const makeGitHubRoutingPermissions = Effect.fn("makeGitHubRoutingPermissions")( + function* (storage: { + readonly read: Effect.Effect< + ReadonlyArray, + ConnectionAttemptError + >; + readonly write: ( + permissions: ReadonlyArray, + ) => Effect.Effect; + }) { + const state = yield* SubscriptionRef.make(yield* storage.read); + const lock = yield* Semaphore.make(1); + const update = ( + transform: ( + current: ReadonlyArray, + ) => ReadonlyArray, + ) => + lock.withPermits(1)( + Effect.gen(function* () { + const current = yield* SubscriptionRef.get(state); + const next = transform(current); + if (next === current) return; + yield* storage.write(next); + yield* SubscriptionRef.set(state, next); + }), + ); + return GitHubRoutingPermissions.of({ + get: (entry) => + SubscriptionRef.get(state).pipe( + Effect.map((permissions) => gitHubRoutingPermissionFor(entry, permissions)), + ), + changes: SubscriptionRef.changes(state), + set: (entry, permission) => { + const connectionKey = gitHubRoutingConnectionKey(entry); + if (connectionKey === null) + return Effect.fail( + new ConnectionBlockedError({ + reason: "configuration", + detail: "This environment does not have a saved connection endpoint.", + }), + ); + return update((current) => [ + ...current.filter((value) => value.environmentId !== entry.target.environmentId), + ...(permission === "off" + ? [] + : [{ environmentId: entry.target.environmentId, connectionKey, permission }]), + ]); + }, + forget: (environmentId) => + update((current) => + current.some((permission) => permission.environmentId === environmentId) + ? current.filter((permission) => permission.environmentId !== environmentId) + : current, + ), + }); + }, +); diff --git a/packages/client-runtime/src/connection/index.ts b/packages/client-runtime/src/connection/index.ts index 127a6288fa1b938837a51fdd5c03080431eca290..5367c7f6b820a36a89481db943f0d800228473b3 100644 --- a/packages/client-runtime/src/connection/index.ts +++ b/packages/client-runtime/src/connection/index.ts @@ -3,6 +3,7 @@ export * as Connectivity from "./connectivity.ts"; export * as CredentialStore from "./credentialStore.ts"; export { type ConnectionDriverProgress, type EnvironmentConnectionLease } from "./driver.ts"; export * from "./errors.ts"; +export * from "./githubRoutingPermissions.ts"; export * as Connection from "./layer.ts"; export * from "./model.ts"; export { diff --git a/packages/client-runtime/src/connection/registry.test.ts b/packages/client-runtime/src/connection/registry.test.ts index ba0d9e94ac657b7d5100408e9bb21a741c61e821..5773acf5ce1f8aa7b62bb5bd7e00b1dfbd244dfd 100644 --- a/packages/client-runtime/src/connection/registry.test.ts +++ b/packages/client-runtime/src/connection/registry.test.ts @@ -47,6 +47,10 @@ import { import * as Persistence from "../platform/persistence.ts"; import * as ConnectionProfileStore from "./profileStore.ts"; import * as EnvironmentRegistry from "./registry.ts"; +import { + GitHubRoutingPermissions, + makeGitHubRoutingPermissions, +} from "./githubRoutingPermissions.ts"; import * as RpcSession from "../rpc/session.ts"; import * as EnvironmentSupervisor from "./supervisor.ts"; import * as ConnectionWakeups from "./wakeups.ts"; @@ -980,10 +984,17 @@ describe("EnvironmentRegistry", () => { label: "Shadowed relay environment", }); const harness = yield* makeHarness([shadowedTarget]); + const permissions = yield* makeGitHubRoutingPermissions({ + read: Effect.succeed([]), + write: () => Effect.void, + }); + const shadowedEntry = { target: shadowedTarget, profile: Option.none(), enabled: true }; + yield* permissions.set(shadowedEntry, "read-write"); yield* Effect.gen(function* () { const registry = yield* EnvironmentRegistry.EnvironmentRegistry; yield* registry.registerPlatform(new PrimaryConnectionRegistration({ target: TARGET })); + expect(yield* permissions.get(shadowedEntry)).toBe("off"); expect( (yield* SubscriptionRef.get(registry.entries)).get(TARGET.environmentId)?.target, @@ -996,7 +1007,11 @@ describe("EnvironmentRegistry", () => { (yield* SubscriptionRef.get(registry.entries)).get(TARGET.environmentId)?.target, ).toEqual(TARGET); expect((yield* Ref.get(harness.storedTargets)).has(TARGET.environmentId)).toBe(false); - }).pipe(Effect.provide(harness.layer), Effect.scoped); + }).pipe( + Effect.provide(harness.layer), + Effect.provideService(GitHubRoutingPermissions, permissions), + Effect.scoped, + ); }), ); @@ -1083,6 +1098,20 @@ describe("EnvironmentRegistry", () => { it.effect("retains a healthy runtime when the platform repeats an identical registration", () => Effect.gen(function* () { const harness = yield* makeHarness([]); + let failWrites = false; + const permissions = yield* makeGitHubRoutingPermissions({ + read: Effect.succeed([]), + write: () => + failWrites + ? Effect.fail( + new ConnectionTransientError({ + reason: "remote-unavailable", + detail: "Storage unavailable", + }), + ) + : Effect.void, + }); + const entry = { target: TARGET, profile: Option.none(), enabled: true }; yield* Effect.gen(function* () { const registry = yield* EnvironmentRegistry.EnvironmentRegistry; @@ -1093,11 +1122,43 @@ describe("EnvironmentRegistry", () => { TARGET.environmentId, (state) => state.phase === "connected", ); + yield* permissions.set(entry, "read-write"); yield* registry.registerPlatform(registration); expect(yield* Ref.get(harness.sessions)).toHaveLength(1); - }).pipe(Effect.provide(harness.layer), Effect.scoped); + expect(yield* permissions.get(entry)).toBe("read-write"); + + const changedTarget = new PrimaryConnectionTarget({ + ...TARGET, + httpBaseUrl: "https://changed.example.test", + }); + failWrites = true; + yield* registry.registerPlatform( + new PrimaryConnectionRegistration({ target: changedTarget }), + ); + expect( + (yield* SubscriptionRef.get(registry.entries)).get(TARGET.environmentId)?.target, + ).toEqual(TARGET); + yield* registry.reconcilePlatform([]); + expect((yield* SubscriptionRef.get(registry.entries)).has(TARGET.environmentId)).toBe(true); + + failWrites = false; + yield* registry.registerPlatform( + new PrimaryConnectionRegistration({ target: changedTarget }), + ); + expect(yield* permissions.get(entry)).toBe("off"); + yield* registry.registerPlatform(registration); + expect(yield* permissions.get(entry)).toBe("off"); + yield* permissions.set(entry, "read-write"); + yield* registry.reconcilePlatform([]); + yield* registry.registerPlatform(registration); + expect(yield* permissions.get(entry)).toBe("off"); + }).pipe( + Effect.provide(harness.layer), + Effect.provideService(GitHubRoutingPermissions, permissions), + Effect.scoped, + ); }), ); diff --git a/packages/client-runtime/src/connection/registry.ts b/packages/client-runtime/src/connection/registry.ts index dda8ee429eb43a86571fbcea613e4b63067899b6..0949916356a05fd7258d18972382955d1df2c36f 100644 --- a/packages/client-runtime/src/connection/registry.ts +++ b/packages/client-runtime/src/connection/registry.ts @@ -34,6 +34,10 @@ import * as Persistence from "../platform/persistence.ts"; import * as EnvironmentSupervisor from "./supervisor.ts"; import * as ConnectionDriver from "./driver.ts"; import * as ConnectionWakeups from "./wakeups.ts"; +import { + GitHubRoutingPermissions, + gitHubRoutingConnectionKey, +} from "./githubRoutingPermissions.ts"; const isSshConnectionProfile = Schema.is(SshConnectionProfile); @@ -146,6 +150,7 @@ export const make = Effect.gen(function* () { const ownedDataCleanup = yield* Persistence.EnvironmentOwnedDataCleanup; const profiles = yield* ConnectionProfileStore.ConnectionProfileStore; const credentials = yield* ConnectionCredentialStore.ConnectionCredentialStore; + const githubRoutingPermissions = yield* GitHubRoutingPermissions; const connectivity = yield* Connectivity.Connectivity; const driver = yield* ConnectionDriver.ConnectionDriver; const wakeups = yield* ConnectionWakeups.ConnectionWakeups; @@ -418,11 +423,24 @@ export const make = Effect.gen(function* () { if ((yield* Ref.get(platformEnvironmentIds)).has(environmentId)) { return; } - // Editing a saved environment re-registers it; that must not switch a - // disabled one back on. + // Editing a saved environment must preserve its disabled state. const previous = (yield* SubscriptionRef.get(entries)).get(environmentId); const entry: ConnectionCatalogEntry = previous === undefined ? registered : { ...registered, enabled: previous.enabled }; + if ( + previous !== undefined && + gitHubRoutingConnectionKey(previous) !== gitHubRoutingConnectionKey(entry) + ) { + yield* githubRoutingPermissions.forget(environmentId).pipe( + Effect.mapError( + (error) => + new Persistence.ConnectionPersistenceError({ + operation: "register-connection", + message: error.message, + }), + ), + ); + } yield* registrations.register(registration); yield* Ref.update(persistedTargetsByEnvironment, (current) => { const next = new Map(current); @@ -441,6 +459,29 @@ export const make = Effect.gen(function* () { yield* withLeaseLock( target.environmentId, Effect.gen(function* () { + const previous = (yield* SubscriptionRef.get(entries)).get(target.environmentId); + const persistedTarget = (yield* Ref.get(persistedTargetsByEnvironment)).get( + target.environmentId, + ); + if ( + persistedTarget !== undefined || + (previous !== undefined && + gitHubRoutingConnectionKey(previous) !== gitHubRoutingConnectionKey(entry)) + ) { + const revoked = yield* githubRoutingPermissions.forget(target.environmentId).pipe( + Effect.tapError((error) => + Effect.logWarning( + "Could not clear GitHub routing permission for a platform environment.", + { + environmentId: target.environmentId, + error, + }, + ), + ), + Effect.exit, + ); + if (Exit.isFailure(revoked)) return; + } yield* Ref.update(platformEnvironmentIds, (current) => { const next = new Set(current); next.add(target.environmentId); @@ -462,9 +503,6 @@ export const make = Effect.gen(function* () { ); } - const persistedTarget = (yield* Ref.get(persistedTargetsByEnvironment)).get( - target.environmentId, - ); if (persistedTarget !== undefined) { yield* registrations.remove(persistedTarget).pipe( Effect.tap(() => @@ -502,6 +540,19 @@ export const make = Effect.gen(function* () { environmentId, Effect.gen(function* () { const entry = (yield* SubscriptionRef.get(entries)).get(environmentId); + const revoked = yield* githubRoutingPermissions.forget(environmentId).pipe( + Effect.tapError((error) => + Effect.logWarning( + "Could not clear GitHub routing permission after platform removal.", + { + environmentId, + error, + }, + ), + ), + Effect.exit, + ); + if (Exit.isFailure(revoked)) return; yield* Ref.update(platformEnvironmentIds, (current) => { const next = new Set(current); next.delete(environmentId); @@ -582,6 +633,7 @@ export const make = Effect.gen(function* () { ? yield* profiles.get(target.connectionId) : Option.none(); + yield* githubRoutingPermissions.forget(environmentId); yield* registrations.remove(target); yield* Ref.update(persistedTargetsByEnvironment, (current) => { const next = new Map(current); diff --git a/packages/client-runtime/src/connection/resolver.test.ts b/packages/client-runtime/src/connection/resolver.test.ts index 35d9d9c54e0d387ae909b542898ef1a68fa7ac41..ecc5d7153914b68887883f7ba1df5f9ed4887f65 100644 --- a/packages/client-runtime/src/connection/resolver.test.ts +++ b/packages/client-runtime/src/connection/resolver.test.ts @@ -28,6 +28,11 @@ import { type ConnectionTarget, } from "./model.ts"; import * as ConnectionProfileStore from "./profileStore.ts"; +import { + GitHubRoutingPermissions, + gitHubRoutingConnectionKey, + makeGitHubRoutingPermissions, +} from "./githubRoutingPermissions.ts"; const ENVIRONMENT_ID = EnvironmentId.make("environment-1"); const ENDPOINT = { @@ -64,6 +69,7 @@ function collectingTracer(spans: Array): Tracer.Tracer { const makeDependencies = Effect.fn("TestConnectionResolver.makeDependencies")((options?: { readonly profiles?: ReadonlyArray; + readonly profileStore?: ConnectionProfileStore.ConnectionProfileStore["Service"]; readonly credentials?: ReadonlyArray; readonly authorizeBearer?: RemoteEnvironmentAuthorization.RemoteEnvironmentAuthorization["Service"]["authorizeBearer"]; readonly authorizeDpop?: RemoteEnvironmentAuthorization.RemoteEnvironmentAuthorization["Service"]["authorizeDpop"]; @@ -134,7 +140,10 @@ const makeDependencies = Effect.fn("TestConnectionResolver.makeDependencies")((o }); const dependencies = Layer.mergeAll( - Layer.succeed(ConnectionProfileStore.ConnectionProfileStore, profileStore), + Layer.succeed( + ConnectionProfileStore.ConnectionProfileStore, + options?.profileStore ?? profileStore, + ), Layer.succeed(ConnectionCredentialStore.ConnectionCredentialStore, credentialStore), Layer.succeed( ClientCapabilities.PrimaryEnvironmentAuth, @@ -376,6 +385,102 @@ describe("ConnectionResolver", () => { }), ); + for (const scenario of ["unchanged", "changed", "revocation-failed"] as const) { + it.effect(`handles ${scenario} SSH routing consent before saving or authorizing`, () => + Effect.gen(function* () { + const calls: string[] = []; + const target = new SshConnectionTarget({ + environmentId: ENVIRONMENT_ID, + label: "SSH", + connectionId: "ssh-1", + }); + const profile = new SshConnectionProfile({ + connectionId: target.connectionId, + environmentId: ENVIRONMENT_ID, + label: "SSH", + target: SSH_TARGET, + }); + const entry = catalogEntry(target, Option.some(profile)); + const preparedTarget = + scenario === "unchanged" + ? SSH_TARGET + : { ...SSH_TARGET, hostname: "replacement.example.test" }; + const failure = new ConnectionTransientError({ + reason: "remote-unavailable", + detail: "Could not persist routing consent.", + }); + const permissions = yield* makeGitHubRoutingPermissions({ + read: Effect.succeed([ + { + environmentId: ENVIRONMENT_ID, + connectionKey: gitHubRoutingConnectionKey(entry)!, + permission: "read-write", + }, + ]), + write: () => + Effect.sync(() => { + calls.push("revoke"); + }).pipe( + Effect.andThen(scenario === "revocation-failed" ? Effect.fail(failure) : Effect.void), + ), + }); + let savedProfile: ConnectionProfile = profile; + const brokerLayer = yield* makeDependencies({ + profileStore: { + get: () => Effect.sync(() => Option.some(savedProfile)), + put: (value) => + Effect.sync(() => { + calls.push("profile"); + savedProfile = value; + }), + remove: () => Effect.die("unused"), + }, + prepareSsh: () => + Effect.succeed({ + bootstrap: { + target: preparedTarget, + httpBaseUrl: "http://127.0.0.1:4010", + wsBaseUrl: "ws://127.0.0.1:4010", + pairingToken: null, + }, + bearerToken: "ssh-bearer", + }), + authorizeBearer: (input) => + Effect.sync(() => { + calls.push("authorize"); + return { + environmentId: input.expectedEnvironmentId, + label: "SSH", + httpBaseUrl: input.httpBaseUrl, + socketUrl: "ws://127.0.0.1:4010/ws?wsTicket=ssh", + httpAuthorization: { _tag: "Bearer" as const, token: input.bearerToken }, + }; + }), + }); + const broker = yield* ConnectionResolver.ConnectionResolver.pipe( + Effect.provide(brokerLayer), + ); + const prepare = broker + .prepare(entry) + .pipe(Effect.provideService(GitHubRoutingPermissions, permissions)); + if (scenario === "revocation-failed") { + expect(yield* Effect.flip(prepare)).toBe(failure); + expect(savedProfile).toBe(profile); + expect(calls).toEqual(["revoke"]); + } else { + expect((yield* prepare).socketUrl).toContain("wsTicket=ssh"); + expect(savedProfile).toMatchObject({ target: preparedTarget }); + expect(calls).toEqual( + scenario === "unchanged" + ? ["profile", "authorize"] + : ["revoke", "profile", "authorize"], + ); + } + expect(yield* permissions.get(entry)).toBe(scenario === "changed" ? "off" : "read-write"); + }), + ); + } + it.effect("preserves relay authorization failure classification and trace details", () => Effect.gen(function* () { const target = new RelayConnectionTarget({ diff --git a/packages/client-runtime/src/connection/resolver.ts b/packages/client-runtime/src/connection/resolver.ts index 885529249a626edd33588a2ac70b4fb0502c706e..f51c6ac607ba055df6ed9e9d088037d466e46f56 100644 --- a/packages/client-runtime/src/connection/resolver.ts +++ b/packages/client-runtime/src/connection/resolver.ts @@ -17,6 +17,10 @@ import { } from "./catalog.ts"; import * as ConnectionCredentialStore from "./credentialStore.ts"; import { credentialMissingError, environmentMismatchError, profileMissingError } from "./errors.ts"; +import { + GitHubRoutingPermissions, + gitHubRoutingConnectionKey, +} from "./githubRoutingPermissions.ts"; import type { BearerConnectionTarget, ConnectionTarget, @@ -192,14 +196,20 @@ const makeSshBroker = Effect.fn("clientRuntime.connection.broker.makeSsh")(funct expectedEnvironmentId: target.environmentId, target: profile.target, }); - yield* profiles.put( - new SshConnectionProfile({ - connectionId: profile.connectionId, - environmentId: profile.environmentId, - label: profile.label, - target: prepared.bootstrap.target, - }), - ); + const preparedProfile = new SshConnectionProfile({ + connectionId: profile.connectionId, + environmentId: profile.environmentId, + label: profile.label, + target: prepared.bootstrap.target, + }); + if ( + gitHubRoutingConnectionKey(entry) !== + gitHubRoutingConnectionKey({ ...entry, profile: Option.some(preparedProfile) }) + ) { + const permissions = yield* GitHubRoutingPermissions; + yield* permissions.forget(target.environmentId); + } + yield* profiles.put(preparedProfile); const authorized = yield* remote.authorizeBearer({ expectedEnvironmentId: target.environmentId, httpBaseUrl: prepared.bootstrap.httpBaseUrl, diff --git a/packages/client-runtime/src/platform/storageDocument.test.ts b/packages/client-runtime/src/platform/storageDocument.test.ts index ae0f0426866633cae251e3b848748dda535034fc..a0c7de3d093bd767f9a5334df0cac73b48348bfc 100644 --- a/packages/client-runtime/src/platform/storageDocument.test.ts +++ b/packages/client-runtime/src/platform/storageDocument.test.ts @@ -1,6 +1,8 @@ import { EnvironmentId } from "@t3tools/contracts"; import { describe, expect, it } from "@effect/vitest"; import * as Schema from "effect/Schema"; +import * as Effect from "effect/Effect"; +import * as Option from "effect/Option"; import * as TokenStore from "../authorization/tokenStore.ts"; import { @@ -13,9 +15,15 @@ import { } from "../connection/catalog.ts"; import { BearerConnectionTarget, + ConnectionTransientError, + PrimaryConnectionTarget, RelayConnectionTarget, SshConnectionTarget, } from "../connection/model.ts"; +import { + GitHubRoutingPermissions, + makeGitHubRoutingPermissions, +} from "../connection/githubRoutingPermissions.ts"; import { ConnectionCatalogDocument, EMPTY_CONNECTION_CATALOG_DOCUMENT, @@ -25,6 +33,8 @@ import { setConnectionEnabledInCatalog, } from "./storageDocument.ts"; +const decodeConnectionCatalogDocument = Schema.decodeUnknownEffect(ConnectionCatalogDocument); + const ENVIRONMENT_ID = EnvironmentId.make("environment-1"); const decodeCatalogDocument = Schema.decodeUnknownSync(ConnectionCatalogDocument); @@ -61,6 +71,128 @@ const REMOTE_TOKEN = new TokenStore.RemoteDpopAccessToken({ }); describe("ConnectionCatalogDocument", () => { + it.effect("persists explicit GitHub trust and forgets it when a connection is removed", () => + Effect.gen(function* () { + let document = EMPTY_CONNECTION_CATALOG_DOCUMENT; + const entry = { target: BEARER_TARGET, profile: Option.some(BEARER_PROFILE), enabled: true }; + const storage = { + read: Effect.sync(() => document.githubRoutingPermissions ?? []), + write: (githubRoutingPermissions: NonNullable) => + Effect.gen(function* () { + document = yield* decodeConnectionCatalogDocument({ + ...document, + githubRoutingPermissions, + }).pipe(Effect.orDie); + }), + }; + const permissions = yield* makeGitHubRoutingPermissions(storage); + expect(yield* permissions.get(entry)).toBe("off"); + yield* permissions.set(entry, "read"); + const restarted = yield* makeGitHubRoutingPermissions(storage); + expect(yield* restarted.get(entry)).toBe("read"); + + const changedEndpoint = { + ...entry, + profile: Option.some( + new BearerConnectionProfile({ + ...BEARER_PROFILE, + httpBaseUrl: "https://different.example.test", + }), + ), + }; + expect(yield* restarted.get(changedEndpoint)).toBe("off"); + expect( + yield* restarted.get({ + ...entry, + profile: Option.some( + new BearerConnectionProfile({ + ...BEARER_PROFILE, + wsBaseUrl: "wss://different.example.test", + }), + ), + }), + ).toBe("off"); + expect( + yield* restarted.get({ target: RELAY_TARGET, profile: Option.none(), enabled: true }), + ).toBe("off"); + yield* restarted.set(entry, "read-write"); + expect(yield* restarted.get(entry)).toBe("read-write"); + yield* restarted.forget(ENVIRONMENT_ID); + expect(yield* restarted.get(entry)).toBe("off"); + const afterRemoval = yield* makeGitHubRoutingPermissions(storage); + expect(yield* afterRemoval.get(entry)).toBe("off"); + + yield* permissions.set(entry, "read"); + document = removeConnectionFromCatalog(document, BEARER_TARGET); + const afterCatalogRemoval = yield* makeGitHubRoutingPermissions(storage); + expect(yield* afterCatalogRemoval.get(entry)).toBe("off"); + }), + ); + + it.effect("requires explicit trust for primary, relay, and SSH connections independently", () => + Effect.gen(function* () { + const permissions = yield* makeGitHubRoutingPermissions({ + read: Effect.succeed([]), + write: () => Effect.void, + }); + const entries = [ + { + target: new PrimaryConnectionTarget({ + environmentId: ENVIRONMENT_ID, + label: "Local", + httpBaseUrl: "http://localhost:3000", + wsBaseUrl: "ws://localhost:3000", + }), + profile: Option.none(), + enabled: true, + }, + { target: RELAY_TARGET, profile: Option.none(), enabled: true }, + { + enabled: true, + target: new SshConnectionTarget({ + environmentId: ENVIRONMENT_ID, + label: "SSH", + connectionId: "ssh-1", + }), + profile: Option.some( + new SshConnectionProfile({ + environmentId: ENVIRONMENT_ID, + label: "SSH", + connectionId: "ssh-1", + target: { alias: "work", hostname: "work.example.test", username: "maria", port: 22 }, + }), + ), + }, + ]; + for (const entry of entries) { + expect(yield* permissions.get(entry)).toBe("off"); + yield* permissions.set(entry, "read"); + expect(yield* permissions.get(entry)).toBe("read"); + yield* permissions.set(entry, "off"); + expect(yield* permissions.get(entry)).toBe("off"); + } + }), + ); + + it.effect("does not enable GitHub routing when permission persistence fails", () => + Effect.gen(function* () { + const entry = { target: BEARER_TARGET, profile: Option.some(BEARER_PROFILE), enabled: true }; + expect(yield* (yield* GitHubRoutingPermissions).get(entry)).toBe("off"); + const permissions = yield* makeGitHubRoutingPermissions({ + read: Effect.succeed([]), + write: () => + Effect.fail( + new ConnectionTransientError({ + reason: "remote-unavailable", + detail: "storage unavailable", + }), + ), + }); + yield* permissions.set(entry, "read-write").pipe(Effect.flip); + expect(yield* permissions.get(entry)).toBe("off"); + }), + ); + it.each([ { name: "legacy", accountId: undefined }, { name: "account-bound", accountId: "account-1" }, diff --git a/packages/client-runtime/src/platform/storageDocument.ts b/packages/client-runtime/src/platform/storageDocument.ts index 1ef448b3403eb4ebe5a32e6b7d7cbfb5ae159089..2c163f4bfd64a134dd1f7782c230fecd059145cd 100644 --- a/packages/client-runtime/src/platform/storageDocument.ts +++ b/packages/client-runtime/src/platform/storageDocument.ts @@ -9,6 +9,7 @@ import { } from "../connection/catalog.ts"; import { type ConnectionTarget, PersistedConnectionTarget } from "../connection/model.ts"; import * as TokenStore from "../authorization/tokenStore.ts"; +import { StoredGitHubRoutingPermission } from "../connection/githubRoutingPermissions.ts"; export const StoredConnectionCredential = Schema.Struct({ connectionId: Schema.String, @@ -22,6 +23,7 @@ export const ConnectionCatalogDocument = Schema.Struct({ profiles: Schema.Array(ConnectionProfile), credentials: Schema.Array(StoredConnectionCredential), remoteDpopTokens: Schema.Array(TokenStore.RemoteDpopAccessToken), + githubRoutingPermissions: Schema.optionalKey(Schema.Array(StoredGitHubRoutingPermission)), // Saved environments the user switched off. They stay registered with their // credentials and cache but never connect until switched back on. Older // documents predate the key, so decoding defaults it to none. @@ -153,7 +155,15 @@ export function removeConnectionFromCatalog( document: ConnectionCatalogDocument, target: ConnectionTarget, ): ConnectionCatalogDocument { - return removeConnectionMetadata(document, target, true); + const next = removeConnectionMetadata(document, target, true); + return document.githubRoutingPermissions === undefined + ? next + : { + ...next, + githubRoutingPermissions: document.githubRoutingPermissions.filter( + (permission) => permission.environmentId !== target.environmentId, + ), + }; } /** Flips the disabled flag for a saved environment; unknown ids are ignored. */ diff --git a/packages/client-runtime/src/state/connections.ts b/packages/client-runtime/src/state/connections.ts index c4f1b1caa600fcc4713d5d2d6df3e0d36dd1e3bd..1a47815120c2186028da36e615cdf3c74339dd64 100644 --- a/packages/client-runtime/src/state/connections.ts +++ b/packages/client-runtime/src/state/connections.ts @@ -9,6 +9,11 @@ import * as EnvironmentRegistry from "../connection/registry.ts"; import type { ConnectionCatalogEntry } from "../connection/catalog.ts"; import { AVAILABLE_CONNECTION_STATE } from "../connection/model.ts"; import * as EnvironmentSupervisor from "../connection/supervisor.ts"; +import { + GitHubRoutingPermissions, + type GitHubRoutingPermission, + type StoredGitHubRoutingPermission, +} from "../connection/githubRoutingPermissions.ts"; import { createAtomCommandScheduler, createRuntimeCommand, @@ -65,6 +70,32 @@ export function createEnvironmentCatalogAtoms( Option.getOrElse(AsyncResult.value(get(catalogAtom)), () => EMPTY_ENVIRONMENT_CATALOG_STATE), ).pipe(Atom.withLabel("environment-catalog-value")); + const githubRoutingPermissionsAtom = runtime.atom( + Stream.unwrap(GitHubRoutingPermissions.pipe(Effect.map((permissions) => permissions.changes))), + { initialValue: [] as ReadonlyArray }, + ); + const githubRoutingPermissionsValueAtom = Atom.make((get) => + Option.getOrElse(AsyncResult.value(get(githubRoutingPermissionsAtom)), () => []), + ).pipe(Atom.withLabel("environment-github-routing-permissions")); + const setGitHubRoutingPermission = createRuntimeCommand(runtime, { + label: "environment-catalog:github-routing-permission", + scheduler: commandScheduler, + concurrency: serial, + execute: Effect.fn(function* (input: { + readonly environmentId: EnvironmentIdType; + readonly permission: GitHubRoutingPermission; + }) { + const registry = yield* EnvironmentRegistry.EnvironmentRegistry; + const entry = (yield* SubscriptionRef.get(registry.entries)).get(input.environmentId); + if (entry === undefined) + return yield* new EnvironmentRegistry.EnvironmentNotRegisteredError({ + environmentId: input.environmentId, + }); + const permissions = yield* GitHubRoutingPermissions; + yield* permissions.set(entry, input.permission); + }), + }); + const networkStatusAtom = runtime.atom( Stream.unwrap( EnvironmentRegistry.EnvironmentRegistry.pipe( @@ -143,6 +174,8 @@ export function createEnvironmentCatalogAtoms( return { catalogAtom, catalogValueAtom, + githubRoutingPermissionsValueAtom, + setGitHubRoutingPermission, networkStatusAtom, networkStatusValueAtom, stateAtom, diff --git a/packages/client-runtime/src/state/pullRequestRouting.ts b/packages/client-runtime/src/state/pullRequestRouting.ts new file mode 100644 index 0000000000000000000000000000000000000000..d058f6a863a6cedb2ac8d75e28a54eb173a420fe --- /dev/null +++ b/packages/client-runtime/src/state/pullRequestRouting.ts @@ -0,0 +1,384 @@ +import { + WS_METHODS, + PullRequestRef, + PullRequestInvalidateInput, + type EnvironmentId, +} from "@t3tools/contracts"; +import * as Effect from "effect/Effect"; +import * as Cause from "effect/Cause"; +import * as Option from "effect/Option"; +import * as Schema from "effect/Schema"; +import * as SubscriptionRef from "effect/SubscriptionRef"; + +import { EnvironmentRegistry, EnvironmentNotRegisteredError } from "../connection/registry.ts"; +import { EnvironmentSupervisor } from "../connection/supervisor.ts"; +import { + GitHubRoutingPermissions, + gitHubRoutingConnectionKey, +} from "../connection/githubRoutingPermissions.ts"; +import type { ConnectionCatalogEntry } from "../connection/catalog.ts"; +import { ConnectionProfileStore } from "../connection/profileStore.ts"; +import { + request, + EnvironmentRpcUnavailableError, + type EnvironmentRpcInput, + type EnvironmentUnaryRpcTag, +} from "../rpc/client.ts"; + +const reads = new Set([ + WS_METHODS.pullRequestsSummary, + WS_METHODS.pullRequestsStack, + WS_METHODS.pullRequestsDetail, + WS_METHODS.pullRequestsActivity, + WS_METHODS.pullRequestsThreadComments, + WS_METHODS.pullRequestsDiffFileContents, + WS_METHODS.pullRequestsReviewerCandidates, + WS_METHODS.pullRequestsLabelCandidates, +]); +const writes = new Set([ + WS_METHODS.pullRequestsRunAction, + WS_METHODS.pullRequestsUpdate, + WS_METHODS.pullRequestsComment, + WS_METHODS.pullRequestsUpdateComment, + WS_METHODS.pullRequestsSubmitReview, + WS_METHODS.pullRequestsReplyToThread, + WS_METHODS.pullRequestsSetReaction, + WS_METHODS.pullRequestsSetThreadResolution, + WS_METHODS.pullRequestsRequestReviewers, + WS_METHODS.pullRequestsSetLabels, +]); +const isRef = Schema.is(PullRequestRef); +const isInvalidation = Schema.is(PullRequestInvalidateInput); +interface RoutedRead { + origin: EnvironmentId; + reference: PullRequestRef; + targets: Set; +} +const routedReads = new WeakMap>(); +const isUnregistered = Schema.is(EnvironmentNotRegisteredError); +const encodeKey = Schema.encodeSync( + Schema.fromJsonString(Schema.Array(Schema.NullOr(Schema.String))), +); + +function isLocal(entry: ConnectionCatalogEntry): boolean { + const url = + entry.target._tag === "PrimaryConnectionTarget" + ? entry.target.httpBaseUrl + : Option.isSome(entry.profile) && entry.profile.value._tag === "BearerConnectionProfile" + ? entry.profile.value.httpBaseUrl + : undefined; + if (url === undefined) return false; + try { + return ["localhost", "127.0.0.1", "[::1]"].includes(new URL(url).hostname); + } catch { + return false; + } +} + +/** A guard rejection is returned before the operation starts. Other write failures are ambiguous. */ +function rejectedBeforeDispatch(error: unknown): boolean { + return ( + typeof error === "object" && + error !== null && + "_tag" in error && + (error._tag === "EnvironmentRpcUnavailableError" || + error._tag === "EnvironmentNotRegisteredError" || + (error._tag === "PullRequestOperationError" && + "operation" in error && + error.operation === "routeIdentity")) + ); +} + +const routingAllowed = Effect.fn("PullRequestRouting.allowed")(function* ( + registry: EnvironmentRegistry["Service"], + originId: EnvironmentId, + destinationId: EnvironmentId, + write: boolean, +) { + const entries = yield* SubscriptionRef.get(registry.entries); + const origin = entries.get(originId); + const destination = entries.get(destinationId); + if (!origin?.enabled || !destination?.enabled) return false; + const permissions = yield* GitHubRoutingPermissions; + const source = yield* permissions.get(origin); + const target = yield* permissions.get(destination); + const allowed = write + ? source === "read-write" && target === "read-write" + : source !== "off" && target !== "off"; + if (!allowed) return false; + for (const entry of [origin, destination]) { + if (entry.target._tag !== "SshConnectionTarget") continue; + const profiles = yield* Effect.serviceOption(ConnectionProfileStore); + if (Option.isNone(profiles)) return false; + const profile = yield* profiles.value + .get(entry.target.connectionId) + .pipe(Effect.orElseSucceed(() => Option.none())); + const key = gitHubRoutingConnectionKey(entry); + if (key === null || key !== gitHubRoutingConnectionKey({ ...entry, profile })) return false; + } + return true; +}); + +function matchesReference(reference: PullRequestRef, filter: PullRequestRef): boolean { + return ( + reference.projectId === filter.projectId && + reference.repository.toLowerCase() === filter.repository.toLowerCase() && + reference.number === filter.number && + (filter.host === undefined || reference.host?.toLowerCase() === filter.host.toLowerCase()) + ); +} + +const invalidateTarget = Effect.fn("PullRequestRouting.invalidateTarget")(function* ( + registry: EnvironmentRegistry["Service"], + origin: EnvironmentId, + target: EnvironmentId, + inputs: ReadonlyArray, +) { + if (target !== origin && !(yield* routingAllowed(registry, origin, target, false))) return; + yield* Effect.forEach( + inputs, + (input) => + registry.run(target, request(WS_METHODS.pullRequestsInvalidate, input)).pipe( + // GitHub already accepted the write. A stalled reader must not delay its confirmation. + Effect.timeoutOption("1 second"), + Effect.orElseSucceed(() => undefined), + ), + { concurrency: 3, discard: true }, + ); +}); + +/** Credentials stay on their environments. Only a verified host and account cross the wire. */ +export function createPullRequestRouter() { + const routedRequest = Effect.fn("PullRequestRouting.request")(function* < + T extends EnvironmentUnaryRpcTag, + >(tag: T, input: EnvironmentRpcInput, sourceRead?: ReturnType>) { + const source = sourceRead ?? request(tag, input); + if (tag === WS_METHODS.pullRequestsInvalidate && isInvalidation(input)) { + const result = yield* request(tag, input); + const origin = yield* EnvironmentSupervisor; + const registry = yield* EnvironmentRegistry; + const used = routedReads.get(registry); + const targets = new Map(); + for (const entry of used?.values() ?? []) { + if (entry.origin !== origin.target.environmentId) continue; + const ref = input.reference; + if (ref !== undefined && !matchesReference(entry.reference, ref)) continue; + for (const target of entry.targets) targets.set(target, entry.reference); + if (input.reference !== undefined) + targets.set(origin.target.environmentId, entry.reference); + } + yield* Effect.forEach( + targets, + ([target, reference]) => + invalidateTarget(registry, origin.target.environmentId, target, [ + input.reference === undefined ? {} : { reference }, + ]), + { concurrency: 4, discard: true }, + ); + return result; + } + if ((!reads.has(tag) && !writes.has(tag)) || !isRef(input)) { + return yield* request(tag, input); + } + const ref = input; + const origin = yield* EnvironmentSupervisor; + const registry = yield* EnvironmentRegistry; + const entries = yield* SubscriptionRef.get(registry.entries); + const sourceEntry = entries.get(origin.target.environmentId); + const used = routedReads.get(registry) ?? new Map(); + routedReads.set(registry, used); + const refKey = encodeKey([ + origin.target.environmentId, + ref.projectId, + ref.host?.toLowerCase() ?? null, + ref.repository.toLowerCase(), + String(ref.number), + ]); + const finish = (operation: ReturnType>) => + operation.pipe( + Effect.tap(() => { + if (!writes.has(tag) || used.size === 0) return Effect.void; + const targets = new Map([ + [origin.target.environmentId, [ref]], + ]); + for (const entry of used.values()) { + if ( + entry.origin !== origin.target.environmentId || + !matchesReference(entry.reference, ref) + ) + continue; + for (const target of [...entry.targets, origin.target.environmentId]) { + const refs = targets.get(target) ?? []; + if (!refs.some((existing) => existing.host === entry.reference.host)) + refs.push(entry.reference); + targets.set(target, refs); + } + } + return Effect.forEach( + targets, + ([target, refs]) => + invalidateTarget(registry, origin.target.environmentId, target, [ + ...refs.map((reference) => ({ reference })), + {}, + ]), + { concurrency: 4, discard: true }, + ); + }), + ); + const alternatives = []; + for (const [id, entry] of entries) { + if (id === origin.target.environmentId) continue; + if (!(yield* routingAllowed(registry, origin.target.environmentId, id, writes.has(tag)))) + continue; + const connected = yield* registry + .run(id, EnvironmentSupervisor.pipe(Effect.flatMap((s) => SubscriptionRef.get(s.session)))) + .pipe(Effect.orElseSucceed(() => Option.none())); + if (Option.isSome(connected)) alternatives.push({ id, local: isLocal(entry) }); + } + if (alternatives.length === 0) return yield* finish(source); + + const identity = yield* request(WS_METHODS.pullRequestsRouting, ref).pipe( + Effect.timeout("2 seconds"), + Effect.catchCause((cause) => + Cause.hasInterrupts(cause) ? Effect.interrupt : Effect.succeed(null), + ), + ); + // Old servers and unknown accounts retain the existing path. + if (identity === null || identity.provider !== "github") return yield* finish(source); + + const routedInput = { + ...input, + host: identity.host, + expectedAccountId: identity.accountId, + }; + const guardedSource = sourceRead ?? (yield* Effect.cached(request(tag, routedInput))); + const local = alternatives.filter((entry) => entry.local); + const remote = alternatives.filter((entry) => !entry.local); + const sourceLocal = sourceEntry !== undefined && isLocal(sourceEntry); + const candidates = [ + ...(sourceLocal && writes.has(tag) ? [origin.target.environmentId] : []), + ...local.map((entry) => entry.id), + ...(!sourceLocal && writes.has(tag) ? [origin.target.environmentId] : []), + ...remote.map((entry) => entry.id), + ...(reads.has(tag) ? [origin.target.environmentId] : []), + ]; + const run = (id: EnvironmentId): ReturnType> => + (id === origin.target.environmentId + ? guardedSource + : registry.run(id, request(tag, routedInput)) + ).pipe( + Effect.tap(() => + Effect.sync(() => { + const entry = used.get(refKey) ?? { + origin: origin.target.environmentId, + reference: { ...ref, host: identity.host }, + targets: new Set(), + }; + entry.targets.add(id); + if (used.size >= 256 && !used.has(refKey)) { + const oldest = used.keys().next().value; + if (oldest !== undefined) used.delete(oldest); + } + used.set(refKey, entry); + }), + ), + Effect.mapError((error) => + isUnregistered(error) + ? new EnvironmentRpcUnavailableError({ + environmentId: id, + message: "The environment was removed.", + }) + : error, + ), + ); + const visit = (index: number): ReturnType> => { + const id = candidates[index]; + if (id === undefined) return guardedSource; + return Effect.gen(function* () { + if (id !== origin.target.environmentId) { + 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. + const alternate = yield* registry + .run(id, request(WS_METHODS.pullRequestsRoutingIdentity, { host: identity.host })) + .pipe( + Effect.timeout("2 seconds"), + Effect.catchCause((cause) => + Cause.hasInterrupts(cause) ? Effect.interrupt : Effect.succeed(null), + ), + ); + if ( + alternate === null || + alternate.provider !== "github" || + alternate.host.toLowerCase() !== identity.host.toLowerCase() || + alternate.accountId !== identity.accountId + ) { + return yield* visit(index + 1); + } + if (!(yield* routingAllowed(registry, origin.target.environmentId, id, writes.has(tag)))) + return yield* visit(index + 1); + } + return yield* run(id).pipe( + Effect.catch((error) => { + if ( + (reads.has(tag) || rejectedBeforeDispatch(error)) && + index + 1 < candidates.length + ) { + return visit(index + 1); + } + return Effect.fail(error); + }), + Effect.map((result) => + typeof result === "object" && result !== null && "projectId" in result + ? { + ...result, + projectId: ref.projectId, + ...(tag === WS_METHODS.pullRequestsDetail + ? { projectTitle: identity.projectTitle, workspaceRoot: identity.workspaceRoot } + : {}), + } + : result, + ), + ); + }); + }; + return yield* finish(visit(0)); + }); + + return Effect.fn("PullRequestRouting.readOrWrite")(function* ( + tag: T, + input: EnvironmentRpcInput, + ) { + if (!reads.has(tag) || !isRef(input)) return yield* routedRequest(tag, input); + const registry = yield* EnvironmentRegistry; + const entries = yield* SubscriptionRef.get(registry.entries); + if (entries.size < 2) return yield* request(tag, input); + const origin = yield* EnvironmentSupervisor; + let allowed = false; + for (const id of entries.keys()) { + if ( + id !== origin.target.environmentId && + (yield* routingAllowed(registry, origin.target.environmentId, id, false)) + ) { + allowed = true; + break; + } + } + if (!allowed) return yield* request(tag, input); + const strictInput = { ...input, allowStale: false }; + const source = yield* Effect.cached(request(tag, strictInput)); + // Cached source reads usually finish before another environment can verify its account. + // Hedge slow reads only; never race mutations or retry an ambiguous write. + return yield* Effect.race( + source, + routedRequest(tag, strictInput, source).pipe(Effect.delay("75 millis")), + ).pipe( + Effect.catch((error) => + input.allowStale !== false && + (tag === WS_METHODS.pullRequestsSummary || tag === WS_METHODS.pullRequestsDetail) + ? request(tag, input) + : Effect.fail(error), + ), + ); + }); +} diff --git a/packages/client-runtime/src/state/pullRequests.test.ts b/packages/client-runtime/src/state/pullRequests.test.ts index 6187cf2e7bd0b20a3d04494715dee16b54564dd9..670575f6bbe161256eee68dc23ebe069e6685f2d 100644 --- a/packages/client-runtime/src/state/pullRequests.test.ts +++ b/packages/client-runtime/src/state/pullRequests.test.ts @@ -1,4 +1,10 @@ -import { EnvironmentId, ProjectId, WS_METHODS, type PullRequestStack } from "@t3tools/contracts"; +import { + EnvironmentId, + ProjectId, + PullRequestOperationError, + WS_METHODS, + type PullRequestStack, +} from "@t3tools/contracts"; import { expect, it } from "@effect/vitest"; import * as Data from "effect/Data"; import * as Effect from "effect/Effect"; @@ -12,11 +18,15 @@ import { AsyncResult, Atom, AtomRegistry } from "effect/unstable/reactivity"; import { AVAILABLE_CONNECTION_STATE, + ConnectionTransientError, PrimaryConnectionTarget, + SshConnectionTarget, type PreparedConnection, type SupervisorConnectionState, } from "../connection/model.ts"; import * as EnvironmentRegistry from "../connection/registry.ts"; +import { SshConnectionProfile, type ConnectionCatalogEntry } from "../connection/catalog.ts"; +import { ConnectionProfileStore } from "../connection/profileStore.ts"; import * as EnvironmentSupervisor from "../connection/supervisor.ts"; import type { WsRpcProtocolClient } from "../rpc/protocol.ts"; import type { RpcSession } from "../rpc/session.ts"; @@ -26,9 +36,200 @@ import { } from "./pullRequests.ts"; import { PullRequestDiffLoader } from "./pullRequestDiffHttp.ts"; import { executeAtomQuery } from "./runtime.ts"; +import { createPullRequestRouter } from "./pullRequestRouting.ts"; +import { GitHubRoutingPermissions } from "../connection/githubRoutingPermissions.ts"; + +const trustedRouting = { + get: () => Effect.succeed("read-write" as const), + changes: Stream.empty, + set: () => Effect.void, + forget: () => Effect.void, +}; class MutationRefused extends Data.TaggedError("MutationRefused") {} +for (const scenario of [ + "prefers the local environment with the same github account", + "falls back before mutation when the local account differs", + "never retries an ambiguous mutation failure", + "returns a fast source read without checking alternate identities", + "keeps single-environment requests free of identity lookups", + "keeps a local origin ahead of another local environment", + "keeps mutations on an old origin server without retrying them", + "skips an old alternate server before dispatching a mutation", + "returns a successful mutation when source invalidation stalls", + "does not dispatch after routing permission is revoked during the probe", + "keeps mutations on the origin when the alternate is disabled", + "does not dispatch after the alternate is disabled during the probe", + "preserves a local source account rejection when alternate accounts differ", +] as const) { + (scenario === "returns a successful mutation when source invalidation stalls" + ? it.live + : it.effect)(scenario, () => + Effect.scoped( + Effect.gen(function* () { + const calls: string[] = []; + const inputs: unknown[] = []; + let trusted = true; + let disableAlternate = Effect.void; + const switchedAccount = + scenario === "preserves a local source account rejection when alternate accounts differ"; + const mismatch = + scenario === "falls back before mutation when the local account differs" || + switchedAccount; + const ambiguous = scenario === "never retries an ambiguous mutation failure"; + const reading = + scenario === "returns a fast source read without checking alternate identities"; + const single = scenario === "keeps single-environment requests free of identity lookups"; + const localOrigin = + scenario === "keeps a local origin ahead of another local environment" || switchedAccount; + const oldOrigin = + scenario === "keeps mutations on an old origin server without retrying them"; + const oldAlternate = + scenario === "skips an old alternate server before dispatching a mutation"; + const failure = new PullRequestOperationError({ + operation: switchedAccount ? "routeIdentity" : "runAction", + detail: "connection lost after dispatch", + }); + const clientFor = (local: boolean) => { + const name = local ? "local" : "origin"; + return { + [local ? WS_METHODS.pullRequestsRoutingIdentity : WS_METHODS.pullRequestsRouting]: ( + input: unknown, + ) => + Effect.gen(function* () { + if (local) expect(input).toEqual({ host: "github.com" }); + calls.push(`${name}:identity`); + if ( + local && + scenario === "does not dispatch after the alternate is disabled during the probe" + ) + yield* disableAlternate; + if ( + local && + scenario === + "does not dispatch after routing permission is revoked during the probe" + ) + trusted = false; + if ((!local && oldOrigin) || (local && oldAlternate)) { + return yield* Effect.die( + `Unknown request tag: ${WS_METHODS.pullRequestsRouting}`, + ); + } + return { + host: "github.com", + provider: "github", + viewer: "maria-rcks", + accountId: local && mismatch ? "456" : "123", + }; + }), + [WS_METHODS.pullRequestsRunAction]: (input: unknown) => + Effect.gen(function* () { + calls.push(`${name}:mutation`); + inputs.push(input); + if ( + !local && + switchedAccount && + (input as { expectedAccountId?: string }).expectedAccountId === "123" + ) + return yield* failure; + if (local && ambiguous) return yield* failure; + }), + [WS_METHODS.pullRequestsSummary]: (input: { allowStale?: boolean }) => + Effect.gen(function* () { + calls.push(`${name}:read`); + expect(input.allowStale).toBe(false); + if (local) return yield* failure; + return null; + }), + [WS_METHODS.pullRequestsInvalidate]: () => + Effect.gen(function* () { + calls.push(`${name}:invalidate`); + if ( + !local && + scenario === "returns a successful mutation when source invalidation stalls" + ) { + return yield* Effect.never; + } + }), + } as unknown as WsRpcProtocolClient; + }; + const { environmentRegistry, supervisor } = yield* makeTestRuntime( + clientFor(false), + single ? undefined : clientFor(true), + localOrigin, + ); + disableAlternate = SubscriptionRef.update( + environmentRegistry.entries, + (entries) => + new Map( + [...entries].map(([id, entry]) => [ + id, + id === TARGET.environmentId ? entry : { ...entry, enabled: false }, + ]), + ), + ); + const disabled = + scenario === "keeps mutations on the origin when the alternate is disabled"; + const disabledDuringProbe = + scenario === "does not dispatch after the alternate is disabled during the probe"; + if (disabled) yield* disableAlternate; + const input = { + projectId: ProjectId.make("project-1"), + host: "github.com", + repository: "acme/web", + number: 7, + action: "merge" as const, + }; + const route = createPullRequestRouter(); + const result = yield* ( + reading + ? route(WS_METHODS.pullRequestsSummary, input) + : route(WS_METHODS.pullRequestsRunAction, input) + ).pipe( + Effect.provideService(EnvironmentRegistry.EnvironmentRegistry, environmentRegistry), + Effect.provideService(GitHubRoutingPermissions, { + ...trustedRouting, + get: () => Effect.succeed(trusted ? "read-write" : "off"), + }), + Effect.provideService(EnvironmentSupervisor.EnvironmentSupervisor, supervisor), + Effect.result, + ); + + if (switchedAccount) { + expect(result).toMatchObject({ _tag: "Failure", failure }); + expect(calls.filter((call) => call.endsWith(":mutation"))).toEqual(["origin:mutation"]); + expect(inputs).toEqual([{ ...input, expectedAccountId: "123" }]); + } else if (ambiguous) { + expect(result).toMatchObject({ _tag: "Failure", failure }); + expect(calls.filter((call) => call.endsWith(":mutation"))).toEqual(["local:mutation"]); + } else { + expect(result._tag).toBe("Success"); + if (single || disabled) { + expect(calls.filter((call) => !call.endsWith(":invalidate"))).toEqual([ + "origin:mutation", + ]); + } else if (reading) expect(calls).toEqual(["origin:read"]); + else if ( + mismatch || + localOrigin || + oldOrigin || + oldAlternate || + !trusted || + disabledDuringProbe + ) + expect(calls.filter((call) => call.endsWith(":mutation"))).toEqual(["origin:mutation"]); + else { + expect(calls.filter((call) => call.endsWith(":mutation"))).toEqual(["local:mutation"]); + expect(calls).toContain("origin:invalidate"); + expect(inputs).toEqual([{ ...input, expectedAccountId: "123" }]); + } + } + }), + ), + ); +} + const TARGET = new PrimaryConnectionTarget({ environmentId: EnvironmentId.make("environment-1"), label: "Test environment", @@ -38,7 +239,11 @@ const TARGET = new PrimaryConnectionTarget({ function session(client: WsRpcProtocolClient): RpcSession { return { - client, + client: { + ...client, + [WS_METHODS.pullRequestsInvalidate]: + client[WS_METHODS.pullRequestsInvalidate] ?? (() => Effect.void), + }, initialConfig: Effect.never, subscribeServerConfig: (input) => client.subscribeServerConfig(input), ready: Effect.void, @@ -47,7 +252,18 @@ function session(client: WsRpcProtocolClient): RpcSession { }; } -const makeTestRuntime = Effect.fn("makeTestRuntime")(function* (client: WsRpcProtocolClient) { +const makeTestRuntime = Effect.fn("makeTestRuntime")(function* ( + client: WsRpcProtocolClient, + localClient?: WsRpcProtocolClient, + localOrigin = false, +) { + const originTarget = localOrigin + ? new PrimaryConnectionTarget({ + ...TARGET, + httpBaseUrl: "http://localhost:3774", + wsBaseUrl: "ws://localhost:3774", + }) + : TARGET; const connectionState: SupervisorConnectionState = { ...AVAILABLE_CONNECTION_STATE, desired: true, @@ -57,7 +273,7 @@ const makeTestRuntime = Effect.fn("makeTestRuntime")(function* (client: WsRpcPro generation: 1, }; const supervisor = EnvironmentSupervisor.EnvironmentSupervisor.of({ - target: TARGET, + target: originTarget, state: yield* SubscriptionRef.make(connectionState), session: yield* SubscriptionRef.make(Option.some(session(client))), prepared: yield* SubscriptionRef.make(Option.none()), @@ -65,9 +281,40 @@ const makeTestRuntime = Effect.fn("makeTestRuntime")(function* (client: WsRpcPro disconnect: Effect.void, retryNow: Effect.void, } satisfies EnvironmentSupervisor.EnvironmentSupervisor["Service"]); + const localTarget = new PrimaryConnectionTarget({ + environmentId: EnvironmentId.make("local-environment"), + label: "Local environment", + httpBaseUrl: "http://localhost:3773", + wsBaseUrl: "ws://localhost:3773", + }); + const localSupervisor = { + ...supervisor, + target: localTarget, + session: yield* SubscriptionRef.make(Option.some(session(localClient ?? client))), + }; const environmentRegistry = EnvironmentRegistry.EnvironmentRegistry.of({ - run: (_environmentId, effect) => - Effect.provideService(effect, EnvironmentSupervisor.EnvironmentSupervisor, supervisor), + entries: yield* SubscriptionRef.make>( + new Map([ + [ + originTarget.environmentId, + { target: originTarget, profile: Option.none(), enabled: true }, + ], + ...(localClient === undefined + ? [] + : [ + [ + localTarget.environmentId, + { target: localTarget, profile: Option.none(), enabled: true }, + ] as const, + ]), + ]), + ), + run: (environmentId, effect) => + Effect.provideService( + effect, + EnvironmentSupervisor.EnvironmentSupervisor, + environmentId === localTarget.environmentId ? localSupervisor : supervisor, + ), runStream: (_environmentId, stream) => Stream.provideService(stream, EnvironmentSupervisor.EnvironmentSupervisor, supervisor), followStream: (_environmentId, stream) => @@ -86,9 +333,427 @@ const makeTestRuntime = Effect.fn("makeTestRuntime")(function* (client: WsRpcPro const registry = yield* Effect.acquireRelease(Effect.sync(AtomRegistry.make), (registry) => Effect.sync(() => registry.dispose()), ); - return { runtime, atoms, registry }; + return { runtime, atoms, registry, environmentRegistry, supervisor }; }); +for (const permission of ["default", "origin-off", "destination-off", "read-only"] as const) { + it.effect(`does not probe another environment with ${permission} routing permission`, () => + Effect.scoped( + Effect.gen(function* () { + const calls: string[] = []; + const failure = new PullRequestOperationError({ + operation: "summary", + detail: "source failed", + }); + const client = { + [WS_METHODS.pullRequestsSummary]: () => + Effect.suspend(() => { + calls.push("source-read"); + return Effect.fail(failure); + }), + [WS_METHODS.pullRequestsRunAction]: () => + Effect.sync(() => { + calls.push("source-write"); + }), + } as unknown as WsRpcProtocolClient; + const alternate = { + [WS_METHODS.pullRequestsRoutingIdentity]: () => + Effect.die("untrusted environment was contacted"), + } as unknown as WsRpcProtocolClient; + const { environmentRegistry, supervisor } = yield* makeTestRuntime(client, alternate); + const ref = { + projectId: ProjectId.make("project-1"), + repository: "private/repo", + number: 7, + }; + const request = Effect.gen(function* () { + const route = createPullRequestRouter(); + if (permission !== "read-only") + yield* route(WS_METHODS.pullRequestsSummary, ref).pipe(Effect.flip); + yield* route(WS_METHODS.pullRequestsRunAction, { ...ref, action: "merge" }); + }).pipe( + Effect.provideService(EnvironmentRegistry.EnvironmentRegistry, environmentRegistry), + Effect.provideService(EnvironmentSupervisor.EnvironmentSupervisor, supervisor), + ); + yield* permission === "default" + ? request + : request.pipe( + Effect.provideService(GitHubRoutingPermissions, { + ...trustedRouting, + get: (entry) => + Effect.succeed( + permission === "read-only" + ? "read" + : (entry.target.environmentId === TARGET.environmentId) === + (permission === "origin-off") + ? "off" + : "read-write", + ), + }), + ); + expect(calls).toEqual( + permission === "read-only" ? ["source-write"] : ["source-read", "source-write"], + ); + }), + ), + ); +} + +for (const side of ["origin", "destination"] as const) { + for (const stored of ["matching", "changed", "missing", "unavailable", "failed"] as const) { + it.effect(`checks the current ${side} SSH profile before routing with ${stored} storage`, () => + Effect.scoped( + Effect.gen(function* () { + const calls: string[] = []; + const identity = { + host: "github.com", + provider: "github", + viewer: "maria", + accountId: "123", + }; + const client = { + [WS_METHODS.pullRequestsRouting]: () => + Effect.sync(() => { + calls.push("source-probe"); + return identity; + }), + [WS_METHODS.pullRequestsRunAction]: (input: { expectedAccountId?: string }) => + Effect.gen(function* () { + if (input.expectedAccountId !== undefined) { + return yield* new PullRequestOperationError({ + operation: "routeIdentity", + detail: "Source guard refused.", + }); + } + calls.push("source-write"); + }), + [WS_METHODS.pullRequestsInvalidate]: () => Effect.void, + } as unknown as WsRpcProtocolClient; + const alternate = { + [WS_METHODS.pullRequestsRoutingIdentity]: () => + Effect.sync(() => { + calls.push("alternate-probe"); + return identity; + }), + [WS_METHODS.pullRequestsRunAction]: () => + Effect.sync(() => { + calls.push("alternate-write"); + }), + [WS_METHODS.pullRequestsInvalidate]: () => Effect.void, + } as unknown as WsRpcProtocolClient; + const { environmentRegistry, supervisor } = yield* makeTestRuntime(client, alternate); + const environmentId = + side === "origin" ? TARGET.environmentId : EnvironmentId.make("local-environment"); + const profile = new SshConnectionProfile({ + connectionId: "ssh-1", + environmentId, + label: "SSH", + target: { alias: "work", hostname: "work.example.test", username: "maria", port: 22 }, + }); + yield* SubscriptionRef.update(environmentRegistry.entries, (entries) => + new Map(entries).set(environmentId, { + target: new SshConnectionTarget({ + environmentId, + connectionId: profile.connectionId, + label: "SSH", + }), + profile: Option.some(profile), + enabled: true, + }), + ); + const read = Effect.suspend(() => + stored === "failed" + ? Effect.fail( + new ConnectionTransientError({ + reason: "remote-unavailable", + detail: "Profile storage unavailable.", + }), + ) + : Effect.succeed( + stored === "missing" + ? Option.none() + : Option.some( + stored === "changed" + ? new SshConnectionProfile({ + ...profile, + target: { ...profile.target, hostname: "replacement.example.test" }, + }) + : profile, + ), + ), + ); + const route = createPullRequestRouter()(WS_METHODS.pullRequestsRunAction, { + projectId: ProjectId.make("project-1"), + repository: "private/repo", + number: 7, + action: "merge", + }).pipe( + Effect.provideService(EnvironmentRegistry.EnvironmentRegistry, environmentRegistry), + Effect.provideService(EnvironmentSupervisor.EnvironmentSupervisor, supervisor), + // This also represents a user enabling the stale catalog entry after re-resolution. + Effect.provideService(GitHubRoutingPermissions, trustedRouting), + ); + yield* stored === "unavailable" + ? route + : route.pipe( + Effect.provideService(ConnectionProfileStore, { + get: () => read, + put: () => Effect.die("unused"), + remove: () => Effect.die("unused"), + }), + ); + expect(calls).toEqual( + stored === "matching" + ? ["source-probe", "alternate-probe", "alternate-write"] + : ["source-write"], + ); + }), + ), + ); + } +} + +for (const probe of ["origin", "alternate"] as const) { + it.live(`bounds a stalled ${probe} metadata probe without repeating a strict source read`, () => + Effect.scoped( + Effect.gen(function* () { + let sourceReads = 0; + const identity = { + host: "github.com", + provider: "github", + viewer: "maria", + accountId: "123", + }; + const client = { + [WS_METHODS.pullRequestsRouting]: () => + probe === "origin" ? Effect.never : Effect.succeed(identity), + [WS_METHODS.pullRequestsSummary]: () => + Effect.suspend(() => { + sourceReads += 1; + return Effect.fail( + new PullRequestOperationError({ operation: "summary", detail: "source failed" }), + ); + }), + } as unknown as WsRpcProtocolClient; + const alternate = { + [WS_METHODS.pullRequestsRoutingIdentity]: () => Effect.never, + } as unknown as WsRpcProtocolClient; + const { environmentRegistry, supervisor } = yield* makeTestRuntime(client, alternate); + const error = yield* createPullRequestRouter()(WS_METHODS.pullRequestsSummary, { + projectId: ProjectId.make("project-1"), + repository: "acme/web", + number: 7, + allowStale: false, + }).pipe( + Effect.provideService(EnvironmentRegistry.EnvironmentRegistry, environmentRegistry), + Effect.provideService(EnvironmentSupervisor.EnvironmentSupervisor, supervisor), + Effect.provideService(GitHubRoutingPermissions, trustedRouting), + Effect.flip, + ); + expect(error._tag).toBe("PullRequestOperationError"); + expect(sourceReads).toBe(1); + }), + ), + ); +} + +for (const source of ["pending", "pending-local", "failed", "offline"] as const) { + it.live( + source === "offline" + ? "returns held source data only after both fresh paths fail" + : `hedges a ${source} source read to local and interrupts the losing read`, + () => + Effect.scoped( + Effect.gen(function* () { + let interrupted = false; + const calls: string[] = []; + const clientFor = (local: boolean) => + ({ + [local ? WS_METHODS.pullRequestsRoutingIdentity : WS_METHODS.pullRequestsRouting]: + () => + Effect.succeed({ + host: "github.com", + provider: "github", + viewer: "maria-rcks", + accountId: "123", + }), + [WS_METHODS.pullRequestsSummary]: (input: { allowStale?: boolean }) => + Effect.gen(function* () { + calls.push(local ? "local" : input.allowStale === false ? "origin" : "held"); + if (source === "offline" && !local && input.allowStale === undefined) { + return { state: "open" }; + } + expect(input.allowStale).toBe(false); + if (local && source !== "offline") return null; + if (source !== "pending" && source !== "pending-local") + return yield* new PullRequestOperationError({ + operation: "summary", + detail: "github unreachable", + }); + return yield* Effect.never.pipe( + Effect.onInterrupt(() => + Effect.sync(() => { + interrupted = true; + }), + ), + ); + }), + }) as unknown as WsRpcProtocolClient; + const { environmentRegistry, supervisor } = yield* makeTestRuntime( + clientFor(false), + clientFor(true), + source === "pending-local", + ); + const result = yield* createPullRequestRouter()(WS_METHODS.pullRequestsSummary, { + projectId: ProjectId.make("project-1"), + repository: "acme/web", + number: 7, + }).pipe( + Effect.provideService(EnvironmentRegistry.EnvironmentRegistry, environmentRegistry), + Effect.provideService(GitHubRoutingPermissions, trustedRouting), + Effect.provideService(EnvironmentSupervisor.EnvironmentSupervisor, supervisor), + ); + if (source === "offline") { + expect(result).toEqual({ state: "open" }); + expect(calls).toEqual(["origin", "local", "held"]); + } else { + expect(result).toBeNull(); + expect(calls).toEqual(["origin", "local"]); + } + expect(interrupted).toBe(source === "pending" || source === "pending-local"); + }), + ), + ); +} + +it.live("keeps source workspace metadata when an alternate answers a detail read", () => + Effect.scoped( + Effect.gen(function* () { + const clientFor = (local: boolean) => + ({ + [local ? WS_METHODS.pullRequestsRoutingIdentity : WS_METHODS.pullRequestsRouting]: () => + Effect.succeed({ + host: "github.com", + provider: "github", + viewer: "maria-rcks", + accountId: "123", + projectTitle: local ? "local project" : "source project", + workspaceRoot: local ? "/Users/local/repo" : "/srv/source/repo", + }), + [WS_METHODS.pullRequestsDetail]: () => + local + ? Effect.succeed({ + projectId: "local-project", + projectTitle: "local project", + workspaceRoot: "/Users/local/repo", + title: "github title", + }) + : Effect.never, + }) as unknown as WsRpcProtocolClient; + const { environmentRegistry, supervisor } = yield* makeTestRuntime( + clientFor(false), + clientFor(true), + ); + const result = yield* createPullRequestRouter()(WS_METHODS.pullRequestsDetail, { + projectId: ProjectId.make("project-1"), + repository: "acme/web", + number: 7, + }).pipe( + Effect.provideService(EnvironmentRegistry.EnvironmentRegistry, environmentRegistry), + Effect.provideService(GitHubRoutingPermissions, trustedRouting), + Effect.provideService(EnvironmentSupervisor.EnvironmentSupervisor, supervisor), + ); + expect(result).toEqual({ + projectId: "project-1", + projectTitle: "source project", + workspaceRoot: "/srv/source/repo", + title: "github title", + }); + }), + ), +); + +it.live( + "refreshes identity before writes and invalidates prior readers across router instances", + () => + Effect.scoped( + Effect.gen(function* () { + let originAccountId = "123"; + const mutations: { environment: string; expectedAccountId: string }[] = []; + const invalidations: { environment: string; input: unknown }[] = []; + const identities: string[] = []; + const clientFor = (local: boolean) => { + const environment = local ? "local" : "origin"; + return { + [local ? WS_METHODS.pullRequestsRoutingIdentity : WS_METHODS.pullRequestsRouting]: () => + Effect.sync(() => { + identities.push(environment); + return { + host: "github.com", + provider: "github", + viewer: "maria-rcks", + accountId: local ? "123" : originAccountId, + }; + }), + [WS_METHODS.pullRequestsSummary]: () => (local ? Effect.succeed(null) : Effect.never), + [WS_METHODS.pullRequestsRunAction]: (input: { expectedAccountId: string }) => + Effect.sync(() => { + mutations.push({ environment, expectedAccountId: input.expectedAccountId }); + }), + [WS_METHODS.pullRequestsInvalidate]: (input: unknown) => + Effect.sync(() => { + invalidations.push({ environment, input }); + }), + } as unknown as WsRpcProtocolClient; + }; + const { environmentRegistry, supervisor } = yield* makeTestRuntime( + clientFor(false), + clientFor(true), + ); + const reference = { + projectId: ProjectId.make("project-1"), + repository: "acme/web", + number: 7, + }; + const hostedReference = { ...reference, host: "github.com", allowStale: false }; + const route = createPullRequestRouter(); + yield* Effect.gen(function* () { + yield* route(WS_METHODS.pullRequestsSummary, reference); + originAccountId = "456"; + yield* route(WS_METHODS.pullRequestsRunAction, { ...reference, action: "merge" }); + + expect(identities.filter((environment) => environment === "origin")).toHaveLength(2); + expect(mutations).toEqual([{ environment: "origin", expectedAccountId: "456" }]); + expect(invalidations).toEqual( + expect.arrayContaining([ + { environment: "origin", input: { reference: expect.objectContaining(reference) } }, + { environment: "origin", input: { reference: hostedReference } }, + { environment: "local", input: { reference: hostedReference } }, + { environment: "origin", input: {} }, + { environment: "local", input: {} }, + ]), + ); + + invalidations.length = 0; + const refresh = createPullRequestRouter(); + yield* refresh(WS_METHODS.pullRequestsInvalidate, { reference }); + expect(invalidations).toContainEqual({ + environment: "local", + input: { reference: hostedReference }, + }); + + invalidations.length = 0; + yield* refresh(WS_METHODS.pullRequestsInvalidate, {}); + expect(invalidations).toContainEqual({ environment: "local", input: {} }); + }).pipe( + Effect.provideService(EnvironmentRegistry.EnvironmentRegistry, environmentRegistry), + Effect.provideService(GitHubRoutingPermissions, trustedRouting), + Effect.provideService(EnvironmentSupervisor.EnvironmentSupervisor, supervisor), + ); + }), + ), +); + it.effect("keeps concurrent diff file reads on different hosts separate", () => Effect.scoped( Effect.gen(function* () { diff --git a/packages/client-runtime/src/state/pullRequests.ts b/packages/client-runtime/src/state/pullRequests.ts index 26a0c08c13edfc94fa012f2942e24fa78fad9d10..1086066de682e6248fb6de99c31b635cd5bff409 100644 --- a/packages/client-runtime/src/state/pullRequests.ts +++ b/packages/client-runtime/src/state/pullRequests.ts @@ -21,6 +21,7 @@ import { createEnvironmentRpcSubscriptionAtomFamily, createEnvironmentQueryAtomFamily, } from "./runtime.ts"; +import { createPullRequestRouter } from "./pullRequestRouting.ts"; import { PullRequestDiffLoader } from "./pullRequestDiffHttp.ts"; import type { EnvironmentRegistry } from "../connection/registry.ts"; import { EnvironmentSupervisor } from "../connection/supervisor.ts"; @@ -99,9 +100,11 @@ export function createLinkedPullRequestSummaryAtomFamily( runtime: Atom.AtomRuntime, refreshes = createPullRequestRefreshAtomFamily(runtime), ) { + const routedRequest = createPullRequestRouter(); return createEnvironmentRpcQueryAtomFamily(runtime, { label: "environment-data:pull-requests:linked-summary", tag: WS_METHODS.pullRequestsSummary, + execute: (input) => routedRequest(WS_METHODS.pullRequestsSummary, input), staleTimeMs: 60_000, refreshIntervalMs: 60_000, idleTtlMs: LINKED_PULL_REQUEST_IDLE_TTL_MS, @@ -114,9 +117,11 @@ export function createPullRequestStackAtomFamily( runtime: Atom.AtomRuntime, refreshes = createPullRequestRefreshAtomFamily(runtime), ) { + const routedRequest = createPullRequestRouter(); return createEnvironmentRpcQueryAtomFamily(runtime, { label: "environment-data:pull-requests:stack", tag: WS_METHODS.pullRequestsStack, + execute: (input) => routedRequest(WS_METHODS.pullRequestsStack, input), staleTimeMs: 60_000, idleTtlMs: LINKED_PULL_REQUEST_IDLE_TTL_MS, refreshTrigger: ({ environmentId }) => refreshes({ environmentId, input: {} }), @@ -148,6 +153,7 @@ export function createPullRequestEnvironmentAtoms( ) { const refreshes = createPullRequestRefreshAtomFamily(runtime); const commandScheduler = createAtomCommandScheduler(); + const routedRequest = createPullRequestRouter(); const serialPerEnvironment = { mode: "serial", key: ({ environmentId }: { readonly environmentId: string }) => environmentId, @@ -156,6 +162,7 @@ export function createPullRequestEnvironmentAtoms( createEnvironmentRpcQueryAtomFamily(runtime, { label: "environment-data:pull-requests:activity", tag: WS_METHODS.pullRequestsActivity, + execute: (input) => routedRequest(WS_METHODS.pullRequestsActivity, input), staleTimeMs: 60_000, refreshTrigger: ({ environmentId }) => refreshes({ environmentId, input: {} }), }), @@ -164,6 +171,7 @@ export function createPullRequestEnvironmentAtoms( createEnvironmentRpcQueryAtomFamily(runtime, { label: "environment-data:pull-requests:detail", tag: WS_METHODS.pullRequestsDetail, + execute: (input) => routedRequest(WS_METHODS.pullRequestsDetail, input), staleTimeMs: 60_000, refreshTrigger: ({ environmentId }) => refreshes({ environmentId, input: {} }), }), @@ -172,6 +180,7 @@ export function createPullRequestEnvironmentAtoms( createEnvironmentRpcQueryAtomFamily(runtime, { label: "environment-data:pull-requests:label-candidates", tag: WS_METHODS.pullRequestsLabelCandidates, + execute: (input) => routedRequest(WS_METHODS.pullRequestsLabelCandidates, input), staleTimeMs: 60_000, }), ); @@ -179,6 +188,7 @@ export function createPullRequestEnvironmentAtoms( createEnvironmentRpcQueryAtomFamily(runtime, { label: "environment-data:pull-requests:reviewer-candidates", tag: WS_METHODS.pullRequestsReviewerCandidates, + execute: (input) => routedRequest(WS_METHODS.pullRequestsReviewerCandidates, input), staleTimeMs: 60_000, }), ); @@ -215,6 +225,7 @@ export function createPullRequestEnvironmentAtoms( threadComments: createEnvironmentRpcCommand(runtime, { label: "environment-data:pull-requests:thread-comments", tag: WS_METHODS.pullRequestsThreadComments, + execute: (input) => routedRequest(WS_METHODS.pullRequestsThreadComments, input), scheduler: commandScheduler, concurrency: { mode: "singleFlight", @@ -241,6 +252,7 @@ export function createPullRequestEnvironmentAtoms( diffFileContents: createEnvironmentRpcCommand(runtime, { label: "environment-data:pull-requests:diff-file-contents", tag: WS_METHODS.pullRequestsDiffFileContents, + execute: (input) => routedRequest(WS_METHODS.pullRequestsDiffFileContents, input), scheduler: commandScheduler, concurrency: { mode: "singleFlight", @@ -261,24 +273,28 @@ export function createPullRequestEnvironmentAtoms( runAction: createEnvironmentRpcCommand(runtime, { label: "environment-data:pull-requests:run-action", tag: WS_METHODS.pullRequestsRunAction, + execute: (input) => routedRequest(WS_METHODS.pullRequestsRunAction, input), scheduler: commandScheduler, concurrency: serialPerEnvironment, }), update: createEnvironmentRpcCommand(runtime, { label: "environment-data:pull-requests:update", tag: WS_METHODS.pullRequestsUpdate, + execute: (input) => routedRequest(WS_METHODS.pullRequestsUpdate, input), scheduler: commandScheduler, concurrency: serialPerEnvironment, }), comment: createEnvironmentRpcCommand(runtime, { label: "environment-data:pull-requests:comment", tag: WS_METHODS.pullRequestsComment, + execute: (input) => routedRequest(WS_METHODS.pullRequestsComment, input), scheduler: commandScheduler, concurrency: serialPerEnvironment, }), updateComment: createEnvironmentRpcCommand(runtime, { label: "environment-data:pull-requests:update-comment", tag: WS_METHODS.pullRequestsUpdateComment, + execute: (input) => routedRequest(WS_METHODS.pullRequestsUpdateComment, input), scheduler: commandScheduler, concurrency: serialPerEnvironment, onSuccess: ({ environmentId, input: { projectId, host, repository, number } }, registry) => @@ -294,12 +310,14 @@ export function createPullRequestEnvironmentAtoms( submitReview: createEnvironmentRpcCommand(runtime, { label: "environment-data:pull-requests:submit-review", tag: WS_METHODS.pullRequestsSubmitReview, + execute: (input) => routedRequest(WS_METHODS.pullRequestsSubmitReview, input), scheduler: commandScheduler, concurrency: serialPerEnvironment, }), replyToThread: createEnvironmentRpcCommand(runtime, { label: "environment-data:pull-requests:reply-to-thread", tag: WS_METHODS.pullRequestsReplyToThread, + execute: (input) => routedRequest(WS_METHODS.pullRequestsReplyToThread, input), scheduler: commandScheduler, concurrency: serialPerEnvironment, }), @@ -313,6 +331,7 @@ export function createPullRequestEnvironmentAtoms( requestReviewers: createEnvironmentRpcCommand(runtime, { label: "environment-data:pull-requests:request-reviewers", tag: WS_METHODS.pullRequestsRequestReviewers, + execute: (input) => routedRequest(WS_METHODS.pullRequestsRequestReviewers, input), scheduler: commandScheduler, concurrency: serialPerEnvironment, onSuccess: (target, registry) => @@ -389,6 +408,7 @@ export function createPullRequestEnvironmentAtoms( setLabels: createEnvironmentRpcCommand(runtime, { label: "environment-data:pull-requests:set-labels", tag: WS_METHODS.pullRequestsSetLabels, + execute: (input) => routedRequest(WS_METHODS.pullRequestsSetLabels, input), scheduler: commandScheduler, concurrency: serialPerEnvironment, onSuccess: (target, registry) => @@ -424,12 +444,14 @@ export function createPullRequestEnvironmentAtoms( setThreadResolution: createEnvironmentRpcCommand(runtime, { label: "environment-data:pull-requests:set-thread-resolution", tag: WS_METHODS.pullRequestsSetThreadResolution, + execute: (input) => routedRequest(WS_METHODS.pullRequestsSetThreadResolution, input), scheduler: commandScheduler, concurrency: serialPerEnvironment, }), setReaction: createEnvironmentRpcCommand(runtime, { label: "environment-data:pull-requests:set-reaction", tag: WS_METHODS.pullRequestsSetReaction, + execute: (input) => routedRequest(WS_METHODS.pullRequestsSetReaction, input), scheduler: commandScheduler, concurrency: serialPerEnvironment, }), @@ -441,6 +463,7 @@ export function createPullRequestEnvironmentAtoms( invalidate: createEnvironmentRpcCommand(runtime, { label: "environment-data:pull-requests:invalidate", tag: WS_METHODS.pullRequestsInvalidate, + execute: (input) => routedRequest(WS_METHODS.pullRequestsInvalidate, input), scheduler: commandScheduler, concurrency: serialPerEnvironment, }), diff --git a/packages/client-runtime/src/state/runtime.ts b/packages/client-runtime/src/state/runtime.ts index 46cb9fa0f11595dcb7981543e25d3045091d05ce..84946bf9e165a8f0e8c833da762657c34791fd7f 100644 --- a/packages/client-runtime/src/state/runtime.ts +++ b/packages/client-runtime/src/state/runtime.ts @@ -11,6 +11,8 @@ import type { ConnectionAttemptError } from "../connection/model.ts"; import { EnvironmentNotRegisteredError, EnvironmentRegistry } from "../connection/registry.ts"; import { type EnvironmentRpcInput, + type EnvironmentRpcSuccess, + type EnvironmentRpcFailure, type EnvironmentRpcStreamFailure, type EnvironmentRpcStreamValue, type EnvironmentSubscriptionRpcTag, @@ -588,7 +590,12 @@ export function createEnvironmentSubscriptionAtomFamily( export function createEnvironmentCommand( runtime: Atom.AtomRuntime, - options: EnvironmentCommandAtomOptions, + options: EnvironmentCommandAtomOptions< + Input, + A, + E, + EnvironmentSupervisor | EnvironmentRegistry | R + >, ) { return createRuntimeCommand(runtime, { label: options.label, @@ -607,6 +614,13 @@ export function createEnvironmentRpcQueryAtomFamily, + ) => Effect.Effect< + EnvironmentRpcSuccess, + EnvironmentRpcFailure | EnvironmentRpcUnavailableError, + EnvironmentSupervisor | EnvironmentRegistry + >; readonly staleTimeMs?: number; readonly idleTtlMs?: number; readonly refreshIntervalMs?: number; @@ -624,7 +638,8 @@ export function createEnvironmentRpcQueryAtomFamily) => request(options.tag, input), + execute: (input: EnvironmentRpcInput) => + options.execute?.(input) ?? request(options.tag, input), }); } @@ -665,6 +680,13 @@ export function createEnvironmentRpcCommand, + ) => Effect.Effect< + EnvironmentRpcSuccess, + EnvironmentRpcFailure | EnvironmentRpcUnavailableError, + EnvironmentSupervisor | EnvironmentRegistry + >; readonly scheduler?: AtomCommandScheduler; readonly concurrency?: AtomCommandConcurrency<{ readonly environmentId: EnvironmentIdType; @@ -695,7 +717,7 @@ export function createEnvironmentRpcCommand options.onSuccess?.(target, registry) ?? Effect.void), Effect.ensuring(options.onSettled?.(target, registry) ?? Effect.void), ); diff --git a/packages/contracts/src/pullRequest.ts b/packages/contracts/src/pullRequest.ts index 0194521d0d8949f1efbd0e17324e447a2c74b740..d2bd6a839d3d5925b0d65847c02b5d2f2b03ee03 100644 --- a/packages/contracts/src/pullRequest.ts +++ b/packages/contracts/src/pullRequest.ts @@ -643,11 +643,39 @@ export type PullRequestListResult = typeof PullRequestListResult.Type; export const PullRequestRef = Schema.Struct({ projectId: ProjectId, host: Schema.optional(TrimmedNonEmptyString), + /** Refuse a routed operation unless this GitHub account still owns the active credential. */ + expectedAccountId: Schema.optional(TrimmedNonEmptyString), + /** Let another environment answer when this one's cached response has expired. */ + allowStale: Schema.optional(Schema.Boolean), repository: TrimmedNonEmptyString, number: PositiveInt, }); export type PullRequestRef = typeof PullRequestRef.Type; +/** Account discovery never needs the originating project's private repository metadata. */ +export const PullRequestRoutingIdentityInput = Schema.Struct({ + host: TrimmedNonEmptyString, +}); +export type PullRequestRoutingIdentityInput = typeof PullRequestRoutingIdentityInput.Type; + +export const PullRequestRoutingIdentityResult = Schema.Struct({ + accountId: TrimmedNonEmptyString, + host: TrimmedNonEmptyString, + provider: Schema.Literal("github"), + viewer: TrimmedNonEmptyString, +}); +export type PullRequestRoutingIdentityResult = typeof PullRequestRoutingIdentityResult.Type; + +export const PullRequestRoutingResult = Schema.Struct({ + accountId: TrimmedNonEmptyString, + host: TrimmedNonEmptyString, + provider: SourceControlProviderKind, + viewer: TrimmedNonEmptyString, + projectTitle: TrimmedNonEmptyString, + workspaceRoot: TrimmedNonEmptyString, +}); +export type PullRequestRoutingResult = typeof PullRequestRoutingResult.Type; + export const PullRequestLinkedThreadsResult = Schema.Struct({ threads: Schema.Array( Schema.Struct({ diff --git a/packages/contracts/src/rpc.ts b/packages/contracts/src/rpc.ts index 5e11aeb28fa367bad0cb5f14a7003e7ead885081..7ec895137eb455e3a4ca089178e81db0f286f69d 100644 --- a/packages/contracts/src/rpc.ts +++ b/packages/contracts/src/rpc.ts @@ -115,6 +115,9 @@ import { PullRequestOperationError, PullRequestReactionInput, PullRequestRef, + PullRequestRoutingResult, + PullRequestRoutingIdentityInput, + PullRequestRoutingIdentityResult, PullRequestStack, PullRequestLinkedThreadsResult, PullRequestSummary, @@ -371,6 +374,8 @@ export const WS_METHODS = { pullRequestsList: "pullRequests.list", pullRequestsListStats: "pullRequests.listStats", pullRequestsSummary: "pullRequests.summary", + pullRequestsRouting: "pullRequests.routing", + pullRequestsRoutingIdentity: "pullRequests.routingIdentity", pullRequestsStack: "pullRequests.stack", pullRequestsLinkedThreads: "pullRequests.linkedThreads", pullRequestsDetail: "pullRequests.detail", @@ -672,6 +677,18 @@ const WsPullRequestsListStatsRpc = Rpc.make(WS_METHODS.pullRequestsListStats, { error: PullRequestRpcError, }); +const WsPullRequestsRoutingRpc = Rpc.make(WS_METHODS.pullRequestsRouting, { + payload: PullRequestRef, + success: PullRequestRoutingResult, + error: PullRequestRpcError, +}); + +const WsPullRequestsRoutingIdentityRpc = Rpc.make(WS_METHODS.pullRequestsRoutingIdentity, { + payload: PullRequestRoutingIdentityInput, + success: PullRequestRoutingIdentityResult, + error: PullRequestRpcError, +}); + const WsPullRequestsSummaryRpc = Rpc.make(WS_METHODS.pullRequestsSummary, { payload: PullRequestRef, success: PullRequestSummary, @@ -1316,6 +1333,8 @@ export const WsRpcGroup = RpcGroup.make( WsPullRequestsListRpc, WsPullRequestsListStatsRpc, WsPullRequestsSummaryRpc, + WsPullRequestsRoutingRpc, + WsPullRequestsRoutingIdentityRpc, WsPullRequestsStackRpc, WsPullRequestsLinkedThreadsRpc, WsPullRequestsDetailRpc,