From 3bfbe59c2de1d96070f3761ff7342d53649b6d58 Mon Sep 17 00:00:00 2001 From: Bertrand Date: Sat, 19 Sep 2026 15:42:26 -0700 Subject: [PATCH 1/2] fix(server): recognize GitHub HTTP 404 stack responses --- .../pullRequest/GitHubPullRequestCli.test.ts | 56 +++++++++++++------ apps/server/src/vcs/VcsProcess.ts | 1 + 2 files changed, 41 insertions(+), 16 deletions(-) diff --git a/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts b/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts index fd5ab04e8a81..41619fad564c 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts @@ -9,6 +9,7 @@ import * as TestClock from "effect/testing/TestClock"; import { ChildProcessSpawner } from "effect/unstable/process"; import * as GitHubCli from "../sourceControl/GitHubCli.ts"; +import * as ProcessRunner from "../processRunner.ts"; import * as VcsProcess from "../vcs/VcsProcess.ts"; import * as SourceControlRateLimit from "../sourceControl/SourceControlRateLimit.ts"; import * as GitHubGraphQlBudget from "../sourceControl/githubGraphQlBudget.ts"; @@ -650,28 +651,51 @@ layer("GitHubPullRequestCli.layer", (it) => { }), ); - it.effect("reads a host that refuses the stacks preview as not stacked", () => + it.effect.each([ + ["gh: Not Found (HTTP 404)", null], + ["HTTP 404: Not Found (https://github.example/api/v3/repos/acme/web/stacks)", null], + ["gh: Resource not accessible by integration (HTTP 403)", "GitHubCliCommandError"], + ["gh: Service Unavailable (HTTP 503)", "GitHubCliCommandError"], + ["dial tcp: lookup github.example: no such host", "GitHubCliCommandError"], + ["To get started with GitHub CLI, please run: gh auth login", "GitHubCliAuthenticationError"], + ["gh: Too Many Requests (HTTP 429)", "GitHubCliRateLimitError"], + ] as const)("classifies the raw stacks API failure: %s", ([stderr, expectedError]) => Effect.gen(function* () { - // The CLI classifies a missing preview endpoint as not found. - mockedExecute.mockReturnValueOnce( - Effect.fail( - new GitHubCli.GitHubPullRequestNotFoundError({ - command: "gh", - cwd: "/w", - cause: new Error("HTTP 404: Not Found (https://api.github.com/repos/acme/web/stacks)"), - }), - ), + const vcs = yield* VcsProcess.make.pipe( + Effect.provideService(ProcessRunner.ProcessRunner, { + run: () => + Effect.succeed({ + stdout: "", + stderr, + code: ChildProcessSpawner.ExitCode(1), + timedOut: false, + stdoutTruncated: false, + stderrTruncated: false, + stdoutInvalidUtf8: false, + stderrInvalidUtf8: false, + }), + }), ); - const cli = yield* GitHubPullRequestCli.GitHubPullRequestCli; - - const stack = yield* cli.getPullRequestStack({ + const github = yield* GitHubCli.make.pipe( + Effect.provideService(VcsProcess.VcsProcess, vcs), + Effect.provide(Layer.merge(GitHubGraphQlBudget.layer, SourceControlRateLimit.layer)), + ); + const cli = yield* GitHubPullRequestCli.make.pipe( + Effect.provideService(GitHubCli.GitHubCli, github), + Effect.provide(GitHubGraphQlBudget.layer), + ); + const read = cli.getPullRequestStack({ cwd: "/w", repository: "acme/web", - host: "github.com", + host: "github.example", number: 7, }); - - assert.isNull(stack); + if (expectedError === null) { + assert.isNull(yield* read); + } else { + const error = yield* Effect.flip(read); + assert.strictEqual(error._tag, expectedError); + } }), ); diff --git a/apps/server/src/vcs/VcsProcess.ts b/apps/server/src/vcs/VcsProcess.ts index 25f3a23c368a..f4d54fd51bcf 100644 --- a/apps/server/src/vcs/VcsProcess.ts +++ b/apps/server/src/vcs/VcsProcess.ts @@ -93,6 +93,7 @@ const classifyNonZeroExit = (command: string, stderr: string): VcsProcessExitFai (normalized.includes("could not resolve to a pullrequest") || normalized.includes("repository.pullrequest") || normalized.includes("no pull requests found for branch") || + normalized.includes("http 404") || normalized.includes("pull request not found"))) || (command === "glab" && (normalized.includes("merge request not found") || From bedb2a19bad6f05fc39edf9189f946a12b8fea37 Mon Sep 17 00:00:00 2001 From: Bertrand Date: Sat, 19 Sep 2026 16:08:10 -0700 Subject: [PATCH 2/2] fix(server): verify PR access before stacks fallback --- .../pullRequest/GitHubPullRequestCli.test.ts | 138 +++++++++++++----- .../src/pullRequest/GitHubPullRequestCli.ts | 22 ++- 2 files changed, 122 insertions(+), 38 deletions(-) diff --git a/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts b/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts index 41619fad564c..6d02a3b5a6ea 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestCli.test.ts @@ -652,50 +652,122 @@ layer("GitHubPullRequestCli.layer", (it) => { ); it.effect.each([ - ["gh: Not Found (HTTP 404)", null], - ["HTTP 404: Not Found (https://github.example/api/v3/repos/acme/web/stacks)", null], - ["gh: Resource not accessible by integration (HTTP 403)", "GitHubCliCommandError"], - ["gh: Service Unavailable (HTTP 503)", "GitHubCliCommandError"], - ["dial tcp: lookup github.example: no such host", "GitHubCliCommandError"], - ["To get started with GitHub CLI, please run: gh auth login", "GitHubCliAuthenticationError"], - ["gh: Too Many Requests (HTTP 429)", "GitHubCliRateLimitError"], - ] as const)("classifies the raw stacks API failure: %s", ([stderr, expectedError]) => - Effect.gen(function* () { - const vcs = yield* VcsProcess.make.pipe( - Effect.provideService(ProcessRunner.ProcessRunner, { - run: () => + ["gh: Not Found (HTTP 404)", "", null], + ["HTTP 404: Not Found (https://github.example/api/v3/repos/acme/web/stacks)", "", null], + ["gh: Resource not accessible by integration (HTTP 403)", undefined, "GitHubCliCommandError"], + ["gh: Service Unavailable (HTTP 503)", undefined, "GitHubCliCommandError"], + ["dial tcp: lookup github.example: no such host", undefined, "GitHubCliCommandError"], + [ + "To get started with GitHub CLI, please run: gh auth login", + undefined, + "GitHubCliAuthenticationError", + ], + ["gh: Too Many Requests (HTTP 429)", undefined, "GitHubCliRateLimitError"], + ["gh: Not Found (HTTP 404)", "gh: Not Found (HTTP 404)", "GitHubPullRequestNotFoundError"], + ["gh: Not Found (HTTP 404)", "gh: Bad credentials (HTTP 401)", "GitHubCliCommandError"], + [ + "gh: Not Found (HTTP 404)", + "gh: Resource not accessible by integration (HTTP 403)", + "GitHubCliCommandError", + ], + [ + "gh: Not Found (HTTP 404)", + "To get started with GitHub CLI, please run: gh auth login", + "GitHubCliAuthenticationError", + ], + ["gh: Not Found (HTTP 404)", "gh: Service Unavailable (HTTP 503)", "GitHubCliCommandError"], + ["gh: Not Found (HTTP 404)", "gh: Too Many Requests (HTTP 429)", "GitHubCliRateLimitError"], + ] as const)( + "classifies the raw stacks API failure: %s; PR access: %s", + ([stderr, accessStderr, expectedError]) => + Effect.gen(function* () { + const run = vi.fn(); + for (const response of accessStderr === undefined ? [stderr] : [stderr, accessStderr]) { + run.mockReturnValueOnce( Effect.succeed({ stdout: "", - stderr, - code: ChildProcessSpawner.ExitCode(1), + stderr: response, + code: ChildProcessSpawner.ExitCode(response === "" ? 0 : 1), timedOut: false, stdoutTruncated: false, stderrTruncated: false, stdoutInvalidUtf8: false, stderrInvalidUtf8: false, }), - }), - ); - const github = yield* GitHubCli.make.pipe( - Effect.provideService(VcsProcess.VcsProcess, vcs), - Effect.provide(Layer.merge(GitHubGraphQlBudget.layer, SourceControlRateLimit.layer)), - ); - const cli = yield* GitHubPullRequestCli.make.pipe( - Effect.provideService(GitHubCli.GitHubCli, github), - Effect.provide(GitHubGraphQlBudget.layer), + ); + } + const vcs = yield* VcsProcess.make.pipe( + Effect.provideService(ProcessRunner.ProcessRunner, { run }), + ); + const github = yield* GitHubCli.make.pipe( + Effect.provideService(VcsProcess.VcsProcess, vcs), + Effect.provide(Layer.merge(GitHubGraphQlBudget.layer, SourceControlRateLimit.layer)), + ); + const cli = yield* GitHubPullRequestCli.make.pipe( + Effect.provideService(GitHubCli.GitHubCli, github), + Effect.provide(GitHubGraphQlBudget.layer), + ); + const read = cli.getPullRequestStack({ + cwd: "/w", + repository: "acme/web", + host: "github.example", + number: 7, + includeDetails: true, + }); + if (expectedError === null) { + assert.isNull(yield* read); + } else { + const error = yield* Effect.flip(read); + assert.strictEqual(error._tag, expectedError); + } + expect(run).toHaveBeenCalledTimes(accessStderr === undefined ? 1 : 2); + if (accessStderr !== undefined) { + expect(run.mock.calls[1]?.[0]?.args).toEqual([ + "api", + "--hostname", + "github.example", + "repos/acme/web/pulls/7/commits?per_page=1", + "--silent", + ]); + } + }), + ); + + it.effect("preserves a known stack when its detail request returns not found", () => + Effect.gen(function* () { + mockedExecute.mockReturnValueOnce( + Effect.succeed( + output( + encodeJson([ + { + number: 3, + url: "https://api.github.com/repos/acme/web/stacks/3", + base: { ref: "main" }, + pull_requests: [{ number: 7, head: { ref: "feat/two" }, state: "open" }], + }, + ]), + ), + ), ); - const read = cli.getPullRequestStack({ + const failure = new GitHubCli.GitHubPullRequestNotFoundError({ + command: "gh", cwd: "/w", - repository: "acme/web", - host: "github.example", - number: 7, + cause: new Error("gh: Not Found (HTTP 404)"), }); - if (expectedError === null) { - assert.isNull(yield* read); - } else { - const error = yield* Effect.flip(read); - assert.strictEqual(error._tag, expectedError); - } + mockedExecute.mockReturnValueOnce(Effect.fail(failure)); + const cli = yield* GitHubPullRequestCli.GitHubPullRequestCli; + const error = yield* Effect.flip( + cli.getPullRequestStack({ + cwd: "/w", + repository: "acme/web", + host: "github.com", + number: 7, + includeDetails: true, + }), + ); + + assert.strictEqual(error, failure); + expect(mockedExecute).toHaveBeenCalledTimes(2); }), ); diff --git a/apps/server/src/pullRequest/GitHubPullRequestCli.ts b/apps/server/src/pullRequest/GitHubPullRequestCli.ts index 723fcfc4d9b9..80679eb41909 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestCli.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestCli.ts @@ -2028,6 +2028,23 @@ export const make = Effect.gen(function* () { }), ); }), + // A listing 404 can mean an unsupported stacks API or denied access. The PR + // commits endpoint requires the same pull-request read permission as stacks. + Effect.catchTags({ + GitHubPullRequestNotFoundError: () => + github + .execute({ + cwd: input.cwd, + args: [ + "api", + "--hostname", + input.host, + `repos/${owner}/${name}/pulls/${input.number}/commits?per_page=1`, + "--silent", + ], + }) + .pipe(Effect.as(null)), + }), Effect.flatMap((stack) => { if (!input.includeDetails || stack === null) return Effect.succeed(stack); return github @@ -2056,11 +2073,6 @@ export const make = Effect.gen(function* () { }), ); }), - // Hosts without the stacks preview return 404. Other failures must preserve the - // previously synced stack and let the caller retry. - Effect.catchTags({ - GitHubPullRequestNotFoundError: () => Effect.succeed(null), - }), ); },