feat(tui): /loop — repeat a prompt or command on an interval or self-paced

[Vasanthdev2004]

Adds /loop — repeat a prompt or slash command, in two modes. Fixes #501.

• Fixed interval — /loop 5m /babysit-prs re-fires every 5 minutes (also a trailing form: /loop watch CI every 2m). Because it runs off a Go timer rather than a cron backend, arbitrary intervals like 90s or 7m just work.
• Self-paced — bare /loop keep tidying the docs runs an iteration, then checks back on an adaptive cadence (2m early, widening to a ~30m heartbeat as it matures) and stops when there's no more progress.

Manage with /loop list and /loop stop [id|all], and a persistent footer segment (↻ N loops · next 3:05pm) keeps a running loop visible.

How it works

Loops are foreground and session-scoped — a loop only fires while the session is idle between turns (it never interrupts a streaming turn) and lives with its session. It rides machinery that's already there: an idle poll tick (~1s) fires the earliest due loop through the same seam launchQueuedMessageIfReady uses, so a user's queued prompt always wins and the two never collide. When an iteration finishes, the agentResponseMsg completion seam schedules the next wake or stops the loop.

Safety

A loop can't run away: a max-iteration cap, a doom-loop guard (stops after 5 identical results in a row), and /loop stop. /clear notes that loops keep running (it wipes the transcript, not the session). Only a prompt or a custom /command may loop — not a built-in like /model.

Tested

Unit tests for parsing/clamps/cadence, controller tests for the full lifecycle (start → advance → self-paced delay → done/doom/cap → stop → busy-skip), and end-to-end tests that drive /loop through Update. go build ./..., go vet, and the full internal/tui suite are green.

Deliberately out of scope (follow-ups)

• Cross-restart durability (a persisted loop store + missed-run catch-up). Session-scoped is the right semantic for a context-carrying loop, so that's a separate call.
• A detached background daemon that runs loops with no TUI attached.
• An explicit loop_control tool for the model to set its own self-paced delay / done signal — the channel (loopNextWake/loopDone) is already wired; today self-paced uses the adaptive cadence + doom-guard.

Summary by CodeRabbit

• New Features
  • Added a /loop command to repeat prompts or custom /commands on a fixed interval or in self-paced mode.
  • Includes list/status and stopping loops by id or stopping all.
  • Shows an “active loops” segment in the TUI status line with next wake timing.
• Documentation
  • Updated README with /loop usage examples.
• Bug Fixes
  • Improved loop execution to fire only when the session is idle; added reliable stop/cancel/clear/exit protections and session switch/resume cleanup.
• Tests
  • Added extensive unit tests for /loop parsing, scheduling, control handling, and confirmation/UI behaviors.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9dc302c2-f33b-444a-9be7-4327403ced95

📥 Commits

Reviewing files that changed from the base of the PR and between 47c799e and 8d1e3b0.

📒 Files selected for processing (8)

• README.md
• internal/tui/commands.go
• internal/tui/loop.go
• internal/tui/loop_controller_test.go
• internal/tui/loop_test.go
• internal/tui/model.go
• internal/tui/session.go
• internal/tui/view.go

✅ Files skipped from review due to trivial changes (1)

• README.md

🚧 Files skipped from review as they are similar to previous changes (7)

• internal/tui/commands.go
• internal/tui/view.go
• internal/tui/session.go
• internal/tui/loop_controller_test.go
• internal/tui/loop_test.go
• internal/tui/loop.go
• internal/tui/model.go

---

Walkthrough

Adds a session-scoped /loop command for fixed-interval and self-paced repetition. It wires loop execution into idle ticks and turn completion, shows loop status in the footer, blocks session exit/clear with active loops, and adds coverage for parsing, scheduling, and lifecycle behavior.

Changes

/loop command implementation

Layer / File(s)	Summary
Loop parsing and registration internal/tui/commands.go, internal/tui/loop.go, README.md	Registers /loop, defines loop state and argument parsing, clamps intervals, validates targets, and documents the command usage.
Loop scheduling and turn completion internal/tui/loop.go, internal/tui/model.go	Starts, stops, fires, and advances loops; handles idle tick polling, completion-path advancement, and cancel re-arming.
Loop status, confirmations, and session cleanup internal/tui/model.go, internal/tui/session.go, internal/tui/view.go	Adds loop footer/status display, /clear and /exit confirmation gates, and loop teardown/reporting on session changes.
Tests internal/tui/loop_test.go, internal/tui/loop_controller_test.go	Adds parsing, timing, control-line, scheduling, lifecycle, and confirmation-flow tests.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

• Gitlawb/zero#83: Both PRs touch internal/tui/model.go around completion-time transcript/state updates.
• Gitlawb/zero#160: Both PRs modify the TUI turn-completion and queued-follow-up flow in internal/tui/model.go.
• Gitlawb/zero#167: Both PRs affect response truncation handling that the TUI completion path relies on.

Suggested reviewers: anandh8x

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The core /loop behavior matches #501, but the diff does not show the required loop.md playbook, so the issue is not fully satisfied.	Add the loop.md maintenance playbook, or include it in the PR diff, and verify all linked-issue UX requirements are present.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is concise and accurately summarizes the new /loop feature and its fixed-interval/self-paced modes.
Out of Scope Changes check	✅ Passed	The changes stay within the /loop feature, its session/UX plumbing, and tests; no unrelated scope is evident.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/loop-command

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (2)

internal/tui/loop_controller_test.go (2)

147-159: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Minor: brittle fixed-length slicing in footer assertion.

got[:6] relies on the label always being ≥6 chars; safe today given "%d loop"/"%d loops" formatting, but strings.HasPrefix(got, "1 loop") would be more robust against future footer text changes without risking an index-out-of-range panic.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/tui/loop_controller_test.go` around lines 147 - 159, The footer
summary test in TestLoopFooterSummary uses brittle fixed-length slicing on got,
which can panic and is less resilient to wording changes. Update the assertion
to check the prefix of loopFooterSummary using the existing test setup around
loopTestModel and startFixedLoop, so it verifies the summary starts with the
expected loop count without indexing into the string.

---

111-125: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Missing coverage for bare /loop stop targeting the sole active loop.

parseLoopCommand's comment (in internal/tui/loop.go) notes bare "stop" is ambiguous and "the caller may special-case a single active loop," but this test only covers stopLoop(id) with an explicit id and stopAllLoops(). Consider adding a case with a single active loop and stopLoop("") to lock in that special-casing.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/tui/loop_controller_test.go` around lines 111 - 125, Add coverage in
TestStopLoopByID for the bare stop path handled by parseLoopCommand/stopLoop:
create a single active loop, call stopLoop("") on the loop controller model, and
assert it removes that sole loop rather than treating it as an invalid or no-op
command. Keep the existing explicit-id and stopAllLoops checks, but extend the
test to lock in the single-active-loop special case referenced by stopLoop and
parseLoopCommand.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/tui/loop.go`:
- Around line 339-375: The advanceLoop method in model is ignoring runErr, so
failing loops keep being re-scheduled until the iteration cap. Update
advanceLoop to incorporate the error state into the stop/backoff decision path:
either stop immediately on repeated failures or count consecutive errors toward
the same no-progress guard used with lastResult/repeatRun. Make sure the fix is
applied in the advanceLoop logic before nextRunAt is set, and use the existing
loop state fields on the loop model to track failure streaks if needed.

In `@internal/tui/model.go`:
- Around line 1912-1928: Clear the loop state when a run is cancelled:
`cancelRun()` currently resets `activeRunID` but leaves `activeLoopID` set,
which lets a cancelled loop iteration bypass `advanceLoop` and corrupt later
turns. Update the cancel/flush path in `internal/tui/model.go` so the
interrupted loop is explicitly cleared or advanced using the loop identifier,
and ensure any loop bookkeeping in `advanceLoop`, `removeLoop`, and the
`activeLoopID` handling stays in sync.

---

Nitpick comments:
In `@internal/tui/loop_controller_test.go`:
- Around line 147-159: The footer summary test in TestLoopFooterSummary uses
brittle fixed-length slicing on got, which can panic and is less resilient to
wording changes. Update the assertion to check the prefix of loopFooterSummary
using the existing test setup around loopTestModel and startFixedLoop, so it
verifies the summary starts with the expected loop count without indexing into
the string.
- Around line 111-125: Add coverage in TestStopLoopByID for the bare stop path
handled by parseLoopCommand/stopLoop: create a single active loop, call
stopLoop("") on the loop controller model, and assert it removes that sole loop
rather than treating it as an invalid or no-op command. Keep the existing
explicit-id and stopAllLoops checks, but extend the test to lock in the
single-active-loop special case referenced by stopLoop and parseLoopCommand.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b6ea9e6b-29f5-4aa9-8e0e-24453227d3f8

📥 Commits

Reviewing files that changed from the base of the PR and between f401c66 and 6d1b487.

📒 Files selected for processing (7)

• README.md
• internal/tui/commands.go
• internal/tui/loop.go
• internal/tui/loop_controller_test.go
• internal/tui/loop_test.go
• internal/tui/model.go
• internal/tui/view.go

[github-actions]

Zero automated PR review

Verdict: No blockers found

Blockers

• None found.

Validation

• [pass] Diff hygiene: git diff --check
• [pass] Tests: go test ./...
• [pass] Build: go run ./cmd/zero-release build
• [pass] Smoke build: go run ./cmd/zero-release smoke

Scope

Head: 8d1e3b09c159
Changed files (8): README.md, internal/tui/commands.go, internal/tui/loop.go, internal/tui/loop_controller_test.go, internal/tui/loop_test.go, internal/tui/model.go, internal/tui/session.go, internal/tui/view.go

This deterministic review checks validation status and basic diff hygiene. A human reviewer still owns product judgment and design quality.

[jatmn]

I found a couple of issues that need to be addressed before this is ready.

Findings

• [P2] Stop carrying loops across session switches
  internal/tui/session.go:51
  Loops are described as session-scoped, but startNewSession() clears the session/transcript state without clearing m.loops or invalidating the loop ticker. The same pattern exists on other session replacement paths like /resume, so an active loop from the previous conversation can keep firing after the user starts or resumes an unrelated session, injecting the old loop prompt into the new context. Please either stop active loops when the TUI switches/rebuilds sessions or persist and reattach them to their owning session instead of leaving them as global model state.

• [P2] Wire the self-paced control path before advertising it
  internal/tui/loop.go:378
  The self-paced path still depends on m.loopNextWake and m.loopDone, but there is no runtime writer for either field: the only production writes reset them before a loop fires, and the only non-reset writes are tests. That means a bare /loop keep tidying docs cannot actually let the model choose its next wake or report done as issue #501 and the in-app usage text describe; it can only follow the hard-coded adaptive delay until it hits repeated-output/failure/iteration guards. Please either add the promised loop-control channel/tool and playbook wiring, or narrow the command/docs/PR scope so self-paced mode is honestly described as adaptive polling without model-controlled completion.

• [P2] Reject missing custom slash commands instead of looping them as prompts
  internal/tui/loop.go:337
validateLoopTarget() accepts any slash-prefixed commandUnknown value so custom commands can be scheduled, but fireLoopPrompt() falls back to launchPrompt(prompt) when handleUserCommand() cannot resolve the command. As a result, /loop 5m /babysit-prs succeeds even if that custom command file does not exist, and every tick sends the literal text /babysit-prs to the model instead of following the normal unknown-command error path. Please resolve custom slash commands when creating the loop, or stop/report the loop on the first unresolved command, so only real custom commands are scheduled.

• [P2] Add the active-loop leave gate before /clear and /quit
internal/tui/model.go:3685
  The linked issue calls for a leave-warning gate before /quit and /clear when loops are active, but the current handlers do not gate either path: /clear wipes the transcript first and only then appends a note that loops are still running, while /exit//quit goes straight to quit() when idle without checking loopActive(). For a foreground token-spending loop, the user needs a chance to stop or confirm before clearing the visible context it will continue from or closing the session. Please add the pre-action warning/confirmation gate for active loops on both paths.

[Vasanthdev2004]

Thanks, these were all fair. Pushed 47c799e.

1. Session scope — loops now stop on /new and /resume (clearLoopsForSessionSwitch), so a loop from the old conversation can't keep firing into a new one. Went with stopping them rather than persist-and-reattach: v1 keeps loops as in-memory state, and cross-restart durability is already parked as a follow-up in the issue.

2. Self-paced control path — made it real instead of narrowing the docs. Each self-paced iteration now runs a maintenance playbook, and the model ends its reply with a control line: LOOP: DONE stops the loop, LOOP: CONTINUE [interval] sets the next wake. advanceLoop parses that, so the done/next-wake decision comes from the model's actual output. Dropped loopNextWake/loopDone entirely — they were the dead fields you flagged.

3. Custom commands — validateLoopTarget is a method now and resolves the command against the loaded user commands at creation time, rejecting anything that doesn't exist. So /loop 5m /babysit-prs fails up front when there's no such command instead of scheduling it and re-sending the literal text every tick.

4. Leave gate — /clear and /quit warn once and need a second confirm while loops are active; any other command in between disarms it.

Also folded in the two remaining scope items the pre-merge check wanted: the loop.md playbook (it's the self-paced preamble) and age-based auto-expire (24h, next to the iteration cap), plus the two CodeRabbit test nits.

One extra note since it touches what you flagged: I ran a review pass over the above and caught a few edge cases in the new code — the control-line regex missed CRLF replies and a LOOP: DONE line with a trailing note (so a self-paced loop could drop its own stop signal), and a loop could fire behind an open /resume picker and complete into the session you switched to. Both fixed in the same push with tests.

[jatmn]

Thanks for the update. I rechecked the changed paths and found an issue that still needs to be addressed.

Findings

• [P2] Keep the remaining loops scheduled after stopping one loop
  internal/tui/loop.go:312
stopLoop(id) removes the requested loop by returning removeLoop(...), but removeLoop always invalidates the current poll tick and sets loopTicking=false. Unlike the advanceLoop completion path, the manual /loop stop <id> path never calls ensureLoopTick() afterward, so if a user has multiple loops and stops only one of them, the remaining loops stay in m.loops and still appear in /loop list/the footer, but no poll tick is scheduled to fire them anymore. Please restart the loop ticker whenever a manual single-loop stop leaves other loops active, and add coverage for that multi-loop stop case.

• [P2] Apply the self-paced control protocol to custom command loops
  internal/tui/loop.go:352
  Bare /loop /some-command is accepted as a self-paced loop because the command usage advertises <prompt|/command> and parseLoopCommand makes every no-interval target self-paced. However firePrompt() only wraps self-paced non-slash prompts in loopSelfPacePlaybook; slash targets are sent verbatim to handleUserCommand(), which expands the custom command template and launches it normally. Unless every custom command author already included the hidden LOOP: DONE / LOOP: CONTINUE protocol in their template, the model is never told how to stop the self-paced command or choose its next wake, so the advertised self-paced behavior degrades to adaptive polling until another safety guard fires. Please either wrap/augment expanded custom-command prompts with the same loop-control instructions, or reject/narrow bare self-paced /command loops so only fixed-interval command loops are advertised.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (3)

internal/tui/loop_controller_test.go (3)

181-190: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Test doesn't confirm the skipped loop stays scheduled.

Confirms a due loop won't fire behind a picker, but doesn't assert nextRunAt is untouched afterward. If fireDueLoopIfIdle accidentally cleared/advanced nextRunAt on the busy path, this loop would silently stop firing forever and this test would still pass.

Suggested addition

 	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")
 	}
+	if got.loops[0].nextRunAt.IsZero() {
+		t.Fatal("a skipped due loop must keep its nextRunAt so it can fire on a later idle tick")
+	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/tui/loop_controller_test.go` around lines 181 - 190, The test in
fireDueLoopIfIdle only verifies that a due loop does not launch behind an open
picker/modal, but it does not verify that the loop remains scheduled. Update
TestFireDueLoopSkipsBehindModal in loop_controller_test.go to assert that the
loop’s nextRunAt (and any related scheduling state on the returned model from
fireDueLoopIfIdle) is unchanged when m.picker is set, so a skipped run cannot be
accidentally cleared or advanced.

---

356-393: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Confirm-gate test doesn't verify the transcript is actually cleared.

TestLoopClearRequiresConfirm thoroughly checks that loops survive /clear, but the second confirmed /clear (n3 → n4) never asserts the transcript/screen was actually wiped. As written, a bug that swallows the confirmed clear entirely (no-op) would still pass this test since only loopLeavePrompt and loop count are checked.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/tui/loop_controller_test.go` around lines 356 - 393,
TestLoopClearRequiresConfirm only checks confirm state and loop count, so it can
miss a confirmed /clear that does nothing. Update the test around the n3 -> n4
transition to also assert the transcript/screen state is actually wiped after
the second /clear, using the existing model and Update flow in
TestLoopClearRequiresConfirm so a no-op clear will fail.

---

201-213: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Footer summary assertion is too weak to catch a missing next-wake time.

The PR requires the footer to show "loop count and next wake time." This test only checks the "1 loop" prefix and never verifies the wake time actually appears, so a regression that drops the wake time (or formats it wrong) would pass unnoticed.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/tui/loop_controller_test.go` around lines 201 - 213, Strengthen
TestLoopFooterSummary so it verifies both parts of loopFooterSummary output: the
loop count and the next wake time. Update the assertions to check that the
returned string still starts with the expected count from loopFooterSummary, and
also contains the formatted next-run time derived from m.loops[0].nextRunAt /
now so a missing or malformed wake time will fail the test.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/tui/session.go`:
- Around line 204-212: The resume flow in session switching is clearing loops
even when `/resume latest` or `/resume <currentID>` resolves to the
already-active session. Update the resume handling in session.go so the call to
clearLoopsForSessionSwitch() is guarded by an actual session ID change, and only
emit the “Stopped %d loop(s) tied to the previous session.” row when switching
to a different session.

---

Nitpick comments:
In `@internal/tui/loop_controller_test.go`:
- Around line 181-190: The test in fireDueLoopIfIdle only verifies that a due
loop does not launch behind an open picker/modal, but it does not verify that
the loop remains scheduled. Update TestFireDueLoopSkipsBehindModal in
loop_controller_test.go to assert that the loop’s nextRunAt (and any related
scheduling state on the returned model from fireDueLoopIfIdle) is unchanged when
m.picker is set, so a skipped run cannot be accidentally cleared or advanced.
- Around line 356-393: TestLoopClearRequiresConfirm only checks confirm state
and loop count, so it can miss a confirmed /clear that does nothing. Update the
test around the n3 -> n4 transition to also assert the transcript/screen state
is actually wiped after the second /clear, using the existing model and Update
flow in TestLoopClearRequiresConfirm so a no-op clear will fail.
- Around line 201-213: Strengthen TestLoopFooterSummary so it verifies both
parts of loopFooterSummary output: the loop count and the next wake time. Update
the assertions to check that the returned string still starts with the expected
count from loopFooterSummary, and also contains the formatted next-run time
derived from m.loops[0].nextRunAt / now so a missing or malformed wake time will
fail the test.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 15155f8e-6ba2-44c1-92c3-c25e5a03ca21

📥 Commits

Reviewing files that changed from the base of the PR and between 504f7be and 47c799e.

📒 Files selected for processing (5)

• internal/tui/loop.go
• internal/tui/loop_controller_test.go
• internal/tui/loop_test.go
• internal/tui/model.go
• internal/tui/session.go

🚧 Files skipped from review as they are similar to previous changes (1)

• internal/tui/loop.go

[kevincodex1]

bro @Vasanthdev2004 please rebase to main and fix conflicts

[Vasanthdev2004]

Rebased onto main (kept the new /retry, /edit, /copy, /export next to /loop) and fixed the round-2 findings.

Ticker re-arm — you're right, stopLoop went through removeLoop, which kills the poll ticker and never restarted it, so stopping one of several loops stranded the rest with nothing scheduled to fire them. stopLoop now re-arms via ensureLoopTick after the removal (which no-ops when it was the last loop). Added a multi-loop-stop test asserting the remaining loop stays scheduled.

Self-paced /command — went with narrowing rather than wrapping. A /command loop now needs an interval; a bare self-paced /command is rejected with a pointer to the interval form. Self-paced mode is for a free-form goal — it gets the playbook + LOOP: control line — and a custom command's expanded template doesn't carry that, so accepting it would just silently degrade to polling, exactly as you said. Test added.

(CodeRabbit) /resume clearing loops on a same-session resume — good catch. /resume latest or /resume <currentID> can land on the already-active session, whose loops belong to it, so the teardown is now guarded on an actual session-id change.

Also picked up the three test-strengthening nits: the skipped-behind-modal loop now asserts it stays scheduled, the footer test checks the next-wake time, and the confirmed /clear test asserts the transcript is actually wiped (so a no-op clear would fail).

Build, vet, and the tui suite are green locally, and it's mergeable again.

[jatmn]

Thanks for the update. I rechecked the previously discussed paths and do not see any remaining actionable issues from my side.

@Vasanthdev2004 LGTM