diff --git a/README.md b/README.md index e6465d0..8e40d83 100644 --- a/README.md +++ b/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 diff --git a/cmd/agent/main.go b/cmd/agent/main.go index eb4c2c6..dd4cc6d 100644 --- a/cmd/agent/main.go +++ b/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,7 +3337,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 @@ -3340,12 +3345,59 @@ func (a *app) mode() mode { 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: diff --git a/cmd/agent/main_test.go b/cmd/agent/main_test.go index eefd93f..8ffde4c 100644 --- a/cmd/agent/main_test.go +++ b/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()) } diff --git a/cmd/agent/manual_mode_test.go b/cmd/agent/manual_mode_test.go index 0cdbe9e..7d18a00 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) @@ -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") + } +} diff --git a/cmd/agent/tui.go b/cmd/agent/tui.go index f4b9f69..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 requested via shift+tab mid-task; applied on task done + 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 @@ -702,7 +702,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 { @@ -1001,18 +1002,20 @@ 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 - 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. + 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). 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,16 +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" } -// 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. +// applyPendingMode applies a mode chosen via shift+tab that the task ended +// 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() { - 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))) } } @@ -2105,8 +2109,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) @@ -2122,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 { @@ -2133,6 +2140,10 @@ func (m *tuiModel) forceDetach() (tea.Model, tea.Cmd) { m.cancel = nil m.retry = nil 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 @@ -3338,6 +3349,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..290c864 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 { @@ -976,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 @@ -1182,7 +1216,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 +1268,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 +1433,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 +1442,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 +1481,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 +2174,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 +2651,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") + } +}