diff --git a/packages/bash-local/README.md b/packages/bash-local/README.md index cceefbb67c..fe585492d8 100644 --- a/packages/bash-local/README.md +++ b/packages/bash-local/README.md @@ -18,7 +18,7 @@ Local-subprocess implementation of the `@deepseek-ai/dsh-bash` executor seam: `L Design surveyed against the bash tools of Claude Code, OpenCode, Codex, and pi; the notable choices: -- **Spawn per call, no shell state** — every call is a fresh non-login `bash -c` (deterministic; no rc files). All four surveyed tools spawn per call. `TODO(stateful-shell)` in `src/run.ts` records the two proven stateful designs (Claude Code's cwd-only persistence; Codex's PTY exec sessions) for when real workflows demand them. +- **Spawn per call, no shell state** — every call is a fresh non-login `bash -c` (deterministic; no rc files). All four surveyed tools spawn per call. `XXX(stateful-shell)` in `src/run.ts` records the two proven stateful designs (Claude Code's cwd-only persistence; Codex's PTY exec sessions) for when real workflows demand them. - **Process-group kills with escalation** — children are spawned `detached` (own process group); kills send SIGTERM to the group, then SIGKILL after a 3s grace (OpenCode's escalation; pipelines and subshells die with the parent). ESRCH is tolerated; daemons that re-parent away from the group can still survive — same caveat as the surveyed tools. - **Tail-keep truncation + spill files** — output beyond `maxOutputBytes` keeps the in-memory TAIL (errors/results cluster at the end — pi/OpenCode rationale) while the FULL stream is appended to a temp file whose path is reported. The model can `grep`/`tail` the spill file with bash itself. - **Model-friendly env** — `NO_COLOR=1 TERM=dumb PAGER=cat GIT_PAGER=cat` (Codex's hardcoded set) so pagers and ANSI color don't garble results. diff --git a/packages/bash-local/src/run.ts b/packages/bash-local/src/run.ts index 93551c8e6b..d78ef815c6 100644 --- a/packages/bash-local/src/run.ts +++ b/packages/bash-local/src/run.ts @@ -245,7 +245,7 @@ export interface RunningBash { * RESOLVES with a {@link SpawnOutcome} describing what happened, so callers * shape one consistent report for the model. * - * TODO(stateful-shell): per the agent-tool survey there are two proven + * XXX(stateful-shell): per the agent-tool survey there are two proven * stateful designs worth revisiting — Claude Code persists ONLY cwd between * calls (captures `pwd -P` after each command), and Codex keeps whole PTY * exec sessions addressable via session ids + stdin writes. We deliberately diff --git a/packages/tool-bash/README.md b/packages/tool-bash/README.md index f5e0e1dfda..5a53bce17f 100644 --- a/packages/tool-bash/README.md +++ b/packages/tool-bash/README.md @@ -30,7 +30,7 @@ Result text: stdout, then a `[stderr]` section, then status markers — `[timed ### Task ownership (cross-session isolation) -The owning agent is recorded per task id at spawn and kept for the lifetime of the loaded plugin instance (it is **not** cleared on completion). `bash_output`/`bash_kill` reject a task owned by a *different* agent with `task belongs to another session` (a task started with no agent — a non-loop caller — has no owner and is open to anyone; a call with no `exec.agent` cannot access an owned task). Task ids are global and predictable, so under multi-session ACP this ownership check is the fence that stops one session's agent from reading or killing another session's background task. (`TODO(tool-bash-owner-hmr)`: an independent HMR reload of this plugin starts a fresh map, so a task spawned before the reload becomes un-owned — acceptable as HMR is dev-only and the session boundary is one user's cooperative editor; a durable fix attaches ownership to the executor/task lifetime.) +The owning agent is recorded per task id at spawn and kept for the lifetime of the loaded plugin instance (it is **not** cleared on completion). `bash_output`/`bash_kill` reject a task owned by a *different* agent with `task belongs to another session` (a task started with no agent — a non-loop caller — has no owner and is open to anyone; a call with no `exec.agent` cannot access an owned task). Task ids are global and predictable, so under multi-session ACP this ownership check is the fence that stops one session's agent from reading or killing another session's background task. (`XXX(tool-bash-owner-hmr)`: an independent HMR reload of this plugin starts a fresh map, so a task spawned before the reload becomes un-owned — acceptable as HMR is dev-only and the session boundary is one user's cooperative editor; a durable fix attaches ownership to the executor/task lifetime.) ## UI presentation diff --git a/packages/tool-bash/src/index.ts b/packages/tool-bash/src/index.ts index 7c52705a44..1b01880e5a 100644 --- a/packages/tool-bash/src/index.ts +++ b/packages/tool-bash/src/index.ts @@ -20,7 +20,7 @@ * ownership check is the fence that stops one session's agent from reading or * killing another session's background task. * - * TODO(tool-bash-owner-hmr): the ownership map is per-plugin-instance, so an + * XXX(tool-bash-owner-hmr): the ownership map is per-plugin-instance, so an * independent HMR reload of `tool-bash` (without reloading `dsh-bash`) starts a * fresh map and a task spawned before the reload becomes un-owned (open to any * caller). This is acceptable today — HMR is dev-only, the ACP session boundary @@ -276,7 +276,7 @@ export function apply(ctx: Context): void { // another session's background task. A task with no recorded owner (started by // a non-loop caller, `exec.agent` absent) is unowned and accessible to anyone. // An independent `tool-bash` HMR reload resets this map — see the - // TODO(tool-bash-owner-hmr) note in the module doc. + // XXX(tool-bash-owner-hmr) note in the module doc. const taskOwner = new Map() /** diff --git a/packages/tool-bash/tests/tools.spec.ts b/packages/tool-bash/tests/tools.spec.ts index cb54a8a60f..46194f33d2 100644 --- a/packages/tool-bash/tests/tools.spec.ts +++ b/packages/tool-bash/tests/tools.spec.ts @@ -410,7 +410,7 @@ describe('background task ownership (cross-session isolation)', () => { }) it('documents the HMR caveat: an independent tool-bash reload resets ownership', async () => { - // The ownership map is per-plugin-instance (TODO(tool-bash-owner-hmr)). When + // The ownership map is per-plugin-instance (XXX(tool-bash-owner-hmr)). When // ONLY tool-bash is reloaded (bash/executor + task survive), the new instance // has an empty map, so the previously-owned task becomes unowned (open). This // test pins that documented behavior — a regression here (e.g. an accidental