Skip to content

Commit 65f18ea

Browse files
committed
fix(control): reset todoState and flush transcript after cancel strip
After stripTurnMessagesAfter truncates the session transcript, two states remained inconsistent: 1. Agent.todoState was not updated. Session.Replace only swaps the message slice; it does not call rebuildTodoState. So a cancelled turn that had a successful todo_write left the in_progress item in the canonical task list, visible via Controller.Todos(), goal readiness, and the task panel on the next turn. Fix: expose Agent.RebuildTodoState() and call it after Replace. 2. A mid-turn autosave may have already written the partial transcript to disk. snapshotActivityIfChanged skips the write when messageCount() returns to startMessages (which it does after the strip), so the stale file was never overwritten. A restart or session resume would reload the partial turn. Fix: call Session.Save(path) directly after the strip to force a flush, bypassing the HasContent() guard that snapshot() uses (so the clean state is written even when the strip leaves only a system message). Tests: extend TestTurnOrchestratorCancelStripsIncompleteTurn to assert Todos() is empty after cancel, and add TestTurnOrchestratorCancelFlushesCleanTranscriptToDisk to verify the on-disk transcript matches the pre-cancel message count. Co-authored-by: SivanCola <32437197+SivanCola@users.noreply.github.com>
1 parent 66dadfc commit 65f18ea

3 files changed

Lines changed: 99 additions & 1 deletion

File tree

internal/agent/agent.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1086,6 +1086,13 @@ func (a *Agent) emitTodoState(todos []evidence.TodoItem, itemIndex int) {
10861086
a.sink.Emit(event.Event{Kind: event.ToolResult, Tool: t})
10871087
}
10881088

1089+
// RebuildTodoState re-derives canonical task state from the current session
1090+
// transcript. Call after externally truncating the session (e.g. after a
1091+
// user-cancel strip) so Agent.todoState stays consistent with the messages.
1092+
func (a *Agent) RebuildTodoState() {
1093+
a.rebuildTodoState(a.Session().Snapshot())
1094+
}
1095+
10891096
// rebuildTodoState reconstructs the canonical task list from a transcript: the
10901097
// latest successful todo_write is the base, then every complete_step after it
10911098
// advances an item. Deterministic from persisted messages, so it survives a

internal/control/controller.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2131,6 +2131,24 @@ func (c *Controller) stripTurnMessagesAfter(idx int) {
21312131
return
21322132
}
21332133
c.executor.Session().Replace(msgs[:idx])
2134+
// Rebuild canonical todo state from the truncated transcript so
2135+
// Controller.Todos(), goal readiness, and the task panel no longer see
2136+
// the in_progress items written by the cancelled turn.
2137+
c.executor.RebuildTodoState()
2138+
// The mid-turn autosave may have already written a partial transcript to
2139+
// disk. snapshotActivityIfChanged skips the write when messageCount()
2140+
// returns to startMessages, so force a flush here to overwrite the stale
2141+
// file. We call Session.Save directly to cover the edge case where the
2142+
// strip leaves only a system message (HasContent() == false), which would
2143+
// cause snapshot() to return early without writing.
2144+
c.mu.Lock()
2145+
path := c.sessionPath
2146+
c.mu.Unlock()
2147+
if path != "" {
2148+
if err := c.executor.Session().Save(path); err != nil {
2149+
slog.Warn("controller: post-cancel transcript flush", "err", err)
2150+
}
2151+
}
21342152
}
21352153

21362154
func (c *Controller) snapshotActivityIfChanged(startMessages int) {

internal/control/turn_orchestrator_test.go

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111

1212
"reasonix/internal/agent"
1313
"reasonix/internal/event"
14+
"reasonix/internal/evidence"
1415
"reasonix/internal/hook"
1516
"reasonix/internal/provider"
1617
"reasonix/internal/tool"
@@ -329,12 +330,18 @@ func TestTurnOrchestratorCancelStripsIncompleteTurn(t *testing.T) {
329330
err: context.Canceled,
330331
}
331332

332-
c := New(Options{Runner: runner, Executor: agent.New(nil, nil, sess, agent.Options{}, event.Discard)})
333+
ex := agent.New(nil, nil, sess, agent.Options{}, event.Discard)
334+
c := New(Options{Runner: runner, Executor: ex})
333335
// Simulate a user-initiated cancel: set the cancelling flag.
334336
c.mu.Lock()
335337
c.canceling = true
336338
c.mu.Unlock()
337339

340+
// Pre-seed todoState as if a successful todo_write from the cancelled turn
341+
// had already updated it — this is the state the runner leaves behind before
342+
// returning context.Canceled, and what RebuildTodoState must clear.
343+
ex.ReplaceTodoState([]evidence.TodoItem{{Content: "add abc", Status: "in_progress"}})
344+
338345
o := newTurnOrchestrator(c)
339346
err := o.runTurnWithRawDisplay(context.Background(), "add config file abc", "add config file abc", "")
340347
if !errors.Is(err, context.Canceled) {
@@ -347,6 +354,72 @@ func TestTurnOrchestratorCancelStripsIncompleteTurn(t *testing.T) {
347354
if len(msgs) != preCount {
348355
t.Fatalf("session messages after cancel = %d, want pre-turn count %d: %+v", len(msgs), preCount, msgs)
349356
}
357+
358+
// todoState must also be reset: the in_progress todo written by the
359+
// cancelled turn must not survive the strip.
360+
if todos := c.Todos(); len(todos) != 0 {
361+
t.Fatalf("Todos() after cancel = %v, want empty — cancelled todo_write leaked into canonical state", todos)
362+
}
363+
}
364+
365+
// TestTurnOrchestratorCancelFlushesCleanTranscriptToDisk verifies that after a
366+
// user-cancel strip the cleaned transcript is written to disk, so a restart or
367+
// session resume does not reload the partial turn from a stale mid-turn
368+
// autosave. See #5286.
369+
func TestTurnOrchestratorCancelFlushesCleanTranscriptToDisk(t *testing.T) {
370+
sess := agent.NewSession("system")
371+
sess.Add(provider.Message{Role: provider.RoleUser, Content: "earlier turn"})
372+
sess.Add(provider.Message{Role: provider.RoleAssistant, Content: "done"})
373+
// Count only non-system messages; the system prompt is not written to the
374+
// .jsonl by Session.Save (it is reconstructed from the session options).
375+
wantNonSystem := 0
376+
for _, m := range sess.Messages {
377+
if m.Role != provider.RoleSystem {
378+
wantNonSystem++
379+
}
380+
}
381+
382+
runner := &cancelStrippingRunner{
383+
session: sess,
384+
add: []provider.Message{
385+
{Role: provider.RoleAssistant, Content: "working…", ToolCalls: []provider.ToolCall{
386+
{ID: "d1", Name: "todo_write", Arguments: `{"todos":[{"content":"task","status":"in_progress"}]}`},
387+
}},
388+
{Role: provider.RoleTool, Content: "Todos updated.", ToolCallID: "d1", Name: "todo_write"},
389+
},
390+
err: context.Canceled,
391+
}
392+
393+
sessionPath := agent.NewSessionPath(t.TempDir(), "test-model")
394+
c := New(Options{
395+
Runner: runner,
396+
Executor: agent.New(nil, nil, sess, agent.Options{}, event.Discard),
397+
SessionPath: sessionPath,
398+
})
399+
c.mu.Lock()
400+
c.canceling = true
401+
c.mu.Unlock()
402+
403+
o := newTurnOrchestrator(c)
404+
if err := o.runTurnWithRawDisplay(context.Background(), "do something", "do something", ""); !errors.Is(err, context.Canceled) {
405+
t.Fatalf("expected context.Canceled, got %v", err)
406+
}
407+
408+
// Load the session file written after the strip and verify it contains only
409+
// the pre-cancel messages — not the partial assistant/tool messages.
410+
loaded, err := agent.LoadSession(sessionPath)
411+
if err != nil {
412+
t.Fatalf("LoadSession: %v", err)
413+
}
414+
nonSystem := 0
415+
for _, m := range loaded.Messages {
416+
if m.Role != provider.RoleSystem {
417+
nonSystem++
418+
}
419+
}
420+
if nonSystem != wantNonSystem {
421+
t.Fatalf("on-disk message count (non-system) = %d, want %d — stale partial turn still on disk", nonSystem, wantNonSystem)
422+
}
350423
}
351424

352425
// cancelStrippingRunner adds messages to a session then returns a fixed error,

0 commit comments

Comments
 (0)