Repository navigation
A supersession line is lost when the post-turn memory pass beats the turn's hub closing #961
Description
TestASupersessionSaysWhatItReplaced fails about one run in eight on a clean dev, and it is a race between the post-turn memory pass and the turn's hub closing.
Found while reading a red touched packages on #954. The test is not in .github/known-red.txt, and the repository's own rule is that a internal/session test which fails intermittently is a bug report rather than a shape to rerun past.
Replication
On a clean checkout of dev (measured at 8f45dd5e0), with no other suite running:
go test -count=15 -run '^TestASupersessionSaysWhatItReplaced$' ./internal/session/2 failures in 15 runs. Reproduced in two separate worktrees of the same sha, and once on a GitHub runner (run 34651246295, job 103433765762, touched packages). The failure is always the same and always says the notice list was empty:
--- FAIL: TestASupersessionSaysWhatItReplaced (0.03s)
memory_test.go:1233: the supersession said [], want it to name both halves
It is not caused by load: the 15-run loop above was the only thing on the box. It is order-dependent enough that a full ./internal/session run passes far more often than the isolated loop.
What is happening
Agent.sayMemory (internal/session/memory.go:1033) chooses where a memory line goes by reading a.hub under the lock:
a.mu.Lock()
hub := a.hub
a.mu.Unlock()
if hub != nil {
memoryNotice(hub, text)
return
}
a.memory.queueNotice(text)The post-turn pass that writes the supersession runs in its own goroutine on the session's lifetime (Agent.learnFromTurn, memory.go:826), started from loop.go:1118 — which is BEFORE the turn's hub is closed and cleared. So the two race:
- The goroutine loses.
a.hubis already nil, the line is queued,refreshMemorydrains it on the next stream (memory.go:431), and the test passes. - The goroutine wins.
a.hubis still the turn's hub, which has no subscribers left and is about to close.memoryNoticesends the line there, it reaches nobody, and — this is the defect — it is never queued, so no later stream can say it either. The line is lost for the life of the session.
The test only sees the flake. The product loses a real line: a person who was told nothing about a supersession has a memory silently replaced, which is the one memory event the design says must be announced (memory.go:1009).
Acceptance
End-to-end first, at the real door: with the post-turn pass forced to run while a turn's hub is still set, Agent.refreshMemory on the next stream must still emit superseded · deploys on Fridays → deploys on Tuesdays as an EventNotice. The existing test then holds at -count=50 rather than one in eight.
The narrow fix is for sayMemory to queue when the hub it found cannot deliver, rather than deciding on hub != nil alone — the hub is closing, not absent, and those are different facts. Whoever owns memory should pick the shape; this report does not.
Not this
Nothing in #954 touches memory.go, sayMemory, or the hub lifetime. The flake was bisected across all six of that branch's commits and reproduces identically with none of them present.
Drafted with CodeAF · reviewed and owned by the author
Activity
AbirAbbas commentedon Sep 16, 2026
Fixed by #952.
eventHub.send(internal/session/agent.go) now reports whether the event landed, andAgent.sayMemory(internal/session/memory.go) queues the supersession notice whenever delivery fails, so a hub that is closing rather than absent no longer loses the line.Checked on
devat77293538c:TestASupersessionSaysWhatItReplacedstill pins the original behaviour, andTestALineSaidOntoAClosedStreamIsHeldInsteaddeterministically pins the interleaving this issue describes, the post-turn pass beating the turn's hub closing.