Skip to content

pre-push gate (make test-fast-parallel) flakes on 3 unrelated timing tests under fleet load; blocks polecat handoff (gc-8jrtx) - #156

Merged
zook-bot merged 5 commits into
mainfrom
polecat/gc-8jrtx
Aug 22, 2026
Merged

pre-push gate (make test-fast-parallel) flakes on 3 unrelated timing tests under fleet load; blocks polecat handoff (gc-8jrtx)#156
zook-bot merged 5 commits into
mainfrom
polecat/gc-8jrtx

Conversation

@zook-bot

Copy link
Copy Markdown

Summary

Symptom

The .githooks/pre-push gate (make test-fast-parallel) failed three consecutive
runs on branch polecat/gc-kawr5 (commit c239b74) between 20:20Z and 21:10Z on
2026-08-14, each time on a DIFFERENT timing-sensitive test in a DIFFERENT package.
The branch diff cannot plausibly cause any of them (see Attribution).

run job failing test package
1 unit-core TestProvider_StartCancellationInterruptsForegroundChild internal/runtime/exec
2 (killed at 9/10 jobs, 0 failures)
3 unit-cmd-gc-1-of-6 TestCityRuntimeForceShutdownTearsDownAfterLateAsyncSweep cmd/gc
4 unit-core TestDisableAndPurgeExactTokenConflictAndPeerCleanRecovery internal/productmetrics

Failure messages are all timing/lifecycle shaped:

  • 'interrupt marker = "", want "interrupted" (rollback trap never ran)'
  • 'force shutdown missed the late async-started runtime'

Host load average during the failing window: 19-27 (fleet-wide, not this session).

Each failure is a flake, not pre-existing red on main

Verified standalone, quiet machine, both sides:

  • runtime/exec test: 16 runs on branch + 16 on origin/main (8 sequential, 8 under
    4-way concurrent stress each) => 0 failures either side. It also PASSED inside
    run 4's own gate (ok internal/runtime/exec 12.707s).
  • productmetrics test: 6 runs on branch + 6 on origin/main => 6/6 PASS both sides.

So main is not red for these; they only fail inside the full parallel gate.

Attribution: not the branch diff

Branch diff is 4 files: internal/pricing/defaults.go (data constants),
internal/pricing/defaults_test.go (test table), one COMMENT word in
cmd/gc/doctor_agent_token_telemetry.go, and a markdown spec file.

  • The cmd/gc change is a comment => provably inert at runtime.
  • go list -deps ./internal/runtime/exec shows no dependency on internal/pricing
    or cmd/gc, so the branch cannot reach that package at all.

Contributing factor and partial mitigation

Default LOCAL_TEST_JOBS is 2 with GC_TEST_INNER_P=2, so ~4 concurrent test
processes stack on top of an already-loaded host. Re-running with
LOCAL_TEST_JOBS=1 (documented override, honored via ?=; the hook comment
explicitly preserves it) got substantially further -- 151 packages ok in
unit-core, including the run-1 offender -- before a different test flaked.

Why this matters

The gate is the designated push-time check (gastownhall#3628), so a polecat cannot hand off
to the refinery while it flakes, and --no-verify is not an acceptable escape.
At roughly a 1-in-1 failure rate per ~18-minute run under this load, a polecat
can burn its whole session without landing verified work.

Suggested directions (not prescriptive)

  • Make the three named tests robust to CPU starvation (deadline-based waits with
    generous, load-independent budgets rather than fixed sleeps/timeouts).
  • Consider having the gate auto-serialize (LOCAL_TEST_JOBS=1) when 1-min load
    exceeds some multiple of core count.
  • Consider a single scoped retry of only the failed job before failing the push,
    which would have turned all three of these runs green.

Evidence

Gate logs retained: /var/tmp/rp/push-gc-88opu-try{1,3,4}.log and the per-job
logs under /var/tmp/rp/gc-local-tests.*/ (unit-core.log, unit-cmd-gc-1-of-6.log).

Implementation notes

2026-08-14T21:25Z DECISIVE CONTROL — origin/main is RED in the same shard; branch is not.

Ran the shard runner directly on both sides, same host, same env
(./scripts/test-go-test-shard ./cmd/gc 1 6, TMPDIR=/var/tmp/rp, GC_TEST_INNER_P=2):

origin/main (6757dc0) : rc=1, 2 failures
--- FAIL: TestFreshManagedBdCityInitSeedsPinnedHQDatabaseAndKeepsGCPrefix (32.40s)
--- FAIL: TestEvaluatePoolDefaultScaleCheckIgnoresRoutedActiveUnassignedWork (0.04s)
polecat/gc-kawr5 (c239b74) : rc=0, 0 failures

So cmd/gc shard 1 fails on main itself, with a DIFFERENT failure set than the two
gate runs that failed on the branch. Across five observations the failing test set
is unstable on both sides — five distinct tests so far:

internal/runtime/exec TestProvider_StartCancellationInterruptsForegroundChild
cmd/gc TestCityRuntimeForceShutdownTearsDownAfterLateAsyncSweep (x2, branch)
internal/productmetrics TestDisableAndPurgeExactTokenConflictAndPeerCleanRecovery
cmd/gc TestFreshManagedBdCityInitSeedsPinnedHQDatabaseAndKeepsGCPrefix (main)
cmd/gc TestEvaluatePoolDefaultScaleCheckIgnoresRoutedActiveUnassignedWork (main)

Isolation results (same host):
TestCityRuntimeForceShutdown... 20/20 PASS on main AND 20/20 PASS on branch
TestProvider_StartCancellation.. 16/16 PASS on main AND 16/16 PASS on branch
TestDisableAndPurge... 6/6 PASS on main AND 6/6 PASS on branch

Every one passes standalone and fails only inside its shard, i.e. cross-test
interference within a shard process, not a defect in any single test and not a
branch regression.

IMPACT (raised): this is not merely 'a flaky test' — the designated push-time gate
cannot pass reliably for ANY branch right now, because main itself fails it. No
polecat can complete a refinery handoff without either getting lucky or resorting
to --no-verify (which must not happen). Suggest an operator/mayor decision on
whether to quarantine the unstable tests, add a scoped single-job retry, or treat
the gate as advisory until the interference is fixed.

Reported by gascity/gc-toolkit.furiosa while trying to land gc-88opu (branch
polecat/gc-kawr5). That branch's own targeted gates are green; only the shared
gate blocks it.

CLASS GATE — operator-approved 2026-08-22 (subject gc-b0pmq, visit gc-qcvca sitting 3)

You are the gate for a class of TEN load-sensitive test beads, not just your own
three tests. Nine siblings are PARKED BEHIND YOU by real dependency edges added
this sitting, and each left gc bd ready as a result:

gc-8ors6 TestControllerSocketPathUsesShortCanonicalPathForLongAlias (TMPDIR length mod 8)
gc-b3g52 cmd/gc events-watch — 50ms wall-clock deadline vs loopback under shard load
gc-cr6lj TestRunCheckTimeoutBoundsFix (internal/doctor)
gc-ek4pu TestProductMetricsDirectChildEnvSessionSubmitPoller [p3]
gc-i344n TestCustomTypesCheck_TableDriftUsesTestOwnedDoltContext
gc-nnl64 TestWaitForSupervisorReadyUsesHookedTimeout (cmd/gc)
gc-o95fg TestRecordOnceFreshQuotaBootstrapNeverReplacesDestinationRace (internal/productmetrics)
gc-t9zf7 TestProvider_StartCancellationInterruptsForegroundChild (internal/runtime/exec)
gc-var24 TestDoctorScriptSteadyHealthySkipsSweep (push gate under parallel fan-out)

THE EDGE IS THE PARK, and it is load-bearing. Closing this bead un-parks all
nine and returns them to the board as candidates. That is deliberate: if your
work does not actually dispose of a sibling, it SHOULD come back. Do not delete
these edges as tidy-up.

Your question is the class question, and it has a known counter-example.
Decide whether these are one load-isolation problem or N independent races.
gc-04375 sat in YOUR OWN table of flakes, was fixed PER-TEST, landed as #153
and root-caused to a scheduling race (0.00s failure = immediate assertion), NOT
load noise. So "park them all as load noise" is a live risk: it can bury a real
per-test race. Your own evidence is the other side of that: 16 runs each side,
zero failures, which is what proved these are flakes rather than a red main.

Concretely, for each of the nine: either (a) it is genuinely load-sensitive and
the class fix covers it, or (b) it is an independent race that needs its own
fix — in which case say so on that bead and let the un-park return it. Record
which, per bead. A class fix that silently covers only the easy half is the
failure mode to avoid.

Standing disposition lens for this rig applies (subject gc-b0pmq). Classify
before fixing: does the defect exist on upstream/main (CHECK, do not assume),
and is the mechanism fork-local or upstream? Genuine upstream defect -> fix in
fork first + tag upstream_pr_candidate; fork-only debt -> kill it; fixable by
config/toolchain -> do that, zero divergence beats a code change. Never disable
an upstream check to silence it locally — scope it instead.
Note also the visit-6 ruling still in force: a self-correcting resource sawtooth
is ACCEPTED as steady state rather than fixed fork-locally, and a gate that
refuses under it is correct behaviour to preserve, not throughput to recover.
2026-08-22 CLASS GATE ANSWERED (gascity/gc-toolkit.furiosa, branch polecat/gc-8jrtx).

VERDICT: N independent problems, NOT one load-isolation problem. Ten beads, at least six distinct mechanisms; only THREE share a fix. Full analysis committed at specs/gc-8jrtx/class-triage.md; per-bead verdicts recorded on all nine siblings as metadata flake_verdict / flake_mechanism plus a note each.

FIXED ON THIS BRANCH (5):

  • gc-b3g52, gc-nnl64, gc-cr6lj — the genuine class: a test-side wall-clock margin too small for an assertion that is not about time. Shared fix = size the deadline to never fire, and NAME the one case where expiry IS the behaviour under test.
  • gc-8ors6 — NOT load at all; deterministic in len(TMPDIR). Its filed root cause was WRONG and both its fix candidates would have shipped a no-op (the asserted canonical path resolves the symlink away, so it never contains aliasName). Corrected on the bead; fix verified in both directions.
  • gc-i344n — cleanup race, not load; unguarded home TempDir. Confirmed physically: 20 leaked fixture dirs in /var/tmp/rp, every one .../001/.beads (home), never .../002 (the guarded dir). Fixed at both sites in the file.

HANDED BACK (4), each with a diagnosis rather than a re-run:

  • gc-t9zf7 — the class fix was ALREADY APPLIED here on 08-11 (dc683ff/flaky: TestProvider_StartCancellationInterruptsForegroundChild races the rollback trap under load (gc-o7nj1) #125) and did NOT work; verified present at both later observations. The binding deadline is in PRODUCTION: execgrace.Apply(cmd, 2s) sets WaitDelay, so the child has 2s to run its INT trap before SIGKILL. A test-side poll cannot rescue a marker never written. This is a product question about the cooperative-cancellation grace, not a test defect.
  • gc-var24 — load-sensitive by construction (live Dolt probe as a precondition), but the visit-6 ruling engages: the gate refusing under a resource sawtooth is behaviour to preserve. Endorsed its candidate 1 (inject the probe) and explicitly rejected candidate 2 (retry), which would paper over the refusal.
  • gc-ek4pu — real non-atomic publication race; filed analysis endorsed unchanged. Same '>' truncate-before-write mechanism as the one dc683ff documented in internal/runtime/exec — worth a sweep for other spy-child scripts.
  • gc-o95fg — the 'load-sensitive' label is UNSUPPORTED: the test has no clock, sleep, goroutine or t.Parallel at all. Recovered the real failure from the retained log ('load quota: absent', 0.06s — the gc-04375 immediate-assertion tell). Two leads recorded; lead 1 is that RecordDropped is returned from 11+ sites and conflates the behaviour under test with ~10 storage failures, so the first assertion can pass vacuously. This bead's own third test (TestDisableAndPurge...) has the identical shape and is covered there.

ON THE 'ONE CLASS FIX' RISK THE BEAD NAMED: it was real. Three of the nine (gc-8ors6, gc-i344n, gc-o95fg) have no timing dependence whatsoever, and a blanket load-noise disposition would have buried them — plus gc-t9zf7, which proves a test-side load fix can be applied and still leave the defect. gc-nnl64/gc-o95fg/gc-t9zf7 were batch-filed in the same minute from one run with identical boilerplate calling all three the same shape; they are three different mechanisms.

GATE NOTE: core.hooksPath is /dev/null in the shared rig .git/config, so the pre-push hook did NOT run on push (RC=0 in <20s, zero job lines — the documented silent-no-op signature). Ran make test-fast-parallel manually instead; result recorded below.
2026-08-22 GATE RESULT (run manually — see gate note above; core.hooksPath=/dev/null in the shared rig .git/config disables the pre-push hook, so 'git push' returned RC=0 in <20s with zero job lines, the documented silent-no-op signature).

Ran 'make test-fast-parallel' myself, twice, TMPDIR=/var/tmp/rp EXTRA_TEST_ENV='GC_TEST_INNER_P=2', header 'Running 10 fast job(s) with LOCAL_TEST_JOBS=2 inner_p=2'.

RUN 1 (78c97f7, 18:15-18:38Z): 8/10 ok.

  • unit-cmd-gc-3-of-6: TestControllerTestHasNoUnmigratedRawHangDeadlines — MINE, and fixed.
    controllerTestExcludedHangDeadlineLines is a LINE-NUMBER-keyed allowlist, and this
    branch's 9-line comment in controller_test.go shifted all four documented exclusions
    by exactly +9 (421->430, 877->886, 927->936, 1456->1465). Rebased the keys; filed
    gc-h915a for the underlying brittleness (any edit above line 1456 falsifies the list).
  • unit-core: TestDoctorScriptSteadyHealthySkipsSweep — that is gc-var24 itself, at
    1004ms vs the 1000ms threshold. Pre-existing, classified, deliberately not fixed.

RUN 2 (a458388, 18:45-19:03Z): 9/10 ok. ALL SIX cmd/gc shards green, including the
rebased lint. Only failure: TestProvider_StartCancellationInterruptsForegroundChild —
that is gc-t9zf7 itself, at exactly 7.02s, the same duration as its 08-20 observation.

ATTRIBUTION IS STRUCTURAL, NOT SAMPLED. This branch changes ZERO non-test Go files (7
_test.go files plus one markdown spec); _test.go files are not importable by any package;
and 'go list -deps -test' has reach-count 0 from both examples/bd/dolt and
internal/runtime/exec to cmd/gc and internal/doctor. Neither failure can be branch-caused.

SO THE TWO RESIDUAL REDS ARE EXACTLY TWO OF THE FOUR BEADS THIS TRIAGE HANDED BACK, and
each recurrence is live confirmation of its recorded diagnosis (both appended to their own
beads). That is also direct evidence for this bead's original IMPACT claim: across two
consecutive full-gate runs the gate never went green, and never once for a reason this
branch introduced. Every failure attributable to my diff was found and fixed; the gate
still cannot pass, because gc-var24 and gc-t9zf7 remain open by design.

Did NOT use --no-verify and did not weaken any gate. go vet ./... clean.

Branch polecat/gc-8jrtx pushed and verified: remote a458388 == local HEAD.
Implemented: class-gate triage of ten load-sensitive flake beads. Verdict: N independent problems, not one — six distinct mechanisms, only three sharing a fix. Fixed 5 on this branch (gc-b3g52, gc-nnl64, gc-cr6lj = the genuine class; gc-8ors6 = deterministic in TMPDIR length, and its filed root cause was wrong in a way that would have shipped a no-op; gc-i344n = unguarded home TempDir cleanup race, confirmed by 20 leaked fixture dirs). Handed back 4 with diagnoses (gc-t9zf7, gc-var24, gc-ek4pu, gc-o95fg). Per-bead verdicts recorded on all nine siblings; analysis at specs/gc-8jrtx/class-triage.md. Diff is test-only: 7 _test.go files + 1 spec, zero non-test Go files. Filed gc-h915a for a line-number-keyed lint allowlist this work exposed.
2026-08-22 REFINERY GATE (gascity/gc-toolkit.refinery, wisp gc-wisp-jl9).

Independently re-ran the push gate on the branch head, since core.hooksPath=/dev/null
disables the pre-push hook in the shared rig .git/config (the polecat's gate note is
correct about that).

make test-fast-parallel EXTRA_TEST_ENV='GC_TEST_INNER_P=2', TMPDIR=/var/tmp/rp-8jrtx
HEAD a458388, 19:10:24Z -> 19:22:21Z (11m57s), sentinel RC=0
Header 'Running 10 fast job(s) with LOCAL_TEST_JOBS=2 inner_p=2'; 10/10 jobs ok,
including unit-core AND all six cmd/gc shards. Zero FAIL lines.

GREEN, including the two tests that were red in the polecat's runs
(gc-var24 TestDoctorScriptSteadyHealthySkipsSweep, gc-t9zf7
TestProvider_StartCancellationInterruptsForegroundChild) — consistent with both being
flakes rather than deterministic reds, and with this bead's own thesis.

ATTRIBUTION RE-VERIFIED INDEPENDENTLY, structurally: the diff is 7 _test.go files
(cmd/gc x5, internal/doctor x2) plus one markdown spec, zero non-test Go. A _test.go
file compiles only into its own package's test binary and is not importable, so only
the cmd/gc and internal/doctor test binaries can change behaviour. Both previously-red
tests live OUTSIDE those packages — TestDoctorScriptSteadyHealthySkipsSweep is in
examples/bd/dolt/dog_exec_scripts_test.go (not internal/doctor, despite the name) and
TestProvider_StartCancellationInterruptsForegroundChild is in internal/runtime/exec —
so neither is reachable from this diff. No bead filed for them: gc-var24 and gc-t9zf7
are already open and carry this triage's diagnoses.

Rebase onto origin/main was a no-op (branch already contained main tip 9e89da7);
push was a fast-forward no-op; remote confirmed at a458388.

Parked as a pre-open gating anchor (merge_result=pre_open_gate, check_set=codex).
Codex signoff dispatched as gc-sdyqf against the branch compare-range
origin/main...origin/polecat/gc-8jrtx. pre-open-resolve.sh opens the PR once
check.codex=green@a458388.

Refinery handoff

  • Issue: gc-8jrtx (bug, P2)
  • Source branch: polecat/gc-8jrtx
  • Target: main
  • Codex signed off pre-open at a458388e; PR opened codex-green.

refinery costing added 5 commits August 22, 2026 17:50
…cause (gc-8jrtx)

gc-8jrtx is the class gate for ten load-sensitive test beads. Triage found the
class is not one problem, so this commit fixes only the four whose mechanism was
identified and verified; the rest are classified on their own beads.

controller_test.go / path_helpers_test.go (gc-8ors6). The bead diagnosed the
failure as "len(base) mod 8", the loop growing aliasName in 8-char steps racing
a hard <=100 assertion. That is wrong, and its proposed fixes (overshoot by 1,
compute the length arithmetically) would have been no-ops: the asserted
canonical path resolves the symlink away, so it never contains aliasName at all.
The real precondition is a hard threshold on base alone -- canonical is
base + "/city/.gc/controller.sock", so the fixture is constructible only when
len(base) <= 75. Reproduced the reported 102 exactly from the recorded path.
shortSocketTempDir guarantees a short root only on macOS; on Linux it inherits
$TMPDIR, and the gate sets a long one. New shortSocketTempDirWithinLimit takes
the reserve the caller needs and falls back to /tmp -- the short root the macOS
branch and production's own controllerSocketPath fallback already use.
Verified: reverting just this call site fails at 0.00s under the recorded
TMPDIR (123 > 100); with the fix it PASSes (not skips) 5/5 there and 5/5 under
a short TMPDIR.

cmd_events_test.go (gc-b3g52). doEventsWatch's argument is a whole-watch
deadline. Seven call sites expect an early return -- a buffered-replay match or
a rejected scope -- so their 50ms literal only had to cover scheduling, an
httptest handler and a loopback hop, and a loaded shard defeated it as
"context deadline exceeded" rather than as any assertion about behaviour. They
now share eventsWatchTestDeadline, sized never to fire; the call still returns
on the match, so nothing gets slower. The eighth site is
TestDoEventsWatchTimesOutWithoutMatch, where expiry IS the behaviour under
test: it keeps its short value under eventsWatchTestExpiryDeadline, named so
the next test copies the right one.

cmd_supervisor_test.go (gc-nnl64). The success-path test pinned
supervisorReadyTimeout to 25ms while its hook reports ready on the 4th poll of
a 1ms ticker -- four scheduler turns, which a loaded shard does not guarantee
inside 25ms. waitForSupervisorPID returns the moment the hook answers, so a
generous deadline costs nothing on this path, and the 25ms never proved the
timeout hook was used anyway (the default 15s would pass identically). The
timeout's own behaviour stays covered by the zero-timeout sibling test.

checks_custom_types_test.go (gc-i344n). home := t.TempDir() lacked the
retryRemoveAllForTest guard its sibling dir already had, and home is where bd
writes ~/.beads -- so TempDir's own RemoveAll raced a still-exiting bd/dolt
child and reported "directory not empty", which testing counts as a FAILURE
even for a run that correctly skipped. Confirmed physically: /var/tmp/rp holds
20 leaked dirs, every one of them .../001/.beads (home), never .../002 (the
guarded dir). Both tests in the file have the shape and both leak, so both are
fixed. Cleanups run LIFO, so the retry is registered right after t.TempDir() to
sit immediately ahead of the removal it drains for.

Validation: go vet clean on both packages; the targeted cmd/gc tests pass
-count=3; internal/doctor passes in full. The doctor race itself is not
reproducible on this host (bd is a CGO_ENABLED=0 build, so both drift tests
skip) -- the evidence there is the asymmetry plus the leaked-directory census.
…rgin (gc-cr6lj)

The test pinned CheckTimeout to 25ms and needed that single value to separate
two very different things: an initial Run that returns immediately, and a Fix
that never returns at all. Doctor.Run races the check goroutine's first
scheduling against time.After(CheckTimeout), so under a loaded parallel shard
the immediate Run can lose that race. The fast initial failure is then
classified "timed out", the fix path is skipped entirely, and the test fails on
fixCalls = 0 -- an assertion about whether the host was idle, not about the
timeout bounding a wedged remediation.

Raising CheckTimeout to 2s widens that separation by ~80x while preserving
exactly what the test proves: the wedged Fix blocks forever by construction, so
it still reaches the timeout and still yields the unconfirmed-remediation
result the assertions check. The elapsed guard moves to 30s for the same
reason it existed -- it catches an unbounded wedge, and a bounded run now
costs about CheckTimeout, so a 2s guard would have been measuring the fix it
was meant to tolerate.

Chose the wider margin over a clock seam deliberately: injecting a clock into
Doctor is a production change made solely for a test, and the test does not
need to observe time, only to not be defeated by it.

Cost is ~2s of wall clock in a package that already takes 16s.

Validation: passes -count=3 here and -count=3 under an added CPU load that
took the host to load 41 (the reported failures were at load 15-27). Noting
honestly that the old 25ms value ALSO passed under that synthetic load -- a
tight-loop CPU hog is not the gate's profile of parallel go test processes
doing I/O, allocation and process spawning, so this is not an on-demand
reproduction. The change is justified by the mechanism above, and it is
strictly margin-widening on assertions that do not test timing.
…icts (gc-8jrtx)

gc-8jrtx is the class gate for ten load-sensitive test beads, nine of them
parked behind it by dependency edges. Its question was whether they are one
load-isolation problem or N independent races, with the explicit warning that
"park them all as load noise" risks burying a real per-test race -- as gc-04375
proved when it turned out to be a product bug, not load noise.

Answer: N. At least six distinct mechanisms across ten beads, only three of
which share a fix. Two are not load-sensitive at all, one had already received
the class fix nine days before the observation used to file it and failed
anyway, and one filed root cause was specific, plausible and wrong in a way
that would have shipped a no-op.

This records the per-bead verdict and its evidence so the un-park returns each
bead with a diagnosis rather than a re-run. Filed under specs/ rather than
docs/ because it is a record of what was decided on this bead, not an
authoritative statement of current behaviour.
…c-8jrtx)

Self-review touch-up. The gc-i344n cleanup-guard rationale landed immediately
after the existing HOME-pinning rationale with no separator, so two unrelated
explanations read as one paragraph. Blank comment line between them; no code
change.
…ors6 comment (gc-8jrtx)

controllerTestExcludedHangDeadlineLines is keyed by LINE NUMBER, so the
nine-line comment this branch added to controller_test.go at ~line 308 moved
all four documented exclusions down by exactly nine: 421->430, 877->886,
927->936, 1456->1465. Both halves of the lint then failed -- the offender scan
saw four unexcluded raw deadlines, and the staleness guard saw four exclusions
that no longer matched.

The four sites are unchanged and still correct as they stand per
TESTING.md:1364-1371; only their coordinates moved. Rebased the keys by +9 and
confirmed the content at each new line is the same raw-literal deadline the
reason text describes.

Caught by the local gate run (unit-cmd-gc-3-of-6), not by CI. Filed gc-h915a
for the underlying brittleness: any edit anywhere above line 1456 of
controller_test.go breaks this lint, which makes it a tripwire on unrelated
work rather than on the invariant it means to pin.
@zook-bot

Copy link
Copy Markdown
Author

Codex signoff (pre-open, comment-only — not an approval):

VERDICT: COMMENT
Reviewed branch: polecat/gc-8jrtx
Reviewed base: main
Reviewed commit: a458388

Scope checked: full diff origin/main...a458388 (8 files: cmd/gc tests, internal/doctor tests, specs/gc-8jrtx/class-triage.md); anchor bead gc-8jrtx notes; sibling flake_verdict/flake_mechanism metadata for all nine parked beads.

Findings: None. I found no P0/P1 issues. The branch is test-only plus the committed triage artifact; the changed tests keep their assertions on behavior while replacing incidental, too-small wall-clock margins or non-deterministic fixture setup.

Verification:

  • go test ./cmd/gc ./internal/doctor -> PASS at reviewed commit (cmd/gc 461.684s, internal/doctor 14.610s).
  • git diff --check origin/main...a458388 -> PASS.
  • make test-fast-parallel with TMPDIR=/var/tmp/gc-sdyqf-fast.lqtaID and EXTRA_TEST_ENV='GC_TEST_INNER_P=2' -> FAIL: unit-cmd-gc-1-of-6, TestSupervisorCreatesControllerSocketForManagedCity. Attribution: not a branch regression. The failing test is outside the changed test files and production code is unchanged; the exact test passed 5/5 on the reviewed commit under the same TMPDIR shape, while origin/main failed the exact same test 4/5 under the same TMPDIR shape. All other fast-gate jobs completed green.

Not checked: no GitHub PR review was posted because this is a pre-open review_branch gate; verdict is recorded here for pre-open replay.

@zook-bot
zook-bot merged commit 050ca30 into main Aug 22, 2026
72 of 84 checks passed
zook-bot added a commit that referenced this pull request Aug 25, 2026
…tests under fleet load; blocks polecat handoff (gc-8jrtx) (#156)

* test: make four push-gate flakes deterministic, and correct one root cause (gc-8jrtx)

gc-8jrtx is the class gate for ten load-sensitive test beads. Triage found the
class is not one problem, so this commit fixes only the four whose mechanism was
identified and verified; the rest are classified on their own beads.

controller_test.go / path_helpers_test.go (gc-8ors6). The bead diagnosed the
failure as "len(base) mod 8", the loop growing aliasName in 8-char steps racing
a hard <=100 assertion. That is wrong, and its proposed fixes (overshoot by 1,
compute the length arithmetically) would have been no-ops: the asserted
canonical path resolves the symlink away, so it never contains aliasName at all.
The real precondition is a hard threshold on base alone -- canonical is
base + "/city/.gc/controller.sock", so the fixture is constructible only when
len(base) <= 75. Reproduced the reported 102 exactly from the recorded path.
shortSocketTempDir guarantees a short root only on macOS; on Linux it inherits
$TMPDIR, and the gate sets a long one. New shortSocketTempDirWithinLimit takes
the reserve the caller needs and falls back to /tmp -- the short root the macOS
branch and production's own controllerSocketPath fallback already use.
Verified: reverting just this call site fails at 0.00s under the recorded
TMPDIR (123 > 100); with the fix it PASSes (not skips) 5/5 there and 5/5 under
a short TMPDIR.

cmd_events_test.go (gc-b3g52). doEventsWatch's argument is a whole-watch
deadline. Seven call sites expect an early return -- a buffered-replay match or
a rejected scope -- so their 50ms literal only had to cover scheduling, an
httptest handler and a loopback hop, and a loaded shard defeated it as
"context deadline exceeded" rather than as any assertion about behaviour. They
now share eventsWatchTestDeadline, sized never to fire; the call still returns
on the match, so nothing gets slower. The eighth site is
TestDoEventsWatchTimesOutWithoutMatch, where expiry IS the behaviour under
test: it keeps its short value under eventsWatchTestExpiryDeadline, named so
the next test copies the right one.

cmd_supervisor_test.go (gc-nnl64). The success-path test pinned
supervisorReadyTimeout to 25ms while its hook reports ready on the 4th poll of
a 1ms ticker -- four scheduler turns, which a loaded shard does not guarantee
inside 25ms. waitForSupervisorPID returns the moment the hook answers, so a
generous deadline costs nothing on this path, and the 25ms never proved the
timeout hook was used anyway (the default 15s would pass identically). The
timeout's own behaviour stays covered by the zero-timeout sibling test.

checks_custom_types_test.go (gc-i344n). home := t.TempDir() lacked the
retryRemoveAllForTest guard its sibling dir already had, and home is where bd
writes ~/.beads -- so TempDir's own RemoveAll raced a still-exiting bd/dolt
child and reported "directory not empty", which testing counts as a FAILURE
even for a run that correctly skipped. Confirmed physically: /var/tmp/rp holds
20 leaked dirs, every one of them .../001/.beads (home), never .../002 (the
guarded dir). Both tests in the file have the shape and both leak, so both are
fixed. Cleanups run LIFO, so the retry is registered right after t.TempDir() to
sit immediately ahead of the removal it drains for.

Validation: go vet clean on both packages; the targeted cmd/gc tests pass
-count=3; internal/doctor passes in full. The doctor race itself is not
reproducible on this host (bd is a CGO_ENABLED=0 build, so both drift tests
skip) -- the evidence there is the asymmetry plus the leaked-directory census.

* test(doctor): give TestRunCheckTimeoutBoundsFix a load-independent margin (gc-cr6lj)

The test pinned CheckTimeout to 25ms and needed that single value to separate
two very different things: an initial Run that returns immediately, and a Fix
that never returns at all. Doctor.Run races the check goroutine's first
scheduling against time.After(CheckTimeout), so under a loaded parallel shard
the immediate Run can lose that race. The fast initial failure is then
classified "timed out", the fix path is skipped entirely, and the test fails on
fixCalls = 0 -- an assertion about whether the host was idle, not about the
timeout bounding a wedged remediation.

Raising CheckTimeout to 2s widens that separation by ~80x while preserving
exactly what the test proves: the wedged Fix blocks forever by construction, so
it still reaches the timeout and still yields the unconfirmed-remediation
result the assertions check. The elapsed guard moves to 30s for the same
reason it existed -- it catches an unbounded wedge, and a bounded run now
costs about CheckTimeout, so a 2s guard would have been measuring the fix it
was meant to tolerate.

Chose the wider margin over a clock seam deliberately: injecting a clock into
Doctor is a production change made solely for a test, and the test does not
need to observe time, only to not be defeated by it.

Cost is ~2s of wall clock in a package that already takes 16s.

Validation: passes -count=3 here and -count=3 under an added CPU load that
took the host to load 41 (the reported failures were at load 15-27). Noting
honestly that the old 25ms value ALSO passed under that synthetic load -- a
tight-loop CPU hog is not the gate's profile of parallel go test processes
doing I/O, allocation and process spawning, so this is not an on-demand
reproduction. The change is justified by the mechanism above, and it is
strictly margin-widening on assertions that do not test timing.

* docs(specs): record the gc-8jrtx flake-class triage and per-bead verdicts (gc-8jrtx)

gc-8jrtx is the class gate for ten load-sensitive test beads, nine of them
parked behind it by dependency edges. Its question was whether they are one
load-isolation problem or N independent races, with the explicit warning that
"park them all as load noise" risks burying a real per-test race -- as gc-04375
proved when it turned out to be a product bug, not load noise.

Answer: N. At least six distinct mechanisms across ten beads, only three of
which share a fix. Two are not load-sensitive at all, one had already received
the class fix nine days before the observation used to file it and failed
anyway, and one filed root cause was specific, plausible and wrong in a way
that would have shipped a no-op.

This records the per-bead verdict and its evidence so the un-park returns each
bead with a diagnosis rather than a re-run. Filed under specs/ rather than
docs/ because it is a record of what was decided on this bead, not an
authoritative statement of current behaviour.

* style(doctor): separate the two rationales on the pinned test HOME (gc-8jrtx)

Self-review touch-up. The gc-i344n cleanup-guard rationale landed immediately
after the existing HOME-pinning rationale with no separator, so two unrelated
explanations read as one paragraph. Blank comment line between them; no code
change.

* test(cmd/gc): rebase the hang-deadline exclusion lines after the gc-8ors6 comment (gc-8jrtx)

controllerTestExcludedHangDeadlineLines is keyed by LINE NUMBER, so the
nine-line comment this branch added to controller_test.go at ~line 308 moved
all four documented exclusions down by exactly nine: 421->430, 877->886,
927->936, 1456->1465. Both halves of the lint then failed -- the offender scan
saw four unexcluded raw deadlines, and the staleness guard saw four exclusions
that no longer matched.

The four sites are unchanged and still correct as they stand per
TESTING.md:1364-1371; only their coordinates moved. Rebased the keys by +9 and
confirmed the content at each new line is the same raw-literal deadline the
reason text describes.

Caught by the local gate run (unit-cmd-gc-3-of-6), not by CI. Filed gc-h915a
for the underlying brittleness: any edit anywhere above line 1456 of
controller_test.go breaks this lint, which makes it a tripwire on unrelated
work rather than on the invariant it means to pin.

---------

Co-authored-by: refinery costing <refinery@local>
zook-bot added a commit that referenced this pull request Aug 25, 2026
…tests under fleet load; blocks polecat handoff (gc-8jrtx) (#156)

* test: make four push-gate flakes deterministic, and correct one root cause (gc-8jrtx)

gc-8jrtx is the class gate for ten load-sensitive test beads. Triage found the
class is not one problem, so this commit fixes only the four whose mechanism was
identified and verified; the rest are classified on their own beads.

controller_test.go / path_helpers_test.go (gc-8ors6). The bead diagnosed the
failure as "len(base) mod 8", the loop growing aliasName in 8-char steps racing
a hard <=100 assertion. That is wrong, and its proposed fixes (overshoot by 1,
compute the length arithmetically) would have been no-ops: the asserted
canonical path resolves the symlink away, so it never contains aliasName at all.
The real precondition is a hard threshold on base alone -- canonical is
base + "/city/.gc/controller.sock", so the fixture is constructible only when
len(base) <= 75. Reproduced the reported 102 exactly from the recorded path.
shortSocketTempDir guarantees a short root only on macOS; on Linux it inherits
$TMPDIR, and the gate sets a long one. New shortSocketTempDirWithinLimit takes
the reserve the caller needs and falls back to /tmp -- the short root the macOS
branch and production's own controllerSocketPath fallback already use.
Verified: reverting just this call site fails at 0.00s under the recorded
TMPDIR (123 > 100); with the fix it PASSes (not skips) 5/5 there and 5/5 under
a short TMPDIR.

cmd_events_test.go (gc-b3g52). doEventsWatch's argument is a whole-watch
deadline. Seven call sites expect an early return -- a buffered-replay match or
a rejected scope -- so their 50ms literal only had to cover scheduling, an
httptest handler and a loopback hop, and a loaded shard defeated it as
"context deadline exceeded" rather than as any assertion about behaviour. They
now share eventsWatchTestDeadline, sized never to fire; the call still returns
on the match, so nothing gets slower. The eighth site is
TestDoEventsWatchTimesOutWithoutMatch, where expiry IS the behaviour under
test: it keeps its short value under eventsWatchTestExpiryDeadline, named so
the next test copies the right one.

cmd_supervisor_test.go (gc-nnl64). The success-path test pinned
supervisorReadyTimeout to 25ms while its hook reports ready on the 4th poll of
a 1ms ticker -- four scheduler turns, which a loaded shard does not guarantee
inside 25ms. waitForSupervisorPID returns the moment the hook answers, so a
generous deadline costs nothing on this path, and the 25ms never proved the
timeout hook was used anyway (the default 15s would pass identically). The
timeout's own behaviour stays covered by the zero-timeout sibling test.

checks_custom_types_test.go (gc-i344n). home := t.TempDir() lacked the
retryRemoveAllForTest guard its sibling dir already had, and home is where bd
writes ~/.beads -- so TempDir's own RemoveAll raced a still-exiting bd/dolt
child and reported "directory not empty", which testing counts as a FAILURE
even for a run that correctly skipped. Confirmed physically: /var/tmp/rp holds
20 leaked dirs, every one of them .../001/.beads (home), never .../002 (the
guarded dir). Both tests in the file have the shape and both leak, so both are
fixed. Cleanups run LIFO, so the retry is registered right after t.TempDir() to
sit immediately ahead of the removal it drains for.

Validation: go vet clean on both packages; the targeted cmd/gc tests pass
-count=3; internal/doctor passes in full. The doctor race itself is not
reproducible on this host (bd is a CGO_ENABLED=0 build, so both drift tests
skip) -- the evidence there is the asymmetry plus the leaked-directory census.

* test(doctor): give TestRunCheckTimeoutBoundsFix a load-independent margin (gc-cr6lj)

The test pinned CheckTimeout to 25ms and needed that single value to separate
two very different things: an initial Run that returns immediately, and a Fix
that never returns at all. Doctor.Run races the check goroutine's first
scheduling against time.After(CheckTimeout), so under a loaded parallel shard
the immediate Run can lose that race. The fast initial failure is then
classified "timed out", the fix path is skipped entirely, and the test fails on
fixCalls = 0 -- an assertion about whether the host was idle, not about the
timeout bounding a wedged remediation.

Raising CheckTimeout to 2s widens that separation by ~80x while preserving
exactly what the test proves: the wedged Fix blocks forever by construction, so
it still reaches the timeout and still yields the unconfirmed-remediation
result the assertions check. The elapsed guard moves to 30s for the same
reason it existed -- it catches an unbounded wedge, and a bounded run now
costs about CheckTimeout, so a 2s guard would have been measuring the fix it
was meant to tolerate.

Chose the wider margin over a clock seam deliberately: injecting a clock into
Doctor is a production change made solely for a test, and the test does not
need to observe time, only to not be defeated by it.

Cost is ~2s of wall clock in a package that already takes 16s.

Validation: passes -count=3 here and -count=3 under an added CPU load that
took the host to load 41 (the reported failures were at load 15-27). Noting
honestly that the old 25ms value ALSO passed under that synthetic load -- a
tight-loop CPU hog is not the gate's profile of parallel go test processes
doing I/O, allocation and process spawning, so this is not an on-demand
reproduction. The change is justified by the mechanism above, and it is
strictly margin-widening on assertions that do not test timing.

* docs(specs): record the gc-8jrtx flake-class triage and per-bead verdicts (gc-8jrtx)

gc-8jrtx is the class gate for ten load-sensitive test beads, nine of them
parked behind it by dependency edges. Its question was whether they are one
load-isolation problem or N independent races, with the explicit warning that
"park them all as load noise" risks burying a real per-test race -- as gc-04375
proved when it turned out to be a product bug, not load noise.

Answer: N. At least six distinct mechanisms across ten beads, only three of
which share a fix. Two are not load-sensitive at all, one had already received
the class fix nine days before the observation used to file it and failed
anyway, and one filed root cause was specific, plausible and wrong in a way
that would have shipped a no-op.

This records the per-bead verdict and its evidence so the un-park returns each
bead with a diagnosis rather than a re-run. Filed under specs/ rather than
docs/ because it is a record of what was decided on this bead, not an
authoritative statement of current behaviour.

* style(doctor): separate the two rationales on the pinned test HOME (gc-8jrtx)

Self-review touch-up. The gc-i344n cleanup-guard rationale landed immediately
after the existing HOME-pinning rationale with no separator, so two unrelated
explanations read as one paragraph. Blank comment line between them; no code
change.

* test(cmd/gc): rebase the hang-deadline exclusion lines after the gc-8ors6 comment (gc-8jrtx)

controllerTestExcludedHangDeadlineLines is keyed by LINE NUMBER, so the
nine-line comment this branch added to controller_test.go at ~line 308 moved
all four documented exclusions down by exactly nine: 421->430, 877->886,
927->936, 1456->1465. Both halves of the lint then failed -- the offender scan
saw four unexcluded raw deadlines, and the staleness guard saw four exclusions
that no longer matched.

The four sites are unchanged and still correct as they stand per
TESTING.md:1364-1371; only their coordinates moved. Rebased the keys by +9 and
confirmed the content at each new line is the same raw-literal deadline the
reason text describes.

Caught by the local gate run (unit-cmd-gc-3-of-6), not by CI. Filed gc-h915a
for the underlying brittleness: any edit anywhere above line 1456 of
controller_test.go breaks this lint, which makes it a tripwire on unrelated
work rather than on the invariant it means to pin.

---------

Co-authored-by: refinery costing <refinery@local>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants