feat(acp): multiplex N concurrent ACP sessions + bash task ownership (RFC 011)
Lifts the RFC 010 single-session-per-connection cap: the bridge now runs N concurrent sessions over one connection, each mapped to its own LoopAgent. - packages/acp: live sessions held in a Map<sessionId, SessionRecord> with an agent→sessionId reverse WeakMap so agent/* events (which carry only the Agent) demux in O(1). Every session/event and agent/status is routed strictly to its owning record — concurrent sessions never cross-settle or interleave their session/update notifications. Per-session state: one in-flight prompt each, session/cancel aborts+settles only its own agent/prompt, session/load reserves a per-id load slot (distinct ids load concurrently; re-loading a live id is rejected), and disposal drains every live session in parallel to quiescence. - packages/tool-bash: record each background task's owning agent at spawn and keep it for the executor's lifetime (NOT cleared on completion). bash_output/bash_kill reject a task owned by a different agent (a task with no owner is open; a no-agent caller can't access an owned task). Task ids are global and predictable, so this is the fence that stops one session's agent from reading/killing another session's background task. - Per-session permission ownership and a per-agent disposer seam stay deferred (depend on the deferred permission gate); the reverse map the gate will route through is in place. RFC 011 stays `proposed`. - Tests: two sessions stream concurrently without interleave; cross-session cancel isolation; per-session in-flight enforcement; dispose-all-to-quiescence; bash cross-session read/kill rejected (+ no-agent and unowned-task cases). - Docs: RFC 011 implementation-status note; acp + tool-bash READMEs; example MVP-limitations updated. 100% per-file coverage maintained.
This commit is contained in:
Tianyi Cui
@@ -28,6 +28,10 @@ Result text: stdout, then a `[stderr]` section, then status markers — `[timed
|
||||
|
||||
`task_id` → SIGTERM→SIGKILL on the task's process group. Killing an already-finished task is a reported no-op; unknown ids are errors.
|
||||
|
||||
### 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 <id> 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.)
|
||||
|
||||
## Background completion notices
|
||||
|
||||
When a background task finishes, a short notice is injected into the owning agent's session (`agent.inject()`, source `{kind: 'plugin', plugin: 'tool-bash'}`). Injection is **durable context for the next model request, not a wake-up** — an idle agent stays idle until something sends a message. That's why the tool descriptions tell the model to poll with `bash_output`.
|
||||
|
||||
@@ -11,6 +11,23 @@
|
||||
* message, which is why the tool descriptions tell the model to poll with
|
||||
* `bash_output`.
|
||||
*
|
||||
* Task ownership: the owning agent is recorded per task id at spawn and kept
|
||||
* for the lifetime of THIS plugin instance (it is NOT cleared on task
|
||||
* completion — a finished task must stay un-readable / un-killable by a
|
||||
* different agent). `bash_output`/`bash_kill` reject a task owned by a DIFFERENT
|
||||
* agent (a task with no recorded owner is open to anyone). Task ids are global
|
||||
* and predictable (`bash-1`, …); under multi-session ACP (RFC 011) 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): 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
|
||||
* is one user's cooperative editor (not an adversarial trust boundary), and the
|
||||
* executor's own disposal kills its tasks — but a durable fix would attach
|
||||
* ownership to the executor/task lifetime via a `dsh-bash` seam.
|
||||
*
|
||||
* TODO(permissions): commands run with the executor's full authority. The
|
||||
* permission/sandbox seam is the `tools/execute` waterfall (veto/ask) plus
|
||||
* sandboxing `BashExecutor` implementations — see docs/architecture.md
|
||||
@@ -115,12 +132,33 @@ function statusLine(task: BashTask): string {
|
||||
}
|
||||
|
||||
export function apply(ctx: Context): void {
|
||||
// Owning agent per background task id, recorded at spawn. Kept for the
|
||||
// lifetime of THIS plugin instance (NOT cleared on completion): a completed
|
||||
// task must stay un-readable / un-killable by a DIFFERENT agent, so the
|
||||
// ownership record outlives the task. Under multi-session ACP (RFC 011) this
|
||||
// is the isolation fence — one session's agent must never read or kill
|
||||
// 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.
|
||||
const taskOwner = new Map<string, Agent>()
|
||||
|
||||
/**
|
||||
* Authorize a `bash_output`/`bash_kill` call against a task's owner. Rejects
|
||||
* when the task has a recorded owner and the caller is not that exact agent —
|
||||
* including the conservative no-agent case (`exec.agent` absent cannot prove
|
||||
* ownership of an owned task). An unowned task (no record) is allowed.
|
||||
*/
|
||||
const assertTaskAccess = (taskId: string, exec: { agent?: Agent }): void => {
|
||||
const owner = taskOwner.get(taskId)
|
||||
if (owner !== undefined && owner !== exec.agent) {
|
||||
throw new Error(`task ${taskId} belongs to another session`)
|
||||
}
|
||||
}
|
||||
|
||||
// Background completion → inject a notice into the owning agent's session.
|
||||
// Tracks the agent per task id; entries drop once notified.
|
||||
const owners = new Map<string, Agent>()
|
||||
ctx.bash.onTaskDone((task) => {
|
||||
const agent = owners.get(task.id)
|
||||
owners.delete(task.id)
|
||||
const agent = taskOwner.get(task.id)
|
||||
if (!agent) return
|
||||
try {
|
||||
agent.inject(
|
||||
@@ -171,7 +209,7 @@ export function apply(ctx: Context): void {
|
||||
}
|
||||
if (args.run_in_background === true) {
|
||||
const task = ctx.bash.start(ctx.bash.resolve(request))
|
||||
if (exec.agent) owners.set(task.id, exec.agent)
|
||||
if (exec.agent) taskOwner.set(task.id, exec.agent)
|
||||
return [{ type: 'text', text: `started background task ${task.id}` }]
|
||||
}
|
||||
const result = await ctx.bash.run(ctx.bash.resolve(request))
|
||||
@@ -190,8 +228,10 @@ export function apply(ctx: Context): void {
|
||||
},
|
||||
// execute is synchronous (registry reads + string shaping) but the
|
||||
// ToolDefinition contract wants a Promise — hence resolve(), not async.
|
||||
execute(args) {
|
||||
const read = ctx.bash.readOutput(validateTaskId(args.task_id))
|
||||
execute(args, exec) {
|
||||
const id = validateTaskId(args.task_id)
|
||||
assertTaskAccess(id, exec)
|
||||
const read = ctx.bash.readOutput(id)
|
||||
let text = read.delta.length > 0 ? read.delta : '(no new output)'
|
||||
if (read.lossy) {
|
||||
const paths = [read.stdoutSpillPath, read.stderrSpillPath].filter((p): p is string => p !== undefined)
|
||||
@@ -208,8 +248,9 @@ export function apply(ctx: Context): void {
|
||||
parameters: {
|
||||
task_id: { type: 'string', required: true, description: 'Task id returned by the bash tool.' },
|
||||
},
|
||||
execute(args) {
|
||||
execute(args, exec) {
|
||||
const id = validateTaskId(args.task_id)
|
||||
assertTaskAccess(id, exec)
|
||||
const killed = ctx.bash.kill(id)
|
||||
return Promise.resolve([{
|
||||
type: 'text',
|
||||
|
||||
@@ -338,6 +338,106 @@ describe('background tools', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('background task ownership (cross-session isolation)', () => {
|
||||
/** Run a tool on behalf of a specific agent (sets exec.agent). */
|
||||
function callAs(ctx: Context, agent: import('@deepseek-ai/dsh-agent').Agent | undefined, name: string, args: unknown) {
|
||||
return ctx.tools.execute({ callId: CallId(`own-${++callCounter}`), name, arguments: args, ...agent ? { agent } : {} })
|
||||
}
|
||||
// Distinct identities — ownership is by agent object identity, not id.
|
||||
const fakeAgent = () => ({ inject: () => undefined }) as unknown as import('@deepseek-ai/dsh-agent').Agent
|
||||
|
||||
it('rejects bash_output/bash_kill for a task owned by a DIFFERENT agent', async () => {
|
||||
const ctx = await setup()
|
||||
const a = fakeAgent()
|
||||
const b = fakeAgent()
|
||||
// Agent A starts a long-running background task.
|
||||
const started = await callAs(ctx, a, 'bash', { command: 'sleep 60', description: 'bg', run_in_background: true })
|
||||
const id = /task (bash-\d+)/.exec(text(started))![1]!
|
||||
|
||||
// Agent B cannot read or kill A's task.
|
||||
const readByB = await callAs(ctx, b, 'bash_output', { task_id: id })
|
||||
expect(readByB.isError).toBe(true)
|
||||
expect(text(readByB)).toMatch(/belongs to another session/)
|
||||
const killByB = await callAs(ctx, b, 'bash_kill', { task_id: id })
|
||||
expect(killByB.isError).toBe(true)
|
||||
expect(text(killByB)).toMatch(/belongs to another session/)
|
||||
|
||||
// The task is still running (B's kill did nothing) — A can still kill it.
|
||||
const killByA = await callAs(ctx, a, 'bash_kill', { task_id: id })
|
||||
expect(killByA.isError).toBe(false)
|
||||
expect(text(killByA)).toBe(`killed background task ${id}`)
|
||||
})
|
||||
|
||||
it('the no-agent (non-loop) caller cannot access an owned task', async () => {
|
||||
const ctx = await setup()
|
||||
const a = fakeAgent()
|
||||
const started = await callAs(ctx, a, 'bash', { command: 'sleep 60', description: 'bg', run_in_background: true })
|
||||
const id = /task (bash-\d+)/.exec(text(started))![1]!
|
||||
// A call with no exec.agent cannot prove ownership of an owned task.
|
||||
const read = await callAs(ctx, undefined, 'bash_output', { task_id: id })
|
||||
expect(read.isError).toBe(true)
|
||||
expect(text(read)).toMatch(/belongs to another session/)
|
||||
await callAs(ctx, a, 'bash_kill', { task_id: id }) // cleanup
|
||||
})
|
||||
|
||||
it('an UNOWNED task (started with no agent) is accessible to anyone', async () => {
|
||||
const ctx = await setup()
|
||||
// Started by a non-loop caller (no exec.agent) → no recorded owner.
|
||||
const started = await callAs(ctx, undefined, 'bash', { command: 'sleep 60', description: 'bg', run_in_background: true })
|
||||
const id = /task (bash-\d+)/.exec(text(started))![1]!
|
||||
// Any agent (and the no-agent caller) may read/kill it.
|
||||
const read = await callAs(ctx, fakeAgent(), 'bash_output', { task_id: id })
|
||||
expect(read.isError).toBe(false)
|
||||
const killed = await callAs(ctx, undefined, 'bash_kill', { task_id: id })
|
||||
expect(killed.isError).toBe(false)
|
||||
})
|
||||
|
||||
it('the owner can still access its task AFTER it completes (owner record persists)', async () => {
|
||||
const ctx = await setup()
|
||||
const a = fakeAgent()
|
||||
const b = fakeAgent()
|
||||
const started = await callAs(ctx, a, 'bash', { command: 'echo done', description: 'bg', run_in_background: true })
|
||||
const id = /task (bash-\d+)/.exec(text(started))![1]!
|
||||
await ctx.bash.get(id)!.done
|
||||
// Completion does NOT clear ownership: B is still rejected, A still allowed.
|
||||
const readByB = await callAs(ctx, b, 'bash_output', { task_id: id })
|
||||
expect(readByB.isError).toBe(true)
|
||||
expect(text(readByB)).toMatch(/belongs to another session/)
|
||||
const readByA = await callAs(ctx, a, 'bash_output', { task_id: id })
|
||||
expect(readByA.isError).toBe(false)
|
||||
})
|
||||
|
||||
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
|
||||
// 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
|
||||
// global map) would change it.
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(SystemPrompt)
|
||||
await ctx.plugin(ToolRegistry)
|
||||
await ctx.plugin(LocalBashExecutor, { timeoutMs: 10_000 })
|
||||
;(ctx.bash as LocalBashExecutor).internals = { spillDir, graceMs: 200 }
|
||||
const fiber = await ctx.plugin(ToolBash)
|
||||
|
||||
const a = fakeAgent()
|
||||
const b = fakeAgent()
|
||||
const started = await callAs(ctx, a, 'bash', { command: 'sleep 60', description: 'bg', run_in_background: true })
|
||||
const id = /task (bash-\d+)/.exec(text(started))![1]!
|
||||
// Before reload: B is rejected (A owns it).
|
||||
expect((await callAs(ctx, b, 'bash_output', { task_id: id })).isError).toBe(true)
|
||||
|
||||
// Reload ONLY tool-bash; the executor and its running task survive.
|
||||
await fiber.dispose()
|
||||
await ctx.plugin(ToolBash)
|
||||
expect(ctx.bash.get(id)?.status).toBe('running')
|
||||
|
||||
// After reload the fresh map has no owner → B can now access it (the caveat).
|
||||
expect((await callAs(ctx, b, 'bash_output', { task_id: id })).isError).toBe(false)
|
||||
await callAs(ctx, b, 'bash_kill', { task_id: id }) // cleanup
|
||||
})
|
||||
})
|
||||
|
||||
describe('renderResult', () => {
|
||||
const base = {
|
||||
exitCode: 0 as number | null,
|
||||
|
||||
Reference in New Issue
Block a user