fix(ci): pin test-wasm-browsers' timeout and retry-wrapper shape in ci_wiring - #440
Merged
Merged
Conversation
…i_wiring
The `nick-fields/retry@v3` wrapper around `test-wasm-browsers` inverted its
own failure handling: `retry_on: error` never retries a timeout, the only
failure shape this job has actually hit (three wedged safari sessions on
2026-09-03, ~22 minutes each with ~17 of silence), while it does retry a
genuine non-zero exit that should just have been reported. Its `command:`
input also carried a literal backslash (`--\${{ matrix.browser }}`) that
only worked because the action handed the string to a shell that stripped
it.
`dobby-coder` has no `workflows: write`, so the fix -- a job-level
`timeout-minutes: 12`, `WASM_BINDGEN_TEST_TIMEOUT` 120 -> 30, and the
wrapper replaced by a bare `run:` with the backslash gone -- is posted as
a comment on #432 for a maintainer to apply, not pushed here.
Adds `the_wasm_browser_job_has_an_honest_timeout_and_no_retry_wrapper` to
pg-core/tests/ci_wiring.rs: the job carries a job-level `timeout-minutes`,
no step uses `nick-fields/retry`, the test command is the bare,
backslash-free `run:` line, and all three browsers stay in the matrix.
Verified locally against a scratch copy of the patch: the four checks pass
once applied, and each goes red on its own when the corresponding half of
the regression is reintroduced (timeout removed, retry wrapper restored,
backslash restored, safari dropped from the matrix).
This assertion is red on this branch until the workflow patch lands, same
shape as #429/#438.
Part of #247. Implements #416's decisions for #432.
Contributor
Author
Workflow patch for
|
`dobby-coder` has no `workflows: write`, so #440 landed the four `ci_wiring.rs` assertions deliberately RED and posted this build.yml patch for a maintainer. Applied verbatim from that comment, which is itself verbatim from #432's section 1. Three substantive changes to `test-wasm-browsers`: a job-level `timeout-minutes: 12` (~2x the slowest observed success, against a 20-minute budget that was ~48x the suite's 24.68s runtime), `WASM_BINDGEN_TEST_TIMEOUT` 120 -> 30, and `nick-fields/retry` replaced by a bare `run:`. The wrapper was configured backwards -- `retry_on: error` re-rolled genuine non-zero exits while never engaging for a timeout, the only shape this job has ever hit. Dropping it also deletes the literal backslash in `--\${{ matrix.browser }}`, which survived into the resolved command and worked only because the action handed it to a shell that stripped it. The matrix, `runs-on`, checkout, `cargo install wasm-pack` and the firefox apt step are unchanged; all three browsers stay, safari included. With this applied, `--test ci_wiring` is 16/16 locally, including `the_wasm_browser_job_has_an_honest_timeout_and_no_retry_wrapper` and #429's `no_workflow_step_captures_dollar_question_mark`.
Contributor
|
Workflow half applied — The new budget is confirmed honest by this run rather than by argument. All three legs passed well inside |
rubenhensen
approved these changes
Sep 10, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves the implementation half of #416, part of #247. Closes #432.
CI status
Test workspace (pg-core)is red becausethe_wasm_browser_job_has_an_honest_timeout_and_no_retry_wrapperfails — on purpose, see below.Test workspace (pg-pkg),Test workspace (pg-cli)andTest workspace (cryptify)show red alongside it, but that'stest's matrix (crate: [pg-core, pg-pkg, pg-cli, pg-ffi, cryptify]) fail-fast cancelling the other legs the momentpg-coregoes red — each job log ends mid-compile with##[error]The operation was canceled., not a test that ran and failed.pg-ffifinished compiling before the cancellation reached it, which is why that one leg still shows green. None of the four are required checks: only the twoWire compatcontexts are (scripts/ruleset-drift.shreads the ruleset back and confirms that set). There's nothing left to fix from this container — the one real failure is the pin, and it stays red until a maintainer applies the workflow patch below, sincedobby-coderhas noworkflows: write.What this PR does
test-wasm-browsersis wrapped innick-fields/retry@v3withretry_on: errorand a 20-minute per-attempt timeout, and itscommand:input carries a literal backslash (--\${{ matrix.browser }}). Both are defects #416 found and settled:errorin that action's taxonomy, somax_attempts: 2never engaged on the only failure shape this job has hit — three wedged safari sessions on 2026-09-03, ~22 minutes each with ~17 minutes of total silence. Meanwhile a genuine non-zero exit does get retried, hiding a real failure behind a second roll.command:string to a shell that stripped it beforewasm-packever saw it.Since
dobby-coderhas noworkflows: write,.github/workflows/build.ymlisn't touched by this PR. This PR adds the pin:the_wasm_browser_job_has_an_honest_timeout_and_no_retry_wrapperinpg-core/tests/ci_wiring.rs, asserting againsttest-wasm-browsers' job block:timeout-minutesnick-fields/retryrun: wasm-pack test --release --headless --${{ matrix.browser }} ./pg-wasmchrome,firefox,safari) stay in the matrixThese assertions are RED on this branch, on purpose, until the workflow half below is applied — same shape as #429/#438.
The workflow patch (a maintainer applies this)
A one-paste
gh api -X PUTcommand that applies this directly to this PR's branch (no checkout needed) is in this comment — also posted on #432 per @rubenhensen's amendment there.Replace the whole
test-wasm-browsersjob (currentlybuild.yml:81-107) with:Three substantive changes:
timeout-minutes: 12at job level,WASM_BINDGEN_TEST_TIMEOUT120 → 30, and the retry action replaced by a barerun:with the backslash gone. The matrix,runs-on, checkout,cargo install wasm-packand the firefox apt step are unchanged.no_workflow_step_captures_dollar_question_markstill passes against it — no step here captures$?.Verification
cargo test --manifest-path pg-core/Cargo.toml --features test,rust,stream --test ci_wiring:build.yml): 15/16 pass,the_wasm_browser_job_has_an_honest_timeout_and_no_retry_wrapperfails as expected (no job-leveltimeout-minutesyet).build.ymlwith the patch above applied: all 16 pass, nothing else broke.Checked each assertion is neither fail-open nor fail-closed-for-the-wrong-reason, against the same local patched copy:
timeout-minutes: 12→ assertion 1 (job-level timeout) goes rednick-fields/retry@v3step → assertion 2 (no retry wrapper) goes red--\${{ matrix.browser }}→ assertion 3 (bare run, no backslash) goes redsafarimatrix entry → assertion 4 (all three browsers) goes redOne thing this caught along the way: the patch's own explanatory comments mention
nick-fields/retryby name in prose (e.g. "notnick-fields/retry"), so a naivejob.contains("nick-fields/retry")check false-reds even after the correct patch lands. Assertion 2 checks for the literaluses: nick-fields/retryinvocation instead.After verifying, the local workflow edit was fully reverted —
git diff --name-only origin/main...HEAD -- .github/workflows/prints nothing; no.github/workflows/changes are part of this PR.cargo fmt/cargo clippy(--features test,rust,stream -- -D warnings) pass clean onpg-core.Explicitly out of scope, per #432: filing an issue when this job goes red — deferred to the fleet-wide issue-filing convergence question (#416).
Done when
.github/workflows/build.ymlonmaincarries the job exactly as in the patch above (maintainer action)pg-core/tests/ci_wiring.rsRun wasm tests in browsersrun onmainis green on all three legs after the patch landsReviewed by dobby: code-comments rule (only rule in force) and Review Dobby 2 findings, over the full PR diff (pg-core/tests/ci_wiring.rs) — pg-core built clean,
cargo fmt --checkandcargo clippy --features test,rust,stream -- -D warningspass,cargo test --test ci_wiringindependently reproduced 15/16 passing withthe_wasm_browser_job_has_an_honest_timeout_and_no_retry_wrapperfailing at line 1028 exactly as documented — approve.Attention: pg-core/tests/ci_wiring.rs:1028 — that failing assertion is RED by design (dobby-coder lacks
workflows: write) pending a maintainer applying the build.yml patch already posted on this PR and on #432; independently reverified against a local patched copy (16/16 pass), then reverted. No PR Dobby action needed.