feat(tui): dedicated renderers for background-agent tools (#3103)#3117
feat(tui): dedicated renderers for background-agent tools (#3103)#3117aheritier wants to merge 1 commit into
Conversation
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
This PR adds dedicated TUI renderers for the four background-agent tools (run_background_agent, list_background_agents, view_background_agent, stop_background_agent). The implementation correctly mirrors the transfertask sibling pattern, and factory registration is complete and correct — all four tool-name constants map to the right constructors.
No high or medium severity bugs found. The drafter identified five low-severity observations:
renderRunusesjson.Unmarshaldirectly rather than the partial-JSONtoolcommon.ExtractFieldpath — streaming partial payloads produce an emptyparams.Task(card renders an orphaned icon). Thetransfertasksibling has the same limitation, so this is consistent with the codebase.iconWidthcomputed fromlipgloss.Widthof an ANSI-styled string is correct today but would silently misalign wrapped-line indents if margin conventions changed.renderListintentionally surfaces error-status content as a result, which is reasonable but asymmetric withNoArgsRenderer.TestRunCard_StatusIconhard-codes spinner frame"⠋", creating a fragile coupling to spinner implementation details.TestRunCard_IconWidthIsStabledoes not assertlen(parts)==3before indexingparts[len(parts)-1], so a degenerate render could silently measure the wrong column.
None of these warrant blocking the PR. Test coverage for status-icon transitions, icon-width stability, view/stop task-ID extraction, and list result visibility is solid.
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟡 NEEDS ATTENTION
One medium-severity finding in the new backgroundagent renderer package. The PR correctly mirrors the transfertask pattern and the factory registration is complete. The finding is a known streaming limitation that toolcommon.ParseArgs was designed to address.
d38caed to
3871fdb
Compare
cf40afa to
d0f18c0
Compare
3871fdb to
af97d1c
Compare
d0f18c0 to
d432398
Compare
af97d1c to
e19f53b
Compare
d432398 to
0ed315d
Compare
The four background-agent tools (run/list/view/stop) had no entry in the tool factory, so they fell through to defaulttool and rendered as raw JSON/text blobs. Give them dedicated, pure-presentation renderers so a dispatched fleet reads as delegations: - run_background_agent: a delegation-style card "<sender> dispatches <agent>" with the task text, led by a status-aware icon (animated spinner while Running/Pending, checkmark on success, cross on error). Reuses the transfer_task card's fixed-width single-glyph icon so the wrapped task-text indent stays stable across statuses. - view_background_agent / stop_background_agent: SimpleRendererWithResult showing the task id (argument) plus the already human-formatted result text once the call lands. - list_background_agents: shows the tool result (the task listing). The plan suggested NoArgsRenderer, but that drops the result, which is the only useful payload for list until the C.2 live surface lands; showing it also avoids regressing below the prior defaulttool behavior. No runtime change. Refs #3103
e19f53b to
79ea210
Compare
0ed315d to
fc080ad
Compare
Summary
The four background-agent tools (
run/list/view/stop_background_agent) had no entry in the tool factory, so they fell through todefaulttooland rendered as raw JSON / text blobs. This gives them dedicated, presentation-only renderers so a dispatched fleet reads like delegations. Builds on #3102.run_background_agent→ a delegation-style cardroot dispatches worker(accent badges) + the task text, led by the same status-aware fixed-width icon as thetransfer_taskcard (TUI: make a single delegation unambiguous (transfer_task) #3101).view/stop_background_agent→SimpleRendererWithResultshowing the task id (argument) + the formatted result.list_background_agents→ shows the task listing. (The plan suggestedNoArgsRenderer, but that drops the result — the only useful payload until a live surface lands — so the listing is preserved.)No runtime change. The live "Background agents (N)" sidebar panel (Appendix C.2) is intentionally out of scope here and remains approval-gated.
Screenshots
run_background_agentfans out to several workers in parallel — each renders as aroot dispatches workercard:list_background_agentsshows the live fleet (three workers running concurrently):Tests
pkg/tui/components/tool/backgroundagent—runper-ToolStatusglyph + fixed-width invariant;view/stopshow task id + result;listshows its result; ANSI-stripped.task build/task lintclean; touched-package tests pass (same unrelatedteamloaderDMR failure as #3101).Stack
Top of the stack — stacked on #3102 (← #3101). Tracking #3104.
Refs #3103