Repository navigation
A five-second deadline in ranNodes.await reports box load as "no node ran" #967
Description
TestAPersonsTurnIsNeverBoundedByTheWall fails about one run in eighteen on a clean dev, and the cause is a fixed five-second real-time deadline in a shared test helper rather than anything about the wall.
Found on a pull request whose diff cannot reach it (#963: one line in Agent.Close). It is not in .github/known-red.txt, and the repository's rule is that an internal/session test failing only beside other work is a bug report rather than a shape to rerun past.
Replication
On a clean checkout of dev (measured at 91003c79f), with other suites running on the same box:
go test -count=24 -run '^TestAPersonsTurnIsNeverBoundedByTheWall$' ./internal/session/3 failures in 54 runs on clean dev. Always the same, always after exactly five seconds:
--- FAIL: TestAPersonsTurnIsNeverBoundedByTheWall (5.01s)
turnwall_test.go:220: no node ran
Every failure observed happened while another package's suite held the box. In 30 interleaved runs on a quiet box the same sha failed 0 times.
What is happening
turnwall_test.go:220 is ran.await(t), and await is a shared helper with a hard wall-clock deadline (task_test.go:258):
func (r ranNodes) await(t *testing.T) *TaskNode {
select {
case node := <-r:
return node
case <-time.After(5 * time.Second):
t.Fatal("no node ran")
}
}The test submits a turn that must divide and then run a node through the stub. On a loaded box that work can take longer than five seconds, and the helper then reports it as "no node ran" — a sentence that reads as a hang or a seam that never fired, which is what makes this expensive: the failure names the wall law, and the wall law is not what broke.
At least six tests share this helper (checkpoint_quick_test.go ×4, loop_speed_test.go, turnwall_test.go), so the same five seconds bounds all of them.
Acceptance
go test -count=50 -run '^TestAPersonsTurnIsNeverBoundedByTheWall$' ./internal/session/ green with another package's suite running beside it.
The shape the repository already prefers is a deterministic clock over a real deadline; where a real one is unavoidable, it should scale with -timeout or say what it actually observed ("the graph had N nodes, none started") rather than asserting a hang. Whoever owns the task fixtures should pick; this report does not.
Not this
- Not questions: a clock armed on a question is let go when the session closes #963: its whole diff is one line in
Agent.Closeplus a two-line wrapper, and it fails identically ondevwithout that commit. - Not questions: asking back gets an answer with the question still open, and the chip counts only what waits on you #960 either, which is worth stating because the arithmetic invites it: measured naively, the sha before questions: asking back gets an answer with the question still open, and the chip counts only what waits on you #960 failed 0 times in 24 runs and the sha after failed 3 in 24. That is box load, not a regression —
git diff ed8371321 91003c79f -- internal/session/is empty, so the package under test is byte-identical across that merge.
Drafted with CodeAF · reviewed and owned by the author
Activity
santoshkumarradha commentedon Sep 11, 2026
This repository already has the law, and it names this exact failure.
PERF.md:24:ranNodes.await's five seconds is a stopwatch, and its red means the box was busy. The sentence about people learning to re-run rather than read is the cost already being paid: the failure saysno node ran, which is a claim about a seam that never fired, so the first hour of reading it goes into the wall law and the task graph rather than into the clock.That the law is written for perf gates rather than for a fixture helper is the only reason this is not already a violation on paper. The reasoning does not change with the caller.
One narrowing for whoever picks it up: the deadline cannot simply be raised. Five seconds to thirty makes the same lie take six times as long to tell. What the helper owes its reader is what it actually observed — the graph had N nodes and none started, versus the graph was never built — so that a timeout is legible as a timeout. Waiting on the event rather than polling for it, where the seam allows, removes the deadline instead of moving it.
santoshkumarradha commentedon Sep 11, 2026
Scope settled: this issue stays narrow, and the convention behind it is #968.
Lane F swept the shape across three packages and found it is not an isolated helper —
internal/session's test files alone carry over 200 deadline branches, and a random sample of eighteen showed seventeen asserting a cause the test did not observe (the node never started,the parked call was never handed its answer,closing the watch did not come back).internal/tui3andinternal/remoteare the same.That is the size of a convention, not a bug list, and the number must not be read as a defect count: a ten-second wait that says "the node never started" is honest on a quiet box and only misleads when the deadline is tight relative to the work it waits for. That is what makes
ranNodes.await's five seconds for divide-then-run the one that actually fails.So the split:
Merging them would bury a real failing test under a wording change to two hundred call sites, and there is no reason to change a message that has never misled anybody.
I am taking this one as soon as #963 lands.
santoshkumarradha commentedon Sep 11, 2026
One more piece of evidence, and it is the strongest either lane has that this is a fixture and not a regression: the same CI job came back green on the same branch.
#963's
touched packages— the job that produced this red — passed on the next run of the identical tree (4m41s, head5934a3d17, all three jobs green). Nothing about the diff changed between the red and the green.Put beside the other direction already recorded here, the picture closes:
touched packages, then green on the same treedevwithout the commitgit diffof the suspected merge is empty forinternal/sessionSo whoever picks this up is chasing
ranNodes.await's five seconds, not a regression, and does not need to bisect anything. The failure is a timeout wearing the words of a seam that never fired.tests: a wait that runs out says it timed out, instead of naming a ca…