diff --git a/src/queue/processors.ts b/src/queue/processors.ts index f4623c4de7..49dfa2ff17 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -10807,6 +10807,17 @@ async function maybePublishPrPublicSurface( }); } } + // #8686: when both the primary publish and its fallback fail (or there was no pending id to fall + // back on), record the same failedOutputs entry check_run/comment/label already push — otherwise a + // gate-only repo silently leaves publishedOutputs+failedOutputs empty and finishPublicSurfacePublication + // skips pr_public_surface_failed / PostHog escalation / transient retry entirely. + if (!gateFinalized) { + failedOutputs.push({ + output: "gate_check_run", + error: gateCheckResult.warning, + transient: false, + }); + } } } catch (checkError) { if (isGitHubRateLimitedError(checkError)) throw checkError; @@ -10849,6 +10860,16 @@ async function maybePublishPrPublicSurface( }); } } + // #8686: final failure after fallback also failed (or was unavailable) — same failedOutputs contract + // as check_run so gate-only repos get operator-visible failure + transient retry when applicable. + if (!gateFinalized) { + const message = errorMessage(checkError); + failedOutputs.push({ + output: "gate_check_run", + error: message, + transient: isGitHubTransientPublishError(checkError), + }); + } await recordAuditEvent(env, { eventType: "github_app.gate_check_failed_nonfatal", actor: author, diff --git a/test/unit/queue-4.test.ts b/test/unit/queue-4.test.ts index a9d94eb0ea..e94ae733e6 100644 --- a/test/unit/queue-4.test.ts +++ b/test/unit/queue-4.test.ts @@ -1496,16 +1496,10 @@ describe("queue processors", () => { }); }); - it("does not stamp a gate-only surface when the incomplete-surface audit write fails", async () => { - const originalRecordAuditEvent = repositoriesModule.recordAuditEvent; - let incompleteAuditWrites = 0; - const auditSpy = vi.spyOn(repositoriesModule, "recordAuditEvent").mockImplementation(async (auditEnv, event) => { - if (event.eventType === "github_app.pr_public_surface_incomplete") { - incompleteAuditWrites += 1; - throw new Error("audit failed"); - } - await originalRecordAuditEvent(auditEnv, event); - }); + it("does not stamp a gate-only surface when gate publish fails transiently (#8686)", async () => { + // Gate-only repo: pending POST succeeds, completed PATCH + errored fallback both 500 → failedOutputs + // gets a transient gate_check_run entry and finishPublicSurfacePublication retries (no surface stamp). + // Pre-#8686 this path left failedOutputs empty and only wrote pr_public_surface_incomplete. const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); await persistRegistrySnapshot( asCloudEnv(env), @@ -1531,23 +1525,36 @@ describe("queue processors", () => { if (url.includes("/check-runs/973") && method === "PATCH") return new Response("check update failed", { status: 500 }); return new Response("not found", { status: 404 }); }); + const captureSpy = vi.spyOn(posthogModule, "capturePostHogReviewFailure"); - await processJob(env, { - type: "github-webhook", - deliveryId: "gate-missing-zero-output", - eventName: "pull_request", - payload: { - action: "opened", - installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } }, - repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, - pull_request: { number: 83, title: "Gate only missing", state: "open", user: { login: "contributor" }, head: { sha: "gate-zero-missing" }, labels: [], body: "No issue link." }, - }, - }); + await expect( + processJob(env, { + type: "github-webhook", + deliveryId: "gate-missing-zero-output", + eventName: "pull_request", + payload: { + action: "opened", + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + pull_request: { number: 83, title: "Gate only missing", state: "open", user: { login: "contributor" }, head: { sha: "gate-zero-missing" }, labels: [], body: "No issue link." }, + }, + }), + ).rejects.toMatchObject({ retryKind: "public_surface_publish_transient" }); - expect(incompleteAuditWrites).toBe(1); + const aggregate = await env.DB.prepare("select detail, metadata_json from audit_events where event_type = ?") + .bind("github_app.pr_public_surface_failed") + .first<{ detail: string; metadata_json: string }>(); + expect(aggregate).toMatchObject({ detail: "gate_check_run" }); + expect(aggregate?.metadata_json).toContain('"output":"gate_check_run"'); + expect(aggregate?.metadata_json).toContain('"transient":true'); + expect(captureSpy).toHaveBeenCalledWith( + expect.any(Error), + expect.objectContaining({ kind: "publish", repo: "JSONbored/gittensory", failedOutputs: ["gate_check_run"] }), + "pr_public_surface_publish_failed", + ); const stored = await getPullRequest(env, "JSONbored/gittensory", 83); expect(stored?.lastPublishedSurfaceSha ?? null).toBeNull(); - auditSpy.mockRestore(); + captureSpy.mockRestore(); }); it("does not stamp a comment surface when the incomplete-surface audit write fails", async () => { @@ -1751,6 +1758,142 @@ describe("queue processors", () => { expect(audit?.detail).toMatch(/Checks: write permission is missing/i); }); + it("records gate_check_run in failedOutputs when primary and fallback both fail with permission_missing (#8686)", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await persistRegistrySnapshot( + asCloudEnv(env), + normalizeRegistryPayload( + { "JSONbored/gittensory": { emission_share: 0.01, issue_discovery_share: 0 } }, + { kind: "raw-github", url: "https://example.test" }, + "2026-05-23T00:00:00.000Z", + ), + ); + await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, 123); + await upsertRepositorySettings(env, { + repoFullName: "JSONbored/gittensory", + autoLabelEnabled: false, + requireLinkedIssue: true, + }); + // Gate-only surface: no comment/label/check_run sibling can mask an empty failedOutputs. + await upsertRepoFocusManifest(env, "JSONbored/gittensory", { settings: { reviewCheckMode: "required", commentMode: "off", publicSurface: "off", checkRunMode: "off" } }); + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = init?.method ?? "GET"; + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/commits/gateDoubleFail/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/check-runs") && method === "POST") { + const body = JSON.parse(String(init?.body ?? "{}")) as { status?: string }; + // Pending in_progress succeeds so the permission_missing finalize path has a check id to fall back on. + if (body.status === "in_progress") { + return Response.json({ id: 501, html_url: "https://example.test/check/501" }, { status: 201 }); + } + // Primary completed publish → permission_missing (createOrUpdateGateCheckRun). + return new Response(JSON.stringify({ message: "Resource not accessible by integration" }), { status: 403 }); + } + // Fallback createOrUpdateErroredGateCheckRun PATCHes the pending id — also permission_missing. + if (url.includes("/check-runs/") && method === "PATCH") { + return new Response(JSON.stringify({ message: "Resource not accessible by integration" }), { status: 403 }); + } + return new Response("not found", { status: 404 }); + }); + const captureSpy = vi.spyOn(posthogModule, "capturePostHogReviewFailure"); + + await processJob(env, { + type: "github-webhook", + deliveryId: "gate-double-fail-permission", + eventName: "pull_request", + payload: { + action: "opened", + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + pull_request: { number: 86, title: "Gate without issue", state: "open", user: { login: "contributor" }, head: { sha: "gateDoubleFail" }, labels: [], body: "No issue link." }, + }, + }); + + const permissionAudit = await env.DB.prepare("select event_type from audit_events where event_type = ?") + .bind("github_app.gate_check_permission_missing") + .first<{ event_type: string }>(); + expect(permissionAudit?.event_type).toBe("github_app.gate_check_permission_missing"); + + const aggregate = await env.DB.prepare("select detail, metadata_json from audit_events where event_type = ?") + .bind("github_app.pr_public_surface_failed") + .first<{ detail: string; metadata_json: string }>(); + expect(aggregate).toMatchObject({ detail: "gate_check_run" }); + expect(aggregate?.metadata_json).toContain('"output":"gate_check_run"'); + expect(aggregate?.metadata_json).toContain('"transient":false'); + expect(captureSpy).toHaveBeenCalledWith( + expect.any(Error), + expect.objectContaining({ kind: "publish", repo: "JSONbored/gittensory", failedOutputs: ["gate_check_run"] }), + "pr_public_surface_publish_failed", + ); + captureSpy.mockRestore(); + }); + + it("retries when gate_check_run primary throw and fallback both fail transiently (#8686)", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await persistRegistrySnapshot( + asCloudEnv(env), + normalizeRegistryPayload( + { "JSONbored/gittensory": { emission_share: 0.01, issue_discovery_share: 0 } }, + { kind: "raw-github", url: "https://example.test" }, + "2026-05-23T00:00:00.000Z", + ), + ); + await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, 123); + await upsertRepositorySettings(env, { + repoFullName: "JSONbored/gittensory", + autoLabelEnabled: false, + requireLinkedIssue: true, + }); + await upsertRepoFocusManifest(env, "JSONbored/gittensory", { settings: { reviewCheckMode: "required", commentMode: "off", publicSurface: "off", checkRunMode: "off" } }); + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = init?.method ?? "GET"; + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/commits/gateTransientFail/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/check-runs") && method === "POST") { + const body = JSON.parse(String(init?.body ?? "{}")) as { status?: string }; + if (body.status === "in_progress") { + return Response.json({ id: 502, html_url: "https://example.test/check/502" }, { status: 201 }); + } + // Primary completed publish throws (octokit path) → catch + fallback. + return new Response("GitHub gate check API failed", { status: 500 }); + } + if (url.includes("/check-runs/") && method === "PATCH") { + return new Response("GitHub gate check fallback failed", { status: 500 }); + } + return new Response("not found", { status: 404 }); + }); + const captureSpy = vi.spyOn(posthogModule, "capturePostHogReviewFailure"); + + await expect( + processJob(env, { + type: "github-webhook", + deliveryId: "gate-double-fail-transient", + eventName: "pull_request", + payload: { + action: "opened", + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + pull_request: { number: 87, title: "Gate without issue", state: "open", user: { login: "contributor" }, head: { sha: "gateTransientFail" }, labels: [], body: "No issue link." }, + }, + }), + ).rejects.toMatchObject({ retryKind: "public_surface_publish_transient" }); + + const aggregate = await env.DB.prepare("select detail, metadata_json from audit_events where event_type = ?") + .bind("github_app.pr_public_surface_failed") + .first<{ detail: string; metadata_json: string }>(); + expect(aggregate).toMatchObject({ detail: "gate_check_run" }); + expect(aggregate?.metadata_json).toContain('"output":"gate_check_run"'); + expect(aggregate?.metadata_json).toContain('"transient":true'); + expect(captureSpy).toHaveBeenCalledWith( + expect.any(Error), + expect.objectContaining({ kind: "publish", repo: "JSONbored/gittensory", failedOutputs: ["gate_check_run"] }), + "pr_public_surface_publish_failed", + ); + captureSpy.mockRestore(); + }); + it("marks closed PR gates skipped without creating late first comments", async () => { const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); await persistRegistrySnapshot(