From 52a461968825f9a519be09cc44f94022081485c8 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sat, 12 Sep 2026 09:28:46 -0400 Subject: [PATCH 01/14] tasks: a turn that never asked the model does not spend the person's press The steering queue is read at a STEP boundary, so a card pressed while the turn's last request is already out reaches no request at all: the note is recorded at that turn's end drain and answered by the turn that wakes for it. The floor took the hold back at the end of the first turn anyway, which put the chips back on the card within milliseconds of the press and then handed the model a decision the person was holding again. The queue is the fact: a hand-over note still waiting on it is a question nobody has been given, and the floor leaves that hold where it is. The note carries the landing it hands over so the floor can ask. AND NOTHING BEATS AFTER THE CONVERSATION HAS CLOSED. A held phase is re-said on a ticker until it ends, and a session closed mid-stage left that goroutine running: it posted a conversation's stage after the conversation had gone, and under the detector it was a live goroutine reading this package's own beat while the next test wrote it. Close ends the stage properly and tellPhase refuses a closed session, which is #997's law one lane over. Co-Authored-By: Claude Opus 5 (1M context) --- internal/session/agent.go | 51 ++++++++++++++ internal/session/phasenews.go | 13 +++- internal/session/phasenews_test.go | 53 ++++++++++++++ internal/session/task_audit.go | 6 +- internal/session/task_run.go | 35 ++++++++-- internal/session/tools_tasks_test.go | 100 ++++++++++++++++++++++++++- 6 files changed, 248 insertions(+), 10 deletions(-) diff --git a/internal/session/agent.go b/internal/session/agent.go index 1ea51a5b2f..60f2c74d69 100644 --- a/internal/session/agent.go +++ b/internal/session/agent.go @@ -1226,6 +1226,14 @@ type userMessage struct { // request actually carried the words. directions []uint64 + // handsOver names the landings whose DECISION this message hands to the + // model — the person's own "let aforge decide this one" + // ([Agent.HandUnverifiedToModel]). It is read off the queue and nowhere else, + // for `directions`' own reason: a press still waiting here is one no request + // has carried, and the floor that takes a hand-over back must not spend it on + // a turn that never asked (task_run.go's [Agent.handBackUnsettled]). + handsOver []uint64 + // crossed is what the record keeps about a line the person sent ACROSS to // this agent from the room they were standing in (task_room.go's // [Agent.SteerTask]) — the instant they sent it, and the engine's own one-fact @@ -2254,6 +2262,17 @@ func (a *Agent) Close() error { // because it is the one background lane that owes nothing to the quit: a // beat holds no write anybody is waiting for. a.stopLaneBeat() + // AND NO BEAT ON A HELD PHASE OUTLIVES THE SESSION THAT ARMED IT. A stage is + // re-said on a ticker of its own until it ends (phasenews.go's + // [Agent.beatHeldPhase]), and a session that closed mid-stage left that + // goroutine running: it went on posting a conversation's stage after the + // conversation had gone, and under the detector it was a live goroutine + // reading the package's own beat while the next test wrote it. The end is + // said properly rather than merely dropped, so a surface takes the clock down + // instead of holding a stage nobody will ever finish. [Agent.tellPhase] + // refuses a closed session, which is what keeps a turn still unwinding below + // from arming another one. + a.endPhase() // EVERY CANCEL FIRST, THEN THE JOINS. The naming errand may be asleep in a // backoff or parked on a provider, and it is the one thing here that owes @@ -2713,6 +2732,28 @@ func (a *Agent) drainSteering(hub *eventHub) int { return landed } +// handOversWaiting is every landing whose hand-over note is still on the +// steering queue — the presses the model has not been given yet. +// +// IT IS THE WHOLE OF WHAT THE FLOOR NEEDS (task_run.go's +// [Agent.handBackUnsettled]). A note on this queue has reached no request, so +// the turn that is ending never asked about it; a note that has left it is in +// the transcript, which is what the next request carries. +func (a *Agent) handOversWaiting() map[uint64]bool { + a.mu.Lock() + defer a.mu.Unlock() + var waiting map[uint64]bool + for _, message := range a.steering { + for _, id := range message.handsOver { + if waiting == nil { + waiting = map[uint64]bool{} + } + waiting[id] = true + } + } + return waiting +} + // queuedDirections is every node receipt id on one queue. func queuedDirections(queued []userMessage) []uint64 { var ids []uint64 @@ -2962,6 +3003,16 @@ func (a *Agent) enqueueSteering(text string) { a.enqueueNote(wakeNote(text)) } +// enqueueHandOver is [Agent.enqueueSteering] for the one note that also changes +// who is holding a question: the person's "let aforge decide this one". The id +// rides with the line so that the drain in front of the next request is what +// turns the press into a question the model has (task_audit.go's [handOver]). +func (a *Agent) enqueueHandOver(id uint64, text string) { + note := wakeNote(text) + note.handsOver = []uint64{id} + a.enqueueNote(note) +} + // enqueueJobNote is the registry's owed lane, and what it carries is the ending // as [jobRegistry.settleExit] composed it. The whole log remains available // through `jobs output` for anything past the tail. diff --git a/internal/session/phasenews.go b/internal/session/phasenews.go index c39010b775..43641ee93c 100644 --- a/internal/session/phasenews.go +++ b/internal/session/phasenews.go @@ -400,8 +400,19 @@ func (a *Agent) tellPhaseThen(phase provider.Phase, detail, then string, since t return } a.mu.Lock() - model := a.model + closed, model := a.closed, a.model a.mu.Unlock() + // A CONVERSATION THAT HAS CLOSED SAYS NOTHING ABOUT WHAT IT IS DOING, and + // arms no beat to go on saying it. It is the one door's law said about this + // heart — the reading beside a turn writes nothing into a closed conversation + // (taskdelta.go), a clock armed on a question decides nothing after the door + // shuts (asklane.go) — and it is the half [Agent.Close] cannot do by itself: + // the close ends whatever is held, and this is what stops a phase being armed + // behind it. Without both, a beat went on posting a stage for a session that + // had gone, which is the stale clock this whole file exists to end. + if closed { + return + } now := time.Now() if since.IsZero() { since = now diff --git a/internal/session/phasenews_test.go b/internal/session/phasenews_test.go index b8155857c5..d5f885d312 100644 --- a/internal/session/phasenews_test.go +++ b/internal/session/phasenews_test.go @@ -436,6 +436,59 @@ func TestAHeldPhaseKeepsSayingItselfAndStopsWhenItEnds(t *testing.T) { } } +// TestNothingBeatsAfterTheConversationHasClosed is the beat's half of the law +// every other thing this session arms already keeps: nothing armed outlives the +// session that armed it (steer.go's grace, asklane.go's clock, taskdelta.go's +// reading beside the work). +// +// A stage is re-said on a ticker until it ends, and a session closed mid-stage +// left that ticker running: it went on posting a conversation's stage after the +// conversation had gone. Under the detector it was worse than untidy — a live +// goroutine reading this package's own beat while the next test wrote it, which +// is a data race reported against whichever test happened to be running. +func TestNothingBeatsAfterTheConversationHasClosed(t *testing.T) { + log := watchPhases(t) + held := phaseHeldBeat + phaseHeldBeat = 2 * time.Millisecond + t.Cleanup(func() { phaseHeldBeat = held }) + + agent, _ := newTestAgent(t, &scriptedCompleter{}, nil) + agent.tellPhase(provider.PhaseChecking, "whether the work is finished", time.Now()) + deadline := time.Now().Add(5 * time.Second) + for { + if said, _ := heldSince(log.all(), provider.PhaseChecking); said >= 2 { + break + } + if time.Now().After(deadline) { + t.Fatal("the stage never beat at all, so this proves nothing about it stopping") + } + time.Sleep(time.Millisecond) + } + + if err := agent.Close(); err != nil { + t.Fatalf("closing the conversation: %v", err) + } + settled, _ := heldSince(log.all(), provider.PhaseChecking) + // AND NOTHING ARMS A NEW ONE BEHIND THE CLOSE, which is the half the close + // cannot do by itself: a turn still unwinding says its stages on the way out. + agent.tellPhase(provider.PhaseRunning, "a tool nobody is running", time.Now()) + // Several beats' worth of silence, for [TestAHeldPhaseKeepsSayingItselfAndStopsWhenItEnds]'s + // reason: absence is the claim, and this is how long it takes to be false. + time.Sleep(20 * time.Millisecond) + if after, _ := heldSince(log.all(), provider.PhaseChecking); after != settled { + t.Fatalf("the beat said the stage %d more times after the conversation closed", after-settled) + } + if said, _ := heldSince(log.all(), provider.PhaseRunning); said != 0 { + t.Fatalf("a closed conversation said what it was doing %d times", said) + } + // AND THE CLOCK CAME DOWN rather than being left on the screen for a + // conversation that has gone. + last := log.all() + if len(last) == 0 || last[len(last)-1].Phase != "" { + t.Fatalf("the closed conversation left its stage on the screen; phases were %v", phaseWords(last)) + } +} + // TestTheBeatIsComfortablyInsideTheWindowASurfaceDrops is the one-source-of-truth // law between two packages, checked rather than commented. // diff --git a/internal/session/task_audit.go b/internal/session/task_audit.go index 09fb74f981..f2ee669187 100644 --- a/internal/session/task_audit.go +++ b/internal/session/task_audit.go @@ -2634,7 +2634,11 @@ func (a *Agent) HandUnverifiedToModel(id uint64) error { // back when the model's turn ends without an answer (agent.go). node.holdsDecision(TaskAskOwnerModel) notice := node.notice() - a.enqueueSteering(handOverLead + "\n" + + // AND THE NOTE CARRIES THE NODE IT HANDS OVER, so the drain that puts it in + // front of the model is the thing that says the model has been asked + // ([Agent.markHandOverAsked]). Without that link the floor could not tell a + // press the model has read from one still waiting on the queue. + a.enqueueHandOver(id, handOverLead+"\n"+ taskNote(notice, taskURI(node.journalPath()), TaskSettleAuto, a.quietAddress())) a.emitTaskUpdate(notice) return nil diff --git a/internal/session/task_run.go b/internal/session/task_run.go index 506ca62ff3..c26a937a4e 100644 --- a/internal/session/task_run.go +++ b/internal/session/task_run.go @@ -817,12 +817,13 @@ type TaskNode struct { // announcing the end raced past. stopped bool // handed is the receipt for THIS SESSION'S own hand-over press: the person - // asked aforge to decide this one card, and the note that asks it has been - // put in front of the model. It is not the same fact as [TaskNode.decider] - // being the model — a landing under `task.settle = auto` writes that by - // policy and presses nothing — and it travels with the owner through - // [TaskNode.givesBackLocked] so a hand-back leaves no receipt behind - // (task_audit.go's [TaskNode.wasHandedOver] states the whole rule). + // asked aforge to decide this one card, and the note that asks it is either + // waiting on the steering queue or in front of the model. It is + // not the same fact as [TaskNode.decider] being the model — a landing under + // `task.settle = auto` writes that by policy and presses nothing — and it + // travels with the owner through [TaskNode.givesBackLocked] so a hand-back + // leaves no receipt behind (task_audit.go's [TaskNode.wasHandedOver] states + // the whole rule). handed bool // stopReason is what whoever pulled the stop said they were stopping it FOR, // and "" for every stop that came with no words — which is every one a person @@ -4404,6 +4405,22 @@ func (a *Agent) handToModelOnAuto(node *TaskNode) { // about whether the model tried: one turn is the whole window, and a model that // wants longer has the same verb next turn once somebody hands it back. // +// THE WINDOW IS THE TURN THAT ASKED, AND A TURN THAT NEVER ASKED IS NOT ONE. A +// press lands on the steering queue and a queue is read at a STEP boundary, so a +// card pressed while the turn's last request is already out reaches no request +// at all: it is recorded at this turn's own end drain — which runs immediately +// BELOW this call, and is what makes the question the next request carries — and +// answered by the turn that wakes for it. Taking the hold back here would spend +// the person's press on a turn that never heard it and then hand the model a +// decision the person is holding again, which is two hands on one question and +// the exact thing the receipt exists to prevent. +// +// SO THE QUEUE IS THE FACT, and it needs no second one written down: a hand-over +// note still waiting on it is a question nobody has been given yet, and this +// floor leaves that hold exactly where it is. It is assignment.go's own law +// about a line said to a node, said here about a decision handed to the model — +// a line accepted after the last drain was never in front of anybody. +// // IT PUBLISHES ON THE ORDINARY TASK LANE. A surface already folds every // [EventTaskUpdate] into the row it is drawing, and the notice now carries who is // deciding ([TaskNotice.Decider]), so the hand-back is one more update about a @@ -4413,6 +4430,9 @@ func (a *Agent) handBackUnsettled() { if graph == nil { return } + // Read before the graph is taken, because this lock is never held while that + // one is ([Agent.drainSteering] states the ordering). + waiting := a.handOversWaiting() var handed []*TaskNode graph.mu.Lock() for _, id := range graph.order { @@ -4420,6 +4440,9 @@ func (a *Agent) handBackUnsettled() { if node == nil || node.decider != TaskAskOwnerModel || !a.readsTheDecisionLocked(node) { continue } + if waiting[id] { + continue + } node.givesBackLocked() // A NODE THAT WAS ACTUALLY SETTLED IS NOT NEWS. The model spent its verb, // the resolution published its own landing, and a second update saying the diff --git a/internal/session/tools_tasks_test.go b/internal/session/tools_tasks_test.go index 2caeda2c45..1908fe7fe0 100644 --- a/internal/session/tools_tasks_test.go +++ b/internal/session/tools_tasks_test.go @@ -11,14 +11,18 @@ package session // took one would be testing the runner again. import ( + "context" "encoding/json" "errors" "fmt" "os" "path/filepath" "strings" + "sync" "testing" "time" + + "github.com/Agent-Field/agentfield/sdk/go/ai" ) // A MODEL CAN LOOK AT WORK THAT IS STILL GOING. It names the task, and what @@ -601,11 +605,45 @@ func TestHandingAYourCallToTheModelAndItsResolveAreOneRoad(t *testing.T) { } } +// modelHoldsTheTurn parks the model inside its first call and hands back the two +// facts a test needs about that window: that it has begun, and the way to end +// it. +// +// IT IS HOW A TEST SAYS "WHILE THE MODEL IS HOLDING THIS", and it is the +// opposite of a tolerance. Handing a decision over WAKES A TURN on an idle +// session (agent.go's [Agent.enqueueSteering]), and against a scripted model the +// whole life of that turn is a few microseconds: it drains the steering queue +// before its request and the floor hands every model-held landing back after it +// (task_run.go's [Agent.handBackUnsettled]). A test that pressed and then read +// either fact was reading whichever of those the turn had got to — measured on +// the Spark, three runs in thirty under -race, as a steering queue that shrank +// by two between two reads. Parking the model call freezes exactly the window +// these tests are about: no queue is drained while a turn is inside its request, +// and no second turn starts while one is running. +func modelHoldsTheTurn() (held step, entered <-chan struct{}, release func()) { + arrived, let := make(chan struct{}), make(chan struct{}) + var once, letGo sync.Once + held = func(ctx context.Context, _ []ai.Message) (*ai.Response, error) { + once.Do(func() { close(arrived) }) + select { + case <-let: + case <-ctx.Done(): + // A cancelled turn is the session closing on top of the park, which + // every one of these tests does at cleanup. + return nil, ctx.Err() + } + return textResponse("noted"), nil + } + return held, arrived, func() { letGo.Do(func() { close(let) }) } +} + // AND A SECOND PRESS IS NOT A SECOND HAND-OVER. Pressing "let aforge decide" // twice sent the model two identical lines about one decision it was already // holding; the answer now is the plain fact. func TestHandingTheSameDecisionOverTwiceSaysItIsAlreadyHandedOver(t *testing.T) { - agent, _ := newTestAgent(t, &scriptedCompleter{}, func(config *Config) { + held, entered, release := modelHoldsTheTurn() + t.Cleanup(release) + agent, _ := newTestAgent(t, &scriptedCompleter{steps: []step{held}}, func(config *Config) { config.AskConsent = true }) graph := stubbedGraph(agent, func(node *TaskNode) { @@ -619,6 +657,9 @@ func TestHandingTheSameDecisionOverTwiceSaysItIsAlreadyHandedOver(t *testing.T) if err := agent.HandUnverifiedToModel(id); err != nil { t.Fatalf("handing the decision over: %v", err) } + // The press has woken a turn; everything below is asserted while that turn is + // parked in its request and nothing can drain or hand anything back. + waitUntilClosed(t, "the model to be holding the turn", entered, 10*time.Second) said := len(steeringQueue(agent)) err := agent.HandUnverifiedToModel(id) if err == nil { @@ -647,6 +688,56 @@ func TestHandingTheSameDecisionOverTwiceSaysItIsAlreadyHandedOver(t *testing.T) } } +// AND A TURN THAT NEVER ASKED THE MODEL DOES NOT SPEND THE PRESS. This is the +// floor's own law read the other way round (task_run.go's +// [Agent.handBackUnsettled]): a press lands on the steering queue, a queue is +// read at a STEP boundary, and a card pressed while the turn's last request is +// already out reaches no request at all. Handing it back at the end of that turn +// put the chips back on the card within milliseconds of the press and then told +// the model to decide a question the person was holding again. +func TestAPressNoRequestCarriedKeepsTheDecisionForTheTurnThatReadsIt(t *testing.T) { + held, entered, release := modelHoldsTheTurn() + t.Cleanup(release) + agent, _ := newTestAgent(t, &scriptedCompleter{steps: []step{held}}, func(config *Config) { + config.AskConsent = true + }) + graph := stubbedGraph(agent, func(node *TaskNode) { + node.finish("UNVERIFIED — the auditor answered neither VERIFIED nor REFUTED", nil, "", "") + node.graph.complete(node, TaskUnverified) + }) + // THE TURN IS IN FLIGHT BEFORE THE PRESS, which is the whole shape: its + // request has gone out, so nothing it does now will carry a line queued after + // this point. + events := mustSubmit(t, agent, "keep thinking") + waitUntilClosed(t, "the model to be holding the turn", entered, 10*time.Second) + + id := graph.reserve() + graph.admit(id, taskSpec{title: "Hidden rental digs", named: true, brief: "b", acceptance: "a"}) + waitDoneNode(t, graph.node(id)) + if err := agent.HandUnverifiedToModel(id); err != nil { + t.Fatalf("handing the decision over: %v", err) + } + if waiting := agent.handOversWaiting(); !waiting[id] { + t.Fatal("the press left no note waiting for the model on the queue") + } + + // The floor, run by hand at the instant the turn in flight would run it. + agent.handBackUnsettled() + if node := agent.taskNode(id); node.decidedBy() != TaskAskOwnerModel || !node.wasHandedOver() { + t.Fatal("a turn that never carried the note took the decision back off the model") + } + + // AND THE FLOOR STILL FALLS. Once the woken turn has put the note in front of + // the model, the end of THAT turn hands the question back exactly as it + // always did — the window moved to the turn that asked, it did not go away. + release() + collect(t, events) + waitFor(t, "the turn that read the note to hand the decision back", func() bool { + node := agent.taskNode(id) + return node != nil && node.decidedBy() == TaskAskOwnerPerson && !node.wasHandedOver() + }) +} + // TestAnsweringLetAforgeDecideTwiceStandsRatherThanRefusing is the same press // from the OTHER side — the door a person's key goes through // ([Agent.applyLanding]). The row somebody is looking at was drawn before the @@ -655,7 +746,11 @@ func TestHandingTheSameDecisionOverTwiceSaysItIsAlreadyHandedOver(t *testing.T) // refusal would put trouble on a card whose question was answered correctly. // Every other refusal on that key is still handed back. func TestAnsweringLetAforgeDecideTwiceStandsRatherThanRefusing(t *testing.T) { - agent, _ := newTestAgent(t, &scriptedCompleter{}, func(config *Config) { + // The model holds the turn for both presses, for [modelHoldsTheTurn]'s reason: + // this door hands over through the same one and wakes the same turn. + held, entered, release := modelHoldsTheTurn() + t.Cleanup(release) + agent, _ := newTestAgent(t, &scriptedCompleter{steps: []step{held}}, func(config *Config) { config.AskConsent = true }) graph := stubbedGraph(agent, func(node *TaskNode) { @@ -670,6 +765,7 @@ func TestAnsweringLetAforgeDecideTwiceStandsRatherThanRefusing(t *testing.T) { if err := agent.applyLanding(answer, LandingDecideKey, ""); err != nil { t.Fatalf("the first press: %v", err) } + waitUntilClosed(t, "the model to be holding the turn", entered, 10*time.Second) said := len(steeringQueue(agent)) if err := agent.applyLanding(answer, LandingDecideKey, ""); err != nil { t.Fatalf("the second press was refused: %v", err) From fb1adae1713c7c5979c1696100eae6fbe11312c1 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sat, 12 Sep 2026 09:30:20 -0400 Subject: [PATCH 02/14] manual: which turn counts when you hand a decision over (#999) Co-Authored-By: Claude Opus 5 (1M context) --- .../unreleased/999-handover-twice-race.md | 16 ++++++++++++++++ internal/manual/chat/task-rooms-after-restart.md | 4 +++- internal/manual/chat/tasks.md | 6 ++++++ 3 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 docs/changes/unreleased/999-handover-twice-race.md diff --git a/docs/changes/unreleased/999-handover-twice-race.md b/docs/changes/unreleased/999-handover-twice-race.md new file mode 100644 index 0000000000..6308f37abc --- /dev/null +++ b/docs/changes/unreleased/999-handover-twice-race.md @@ -0,0 +1,16 @@ +--- +kind: fixed +title: a press the turn never read is not a press that turn can spend +pr: 999 +surface: [engine, chat] +invalidates: + - "\"A task never stays unowned past the end of a turn\" was read as the end of WHICHEVER turn ends next. It is the end of the turn that actually put the question in front of the model. A press lands on the steering queue and a queue is read at a STEP boundary, so a card pressed while the turn's last request is already out reaches no request at all — it is recorded at that turn's end drain and answered by the turn that wakes for it. `Agent.handBackUnsettled` took the hold back at the end of the first turn anyway, which put the chips back on the card within milliseconds of the press and then handed the model a decision the person was holding again. The queue is now the fact the floor asks: a hand-over note still waiting on it is a question nobody has been given, and that hold is left where it is. No second state is written down; the note carries the landing it hands over (`userMessage.handsOver`) so the floor can ask. `internal/manual/chat`'s `tasks.md` and `task-rooms-after-restart.md` said the old thing and now say which turn counts." + - "`Agent.Close` let go of a steer's grace, a clock armed on a question and the reading beside the work (#963, #997) but not of a held stage's beat, which is re-said on a ticker of its own until the stage ends (`Agent.beatHeldPhase`). A session closed mid-stage left that goroutine posting a conversation's stage after the conversation had gone. `Close` now ends the stage properly — so a surface takes the clock down rather than holding one nobody will finish — and `Agent.tellPhase` refuses a closed session, which is the half the close cannot do by itself while a turn is still unwinding. Under `-race` the old goroutine was a live reader of this package's own `phaseHeldBeat` while the next test wrote it, reported against whichever test happened to be running (#959)." + - "`internal/session`'s hand-over fixtures asserted about the window in which the model is holding a decision by pressing and then reading. There is no such window against a scripted model: the press wakes a turn whose whole life is microseconds. `modelHoldsTheTurn` parks the model inside its call — no queue drains while a turn is inside a request, and no second turn starts while one is running — so the window is stated rather than raced. `TestHandingTheSameDecisionOverTwiceSaysItIsAlreadyHandedOver` failed 3 runs in 30 under `-race` on plain `dev` @ 82624af79 and is one of #959's faces; it is not a known red and never was." +--- + +Two doors, one law: nothing a turn never read is spent by that turn ending, and +nothing armed beside a turn outlives the session that armed it. The first is +assignment.go's own rule about a line said to a node, said about a decision +handed to the model; the second is #997's rule about a reading beside the work, +said about the stage a surface draws a clock for. diff --git a/internal/manual/chat/task-rooms-after-restart.md b/internal/manual/chat/task-rooms-after-restart.md index ce3feb6ce1..08ec4abbde 100644 --- a/internal/manual/chat/task-rooms-after-restart.md +++ b/internal/manual/chat/task-rooms-after-restart.md @@ -173,7 +173,9 @@ say it may still say. **And it comes back to you by itself.** A task never stays unowned past the end of a turn: if aforge's turn ends with the question still unanswered, the decision moves back to you -and the card draws its chips, whether or not you noticed it had been handed over. Where +and the card draws its chips, whether or not you noticed it had been handed over. The turn +that counts is the one that actually read the question — hand it over while aforge is +already mid-answer and it waits for the next turn rather than bouncing back unread. Where aforge's last message asked you something about that task, those chips are the answer surface for that question — its words above, the chips below, one ask. diff --git a/internal/manual/chat/tasks.md b/internal/manual/chat/tasks.md index 33b6c641e0..8d290de21a 100644 --- a/internal/manual/chat/tasks.md +++ b/internal/manual/chat/tasks.md @@ -4867,6 +4867,12 @@ its chips — you do not have to notice it. And where the model's last message a about that task, the chips are the answer surface for that question: its words above, the chips under them, one ask rather than two. +**The turn that counts is the one that actually reads it.** Hand a decision over while +aforge is already mid-answer and the note can arrive too late for that turn to see — so the +card keeps saying `aforge is deciding` and the question waits for the next turn, which is +usually the one aforge starts by itself a moment later. It does not bounce straight back at +you unread, and it does not arrive twice. + **And closing aforge ends the turn too.** If you quit, crash or come back to the conversation later, a task aforge was deciding is yours again the moment the conversation opens — the card draws its chips rather than the `aforge is deciding` row, because the turn From 948117a59314bcae3c517fbab7599ca078a8c5d4 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sat, 12 Sep 2026 09:32:10 -0400 Subject: [PATCH 03/14] laws: the hand-over note says who it is addressed to (#999) Co-Authored-By: Claude Opus 5 (1M context) --- internal/session/principal_structure_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/session/principal_structure_test.go b/internal/session/principal_structure_test.go index b4e5654a5b..bd7b7cab0a 100644 --- a/internal/session/principal_structure_test.go +++ b/internal/session/principal_structure_test.go @@ -49,6 +49,12 @@ var wakeRoads = map[string]string{ "which is the one wake that is already a person's own sentence", "enqueueSteering": "the generic lane. Its callers own the addressing: a landing goes through " + "[Agent.deliverTaskNote] below, a standing firing is the person's own armed words", + "enqueueHandOver": "the person's own \"let aforge decide this one\", pressed on the card that " + + "is asking ([Agent.HandUnverifiedToModel]). It is [enqueueSteering]'s lane with the " + + "landing riding along so the floor can tell a press the model has been given from one " + + "still waiting on the queue, and it needs no addressing of its own: the words are aimed " + + "at the MODEL, and the person they would otherwise have to reach is the one whose press " + + "just wrote them", "spoken": "the person's own line, said into a node (task_room.go). The other speaker on " + "this road is another agent in this session, and it is addressed to the node's own " + "runner rather than to anybody out there ([relayNote])", From e3ef256629b5c2e7c9c2aeeefcdb62f769169672 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sat, 12 Sep 2026 09:35:52 -0400 Subject: [PATCH 04/14] phase: the quit joins the beats it ends, not only cancels them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closing stop is what ENDS a beat; it is not what makes it gone. A beat armed in the last instant before Close may not have been scheduled at all yet — it wakes after the session has left, reads this package's own beat interval and returns — which is a goroutine outliving the thing that armed it however quickly it exits, and under the detector a live reader of phaseHeldBeat while the next test writes it. Each beat registers a channel it closes on its way out; Close ends the stage and then waits for them. The wait is the quit's door alone: ending a stage on a turn's path still cancels and carries on. Co-Authored-By: Claude Opus 5 (1M context) --- internal/session/agent.go | 12 ++++--- internal/session/phasenews.go | 54 ++++++++++++++++++++++++++++-- internal/session/phasenews_test.go | 27 +++++++++++++++ internal/session/task_audit.go | 8 ++--- internal/session/task_run.go | 4 +-- 5 files changed, 92 insertions(+), 13 deletions(-) diff --git a/internal/session/agent.go b/internal/session/agent.go index 60f2c74d69..e5ede9d5db 100644 --- a/internal/session/agent.go +++ b/internal/session/agent.go @@ -2271,8 +2271,12 @@ func (a *Agent) Close() error { // said properly rather than merely dropped, so a surface takes the clock down // instead of holding a stage nobody will ever finish. [Agent.tellPhase] // refuses a closed session, which is what keeps a turn still unwinding below - // from arming another one. + // from arming another one, and the wait is what makes the cancel a JOIN: a + // beat armed in the last instant before this may not have been scheduled yet, + // and a goroutine that wakes after the session has left is one this session + // still owns however quickly it returns. a.endPhase() + a.waitForPhaseBeats() // EVERY CANCEL FIRST, THEN THE JOINS. The naming errand may be asleep in a // backoff or parked on a provider, and it is the one thing here that owes @@ -3004,9 +3008,9 @@ func (a *Agent) enqueueSteering(text string) { } // enqueueHandOver is [Agent.enqueueSteering] for the one note that also changes -// who is holding a question: the person's "let aforge decide this one". The id -// rides with the line so that the drain in front of the next request is what -// turns the press into a question the model has (task_audit.go's [handOver]). +// who is holding a question: the person's "let aforge decide this one". The +// landing rides with the line so that the queue can be asked which presses the +// model has not been given yet ([Agent.handOversWaiting]). func (a *Agent) enqueueHandOver(id uint64, text string) { note := wakeNote(text) note.handsOver = []uint64{id} diff --git a/internal/session/phasenews.go b/internal/session/phasenews.go index 43641ee93c..a07dc5a93c 100644 --- a/internal/session/phasenews.go +++ b/internal/session/phasenews.go @@ -369,6 +369,17 @@ type phaseHeart struct { // screen, which is the same defect as a stale phase and harder to see. held PhaseNews stop chan struct{} + // beating is one channel per beat goroutine this heart has armed that has not + // returned yet, closed by the beat itself on its way out. + // + // IT IS THERE SO THE QUIT CAN JOIN THEM AND NOT MERELY CANCEL THEM. Closing + // `stop` is what ENDS a beat; it is not what makes it gone. A beat armed in + // the last instant before [Agent.Close] may not have been scheduled at all + // yet — it then wakes after the session has left, reads this package's own + // beat interval and returns — which is a goroutine outliving the thing that + // armed it however quickly it exits. A session that has closed owns nothing + // still running, so the close waits ([Agent.waitForPhaseBeats]). + beating []chan struct{} } // tellPhase is how this package says what a turn is doing between requests, and @@ -442,10 +453,11 @@ func (a *Agent) tellPhaseThen(phase provider.Phase, detail, then string, since t a.phase.mu.Lock() defer a.phase.mu.Unlock() a.dropHeldPhaseLocked() - stop := make(chan struct{}) + stop, done := make(chan struct{}), make(chan struct{}) a.phase.held, a.phase.stop = news, stop + a.phase.beating = append(a.phase.beating, done) postPhaseNews(news) - guard.Go("phase beat", func() { a.beatHeldPhase(stop) }) + guard.Go("phase beat", func() { a.beatHeldPhase(stop, done) }) } // interruptPhase says a phase over the top of whatever the turn was already @@ -515,7 +527,8 @@ func (a *Agent) dropHeldPhaseLocked() { // beatHeldPhase re-says one held phase until it ends. It is the whole lifetime // of the goroutine [Agent.tellPhase] spawns: it starts with a phase and it // returns when that phase is over, and there is no other exit. -func (a *Agent) beatHeldPhase(stop chan struct{}) { +func (a *Agent) beatHeldPhase(stop, done chan struct{}) { + defer a.beatEnded(done) beat := time.NewTicker(phaseHeldBeat) defer beat.Stop() for { @@ -530,6 +543,41 @@ func (a *Agent) beatHeldPhase(stop chan struct{}) { } } +// beatEnded is one beat's own last act: it takes itself off the heart and +// releases whoever is waiting for it ([Agent.waitForPhaseBeats]). The close is +// outside the lock the quit reads that list under, so a join and an ending beat +// cannot cross. +func (a *Agent) beatEnded(done chan struct{}) { + a.phase.mu.Lock() + kept := a.phase.beating[:0] + for _, other := range a.phase.beating { + if other != done { + kept = append(kept, other) + } + } + a.phase.beating = kept + a.phase.mu.Unlock() + close(done) +} + +// waitForPhaseBeats waits for every beat this heart has armed to have RETURNED, +// and is the quit's own door ([Agent.Close]) and nobody else's. +// +// IT IS NOT ON A TURN'S PATH, deliberately: ending a stage cancels its beat and +// carries on, because the only wait a person is ever made to feel is the model +// generating. This is the one caller that has to know the goroutine is gone +// rather than going, and it has already told [Agent.tellPhase] that nothing may +// arm another one. +func (a *Agent) waitForPhaseBeats() { + a.phase.mu.Lock() + pending := make([]chan struct{}, len(a.phase.beating)) + copy(pending, a.phase.beating) + a.phase.mu.Unlock() + for _, done := range pending { + <-done + } +} + // sayHeldPhaseAgain re-says the phase this beat belongs to and reports whether // there is still one to say. // diff --git a/internal/session/phasenews_test.go b/internal/session/phasenews_test.go index d5f885d312..e7a513d16d 100644 --- a/internal/session/phasenews_test.go +++ b/internal/session/phasenews_test.go @@ -489,6 +489,33 @@ func TestNothingBeatsAfterTheConversationHasClosed(t *testing.T) { } } +// TestTheQuitJoinsTheBeatsItEnds is the other half of the close, and the half a +// cancel on its own does not buy. +// +// Closing `stop` is what ENDS a beat; it is not what makes it gone. A beat armed +// in the last instant before the quit may not have been scheduled at all yet — +// it wakes afterwards, reads this package's own beat interval, finds the stop +// closed and returns — and a goroutine that runs after the session has left is +// one the session still owns however quickly it exits. Fifty arms and ends, +// because the beat this is about is the one that never started: wait for none of +// them and at least one of the fifty is still pending here. +func TestTheQuitJoinsTheBeatsItEnds(t *testing.T) { + agent, _ := newTestAgent(t, &scriptedCompleter{}, nil) + for round := 0; round < 50; round++ { + agent.tellPhase(provider.PhaseRunning, "a tool", time.Now()) + // The two acts [Agent.Close] does, in its order. + agent.endPhase() + agent.waitForPhaseBeats() + agent.phase.mu.Lock() + left := len(agent.phase.beating) + agent.phase.mu.Unlock() + if left != 0 { + t.Fatalf("round %d: the quit ended the stage and left %d beat(s) still running behind it", + round, left) + } + } +} + // TestTheBeatIsComfortablyInsideTheWindowASurfaceDrops is the one-source-of-truth // law between two packages, checked rather than commented. // diff --git a/internal/session/task_audit.go b/internal/session/task_audit.go index f2ee669187..2864e5cff9 100644 --- a/internal/session/task_audit.go +++ b/internal/session/task_audit.go @@ -2634,10 +2634,10 @@ func (a *Agent) HandUnverifiedToModel(id uint64) error { // back when the model's turn ends without an answer (agent.go). node.holdsDecision(TaskAskOwnerModel) notice := node.notice() - // AND THE NOTE CARRIES THE NODE IT HANDS OVER, so the drain that puts it in - // front of the model is the thing that says the model has been asked - // ([Agent.markHandOverAsked]). Without that link the floor could not tell a - // press the model has read from one still waiting on the queue. + // AND THE NOTE CARRIES THE NODE IT HANDS OVER, so that the queue itself + // answers whether the model has been given this question: without the link, + // the floor could not tell a press a request has carried from one still + // waiting on the queue (task_run.go's [Agent.handBackUnsettled]). a.enqueueHandOver(id, handOverLead+"\n"+ taskNote(notice, taskURI(node.journalPath()), TaskSettleAuto, a.quietAddress())) a.emitTaskUpdate(notice) diff --git a/internal/session/task_run.go b/internal/session/task_run.go index c26a937a4e..4f3097755c 100644 --- a/internal/session/task_run.go +++ b/internal/session/task_run.go @@ -818,8 +818,8 @@ type TaskNode struct { stopped bool // handed is the receipt for THIS SESSION'S own hand-over press: the person // asked aforge to decide this one card, and the note that asks it is either - // waiting on the steering queue or in front of the model. It is - // not the same fact as [TaskNode.decider] being the model — a landing under + // waiting on the steering queue or in front of the model. It is not the same + // fact as [TaskNode.decider] being the model — a landing under // `task.settle = auto` writes that by policy and presses nothing — and it // travels with the owner through [TaskNode.givesBackLocked] so a hand-back // leaves no receipt behind (task_audit.go's [TaskNode.wasHandedOver] states From f4c89a3c9684dcea43b837b3419700088d144062 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sat, 12 Sep 2026 09:36:20 -0400 Subject: [PATCH 05/14] changelog: the quit joins the beat it ends (#999) Co-Authored-By: Claude Opus 5 (1M context) --- docs/changes/unreleased/999-handover-twice-race.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changes/unreleased/999-handover-twice-race.md b/docs/changes/unreleased/999-handover-twice-race.md index 6308f37abc..c4d0601d9c 100644 --- a/docs/changes/unreleased/999-handover-twice-race.md +++ b/docs/changes/unreleased/999-handover-twice-race.md @@ -5,7 +5,7 @@ pr: 999 surface: [engine, chat] invalidates: - "\"A task never stays unowned past the end of a turn\" was read as the end of WHICHEVER turn ends next. It is the end of the turn that actually put the question in front of the model. A press lands on the steering queue and a queue is read at a STEP boundary, so a card pressed while the turn's last request is already out reaches no request at all — it is recorded at that turn's end drain and answered by the turn that wakes for it. `Agent.handBackUnsettled` took the hold back at the end of the first turn anyway, which put the chips back on the card within milliseconds of the press and then handed the model a decision the person was holding again. The queue is now the fact the floor asks: a hand-over note still waiting on it is a question nobody has been given, and that hold is left where it is. No second state is written down; the note carries the landing it hands over (`userMessage.handsOver`) so the floor can ask. `internal/manual/chat`'s `tasks.md` and `task-rooms-after-restart.md` said the old thing and now say which turn counts." - - "`Agent.Close` let go of a steer's grace, a clock armed on a question and the reading beside the work (#963, #997) but not of a held stage's beat, which is re-said on a ticker of its own until the stage ends (`Agent.beatHeldPhase`). A session closed mid-stage left that goroutine posting a conversation's stage after the conversation had gone. `Close` now ends the stage properly — so a surface takes the clock down rather than holding one nobody will finish — and `Agent.tellPhase` refuses a closed session, which is the half the close cannot do by itself while a turn is still unwinding. Under `-race` the old goroutine was a live reader of this package's own `phaseHeldBeat` while the next test wrote it, reported against whichever test happened to be running (#959)." + - "`Agent.Close` let go of a steer's grace, a clock armed on a question and the reading beside the work (#963, #997) but not of a held stage's beat, which is re-said on a ticker of its own until the stage ends (`Agent.beatHeldPhase`). A session closed mid-stage left that goroutine posting a conversation's stage after the conversation had gone. `Close` now ends the stage properly — so a surface takes the clock down rather than holding one nobody will finish — and then WAITS for the beat to have returned: closing `stop` ends a beat, it does not make it gone, and one armed in the last instant before the quit may not have been scheduled at all yet. `Agent.tellPhase` refuses a closed session, which is the half the close cannot do by itself while a turn is still unwinding. The join is the quit's door alone (`Agent.waitForPhaseBeats`): ending a stage on a turn's path still cancels and carries on, because the only wait a person is made to feel is the model generating. Under `-race` the old goroutine was a live reader of this package's own `phaseHeldBeat` while the next test wrote it, reported against whichever test happened to be running (#959)." - "`internal/session`'s hand-over fixtures asserted about the window in which the model is holding a decision by pressing and then reading. There is no such window against a scripted model: the press wakes a turn whose whole life is microseconds. `modelHoldsTheTurn` parks the model inside its call — no queue drains while a turn is inside a request, and no second turn starts while one is running — so the window is stated rather than raced. `TestHandingTheSameDecisionOverTwiceSaysItIsAlreadyHandedOver` failed 3 runs in 30 under `-race` on plain `dev` @ 82624af79 and is one of #959's faces; it is not a known red and never was." --- From 1af4eb7ec0e7f055884467e5ee5a9f1acc080e45 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sat, 12 Sep 2026 09:38:23 -0400 Subject: [PATCH 06/14] tasks: name the law at the check, not only above the function Co-Authored-By: Claude Opus 5 (1M context) --- internal/session/task_run.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/session/task_run.go b/internal/session/task_run.go index 4f3097755c..c46eb4e696 100644 --- a/internal/session/task_run.go +++ b/internal/session/task_run.go @@ -4440,6 +4440,9 @@ func (a *Agent) handBackUnsettled() { if node == nil || node.decider != TaskAskOwnerModel || !a.readsTheDecisionLocked(node) { continue } + // THE PRESS NOBODY HAS BEEN GIVEN YET. Its note is still on the queue, so + // this turn never asked about it and the turn that reads it owns the + // window. if waiting[id] { continue } From cc0c4ec3409932336de1581b50a056bb9135eff1 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sat, 12 Sep 2026 10:18:23 -0400 Subject: [PATCH 07/14] phase: the refusal and the arming are one hold, and one door for a reading's own write MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reading closed under a.mu, letting the lock go and arming the beat under the heart's own lock left Close's end-and-join fitting exactly in the gap: a beat armed after it outlived the session, which is the leak the join was written to close. Arming now happens inside the door that reads closed, so an arming either happens before the quit takes that lock and is joined by it, or does nothing. writeIfOpen is that door and there is one of it. refreshMemory landed its routed block into a closed conversation's transcript with no check at all; it goes through the same door now. #997's refreshElsewhere is the third caller and adopts it when that branch lands — seam reported, not reached across. The hand-rolled channel list is gone: phaseBeats is a sync.WaitGroup, the shape the Agent already uses for memoryJobs and titleJobs, and the Add is taken under the same lock the quit sets closed under so it can never race the Wait. Co-Authored-By: Claude Opus 5 (1M context) --- internal/session/agent.go | 34 ++++++++++++ internal/session/memory.go | 30 ++++++---- internal/session/phasenews.go | 88 ++++++++++++------------------ internal/session/phasenews_test.go | 86 ++++++++++++++++++++++------- internal/session/session.go | 8 +++ 5 files changed, 160 insertions(+), 86 deletions(-) diff --git a/internal/session/agent.go b/internal/session/agent.go index e5ede9d5db..894e289be9 100644 --- a/internal/session/agent.go +++ b/internal/session/agent.go @@ -2961,6 +2961,40 @@ func (a *Agent) takeReplyTags() []TaskReplyTag { return tags } +// writeIfOpen is THE ONE DOOR for anything a reading beside the work leaves +// behind, and it answers one question: is there still a conversation to leave it +// in. +// +// NOTHING WRITES INTO A CONVERSATION THAT HAS CLOSED. After [Agent.Close] the +// queues are drained and nobody will read another word, the folder behind the +// person's path may already be gone, and what a late write leaves there is a +// claim about a session that is over — a memory block landed at the tail of a +// transcript nothing will carry (memory.go), a stage a surface goes on drawing a +// clock for (phasenews.go), a stamp saying this conversation's model was told +// (taskdelta.go). Each of those readings had its own idea of the question, and +// two of them had none at all. +// +// THE CHECK AND THE WRITE ARE ONE HOLD OF a.mu, which is the whole reason it is +// a door and not a helper. A reading that asks `closed` and writes afterwards is +// a reading [Agent.Close] fits inside: the close takes this lock, sets the flag, +// and goes on to wait for what it can see — and the write that lands a moment +// later is exactly the thing it was waiting for. So the write runs under the +// lock, and the caller is told whether it happened. +// +// IT IS FOR THE SMALL WRITE AND NEVER FOR THE SLOW ONE. Everything under this +// lock is on the path of the person's next keystroke: an assignment, a queue, a +// count, a deferred write handed to its own writer. A reading that has to reach +// a disk or a provider does that outside and brings the ANSWER here. +func (a *Agent) writeIfOpen(write func()) bool { + a.mu.Lock() + defer a.mu.Unlock() + if a.closed { + return false + } + write() + return true +} + // enqueueSteering puts one OWED line the session authored — principally a task // node landing — onto the queue the person's steering rides, and wakes the // session if nobody is working. diff --git a/internal/session/memory.go b/internal/session/memory.go index ef83d2a51b..a1946c35f0 100644 --- a/internal/session/memory.go +++ b/internal/session/memory.go @@ -449,17 +449,25 @@ func (a *Agent) refreshMemory(ctx context.Context, cue string) bool { // model's own first word. block := a.routedMemory(ctx, cue, a.sayMemory, true) - a.mu.Lock() - moved := a.memoryText != block - a.memoryText = block - // AND IT LANDS AT THE TAIL, not in message[0]. The drain immediately before - // the first request would land it anyway (loop.go), and it is landed here as - // well so that this pass is complete on its own: what it routed is in front - // of the model the moment it has routed it, on any road that reaches here. - // The lander does nothing at all when the block has not moved, which on a - // conversation whose subject is holding still is every turn after the first. - a.landVolatileLocked() - a.mu.Unlock() + // AND THE LANDING GOES THROUGH THE ONE DOOR FOR A READING'S OWN WRITE + // ([Agent.writeIfOpen], agent.go). The cancel is what stops this pass waiting + // on a provider; this is the other half — a pass that had already routed its + // block when the session closed used to land it anyway, at the tail of a + // transcript no request will ever carry. A block nobody can read did not + // move, so a closed session answers false. + var moved bool + a.writeIfOpen(func() { + moved = a.memoryText != block + a.memoryText = block + // AND IT LANDS AT THE TAIL, not in message[0]. The drain immediately + // before the first request would land it anyway (loop.go), and it is + // landed here as well so that this pass is complete on its own: what it + // routed is in front of the model the moment it has routed it, on any + // road that reaches here. The lander does nothing at all when the block + // has not moved, which on a conversation whose subject is holding still + // is every turn after the first. + a.landVolatileLocked() + }) return moved } diff --git a/internal/session/phasenews.go b/internal/session/phasenews.go index a07dc5a93c..c9f78dee63 100644 --- a/internal/session/phasenews.go +++ b/internal/session/phasenews.go @@ -369,17 +369,6 @@ type phaseHeart struct { // screen, which is the same defect as a stale phase and harder to see. held PhaseNews stop chan struct{} - // beating is one channel per beat goroutine this heart has armed that has not - // returned yet, closed by the beat itself on its way out. - // - // IT IS THERE SO THE QUIT CAN JOIN THEM AND NOT MERELY CANCEL THEM. Closing - // `stop` is what ENDS a beat; it is not what makes it gone. A beat armed in - // the last instant before [Agent.Close] may not have been scheduled at all - // yet — it then wakes after the session has left, reads this package's own - // beat interval and returns — which is a goroutine outliving the thing that - // armed it however quickly it exits. A session that has closed owns nothing - // still running, so the close waits ([Agent.waitForPhaseBeats]). - beating []chan struct{} } // tellPhase is how this package says what a turn is doing between requests, and @@ -411,19 +400,8 @@ func (a *Agent) tellPhaseThen(phase provider.Phase, detail, then string, since t return } a.mu.Lock() - closed, model := a.closed, a.model + model := a.model a.mu.Unlock() - // A CONVERSATION THAT HAS CLOSED SAYS NOTHING ABOUT WHAT IT IS DOING, and - // arms no beat to go on saying it. It is the one door's law said about this - // heart — the reading beside a turn writes nothing into a closed conversation - // (taskdelta.go), a clock armed on a question decides nothing after the door - // shuts (asklane.go) — and it is the half [Agent.Close] cannot do by itself: - // the close ends whatever is held, and this is what stops a phase being armed - // behind it. Without both, a beat went on posting a stage for a session that - // had gone, which is the stale clock this whole file exists to end. - if closed { - return - } now := time.Now() if since.IsZero() { since = now @@ -450,14 +428,38 @@ func (a *Agent) tellPhaseThen(phase provider.Phase, detail, then string, since t Subject: a.newsSubject(), At: now, } + // A CONVERSATION THAT HAS CLOSED SAYS NOTHING ABOUT WHAT IT IS DOING, and arms + // no beat to go on saying it — which is [Agent.writeIfOpen]'s law said about + // this heart rather than about a file (agent.go). + // + // THE REFUSAL AND THE ARMING ARE ONE HOLD OF a.mu, and they have to be. Read + // `closed`, let the lock go and arm afterwards and [Agent.Close] fits exactly + // in the gap: it ends the held stage, waits for the beats it knows about, and + // returns — and the beat armed a moment later outlives the session, which is + // the leak this was written to close. Inside the door, an arming either + // happens before the close takes the lock and is therefore JOINED by it, or + // finds the door shut and does nothing. + a.writeIfOpen(func() { a.armPhaseLocked(news) }) +} + +// armPhaseLocked replaces whatever this heart was saying with one phase and +// starts the beat that keeps saying it. It is called with a.mu held and takes +// the heart's own lock inside it, which is the ONE nesting of these two locks +// and always in this order: nothing that holds the heart's lock ever asks this +// agent for anything ([Agent.sayHeldPhaseAgain] posts to a reader documented +// never to block, and [postPhaseNews] touches nothing of this session's). +func (a *Agent) armPhaseLocked(news PhaseNews) { a.phase.mu.Lock() defer a.phase.mu.Unlock() a.dropHeldPhaseLocked() - stop, done := make(chan struct{}), make(chan struct{}) + stop := make(chan struct{}) a.phase.held, a.phase.stop = news, stop - a.phase.beating = append(a.phase.beating, done) + // THE COUNT IS TAKEN BEFORE THE GOROUTINE EXISTS, under the same lock the + // quit's own `closed` is written under, so a beat can never be added to the + // count after [Agent.Close] has begun waiting on it. + a.phaseBeats.Add(1) postPhaseNews(news) - guard.Go("phase beat", func() { a.beatHeldPhase(stop, done) }) + guard.Go("phase beat", func() { a.beatHeldPhase(stop) }) } // interruptPhase says a phase over the top of whatever the turn was already @@ -527,8 +529,8 @@ func (a *Agent) dropHeldPhaseLocked() { // beatHeldPhase re-says one held phase until it ends. It is the whole lifetime // of the goroutine [Agent.tellPhase] spawns: it starts with a phase and it // returns when that phase is over, and there is no other exit. -func (a *Agent) beatHeldPhase(stop, done chan struct{}) { - defer a.beatEnded(done) +func (a *Agent) beatHeldPhase(stop chan struct{}) { + defer a.phaseBeats.Done() beat := time.NewTicker(phaseHeldBeat) defer beat.Stop() for { @@ -543,39 +545,17 @@ func (a *Agent) beatHeldPhase(stop, done chan struct{}) { } } -// beatEnded is one beat's own last act: it takes itself off the heart and -// releases whoever is waiting for it ([Agent.waitForPhaseBeats]). The close is -// outside the lock the quit reads that list under, so a join and an ending beat -// cannot cross. -func (a *Agent) beatEnded(done chan struct{}) { - a.phase.mu.Lock() - kept := a.phase.beating[:0] - for _, other := range a.phase.beating { - if other != done { - kept = append(kept, other) - } - } - a.phase.beating = kept - a.phase.mu.Unlock() - close(done) -} - // waitForPhaseBeats waits for every beat this heart has armed to have RETURNED, // and is the quit's own door ([Agent.Close]) and nobody else's. // // IT IS NOT ON A TURN'S PATH, deliberately: ending a stage cancels its beat and // carries on, because the only wait a person is ever made to feel is the model // generating. This is the one caller that has to know the goroutine is gone -// rather than going, and it has already told [Agent.tellPhase] that nothing may -// arm another one. +// rather than going, and by the time it runs the door every arming goes through +// is already shut ([Agent.writeIfOpen]), so nothing can be added to the count +// while it waits. func (a *Agent) waitForPhaseBeats() { - a.phase.mu.Lock() - pending := make([]chan struct{}, len(a.phase.beating)) - copy(pending, a.phase.beating) - a.phase.mu.Unlock() - for _, done := range pending { - <-done - } + a.phaseBeats.Wait() } // sayHeldPhaseAgain re-says the phase this beat belongs to and reports whether diff --git a/internal/session/phasenews_test.go b/internal/session/phasenews_test.go index e7a513d16d..3fc11463b2 100644 --- a/internal/session/phasenews_test.go +++ b/internal/session/phasenews_test.go @@ -489,30 +489,74 @@ func TestNothingBeatsAfterTheConversationHasClosed(t *testing.T) { } } -// TestTheQuitJoinsTheBeatsItEnds is the other half of the close, and the half a -// cancel on its own does not buy. +// TestNothingIsSaidAfterTheQuitEvenWhileStagesAreBeingArmED is the close's other +// half, and the half a cancel on its own does not buy. // -// Closing `stop` is what ENDS a beat; it is not what makes it gone. A beat armed -// in the last instant before the quit may not have been scheduled at all yet — -// it wakes afterwards, reads this package's own beat interval, finds the stop -// closed and returns — and a goroutine that runs after the session has left is -// one the session still owns however quickly it exits. Fifty arms and ends, -// because the beat this is about is the one that never started: wait for none of -// them and at least one of the fifty is still pending here. -func TestTheQuitJoinsTheBeatsItEnds(t *testing.T) { +// Closing `stop` ENDS a beat; it does not make it gone, and it says nothing at +// all about the beat that is armed a moment later. Both halves are needed and +// both are here: the quit waits for the beats it ended ([Agent.waitForPhaseBeats]), +// and the door every arming goes through is shut under the same lock the quit +// sets `closed` under ([Agent.writeIfOpen]), so an arming either happens before +// the close takes that lock — and is therefore joined by it — or does nothing. +// +// THE TEST DRIVES THAT INTERLEAVING RATHER THAN HOPING FOR IT: a goroutine arms +// stages as fast as it can while [Agent.Close] runs through it. The claim is the +// one a person could see — once the quit has returned, this conversation says +// nothing more about what it is doing, ever — so the assertion is that the news +// log does not grow after Close returns. +func TestNothingIsSaidAfterTheQuitEvenWhileStagesAreBeingArmed(t *testing.T) { + log := watchPhases(t) + held := phaseHeldBeat + phaseHeldBeat = time.Millisecond + t.Cleanup(func() { phaseHeldBeat = held }) + agent, _ := newTestAgent(t, &scriptedCompleter{}, nil) - for round := 0; round < 50; round++ { - agent.tellPhase(provider.PhaseRunning, "a tool", time.Now()) - // The two acts [Agent.Close] does, in its order. - agent.endPhase() - agent.waitForPhaseBeats() - agent.phase.mu.Lock() - left := len(agent.phase.beating) - agent.phase.mu.Unlock() - if left != 0 { - t.Fatalf("round %d: the quit ended the stage and left %d beat(s) still running behind it", - round, left) + arming := make(chan struct{}) + armed := make(chan struct{}) + go func() { + defer close(armed) + for round := 0; ; round++ { + select { + case <-arming: + return + default: + } + agent.tellPhase(provider.PhaseRunning, "a tool", time.Now()) } + }() + // The stages are really being said before the quit starts, or this would be a + // test of a race that never happened. + deadline := time.Now().Add(5 * time.Second) + for { + if said, _ := heldSince(log.all(), provider.PhaseRunning); said >= 4 { + break + } + if time.Now().After(deadline) { + t.Fatal("no stage was ever said, so the quit has nothing to race") + } + time.Sleep(time.Millisecond) + } + + if err := agent.Close(); err != nil { + t.Fatalf("closing the conversation: %v", err) + } + close(arming) + <-armed + // The armer may have been mid-call when the quit returned; its refusal is + // silent, so the count can only be the one the quit left behind. + settled := len(log.all()) + // Several beats' worth of silence, for [TestAHeldPhaseKeepsSayingItselfAndStopsWhenItEnds]'s + // reason: absence is the claim, and this is how long it takes to be false. + time.Sleep(20 * time.Millisecond) + if after := len(log.all()); after != settled { + t.Fatalf("the conversation said %d more things about what it was doing after the quit returned", + after-settled) + } + // AND THE CLOCK CAME DOWN rather than being left on the screen for a + // conversation that has gone. + last := log.all() + if len(last) == 0 || last[len(last)-1].Phase != "" { + t.Fatalf("the closed conversation left its stage on the screen; phases were %v", phaseWords(last)) } } diff --git a/internal/session/session.go b/internal/session/session.go index b456b9a6ab..e545fa47dd 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -3044,6 +3044,14 @@ type Agent struct { // ([Agent.oweToldStampLocked]). toldAtOwed time.Time + // phaseBeats counts the beat goroutines the phase heart has armed + // (phasenews.go). It is the same shape [Agent.memoryJobs] is and for the same + // reason: the quit waits for them rather than merely cancelling them, and the + // Add is taken under `a.mu` inside [Agent.writeIfOpen] — the door that also + // refuses an arming once the session has closed — so nothing can join the + // count after [Agent.Close] has begun waiting on it. + phaseBeats sync.WaitGroup + // toolCompact is the reduced form of this session's frozen tool history, // carried between requests rather than rebuilt on each one (toolcompact.go). toolCompact toolCompactMemo From cab0ea2d3b1a1a0ec930652e39d01d3b8ad88b7f Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sat, 12 Sep 2026 10:23:26 -0400 Subject: [PATCH 08/14] tasks: how far a decision's note has got is a fact on the node MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The floor that takes a hand-over back runs at the end of EVERY agent's turn, and a worker reads its own children's decisions — so a fact kept per queue let a worker's turn spend a press made, and still waiting, on the conversation. TaskNode.handNote carries it instead: owed until a request carries the note, read afterwards, written in the same hold as the owner so no floor can run between the two. The settle policy's own hand-over is the same question through another door and had the same defect. The note carries its landing now on both roads, and the policy road's mark is written where a reader actually takes the note, because a landing folded into a parent's report never gets one. A note is read at THE REQUEST and not at the drain: a note drained at a step boundary whose turn is then cut has left the queue and reached nobody. And when no turn follows — interrupted, failed, owing nobody a word — the agent that owes the note hands it back at once rather than leaving the card saying aforge is deciding until the person's next turn. Co-Authored-By: Claude Opus 5 (1M context) --- .../unreleased/999-handover-twice-race.md | 14 +- internal/manual/chat/task-controls.md | 14 ++ internal/session/agent.go | 129 +++++++++++++++--- internal/session/loop.go | 7 + internal/session/session.go | 7 + internal/session/task_audit.go | 71 +++++++++- internal/session/task_run.go | 72 +++++++--- internal/session/task_states_test.go | 81 +++++++++++ internal/session/tools_tasks_test.go | 47 ++++++- 9 files changed, 393 insertions(+), 49 deletions(-) diff --git a/docs/changes/unreleased/999-handover-twice-race.md b/docs/changes/unreleased/999-handover-twice-race.md index c4d0601d9c..a57e010be7 100644 --- a/docs/changes/unreleased/999-handover-twice-race.md +++ b/docs/changes/unreleased/999-handover-twice-race.md @@ -4,13 +4,17 @@ title: a press the turn never read is not a press that turn can spend pr: 999 surface: [engine, chat] invalidates: - - "\"A task never stays unowned past the end of a turn\" was read as the end of WHICHEVER turn ends next. It is the end of the turn that actually put the question in front of the model. A press lands on the steering queue and a queue is read at a STEP boundary, so a card pressed while the turn's last request is already out reaches no request at all — it is recorded at that turn's end drain and answered by the turn that wakes for it. `Agent.handBackUnsettled` took the hold back at the end of the first turn anyway, which put the chips back on the card within milliseconds of the press and then handed the model a decision the person was holding again. The queue is now the fact the floor asks: a hand-over note still waiting on it is a question nobody has been given, and that hold is left where it is. No second state is written down; the note carries the landing it hands over (`userMessage.handsOver`) so the floor can ask. `internal/manual/chat`'s `tasks.md` and `task-rooms-after-restart.md` said the old thing and now say which turn counts." - - "`Agent.Close` let go of a steer's grace, a clock armed on a question and the reading beside the work (#963, #997) but not of a held stage's beat, which is re-said on a ticker of its own until the stage ends (`Agent.beatHeldPhase`). A session closed mid-stage left that goroutine posting a conversation's stage after the conversation had gone. `Close` now ends the stage properly — so a surface takes the clock down rather than holding one nobody will finish — and then WAITS for the beat to have returned: closing `stop` ends a beat, it does not make it gone, and one armed in the last instant before the quit may not have been scheduled at all yet. `Agent.tellPhase` refuses a closed session, which is the half the close cannot do by itself while a turn is still unwinding. The join is the quit's door alone (`Agent.waitForPhaseBeats`): ending a stage on a turn's path still cancels and carries on, because the only wait a person is made to feel is the model generating. Under `-race` the old goroutine was a live reader of this package's own `phaseHeldBeat` while the next test wrote it, reported against whichever test happened to be running (#959)." + - "\"A task never stays unowned past the end of a turn\" was read as the end of WHICHEVER turn ends next. It is the end of the turn that actually put the question in front of the model. A note lands on the steering queue and a queue is read at a STEP boundary, so a card pressed while the turn's last request is already out reaches no request at all — it is recorded at that turn's end drain and answered by the turn that wakes for it. `Agent.handBackUnsettled` took the hold back at the end of the first turn anyway, which put the chips back on the card within milliseconds of the press and then handed the model a decision the person was holding again. How far one node's note has got is now a fact ON THE NODE (`TaskNode.handNote`, `decisionNote`): owed until a request carries it, read afterwards. It is on the node and not on a queue because the floor runs at the end of EVERY agent's turn and a worker reads its own children's decisions — a per-queue fact let a worker's turn spend a press made, and still waiting, somewhere else." + - "`task.settle = auto` is the same question through another door and had the same defect: the landing is the model's by policy and the landing note is the whole of the asking, so a turn ending between the two spent it. The note carries its landing now (`userMessage.handsOver`, set by both the press and `Agent.postTaskMessage`), and the policy road's mark is written where a reader actually takes the note — a landing folded into a parent's report never gets a note at all, and marking at the policy would have held that card open forever." + - "A decision the model was given is marked read at THE REQUEST and not at the drain (`Agent.decisionNotesCarried`, called from the one line in `loop.go` where a request goes out). A note drained at a step boundary whose turn is then cut — an interrupt, an oversize refusal, a checkpoint that ends the turn — has left the queue and reached nobody, and a mark at the drain would have let the floor spend a question no model read." + - "A press that no turn will ever carry now comes back at once instead of waiting for the person's next turn. A turn that ends with nothing following it — interrupted, failed, or simply owing nobody a word — hands back what that agent owed (`Agent.handBackUnreadNotes`), because there is no longer a turn coming to ask. Until this, such a node stayed model-owned and home stopped counting it as something needing a person (`taskstatus.go`). `internal/manual/chat`'s `task-controls.md` and `tasks.md` said the old thing and now say which turn counts and what a stopped turn does." + - "`Agent.Close` let go of a steer's grace and a clock armed on a question (#963) but not of a held stage's beat, which is re-said on a ticker of its own until the stage ends (`Agent.beatHeldPhase`). A session closed mid-stage left that goroutine posting a conversation's stage after the conversation had gone — under `-race`, a live reader of this package's own `phaseHeldBeat` while the next test wrote it (#959). `Close` ends the stage properly, so a surface takes the clock down, and then WAITS for the beat: `Agent.phaseBeats` is a `sync.WaitGroup` like `memoryJobs`, and the Add is taken inside the door that refuses an arming on a closed session, so nothing can join the count after the quit has begun waiting on it. The join is the quit's door alone; ending a stage on a turn's path still cancels and carries on." + - "There is now ONE door for anything a reading beside the work leaves behind: `Agent.writeIfOpen` (agent.go) checks `closed` and performs the write in the SAME hold of `a.mu`, because a reading that asks and then writes is a reading `Close` fits inside. `Agent.refreshMemory` landed its routed block into a closed conversation's transcript with no check at all and goes through it now, and so does the phase heart's arming. #997's `refreshElsewhere` is the third caller and adopts it when that branch lands." - "`internal/session`'s hand-over fixtures asserted about the window in which the model is holding a decision by pressing and then reading. There is no such window against a scripted model: the press wakes a turn whose whole life is microseconds. `modelHoldsTheTurn` parks the model inside its call — no queue drains while a turn is inside a request, and no second turn starts while one is running — so the window is stated rather than raced. `TestHandingTheSameDecisionOverTwiceSaysItIsAlreadyHandedOver` failed 3 runs in 30 under `-race` on plain `dev` @ 82624af79 and is one of #959's faces; it is not a known red and never was." --- Two doors, one law: nothing a turn never read is spent by that turn ending, and -nothing armed beside a turn outlives the session that armed it. The first is +nothing a closed conversation would say or write happens at all. The first is assignment.go's own rule about a line said to a node, said about a decision -handed to the model; the second is #997's rule about a reading beside the work, -said about the stage a surface draws a clock for. +handed to the model; the second is one door (`Agent.writeIfOpen`) where three +readings each had their own idea of the question, and two of them had none. diff --git a/internal/manual/chat/task-controls.md b/internal/manual/chat/task-controls.md index 7d57d3f1fa..2bbd84e585 100644 --- a/internal/manual/chat/task-controls.md +++ b/internal/manual/chat/task-controls.md @@ -97,6 +97,20 @@ second line about the same decision — and it is not shown to you as trouble, b state it asks for is the state that already holds: the reason row goes on reading `aforge is deciding`. If you want it back, press `take it back` and the answers return to you. +**If the card stayed `your call` and nothing above fits, the press landed after aforge's +last question went out.** Your press puts a line on the queue, and a queue is read between +steps — so pressing while the answer is already streaming can mean that turn never sees it. +What happens then: the card keeps saying `aforge is deciding`, aforge starts one more turn +by itself to read the line, and that turn is the one it has to answer in. The card comes +back to you at the end of it, whether or not aforge spent a verb on it. It used to come +back within a moment of your press instead, and the model was then handed a decision you +were holding again — two of you answering one question. + +**A turn you stop is the end of it.** If you press `esc` — or the turn fails — before the +line has been read, nothing is coming that would put the question in front of aforge, so +the card draws its chips again straight away and the task counts on your home page as +something needing you. Press `let aforge decide` again when you want it asked. + **A recovered task is this conversation's task.** Reopening a conversation brings its graph back — `recovered task graph: 1 done · 1 your call` — and everything the card offers works on those tasks, the model's answers included. diff --git a/internal/session/agent.go b/internal/session/agent.go index 894e289be9..242e4f2082 100644 --- a/internal/session/agent.go +++ b/internal/session/agent.go @@ -1785,7 +1785,14 @@ func (a *Agent) startTurnLocked(ctx context.Context, user userMessage, watcher * // there must be no window in which a Submit could start a turn and // have the follow-up land behind it, out of the order the person // typed them in. + // followed says A TURN COMES AFTER THIS ONE, which is the whole of + // what an unread hand-over note needs to know: any turn that starts + // carries this transcript, so a note in it is a question that will be + // put in front of the model. When nothing follows, nothing will, and + // what this agent owes goes back to the person below. + followed := false if next, ok := a.nextFollowUpLocked(completed); ok { + followed = true // context.Background rather than the finished turn's: the // Submit that would have carried a context never happened, and // a follow-up inheriting a cancelled one would end before it @@ -1793,6 +1800,7 @@ func (a *Agent) startTurnLocked(ctx context.Context, user userMessage, watcher * // a.cancel, which this call replaces. a.startTurnLocked(context.Background(), next.message, next.stream) } else if completed && unanswered && a.wakeLocked() { + followed = true // SOMETHING LANDED IN THE LAST SECONDS OF THIS TURN. A task's // note, or the person typing while the answer was still // streaming: steering lands at a STEP boundary, and a turn whose @@ -1812,6 +1820,18 @@ func (a *Agent) startTurnLocked(ctx context.Context, user userMessage, watcher * // their transcript and silence, which is what they asked for. } a.mu.Unlock() + // AND A DECISION NOBODY IS NOW GOING TO BE ASKED ABOUT COMES BACK. + // The floor above left every unread hand-over where it was, because + // one turn ending must not spend a press the next turn will read; + // this is the case where there is no next turn, and a question left + // with the model then sits in nobody's hands + // ([Agent.handBackUnreadNotes]). It is after the unlock because it + // takes the graph's lock, and after the disowned-turn return above + // because a turn that was abandoned has had every act here done for + // it already. + if !followed { + a.handBackUnreadNotes() + } // AND NO QUESTION OF THE MODEL'S OWN OUTLIVES THE TURN THAT RAISED // IT UNLESS IT SAID IT WOULD. It is [Agent.handBackUnsettled]'s law // one lane over: a ratify nobody answered and a question somebody @@ -2736,26 +2756,94 @@ func (a *Agent) drainSteering(hub *eventHub) int { return landed } -// handOversWaiting is every landing whose hand-over note is still on the -// steering queue — the presses the model has not been given yet. +// decisionNotesCarried says a request has gone out carrying the notes this +// agent's transcript holds, so the landings they hand over are questions the +// model HAS (task_audit.go's [decisionNote]). +// +// IT IS CALLED AT THE ONE LINE IN THIS PACKAGE WHERE A REQUEST ACTUALLY GOES OUT +// (loop.go) and nowhere else, which is the difference between this and marking +// at the drain: a note drained at a step boundary whose turn is then cut — an +// interrupt, an oversize refusal, a checkpoint that ends the turn — has left the +// queue and reached nobody, and a mark made at the drain would let the floor +// spend a question no model ever read. // -// IT IS THE WHOLE OF WHAT THE FLOOR NEEDS (task_run.go's -// [Agent.handBackUnsettled]). A note on this queue has reached no request, so -// the turn that is ending never asked about it; a note that has left it is in -// the transcript, which is what the next request carries. -func (a *Agent) handOversWaiting() map[uint64]bool { +// IT IS THIS AGENT'S OWN NOTES AND NOT THE GRAPH'S. A worker's request carries +// what a worker was told; a press made on the conversation is not a question a +// worker's turn has asked ([Agent.readsTheDecisionLocked] lets a worker read its +// children's decisions, which is what made that confusion possible). +func (a *Agent) decisionNotesCarried() { a.mu.Lock() - defer a.mu.Unlock() - var waiting map[uint64]bool + owed := a.decisionNotesOwed + a.decisionNotesOwed = nil + a.mu.Unlock() + if len(owed) == 0 { + return + } + graph := a.tasker() + if graph == nil { + return + } + // Outside this agent's lock, because it takes the graph's and there is no + // order in which those two are ever taken the other way round. + graph.mu.Lock() + defer graph.mu.Unlock() + for _, id := range owed { + if node := graph.nodes[id]; node != nil && node.handNote == decisionNoteOwed { + node.handNote = decisionNoteRead + } + } +} + +// handBackUnreadNotes gives back every decision THIS agent owes the model a note +// about and is now never going to deliver, because its turn has ended and +// nothing follows it. +// +// IT IS THE OTHER HALF OF THE FLOOR'S OWN LAW (task_run.go's +// [Agent.handBackUnsettled]). The floor leaves an unread hold alone so that one +// turn ending cannot spend a press the next turn will read; that is right while +// a next turn is coming, and wrong the moment one is not. A turn somebody +// interrupted, or one that ended owing nothing anybody is waiting for, starts no +// successor — and a question left with the model then would sit in nobody's +// hands: the card stops offering its chips and home stops counting it as +// something needing a person (taskstatus.go). +// +// It is every note this agent holds: the ones still on its queue and the ones +// its transcript took at a drain no request followed. +func (a *Agent) handBackUnreadNotes() { + a.mu.Lock() + owed := a.decisionNotesOwed + a.decisionNotesOwed = nil for _, message := range a.steering { - for _, id := range message.handsOver { - if waiting == nil { - waiting = map[uint64]bool{} - } - waiting[id] = true + owed = append(owed, message.handsOver...) + } + a.mu.Unlock() + if len(owed) == 0 { + return + } + graph := a.tasker() + if graph == nil { + return + } + var handed []*TaskNode + graph.mu.Lock() + for _, id := range owed { + node := graph.nodes[id] + if node == nil || node.handNote != decisionNoteOwed { + continue + } + node.givesBackLocked() + // A NODE THAT WAS ACTUALLY SETTLED IS NOT NEWS, for the floor's own + // reason: the resolution published its own landing. + if node.state == TaskUnverified { + handed = append(handed, node) } } - return waiting + graph.mu.Unlock() + // The notices are read with the lock let go of ([TaskNode.notice] states the + // ordering). + for _, node := range handed { + a.emitTaskUpdate(node.notice()) + } } // queuedDirections is every node receipt id on one queue. @@ -2792,6 +2880,13 @@ func (a *Agent) drainQueuedLocked(hub *eventHub, includeAmbient bool) (int, bool a.ambient = nil } sourceCount := len(queued) + // WHICH DECISIONS THIS AGENT'S TRANSCRIPT NOW HOLDS A NOTE ABOUT. It is taken + // at BOTH drains, because both put the note in the transcript and the next + // request carries it either way; what turns it into a question the model has + // been given is that request ([Agent.decisionNotesCarried]). + for _, message := range queued { + a.decisionNotesOwed = append(a.decisionNotesOwed, message.handsOver...) + } queued = coalesceSessionNotes(queued) owed := false for _, message := range queued { @@ -3043,8 +3138,8 @@ func (a *Agent) enqueueSteering(text string) { // enqueueHandOver is [Agent.enqueueSteering] for the one note that also changes // who is holding a question: the person's "let aforge decide this one". The -// landing rides with the line so that the queue can be asked which presses the -// model has not been given yet ([Agent.handOversWaiting]). +// landing rides with the line so that the drain, and the request that follows +// it, can say which decisions the model has been given ([Agent.decisionNotesCarried]). func (a *Agent) enqueueHandOver(id uint64, text string) { note := wakeNote(text) note.handsOver = []uint64{id} diff --git a/internal/session/loop.go b/internal/session/loop.go index e19dead879..f6dd1b29dc 100644 --- a/internal/session/loop.go +++ b/internal/session/loop.go @@ -876,6 +876,13 @@ func (a *Agent) runTurn(ctx context.Context, hub *eventHub, user userMessage) bo // which is exactly the news an outside reader wants (task_beat.go). It is // nil for a conversation, whose liveness the presence file already carries // (taskpresence.go). + // AND EVERY DECISION THIS TRANSCRIPT HOLDS A NOTE ABOUT IS A QUESTION THE + // MODEL HAS FROM HERE. It is on this line for the reason the beat and the + // clock below it are: this is the one place in this package where a + // request actually goes out, and a note that left the queue at a drain + // this turn was then cut short of has reached nobody + // (agent.go's [Agent.decisionNotesCarried]). + a.decisionNotesCarried() a.config.beat.began() // AND THE FIRST-TOKEN CLOCK, on the same line and for a related reason: // this is the one place in this package where a request actually goes diff --git a/internal/session/session.go b/internal/session/session.go index e545fa47dd..4ce5ade1ac 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -3044,6 +3044,13 @@ type Agent struct { // ([Agent.oweToldStampLocked]). toldAtOwed time.Time + // decisionNotesOwed names the landings whose decision-handing notes THIS + // agent's transcript holds with no request having gone out since + // (task_audit.go's [decisionNote]). A drain appends to it, the request that + // follows spends it ([Agent.decisionNotesCarried]), and a turn that ends with + // nothing following hands back what is left ([Agent.handBackUnreadNotes]). + decisionNotesOwed []uint64 + // phaseBeats counts the beat goroutines the phase heart has armed // (phasenews.go). It is the same shape [Agent.memoryJobs] is and for the same // reason: the quit waits for them rather than merely cancelling them, and the diff --git a/internal/session/task_audit.go b/internal/session/task_audit.go index 2864e5cff9..4368335e38 100644 --- a/internal/session/task_audit.go +++ b/internal/session/task_audit.go @@ -2634,10 +2634,9 @@ func (a *Agent) HandUnverifiedToModel(id uint64) error { // back when the model's turn ends without an answer (agent.go). node.holdsDecision(TaskAskOwnerModel) notice := node.notice() - // AND THE NOTE CARRIES THE NODE IT HANDS OVER, so that the queue itself - // answers whether the model has been given this question: without the link, - // the floor could not tell a press a request has carried from one still - // waiting on the queue (task_run.go's [Agent.handBackUnsettled]). + // AND THE NOTE CARRIES THE NODE IT HANDS OVER, which is what lets the drain + // and the request that follows it say the model has been given this question + // (agent.go's [Agent.decisionNotesCarried]). a.enqueueHandOver(id, handOverLead+"\n"+ taskNote(notice, taskURI(node.journalPath()), TaskSettleAuto, a.quietAddress())) a.emitTaskUpdate(notice) @@ -2680,6 +2679,61 @@ func (n *TaskNode) wasHandedOver() bool { return n.handed } +// decisionNote is HOW FAR THE NOTE THAT HANDS ONE NODE'S DECISION TO THE MODEL +// HAS GOT, and it exists because the floor that takes a hand-over back has to +// know whether the model was ever actually asked. +// +// THE WINDOW IS THE TURN THAT PUT THE QUESTION IN FRONT OF THE MODEL, NOT +// WHICHEVER TURN HAPPENS TO END NEXT. A note lands on the steering queue and a +// queue is read at a STEP boundary: press the card while the turn's last request +// is already out and the note reaches no request at all — it is recorded at the +// turn's end drain and answered by the turn that wakes for it. The floor used to +// take the hold back at the end of that first turn anyway, which spent the +// person's press on a turn that never heard it and then handed the model a +// decision the person was holding again. Two hands on one question is the exact +// thing the receipt exists to prevent. +// +// It is assignment.go's own law about a direction said to a node, said here +// about a decision handed to the model: a line accepted after the last drain was +// never in front of anybody, and the state it promises must not be spent on it. +type decisionNote uint8 + +const ( + // decisionNoteNone is every node with nothing asking the model to decide it — + // including one the settle policy marked the model's whose news was folded + // into a parent's report rather than delivered ([Agent.postTaskMessage]). + decisionNoteNone decisionNote = iota + // decisionNoteOwed is a note a reader has taken and no request has carried: + // on a queue, or in a transcript no request has gone out since. The floor + // leaves this hold alone; the agent that owes the note hands it back itself + // if its turn ends with nothing following (agent.go's + // [Agent.handBackUnreadNotes]). + decisionNoteOwed + // decisionNoteRead is a note a request carried: the model has the question, + // and the turn it went out in is the window it has to answer in. + decisionNoteRead +) + +// owesDecisionNote marks one node as having a note on its way to the model. +// +// IT IS WRITTEN WHERE THAT BECOMES TRUE, which is two places because there are +// two roads and they differ in exactly this. A press writes it in the same hold +// as the owner ([TaskNode.holdsDecision]) because the press enqueues its own +// note in the next breath and cannot fail to; a landing under +// `task.settle = auto` writes it when a reader has actually taken the note, +// because that news may be folded into a parent's report instead of delivered. +func (n *TaskNode) owesDecisionNote() { + if n == nil || n.graph == nil { + return + } + n.graph.mu.Lock() + defer n.graph.mu.Unlock() + if n.decider != TaskAskOwnerModel || n.handNote != decisionNoteNone { + return + } + n.handNote = decisionNoteOwed +} + // decidedBy reads who is holding one node's question, with the graph taken for // the read the way every other reader of a node's fields takes it. func (n *TaskNode) decidedBy() TaskAskOwner { @@ -2698,7 +2752,7 @@ func (n *TaskNode) decidedBy() TaskAskOwner { // (task_run.go). A road that wrote only the owner would leave a receipt behind // and refuse the next press. func (n *TaskNode) givesBackLocked() { - n.decider, n.handed = TaskAskOwnerPerson, false + n.decider, n.handed, n.handNote = TaskAskOwnerPerson, false, decisionNoteNone } // holdsDecision writes who is holding one node's question. It is the graph's @@ -2719,7 +2773,12 @@ func (n *TaskNode) holdsDecision(owner TaskAskOwner) { // the one that hands the model the note, and the next press on the same node // is answered with what is already true rather than sending a second copy of // one decision ([TaskNode.wasHandedOver]). - n.decider, n.handed = owner, true + // + // AND THE NOTE IS OWED IN THE SAME HOLD AS THE OWNER. The press enqueues it + // in the next breath, and writing the two under one lock is what keeps a + // floor running in between from spending a press nobody has been given + // ([decisionNote]). + n.decider, n.handed, n.handNote = owner, true, decisionNoteOwed } // TakeBackDecision is [Agent.HandUnverifiedToModel] in reverse: the person diff --git a/internal/session/task_run.go b/internal/session/task_run.go index c46eb4e696..6577784748 100644 --- a/internal/session/task_run.go +++ b/internal/session/task_run.go @@ -817,14 +817,19 @@ type TaskNode struct { // announcing the end raced past. stopped bool // handed is the receipt for THIS SESSION'S own hand-over press: the person - // asked aforge to decide this one card, and the note that asks it is either - // waiting on the steering queue or in front of the model. It is not the same - // fact as [TaskNode.decider] being the model — a landing under - // `task.settle = auto` writes that by policy and presses nothing — and it - // travels with the owner through [TaskNode.givesBackLocked] so a hand-back - // leaves no receipt behind (task_audit.go's [TaskNode.wasHandedOver] states - // the whole rule). + // asked aforge to decide this one card. It is not the same fact as + // [TaskNode.decider] being the model — a landing under `task.settle = auto` + // writes that by policy and presses nothing — and it travels with the owner + // through [TaskNode.givesBackLocked] so a hand-back leaves no receipt behind + // (task_audit.go's [TaskNode.wasHandedOver] states the whole rule). handed bool + // handNote is HOW FAR THE NOTE THAT HANDS THIS DECISION TO THE MODEL HAS GOT + // (task_audit.go's [decisionNote]), and it is on the NODE rather than on any + // one agent's queue because the floor that takes a hand-over back runs at the + // end of every agent's turn: a worker reads its own children's decisions + // ([Agent.readsTheDecisionLocked]), and a fact kept per queue let a worker's + // turn spend a press that was made, and is still waiting, somewhere else. + handNote decisionNote // stopReason is what whoever pulled the stop said they were stopping it FOR, // and "" for every stop that came with no words — which is every one a person // pulls, their card being a decision and not a sentence (cancel.go). It is @@ -3881,6 +3886,16 @@ func (n *TaskNode) settlesNote(claim noteClaim) durableDelivery { // written by the caller, after the seam. func (a *Agent) postTaskMessage(node *TaskNode, tag TaskReplyTag, note, record string, durable []durableDelivery, mark func()) deliveryReceipt { message := wakeNote(note) + // AND WHERE THIS LANDING'S DECISION IS THE MODEL'S, THE NOTE SAYS SO. Under + // `task.settle = auto` the landing hands the question over by policy + // ([Agent.handToModelOnAuto]) and THIS note is the whole of the asking, so it + // carries the landing the way a press's own note does (task_audit.go's + // [Agent.HandUnverifiedToModel]) — which is what lets the drain and the + // request tell a question the model has been given from one still on its way. + hands := node.decidedBy() == TaskAskOwnerModel + if hands { + message.handsOver = []uint64{tag.ID} + } // THE TAG IS THE CALLER'S SNAPSHOT AND IS NOT RE-READ FROM THE NODE HERE. // This delivery happens after a claim is won and a reader is found, and a // node can be re-armed and revised in that gap: composing the tag here would @@ -3911,6 +3926,13 @@ func (a *Agent) postTaskMessage(node *TaskNode, tag TaskReplyTag, note, record s } return got } + // AND THE NOTE IS OWED FROM THE MOMENT A READER HAS IT. Before this line + // there is no note anywhere — the fold above took it into a report instead — + // and a decision marked owed with nothing to carry it would be a card saying + // aforge is deciding with no question in front of anybody. + if hands { + node.owesDecisionNote() + } got.reader.handOverTaskNews(mark) return got } @@ -4386,6 +4408,12 @@ func (a *Agent) handToModelOnAuto(node *TaskNode) { if node.state != TaskUnverified || node.merge == mergeConflicted || node.shifted { return } + // THE OWNER IS WRITTEN HERE AND THE NOTE'S OWN MARK IS NOT, because on this + // road the note may never reach a queue at all: a landing whose news is + // folded into a parent's report has no delivery of its own + // ([Agent.postTaskMessage]). The mark is made where a reader actually takes + // the note, so a question nobody was handed is never held open waiting for an + // answer to a note that does not exist. node.decider = TaskAskOwnerModel } @@ -4415,11 +4443,20 @@ func (a *Agent) handToModelOnAuto(node *TaskNode) { // decision the person is holding again, which is two hands on one question and // the exact thing the receipt exists to prevent. // -// SO THE QUEUE IS THE FACT, and it needs no second one written down: a hand-over -// note still waiting on it is a question nobody has been given yet, and this -// floor leaves that hold exactly where it is. It is assignment.go's own law -// about a line said to a node, said here about a decision handed to the model — -// a line accepted after the last drain was never in front of anybody. +// SO THE NODE CARRIES HOW FAR ITS OWN NOTE HAS GOT ([decisionNote]), and this +// floor leaves a hold alone until a request has actually carried it. It is +// assignment.go's own law about a line said to a node, said here about a +// decision handed to the model — a line accepted after the last drain was never +// in front of anybody. THE FACT IS ON THE NODE AND NOT ON A QUEUE because this +// floor runs at the end of EVERY agent's turn and a worker reads its own +// children's decisions: the press is one thing, and whose turn is ending is +// another. +// +// A NOTE NOBODY WILL EVER CARRY IS NOT LEFT HELD EITHER. When a turn ends and +// nothing follows it — no follow-up, no wake, an interrupted turn — the agent +// that owes the note hands back exactly what it owes +// ([Agent.handBackUnreadNotes]), because there is no longer a turn coming that +// would put the question in front of anybody. // // IT PUBLISHES ON THE ORDINARY TASK LANE. A surface already folds every // [EventTaskUpdate] into the row it is drawing, and the notice now carries who is @@ -4430,9 +4467,6 @@ func (a *Agent) handBackUnsettled() { if graph == nil { return } - // Read before the graph is taken, because this lock is never held while that - // one is ([Agent.drainSteering] states the ordering). - waiting := a.handOversWaiting() var handed []*TaskNode graph.mu.Lock() for _, id := range graph.order { @@ -4440,10 +4474,10 @@ func (a *Agent) handBackUnsettled() { if node == nil || node.decider != TaskAskOwnerModel || !a.readsTheDecisionLocked(node) { continue } - // THE PRESS NOBODY HAS BEEN GIVEN YET. Its note is still on the queue, so - // this turn never asked about it and the turn that reads it owns the - // window. - if waiting[id] { + // THE DECISION NOBODY HAS BEEN GIVEN YET. No request has carried its + // note, so no turn has asked about it and the turn that reads it owns + // the window. + if node.handNote == decisionNoteOwed { continue } node.givesBackLocked() diff --git a/internal/session/task_states_test.go b/internal/session/task_states_test.go index 535ff573e7..8454db028d 100644 --- a/internal/session/task_states_test.go +++ b/internal/session/task_states_test.go @@ -652,6 +652,87 @@ func TestTheFloorOnlyTakesBackWhatThisTurnWasAsked(t *testing.T) { } } +// AND A TURN TAKES BACK ONLY WHAT IT WAS ASKED IN THE OTHER SENSE TOO: a +// decision whose note no request has carried is not this turn's to spend, +// whoever is ending one. The press is made on the conversation and the note sits +// on the conversation's queue; a worker finishing a piece of work knows nothing +// about it, and until this the worker's own floor took it back. +func TestAWorkerTurnDoesNotSpendADecisionNobodyHasBeenGiven(t *testing.T) { + graph := &TaskGraph{nodes: map[uint64]*TaskNode{}, order: []uint64{1, 2}} + root := &TaskNode{graph: graph, id: 1, state: TaskRunning, spec: taskSpec{title: "the whole job"}} + piece := &TaskNode{ + graph: graph, id: 2, parent: 1, state: TaskUnverified, + decider: TaskAskOwnerModel, handNote: decisionNoteOwed, + spec: taskSpec{title: "a piece of it"}, + } + graph.nodes[1], graph.nodes[2] = root, piece + + worker := &Agent{config: Config{tasker: graph, taskID: 1}} + worker.handBackUnsettled() + if piece.decider != TaskAskOwnerModel { + t.Fatal("a worker's turn ending spent a decision whose note no request had carried") + } + // AND IT IS NOT A DOOR THAT STAYS SHUT. Once a request has carried the note, + // the next turn to end is the window closing, exactly as it always was. + graph.mu.Lock() + piece.handNote = decisionNoteRead + graph.mu.Unlock() + worker.handBackUnsettled() + if piece.decider != TaskAskOwnerPerson { + t.Fatalf("the floor did not take back a question the model had been asked (%q)", piece.decider) + } +} + +// THE SETTLE POLICY'S OWN HAND-OVER IS THE SAME QUESTION THROUGH ANOTHER DOOR. +// Under `task.settle = auto` a landing is the model's to decide by policy +// ([Agent.handToModelOnAuto]) and the landing note is the whole of the asking, +// so a turn that ends between the landing and the request that carries that note +// must no more spend it than it may spend a press. +func TestALandingHandedOverByPolicyIsNotSpentBeforeItsNoteIsCarried(t *testing.T) { + graph, node := floorGraph("") + agent := &Agent{config: Config{tasker: graph}} + agent.handToModelOnAuto(node) + // The note reaches a reader — which is where this road writes the mark, + // because a landing folded into a parent's report never gets a note at all. + node.owesDecisionNote() + agent.handBackUnsettled() + if node.decider != TaskAskOwnerModel { + t.Fatal("the floor spent a policy hand-over before its note reached a request") + } + // AND THE REQUEST IS WHAT MAKES IT A QUESTION THE MODEL HAS. + agent.decisionNotesOwed = []uint64{node.id} + agent.decisionNotesCarried() + if got := decisionNoteOf(node); got != decisionNoteRead { + t.Fatalf("the note the request carried is %v, want it read", got) + } + agent.handBackUnsettled() + if node.decider != TaskAskOwnerPerson { + t.Fatalf("the floor did not take back the landing after the model was asked (%q)", node.decider) + } +} + +// AND WHEN NO TURN FOLLOWS, WHAT WAS NEVER ASKED COMES BACK AT ONCE. The floor +// leaves an unread hand-over alone so that one turn ending cannot spend a press +// the next turn will read — which is right while a next turn is coming, and +// wrong the moment one is not: a question nobody will be asked sits in nobody's +// hands, and home stops counting it as something needing a person. +func TestWhatNoTurnWillEverCarryComesBackToThePerson(t *testing.T) { + graph, node := floorGraph("") + agent := &Agent{config: Config{tasker: graph}} + // The press's own mark, written exactly as [Agent.HandUnverifiedToModel] + // writes it, and the note in this agent's hands with no request gone out. + node.holdsDecision(TaskAskOwnerModel) + agent.decisionNotesOwed = []uint64{node.id} + // The turn that would have carried it has ended with nothing following. + agent.handBackUnreadNotes() + if node.decider != TaskAskOwnerPerson { + t.Fatalf("a press nothing was ever going to carry was left with the model (%q)", node.decider) + } + if node.wasHandedOver() { + t.Fatal("the hand-back left the press's receipt behind") + } +} + // ── the floor across a restart ────────────────────────────────────────────── // WHO IS DECIDING SURVIVES THE PROCESS, AND IS HANDED BACK ON THE WAY IN. The diff --git a/internal/session/tools_tasks_test.go b/internal/session/tools_tasks_test.go index 1908fe7fe0..36dc279135 100644 --- a/internal/session/tools_tasks_test.go +++ b/internal/session/tools_tasks_test.go @@ -717,8 +717,8 @@ func TestAPressNoRequestCarriedKeepsTheDecisionForTheTurnThatReadsIt(t *testing. if err := agent.HandUnverifiedToModel(id); err != nil { t.Fatalf("handing the decision over: %v", err) } - if waiting := agent.handOversWaiting(); !waiting[id] { - t.Fatal("the press left no note waiting for the model on the queue") + if note := decisionNoteOf(graph.node(id)); note != decisionNoteOwed { + t.Fatalf("the press marked the node %v, want the note owed and unread", note) } // The floor, run by hand at the instant the turn in flight would run it. @@ -738,6 +738,49 @@ func TestAPressNoRequestCarriedKeepsTheDecisionForTheTurnThatReadsIt(t *testing. }) } +// AND A TURN SOMEBODY STOPPED HANDS BACK WHAT IT NEVER ASKED. This is the same +// law from the other end (task_states_test.go's +// [TestWhatNoTurnWillEverCarryComesBackToThePerson]), driven through the real +// turn: an interrupted turn starts no successor, so the press it was carrying +// reaches nobody and the card has to draw its chips again — until this it stayed +// `aforge is deciding` until the person's NEXT turn, and home stopped counting +// it as something needing them (taskstatus.go). +func TestAnInterruptedTurnHandsBackThePressItNeverCarried(t *testing.T) { + held, entered, release := modelHoldsTheTurn() + t.Cleanup(release) + agent, _ := newTestAgent(t, &scriptedCompleter{steps: []step{held}}, func(config *Config) { + config.AskConsent = true + }) + graph := stubbedGraph(agent, func(node *TaskNode) { + node.finish("UNVERIFIED — the auditor answered neither VERIFIED nor REFUTED", nil, "", "") + node.graph.complete(node, TaskUnverified) + }) + events := mustSubmit(t, agent, "keep thinking") + waitUntilClosed(t, "the model to be holding the turn", entered, 10*time.Second) + + id := graph.reserve() + graph.admit(id, taskSpec{title: "Hidden rental digs", named: true, brief: "b", acceptance: "a"}) + waitDoneNode(t, graph.node(id)) + if err := agent.HandUnverifiedToModel(id); err != nil { + t.Fatalf("handing the decision over: %v", err) + } + + agent.Interrupt() + collect(t, events) + waitFor(t, "the stopped turn to hand back what it never asked", func() bool { + node := agent.taskNode(id) + return node != nil && node.decidedBy() == TaskAskOwnerPerson && !node.wasHandedOver() + }) +} + +// decisionNoteOf reads how far one node's decision note has got, under the lock +// every other reader of a node's fields takes. +func decisionNoteOf(node *TaskNode) decisionNote { + node.graph.mu.Lock() + defer node.graph.mu.Unlock() + return node.handNote +} + // TestAnsweringLetAforgeDecideTwiceStandsRatherThanRefusing is the same press // from the OTHER side — the door a person's key goes through // ([Agent.applyLanding]). The row somebody is looking at was drawn before the From d21c8318e9e609aac2a03afff3f49444497e74f6 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sat, 12 Sep 2026 11:10:31 -0400 Subject: [PATCH 09/14] delta: the reading beside a turn takes the one door #997 named it for #997 wrote its closed-check-and-owe as one hold of a.mu and said in the comment that the region becomes a call to writeIfOpen when it lands. It has landed: the three lines are its closure now, and the door's own comment names all three readings that go through it. The early-out above it stays its own check, which is what its comment asks for: a closed session does not even walk the index, and that is a decision about the READING rather than about the write it owes. Co-Authored-By: Claude Opus 5 (1M context) --- internal/session/taskdelta.go | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/internal/session/taskdelta.go b/internal/session/taskdelta.go index 0f2ce42493..526f3437c6 100644 --- a/internal/session/taskdelta.go +++ b/internal/session/taskdelta.go @@ -621,12 +621,11 @@ func (a *Agent) refreshElsewhere(ctx context.Context) { // directory not empty`, and the file left behind was this stamp. // // THE CHECK AND THE WRITES ARE ONE HOLD BECAUSE THE CHECK IS TRUE ONLY FOR - // AS LONG AS IT IS HELD — `Close` sets `closed` under this same lock. The - // shape is deliberate and temporary: #999 lands [Agent.writeIfOpen], the one - // door for exactly this — write nothing into a conversation that has closed, - // already carried by the memory pass and the phase news — and this region - // becomes a call to it with the three lines below as its closure. Nothing but - // those three lines belongs between the check and the unlock until it does. + // AS LONG AS IT IS HELD — `Close` sets `closed` under this same lock. That is + // [Agent.writeIfOpen] (agent.go), the one door for exactly this, and this + // reading is the third caller of it beside the memory pass and the phase + // heart: the three lines below are its closure and nothing else belongs + // inside them. // // AND THE TURN ENDING IS NOT THAT DOOR. The context is what stops this // reading STARTING work — the walk above — and it deliberately does not stop @@ -654,15 +653,11 @@ func (a *Agent) refreshElsewhere(ctx context.Context) { // note the drain lands immediately before the next request, and not in // message[0] where it used to sit: writing it there re-priced every message // of the conversation behind it each time another window landed something. - a.mu.Lock() - if a.closed { - a.mu.Unlock() - return - } - a.oweToldStampLocked(dir, now) - a.elsewhereTold = deltaRemember(a.elsewhereTold, fresh, deltaLandedRows) - a.elsewhereText = renderElsewhereBlock(a.elsewhereTold, live) - a.mu.Unlock() + a.writeIfOpen(func() { + a.oweToldStampLocked(dir, now) + a.elsewhereTold = deltaRemember(a.elsewhereTold, fresh, deltaLandedRows) + a.elsewhereText = renderElsewhereBlock(a.elsewhereTold, live) + }) } // toldStamp is the one deferred write this reading owes told.json. From 58a241f40a3d13d9ed12c29ec5cfa8000a7ffb3c Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sat, 12 Sep 2026 11:10:37 -0400 Subject: [PATCH 10/14] changelog: the third reading takes the door (#999) Co-Authored-By: Claude Opus 5 (1M context) --- docs/changes/unreleased/999-handover-twice-race.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changes/unreleased/999-handover-twice-race.md b/docs/changes/unreleased/999-handover-twice-race.md index a57e010be7..1c709d2647 100644 --- a/docs/changes/unreleased/999-handover-twice-race.md +++ b/docs/changes/unreleased/999-handover-twice-race.md @@ -9,7 +9,7 @@ invalidates: - "A decision the model was given is marked read at THE REQUEST and not at the drain (`Agent.decisionNotesCarried`, called from the one line in `loop.go` where a request goes out). A note drained at a step boundary whose turn is then cut — an interrupt, an oversize refusal, a checkpoint that ends the turn — has left the queue and reached nobody, and a mark at the drain would have let the floor spend a question no model read." - "A press that no turn will ever carry now comes back at once instead of waiting for the person's next turn. A turn that ends with nothing following it — interrupted, failed, or simply owing nobody a word — hands back what that agent owed (`Agent.handBackUnreadNotes`), because there is no longer a turn coming to ask. Until this, such a node stayed model-owned and home stopped counting it as something needing a person (`taskstatus.go`). `internal/manual/chat`'s `task-controls.md` and `tasks.md` said the old thing and now say which turn counts and what a stopped turn does." - "`Agent.Close` let go of a steer's grace and a clock armed on a question (#963) but not of a held stage's beat, which is re-said on a ticker of its own until the stage ends (`Agent.beatHeldPhase`). A session closed mid-stage left that goroutine posting a conversation's stage after the conversation had gone — under `-race`, a live reader of this package's own `phaseHeldBeat` while the next test wrote it (#959). `Close` ends the stage properly, so a surface takes the clock down, and then WAITS for the beat: `Agent.phaseBeats` is a `sync.WaitGroup` like `memoryJobs`, and the Add is taken inside the door that refuses an arming on a closed session, so nothing can join the count after the quit has begun waiting on it. The join is the quit's door alone; ending a stage on a turn's path still cancels and carries on." - - "There is now ONE door for anything a reading beside the work leaves behind: `Agent.writeIfOpen` (agent.go) checks `closed` and performs the write in the SAME hold of `a.mu`, because a reading that asks and then writes is a reading `Close` fits inside. `Agent.refreshMemory` landed its routed block into a closed conversation's transcript with no check at all and goes through it now, and so does the phase heart's arming. #997's `refreshElsewhere` is the third caller and adopts it when that branch lands." + - "There is now ONE door for anything a reading beside the work leaves behind: `Agent.writeIfOpen` (agent.go) checks `closed` and performs the write in the SAME hold of `a.mu`, because a reading that asks and then writes is a reading `Close` fits inside. `Agent.refreshMemory` landed its routed block into a closed conversation's transcript with no check at all and goes through it now, and so does the phase heart's arming. #997's `refreshElsewhere` is the third caller and takes the door in this change — its own comment said the region would become a call to it — keeping its earlier early-out, which is a decision about whether to READ at all rather than about the write it owes." - "`internal/session`'s hand-over fixtures asserted about the window in which the model is holding a decision by pressing and then reading. There is no such window against a scripted model: the press wakes a turn whose whole life is microseconds. `modelHoldsTheTurn` parks the model inside its call — no queue drains while a turn is inside a request, and no second turn starts while one is running — so the window is stated rather than raced. `TestHandingTheSameDecisionOverTwiceSaysItIsAlreadyHandedOver` failed 3 runs in 30 under `-race` on plain `dev` @ 82624af79 and is one of #959's faces; it is not a known red and never was." --- From 25cae17306e88679d18f06096fe6483f5395eceb Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sat, 12 Sep 2026 11:36:44 -0400 Subject: [PATCH 11/14] tasks: one bool, marked before the note is sent and cleared when a model answers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The tri-state was write-only: nothing read 'read' apart from the guard that made a second note to an already-read node a no-op, which is the bug — a fresh note is a question nobody has been given, whatever became of the first. One bool, owed again every time. The mark moves to BEFORE the send, and is taken back when nobody takes the note. Written after deliverTo it was a mark the reader could already have cleared by draining and requesting first, and the node was then pinned owed for the life of the process: a card reading 'aforge is deciding' that no floor and no second press could get out of. Only the caller knows whether a note asks about the node it is posted with, so it says: the re-addressing note about a parent's CHILDREN no longer stamps the parent. The mark is cleared when a model ANSWERS, not when a request leaves. A request that reaches no model asked nobody anything, and clearing it on the way out handed the card back while the sentence asking for it sat unread in the transcript, to be sent again next turn. And the three hand-backs are one walk with one question each: whose decisions this agent may take back, and whether a question nobody has been given is left alone. Co-Authored-By: Claude Opus 5 (1M context) --- internal/session/abandon.go | 9 ++ internal/session/agent.go | 99 +++++----------- internal/session/loop.go | 17 +-- internal/session/task_audit.go | 67 +++++------ internal/session/task_run.go | 167 +++++++++++++++++---------- internal/session/task_states_test.go | 10 +- internal/session/tools_tasks_test.go | 10 +- 7 files changed, 196 insertions(+), 183 deletions(-) diff --git a/internal/session/abandon.go b/internal/session/abandon.go index b62f39d99c..0c20c9d9b1 100644 --- a/internal/session/abandon.go +++ b/internal/session/abandon.go @@ -138,6 +138,15 @@ func (a *Agent) Abandon(reason AbandonReason) (Usage, bool) { // is what lets it sit here. a.nudgePresence() a.mu.Unlock() + // AND A DECISION THIS TURN WAS CARRYING AND NEVER ASKED COMES BACK TO THE + // PERSON. A hard stop starts no successor — that is what makes it hard — so a + // note the drain above just took is one nothing will ever put in front of the + // model, and the ordinary ending's own answer to that + // ([Agent.handBackUnread]) cannot reach here: the goroutine still running + // under this turn finds the number moved and cleans up nothing. Without it + // the card stayed `aforge is deciding` on work nobody was deciding, and the + // second press answered `already handed to aforge` with no way back. + a.handBackUnread() // THE WAITS END FIRST, then the request is cut, then the surface is freed. // The order is what makes the freeing honest: a hub closed before the waits diff --git a/internal/session/agent.go b/internal/session/agent.go index 242e4f2082..9566cfb3e9 100644 --- a/internal/session/agent.go +++ b/internal/session/agent.go @@ -1790,7 +1790,15 @@ func (a *Agent) startTurnLocked(ctx context.Context, user userMessage, watcher * // carries this transcript, so a note in it is a question that will be // put in front of the model. When nothing follows, nothing will, and // what this agent owes goes back to the person below. - followed := false + // + // A WORKER'S TURN ENDING IS NEVER THE LAST ONE. Its turns belong to + // the runner that drives it — [Agent.wakeLocked] declines inside a + // task by construction — and the runner re-enters it the moment one + // of its own pieces reports ([Agent.resumeTurn], task_child_run.go's + // fold). Reading `followed` off this agent's own doors would make it + // structurally false for every worker turn, and the hand-back would + // then race the very resume that carries the note. + followed := a.config.InTask if next, ok := a.nextFollowUpLocked(completed); ok { followed = true // context.Background rather than the finished turn's: the @@ -1825,12 +1833,17 @@ func (a *Agent) startTurnLocked(ctx context.Context, user userMessage, watcher * // one turn ending must not spend a press the next turn will read; // this is the case where there is no next turn, and a question left // with the model then sits in nobody's hands - // ([Agent.handBackUnreadNotes]). It is after the unlock because it - // takes the graph's lock, and after the disowned-turn return above - // because a turn that was abandoned has had every act here done for - // it already. + // ([Agent.handBackUnread]). + // + // THE ANSWER WAS DECIDED UNDER THE LOCK AND IS ONLY ACTED ON HERE: + // `followed` is read in the same hold that started the successor, so + // a Submit arriving in this gap cannot make it stale — it starts a + // turn of its own, and a turn started after this decision was taken + // is one the person is sitting in front of. The act is outside + // because it takes the graph's lock, which this one is never held + // across. if !followed { - a.handBackUnreadNotes() + a.handBackUnread() } // AND NO QUESTION OF THE MODEL'S OWN OUTLIVES THE TURN THAT RAISED // IT UNLESS IT SAID IT WOULD. It is [Agent.handBackUnsettled]'s law @@ -2756,16 +2769,16 @@ func (a *Agent) drainSteering(hub *eventHub) int { return landed } -// decisionNotesCarried says a request has gone out carrying the notes this -// agent's transcript holds, so the landings they hand over are questions the -// model HAS (task_audit.go's [decisionNote]). +// decisionNotesCarried says a model has ANSWERED a request carrying the notes +// this agent's transcript holds, so the landings they hand over are questions +// the model has (task_audit.go's [TaskNode.owesDecisionNote]). // -// IT IS CALLED AT THE ONE LINE IN THIS PACKAGE WHERE A REQUEST ACTUALLY GOES OUT -// (loop.go) and nowhere else, which is the difference between this and marking -// at the drain: a note drained at a step boundary whose turn is then cut — an -// interrupt, an oversize refusal, a checkpoint that ends the turn — has left the -// queue and reached nobody, and a mark made at the drain would let the floor -// spend a question no model ever read. +// IT IS SAID AT THE ANSWER AND NOT AT THE SENDING. A request that never reaches +// a model — the ladder giving up on every machine, a cut context, a refusal +// before the wire — has asked nobody anything, and a mark made on the way out +// would let the floor spend a press whose sentence is still sitting unread in +// the transcript, to be sent again next turn. The answer is the first moment the +// question has demonstrably been in front of the model. // // IT IS THIS AGENT'S OWN NOTES AND NOT THE GRAPH'S. A worker's request carries // what a worker was told; a press made on the conversation is not a question a @@ -2788,62 +2801,10 @@ func (a *Agent) decisionNotesCarried() { graph.mu.Lock() defer graph.mu.Unlock() for _, id := range owed { - if node := graph.nodes[id]; node != nil && node.handNote == decisionNoteOwed { - node.handNote = decisionNoteRead - } - } -} - -// handBackUnreadNotes gives back every decision THIS agent owes the model a note -// about and is now never going to deliver, because its turn has ended and -// nothing follows it. -// -// IT IS THE OTHER HALF OF THE FLOOR'S OWN LAW (task_run.go's -// [Agent.handBackUnsettled]). The floor leaves an unread hold alone so that one -// turn ending cannot spend a press the next turn will read; that is right while -// a next turn is coming, and wrong the moment one is not. A turn somebody -// interrupted, or one that ended owing nothing anybody is waiting for, starts no -// successor — and a question left with the model then would sit in nobody's -// hands: the card stops offering its chips and home stops counting it as -// something needing a person (taskstatus.go). -// -// It is every note this agent holds: the ones still on its queue and the ones -// its transcript took at a drain no request followed. -func (a *Agent) handBackUnreadNotes() { - a.mu.Lock() - owed := a.decisionNotesOwed - a.decisionNotesOwed = nil - for _, message := range a.steering { - owed = append(owed, message.handsOver...) - } - a.mu.Unlock() - if len(owed) == 0 { - return - } - graph := a.tasker() - if graph == nil { - return - } - var handed []*TaskNode - graph.mu.Lock() - for _, id := range owed { - node := graph.nodes[id] - if node == nil || node.handNote != decisionNoteOwed { - continue - } - node.givesBackLocked() - // A NODE THAT WAS ACTUALLY SETTLED IS NOT NEWS, for the floor's own - // reason: the resolution published its own landing. - if node.state == TaskUnverified { - handed = append(handed, node) + if node := graph.nodes[id]; node != nil { + node.noteOwed = false } } - graph.mu.Unlock() - // The notices are read with the lock let go of ([TaskNode.notice] states the - // ordering). - for _, node := range handed { - a.emitTaskUpdate(node.notice()) - } } // queuedDirections is every node receipt id on one queue. diff --git a/internal/session/loop.go b/internal/session/loop.go index f6dd1b29dc..6fabc133e4 100644 --- a/internal/session/loop.go +++ b/internal/session/loop.go @@ -876,13 +876,6 @@ func (a *Agent) runTurn(ctx context.Context, hub *eventHub, user userMessage) bo // which is exactly the news an outside reader wants (task_beat.go). It is // nil for a conversation, whose liveness the presence file already carries // (taskpresence.go). - // AND EVERY DECISION THIS TRANSCRIPT HOLDS A NOTE ABOUT IS A QUESTION THE - // MODEL HAS FROM HERE. It is on this line for the reason the beat and the - // clock below it are: this is the one place in this package where a - // request actually goes out, and a note that left the queue at a drain - // this turn was then cut short of has reached nobody - // (agent.go's [Agent.decisionNotesCarried]). - a.decisionNotesCarried() a.config.beat.began() // AND THE FIRST-TOKEN CLOCK, on the same line and for a related reason: // this is the one place in this package where a request actually goes @@ -897,6 +890,16 @@ func (a *Agent) runTurn(ctx context.Context, hub *eventHub, user userMessage) bo pace.sending(sentAt) response, answered, err := a.completeWithRetryReasoning(ctx, hub, model, rung, partial, reached, reasoning, warm, forming, frozenToolHistory) a.config.beat.ended() + // AND A MODEL THAT ANSWERED HAS BEEN ASKED EVERY QUESTION THIS TRANSCRIPT + // WAS CARRYING. It is said at the ANSWER rather than on the line above + // where the request leaves, because a request that reaches no model — + // the ladder giving up on every machine, a cut context — asked nobody + // anything, and a decision marked read there would be a card handed back + // while the sentence that asks for it is still in the transcript, to be + // sent again next turn (agent.go's [Agent.decisionNotesCarried]). + if err == nil { + a.decisionNotesCarried() + } // THE MODEL THIS TURN IS ON CAN CHANGE UNDER IT. A step whose budget of // cut streams ran out moves to the next model in the chain and says so, // and everything the rest of the turn attributes — the usage rows, the diff --git a/internal/session/task_audit.go b/internal/session/task_audit.go index 4368335e38..0ef5149350 100644 --- a/internal/session/task_audit.go +++ b/internal/session/task_audit.go @@ -2679,9 +2679,10 @@ func (n *TaskNode) wasHandedOver() bool { return n.handed } -// decisionNote is HOW FAR THE NOTE THAT HANDS ONE NODE'S DECISION TO THE MODEL -// HAS GOT, and it exists because the floor that takes a hand-over back has to -// know whether the model was ever actually asked. +// owesDecisionNote marks one node as having a note on its way to the model that +// asks it to decide — and it is ONE BOOLEAN because there are only two states a +// floor can act on: a question nobody has been given, and a question the model +// has. // // THE WINDOW IS THE TURN THAT PUT THE QUESTION IN FRONT OF THE MODEL, NOT // WHICHEVER TURN HAPPENS TO END NEXT. A note lands on the steering queue and a @@ -2693,45 +2694,39 @@ func (n *TaskNode) wasHandedOver() bool { // decision the person was holding again. Two hands on one question is the exact // thing the receipt exists to prevent. // -// It is assignment.go's own law about a direction said to a node, said here -// about a decision handed to the model: a line accepted after the last drain was -// never in front of anybody, and the state it promises must not be spent on it. -type decisionNote uint8 - -const ( - // decisionNoteNone is every node with nothing asking the model to decide it — - // including one the settle policy marked the model's whose news was folded - // into a parent's report rather than delivered ([Agent.postTaskMessage]). - decisionNoteNone decisionNote = iota - // decisionNoteOwed is a note a reader has taken and no request has carried: - // on a queue, or in a transcript no request has gone out since. The floor - // leaves this hold alone; the agent that owes the note hands it back itself - // if its turn ends with nothing following (agent.go's - // [Agent.handBackUnreadNotes]). - decisionNoteOwed - // decisionNoteRead is a note a request carried: the model has the question, - // and the turn it went out in is the window it has to answer in. - decisionNoteRead -) - -// owesDecisionNote marks one node as having a note on its way to the model. +// IT IS WRITTEN BEFORE THE NOTE IS SENT AND TAKEN BACK IF NOBODY TAKES IT +// ([TaskNode.decisionNoteDropped]), never after. A mark written after the queue +// has the note is a mark a reader that drains and requests first has already +// cleared — and the node is then pinned owed for the life of the process, which +// is a card that says aforge is deciding until the session ends. // -// IT IS WRITTEN WHERE THAT BECOMES TRUE, which is two places because there are -// two roads and they differ in exactly this. A press writes it in the same hold -// as the owner ([TaskNode.holdsDecision]) because the press enqueues its own -// note in the next breath and cannot fail to; a landing under -// `task.settle = auto` writes it when a reader has actually taken the note, -// because that news may be folded into a parent's report instead of delivered. +// AND IT IS OWED AGAIN EVERY TIME, which is why nothing here asks what the mark +// already said. A second note about the same node — a press after a landing, a +// re-audit that lands while one is queued — is a question nobody has been given +// yet, whatever became of the first. func (n *TaskNode) owesDecisionNote() { if n == nil || n.graph == nil { return } n.graph.mu.Lock() defer n.graph.mu.Unlock() - if n.decider != TaskAskOwnerModel || n.handNote != decisionNoteNone { + if n.decider != TaskAskOwnerModel { return } - n.handNote = decisionNoteOwed + n.noteOwed = true +} + +// decisionNoteDropped is that in reverse, for the note that never reached +// anybody: a delivery nobody accepted, or one folded into a parent's report +// rather than queued ([Agent.postTaskMessage]). A hold owed a note that does not +// exist is a card waiting on an answer nobody was ever asked for. +func (n *TaskNode) decisionNoteDropped() { + if n == nil || n.graph == nil { + return + } + n.graph.mu.Lock() + defer n.graph.mu.Unlock() + n.noteOwed = false } // decidedBy reads who is holding one node's question, with the graph taken for @@ -2752,7 +2747,7 @@ func (n *TaskNode) decidedBy() TaskAskOwner { // (task_run.go). A road that wrote only the owner would leave a receipt behind // and refuse the next press. func (n *TaskNode) givesBackLocked() { - n.decider, n.handed, n.handNote = TaskAskOwnerPerson, false, decisionNoteNone + n.decider, n.handed, n.noteOwed = TaskAskOwnerPerson, false, false } // holdsDecision writes who is holding one node's question. It is the graph's @@ -2777,8 +2772,8 @@ func (n *TaskNode) holdsDecision(owner TaskAskOwner) { // AND THE NOTE IS OWED IN THE SAME HOLD AS THE OWNER. The press enqueues it // in the next breath, and writing the two under one lock is what keeps a // floor running in between from spending a press nobody has been given - // ([decisionNote]). - n.decider, n.handed, n.handNote = owner, true, decisionNoteOwed + // ([TaskNode.owesDecisionNote]). + n.decider, n.handed, n.noteOwed = owner, true, true } // TakeBackDecision is [Agent.HandUnverifiedToModel] in reverse: the person diff --git a/internal/session/task_run.go b/internal/session/task_run.go index 6577784748..15c1c3952f 100644 --- a/internal/session/task_run.go +++ b/internal/session/task_run.go @@ -823,13 +823,14 @@ type TaskNode struct { // through [TaskNode.givesBackLocked] so a hand-back leaves no receipt behind // (task_audit.go's [TaskNode.wasHandedOver] states the whole rule). handed bool - // handNote is HOW FAR THE NOTE THAT HANDS THIS DECISION TO THE MODEL HAS GOT - // (task_audit.go's [decisionNote]), and it is on the NODE rather than on any - // one agent's queue because the floor that takes a hand-over back runs at the - // end of every agent's turn: a worker reads its own children's decisions - // ([Agent.readsTheDecisionLocked]), and a fact kept per queue let a worker's - // turn spend a press that was made, and is still waiting, somewhere else. - handNote decisionNote + // noteOwed says A NOTE ASKING THE MODEL TO DECIDE THIS NODE IS ON ITS WAY AND + // NO REQUEST HAS CARRIED IT (task_audit.go's [TaskNode.owesDecisionNote]). It + // is on the NODE rather than on any one agent's queue because the floor that + // takes a hand-over back runs at the end of every agent's turn: a worker reads + // its own children's decisions ([Agent.readsTheDecisionLocked]), and a fact + // kept per queue let a worker's turn spend a press that was made, and is + // still waiting, somewhere else. + noteOwed bool // stopReason is what whoever pulled the stop said they were stopping it FOR, // and "" for every stop that came with no words — which is every one a person // pulls, their card being a decision and not a sentence (cancel.go). It is @@ -3840,7 +3841,7 @@ func (a *Agent) deliverTaskNote(node *TaskNode, attempt int, tag TaskReplyTag, n // that closed with the note unread re-tells it on resume rather than losing // it (task_store.go). got := a.postTaskMessage(node, tag, note, record, []durableDelivery{node.settlesNote(claim)}, - func() { node.noteQueued(claim) }) + func() { node.noteQueued(claim) }, true) if !got.accepted() { // Nobody is left to read it, so the claim goes back and no mark is made. node.releaseNote(claim) @@ -3884,17 +3885,30 @@ func (n *TaskNode) settlesNote(claim noteClaim) durableDelivery { // under one lock ([Agent.handOverTaskNews]) because the waiter reads them as one // fact ([Agent.taskNewsStanding]). The checkpoint the mark owes the disk is // written by the caller, after the seam. -func (a *Agent) postTaskMessage(node *TaskNode, tag TaskReplyTag, note, record string, durable []durableDelivery, mark func()) deliveryReceipt { +func (a *Agent) postTaskMessage(node *TaskNode, tag TaskReplyTag, note, record string, durable []durableDelivery, mark func(), asksAboutIt bool) deliveryReceipt { message := wakeNote(note) - // AND WHERE THIS LANDING'S DECISION IS THE MODEL'S, THE NOTE SAYS SO. Under - // `task.settle = auto` the landing hands the question over by policy - // ([Agent.handToModelOnAuto]) and THIS note is the whole of the asking, so it - // carries the landing the way a press's own note does (task_audit.go's - // [Agent.HandUnverifiedToModel]) — which is what lets the drain and the - // request tell a question the model has been given from one still on its way. - hands := node.decidedBy() == TaskAskOwnerModel + // AND WHERE THIS NOTE IS THE ASKING, IT SAYS SO. Under `task.settle = auto` a + // landing hands its own question over by policy ([Agent.handToModelOnAuto]) + // and the landing note is the whole of the asking, so it carries the node the + // way a press's own note does (task_audit.go's + // [Agent.HandUnverifiedToModel]) — which is what lets the request that + // carries it say the model has been given the question. + // + // ONLY THE CALLER KNOWS WHETHER THIS SENTENCE ASKS ABOUT THIS NODE. The other + // road through here re-addresses what a parent's CHILDREN are still waiting + // on ([Agent.bubbleUnverifiedChildren]): the node it posts with is the + // parent, and a sentence that asks nothing about the parent must not stamp + // the parent as asked. + hands := asksAboutIt && node.decidedBy() == TaskAskOwnerModel if hands { message.handsOver = []uint64{tag.ID} + // THE MARK IS WRITTEN BEFORE THE NOTE IS SENT, and taken back below if + // nobody takes it. A mark written after the send is a mark the reader may + // already have cleared — it drains, it requests, it finds nothing owed — + // and the node is then pinned owed for the life of the process: a card + // that reads `aforge is deciding` until the session ends, which no floor + // and no second press can get out of. + node.owesDecisionNote() } // THE TAG IS THE CALLER'S SNAPSHOT AND IS NOT RE-READ FROM THE NODE HERE. // This delivery happens after a claim is won and a reader is found, and a @@ -3904,6 +3918,10 @@ func (a *Agent) postTaskMessage(node *TaskNode, tag TaskReplyTag, note, record s message.delivered = durable got := deliverTo(delivery{origin: fromRuntime, kind: msgResult, note: message, record: record}, a.taskNoteReaders(node)...) if !got.accepted() { + // NOBODY TOOK THE QUESTION, so nobody is owed an answer to it. + if hands { + node.decisionNoteDropped() + } return got } // AND A FOLD HAS NO QUEUE AND NOBODY TO WAKE. The writes a queued delivery @@ -3916,6 +3934,12 @@ func (a *Agent) postTaskMessage(node *TaskNode, tag TaskReplyTag, note, record s // at once — which is what writes the landing down as announced, so that a // restart does not tell a folded piece a second time. if got.reader == nil { + // A FOLD IS NOT A QUESTION EITHER. The note went into a parent's report + // rather than onto a queue, so there is nothing for a request to carry + // and nothing to hold the card open for. + if hands { + node.decisionNoteDropped() + } if mark != nil { mark() } @@ -3926,13 +3950,6 @@ func (a *Agent) postTaskMessage(node *TaskNode, tag TaskReplyTag, note, record s } return got } - // AND THE NOTE IS OWED FROM THE MOMENT A READER HAS IT. Before this line - // there is no note anywhere — the fold above took it into a report instead — - // and a decision marked owed with nothing to carry it would be a card saying - // aforge is deciding with no question in front of anybody. - if hands { - node.owesDecisionNote() - } got.reader.handOverTaskNews(mark) return got } @@ -4463,39 +4480,83 @@ func (a *Agent) handToModelOnAuto(node *TaskNode) { // deciding ([TaskNotice.Decider]), so the hand-back is one more update about a // node rather than a channel of its own. func (a *Agent) handBackUnsettled() { + a.saysWhoIsDeciding(a.handBackModelHeld(true)) +} + +// handBackUnread is this floor with the exception lifted, for the one case the +// exception is wrong in: THE TURN THAT ENDED IS THE LAST ONE. No follow-up, no +// wake, a turn somebody stopped — so the note this agent's drain took is never +// going to be put in front of anybody, and a question left with the model then +// sits in nobody's hands: the card stops offering its chips and home stops +// counting it as something needing a person (taskstatus.go). +// +// It takes [Agent.readsTheDecisionLocked]'s own question for the reason the +// floor does: a decision whose window belongs to a live parent worker is not +// this conversation's to end. +func (a *Agent) handBackUnread() { + a.saysWhoIsDeciding(a.handBackModelHeld(false)) +} + +// handBackModelHeld is THE ONE WALK, and the three roads that give a decision +// back differ in two answers rather than in three loops: whose decisions this +// agent may take back, and whether a question nobody has been given yet is left +// alone. [TaskGraph.handBackOnLoad] is the third — every model-held node, +// nothing left alone — and it says in its own words why it answers both +// differently. +func (a *Agent) handBackModelHeld(keepUnread bool) []*TaskNode { graph := a.tasker() if graph == nil { - return + return nil + } + return graph.handBackHeld(func(node *TaskNode) bool { + if keepUnread && node.noteOwed { + // THE DECISION NOBODY HAS BEEN GIVEN YET. No request has carried its + // note, so no turn has asked about it and the turn that reads it owns + // the window. + return false + } + return a.readsTheDecisionLocked(node) + }) +} + +// saysWhoIsDeciding publishes one hand-back on the ordinary task lane. The +// notices are read with the graph let go of, because reading one asks the room +// and the child agent for the spend, each of which is a lock of its own +// ([TaskNode.notice] states the ordering). +func (a *Agent) saysWhoIsDeciding(handed []*TaskNode) { + for _, node := range handed { + a.emitTaskUpdate(node.notice()) + } +} + +// handBackHeld walks the graph once and gives back every model-held decision the +// caller's own question says yes to, answering the ones that are news. +// +// A NODE THAT WAS ACTUALLY SETTLED IS NOT NEWS. The model spent its verb, the +// resolution published its own landing, and a second update saying the question +// is back with the person would put a card up over work that has finished being +// decided. +func (g *TaskGraph) handBackHeld(takes func(*TaskNode) bool) []*TaskNode { + if g == nil { + return nil } var handed []*TaskNode - graph.mu.Lock() - for _, id := range graph.order { - node := graph.nodes[id] - if node == nil || node.decider != TaskAskOwnerModel || !a.readsTheDecisionLocked(node) { + g.mu.Lock() + defer g.mu.Unlock() + for _, id := range g.order { + node := g.nodes[id] + if node == nil || node.decider != TaskAskOwnerModel { continue } - // THE DECISION NOBODY HAS BEEN GIVEN YET. No request has carried its - // note, so no turn has asked about it and the turn that reads it owns - // the window. - if node.handNote == decisionNoteOwed { + if takes != nil && !takes(node) { continue } node.givesBackLocked() - // A NODE THAT WAS ACTUALLY SETTLED IS NOT NEWS. The model spent its verb, - // the resolution published its own landing, and a second update saying the - // question is back with the person would put a card up over work that has - // finished being decided. if node.state == TaskUnverified { handed = append(handed, node) } } - graph.mu.Unlock() - // The notices are read with the lock let go of, because reading one asks the - // room and the child agent for the spend, each of which is a lock of its own - // ([TaskNode.notice] states the ordering). - for _, node := range handed { - a.emitTaskUpdate(node.notice()) - } + return handed } // handBackOnLoad is THE SAME FLOOR APPLIED TO A GRAPH COMING OFF THE DISK, and @@ -4521,23 +4582,7 @@ func (a *Agent) handBackUnsettled() { // line [Agent.handBackUnsettled] draws for the same reason: a node the model // actually settled has published its own landing already. func (g *TaskGraph) handBackOnLoad() []*TaskNode { - if g == nil { - return nil - } - var handed []*TaskNode - g.mu.Lock() - for _, id := range g.order { - node := g.nodes[id] - if node == nil || node.decider != TaskAskOwnerModel { - continue - } - node.givesBackLocked() - if node.state == TaskUnverified { - handed = append(handed, node) - } - } - g.mu.Unlock() - return handed + return g.handBackHeld(nil) } // readsTheDecisionLocked reports that THIS agent's turn is the turn one node's @@ -4984,7 +5029,7 @@ func (a *Agent) bubbleUnverifiedChildren(node *TaskNode) { // ([Agent.deliverTaskNote]) — so nothing is claimed or marked here. // The sentence says only what happened and whose decision it now is, so it // is its own record ([delivery.record]). - a.postTaskMessage(node, node.resultTag(), readdressedLead(node, waiting), "", nil, nil) + a.postTaskMessage(node, node.resultTag(), readdressedLead(node, waiting), "", nil, nil, false) } // park hands a RUNNING node's lane back while it waits on the work it handed diff --git a/internal/session/task_states_test.go b/internal/session/task_states_test.go index 8454db028d..a7be9dcba5 100644 --- a/internal/session/task_states_test.go +++ b/internal/session/task_states_test.go @@ -662,7 +662,7 @@ func TestAWorkerTurnDoesNotSpendADecisionNobodyHasBeenGiven(t *testing.T) { root := &TaskNode{graph: graph, id: 1, state: TaskRunning, spec: taskSpec{title: "the whole job"}} piece := &TaskNode{ graph: graph, id: 2, parent: 1, state: TaskUnverified, - decider: TaskAskOwnerModel, handNote: decisionNoteOwed, + decider: TaskAskOwnerModel, noteOwed: true, spec: taskSpec{title: "a piece of it"}, } graph.nodes[1], graph.nodes[2] = root, piece @@ -675,7 +675,7 @@ func TestAWorkerTurnDoesNotSpendADecisionNobodyHasBeenGiven(t *testing.T) { // AND IT IS NOT A DOOR THAT STAYS SHUT. Once a request has carried the note, // the next turn to end is the window closing, exactly as it always was. graph.mu.Lock() - piece.handNote = decisionNoteRead + piece.noteOwed = false graph.mu.Unlock() worker.handBackUnsettled() if piece.decider != TaskAskOwnerPerson { @@ -702,8 +702,8 @@ func TestALandingHandedOverByPolicyIsNotSpentBeforeItsNoteIsCarried(t *testing.T // AND THE REQUEST IS WHAT MAKES IT A QUESTION THE MODEL HAS. agent.decisionNotesOwed = []uint64{node.id} agent.decisionNotesCarried() - if got := decisionNoteOf(node); got != decisionNoteRead { - t.Fatalf("the note the request carried is %v, want it read", got) + if decisionNoteOwed(node) { + t.Fatal("the node still owes a question after the request carried its note") } agent.handBackUnsettled() if node.decider != TaskAskOwnerPerson { @@ -724,7 +724,7 @@ func TestWhatNoTurnWillEverCarryComesBackToThePerson(t *testing.T) { node.holdsDecision(TaskAskOwnerModel) agent.decisionNotesOwed = []uint64{node.id} // The turn that would have carried it has ended with nothing following. - agent.handBackUnreadNotes() + agent.handBackUnread() if node.decider != TaskAskOwnerPerson { t.Fatalf("a press nothing was ever going to carry was left with the model (%q)", node.decider) } diff --git a/internal/session/tools_tasks_test.go b/internal/session/tools_tasks_test.go index 36dc279135..da5dc895ee 100644 --- a/internal/session/tools_tasks_test.go +++ b/internal/session/tools_tasks_test.go @@ -717,8 +717,8 @@ func TestAPressNoRequestCarriedKeepsTheDecisionForTheTurnThatReadsIt(t *testing. if err := agent.HandUnverifiedToModel(id); err != nil { t.Fatalf("handing the decision over: %v", err) } - if note := decisionNoteOf(graph.node(id)); note != decisionNoteOwed { - t.Fatalf("the press marked the node %v, want the note owed and unread", note) + if !decisionNoteOwed(graph.node(id)) { + t.Fatal("the press left the node owing the model no question") } // The floor, run by hand at the instant the turn in flight would run it. @@ -773,12 +773,12 @@ func TestAnInterruptedTurnHandsBackThePressItNeverCarried(t *testing.T) { }) } -// decisionNoteOf reads how far one node's decision note has got, under the lock +// decisionNoteOwed reads whether a node is still owed a question, under the lock // every other reader of a node's fields takes. -func decisionNoteOf(node *TaskNode) decisionNote { +func decisionNoteOwed(node *TaskNode) bool { node.graph.mu.Lock() defer node.graph.mu.Unlock() - return node.handNote + return node.noteOwed } // TestAnsweringLetAforgeDecideTwiceStandsRatherThanRefusing is the same press From 0e31d67cca41a587e965c7c7ce8cb08af4bafd5f Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sat, 12 Sep 2026 11:38:55 -0400 Subject: [PATCH 12/14] tests: the real roads, and the fixture that already existed The hard stop and the worker turn are both driven now rather than asserted at the door: Abandon moves the session past the turn and the goroutine under it cleans up nothing, and a worker's turn ends while its runner is about to carry what the drain took. heldTurn was already in this package; modelHoldsTheTurn was a second copy of it. The step lives on the fixture now, its two ends are said once, and the armer yields rather than spinning. The beat join takes the same grace every other join in Close takes, and writeIfOpen's comment says what it is the door FOR rather than claiming every closed check in the package. Co-Authored-By: Claude Opus 5 (1M context) --- internal/session/agent.go | 13 ++- internal/session/phasenews.go | 17 ++- internal/session/phasenews_test.go | 4 + internal/session/steer_test.go | 33 ++++++ internal/session/tools_tasks_test.go | 157 ++++++++++++++++++--------- 5 files changed, 166 insertions(+), 58 deletions(-) diff --git a/internal/session/agent.go b/internal/session/agent.go index 9566cfb3e9..d2b45c4b9f 100644 --- a/internal/session/agent.go +++ b/internal/session/agent.go @@ -3017,9 +3017,16 @@ func (a *Agent) takeReplyTags() []TaskReplyTag { return tags } -// writeIfOpen is THE ONE DOOR for anything a reading beside the work leaves -// behind, and it answers one question: is there still a conversation to leave it -// in. +// writeIfOpen is THE ONE DOOR FOR A READING BESIDE THE WORK, and it answers one +// question: is there still a conversation to leave this in. +// +// IT IS NOT EVERY `closed` CHECK IN THIS PACKAGE, and does not try to be. A +// door a caller is waiting at answers for itself — an enqueue that must tell its +// caller the note was dropped, a submit that must refuse in words — and reads +// `closed` inline under the same lock as the rest of its work. What comes +// through here is the other shape: a reading nobody is waiting for, which +// started while the conversation was open and comes back with something to leave +// behind. // // NOTHING WRITES INTO A CONVERSATION THAT HAS CLOSED. After [Agent.Close] the // queues are drained and nobody will read another word, the folder behind the diff --git a/internal/session/phasenews.go b/internal/session/phasenews.go index c9f78dee63..baeaeaa1f2 100644 --- a/internal/session/phasenews.go +++ b/internal/session/phasenews.go @@ -555,7 +555,22 @@ func (a *Agent) beatHeldPhase(stop chan struct{}) { // is already shut ([Agent.writeIfOpen]), so nothing can be added to the count // while it waits. func (a *Agent) waitForPhaseBeats() { - a.phaseBeats.Wait() + // AND THE JOIN IS GRACED LIKE EVERY OTHER ONE THE QUIT MAKES + // ([Agent.waitForTitle]). A beat's whole remaining life once its stop is + // closed is one `select` and a return, so this grace has never expired and + // should not; what it buys is the law the close keeps everywhere else — + // nothing the quit waits for can hold the person's own exit open. + settled := make(chan struct{}) + go func() { + a.phaseBeats.Wait() + close(settled) + }() + timer := time.NewTimer(closeGrace) + defer timer.Stop() + select { + case <-settled: + case <-timer.C: + } } // sayHeldPhaseAgain re-says the phase this beat belongs to and reports whether diff --git a/internal/session/phasenews_test.go b/internal/session/phasenews_test.go index 3fc11463b2..2db5669b08 100644 --- a/internal/session/phasenews_test.go +++ b/internal/session/phasenews_test.go @@ -4,6 +4,7 @@ import ( "context" "os" "path/filepath" + "runtime" "strings" "sync" "testing" @@ -522,6 +523,9 @@ func TestNothingIsSaidAfterTheQuitEvenWhileStagesAreBeingArmed(t *testing.T) { default: } agent.tellPhase(provider.PhaseRunning, "a tool", time.Now()) + // A yield rather than a spin: the point is to be arming ACROSS the + // quit, not to starve the goroutine running it. + runtime.Gosched() } }() // The stages are really being said before the quit starts, or this would be a diff --git a/internal/session/steer_test.go b/internal/session/steer_test.go index d6420e36af..d90fe8d9d3 100644 --- a/internal/session/steer_test.go +++ b/internal/session/steer_test.go @@ -40,12 +40,45 @@ import ( type heldTurn struct { entered chan struct{} release chan struct{} + arrived sync.Once + let1 sync.Once } func newHeldTurn() *heldTurn { return &heldTurn{entered: make(chan struct{}), release: make(chan struct{})} } +// step is the parked call itself, and it is HOW A TEST SAYS "WHILE THE MODEL IS +// HOLDING THIS" rather than racing it. +// +// Several doors in this package wake a turn when the session is idle — a press +// that hands a decision over, a landing's own note (agent.go's +// [Agent.enqueueSteering]) — and against a scripted model the whole life of that +// turn is a few microseconds: it drains the steering queue before its request +// and the floor gives every model-held landing back after it. A test that +// pressed and then read either fact was reading whichever of those the turn had +// got to. Parking the model call freezes exactly that window: no queue is +// drained while a turn is inside its request, and no second turn starts while +// one is running. +func (h *heldTurn) step(answer string) step { + return func(ctx context.Context, _ []ai.Message) (*ai.Response, error) { + h.arrive() + select { + case <-h.release: + case <-ctx.Done(): + // A cancelled turn is a stop or the session closing on top of the + // park, which several of these tests do on purpose. + return nil, ctx.Err() + } + return textResponse(answer), nil + } +} + +// arrive and let are the two ends of that park, said once however many times a +// scripted turn comes back through the step or a cleanup lets it go. +func (h *heldTurn) arrive() { h.arrived.Do(func() { close(h.entered) }) } +func (h *heldTurn) let() { h.let1.Do(func() { close(h.release) }) } + func (h *heldTurn) wait(t *testing.T) { t.Helper() select { diff --git a/internal/session/tools_tasks_test.go b/internal/session/tools_tasks_test.go index da5dc895ee..3b812b69d0 100644 --- a/internal/session/tools_tasks_test.go +++ b/internal/session/tools_tasks_test.go @@ -11,18 +11,14 @@ package session // took one would be testing the runner again. import ( - "context" "encoding/json" "errors" "fmt" "os" "path/filepath" "strings" - "sync" "testing" "time" - - "github.com/Agent-Field/agentfield/sdk/go/ai" ) // A MODEL CAN LOOK AT WORK THAT IS STILL GOING. It names the task, and what @@ -605,45 +601,13 @@ func TestHandingAYourCallToTheModelAndItsResolveAreOneRoad(t *testing.T) { } } -// modelHoldsTheTurn parks the model inside its first call and hands back the two -// facts a test needs about that window: that it has begun, and the way to end -// it. -// -// IT IS HOW A TEST SAYS "WHILE THE MODEL IS HOLDING THIS", and it is the -// opposite of a tolerance. Handing a decision over WAKES A TURN on an idle -// session (agent.go's [Agent.enqueueSteering]), and against a scripted model the -// whole life of that turn is a few microseconds: it drains the steering queue -// before its request and the floor hands every model-held landing back after it -// (task_run.go's [Agent.handBackUnsettled]). A test that pressed and then read -// either fact was reading whichever of those the turn had got to — measured on -// the Spark, three runs in thirty under -race, as a steering queue that shrank -// by two between two reads. Parking the model call freezes exactly the window -// these tests are about: no queue is drained while a turn is inside its request, -// and no second turn starts while one is running. -func modelHoldsTheTurn() (held step, entered <-chan struct{}, release func()) { - arrived, let := make(chan struct{}), make(chan struct{}) - var once, letGo sync.Once - held = func(ctx context.Context, _ []ai.Message) (*ai.Response, error) { - once.Do(func() { close(arrived) }) - select { - case <-let: - case <-ctx.Done(): - // A cancelled turn is the session closing on top of the park, which - // every one of these tests does at cleanup. - return nil, ctx.Err() - } - return textResponse("noted"), nil - } - return held, arrived, func() { letGo.Do(func() { close(let) }) } -} - // AND A SECOND PRESS IS NOT A SECOND HAND-OVER. Pressing "let aforge decide" // twice sent the model two identical lines about one decision it was already // holding; the answer now is the plain fact. func TestHandingTheSameDecisionOverTwiceSaysItIsAlreadyHandedOver(t *testing.T) { - held, entered, release := modelHoldsTheTurn() - t.Cleanup(release) - agent, _ := newTestAgent(t, &scriptedCompleter{steps: []step{held}}, func(config *Config) { + parked := newHeldTurn() + t.Cleanup(parked.let) + agent, _ := newTestAgent(t, &scriptedCompleter{steps: []step{parked.step("noted")}}, func(config *Config) { config.AskConsent = true }) graph := stubbedGraph(agent, func(node *TaskNode) { @@ -659,7 +623,7 @@ func TestHandingTheSameDecisionOverTwiceSaysItIsAlreadyHandedOver(t *testing.T) } // The press has woken a turn; everything below is asserted while that turn is // parked in its request and nothing can drain or hand anything back. - waitUntilClosed(t, "the model to be holding the turn", entered, 10*time.Second) + parked.wait(t) said := len(steeringQueue(agent)) err := agent.HandUnverifiedToModel(id) if err == nil { @@ -696,9 +660,9 @@ func TestHandingTheSameDecisionOverTwiceSaysItIsAlreadyHandedOver(t *testing.T) // put the chips back on the card within milliseconds of the press and then told // the model to decide a question the person was holding again. func TestAPressNoRequestCarriedKeepsTheDecisionForTheTurnThatReadsIt(t *testing.T) { - held, entered, release := modelHoldsTheTurn() - t.Cleanup(release) - agent, _ := newTestAgent(t, &scriptedCompleter{steps: []step{held}}, func(config *Config) { + parked := newHeldTurn() + t.Cleanup(parked.let) + agent, _ := newTestAgent(t, &scriptedCompleter{steps: []step{parked.step("noted")}}, func(config *Config) { config.AskConsent = true }) graph := stubbedGraph(agent, func(node *TaskNode) { @@ -709,7 +673,7 @@ func TestAPressNoRequestCarriedKeepsTheDecisionForTheTurnThatReadsIt(t *testing. // request has gone out, so nothing it does now will carry a line queued after // this point. events := mustSubmit(t, agent, "keep thinking") - waitUntilClosed(t, "the model to be holding the turn", entered, 10*time.Second) + parked.wait(t) id := graph.reserve() graph.admit(id, taskSpec{title: "Hidden rental digs", named: true, brief: "b", acceptance: "a"}) @@ -730,7 +694,7 @@ func TestAPressNoRequestCarriedKeepsTheDecisionForTheTurnThatReadsIt(t *testing. // AND THE FLOOR STILL FALLS. Once the woken turn has put the note in front of // the model, the end of THAT turn hands the question back exactly as it // always did — the window moved to the turn that asked, it did not go away. - release() + parked.let() collect(t, events) waitFor(t, "the turn that read the note to hand the decision back", func() bool { node := agent.taskNode(id) @@ -746,9 +710,9 @@ func TestAPressNoRequestCarriedKeepsTheDecisionForTheTurnThatReadsIt(t *testing. // `aforge is deciding` until the person's NEXT turn, and home stopped counting // it as something needing them (taskstatus.go). func TestAnInterruptedTurnHandsBackThePressItNeverCarried(t *testing.T) { - held, entered, release := modelHoldsTheTurn() - t.Cleanup(release) - agent, _ := newTestAgent(t, &scriptedCompleter{steps: []step{held}}, func(config *Config) { + parked := newHeldTurn() + t.Cleanup(parked.let) + agent, _ := newTestAgent(t, &scriptedCompleter{steps: []step{parked.step("noted")}}, func(config *Config) { config.AskConsent = true }) graph := stubbedGraph(agent, func(node *TaskNode) { @@ -756,7 +720,7 @@ func TestAnInterruptedTurnHandsBackThePressItNeverCarried(t *testing.T) { node.graph.complete(node, TaskUnverified) }) events := mustSubmit(t, agent, "keep thinking") - waitUntilClosed(t, "the model to be holding the turn", entered, 10*time.Second) + parked.wait(t) id := graph.reserve() graph.admit(id, taskSpec{title: "Hidden rental digs", named: true, brief: "b", acceptance: "a"}) @@ -773,6 +737,91 @@ func TestAnInterruptedTurnHandsBackThePressItNeverCarried(t *testing.T) { }) } +// AND A HARD STOP IS THE SAME LAW AGAIN. [Agent.Abandon] is what a second stop +// press reaches when the first one did not land (internal/tui3's app.go): it +// moves the session past the turn and the goroutine still running under it +// cleans up nothing, so the ordinary ending's own hand-back cannot run. Until +// this, the card stayed `aforge is deciding` on work nobody was deciding and the +// next press answered `already handed to aforge`, which on v3 has no way back. +func TestAnAbandonedTurnHandsBackThePressItNeverCarried(t *testing.T) { + parked := newHeldTurn() + t.Cleanup(parked.let) + agent, _ := newTestAgent(t, &scriptedCompleter{steps: []step{parked.step("noted")}}, func(config *Config) { + config.AskConsent = true + }) + graph := stubbedGraph(agent, func(node *TaskNode) { + node.finish("UNVERIFIED — the auditor answered neither VERIFIED nor REFUTED", nil, "", "") + node.graph.complete(node, TaskUnverified) + }) + events := mustSubmit(t, agent, "keep thinking") + parked.wait(t) + + id := graph.reserve() + graph.admit(id, taskSpec{title: "Hidden rental digs", named: true, brief: "b", acceptance: "a"}) + waitDoneNode(t, graph.node(id)) + if err := agent.HandUnverifiedToModel(id); err != nil { + t.Fatalf("handing the decision over: %v", err) + } + + if _, letGo := agent.Abandon(AbandonStopTimeout); !letGo { + t.Fatal("the turn was not there to abandon") + } + if node := agent.taskNode(id); node.decidedBy() != TaskAskOwnerPerson || node.wasHandedOver() { + t.Fatalf("the abandoned turn left the decision with the model (%q)", node.decidedBy()) + } + // AND THE PRESS CAN BE MADE AGAIN, which is the whole of what being stuck + // cost: the refusal is about who is holding the question, not a door that + // closes for good. + if err := agent.HandUnverifiedToModel(id); err != nil { + t.Fatalf("handing it over after the stop: %v", err) + } + parked.let() + collect(t, events) +} + +// AND A WORKER'S TURN ENDING IS NOT THE END OF THE ROAD. A task node's turns +// belong to the runner that drives it — nothing in this agent wakes one — and +// the runner re-enters it the moment one of its pieces reports, carrying +// whatever its last drain took. Reading "does a turn follow me" off this agent's +// own doors makes it structurally false for every worker turn, and the hand-back +// then races the resume that carries the note: two hands on one question. +func TestAWorkerTurnEndingDoesNotHandBackWhatItsRunnerWillCarry(t *testing.T) { + conversation, _ := newTestAgent(t, &scriptedCompleter{}, nil) + graph := stubbedGraph(conversation, func(node *TaskNode) { + node.finish("UNVERIFIED — the auditor answered neither VERIFIED nor REFUTED", nil, "", "") + node.graph.complete(node, TaskUnverified) + }) + id := graph.reserve() + graph.admit(id, taskSpec{title: "a piece of it", named: true, brief: "b", acceptance: "a"}) + waitDoneNode(t, graph.node(id)) + graph.mu.Lock() + graph.node(id).parent = 7 + graph.mu.Unlock() + + parked := newHeldTurn() + t.Cleanup(parked.let) + worker, _ := newTestAgent(t, &scriptedCompleter{steps: []step{parked.step("noted")}}, func(config *Config) { + config.InTask, config.tasker, config.taskID = true, graph, 7 + }) + events := mustSubmit(t, worker, "get on with it") + parked.wait(t) + + // The question reaches this worker's queue while its one request is already + // in flight, so nothing in THIS turn will carry it: the end drain takes it + // into the transcript and the runner's next turn is what asks it. + graph.node(id).holdsDecision(TaskAskOwnerModel) + worker.enqueueHandOver(id, "the person has asked you to make this decision rather than making it themselves.") + parked.let() + collect(t, events) + + if node := graph.node(id); node.decidedBy() != TaskAskOwnerModel { + t.Fatalf("the worker's turn ending handed back a question its runner is about to ask (%q)", node.decidedBy()) + } + if !decisionNoteOwed(graph.node(id)) { + t.Fatal("the worker's turn ending spent a note no request had carried") + } +} + // decisionNoteOwed reads whether a node is still owed a question, under the lock // every other reader of a node's fields takes. func decisionNoteOwed(node *TaskNode) bool { @@ -789,11 +838,11 @@ func decisionNoteOwed(node *TaskNode) bool { // refusal would put trouble on a card whose question was answered correctly. // Every other refusal on that key is still handed back. func TestAnsweringLetAforgeDecideTwiceStandsRatherThanRefusing(t *testing.T) { - // The model holds the turn for both presses, for [modelHoldsTheTurn]'s reason: + // The model holds the turn for both presses, for [heldTurn.step]'s reason: // this door hands over through the same one and wakes the same turn. - held, entered, release := modelHoldsTheTurn() - t.Cleanup(release) - agent, _ := newTestAgent(t, &scriptedCompleter{steps: []step{held}}, func(config *Config) { + parked := newHeldTurn() + t.Cleanup(parked.let) + agent, _ := newTestAgent(t, &scriptedCompleter{steps: []step{parked.step("noted")}}, func(config *Config) { config.AskConsent = true }) graph := stubbedGraph(agent, func(node *TaskNode) { @@ -808,7 +857,7 @@ func TestAnsweringLetAforgeDecideTwiceStandsRatherThanRefusing(t *testing.T) { if err := agent.applyLanding(answer, LandingDecideKey, ""); err != nil { t.Fatalf("the first press: %v", err) } - waitUntilClosed(t, "the model to be holding the turn", entered, 10*time.Second) + parked.wait(t) said := len(steeringQueue(agent)) if err := agent.applyLanding(answer, LandingDecideKey, ""); err != nil { t.Fatalf("the second press was refused: %v", err) From 5ac0666c52488102f7a321233786f99ddb4bd476 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sat, 12 Sep 2026 11:40:10 -0400 Subject: [PATCH 13/14] manual: the new topic gets its own heading, and two pages stop denying it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The press that lands after the last question went out is its own question a person asks, in their own words, so it is its own heading — with 'you decide' beside 'let aforge decide' because people search for the chip they pressed. tasks.md and task-rooms-after-restart.md said the question never bounces back unread. That is true while a turn follows, and the opposite when one does not: a stopped turn, or one that reached no model at all, draws the chips again at once. Both pages say so now. Co-Authored-By: Claude Opus 5 (1M context) --- internal/manual/chat/task-controls.md | 37 ++++++++++++------- .../manual/chat/task-rooms-after-restart.md | 4 +- internal/manual/chat/tasks.md | 5 ++- 3 files changed, 30 insertions(+), 16 deletions(-) diff --git a/internal/manual/chat/task-controls.md b/internal/manual/chat/task-controls.md index 2bbd84e585..663d38d93c 100644 --- a/internal/manual/chat/task-controls.md +++ b/internal/manual/chat/task-controls.md @@ -75,6 +75,29 @@ and spending. A real stop never reaches the check and is never handed back for a done`, or `stopped`, or `your call`, in the words your screen is showing. Work running in **another aforge window** cannot be stopped from here — the window that owns it has to. +## I pressed let aforge decide while it was answering — you decide did nothing, the card went back to your call straight away, which turn does aforge get + +Your press puts a line on the queue, and a queue is read **between steps** — so pressing +while the answer is already streaming can mean that turn never sees it. What happens then: +the card keeps saying `aforge is deciding`, aforge starts one more turn by itself to read +the line, and **that** turn is the one it has to answer in. The card comes back to you at +the end of it, whether or not aforge spent a verb on the decision. + +It used to come back within a moment of your press instead — before aforge had been asked +anything — and aforge was then handed a decision you were holding again, which is two of +you answering one question. + +**A turn you stop is the end of it.** Press `esc`, press stop twice, or have the turn fail, +and nothing is coming that would put the question in front of aforge: the card draws its +chips again straight away and the task counts on your home page as something needing you. +Press it again when you want it asked. The same is true of a turn that ends having reached +no model at all — a run of machines that all refused — because nothing was asked there +either. + +**A task aforge is working on is different.** Its turns belong to the run that drives it and +another one always follows, so a question handed over inside a task waits for the next turn +of that run rather than coming back to you. + ## I told aforge to decide and it still says your call — the model says there is no graph left, the task graph expired, I pressed let aforge decide twice Pressing `let aforge decide` on a your-call card hands **that one card** to the model @@ -97,20 +120,6 @@ second line about the same decision — and it is not shown to you as trouble, b state it asks for is the state that already holds: the reason row goes on reading `aforge is deciding`. If you want it back, press `take it back` and the answers return to you. -**If the card stayed `your call` and nothing above fits, the press landed after aforge's -last question went out.** Your press puts a line on the queue, and a queue is read between -steps — so pressing while the answer is already streaming can mean that turn never sees it. -What happens then: the card keeps saying `aforge is deciding`, aforge starts one more turn -by itself to read the line, and that turn is the one it has to answer in. The card comes -back to you at the end of it, whether or not aforge spent a verb on it. It used to come -back within a moment of your press instead, and the model was then handed a decision you -were holding again — two of you answering one question. - -**A turn you stop is the end of it.** If you press `esc` — or the turn fails — before the -line has been read, nothing is coming that would put the question in front of aforge, so -the card draws its chips again straight away and the task counts on your home page as -something needing you. Press `let aforge decide` again when you want it asked. - **A recovered task is this conversation's task.** Reopening a conversation brings its graph back — `recovered task graph: 1 done · 1 your call` — and everything the card offers works on those tasks, the model's answers included. diff --git a/internal/manual/chat/task-rooms-after-restart.md b/internal/manual/chat/task-rooms-after-restart.md index 08ec4abbde..2a6f84e888 100644 --- a/internal/manual/chat/task-rooms-after-restart.md +++ b/internal/manual/chat/task-rooms-after-restart.md @@ -175,7 +175,9 @@ say it may still say. if aforge's turn ends with the question still unanswered, the decision moves back to you and the card draws its chips, whether or not you noticed it had been handed over. The turn that counts is the one that actually read the question — hand it over while aforge is -already mid-answer and it waits for the next turn rather than bouncing back unread. Where +already mid-answer and it waits for the next turn rather than bouncing back unread, unless +that turn was stopped or failed before any model saw it, in which case nothing is coming to +ask and the chips return at once. Where aforge's last message asked you something about that task, those chips are the answer surface for that question — its words above, the chips below, one ask. diff --git a/internal/manual/chat/tasks.md b/internal/manual/chat/tasks.md index 8d290de21a..d485ffa082 100644 --- a/internal/manual/chat/tasks.md +++ b/internal/manual/chat/tasks.md @@ -4871,7 +4871,10 @@ chips under them, one ask rather than two. aforge is already mid-answer and the note can arrive too late for that turn to see — so the card keeps saying `aforge is deciding` and the question waits for the next turn, which is usually the one aforge starts by itself a moment later. It does not bounce straight back at -you unread, and it does not arrive twice. +you unread, and it does not arrive twice. **Unless nothing follows:** a turn you stopped, or +one that failed before reaching a model, is the end of the road — nothing is coming that +would ask the question, so the card draws its chips again at once and the task counts on +home as something needing you (*Task controls*). **And closing aforge ends the turn too.** If you quit, crash or come back to the conversation later, a task aforge was deciding is yours again the moment the conversation From 637190ce92f3aa6ad5ce861ebbec16d241387be0 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sat, 12 Sep 2026 11:40:23 -0400 Subject: [PATCH 14/14] changelog: what the review moved (#999) Co-Authored-By: Claude Opus 5 (1M context) --- docs/changes/unreleased/999-handover-twice-race.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/changes/unreleased/999-handover-twice-race.md b/docs/changes/unreleased/999-handover-twice-race.md index 1c709d2647..3fcf1b0a40 100644 --- a/docs/changes/unreleased/999-handover-twice-race.md +++ b/docs/changes/unreleased/999-handover-twice-race.md @@ -4,10 +4,10 @@ title: a press the turn never read is not a press that turn can spend pr: 999 surface: [engine, chat] invalidates: - - "\"A task never stays unowned past the end of a turn\" was read as the end of WHICHEVER turn ends next. It is the end of the turn that actually put the question in front of the model. A note lands on the steering queue and a queue is read at a STEP boundary, so a card pressed while the turn's last request is already out reaches no request at all — it is recorded at that turn's end drain and answered by the turn that wakes for it. `Agent.handBackUnsettled` took the hold back at the end of the first turn anyway, which put the chips back on the card within milliseconds of the press and then handed the model a decision the person was holding again. How far one node's note has got is now a fact ON THE NODE (`TaskNode.handNote`, `decisionNote`): owed until a request carries it, read afterwards. It is on the node and not on a queue because the floor runs at the end of EVERY agent's turn and a worker reads its own children's decisions — a per-queue fact let a worker's turn spend a press made, and still waiting, somewhere else." + - "\"A task never stays unowned past the end of a turn\" was read as the end of WHICHEVER turn ends next. It is the end of the turn that actually put the question in front of the model. A note lands on the steering queue and a queue is read at a STEP boundary, so a card pressed while the turn's last request is already out reaches no request at all — it is recorded at that turn's end drain and answered by the turn that wakes for it. `Agent.handBackUnsettled` took the hold back at the end of the first turn anyway, which put the chips back on the card within milliseconds of the press and then handed the model a decision the person was holding again. Whether a node is still owed a question is now a fact ON THE NODE (`TaskNode.noteOwed`), one boolean and owed again every time a fresh note is sent — written BEFORE the note goes out and taken back if nobody takes it, because a mark written after the send is one a reader that drains and requests first has already cleared, which pinned the node owed for the life of the process. It is on the node and not on a queue because the floor runs at the end of EVERY agent's turn and a worker reads its own children's decisions — a per-queue fact let a worker's turn spend a press made, and still waiting, somewhere else." - "`task.settle = auto` is the same question through another door and had the same defect: the landing is the model's by policy and the landing note is the whole of the asking, so a turn ending between the two spent it. The note carries its landing now (`userMessage.handsOver`, set by both the press and `Agent.postTaskMessage`), and the policy road's mark is written where a reader actually takes the note — a landing folded into a parent's report never gets a note at all, and marking at the policy would have held that card open forever." - - "A decision the model was given is marked read at THE REQUEST and not at the drain (`Agent.decisionNotesCarried`, called from the one line in `loop.go` where a request goes out). A note drained at a step boundary whose turn is then cut — an interrupt, an oversize refusal, a checkpoint that ends the turn — has left the queue and reached nobody, and a mark at the drain would have let the floor spend a question no model read." - - "A press that no turn will ever carry now comes back at once instead of waiting for the person's next turn. A turn that ends with nothing following it — interrupted, failed, or simply owing nobody a word — hands back what that agent owed (`Agent.handBackUnreadNotes`), because there is no longer a turn coming to ask. Until this, such a node stayed model-owned and home stopped counting it as something needing a person (`taskstatus.go`). `internal/manual/chat`'s `task-controls.md` and `tasks.md` said the old thing and now say which turn counts and what a stopped turn does." + - "A decision the model was given is marked read when a model ANSWERS (`Agent.decisionNotesCarried`, called where the response returns) and not at the drain or at the sending: a request that reaches no model — every machine refusing, a cut context — asked nobody anything. A note drained at a step boundary whose turn is then cut — an interrupt, an oversize refusal, a checkpoint that ends the turn — has left the queue and reached nobody, and a mark at the drain would have let the floor spend a question no model read." + - "A press that no turn will ever carry now comes back at once instead of waiting for the person's next turn. A turn that ends with nothing following it — interrupted, failed, or simply owing nobody a word — hands back what that agent owed (`Agent.handBackUnread`), and so does `Agent.Abandon`, the hard stop whose own turn goroutine cleans up nothing. A TASK WORKER'S turn ending is never the last one — its turns belong to the runner that re-enters it — so a worker hands nothing back there. The three hand-back roads are one walk (`TaskGraph.handBackHeld`) with two questions: whose decisions this agent may take back, and whether a question nobody has been given is left alone; because there is no longer a turn coming to ask. Until this, such a node stayed model-owned and home stopped counting it as something needing a person (`taskstatus.go`). `internal/manual/chat`'s `task-controls.md` and `tasks.md` said the old thing and now say which turn counts and what a stopped turn does." - "`Agent.Close` let go of a steer's grace and a clock armed on a question (#963) but not of a held stage's beat, which is re-said on a ticker of its own until the stage ends (`Agent.beatHeldPhase`). A session closed mid-stage left that goroutine posting a conversation's stage after the conversation had gone — under `-race`, a live reader of this package's own `phaseHeldBeat` while the next test wrote it (#959). `Close` ends the stage properly, so a surface takes the clock down, and then WAITS for the beat: `Agent.phaseBeats` is a `sync.WaitGroup` like `memoryJobs`, and the Add is taken inside the door that refuses an arming on a closed session, so nothing can join the count after the quit has begun waiting on it. The join is the quit's door alone; ending a stage on a turn's path still cancels and carries on." - "There is now ONE door for anything a reading beside the work leaves behind: `Agent.writeIfOpen` (agent.go) checks `closed` and performs the write in the SAME hold of `a.mu`, because a reading that asks and then writes is a reading `Close` fits inside. `Agent.refreshMemory` landed its routed block into a closed conversation's transcript with no check at all and goes through it now, and so does the phase heart's arming. #997's `refreshElsewhere` is the third caller and takes the door in this change — its own comment said the region would become a call to it — keeping its earlier early-out, which is a decision about whether to READ at all rather than about the write it owes." - "`internal/session`'s hand-over fixtures asserted about the window in which the model is holding a decision by pressing and then reading. There is no such window against a scripted model: the press wakes a turn whose whole life is microseconds. `modelHoldsTheTurn` parks the model inside its call — no queue drains while a turn is inside a request, and no second turn starts while one is running — so the window is stated rather than raced. `TestHandingTheSameDecisionOverTwiceSaysItIsAlreadyHandedOver` failed 3 runs in 30 under `-race` on plain `dev` @ 82624af79 and is one of #959's faces; it is not a known red and never was."