Skip to content

feat(coding-agent): /ps and /subagents, and release background commands on quit only - #351

Merged
liuedcson merged 3 commits into
mainfrom
feat/terminal-commands-and-fixes
Nov 7, 2026
Merged

liuedcson merged 3 commits into
mainfrom
feat/terminal-commands-and-fixes

Conversation

@liuedcson

Copy link
Copy Markdown
Collaborator

Background

Two follow-ups to the process-registry work in #349.

A bug in that change. session_shutdown fires for five reasons and only one of them ends the
session:

reason: "quit" | "reload" | "new" | "resume" | "fork"

new, resume and fork replace the runtime while the old conversation stays on disk — and, in a
multi-session host, on screen — and reload keeps the same session object. The cleanup released on
all 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. new also
reaches the handler from a session that is still open elsewhere.

process-registry.ts promises 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 quit only (features/step.ts)

if (event.reason === "quit") releaseSessionProcesses(ctx);

quit covers a host that is exiting and a session that is deleted (WebSession.delete →
close() → dispose()). A host killed outright is covered by the process.exit sweep.

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 terminal ps output it is
modelled on.

  • kill all narrows to command pids. releaseOwnerGracefully is owner-wide and kind-agnostic by
    design, so passing the whole owner would also stop the session's subagent trees — not what the
    command promises.
  • It reports before it awaits: releasing a tree takes the SIGTERM grace window, and three silent
    seconds read as ignored.
  • A repeat kill all during that window counts the commands already stopping instead of answering
    "Stopped 0".
  • notify is captured before the await, because a fork during the wait invalidates ctx and a
    stale 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_CHANNEL rather than the lane map: it observes lanes without extending their
lifetime. Settled lanes are evicted from its map (it never shrank otherwise). skipped is not
counted 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 yet
populated says starting instead of printing a bare heading.

4. Both are terminal commands

Web and Desktop draw the same state in their own interface, and the notify a transcript command
depends on is a no-op in an RPC host — a command there would look like nothing happened.
registerCommand runs once per process and cannot see the host's mode, so the check is at
invocation.

5. Session status line (features/step.ts)

publishBackgroundCommandStatus reports N background commands in the footer, subscribed per
session 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 aboveEditor widget for background lanes. A panel that sits above the editor
for as long as a lane runs competes with the conversation, and its rows cannot show which step a
child is on. publishLaneSnapshot is called from the lifecycle wrapper before the host hook, so
snapshot publishing is unchanged — verified by deleting the call, which fails five tests in
step-subagent-events.test.ts. The blocking subagent call keeps its inline list: that is the
turn'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 all correctness, refusal
    in a non-terminal host, usage errors.
  • test/step-subagents.test.ts (7) — lane/task/activity rendering, thinking fallback, settled lanes
    drop out, folding, refusal in a non-terminal host.
  • test/session-process-release.test.ts (4) — session_shutdown releases on quit and on nothing
    else. Reverting the guard makes this test fail, and that was checked.

Also: test/subagent-background-tui.test.ts now asserts no widget is ever set; re-adding one makes
it 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 check passes, including all 16 repository guards.

边界

  • Real Windows hosts are not verified. The parent-death supervisor's taskkill path is exercised by
    inference only.
  • /ps and /subagents do not page; they list up to 16 rows and fold the rest.

…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.
`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.
@liuedcson
liuedcson merged commit 57c5682 into main Nov 7, 2026
1 of 2 checks passed
@liuedcson
liuedcson deleted the feat/terminal-commands-and-fixes branch November 7, 2026 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant