fix(test): make the exec-timeout tests deterministic under load

[konih]

Problem

task check — the gate AGENTS.md rule 4 requires green before every commit — was
non-deterministic on main (7247e6d). Under full-suite -race load:

• internal/provider TestExecDigestPin/match_allows_exec → signal: killed
• hack/spikes/provider TestContract and TestIsolation → signal: killed

Diagnosis (verified, not assumed)

The reported root cause is correct, and I measured it rather than taking it on trust.

Repro. 36 CPU burners (18-core machine) + go test -race -count=1 ./..., 6 runs.
-count=1 matters: without it the Go test cache serves runs 2..n and the flake looks
intermittent when it is actually deterministic under load.

Result: 6/6 runs failed. Every failure landed exactly on its deadline —
TestExecDigestPin/match_allows_exec (1.00s), TestContract (5.01s),
TestIsolation (5.00s).

Instrumentation. I temporarily raised each call site's deadline to 120s and logged
elapsed wall-clock, then re-ran the same loaded full suite:

call site	deadline	measured elapsed under load	unloaded
internal/provider match_allows_exec	1s	1.32s, 1.58s, 1.76s, 2.46s	~200ms
spikes TestContract exec	5s	3.50s, 4.12s, 4.43s, 4.46s, 5.31s	~200ms
spikes TestIsolation exec	5s	1.24s, 2.39s, 2.42s, 2.81s, 3.25s	~200ms
spikes TestContract HTTP	5s	1.6ms – 3.0ms	—

Every one of those completed with err=<nil> at the 120s deadline. That rules out the
alternatives explicitly: a deadlock would have shown elapsed ≈ 120s, a leaked process or
unbuffered channel would not resolve at all, and a data race would have tripped -race.
It is a healthy child process losing a race against a short wall-clock deadline. Confirmed:
timeout-under-load.

A secondary probe explains the magnitude. Running the affected package alone under the same
36 burners, the child costs ~200ms on its first exec and ~12ms on every subsequent one.
The first exec of a just-written binary pays page-in plus macOS code-signature validation; in
the full-suite run 18 sibling package binaries are compiling and executing at the same time,
stretching that first spawn 6–25x. Consistent with the observation that the failing test is
always the first spawn of each fixture binary in its package.

Fix

A named constant per package with the measurement written into the comment:

const execTestTimeout = 60 * time.Second

Applied to every call site that spawns the real child (internal/provider/exec_test.go,
internal/provider/isolation_test.go, hack/spikes/provider/{contract,isolation}_test.go).

Why a generous named constant rather than deriving it from t.Deadline(). The derived
variant needs branches for "no deadline set" (-timeout 0) and "budget already exhausted"
that no run in this repo ever executes. A repo that has been bitten repeatedly by
tests-that-cannot-fail should not gain unexercised logic in its test helpers. 60s is ~11x the
worst measured spawn (5.31s) and costs nothing when things are healthy — the call returns in
milliseconds. The comment records the measurement so nobody optimises it back down.

What is not weakened. Nothing in these tests asserts timeout behaviour; they pin digest
verification (REQ-E5-S03-02) and argv/env scrubbing (REQ-E5-S03-01/03), for which the deadline
is incidental plumbing. The dedicated timeout assertions are untouched and already robust:
TestTransportTimeoutUnavailable (50ms) and spike TestStates (100ms) both block their
handler until the test releases it, so a slow machine can only make them slower, never wrong.

Failure messages now name the deadline. A recurrence reads
matching pin must allow exec (timeout 1m0s): signal: killed rather than a bare
signal: killed that reads like a crash — the ambiguity that cost four agents a session.

Proof the assertions still bite (mutation, both polarities)

The risk with a timeout fix is neutering the assertion. Four mutations, each grepped in the
file to confirm it landed before believing the result, each reverted after:

#	mutation	expected	observed
A	VerifyExecDigest → return nil (fail-open)	refusal subtests RED, match_allows_exec still green	RED: missing_pin_refuses, mismatch_refuses. match_allows_exec passed — the pin matched anyway.
B	invert !strings.EqualFold(got, pin)	match_allows_exec RED	RED: match_allows_exec, TestIsolationNoWriteToken, TestIsolationNoCredentialInArgv
C	change the child's === ARGV DUMP === banner	dump assertion RED	RED: match_allows_exec on the stdout-content assertion
D	spike ScrubEnv → append(os.Environ(), configured...)	spike TestIsolation RED	RED: 8 leaked-canary errors + 10 credential-name errors

Polarity B is the load-bearing one: it is the only mutation that reds the exact subtest whose
timeout changed, and its output shows the new diagnostic ((timeout 1m0s)) working. A alone
would have proven nothing about this change.

git status was verified clean after each revert — internal/provider/transport.go is PR #26's
file and no mutation hunk is in this diff.

Before / after

	command	result
before	go test -race -count=1 ./... × 6, 36 burners	6/6 FAILED
after	identical script, identical burner count	0/6 failed
after	task check × 2	exit 0, exit 0 — zero failure lines
after	go test -race -count=5 ./internal/provider/...	ok
after	go test -race -count=5 ./hack/spikes/provider/...	ok

Note on the two task check runs: task test is go test -race ./... with no -count=1, so
the second run is partly cache-served and mostly proves the gate is satisfied. The load claim
rests on the 0/6 re-run of the verbatim before-script.

Files touched

• internal/provider/exec_test.go — constant + 6 call sites + diagnostic message
• internal/provider/isolation_test.go — 2 call sites (5s → constant) + diagnostic messages
• hack/spikes/provider/main_test.go — constant next to the fixture-binary vars
• hack/spikes/provider/contract_test.go — exec + HTTP call sites
• hack/spikes/provider/isolation_test.go — exec call site
• CHANGELOG.md — regenerated (task changelog-write)

No production code changed. Nothing outside this lane's ownership: transport.go,
transport_test.go, internal/forge/**, internal/compare/**, schemas/**,
docs/architecture/**, .github/workflows/**, Taskfile.yml, cliff.toml are all untouched.

Residual risks

• Not fixed (deliberate): spike TestStates uses one 100ms deadline for all three
  handlers, including the non-blocking garbage/stale cases where the deadline is
  incidental. In principle that can flake under load. It did not flake in 6/6 loaded runs,
  the 100ms is load-bearing for the timeout handler's assertion, and if garbage/stale
  ever exceed it the test fails loud and correct (unavailable ≠ invalid), never fail-open.
  Restructuring a table-driven test for an unreproduced hypothesis is scope creep in a lane
  billed as surgical. Logged here so the next reader knows it was considered.
• 60s is an empirical margin, not a proof. If CI hardware is ever an order of magnitude more
  contended than this measurement, the constant is the single place to raise — and the comment
  says so.

Note on the base

origin/main advanced from 7247e6d to a2a2b17 (PRs #23/#24) while this lane ran, producing
a CHANGELOG.md-only conflict. Resolved by merging origin/main into the lane — the pattern
4d8f577 already established in this repo — and regenerating with task changelog-write. No
force-push, no history rewrite. git diff origin/main...HEAD is exactly the six files listed
above. Both task check runs and the 0/6 loaded re-run above were performed on the merged
tree.

---

Review round 2 (F1, F2, F5)

F1 — leaked credential names, never values (both files). The per-line assertion in
internal/provider/isolation_test.go and hack/spikes/provider/isolation_test.go printed
the full NAME=VALUE. That branch fires exactly when ScrubEnv has regressed, which is exactly
when the line holds a real host credential — so the test that detects a leak also amplified
it into whatever log captured the failure. Both now print only the name. The assertion is
untouched: it still says which variable leaked.

F2 — redacted sanity dumps (taken, optional). The five dump:\n%s messages now print
through redactDumpValues, masking the value half of every NAME=VALUE line while leaving the
=== SECTION === banners and the stdin JSON readable. Assertions always run against the raw
dump — only the printed form changes — so no check is weakened.

Safe-mutation evidence (reviewer's shape: leaks only the tests' own configured canaries,
never touches os.Environ(), so no real credential is exposed):

mutation	expected	observed (both packages)
secretName filter disabled for configured env only	leak assertions RED	RED — 4 leaked-canary errors + 2 credential-name errors, the latter now reading UPSTREAM_TOKEN / LDAP_SECRET with no values
configured env dropped entirely	sanity Fatalf RED	RED — fires in both packages, printing === ENV DUMP === / PATH=<redacted> / intact stdin JSON

The second mutation also caught a defect in my first cut of redactDumpValues: it redacted the
=== SECTION === banners themselves (they contain =), gutting the diagnostic. Fixed with a
name != "" guard and re-verified.

F5 — base refreshed to 9e50e17. Merged origin/main in (no force-push), merge subject
:wrench: chore(release): … so cliff.toml's first parser skips it, and task changelog-write
committed last. task changelog-verify green standalone (verify-changelog: ok).
internal/provider/transport.go and hack/spikes/provider/transport.go are byte-identical to
origin/main — every mutation was reverted and verified.

Gates re-run on the merged head, graded on stage banners, not exit code: both task check
runs reached all 13 stages (fmt, vet, lint, test, coverage, build, dogfood-comparison, compare-exitgate-test, changelog-verify, release-changelog-gate-test, release-verify-tag-gate-test, docs-gates, lint-depguard-test), exit 0, zero failure lines.
Loaded repro re-run on this head: 0/6.

Correction to the original report: the flake also reds task coverage
(go test -coverprofile ./internal/...), not only task test — main is genuinely broken at
two stages, not one.

Containment re-verified on the final tree: 0 hits for glpat-, github_pat_, ics_v1_,
ATATT, and the two hex token prefixes across the pushed range, the tracked tree, both gate
logs, the commit messages, and this PR body.

Base refreshed again: origin/main advanced to 669d805 (PR #25) while round 2 was
verifying. Merged in the same way, task changelog-write committed last, gates re-run — that
PR adds a 14th stage, so both runs now reach all 14 banners (… lint-depguard-test, lint-workflow-pins-test), exit 0, zero failure lines. git diff origin/main...HEAD remains
exactly the six files.