mcpx testing: flake policy -- race, shuffle, repeat runs, quarantine with expiry, a second OS #1973

Relocated from dezren39/darwin-nix#130 on 2026-10-28 (placement audit): mcpx's single source lives in developing-today/code (pkgs/mcpx); re-home tracked at #309. Original body below; issue references inside it are historical (dezren39/nix numbering).

Parent: #99

Summary

A flaky test found only on the lead's third full-suite run is the motivating case: a flake that one run in three exposes is invisible to a CI that runs once, and invisible to agents who (per #127) will no longer run the full suite at all. This issue decides how flakes are found (race detector, shuffled order, repeat runs), where that costs time (changed tests per PR, everything nightly), and what happens when one is found (fix vs quarantine; never a silent retry). It also records that macOS vs Linux differences (#35) are a flake source CI cannot see today.

Original notes

• testing
  • importantly, as we add more testing agents should not run every single test basically ever. below documents a rough idea of a ci job design/policy, the main priority is being short as possible.
  • agents should make at least 1 simple, new test for every bug observed from any source, when cheap to do so-- aggresively test many permutations.

(Why these: repeat runs and -race cost time the second line forbids spending on every PR, so most of it moves to scheduled jobs; "aggresively test many permutations" includes permutations of order and schedule, which is what -shuffle and -race supply. The seed bullet under the third line is owned by #93 and quoted in #128; the only part used here is that the shuffle seed derives from the run seed.)

Current state

• CI runs go test ./... -count=1 once, ubuntu-latest only, no -race, no -shuffle (.github/workflows/mcpx.yml:65, unchanged at 05c78b2). No test calls t.Parallel(), so the main nondeterminism is order, timing, subprocesses and goroutines inside the daemon.
• Today the only repeat runs are by hand, in the integrator's pre-merge routine: PR Update Rust crate bacon to 2.14.0 #69/Update Rust crate globwalk to 0.9.0 #70 "Suite 2× clean", Phase 2: delegated Manage cannot add members #171 "Suite 3× clean before my change and 2× after", Phase 5: tag event bodies must travel with the event #182 "Suite 3× clean" (plus a daemon count across the three). Update Rust crate tracing to v0.1.40 [SECURITY] #127 rule 7 keeps that exception until the jobs below replace it.
• Flakes and races found so far, all by repetition or by -race run by hand:
  • PR build(deps): bump build/buck/preludes/buck2-prelude from d956689 to 4a718b8 #46: "A flaky client test: the older fake answered the client's own replies ... One run in three panicked. Found by running the suite repeatedly, not once."
  • PR chore(deps): bump rustix from 0.37.19 to 0.37.25 in /sources/pavex/examples/realworld #56 / test #61: a data race in internal/tasks + internal/mcpserver, "~1 run in 5 under -race", "Invisible because the suite does not run -race."
  • internal/testsupport/build.go:47-53: building the binary per test under go test ./... load "produc[ed] a different spurious failure on each run".
  • fix(deps): update rust crate axum to 0.6.20 #35: two tests passed on macOS and failed on Linux (the tests were wrong). Development happens on macOS; CI is Linux. Once agents stop running the full suite locally (Update Rust crate tracing to v0.1.40 [SECURITY] #127), nothing runs the full suite on macOS any more.
• go help testflag: -shuffle off,on,N randomizes test order within a package and reports the seed; -count n runs each test n times; results are reported per run. The race detector costs roughly 2-20x CPU and 5-10x memory (https://go.dev/doc/articles/race_detector); since Go 1.20 it needs no cgo on macOS, and on Linux needs a C linker (present on ubuntu-latest) (https://go.dev/doc/go1.20).
• The repo is public, so GitHub-hosted macOS runners cost no minutes (they are slower to start and fewer run concurrently).

Proposed policy

1. No retries that hide flakes. No gotestsum --rerun-fails, no "re-run failed jobs until green" as a merge path. A test that failed once and passed on retry is a found flake: it gets an issue.
2. Changed tests are repeated before merge. Tests added or modified in a PR (selected by the Update Rust crate tracing to v0.1.40 [SECURITY] #127 script) run in a PR job with -race -shuffle=<derived> -count=N -- N=10 for unit, N=3 for e2e as a start. Runs in parallel with the main matrix so it does not lengthen the longest job (Bump golang.org/x/crypto from 0.16.0 to 0.17.0 in /sources/launcher #95's budget).
3. The whole suite is stressed on a schedule. Nightly (or per N commits to main): go test -race -shuffle=<derived> -count=3 ./... on ubuntu and macOS. Failures open or update an issue automatically; the job never blocks a PR.
4. -shuffle seed derives from the run seed (Bump golang.org/x/crypto from 0.10.0 to 0.17.0 in /sources/graph #93), e.g. the first 63 bits of its hash, so a failing order is reproduced from the one seed CI printed. -shuffle=on (clock-seeded) is not used in CI.
5. Fix now, or quarantine with an expiry. A quarantined test calls a helper, e.g. testsupport.Quarantine(t, "#123", "2026-11-15"), that skips it until the date and then fails -- so a quarantine cannot become permanent. The nightly job runs quarantined tests anyway and reports them without blocking. A guard lists quarantine calls and fails on a missing issue link or an expired date (Add omasnap upstream repository to flake inputs #129).
6. Reproduce locally by name only: go test -count=20 -race -shuffle=<seed> ./internal/<pkg> -run '^TestX$'. Never "run the whole suite a few times" locally as a way to reproduce (Update Rust crate tracing to v0.1.40 [SECURITY] #127: cost; chore(deps): bump build/buck/preludes/buck2-prelude from d956689 to e7457f6 #52's leak is fixed by Phase 5: tag event bodies must travel with the event #182, so the cost is time only).

Approaches

A -- retry on failure (least recommended)

Let CI retry failed tests (or jobs) once or twice and merge on green. It is what many projects do because it makes the dashboard quiet. It is exactly wrong here: the #46 flake panicked one run in three, so a single retry passes it ~89% of the time and the bug ships; retries also double the time of every red run, against the "short as possible" priority, and teach everyone that red means "try again". Rejected.

B -- fix-now always, no quarantine

Any flake blocks all merges until fixed. Honest and simple. In practice a flake in, say, the TUI blocks an unrelated protocol fix, and the path of least resistance becomes deleting or t.Skip-ing the test with no record -- a worse quarantine than a tracked one. Fine as the default intent; too rigid as the only rule.

C -- detect by repetition off the critical path, fix-now by default, quarantine only with issue and expiry (recommended)

Policy 1-6 above: -race/-shuffle/-count on changed tests per PR (cheap, parallel), the whole suite stressed nightly on two OSes, no hiding retries, quarantine as a dated, linked, guarded exception. Recommended because it finds the classes that have actually bitten mcpx (order/timing flakes, data races, OS differences) at a cost outside the PR's longest job, and it makes the escape hatch visible and self-expiring instead of forbidding it and getting silent skips.

Sub-choice: other operating systems

Recommended: macOS nightly now, promote to a PR shard if a macOS-only bug escapes. Other OSes and distros (alpine, fedora, windows) belong to the environment matrix in #97; this issue only asks that at least one non-ubuntu OS runs the suite on a schedule.

Open questions

• N for -count per tier (10 unit / 3 e2e is a guess; measure against Bump golang.org/x/crypto from 0.16.0 to 0.17.0 in /sources/launcher #95's budget).
• Does -race on the e2e package help? The race detector instruments the test binary, not the mcpx binary it builds. A race-enabled mcpx needs go build -race ./cmd/mcpx in testsupport behind an env switch -- probably worth it for the nightly job.
• Is the nightly job allowed to open issues automatically, or only post to a tracker comment?
• Quarantine helper name and where the "expired" check runs (test itself vs a guard vs CI).

Ease · Impact · Related

Ease M · Impact high
Depends on / related: #93 (run seed -> shuffle seed), #95 (PR job, nightly job, budget), #97 (OS/environment matrix), #35, #52 (closed, fixed in #182), #46, #56, #61; siblings #127 (rule 7: the integrator's repeat runs, which this issue moves into CI), #128, #129, #131.

Security/auth deferred; see tracker.

Acceptance criteria

• A flake policy is decided and recorded (on this issue and in the contributor page, mcpx testing: a contributor test page agents load, kept out of the user-facing AGENTS.md #1976): how flakes are found (race detector, shuffled order, repeat runs) and where that time is spent (changed tests per PR, full coverage nightly).
• The found-a-flake process is defined: fix vs quarantine, quarantine always with an expiry, and never a silent retry.
• The macOS-vs-Linux flake source is recorded with a route to visibility (second OS in CI) or an explicit deferral with its reason.
• The work described in the Summary/Scope above is delivered via a branch + PR (no direct push to main); verification runs in GitHub Actions CI, not on the sandbox.
• Evidence (policy text and any CI changes) is recorded on this issue before it is closed.