Modes: a switch mid-task takes effect on the model's next turn

README.md

@@ -464,10 +464,16 @@ for the session only.
 When a plan-mode task finishes with a plan, you get a one-key **handshake**:
 **enter** to accept — switches to auto and immediately executes the plan —
 **m** to execute it in manual instead, asking before each change, or **esc** to
-keep planning (stay in plan mode; type to refine). If you switched to manual
-with shift+tab while the plan was being written, **enter** runs it in manual. Toggling the mode
-**mid-task** applies at the next turn, never mid-run (the running agent reads the
-mode live).
+keep planning (stay in plan mode; type to refine). A switch to manual that the
+model took while still writing the plan simply carries on in manual. If the
+switch came too late for that, the handshake shows and **enter** runs the plan
+in manual. Switching the mode
+**mid-task** takes effect at the start of the model's next turn, never in the
+middle of one. The log says from which step (`⇄ manual mode on — from step 3`),
+and a switch into or out of plan mode is said to the model too. If the task
+ends before the model takes another turn, the switch applies to the next task.
+A sub-agent already running keeps the plan or auto mode it started with, while
+manual applies to it at once.
 
 ## Permissions
 

cmd/agent/main.go

@@ -617,8 +617,10 @@ type app struct {
 	instrSrc        string                     // project instructions file in effect, "" if none
 	promptSrc       string                     // "built-in" or the system.md override path
 	facts           []string                   // durable project facts learned over time (per workspace)
-	planMode        bool                       // plan (propose) vs auto (execute); survives re-wire
+	planMode        atomic.Bool                // plan (propose) vs auto (execute); survives re-wire; the agent's goroutine sets it too (takeMode)
 	manual          atomic.Bool                // manual mode: every change asks (see manualStops); read by tool goroutines
+	modeMu          sync.Mutex                 // guards queuedMode
+	queuedMode      *mode                      // a mode chosen mid-task, for the agent's next turn (see takeMode)
 	readOnly        bool                       // -read-only: answer one task, change nothing, keep no session
 	goal            goalState                  // standing goal pursued by the judge loop (per workspace)
 	windowDetected  bool                       // got the real loaded context window (vs a default/guess)
@@ -741,7 +743,8 @@ func build(workspace, sessionName string, overrides []string, readOnly bool, rea
 		usageStore, _ = usage.Open("")
 	}
 
-	a := &app{cfg: cfg, workspace: cfg.Workspace, kb: kb, usage: usageStore, skills: skills, reader: reader, readOnly: readOnly, planMode: readOnly}
+	a := &app{cfg: cfg, workspace: cfg.Workspace, kb: kb, usage: usageStore, skills: skills, reader: reader, readOnly: readOnly}
+	a.planMode.Store(readOnly)
 	a.stdin = newStdinOwner(reader)                     // sole reader of `reader`'s actual bytes from here on (see stdinOwner)
 	a.approver = &stdinApprover{stdin: a.stdin, app: a} // set after: it references the app for session-allow
 	// Cancel background jobs (killing external-agent subprocesses too), shut down
@@ -938,7 +941,7 @@ func (a *app) resolveSpawn(profile, dir string) (spawnPlan, bool, config.AgentPr
 		// no read-only mode to hand them the way SetPlanMode gives an LLM sub-agent
 		// (see runSpawnPlan), so plan mode can only keep its "stays read-only"
 		// guarantee (see the subagents skill) by refusing the launch outright.
-		if a.planMode {
+		if a.planMode.Load() {
 			return spawnPlan{}, true, p, fmt.Errorf("plan mode is ON — external agent %q was NOT launched (it runs outside the sandbox with no read-only mode); list it as a step in your plan, then finish", profile)
 		}
 		// Resolve dir HERE, synchronously, same as the LLM branch below —
@@ -1037,7 +1040,7 @@ func (a *app) resolveSpawn(profile, dir string) (spawnPlan, bool, config.AgentPr
 	}
 	return spawnPlan{
 		profile: profile, provider: provider, llmCfg: llmCfg, rolePrompt: p.Prompt,
-		subReg: subReg, subPol: planPol, subWorkspace: subWorkspace, planMode: a.planMode, spawnDefault: a.cfg.Spawn.Default, outsideJail: outsideJail,
+		subReg: subReg, subPol: planPol, subWorkspace: subWorkspace, planMode: a.planMode.Load(), spawnDefault: a.cfg.Spawn.Default, outsideJail: outsideJail,
 		tracer: tracer, priceOverrides: priceOverrides, goalMaxSteps: a.cfg.GoalMaxSteps,
 		shell: tool.Shell(a.cfg.Run.Shell),
 	}, false, p, nil
@@ -3262,15 +3265,17 @@ func (a *app) wire() error {
 	a.ag.SetHistory(prior)
 	a.ag.SeedHistoryGen(priorGen)
 	a.ag.SeedFrontTrim(priorTrim)
-	a.ag.SetPlanMode(a.planMode) // carry the mode into the rebuilt agent
+	a.ag.SetPlanMode(a.planMode.Load()) // carry the mode into the rebuilt agent
 	if a.readOnly {
 		run, _ := a.readOnlyRun()
 		a.ag.SetReadOnly(run)
 	}
 	a.ag.SetBeforeTurn(a.beforeTurn) // finished background jobs fold in between steps of a running task
 	a.ag.SetSteers(a.drainSteers)    // so do /steer notes, and before a final answer is accepted
-	a.ag.SetAsides(a.drainAsides)    // /btw side questions answered between steps, one no-tools turn each
-	if !a.readOnly {                 // a throwaway session leaves nothing behind
+	ag := a.ag
+	ag.SetModeSwitch(func() (bool, string, bool) { return a.takeMode(ag.Detached) }) // and a mode chosen with shift+tab mid-task
+	a.ag.SetAsides(a.drainAsides)                                                    // /btw side questions answered between steps, one no-tools turn each
+	if !a.readOnly {                                                                 // a throwaway session leaves nothing behind
 		a.ag.SetArchiver(&sessionArchiver{path: a.archivePath()})
 	}
 	a.ensureShadow()                                           // once per process, here on the wiring goroutine
@@ -3332,20 +3337,67 @@ func (m mode) String() string {
 
 func (a *app) mode() mode {
 	switch {
-	case a.planMode:
+	case a.planMode.Load():
 		return modePlan
 	case a.manual.Load():
 		return modeManual
 	}
 	return modeAuto
 }
 
-// setMode switches the mode and returns a one-line confirmation.
+// queueMode holds a mode chosen while a task runs, for the agent to take at the
+// start of the model's next turn (takeMode). A later choice replaces it.
+func (a *app) queueMode(m mode) {
+	a.modeMu.Lock()
+	a.queuedMode = &m
+	a.modeMu.Unlock()
+}
+
+// popQueuedMode takes a queued mode the agent has not: the task ended, or was
+// detached, first.
+func (a *app) popQueuedMode() *mode {
+	a.modeMu.Lock()
+	defer a.modeMu.Unlock()
+	q := a.queuedMode
+	a.queuedMode = nil
+	return q
+}
+
+// takeMode is the agent's mode switch (SetModeSwitch), called on its own
+// goroutine at the start of a turn. The app's mode changes right here, both
+// halves at once: manual for the call gate, and plan for a sub-agent the turn
+// may spawn — which used to read the old mode until the UI caught up. The
+// agent applies plan mode to itself; the "mode" event only tells the UI.
+//
+// The take and the stores are one step under modeMu, and detached is asked
+// inside it: a detached run's take either lands before force-detach pops the
+// queue — and the fresh agent is built from the mode it set — or finds itself
+// detached and takes nothing, not even a choice made since for the next run.
+func (a *app) takeMode(detached func() bool) (plan bool, name string, ok bool) {
+	a.modeMu.Lock()
+	defer a.modeMu.Unlock()
+	if detached() {
+		return false, "", false
+	}
+	q := a.queuedMode
+	a.queuedMode = nil
+	if q == nil {
+		return false, "", false
+	}
+	a.planMode.Store(*q == modePlan)
+	a.manual.Store(*q == modeManual)
+	return *q == modePlan, q.String(), true
+}
+
+// setMode switches the mode and returns a one-line confirmation. A mode set
+// outright replaces one still queued: /plan typed after a shift+tab during
+// /compact must not be undone by the next task's first turn.
 func (a *app) setMode(m mode) string {
-	a.planMode = m == modePlan
+	a.popQueuedMode()
+	a.planMode.Store(m == modePlan)
 	a.manual.Store(m == modeManual)
 	if a.ag != nil {
-		a.ag.SetPlanMode(a.planMode)
+		a.ag.SetPlanMode(m == modePlan)
 	}
 	switch m {
 	case modePlan:

cmd/agent/main_test.go

@@ -1808,17 +1808,17 @@ func TestIsCommandLine(t *testing.T) {
 	}
 }
 
-// shift+tab during a task doesn't flip the mode live (that races the running
-// agent) — it queues the switch and applies it when the task finishes.
+// shift+tab during a task doesn't flip the mode mid-turn — it queues the
+// switch, for the agent's next turn or, failing that, the task's end.
 func TestDeferredModeSwitchWhileRunning(t *testing.T) {
 	m := &tuiModel{state: stRunning, input: textarea.New(), app: &app{cfg: config.Default()}}
-	m.app.planMode = false // start in auto
+	m.app.planMode.Store(false) // start in auto
 
 	m.handleKey(tea.KeyMsg{Type: tea.KeyShiftTab})
 	if m.pendingMode == nil || *m.pendingMode != modePlan {
 		t.Fatalf("shift+tab while running should queue plan mode, pending=%v", m.pendingMode)
 	}
-	if m.app.planMode {
+	if m.app.planMode.Load() {
 		t.Error("mode must NOT flip live while a task runs")
 	}
 	m.handleKey(tea.KeyMsg{Type: tea.KeyShiftTab}) // moves the queued target on
@@ -1830,10 +1830,9 @@ func TestDeferredModeSwitchWhileRunning(t *testing.T) {
 		t.Errorf("third shift+tab should come back to auto, got %v", *m.pendingMode)
 	}
 
-	plan := modePlan
-	m.pendingMode = &plan
+	m.app.queueMode(modePlan) // what the task ended without taking
 	m.applyPendingMode()
-	if !m.app.planMode {
+	if !m.app.planMode.Load() {
 		t.Error("applyPendingMode should have switched to plan")
 	}
 	if m.pendingMode != nil {
@@ -2327,7 +2326,7 @@ func TestPlanReviewHandshake(t *testing.T) {
 		m.app.windowDetected = true // detectWindowCmd → nil (no probe)
 		m.app.client = llm.NewOpenAIClient(config.LLM{})
 		m.app.ag = agent.New(m.app.client, tool.NewRegistry(tool.NewCalc()), nil, nil, "", 5)
-		m.app.planMode = true
+		m.app.planMode.Store(true)
 		m.planTask = true
 		return m
 	}
@@ -2340,16 +2339,16 @@ func TestPlanReviewHandshake(t *testing.T) {
 	}
 	// enter = accept → switch to auto, a run starts (the cmd's goroutine isn't executed here)
 	m.handleKey(tea.KeyMsg{Type: tea.KeyEnter})
-	if m.app.planMode || m.state != stRunning {
-		t.Errorf("accept → planMode=%v state=%v; want auto + running", m.app.planMode, m.state)
+	if m.app.planMode.Load() || m.state != stRunning {
+		t.Errorf("accept → planMode=%v state=%v; want auto + running", m.app.planMode.Load(), m.state)
 	}
 
 	// esc = keep planning → idle, plan mode preserved
 	m = newM()
 	m.Update(taskDoneMsg{})
 	m.handleKey(tea.KeyMsg{Type: tea.KeyEsc})
-	if m.state != stIdle || !m.app.planMode {
-		t.Errorf("keep planning → state=%v planMode=%v; want stIdle + plan kept", m.state, m.app.planMode)
+	if m.state != stIdle || !m.app.planMode.Load() {
+		t.Errorf("keep planning → state=%v planMode=%v; want stIdle + plan kept", m.state, m.app.planMode.Load())
 	}
 
 	// a cancelled plan run must NOT pop the handshake
@@ -3826,7 +3825,8 @@ func TestSpawnExternalAgentBlockedInPlanMode(t *testing.T) {
 	cfg.Agents = map[string]config.AgentProfile{
 		"touch": {Kind: "external", Command: "touch", Args: []string{"{task}"}},
 	}
-	a := &app{cfg: cfg, workspace: ws, approver: fixedApprover(true), planMode: true}
+	a := &app{cfg: cfg, workspace: ws, approver: fixedApprover(true)}
+	a.planMode.Store(true)
 
 	_, err := a.spawnAgent(context.Background(), "touch", marker, "")
 	if err == nil || !strings.Contains(err.Error(), "plan mode") {
@@ -6250,7 +6250,7 @@ func TestModeLine(t *testing.T) {
 	if !strings.Contains(m.modeLine(), "auto mode on") {
 		t.Errorf("auto: %q", m.modeLine())
 	}
-	m.app.planMode = true
+	m.app.planMode.Store(true)
 	if !strings.Contains(m.modeLine(), "plan mode on") {
 		t.Errorf("plan: %q", m.modeLine())
 	}

cmd/agent/manual_mode_test.go

@@ -1,14 +1,17 @@
 package main
 
 import (
+	"bufio"
 	"context"
 	"strings"
 	"testing"
 
 	"github.com/charmbracelet/bubbles/textarea"
 	tea "github.com/charmbracelet/bubbletea"
 
+	"github.com/ipsupport-llc/ipsupport-code/internal/agent"
 	"github.com/ipsupport-llc/ipsupport-code/internal/config"
+	"github.com/ipsupport-llc/ipsupport-code/internal/llm"
 	"github.com/ipsupport-llc/ipsupport-code/internal/tool"
 )
 
@@ -84,8 +87,8 @@ func TestModesCycle(t *testing.T) {
 	want := []mode{modePlan, modeManual, modeAuto}
 	for _, w := range want {
 		a.setMode(a.mode().next())
-		if a.mode() != w || a.planMode != (w == modePlan) || a.manual.Load() != (w == modeManual) {
-			t.Fatalf("mode = %v (plan %v, manual %v), want %v", a.mode(), a.planMode, a.manual.Load(), w)
+		if a.mode() != w || a.planMode.Load() != (w == modePlan) || a.manual.Load() != (w == modeManual) {
+			t.Fatalf("mode = %v (plan %v, manual %v), want %v", a.mode(), a.planMode.Load(), a.manual.Load(), w)
 		}
 		if line := m.modeLine(); !strings.Contains(line, w.String()+" mode on") {
 			t.Errorf("indicator %q, want %s", line, w)
@@ -145,3 +148,132 @@ func TestManualModeJudgesARecoveredCall(t *testing.T) {
 		t.Errorf("numeric path: asked %d time(s), result %q; want a prompt", ap.calls-before, msg)
 	}
 }
+
+// shift+tab mid-task queues the mode for the agent's next turn; the agent takes
+// it once, and the app's mode changes right then — manual for the gate, plan
+// for a sub-agent the turn spawns. The "mode" event only tells the UI: read
+// late, it must not set back a newer switch. A task that ends first drops the
+// queue and applies the choice itself.
+func TestAModeChosenMidTaskIsTakenAtTheNextTurn(t *testing.T) {
+	a := &app{cfg: config.Default()}
+	m := &tuiModel{state: stRunning, input: textarea.New(), app: a}
+
+	m.handleKey(tea.KeyMsg{Type: tea.KeyShiftTab}) // plan
+	m.handleKey(tea.KeyMsg{Type: tea.KeyShiftTab}) // manual — the later choice wins
+	if a.mode() != modeAuto {
+		t.Fatal("the mode flipped mid-turn")
+	}
+	if log := strings.Join(m.history, "\n"); !strings.Contains(log, "from the model's next turn") || strings.Contains(log, "next task") {
+		t.Errorf("hint does not say when it applies:\n%s", log)
+	}
+	plan, name, ok := a.takeMode(func() bool { return false })
+	if !ok || plan || name != "manual" || a.mode() != modeManual {
+		t.Fatalf("takeMode = (%v, %q, %v), mode %v; want manual, taken, and in effect", plan, name, ok, a.mode())
+	}
+	if _, _, ok := a.takeMode(func() bool { return false }); ok {
+		t.Error("a queued mode was taken twice")
+	}
+	m.Update(eventMsg(uiEvent{kind: "mode", fields: map[string]any{"mode": "manual", "step": 2}}))
+	if log := strings.Join(m.history, "\n"); !strings.Contains(log, "manual mode on — from step 2") {
+		t.Errorf("the log does not say when the switch took effect:\n%s", log)
+	}
+
+	// A newer switch taken before an older event is read stands.
+	a.queueMode(modePlan)
+	a.takeMode(func() bool { return false })
+	m.Update(eventMsg(uiEvent{kind: "mode", fields: map[string]any{"mode": "manual", "step": 3}}))
+	if a.mode() != modePlan {
+		t.Errorf("a late event set the mode back to %v", a.mode())
+	}
+
+	// shift+tab works while an approval waits — the task is running.
+	m.state = stApprove
+	m.handleKey(tea.KeyMsg{Type: tea.KeyShiftTab})
+	if m.pendingMode == nil {
+		t.Error("shift+tab did nothing while an approval was waiting")
+	}
+
+	// The task ends before the agent took the choice: it applies, and the queue
+	// is emptied so the next task does not take it again.
+	m.state = stRunning
+	want := *m.pendingMode
+	m.applyPendingMode()
+	if a.mode() != want {
+		t.Errorf("mode = %v after the task ended, want the choice (%v) applied", a.mode(), want)
+	}
+	if _, _, ok := a.takeMode(func() bool { return false }); ok {
+		t.Error("the queue outlived the task that never took it")
+	}
+
+	// A mode set outright — an idle shift+tab, /plan — replaces one still
+	// queued, say from a shift+tab during /compact.
+	for _, set := range []func(){
+		func() { m.state = stIdle; m.handleKey(tea.KeyMsg{Type: tea.KeyShiftTab}) },
+		func() { a.setMode(modePlan) },
+	} {
+		a.queueMode(modeAuto)
+		set()
+		if _, _, ok := a.takeMode(func() bool { return false }); ok {
+			t.Error("a mode set outright left an older queued one for the next task to take")
+		}
+	}
+
+	// A choice the agent already took is not applied again at the task's end.
+	a.setMode(modeAuto)
+	m.state = stRunning
+	m.handleKey(tea.KeyMsg{Type: tea.KeyShiftTab}) // plan
+	a.takeMode(func() bool { return false })
+	a.setMode(modeManual) // anything after, e.g. the user's own /manual once idle — stand-in
+	m.pendingMode = &[]mode{modePlan}[0]
+	m.applyPendingMode()
+	if a.mode() != modeManual {
+		t.Errorf("mode = %v: a choice the agent had taken was applied again at the end", a.mode())
+	}
+}
+
+// A task switched into plan mid-run ends with PLAN READY, however the events
+// are ordered: the handshake is decided by the mode the run ended in.
+func TestAPlanSwitchedToMidTaskGetsItsHandshake(t *testing.T) {
+	m := &tuiModel{state: stRunning, input: textarea.New(), bridge: newBridge(), ctx: context.Background(), app: &app{cfg: config.Default()}}
+	m.app.windowDetected = true
+	m.app.client = llm.NewOpenAIClient(config.LLM{})
+	m.app.ag = agent.New(m.app.client, tool.NewRegistry(tool.NewCalc()), nil, nil, "", 5)
+	m.planTask = false // started in auto
+	m.app.queueMode(modePlan)
+	m.app.takeMode(func() bool { return false }) // taken mid-run; its event not read yet
+	m.Update(taskDoneMsg{})
+	if m.state != stPlanReview {
+		t.Errorf("state = %v, want PLAN READY for a run that ended in plan mode", m.state)
+	}
+}
+
+// A mode chosen for a run that is then force-detached applies at once, and the
+// fresh agent does not take it later — over a mode chosen since, and possibly
+// from manual back to auto.
+func TestAForceDetachedRunsModeIsNotLeftQueued(t *testing.T) {
+	cfg := config.Default()
+	cfg.Workspace = t.TempDir()
+	a := &app{cfg: cfg, workspace: cfg.Workspace, reader: bufio.NewReader(strings.NewReader("")), approver: fixedApprover(true)}
+	if err := a.wire(); err != nil {
+		t.Fatal(err)
+	}
+	m := &tuiModel{app: a, state: stRunning, taskCancelled: true, cancel: func() {}, input: textarea.New()}
+	m.epoch = a.taskEpoch.Add(1)
+	m.handleKey(tea.KeyMsg{Type: tea.KeyShiftTab}) // plan, for the wedged run
+	m.forceDetach()
+	if a.mode() != modePlan || m.pendingMode != nil {
+		t.Errorf("after force-detach: mode %v, pending %v; want the choice applied", a.mode(), m.pendingMode)
+	}
+	if _, _, ok := a.takeMode(func() bool { return false }); ok {
+		t.Error("the fresh agent would take the abandoned run's mode on its first turn")
+	}
+	// A detached run that reaches its take late takes nothing — not even a
+	// choice made since for the next run.
+	a.queueMode(modeManual)
+	if _, _, ok := a.takeMode(func() bool { return true }); ok || a.mode() == modeManual {
+		t.Error("a detached run took the next run's mode")
+	}
+	if q := a.popQueuedMode(); q == nil || *q != modeManual {
+		t.Error("the next run's mode was lost to a detached run")
+	}
+}