Skip to content

fix(ci): classify hermit retry timeouts by watchdog kill, not whole seconds - #9274

Merged
matt2e merged 1 commit into
mainfrom
mtoohey/flake-ci-hermit-retry
Nov 9, 2026
Merged

matt2e merged 1 commit into
mainfrom
mtoohey/flake-ci-hermit-retry

Conversation

@matt2e

@matt2e matt2e commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Split out of the flakey-monday flake-fix series. scripts/ci-hermit-retry.sh decided an attempt had "timed out" by comparing whole-second elapsed time with the deadline, so an instant failure that straddled a second boundary was misreported as a timeout. The watchdog now reports through its exit status whether it actually killed the attempt.

Commit (cherry-picked from flakey-monday, original message kept):

  • fix(ci): report a hermit retry timeout only when the watchdog fired

Root cause and fix

node:test "persistent failure backs off between attempts and stops at the budget" (scripts/ci-hermit-retry.test.mjs): expected "attempt 3/3 failed (exit 7)" and got "attempt 3/3 timed out after 1s".

  • Root cause (confirmed): the script checked $((SECONDS - started)) -ge deadline. Bash's $SECONDS counts whole wall-clock seconds, and the test uses a 1 s deadline (CI_HERMIT_ATTEMPT_SECONDS=1). An instant exit 7 that crossed a second boundary looked like it had run for the whole deadline.
  • Fix: the watchdog subshell exits 0 if it is stopped before signalling the attempt, and 124 once it has sent TERM (or TERM then KILL). The script reports a timeout only when the watchdog exits 124; elapsed time is no longer used. Bash 3.2 (macOS) supports this. A new test starts 0.8 s into a wall-clock second and runs a 0.4 s attempt that exits 7, so it crosses a second boundary but finishes well inside its 1 s deadline, and asserts it is reported as "failed (exit 7)".

Related issue

N/A (flake fix; none found).

Testing

Human testing is still required. This PR does not carry buzz-review-completed; the author has not yet run the script or its tests themselves on this branch.

Verification

Verified locally on the flakey-monday branch before the split:

  • The new test against the old script: failed with "attempt 1/1 timed out after 1s", the CI symptom.
  • The whole ci-hermit-retry.test.mjs suite with the fix: 20/20 passed.

Nothing was re-run on this split branch: the workstation was overloaded, so no tests, builds or linters were executed after cherry-picking. CI on this PR is the verification for the split branch.

🤖 Generated with Claude Code

Fixes the flake in the node:test case "persistent failure backs off
between attempts and stops at the budget"
(scripts/ci-hermit-retry.test.mjs). It expected "attempt 3/3 failed
(exit 7)" and got "attempt 3/3 timed out after 1s".

Root cause (confirmed): scripts/ci-hermit-retry.sh decided "timed out"
by checking `$((SECONDS - started)) -ge deadline`. Bash's $SECONDS
counts whole wall-clock seconds, and the test uses a 1 s deadline
(CI_HERMIT_ATTEMPT_SECONDS=1). An instant `exit 7` that straddled a
second boundary therefore looked like it had run for the whole
deadline.

Fix: the watchdog subshell now reports what it did through its exit
status. It exits 0 if it is stopped before it signals the attempt, and
124 once it has sent TERM (or TERM then KILL). The script reports a
timeout only when the watchdog exits 124. Elapsed time is no longer
used. Bash 3.2 (macOS) supports this. A new test starts 0.8 s into a
wall-clock second and runs a 0.4 s attempt that exits 7. The attempt
crosses a second boundary but finishes well inside its 1 s deadline,
and the test asserts it is reported as "failed (exit 7)", not as a
timeout.

Verified locally:
- the new test against the old script: failed with "attempt 1/1 timed
  out after 1s", the CI symptom
- the whole ci-hermit-retry.test.mjs suite with the fix: 20/20 passed

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Signed-off-by: mtoohey <[email protected]>
(cherry picked from commit 3ae06d410f1a36e3e04bd1adb27d5d067629a873)
@matt2e
matt2e requested a review from a team as a code owner November 9, 2026 04:44
@github-actions github-actions Bot added the codex-security-review-current The posted Codex security review matches its recorded range. label Nov 9, 2026
@github-actions

github-actions Bot commented Nov 9, 2026

Copy link
Copy Markdown

🔐 Codex Security Review

Note: This is an automated, security-focused review generated by Codex.
Use it as a supplement to human review; false positives are possible.

Scope

  • Exact PR diff: 65e864301d2e51a93003816b45e77cae96be901f...bdf8ab645addb698e8e2f10737a44a85c56cd4f9
  • Model: gpt-6.1-sol

💡 Click "edited" above to see earlier reviews for this PR.


Review Summary

Overall Risk: NONE

No concrete security, correctness, or reliability findings in the authorized PR range.

Findings

No concrete security, correctness, or reliability findings were identified.

Notes

  • Reviewed the watchdog changes, regression tests, CI callers, and relevant history by read-only inspection. Scripts and tests were not executed.

Generated by Codex Security Review |
Requested by: @matt2e |
Workflow run

@kazemek21 kazemek21 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed the PR and went through the Nexus review-tracker notes. I didn't see any P2-or-higher issues in this change.

Posted by Codex (gpt-6.1-sol) on behalf of kylez

@matt2e
matt2e merged commit d2c3234 into main Nov 9, 2026
93 checks passed
@matt2e
matt2e deleted the mtoohey/flake-ci-hermit-retry branch November 9, 2026 21:40
wpfleger96 added a commit that referenced this pull request Nov 10, 2026
…e-mode

* origin/main:
  ci(docker): skip relay image publishes for main pushes outside the build context (#9310)
  Add manual-only relay image publishing (#9303)
  fix(mobile): bump Flutter to 3.47.9 for Android TLS fix (#9309)
  Serve video and poster previews for invite links (#9306)
  docs: point the README to Buzz 1.0 in block/buzz-app (#9296)
  fix(db): give the replica-fence probe its own pool and instrument it (#9297)
  fix(relay): never close connections when cross-node fan-out falls behind (#9302)
  test: fix timing-dependent unit tests in buzz-cli, buzz-agent and buzz-admin (#9273)
  fix(ci): classify hermit retry timeouts by watchdog kill, not whole seconds (#9274)
  test(ios): wait for a key, foreground-active window before presenting in RunnerTests (#9275)
  test(desktop): wait on real readiness signals in audience and community-rail specs (#9277)

Signed-off-by: Will Pfleger <[email protected]>

This branch was previously deployed

1 inactive deployment
codex-review — bdf8ab64 Deployed Nov 9, 2026 by matt2e via Run Codex Security Review #13951
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

codex-security-review-current The posted Codex security review matches its recorded range.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants