From 2aa42ac7d25d47ac0253bdd2be845e1fd7bbfc5d Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Mon, 5 Oct 2026 17:49:55 -0700 Subject: [PATCH 1/9] feat(server): GitHubCli reads and writes GitHub over the API instead of gh Pull request lookups, pr view, repo view, default branch, repo create and pr create go through GitHubApi; pr checkout runs plain git. The repository is resolved from git remotes the way gh picks it. execute stays, deprecated, for the pull request layer until it moves too. Co-Authored-By: Claude Opus 5.5 (1M context) --- apps/server/src/sourceControl/GitHubApi.ts | 3 +- apps/server/src/sourceControl/GitHubCli.ts | 1239 +++++++++++--------- 2 files changed, 710 insertions(+), 532 deletions(-) diff --git a/apps/server/src/sourceControl/GitHubApi.ts b/apps/server/src/sourceControl/GitHubApi.ts index a6cc60b50aa8..01aa1c9ccfcf 100644 --- a/apps/server/src/sourceControl/GitHubApi.ts +++ b/apps/server/src/sourceControl/GitHubApi.ts @@ -145,6 +145,7 @@ export interface GitHubGraphQlInput { readonly query: string; readonly variables?: Readonly>; readonly allowReserve?: boolean; + readonly maxResponseBytes?: number; } export class GitHubApi extends Context.Service< @@ -567,7 +568,7 @@ export const make = Effect.gen(function* () { HttpClientRequest.acceptJson, HttpClientRequest.bodyJsonUnsafe({ query, variables: input.variables ?? {} }), ), - maxResponseBytes: DEFAULT_MAX_RESPONSE_BYTES, + maxResponseBytes: input.maxResponseBytes ?? DEFAULT_MAX_RESPONSE_BYTES, allowReserve, acceptNotModified: false, graphql: true, diff --git a/apps/server/src/sourceControl/GitHubCli.ts b/apps/server/src/sourceControl/GitHubCli.ts index 88ddf1349e86..5032c289e9a6 100644 --- a/apps/server/src/sourceControl/GitHubCli.ts +++ b/apps/server/src/sourceControl/GitHubCli.ts @@ -1,10 +1,8 @@ -import * as Cache from "effect/Cache"; -import * as Clock from "effect/Clock"; -import * as Duration from "effect/Duration"; -import * as Exit from "effect/Exit"; import * as Context from "effect/Context"; import * as DateTime from "effect/DateTime"; import * as Effect from "effect/Effect"; +import * as Exit from "effect/Exit"; +import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; import * as PlatformError from "effect/PlatformError"; @@ -21,15 +19,19 @@ import { } from "@t3tools/contracts"; import { normalizeGitRemoteUrl } from "@t3tools/shared/git"; import { decodeJsonResult } from "@t3tools/shared/schemaJson"; +import { + detectSourceControlProviderFromRemoteUrl, + isSshRemoteUrl, +} from "@t3tools/shared/sourceControl"; +import * as GitVcsDriver from "../vcs/GitVcsDriver.ts"; import * as VcsProcess from "../vcs/VcsProcess.ts"; import * as GitHubApi from "./GitHubApi.ts"; +import * as GitHubCredentials from "./GitHubCredentials.ts"; import * as GitHubGraphQlBudget from "./githubGraphQlBudget.ts"; import * as SourceControlRateLimit from "./SourceControlRateLimit.ts"; import { decodeGitHubPullRequestEntries, - decodeGitHubPullRequestJson, - decodeGitHubPullRequestListJson, type NormalizedGitHubPullRequestRecord, } from "./gitHubPullRequests.ts"; @@ -40,6 +42,8 @@ export const PinnedGitHubCredential = GitHubApi.PinnedGitHubCredential; export const AllowGitHubReserve = GitHubApi.AllowGitHubReserve; +// The hosts a deprecated `execute` command reaches, so a pinned credential never leaves its +// host. Deleted with `execute`. function commandHosts(args: ReadonlyArray): Array { const hosts: Array = []; const repositoryHost = (repository: string | undefined) => { @@ -83,7 +87,7 @@ export class GitHubCliUnavailableError extends Schema.TaggedError; @@ -302,16 +338,16 @@ export class GitHubCli extends Context.Service< }) => Effect.Effect, GitHubCliError>; /** - * Pull requests whose head is `headSelector`, in the repository `gh pr list` would read in - * `cwd`. Lookups on one repository that arrive together share one GraphQL document; a - * checkout whose repository gh could pick another way is asked through `gh pr list`. + * Pull requests whose head is `headSelector` (a branch, or `owner:branch` for a fork), in the + * repository `gh pr list` would read in `cwd`. Lookups on one repository that arrive + * together share one GraphQL document. */ readonly listPullRequestsByHead: (input: { readonly cwd: string; readonly headSelector: string; readonly state: "open" | "closed" | "merged" | "all"; readonly limit: number; - /** The checkout's GitHub host. Without it the lookup is not batched. */ + /** The checkout's GitHub API host. Without it, the host comes from the git remotes. */ readonly rateLimitHost?: string; }) => Effect.Effect, GitHubCliError>; @@ -353,67 +389,73 @@ export class GitHubCli extends Context.Service< } >()("t3/sourceControl/GitHubCli") {} -const RawGitHubRepositoryCloneUrlsSchema = Schema.Struct({ - nameWithOwner: TrimmedNonEmptyString, - url: TrimmedNonEmptyString, - sshUrl: TrimmedNonEmptyString, +/** + * The repository `gh pr list` reads in a checkout, picked the way gh picks one without a + * prompt: the remote `gh repo set-default` marked, else the first of upstream, github, origin + * (in any case), else the only remote. `remotes` is `git remote -v` output and `resolved` is the output of + * `git config --get-regexp '^remote\..*\.gh-resolved$'`. + * + * Null whenever gh might weigh the remotes differently: a remote on another host or under an + * SSH alias, more than one mark, or several remotes with none of those names. Callers then + * fall back to the provider's remote, then to the best-ranked remote on the host. + */ +export function selectGitHubBaseRepository(input: { + readonly remotes: string; + readonly resolved: string; + readonly host: string; +}): { readonly owner: string; readonly name: string } | null { + const host = input.host.toLowerCase(); + const repositories = new Map(); + for (const line of input.remotes.split("\n")) { + const match = /^(\S+)\s+(\S+)\s+\(fetch\)$/u.exec(line.trim()); + if (!match) continue; + const [remoteHost, owner, name, ...rest] = normalizeGitRemoteUrl(match[2]!).split("/"); + if (remoteHost !== host || !owner || !name || rest.length > 0) return null; + repositories.set(match[1]!, { owner, name }); + } + const marks = input.resolved + .split("\n") + .map((line) => /^remote\.(.+)\.gh-resolved\s+(\S+)$/u.exec(line.trim())) + .filter((match): match is RegExpExecArray => match !== null && repositories.has(match[1]!)); + if (marks.length > 1) return null; + const [mark] = marks; + if (mark) { + if (mark[2] === "base") return repositories.get(mark[1]!) ?? null; + const [owner, name, ...rest] = mark[2]!.toLowerCase().split("/"); + return owner && name && rest.length === 0 ? { owner, name } : null; + } + // gh sorts remotes by these names, case-insensitively, and takes the first. A tie for the + // top place has no defined winner. + const score = (remoteName: string) => + ["origin", "github", "upstream"].indexOf(remoteName.toLowerCase()) + 1; + const ranked = [...repositories.entries()].toSorted( + ([left], [right]) => score(right) - score(left), + ); + const [top, next] = ranked; + return top !== undefined && (next === undefined || score(top[0]) > score(next[0])) + ? top[1] + : null; +} + +const RawRepositorySchema = Schema.Struct({ + full_name: TrimmedNonEmptyString, + html_url: TrimmedNonEmptyString, + ssh_url: TrimmedNonEmptyString, + default_branch: Schema.optional(Schema.NullOr(Schema.String)), }); -const decodeRawGitHubRepositoryCloneUrls = Schema.decodeEffect( - Schema.fromJsonString(RawGitHubRepositoryCloneUrlsSchema), -); +const decodeRawRepository = decodeJsonResult(RawRepositorySchema); -function normalizeRepositoryCloneUrls( - raw: Schema.Schema.Type, +function repositoryCloneUrls( + raw: Schema.Schema.Type, ): GitHubRepositoryCloneUrls { - return { - nameWithOwner: raw.nameWithOwner, - url: raw.url, - sshUrl: raw.sshUrl, - }; + return { nameWithOwner: raw.full_name, url: raw.html_url, sshUrl: raw.ssh_url }; } -/** - * `gh repo create` prints the canonical URL of the new repository on stdout - * (e.g. `https://github.com/owner/repo`). Reading it back here avoids a - * follow-up `gh repo view`, which can race GitHub's GraphQL eventual - * consistency window and falsely report the just-created repo as missing. - */ -function deriveRepositoryCloneUrlsFromCreateOutput( - stdout: string, - repository: string, -): GitHubRepositoryCloneUrls { - const fallbackHost = "github.com"; - const match = stdout.match(/https?:\/\/[^\s]+/); - if (match) { - const cleaned = match[0].replace(/\.git$/, ""); - try { - const parsed = new URL(cleaned); - const pathname = parsed.pathname.replace(/^\/+|\/+$/g, ""); - const segments = pathname.split("/").filter(Boolean); - if (segments.length === 2) { - const nameWithOwner = `${segments[0]}/${segments[1]}`; - return { - nameWithOwner, - url: `${parsed.origin}/${nameWithOwner}`, - sshUrl: `git@${parsed.host}:${nameWithOwner}.git`, - }; - } - } catch { - // Fall through to the input-derived defaults below. - } - } - return { - nameWithOwner: repository, - url: `https://${fallbackHost}/${repository}`, - sshUrl: `git@${fallbackHost}:${repository}.git`, - }; -} +const decodeViewerLogin = decodeJsonResult(Schema.Struct({ login: TrimmedNonEmptyString })); type PullRequestListState = "open" | "closed" | "merged" | "all"; -const PULL_REQUEST_LIST_JSON_FIELDS = - "number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner"; -/** The `gh pr list --json` fields above, as GraphQL selects them. */ +/** The pull request fields every read selects, in GraphQL. */ const PULL_REQUEST_NODE_SELECTION = "number title url baseRefName headRefName state isDraft mergedAt closedAt updatedAt isCrossRepository headRepository { name nameWithOwner } headRepositoryOwner { login }"; const GRAPHQL_STATES: Record> = { @@ -424,7 +466,7 @@ const GRAPHQL_STATES: Record> = { }; /** * Head lookups per GraphQL document. A document of a hundred costs one point, the same as one - * `gh pr list`, but half that keeps each answer near half a second. + * single lookup, but half that keeps each answer near half a second. */ const HEAD_LOOKUPS_PER_DOCUMENT = 50; /** @@ -433,8 +475,13 @@ const HEAD_LOOKUPS_PER_DOCUMENT = 50; */ const HEAD_LOOKUP_BATCH_WINDOW = "50 millis"; /** A full document is 5,000 rows of well under 2 KB each. */ -const HEAD_LOOKUP_MAX_OUTPUT_BYTES = 16_000_000; -const RATE_LIMIT_READING = "query { rateLimit { cost limit remaining resetAt } }"; +const HEAD_LOOKUP_MAX_RESPONSE_BYTES = 16_000_000; +/** + * Rows read for a fork's `owner:branch` head, which GitHub cannot filter by owner. A branch + * named like a busy default (`main`) is the case this bounds; the owner's own row is near the + * top because the newest come first. + */ +const OWNER_HEAD_SCAN_LIMIT = 100; class PullRequestsByHeadRead extends Request.Class< { @@ -445,25 +492,17 @@ class PullRequestsByHeadRead extends Request.Class< readonly headRefName: string; readonly state: PullRequestListState; readonly limit: number; + readonly allowReserve: boolean; }, ReadonlyArray, GitHubCliError > {} -const GraphQlVariables = Schema.Record( - Schema.String, - Schema.Union([Schema.String, Schema.Array(Schema.String)]), -); -/** A GraphQL request body for `gh api graphql --input -`. */ -const encodeGraphQlRequest = Schema.encodeSync( - Schema.fromJsonString(Schema.Struct({ query: Schema.String, variables: GraphQlVariables })), -); - /** One aliased `pullRequests` connection per lookup, each head and state passed as a variable. */ function buildPullRequestsByHeadQuery( lookups: ReadonlyArray>, -): { readonly document: string; readonly variables: typeof GraphQlVariables.Type } { - const variables: Record> = {}; +): { readonly document: string; readonly variables: Record } { + const variables: Record = {}; const declarations: string[] = ["$owner: String!", "$name: String!"]; const selections: string[] = []; for (const [index, lookup] of lookups.entries()) { @@ -482,83 +521,139 @@ function buildPullRequestsByHeadQuery( }; } -/** The reset time of a `rateLimit` reading, which sets when the next one is due. */ -const decodeRateLimitReading = (raw: string) => - Result.map( - decodeJsonResult( - Schema.Struct({ - data: Schema.Struct({ rateLimit: Schema.Struct({ resetAt: Schema.String }) }), - }), - )(raw), - (reading) => reading.data.rateLimit.resetAt, - ); +const PULL_REQUEST_BY_NUMBER_QUERY = `query PullRequestByNumber($owner: String!, $name: String!, $number: Int!) { + repository(owner: $owner, name: $name) { + pullRequest(number: $number) { ${PULL_REQUEST_NODE_SELECTION} } + } +}`; const decodePullRequestsByHead = decodeJsonResult( Schema.Struct({ data: Schema.Struct({ - repository: Schema.Record( - Schema.String, - Schema.NullOr(Schema.Struct({ nodes: Schema.Array(Schema.Unknown) })), + repository: Schema.NullOr( + Schema.Record( + Schema.String, + Schema.NullOr(Schema.Struct({ nodes: Schema.Array(Schema.Unknown) })), + ), ), }), }), ); -/** - * The repository `gh pr list` reads in a checkout, picked the way gh picks one without a - * prompt: the remote `gh repo set-default` marked, else the first of upstream, github, origin - * (in any case), else the only remote. `remotes` is `git remote -v` output and `resolved` is the output of - * `git config --get-regexp '^remote\..*\.gh-resolved$'`. - * - * Null whenever gh might weigh the remotes differently: a remote on another host or under an - * SSH alias, more than one mark, or several remotes with none of those names. Callers then - * ask gh itself. - */ -export function selectGitHubBaseRepository(input: { - readonly remotes: string; - readonly resolved: string; +const decodePullRequestByNumber = decodeJsonResult( + Schema.Struct({ + data: Schema.Struct({ + repository: Schema.NullOr(Schema.Struct({ pullRequest: Schema.NullOr(Schema.Unknown) })), + }), + }), +); + +/** A repository on a GitHub host: the API it is read through, and its owner and name. */ +export interface GitHubRepositoryLocator { readonly host: string; -}): { readonly owner: string; readonly name: string } | null { - const host = input.host.toLowerCase(); - const repositories = new Map(); - for (const line of input.remotes.split("\n")) { - const match = /^(\S+)\s+(\S+)\s+\(fetch\)$/u.exec(line.trim()); - if (!match) continue; - const [remoteHost, owner, name, ...rest] = normalizeGitRemoteUrl(match[2]!).split("/"); - if (remoteHost !== host || !owner || !name || rest.length > 0) return null; - repositories.set(match[1]!, { owner, name }); + readonly owner: string; + readonly name: string; +} + +/** `owner/name` or `host/owner/name`, as `gh --repo` and GH_REPO take them. */ +export function parseGitHubRepositorySelector( + selector: string, + defaultHost: string, +): GitHubRepositoryLocator | null { + const trimmed = selector.trim().replace(/\.git$/i, ""); + if (/^https?:\/\//i.test(trimmed)) { + try { + const url = new URL(trimmed); + const [owner, name, ...rest] = url.pathname.split("/").filter(Boolean); + return owner && name && rest.length === 0 + ? { host: url.host.toLowerCase(), owner, name } + : null; + } catch { + return null; + } } - const marks = input.resolved - .split("\n") - .map((line) => /^remote\.(.+)\.gh-resolved\s+(\S+)$/u.exec(line.trim())) - .filter((match): match is RegExpExecArray => match !== null && repositories.has(match[1]!)); - if (marks.length > 1) return null; - const [mark] = marks; - if (mark) { - if (mark[2] === "base") return repositories.get(mark[1]!) ?? null; - const [owner, name, ...rest] = mark[2]!.toLowerCase().split("/"); - return owner && name && rest.length === 0 ? { owner, name } : null; + const parts = trimmed.split("/").filter(Boolean); + if (parts.length === 2) return { host: defaultHost, owner: parts[0]!, name: parts[1]! }; + if (parts.length === 3) + return { host: parts[0]!.toLowerCase(), owner: parts[1]!, name: parts[2]! }; + return null; +} + +/** + * A pull request reference the way `gh pr view` takes one: a number (`#7` too), a pull request + * URL, or a branch name. + */ +export function parsePullRequestReference( + reference: string, +): + | { readonly kind: "number"; readonly number: number } + | { readonly kind: "url"; readonly locator: GitHubRepositoryLocator; readonly number: number } + | { readonly kind: "branch"; readonly headSelector: string } { + const trimmed = reference.trim(); + const numbered = /^#?([1-9]\d*)$/.exec(trimmed); + if (numbered) return { kind: "number", number: Number(numbered[1]) }; + if (/^https?:\/\//i.test(trimmed)) { + try { + const url = new URL(trimmed); + const match = /^\/([^/]+)\/([^/]+)\/pull\/([1-9]\d*)(?:\/.*)?$/.exec(url.pathname); + if (match) { + return { + kind: "url", + locator: { host: url.host.toLowerCase(), owner: match[1]!, name: match[2]! }, + number: Number(match[3]), + }; + } + } catch { + // Not a URL after all; read it as a branch. + } } - // gh sorts remotes by these names, case-insensitively, and takes the first. A tie for the - // top place has no defined winner. - const score = (remoteName: string) => - ["origin", "github", "upstream"].indexOf(remoteName.toLowerCase()) + 1; - const ranked = [...repositories.entries()].toSorted( - ([left], [right]) => score(right) - score(left), - ); - const [top, next] = ranked; - return top !== undefined && (next === undefined || score(top[0]) > score(next[0])) - ? top[1] - : null; + return { kind: "branch", headSelector: trimmed }; +} + +/** + * The GitHub host a remote URL is served from, or null for a remote that is not GitHub. An SSH + * alias (`git@github-work:owner/repo`) names no API host of its own; it is read through + * `github.com`, which is what such an alias almost always stands for (issue #6198). + */ +export function gitHubApiHostForRemote(remoteUrl: string): string | null { + const provider = detectSourceControlProviderFromRemoteUrl(remoteUrl); + if (provider?.kind !== "github") return null; + const host = new URL(provider.baseUrl).host.toLowerCase(); + if (host === "github.com" || host.endsWith(".ghe.com") || host.includes(".")) return host; + return isSshRemoteUrl(remoteUrl) ? "github.com" : host; +} + +/** The local branch a pull request checks out into, the way `gh pr checkout` names it. */ +export function pullRequestCheckoutBranchName(input: { + readonly headRefName: string; + readonly headOwner: string | null; + readonly isCrossRepository: boolean; + readonly defaultBranch: string | null; +}): string { + // gh prefixes the owner only where the fork's branch would take over the default branch's + // name, which is the one collision every fork pull request from `main` would hit. + return input.isCrossRepository && + input.headOwner !== null && + input.defaultBranch !== null && + input.headRefName === input.defaultBranch + ? `${input.headOwner}/${input.headRefName}` + : input.headRefName; } /** @public Service construction is part of the canonical Effect module API. */ export const make = Effect.gen(function* () { const process = yield* VcsProcess.VcsProcess; - const budget = yield* GitHubGraphQlBudget.GitHubGraphQlBudget; - const limits = yield* SourceControlRateLimit.SourceControlRateLimit; + const api = yield* GitHubApi.GitHubApi; + const git = yield* GitVcsDriver.GitVcsDriver; + const fileSystem = yield* FileSystem.FileSystem; - const executeRaw: GitHubCli["Service"]["execute"] = Effect.fn("GitHubCli.executeRaw")( + /** + * Runs `gh` itself. + * + * @deprecated Only the pull request layer still calls this, until it moves onto `GitHubApi`; + * it is deleted with that change. New code reads GitHub through `GitHubApi`. + */ + const execute: GitHubCli["Service"]["execute"] = Effect.fn("GitHubCli.execute")( function* (input) { const credential = yield* PinnedGitHubCredential; if (credential !== null && !targetsVerifiedHost(input.args, credential.host)) { @@ -629,272 +724,170 @@ export const make = Effect.gen(function* () { }, ); - /** - * A GraphQL `rateLimit` reading per host and credential, so the budget knows the balance - * before reads that cannot report their own cost (`gh pr list`). GraphQL documents keep it - * current in between. Each reading costs one point and is redone at the reset, or after ten - * minutes so a `gh auth switch` is not priced against the old account for a whole hour. - */ - const budgetReading = yield* Cache.makeWith( - (key: string) => { - const host = key.split("\0")[0]!; - return executeRaw({ - cwd: globalThis.process.cwd(), - args: ["api", "graphql", "--hostname", host, "-f", `query=${RATE_LIMIT_READING}`], - }).pipe( - Effect.tap((result) => budget.observe(host, result.stdout)), - Effect.flatMap((result) => - Clock.currentTimeMillis.pipe( - Effect.map((now) => { - const resetAtMs = Date.parse( - decodeRateLimitReading(result.stdout).pipe( - Result.match({ onFailure: () => "", onSuccess: (reading) => reading }), - ), - ); - // A reset that is missing or already past (a skewed clock) waits a minute, so a - // bad reading cannot ask again on every read. - return Duration.millis( - Number.isFinite(resetAtMs) && resetAtMs > now - ? Math.min(resetAtMs - now, 600_000) - : 60_000, - ); - }), - ), - ), - ); - }, - { - capacity: 32, - timeToLive: (exit) => (Exit.isSuccess(exit) ? exit.value : Duration.minutes(1)), - }, - ); + const gitRead = (cwd: string, args: ReadonlyArray) => + process.run({ + operation: "GitHubCli.resolveRepository", + command: "git", + args, + cwd, + allowNonZeroExit: true, + timeoutMs: 5_000, + }); + + const commandFailure = (cwd: string, detail: string) => + new GitHubCliCommandError({ command: "gh", cwd, cause: new Error(detail) }); /** - * Runs a GraphQL-priced read under its host's pause and the GraphQL budget. `run` receives - * the document with `rateLimit` added, so a GraphQL read can report what it spent; a CLI read - * ignores it and is priced at one point. + * The repository `gh` would act on in `cwd`: GH_REPO, else the remote `gh` would pick, else + * the remote the caller resolved the provider from, else the best-ranked GitHub remote. */ - const guardedRead = (input: { + const resolveRepository = Effect.fn("GitHubCli.resolveRepository")(function* (input: { readonly cwd: string; - readonly host: string; - readonly document: string; - readonly allowReserve: boolean; - readonly run: (document: string) => Effect.Effect; - }) => - Effect.gen(function* () { - const credential = yield* PinnedGitHubCredential; - const key = { provider: "github" as const, host: input.host }; - const guarded = Effect.gen(function* () { - const lease = yield* limits.check( - key, - input.allowReserve ? { allowPaused: true } : undefined, - ); - // A failed reading leaves the budget unknown; it never blocks the read itself. - yield* Cache.get( - budgetReading, - `${input.host}\0${yield* SourceControlRateLimit.CredentialScope}`, - ).pipe(Effect.ignore); - return yield* budget - .query( - input.host, - input.document, - input.allowReserve ? { allowReserve: true } : undefined, - ) - .pipe( - Effect.flatMap(input.run), - Effect.tap(() => limits.recordSuccess({ ...key, lease })), - Effect.tapError((error) => - error._tag === "GitHubCliRateLimitError" - ? limits.recordRateLimit({ ...key, lease }) - : Effect.void, - ), - ); - }); - return yield* guarded.pipe( - Effect.provideService( - SourceControlRateLimit.CredentialScope, - credential?.credentialFingerprint ?? (yield* SourceControlRateLimit.CredentialScope), - ), - Effect.catchTags({ - SourceControlRateLimitPausedError: (cause) => - Effect.fail( - new GitHubCliRateLimitError({ - command: "gh", - cwd: input.cwd, - retryAt: cause.retryAt, - cause, - }), - ), - }), - ); + readonly host?: string | undefined; + }) { + const envRepository = globalThis.process.env.GH_REPO?.trim(); + const defaultHost = ( + input.host ?? + globalThis.process.env.GH_HOST ?? + "github.com" + ).toLowerCase(); + if (envRepository) { + const locator = parseGitHubRepositorySelector(envRepository, defaultHost); + if (locator !== null) return locator; + } + const [remotes, resolved] = yield* Effect.all([ + gitRead(input.cwd, ["remote", "-v"]), + gitRead(input.cwd, ["config", "--get-regexp", "^remote\\..*\\.gh-resolved$"]), + ]).pipe(Effect.orElseSucceed(() => [null, null] as const)); + const remoteOutput = remotes?.exitCode === 0 ? remotes.stdout : ""; + const fetchRemotes = remoteOutput + .split("\n") + .map((line) => /^(\S+)\s+(\S+)\s+\(fetch\)$/u.exec(line.trim())) + .filter((match): match is RegExpExecArray => match !== null) + .map((match) => ({ + name: match[1]!, + url: match[2]!, + host: gitHubApiHostForRemote(match[2]!), + })); + const host = + input.host?.toLowerCase() ?? + fetchRemotes.find((remote) => remote.name === "origin" && remote.host !== null)?.host ?? + fetchRemotes.find((remote) => remote.host !== null)?.host ?? + defaultHost; + const selected = selectGitHubBaseRepository({ + remotes: remoteOutput, + resolved: resolved?.exitCode === 0 ? resolved.stdout : "", + host, }); - - const execute: GitHubCli["Service"]["execute"] = Effect.fn("GitHubCli.execute")( - function* (input) { - const [command, action] = input.args; - if ( - !( - (command === "pr" && (action === "list" || action === "view")) || - (command === "repo" && action === "view") - ) - ) - return yield* executeRaw(input); - const credential = yield* PinnedGitHubCredential; - if (credential !== null && !targetsVerifiedHost(input.args, credential.host)) - return yield* executeRaw(input); - return yield* guardedRead({ - cwd: input.cwd, - host: ( - credential?.host ?? - commandHosts(input.args).find((host) => host !== null) ?? - input.rateLimitHost ?? - input.env?.GH_HOST ?? - globalThis.process.env.GH_HOST ?? - "github.com" - ).toLowerCase(), - document: "query {}", - allowReserve: input.allowReserve ?? (yield* AllowGitHubReserve), - run: () => executeRaw(input), + if (selected !== null) return { host, ...selected }; + // gh's own order without its prompt: upstream, github, origin, then the first remote, among + // the ones on this host (an SSH alias counts as its API host). + const rank = (name: string) => ["upstream", "github", "origin"].indexOf(name.toLowerCase()); + const candidates = fetchRemotes + .filter((remote) => remote.host === host) + .toSorted((left, right) => { + const l = rank(left.name); + const r = rank(right.name); + return (l === -1 ? 99 : l) - (r === -1 ? 99 : r); }); - }, - ); + for (const remote of candidates) { + const [owner, name, ...rest] = normalizeGitRemoteUrl(remote.url).split("/").slice(1); + if (owner && name && rest.length === 0) return { host, owner, name }; + } + return yield* commandFailure( + input.cwd, + `No GitHub repository on ${host} was found among this checkout's git remotes.`, + ); + }); - const listPullRequestsWithCli = (input: { - readonly cwd: string; - readonly headSelector: string; - readonly state: PullRequestListState; - readonly limit: number; - readonly rateLimitHost?: string | undefined; - }) => - execute({ - cwd: input.cwd, - ...(input.rateLimitHost === undefined ? {} : { rateLimitHost: input.rateLimitHost }), - args: [ - "pr", - "list", - "--head", - input.headSelector, - "--state", - input.state, - "--limit", - String(input.limit), - "--json", - PULL_REQUEST_LIST_JSON_FIELDS, - ], - }).pipe( - Effect.flatMap((result) => { - const raw = result.stdout.trim(); - if (raw.length === 0) return Effect.succeed([]); - const decoded = decodeGitHubPullRequestListJson(raw); + const graphqlJson = ( + cwd: string, + input: GitHubApi.GitHubGraphQlInput, + decode: (raw: string) => Result.Result, + onDecodeFailure: (cause: unknown) => GitHubCliError, + ) => + api.graphql(input).pipe( + Effect.mapError((error) => fromGitHubApiError(cwd, error)), + Effect.flatMap((raw) => { + const decoded = decode(raw); return Result.isSuccess(decoded) ? Effect.succeed(decoded.success) - : Effect.fail( - new GitHubChangeRequestListDecodeError({ - command: "gh", - cwd: input.cwd, - cause: decoded.failure, - }), - ); + : Effect.fail(onDecodeFailure(decoded.failure)); }), ); - const git = (cwd: string, args: ReadonlyArray) => - process.run({ - operation: "GitHubCli.baseRepository", - command: "git", - args, - cwd, - allowNonZeroExit: true, - timeoutMs: 5_000, - }); + const rest = (cwd: string, input: GitHubApi.GitHubRestInput) => + api.rest(input).pipe(Effect.mapError((error) => fromGitHubApiError(cwd, error))); - /** The repository gh reads in `cwd`, or null when gh could pick it another way. */ - const resolveBaseRepository = (cwd: string, host: string) => - globalThis.process.env.GH_REPO - ? Effect.succeed(null) - : Effect.all([ - git(cwd, ["remote", "-v"]), - git(cwd, ["config", "--get-regexp", "^remote\\..*\\.gh-resolved$"]), - ]).pipe( - Effect.map(([remotes, resolved]) => - remotes.exitCode === 0 - ? selectGitHubBaseRepository({ - remotes: remotes.stdout, - resolved: resolved.exitCode === 0 ? resolved.stdout : "", - host, - }) - : null, - ), - Effect.orElseSucceed(() => null), - ); + /** Pull requests whose head branch is `headRefName` on one repository. */ + const readPullRequestsByHead = (input: { + readonly cwd: string; + readonly locator: GitHubRepositoryLocator; + readonly lookups: ReadonlyArray< + Pick + >; + readonly allowReserve: boolean; + readonly onDecodeFailure: (cause: unknown) => GitHubCliError; + }) => { + const query = buildPullRequestsByHeadQuery(input.lookups); + return graphqlJson( + input.cwd, + { + host: input.locator.host, + operation: "listPullRequestsByHead", + query: query.document, + variables: { owner: input.locator.owner, name: input.locator.name, ...query.variables }, + allowReserve: input.allowReserve, + // Up to 50 heads of 100 rows each. A default branch such as `main` can match a hundred + // fork pull requests, so the usual cap would cut the answer short. + maxResponseBytes: HEAD_LOOKUP_MAX_RESPONSE_BYTES, + }, + decodePullRequestsByHead, + input.onDecodeFailure, + ).pipe( + Effect.flatMap((decoded) => + decoded.data.repository === null + ? Effect.fail( + new GitHubPullRequestNotFoundError({ + command: "gh", + cwd: input.cwd, + cause: new Error("The repository could not be read."), + }), + ) + : Effect.succeed(decoded.data.repository), + ), + ); + }; const headResolver = RequestResolver.makeGrouped({ key: ({ request, context }) => - JSON.stringify([ + [ request.host, request.owner, request.name, - Context.getOrElse(context, PinnedGitHubCredential, () => null)?.credentialFingerprint ?? - null, + String(request.allowReserve), + Context.getOrElse(context, PinnedGitHubCredential, () => null)?.credentialFingerprint ?? "", Context.getOrElse(context, SourceControlRateLimit.CredentialScope, () => ""), - ]), + ].join("\0"), resolver: (entries) => { const [first] = entries; - const { cwd, host, owner, name } = first.request; - const query = buildPullRequestsByHeadQuery(entries.map((entry) => entry.request)); - const readCli = (entry: (typeof entries)[number]) => - listPullRequestsWithCli({ - cwd: entry.request.cwd, - headSelector: entry.request.headRefName, - state: entry.request.state, - limit: entry.request.limit, - rateLimitHost: entry.request.host, - }).pipe( - Effect.exit, - Effect.map((exit) => entry.completeUnsafe(exit)), - ); - return guardedRead({ + const { cwd, host, owner, name, allowReserve } = first.request; + return readPullRequestsByHead({ cwd, - host, - document: query.document, - allowReserve: false, - run: (document) => - executeRaw({ - cwd, - args: ["api", "graphql", "--hostname", host, "--input", "-"], - // Up to 50 heads of 100 rows each. A default branch such as `main` can match a - // hundred fork pull requests, so the 1 MB default would cut the answer short. - maxOutputBytes: HEAD_LOOKUP_MAX_OUTPUT_BYTES, - stdin: encodeGraphQlRequest({ - query: document, - variables: { owner, name, ...query.variables }, - }), - }).pipe(Effect.tap((result) => budget.observe(host, result.stdout))), + locator: { host, owner, name }, + lookups: entries.map((entry) => entry.request), + allowReserve, + onDecodeFailure: (cause) => + new GitHubChangeRequestListDecodeError({ command: "gh", cwd, cause }), }).pipe( - Effect.flatMap((result) => { - const decoded = decodePullRequestsByHead(result.stdout); - if (!Result.isSuccess(decoded)) { - return Effect.forEach(entries, readCli, { discard: true }); + Effect.map((aliases) => { + for (const [index, entry] of entries.entries()) { + const alias = aliases[`h${index}`]; + entry.completeUnsafe( + Exit.succeed(alias == null ? [] : decodeGitHubPullRequestEntries(alias.nodes)), + ); } - const aliases = decoded.success.data.repository; - return Effect.forEach( - entries, - (entry, index) => { - const alias = aliases[`h${index}`]; - if (alias == null) return readCli(entry); - entry.completeUnsafe(Exit.succeed(decodeGitHubPullRequestEntries(alias.nodes))); - return Effect.void; - }, - { discard: true }, - ); }), - // A document GitHub refused as a whole (a renamed repository, a field an older - // Enterprise host lacks) leaves each lookup to gh. A rate limit fails them all: - // asking one at a time would only spend what the pause is saving. - Effect.catchIf( - (error) => error._tag !== "GitHubCliRateLimitError", - () => Effect.forEach(entries, readCli, { discard: true }), - ), Effect.catchCause((cause) => Effect.sync(() => { for (const entry of entries) entry.completeUnsafe(Exit.failCause(cause)); @@ -907,174 +900,358 @@ export const make = Effect.gen(function* () { RequestResolver.batchN(HEAD_LOOKUPS_PER_DOCUMENT), ); - const listPullRequestsByHead: GitHubCli["Service"]["listPullRequestsByHead"] = Effect.fn( - "GitHubCli.listPullRequestsByHead", - )(function* (input) { - const host = input.rateLimitHost?.toLowerCase(); - const credential = yield* PinnedGitHubCredential; - // `owner:branch` selectors and other hosts keep gh's own handling. - const repository = - host === undefined || - input.headSelector.includes(":") || - (credential !== null && credential.host !== host) - ? null - : yield* resolveBaseRepository(input.cwd, host); - if (host === undefined || repository === null) { - return yield* listPullRequestsWithCli(input); - } - return yield* Effect.request( + const listByHead = Effect.fn("GitHubCli.listByHead")(function* (input: { + readonly cwd: string; + readonly headSelector: string; + readonly state: PullRequestListState; + readonly limit: number; + readonly rateLimitHost?: string | undefined; + readonly allowReserve: boolean; + }) { + const locator = yield* resolveRepository({ cwd: input.cwd, host: input.rateLimitHost }); + const limit = Math.min(Math.max(Math.trunc(input.limit), 1), 100); + // `owner:branch` names a fork's branch. GitHub filters on the branch name only, so the + // owner is matched on the rows it returns. + const ownerMatch = /^([^:/\s]+):(.+)$/u.exec(input.headSelector.trim()); + const headRefName = ownerMatch?.[2] ?? input.headSelector.trim(); + const rows = yield* Effect.request( new PullRequestsByHeadRead({ cwd: input.cwd, - host, - owner: repository.owner, - name: repository.name, - headRefName: input.headSelector, + ...locator, + headRefName, state: input.state, - limit: Math.min(Math.max(Math.trunc(input.limit), 1), 100), + limit: ownerMatch ? OWNER_HEAD_SCAN_LIMIT : limit, + allowReserve: input.allowReserve, }), headResolver, ); + if (!ownerMatch) return rows; + const headOwner = ownerMatch[1]!.toLowerCase(); + return rows + .filter((row) => row.headRepositoryOwnerLogin?.toLowerCase() === headOwner) + .slice(0, limit); }); - return GitHubCli.of({ - execute, - listPullRequestsByHead, - listOpenPullRequests: (input) => - execute({ + const toSummaries = (rows: ReadonlyArray) => + rows.map(pullRequestSummary); + + const readPullRequest = Effect.fn("GitHubCli.readPullRequest")(function* (input: { + readonly cwd: string; + readonly reference: string; + readonly rateLimitHost?: string | undefined; + }) { + const parsed = parsePullRequestReference(input.reference); + if (parsed.kind === "branch") { + // `gh pr view ` prefers an open pull request, then the newest of any state. + const [open] = yield* listByHead({ cwd: input.cwd, - ...(input.rateLimitHost === undefined ? {} : { rateLimitHost: input.rateLimitHost }), + headSelector: parsed.headSelector, + state: "open", + limit: 1, + rateLimitHost: input.rateLimitHost, allowReserve: true, - args: [ - "pr", - "list", - "--head", - input.headSelector, - "--state", - "open", - "--limit", - String(input.limit ?? 1), - "--json", - "number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,isCrossRepository,headRepository,headRepositoryOwner", - ], - }).pipe( - Effect.map((result) => result.stdout.trim()), - Effect.flatMap((raw) => - raw.length === 0 - ? Effect.succeed([]) - : Effect.sync(() => decodeGitHubPullRequestListJson(raw)).pipe( - Effect.flatMap((decoded) => { - if (!Result.isSuccess(decoded)) { - return Effect.fail( - new GitHubPullRequestListDecodeError({ - command: "gh", - cwd: input.cwd, - cause: decoded.failure, - }), - ); - } - - return Effect.succeed(decoded.success.map(pullRequestSummary)); - }), - ), - ), - ), - getPullRequest: (input) => - execute({ - cwd: input.cwd, - ...(input.rateLimitHost === undefined ? {} : { rateLimitHost: input.rateLimitHost }), + }); + const found = + open ?? + (yield* listByHead({ + cwd: input.cwd, + headSelector: parsed.headSelector, + state: "all", + limit: 1, + rateLimitHost: input.rateLimitHost, + allowReserve: true, + }))[0]; + if (found === undefined) { + return yield* new GitHubPullRequestNotFoundError({ + command: "gh", + cwd: input.cwd, + cause: new Error("No pull request has this head branch."), + }); + } + return found; + } + const locator = + parsed.kind === "url" + ? parsed.locator + : yield* resolveRepository({ cwd: input.cwd, host: input.rateLimitHost }); + const decodeFailure = (cause: unknown) => + new GitHubPullRequestDecodeError({ command: "gh", cwd: input.cwd, cause }); + const decoded = yield* graphqlJson( + input.cwd, + { + host: locator.host, + operation: "getPullRequest", + query: PULL_REQUEST_BY_NUMBER_QUERY, + variables: { owner: locator.owner, name: locator.name, number: parsed.number }, allowReserve: true, - args: [ - "pr", - "view", - input.reference, - "--json", - "number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner", - ], - }).pipe( - Effect.map((result) => result.stdout.trim()), - Effect.flatMap((raw) => - Effect.sync(() => decodeGitHubPullRequestJson(raw)).pipe( - Effect.flatMap((decoded) => { - if (!Result.isSuccess(decoded)) { - return Effect.fail( - new GitHubPullRequestDecodeError({ - command: "gh", - cwd: input.cwd, - cause: decoded.failure, - }), - ); - } - - return Effect.succeed(pullRequestSummary(decoded.success)); - }), - ), - ), - ), - getRepositoryCloneUrls: (input) => - execute({ - cwd: input.cwd, - args: ["repo", "view", input.repository, "--json", "nameWithOwner,url,sshUrl"], - }).pipe( - Effect.map((result) => result.stdout.trim()), - Effect.flatMap((raw) => - decodeRawGitHubRepositoryCloneUrls(raw).pipe( - Effect.mapError( - (cause) => - new GitHubRepositoryDecodeError({ - command: "gh", - cwd: input.cwd, - cause, - }), - ), - ), - ), - Effect.map(normalizeRepositoryCloneUrls), - ), - createRepository: (input) => - execute({ + }, + decodePullRequestByNumber, + decodeFailure, + ); + const node = decoded.data.repository?.pullRequest; + if (node == null) { + return yield* new GitHubPullRequestNotFoundError({ + command: "gh", cwd: input.cwd, - args: ["repo", "create", input.repository, `--${input.visibility}`], - }).pipe( - Effect.map((result) => - deriveRepositoryCloneUrlsFromCreateOutput(result.stdout, input.repository), - ), - ), - createPullRequest: (input) => - execute({ + cause: new Error("The pull request does not exist."), + }); + } + const [record] = decodeGitHubPullRequestEntries([node]); + if (record === undefined) return yield* decodeFailure(new Error("Malformed pull request.")); + return record; + }); + + const readRepository = Effect.fn("GitHubCli.readRepository")(function* ( + cwd: string, + locator: GitHubRepositoryLocator, + ) { + const response = yield* rest(cwd, { + host: locator.host, + operation: "getRepository", + path: `repos/${encodeURIComponent(locator.owner)}/${encodeURIComponent(locator.name)}`, + allowReserve: true, + }); + const decoded = decodeRawRepository(response.body); + if (Result.isFailure(decoded)) { + return yield* new GitHubRepositoryDecodeError({ + command: "gh", + cwd, + cause: decoded.failure, + }); + } + return decoded.success; + }); + + const readViewerLogin = Effect.fn("GitHubCli.readViewerLogin")(function* ( + cwd: string, + host: string, + ) { + const response = yield* rest(cwd, { host, operation: "getViewer", path: "user" }); + const decoded = decodeViewerLogin(response.body); + if (Result.isFailure(decoded)) { + return yield* new GitHubCliCommandError({ command: "gh", cwd, cause: decoded.failure }); + } + return decoded.success.login; + }); + + const gitFailure = (cwd: string) => (cause: unknown) => + new GitHubCliCommandError({ command: "gh", cwd, cause }); + + const runGit = (cwd: string, operation: string, args: ReadonlyArray) => + git.execute({ operation: `GitHubCli.checkoutPullRequest.${operation}`, cwd, args }); + + /** + * `gh pr checkout` in plain git: the head branch is fetched from the remote that holds it (a + * fork gets a remote of its own), checked out under the name gh would give it, and set to + * track the head. A head branch that is gone is read from the base's `refs/pull//head`. + * An existing branch fast-forwards, or with `force` is reset to the pull request. + */ + const checkoutPullRequest: GitHubCli["Service"]["checkoutPullRequest"] = Effect.fn( + "GitHubCli.checkoutPullRequest", + )(function* (input) { + const reference = parsePullRequestReference(input.reference); + const pullRequest = yield* readPullRequest({ cwd: input.cwd, reference: input.reference }); + const base = + reference.kind === "url" ? reference.locator : yield* resolveRepository({ cwd: input.cwd }); + const baseNameWithOwner = `${base.owner}/${base.name}`.toLowerCase(); + const headNameWithOwner = pullRequest.headRepositoryNameWithOwner ?? null; + const isCrossRepository = + pullRequest.isCrossRepository ?? + (headNameWithOwner !== null && headNameWithOwner.toLowerCase() !== baseNameWithOwner); + const headOwner = + pullRequest.headRepositoryOwnerLogin ?? headNameWithOwner?.split("/")[0] ?? null; + + const remotes = yield* gitRead(input.cwd, ["remote", "-v"]).pipe( + Effect.map((result) => (result.exitCode === 0 ? result.stdout : "")), + Effect.mapError(gitFailure(input.cwd)), + ); + const remoteFor = (nameWithOwner: string) => + remotes + .split("\n") + .map((line) => /^(\S+)\s+(\S+)\s+\(fetch\)$/u.exec(line.trim())) + .find( + (match) => + match !== null && + normalizeGitRemoteUrl(match[2]!).split("/").slice(1).join("/") === + nameWithOwner.toLowerCase(), + )?.[1] ?? null; + const baseRemote = Effect.suspend(() => { + const known = remoteFor(baseNameWithOwner); + return known === null ? git.resolvePrimaryRemoteName(input.cwd) : Effect.succeed(known); + }); + const defaultBranch = isCrossRepository + ? yield* readRepository(input.cwd, base).pipe( + Effect.map((repository) => repository.default_branch ?? null), + Effect.orElseSucceed(() => null), + ) + : null; + const localBranch = pullRequestCheckoutBranchName({ + headRefName: pullRequest.headRefName, + headOwner, + isCrossRepository, + defaultBranch, + }); + + /** The remote the head branch lives on; a fork the checkout does not know yet is added. */ + const headRemote = Effect.gen(function* () { + if (!isCrossRepository) return yield* baseRemote; + if (headNameWithOwner === null) return yield* commandFailure(input.cwd, "The fork is gone."); + const known = remoteFor(headNameWithOwner); + if (known !== null) return known; + const [owner, name] = headNameWithOwner.split("/"); + const fork = yield* readRepository(input.cwd, { + host: base.host, + owner: owner!, + name: name!, + }); + const originUrl = yield* git.readConfigValue(input.cwd, "remote.origin.url"); + return yield* git.ensureRemote({ cwd: input.cwd, - args: [ - "pr", - "create", - "--base", - input.baseBranch, - "--head", - input.headSelector, - "--title", - input.title, - "--body-file", - input.bodyFile, - ], - }).pipe(Effect.asVoid), - getDefaultBranch: (input) => - execute({ + preferredName: headOwner ?? "fork", + url: originUrl !== null && isSshRemoteUrl(originUrl) ? fork.ssh_url : fork.html_url, + }); + }); + + const exists = (yield* git + .listLocalBranchNames(input.cwd) + .pipe(Effect.mapError(gitFailure(input.cwd)))).includes(localBranch); + + // The commit to check out, fetched from the head branch where it still exists. + const target = yield* Effect.gen(function* () { + const remoteName = yield* headRemote; + yield* git.fetchRemoteTrackingBranch({ cwd: input.cwd, - ...(input.rateLimitHost === undefined ? {} : { rateLimitHost: input.rateLimitHost }), - args: ["repo", "view", "--json", "defaultBranchRef", "--jq", ".defaultBranchRef.name"], - }).pipe( - Effect.map((value) => { - const trimmed = value.stdout.trim(); - return trimmed.length > 0 ? trimmed : null; + remoteName, + remoteBranch: pullRequest.headRefName, + }); + return { + ref: `refs/remotes/${remoteName}/${pullRequest.headRefName}`, + upstream: { remoteName, remoteBranch: pullRequest.headRefName }, + }; + }).pipe( + Effect.catch(() => + Effect.gen(function* () { + yield* runGit(input.cwd, "fetchPullRef", [ + "fetch", + "--quiet", + "--no-tags", + yield* baseRemote, + `refs/pull/${pullRequest.number}/head`, + ]); + const { commitSha } = yield* git.resolveCommit({ + cwd: input.cwd, + revision: "FETCH_HEAD", + }); + return { ref: commitSha, upstream: null }; }), ), - checkoutPullRequest: (input) => - execute({ + Effect.mapError(gitFailure(input.cwd)), + ); + + yield* Effect.gen(function* () { + if (!exists) yield* runGit(input.cwd, "branch", ["branch", localBranch, target.ref]); + if (target.upstream !== null) { + yield* git.setBranchUpstream({ cwd: input.cwd, branch: localBranch, ...target.upstream }); + } + yield* Effect.scoped(git.switchRef({ cwd: input.cwd, refName: localBranch })); + if (exists) { + yield* runGit( + input.cwd, + "sync", + input.force === true + ? ["reset", "--hard", "--quiet", target.ref] + : ["merge", "--ff-only", "--quiet", target.ref], + ); + } + }).pipe(Effect.mapError(gitFailure(input.cwd))); + }); + + return GitHubCli.of({ + execute, + listPullRequestsByHead: (input) => + AllowGitHubReserve.pipe( + Effect.flatMap((allowReserve) => listByHead({ ...input, allowReserve })), + ), + listOpenPullRequests: (input) => + listByHead({ cwd: input.cwd, - args: ["pr", "checkout", input.reference, ...(input.force ? ["--force"] : [])], - }).pipe(Effect.asVoid), + headSelector: input.headSelector, + state: "open", + limit: input.limit ?? 1, + rateLimitHost: input.rateLimitHost, + allowReserve: true, + }).pipe(Effect.map(toSummaries)), + getPullRequest: (input) => readPullRequest(input).pipe(Effect.map(pullRequestSummary)), + getRepositoryCloneUrls: (input) => + Effect.gen(function* () { + const fallbackHost = (yield* resolveRepository({ cwd: input.cwd }).pipe( + Effect.map((locator) => locator.host), + Effect.orElseSucceed(() => globalThis.process.env.GH_HOST ?? "github.com"), + )).toLowerCase(); + const locator = parseGitHubRepositorySelector(input.repository, fallbackHost); + if (locator === null) { + return yield* commandFailure(input.cwd, "Repositories are named owner/name."); + } + return repositoryCloneUrls(yield* readRepository(input.cwd, locator)); + }), + createRepository: (input) => + Effect.gen(function* () { + const locator = parseGitHubRepositorySelector( + input.repository, + (globalThis.process.env.GH_HOST ?? "github.com").toLowerCase(), + ); + const viewer = locator === null ? null : yield* readViewerLogin(input.cwd, locator.host); + const owner = locator?.owner ?? viewer; + const name = locator?.name ?? input.repository.trim(); + const host = locator?.host ?? "github.com"; + const isViewer = viewer !== null && owner?.toLowerCase() === viewer.toLowerCase(); + const response = yield* rest(input.cwd, { + host, + operation: "createRepository", + method: "POST", + path: + isViewer || owner === null ? "user/repos" : `orgs/${encodeURIComponent(owner)}/repos`, + body: { name, private: input.visibility === "private" }, + }); + const decoded = decodeRawRepository(response.body); + if (Result.isFailure(decoded)) { + return yield* new GitHubRepositoryDecodeError({ + command: "gh", + cwd: input.cwd, + cause: decoded.failure, + }); + } + return repositoryCloneUrls(decoded.success); + }), + createPullRequest: (input) => + Effect.gen(function* () { + const locator = yield* resolveRepository({ cwd: input.cwd }); + const body = yield* fileSystem + .readFileString(input.bodyFile) + .pipe(Effect.mapError(gitFailure(input.cwd))); + yield* rest(input.cwd, { + host: locator.host, + operation: "createPullRequest", + method: "POST", + path: `repos/${encodeURIComponent(locator.owner)}/${encodeURIComponent(locator.name)}/pulls`, + // `owner:branch` is how the REST API takes a fork's head, the same as `gh --head`. + body: { base: input.baseBranch, head: input.headSelector, title: input.title, body }, + }); + }), + getDefaultBranch: (input) => + Effect.gen(function* () { + const locator = yield* resolveRepository({ cwd: input.cwd, host: input.rateLimitHost }); + const repository = yield* readRepository(input.cwd, locator); + const branch = repository.default_branch?.trim() ?? ""; + return branch.length > 0 ? branch : null; + }), + checkoutPullRequest, }); }); export const layer = Layer.effect(GitHubCli, make).pipe( + Layer.provideMerge(GitHubApi.layer), + Layer.provideMerge(GitHubCredentials.layer), Layer.provideMerge(GitHubGraphQlBudget.layer), Layer.provideMerge(SourceControlRateLimit.layer), ); From 6b461041b72cb4e23132e189ddf98773eeb6291b Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Mon, 5 Oct 2026 17:54:03 -0700 Subject: [PATCH 2/9] feat(server): media fetch, link previews and discovery use the GitHub API credential GitHubMediaFetch takes its token from GitHubCredentials and drops its own cache. resolveLink reads the issue over REST. GitHub discovery reports an environment token as signed in through GET /user, and keeps gh auth status otherwise. Co-Authored-By: Claude Opus 5.5 (1M context) --- apps/server/src/assets/GitHubMediaFetch.ts | 51 ++--------- .../GitHubSourceControlProvider.ts | 85 ++++++++++++++++--- .../SourceControlProviderRegistry.ts | 3 +- 3 files changed, 85 insertions(+), 54 deletions(-) diff --git a/apps/server/src/assets/GitHubMediaFetch.ts b/apps/server/src/assets/GitHubMediaFetch.ts index 39ed14f8b609..7b26c3e69acd 100644 --- a/apps/server/src/assets/GitHubMediaFetch.ts +++ b/apps/server/src/assets/GitHubMediaFetch.ts @@ -12,7 +12,7 @@ import { type HttpClientResponse, } from "effect/http"; -import * as GitHubCli from "../sourceControl/GitHubCli.ts"; +import * as GitHubCredentials from "../sourceControl/GitHubCredentials.ts"; /** * Exactly the hosts the credential is for. Everything a redirect leads to — the presigned @@ -37,8 +37,6 @@ const isCredentialedHost = (url: string) => { const MAX_REDIRECTS = 3; /** Following the redirect here, rather than in `fetch`, is what keeps the token on GitHub. */ const MANUAL_REDIRECT: RequestInit = { redirect: "manual" }; -const TOKEN_CACHE_TTL_MS = 5 * 60_000; -const TOKEN_CACHE_MAX_ENTRIES = 32; /** Passed through so a seek in a long video costs one upstream range request, not a full download. */ const FORWARDED_REQUEST_HEADERS = ["range", "if-range"] as const; const FORWARDED_RESPONSE_HEADERS = [ @@ -57,45 +55,14 @@ const SVG_CONTENT_TYPE = "image/svg+xml"; const SVG_CONTENT_SECURITY_POLICY = "default-src 'none'; style-src 'unsafe-inline'; sandbox"; /** - * A media request per image and one per video seek, each of which would otherwise spawn `gh`. - * The token is what `gh auth token` would print again on the next call, and it is held no longer - * than a signed asset URL lives. + * The github.com credential, or null without one: a public asset still loads, and a private one + * fails the way it does in a browser that is not signed in. */ -const tokenCache = new Map(); - -const githubToken = Effect.fn("GitHubMediaFetch.githubToken")(function* (input: { - readonly cwd: string; - readonly host: string; -}) { - // `gh` stores a token per host, not per repository, so the directory it runs in is not part - // of the answer and must not fragment the cache a client could otherwise churn. This route - // pins no credential; if it ever does, the pin belongs in this key. - const key = input.host; - const now = yield* Clock.currentTimeMillis; - const cached = tokenCache.get(key); - if (cached !== undefined && now - cached.at < TOKEN_CACHE_TTL_MS) return cached.token; - const github = yield* GitHubCli.GitHubCli; - // No credential is a normal state: a public asset still loads, and a private one fails the way - // it does in a browser that is not signed in. - const token = yield* github - .execute({ - cwd: input.cwd, - args: ["auth", "token", "--hostname", input.host], - env: { GH_DEBUG: "" }, - }) - .pipe( - Effect.map((output) => output.stdout.trim()), - Effect.orElseSucceed(() => ""), - ); - // A login or recovered CLI failure must take effect on the next media request. - if (token.length === 0) return null; - if (tokenCache.size >= TOKEN_CACHE_MAX_ENTRIES) { - tokenCache.delete(tokenCache.keys().next().value!); - } - const redacted = Redacted.make(token); - tokenCache.set(key, { at: now, token: redacted }); - return redacted; -}); +const githubToken = GitHubCredentials.GitHubCredentials.pipe( + Effect.flatMap((credentials) => credentials.get("github.com")), + Effect.map((credential) => credential.token), + Effect.orElseSucceed(() => null), +); /** * Follows GitHub's redirect to the signed object itself, and never carries the credential off @@ -146,7 +113,7 @@ export const githubMediaResponse = Effect.fn("GitHubMediaFetch.githubMediaRespon requestHeaders: Record, ) { // Both media hosts are served by github.com's account, which is the host `gh` stores it under. - const token = yield* githubToken({ cwd: asset.cwd, host: "github.com" }); + const token = yield* githubToken; const forwarded: Record = {}; for (const name of FORWARDED_REQUEST_HEADERS) { const value = requestHeaders[name]; diff --git a/apps/server/src/sourceControl/GitHubSourceControlProvider.ts b/apps/server/src/sourceControl/GitHubSourceControlProvider.ts index 905146d5a733..d9c4b9388642 100644 --- a/apps/server/src/sourceControl/GitHubSourceControlProvider.ts +++ b/apps/server/src/sourceControl/GitHubSourceControlProvider.ts @@ -2,22 +2,31 @@ import * as Schema from "effect/Schema"; import * as DateTime from "effect/DateTime"; import * as Effect from "effect/Effect"; import * as Option from "effect/Option"; -import { SourceControlProviderError, type ChangeRequest } from "@t3tools/contracts"; +import * as Result from "effect/Result"; +import { + SourceControlProviderError, + type ChangeRequest, + type SourceControlProviderDiscoveryItem, +} from "@t3tools/contracts"; +import * as GitHubApi from "./GitHubApi.ts"; import * as GitHubCli from "./GitHubCli.ts"; import { findAuthenticatedGitHubAccount, parseGitHubAuthStatus } from "./gitHubAuthStatus.ts"; import * as SourceControlProvider from "./SourceControlProvider.ts"; import { combinedAuthOutput, firstSafeAuthLine, + probeSourceControlProvider, providerAuth, type SourceControlAuthProbeInput, type SourceControlCliDiscoverySpec, + type SourceControlManagedCliDiscoverySpec, } from "./SourceControlProviderDiscovery.ts"; +import * as VcsProcess from "../vcs/VcsProcess.ts"; const decodeLinkSubject = Schema.decodeUnknownEffect( Schema.fromJsonString( - Schema.Struct({ title: Schema.String, body: Schema.NullOr(Schema.String) }), + Schema.Struct({ title: Schema.String, body: Schema.optional(Schema.NullOr(Schema.String)) }), ), ); @@ -111,8 +120,62 @@ export const discovery = { "Install the GitHub command-line tool (`gh`) via https://cli.github.com/ or your package manager (for example `brew install gh`).", } satisfies SourceControlCliDiscoverySpec; +const decodeViewer = Schema.decodeUnknownOption( + Schema.fromJsonString(Schema.Struct({ login: Schema.String })), +); + +/** The environment variable gh would take a github.com token from, if one is set. */ +function environmentTokenVariable(): string | null { + return ["GH_TOKEN", "GITHUB_TOKEN"].find((name) => globalThis.process.env[name]?.trim()) ?? null; +} + +/** + * GitHub is usable with a token from the environment or with `gh` to hand one over. An + * environment token is checked against the API, since `gh auth status` may not know it. + */ +export const makeDiscovery = Effect.gen(function* () { + const api = yield* GitHubApi.GitHubApi; + const process = yield* VcsProcess.VcsProcess; + return { + type: "managed-cli", + kind: discovery.kind, + label: discovery.label, + installHint: discovery.installHint, + probe: Effect.fn("GitHubSourceControlProvider.discovery")(function* (cwd: string) { + const cli = yield* probeSourceControlProvider({ cwd, process, spec: discovery }); + const variable = environmentTokenVariable(); + if (variable === null) return cli; + const viewer = yield* api + .rest({ host: "github.com", operation: "discovery", path: "user" }) + .pipe(Effect.result); + const login = Result.isSuccess(viewer) + ? Option.getOrUndefined(decodeViewer(viewer.success.body))?.login + : undefined; + return { + ...cli, + status: "available" as const, + auth: + login !== undefined + ? providerAuth({ + status: "authenticated", + account: login, + host: "github.com", + detail: `Using the token in ${variable} from the server environment.`, + }) + : providerAuth({ + status: "unauthenticated", + host: "github.com", + detail: `GitHub refused the token in ${variable}. Replace it, or unset it to use \`gh auth login\`.`, + }), + } satisfies SourceControlProviderDiscoveryItem; + }), + refineUnknownRemote: () => Effect.succeed(null), + } satisfies SourceControlManagedCliDiscoverySpec; +}); + export const make = Effect.gen(function* () { const github = yield* GitHubCli.GitHubCli; + const api = yield* GitHubApi.GitHubApi; const listChangeRequests: SourceControlProvider.SourceControlProvider["Service"]["listChangeRequests"] = (input) => { @@ -188,15 +251,15 @@ export const make = Effect.gen(function* () { input: { readonly cwd: string; readonly url: URL }, endpoint: string, ) { - const result = yield* github - .execute({ - cwd: input.cwd, - args: ["api", "--hostname", input.url.host, endpoint, "--jq", "{title, body}"], - env: { GH_PROMPT_DISABLED: "1" }, - timeoutMs: 3_000, - maxOutputBytes: 32_000, + const result = yield* api + .rest({ + host: input.url.host, + operation: "resolveLink", + path: endpoint, + maxResponseBytes: 1_000_000, }) .pipe( + Effect.timeout("3 seconds"), Effect.mapError( (cause) => new SourceControlProviderError({ @@ -208,7 +271,7 @@ export const make = Effect.gen(function* () { }), ), ); - const subject = yield* decodeLinkSubject(result.stdout).pipe( + const subject = yield* decodeLinkSubject(result.body).pipe( Effect.mapError( (cause) => new SourceControlProviderError({ @@ -220,7 +283,7 @@ export const make = Effect.gen(function* () { }), ), ); - return { title: subject.title, body: subject.body }; + return { title: subject.title, body: subject.body ?? null }; }); return SourceControlProvider.SourceControlProvider.of({ diff --git a/apps/server/src/sourceControl/SourceControlProviderRegistry.ts b/apps/server/src/sourceControl/SourceControlProviderRegistry.ts index a497c852bcd6..7a626b13f50f 100644 --- a/apps/server/src/sourceControl/SourceControlProviderRegistry.ts +++ b/apps/server/src/sourceControl/SourceControlProviderRegistry.ts @@ -305,6 +305,7 @@ export const makeWithProviders = Effect.fn("makeSourceControlProviderRegistryWit export const make = Effect.gen(function* () { const github = yield* GitHubSourceControlProvider.make; + const githubDiscovery = yield* GitHubSourceControlProvider.makeDiscovery; const gitlab = yield* GitLabSourceControlProvider.make; const forgejo = yield* ForgejoSourceControlProvider.make; const forgejoDiscovery = yield* ForgejoSourceControlProvider.makeDiscovery; @@ -315,7 +316,7 @@ export const make = Effect.gen(function* () { { kind: "github", provider: github, - discovery: GitHubSourceControlProvider.discovery, + discovery: githubDiscovery, }, { kind: "gitlab", From 243ccd28fd2c801f11cdb465753232ec6ebf4df8 Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Mon, 5 Oct 2026 18:26:07 -0700 Subject: [PATCH 3/9] test(server): GitHubCli tests run against a mocked GitHubApi and git driver Covers repository resolution from remotes (including an SSH alias read through github.com), batched head lookups, owner:branch selectors, error mapping, pr view by number and URL, cross-repository pr create, org repo create, and pr checkout for same-repository and fork pull requests. Co-Authored-By: Claude Opus 5.5 (1M context) --- apps/server/src/assets/AssetAccess.test.ts | 33 +- apps/server/src/git/GitManager.test.ts | 5 +- .../src/sourceControl/GitHubCli.test.ts | 1240 ++++++----------- apps/server/src/sourceControl/GitHubCli.ts | 9 +- .../GitHubSourceControlProvider.test.ts | 104 +- .../SourceControlDiscovery.test.ts | 2 + .../SourceControlProviderRegistry.test.ts | 14 +- 7 files changed, 489 insertions(+), 918 deletions(-) diff --git a/apps/server/src/assets/AssetAccess.test.ts b/apps/server/src/assets/AssetAccess.test.ts index b3381befa3ba..35dc647c3045 100644 --- a/apps/server/src/assets/AssetAccess.test.ts +++ b/apps/server/src/assets/AssetAccess.test.ts @@ -16,12 +16,12 @@ import * as DateTime from "effect/DateTime"; import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; +import * as Redacted from "effect/Redacted"; import * as Path from "effect/Path"; import * as PlatformError from "effect/PlatformError"; import * as Schema from "effect/Schema"; import * as TestClock from "effect/testing/TestClock"; import { HttpClient, HttpClientResponse, HttpServerResponse } from "effect/http"; -import { ChildProcessSpawner } from "effect/process"; import { vi } from "vite-plus/test"; import * as ServerSecretStore from "../auth/ServerSecretStore.ts"; @@ -35,7 +35,7 @@ import { ASSET_ROUTE_PREFIX, issueAssetUrl, resolveAsset } from "./AssetAccess.t import * as NativeAppIconResolver from "./NativeAppIconResolver.ts"; import { openMediaFile } from "./MediaFile.ts"; import { symlinksSupported } from "@t3tools/shared/testing/symlinks"; -import * as GitHubCli from "../sourceControl/GitHubCli.ts"; +import * as GitHubCredentials from "../sourceControl/GitHubCredentials.ts"; import { githubMediaResponse } from "./GitHubMediaFetch.ts"; vi.mock("node:fs/promises", async (importOriginal) => { @@ -126,7 +126,7 @@ const layerTest = Layer.mergeAll( ).pipe(Layer.provideMerge(NodeServices.layer)); describe("AssetAccess", () => { - it.effect("loads private media immediately after login and reuses the found credential", () => { + it.effect("loads private media immediately after login with the GitHub credential", () => { let lookups = 0; const authorizations: Array = []; return Effect.gen(function* () { @@ -138,19 +138,26 @@ describe("AssetAccess", () => { expect((yield* githubMediaResponse(asset, {})).status).toBe(404); expect((yield* githubMediaResponse(asset, {})).status).toBe(200); expect((yield* githubMediaResponse(asset, {})).status).toBe(200); - expect(lookups).toBe(2); + // Caching the token is GitHubCredentials' job; this asks it every time. + expect(lookups).toBe(3); expect(authorizations).toEqual([undefined, "Bearer signed-in", "Bearer signed-in"]); }).pipe( Effect.provide( - Layer.mock(GitHubCli.GitHubCli)({ - execute: () => - Effect.sync(() => ({ - exitCode: ChildProcessSpawner.ExitCode(0), - stdout: ++lookups === 1 ? "" : "signed-in", - stderr: "", - stdoutTruncated: false, - stderrTruncated: false, - })), + Layer.mock(GitHubCredentials.GitHubCredentials)({ + get: (host) => + ++lookups === 1 + ? Effect.fail( + new GitHubCredentials.GitHubCredentialUnavailableError({ + host, + reason: "unauthenticated", + }), + ) + : Effect.succeed({ + host, + token: Redacted.make("signed-in"), + source: "gh" as const, + fingerprint: "fingerprint", + }), }), ), Effect.provideService( diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index 43eaa08de53a..e1d7bceb6418 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -36,6 +36,7 @@ import { ThreadId, } from "@t3tools/contracts"; import * as DateTime from "effect/DateTime"; +import * as GitHubApi from "../sourceControl/GitHubApi.ts"; import * as GitHubCli from "../sourceControl/GitHubCli.ts"; import { decodeGitHubPullRequestListJson } from "../sourceControl/gitHubPullRequests.ts"; import * as GitLabCli from "../sourceControl/GitLabCli.ts"; @@ -725,6 +726,7 @@ function makeManager(input?: { }), ), Effect.provide(Layer.succeed(GitHubCli.GitHubCli, gitHubCli)), + Effect.provide(Layer.mock(GitHubApi.GitHubApi)({})), ), ); @@ -2660,8 +2662,7 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { provider: "github", providerOperation: "listChangeRequests", providerCommand: "gh", - errorDetail: - "GitHub API rate limit exceeded. For the GraphQL quota and reset time, run `gh api graphql -f query='{rateLimit{remaining resetAt}}'`; `gh api rate_limit` reports REST.", + errorDetail: "GitHub API rate limit exceeded. Requests resume when the limit resets.", }); const loggedText = [ warning?.message ?? "", diff --git a/apps/server/src/sourceControl/GitHubCli.test.ts b/apps/server/src/sourceControl/GitHubCli.test.ts index 88c877605381..589d2cc5e48c 100644 --- a/apps/server/src/sourceControl/GitHubCli.test.ts +++ b/apps/server/src/sourceControl/GitHubCli.test.ts @@ -1,157 +1,108 @@ -import { assert, it, afterEach, describe, expect, vi } from "@effect/vitest"; -import * as Cache from "effect/Cache"; +import { assert, it, describe } from "@effect/vitest"; +import * as NodeServices from "@effect/platform-node/NodeServices"; import * as TestClock from "effect/testing/TestClock"; -import * as Clock from "effect/Clock"; -import * as DateTime from "effect/DateTime"; import * as Effect from "effect/Effect"; import * as Fiber from "effect/Fiber"; +import * as FileSystem from "effect/FileSystem"; 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/process"; -import { VcsProcessExitError, VcsProcessSpawnError } from "@t3tools/contracts"; +import * as GitVcsDriver from "../vcs/GitVcsDriver.ts"; import * as VcsProcess from "../vcs/VcsProcess.ts"; +import * as GitHubApi from "./GitHubApi.ts"; import * as GitHubCli from "./GitHubCli.ts"; -import * as GitHubGraphQlBudget from "./githubGraphQlBudget.ts"; -import * as SourceControlRateLimit from "./SourceControlRateLimit.ts"; +import * as GitHubCredentials from "./GitHubCredentials.ts"; -const encodeGitHubCliError = Schema.encodeEffect(Schema.fromJsonString(GitHubCli.GitHubCliError)); +const encodeJson = Schema.encodeSync(Schema.fromJsonString(Schema.Unknown)); -const processOutput = (stdout: string): VcsProcess.VcsProcessOutput => ({ - exitCode: ChildProcessSpawner.ExitCode(0), +const processOutput = (stdout: string, exitCode = 0): VcsProcess.VcsProcessOutput => ({ + exitCode: ChildProcessSpawner.ExitCode(exitCode), stdout, stderr: "", stdoutTruncated: false, stderrTruncated: false, }); -const quotaOutput = (remaining = 5000, resetAt = "2099-01-01T00:00:00Z") => - processOutput( - JSON.stringify({ data: { rateLimit: { cost: 1, limit: 5000, remaining, resetAt } } }), - ); - -const isBudgetReading = (input: VcsProcess.VcsProcessInput) => - input.args[0] === "api" && - input.args[1] === "graphql" && - input.args.at(-1)?.includes("rateLimit"); +const remotesOutput = (...entries: ReadonlyArray) => + entries + .flatMap(([name, url]) => [`${name}\t${url} (fetch)`, `${name}\t${url} (push)`]) + .join("\n"); -const mockRun = vi.fn(); - -// Budget readings are answered here, so `mockRun` sees only the commands under test. -const layer = GitHubCli.layer.pipe( - Layer.provide( - Layer.mock(VcsProcess.VcsProcess)({ - run: (input) => (isBudgetReading(input) ? Effect.succeed(quotaOutput()) : mockRun(input)), - }), - ), -); +const restResponse = (body: unknown, status = 200): GitHubApi.GitHubRestResponse => ({ + status, + headers: {}, + body: body === undefined ? "" : encodeJson(body), + truncated: false, +}); -afterEach(() => { - mockRun.mockReset(); +const node = (number: number, headRefName: string, owner = "acme") => ({ + number, + title: `PR ${number}`, + url: `https://github.com/acme/web/pull/${number}`, + baseRefName: "main", + headRefName, + state: "OPEN", + isCrossRepository: owner !== "acme", + updatedAt: "2026-01-02T00:00:00Z", + headRepository: { name: "web", nameWithOwner: `${owner}/web` }, + headRepositoryOwner: { login: owner }, }); -it.effect("reads the GraphQL budget once per window, preserves the reserve, and resumes", () => - Effect.gen(function* () { - let readings = 0; - const commands: string[] = []; - let remaining = 501; - let resetAt = DateTime.formatIso( - DateTime.makeUnsafe((yield* Clock.currentTimeMillis) + 60_000), - ); - const gh = yield* GitHubCli.make.pipe( - Effect.provideService(VcsProcess.VcsProcess, { - run: (input) => - Effect.sync(() => { - if (isBudgetReading(input)) { - readings++; - assert.strictEqual(input.args[3], "enterprise.test"); - return quotaOutput(remaining, resetAt); - } - commands.push(input.args.slice(0, 2).join(" ")); - return processOutput("[]"); - }), - }), - ); - const read = (command: string) => - gh.execute({ - cwd: "/repo", - args: - command === "repo" - ? ["repo", "view", "enterprise.test/acme/web", "--json", "name"] - : ["pr", command, "--repo=enterprise.test/acme/web", "--json", "number"], +/** + * A GitHubCli over a mocked GitHubApi, git driver and process. `remotes` is what + * `git remote -v` prints; `git` records every driver call. + */ +function harness(input: { + readonly remotes: string; + readonly api: Partial; + readonly localBranches?: ReadonlyArray; +}) { + const git: Array = []; + const record = + (name: string, value: A) => + (args: unknown) => + Effect.sync(() => { + git.push([name, args]); + return value; }); - yield* read("list"); - const failure = yield* read("view").pipe(Effect.flip); - assert.strictEqual(failure._tag, "GitHubCliRateLimitError"); - assert.deepStrictEqual(commands, ["pr list"]); - yield* read("view").pipe(Effect.provideService(GitHubCli.AllowGitHubReserve, true)); - yield* gh.execute({ cwd: "/repo", args: ["pr", "merge", "1"] }); - assert.deepStrictEqual(commands, ["pr list", "pr view", "pr merge"]); - // One reading covers the whole window. - assert.strictEqual(readings, 1); - yield* TestClock.adjust("1 minute"); - remaining = 5000; - resetAt = DateTime.formatIso(DateTime.makeUnsafe((yield* Clock.currentTimeMillis) + 60_000)); - yield* Effect.all([read("list"), read("repo")], { concurrency: 2 }); - assert.strictEqual(readings, 2); - assert.deepStrictEqual(commands.slice(3).toSorted(), ["pr list", "repo view"]); - }).pipe(Effect.provide(Layer.merge(GitHubGraphQlBudget.layer, SourceControlRateLimit.layer))), -); - -it.effect("reads the budget again at a near reset, and every ten minutes in a long window", () => - Effect.gen(function* () { - let readings = 0; - const startedAt = yield* Clock.currentTimeMillis; - let resetAt = DateTime.formatIso(DateTime.makeUnsafe(startedAt + 10_000)); - const gh = yield* GitHubCli.make.pipe( - Effect.provideService(VcsProcess.VcsProcess, { - run: (input) => - Effect.sync(() => { - if (!isBudgetReading(input)) return processOutput("[]"); - readings++; - return quotaOutput(5000, resetAt); - }), + const driver = Layer.mock(GitVcsDriver.GitVcsDriver)({ + execute: (args) => + Effect.sync(() => { + git.push(["execute", args.args]); + return processOutput(""); }), - ); - const read = gh.execute({ cwd: "/repo", args: ["pr", "list"] }); - yield* read; - // The window resets ten seconds in, so the reading expires with it. - resetAt = DateTime.formatIso(DateTime.makeUnsafe(startedAt + 10_000 + 3_600_000)); - yield* TestClock.adjust("10 seconds"); - yield* read; - assert.strictEqual(readings, 2); - yield* TestClock.adjust("9 minutes"); - yield* read; - assert.strictEqual(readings, 2); - yield* TestClock.adjust("1 minute"); - yield* read; - assert.strictEqual(readings, 3); - }).pipe(Effect.provide(Layer.merge(GitHubGraphQlBudget.layer, SourceControlRateLimit.layer))), -); - -it.effect("reads anyway when the budget reading fails", () => - Effect.gen(function* () { - const gh = yield* GitHubCli.make.pipe( - Effect.provideService(VcsProcess.VcsProcess, { - run: (input) => - isBudgetReading(input) - ? Effect.fail( - new VcsProcessSpawnError({ - operation: "GitHubCli.execute", - command: "gh", - cwd: "/gone", - cause: new Error("ENOENT"), - }), - ) - : Effect.succeed(processOutput("[]")), + resolvePrimaryRemoteName: () => Effect.succeed("origin"), + readConfigValue: () => Effect.succeed("git@github.com:acme/web.git"), + ensureRemote: (args) => + Effect.sync(() => { + git.push(["ensureRemote", args]); + return args.preferredName; }), - ); - const result = yield* gh.execute({ cwd: "/repo", args: ["pr", "list"] }); - assert.strictEqual(result.stdout, "[]"); - }).pipe(Effect.provide(Layer.merge(GitHubGraphQlBudget.layer, SourceControlRateLimit.layer))), -); + fetchRemoteTrackingBranch: (args) => record("fetchRemoteTrackingBranch", undefined)(args), + setBranchUpstream: (args) => record("setBranchUpstream", undefined)(args), + switchRef: (args) => record("switchRef", { refName: args.refName })(args) as never, + listLocalBranchNames: () => Effect.succeed([...(input.localBranches ?? [])]), + resolveCommit: () => Effect.succeed({ commitSha: "abc123" }), + }); + const process = Layer.mock(VcsProcess.VcsProcess)({ + run: (args) => + Effect.succeed( + args.args[0] === "remote" ? processOutput(input.remotes) : processOutput("", 1), + ), + }); + const layer = Layer.effect(GitHubCli.GitHubCli, GitHubCli.make).pipe( + Layer.provide( + Layer.mergeAll( + driver, + process, + Layer.mock(GitHubApi.GitHubApi)(input.api), + NodeServices.layer, + ), + ), + ); + return { layer, git }; +} describe("selectGitHubBaseRepository", () => { const remotes = (...entries: ReadonlyArray) => @@ -223,50 +174,95 @@ describe("selectGitHubBaseRepository", () => { }); }); -describe("GitHubCli.listPullRequestsByHead", () => { - const remoteOutput = - "origin\tgit@github.com:acme/web.git (fetch)\norigin\tgit@github.com:acme/web.git (push)\n"; - const node = (number: number, headRefName: string) => ({ - number, - title: `PR ${number}`, - url: `https://github.com/acme/web/pull/${number}`, - baseRefName: "main", - headRefName, - state: "MERGED", - mergedAt: "2026-01-01T00:00:00Z", - updatedAt: "2026-01-02T00:00:00Z", - headRepository: { name: "web", nameWithOwner: "acme/web" }, - headRepositoryOwner: { login: "acme" }, +describe("GitHubCli repository resolution", () => { + it.effect("reads the repository gh would pick from the remotes", () => { + const paths: string[] = []; + const { layer } = harness({ + remotes: remotesOutput( + ["origin", "git@github.com:me/web.git"], + ["upstream", "https://github.com/acme/web.git"], + ), + api: { + rest: (input) => + Effect.sync(() => { + paths.push(`${input.host} ${input.path}`); + return restResponse({ + full_name: "acme/web", + html_url: "https://github.com/acme/web", + ssh_url: "git@github.com:acme/web.git", + default_branch: "trunk", + }); + }), + }, + }); + return Effect.gen(function* () { + const gh = yield* GitHubCli.GitHubCli; + assert.strictEqual(yield* gh.getDefaultBranch({ cwd: "/repo" }), "trunk"); + assert.deepStrictEqual(paths, ["github.com repos/acme/web"]); + }).pipe(Effect.provide(layer)); }); - const decodeRequest = Schema.decodeSync( - Schema.fromJsonString( - Schema.Struct({ - query: Schema.String, - variables: Schema.Record(Schema.String, Schema.Unknown), - }), - ), - ); - const jsonOutput = (value: unknown) => processOutput(JSON.stringify(value)); - const git = (input: VcsProcess.VcsProcessInput) => - input.args[0] === "remote" - ? processOutput(remoteOutput) - : { ...processOutput(""), exitCode: ChildProcessSpawner.ExitCode(1) }; - it.effect("reads heads on one repository in one GraphQL document", () => - Effect.gen(function* () { - const documents: Array<{ query: string; variables: Record }> = []; - mockRun.mockImplementation((input) => - Effect.sync(() => { - if (input.command === "git") return git(input); - documents.push(decodeRequest(input.stdin ?? "")); - return jsonOutput({ - data: { - repository: { h0: { nodes: [node(7, "feature/a")] }, h1: { nodes: [] } }, - rateLimit: { cost: 1, limit: 5000, remaining: 4999, resetAt: "2099-01-01T00:00:00Z" }, - }, - }); - }), + it.effect("reads an SSH alias remote through github.com", () => { + const hosts: string[] = []; + const { layer } = harness({ + remotes: remotesOutput(["origin", "git@github-work:acme/web.git"]), + api: { + rest: (input) => + Effect.sync(() => { + hosts.push(`${input.host} ${input.path}`); + return restResponse({ + full_name: "acme/web", + html_url: "https://github.com/acme/web", + ssh_url: "git@github.com:acme/web.git", + default_branch: "main", + }); + }), + }, + }); + return Effect.gen(function* () { + const gh = yield* GitHubCli.GitHubCli; + yield* gh.getDefaultBranch({ cwd: "/repo" }); + assert.deepStrictEqual(hosts, ["github.com repos/acme/web"]); + assert.strictEqual( + GitHubCli.gitHubApiHostForRemote("git@github.example.com:a/b.git"), + "github.example.com", ); + assert.strictEqual(GitHubCli.gitHubApiHostForRemote("git@gitlab.com:a/b.git"), null); + }).pipe(Effect.provide(layer)); + }); + + it.effect("fails clearly when no remote is on GitHub", () => { + const { layer } = harness({ + remotes: remotesOutput(["origin", "git@gitlab.com:a/b.git"]), + api: {}, + }); + return Effect.gen(function* () { + const gh = yield* GitHubCli.GitHubCli; + const error = yield* gh.getDefaultBranch({ cwd: "/repo" }).pipe(Effect.flip); + assert.strictEqual(error._tag, "GitHubCliCommandError"); + assert.include(String((error.cause as Error).message), "No GitHub repository"); + }).pipe(Effect.provide(layer)); + }); +}); + +describe("GitHubCli.listPullRequestsByHead", () => { + const remotes = remotesOutput(["origin", "git@github.com:acme/web.git"]); + + it.effect("reads heads on one repository in one GraphQL document", () => { + const documents: Array = []; + const { layer } = harness({ + remotes, + api: { + graphql: (input) => + Effect.sync(() => { + documents.push(input); + return encodeJson({ + data: { repository: { h0: { nodes: [node(7, "feature/a")] }, h1: { nodes: [] } } }, + }); + }), + }, + }); + return Effect.gen(function* () { const gh = yield* GitHubCli.GitHubCli; const lookups = yield* Effect.all( ["feature/a", "feature/b"].map((headSelector) => @@ -283,12 +279,11 @@ describe("GitHubCli.listPullRequestsByHead", () => { yield* TestClock.adjust("50 millis"); const [first, second] = yield* Fiber.join(lookups); assert.deepStrictEqual( - first?.map((pr) => [pr.number, pr.state, pr.headRepositoryNameWithOwner]), - [[7, "merged", "acme/web"]], + first?.map((pr) => pr.number), + [7], ); assert.deepStrictEqual(second, []); assert.strictEqual(documents.length, 1); - assert.include(documents[0]!.query, "rateLimit"); assert.deepStrictEqual(documents[0]!.variables, { owner: "acme", name: "web", @@ -297,701 +292,278 @@ describe("GitHubCli.listPullRequestsByHead", () => { h1: "feature/b", s1: ["OPEN", "CLOSED", "MERGED"], }); - }).pipe(Effect.provide(layer)), - ); + }).pipe(Effect.provide(layer)); + }); - it.effect("asks gh pr list when gh could read another repository", () => - Effect.gen(function* () { - const commands: Array> = []; - mockRun.mockImplementation((input) => - Effect.sync(() => { - commands.push([input.command, ...input.args]); - if (input.command === "git") { - return processOutput( - input.args[0] === "remote" - ? "a\tgit@github.com:me/web.git (fetch)\nb\tgit@github.com:acme/web.git (fetch)\n" - : "", - ); - } - return input.args[3] === "feature/empty" - ? processOutput("") - : jsonOutput([node(8, "feature/a")]); - }), - ); + it.effect("matches an owner:branch selector on the head owner", () => { + const { layer } = harness({ + remotes, + api: { + graphql: (input) => + Effect.succeed( + encodeJson({ + data: { + repository: { + h0: { + nodes: + input.variables?.h0 === "main" + ? [node(9, "main", "someone"), node(8, "main", "me"), node(7, "main", "me")] + : [], + }, + }, + }, + }), + ), + }, + }); + return Effect.gen(function* () { const gh = yield* GitHubCli.GitHubCli; - const pullRequests = yield* gh.listPullRequestsByHead({ - cwd: "/repo", - headSelector: "feature/a", - state: "all", - limit: 100, - rateLimitHost: "github.com", - }); + const open = yield* gh + .listOpenPullRequests({ cwd: "/repo", headSelector: "me:main", limit: 1 }) + .pipe(Effect.forkChild); + yield* TestClock.adjust("50 millis"); assert.deepStrictEqual( - pullRequests.map((pr) => pr.number), + (yield* Fiber.join(open)).map((pr) => pr.number), [8], ); - assert.deepStrictEqual(commands.at(-1), [ - "gh", - "pr", - "list", - "--head", - "feature/a", - "--state", - "all", - "--limit", - "100", - "--json", - "number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner", - ]); - const empty = yield* gh.listPullRequestsByHead({ - cwd: "/repo", - headSelector: "feature/empty", - state: "all", - limit: 100, - rateLimitHost: "github.com", - }); - assert.deepStrictEqual(empty, []); - }).pipe(Effect.provide(layer)), - ); + }).pipe(Effect.provide(layer)); + }); - it.effect("fails a rate-limited document whole instead of asking head by head", () => - Effect.gen(function* () { - let ghCalls = 0; - mockRun.mockImplementation((input) => { - if (input.command === "git") return Effect.succeed(git(input)); - ghCalls++; - return Effect.fail( - new VcsProcessExitError({ - operation: "GitHubCli.execute", - command: "gh", - cwd: "/repo", - exitCode: 1, - failureKind: "rate-limited", - detail: "API rate limit exceeded.", - stderrLength: 24, - stderrTruncated: false, - }), - ); - }); + it.effect("maps API failures onto the errors callers handle", () => { + const { layer } = harness({ + remotes, + api: { + graphql: (input) => + Effect.fail( + input.variables?.h0 === "missing" + ? new GitHubCredentials.GitHubCredentialUnavailableError({ + host: "github.com", + reason: "cli-missing", + }) + : new GitHubApi.GitHubApiRateLimitError({ + host: "github.com", + operation: "x", + retryAt: 123, + }), + ), + }, + }); + return Effect.gen(function* () { const gh = yield* GitHubCli.GitHubCli; - const lookups = yield* Effect.all( - ["feature/a", "feature/b"].map((headSelector) => - gh - .listPullRequestsByHead({ - cwd: "/repo", - headSelector, - state: "all", - limit: 100, - rateLimitHost: "github.com", - }) - .pipe(Effect.flip), - ), - { concurrency: "unbounded" }, - ).pipe(Effect.forkChild); + const read = (headSelector: string) => + gh + .listPullRequestsByHead({ cwd: "/repo", headSelector, state: "open", limit: 1 }) + .pipe(Effect.flip, Effect.forkChild); + const missing = yield* read("missing"); yield* TestClock.adjust("50 millis"); - const errors = yield* Fiber.join(lookups); - assert.deepStrictEqual( - errors.map((error) => error._tag), - ["GitHubCliRateLimitError", "GitHubCliRateLimitError"], - ); - assert.strictEqual(ghCalls, 1); - }).pipe(Effect.provide(layer)), - ); -}); - -describe("GitHubCli.layer", () => { - it.effect("shares the registry budget with CLI reads through nested layer providers", () => - Effect.gen(function* () { - const budget = yield* GitHubGraphQlBudget.GitHubGraphQlBudget; - const gh = yield* GitHubCli.GitHubCli; - yield* budget.observe("github.com", quotaOutput(0).stdout); - const error = yield* gh.execute({ cwd: "/repo", args: ["pr", "list"] }).pipe(Effect.flip); + assert.strictEqual((yield* Fiber.join(missing))._tag, "GitHubCliUnavailableError"); + const limited = yield* read("limited"); + yield* TestClock.adjust("50 millis"); + const error = yield* Fiber.join(limited); assert.strictEqual(error._tag, "GitHubCliRateLimitError"); - expect(mockRun).not.toHaveBeenCalled(); - }).pipe(Effect.provide(layer.pipe(Layer.provide(GitHubGraphQlBudget.layer)))), - ); + assert.propertyVal(error, "retryAt", 123); + }).pipe(Effect.provide(layer)); + }); +}); - it.effect("keeps quota snapshots separate for verified credentials on the same host", () => - Effect.gen(function* () { - let reads = 0; - const gh = yield* GitHubCli.make.pipe( - Effect.provideService(VcsProcess.VcsProcess, { - run: (input) => - Effect.sync(() => { - if (isBudgetReading(input)) { - return quotaOutput(input.env?.GH_TOKEN === "empty" ? 0 : 5000); - } - reads++; - return processOutput("[]"); - }), - }), - ); - const read = (token: string) => - gh.execute({ cwd: "/repo", args: ["pr", "list", "--repo", "github.com/acme/web"] }).pipe( - Effect.provideService(GitHubCli.PinnedGitHubCredential, { - host: "github.com", - token: Redacted.make(token), - credentialFingerprint: token, +describe("GitHubCli.getPullRequest", () => { + it.effect("reads a pull request by number, and by URL on its own repository", () => { + const variables: Array = []; + const { layer } = harness({ + remotes: remotesOutput(["origin", "git@github.com:acme/web.git"]), + api: { + graphql: (input) => + Effect.sync(() => { + variables.push(input.variables); + return encodeJson({ data: { repository: { pullRequest: node(42, "feature") } } }); }), - ); - yield* read("empty").pipe(Effect.flip); - yield* read("healthy"); - yield* read("empty").pipe(Effect.flip); - assert.strictEqual(reads, 1); - }).pipe(Effect.provide(Layer.merge(GitHubGraphQlBudget.layer, SourceControlRateLimit.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")), - ); + }, + }); + return Effect.gen(function* () { 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", + assert.strictEqual((yield* gh.getPullRequest({ cwd: "/repo", reference: "#42" })).number, 42); + yield* gh.getPullRequest({ + cwd: "/repo", + reference: "https://github.com/other/thing/pull/42", }); - const results = yield* Effect.forEach( - ["github.com", "github.example.test"], - (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)), - ); + assert.deepStrictEqual(variables, [ + { owner: "acme", name: "web", number: 42 }, + { owner: "other", name: "thing", number: 42 }, + ]); + }).pipe(Effect.provide(layer)); + }); - it.effect("refuses other or implicit hosts before exposing a scoped credential to gh", () => - Effect.gen(function* () { + it.effect("fails a missing pull request as not found", () => { + const { layer } = harness({ + remotes: remotesOutput(["origin", "git@github.com:acme/web.git"]), + api: { + graphql: () => Effect.succeed(encodeJson({ data: { repository: { pullRequest: null } } })), + }, + }); + return 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)), - ); + const error = yield* gh.getPullRequest({ cwd: "/repo", reference: "7" }).pipe(Effect.flip); + assert.strictEqual(error._tag, "GitHubPullRequestNotFoundError"); + }).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", +describe("GitHubCli writes", () => { + it.effect("creates a cross-repository pull request with an owner:branch head", () => { + const requests: Array = []; + const { layer } = harness({ + remotes: remotesOutput( + ["origin", "git@github.com:me/web.git"], + ["upstream", "git@github.com:acme/web.git"], + ), + api: { + rest: (input) => + Effect.sync(() => { + requests.push(input); + return restResponse({ number: 1 }, 201); }), - ); - 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({ - operation: "GitHubCli.execute", - command: "gh", - cwd: context.cwd, - cause: PlatformError.systemError({ - _tag: "NotFound", - module: "FileSystem", - method: "access", - pathOrDescriptor: context.cwd, - }), + }, }); - - const commandFailure = GitHubCli.fromVcsError(context, missingCwd); - - assert.equal(commandFailure._tag, "GitHubCliCommandError"); - assert.strictEqual(commandFailure.cause, missingCwd); - assert.notProperty(commandFailure, "operation"); - }); - - it.effect("parses pull request view output", () => - Effect.gen(function* () { - mockRun.mockReturnValueOnce( - Effect.succeed( - processOutput( - // @effect-diagnostics-next-line preferSchemaOverJson:off - JSON.stringify({ - number: 42, - title: "Add PR thread creation", - url: "https://github.com/pingdotgg/codething-mvp/pull/42", - baseRefName: "main", - headRefName: "feature/pr-threads", - state: "OPEN", - isDraft: true, - mergedAt: null, - updatedAt: "2026-08-24T12:34:56Z", - isCrossRepository: true, - headRepository: { - nameWithOwner: "octocat/codething-mvp", - }, - headRepositoryOwner: { - login: "octocat", - }, - }), - ), - ), - ); - + return Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const bodyFile = yield* fs.makeTempFileScoped({ suffix: ".md" }); + yield* fs.writeFileString(bodyFile, "Body"); const gh = yield* GitHubCli.GitHubCli; - const result = yield* gh.getPullRequest({ + yield* gh.createPullRequest({ cwd: "/repo", - reference: "#42", - }); - - assert.deepStrictEqual(result, { - number: 42, - title: "Add PR thread creation", - url: "https://github.com/pingdotgg/codething-mvp/pull/42", - baseRefName: "main", - headRefName: "feature/pr-threads", - state: "open", - closedAt: null, - mergedAt: null, - isDraft: true, - updatedAt: "2026-08-24T12:34:56.000Z", - isCrossRepository: true, - headRepositoryNameWithOwner: "octocat/codething-mvp", - headRepositoryOwnerLogin: "octocat", + baseBranch: "main", + headSelector: "me:feature", + title: "Title", + bodyFile, }); - expect(mockRun).toHaveBeenCalledWith({ - operation: "GitHubCli.execute", - command: "gh", - args: [ - "pr", - "view", - "#42", - "--json", - "number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner", - ], - cwd: "/repo", - timeoutMs: 30_000, + assert.strictEqual(requests[0]!.method, "POST"); + assert.strictEqual(requests[0]!.path, "repos/acme/web/pulls"); + assert.deepStrictEqual(requests[0]!.body, { + base: "main", + head: "me:feature", + title: "Title", + body: "Body", }); - }).pipe(Effect.provide(layer)), - ); - - it.effect("trims pull request fields decoded from gh json", () => - Effect.gen(function* () { - mockRun.mockReturnValueOnce( - Effect.succeed( - processOutput( - // @effect-diagnostics-next-line preferSchemaOverJson:off - JSON.stringify({ - number: 42, - title: " Add PR thread creation \n", - url: " https://github.com/pingdotgg/codething-mvp/pull/42 ", - baseRefName: " main ", - headRefName: "\tfeature/pr-threads\t", - state: "OPEN", - mergedAt: null, - isCrossRepository: true, - headRepository: { - nameWithOwner: " octocat/codething-mvp ", - }, - headRepositoryOwner: { - login: " octocat ", - }, - }), - ), - ), - ); + }).pipe(Effect.provide(layer), Effect.provide(NodeServices.layer), Effect.scoped); + }); + it.effect("creates a repository under an organization the viewer is not", () => { + const requests: Array = []; + const { layer } = harness({ + remotes: "", + api: { + rest: (input) => + Effect.sync(() => { + requests.push(`${input.method ?? "GET"} ${input.path}`); + return input.path === "user" + ? restResponse({ login: "me" }) + : restResponse({ + full_name: "acme/new", + html_url: "https://github.com/acme/new", + ssh_url: "git@github.com:acme/new.git", + }); + }), + }, + }); + return Effect.gen(function* () { const gh = yield* GitHubCli.GitHubCli; - const result = yield* gh.getPullRequest({ + const urls = yield* gh.createRepository({ cwd: "/repo", - reference: "#42", + repository: "acme/new", + visibility: "private", }); - - assert.deepStrictEqual(result, { - number: 42, - title: "Add PR thread creation", - url: "https://github.com/pingdotgg/codething-mvp/pull/42", - baseRefName: "main", - headRefName: "feature/pr-threads", - state: "open", - closedAt: null, - mergedAt: null, - isCrossRepository: true, - headRepositoryNameWithOwner: "octocat/codething-mvp", - headRepositoryOwnerLogin: "octocat", - }); - }).pipe(Effect.provide(layer)), - ); - - it.effect("skips invalid entries when parsing pr lists", () => - Effect.gen(function* () { - mockRun.mockReturnValueOnce( - Effect.succeed( - processOutput( - // @effect-diagnostics-next-line preferSchemaOverJson:off - JSON.stringify([ - { - number: 0, - title: "invalid", - url: "https://github.com/pingdotgg/codething-mvp/pull/0", - baseRefName: "main", - headRefName: "feature/invalid", - }, - { - number: 43, - title: " Valid PR ", - url: " https://github.com/pingdotgg/codething-mvp/pull/43 ", - baseRefName: " main ", - headRefName: " feature/pr-list ", - headRepository: { - nameWithOwner: " ", - }, - headRepositoryOwner: { - login: " ", - }, - }, - ]), - ), - ), - ); - - const gh = yield* GitHubCli.GitHubCli; - const result = yield* gh.listOpenPullRequests({ - cwd: "/repo", - headSelector: "feature/pr-list", + assert.deepStrictEqual(urls, { + nameWithOwner: "acme/new", + url: "https://github.com/acme/new", + sshUrl: "git@github.com:acme/new.git", }); + assert.deepStrictEqual(requests, ["GET user", "POST orgs/acme/repos"]); + }).pipe(Effect.provide(layer)); + }); +}); - assert.deepStrictEqual(result, [ - { - number: 43, - title: "Valid PR", - url: "https://github.com/pingdotgg/codething-mvp/pull/43", - baseRefName: "main", - headRefName: "feature/pr-list", - state: "open", - closedAt: null, - mergedAt: null, - }, - ]); - }).pipe(Effect.provide(layer)), - ); +describe("GitHubCli.checkoutPullRequest", () => { + const repository = (fullName: string, defaultBranch = "main") => + restResponse({ + full_name: fullName, + html_url: `https://github.com/${fullName}`, + ssh_url: `git@github.com:${fullName}.git`, + default_branch: defaultBranch, + }); - it.effect("keeps pull requests from gh versions without headRepository.nameWithOwner", () => - // gh < 2.47 (e.g. Ubuntu-packaged 2.46) exports headRepository as - // {id, name} only. These entries must decode instead of being dropped, - // with nameWithOwner rebuilt from the owner login. - Effect.gen(function* () { - mockRun.mockReturnValueOnce( - Effect.succeed( - processOutput( - // @effect-diagnostics-next-line preferSchemaOverJson:off - JSON.stringify([ - { - number: 2829, - title: "Codex turn mapping", - url: "https://github.com/pingdotgg/codething-mvp/pull/2829", - baseRefName: "main", - headRefName: "t3code/codex-turn-mapping", - state: "OPEN", - mergedAt: null, - isCrossRepository: false, - headRepository: { - id: "R_kgDORLtfbQ", - name: "codething-mvp", - }, - headRepositoryOwner: { - id: "MDEyOk9yZ2FuaXphdGlvbjg5MTkxNzI3", - login: "pingdotgg", - }, - }, - ]), + it.effect("checks a same-repository pull request out from its head branch", () => { + const { layer, git } = harness({ + remotes: remotesOutput(["origin", "git@github.com:acme/web.git"]), + api: { + graphql: () => + Effect.succeed( + encodeJson({ data: { repository: { pullRequest: node(5, "feature/x") } } }), ), - ), - ); - + }, + }); + return Effect.gen(function* () { const gh = yield* GitHubCli.GitHubCli; - const result = yield* gh.listOpenPullRequests({ - cwd: "/repo", - headSelector: "t3code/codex-turn-mapping", - }); - - assert.deepStrictEqual(result, [ - { - number: 2829, - title: "Codex turn mapping", - url: "https://github.com/pingdotgg/codething-mvp/pull/2829", - baseRefName: "main", - headRefName: "t3code/codex-turn-mapping", - state: "open", - closedAt: null, - mergedAt: null, - isCrossRepository: false, - headRepositoryNameWithOwner: "pingdotgg/codething-mvp", - headRepositoryOwnerLogin: "pingdotgg", - }, + yield* gh.checkoutPullRequest({ cwd: "/repo", reference: "5" }); + assert.deepStrictEqual(git, [ + [ + "fetchRemoteTrackingBranch", + { cwd: "/repo", remoteName: "origin", remoteBranch: "feature/x" }, + ], + ["execute", ["branch", "feature/x", "refs/remotes/origin/feature/x"]], + [ + "setBranchUpstream", + { cwd: "/repo", branch: "feature/x", remoteName: "origin", remoteBranch: "feature/x" }, + ], + ["switchRef", { cwd: "/repo", refName: "feature/x" }], ]); - }).pipe(Effect.provide(layer)), - ); - - it.effect("reads repository clone URLs", () => - Effect.gen(function* () { - mockRun.mockReturnValueOnce( - Effect.succeed( - processOutput( - // @effect-diagnostics-next-line preferSchemaOverJson:off - JSON.stringify({ - nameWithOwner: "octocat/codething-mvp", - url: "https://github.com/octocat/codething-mvp", - sshUrl: "git@github.com:octocat/codething-mvp.git", - }), - ), - ), - ); - - const gh = yield* GitHubCli.GitHubCli; - const result = yield* gh.getRepositoryCloneUrls({ - cwd: "/repo", - repository: "octocat/codething-mvp", - }); - - assert.deepStrictEqual(result, { - nameWithOwner: "octocat/codething-mvp", - url: "https://github.com/octocat/codething-mvp", - sshUrl: "git@github.com:octocat/codething-mvp.git", - }); - }).pipe(Effect.provide(layer)), - ); + }).pipe(Effect.provide(layer)); + }); - it.effect("creates repositories and parses clone URLs from create output", () => - Effect.gen(function* () { - mockRun.mockReturnValueOnce( - Effect.succeed( - processOutput( - "✓ Created repository octocat/codething-mvp on github.com\nhttps://github.com/octocat/codething-mvp\n", + it.effect("adds a remote for a fork and names a default-branch head after its owner", () => { + const { layer, git } = harness({ + remotes: remotesOutput(["origin", "git@github.com:acme/web.git"]), + localBranches: ["someone/main"], + api: { + graphql: () => + Effect.succeed( + encodeJson({ data: { repository: { pullRequest: node(6, "main", "someone") } } }), ), - ), - ); - - const gh = yield* GitHubCli.GitHubCli; - const result = yield* gh.createRepository({ - cwd: "/repo", - repository: "octocat/codething-mvp", - visibility: "private", - }); - - assert.deepStrictEqual(result, { - nameWithOwner: "octocat/codething-mvp", - url: "https://github.com/octocat/codething-mvp", - sshUrl: "git@github.com:octocat/codething-mvp.git", - }); - expect(mockRun).toHaveBeenCalledTimes(1); - expect(mockRun).toHaveBeenNthCalledWith(1, { - operation: "GitHubCli.execute", - command: "gh", - args: ["repo", "create", "octocat/codething-mvp", "--private"], - cwd: "/repo", - timeoutMs: 30_000, - }); - }).pipe(Effect.provide(layer)), - ); - - it.effect("falls back to constructed URLs when create output omits a URL", () => - Effect.gen(function* () { - mockRun.mockReturnValueOnce(Effect.succeed(processOutput(""))); - - const gh = yield* GitHubCli.GitHubCli; - const result = yield* gh.createRepository({ - cwd: "/repo", - repository: "octocat/codething-mvp", - visibility: "private", - }); - - assert.deepStrictEqual(result, { - nameWithOwner: "octocat/codething-mvp", - url: "https://github.com/octocat/codething-mvp", - sshUrl: "git@github.com:octocat/codething-mvp.git", - }); - }).pipe(Effect.provide(layer)), - ); - - it.effect("surfaces a friendly error when the pull request is not found", () => - Effect.gen(function* () { - const cause = new VcsProcessExitError({ - operation: "GitHubCli.execute", - command: "gh pr view", - cwd: "/repo", - exitCode: 1, - failureKind: "not-found", - detail: - "GraphQL: Could not resolve to a PullRequest with the number of 4888. (repository.pullRequest)", - }); - mockRun.mockReturnValueOnce(Effect.fail(cause)); - + rest: (input) => + Effect.succeed(repository(input.path === "repos/acme/web" ? "acme/web" : "someone/web")), + }, + }); + return Effect.gen(function* () { const gh = yield* GitHubCli.GitHubCli; - const error = yield* gh - .getPullRequest({ - cwd: "/repo", - reference: "4888", - }) - .pipe(Effect.flip); - - assert.equal(error.message.includes("Pull request not found"), true); - assert.strictEqual(error._tag, "GitHubPullRequestNotFoundError"); - assert.strictEqual(error.command, "gh"); - assert.strictEqual(error.cwd, "/repo"); - assert.strictEqual(error.cause, cause); - assert.equal(error.message.includes(cause.detail), false); - }).pipe(Effect.provide(layer)), - ); + yield* gh.checkoutPullRequest({ cwd: "/repo", reference: "6", force: true }); + assert.deepStrictEqual(git, [ + [ + "ensureRemote", + { cwd: "/repo", preferredName: "someone", url: "git@github.com:someone/web.git" }, + ], + [ + "fetchRemoteTrackingBranch", + { cwd: "/repo", remoteName: "someone", remoteBranch: "main" }, + ], + [ + "setBranchUpstream", + { cwd: "/repo", branch: "someone/main", remoteName: "someone", remoteBranch: "main" }, + ], + ["switchRef", { cwd: "/repo", refName: "someone/main" }], + ["execute", ["reset", "--hard", "--quiet", "refs/remotes/someone/main"]], + ]); + }).pipe(Effect.provide(layer)); + }); - it.effect("surfaces an actionable rate-limit error without exposing provider stderr", () => - Effect.gen(function* () { - const cause = new VcsProcessExitError({ - operation: "GitHubCli.execute", - command: "gh", - cwd: "/repo", - exitCode: 1, - failureKind: "rate-limited", - detail: "API rate limit exceeded.", - stderrLength: 82, - stderrTruncated: false, + it("names the local branch the way gh pr checkout does", () => { + const name = (headRefName: string, isCrossRepository: boolean) => + GitHubCli.pullRequestCheckoutBranchName({ + headRefName, + headOwner: "someone", + isCrossRepository, + defaultBranch: "main", }); - mockRun.mockReturnValueOnce(Effect.fail(cause)); - - const gh = yield* GitHubCli.GitHubCli; - const error = yield* gh - .listOpenPullRequests({ - cwd: "/repo", - headSelector: "feature/rate-limited", - }) - .pipe(Effect.flip); - - assert.strictEqual(error._tag, "GitHubCliRateLimitError"); - assert.include(error.detail, "GitHub API rate limit exceeded"); - assert.include(error.detail, "gh api rate_limit"); - assert.strictEqual(error.cause, cause); - assert.notInclude(error.message, "user ID"); - const paused = yield* gh - .execute({ cwd: "/other-repo", args: ["pr", "list"] }) - .pipe(Effect.flip); - assert.strictEqual(paused._tag, "GitHubCliRateLimitError"); - expect(mockRun).toHaveBeenCalledTimes(1); - yield* TestClock.adjust("30 seconds"); - mockRun.mockReturnValueOnce(Effect.succeed(processOutput("[]"))); - yield* gh.execute({ cwd: "/other-repo", args: ["pr", "list"] }); - expect(mockRun).toHaveBeenCalledTimes(2); - }).pipe(Effect.provide(layer)), - ); + assert.strictEqual(name("main", true), "someone/main"); + assert.strictEqual(name("feature", true), "feature"); + assert.strictEqual(name("main", false), "main"); + }); }); - -it.effect("accepts conditional 304 responses and preserves HTTP errors and retry delays", () => - Effect.gen(function* () { - const gh = yield* GitHubCli.GitHubCli; - const request = { - cwd: "/repo", - args: [ - "api", - "repos/acme/web/pulls/1", - "--hostname", - "github.com", - "--include", - "-H", - 'If-None-Match: "one"', - ], - acceptNotModified: true, - }; - const respond = (status: number, headers = "") => - mockRun.mockImplementation(() => - Effect.succeed({ - ...processOutput(`HTTP/2.0 ${status}\r\n${headers}\r\n`), - exitCode: ChildProcessSpawner.ExitCode(1), - }), - ); - respond(304); - expect((yield* gh.execute(request)).stdout).toContain("304"); - expect(mockRun.mock.calls[0]?.[0].allowNonZeroExit).toBe(true); - respond(401); - expect((yield* gh.execute(request).pipe(Effect.flip))._tag).toBe( - "GitHubCliAuthenticationError", - ); - respond(403); - expect((yield* gh.execute(request).pipe(Effect.flip))._tag).toBe("GitHubCliCommandError"); - for (const status of [403, 429]) { - respond(status, "Retry-After: 120\r\n"); - expect(yield* gh.execute(request).pipe(Effect.flip)).toMatchObject({ - _tag: "GitHubCliRateLimitError", - retryAt: (yield* Clock.currentTimeMillis) + 120_000, - }); - } - respond( - 403, - `X-RateLimit-Remaining: 0\r\nX-RateLimit-Reset: ${Math.floor((yield* Clock.currentTimeMillis) / 1_000) + 60}\r\n`, - ); - expect(yield* gh.execute(request).pipe(Effect.flip)).toMatchObject({ - _tag: "GitHubCliRateLimitError", - retryAt: (yield* Clock.currentTimeMillis) + 60_000, - }); - respond(500); - expect(yield* gh.execute(request).pipe(Effect.flip)).toMatchObject({ - _tag: "GitHubCliCommandError", - httpStatus: 500, - }); - }).pipe(Effect.provide(layer)), -); diff --git a/apps/server/src/sourceControl/GitHubCli.ts b/apps/server/src/sourceControl/GitHubCli.ts index 5032c289e9a6..fec3bad683fa 100644 --- a/apps/server/src/sourceControl/GitHubCli.ts +++ b/apps/server/src/sourceControl/GitHubCli.ts @@ -617,10 +617,13 @@ export function parsePullRequestReference( */ export function gitHubApiHostForRemote(remoteUrl: string): string | null { const provider = detectSourceControlProviderFromRemoteUrl(remoteUrl); - if (provider?.kind !== "github") return null; + if (provider === null) return null; const host = new URL(provider.baseUrl).host.toLowerCase(); - if (host === "github.com" || host.endsWith(".ghe.com") || host.includes(".")) return host; - return isSshRemoteUrl(remoteUrl) ? "github.com" : host; + // A dotless SSH host is an alias from ~/.ssh/config, never a real API host. + if (isSshRemoteUrl(remoteUrl) && !host.includes(".")) { + return host.includes("github") ? "github.com" : null; + } + return provider.kind === "github" ? host : null; } /** The local branch a pull request checks out into, the way `gh pr checkout` names it. */ diff --git a/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts b/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts index a486c27ab66c..15816a92eecb 100644 --- a/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts +++ b/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts @@ -3,11 +3,12 @@ import * as DateTime from "effect/DateTime"; import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; +import * as Schema from "effect/Schema"; import { ChildProcessSpawner } from "effect/process"; import * as VcsProcess from "../vcs/VcsProcess.ts"; +import * as GitHubApi from "./GitHubApi.ts"; import * as GitHubCli from "./GitHubCli.ts"; -import * as GitHubGraphQlBudget from "./githubGraphQlBudget.ts"; import { parseGitHubAuthStatus } from "./gitHubAuthStatus.ts"; import * as GitHubSourceControlProvider from "./GitHubSourceControlProvider.ts"; @@ -25,37 +26,24 @@ const processResult = ( stderrTruncated: false, }); -function makeProvider(github: Partial) { +function makeProvider( + github: Partial, + api: Partial = {}, +) { return GitHubSourceControlProvider.make.pipe( Effect.provide(Layer.mock(GitHubCli.GitHubCli)(github)), + Effect.provide(Layer.mock(GitHubApi.GitHubApi)(api)), ); } -it.effect("uses the enterprise quota for a current-repository default branch read", () => - Effect.gen(function* () { - // github.com is out of quota; the enterprise read must not be priced against it. - const budget = yield* GitHubGraphQlBudget.GitHubGraphQlBudget; - yield* budget.observe( - "github.com", - '{"data":{"rateLimit":{"cost":1,"limit":5000,"remaining":0,"resetAt":"2099-01-01T00:00:00Z"}}}', - ); - const provider = yield* GitHubSourceControlProvider.make; - const branch = yield* provider.getDefaultBranch({ - cwd: "/enterprise-repo", - context: { - provider: { kind: "github", name: "GitHub Enterprise", baseUrl: "https://enterprise.test" }, - remoteName: "origin", - remoteUrl: "https://enterprise.test/acme/web.git", - }, - }); - assert.strictEqual(branch, "main"); - }).pipe( - Effect.provide(GitHubCli.layer), - Effect.provideService(VcsProcess.VcsProcess, { - run: () => Effect.succeed(processResult("main")), - }), - ), -); +const encodeJson = Schema.encodeSync(Schema.fromJsonString(Schema.Unknown)); + +const restResponse = (body: string): GitHubApi.GitHubRestResponse => ({ + status: 200, + headers: {}, + body, + truncated: false, +}); it.effect("maps GitHub PR summaries into provider-neutral change requests", () => Effect.gen(function* () { @@ -412,27 +400,20 @@ it.effect.each(["pull", "issues"])( "resolves %s subjects on the linked host without using the checkout", (kind) => Effect.gen(function* () { - const provider = yield* makeProvider({ - execute: (input) => { - assert.deepStrictEqual(input.args, [ - "api", - "--hostname", - "github.com", - "repos/owner/repo/issues/42", - "--jq", - "{title, body}", - ]); - assert.strictEqual(input.maxOutputBytes, 32_000); - assert.strictEqual(input.timeoutMs, 3_000); - return Effect.succeed({ - exitCode: ChildProcessSpawner.ExitCode(0), - stdout: JSON.stringify({ title: "Pairing expiry", body: "Preserve remote access" }), - stderr: "", - stdoutTruncated: false, - stderrTruncated: false, - }); + const provider = yield* makeProvider( + {}, + { + rest: (input) => { + assert.strictEqual(input.host, "github.com"); + assert.strictEqual(input.path, "repos/owner/repo/issues/42"); + return Effect.succeed( + restResponse( + encodeJson({ title: "Pairing expiry", body: "Preserve remote access", id: 1 }), + ), + ); + }, }, - }); + ); const lookup = provider.resolveLink?.({ cwd: "/unrelated", url: new URL(`https://github.com/owner/repo/${kind}/42`), @@ -456,23 +437,20 @@ it.effect.each(["read", "decode"] as const)( "retains the %s failure without exposing its raw contents", (stage) => Effect.gen(function* () { - const cause = new GitHubCli.GitHubCliCommandError({ - command: "gh", - cwd: "/repo", - cause: new Error("private response text"), - }); - const provider = yield* makeProvider({ - execute: () => - stage === "read" - ? Effect.fail(cause) - : Effect.succeed({ - exitCode: ChildProcessSpawner.ExitCode(0), - stdout: "private response text", - stderr: "", - stdoutTruncated: false, - stderrTruncated: false, - }), + const cause = new GitHubApi.GitHubApiResponseError({ + host: "github.com", + operation: "resolveLink", + status: 500, }); + const provider = yield* makeProvider( + {}, + { + rest: () => + stage === "read" + ? Effect.fail(cause) + : Effect.succeed(restResponse("private response text")), + }, + ); const lookup = provider.resolveLink?.({ cwd: "/repo", url: new URL("https://github.com/owner/repo/issues/42"), diff --git a/apps/server/src/sourceControl/SourceControlDiscovery.test.ts b/apps/server/src/sourceControl/SourceControlDiscovery.test.ts index 0705523cccb9..672963fc9ec4 100644 --- a/apps/server/src/sourceControl/SourceControlDiscovery.test.ts +++ b/apps/server/src/sourceControl/SourceControlDiscovery.test.ts @@ -16,6 +16,7 @@ import * as VcsDriverRegistry from "../vcs/VcsDriverRegistry.ts"; import * as VcsProcess from "../vcs/VcsProcess.ts"; import * as AzureDevOpsCli from "./AzureDevOpsCli.ts"; import * as BitbucketApi from "./BitbucketApi.ts"; +import * as GitHubApi from "./GitHubApi.ts"; import * as GitHubCli from "./GitHubCli.ts"; import * as GitLabCli from "./GitLabCli.ts"; import * as ForgejoCli from "./ForgejoCli.ts"; @@ -38,6 +39,7 @@ const layerSourceControlProviderRegistryTest = (input: { Layer.mock(AzureDevOpsCli.AzureDevOpsCli)({}), Layer.mock(BitbucketApi.BitbucketApi)(input.bitbucket), Layer.mock(GitHubCli.GitHubCli)({}), + Layer.mock(GitHubApi.GitHubApi)({}), Layer.mock(GitLabCli.GitLabCli)({}), Layer.mock(ForgejoCli.ForgejoCli)({ listLogins: () => Effect.succeed([]) }), Layer.mock(VcsDriverRegistry.VcsDriverRegistry)({}), diff --git a/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts b/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts index 1629ff385c3f..82ef039bd763 100644 --- a/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts +++ b/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts @@ -13,6 +13,7 @@ import * as VcsDriverRegistry from "../vcs/VcsDriverRegistry.ts"; import * as VcsProcess from "../vcs/VcsProcess.ts"; import * as AzureDevOpsCli from "./AzureDevOpsCli.ts"; import * as BitbucketApi from "./BitbucketApi.ts"; +import * as GitHubApi from "./GitHubApi.ts"; import * as GitHubCli from "./GitHubCli.ts"; import * as GitLabCli from "./GitLabCli.ts"; import * as ForgejoCli from "./ForgejoCli.ts"; @@ -41,6 +42,7 @@ function makeRegistry(input: { }>; readonly process?: Partial; readonly github?: Partial; + readonly githubApi?: Partial; readonly gitlab?: Partial; readonly resolve?: VcsDriverRegistry.VcsDriverRegistry["Service"]["resolve"]; }) { @@ -95,6 +97,7 @@ function makeRegistry(input: { Layer.mock(AzureDevOpsCli.AzureDevOpsCli)({}), Layer.mock(BitbucketApi.BitbucketApi)({}), Layer.mock(GitHubCli.GitHubCli)(input.github ?? {}), + Layer.mock(GitHubApi.GitHubApi)(input.githubApi ?? {}), Layer.mock(GitLabCli.GitLabCli)(input.gitlab ?? {}), Layer.mock(ForgejoCli.ForgejoCli)({ listLogins: () => Effect.succeed([]) }), ServerConfig.layerTest(process.cwd(), { @@ -305,9 +308,14 @@ it.effect( Effect.gen(function* () { const registry = yield* makeRegistry({ remotes: [{ name: "origin", url: "https://github.com/unrelated/checkout.git" }], - github: { - execute: () => - Effect.succeed(processOutput(JSON.stringify({ title: "GitHub issue", body: null }))), + githubApi: { + rest: () => + Effect.succeed({ + status: 200, + headers: {}, + body: JSON.stringify({ title: "GitHub issue", body: null }), + truncated: false, + }), }, gitlab: { execute: () => From a4e12eb3da82767be0bda2c24efd293e6bc42b7b Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Mon, 5 Oct 2026 18:40:20 -0700 Subject: [PATCH 4/9] refactor(server): drop GitHubCli.execute now that nothing runs gh for API calls Co-Authored-By: Claude Opus 5.5 (1M context) --- apps/server/src/git/GitManager.test.ts | 7 +- .../src/sourceControl/GitHubCli.test.ts | 1 + apps/server/src/sourceControl/GitHubCli.ts | 134 ------------------ .../GitHubSourceControlProvider.test.ts | 1 + .../SourceControlProviderRegistry.test.ts | 1 + 5 files changed, 8 insertions(+), 136 deletions(-) diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index e1d7bceb6418..6b4b5cf0578b 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -384,7 +384,11 @@ function createGitHubCliWithFakeGh(scenario: FakeGhScenario = {}): { ); const ghCalls: string[] = []; - const execute: GitHubCli.GitHubCli["Service"]["execute"] = (input) => { + // The fake still speaks in gh's command shapes; the service methods below translate to them. + const execute = (input: { + readonly cwd: string; + readonly args: ReadonlyArray; + }): Effect.Effect => { const args = [...input.args]; ghCalls.push(args.join(" ")); @@ -519,7 +523,6 @@ function createGitHubCliWithFakeGh(scenario: FakeGhScenario = {}): { return { service: { - execute, // The fake answers the CLI shape, so batched lookups read it the way the fallback does. listPullRequestsByHead: (input) => execute({ diff --git a/apps/server/src/sourceControl/GitHubCli.test.ts b/apps/server/src/sourceControl/GitHubCli.test.ts index 589d2cc5e48c..4c1099960fa0 100644 --- a/apps/server/src/sourceControl/GitHubCli.test.ts +++ b/apps/server/src/sourceControl/GitHubCli.test.ts @@ -34,6 +34,7 @@ const restResponse = (body: unknown, status = 200): GitHubApi.GitHubRestResponse headers: {}, body: body === undefined ? "" : encodeJson(body), truncated: false, + invalidUtf8: false, }); const node = (number: number, headRefName: string, owner = "acme") => ({ diff --git a/apps/server/src/sourceControl/GitHubCli.ts b/apps/server/src/sourceControl/GitHubCli.ts index fec3bad683fa..f124ca04b76c 100644 --- a/apps/server/src/sourceControl/GitHubCli.ts +++ b/apps/server/src/sourceControl/GitHubCli.ts @@ -6,7 +6,6 @@ import * as FileSystem from "effect/FileSystem"; 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 Request from "effect/Request"; import * as RequestResolver from "effect/RequestResolver"; import * as Result from "effect/Result"; @@ -35,47 +34,11 @@ import { type NormalizedGitHubPullRequestRecord, } from "./gitHubPullRequests.ts"; -const DEFAULT_TIMEOUT_MS = 30_000; - /** Server-local credential scope; never put its value in RPC payloads or cache keys. */ export const PinnedGitHubCredential = GitHubApi.PinnedGitHubCredential; export const AllowGitHubReserve = GitHubApi.AllowGitHubReserve; -// The hosts a deprecated `execute` command reaches, so a pinned credential never leaves its -// host. Deleted with `execute`. -function commandHosts(args: ReadonlyArray): Array { - 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; -} - -function targetsVerifiedHost(args: ReadonlyArray, host: string): boolean { - const hosts = commandHosts(args); - return hosts.length > 0 && hosts.every((target) => target === host); -} - const gitHubCliFailureFields = { command: Schema.Literal("gh"), cwd: Schema.String, @@ -311,25 +274,6 @@ export interface GitHubRepositoryCloneUrls { export class GitHubCli extends Context.Service< GitHubCli, { - /** - * Runs `gh` itself. - * - * @deprecated Kept only for the pull request layer until it reads through `GitHubApi`; it is - * removed with that change. Everything else here goes through `GitHubApi`. - */ - readonly execute: (input: { - readonly cwd: string; - readonly args: ReadonlyArray; - readonly timeoutMs?: number; - /** Piped to the child's stdin, for payloads that must never appear in argv. */ - readonly stdin?: string; - readonly env?: NodeJS.ProcessEnv; - readonly maxOutputBytes?: number; - readonly rateLimitHost?: string; - readonly allowReserve?: boolean; - readonly acceptNotModified?: boolean; - }) => Effect.Effect; - readonly listOpenPullRequests: (input: { readonly cwd: string; readonly headSelector: string; @@ -650,83 +594,6 @@ export const make = Effect.gen(function* () { const git = yield* GitVcsDriver.GitVcsDriver; const fileSystem = yield* FileSystem.FileSystem; - /** - * Runs `gh` itself. - * - * @deprecated Only the pull request layer still calls this, until it moves onto `GitHubApi`; - * it is deleted with that change. New code reads GitHub through `GitHubApi`. - */ - 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: "", - }; - const result = yield* process - .run({ - operation: "GitHubCli.execute", - command: "gh", - args: input.args, - cwd: input.cwd, - timeoutMs: input.timeoutMs ?? DEFAULT_TIMEOUT_MS, - ...(input.acceptNotModified ? { allowNonZeroExit: true } : {}), - ...(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))); - if (result.exitCode !== 0 && input.acceptNotModified) { - const status = /^HTTP\/\S+ (\d+)/.exec(result.stdout)?.[1]; - if (status !== "304" || !input.args.includes("--include")) { - const context = { command: "gh" as const, cwd: input.cwd, cause: undefined }; - const headers = result.stdout.split(/\r?\n\r?\n/, 1)[0] ?? ""; - const header = (name: string) => - new RegExp(`^${name}:\\s*(.*)$`, "im").exec(headers)?.[1]?.trim(); - if ( - status === "429" || - (status === "403" && - (header("x-ratelimit-remaining") === "0" || - header("retry-after") !== undefined || - /rate limit/i.test(result.stderr))) - ) { - const now = DateTime.toEpochMillis(yield* DateTime.now); - const reset = Number(header("x-ratelimit-reset")) * 1_000; - const retryAt = - SourceControlRateLimit.retryAtFromHeader(header("retry-after"), now) ?? - (Number.isFinite(reset) && reset > now ? reset : undefined); - return yield* new GitHubCliRateLimitError({ - ...context, - ...(retryAt === undefined ? {} : { retryAt }), - }); - } - if (status === "401") return yield* new GitHubCliAuthenticationError(context); - return yield* new GitHubCliCommandError({ - ...context, - ...(status === undefined ? {} : { httpStatus: Number(status) }), - }); - } - } - return result; - }, - ); - const gitRead = (cwd: string, args: ReadonlyArray) => process.run({ operation: "GitHubCli.resolveRepository", @@ -1170,7 +1037,6 @@ export const make = Effect.gen(function* () { }); return GitHubCli.of({ - execute, listPullRequestsByHead: (input) => AllowGitHubReserve.pipe( Effect.flatMap((allowReserve) => listByHead({ ...input, allowReserve })), diff --git a/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts b/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts index 15816a92eecb..16fb2730b32d 100644 --- a/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts +++ b/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts @@ -43,6 +43,7 @@ const restResponse = (body: string): GitHubApi.GitHubRestResponse => ({ headers: {}, body, truncated: false, + invalidUtf8: false, }); it.effect("maps GitHub PR summaries into provider-neutral change requests", () => diff --git a/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts b/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts index 82ef039bd763..39dbcaba04ff 100644 --- a/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts +++ b/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts @@ -315,6 +315,7 @@ it.effect( headers: {}, body: JSON.stringify({ title: "GitHub issue", body: null }), truncated: false, + invalidUtf8: false, }), }, gitlab: { From cb590f8344a9ec3c50ac382a08d5457c863b6796 Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Mon, 5 Oct 2026 20:17:07 -0700 Subject: [PATCH 5/9] refactor(server): source control maps credential errors by tag Co-Authored-By: Claude Opus 5.5 (1M context) --- apps/server/src/assets/AssetAccess.test.ts | 7 +------ apps/server/src/sourceControl/GitHubCli.test.ts | 5 +---- apps/server/src/sourceControl/GitHubCli.ts | 7 +++---- 3 files changed, 5 insertions(+), 14 deletions(-) diff --git a/apps/server/src/assets/AssetAccess.test.ts b/apps/server/src/assets/AssetAccess.test.ts index 35dc647c3045..12ec00256973 100644 --- a/apps/server/src/assets/AssetAccess.test.ts +++ b/apps/server/src/assets/AssetAccess.test.ts @@ -146,12 +146,7 @@ describe("AssetAccess", () => { Layer.mock(GitHubCredentials.GitHubCredentials)({ get: (host) => ++lookups === 1 - ? Effect.fail( - new GitHubCredentials.GitHubCredentialUnavailableError({ - host, - reason: "unauthenticated", - }), - ) + ? Effect.fail(new GitHubCredentials.GitHubNotSignedInError({ host })) : Effect.succeed({ host, token: Redacted.make("signed-in"), diff --git a/apps/server/src/sourceControl/GitHubCli.test.ts b/apps/server/src/sourceControl/GitHubCli.test.ts index 4c1099960fa0..4ea30112d93e 100644 --- a/apps/server/src/sourceControl/GitHubCli.test.ts +++ b/apps/server/src/sourceControl/GitHubCli.test.ts @@ -337,10 +337,7 @@ describe("GitHubCli.listPullRequestsByHead", () => { graphql: (input) => Effect.fail( input.variables?.h0 === "missing" - ? new GitHubCredentials.GitHubCredentialUnavailableError({ - host: "github.com", - reason: "cli-missing", - }) + ? new GitHubCredentials.GitHubCliMissingError({ host: "github.com" }) : new GitHubApi.GitHubApiRateLimitError({ host: "github.com", operation: "x", diff --git a/apps/server/src/sourceControl/GitHubCli.ts b/apps/server/src/sourceControl/GitHubCli.ts index f124ca04b76c..886e420ad54e 100644 --- a/apps/server/src/sourceControl/GitHubCli.ts +++ b/apps/server/src/sourceControl/GitHubCli.ts @@ -219,10 +219,9 @@ export function fromVcsError( export function fromGitHubApiError(cwd: string, error: GitHubApi.GitHubApiError): GitHubCliError { const context = { command: "gh" as const, cwd, cause: error }; switch (error._tag) { - case "GitHubCredentialUnavailableError": - return error.reason === "cli-missing" - ? new GitHubCliUnavailableError(context) - : new GitHubCliAuthenticationError(context); + case "GitHubCliMissingError": + return new GitHubCliUnavailableError(context); + case "GitHubNotSignedInError": case "GitHubApiAuthenticationError": return new GitHubCliAuthenticationError(context); case "GitHubApiRateLimitError": From 1da484ef854a82fff4c8bac907e82e90d1daf4f9 Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Mon, 5 Oct 2026 20:54:16 -0700 Subject: [PATCH 6/9] refactor(server): source control errors carry their sentence in message; GitHub env reads go through HostProcessEnvironment Co-Authored-By: Claude Opus 5.5 (1M context) --- apps/server/src/sourceControl/GitHubCli.ts | 69 +++++-------------- .../GitHubSourceControlProvider.ts | 25 ++++--- 2 files changed, 30 insertions(+), 64 deletions(-) diff --git a/apps/server/src/sourceControl/GitHubCli.ts b/apps/server/src/sourceControl/GitHubCli.ts index 886e420ad54e..149f594b32ba 100644 --- a/apps/server/src/sourceControl/GitHubCli.ts +++ b/apps/server/src/sourceControl/GitHubCli.ts @@ -24,6 +24,8 @@ import { } from "@t3tools/shared/sourceControl"; import * as GitVcsDriver from "../vcs/GitVcsDriver.ts"; +import { HostProcessEnvironment } from "@t3tools/shared/hostProcess"; + import * as VcsProcess from "../vcs/VcsProcess.ts"; import * as GitHubApi from "./GitHubApi.ts"; import * as GitHubCredentials from "./GitHubCredentials.ts"; @@ -49,12 +51,8 @@ export class GitHubCliUnavailableError extends Schema.TaggedError locator.host), - Effect.orElseSucceed(() => globalThis.process.env.GH_HOST ?? "github.com"), + Effect.orElseSucceed(() => environment.GH_HOST ?? "github.com"), )).toLowerCase(); const locator = parseGitHubRepositorySelector(input.repository, fallbackHost); if (locator === null) { @@ -1066,7 +1029,7 @@ export const make = Effect.gen(function* () { Effect.gen(function* () { const locator = parseGitHubRepositorySelector( input.repository, - (globalThis.process.env.GH_HOST ?? "github.com").toLowerCase(), + (environment.GH_HOST ?? "github.com").toLowerCase(), ); const viewer = locator === null ? null : yield* readViewerLogin(input.cwd, locator.host); const owner = locator?.owner ?? viewer; diff --git a/apps/server/src/sourceControl/GitHubSourceControlProvider.ts b/apps/server/src/sourceControl/GitHubSourceControlProvider.ts index d9c4b9388642..0a73b862b5ec 100644 --- a/apps/server/src/sourceControl/GitHubSourceControlProvider.ts +++ b/apps/server/src/sourceControl/GitHubSourceControlProvider.ts @@ -9,6 +9,8 @@ import { type SourceControlProviderDiscoveryItem, } from "@t3tools/contracts"; +import { HostProcessEnvironment } from "@t3tools/shared/hostProcess"; + import * as GitHubApi from "./GitHubApi.ts"; import * as GitHubCli from "./GitHubCli.ts"; import { findAuthenticatedGitHubAccount, parseGitHubAuthStatus } from "./gitHubAuthStatus.ts"; @@ -125,8 +127,8 @@ const decodeViewer = Schema.decodeUnknownOption( ); /** The environment variable gh would take a github.com token from, if one is set. */ -function environmentTokenVariable(): string | null { - return ["GH_TOKEN", "GITHUB_TOKEN"].find((name) => globalThis.process.env[name]?.trim()) ?? null; +function environmentTokenVariable(environment: NodeJS.ProcessEnv): string | null { + return ["GH_TOKEN", "GITHUB_TOKEN"].find((name) => environment[name]?.trim()) ?? null; } /** @@ -136,6 +138,7 @@ function environmentTokenVariable(): string | null { export const makeDiscovery = Effect.gen(function* () { const api = yield* GitHubApi.GitHubApi; const process = yield* VcsProcess.VcsProcess; + const environment = yield* HostProcessEnvironment; return { type: "managed-cli", kind: discovery.kind, @@ -143,7 +146,7 @@ export const makeDiscovery = Effect.gen(function* () { installHint: discovery.installHint, probe: Effect.fn("GitHubSourceControlProvider.discovery")(function* (cwd: string) { const cli = yield* probeSourceControlProvider({ cwd, process, spec: discovery }); - const variable = environmentTokenVariable(); + const variable = environmentTokenVariable(environment); if (variable === null) return cli; const viewer = yield* api .rest({ host: "github.com", operation: "discovery", path: "user" }) @@ -201,7 +204,7 @@ export const make = Effect.gen(function* () { reference: SourceControlProvider.transportSafeSourceControlErrorValue( input.headSelector, ), - detail: error.detail, + detail: error.message, cause: error, }), ), @@ -240,7 +243,7 @@ export const make = Effect.gen(function* () { reference: SourceControlProvider.transportSafeSourceControlErrorValue( input.headSelector, ), - detail: error.detail, + detail: error.message, cause: error, }), ), @@ -318,7 +321,7 @@ export const make = Effect.gen(function* () { reference: SourceControlProvider.transportSafeSourceControlErrorValue( input.reference, ), - detail: error.detail, + detail: error.message, cause: error, }), ), @@ -343,7 +346,7 @@ export const make = Effect.gen(function* () { reference: SourceControlProvider.transportSafeSourceControlErrorValue( input.headSelector, ), - detail: error.detail, + detail: error.message, cause: error, }), ), @@ -360,7 +363,7 @@ export const make = Effect.gen(function* () { repository: SourceControlProvider.transportSafeSourceControlErrorValue( input.repository, ), - detail: error.detail, + detail: error.message, cause: error, }), ), @@ -377,7 +380,7 @@ export const make = Effect.gen(function* () { repository: SourceControlProvider.transportSafeSourceControlErrorValue( input.repository, ), - detail: error.detail, + detail: error.message, cause: error, }), ), @@ -398,7 +401,7 @@ export const make = Effect.gen(function* () { operation: "getDefaultBranch", command: error.command, cwd: input.cwd, - detail: error.detail, + detail: error.message, cause: error, }), ), @@ -415,7 +418,7 @@ export const make = Effect.gen(function* () { reference: SourceControlProvider.transportSafeSourceControlErrorValue( input.reference, ), - detail: error.detail, + detail: error.message, cause: error, }), ), From 56093efc3ebed1abaf83f75dffd14cc97d9417c2 Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Mon, 5 Oct 2026 21:45:35 -0700 Subject: [PATCH 7/9] chore(server): drop gh-only helpers and exports nothing uses anymore Co-Authored-By: Claude Opus 5.5 (1M context) --- apps/server/src/sourceControl/GitHubCli.ts | 51 +++---------------- .../src/sourceControl/gitHubPullRequests.ts | 11 ---- 2 files changed, 6 insertions(+), 56 deletions(-) diff --git a/apps/server/src/sourceControl/GitHubCli.ts b/apps/server/src/sourceControl/GitHubCli.ts index 149f594b32ba..bfb9d70c8022 100644 --- a/apps/server/src/sourceControl/GitHubCli.ts +++ b/apps/server/src/sourceControl/GitHubCli.ts @@ -5,17 +5,12 @@ import * as Exit from "effect/Exit"; import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; -import * as PlatformError from "effect/PlatformError"; import * as Request from "effect/Request"; import * as RequestResolver from "effect/RequestResolver"; import * as Result from "effect/Result"; import * as Schema from "effect/Schema"; -import { - TrimmedNonEmptyString, - type SourceControlRepositoryVisibility, - type VcsError, -} from "@t3tools/contracts"; +import { TrimmedNonEmptyString, type SourceControlRepositoryVisibility } from "@t3tools/contracts"; import { normalizeGitRemoteUrl } from "@t3tools/shared/git"; import { decodeJsonResult } from "@t3tools/shared/schemaJson"; import { @@ -36,9 +31,6 @@ import { type NormalizedGitHubPullRequestRecord, } from "./gitHubPullRequests.ts"; -/** Server-local credential scope; never put its value in RPC payloads or cache keys. */ -export const PinnedGitHubCredential = GitHubApi.PinnedGitHubCredential; - export const AllowGitHubReserve = GitHubApi.AllowGitHubReserve; const gitHubCliFailureFields = { @@ -149,40 +141,8 @@ export type GitHubCliError = typeof GitHubCliError.Type; export const isGitHubCliError = Schema.is(GitHubCliError); -export function fromVcsError( - context: { - readonly command: "gh"; - readonly cwd: string; - }, - error: VcsError, -): GitHubCliError { - if ( - error._tag === "VcsProcessSpawnError" && - error.cause instanceof PlatformError.PlatformError && - error.cause.reason._tag === "NotFound" && - error.cause.reason.module === "ChildProcess" && - error.cause.reason.method === "spawn" - ) { - return new GitHubCliUnavailableError({ ...context, cause: error }); - } - - if (error._tag === "VcsProcessExitError") { - if (error.failureKind === "authentication") { - return new GitHubCliAuthenticationError({ ...context, cause: error }); - } - if (error.failureKind === "rate-limited") { - return new GitHubCliRateLimitError({ ...context, cause: error }); - } - if (error.failureKind === "not-found") { - return new GitHubPullRequestNotFoundError({ ...context, cause: error }); - } - } - - return new GitHubCliCommandError({ ...context, cause: error }); -} - /** Maps a GitHub API failure onto the errors callers of this service already handle. */ -export function fromGitHubApiError(cwd: string, error: GitHubApi.GitHubApiError): GitHubCliError { +function fromGitHubApiError(cwd: string, error: GitHubApi.GitHubApiError): GitHubCliError { const context = { command: "gh" as const, cwd, cause: error }; switch (error._tag) { case "GitHubCliMissingError": @@ -465,7 +425,7 @@ export interface GitHubRepositoryLocator { } /** `owner/name` or `host/owner/name`, as `gh --repo` and GH_REPO take them. */ -export function parseGitHubRepositorySelector( +function parseGitHubRepositorySelector( selector: string, defaultHost: string, ): GitHubRepositoryLocator | null { @@ -492,7 +452,7 @@ export function parseGitHubRepositorySelector( * A pull request reference the way `gh pr view` takes one: a number (`#7` too), a pull request * URL, or a branch name. */ -export function parsePullRequestReference( +function parsePullRequestReference( reference: string, ): | { readonly kind: "number"; readonly number: number } @@ -698,7 +658,8 @@ export const make = Effect.gen(function* () { request.owner, request.name, String(request.allowReserve), - Context.getOrElse(context, PinnedGitHubCredential, () => null)?.credentialFingerprint ?? "", + Context.getOrElse(context, GitHubApi.PinnedGitHubCredential, () => null) + ?.credentialFingerprint ?? "", Context.getOrElse(context, SourceControlRateLimit.CredentialScope, () => ""), ].join("\0"), resolver: (entries) => { diff --git a/apps/server/src/sourceControl/gitHubPullRequests.ts b/apps/server/src/sourceControl/gitHubPullRequests.ts index c29a3806e13d..2309993f0300 100644 --- a/apps/server/src/sourceControl/gitHubPullRequests.ts +++ b/apps/server/src/sourceControl/gitHubPullRequests.ts @@ -111,7 +111,6 @@ function normalizeGitHubPullRequestRecord( } const decodeGitHubPullRequestList = decodeJsonResult(Schema.Array(Schema.Unknown)); -const decodeGitHubPullRequest = decodeJsonResult(GitHubPullRequestSchema); const decodeGitHubPullRequestEntry = Schema.decodeUnknownExit(GitHubPullRequestSchema); /** @@ -139,13 +138,3 @@ export function decodeGitHubPullRequestListJson( > { return Result.map(decodeGitHubPullRequestList(raw), decodeGitHubPullRequestEntries); } - -export function decodeGitHubPullRequestJson( - raw: string, -): Result.Result> { - const result = decodeGitHubPullRequest(raw); - if (Result.isSuccess(result)) { - return Result.succeed(normalizeGitHubPullRequestRecord(result.success)); - } - return Result.fail(result.failure); -} From 86f18022ec41ff5330aebf24a5eef1e797823929 Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Mon, 5 Oct 2026 23:38:08 -0700 Subject: [PATCH 8/9] fix(server): PR checkout, source control errors and discovery hold up from review - A fork checkout fails when the base's default branch cannot be read. The plain head name could be the local default branch, which a forced checkout would reset to the fork's commit. - An existing branch keeps its upstream until the checkout's sync succeeds. - A provider host hint that is a dotless SSH alias (`github`) resolves to github.com, the same as the remote does. - GitHubCliCommandError says its cause's reason (GitHub's 422 message, the git step that failed) instead of only "GitHub request failed". - Pull requests are created with maintainer_can_modify, as gh does. - A bare repository name is created on GH_HOST, as gh does. - Discovery only blames an env token when GitHub refused it. - A transient gh failure maps to a command error. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/sourceControl/GitHubCli.test.ts | 40 ++++++++++++++++--- apps/server/src/sourceControl/GitHubCli.ts | 40 +++++++++++++++---- .../GitHubSourceControlProvider.ts | 17 +++++--- 3 files changed, 79 insertions(+), 18 deletions(-) diff --git a/apps/server/src/sourceControl/GitHubCli.test.ts b/apps/server/src/sourceControl/GitHubCli.test.ts index 4ea30112d93e..cb4043b195c2 100644 --- a/apps/server/src/sourceControl/GitHubCli.test.ts +++ b/apps/server/src/sourceControl/GitHubCli.test.ts @@ -206,7 +206,7 @@ describe("GitHubCli repository resolution", () => { it.effect("reads an SSH alias remote through github.com", () => { const hosts: string[] = []; const { layer } = harness({ - remotes: remotesOutput(["origin", "git@github-work:acme/web.git"]), + remotes: remotesOutput(["origin", "git@github:acme/web.git"]), api: { rest: (input) => Effect.sync(() => { @@ -223,7 +223,9 @@ describe("GitHubCli repository resolution", () => { return Effect.gen(function* () { const gh = yield* GitHubCli.GitHubCli; yield* gh.getDefaultBranch({ cwd: "/repo" }); - assert.deepStrictEqual(hosts, ["github.com repos/acme/web"]); + // A provider's host hint for the same alias (`github` here) resolves the same way. + yield* gh.getDefaultBranch({ cwd: "/repo", rateLimitHost: "github" }); + assert.deepStrictEqual(hosts, ["github.com repos/acme/web", "github.com repos/acme/web"]); assert.strictEqual( GitHubCli.gitHubApiHostForRemote("git@github.example.com:a/b.git"), "github.example.com", @@ -441,6 +443,7 @@ describe("GitHubCli writes", () => { head: "me:feature", title: "Title", body: "Body", + maintainer_can_modify: true, }); }).pipe(Effect.provide(layer), Effect.provide(NodeServices.layer), Effect.scoped); }); @@ -508,15 +511,42 @@ describe("GitHubCli.checkoutPullRequest", () => { { cwd: "/repo", remoteName: "origin", remoteBranch: "feature/x" }, ], ["execute", ["branch", "feature/x", "refs/remotes/origin/feature/x"]], + ["switchRef", { cwd: "/repo", refName: "feature/x" }], [ "setBranchUpstream", { cwd: "/repo", branch: "feature/x", remoteName: "origin", remoteBranch: "feature/x" }, ], - ["switchRef", { cwd: "/repo", refName: "feature/x" }], ]); }).pipe(Effect.provide(layer)); }); + it.effect("refuses a fork checkout when the base's default branch cannot be read", () => { + const { layer, git } = harness({ + remotes: remotesOutput(["origin", "git@github.com:acme/web.git"]), + localBranches: ["main"], + api: { + graphql: () => + Effect.succeed( + encodeJson({ data: { repository: { pullRequest: node(6, "main", "someone") } } }), + ), + rest: () => + Effect.fail( + new GitHubApi.GitHubApiRequestError({ + host: "github.com", + operation: "x", + cause: "offline", + }), + ), + }, + }); + return Effect.gen(function* () { + const gh = yield* GitHubCli.GitHubCli; + yield* Effect.flip(gh.checkoutPullRequest({ cwd: "/repo", reference: "6", force: true })); + // Nothing touched the local branches: `main` must not be reset to the fork's commit. + assert.deepStrictEqual(git, []); + }).pipe(Effect.provide(layer)); + }); + it.effect("adds a remote for a fork and names a default-branch head after its owner", () => { const { layer, git } = harness({ remotes: remotesOutput(["origin", "git@github.com:acme/web.git"]), @@ -542,12 +572,12 @@ describe("GitHubCli.checkoutPullRequest", () => { "fetchRemoteTrackingBranch", { cwd: "/repo", remoteName: "someone", remoteBranch: "main" }, ], + ["switchRef", { cwd: "/repo", refName: "someone/main" }], + ["execute", ["reset", "--hard", "--quiet", "refs/remotes/someone/main"]], [ "setBranchUpstream", { cwd: "/repo", branch: "someone/main", remoteName: "someone", remoteBranch: "main" }, ], - ["switchRef", { cwd: "/repo", refName: "someone/main" }], - ["execute", ["reset", "--hard", "--quiet", "refs/remotes/someone/main"]], ]); }).pipe(Effect.provide(layer)); }); diff --git a/apps/server/src/sourceControl/GitHubCli.ts b/apps/server/src/sourceControl/GitHubCli.ts index bfb9d70c8022..95501d3c163a 100644 --- a/apps/server/src/sourceControl/GitHubCli.ts +++ b/apps/server/src/sourceControl/GitHubCli.ts @@ -80,7 +80,12 @@ export class GitHubCliCommandError extends Schema.TaggedError remote.name === "origin" && remote.host !== null)?.host ?? fetchRemotes.find((remote) => remote.host !== null)?.host ?? defaultHost; @@ -872,10 +885,12 @@ export const make = Effect.gen(function* () { const known = remoteFor(baseNameWithOwner); return known === null ? git.resolvePrimaryRemoteName(input.cwd) : Effect.succeed(known); }); + // A fork's branch named like the base's default branch is checked out under the owner's + // prefix. Without the default branch that collision cannot be ruled out, and the checkout + // would reset the local default branch to the fork's commit, so it fails instead. const defaultBranch = isCrossRepository ? yield* readRepository(input.cwd, base).pipe( Effect.map((repository) => repository.default_branch ?? null), - Effect.orElseSucceed(() => null), ) : null; const localBranch = pullRequestCheckoutBranchName({ @@ -943,9 +958,6 @@ export const make = Effect.gen(function* () { yield* Effect.gen(function* () { if (!exists) yield* runGit(input.cwd, "branch", ["branch", localBranch, target.ref]); - if (target.upstream !== null) { - yield* git.setBranchUpstream({ cwd: input.cwd, branch: localBranch, ...target.upstream }); - } yield* Effect.scoped(git.switchRef({ cwd: input.cwd, refName: localBranch })); if (exists) { yield* runGit( @@ -956,6 +968,11 @@ export const make = Effect.gen(function* () { : ["merge", "--ff-only", "--quiet", target.ref], ); } + // Tracking is set once the branch is the pull request's, so a sync that fails leaves an + // existing branch's upstream as it was. + if (target.upstream !== null) { + yield* git.setBranchUpstream({ cwd: input.cwd, branch: localBranch, ...target.upstream }); + } }).pipe(Effect.mapError(gitFailure(input.cwd))); }); @@ -995,7 +1012,7 @@ export const make = Effect.gen(function* () { const viewer = locator === null ? null : yield* readViewerLogin(input.cwd, locator.host); const owner = locator?.owner ?? viewer; const name = locator?.name ?? input.repository.trim(); - const host = locator?.host ?? "github.com"; + const host = locator?.host ?? (environment.GH_HOST?.trim().toLowerCase() || "github.com"); const isViewer = viewer !== null && owner?.toLowerCase() === viewer.toLowerCase(); const response = yield* rest(input.cwd, { host, @@ -1027,7 +1044,14 @@ export const make = Effect.gen(function* () { method: "POST", path: `repos/${encodeURIComponent(locator.owner)}/${encodeURIComponent(locator.name)}/pulls`, // `owner:branch` is how the REST API takes a fork's head, the same as `gh --head`. - body: { base: input.baseBranch, head: input.headSelector, title: input.title, body }, + // gh allows maintainer edits unless told otherwise; the API's default is not documented. + body: { + base: input.baseBranch, + head: input.headSelector, + title: input.title, + body, + maintainer_can_modify: true, + }, }); }), getDefaultBranch: (input) => diff --git a/apps/server/src/sourceControl/GitHubSourceControlProvider.ts b/apps/server/src/sourceControl/GitHubSourceControlProvider.ts index 0a73b862b5ec..8e183e8b9ace 100644 --- a/apps/server/src/sourceControl/GitHubSourceControlProvider.ts +++ b/apps/server/src/sourceControl/GitHubSourceControlProvider.ts @@ -165,11 +165,18 @@ export const makeDiscovery = Effect.gen(function* () { host: "github.com", detail: `Using the token in ${variable} from the server environment.`, }) - : providerAuth({ - status: "unauthenticated", - host: "github.com", - detail: `GitHub refused the token in ${variable}. Replace it, or unset it to use \`gh auth login\`.`, - }), + : Result.isFailure(viewer) && viewer.failure._tag !== "GitHubApiAuthenticationError" + ? // Only a refusal says the token is bad; a network error or a pause says nothing. + providerAuth({ + status: "unknown", + host: "github.com", + detail: `Could not check the token in ${variable}: ${viewer.failure.message}`, + }) + : providerAuth({ + status: "unauthenticated", + host: "github.com", + detail: `GitHub refused the token in ${variable}. Replace it, or unset it to use \`gh auth login\`.`, + }), } satisfies SourceControlProviderDiscoveryItem; }), refineUnknownRemote: () => Effect.succeed(null), From 4007410457897033b3beb7d84b11bb9c3a05db42 Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Tue, 6 Oct 2026 00:17:31 -0700 Subject: [PATCH 9/9] chore(server): satisfy Effect layer diagnostics after the GitHub API move GitHubCli now needs GitVcsDriver, so server.ts provides it after the source control registry instead of beside it, and tests merge their mocks into one provide. Co-Authored-By: Claude Opus 5.5 (1M context) --- apps/server/src/git/GitManager.test.ts | 8 ++++++-- apps/server/src/server.ts | 3 ++- apps/server/src/sourceControl/GitHubCli.test.ts | 2 +- .../src/sourceControl/GitHubSourceControlProvider.test.ts | 5 +++-- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index 6b4b5cf0578b..42fdc87afe57 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -728,8 +728,12 @@ function makeManager(input?: { discover: Effect.succeed([]), }), ), - Effect.provide(Layer.succeed(GitHubCli.GitHubCli, gitHubCli)), - Effect.provide(Layer.mock(GitHubApi.GitHubApi)({})), + Effect.provide( + Layer.merge( + Layer.succeed(GitHubCli.GitHubCli, gitHubCli), + Layer.mock(GitHubApi.GitHubApi)({}), + ), + ), ), ); diff --git a/apps/server/src/server.ts b/apps/server/src/server.ts index 3f528cd85f15..be6cf84dd81e 100644 --- a/apps/server/src/server.ts +++ b/apps/server/src/server.ts @@ -558,7 +558,8 @@ const layerRuntimeCoreDependenciesBase = Layer.mergeAll( Layer.provideMerge(Layer.merge(ProjectStore.layer, ThreadSearch.layer)), Layer.provideMerge(layerServerSettings), // The asset route uses the registry's GitHub credential for private PR media. - Layer.provideMerge(Layer.mergeAll(layerSourceControlProviderRegistry, GitHubCli.layer)), + Layer.provideMerge(layerSourceControlProviderRegistry), + Layer.provideMerge(GitHubCli.layer), Layer.provideMerge(layerGit), Layer.provideMerge(layerVcs), Layer.provideMerge(Layer.mergeAll(layerTerminal, layerPreview, layerDevice)), diff --git a/apps/server/src/sourceControl/GitHubCli.test.ts b/apps/server/src/sourceControl/GitHubCli.test.ts index cb4043b195c2..2efab4674fc2 100644 --- a/apps/server/src/sourceControl/GitHubCli.test.ts +++ b/apps/server/src/sourceControl/GitHubCli.test.ts @@ -445,7 +445,7 @@ describe("GitHubCli writes", () => { body: "Body", maintainer_can_modify: true, }); - }).pipe(Effect.provide(layer), Effect.provide(NodeServices.layer), Effect.scoped); + }).pipe(Effect.provide(Layer.merge(layer, NodeServices.layer)), Effect.scoped); }); it.effect("creates a repository under an organization the viewer is not", () => { diff --git a/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts b/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts index 16fb2730b32d..6376e2d868e5 100644 --- a/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts +++ b/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts @@ -31,8 +31,9 @@ function makeProvider( api: Partial = {}, ) { return GitHubSourceControlProvider.make.pipe( - Effect.provide(Layer.mock(GitHubCli.GitHubCli)(github)), - Effect.provide(Layer.mock(GitHubApi.GitHubApi)(api)), + Effect.provide( + Layer.merge(Layer.mock(GitHubCli.GitHubCli)(github), Layer.mock(GitHubApi.GitHubApi)(api)), + ), ); }