feat(tui): live "Background agents (N)" sidebar panel#3118
Open
aheritier wants to merge 2 commits into
Open
Conversation
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
This PR adds a live "Background agents (N)" sidebar panel with a ~1s poll loop, a race-free Snapshot() read-model, and the Phase-2 breadcrumb count (+N background). The implementation is solid — no CONFIRMED or LIKELY bugs were found.
Areas reviewed:
Snapshot()race safety — Fields read are immutable after task creation; atomic load for status. Race-free by design, confirmed clean. ✅- Poll lifecycle (
startBackgroundAgentPoll/handleBackgroundAgentPoll) — ThebackgroundPollActiveguard works correctly in Bubbletea's serialised Elm loop. Eachtea.Tickfires exactly once; the loop is strictly sequential (no stale in-flight ticks possible). ✅ SetBackgroundAgentsno-op guard — Correctly skips cache invalidation only when the panel was already empty before the call; drains properly when tasks finish. ✅delegationBreadcrumbwidth arithmetic — Theavail <= 0guard fires before anyansi.Truncatecall; no negative-width truncation possible. ✅- Optional
Snapshoterinterface assertion (app.go) — Safe two-value type assertion (l, ok := ...), no panic path. ✅ - Sidebar rendering —
backgroundAgentsSectiongap calculation useslipgloss.Widthwhich correctly strips ANSI; layout is correct. ✅ FormatDurationwith negative durations — Clock skew could produce a display glitch (-1s) but no crash; acceptable for a live elapsed timer. ✅
No action required — the implementation matches the design intent described in the PR and the Phase 3 C.2 spec.
|
❌ PR Review Failed — The review agent encountered an error and could not complete the review. View logs. |
cf40afa to
d0f18c0
Compare
b0f9f58 to
5f1c3ef
Compare
d0f18c0 to
d432398
Compare
5f1c3ef to
8d9af10
Compare
The background-agent Handler keeps its tasks in an unexported concurrent map, so the TUI had no way to observe the genuinely-concurrent fleet spawned by run_background_agent. Expose a public, lock-safe read model without touching task lifecycle: - agent.Handler.Snapshot() returns []TaskInfo (id, agent, task text, status string, start time). It ranges the tasks map reading only fields fixed at creation plus the atomic status, so it is safe to call concurrently with running task goroutines and never leaks the internal *task. runCollecting's event-drop semantics are unchanged. - LocalRuntime.BackgroundAgents() surfaces the snapshot; the App reaches it through an optional interface so remote runtimes report none, like the other read-only runtime accessors. This is the seam the TUI's live background-agent surface polls. Refs #3103
Concurrent run_background_agent tasks were invisible in the TUI: their events are dropped by runCollecting and the goroutines outlive the per-turn event sink, so there is no live stream to forward. Poll the runtime snapshot instead and surface the fleet: - The chat page starts a 1s tea.Tick when a stream begins and keeps it running while a stream works or any background task is still running (tasks can outlive the turn), feeding sidebar.SetBackgroundAgents. The loop self-stops when idle, so ordinary single-agent turns pay nothing. - The sidebar renders a "Background agents (N)" panel: one row per running task with a colored activity dot, the sub-agent's name in its accent color, and the elapsed run time. It keeps only running tasks (sorted by start time) and hides when empty. - The Phase 2 delegation breadcrumb gains the deferred "+N background" count, fitted so the chain elides before the count is clipped. - Export toolcommon.FormatDuration to reuse the shared elapsed-time formatter for the per-row run time. Refs #3103
d432398 to
0ed315d
Compare
8d9af10 to
9588e5f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Until now, a dispatched
run_background_agentfleet was essentially invisible in the TUI: events are dropped by design (runCollecting), so the human only saw the initial "task started" line and whatever the model chose to poll. This adds a live, at-a-glance surface:Handler.Snapshot() []TaskInfoover the existingtasksconcurrent map — strictly read-only, race-free, no new locks, never exposes*task, and no change to execution / event-drop semantics.tea.Tick) that runs while a stream is working or any task is still running (background tasks outlive the turn), and self-stops when idle.Background agents (N): one row per running task —● <agent> <elapsed>(accent dot + name, right-aligned elapsed), hidden entirely when nothing runs.+N backgroundbreadcrumb count (theTODO(#3103)left in TUI: make sequential / multiple delegations legible #3102).Why polling, not event-push
The background goroutines outlive the per-turn event sink (that's why events are dropped today), so there's no live stream to forward safely. Polling a lock-safe snapshot is simple, self-limiting (stops when empty), and adds no runtime coupling.
Screenshots
Background agents (3)panel while three researchers run in parallel — plus the Phase-2 breadcrumb gaining+3 backgroundduring a concurrenttransfer_task:The panel is live: the count drains (3 → 1) and the section disappears as tasks finish:
Tests
pkg/tools/builtin/agent—Snapshot()table test (running/completed/stopped/empty); passesgo test -race.pkg/tui/page/chat— poll idempotency, continue/stop lifecycle, running-task detection.pkg/tui/components/sidebar— panel render/hide, filter/sort/drain,+N background.task build/task lintclean; touched-package tests green (only the unrelatedpkg/teamloader/TestLoadExamplesDMR 416 fails). Cleared local review (incl. a-racepass on the snapshot).Deviations (deliberate)
●conveys state.+N backgroundis appended only to the depth>1 delegation breadcrumb; the standalone panel covers the no-delegation case.Stack
4th in the delegation-readability stack, stacked on #3117 (Phase 3 C.1): #3115 ← #3116 ← #3117 ← this. Tracking #3104. C.3 (depth indentation) remains an out-of-scope stretch.
Refs #3103