Skip to content

Commit d2398dc

Browse files
committed
Restrict continue-on-error to capped restores
Review follow-ups: - The contract test allowed continue-on-error on any step, so a test or build step tolerating failure would slip past it. Allow it only on rust-cache / actions/cache/restore steps that also carry a timeout, and require every such restore in ci.yml to be capped. - Cap the macOS vexctl actions/cache/restore the same way. If that restore times out on main, skip the save step, which would otherwise run with an empty key and fail the job. - Correct the comment: the hang was in yarn-classic-matrix, and an uncapped restore runs to that job's own timeout. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LZ8V19imL1WHdq6hZdoums
1 parent 05d96a9 commit d2398dc

2 files changed

Lines changed: 27 additions & 6 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,12 @@ jobs:
118118
# version). rust-cache also hashes the toolchain, Cargo manifests
119119
# and Rust environment. Check/coverage/release keep separate caches.
120120
# A cache restore is an optimisation, so cap it and carry on cold.
121-
# Healthy restores take under 30 s. On 2026-10-26 one hung for 14+
122-
# min and held merge-queue head #1719 (and 12 entries behind it)
123-
# until it was cancelled; uncapped it would have run to the 40-min
124-
# job timeout. Later `Cache cargo` steps carry the same two keys.
121+
# Healthy restores take under 30 s on every runner OS. On 2026-10-26
122+
# one hung for 14+ min in yarn-classic-matrix and held merge-queue
123+
# head #1719 (and 12 entries behind it) until it was cancelled;
124+
# uncapped it runs to the job's own timeout. Every cache restore in
125+
# this file carries the same two keys (enforced by
126+
# scripts/tests/test_merge_queue_fail_fast.py).
125127
uses: Swatinem/rust-cache@c19371144df3bb44fab255c43d04cbc2ab54d1c4 # v2.9.1
126128
timeout-minutes: 5
127129
continue-on-error: true
@@ -442,6 +444,9 @@ jobs:
442444
id: vexctl-cache
443445
if: needs.release-readiness.outputs.reuse != 'true' && runner.os == 'macOS'
444446
uses: actions/cache/restore@caa296126883cff596d87d8935842f9db880ef25 # v5.1.0
447+
# Capped like `Cache cargo`: a miss only means compiling vexctl.
448+
timeout-minutes: 5
449+
continue-on-error: true
445450
with:
446451
path: ${{ runner.temp }}/vexctl-bin
447452
key: vexctl-${{ env.VEXCTL_VERSION }}-go${{ steps.go.outputs.go-version }}-${{ runner.os }}-${{ runner.arch }}
@@ -515,6 +520,7 @@ jobs:
515520
&& runner.os == 'macOS'
516521
&& github.ref == 'refs/heads/main'
517522
&& steps.vexctl-cache.outputs.cache-hit != 'true'
523+
&& steps.vexctl-cache.outcome == 'success'
518524
uses: actions/cache/save@caa296126883cff596d87d8935842f9db880ef25 # v5.1.0
519525
with:
520526
path: ${{ runner.temp }}/vexctl-bin

‎scripts/tests/test_merge_queue_fail_fast.py‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,24 @@ class CiWorkflowContract(unittest.TestCase):
115115
def test_no_job_tolerates_failure(self):
116116
# A job-level continue-on-error would let ci-ok pass despite a failed
117117
# job, and this script would then cancel a run that could still land.
118-
# Step-level ones (8+ spaces, e.g. the capped `Cache cargo` restores)
119-
# leave the job's conclusion to its other steps, so they are fine.
118+
# A step-level one hides that step's failure the same way, so only
119+
# the capped cache restores may carry it: a miss there just means
120+
# building cold.
120121
self.assertIsNone(re.search(r"^ continue-on-error:", self.text, re.M))
122+
steps = re.split(r"(?m)^ - ", self.text)
123+
tolerant = [s for s in steps if re.search(r"(?m)^\s*continue-on-error:", s)]
124+
self.assertTrue(tolerant)
125+
for step in tolerant:
126+
self.assertRegex(step, r"(?m)^ uses: (Swatinem/rust-cache|actions/cache/restore)@", step[:80])
127+
self.assertRegex(step, r"(?m)^ timeout-minutes: \d+$", step[:80])
128+
self.assertRegex(step, r"(?m)^ continue-on-error: true$", step[:80])
129+
130+
def test_every_cache_restore_is_capped(self):
131+
# An uncapped restore that hangs holds the queue head until the job
132+
# times out (#1730).
133+
for step in re.split(r"(?m)^ - ", self.text):
134+
if re.search(r"(?m)^ uses: (Swatinem/rust-cache|actions/cache/restore)@", step):
135+
self.assertRegex(step, r"(?m)^ timeout-minutes: \d+$", step[:80])
121136

122137
def test_gate_runs_on_a_cancelled_run(self):
123138
gate = self.text[self.text.index("\n ci-ok:"):]

0 commit comments

Comments
 (0)