Merge pull request #5303 from myipanta/fix/5286-interrupted-task-re-execution

fix(control): strip incomplete turn messages after user cancel (#5286)

Author: SivanCola

internal/agent/agent.go

@@ -1086,6 +1086,13 @@ func (a *Agent) emitTodoState(todos []evidence.TodoItem, itemIndex int) {
 	a.sink.Emit(event.Event{Kind: event.ToolResult, Tool: t})
 }
 
+// RebuildTodoState re-derives canonical task state from the current session
+// transcript. Call after externally truncating the session (e.g. after a
+// user-cancel strip) so Agent.todoState stays consistent with the messages.
+func (a *Agent) RebuildTodoState() {
+	a.rebuildTodoState(a.Session().Snapshot())
+}
+
 // rebuildTodoState reconstructs the canonical task list from a transcript: the
 // latest successful todo_write is the base, then every complete_step after it
 // advances an item. Deterministic from persisted messages, so it survives a

internal/control/controller.go

@@ -2116,6 +2116,41 @@ func (c *Controller) messageCount() int {
 	return len(c.executor.Session().Snapshot())
 }
 
+// stripTurnMessagesAfter truncates the executor's session to keep only messages
+// before the given index, discarding an incomplete turn (the user prompt plus
+// every assistant / tool message that followed).  It is called when the user
+// explicitly cancels a turn so the next prompt starts clean — the model won't
+// see leftover in-progress todo items or partial tool calls and re-execute
+// interrupted work.
+func (c *Controller) stripTurnMessagesAfter(idx int) {
+	if c.executor == nil {
+		return
+	}
+	msgs := c.executor.Session().Snapshot()
+	if len(msgs) <= idx {
+		return
+	}
+	c.executor.Session().Replace(msgs[:idx])
+	// Rebuild canonical todo state from the truncated transcript so
+	// Controller.Todos(), goal readiness, and the task panel no longer see
+	// the in_progress items written by the cancelled turn.
+	c.executor.RebuildTodoState()
+	// The mid-turn autosave may have already written a partial transcript to
+	// disk.  snapshotActivityIfChanged skips the write when messageCount()
+	// returns to startMessages, so force a flush here to overwrite the stale
+	// file.  We call Session.Save directly to cover the edge case where the
+	// strip leaves only a system message (HasContent() == false), which would
+	// cause snapshot() to return early without writing.
+	c.mu.Lock()
+	path := c.sessionPath
+	c.mu.Unlock()
+	if path != "" {
+		if err := c.executor.Session().Save(path); err != nil {
+			slog.Warn("controller: post-cancel transcript flush", "err", err)
+		}
+	}
+}
+
 func (c *Controller) snapshotActivityIfChanged(startMessages int) {
 	if c.messageCount() <= startMessages {
 		return

internal/control/turn_orchestrator.go

@@ -2,6 +2,7 @@ package control
 
 import (
 	"context"
+	"errors"
 
 	"reasonix/internal/agent"
 	"reasonix/internal/jobs"
@@ -50,6 +51,16 @@ func (o *turnOrchestrator) runTurnWithRawDisplay(ctx context.Context, input, raw
 		defer func() { c.hooks.Stop(context.Background(), lastAssistantText(c.History()), turn) }()
 	}
 	if err := c.runner.Run(ctx, input); err != nil {
+		// When the user explicitly cancels (Ctrl+C), the incomplete turn's
+		// assistant messages and tool results are already saved to the
+		// session.  If they stay, the next turn's model sees leftover
+		// in-progress todo items and partial tool calls and may re-execute
+		// the interrupted work (the next user message looks like a
+		// continuation instead of a fresh task).  Strip the turn so the
+		// next prompt starts clean.
+		if errors.Is(err, context.Canceled) && c.CancelRequested() {
+			c.stripTurnMessagesAfter(startMessages)
+		}
 		return err
 	}
 	c.mu.Lock()
@@ -80,6 +91,9 @@ func (o *turnOrchestrator) runTurnWithRawDisplay(ctx context.Context, input, raw
 	c.approval.setPlanAutoApprove(true)
 	defer c.approval.setPlanAutoApprove(false)
 	if err := c.runner.Run(ctx, c.ComposeSynthetic(planApprovedMessage)); err != nil {
+		if errors.Is(err, context.Canceled) && c.CancelRequested() {
+			c.stripTurnMessagesAfter(execStart)
+		}
 		return err
 	}
 	if todoArgs != "" && !c.hasTodoUpdateSince(execStart) {

internal/control/turn_orchestrator_test.go

@@ -3,13 +3,15 @@ package control
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"os"
 	"path/filepath"
 	"strings"
 	"testing"
 
 	"reasonix/internal/agent"
 	"reasonix/internal/event"
+	"reasonix/internal/evidence"
 	"reasonix/internal/hook"
 	"reasonix/internal/provider"
 	"reasonix/internal/tool"
@@ -301,3 +303,137 @@ func TestTurnOrchestratorStopHookCancelledContext(t *testing.T) {
 		t.Fatalf("Stop hooks called = %d; want 1", stopCalls)
 	}
 }
+
+// TestTurnOrchestratorCancelStripsIncompleteTurn verifies that when the user
+// explicitly cancels a turn (Ctrl+C), the incomplete turn's messages are
+// stripped from the session so the next turn starts clean.  Without this, the
+// model sees leftover in-progress todo items and partial tool calls and may
+// re-execute the interrupted work.  See #5286.
+func TestTurnOrchestratorCancelStripsIncompleteTurn(t *testing.T) {
+	sess := agent.NewSession("you are a helpful agent")
+	// Pre-populate with a few messages from an earlier turn.
+	sess.Add(provider.Message{Role: provider.RoleUser, Content: "previous work"})
+	sess.Add(provider.Message{Role: provider.RoleAssistant, Content: "done"})
+	preCount := len(sess.Messages)
+
+	// runner that simulates a cancelled turn: it adds the user message plus
+	// some tool-call garbage the real agent would leave behind, then returns
+	// context.Canceled.
+	runner := &cancelStrippingRunner{
+		session: sess,
+		add: []provider.Message{
+			{Role: provider.RoleAssistant, Content: "let me do that", ToolCalls: []provider.ToolCall{
+				{ID: "c1", Name: "todo_write", Arguments: `{"todos":[{"content":"add abc","status":"in_progress"}]}`},
+			}},
+			{Role: provider.RoleTool, Content: "Todos updated: 1 total — 0 completed, 1 in_progress, 0 pending.", ToolCallID: "c1", Name: "todo_write"},
+		},
+		err: context.Canceled,
+	}
+
+	ex := agent.New(nil, nil, sess, agent.Options{}, event.Discard)
+	c := New(Options{Runner: runner, Executor: ex})
+	// Simulate a user-initiated cancel: set the cancelling flag.
+	c.mu.Lock()
+	c.canceling = true
+	c.mu.Unlock()
+
+	// Pre-seed todoState as if a successful todo_write from the cancelled turn
+	// had already updated it — this is the state the runner leaves behind before
+	// returning context.Canceled, and what RebuildTodoState must clear.
+	ex.ReplaceTodoState([]evidence.TodoItem{{Content: "add abc", Status: "in_progress"}})
+
+	o := newTurnOrchestrator(c)
+	err := o.runTurnWithRawDisplay(context.Background(), "add config file abc", "add config file abc", "")
+	if !errors.Is(err, context.Canceled) {
+		t.Fatalf("expected context.Canceled, got %v", err)
+	}
+
+	// The incomplete turn (user prompt + assistant + tool result) must be
+	// stripped; the session must only contain the pre-turn messages.
+	msgs := sess.Messages
+	if len(msgs) != preCount {
+		t.Fatalf("session messages after cancel = %d, want pre-turn count %d: %+v", len(msgs), preCount, msgs)
+	}
+
+	// todoState must also be reset: the in_progress todo written by the
+	// cancelled turn must not survive the strip.
+	if todos := c.Todos(); len(todos) != 0 {
+		t.Fatalf("Todos() after cancel = %v, want empty — cancelled todo_write leaked into canonical state", todos)
+	}
+}
+
+// TestTurnOrchestratorCancelFlushesCleanTranscriptToDisk verifies that after a
+// user-cancel strip the cleaned transcript is written to disk, so a restart or
+// session resume does not reload the partial turn from a stale mid-turn
+// autosave.  See #5286.
+func TestTurnOrchestratorCancelFlushesCleanTranscriptToDisk(t *testing.T) {
+	sess := agent.NewSession("system")
+	sess.Add(provider.Message{Role: provider.RoleUser, Content: "earlier turn"})
+	sess.Add(provider.Message{Role: provider.RoleAssistant, Content: "done"})
+	// Count only non-system messages; the system prompt is not written to the
+	// .jsonl by Session.Save (it is reconstructed from the session options).
+	wantNonSystem := 0
+	for _, m := range sess.Messages {
+		if m.Role != provider.RoleSystem {
+			wantNonSystem++
+		}
+	}
+
+	runner := &cancelStrippingRunner{
+		session: sess,
+		add: []provider.Message{
+			{Role: provider.RoleAssistant, Content: "working…", ToolCalls: []provider.ToolCall{
+				{ID: "d1", Name: "todo_write", Arguments: `{"todos":[{"content":"task","status":"in_progress"}]}`},
+			}},
+			{Role: provider.RoleTool, Content: "Todos updated.", ToolCallID: "d1", Name: "todo_write"},
+		},
+		err: context.Canceled,
+	}
+
+	sessionPath := agent.NewSessionPath(t.TempDir(), "test-model")
+	c := New(Options{
+		Runner:      runner,
+		Executor:    agent.New(nil, nil, sess, agent.Options{}, event.Discard),
+		SessionPath: sessionPath,
+	})
+	c.mu.Lock()
+	c.canceling = true
+	c.mu.Unlock()
+
+	o := newTurnOrchestrator(c)
+	if err := o.runTurnWithRawDisplay(context.Background(), "do something", "do something", ""); !errors.Is(err, context.Canceled) {
+		t.Fatalf("expected context.Canceled, got %v", err)
+	}
+
+	// Load the session file written after the strip and verify it contains only
+	// the pre-cancel messages — not the partial assistant/tool messages.
+	loaded, err := agent.LoadSession(sessionPath)
+	if err != nil {
+		t.Fatalf("LoadSession: %v", err)
+	}
+	nonSystem := 0
+	for _, m := range loaded.Messages {
+		if m.Role != provider.RoleSystem {
+			nonSystem++
+		}
+	}
+	if nonSystem != wantNonSystem {
+		t.Fatalf("on-disk message count (non-system) = %d, want %d — stale partial turn still on disk", nonSystem, wantNonSystem)
+	}
+}
+
+// cancelStrippingRunner adds messages to a session then returns a fixed error,
+// simulating an agent that was interrupted mid-turn.
+type cancelStrippingRunner struct {
+	session *agent.Session
+	add     []provider.Message
+	err     error
+}
+
+func (r *cancelStrippingRunner) Run(ctx context.Context, input string) error {
+	r.session.Add(provider.Message{Role: provider.RoleUser, Content: input})
+	for _, m := range r.add {
+		r.session.Add(m)
+	}
+	return r.err
+}