From fa14811d2ead77ba2d1e85ed40cdb2b3883fb1c6 Mon Sep 17 00:00:00 2001 From: Roman Romenskyi Date: Mon, 19 Oct 2026 21:18:02 -0600 Subject: [PATCH 1/4] Modes: a switch mid-task takes effect on the model's next turn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit shift+tab during a task said "applies on the next task", and did: the pending mode was applied only when the whole task ended — while the README claimed "the next turn". Now it is the next turn. The choice is queued for the agent, which takes it at the start of the model's next turn, before the call — never in the middle of a turn, so a turn's calls are judged under one mode. Manual takes effect for the call gate right there; a switch into or out of plan mode goes into the conversation as plan mode's directive or its OFF note, and the plan backstop follows. A "mode" event brings the UI along and logs "⇄ manual mode on — from step N"; switching into plan mid-task gets the PLAN READY handshake at the end. A task that ends before the model takes another turn applies the choice to the next task, as before. Read-only mode is not switchable this way. Agent.planMode is atomic now: the agent writes it between turns. Co-Authored-By: Claude Opus 5.5 --- README.md | 8 ++-- cmd/agent/main.go | 45 +++++++++++++++++++++ cmd/agent/main_test.go | 4 +- cmd/agent/manual_mode_test.go | 50 ++++++++++++++++++++++++ cmd/agent/tui.go | 36 +++++++++++++---- internal/agent/agent.go | 58 +++++++++++++++++++++------- internal/agent/agent_test.go | 73 ++++++++++++++++++++++++++++++++++- 7 files changed, 248 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index e6465d0..05fc57e 100644 --- a/README.md +++ b/README.md @@ -465,9 +465,11 @@ 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). +with shift+tab while the plan was being written, **enter** runs it 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. ## Permissions diff --git a/cmd/agent/main.go b/cmd/agent/main.go index eb4c2c6..9532331 100644 --- a/cmd/agent/main.go +++ b/cmd/agent/main.go @@ -619,6 +619,8 @@ type app struct { facts []string // durable project facts learned over time (per workspace) planMode bool // plan (propose) vs auto (execute); survives re-wire 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) @@ -3269,6 +3271,7 @@ func (a *app) wire() error { } 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.SetModeSwitch(a.takeMode) // 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()}) @@ -3320,6 +3323,17 @@ const ( // first, so a single press still does what it always did. func (m mode) next() mode { return (m + 1) % 3 } +// modeNamed is the mode a String names; auto for anything else. +func modeNamed(s string) mode { + switch s { + case "plan": + return modePlan + case "manual": + return modeManual + } + return modeAuto +} + func (m mode) String() string { switch m { case modePlan: @@ -3340,6 +3354,37 @@ func (a *app) mode() mode { return modeAuto } +// 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() +} + +// dropQueuedMode forgets a queued mode the task ended before taking. +func (a *app) dropQueuedMode() { + a.modeMu.Lock() + a.queuedMode = nil + a.modeMu.Unlock() +} + +// takeMode is the agent's mode switch (SetModeSwitch), called on its own +// goroutine at the start of a turn: manual takes effect for the call gate +// right here; plan mode the agent applies itself. The UI hears of it from the +// "mode" event and brings the rest of the app's state along. +func (a *app) takeMode() (plan bool, name string, ok bool) { + a.modeMu.Lock() + q := a.queuedMode + a.queuedMode = nil + a.modeMu.Unlock() + if q == nil { + return false, "", false + } + a.manual.Store(*q == modeManual) + return *q == modePlan, q.String(), true +} + // setMode switches the mode and returns a one-line confirmation. func (a *app) setMode(m mode) string { a.planMode = m == modePlan diff --git a/cmd/agent/main_test.go b/cmd/agent/main_test.go index eefd93f..808ebbb 100644 --- a/cmd/agent/main_test.go +++ b/cmd/agent/main_test.go @@ -1808,8 +1808,8 @@ 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 diff --git a/cmd/agent/manual_mode_test.go b/cmd/agent/manual_mode_test.go index 0cdbe9e..f9fcbb1 100644 --- a/cmd/agent/manual_mode_test.go +++ b/cmd/agent/manual_mode_test.go @@ -145,3 +145,53 @@ 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, manual taking effect for the gate right then; the "mode" event +// brings the UI along — and 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() + if !ok || plan || name != "manual" || !a.manual.Load() { + t.Fatalf("takeMode = (%v, %q, %v), manual on = %v; want manual, taken, and on for the gate", plan, name, ok, a.manual.Load()) + } + if _, _, ok := a.takeMode(); ok { + t.Error("a queued mode was taken twice") + } + + m.Update(eventMsg(uiEvent{kind: "mode", fields: map[string]any{"mode": "manual", "step": 2}})) + if a.mode() != modeManual || m.pendingMode != nil { + t.Errorf("after the mode event: mode %v, pending %v; want manual and nothing pending", a.mode(), m.pendingMode) + } + 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 switch into plan mid-task offers the plan handshake at the end. + m.Update(eventMsg(uiEvent{kind: "mode", fields: map[string]any{"mode": "plan", "step": 3}})) + if !m.planTask { + t.Error("a plan switched to mid-task will not get its PLAN READY handshake") + } + + // The task ends before the agent took the choice: it applies, and the queue + // is dropped so the next task does not take it again. + m.handleKey(tea.KeyMsg{Type: tea.KeyShiftTab}) + m.applyPendingMode() + if a.mode() != modeManual { + t.Errorf("mode = %v after the task ended, want the choice applied", a.mode()) + } + if _, _, ok := a.takeMode(); ok { + t.Error("the queue outlived the task that never took it") + } +} diff --git a/cmd/agent/tui.go b/cmd/agent/tui.go index f4b9f69..b5efc22 100644 --- a/cmd/agent/tui.go +++ b/cmd/agent/tui.go @@ -83,7 +83,7 @@ type tuiModel struct { steer []string // /btw asides for the current run — pinned above the input, not in scrollback; cleared when the task ends histIdx int // recall cursor into app.promptHist; == len means "not browsing" histDraft string // in-progress input saved when history browsing begins - pendingMode *mode // a mode requested via shift+tab mid-task; applied on task done + pendingMode *mode // a mode chosen via shift+tab mid-task: the agent takes it on its next turn ("mode" event), or the task's end applies it taskDoneAway bool // a task finished while a modal panel was open over it; finalize on panel close planTask bool // the running task started in plan mode → offer accept-plan when it finishes taskCancelled bool // the running task was cancelled by esc → don't offer accept-plan @@ -646,6 +646,9 @@ func (m *tuiModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if e.kind == "steer" { m.steerDelivered(e) } + if e.kind == "mode" { + m.modeTaken(e) + } m.push(m.renderEvent(e)...) if e.kind == "final" { m.subs = nil // a stray sub-agent line can't outlive the task @@ -1004,15 +1007,17 @@ func (m *tuiModel) handleKey(k tea.KeyMsg) (tea.Model, tea.Cmd) { m.app.setMode(m.app.mode().next()) // cycle auto → plan → manual; the bottom indicator updates m.pendingMode = nil case stRunning: - // Can't flip live — the running agent reads the mode (flipping races + - // splits the run). Remember it and apply when this task finishes. + // Not flipped here, mid-turn — a turn's calls would be judged under two + // modes. Queued for the agent, which takes it at the start of the + // model's next turn and says so (the "mode" event). cur := m.app.mode() if m.pendingMode != nil { cur = *m.pendingMode } next := cur.next() m.pendingMode = &next - m.push(cDim.Render(" ⇄ " + modeName(next) + " — applies on the next task")) + m.app.queueMode(next) + m.push(cDim.Render(" ⇄ " + modeName(next) + " — from the model's next turn")) } // In a modal panel (config/sessions/agents/rewind) shift+tab is inert. return m, nil @@ -1360,10 +1365,24 @@ func (m *tuiModel) planReviewKeys() string { // modeName labels a mode for the mid-task mode-switch hint. func modeName(m mode) string { return m.String() + " mode" } -// applyPendingMode applies a plan/auto switch requested (via shift+tab) while a -// task was running — deferred so it never flips the mode mid-run. No-op if none -// pending or it already matches the current mode. +// modeTaken brings the app along when the agent took a queued mode at the +// start of a turn: the indicator, the plan handshake at the end, and a pending +// choice that it was. +func (m *tuiModel) modeTaken(e uiEvent) { + name, _ := e.fields["mode"].(string) + md := modeNamed(name) + m.app.setMode(md) + m.planTask = md == modePlan + if m.pendingMode != nil && *m.pendingMode == md { + m.pendingMode = nil + } +} + +// applyPendingMode applies a mode chosen via shift+tab that the task ended +// before taking — the model's last turn was already under way. No-op if none +// is pending or it already matches the current mode. func (m *tuiModel) applyPendingMode() { + m.app.dropQueuedMode() if m.pendingMode == nil { return } @@ -3338,6 +3357,9 @@ func (m *tuiModel) renderEvent(e uiEvent) []string { } return []string{cOk.Render(fmt.Sprintf(" ✓ job #%d (%v) finished", id, e.fields["profile"])) + cDim.Render(" — result lands next turn · /jobs result "+strconv.Itoa(id)+" to read now")} + case "mode": + name, _ := e.fields["mode"].(string) + return []string{cDim.Render(fmt.Sprintf(" ⇄ %s mode on — from step %d", name, toInt(e.fields["step"])))} case "steer": // The moment a /steer note reached the model — the pinned note said only // that it would, and nothing in the log said when, or that it ever did. diff --git a/internal/agent/agent.go b/internal/agent/agent.go index 3e19e75..71cf98f 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -88,7 +88,7 @@ type Agent struct { historyMu sync.Mutex history []llm.Message maxHistory int - planMode bool + planMode atomic.Bool // set between turns only: by the caller while idle, or by switchMode // readOnly is plan mode that answers the task instead of proposing a plan; // readOnlyRun lets the run tool through, its commands confined to a sandbox // where nothing they could change is writable. @@ -118,7 +118,8 @@ type Agent struct { // the seam for finished background jobs that must reach the model between // turns WITHOUT interrupting the run. Must be safe to call concurrently. beforeTurn func() []llm.Message - steers func() []string // /steer notes typed mid-run — see SetSteers + steers func() []string // /steer notes typed mid-run — see SetSteers + modeSwitch func() (plan bool, mode string, ok bool) // a mode chosen mid-run — see SetModeSwitch // Goal pursuit: when the model finalizes but a judge (judgeGoal) decides the // goal isn't met, it's re-fed the goal and continues, up to maxReturns (a TTL). @@ -312,7 +313,7 @@ func (a *Agent) turnNote(hist []llm.Message) string { } else { parts = append(parts, readOnlyDirective+readOnlyNoRunNote) } - case a.planMode: + case a.planMode.Load(): // Every task while it is on, not once: the directive must survive the // history trimming its first mention away. parts = append(parts, planDirective) @@ -322,7 +323,7 @@ func (a *Agent) turnNote(hist []llm.Message) string { // already in its history and no memory of having sent it. parts = append(parts, planOffNote) } - if a.maxReturns > 0 && !a.planMode { // the judge decides, as in Run + if a.maxReturns > 0 && !a.planMode.Load() { // the judge decides, as in Run parts = append(parts, goalCheckNote) } if len(parts) == 0 { @@ -440,6 +441,29 @@ func (a *Agent) deliverSteers(msgs *[]llm.Message, step int) bool { return true } +// switchMode applies a mode chosen while the task runs, at the start of the +// model's next turn — it used to wait for the whole task to end. A switch into +// or out of plan mode is said in the conversation too, so the model hears of +// it on the turn it takes effect. Read-only mode is not the user's to switch, +// and a detached run takes nothing. +func (a *Agent) switchMode(msgs *[]llm.Message, step int) { + if a.modeSwitch == nil || a.readOnly || a.detached.Load() { + return + } + plan, mode, ok := a.modeSwitch() + if !ok { + return + } + if was := a.planMode.Swap(plan); was != plan { + note := planOffNote + if plan { + note = planDirective + } + *msgs = append(*msgs, llm.User(Note(note))) + } + a.emit("mode", map[string]any{"mode": mode, "step": step + 1}) +} + // SetJudgeLLM points the goal judge at its own connection instead of the main // one. nil (the default) means the judge shares a.llm. // @@ -489,6 +513,12 @@ func (a *Agent) SetBeforeTurn(fn func() []llm.Message) { a.beforeTurn = fn } // concurrency-safe, like SetBeforeTurn's. func (a *Agent) SetSteers(fn func() []string) { a.steers = fn } +// SetModeSwitch registers where a mode chosen while a task runs (shift+tab) +// comes from: fn reports a switch waiting, if any — whether it is plan mode, +// and its name for the "mode" event — and takes it. It is asked at the start +// of each of the model's turns, which is when the switch takes effect. +func (a *Agent) SetModeSwitch(fn func() (plan bool, mode string, ok bool)) { a.modeSwitch = fn } + // SetAsides registers the /btw side-channel: fn is drained between steps of a // running task, and each question gets a one-turn, no-tools answer (emitted as an // "aside" event) using the live conversation, without steering the task. @@ -562,21 +592,22 @@ func (a *Agent) HasRiskObserver() bool { return a.riskObserver != nil } // SetPlanMode toggles plan mode. In plan mode the agent investigates with // read-only tools and proposes a plan; mutating tool calls are refused, so it // can't change anything until switched back to auto. -func (a *Agent) SetPlanMode(on bool) { a.planMode = on } +func (a *Agent) SetPlanMode(on bool) { a.planMode.Store(on) } // SetReadOnly turns read-only mode on: plan mode's refusal of every call that // changes state, but the task is answered rather than planned. With run, the // run tool is let through: the caller has confined its commands to an OS // sandbox where the workspace is read-only. func (a *Agent) SetReadOnly(run bool) { - a.planMode, a.readOnly, a.readOnlyRun = true, true, run + a.planMode.Store(true) + a.readOnly, a.readOnlyRun = true, run } // ReadOnly reports whether read-only mode is on, and whether it lets run through. func (a *Agent) ReadOnly() (on, run bool) { return a.readOnly, a.readOnlyRun } // PlanMode reports whether plan mode is on. -func (a *Agent) PlanMode() bool { return a.planMode } +func (a *Agent) PlanMode() bool { return a.planMode.Load() } // SessionLen reports how many remembered messages are in the current session. func (a *Agent) SessionLen() int { @@ -1182,7 +1213,7 @@ func (a *Agent) Run(ctx context.Context, goal string) (tr Transcript, err error) sent := a.turnNote(hist) + goal msgs = append(msgs, llm.User(sent)) tools := a.reg.OpenAITools() - slog.Debug("run start", a.debugArgs("goal", clip(goal, 120), "tools", toolNames(tools), "plan_mode", a.planMode)...) + slog.Debug("run start", a.debugArgs("goal", clip(goal, 120), "tools", toolNames(tools), "plan_mode", a.planMode.Load())...) // Reported live: no single log line said how/why a run ended — reading it // off "model turn"/"stuck check" lines meant inferring it indirectly (e.g. // a tool_calls=[] turn with no following "stuck check" line MIGHT be a @@ -1234,6 +1265,7 @@ func (a *Agent) Run(ctx context.Context, goal string) (tr Transcript, err error) // /steer notes the user typed mid-run, then finished background jobs: // folded into the working set before this turn's model call, so they land // between iterations instead of interrupting the stream. + a.switchMode(&msgs, step) a.deliverSteers(&msgs, step) if a.beforeTurn != nil { if extra := a.beforeTurn(); len(extra) > 0 { @@ -1398,7 +1430,7 @@ func (a *Agent) Run(ctx context.Context, goal string) (tr Transcript, err error) // retry (the judge's own empty-reply retry, the refusal nudge right // below) — this is the same class of problem and deserves the same // treatment before being accepted as a real (if silent) answer. - if !a.planMode && !emptyNudged && !acted && strings.TrimSpace(clean) == "" { + if !a.planMode.Load() && !emptyNudged && !acted && strings.TrimSpace(clean) == "" { msgs = append(msgs, llm.User(emptyReplyNudge)) a.emit("nudge", map[string]any{"told": emptyReplyNudge}) emptyNudged = true @@ -1407,7 +1439,7 @@ func (a *Agent) Run(ctx context.Context, goal string) (tr Transcript, err error) // Refusal guard: a chat model answering an action task by pasting file // contents or claiming it "can't access your files" — and doing nothing // via tools this run. Push back once, hard, before accepting it. - if !a.planMode && !refusalNudged && !acted && looksLikeRefusal(clean) { + if !a.planMode.Load() && !refusalNudged && !acted && looksLikeRefusal(clean) { msgs = append(msgs, llm.User(refusalNudge)) a.emit("nudge", map[string]any{"told": refusalNudge}) refusalNudged = true @@ -1446,7 +1478,7 @@ func (a *Agent) Run(ctx context.Context, goal string) (tr Transcript, err error) // attempt and then refusing to grade it is the one combination that // makes no sense. chatReply := !acted && strings.TrimSpace(clean) != "" - if !a.planMode && a.maxReturns > 0 && !chatReply { + if !a.planMode.Load() && a.maxReturns > 0 && !chatReply { // The goal was re-fed and the model finished WITHOUT doing any work // since. Push it once first: a nudge is cheaper than a judge call and // spends no return. @@ -2139,7 +2171,7 @@ func (a *Agent) judgeOnGiveUp(ctx context.Context, goal string, returns int, had // here, on the two endings where the run produced the least on its own. With // the default TTL of 6, a run that takes its sixth re-feed and then burns its // step budget got no assessment at all. - if a.planMode || a.maxReturns == 0 || !(hadProductiveTurn || a.priorGoalProgress) { + if a.planMode.Load() || a.maxReturns == 0 || !(hadProductiveTurn || a.priorGoalProgress) { return false, "" } verdict, missing := a.judgeGoal(ctx, reason, goal, result, convo) @@ -2616,7 +2648,7 @@ func (a *Agent) execOne(ctx context.Context, c llm.ToolCall) (llm.Message, bool) // Plan mode backstop: refuse mutating calls even if the model ignores the // directive, so a weak model can't change anything while planning. - if a.planMode && a.reg.Mutates(c.Name, action) && !(a.readOnlyRun && c.Name == "run") { + if a.planMode.Load() && a.reg.Mutates(c.Name, action) && !(a.readOnlyRun && c.Name == "run") { msg := fmt.Sprintf("plan mode is ON — %s.%s was NOT run. Don't retry it; list it as a step in your plan, then finish.", c.Name, action) if a.readOnly { msg = fmt.Sprintf("read-only mode — %s.%s was NOT run: nothing may be changed. Don't retry it; say in your answer what you would change.", c.Name, action) diff --git a/internal/agent/agent_test.go b/internal/agent/agent_test.go index f83ccfd..37d7c17 100644 --- a/internal/agent/agent_test.go +++ b/internal/agent/agent_test.go @@ -3853,7 +3853,8 @@ func TestGoalCheckIsSaidUpFront(t *testing.T) { {"no goal", 0, false, false}, {"plan mode", 3, true, false}, } { - a := &Agent{maxReturns: c.returns, planMode: c.plan} + a := &Agent{maxReturns: c.returns} + a.SetPlanMode(c.plan) if said := strings.Contains(a.turnNote(nil), goalCheckNote); said != c.say { t.Errorf("%s: goal check said = %v, want %v", c.name, said, c.say) } @@ -4674,3 +4675,73 @@ func TestARecoveredCallIsJudgedAsItRuns(t *testing.T) { t.Errorf("plan mode let a recovered mutating call run: %q", ran) } } + +// A mode chosen mid-task takes effect at the start of the model's next turn, +// not when the task ends: switched to plan after the first turn, the write the +// model tries on its second is refused, the model is told plan mode is on, and +// the UI hears which step it started on. +func TestAModeChosenMidTaskTakesEffectOnTheNextTurn(t *testing.T) { + wrote := 0 + reg := tool.NewRegistry(tool.NewDomain(tool.DomainSpec{ + Name: "file", Summary: "files", + Actions: []tool.Action{ + {Name: "read", Run: func(context.Context, tool.Args) tool.Result { return tool.Ok("x") }}, + {Name: "write", Mutates: true, Run: func(context.Context, tool.Args) tool.Result { wrote++; return tool.Ok("wrote") }}, + }, + })) + fake := &scriptLLM{replies: []llm.Message{ + toolCallReply("c1", "file", `{"action":"write","params":{"path":"a"}}`), + toolCallReply("c2", "file", `{"action":"write","params":{"path":"b"}}`), + {Role: "assistant", Content: "plan: 1. write b"}, + }} + rt := &recTracer{} + a := New(fake, reg, nil, rt, "", 10) + asked := 0 + a.SetModeSwitch(func() (bool, string, bool) { + asked++ + if asked == 2 { // chosen during the first turn: taken before the second + return true, "plan", true + } + return false, "", false + }) + a.Run(context.Background(), "write a and b") + if wrote != 1 { + t.Errorf("writes = %d, want 1 — the first turn's ran in auto, the second's was refused in plan", wrote) + } + if !a.PlanMode() { + t.Error("the agent is not in plan mode after taking the switch") + } + if !strings.Contains(fmt.Sprint(fake.lastMsgs), planDirective) { + t.Error("the model was never told plan mode is on") + } + if !slices.Contains(rt.kinds, "mode") { + t.Error("no mode event — the UI cannot say when the switch took effect") + } + + // Back out of plan the same way: the OFF note, and the write runs. + fake = &scriptLLM{replies: []llm.Message{ + toolCallReply("c1", "file", `{"action":"read","params":{"path":"a"}}`), + toolCallReply("c2", "file", `{"action":"write","params":{"path":"b"}}`), + {Role: "assistant", Content: "done"}, + }} + a = New(fake, reg, nil, nil, "", 10) + a.SetPlanMode(true) + asked, wrote = 0, 0 + a.SetModeSwitch(func() (bool, string, bool) { + asked++ + return false, "auto", asked == 2 + }) + a.Run(context.Background(), "write b") + if wrote != 1 || !strings.Contains(fmt.Sprint(fake.lastMsgs), planOffNote) { + t.Errorf("writes = %d, off note sent = %v; want the write to run once plan mode is off", wrote, strings.Contains(fmt.Sprint(fake.lastMsgs), planOffNote)) + } + + // Read-only mode is not the user's to switch. + a = New(&scriptLLM{replies: []llm.Message{{Role: "assistant", Content: "ok"}}}, reg, nil, nil, "", 10) + a.SetReadOnly(false) + a.SetModeSwitch(func() (bool, string, bool) { return false, "auto", true }) + a.Run(context.Background(), "look") + if !a.PlanMode() { + t.Error("a mode switch turned read-only mode off") + } +} From 5b7ff44431fd6bdd45bdc60cf4ebbfd4ccc63ce5 Mon Sep 17 00:00:00 2001 From: Roman Romenskyi Date: Mon, 19 Oct 2026 21:28:59 -0600 Subject: [PATCH 2/4] Modes: review fixes for the next-turn switch - takeMode sets the app's plan mode too, on the agent's goroutine: a sub-agent spawned on the turn after a switch to plan read the old mode until the UI caught up, and could write. app.planMode is atomic now. - The "mode" event only clears the pending choice. It called setMode, so an event read late could set back a newer switch the agent had taken since, mid-turn. - PLAN READY is decided by the mode the run ended in, not by the order its last mode event and taskDoneMsg arrive in. - force-detach applies a pending choice and drops the queue, and an idle shift+tab drops it too: the fresh agent took the abandoned run's mode on its first turn, over one chosen since. - shift+tab works while an approval waits. Co-Authored-By: Claude Opus 5.5 --- README.md | 8 ++- cmd/agent/main.go | 25 +++++----- cmd/agent/main_test.go | 21 ++++---- cmd/agent/manual_mode_test.go | 93 ++++++++++++++++++++++++++++------- cmd/agent/tui.go | 24 ++++----- 5 files changed, 119 insertions(+), 52 deletions(-) diff --git a/README.md b/README.md index 05fc57e..8e40d83 100644 --- a/README.md +++ b/README.md @@ -464,12 +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. Switching the mode +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 diff --git a/cmd/agent/main.go b/cmd/agent/main.go index 9532331..dfffc14 100644 --- a/cmd/agent/main.go +++ b/cmd/agent/main.go @@ -617,7 +617,7 @@ 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) @@ -743,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 @@ -940,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 — @@ -1039,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 @@ -3264,7 +3265,7 @@ 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) @@ -3346,7 +3347,7 @@ 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 @@ -3370,9 +3371,10 @@ func (a *app) dropQueuedMode() { } // takeMode is the agent's mode switch (SetModeSwitch), called on its own -// goroutine at the start of a turn: manual takes effect for the call gate -// right here; plan mode the agent applies itself. The UI hears of it from the -// "mode" event and brings the rest of the app's state along. +// 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. func (a *app) takeMode() (plan bool, name string, ok bool) { a.modeMu.Lock() q := a.queuedMode @@ -3381,16 +3383,17 @@ func (a *app) takeMode() (plan bool, name string, ok bool) { 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. func (a *app) setMode(m mode) string { - a.planMode = m == modePlan + 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: diff --git a/cmd/agent/main_test.go b/cmd/agent/main_test.go index 808ebbb..86ef55d 100644 --- a/cmd/agent/main_test.go +++ b/cmd/agent/main_test.go @@ -1812,13 +1812,13 @@ func TestIsCommandLine(t *testing.T) { // 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 @@ -1833,7 +1833,7 @@ func TestDeferredModeSwitchWhileRunning(t *testing.T) { plan := modePlan m.pendingMode = &plan 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 +2327,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 +2340,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 +3826,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 +6251,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()) } diff --git a/cmd/agent/manual_mode_test.go b/cmd/agent/manual_mode_test.go index f9fcbb1..a64257d 100644 --- a/cmd/agent/manual_mode_test.go +++ b/cmd/agent/manual_mode_test.go @@ -1,6 +1,7 @@ package main import ( + "bufio" "context" "strings" "testing" @@ -8,7 +9,9 @@ import ( "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) @@ -147,9 +150,10 @@ func TestManualModeJudgesARecoveredCall(t *testing.T) { } // shift+tab mid-task queues the mode for the agent's next turn; the agent takes -// it once, manual taking effect for the gate right then; the "mode" event -// brings the UI along — and a task that ends first drops the queue and applies -// the choice itself. +// 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} @@ -163,35 +167,90 @@ func TestAModeChosenMidTaskIsTakenAtTheNextTurn(t *testing.T) { t.Errorf("hint does not say when it applies:\n%s", log) } plan, name, ok := a.takeMode() - if !ok || plan || name != "manual" || !a.manual.Load() { - t.Fatalf("takeMode = (%v, %q, %v), manual on = %v; want manual, taken, and on for the gate", plan, name, ok, a.manual.Load()) + 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(); ok { t.Error("a queued mode was taken twice") } - m.Update(eventMsg(uiEvent{kind: "mode", fields: map[string]any{"mode": "manual", "step": 2}})) - if a.mode() != modeManual || m.pendingMode != nil { - t.Errorf("after the mode event: mode %v, pending %v; want manual and nothing pending", a.mode(), m.pendingMode) + if m.pendingMode != nil { + t.Error("the choice the agent took is still pending") } 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 switch into plan mid-task offers the plan handshake at the end. - m.Update(eventMsg(uiEvent{kind: "mode", fields: map[string]any{"mode": "plan", "step": 3}})) - if !m.planTask { - t.Error("a plan switched to mid-task will not get its PLAN READY handshake") + // A newer switch taken before an older event is read stands. + a.queueMode(modePlan) + a.takeMode() + 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 dropped so the next task does not take it again. - m.handleKey(tea.KeyMsg{Type: tea.KeyShiftTab}) + m.state = stRunning + want := *m.pendingMode m.applyPendingMode() - if a.mode() != modeManual { - t.Errorf("mode = %v after the task ended, want the choice applied", a.mode()) + if a.mode() != want { + t.Errorf("mode = %v after the task ended, want the choice (%v) applied", a.mode(), want) } if _, _, ok := a.takeMode(); ok { t.Error("the queue outlived the task that never took it") } + + // An idle switch drops a queue left behind. + a.queueMode(modeAuto) + m.state = stIdle + m.handleKey(tea.KeyMsg{Type: tea.KeyShiftTab}) + if _, _, ok := a.takeMode(); ok { + t.Error("an idle switch left an older queued mode for the next task to take") + } +} + +// 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() // 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(); ok { + t.Error("the fresh agent would take the abandoned run's mode on its first turn") + } } diff --git a/cmd/agent/tui.go b/cmd/agent/tui.go index b5efc22..6c7a6d2 100644 --- a/cmd/agent/tui.go +++ b/cmd/agent/tui.go @@ -705,7 +705,8 @@ func (m *tuiModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.cancel = nil m.retry = nil m.restoreSteers() // steering notes belonged to the run that just ended - m.applyPendingMode() // a shift+tab during the task takes effect now, before the next one + m.planTask = m.app.mode() == modePlan // the mode the run ended in: a switch it took mid-task counts, whatever order the events arrive in + m.applyPendingMode() // a shift+tab the run never took applies now, before the next task detect := m.detectWindowCmd() // model is loaded now — confirm the real window detect = tea.Batch(detect, m.applyPendingConfig()) // settings staged in /config while this task was running if held := m.heldLessons; held != nil { @@ -1006,7 +1007,8 @@ func (m *tuiModel) handleKey(k tea.KeyMsg) (tea.Model, tea.Cmd) { case stIdle: m.app.setMode(m.app.mode().next()) // cycle auto → plan → manual; the bottom indicator updates m.pendingMode = nil - case stRunning: + m.app.dropQueuedMode() + case stRunning, stApprove: // an approval waiting is a task running — and a likely moment to want manual // Not flipped here, mid-turn — a turn's calls would be judged under two // modes. Queued for the agent, which takes it at the start of the // model's next turn and says so (the "mode" event). @@ -1365,15 +1367,12 @@ func (m *tuiModel) planReviewKeys() string { // modeName labels a mode for the mid-task mode-switch hint. func modeName(m mode) string { return m.String() + " mode" } -// modeTaken brings the app along when the agent took a queued mode at the -// start of a turn: the indicator, the plan handshake at the end, and a pending -// choice that it was. +// modeTaken clears the pending choice the agent just took. It changes no mode: +// takeMode already did, and an event read late must not set back a newer +// switch the agent took since. func (m *tuiModel) modeTaken(e uiEvent) { name, _ := e.fields["mode"].(string) - md := modeNamed(name) - m.app.setMode(md) - m.planTask = md == modePlan - if m.pendingMode != nil && *m.pendingMode == md { + if m.pendingMode != nil && *m.pendingMode == modeNamed(name) { m.pendingMode = nil } } @@ -2124,8 +2123,8 @@ func (m *tuiModel) startTask() (context.Context, context.CancelFunc) { m.startTok = c // count completion (generated) tokens for this task m.suggestion = "" m.input.Placeholder = defaultPlaceholder - m.bridge.arm() // fresh abort signal so a previous cancel doesn't deny this task's approvals - m.planTask = m.app.planMode // remember so we can offer accept-plan when it finishes + m.bridge.arm() // fresh abort signal so a previous cancel doesn't deny this task's approvals + m.planTask = m.app.planMode.Load() // remember so we can offer accept-plan when it finishes m.taskCancelled = false m.epoch = m.app.taskEpoch.Add(1) // this run's identity — a force-detach bumps it to orphan the run tctx, cancel := context.WithCancel(m.ctx) @@ -2151,7 +2150,8 @@ func (m *tuiModel) forceDetach() (tea.Model, tea.Cmd) { m.state = stIdle m.cancel = nil m.retry = nil - m.restoreSteers() // the abandoned run will never take them + m.restoreSteers() // the abandoned run will never take them + m.applyPendingMode() // nor a mode chosen for it: it applies now, and the fresh agent must not take it later m.busyMsg = "" m.taskCancelled = false staged := m.applyPendingConfig() // a detached run emits no taskDoneMsg — don't strand staged settings From b9969136802a5d66886f14af4269661860c9c161 Mon Sep 17 00:00:00 2001 From: Roman Romenskyi Date: Mon, 19 Oct 2026 21:53:45 -0600 Subject: [PATCH 3/4] Modes: the queue is what applies; a mode set outright replaces it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - What the task's end (or a force-detach) applies is what is still in the queue — what the agent did not take — instead of the UI's pending copy, which a late or stale "mode" event had to clear by name. The event now only logs. - setMode empties the queue: /plan typed after a shift+tab during /compact was undone by the next task's first turn. - takeMode takes and stores under one lock, and force-detach pops the queue before building the fresh agent, so a detached run's late take cannot leave the app and the new agent in different modes. Co-Authored-By: Claude Opus 5.5 --- cmd/agent/main.go | 31 ++++++++++++------------ cmd/agent/main_test.go | 3 +-- cmd/agent/manual_mode_test.go | 34 +++++++++++++++++++-------- cmd/agent/tui.go | 44 ++++++++++++++--------------------- 4 files changed, 58 insertions(+), 54 deletions(-) diff --git a/cmd/agent/main.go b/cmd/agent/main.go index dfffc14..c2b3974 100644 --- a/cmd/agent/main.go +++ b/cmd/agent/main.go @@ -3324,17 +3324,6 @@ const ( // first, so a single press still does what it always did. func (m mode) next() mode { return (m + 1) % 3 } -// modeNamed is the mode a String names; auto for anything else. -func modeNamed(s string) mode { - switch s { - case "plan": - return modePlan - case "manual": - return modeManual - } - return modeAuto -} - func (m mode) String() string { switch m { case modePlan: @@ -3363,11 +3352,14 @@ func (a *app) queueMode(m mode) { a.modeMu.Unlock() } -// dropQueuedMode forgets a queued mode the task ended before taking. -func (a *app) dropQueuedMode() { +// 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 - a.modeMu.Unlock() + return q } // takeMode is the agent's mode switch (SetModeSwitch), called on its own @@ -3375,11 +3367,15 @@ func (a *app) dropQueuedMode() { // 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: 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 the queue empty. func (a *app) takeMode() (plan bool, name string, ok bool) { a.modeMu.Lock() + defer a.modeMu.Unlock() q := a.queuedMode a.queuedMode = nil - a.modeMu.Unlock() if q == nil { return false, "", false } @@ -3388,8 +3384,11 @@ func (a *app) takeMode() (plan bool, name string, ok bool) { return *q == modePlan, q.String(), true } -// setMode switches the mode and returns a one-line confirmation. +// 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.popQueuedMode() a.planMode.Store(m == modePlan) a.manual.Store(m == modeManual) if a.ag != nil { diff --git a/cmd/agent/main_test.go b/cmd/agent/main_test.go index 86ef55d..8ffde4c 100644 --- a/cmd/agent/main_test.go +++ b/cmd/agent/main_test.go @@ -1830,8 +1830,7 @@ 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.Load() { t.Error("applyPendingMode should have switched to plan") diff --git a/cmd/agent/manual_mode_test.go b/cmd/agent/manual_mode_test.go index a64257d..0adbc06 100644 --- a/cmd/agent/manual_mode_test.go +++ b/cmd/agent/manual_mode_test.go @@ -174,9 +174,6 @@ func TestAModeChosenMidTaskIsTakenAtTheNextTurn(t *testing.T) { t.Error("a queued mode was taken twice") } m.Update(eventMsg(uiEvent{kind: "mode", fields: map[string]any{"mode": "manual", "step": 2}})) - if m.pendingMode != nil { - t.Error("the choice the agent took is still pending") - } 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) } @@ -197,7 +194,7 @@ func TestAModeChosenMidTaskIsTakenAtTheNextTurn(t *testing.T) { } // The task ends before the agent took the choice: it applies, and the queue - // is dropped so the next task does not take it again. + // is emptied so the next task does not take it again. m.state = stRunning want := *m.pendingMode m.applyPendingMode() @@ -208,12 +205,29 @@ func TestAModeChosenMidTaskIsTakenAtTheNextTurn(t *testing.T) { t.Error("the queue outlived the task that never took it") } - // An idle switch drops a queue left behind. - a.queueMode(modeAuto) - m.state = stIdle - m.handleKey(tea.KeyMsg{Type: tea.KeyShiftTab}) - if _, _, ok := a.takeMode(); ok { - t.Error("an idle switch left an older queued mode for the next task to take") + // 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(); 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() + 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()) } } diff --git a/cmd/agent/tui.go b/cmd/agent/tui.go index 6c7a6d2..015278a 100644 --- a/cmd/agent/tui.go +++ b/cmd/agent/tui.go @@ -83,7 +83,7 @@ type tuiModel struct { steer []string // /btw asides for the current run — pinned above the input, not in scrollback; cleared when the task ends histIdx int // recall cursor into app.promptHist; == len means "not browsing" histDraft string // in-progress input saved when history browsing begins - pendingMode *mode // a mode chosen via shift+tab mid-task: the agent takes it on its next turn ("mode" event), or the task's end applies it + pendingMode *mode // the last mode chosen via shift+tab mid-task — the base the next press cycles from; what applies is app.queuedMode taskDoneAway bool // a task finished while a modal panel was open over it; finalize on panel close planTask bool // the running task started in plan mode → offer accept-plan when it finishes taskCancelled bool // the running task was cancelled by esc → don't offer accept-plan @@ -646,9 +646,6 @@ func (m *tuiModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if e.kind == "steer" { m.steerDelivered(e) } - if e.kind == "mode" { - m.modeTaken(e) - } m.push(m.renderEvent(e)...) if e.kind == "final" { m.subs = nil // a stray sub-agent line can't outlive the task @@ -1005,9 +1002,8 @@ func (m *tuiModel) handleKey(k tea.KeyMsg) (tea.Model, tea.Cmd) { case "shift+tab": switch m.state { case stIdle: - m.app.setMode(m.app.mode().next()) // cycle auto → plan → manual; the bottom indicator updates + m.app.setMode(m.app.mode().next()) // cycle auto → plan → manual (and drop a queued one); the bottom indicator updates m.pendingMode = nil - m.app.dropQueuedMode() case stRunning, stApprove: // an approval waiting is a task running — and a likely moment to want manual // Not flipped here, mid-turn — a turn's calls would be judged under two // modes. Queued for the agent, which takes it at the start of the @@ -1367,27 +1363,17 @@ func (m *tuiModel) planReviewKeys() string { // modeName labels a mode for the mid-task mode-switch hint. func modeName(m mode) string { return m.String() + " mode" } -// modeTaken clears the pending choice the agent just took. It changes no mode: -// takeMode already did, and an event read late must not set back a newer -// switch the agent took since. -func (m *tuiModel) modeTaken(e uiEvent) { - name, _ := e.fields["mode"].(string) - if m.pendingMode != nil && *m.pendingMode == modeNamed(name) { - m.pendingMode = nil - } -} - // applyPendingMode applies a mode chosen via shift+tab that the task ended -// before taking — the model's last turn was already under way. No-op if none -// is pending or it already matches the current mode. +// before taking — the model's last turn was already under way. What is still +// in the queue is what was not taken, so a choice the agent did take is never +// applied twice, and no event order can confuse the two. func (m *tuiModel) applyPendingMode() { - m.app.dropQueuedMode() - if m.pendingMode == nil { + m.pendingMode = nil + q := m.app.popQueuedMode() + if q == nil { return } - want := *m.pendingMode - m.pendingMode = nil - if want != m.app.mode() { + if want := *q; want != m.app.mode() { m.push(cDim.Render(" " + m.app.setMode(want))) } } @@ -2140,7 +2126,10 @@ func (m *tuiModel) startTask() (context.Context, context.CancelFunc) { // no-ops too. Nothing blocks; the queue drains onto the fresh agent. func (m *tuiModel) forceDetach() (tea.Model, tea.Cmd) { old := m.app.ag - old.Detach() // silence the wedged run (emit/remember → no-ops) + old.Detach() // silence the wedged run (emit/remember → no-ops) + // Before the fresh agent is built: a mode the run took is in the app's + // mode by now, and one it did not is ours to apply (takeMode's lock). + queued := m.app.popQueuedMode() m.app.taskEpoch.Add(1) // orphan its run: its taskDoneMsg + side effects are stale now m.epoch = m.app.taskEpoch.Load() if err := m.app.wire(); err != nil { @@ -2150,8 +2139,11 @@ func (m *tuiModel) forceDetach() (tea.Model, tea.Cmd) { m.state = stIdle m.cancel = nil m.retry = nil - m.restoreSteers() // the abandoned run will never take them - m.applyPendingMode() // nor a mode chosen for it: it applies now, and the fresh agent must not take it later + m.restoreSteers() // the abandoned run will never take them + m.pendingMode = nil + if queued != nil { // nor a mode chosen for it: it applies now, not on the fresh agent's first turn + m.push(cDim.Render(" " + m.app.setMode(*queued))) + } m.busyMsg = "" m.taskCancelled = false staged := m.applyPendingConfig() // a detached run emits no taskDoneMsg — don't strand staged settings From 2e01d08e4bd65d9f9ae18692f679f11f177cdb84 Mon Sep 17 00:00:00 2001 From: Roman Romenskyi Date: Mon, 19 Oct 2026 21:56:04 -0600 Subject: [PATCH 4/4] Modes: a detached run takes no mode, checked under the take's lock A run paused between its own detached check and the take could, after a force-detach and a new task, take the choice queued for the new run. Co-Authored-By: Claude Opus 5.5 --- cmd/agent/main.go | 19 ++++++++++++------- cmd/agent/manual_mode_test.go | 25 +++++++++++++++++-------- internal/agent/agent.go | 3 +++ 3 files changed, 32 insertions(+), 15 deletions(-) diff --git a/cmd/agent/main.go b/cmd/agent/main.go index c2b3974..dd4cc6d 100644 --- a/cmd/agent/main.go +++ b/cmd/agent/main.go @@ -3272,9 +3272,10 @@ func (a *app) wire() error { } 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.SetModeSwitch(a.takeMode) // 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 + 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 @@ -3368,12 +3369,16 @@ func (a *app) popQueuedMode() *mode { // 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: 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 the queue empty. -func (a *app) takeMode() (plan bool, name string, ok bool) { +// 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 { diff --git a/cmd/agent/manual_mode_test.go b/cmd/agent/manual_mode_test.go index 0adbc06..7d18a00 100644 --- a/cmd/agent/manual_mode_test.go +++ b/cmd/agent/manual_mode_test.go @@ -166,11 +166,11 @@ func TestAModeChosenMidTaskIsTakenAtTheNextTurn(t *testing.T) { 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() + 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(); ok { + 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}})) @@ -180,7 +180,7 @@ func TestAModeChosenMidTaskIsTakenAtTheNextTurn(t *testing.T) { // A newer switch taken before an older event is read stands. a.queueMode(modePlan) - a.takeMode() + 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()) @@ -201,7 +201,7 @@ func TestAModeChosenMidTaskIsTakenAtTheNextTurn(t *testing.T) { if a.mode() != want { t.Errorf("mode = %v after the task ended, want the choice (%v) applied", a.mode(), want) } - if _, _, ok := a.takeMode(); ok { + if _, _, ok := a.takeMode(func() bool { return false }); ok { t.Error("the queue outlived the task that never took it") } @@ -213,7 +213,7 @@ func TestAModeChosenMidTaskIsTakenAtTheNextTurn(t *testing.T) { } { a.queueMode(modeAuto) set() - if _, _, ok := a.takeMode(); ok { + 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") } } @@ -222,7 +222,7 @@ func TestAModeChosenMidTaskIsTakenAtTheNextTurn(t *testing.T) { a.setMode(modeAuto) m.state = stRunning m.handleKey(tea.KeyMsg{Type: tea.KeyShiftTab}) // plan - a.takeMode() + 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() @@ -240,7 +240,7 @@ func TestAPlanSwitchedToMidTaskGetsItsHandshake(t *testing.T) { 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() // taken mid-run; its event not read yet + 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) @@ -264,7 +264,16 @@ func TestAForceDetachedRunsModeIsNotLeftQueued(t *testing.T) { 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(); ok { + 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") + } } diff --git a/internal/agent/agent.go b/internal/agent/agent.go index 71cf98f..290c864 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -1007,6 +1007,9 @@ func (a *Agent) Detach() { a.detached.Store(true) } // Reattach undoes Detach (used only if the agent swap it was part of failed). func (a *Agent) Reattach() { a.detached.Store(false) } +// Detached reports whether Detach silenced this agent. +func (a *Agent) Detached() bool { return a.detached.Load() } + // remember appends the goal and its final answer to the session, trimming to // the most recent maxHistory messages. msgs is the full turn just finished // (all tool calls/results) — actionsDigest pulls a short, deterministic list of