Skip to content

A supersession line is lost when the post-turn memory pass beats the turn's hub closing #961

Closed

Description

@santoshkumarradha

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.hub is already nil, the line is queued, refreshMemory drains it on the next stream (memory.go:431), and the test passes.
  • The goroutine wins. a.hub is still the turn's hub, which has no subscribers left and is about to close. memoryNotice sends 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

  1. AbirAbbas commented on Sep 16, 2026

    @AbirAbbas
    Collaborator

    Fixed by #952. eventHub.send (internal/session/agent.go) now reports whether the event landed, and Agent.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 dev at 77293538c: TestASupersessionSaysWhatItReplaced still pins the original behaviour, and TestALineSaidOntoAClosedStreamIsHeldInstead deterministically pins the interleaving this issue describes, the post-turn pass beating the turn's hub closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething the code does that it should not

Type

No type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions