mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-06 10:48:12 +02:00
fix(ci): retry transient GitHub reads while waiting for source verification (#13429)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Every commit on master is published as an npm canary, and a canary
is the only thing a nightly, a beta and then a stable can be promoted
from
> - A canary only publishes after the release run confirms that Cloud
readiness passed for that exact commit, which it does by polling the
GitHub Actions API for up to 45 minutes
> - That poll used a read that threw on any non-OK response, so one
gateway error ended the wait immediately
> - The release run then failed and the commit published no canary,
although readiness itself had passed
> - A commit with no canary can never be promoted, so this silently
removes commits from the release path
> - This pull request retries the reads that mean "ask again" and leaves
every real failure fast
> - The benefit is that a transient API error costs a few seconds
instead of a release
## Linked Issues or Issue Description
No existing issue. The problem, in the bug report format:
**What happened**
The `Reuse exact-source verification` job failed 12 seconds into a
45-minute wait:
```
Waiting for Cloud source verified v1 for 5054c9ef9b.
GitHub Actions read failed (HTTP 502).
##[error]Process completed with exit code 1.
```
`publish_canary` was skipped, so the commit published no canary. Cloud
readiness for that same commit had already completed successfully.
**Expected behavior**
A gateway error from the GitHub API is a reason to ask again, not a
verdict on the commit. The wait should continue and the canary should
publish.
**Steps to reproduce**
1. Push to master and let Cloud readiness pass for that commit.
2. Have the GitHub Actions API return 502 for any single read the
verification poll makes.
3. The release run fails and no canary is published for that commit.
**Paperclip version or commit**
Present on master. Observed on 2026-09-14 in a release run for
`5054c9ef9`.
## What Changed
- `scripts/cloud-source-verification.mjs` gains `createActionsReader`,
the transport the CLI entry point now uses. It retries transient
transport failures — network errors and HTTP 408, 425, 429, 500, 502,
503 and 504 — with four bounded attempts and a growing backoff.
- Every other non-OK status still throws on the first response. 401, 403
and 404 mean the token or the target is wrong, and waiting those out
would only delay the failure.
- The verification logic itself is untouched. A readiness run that
genuinely failed still stops the release immediately, and an ambiguous
or mismatched run still throws.
## Verification
```
node --test scripts/cloud-source-verification.test.mjs # 16 passed
node scripts/cloud-source-verification.mjs # still exits cleanly with the token message
```
New tests cover a retried gateway error, each transient status with its
growing backoff, a retried network failure and the message that survives
exhaustion, no retry for authorization failures, and the attempt budget.
The pre-existing verification tests are unchanged and still pass.
## Risks
Low risk, and limited to one CI script.
- A genuinely unreachable API now takes four attempts before failing,
which adds a few seconds to a run that was going to fail anyway.
- The retry cannot mask a failed readiness run: that path throws a
different error, from the verification logic rather than the transport.
- No product code and no workflow changes.
## Model Used
- Claude Fable 5 (`claude-fable-5`), 1M context, extended thinking, run
through Claude Code with tool use and code execution.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [ ] I have updated relevant documentation to reflect my changes — no
documented behavior changes
- [x] I have considered and documented any risks above
- [x] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
This commit is contained in:
1 parent
667c79ded2
commit
08adcc70d5
2 files changed
+189
-10
No files matched your search
@@ -61,6 +61,80 @@ export async function readSourceVerification(sha, api) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
// Statuses that say "ask again", not "the answer is no". A release must not be
|
||||
// blocked because GitHub returned a gateway error during a 45-minute poll.
|
||||
const TRANSIENT_READ_STATUSES = new Set([408, 425, 429, 500, 502, 503, 504]);
|
||||
|
||||
// A rate-limited read also says "ask again", and GitHub reports both primary
|
||||
// and secondary rate limits as 403. Only the headers separate that from a
|
||||
// token that may not read Actions, which must still fail at once.
|
||||
function rateLimited(response) {
|
||||
if (response.status !== 403) return false;
|
||||
const header = (name) => response.headers?.get?.(name) ?? null;
|
||||
return header("retry-after") !== null || header("x-ratelimit-remaining") === "0";
|
||||
}
|
||||
|
||||
/** Milliseconds from a Retry-After header, when it carries a sane delay. */
|
||||
function retryAfterMs(response, capMs) {
|
||||
const value = Number(response?.headers?.get?.("retry-after"));
|
||||
if (!Number.isFinite(value) || value <= 0) return null;
|
||||
return Math.min(value * 1_000, capMs);
|
||||
}
|
||||
|
||||
/**
|
||||
* The GitHub Actions read used by the polling below, with transient transport
|
||||
* failures retried: network errors, the statuses above, rate-limited 403s, and
|
||||
* a body that fails while it is being read. Any other non-OK status throws on
|
||||
* the first response, because waiting out a wrong token only delays the news.
|
||||
*
|
||||
* `deadlineAt` bounds retries by the caller's own polling deadline, so a read
|
||||
* cannot extend the wait past the timeout it belongs to.
|
||||
*/
|
||||
export function createActionsReader({
|
||||
token, fetchImpl = fetch, sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms)),
|
||||
attempts = 4, backoffMs = 1_000, log = console.log,
|
||||
now = Date.now, deadlineAt = () => Infinity, maxRetryAfterMs = 60_000,
|
||||
} = {}) {
|
||||
return async (path) => {
|
||||
for (let attempt = 1; ; attempt += 1) {
|
||||
// Retry only while both the attempt budget and the caller's deadline
|
||||
// leave room for the wait this attempt would cost.
|
||||
const waitMs = backoffMs * attempt;
|
||||
const retryable = attempt < attempts && now() + waitMs < deadlineAt();
|
||||
const pause = (response) => sleep(retryAfterMs(response, maxRetryAfterMs) ?? waitMs);
|
||||
let response;
|
||||
try {
|
||||
response = await fetchImpl(`https://api.github.com${path}`, {
|
||||
headers: { Authorization: `Bearer ${token}`, Accept: "application/vnd.github+json", "X-GitHub-Api-Version": "2022-11-28" },
|
||||
signal: AbortSignal.timeout(30_000), redirect: "error",
|
||||
});
|
||||
} catch (cause) {
|
||||
if (!retryable) throw new Error(`GitHub Actions read failed: ${cause.message}`);
|
||||
log(`GitHub Actions read failed (${cause.message}); retrying (${attempt}/${attempts - 1}).`);
|
||||
await pause();
|
||||
continue;
|
||||
}
|
||||
if (response.ok) {
|
||||
try {
|
||||
// A 200 whose body is truncated or undecodable is a transport
|
||||
// failure like any other, so it belongs inside the retry.
|
||||
return await response.json();
|
||||
} catch (cause) {
|
||||
if (!retryable) throw new Error(`GitHub Actions read failed: ${cause.message}`);
|
||||
log(`GitHub Actions read body failed (${cause.message}); retrying (${attempt}/${attempts - 1}).`);
|
||||
await pause();
|
||||
continue;
|
||||
}
|
||||
}
|
||||
if (!retryable || !(TRANSIENT_READ_STATUSES.has(response.status) || rateLimited(response))) {
|
||||
throw new Error(`GitHub Actions read failed (HTTP ${response.status}).`);
|
||||
}
|
||||
log(`GitHub Actions read failed (HTTP ${response.status}); retrying (${attempt}/${attempts - 1}).`);
|
||||
await pause(response);
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
export async function waitForSourceVerification(sha, {
|
||||
api, now = Date.now, sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms)),
|
||||
timeoutMs = 45 * 60_000, intervalMs = 30_000, log = console.log,
|
||||
@@ -80,15 +154,12 @@ export async function waitForSourceVerification(sha, {
|
||||
if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) {
|
||||
try {
|
||||
if (!process.env.GITHUB_TOKEN) throw new Error("GITHUB_TOKEN with Actions read access is required.");
|
||||
const api = async (path) => {
|
||||
const response = await fetch(`https://api.github.com${path}`, {
|
||||
headers: { Authorization: `Bearer ${process.env.GITHUB_TOKEN}`, Accept: "application/vnd.github+json", "X-GitHub-Api-Version": "2022-11-28" },
|
||||
signal: AbortSignal.timeout(30_000), redirect: "error",
|
||||
});
|
||||
if (!response.ok) throw new Error(`GitHub Actions read failed (HTTP ${response.status}).`);
|
||||
return response.json();
|
||||
};
|
||||
const proof = await waitForSourceVerification(process.argv[2], { api });
|
||||
// One deadline for both layers: the reader stops retrying when the poll it
|
||||
// serves is out of time, instead of extending the wait past its timeout.
|
||||
const timeoutMs = 45 * 60_000;
|
||||
const deadline = Date.now() + timeoutMs;
|
||||
const api = createActionsReader({ token: process.env.GITHUB_TOKEN, deadlineAt: () => deadline });
|
||||
const proof = await waitForSourceVerification(process.argv[2], { api, timeoutMs });
|
||||
const message = `Source verification passed for ${proof.sha}: https://github.com/${repository}/actions/runs/${proof.runId}/attempts/${proof.attempt} (job ${proof.jobId}).`;
|
||||
console.log(message);
|
||||
if (process.env.GITHUB_STEP_SUMMARY) await appendFile(process.env.GITHUB_STEP_SUMMARY, `${message}\n`);
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import assert from "node:assert/strict";
|
||||
import test from "node:test";
|
||||
import { readSourceVerification, sourceVerificationJob, waitForSourceVerification } from "./cloud-source-verification.mjs";
|
||||
import { createActionsReader, readSourceVerification, sourceVerificationJob, waitForSourceVerification } from "./cloud-source-verification.mjs";
|
||||
|
||||
const sha = "a".repeat(40);
|
||||
const workflow = { id: 123, path: ".github/workflows/cloud-readiness.yml" };
|
||||
@@ -123,3 +123,111 @@ test("malformed source refs are rejected before any request", async () => {
|
||||
await assert.rejects(readSourceVerification(ref, () => assert.fail("must not request")), /full lowercase/);
|
||||
}
|
||||
});
|
||||
|
||||
// A gateway error during the poll must not decide the release. These cover the
|
||||
// reader's transport only; the verification semantics above are unchanged.
|
||||
function reader({ responses, attempts = 4, ...options }) {
|
||||
const seen = [];
|
||||
const waits = [];
|
||||
const fetchImpl = async () => {
|
||||
const next = responses[seen.length];
|
||||
seen.push(next);
|
||||
if (next instanceof Error) throw next;
|
||||
return {
|
||||
ok: next.status >= 200 && next.status < 300,
|
||||
status: next.status,
|
||||
headers: { get: (name) => next.headers?.[name.toLowerCase()] ?? null },
|
||||
json: async () => {
|
||||
if (next.bodyError) throw next.bodyError;
|
||||
return next.body ?? { ok: true };
|
||||
},
|
||||
};
|
||||
};
|
||||
const api = createActionsReader({
|
||||
token: "t", fetchImpl, attempts, backoffMs: 10,
|
||||
sleep: async (ms) => { waits.push(ms); }, log: () => {}, ...options,
|
||||
});
|
||||
return { api, seen, waits };
|
||||
}
|
||||
|
||||
test("a transient gateway error is retried instead of failing the release", async () => {
|
||||
const { api, seen, waits } = reader({ responses: [{ status: 502 }, { status: 200, body: { id: 1 } }] });
|
||||
assert.deepEqual(await api("/repos/x"), { id: 1 });
|
||||
assert.equal(seen.length, 2);
|
||||
assert.deepEqual(waits, [10]);
|
||||
});
|
||||
|
||||
test("every transient status is retried, and the backoff grows", async () => {
|
||||
for (const status of [408, 425, 429, 500, 502, 503, 504]) {
|
||||
const { api, seen, waits } = reader({ responses: [{ status }, { status }, { status: 200, body: { ok: true } }] });
|
||||
await api("/repos/x");
|
||||
assert.equal(seen.length, 3, `status ${status} should be retried`);
|
||||
assert.deepEqual(waits, [10, 20]);
|
||||
}
|
||||
});
|
||||
|
||||
test("a rate-limited 403 is retried, and a forbidden 403 is not", async () => {
|
||||
// GitHub reports both primary and secondary rate limits as 403; only the
|
||||
// headers tell them apart from a token that may not read Actions.
|
||||
for (const headers of [{ "retry-after": "1" }, { "x-ratelimit-remaining": "0" }]) {
|
||||
const { api, seen } = reader({ responses: [{ status: 403, headers }, { status: 200, body: { ok: true } }] });
|
||||
await api("/repos/x");
|
||||
assert.equal(seen.length, 2, `403 with ${JSON.stringify(headers)} should be retried`);
|
||||
}
|
||||
for (const headers of [undefined, { "x-ratelimit-remaining": "4999" }]) {
|
||||
const { api, seen } = reader({ responses: [{ status: 403, headers }, { status: 200 }] });
|
||||
await assert.rejects(api("/repos/x"), /HTTP 403/);
|
||||
assert.equal(seen.length, 1, "a forbidden 403 must fail on the first response");
|
||||
}
|
||||
});
|
||||
|
||||
test("Retry-After sets the wait, capped so one header cannot stall the poll", async () => {
|
||||
const { api, waits } = reader({ responses: [{ status: 429, headers: { "retry-after": "5" } }, { status: 200 }] });
|
||||
await api("/repos/x");
|
||||
assert.deepEqual(waits, [5_000]);
|
||||
|
||||
const capped = reader({ responses: [{ status: 429, headers: { "retry-after": "86400" } }, { status: 200 }], maxRetryAfterMs: 60_000 });
|
||||
await capped.api("/repos/x");
|
||||
assert.deepEqual(capped.waits, [60_000]);
|
||||
});
|
||||
|
||||
test("a body that fails while being read is retried", async () => {
|
||||
const { api, seen } = reader({
|
||||
responses: [{ status: 200, bodyError: new Error("terminated") }, { status: 200, body: { id: 7 } }],
|
||||
});
|
||||
assert.deepEqual(await api("/repos/x"), { id: 7 });
|
||||
assert.equal(seen.length, 2);
|
||||
});
|
||||
|
||||
test("a network failure is retried, and its message survives exhaustion", async () => {
|
||||
const { api, seen } = reader({ responses: Array.from({ length: 4 }, () => new Error("fetch failed")) });
|
||||
await assert.rejects(api("/repos/x"), /GitHub Actions read failed: fetch failed/);
|
||||
assert.equal(seen.length, 4);
|
||||
});
|
||||
|
||||
test("an authorization failure is not retried", async () => {
|
||||
for (const status of [401, 404, 422]) {
|
||||
const { api, seen } = reader({ responses: [{ status }, { status: 200 }] });
|
||||
await assert.rejects(api("/repos/x"), new RegExp(`HTTP ${status}`));
|
||||
assert.equal(seen.length, 1, `status ${status} must fail on the first response`);
|
||||
}
|
||||
});
|
||||
|
||||
test("a transient status that never clears fails after its attempt budget", async () => {
|
||||
const { api, seen } = reader({ responses: Array.from({ length: 4 }, () => ({ status: 502 })) });
|
||||
await assert.rejects(api("/repos/x"), /HTTP 502/);
|
||||
assert.equal(seen.length, 4);
|
||||
});
|
||||
|
||||
test("retries stop at the caller's deadline rather than outliving the poll", async () => {
|
||||
// The wait this attempt would cost does not fit before the deadline, so the
|
||||
// read reports the failure instead of sleeping past the timeout it serves.
|
||||
let clock = 0;
|
||||
const { api, seen, waits } = reader({
|
||||
responses: [{ status: 502 }, { status: 200 }],
|
||||
now: () => clock, deadlineAt: () => 5,
|
||||
});
|
||||
await assert.rejects(api("/repos/x"), /HTTP 502/);
|
||||
assert.equal(seen.length, 1);
|
||||
assert.deepEqual(waits, []);
|
||||
});
|
||||
Reference in new issue
Block a user