ci: run the npm audit gate on releases only; retry transient faults

[bolichen97]

Problem / Motivation

Dependency Audit / Audit Production Dependencies fails with npm audit timed out after 120s for website/package-lock.json whenever the npm registry is slow, and the red X then has to be re-run by hand — three times in a row on #8356 tonight, and repeatedly on other PRs (#8282 had the same).

Why it matters

The gate blocks PR Readiness, nightly and release builds. A flake that needs a maintainer to press "re-run" turns every registry hiccup into a human interruption, and teaches people to treat this gate's red as noise — which is how a real vulnerability gets waved through.

What changed (motivation → approach → change)

Symptom → root cause: two things made a registry hiccup into a terminal failure.

1. On a cold runner npx --yes npm@10.8.2 first downloads that npm — inside the audit's own 120s timeout — before a single advisory is asked for. That is why it is always the first lockfile (website/package-lock.json) that times out.
2. Every failure was terminal. The audit is a read-only, idempotent query, so a stall or connection fault is exactly the class of failure a bounded retry answers — but the gate ran once and gave up.

Change (scripts/check_npm_audit.py):

• Warm-up step. warm_npm resolves the pinned npm once up front (npx --yes npm@10.8.2 --version) and fails closed unless it prints exactly the pinned version, so the download is never charged against an audit and a wrong npm cannot audit.
• Bounded transient retries. _with_transient_retries gives each attempt a 180s ceiling (the previous 120s left no headroom: on this PR's own CI run one lockfile timed out at 120s and then completed on its next attempt — slow, not hung) and retries up to AUDIT_ATTEMPTS (3) with a short backoff when an attempt times out, raises a subprocess error, or exits with a status other than npm's documented 0/1 results and stderr carries one of npm's connection-level markers (TRANSIENT_STDERR_MARKERS: ETIMEDOUT, ECONNRESET, EAI_AGAIN, E503, …). Exit 0/1 are never transient whatever stderr says (1 is the audit answering "vulnerable"); every other failure stays definitive and unretried.
• One shared budget. A Deadline (AUDIT_TOTAL_BUDGET_SECONDS, 720s) threads through the warm-up and every audit: no attempt gets more than the time left, and no retry starts unless the budget still holds its backoff plus a full ceiling (a retry that woke up already short would only time out), so retries cannot outgrow the job's ceiling — raised from 10 to 15 minutes to hold the budget plus checkout/toolchain setup (the test now pins timeout-minutes * 60 > budget + 120). Exhausting attempts or budget fails closed naming the attempt count, so a persistent registry outage reads as one rather than as a flaky gate.

The fail-closed contract is otherwise unchanged: missing tool/manifest/lockfile, malformed reports, undocumented exit statuses and real findings all still fail on the first try.

Releases only. Even so hardened, the gate stayed red on this very PR: the registry answered website/electron/package-lock.json in over 180s twice in a row (see Manual verification). The per-PR dep-audit job is removed from code-review.yml, and the nightly's dependency-vulnerability-gate job (with the needs: on it) from nightly.yml, where the same slow hours had failed the nightly for hours at a stretch and blocked every build behind it. A gate people learn to re-run until green is not a gate. It runs where a vulnerable dependency would actually ship — release.yml, before any wheel or desktop build — so nothing vulnerable is published, and a PR that adds or bumps a dependency is checked by the release that would carry it. Branch protection pins no check by name and PR Readiness aggregates the Code Review workflow conclusion, so nothing else references the removed jobs; the review-prompt lists of PR checks, the spec, the CI doc and the script's expiry-notice comment are updated to match.

Tests

test/test_dependency_vulnerability_gate.py (62 passed):

• is_transient_failure parametrized: connection markers with exit ≥2 are transient; exit 0/1 never are; a non-network stderr or an empty one is definitive.
• Timeout → retried AUDIT_ATTEMPTS times with the declared backoff, then fails closed naming "attempt 3 of 3; giving up".
• Timeout, then ECONNRESET, then a clean report → findings returned after three calls, both retries logged.
• A definitive failure (exit 2, non-network stderr) is not retried and the sleeper is never called.
• Exit 1 with a real high finding and a network marker on stderr is parsed as findings, not retried.
• Budget: a retry that cannot afford its backoff plus a full ceiling is refused; the last attempt is shrunk to the time left; an exhausted budget fails closed without spawning; one Deadline is shared across lockfiles; the workflow's timeout-minutes clears the budget.
• Workflows: neither code-review.yml nor nightly.yml calls the reusable gate or needs: it; release.yml still does, unconditionally, and its build jobs still depend on it.
• Warm-up: command shape; a stall is retried and the version checked; wrong version / empty output / non-zero exit fail closed without retry; main warms before auditing and threads one Deadline through every call.

Manual verification

The CI runs of this PR exercised the real path against a degraded registry. First run (120s ceiling / 480s budget): warm-up passed, website/package-lock.json timed out at 120s on attempt 1 and completed on attempt 2, website/electron/package-lock.json timed out twice and the gate failed closed naming attempt 2 of 3; giving up when the budget could no longer afford a full retry — exactly the designed behaviour, and the measurement behind the 180s / 720s numbers now in the diff. Second run (180s / 720s): website passed first time, website/electron timed out at 180s twice, and site was refused a retry the budget could not hold — the registry was hanging for that lockfile, not merely slow, which is what settled the decision to take the gate off the PR path rather than widen the numbers again. The nightly run five hours earlier, on a healthy registry, took ~1 minute per lockfile.

Related Issues

Observed on #8356 (three consecutive timeouts, 2026-09-04) and #8282.

Pattern harvest

Rule candidate: review-prompt
Pattern: a CI gate that shells out to a network service with a single attempt and no distinction between a transient (timeout/connection) failure and a definitive one, so infrastructure noise reads as a code failure.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable)
• No secrets, credentials, or internal references in the diff

[github-actions]

Design Review (Fable 5, fork) — ✅ PASS

Design-level review of 99f363a362070e2ea8f042bf81540e09ae1ddf3b via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Fixes both named root causes (cold-runner download, terminal single attempt) while keeping every definitive failure — including exit 1 — fail-closed and unretried, inside a budget the job timeout can absorb.

[DESIGN-REVIEWED] 99f363a

[github-actions]

First Principles Review (Fable 5, fork) — ✅ PASS

Premise-level review of 99f363a362070e2ea8f042bf81540e09ae1ddf3b via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All checks are done. The change is a fix for a reported, reproducible defect with two named causes, both addressed; nothing duplicates an in-repo mechanism, and the docs edits are mandated by the same-commit spec rule. Here is the review.

First-Principles-Verdict: PASS

A reported flake (#8356 ×3, #8282) gets its two counted causes fixed — cold-download inside the audit timeout, and no transient/definitive distinction — nothing rides along.

What this change ships

Intent: stop a slow npm registry from failing the dependency-audit gate so maintainers stop hand-rerunning a red X — a FIX.

1. A registry stall or connection error now retries up to 3 times instead of failing the gate — justified (the reported defect).
2. The pinned npm downloads once up front, before any audit's timer starts — justified (named cause: the first lockfile always paid the download).
3. All attempts share one 480s budget so retries can't outlive the job's 10-minute ceiling — justified (3 lockfiles × 3 × 120s = 1080s would exceed timeout-minutes: 10; the budget is what keeps the fail-closed message reachable).
4. Warm-up verifies the resolved npm prints exactly 10.8.2, failing closed otherwise — justified (fail-closed integrity check on a gate whose workflow already refuses caches for the same reason).
5. Exit 0/1 and non-network errors stay unretried, first-try fail-closed — justified (preserves the documented contract).
6. Two new progress lines in CI output (Resolving npm@…, retrying in Ns) — declared, justified.
7. Spec updates in security.md and ci-and-reviews.md — derived (AGENTS.md same-commit spec rule).

Watch

• The harvested pattern ("CI network call, single attempt, no transient distinction") has 1 counted sibling this leaves unfixed: the single-attempt curl in scripts/build-ffmpeg.sh:30 (grep curl|retry over scripts/; a --retry flag would be the whole fix there). Deferred is fine — different script, different failure surface.

[FIRST-PRINCIPLES-REVIEWED] 99f363a

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed 99f363a362070e2ea8f042bf81540e09ae1ddf3b via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 99f363a

[github-actions]

GPT 5.6 Review (fork) — ✅ no blocking findings

Reviewed 99f363a362070e2ea8f042bf81540e09ae1ddf3b via the fork AI-review pipeline; updated in place on each push.

Review details

FINDING -- scripts/check_npm_audit.py:516 -- "deadline.can_retry()" ignores the pending backoff, so a retry can start with under 120s and falsely time out -> Fix: require remaining budget to cover the backoff plus a full attempt.
[GPT-REVIEWED] 99f363a