feat(bash): rework background shells to the Claude-Code model (unbounded + read/kill by id) - #236
Merged
Merged
Conversation
…ded + read/kill by id) Replaces the timeout-bounded, push-once background-shell design (#233) with Claude Code's proven model: background bash runs UNBOUNDED and the model reads its output and stops it explicitly by id. - New BackgroundShellStore (bg_shell.rs): per-shell live output buffer (unread, drained on read; hard-capped so a never-read flood can't OOM), status (Running/Exited/Killed/Failed), and the drain JoinHandle. Process-global (like the subagent /kill registry) so the bash tool, the new tools, the status bar, and session cleanup share one instance without threading it through every signature; tests inject their own store. - bash background=true: spawn_streaming_shell runs the command detached with no timeout (optional `timeout` = auto-kill-after-N), streaming stdout/stderr into the store; returns a shell id immediately. Permission + sandbox checks run before the spawn as before. PgKillGuard SIGKILLs the process group on abort. - New model-facing tools (mirroring Claude Code): `bash_output` (read new output + status by id) and `kill_shell` (kill by id). Registered in both tool paths, added to BUILTIN_TOOL_NAMES, exposed in the schema. - Reverted #233's TaskKind piggyback on BackgroundStore (push-once delivery doesn't fit a long-lived process): back to subagents-only + running_count(). - Status bar: agents:N from BackgroundStore.running_count(), shells:N from the shell store. /tasks now lists background shells with status. Session swap/end kills all background shells. Tests: store unit tests (drain/cap/kill/finish-first-wins/list), an end-to-end unbounded background bash (streams output, exits clean, count returns to 0), and status-bar per-kind badges. Full feature-matrix suite green at -D warnings (2150 passed).
yogthos
force-pushed
the
feat/background-shells-claude-code-model
branch
from
May 30, 2026 03:03
261f8bd to
9afff34
Compare
allen-munsch
pushed a commit
to allen-munsch/dirge
that referenced
this pull request
May 30, 2026
Post-merge review of dirge-code#236: - HIGH: spawn_streaming_shell never disarmed its PgKillGuard, so on natural (and timeout) exit the guard's Drop fired a second killpg(SIGKILL) against an already-reaped, possibly OS-recycled process-group id. Now disarmed before finish() on every non-aborted completion path (matching run_with_timeout); the guard still fires on the abort/kill path, which is how kill_shell works. - MED-HIGH: background shells (own process group) leaked past normal exit — kill_all() only ran on session swap. Now also called when run_interactive returns (/quit, Ctrl+C/D, EOF) and at the headless session-end points. - MED: register() eviction at capacity dropped a still-running entry's JoinHandle (tokio detaches on drop → orphaned untracked process). Now evicts the oldest TERMINAL entry preferentially, and aborts the handle if a running entry must be evicted as a last resort. - MED: the arg schema description still described the reverted push-once model ('output delivered automatically, do NOT poll, killed at the timeout'). Rewritten to the poll-bash_output / unbounded / kill_shell model. - LOW: fixed a stale test doc-comment referencing the removed TaskKind::Shell; documented the Windows process-tree-kill limitation on spawn_streaming_shell. Tests: eviction aborts (not detaches) an evicted running shell; eviction prefers terminal entries. Full feature-matrix suite green at -D warnings (2166 passed).
allen-munsch
pushed a commit
to allen-munsch/dirge
that referenced
this pull request
Jun 3, 2026
…lls-claude-code-model feat(bash): rework background shells to the Claude-Code model (unbounded + read/kill by id)
allen-munsch
pushed a commit
to allen-munsch/dirge
that referenced
this pull request
Jun 3, 2026
Post-merge review of dirge-code#236: - HIGH: spawn_streaming_shell never disarmed its PgKillGuard, so on natural (and timeout) exit the guard's Drop fired a second killpg(SIGKILL) against an already-reaped, possibly OS-recycled process-group id. Now disarmed before finish() on every non-aborted completion path (matching run_with_timeout); the guard still fires on the abort/kill path, which is how kill_shell works. - MED-HIGH: background shells (own process group) leaked past normal exit — kill_all() only ran on session swap. Now also called when run_interactive returns (/quit, Ctrl+C/D, EOF) and at the headless session-end points. - MED: register() eviction at capacity dropped a still-running entry's JoinHandle (tokio detaches on drop → orphaned untracked process). Now evicts the oldest TERMINAL entry preferentially, and aborts the handle if a running entry must be evicted as a last resort. - MED: the arg schema description still described the reverted push-once model ('output delivered automatically, do NOT poll, killed at the timeout'). Rewritten to the poll-bash_output / unbounded / kill_shell model. - LOW: fixed a stale test doc-comment referencing the removed TaskKind::Shell; documented the Windows process-tree-kill limitation on spawn_streaming_shell. Tests: eviction aborts (not detaches) an evicted running shell; eviction prefers terminal entries. Full feature-matrix suite green at -D warnings (2166 passed).
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
1 participant
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.
Reworks the background-shell feature (#233) to follow Claude Code's proven model, per discussion: unbounded background commands that the model reads and kills by id, instead of the timeout-bounded, push-once-on-completion design.
Why
#233 bounded background shells by a timeout and delivered output once on completion — the wrong shape for a real dev server / watcher (it'd get SIGKILL'd at the timeout and never deliver). Claude Code uses
run_in_background(unbounded) +BashOutput(read by id) +KillShell(kill by id). This mirrors that.Changes
BackgroundShellStore(bg_shell.rs) — per-shell live output buffer (unread, drained on read; hard-capped so a never-read flood can't OOM), status (Running/Exited(code)/Killed/Failed), and the drainJoinHandle. Process-global (the same pattern dirge already uses for the subagent/killregistry) so the bash tool, the new tools, the status bar, and session cleanup share one instance without threading it through every builder/UI signature; tests inject their own store so they stay isolated.bash background=true—spawn_streaming_shellruns the command detached, unbounded (optionaltimeoutbecomes auto-kill-after-N), streaming stdout/stderr into the store as it arrives; returns a shell id immediately. Permission + sandbox checks run before the spawn exactly as for foreground; the existingPgKillGuardSIGKILLs the process group when the drain task is aborted.bash_output(read new output since last call + status, by id) andkill_shell(kill by id). Registered in both tool paths, added toBUILTIN_TOOL_NAMES, schemas exposed.TaskKindpiggyback onBackgroundStore(its push-once delivery doesn't fit a long-lived process) — back to subagents-only +running_count().agents:Nfrom the subagent store,shells:Nfrom the shell store./tasksnow lists background shells with status. Session swap / end kills all background shells so they don't outlive the session.Model-facing flow
bash(command, background=true[, timeout])→"background shell started — id: …"→ model pollsbash_output(id)to follow output and seesexited(code)/killedwhen done →kill_shell(id)to stop it early.Tests
bg_shellunit tests: drain-on-read, unread cap, kill-only-when-running, finish-first-terminal-wins, list.echooutput, exitsExited(0), running count returns to 0.agents:N/shells:Ncounted separately).-D warnings(2150 passed).