Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
38 changes: 27 additions & 11 deletions src/review/enrichment-wire.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,32 @@ function sharedSecretWasNormalized(
return (normalized ?? "") !== raw;
}

// REES's own /v1/ping returns 503 specifically to mean "not configured/ready yet" (server.ts: no
// REES_SHARED_SECRET set on that side) -- the same benign startup-ordering race probeReesSecretAtStartup's
// catch block already extends grace to for a refused connection (GITTENSORY-1J: 7 Sentry events, all in one
// ~5h window, never recurring -- consistent with a one-time deploy/restart race, not a persistent
// misconfiguration). Retry a few times before escalating; any other status is final on the first response.
const REES_PING_NOT_READY_RETRIES = 2;
const REES_PING_NOT_READY_RETRY_DELAY_MS = 500;

async function fetchReesPingWithRetry(url: string, secret: string): Promise<Response> {
const request = () =>
fetch(url, {
method: "POST",
headers: {
"user-agent": "gittensory-selfhost/1.0",
authorization: `Bearer ${secret}`,
},
signal: AbortSignal.timeout(5000),
});
let response = await request();
for (let attempt = 0; attempt < REES_PING_NOT_READY_RETRIES && response.status === 503; attempt += 1) {
await new Promise((resolve) => setTimeout(resolve, REES_PING_NOT_READY_RETRY_DELAY_MS));
response = await request();
}
return response;
}

// Set true once the startup probe confirms REES rejects the shared secret (401/403). Once set,
// buildReviewEnrichment skips every /v1/enrich call for the rest of this process's lifetime instead of
// repeating a call that's confirmed to fail on every PR review, each one logging review_context_fetch_failed.
Expand Down Expand Up @@ -104,17 +130,7 @@ export function probeReesSecretAtStartup(env: Env): void {
// Probe asynchronously — never block the server from starting.
void (async () => {
try {
const response = await fetch(
`${base.replace(/\/+$/, "")}/v1/ping`,
{
method: "POST",
headers: {
"user-agent": "gittensory-selfhost/1.0",
authorization: `Bearer ${sharedSecret}`,
},
signal: AbortSignal.timeout(5000),
},
);
const response = await fetchReesPingWithRetry(`${base.replace(/\/+$/, "")}/v1/ping`, sharedSecret);
if (response.ok) {
console.log(
JSON.stringify({
Expand Down
40 changes: 40 additions & 0 deletions test/unit/enrichment-wire.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,46 @@ describe("probeReesSecretAtStartup", () => {
errSpy.mockRestore();
});

it("REGRESSION (#5006, GITTENSORY-1J): retries a 503 ('not ready yet') a few times before escalating to rees_ping_error", async () => {
const fetchSpy = vi.fn(async () => ({ ok: false, status: 503 }) as Response);
globalThis.fetch = fetchSpy as unknown as typeof fetch;
const errSpy = vi.spyOn(console, "error").mockImplementation(() => {});
probeReesSecretAtStartup(env({ REES_URL: "https://rees.example", REES_SHARED_SECRET: "s3cret" }));
await new Promise((resolve) => setTimeout(resolve, 1100));
expect(fetchSpy).toHaveBeenCalledTimes(3); // the first attempt + 2 retries, all still 503
const parsed = errSpy.mock.calls.map((c) => JSON.parse(c[0] as string));
expect(parsed.some((p) => p.event === "rees_ping_error" && p.status === 503)).toBe(true);
errSpy.mockRestore();
});

it("REGRESSION (#5006): a 503 that clears on retry succeeds without ever escalating to rees_ping_error", async () => {
let calls = 0;
const fetchSpy = vi.fn(async () => {
calls += 1;
return (calls < 2 ? { ok: false, status: 503 } : { ok: true }) as Response;
});
globalThis.fetch = fetchSpy as unknown as typeof fetch;
const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
const errSpy = vi.spyOn(console, "error").mockImplementation(() => {});
probeReesSecretAtStartup(env({ REES_URL: "https://rees.example", REES_SHARED_SECRET: "s3cret" }));
await new Promise((resolve) => setTimeout(resolve, 1100));
expect(fetchSpy).toHaveBeenCalledTimes(2);
expect(logSpy.mock.calls.some((c) => JSON.parse(c[0] as string).event === "rees_ping_ok")).toBe(true);
expect(errSpy).not.toHaveBeenCalled();
logSpy.mockRestore();
errSpy.mockRestore();
});

it("does not retry a non-503 non-ok status — a single attempt is final", async () => {
const fetchSpy = vi.fn(async () => ({ ok: false, status: 500 }) as Response);
globalThis.fetch = fetchSpy as unknown as typeof fetch;
const errSpy = vi.spyOn(console, "error").mockImplementation(() => {});
probeReesSecretAtStartup(env({ REES_URL: "https://rees.example", REES_SHARED_SECRET: "s3cret" }));
await flush();
expect(fetchSpy).toHaveBeenCalledTimes(1);
errSpy.mockRestore();
});

it("warns rees_ping_error (not throw) when the fetch itself rejects — REES may not be up yet", async () => {
const fetchSpy = vi.fn(async () => {
throw new Error("connect ECONNREFUSED");
Expand Down