fix(agent-loop): tighten parallel tool-call safety
This commit is contained in:
@@ -26,6 +26,7 @@ Because the package wires no logger entry, an ACP leaf has **nothing to get wron
|
||||
| Key | Default | Routed to |
|
||||
|---|---|---|
|
||||
| `model` | (required) | the per-session agent template the bridge creates agents from |
|
||||
| `maxParallelToolCalls` | agent-loop default | positive-integer concurrent tool-call cap shared by the bundled loop's agents; `1` is serial |
|
||||
| `persona` | — | the deployment persona template (may reference `{{model}}`/`{{cwd}}`), routed to `dsh-system-prompt` |
|
||||
| `toolOrder` | — | explicit model-facing tool order (a name list with one `'<unlisted-tools>'` rest entry; absent — lexicographic; an unregistered name fails each turn at prompt assembly), routed to `dsh-system-prompt` |
|
||||
| `tools` | `{ mode: 'native' }` | tool-registry presentation config (`native` / `code` / `both`), routed through `dsh-agent-spine-demo` |
|
||||
|
||||
@@ -26,11 +26,17 @@ export const name = 'acp-demo'
|
||||
* deployment persona (forwarded to the system-prompt plugin); `toolOrder` is
|
||||
* the explicit model-facing tool order (forwarded to the system-prompt plugin);
|
||||
* `tools` is the tool registry's config (its presentation `mode`, forwarded
|
||||
* through agent-spine-demo); `persistenceRoot` is the JSONL backend's directory.
|
||||
* through agent-spine-demo); `maxParallelToolCalls` configures the bundled
|
||||
* agent loop; `persistenceRoot` is the JSONL backend's directory.
|
||||
*/
|
||||
export interface Config {
|
||||
/** Model name for ACP-created agents (must have a registered adapter). */
|
||||
model: string
|
||||
/**
|
||||
* Concurrent parallel-safe tool-call cap for the bundled agent loop. A
|
||||
* positive integer; the loop defaults it when omitted and `1` is serial.
|
||||
*/
|
||||
maxParallelToolCalls?: number
|
||||
/** Deployment persona (the system-prompt plugin's `persona` config). */
|
||||
persona?: string
|
||||
/** Explicit model-facing tool order (the system-prompt plugin's `toolOrder` config; see dsh-system-prompt). */
|
||||
@@ -52,6 +58,9 @@ export interface Config {
|
||||
/* jscpd:ignore-start */
|
||||
export const Config: z<Config> = z.object({
|
||||
model: z.string().required(),
|
||||
// A positive integer; a bad value (0, negative, fractional) fails config
|
||||
// validation here rather than being silently dropped from cordis.yml.
|
||||
maxParallelToolCalls: z.number().step(1).min(1),
|
||||
persona: z.string(),
|
||||
// The array default is forced to undefined: ABSENT means "lexicographic
|
||||
// order" (the owning dsh-system-prompt schema does the same), while
|
||||
@@ -79,6 +88,7 @@ export function apply(ctx: Context, config: Config): void {
|
||||
...config.persona !== undefined ? { persona: config.persona } : {},
|
||||
...config.toolOrder !== undefined ? { toolOrder: config.toolOrder } : {},
|
||||
...config.tools !== undefined ? { tools: config.tools } : {},
|
||||
...config.maxParallelToolCalls !== undefined ? { maxParallelToolCalls: config.maxParallelToolCalls } : {},
|
||||
...config.skills !== undefined ? { skills: config.skills } : {},
|
||||
...config.toolBash !== undefined ? { toolBash: config.toolBash } : {},
|
||||
...config.toolTasks !== undefined ? { toolTasks: config.toolTasks } : {},
|
||||
|
||||
@@ -113,6 +113,17 @@ describe('dsh-acp-demo composition', () => {
|
||||
await ctx.fiber.dispose()
|
||||
})
|
||||
|
||||
it('forwards maxParallelToolCalls to the bundled agent loop', async () => {
|
||||
const ctx = await mount({
|
||||
model: 'mock',
|
||||
maxParallelToolCalls: 3,
|
||||
persistenceRoot: '/tmp/dsh-acp-demo-test-parallel',
|
||||
skills: await isolatedSkillsConfig(),
|
||||
})
|
||||
expect(ctx.get('agentLoop')?.config.maxParallelToolCalls).toBe(3)
|
||||
await ctx.fiber.dispose()
|
||||
})
|
||||
|
||||
it('forwards bundled tool config into agent-core', async () => {
|
||||
const ctx = await mount({
|
||||
model: 'mock',
|
||||
|
||||
@@ -14,6 +14,4 @@ The subagent seam: an agent delegating work to a child agent. Like the [bash](..
|
||||
|
||||
The interface lives at `subagent/subagent/`. The in-process `subagent-spawn` / `subagent-fork` backends share the `subagent-inprocess` driver (a library with no provider of its own — both depend on it, neither on the other), the out-of-process `subagent-acp` backend builds on the `subagent-subprocess` library (the credential env scrub, the dispose ladder, isolated config dirs) and ships alongside them here; the test-only `dsh-subagent-mock` (in [support](../support/README.md)) is separate. All **product** packages except the mock.
|
||||
|
||||
`SubagentProvider.start()` must be safe to call concurrently for independent runs: foreground `subagent` calls are parallel-safe, so one parent step may issue several at once. Background starts remain exclusive while registering parent-owned task state. Each backend reads the parent synchronously at start (a snapshot, never mutated or re-read during the run) — `fork` seeds each child from the parent's completed-turn prefix, which the open in-flight turn cannot change, so concurrent forks inside one open step all see the same stable prefix. A resource-limited provider may queue or cap internally, but must not require the loop to serialize every foreground call.
|
||||
|
||||
The proposal and design rationale: [docs/rfc/implemented/feature/2026-06-21-subagent-capability-seam.md](../../docs/rfc/implemented/feature/2026-06-21-subagent-capability-seam.md).
|
||||
|
||||
@@ -186,13 +186,6 @@ export interface SubagentProvider {
|
||||
* honorable when present. If setup fails or `request.signal` aborts before
|
||||
* fulfillment, the provider owns and cleans all partial resources before this
|
||||
* promise rejects. Ownership transfers to the caller only on fulfillment.
|
||||
*
|
||||
* MUST be safe to call concurrently for independent runs: foreground
|
||||
* `subagent` calls are parallel-safe, so a parent step may issue several at once,
|
||||
* each invoking `start()` before an earlier run settles. An implementation
|
||||
* snapshots the parent at start and must not require the parent loop to
|
||||
* serialize every foreground `subagent` call; a resource-limited provider queues or
|
||||
* rejects internally.
|
||||
*/
|
||||
start(request: SubagentStartRequest): Promise<SubagentRun>
|
||||
}
|
||||
|
||||
@@ -26,7 +26,7 @@ With `run_in_background: true`, the tool registers the parent-owned task before
|
||||
|
||||
## Concurrency
|
||||
|
||||
Foreground calls opt into concurrent scheduling because each owns an independent child run and returns only its final answer. Background starts remain exclusive because they register parent-owned task state. Providers must accept concurrent `start()` calls for independent runs; they may queue internally, enforce capacity, or return a typed failure. See the [parallel tool-call RFC](../../../docs/rfc/implemented/feature/2026-07-10-parallel-tool-call-execution.md).
|
||||
Foreground and background calls are exclusive. Children may share the parent's workspace or external resources, and the unary scheduler classifier cannot prove that sibling delegations have disjoint effects. See the [parallel tool-call RFC](../../../docs/rfc/implemented/feature/2026-07-10-parallel-tool-call-execution.md).
|
||||
|
||||
## Model Experience
|
||||
|
||||
|
||||
@@ -174,8 +174,7 @@ export function providerWording(inheritsConversation: boolean): { description: s
|
||||
+ 'completed turns so far (it does not see the current in-flight turn), returning only its final '
|
||||
+ 'result. Use this when the subtask builds on this conversation\'s context — a follow-up analysis, '
|
||||
+ 'a review, a continuation — without consuming this conversation\'s context for the work itself. '
|
||||
+ 'You receive only its final answer, not its intermediate steps. You may issue several subagent '
|
||||
+ 'calls in one message to run independent tasks concurrently when their work scopes do not overlap.',
|
||||
+ 'You receive only its final answer, not its intermediate steps.',
|
||||
promptDescription:
|
||||
'The task for the subagent. It already sees this conversation\'s completed turns, so build on them '
|
||||
+ 'freely and state only what is new.',
|
||||
@@ -187,8 +186,7 @@ export function providerWording(inheritsConversation: boolean): { description: s
|
||||
+ 'and return its final result. Use this to offload focused, independent work — research, a scoped '
|
||||
+ 'implementation, an analysis — so it does not consume this conversation\'s context. The subagent '
|
||||
+ 'runs to completion and you receive only its final answer, not its intermediate steps. Give it a '
|
||||
+ 'complete, standalone prompt: it does not see this conversation. You may issue several subagent '
|
||||
+ 'calls in one message to run independent tasks concurrently when their work scopes do not overlap.',
|
||||
+ 'complete, standalone prompt: it does not see this conversation.',
|
||||
promptDescription:
|
||||
'The complete, self-contained task for the subagent. It does not share this '
|
||||
+ 'conversation\'s context, so include everything it needs.',
|
||||
@@ -254,9 +252,6 @@ export function apply(ctx: Context, config: Config): void {
|
||||
},
|
||||
} : {},
|
||||
},
|
||||
// A foreground call owns only its child run; background mode first
|
||||
// registers parent-owned task state and therefore remains exclusive.
|
||||
isConcurrencySafe: args => args.run_in_background !== true,
|
||||
async execute(args, exec): Promise<ContentBlock[]> {
|
||||
const parent = exec.agent
|
||||
if (!parent) {
|
||||
|
||||
@@ -96,13 +96,13 @@ describe('dsh-tool-subagent', () => {
|
||||
expect(foreground.isError).toBe(false)
|
||||
})
|
||||
|
||||
it('classifies foreground calls as parallel and background starts as exclusive', async () => {
|
||||
it('keeps foreground and background calls exclusive', async () => {
|
||||
const ctx = await setup({ provider: 'mock' })
|
||||
expect(ctx.tools.executionMode({
|
||||
callId: CallId('subagent-safe'),
|
||||
callId: CallId('subagent-foreground'),
|
||||
name: 'subagent',
|
||||
arguments: { description: 'do work', prompt: 'Reply OK' },
|
||||
})).toEqual({ kind: 'parallel' })
|
||||
})).toEqual({ kind: 'exclusive' })
|
||||
expect(ctx.tools.executionMode({
|
||||
callId: CallId('subagent-background'),
|
||||
name: 'subagent',
|
||||
|
||||
Reference in New Issue
Block a user