From 4c62d2303203ffad6abb0f763d6ab73efb3afd05 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Thu, 22 Oct 2026 00:59:56 -0400 Subject: [PATCH] fix(release): tolerate pulls API lag after the fence's acknowledged push (#8397) * fix(release): tolerate pulls API lag after the fence's acknowledged push Two of four release runs on main quarantined the 3.3.0-beta.0 Version Packages transaction (#7897) because the PR read ~0.6s after an acknowledged push still showed the prior head; the API reported the pushed head seconds later. The post-push check now polls (10 x 1s) only while the same PR, still Draft, reports the prior head. Any other mismatch, or a longer lag, fails and quarantines exactly as before. Co-Authored-By: Claude Opus 5.5 (1M context) * fix(release): record fence PR-head lag as a state event; test Ready during lag Co-Authored-By: Claude Opus 5.5 (1M context) * test(release): lagging pulls stub reads without a check-then-use race Co-Authored-By: Claude Opus 5.5 (1M context) --------- Co-authored-by: Claude Opus 5.5 (1M context) (cherry picked from commit 8f4ab4a88c435a9b84a3702f03d81877723b4b69) --- RELEASING.md | 3 ++ scripts/fence-changesets.cjs | 34 +++++++++++++++-- tests/changesets-fence.test.cjs | 66 +++++++++++++++++++++++++++++++-- 3 files changed, 97 insertions(+), 6 deletions(-) diff --git a/RELEASING.md b/RELEASING.md index f9a39f924a2..7eebf41adbc 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -319,6 +319,9 @@ rewrites. The HOLD comment's identity, issue, and body are read back before any push. API/read commands have a 60-second bound; pushes have a 10-minute bound, with process-group termination and read-back reconciliation on timeout. Freshness is checked before and after that push, and again after the action's PR API calls. +After an acknowledged push, the PR read tolerates only the pulls API briefly +reporting the prior head (same PR, still Draft): it polls up to 10 times, one +second apart, and any other mismatch or a longer lag quarantines as before. Drift, push errors (including lost acknowledgements), or an incomplete action trigger restoration of the captured ref with a second lease expecting only the diff --git a/scripts/fence-changesets.cjs b/scripts/fence-changesets.cjs index 223cd9d10d6..160f54548ee 100644 --- a/scripts/fence-changesets.cjs +++ b/scripts/fence-changesets.cjs @@ -267,8 +267,7 @@ function prepare() { state.status = "prepared"; save({ event: "prepared" }); } -function verifyPR(expected) { - const pr = openPR(); +function verifyPR(expected, pr = openPR()) { if (state.pr) { if ( !pr || @@ -280,6 +279,35 @@ function verifyPR(expected) { throw Error("Release PR changed or left Draft"); } else if (pr) throw Error("Another PR appeared during generation"); } +// The pulls API can report the pre-push head for a moment after an +// acknowledged push (seen twice on adcp#7897, read ~0.6s after the push and +// converged within ~7s). Poll only that lag: the same PR, still Draft, still +// at the prior head. Any other mismatch, or a lag that outlasts the bound, +// fails exactly as a single read would. +const PR_CONVERGENCE_ATTEMPTS = 10; +const PR_CONVERGENCE_DELAY_MS = 1000; +function awaitPushedPR(expected) { + for (let attempt = 1; ; attempt++) { + const pr = openPR(); + const lagging = + state.pr && + expected !== state.prior && + pr && + pr.number === state.pr.number && + pr.node_id === state.pr.node_id && + pr.draft && + pr.head.sha === state.prior; + if (!lagging || attempt >= PR_CONVERGENCE_ATTEMPTS) + return verifyPR(expected, pr); + save({ event: "pr-head-lag", attempt }); + Atomics.wait( + new Int32Array(new SharedArrayBuffer(4)), + 0, + 0, + PR_CONVERGENCE_DELAY_MS, + ); + } +} function reconcile(reason) { state.status = "quarantined"; state.restored = false; @@ -418,7 +446,7 @@ function push(args) { if (result.status !== 0 || state.observed !== candidate) throw Error("Push failed or acknowledgement/ref ambiguous"); current(); - verifyPR(candidate); + awaitPushedPR(candidate); state.status = "pushed"; save({ event: "pushed", candidate }); } catch (error) { diff --git a/tests/changesets-fence.test.cjs b/tests/changesets-fence.test.cjs index 2bb50a6510d..2b435dd9823 100644 --- a/tests/changesets-fence.test.cjs +++ b/tests/changesets-fence.test.cjs @@ -124,9 +124,19 @@ function gitStub() { "\t", )[0]; const prs = JSON.parse(fs.readFileSync("prs.json", "utf8")); - for (const pr of prs) { - if (value) pr.head.sha = value; - else pr.state = "closed"; + const lag = { + "pr-head-lag": 3, + "pr-head-lag-forever": 1000, + "pr-head-lag-ready": 1000, + }[env.FAULT]; + if (!rollback && lag && value) + fs.writeFileSync("pr-lag.json", JSON.stringify({ sha: value, reads: lag })); + else { + fs.rmSync("pr-lag.json", { force: true }); + for (const pr of prs) { + if (value) pr.head.sha = value; + else pr.state = "closed"; + } } fs.writeFileSync("prs.json", JSON.stringify(prs)); if ( @@ -171,6 +181,24 @@ function ghStub() { if (env.FAULT === "api-timeout") Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 10000); const prs = JSON.parse(fs.readFileSync("prs.json", "utf8")); + let lag = null; + try { + lag = JSON.parse(fs.readFileSync("pr-lag.json", "utf8")); + } catch {} + if (lag && !args.includes("graphql")) { + // The pulls API serves the pre-push head for a few reads after a push. + if (env.FAULT === "pr-head-lag-ready") { + // Someone marks the PR Ready while the API still lags the push. + for (const pr of prs) pr.draft = false; + } + if (--lag.reads <= 0) for (const pr of prs) pr.head.sha = lag.sha; + fs.writeFileSync("prs.next.json", JSON.stringify(prs), { flag: "wx" }); + fs.renameSync("prs.next.json", "prs.json"); + if (lag.reads > 0) { + fs.writeFileSync("pr-lag.next.json", JSON.stringify(lag), { flag: "wx" }); + fs.renameSync("pr-lag.next.json", "pr-lag.json"); + } else fs.rmSync("pr-lag.json", { force: true }); + } const url = args.find((arg) => arg.startsWith("/repos/")); let result; if (args.includes("graphql")) { @@ -399,6 +427,38 @@ test("ready existing PR is verified Draft before a leased push and stays Draft o "always", ); }); +test("a pulls API that briefly lags the acknowledged push converges instead of quarantining", (t) => { + const f = fixture(t); + ok(f.install()); + const result = f.push({ FAULT: "pr-head-lag" }); + ok(result); + assert.match(result.stdout, /"event":"pr-head-lag"/); + assert.equal(f.observed(), f.candidate); + assert.equal(f.prs()[0].head.sha, f.candidate); + ok(f.finalize()); + assert.equal(f.state().status, "complete"); + assert.equal(f.prs()[0].draft, true); +}); +test("a pulls API lag that outlasts the bound still quarantines and restores the prior ref", (t) => { + const f = fixture(t); + ok(f.install()); + stopped(f.push({ FAULT: "pr-head-lag-forever" })); + assert.equal(f.state().status, "quarantined"); + assert.equal(f.observed(), f.prior); + assert.equal(f.prs()[0].draft, true); +}); +test("a Ready transition during the pulls API lag quarantines without waiting out the bound", (t) => { + const f = fixture(t); + ok(f.install()); + stopped(f.push({ FAULT: "pr-head-lag-ready" })); + assert.equal(f.state().status, "quarantined"); + assert.equal(f.observed(), f.prior); + assert.equal(f.prs()[0].draft, true); + assert.equal( + f.state().events.filter((event) => event.event === "pr-head-lag").length, + 0, + ); +}); test("no PR/ref creates with an empty lease; action-created PR must be Draft", (t) => { const f = fixture(t, false); ok(f.install());