Skip to content

fix(queue): retry PR public-surface publish on a transient GitHub failure #3440

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
loopover-orb merged 1 commit into from
Jul 5, 2026
Merged

fix(queue): retry PR public-surface publish on a transient GitHub failure #3440

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 41 additions & 3 deletions src/queue/processors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ import {
getGithubUserCreatedAt,
getInstallationId,
getRepositoryCollaboratorPermission,
githubErrorStatus,
GITTENSORY_GATE_CHECK_NAME,
isGitHubRateLimitedError,
isForeignAppInstallation,
Expand Down Expand Up @@ -5716,8 +5717,21 @@ type PublicSurfaceOutput = "comment" | "label" | "check_run" | "gate_check_run";
type PublicSurfaceOutputFailure = {
output: PublicSurfaceOutput;
error: string;
// Captured AT CATCH TIME, not reconstructed later: errorMessage() already reduces `error` to a plain string by
// the time it lands here, discarding the `.status`/`.response` shape isGitHubTransientPublishError needs. A
// permission_missing check-run push (no live error object) is correctly "false" via the default below.
transient: boolean;
};

// A revoked/expired installation token mid-request, a GitHub 5xx, or a rate-limit blip are all momentary — the
// job should retry, not silently drop a computed review. A 4xx auth/permission/not-found error is not: retrying
// forever would never converge, so it keeps today's swallow-and-audit behavior.
function isGitHubTransientPublishError(error: unknown): boolean {
if (isGitHubRateLimitedError(error)) return true;
const status = githubErrorStatus(error);
return status !== null && status >= 500;
}

// Intentionally writes to check_summaries only, not audit_events (#2908): this fires on every successful gate-
// check publish, which is a very high-frequency event (every review pass, potentially several times per PR as
// it iterates) -- check_summaries is the purpose-built, already-queryable canonical record for "when was this
Expand Down Expand Up @@ -6926,6 +6940,22 @@ export async function enrichOpenPullRequestsWithChangedFiles(env: Env, repoFullN
});
}

// GITTENSORY-5: a transient publish failure (rate limit / GitHub 5xx / momentary token issue) used to be
// swallowed and only audited — the job still completed "successfully" from the queue's point of view, so a
// review that computed real output silently never reached the PR, with no retry. Extending RetryableJobError
// (same shape as RetryablePullRequestFreshnessUnavailableError / PrActuationLockContendedError above) makes the
// queue retry the whole job instead. Thrown only when NOTHING published at all (see finishPublicSurfacePublication)
// and at least one failure was transient — a permanent 4xx keeps today's swallow-and-audit behavior.
class RetryablePublicSurfacePublishFailedError extends RetryableJobError {
constructor(repoFullName: string, prNumber: number) {
super(`public-surface publish failed transiently for ${repoFullName}#${prNumber}; retrying`, {
retryAfterMs: 60_000,
retryKind: "public_surface_publish_transient",
});
this.name = "RetryablePublicSurfacePublishFailedError";
}
}

async function maybePublishPrPublicSurface(
env: Env,
installationId: number,
Expand Down Expand Up @@ -7367,6 +7397,13 @@ async function maybePublishPrPublicSurface(
head_sha: advisory.headSha,
failedOutputs: failedOutputs.map((failure) => failure.output),
});
// At least one output failed for a reason that can plausibly clear on its own (rate limit / 5xx / momentary
// token issue) — retry the whole job instead of leaving the review permanently unposted. A mix of transient
// and permanent failures still retries: the permanent one re-fails identically next pass and re-audits, but
// the transient one gets the chance it needs, and nothing here is published twice (publishedOutputs is empty).
if (failedOutputs.some((failure) => failure.transient)) {
throw new RetryablePublicSurfacePublishFailedError(repoFullName, pr.number);
}
}
if (gateSurfaceIncomplete) {
await recordAuditEvent(env, {
Expand Down Expand Up @@ -8524,6 +8561,7 @@ async function maybePublishPrPublicSurface(
failedOutputs.push({
output: "check_run",
error: checkRunResult.warning,
transient: false,
});
await recordAuditEvent(env, {
eventType: "github_app.check_run_permission_missing",
Expand All @@ -8539,7 +8577,7 @@ async function maybePublishPrPublicSurface(
}
} catch (error) {
const message = errorMessage(error);
failedOutputs.push({ output: "check_run", error: message });
failedOutputs.push({ output: "check_run", error: message, transient: isGitHubTransientPublishError(error) });
await recordPublicSurfaceOutputFailure(
env,
"check_run",
Expand Down Expand Up @@ -8824,7 +8862,7 @@ async function maybePublishPrPublicSurface(
incr("gittensory_reviews_published_total", { repo: repoFullName });
} catch (error) {
const message = errorMessage(error);
failedOutputs.push({ output: "comment", error: message });
failedOutputs.push({ output: "comment", error: message, transient: isGitHubTransientPublishError(error) });
await recordPublicSurfaceOutputFailure(
env,
"comment",
Expand Down Expand Up @@ -8879,7 +8917,7 @@ async function maybePublishPrPublicSurface(
publishedOutputs.push("label");
} catch (error) {
const message = errorMessage(error);
failedOutputs.push({ output: "label", error: message });
failedOutputs.push({ output: "label", error: message, transient: isGitHubTransientPublishError(error) });
await recordPublicSurfaceOutputFailure(
env,
"label",
Expand Down
164 changes: 145 additions & 19 deletions test/unit/queue.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14171,7 +14171,7 @@ describe("queue processors", () => {
expect(audit?.detail).toMatch(/Checks: write permission is missing/i);
});

it("audits advisory context check publish failures without blocking webhook processing", async () => {
it("audits advisory context check publish failures AND retries the job (GitHub 5xx is transient, GITTENSORY-5)", async () => {
const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() });
await persistRegistrySnapshot(
env,
Expand Down Expand Up @@ -14212,7 +14212,7 @@ describe("queue processors", () => {
pull_request: { number: 25, title: "Context check", state: "open", user: { login: "contributor" }, head: { sha: "context500" }, labels: [], body: "No issue needed." },
},
}),
).resolves.toBeUndefined();
).rejects.toMatchObject({ retryKind: "public_surface_publish_transient" });

const outputFailure = await env.DB.prepare("select event_type, detail from audit_events where event_type = ?")
.bind("github_app.pr_check_run_publish_failed")
Expand All @@ -14224,7 +14224,9 @@ describe("queue processors", () => {
.first<{ detail: string; metadata_json: string }>();
expect(aggregate).toMatchObject({ detail: "check_run" });
expect(aggregate?.metadata_json).toContain('"output":"check_run"');
// The total publish failure (nothing reached the PR) escalates to Sentry at error level, not just the ledger.
expect(aggregate?.metadata_json).toContain('"transient":true');
// The total publish failure (nothing reached the PR) escalates to Sentry at error level, not just the ledger —
// this still fires BEFORE the retryable throw, so the failure stays observable even though the job also retries.
expect(captureSpy).toHaveBeenCalledWith(expect.any(Error), expect.objectContaining({ kind: "publish", repo: "JSONbored/gittensory" }));
captureSpy.mockRestore();
});
Expand Down Expand Up @@ -14357,7 +14359,7 @@ describe("queue processors", () => {
expect(published?.metadata_json).toContain('"output":"comment"');
});

it("records an aggregate public-surface failure when no configured output publishes", async () => {
it("records an aggregate public-surface failure when no configured output publishes (permanent failure, no retry)", async () => {
const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() });
await persistRegistrySnapshot(
env,
Expand All @@ -14384,29 +14386,150 @@ describe("queue processors", () => {
if (url.includes("/users/oktofeesh1/repos")) return Response.json([]);
if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" });
if (url.includes("/issues/31/comments") && method === "GET") return Response.json([]);
if (url.includes("/issues/31/comments") && method === "POST") return new Response("comment failed", { status: 503 });
// A 403 with no rate-limit signal (permissions revoked, not a burst limit) is PERMANENT: retrying forever
// would never converge, so this must keep today's swallow-and-audit behavior, not throw a retryable error.
if (url.includes("/issues/31/comments") && method === "POST") return new Response(JSON.stringify({ message: "Resource not accessible by integration" }), { status: 403 });
return new Response("not found", { status: 404 });
});

await processJob(env, {
type: "github-webhook",
deliveryId: "all-public-outputs-failed",
eventName: "pull_request",
payload: {
action: "opened",
installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } },
repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: true, owner: { login: "JSONbored" } },
pull_request: { number: 31, title: "Miner work", state: "open", user: { login: "oktofeesh1" }, labels: [], body: "Fixes #1" },
},
});
await expect(
processJob(env, {
type: "github-webhook",
deliveryId: "all-public-outputs-failed",
eventName: "pull_request",
payload: {
action: "opened",
installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } },
repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: true, owner: { login: "JSONbored" } },
pull_request: { number: 31, title: "Miner work", state: "open", user: { login: "oktofeesh1" }, labels: [], body: "Fixes #1" },
},
}),
).resolves.toBeUndefined();

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: "comment" });
expect(aggregate?.metadata_json).toContain('"output":"comment"');
expect(aggregate?.metadata_json).toContain('"transient":false');
const published = await env.DB.prepare("select event_type from audit_events where event_type = ?").bind("github_app.pr_public_surface_published").all();
expect(published.results).toEqual([]);
const webhookRow = await env.DB.prepare("select status from webhook_events where delivery_id = ?").bind("all-public-outputs-failed").first<{ status: string }>();
expect(webhookRow?.status).toBe("processed");
});

it("retries the whole job when a transient GitHub 5xx drops every public-surface output (GITTENSORY-5)", async () => {
const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() });
await persistRegistrySnapshot(
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 upsertRepositorySettings(env, {
repoFullName: "JSONbored/gittensory",
commentMode: "detected_contributors_only",
publicSurface: "comment_only",
checkRunMode: "off",
});
vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {
const url = input.toString();
const method = init?.method ?? "GET";
if (url === "https://api.gittensor.io/miners") return Response.json([{ githubUsername: "oktofeesh1", githubId: "123", totalPrs: 2, totalMergedPrs: 2, isEligible: true, credibility: 1 }]);
if (url === "https://api.gittensor.io/miners/123") return Response.json({ repositories: [] });
if (url === "https://api.gittensor.io/miners/123/prs") return Response.json([]);
if (url === "https://mirror.gittensor.io/api/v1/miners/123/issues") return Response.json({ issues: [] });
if (url.endsWith("/users/oktofeesh1")) return Response.json({ login: "oktofeesh1" });
if (url.includes("/users/oktofeesh1/repos")) return Response.json([]);
if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" });
if (url.includes("/issues/32/comments") && method === "GET") return Response.json([]);
// GitHub 5xx during publish: momentary, not the caller's fault — the job must retry, not silently drop the
// review the same way JSONbored/awesome-claude#4251 did (Sentry GITTENSORY-5).
if (url.includes("/issues/32/comments") && method === "POST") return new Response("upstream unavailable", { status: 502 });
return new Response("not found", { status: 404 });
});

await expect(
processJob(env, {
type: "github-webhook",
deliveryId: "transient-publish-failure",
eventName: "pull_request",
payload: {
action: "opened",
installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } },
repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: true, owner: { login: "JSONbored" } },
pull_request: { number: 32, title: "Miner work", state: "open", user: { login: "oktofeesh1" }, labels: [], body: "Fixes #1" },
},
}),
).rejects.toMatchObject({ retryKind: "public_surface_publish_transient" });

// The failure IS still audited (observability doesn't regress) — it just also throws so the queue retries.
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: "comment" });
expect(aggregate?.metadata_json).toContain('"transient":true');
const published = await env.DB.prepare("select event_type from audit_events where event_type = ?").bind("github_app.pr_public_surface_published").all();
expect(published.results).toEqual([]);
// The webhook row is marked "error", not "processed" — a thrown job is exactly what lets the queue retry it.
const webhookRow = await env.DB.prepare("select status from webhook_events where delivery_id = ?").bind("transient-publish-failure").first<{ status: string }>();
expect(webhookRow?.status).toBe("error");
});

it("leaves a fully successful public-surface publish unaffected by the transient-retry check", async () => {
const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() });
await persistRegistrySnapshot(
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 upsertRepositorySettings(env, {
repoFullName: "JSONbored/gittensory",
commentMode: "detected_contributors_only",
publicSurface: "comment_only",
checkRunMode: "off",
});
vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {
const url = input.toString();
const method = init?.method ?? "GET";
if (url === "https://api.gittensor.io/miners") return Response.json([{ githubUsername: "oktofeesh1", githubId: "123", totalPrs: 2, totalMergedPrs: 2, isEligible: true, credibility: 1 }]);
if (url === "https://api.gittensor.io/miners/123") return Response.json({ repositories: [] });
if (url === "https://api.gittensor.io/miners/123/prs") return Response.json([]);
if (url === "https://mirror.gittensor.io/api/v1/miners/123/issues") return Response.json({ issues: [] });
if (url.endsWith("/users/oktofeesh1")) return Response.json({ login: "oktofeesh1" });
if (url.includes("/users/oktofeesh1/repos")) return Response.json([]);
if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" });
if (url.includes("/issues/33/comments") && method === "GET") return Response.json([]);
if (url.includes("/issues/33/comments") && method === "POST") return Response.json({ id: 1 }, { status: 201 });
return new Response("not found", { status: 404 });
});

await expect(
processJob(env, {
type: "github-webhook",
deliveryId: "public-surface-clean-publish",
eventName: "pull_request",
payload: {
action: "opened",
installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } },
repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: true, owner: { login: "JSONbored" } },
pull_request: { number: 33, title: "Miner work", state: "open", user: { login: "oktofeesh1" }, labels: [], body: "Fixes #1" },
},
}),
).resolves.toBeUndefined();

const webhookRow = await env.DB.prepare("select status from webhook_events where delivery_id = ?").bind("public-surface-clean-publish").first<{ status: string }>();
expect(webhookRow?.status).toBe("processed");
const failed = await env.DB.prepare("select event_type from audit_events where event_type = ?").bind("github_app.pr_public_surface_failed").all();
expect(failed.results).toEqual([]);
const published = await env.DB.prepare("select metadata_json from audit_events where event_type = ?").bind("github_app.pr_public_surface_published").first<{ metadata_json: string }>();
expect(published?.metadata_json).toContain('"publishedOutputs":["comment"]');
expect(published?.metadata_json).toContain('"failedOutputs":[]');
});

it("keeps repository and PR webhook processing internal when installation context is absent", async () => {
Expand Down Expand Up @@ -14561,7 +14684,9 @@ describe("queue processors", () => {
if (url.includes("/labels") && method === "GET") return Response.json([]);
if (url.includes("/labels") && method === "POST") {
calls.labels += 1;
return new Response("label failed", { status: 503 });
// A permanent failure (permissions gap, not a momentary blip) — this test is about duplicate-comment
// suppression on a label-only surface, not about retry classification, so it must stay non-transient.
return new Response(JSON.stringify({ message: "Resource not accessible by integration" }), { status: 403 });
}
return new Response("not found", { status: 404 });
});
Expand All @@ -14580,13 +14705,14 @@ describe("queue processors", () => {
}),
).resolves.toBeUndefined();

// gittensor context-label apply (fails 503, recorded) + the best-effort type-label create attempt (also 503,
// gittensor context-label apply (fails 403, recorded) + the best-effort type-label create attempt (also 403,
// swallowed). The context-label failure is still recorded below; the type label never drops the recording.
expect(calls).toEqual({ comments: 0, labels: 2 });
const outputFailure = await env.DB.prepare("select event_type, detail from audit_events where event_type = ?")
.bind("github_app.pr_label_publish_failed")
.first<{ event_type: string; detail: string }>();
expect(outputFailure).toMatchObject({ event_type: "github_app.pr_label_publish_failed", detail: "label failed" });
expect(outputFailure?.event_type).toBe("github_app.pr_label_publish_failed");
expect(outputFailure?.detail).toMatch(/Resource not accessible by integration/);
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 }>();
Expand Down
Loading