Repository navigation
feat(coding-agent): /ps and /subagents, and release background commands on quit only - #351
Merged
Merged
Conversation
…ds on quit only `session_shutdown` fires for five reasons and only one of them ends the session. The cleanup added with the process registry released on all five, so `new`, `resume`, `fork` and `reload` each stopped the session's background commands: opening a new conversation or forking one killed the dev server the user was watching. `process-registry.ts` promises the opposite — `bash` survives a switch and a fork — and the comment beside the call claimed a switch or fork never reaches this handler, which is what made the bug look correct. Gate it on `reason === "quit"`, which covers a host that is exiting and a session that is deleted; a host killed outright is covered by the `process.exit` sweep. Two terminal commands, shaped the same way — print into the transcript, fold when long, scoped to the calling session: - `/ps` lists the session's background commands with pid, state, elapsed time and log path, and stops one (`/ps kill <pid>`) or all (`/ps kill all`). `kill all` narrows to command pids, because the registry's owner release is kind-agnostic and stopping subagent trees is not what it promises. It reports before it waits: releasing a tree takes the SIGTERM grace window, and three silent seconds read as ignored. A repeat `kill all` during that window says how many are already stopping instead of "Stopped 0". `notify` is captured before the await, since a fork during the wait invalidates `ctx`. - `/subagents` lists each running lane with its tasks and the step each child is on — the active tool, or the thinking/reply text while it reasons. It reads `SUBAGENT_LANE_CHANNEL`, so it observes lanes without extending their lifetime, and it evicts settled lanes (the map never shrank otherwise). Both refuse in a non-terminal host rather than silently doing nothing: Web and Desktop draw the same state in their own interface, and the `notify` a transcript command depends on is a no-op there. `registerCommand` runs once per process and cannot see the host's mode, so the check is at invocation. `pi.events?.` because a missing bus discarded the whole capabilities extension, taking `/ps`, plan, tasks and subagent with it. A session status line reports how many background commands are running. `skipped` is not counted as failed (it is a chain step that never ran), elapsed time stops at `updatedAt` so a settled task does not read as still running, and a lane whose roster is not populated yet says "starting". Tests: four `session_shutdown` reasons assert the process survives; reverting the guard makes that test fail.
liuedcson
requested review from
MelodyVAR,
Uking-xxx and
ZouR-Ma
as code owners
November 7, 2026 11:18
`session_shutdown` ran `ctx.ui.setStatus` unconditionally. A host that has already torn down its UI — or a test that drives the handler without one — has no `ui`, so shutdown threw instead of finishing. Clearing a footer line is best effort; a missing UI must not turn teardown into an error.
… command details The previous commit described removing the background-lane widget, and the doc and `lane-events.ts` comment both said it was gone, but the change was only in the desktop tree: `step-subagent.ts` still drew it. Three places claimed a state that did not exist. Remove it here: `updateLaneWidget` and `clearLaneWidget` become the no-ops the host hook expects, and `test/subagent-background-tui.test.ts` now asserts no widget is ever set — re-adding one fails it. `publishLaneSnapshot` is called from the lifecycle wrapper *before* the host hook, so snapshot publishing is unchanged; deleting that call fails five tests in `step-subagent-events.test.ts`. Also from review: - `/ps` and `/subagents` now say why they are terminal-only without claiming another repository's UI as the reason. The checkable reason is local: `notify` is a no-op in an RPC host. - The grace argument is passed explicitly instead of relying on a positional `undefined` to reach the default; a signature change would otherwise silently pick a different window. - `Stopped N` reports the count the user saw in `Stopping N`. The registry's return value includes a tree that exited between the listing and the kill, so it could read higher than what was shown. - The cross-session refusal test now gives the session one command of its own first. Before, the empty-list branch answered and the test would pass even if the pid check were broken. Not changed: `/subagents` needs no session filter. `createAgentSessionServices` builds a `DefaultResourceLoader` per session and does not pass an `eventBus`, so `createEventBus()` runs per session — every subscription sees only its own session's lane snapshots. Verified by comparing `resourceLoader.eventBus` across two sessions.
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.
Background
Two follow-ups to the process-registry work in #349.
A bug in that change.
session_shutdownfires for five reasons and only one of them ends thesession:
new,resumeandforkreplace the runtime while the old conversation stays on disk — and, in amulti-session host, on screen — and
reloadkeeps the same session object. The cleanup released onall five, so opening a new conversation, forking one, resuming another or reloading settings each
stopped that session's background commands: the dev server the user was watching died.
newalsoreaches the handler from a session that is still open elsewhere.
process-registry.tspromises the opposite —bash"must survive a session switch and a fork" —and the comment beside the call claimed a switch or fork never reaches this handler. That comment is
what made the bug look correct, so it is replaced with the reason list.
Nothing to read them with. A session can leave background commands and background subagent lanes
running, and the transcript never shows either. The lane state had a permanent widget; commands had
no surface at all.
实现
1. Release on
quitonly (features/step.ts)quitcovers a host that is exiting and a session that is deleted (WebSession.delete→close()→dispose()). A host killed outright is covered by theprocess.exitsweep.2.
/ps(features/step-ps.ts)Lists the session's background commands with pid, state, elapsed time and log path; stops one
(
/ps kill <pid>) or all (/ps kill all). Folds after 16 rows, like the terminalpsoutput it ismodelled on.
kill allnarrows to command pids.releaseOwnerGracefullyis owner-wide and kind-agnostic bydesign, so passing the whole owner would also stop the session's subagent trees — not what the
command promises.
seconds read as ignored.
kill allduring that window counts the commands already stopping instead of answering"Stopped 0".
notifyis captured before the await, because a fork during the wait invalidatesctxand astale context throws on read.
3.
/subagents(features/step-subagents-command.ts)Lists each running lane with its tasks and the step each child is on — the active tool, or the
thinking/reply text while it reasons, so a child that is reasoning does not read as idle.
Reads
SUBAGENT_LANE_CHANNELrather than the lane map: it observes lanes without extending theirlifetime. Settled lanes are evicted from its map (it never shrank otherwise).
skippedis notcounted as failed — it is a chain step that never ran, a normal ending. Elapsed time stops at
updatedAt, so a settled task no longer reads as still running. A lane whose roster is not yetpopulated says
startinginstead of printing a bare heading.4. Both are terminal commands
Web and Desktop draw the same state in their own interface, and the
notifya transcript commanddepends on is a no-op in an RPC host — a command there would look like nothing happened.
registerCommandruns once per process and cannot see the host's mode, so the check is atinvocation.
5. Session status line (
features/step.ts)publishBackgroundCommandStatusreportsN background commandsin the footer, subscribed persession and released on shutdown. The transcript command answers "which one"; the status line
answers "is anything running" without being asked.
6. No lane widget
Removed the permanent
aboveEditorwidget for background lanes. A panel that sits above the editorfor as long as a lane runs competes with the conversation, and its rows cannot show which step a
child is on.
publishLaneSnapshotis called from the lifecycle wrapper before the host hook, sosnapshot publishing is unchanged — verified by deleting the call, which fails five tests in
step-subagent-events.test.ts. The blockingsubagentcall keeps its inline list: that is theturn's own output and ends with it.
Progress notifications to the model stay out of the transcript, now stated as the deliberate rule it
is rather than justified by the deleted widget. Completion and needs-input still display.
验证
New tests (19):
test/step-ps.test.ts(8) — owner scoping, folding,kill <pid>/kill allcorrectness, refusalin a non-terminal host, usage errors.
test/step-subagents.test.ts(7) — lane/task/activity rendering, thinking fallback, settled lanesdrop out, folding, refusal in a non-terminal host.
test/session-process-release.test.ts(4) —session_shutdownreleases onquitand on nothingelse. Reverting the guard makes this test fail, and that was checked.
Also:
test/subagent-background-tui.test.tsnow asserts no widget is ever set; re-adding one makesit fail, which was checked. The assertion waits for both lanes to settle and then drains a repaint
interval, rather than sampling for a fixed 50 ms — a late publish escaped the short sample.
pnpm run checkpasses, including all 16 repository guards.边界
taskkillpath is exercised byinference only.
/psand/subagentsdo not page; they list up to 16 rows and fold the rest.