Add background agent resume and continuation#3739
Conversation
…-resume # Conflicts: # packages/cli/src/ui/components/background-view/BackgroundTasksDialog.test.tsx # packages/cli/src/ui/components/background-view/BackgroundTasksDialog.tsx # packages/cli/src/ui/components/background-view/BackgroundTasksPill.test.tsx # packages/cli/src/ui/components/background-view/BackgroundTasksPill.tsx # packages/cli/src/ui/contexts/BackgroundTaskViewContext.tsx # packages/core/src/tools/task-stop.test.ts # packages/core/src/tools/task-stop.ts
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
wenshao
left a comment
There was a problem hiding this comment.
[Critical] packages/core/src/tools/shell.ts:57 — stripTrailingBackgroundAmp() strips a final & by inspecting only the last characters of the command string. This rewrites valid foreground shell syntax where & is data inside quotes or substitutions, for example printf '&', echo "foo &", or node -e "console.log('&')".
With is_background: true, the managed background shell path can execute a different command than requested, potentially making it syntactically invalid or changing behavior. Avoid rewriting the command, or make the stripper shell-aware before removing a trailing background operator. At minimum, add tests for quoted, escaped, and substitution cases.
[Suggestion] packages/cli/src/ui/commands/tasksCommand.ts:28 — The /tasks command is named/described as listing “background tasks”, but it only reads config.getBackgroundShellRegistry().getAll() and reports “No background shells.” / “Background shells (...)”. This PR also adds a merged background task UI where background agents and shells are shown together.
A paused/running/completed background agent can appear in the footer/dialog but be invisible to /tasks, even though the command name promises all background tasks. Either merge config.getBackgroundTaskRegistry().getAll() with shell entries here, or rename/scope the command description and output to background shells only.
— gpt-5.5 via Qwen Code /review
|
Follow-up on the latest review items:
Validation run on branch head:
|
|
Comparison with Claude Code after the latest fixes: Aligned behavior:
Intentional differences from Claude Code:
Current capability gaps vs Claude Code:
So at this point the local background-agent resume path is functionally aligned with Claude Code's transcript-first model, while remaining deliberately more conservative in a few safety-sensitive cases. The main remaining differences are capability gaps (worktree-aware resume, replacement-state reconstruction, remote task recovery), not regressions in the current background-agent flow. |
|
Quick blocker check before this lands: 1. The [Critical] at
The 2. Windows CI pending re-run. The previous Windows-latest 20.x / 22.x / 24.x jobs failed; a new run is in-progress on the latest commit. Please confirm green before merge — if it fails again, we need to investigate whether the new background-task lifecycle introduces a Windows-specific regression (path separators in transcript paths, file-locking around sidecar JSONL, etc.). Once both are addressed I'll re-review and approve. Everything else from the prior rounds (5 follow-up fixes in |
wenshao
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅ — gpt-5.5 via Qwen Code /review
wenshao
left a comment
There was a problem hiding this comment.
Approving on fbac57d9f — the :309 [Critical] is fully addressed:
pendingUserMessagefield removed fromrecoverTranscript()(grep 0 hits inbackground-agent-resume.ts)- Trailing
userrecords now stay in reconstructedhistoryinstead of being collapsed intoinitial_messages_override initial_messages_overridemerging logic is gone- Tests updated to assert the new behavior
CI 13/13 green (Windows now passing). All other earlier feedback rounds (cancel/abort split, trailing-& wrapper validation, empty-history bootstrap, session-switch guards, transcript-first recovery) landed cleanly with regression tests.
Solid work — and thanks for the explicit Claude Code alignment writeup; the intentional-differences/capability-gaps framing was very useful to review against. LGTM.
Summary
What changed
BackgroundAgentResumeServiceto discover paused background agents fromsubagents/<sessionId>/pausedin the registry/UISubagentStarthooks, coalesce concurrent resume attempts, and append to the original transcriptsend_messageandtask_stopto handle paused background agents/resumeflow to load paused tasks and show a transient recovery noticeTranscript-first fork resume
The fork path now follows a transcript-first recovery model:
system/agent_bootstrapsystem/agent_launch_promptReview follow-up
I also did a focused review of downstream consumers affected by the transcript schema change:
slash_commandsystem records and otherwise ignores the new system subtypes safelyagent_bootstrap/agent_launch_promptdo not leak into replayed chatuser,assistant,tool_result, andslash_commandrecords for metrics/facets, so the new system records do not skew resultsValidation
npm run typechecknpx vitest run packages/core/src/agents/background-agent-resume.test.ts packages/core/src/agents/agent-transcript.test.ts packages/core/src/tools/send-message.test.ts packages/core/src/tools/task-stop.test.ts packages/core/src/agents/background-tasks.test.ts packages/cli/src/ui/hooks/useResumeCommand.test.ts packages/cli/src/ui/components/background-view/BackgroundTasksDialog.test.tsx packages/cli/src/ui/components/background-view/BackgroundTasksPill.test.tsx packages/cli/src/acp-integration/session/HistoryReplayer.test.ts packages/vscode-ide-companion/src/services/sessionExportService.test.ts packages/cli/src/ui/commands/exportCommand.test.ts packages/cli/src/services/insight/generators/DataProcessor.test.ts packages/core/src/services/sessionService.test.tsgit diff --checkNotes
BackgroundTasksDialog.test.tsxstill emits pre-existing Reactact(...)warnings, but the suite passes.