From 6d7d3946c8c3c04b68ff7eddd3603626f808ae69 Mon Sep 17 00:00:00 2001 From: Vasanthdev2004 Date: Sun, 5 Jul 2026 00:16:23 +0530 Subject: [PATCH 1/9] =?UTF-8?q?feat(tui):=20/loop=20foundation=20=E2=80=94?= =?UTF-8?q?=20command=20parsing,=20loop=20state,=20safety=20bounds?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First slice of the /loop feature (#501): the self-contained core with no TUI wiring yet. parseLoopCommand handles the leading-interval, trailing-"every", bare-self-paced, and list/stop forms; loopState models one session-scoped loop with doom-loop + iteration-cap fields; interval/self-pace clamps and duration formatting round it out, with unit tests. Because Zero drives loops off a Go timer rather than a cron backend, arbitrary intervals (90s, 7m) are supported with no whitelist limitation. --- internal/tui/loop.go | 248 ++++++++++++++++++++++++++++++++++++++ internal/tui/loop_test.go | 111 +++++++++++++++++ 2 files changed, 359 insertions(+) create mode 100644 internal/tui/loop.go create mode 100644 internal/tui/loop_test.go diff --git a/internal/tui/loop.go b/internal/tui/loop.go new file mode 100644 index 00000000..52184924 --- /dev/null +++ b/internal/tui/loop.go @@ -0,0 +1,248 @@ +package tui + +import ( + "fmt" + "regexp" + "strconv" + "strings" + "time" +) + +// The /loop command repeats a prompt or slash command in one of two modes: +// +// - Fixed interval — `/loop 5m ` re-fires on a wall-clock cadence. +// - Self-paced — bare `/loop ` lets the model finish an iteration, +// pick its own next wake (via the loop_control tool), and stop when done. +// +// Loops are foreground and session-scoped: a tick only fires while the session is +// idle between turns (it never interrupts a streaming turn), and the loop lives +// with its session. Everything here is pure/loop-local; the wiring into the turn +// lifecycle (ticker, launch, continuation) lives in model.go. + +type loopMode int + +const ( + loopModeFixed loopMode = iota // re-fire every fixed interval + loopModeSelfPaced // model chooses the next wake each iteration +) + +// Safety bounds. A loop is foreground + session-scoped, but these keep a runaway +// or forgotten loop from burning tokens forever. +const ( + loopMinInterval = 30 * time.Second + loopMaxInterval = 24 * time.Hour + loopMaxIterations = 500 // hard ceiling; the loop auto-stops after this many + + loopSelfPaceMin = 1 * time.Minute + loopSelfPaceMax = 1 * time.Hour + loopSelfPaceDefault = 15 * time.Minute + + // loopDoomThreshold ends a loop after this many consecutive identical final + // answers — a self-paced or interval loop that gets stuck repeating the same + // no-progress result should stop rather than churn. + loopDoomThreshold = 5 + + // loopPollInterval is how often the controller checks whether a loop is due. + // A loop only fires while idle, so this is a cheap between-turn poll. + loopPollInterval = 1 * time.Second +) + +// loopState is one active loop owned by the current session. +type loopState struct { + id string + mode loopMode + prompt string // the prompt or "/command" re-run each iteration + interval time.Duration + iteration int + maxIter int // 0 => loopMaxIterations + createdAt time.Time + nextRunAt time.Time + // lastResult / repeatRun drive doom-loop detection: consecutive identical + // final answers past loopDoomThreshold end the loop. + lastResult string + repeatRun int + paused bool +} + +func (l *loopState) iterationCap() int { + if l.maxIter > 0 { + return l.maxIter + } + return loopMaxIterations +} + +// due reports whether the loop should fire at now (active, not paused, past its +// next-run mark). +func (l *loopState) due(now time.Time) bool { + return !l.paused && !l.nextRunAt.IsZero() && !now.Before(l.nextRunAt) +} + +// cadenceText renders the loop's cadence for status lines. +func (l *loopState) cadenceText() string { + if l.mode == loopModeSelfPaced { + return "self-paced" + } + return "every " + formatLoopDuration(l.interval) +} + +type loopAction int + +const ( + loopActionUsage loopAction = iota // show usage / list when empty + loopActionStart // start a new loop + loopActionStop // stop one by id + loopActionStopAll // stop every loop + loopActionList // list active loops +) + +// loopCommand is the parsed form of a `/loop ...` invocation (the text after the +// command name). +type loopCommand struct { + action loopAction + targetID string + mode loopMode + prompt string + interval time.Duration + // note carries a non-fatal advisory (e.g. an interval that was clamped) to + // surface back to the user. + note string +} + +var loopIntervalRe = regexp.MustCompile(`^(\d+)(s|m|h|d)$`) + +// parseLoopCommand interprets the arguments to `/loop`. Forms: +// +// (empty) -> usage/list +// list | status -> list active loops +// stop | stop all | stop +// -> fixed-interval loop (e.g. "5m /babysit") +// every +// -> self-paced loop (no interval) +func parseLoopCommand(args string) loopCommand { + trimmed := strings.TrimSpace(args) + if trimmed == "" { + return loopCommand{action: loopActionUsage} + } + + first := strings.ToLower(strings.Fields(trimmed)[0]) + switch first { + case "list", "status", "ls": + return loopCommand{action: loopActionList} + case "stop", "cancel", "kill": + rest := strings.TrimSpace(trimmed[len(strings.Fields(trimmed)[0]):]) + if rest == "" || strings.EqualFold(rest, "all") { + if strings.EqualFold(rest, "all") { + return loopCommand{action: loopActionStopAll} + } + // bare "stop" stops all when ambiguous; the caller may special-case a + // single active loop. + return loopCommand{action: loopActionStop, targetID: ""} + } + return loopCommand{action: loopActionStop, targetID: strings.TrimSpace(rest)} + } + + // Leading interval token: "5m ". + if fields := strings.Fields(trimmed); len(fields) >= 2 { + if d, ok := parseLoopInterval(fields[0]); ok { + prompt := strings.TrimSpace(trimmed[len(fields[0]):]) + cmd := loopCommand{action: loopActionStart, mode: loopModeFixed, prompt: prompt} + cmd.interval, cmd.note = clampLoopInterval(d) + return cmd + } + } + + // Trailing "every ". + if prompt, d, ok := splitTrailingEvery(trimmed); ok { + cmd := loopCommand{action: loopActionStart, mode: loopModeFixed, prompt: prompt} + cmd.interval, cmd.note = clampLoopInterval(d) + return cmd + } + + // No interval anywhere -> self-paced. + return loopCommand{action: loopActionStart, mode: loopModeSelfPaced, prompt: trimmed} +} + +// splitTrailingEvery matches " every " (the interval must be a +// real duration token so "check every PR" is not mis-parsed as a cadence). +func splitTrailingEvery(input string) (string, time.Duration, bool) { + fields := strings.Fields(input) + if len(fields) < 3 { + return "", 0, false + } + last := fields[len(fields)-1] + prev := fields[len(fields)-2] + if !strings.EqualFold(prev, "every") { + return "", 0, false + } + d, ok := parseLoopInterval(last) + if !ok { + return "", 0, false + } + prompt := strings.TrimSpace(strings.Join(fields[:len(fields)-2], " ")) + if prompt == "" { + return "", 0, false + } + return prompt, d, true +} + +// parseLoopInterval parses a bare "" token (s/m/h/d) into a duration. +func parseLoopInterval(token string) (time.Duration, bool) { + m := loopIntervalRe.FindStringSubmatch(strings.ToLower(strings.TrimSpace(token))) + if m == nil { + return 0, false + } + n, err := strconv.Atoi(m[1]) + if err != nil || n <= 0 { + return 0, false + } + unit := map[string]time.Duration{ + "s": time.Second, + "m": time.Minute, + "h": time.Hour, + "d": 24 * time.Hour, + }[m[2]] + return time.Duration(n) * unit, true +} + +// clampLoopInterval bounds an interval to [loopMinInterval, loopMaxInterval] and +// returns an advisory note when it was adjusted. +func clampLoopInterval(d time.Duration) (time.Duration, string) { + switch { + case d < loopMinInterval: + return loopMinInterval, fmt.Sprintf("interval raised to the %s minimum", formatLoopDuration(loopMinInterval)) + case d > loopMaxInterval: + return loopMaxInterval, fmt.Sprintf("interval lowered to the %s maximum", formatLoopDuration(loopMaxInterval)) + default: + return d, "" + } +} + +// clampSelfPaceDelay bounds a model-chosen self-paced delay to the self-pace +// band, defaulting an unset/invalid value. +func clampSelfPaceDelay(d time.Duration) time.Duration { + switch { + case d <= 0: + return loopSelfPaceDefault + case d < loopSelfPaceMin: + return loopSelfPaceMin + case d > loopSelfPaceMax: + return loopSelfPaceMax + default: + return d + } +} + +// formatLoopDuration renders a duration compactly (90s, 5m, 2h, 1d) for status +// text, preferring the largest whole unit. +func formatLoopDuration(d time.Duration) string { + switch { + case d%(24*time.Hour) == 0 && d >= 24*time.Hour: + return strconv.Itoa(int(d/(24*time.Hour))) + "d" + case d%time.Hour == 0 && d >= time.Hour: + return strconv.Itoa(int(d/time.Hour)) + "h" + case d%time.Minute == 0 && d >= time.Minute: + return strconv.Itoa(int(d/time.Minute)) + "m" + default: + return strconv.Itoa(int(d/time.Second)) + "s" + } +} diff --git a/internal/tui/loop_test.go b/internal/tui/loop_test.go new file mode 100644 index 00000000..aeb1cfb7 --- /dev/null +++ b/internal/tui/loop_test.go @@ -0,0 +1,111 @@ +package tui + +import ( + "testing" + "time" +) + +func TestParseLoopCommandModes(t *testing.T) { + cases := []struct { + in string + action loopAction + mode loopMode + prompt string + interval time.Duration + }{ + {"", loopActionUsage, 0, "", 0}, + {"list", loopActionList, 0, "", 0}, + {"status", loopActionList, 0, "", 0}, + {"stop all", loopActionStopAll, 0, "", 0}, + {"stop ab12", loopActionStop, 0, "", 0}, + {"5m /babysit-prs", loopActionStart, loopModeFixed, "/babysit-prs", 5 * time.Minute}, + {"90s check the build", loopActionStart, loopModeFixed, "check the build", 90 * time.Second}, + {"watch CI every 2m", loopActionStart, loopModeFixed, "watch CI", 2 * time.Minute}, + {"keep improving the docs", loopActionStart, loopModeSelfPaced, "keep improving the docs", 0}, + // "every PR" is not a cadence — the token after "every" isn't a duration. + {"review every PR", loopActionStart, loopModeSelfPaced, "review every PR", 0}, + } + for _, c := range cases { + got := parseLoopCommand(c.in) + if got.action != c.action { + t.Errorf("%q action = %v, want %v", c.in, got.action, c.action) + } + if c.action == loopActionStart { + if got.mode != c.mode { + t.Errorf("%q mode = %v, want %v", c.in, got.mode, c.mode) + } + if got.prompt != c.prompt { + t.Errorf("%q prompt = %q, want %q", c.in, got.prompt, c.prompt) + } + if got.interval != c.interval { + t.Errorf("%q interval = %v, want %v", c.in, got.interval, c.interval) + } + } + } +} + +func TestParseLoopCommandStopTarget(t *testing.T) { + if got := parseLoopCommand("stop"); got.action != loopActionStop || got.targetID != "" { + t.Fatalf("bare stop = %+v, want stop with empty target", got) + } + if got := parseLoopCommand("stop XyZ9"); got.targetID != "XyZ9" { + t.Fatalf("stop target = %q, want XyZ9", got.targetID) + } +} + +func TestClampLoopInterval(t *testing.T) { + if d, note := clampLoopInterval(5 * time.Second); d != loopMinInterval || note == "" { + t.Fatalf("below-min = (%v,%q), want raised to min with note", d, note) + } + if d, note := clampLoopInterval(48 * time.Hour); d != loopMaxInterval || note == "" { + t.Fatalf("above-max = (%v,%q), want lowered to max with note", d, note) + } + if d, note := clampLoopInterval(5 * time.Minute); d != 5*time.Minute || note != "" { + t.Fatalf("in-range = (%v,%q), want unchanged, no note", d, note) + } +} + +func TestClampSelfPaceDelay(t *testing.T) { + if got := clampSelfPaceDelay(0); got != loopSelfPaceDefault { + t.Errorf("zero delay = %v, want default %v", got, loopSelfPaceDefault) + } + if got := clampSelfPaceDelay(10 * time.Second); got != loopSelfPaceMin { + t.Errorf("below-min = %v, want %v", got, loopSelfPaceMin) + } + if got := clampSelfPaceDelay(3 * time.Hour); got != loopSelfPaceMax { + t.Errorf("above-max = %v, want %v", got, loopSelfPaceMax) + } + if got := clampSelfPaceDelay(10 * time.Minute); got != 10*time.Minute { + t.Errorf("in-band = %v, want unchanged", got) + } +} + +func TestFormatLoopDuration(t *testing.T) { + cases := map[time.Duration]string{ + 30 * time.Second: "30s", + 90 * time.Second: "90s", + 5 * time.Minute: "5m", + 2 * time.Hour: "2h", + 24 * time.Hour: "1d", + } + for d, want := range cases { + if got := formatLoopDuration(d); got != want { + t.Errorf("formatLoopDuration(%v) = %q, want %q", d, got, want) + } + } +} + +func TestLoopStateDue(t *testing.T) { + base := time.Date(2026, 7, 5, 12, 0, 0, 0, time.UTC) + l := &loopState{nextRunAt: base} + if l.due(base.Add(-time.Second)) { + t.Error("loop should not be due before nextRunAt") + } + if !l.due(base) { + t.Error("loop should be due at nextRunAt") + } + l.paused = true + if l.due(base.Add(time.Hour)) { + t.Error("paused loop should never be due") + } +} From 4bc9a97176724a76344fb2f3c97ab02a12252082 Mon Sep 17 00:00:00 2001 From: Vasanthdev2004 Date: Sun, 5 Jul 2026 00:24:01 +0530 Subject: [PATCH 2/9] =?UTF-8?q?feat(tui):=20wire=20/loop=20into=20the=20tu?= =?UTF-8?q?rn=20lifecycle=20=E2=80=94=20dispatch,=20idle=20tick,=20continu?= =?UTF-8?q?ation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second slice of #501. Adds the controller and wires it in: - /loop command registered (commands.go) and dispatched to handleLoopCommand. - Loop state on the model (loops, activeLoopID, loopSeq/loopTicking poll guards, loopNextWake/loopDone self-pace channel). - An idle poll tick (loopTickMsg, ~1s) fires the earliest due loop only when the session is idle — mirrors the launchQueuedMessageIfReady guard so loops and a user's queued prompt never collide; a self-perpetuating tea.Tick that stops when no loops remain. - The agentResponseMsg completion seam advances a loop iteration (schedule next wake for fixed, model delay for self-paced) with doom-loop, iteration-cap, and done stop conditions, restarting the ticker if other loops remain. - /loop list|stop|stop all, footer summary + list rendering, and a guard that only a prompt or custom /command may loop (not a built-in). Controller tests cover start/advance/self-paced-delay/done/doom/cap/stop/busy. --- internal/tui/commands.go | 8 + internal/tui/loop.go | 291 +++++++++++++++++++++++++++ internal/tui/loop_controller_test.go | 146 ++++++++++++++ internal/tui/model.go | 52 ++++- 4 files changed, 492 insertions(+), 5 deletions(-) create mode 100644 internal/tui/loop_controller_test.go diff --git a/internal/tui/commands.go b/internal/tui/commands.go index 0890a6b4..e9866511 100644 --- a/internal/tui/commands.go +++ b/internal/tui/commands.go @@ -47,6 +47,7 @@ const ( commandExport commandNew commandSkills + commandLoop commandUnknown ) @@ -297,6 +298,13 @@ var commandDefinitions = []commandDefinition{ description: "Write the conversation transcript to a file.", kind: commandExport, }, + { + name: "/loop", + usage: "/loop [interval] | /loop list | /loop stop [id|all]", + group: commandGroupSession, + description: "Repeat a prompt or command on an interval (e.g. /loop 5m /babysit-prs), or self-paced when no interval is given.", + kind: commandLoop, + }, { name: "/help", usage: "/help", diff --git a/internal/tui/loop.go b/internal/tui/loop.go index 52184924..b716f0ee 100644 --- a/internal/tui/loop.go +++ b/internal/tui/loop.go @@ -6,6 +6,8 @@ import ( "strconv" "strings" "time" + + tea "charm.land/bubbletea/v2" ) // The /loop command repeats a prompt or slash command in one of two modes: @@ -217,6 +219,295 @@ func clampLoopInterval(d time.Duration) (time.Duration, string) { } } +// loopTickMsg is the between-turn poll that fires a due loop while the session is +// idle. seq invalidates a tick left over from before the loop set changed. +type loopTickMsg struct{ seq int } + +// handleLoopCommand dispatches a parsed `/loop ...` invocation. +func (m model) handleLoopCommand(args string) (model, tea.Cmd) { + cmd := parseLoopCommand(args) + switch cmd.action { + case loopActionList: + return m.appendLoopSystem(m.loopListText()), nil + case loopActionUsage: + if len(m.loops) > 0 { + return m.appendLoopSystem(m.loopListText()), nil + } + return m.appendLoopSystem(loopUsageText()), nil + case loopActionStop: + return m.stopLoop(cmd.targetID) + case loopActionStopAll: + return m.stopAllLoops() + case loopActionStart: + return m.startLoop(cmd) + } + return m, nil +} + +// startLoop registers a new loop and (re)starts the idle poll ticker. The first +// iteration fires on the next idle tick (nextRunAt = now). +func (m model) startLoop(cmd loopCommand) (model, tea.Cmd) { + if reason, ok := validateLoopTarget(cmd.prompt); !ok { + return m.appendLoopSystem(reason), nil + } + m.loopCounter++ + id := "L" + strconv.Itoa(m.loopCounter) + loop := &loopState{ + id: id, + mode: cmd.mode, + prompt: cmd.prompt, + interval: cmd.interval, + createdAt: m.now(), + nextRunAt: m.now(), // fire the first iteration on the next idle tick + } + m.loops = append(m.loops, loop) + note := "" + if cmd.note != "" { + note = " (" + cmd.note + ")" + } + m = m.appendLoopSystem(fmt.Sprintf( + "Loop %s started — %s%s. Runs while this session is idle; stop with /loop stop %s.", + id, loop.cadenceText(), note, id)) + return m.ensureLoopTick() +} + +// stopLoop stops one loop by id, or the single active loop when the id is omitted. +func (m model) stopLoop(id string) (model, tea.Cmd) { + if len(m.loops) == 0 { + return m.appendLoopSystem("No active loops."), nil + } + if id == "" { + if len(m.loops) == 1 { + return m.removeLoop(m.loops[0].id, "Loop "+m.loops[0].id+" stopped."), nil + } + return m.appendLoopSystem("Multiple loops active — use /loop stop or /loop stop all.\n" + m.loopListText()), nil + } + if m.findLoop(id) == nil { + return m.appendLoopSystem("No loop " + id + ". " + m.loopListText()), nil + } + return m.removeLoop(id, "Loop "+id+" stopped."), nil +} + +func (m model) stopAllLoops() (model, tea.Cmd) { + if len(m.loops) == 0 { + return m.appendLoopSystem("No active loops."), nil + } + n := len(m.loops) + m.loops = nil + m.loopSeq++ // invalidate the pending poll tick + m.loopTicking = false + return m.appendLoopSystem(fmt.Sprintf("Stopped all %d loop(s).", n)), nil +} + +// fireDueLoopIfIdle fires the earliest due loop when the session is idle. Called +// from the poll tick; a no-op while a turn, modal, or queued user message is +// pending (the loop simply waits for the next idle tick). +func (m model) fireDueLoopIfIdle() (model, tea.Cmd) { + if m.loopBusy() || len(m.loops) == 0 { + return m, nil + } + now := m.now() + var due *loopState + for _, l := range m.loops { + if l.due(now) && (due == nil || l.nextRunAt.Before(due.nextRunAt)) { + due = l + } + } + if due == nil { + return m, nil + } + m.activeLoopID = due.id + due.nextRunAt = time.Time{} // mark running + m.loopNextWake = 0 + m.loopDone = false + return m.fireLoopPrompt(due.prompt) +} + +// fireLoopPrompt runs a loop's prompt as a turn — a custom /command if it resolves +// to one, otherwise a plain prompt. +func (m model) fireLoopPrompt(prompt string) (model, tea.Cmd) { + if strings.HasPrefix(strings.TrimSpace(prompt), "/") { + if next, teaCmd, ok := m.handleUserCommand(prompt); ok { + return next, teaCmd + } + } + return m.launchPrompt(prompt) +} + +// advanceLoop updates a loop after one of its iterations completes: doom-loop and +// cap checks, then schedules the next wake (or stops the loop). +func (m model) advanceLoop(id, finalAnswer string, runErr error) model { + l := m.findLoop(id) + if l == nil { + return m // stopped mid-run + } + l.iteration++ + res := strings.TrimSpace(finalAnswer) + if res != "" && res == l.lastResult { + l.repeatRun++ + } else { + l.repeatRun = 0 + } + l.lastResult = res + + switch { + case m.loopDone: + return m.removeLoop(id, fmt.Sprintf("Loop %s finished — the task reported done after %d iteration(s).", id, l.iteration)) + case l.iteration >= l.iterationCap(): + return m.removeLoop(id, fmt.Sprintf("Loop %s stopped at its %d-iteration cap.", id, l.iteration)) + case l.repeatRun >= loopDoomThreshold: + return m.removeLoop(id, fmt.Sprintf("Loop %s stopped — %d identical results in a row (no progress).", id, l.repeatRun)) + } + + if l.mode == loopModeSelfPaced { + l.nextRunAt = m.now().Add(clampSelfPaceDelay(m.loopNextWake)) + } else { + l.nextRunAt = m.now().Add(l.interval) + } + return m +} + +func (m model) findLoop(id string) *loopState { + for _, l := range m.loops { + if l.id == id { + return l + } + } + return nil +} + +func (m model) removeLoop(id, note string) model { + kept := make([]*loopState, 0, len(m.loops)) + for _, l := range m.loops { + if l.id != id { + kept = append(kept, l) + } + } + m.loops = kept + m.loopSeq++ // invalidate the pending poll tick; a fresh one is scheduled if loops remain + m.loopTicking = false + if note != "" { + m = m.appendLoopSystem(note) + } + return m +} + +// ensureLoopTick starts the idle poll ticker if it is not already running. +func (m model) ensureLoopTick() (model, tea.Cmd) { + if m.loopTicking || len(m.loops) == 0 { + return m, nil + } + m.loopTicking = true + return m, m.scheduleLoopTick() +} + +func (m model) scheduleLoopTick() tea.Cmd { + seq := m.loopSeq + return tea.Tick(loopPollInterval, func(time.Time) tea.Msg { + return loopTickMsg{seq: seq} + }) +} + +// loopBusy reports whether the session is too busy for a loop to fire — a turn, +// modal, queued user message, or compaction is in flight. Mirrors the guard in +// launchQueuedMessageIfReady so loops and queued prompts never collide. +func (m model) loopBusy() bool { + return m.pending || m.exiting || m.compactInFlight || m.hasQueuedMessage() || + m.pendingPermission != nil || m.pendingAskUser != nil || m.pendingSpecReview != nil +} + +func (m model) appendLoopSystem(text string) model { + m.transcript = reduceTranscript(m.transcript, transcriptAction{kind: actionAppendSystem, text: text}) + return m +} + +// loopActive reports whether any loop is running (for the footer + leave warning). +func (m model) loopActive() bool { return len(m.loops) > 0 } + +// loopFooterSummary renders the persistent "N loops · next 3:05pm" footer segment, +// or "" when no loops are active. +func (m model) loopFooterSummary() string { + if len(m.loops) == 0 { + return "" + } + var next time.Time + for _, l := range m.loops { + if l.nextRunAt.IsZero() || l.paused { + continue + } + if next.IsZero() || l.nextRunAt.Before(next) { + next = l.nextRunAt + } + } + label := fmt.Sprintf("%d loop", len(m.loops)) + if len(m.loops) != 1 { + label += "s" + } + if next.IsZero() { + return label + } + return label + " · next " + next.Format("3:04pm") +} + +func (m model) loopListText() string { + if len(m.loops) == 0 { + return "No active loops." + } + var b strings.Builder + b.WriteString("Active loops:\n") + now := m.now() + for _, l := range m.loops { + when := "running" + if !l.nextRunAt.IsZero() { + if d := l.nextRunAt.Sub(now); d > 0 { + when = "next in " + formatLoopDuration(d.Round(time.Second)) + } else { + when = "due" + } + } + b.WriteString(fmt.Sprintf(" %s · %s · iter %d · %s · %s\n", + l.id, l.cadenceText(), l.iteration, when, truncateLoopPrompt(l.prompt))) + } + b.WriteString("Stop with /loop stop or /loop stop all.") + return b.String() +} + +func loopUsageText() string { + return "Repeat a prompt or command:\n" + + " /loop 5m /babysit-prs — run a command every 5 minutes\n" + + " /loop watch CI every 2m — trailing interval form\n" + + " /loop keep tidying docs — self-paced (the model picks its own cadence and stops when done)\n" + + " /loop list — show active loops\n" + + " /loop stop [id|all] — stop a loop\n" + + "Loops run while this session is idle and stop when you close it." +} + +// validateLoopTarget rejects looping a built-in command (only a prompt or a custom +// /command may loop). +func validateLoopTarget(prompt string) (string, bool) { + p := strings.TrimSpace(prompt) + if p == "" { + return "Nothing to loop — give a prompt or a /command.", false + } + if strings.HasPrefix(p, "/") { + parsed := parseCommand(p) + if parsed.kind != commandPrompt && parsed.kind != commandUnknown { + return "A loop can only run a prompt or a custom /command, not a built-in like " + strings.Fields(p)[0] + ".", false + } + } + return "", true +} + +func truncateLoopPrompt(prompt string) string { + const max = 48 + p := strings.TrimSpace(strings.ReplaceAll(prompt, "\n", " ")) + runes := []rune(p) + if len(runes) <= max { + return p + } + return strings.TrimSpace(string(runes[:max])) + "…" +} + // clampSelfPaceDelay bounds a model-chosen self-paced delay to the self-pace // band, defaulting an unset/invalid value. func clampSelfPaceDelay(d time.Duration) time.Duration { diff --git a/internal/tui/loop_controller_test.go b/internal/tui/loop_controller_test.go new file mode 100644 index 00000000..1362454d --- /dev/null +++ b/internal/tui/loop_controller_test.go @@ -0,0 +1,146 @@ +package tui + +import ( + "context" + "testing" + "time" +) + +func loopTestModel(t *testing.T, now time.Time) model { + t.Helper() + m := newModel(context.Background(), Options{}) + m.now = func() time.Time { return now } + return m +} + +func startFixedLoop(m model, prompt string, interval time.Duration) model { + m, _ = m.startLoop(loopCommand{action: loopActionStart, mode: loopModeFixed, prompt: prompt, interval: interval}) + return m +} + +func TestStartLoopRegistersAndSchedules(t *testing.T) { + now := time.Date(2026, 7, 5, 12, 0, 0, 0, time.UTC) + m := loopTestModel(t, now) + m = startFixedLoop(m, "check the build", 5*time.Minute) + + if len(m.loops) != 1 { + t.Fatalf("want 1 loop, got %d", len(m.loops)) + } + l := m.loops[0] + if l.mode != loopModeFixed || l.interval != 5*time.Minute || l.prompt != "check the build" { + t.Fatalf("unexpected loop %+v", l) + } + // First iteration is due immediately (nextRunAt == now). + if !l.due(now) { + t.Errorf("first iteration should be due at creation") + } + if !m.loopTicking { + t.Errorf("starting a loop should start the poll ticker") + } +} + +func TestAdvanceLoopSchedulesNextWake(t *testing.T) { + now := time.Date(2026, 7, 5, 12, 0, 0, 0, time.UTC) + m := loopTestModel(t, now) + m = startFixedLoop(m, "poll", 5*time.Minute) + id := m.loops[0].id + + m = m.advanceLoop(id, "did some work", nil) + l := m.loops[0] + if l.iteration != 1 { + t.Fatalf("iteration = %d, want 1", l.iteration) + } + if !l.nextRunAt.Equal(now.Add(5 * time.Minute)) { + t.Fatalf("nextRunAt = %v, want +5m", l.nextRunAt) + } +} + +func TestSelfPacedUsesModelDelay(t *testing.T) { + now := time.Date(2026, 7, 5, 12, 0, 0, 0, time.UTC) + m := loopTestModel(t, now) + m, _ = m.startLoop(loopCommand{action: loopActionStart, mode: loopModeSelfPaced, prompt: "keep tidying"}) + id := m.loops[0].id + m.loopNextWake = 10 * time.Minute // as if set by the loop_control tool + m = m.advanceLoop(id, "progress", nil) + if !m.loops[0].nextRunAt.Equal(now.Add(10 * time.Minute)) { + t.Fatalf("self-paced nextRunAt = %v, want +10m", m.loops[0].nextRunAt) + } +} + +func TestLoopDoneStops(t *testing.T) { + now := time.Date(2026, 7, 5, 12, 0, 0, 0, time.UTC) + m := loopTestModel(t, now) + m = startFixedLoop(m, "finish the task", time.Minute) + id := m.loops[0].id + m.loopDone = true + m = m.advanceLoop(id, "all done", nil) + if len(m.loops) != 0 { + t.Fatalf("loop should stop when the task reports done, %d remain", len(m.loops)) + } +} + +func TestLoopDoomGuardStops(t *testing.T) { + now := time.Date(2026, 7, 5, 12, 0, 0, 0, time.UTC) + m := loopTestModel(t, now) + m = startFixedLoop(m, "spin", time.Minute) + id := m.loops[0].id + for i := 0; i < loopDoomThreshold+1 && len(m.loops) > 0; i++ { + m = m.advanceLoop(id, "identical answer", nil) + } + if len(m.loops) != 0 { + t.Fatalf("doom guard should stop a loop repeating identical results") + } +} + +func TestLoopIterationCapStops(t *testing.T) { + now := time.Date(2026, 7, 5, 12, 0, 0, 0, time.UTC) + m := loopTestModel(t, now) + m = startFixedLoop(m, "bounded", time.Minute) + m.loops[0].maxIter = 3 + id := m.loops[0].id + for i := 0; i < 3 && len(m.loops) > 0; i++ { + m = m.advanceLoop(id, "answer "+string(rune('a'+i)), nil) // distinct answers so doom doesn't fire + } + if len(m.loops) != 0 { + t.Fatalf("loop should stop at its iteration cap") + } +} + +func TestStopLoopByID(t *testing.T) { + now := time.Date(2026, 7, 5, 12, 0, 0, 0, time.UTC) + m := loopTestModel(t, now) + m = startFixedLoop(m, "a", time.Minute) + m = startFixedLoop(m, "b", time.Minute) + id := m.loops[0].id + m, _ = m.stopLoop(id) + if len(m.loops) != 1 || m.findLoop(id) != nil { + t.Fatalf("stopLoop(%s) should remove exactly that loop", id) + } + m, _ = m.stopAllLoops() + if len(m.loops) != 0 { + t.Fatalf("stopAllLoops should clear all loops") + } +} + +func TestFireDueLoopSkipsWhenBusy(t *testing.T) { + now := time.Date(2026, 7, 5, 12, 0, 0, 0, time.UTC) + m := loopTestModel(t, now) + m = startFixedLoop(m, "x", time.Minute) + m.pending = true // a turn is in flight + got, cmd := m.fireDueLoopIfIdle() + if got.activeLoopID != "" || cmd != nil { + t.Fatalf("a due loop must not fire while a turn is pending") + } +} + +func TestValidateLoopTargetRejectsBuiltin(t *testing.T) { + if _, ok := validateLoopTarget("/model list"); ok { + t.Error("looping a built-in command should be rejected") + } + if _, ok := validateLoopTarget("keep improving docs"); !ok { + t.Error("a plain prompt should be a valid loop target") + } + if _, ok := validateLoopTarget(""); ok { + t.Error("an empty target should be rejected") + } +} diff --git a/internal/tui/model.go b/internal/tui/model.go index a60ecb73..a6c0d9e4 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -190,10 +190,23 @@ type model struct { // like a frozen terminal (for ANY provider, not just slow ones). Zero = idle. turnStartedAt time.Time queuedMessage string - exiting bool - runCancel context.CancelFunc - runID int - activeRunID int + // loops holds the session's active /loop definitions (see loop.go). activeLoopID + // tags the in-flight run when it is a loop iteration (empty = a user turn), so the + // completion seam knows whether to advance a loop. loopSeq invalidates a stale + // pending poll tick when loops are stopped; loopTicking guards against scheduling + // a second poll ticker. loopNextWake carries a self-paced iteration's model-chosen + // delay from the loop_control tool back to the completion seam. + loops []*loopState + activeLoopID string + loopSeq int + loopCounter int + loopTicking bool + loopNextWake time.Duration + loopDone bool + exiting bool + runCancel context.CancelFunc + runID int + activeRunID int // flushRunIDs holds the ids of runs cancelled while still in flight, mapped // to the session they were recording into AT CANCEL TIME. Each cancelled // agent goroutine keeps running to completion and returns its accumulated @@ -1896,6 +1909,16 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { m.clearComposer() m.clearSuggestions() return m, nil + case loopTickMsg: + // Idle poll for due loops. A stale tick (loops changed) or an empty loop set + // ends the ticker; otherwise fire the earliest due loop if idle and reschedule. + if msg.seq != m.loopSeq || len(m.loops) == 0 { + m.loopTicking = false + return m, nil + } + var fireCmd tea.Cmd + m, fireCmd = m.fireDueLoopIfIdle() + return m, tea.Batch(fireCmd, m.scheduleLoopTick()) case agentResponseMsg: if msg.runID != m.activeRunID { // A run cancelled while in flight still finishes in its goroutine and @@ -2065,8 +2088,25 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { // complete once the turn settles. var sweepCmd tea.Cmd m, sweepCmd = m.maybeGitSweep() + // If this run was a loop iteration, advance that loop (schedule its next wake + // or stop it). Done before launchQueuedMessageIfReady so a user's queued prompt + // still wins the immediate re-launch; the loop fires on the next idle tick. + var loopTickCmd tea.Cmd + if loopID := m.activeLoopID; loopID != "" { + m.activeLoopID = "" + loopFinalAnswer := "" + for _, row := range msg.rows { + if row.kind == rowAssistant && row.final { + loopFinalAnswer = row.text + } + } + m = m.advanceLoop(loopID, loopFinalAnswer, msg.err) + // advanceLoop -> removeLoop may have stopped the ticker; restart it if + // other loops remain. + m, loopTickCmd = m.ensureLoopTick() + } next, queuedCmd := m.launchQueuedMessageIfReady() - return next, tea.Batch(titleCmd, recapCmd, sweepCmd, queuedCmd) + return next, tea.Batch(titleCmd, recapCmd, sweepCmd, queuedCmd, loopTickCmd) case sessionTitleGeneratedMsg: return m.handleSessionTitleGenerated(msg) case recapGeneratedMsg: @@ -3840,6 +3880,8 @@ func (m model) handleSubmit() (tea.Model, tea.Cmd) { return m, nil } return m.startNewSession(), nil + case commandLoop: + return m.handleLoopCommand(command.text) case commandExit: // /exit gets the same protection as Ctrl+C: cancel any in-flight run and // defer the quit until its checkpoint session events flush — quitting From 440d4df367dfb92579d1e86d2d01d980942b45d5 Mon Sep 17 00:00:00 2001 From: Vasanthdev2004 Date: Sun, 5 Jul 2026 00:27:55 +0530 Subject: [PATCH 3/9] feat(tui): /loop self-paced adaptive cadence + footer summary + /clear note MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third slice of #501. - Self-paced loops now use an adaptive cadence (2m early -> 30m heartbeat as iterations accrue), with the loopNextWake channel kept as an override for a future explicit model signal. - Persistent footer segment "↻ N loops · next 3:05pm" while loops are active. - /clear notes that loops keep running (it wipes the transcript, not the session) so a loop never silently fires into a cleared screen. - Tests for adaptive cadence + footer summary. --- internal/tui/loop.go | 24 +++++++++++++++++++++++- internal/tui/loop_controller_test.go | 23 +++++++++++++++++++++++ internal/tui/model.go | 5 +++++ internal/tui/view.go | 7 +++++++ 4 files changed, 58 insertions(+), 1 deletion(-) diff --git a/internal/tui/loop.go b/internal/tui/loop.go index b716f0ee..886c0529 100644 --- a/internal/tui/loop.go +++ b/internal/tui/loop.go @@ -360,13 +360,35 @@ func (m model) advanceLoop(id, finalAnswer string, runErr error) model { } if l.mode == loopModeSelfPaced { - l.nextRunAt = m.now().Add(clampSelfPaceDelay(m.loopNextWake)) + // A model-chosen delay (loopNextWake, set via the loop_control channel) wins; + // otherwise fall back to an adaptive cadence that checks back often while work + // is fresh and settles to a slow heartbeat as iterations accrue. + delay := m.loopNextWake + if delay <= 0 { + delay = adaptiveSelfPaceDelay(l.iteration) + } + l.nextRunAt = m.now().Add(clampSelfPaceDelay(delay)) } else { l.nextRunAt = m.now().Add(l.interval) } return m } +// adaptiveSelfPaceDelay picks a self-paced loop's next-wake by iteration: frequent +// early (work is actively progressing), slowing to a heartbeat as it matures. +func adaptiveSelfPaceDelay(iteration int) time.Duration { + switch { + case iteration < 2: + return 2 * time.Minute + case iteration < 5: + return 5 * time.Minute + case iteration < 10: + return 15 * time.Minute + default: + return 30 * time.Minute + } +} + func (m model) findLoop(id string) *loopState { for _, l := range m.loops { if l.id == id { diff --git a/internal/tui/loop_controller_test.go b/internal/tui/loop_controller_test.go index 1362454d..feea8fa5 100644 --- a/internal/tui/loop_controller_test.go +++ b/internal/tui/loop_controller_test.go @@ -133,6 +133,29 @@ func TestFireDueLoopSkipsWhenBusy(t *testing.T) { } } +func TestAdaptiveSelfPaceDelayWidensOverTime(t *testing.T) { + if adaptiveSelfPaceDelay(0) >= adaptiveSelfPaceDelay(5) { + t.Error("early iterations should check back sooner than later ones") + } + if adaptiveSelfPaceDelay(20) != 30*time.Minute { + t.Errorf("a mature loop should settle to a 30m heartbeat, got %v", adaptiveSelfPaceDelay(20)) + } +} + +func TestLoopFooterSummary(t *testing.T) { + now := time.Date(2026, 7, 5, 15, 0, 0, 0, time.UTC) + m := loopTestModel(t, now) + if m.loopFooterSummary() != "" { + t.Error("no loops -> empty footer summary") + } + m = startFixedLoop(m, "a", 5*time.Minute) + m.loops[0].nextRunAt = now.Add(5 * time.Minute) + got := m.loopFooterSummary() + if got == "" || got[:6] != "1 loop" { + t.Fatalf("footer summary = %q, want it to start with a loop count", got) + } +} + func TestValidateLoopTargetRejectsBuiltin(t *testing.T) { if _, ok := validateLoopTarget("/model list"); ok { t.Error("looping a built-in command should be rejected") diff --git a/internal/tui/model.go b/internal/tui/model.go index a6c0d9e4..fcc96ca1 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -3870,6 +3870,11 @@ func (m model) handleSubmit() (tea.Model, tea.Cmd) { // Scrollback above can't be un-printed; a faint divider marks where the // cleared surface ended and the frontier restarts for the fresh transcript. m.resetFlushFrontier("· cleared ·") + // /clear wipes the transcript but not the session, so loops keep running; + // say so rather than let them silently fire into a "cleared" screen. + if m.loopActive() { + m = m.appendLoopSystem(m.loopFooterSummary() + " still running — /loop stop all to end them.") + } return m, nil case commandNew: // A fresh session mid-run would strand the in-flight turn's events; make the diff --git a/internal/tui/view.go b/internal/tui/view.go index b1a7e557..0057e5f4 100644 --- a/internal/tui/view.go +++ b/internal/tui/view.go @@ -218,6 +218,13 @@ func (m model) statusLine(width int) string { } else if summary := m.backgroundTerminalSummary(); summary != "" { left += separator + zeroTheme.muted.Render(summary) } + // Active loops surface a persistent "↻ N loops · next 3:05pm" segment so a + // running loop is always visible (hidden during an exit/cancel confirm above). + if !m.exitConfirmActive && !m.cancelConfirmActive { + if loopSummary := m.loopFooterSummary(); loopSummary != "" { + left += separator + zeroTheme.accent.Render("↻ ") + zeroTheme.muted.Render(loopSummary) + } + } rightGroups := []string{} // Context-fill gauge: surface it down to the narrow tier (where it matters From 4ffbf4e0fef8be0ce7827ca56245c8a8d47d11dc Mon Sep 17 00:00:00 2001 From: Vasanthdev2004 Date: Sun, 5 Jul 2026 00:29:17 +0530 Subject: [PATCH 4/9] test(tui): drive /loop end-to-end through Update (submit + stale tick) --- internal/tui/loop_controller_test.go | 43 ++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/internal/tui/loop_controller_test.go b/internal/tui/loop_controller_test.go index feea8fa5..65c8b3a2 100644 --- a/internal/tui/loop_controller_test.go +++ b/internal/tui/loop_controller_test.go @@ -4,6 +4,8 @@ import ( "context" "testing" "time" + + tea "charm.land/bubbletea/v2" ) func loopTestModel(t *testing.T, now time.Time) model { @@ -156,6 +158,47 @@ func TestLoopFooterSummary(t *testing.T) { } } +func TestLoopCommandThroughSubmit(t *testing.T) { + now := time.Date(2026, 7, 5, 12, 0, 0, 0, time.UTC) + m := loopTestModel(t, now) + m.input.SetValue("/loop 5m tidy the imports") + updated, _ := m.Update(testKey(tea.KeyEnter)) + next := updated.(model) + if len(next.loops) != 1 { + t.Fatalf("submitting /loop should create a loop, got %d", len(next.loops)) + } + if next.loops[0].mode != loopModeFixed || next.loops[0].interval != 5*time.Minute { + t.Fatalf("unexpected loop from submit: %+v", next.loops[0]) + } + if !next.loopTicking { + t.Fatalf("submitting /loop should start the poll ticker") + } + + // /loop stop all through the same path clears it. + next.input.SetValue("/loop stop all") + updated2, _ := next.Update(testKey(tea.KeyEnter)) + final := updated2.(model) + if len(final.loops) != 0 { + t.Fatalf("/loop stop all should clear loops, got %d", len(final.loops)) + } +} + +func TestStaleLoopTickIgnored(t *testing.T) { + now := time.Date(2026, 7, 5, 12, 0, 0, 0, time.UTC) + m := loopTestModel(t, now) + m = startFixedLoop(m, "x", time.Minute) + m.loopSeq = 7 + // A tick from before the seq changed must be dropped and stop the ticker. + updated, cmd := m.Update(loopTickMsg{seq: 3}) + next := updated.(model) + if next.loopTicking { + t.Error("a stale loop tick should stop the ticker") + } + if cmd != nil { + t.Error("a stale loop tick should not reschedule") + } +} + func TestValidateLoopTargetRejectsBuiltin(t *testing.T) { if _, ok := validateLoopTarget("/model list"); ok { t.Error("looping a built-in command should be rejected") From 9cfdb2fecf0ad4544b2220990f12f2b710771105 Mon Sep 17 00:00:00 2001 From: Vasanthdev2004 Date: Sun, 5 Jul 2026 00:29:58 +0530 Subject: [PATCH 5/9] docs(readme): add /loop to the slash-command table --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index b75c4851..ff32df02 100644 --- a/README.md +++ b/README.md @@ -186,6 +186,7 @@ Common slash commands: | `/spec`, `/plan` | draft and review a plan before building | | `/image` | attach an image for vision-capable models | | `/resume`, `/rewind` | continue or roll back local sessions | +| `/loop` | repeat a prompt or command on an interval (`/loop 5m /babysit-prs`) or self-paced | | `/compact`, `/context` | manage context usage | | `/permissions`, `/tools` | inspect available tools and policy | | `/add-dir` | allow an extra write directory for this session | From 3eafaf584dcb45019c85a0b394fc514b6e73600c Mon Sep 17 00:00:00 2001 From: Vasanthdev2004 Date: Sun, 5 Jul 2026 01:02:13 +0530 Subject: [PATCH 6/9] fix(tui): stop looping on repeated failure + clear loop tag on cancel Addresses CodeRabbit review on #502: - advanceLoop now counts consecutive failed iterations (loopMaxFailures=3) and stops the loop, so a loop whose turns keep erroring no longer churns to the iteration cap (the doom guard only sees identical non-empty answers). - cancelRun clears activeLoopID and re-arms the interrupted loop for its next cadence, so a cancelled iteration is not left running forever and the next unrelated turn is not misattributed as that loop's completion. --- internal/tui/loop.go | 18 +++++++++- internal/tui/loop_controller_test.go | 54 ++++++++++++++++++++++++++++ internal/tui/model.go | 15 ++++++++ 3 files changed, 86 insertions(+), 1 deletion(-) diff --git a/internal/tui/loop.go b/internal/tui/loop.go index 886c0529..6231d01d 100644 --- a/internal/tui/loop.go +++ b/internal/tui/loop.go @@ -44,6 +44,12 @@ const ( // no-progress result should stop rather than churn. loopDoomThreshold = 5 + // loopMaxFailures ends a loop after this many consecutive failed iterations, so + // a loop whose iterations keep erroring stops instead of churning to the + // iteration cap (a failed turn has no final answer, so the doom guard can't see + // the lack of progress). + loopMaxFailures = 3 + // loopPollInterval is how often the controller checks whether a loop is due. // A loop only fires while idle, so this is a cheap between-turn poll. loopPollInterval = 1 * time.Second @@ -63,7 +69,10 @@ type loopState struct { // final answers past loopDoomThreshold end the loop. lastResult string repeatRun int - paused bool + // failRun counts consecutive failed iterations; past loopMaxFailures the loop + // stops (a failed turn has no final answer for the doom guard to compare). + failRun int + paused bool } func (l *loopState) iterationCap() int { @@ -342,6 +351,11 @@ func (m model) advanceLoop(id, finalAnswer string, runErr error) model { return m // stopped mid-run } l.iteration++ + if runErr != nil { + l.failRun++ + } else { + l.failRun = 0 + } res := strings.TrimSpace(finalAnswer) if res != "" && res == l.lastResult { l.repeatRun++ @@ -353,6 +367,8 @@ func (m model) advanceLoop(id, finalAnswer string, runErr error) model { switch { case m.loopDone: return m.removeLoop(id, fmt.Sprintf("Loop %s finished — the task reported done after %d iteration(s).", id, l.iteration)) + case l.failRun >= loopMaxFailures: + return m.removeLoop(id, fmt.Sprintf("Loop %s stopped after %d consecutive failed iterations.", id, l.failRun)) case l.iteration >= l.iterationCap(): return m.removeLoop(id, fmt.Sprintf("Loop %s stopped at its %d-iteration cap.", id, l.iteration)) case l.repeatRun >= loopDoomThreshold: diff --git a/internal/tui/loop_controller_test.go b/internal/tui/loop_controller_test.go index 65c8b3a2..0e93392f 100644 --- a/internal/tui/loop_controller_test.go +++ b/internal/tui/loop_controller_test.go @@ -2,6 +2,7 @@ package tui import ( "context" + "errors" "testing" "time" @@ -199,6 +200,59 @@ func TestStaleLoopTickIgnored(t *testing.T) { } } +func TestLoopStopsAfterConsecutiveFailures(t *testing.T) { + now := time.Date(2026, 7, 5, 12, 0, 0, 0, time.UTC) + m := loopTestModel(t, now) + m = startFixedLoop(m, "flaky", time.Minute) + id := m.loops[0].id + boom := errors.New("boom") + for i := 0; i < loopMaxFailures && len(m.loops) > 0; i++ { + m = m.advanceLoop(id, "", boom) + } + if len(m.loops) != 0 { + t.Fatalf("loop should stop after %d consecutive failed iterations", loopMaxFailures) + } +} + +func TestLoopFailureCounterResetsOnSuccess(t *testing.T) { + now := time.Date(2026, 7, 5, 12, 0, 0, 0, time.UTC) + m := loopTestModel(t, now) + m = startFixedLoop(m, "recovers", time.Minute) + id := m.loops[0].id + m = m.advanceLoop(id, "", errors.New("x")) + m = m.advanceLoop(id, "", errors.New("x")) + m = m.advanceLoop(id, "back to healthy", nil) // a success resets the streak + if len(m.loops) != 1 { + t.Fatalf("a recovered failure streak should not stop the loop") + } + if m.loops[0].failRun != 0 { + t.Fatalf("a successful iteration should reset failRun, got %d", m.loops[0].failRun) + } +} + +func TestCancelRunClearsActiveLoopAndRearms(t *testing.T) { + now := time.Date(2026, 7, 5, 12, 0, 0, 0, time.UTC) + m := loopTestModel(t, now) + m = startFixedLoop(m, "x", 5*time.Minute) + m.activeLoopID = m.loops[0].id + m.loops[0].nextRunAt = time.Time{} // as if the iteration is running + m.pending = true + m.activeRunID = 7 + m.runCancel = func() {} + + m.cancelRun() + + if m.activeLoopID != "" { + t.Fatal("cancel should clear the active-loop tag so the next turn isn't misattributed") + } + if m.loops[0].nextRunAt.IsZero() { + t.Fatal("cancel should re-arm the interrupted loop, not leave it running forever") + } + if !m.loops[0].nextRunAt.Equal(now.Add(5 * time.Minute)) { + t.Fatalf("re-armed nextRunAt = %v, want +5m", m.loops[0].nextRunAt) + } +} + func TestValidateLoopTargetRejectsBuiltin(t *testing.T) { if _, ok := validateLoopTarget("/model list"); ok { t.Error("looping a built-in command should be rejected") diff --git a/internal/tui/model.go b/internal/tui/model.go index fcc96ca1..89e34505 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -4380,6 +4380,21 @@ func (m *model) cancelRun() { m.runCancel() } m.clearStreamingToolCall() // a cancelled file-write must not linger into the next run + // A cancelled loop iteration bypasses the agentResponseMsg completion seam (its + // late message is drained through flushRunIDs, not advanceLoop), so clear the + // loop tag here and re-arm the interrupted loop for its next cadence. Otherwise + // the loop is left "running" forever (nextRunAt stays zero) and the NEXT + // unrelated turn would be misattributed as this loop's completion. + if m.activeLoopID != "" { + if l := m.findLoop(m.activeLoopID); l != nil { + if l.mode == loopModeSelfPaced { + l.nextRunAt = m.now().Add(clampSelfPaceDelay(adaptiveSelfPaceDelay(l.iteration))) + } else { + l.nextRunAt = m.now().Add(l.interval) + } + } + m.activeLoopID = "" + } // Remember the in-flight run — and the session it was recording into — so // its final agentResponseMsg is still drained for session-event persistence // after activeRunID is cleared. Otherwise the checkpoint blobs it captured From de7f0b6a5b4663c2b6fa57fbaeeb67bdb0047c51 Mon Sep 17 00:00:00 2001 From: Vasanthdev2004 Date: Sun, 5 Jul 2026 11:29:23 +0530 Subject: [PATCH 7/9] =?UTF-8?q?fix(tui):=20finish=20/loop=20v1=20=E2=80=94?= =?UTF-8?q?=20real=20self-paced=20completion,=20session=20scope,=20leave?= =?UTF-8?q?=20gate?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Response to the review. The self-paced path was the real gap: it advertised model-chosen completion but nothing ever wrote the done/next-wake fields, so it was just adaptive polling dressed up. Replaced that with an actual control channel — each self-paced iteration runs a short maintenance playbook and ends its reply with a control line (LOOP: DONE / LOOP: CONTINUE [interval]) that advanceLoop parses to stop the loop or pick the next wake. The two dead model fields are gone; done and next-wake now come from the model's output. Rest of the review: - loops stop on a session switch (/new, /resume) instead of firing the previous conversation's prompt into the new one - validateLoopTarget resolves custom /commands up front and rejects unknowns, so a mistyped `/loop 5m /nope` isn't scheduled and then re-sent as literal text - a one-shot confirm gates /clear and /quit while loops are active - age-based auto-expire (24h) next to the iteration cap - the two CodeRabbit test nits (HasPrefix footer check, bare `/loop stop` on a single loop) Hardening from a self-review pass over the above: - normalize CRLF and allow a trailing note on the control line, so `LOOP: DONE — all clean` and \r\n replies still stop the loop - disarm the leave-confirm before the queue/exit early returns, so an interposed prompt can't leave it falsely armed - a loop no longer fires behind an open picker/modal; a run launched behind the /resume picker would otherwise complete into whatever session you switch to --- internal/tui/loop.go | 156 ++++++++++++++++++++---- internal/tui/loop_controller_test.go | 170 +++++++++++++++++++++++++-- internal/tui/loop_test.go | 45 +++++++ internal/tui/model.go | 48 ++++++-- internal/tui/session.go | 12 ++ 5 files changed, 384 insertions(+), 47 deletions(-) diff --git a/internal/tui/loop.go b/internal/tui/loop.go index 6231d01d..6be01fdf 100644 --- a/internal/tui/loop.go +++ b/internal/tui/loop.go @@ -13,13 +13,17 @@ import ( // The /loop command repeats a prompt or slash command in one of two modes: // // - Fixed interval — `/loop 5m ` re-fires on a wall-clock cadence. -// - Self-paced — bare `/loop ` lets the model finish an iteration, -// pick its own next wake (via the loop_control tool), and stop when done. +// - Self-paced — bare `/loop ` runs a maintenance playbook: the model +// does one increment of work each iteration and ends its reply with a control +// line (`LOOP: DONE` / `LOOP: CONTINUE [interval]`) that stops the loop or picks +// the next wake. With no control line it falls back to an adaptive cadence. // // Loops are foreground and session-scoped: a tick only fires while the session is -// idle between turns (it never interrupts a streaming turn), and the loop lives -// with its session. Everything here is pure/loop-local; the wiring into the turn -// lifecycle (ticker, launch, continuation) lives in model.go. +// idle between turns (it never interrupts a streaming turn), the loop lives with the +// session that created it, and switching sessions (/new, /resume) stops it. Safety +// rails cap iterations, age, consecutive failures, and no-progress repetition. +// Everything here is pure/loop-local; the turn-lifecycle wiring (ticker, launch, +// continuation) lives in model.go. type loopMode int @@ -35,6 +39,10 @@ const ( loopMaxInterval = 24 * time.Hour loopMaxIterations = 500 // hard ceiling; the loop auto-stops after this many + // loopMaxAge auto-expires a loop this long after it was created, so a forgotten + // loop stops on its own even if it never reaches the iteration cap. + loopMaxAge = 24 * time.Hour + loopSelfPaceMin = 1 * time.Minute loopSelfPaceMax = 1 * time.Hour loopSelfPaceDefault = 15 * time.Minute @@ -121,6 +129,13 @@ type loopCommand struct { var loopIntervalRe = regexp.MustCompile(`^(\d+)(s|m|h|d)$`) +// loopControlRe matches a self-paced loop's control line at the start of a line, +// tolerant of a leading markdown bullet/quote marker and any trailing note after +// the keyword: "LOOP: DONE", "- loop: continue", "LOOP: CONTINUE 10m — still tidying". +// The leading ^ anchor keeps a mid-sentence mention ("I'll emit LOOP: DONE when…") +// from matching; parseLoopControl normalizes CRLF first so `$` can reach line end. +var loopControlRe = regexp.MustCompile(`(?im)^[^\S\r\n]*(?:[-*>•][^\S\r\n]*)?loop:[^\S\r\n]*(done|continue)\b(?:[^\S\r\n]+(\d+[smhd])\b)?.*$`) + // parseLoopCommand interprets the arguments to `/loop`. Forms: // // (empty) -> usage/list @@ -256,7 +271,7 @@ func (m model) handleLoopCommand(args string) (model, tea.Cmd) { // startLoop registers a new loop and (re)starts the idle poll ticker. The first // iteration fires on the next idle tick (nextRunAt = now). func (m model) startLoop(cmd loopCommand) (model, tea.Cmd) { - if reason, ok := validateLoopTarget(cmd.prompt); !ok { + if reason, ok := m.validateLoopTarget(cmd.prompt); !ok { return m.appendLoopSystem(reason), nil } m.loopCounter++ @@ -327,9 +342,18 @@ func (m model) fireDueLoopIfIdle() (model, tea.Cmd) { } m.activeLoopID = due.id due.nextRunAt = time.Time{} // mark running - m.loopNextWake = 0 - m.loopDone = false - return m.fireLoopPrompt(due.prompt) + return m.fireLoopPrompt(due.firePrompt()) +} + +// firePrompt is the text actually sent for this iteration. A self-paced plain +// prompt is wrapped in the maintenance playbook so the model knows to do one +// increment of work and end with a LOOP: control line; a /command or a +// fixed-interval prompt is sent verbatim. +func (l *loopState) firePrompt() string { + if l.mode == loopModeSelfPaced && !strings.HasPrefix(strings.TrimSpace(l.prompt), "/") { + return loopSelfPacePlaybook(l) + } + return l.prompt } // fireLoopPrompt runs a loop's prompt as a turn — a custom /command if it resolves @@ -343,8 +367,10 @@ func (m model) fireLoopPrompt(prompt string) (model, tea.Cmd) { return m.launchPrompt(prompt) } -// advanceLoop updates a loop after one of its iterations completes: doom-loop and -// cap checks, then schedules the next wake (or stops the loop). +// advanceLoop updates a loop after one of its iterations completes: it folds the +// result into the failure/no-progress guards, honors a self-paced DONE/next-wake +// control line, enforces the iteration and age caps, then schedules the next wake +// (or stops the loop). func (m model) advanceLoop(id, finalAnswer string, runErr error) model { l := m.findLoop(id) if l == nil { @@ -364,22 +390,31 @@ func (m model) advanceLoop(id, finalAnswer string, runErr error) model { } l.lastResult = res + // Only self-paced iterations run the playbook, so only they can carry a control + // line; a failed turn has no trustworthy final answer to read one from. + done, wake := false, time.Duration(0) + if l.mode == loopModeSelfPaced && runErr == nil { + done, wake = parseLoopControl(finalAnswer) + } + switch { - case m.loopDone: + case done: return m.removeLoop(id, fmt.Sprintf("Loop %s finished — the task reported done after %d iteration(s).", id, l.iteration)) case l.failRun >= loopMaxFailures: return m.removeLoop(id, fmt.Sprintf("Loop %s stopped after %d consecutive failed iterations.", id, l.failRun)) case l.iteration >= l.iterationCap(): return m.removeLoop(id, fmt.Sprintf("Loop %s stopped at its %d-iteration cap.", id, l.iteration)) + case !l.createdAt.IsZero() && m.now().Sub(l.createdAt) >= loopMaxAge: + return m.removeLoop(id, fmt.Sprintf("Loop %s expired after %s.", id, formatLoopDuration(loopMaxAge))) case l.repeatRun >= loopDoomThreshold: return m.removeLoop(id, fmt.Sprintf("Loop %s stopped — %d identical results in a row (no progress).", id, l.repeatRun)) } if l.mode == loopModeSelfPaced { - // A model-chosen delay (loopNextWake, set via the loop_control channel) wins; - // otherwise fall back to an adaptive cadence that checks back often while work - // is fresh and settles to a slow heartbeat as iterations accrue. - delay := m.loopNextWake + // A model-chosen next wake (from the control line) wins; otherwise fall back to + // an adaptive cadence that checks back often while work is fresh and settles to + // a slow heartbeat as iterations accrue. + delay := wake if delay <= 0 { delay = adaptiveSelfPaceDelay(l.iteration) } @@ -390,6 +425,49 @@ func (m model) advanceLoop(id, finalAnswer string, runErr error) model { return m } +// parseLoopControl reads a self-paced loop's control signal from the model's final +// answer. The last control line wins (the model may reference the protocol before +// emitting it). Returns done=true to stop the loop, or a next-wake hint (0 = none). +func parseLoopControl(answer string) (done bool, wake time.Duration) { + // Normalize CRLF/CR so the line-end anchor can reach `$` (Go's (?m)$ sits right + // before `\n`, and the horizontal-whitespace class can't consume a stray `\r`). + norm := strings.ReplaceAll(answer, "\r\n", "\n") + norm = strings.ReplaceAll(norm, "\r", "\n") + matches := loopControlRe.FindAllStringSubmatch(norm, -1) + if len(matches) == 0 { + return false, 0 + } + last := matches[len(matches)-1] + if strings.EqualFold(last[1], "done") { + return true, 0 + } + if last[2] != "" { + if d, ok := parseLoopInterval(last[2]); ok { + return false, d + } + } + return false, 0 +} + +// loopSelfPacePlaybook wraps a self-paced loop's goal in the maintenance-loop +// instructions the model follows each iteration: do one real increment of work, +// then emit a single control line so the harness knows whether to stop or when to +// wake next — the self-paced maintenance playbook the feature ships with. +func loopSelfPacePlaybook(l *loopState) string { + var b strings.Builder + b.WriteString("You are running in a self-paced maintenance loop (iteration ") + b.WriteString(strconv.Itoa(l.iteration + 1)) + b.WriteString("). Do one concrete increment of work toward the goal below — actual work, not just a plan — then stop and report.\n\n") + b.WriteString("Goal: ") + b.WriteString(strings.TrimSpace(l.prompt)) + b.WriteString("\n\nEnd your reply with a control line — on its own line, starting with \"LOOP:\":\n") + b.WriteString(" LOOP: DONE — the goal is fully met; the loop stops.\n") + b.WriteString(" LOOP: CONTINUE — more to do; the loop wakes again automatically.\n") + b.WriteString(" LOOP: CONTINUE 10m — more to do; wake after the delay you name (e.g. 5m, 1h).\n") + b.WriteString("A short note after the keyword is fine. Report DONE only when the goal is genuinely complete — the loop trusts that signal to stop.") + return b.String() +} + // adaptiveSelfPaceDelay picks a self-paced loop's next-wake by iteration: frequent // early (work is actively progressing), slowing to a heartbeat as it matures. func adaptiveSelfPaceDelay(iteration int) time.Duration { @@ -447,11 +525,14 @@ func (m model) scheduleLoopTick() tea.Cmd { } // loopBusy reports whether the session is too busy for a loop to fire — a turn, -// modal, queued user message, or compaction is in flight. Mirrors the guard in -// launchQueuedMessageIfReady so loops and queued prompts never collide. +// queued user message, compaction, or ANY blocking modal (permission/askUser/spec +// review, provider or MCP wizard, MCP manager, or an open picker) is in flight. +// The modal check is load-bearing: a loop must not launch a run behind an open +// /resume picker, or that run would complete into whatever session the user then +// switches to. Mirrors launchQueuedMessageIfReady so loops and queued prompts never +// collide. func (m model) loopBusy() bool { - return m.pending || m.exiting || m.compactInFlight || m.hasQueuedMessage() || - m.pendingPermission != nil || m.pendingAskUser != nil || m.pendingSpecReview != nil + return m.pending || m.exiting || m.compactInFlight || m.hasQueuedMessage() || !m.noBlockingModal() } func (m model) appendLoopSystem(text string) model { @@ -520,22 +601,47 @@ func loopUsageText() string { "Loops run while this session is idle and stop when you close it." } -// validateLoopTarget rejects looping a built-in command (only a prompt or a custom -// /command may loop). -func validateLoopTarget(prompt string) (string, bool) { +// validateLoopTarget rejects looping a built-in command or an unresolved custom +// command — only a prompt or an existing custom /command (.zero/commands/.md) +// may loop. Resolving custom commands up front stops a typo'd `/loop 5m /nope` from +// being scheduled and then re-sent as literal text on every tick. +func (m model) validateLoopTarget(prompt string) (string, bool) { p := strings.TrimSpace(prompt) if p == "" { return "Nothing to loop — give a prompt or a /command.", false } if strings.HasPrefix(p, "/") { - parsed := parseCommand(p) - if parsed.kind != commandPrompt && parsed.kind != commandUnknown { + switch parseCommand(p).kind { + case commandPrompt: + // A bare "/" or similar that isn't a command — treat as a plain prompt. + case commandUnknown: + name, _ := splitUserCommand(p) + if _, ok := m.lookupUserCommand(name); !ok { + return "No command /" + name + " — a loop can only run a prompt or an existing custom /command.", false + } + default: return "A loop can only run a prompt or a custom /command, not a built-in like " + strings.Fields(p)[0] + ".", false } } return "", true } +// clearLoopsForSessionSwitch drops every active loop and its ticker. Loops belong to +// the session that created them; carrying them across /new or /resume would fire the +// old session's prompt into an unrelated conversation. Returns the count cleared so +// the caller can note it. Pure state reset — no transcript writes. +func (m model) clearLoopsForSessionSwitch() (model, int) { + n := len(m.loops) + if n == 0 { + return m, 0 + } + m.loops = nil + m.activeLoopID = "" + m.loopTicking = false + m.loopSeq++ // invalidate any pending poll tick + return m, n +} + func truncateLoopPrompt(prompt string) string { const max = 48 p := strings.TrimSpace(strings.ReplaceAll(prompt, "\n", " ")) diff --git a/internal/tui/loop_controller_test.go b/internal/tui/loop_controller_test.go index 0e93392f..6d1f15b2 100644 --- a/internal/tui/loop_controller_test.go +++ b/internal/tui/loop_controller_test.go @@ -3,10 +3,13 @@ package tui import ( "context" "errors" + "strings" "testing" "time" tea "charm.land/bubbletea/v2" + + "github.com/Gitlawb/zero/internal/usercommands" ) func loopTestModel(t *testing.T, now time.Time) model { @@ -63,8 +66,11 @@ func TestSelfPacedUsesModelDelay(t *testing.T) { m := loopTestModel(t, now) m, _ = m.startLoop(loopCommand{action: loopActionStart, mode: loopModeSelfPaced, prompt: "keep tidying"}) id := m.loops[0].id - m.loopNextWake = 10 * time.Minute // as if set by the loop_control tool - m = m.advanceLoop(id, "progress", nil) + // The model carries its chosen next wake in the reply's control line. + m = m.advanceLoop(id, "made progress\nLOOP: CONTINUE 10m", nil) + if len(m.loops) == 0 { + t.Fatal("a CONTINUE control line should keep the loop running") + } if !m.loops[0].nextRunAt.Equal(now.Add(10 * time.Minute)) { t.Fatalf("self-paced nextRunAt = %v, want +10m", m.loops[0].nextRunAt) } @@ -73,15 +79,42 @@ func TestSelfPacedUsesModelDelay(t *testing.T) { func TestLoopDoneStops(t *testing.T) { now := time.Date(2026, 7, 5, 12, 0, 0, 0, time.UTC) m := loopTestModel(t, now) - m = startFixedLoop(m, "finish the task", time.Minute) + m, _ = m.startLoop(loopCommand{action: loopActionStart, mode: loopModeSelfPaced, prompt: "finish the task"}) id := m.loops[0].id - m.loopDone = true - m = m.advanceLoop(id, "all done", nil) + m = m.advanceLoop(id, "all done\nLOOP: DONE", nil) if len(m.loops) != 0 { t.Fatalf("loop should stop when the task reports done, %d remain", len(m.loops)) } } +func TestFixedLoopIgnoresControlLine(t *testing.T) { + now := time.Date(2026, 7, 5, 12, 0, 0, 0, time.UTC) + m := loopTestModel(t, now) + m = startFixedLoop(m, "watch CI", time.Minute) + id := m.loops[0].id + // A fixed-interval loop never runs the playbook, so a stray control line in the + // output is not a completion signal — it keeps its wall-clock cadence. + m = m.advanceLoop(id, "LOOP: DONE", nil) + if len(m.loops) != 1 { + t.Fatal("a fixed-interval loop should ignore a control line and keep running") + } + if !m.loops[0].nextRunAt.Equal(now.Add(time.Minute)) { + t.Fatalf("fixed nextRunAt = %v, want +1m", m.loops[0].nextRunAt) + } +} + +func TestLoopExpiresByAge(t *testing.T) { + now := time.Date(2026, 7, 5, 12, 0, 0, 0, time.UTC) + m := loopTestModel(t, now) + m = startFixedLoop(m, "forgotten", time.Minute) + id := m.loops[0].id + m.loops[0].createdAt = now.Add(-loopMaxAge - time.Hour) // created past the age cap + m = m.advanceLoop(id, "still going", nil) + if len(m.loops) != 0 { + t.Fatal("a loop past its age cap should auto-expire") + } +} + func TestLoopDoomGuardStops(t *testing.T) { now := time.Date(2026, 7, 5, 12, 0, 0, 0, time.UTC) m := loopTestModel(t, now) @@ -119,6 +152,15 @@ func TestStopLoopByID(t *testing.T) { if len(m.loops) != 1 || m.findLoop(id) != nil { t.Fatalf("stopLoop(%s) should remove exactly that loop", id) } + // Bare stop with a single active loop stops that sole loop (parseLoopCommand + // leaves the target empty and stopLoop special-cases the one-loop case). + soleID := m.loops[0].id + m, _ = m.stopLoop("") + if len(m.loops) != 0 || m.findLoop(soleID) != nil { + t.Fatalf("bare stop should remove the sole active loop") + } + m = startFixedLoop(m, "c", time.Minute) + m = startFixedLoop(m, "d", time.Minute) m, _ = m.stopAllLoops() if len(m.loops) != 0 { t.Fatalf("stopAllLoops should clear all loops") @@ -136,6 +178,17 @@ func TestFireDueLoopSkipsWhenBusy(t *testing.T) { } } +func TestFireDueLoopSkipsBehindModal(t *testing.T) { + now := time.Date(2026, 7, 5, 12, 0, 0, 0, time.UTC) + m := loopTestModel(t, now) + m = startFixedLoop(m, "poll", time.Minute) // due immediately + m.picker = &commandPicker{} // e.g. an open /resume session picker + got, cmd := m.fireDueLoopIfIdle() + if got.activeLoopID != "" || cmd != nil { + t.Fatal("a due loop must not launch a run behind an open picker/modal — it would complete into whatever session the user switches to") + } +} + func TestAdaptiveSelfPaceDelayWidensOverTime(t *testing.T) { if adaptiveSelfPaceDelay(0) >= adaptiveSelfPaceDelay(5) { t.Error("early iterations should check back sooner than later ones") @@ -154,7 +207,7 @@ func TestLoopFooterSummary(t *testing.T) { m = startFixedLoop(m, "a", 5*time.Minute) m.loops[0].nextRunAt = now.Add(5 * time.Minute) got := m.loopFooterSummary() - if got == "" || got[:6] != "1 loop" { + if !strings.HasPrefix(got, "1 loop") { t.Fatalf("footer summary = %q, want it to start with a loop count", got) } } @@ -254,13 +307,112 @@ func TestCancelRunClearsActiveLoopAndRearms(t *testing.T) { } func TestValidateLoopTargetRejectsBuiltin(t *testing.T) { - if _, ok := validateLoopTarget("/model list"); ok { + now := time.Date(2026, 7, 5, 12, 0, 0, 0, time.UTC) + m := loopTestModel(t, now) + if _, ok := m.validateLoopTarget("/model list"); ok { t.Error("looping a built-in command should be rejected") } - if _, ok := validateLoopTarget("keep improving docs"); !ok { + if _, ok := m.validateLoopTarget("keep improving docs"); !ok { t.Error("a plain prompt should be a valid loop target") } - if _, ok := validateLoopTarget(""); ok { + if _, ok := m.validateLoopTarget(""); ok { t.Error("an empty target should be rejected") } + // A custom command that does not exist is rejected up front, so it is never + // scheduled and then re-sent as literal "/…" text on every tick. + if _, ok := m.validateLoopTarget("/babysit-prs"); ok { + t.Error("an unresolved custom command should be rejected") + } + // A registered custom command is accepted. + m.userCommands = []usercommands.Command{{Name: "babysit-prs", Template: "check the PRs"}} + if _, ok := m.validateLoopTarget("/babysit-prs"); !ok { + t.Error("an existing custom command should be a valid loop target") + } +} + +func TestClearLoopsForSessionSwitch(t *testing.T) { + now := time.Date(2026, 7, 5, 12, 0, 0, 0, time.UTC) + m := loopTestModel(t, now) + m = startFixedLoop(m, "a", time.Minute) + m = startFixedLoop(m, "b", time.Minute) + m.activeLoopID = m.loops[0].id + m, n := m.clearLoopsForSessionSwitch() + if n != 2 || len(m.loops) != 0 || m.activeLoopID != "" || m.loopTicking { + t.Fatalf("session switch should clear all loop state; got n=%d loops=%d active=%q ticking=%v", + n, len(m.loops), m.activeLoopID, m.loopTicking) + } +} + +func TestStartNewSessionStopsLoops(t *testing.T) { + now := time.Date(2026, 7, 5, 12, 0, 0, 0, time.UTC) + m := loopTestModel(t, now) + m = startFixedLoop(m, "x", time.Minute) + m = m.startNewSession() + if len(m.loops) != 0 || m.loopTicking { + t.Fatal("/new should stop loops tied to the previous session") + } +} + +func TestLoopClearRequiresConfirm(t *testing.T) { + now := time.Date(2026, 7, 5, 12, 0, 0, 0, time.UTC) + m := loopTestModel(t, now) + m = startFixedLoop(m, "x", time.Minute) + + // First /clear with a loop active arms the confirm and does not clear yet. + m.input.SetValue("/clear") + u1, _ := m.Update(testKey(tea.KeyEnter)) + n1 := u1.(model) + if n1.loopLeavePrompt != commandClear { + t.Fatal("first /clear with active loops should arm the confirm, not clear") + } + if len(n1.loops) != 1 { + t.Fatal("the loop should still be active after the warning") + } + + // A different command disarms the confirm. + n1.input.SetValue("/loop list") + u2, _ := n1.Update(testKey(tea.KeyEnter)) + n2 := u2.(model) + if n2.loopLeavePrompt != commandEmpty { + t.Fatal("an intervening command should disarm the leave confirm") + } + + // Re-arm, then a second consecutive /clear confirms (disarms + clears). + n2.input.SetValue("/clear") + u3, _ := n2.Update(testKey(tea.KeyEnter)) + n3 := u3.(model) + n3.input.SetValue("/clear") + u4, _ := n3.Update(testKey(tea.KeyEnter)) + n4 := u4.(model) + if n4.loopLeavePrompt != commandEmpty { + t.Fatal("a second consecutive /clear should confirm and disarm") + } + if len(n4.loops) != 1 { + t.Fatal("/clear keeps loops running (it wipes the screen, not the session)") + } +} + +func TestLeaveConfirmDisarmedByQueuedPrompt(t *testing.T) { + now := time.Date(2026, 7, 5, 12, 0, 0, 0, time.UTC) + m := loopTestModel(t, now) + m = startFixedLoop(m, "x", time.Minute) + + // Arm the /clear confirm. + m.input.SetValue("/clear") + u1, _ := m.Update(testKey(tea.KeyEnter)) + n1 := u1.(model) + if n1.loopLeavePrompt != commandClear { + t.Fatal("first /clear should arm the confirm") + } + + // A prompt submitted while a run streams is queued via an early return in + // handleSubmit; it must still disarm the confirm (the disarm runs before those + // returns). Otherwise a later /clear would be treated as the second press. + n1.pending = true + n1.input.SetValue("a follow-up question") + u2, _ := n1.Update(testKey(tea.KeyEnter)) + n2 := u2.(model) + if n2.loopLeavePrompt != commandEmpty { + t.Fatal("a queued prompt should disarm the leave confirm; the early return must not skip the disarm") + } } diff --git a/internal/tui/loop_test.go b/internal/tui/loop_test.go index aeb1cfb7..cd4c5f91 100644 --- a/internal/tui/loop_test.go +++ b/internal/tui/loop_test.go @@ -1,6 +1,7 @@ package tui import ( + "strings" "testing" "time" ) @@ -95,6 +96,50 @@ func TestFormatLoopDuration(t *testing.T) { } } +func TestParseLoopControl(t *testing.T) { + cases := []struct { + in string + done bool + wake time.Duration + }{ + {"did the work\nLOOP: DONE", true, 0}, + {"progress\nLOOP: CONTINUE", false, 0}, + {"progress\nLOOP: CONTINUE 10m", false, 10 * time.Minute}, + {"progress\n- loop: continue 1h", false, time.Hour}, // markdown bullet, lowercase + {"LOOP: CONTINUE 5m\n...more...\nLOOP: DONE", true, 0}, // last control line wins + {"no control line here", false, 0}, // absent -> adaptive + {"LOOP: DONE — all tests pass", true, 0}, // a trailing note is fine + {"progress\nLOOP: CONTINUE 5m — still tidying", false, 5 * time.Minute}, // note after the interval + {"work done.\r\nLOOP: DONE\r\n", true, 0}, // CRLF endings still parse + {"work\r\nLOOP: CONTINUE 10m\r\n", false, 10 * time.Minute}, // CRLF + interval + {"I will emit LOOP: DONE when finished", false, 0}, // mid-sentence mention doesn't match + {"LOOP: CONTINUE 99z", false, 0}, // bad unit -> continue, no wake hint + } + for _, c := range cases { + done, wake := parseLoopControl(c.in) + if done != c.done || wake != c.wake { + t.Errorf("parseLoopControl(%q) = (%v,%v), want (%v,%v)", c.in, done, wake, c.done, c.wake) + } + } +} + +func TestFirePromptWrapsSelfPacedOnly(t *testing.T) { + self := &loopState{mode: loopModeSelfPaced, prompt: "tidy the docs"} + got := self.firePrompt() + if !strings.Contains(got, "LOOP: DONE") || !strings.Contains(got, "tidy the docs") { + t.Errorf("self-paced firePrompt should embed the playbook and goal, got %q", got) + } + fixed := &loopState{mode: loopModeFixed, prompt: "check the build"} + if fixed.firePrompt() != "check the build" { + t.Errorf("fixed-interval firePrompt should be verbatim, got %q", fixed.firePrompt()) + } + // A self-paced /command is a real command, not a goal — send it verbatim, no playbook. + cmd := &loopState{mode: loopModeSelfPaced, prompt: "/babysit"} + if cmd.firePrompt() != "/babysit" { + t.Errorf("self-paced /command firePrompt should be verbatim, got %q", cmd.firePrompt()) + } +} + func TestLoopStateDue(t *testing.T) { base := time.Date(2026, 7, 5, 12, 0, 0, 0, time.UTC) l := &loopState{nextRunAt: base} diff --git a/internal/tui/model.go b/internal/tui/model.go index 89e34505..c994ab5d 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -194,19 +194,18 @@ type model struct { // tags the in-flight run when it is a loop iteration (empty = a user turn), so the // completion seam knows whether to advance a loop. loopSeq invalidates a stale // pending poll tick when loops are stopped; loopTicking guards against scheduling - // a second poll ticker. loopNextWake carries a self-paced iteration's model-chosen - // delay from the loop_control tool back to the completion seam. - loops []*loopState - activeLoopID string - loopSeq int - loopCounter int - loopTicking bool - loopNextWake time.Duration - loopDone bool - exiting bool - runCancel context.CancelFunc - runID int - activeRunID int + // a second poll ticker. loopLeavePrompt arms a one-shot confirm before /clear or + // /quit while loops are active (see handleSubmit). + loops []*loopState + activeLoopID string + loopSeq int + loopCounter int + loopTicking bool + loopLeavePrompt commandKind + exiting bool + runCancel context.CancelFunc + runID int + activeRunID int // flushRunIDs holds the ids of runs cancelled while still in flight, mapped // to the session they were recording into AT CANCEL TIME. Each cancelled // agent goroutine keeps running to completion and returns its accumulated @@ -3829,6 +3828,13 @@ func (m model) handleSubmit() (tea.Model, tea.Cmd) { return m, nil } command := parseCommand(input) + // A pending /clear or /quit leave-confirmation is armed for the immediately-next + // repeat of that same command only; any other submission — including one queued + // or deferred by the early returns below — disarms it. Runs before those returns + // so an interposed prompt can't be skipped over and leave the gate falsely armed. + if m.loopLeavePrompt != commandEmpty && command.kind != m.loopLeavePrompt { + m.loopLeavePrompt = commandEmpty + } // While exiting (Ctrl+C waiting on the cancelled run's checkpoint flush) a // new run must not start: the deferred tea.Quit would abort it mid-flight // and orphan its checkpoint blobs — the exact loss flushRunIDs prevents. @@ -3862,6 +3868,14 @@ func (m model) handleSubmit() (tea.Model, tea.Cmd) { m.transcript = reduceTranscript(m.transcript, transcriptAction{kind: actionAppendSystem, text: helpText()}) return m, nil case commandClear: + // A foreground loop keeps firing after /clear (it wipes the screen, not the + // session), so warn once before clearing the context it will run into. + if m.loopActive() && m.loopLeavePrompt != commandClear { + m.loopLeavePrompt = commandClear + m = m.appendLoopSystem(m.loopFooterSummary() + " still running — /clear keeps them firing behind a cleared screen. Run /clear again to confirm, or /loop stop all first.") + return m, nil + } + m.loopLeavePrompt = commandEmpty m.transcript = reduceTranscript(m.transcript, transcriptAction{kind: actionClear}) // Clearing wipes the visible transcript only — the session's context is // intact, so the next prompt still replays the full history. Say so, and @@ -3888,6 +3902,14 @@ func (m model) handleSubmit() (tea.Model, tea.Cmd) { case commandLoop: return m.handleLoopCommand(command.text) case commandExit: + // Closing the session stops its foreground loops mid-task; warn once so a + // token-spending loop isn't ended by reflex. + if m.loopActive() && m.loopLeavePrompt != commandExit { + m.loopLeavePrompt = commandExit + m = m.appendLoopSystem(m.loopFooterSummary() + " active — closing the session stops them. Run /exit again to confirm, or /loop stop all first.") + return m, nil + } + m.loopLeavePrompt = commandEmpty // /exit gets the same protection as Ctrl+C: cancel any in-flight run and // defer the quit until its checkpoint session events flush — quitting // immediately would orphan the blobs and break /rewind. diff --git a/internal/tui/session.go b/internal/tui/session.go index 080e9f1c..24f5ec4d 100644 --- a/internal/tui/session.go +++ b/internal/tui/session.go @@ -99,6 +99,12 @@ func (m model) startNewSession() model { // Scrollback above can't be un-printed; a faint divider marks the boundary and // the flush frontier restarts for the fresh transcript (mirrors /clear, /resume). m.resetFlushFrontier("· new session ·") + // Loops belong to the previous session; stop them so they don't fire the old + // conversation's prompt into the fresh one. + if updated, cleared := m.clearLoopsForSessionSwitch(); cleared > 0 { + m = updated + m.transcript = reduceTranscript(m.transcript, transcriptAction{kind: actionAppendSystem, text: fmt.Sprintf("Stopped %d loop(s) tied to the previous session.", cleared)}) + } return m } @@ -213,9 +219,15 @@ func (m model) handleResumeCommand(args string) (model, string) { if m.modelName == "" { m.modelName = session.ModelID } + // The resumed conversation is a different session; drop any loops the previous + // one left running so they don't fire into it. + m, loopsCleared := m.clearLoopsForSessionSwitch() rows := initialTranscript() rows = appendRow(rows, rowSystem, m.formatResumeSummary(*session, len(events))) + if loopsCleared > 0 { + rows = appendRow(rows, rowSystem, fmt.Sprintf("Stopped %d loop(s) tied to the previous session.", loopsCleared)) + } rows = appendTranscriptRowsDedup(rows, transcriptRowsFromSessionEvents(events)) m.transcript = rows // Every rehydrated row is settled by construction, so resetting the flush From ac25352e12632a930db82924214e314b7bd1798f Mon Sep 17 00:00:00 2001 From: Vasanthdev2004 Date: Sun, 5 Jul 2026 16:48:15 +0530 Subject: [PATCH 8/9] =?UTF-8?q?fix(tui):=20address=20/loop=20review=20roun?= =?UTF-8?q?d=202=20=E2=80=94=20ticker=20re-arm,=20self-paced=20command,=20?= =?UTF-8?q?resume=20guard?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - stopLoop re-arms the poll ticker after removing one loop, so stopping one of several no longer strands the rest with no tick scheduled (jatmn). - a bare self-paced /command is rejected with a pointer to the interval form: self-paced drives a free-form goal via the playbook + LOOP: control line, which a custom command's expanded template never carries, so it would silently degrade to adaptive polling (jatmn). - /resume only tears loops down on an actual session-id change; /resume latest or /resume can resolve to the already-active session, whose loops are its own, not a "previous" session's (CodeRabbit). - tests: skipped-behind-modal loop stays scheduled, footer carries the next-wake time, the confirmed /clear actually wipes the transcript, plus multi-loop stop keeping the ticker alive and self-paced-/command rejection. --- internal/tui/loop.go | 14 ++++++-- internal/tui/loop_controller_test.go | 48 ++++++++++++++++++++++++++-- internal/tui/session.go | 11 +++++-- 3 files changed, 65 insertions(+), 8 deletions(-) diff --git a/internal/tui/loop.go b/internal/tui/loop.go index 6be01fdf..0d0ea739 100644 --- a/internal/tui/loop.go +++ b/internal/tui/loop.go @@ -274,6 +274,14 @@ func (m model) startLoop(cmd loopCommand) (model, tea.Cmd) { if reason, ok := m.validateLoopTarget(cmd.prompt); !ok { return m.appendLoopSystem(reason), nil } + // A /command loop must be fixed-interval. Self-paced mode drives a free-form + // goal through the playbook + LOOP: control line (see firePrompt), which a + // custom command's expanded template never carries — so a bare self-paced + // /command would silently degrade to adaptive polling with no way to report + // done. Require an explicit interval for command loops instead. + if cmd.mode == loopModeSelfPaced && strings.HasPrefix(strings.TrimSpace(cmd.prompt), "/") { + return m.appendLoopSystem("A /command loop needs an interval — e.g. /loop 5m " + strings.Fields(cmd.prompt)[0] + ". Self-paced mode (no interval) is for a free-form goal."), nil + } m.loopCounter++ id := "L" + strconv.Itoa(m.loopCounter) loop := &loopState{ @@ -302,14 +310,16 @@ func (m model) stopLoop(id string) (model, tea.Cmd) { } if id == "" { if len(m.loops) == 1 { - return m.removeLoop(m.loops[0].id, "Loop "+m.loops[0].id+" stopped."), nil + return m.removeLoop(m.loops[0].id, "Loop "+m.loops[0].id+" stopped.").ensureLoopTick() } return m.appendLoopSystem("Multiple loops active — use /loop stop or /loop stop all.\n" + m.loopListText()), nil } if m.findLoop(id) == nil { return m.appendLoopSystem("No loop " + id + ". " + m.loopListText()), nil } - return m.removeLoop(id, "Loop "+id+" stopped."), nil + // removeLoop stops the poll ticker; re-arm it so any OTHER loops that remain + // keep firing (ensureLoopTick no-ops when this was the last loop). + return m.removeLoop(id, "Loop "+id+" stopped.").ensureLoopTick() } func (m model) stopAllLoops() (model, tea.Cmd) { diff --git a/internal/tui/loop_controller_test.go b/internal/tui/loop_controller_test.go index 6d1f15b2..500f97a6 100644 --- a/internal/tui/loop_controller_test.go +++ b/internal/tui/loop_controller_test.go @@ -148,10 +148,15 @@ func TestStopLoopByID(t *testing.T) { m = startFixedLoop(m, "a", time.Minute) m = startFixedLoop(m, "b", time.Minute) id := m.loops[0].id - m, _ = m.stopLoop(id) + m, cmd := m.stopLoop(id) if len(m.loops) != 1 || m.findLoop(id) != nil { t.Fatalf("stopLoop(%s) should remove exactly that loop", id) } + // Stopping one of several loops must keep the poll ticker alive for the rest: + // removeLoop kills the ticker, so stopLoop has to re-arm it (jatmn). + if cmd == nil || !m.loopTicking { + t.Fatal("stopping one of several loops must leave the remaining loop scheduled (ticker re-armed)") + } // Bare stop with a single active loop stops that sole loop (parseLoopCommand // leaves the target empty and stopLoop special-cases the one-loop case). soleID := m.loops[0].id @@ -187,6 +192,9 @@ func TestFireDueLoopSkipsBehindModal(t *testing.T) { if got.activeLoopID != "" || cmd != nil { t.Fatal("a due loop must not launch a run behind an open picker/modal — it would complete into whatever session the user switches to") } + if got.loops[0].nextRunAt.IsZero() { + t.Fatal("a loop skipped because a modal is open must stay scheduled, not have its next run cleared") + } } func TestAdaptiveSelfPaceDelayWidensOverTime(t *testing.T) { @@ -207,8 +215,9 @@ func TestLoopFooterSummary(t *testing.T) { m = startFixedLoop(m, "a", 5*time.Minute) m.loops[0].nextRunAt = now.Add(5 * time.Minute) got := m.loopFooterSummary() - if !strings.HasPrefix(got, "1 loop") { - t.Fatalf("footer summary = %q, want it to start with a loop count", got) + wantWake := "next " + now.Add(5*time.Minute).Format("3:04pm") + if !strings.HasPrefix(got, "1 loop") || !strings.Contains(got, wantWake) { + t.Fatalf("footer summary = %q, want it to start with a loop count and contain %q", got, wantWake) } } @@ -330,6 +339,24 @@ func TestValidateLoopTargetRejectsBuiltin(t *testing.T) { } } +func TestSelfPacedCommandLoopRejected(t *testing.T) { + now := time.Date(2026, 7, 5, 12, 0, 0, 0, time.UTC) + m := loopTestModel(t, now) + m.userCommands = []usercommands.Command{{Name: "babysit-prs", Template: "check the PRs"}} + // A /command with no interval parses as self-paced, but self-paced mode drives + // a free-form goal via the playbook + LOOP: protocol, which a command's + // expanded template never carries — so a bare self-paced /command is rejected. + m, _ = m.startLoop(loopCommand{action: loopActionStart, mode: loopModeSelfPaced, prompt: "/babysit-prs"}) + if len(m.loops) != 0 { + t.Fatal("a bare self-paced /command loop should be rejected (a command loop needs an interval)") + } + // The same command with an interval (fixed) is accepted. + m, _ = m.startLoop(loopCommand{action: loopActionStart, mode: loopModeFixed, prompt: "/babysit-prs", interval: 5 * time.Minute}) + if len(m.loops) != 1 { + t.Fatal("a fixed-interval /command loop should be accepted") + } +} + func TestClearLoopsForSessionSwitch(t *testing.T) { now := time.Date(2026, 7, 5, 12, 0, 0, 0, time.UTC) m := loopTestModel(t, now) @@ -390,6 +417,21 @@ func TestLoopClearRequiresConfirm(t *testing.T) { if len(n4.loops) != 1 { t.Fatal("/clear keeps loops running (it wipes the screen, not the session)") } + // The confirmed clear must actually wipe the transcript, not silently no-op: + // only the clear path appends the "Transcript cleared" note, and the pre-clear + // loop-start row must be gone. + clearedNote, staleRow := false, false + for _, r := range n4.transcript { + if strings.Contains(r.text, "Transcript cleared") { + clearedNote = true + } + if strings.Contains(r.text, "Loop L1 started") { + staleRow = true + } + } + if !clearedNote || staleRow { + t.Fatalf("the second /clear must wipe the transcript (clearedNote=%v staleRow=%v)", clearedNote, staleRow) + } } func TestLeaveConfirmDisarmedByQueuedPrompt(t *testing.T) { diff --git a/internal/tui/session.go b/internal/tui/session.go index 24f5ec4d..c5bd6266 100644 --- a/internal/tui/session.go +++ b/internal/tui/session.go @@ -211,6 +211,10 @@ func (m model) handleResumeCommand(args string) (model, string) { return m, "Sessions\nerror: " + err.Error() } + // Capture the current session id before switching so loops are only torn down + // on a real change — `/resume latest` or `/resume ` can resolve to + // the already-active session, whose loops belong to it, not a "previous" one. + previousID := m.activeSession.SessionID m.activeSession = *session m.sessionEvents = append([]sessions.Event{}, events...) if m.providerName == "" { @@ -219,9 +223,10 @@ func (m model) handleResumeCommand(args string) (model, string) { if m.modelName == "" { m.modelName = session.ModelID } - // The resumed conversation is a different session; drop any loops the previous - // one left running so they don't fire into it. - m, loopsCleared := m.clearLoopsForSessionSwitch() + loopsCleared := 0 + if session.SessionID != previousID { + m, loopsCleared = m.clearLoopsForSessionSwitch() + } rows := initialTranscript() rows = appendRow(rows, rowSystem, m.formatResumeSummary(*session, len(events))) From 330b10fe8ef5f2a31b3f0354893169203ea95321 Mon Sep 17 00:00:00 2001 From: Vasanthdev2004 Date: Mon, 6 Jul 2026 08:59:06 +0530 Subject: [PATCH 9/9] docs(readme): clarify /loop runs a prompt or custom command, not built-ins --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index ff32df02..86ce5d13 100644 --- a/README.md +++ b/README.md @@ -186,7 +186,7 @@ Common slash commands: | `/spec`, `/plan` | draft and review a plan before building | | `/image` | attach an image for vision-capable models | | `/resume`, `/rewind` | continue or roll back local sessions | -| `/loop` | repeat a prompt or command on an interval (`/loop 5m /babysit-prs`) or self-paced | +| `/loop` | repeat a prompt or custom `/command` on an interval (`/loop 5m /babysit-prs`) or self-paced | | `/compact`, `/context` | manage context usage | | `/permissions`, `/tools` | inspect available tools and policy | | `/add-dir` | allow an extra write directory for this session |