Repository navigation
feat(core): add subagent concurrency limit - #55983
gianniacquisto wants to merge 4 commits into
Conversation
github-actions
Bot
added
the
needs:compliance
|
The following comment was made by an LLM, it may be inaccurate: |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
The limit works for sibling subagents: the new test and a reproduction with a gated execution confirm that a second subagent waits until the first settles. Tests and type checks pass. One bug and one behaviour question need addressing first.
I checked it with the pull request's tests plus a temporary test of my own for the nested case.
Public API: adds the optional config key experimental.subagent_concurrency (a positive integer) to the config schema, the generated client ConfigEntry type and the docs. It is backwards compatible: when the key is unset, nothing changes.
Concerns:
- Nested subagents deadlock (blocking). With
subagent_depth >= 2, a running child holds its permit while its own tool call waits for one. With a limit of 1 (or N nested levels), this never resolves. I reproduced it: the nested call never gets a permit. See the inline comment. - Steering a running child takes a second permit. Continuing an existing
sessionIDthat is still running takes a new permit even though no new run starts. At limit 1, the steer waits until the child finishes instead of steering it. - Background calls block the parent's turn.
background: truenow waits for a permit before returning, so a parent that fans out several background subagents is stuck on the tool call instead of continuing. If that is intended, the docs should say so; otherwise the wait could happen inside the background path.
| yield* subagents.start(recovery) | ||
| // A child executes from prompt admission until its job settles. Settlement outlives | ||
| // background calls, so the permit releases on settlement rather than the tool call. | ||
| const semaphore = semaphoreFor(experimental?.subagent_concurrency) |
There was a problem hiding this comment.
When existing is a child that is still running, sessions.prompt just steers it and subagents.start joins the same job. This still takes a second permit and starts a second waiter on the same job. At limit 1, the steer waits until the child finishes instead of steering it, and at higher limits one child counts twice. A permit should only be taken when this call actually starts a new run.
This deadlocks nested subagents. When subagent_depth >= 2, a child that calls the subagent tool still holds its own permit, because its job is waiting on this very tool call. With subagent_concurrency: 1, the nested call waits on take(1) forever. I reproduced this with the gated execution from your test: a foreground call from a running child's session never admits its prompt. Without the limit, it does. One fix is to take a permit only for top-level subagents. Another is to let a nested call skip the wait when its parent session already holds a permit.
There was a problem hiding this comment.
Both fixed in 78a07d5, and the rule is the same for both: a permit marks a run, and neither of these calls starts one.
- The steer takes no permit. A call takes one only when it starts a run:
holding = concurrency !== undefined && (existing === undefined || (yield* jobs.get(existing.id))?.status !== "running"). Continuing a running child joins the job it already runs, so the child counts once.steers a running child without taking a second permitasserts the steer reaches the child while its permit is still held, peak execution at one. - Nesting runs under the parent's permit.
depth === 0gates, the same depth the tool already walks forsubagent_depth. Waiting is not repairable by ordering: the ancestor holds its permit until it settles, and it cannot settle while this call is outstanding.runs a nested subagent under the permit of the child it descends fromcoverssubagent_depth: 2withsubagent_concurrency: 1.
Both hang on the previous head (30s test timeout) and pass here. The limit counts top-level subagents, which the config docs now say.
github-actions
Bot
removed
the
needs:compliance
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
Adds experimental.subagent_concurrency, which caps how many subagents execute at the same time. Subagent calls take a permit before admitting the child's prompt and release it when the child's job settles, so foreground and background calls share the same accounting. Closes anomalyco#27110
5567db8 to
117eb62
Compare
balabommablock-cpu
left a comment
balabommablock-cpu
left a comment
There was a problem hiding this comment.
Tested at 9b0aa59. The 12 tests in tool-subagent.test.ts pass. I found one case that can hang, with nested subagents.
With subagent_concurrency: 1 and subagent_depth: 2, I started a background subagent (it takes the only permit). Then I made that running child call the subagent tool itself, in the foreground. The inner call waits for a permit that its own parent holds. In my test it was still waiting after 2 s. In a real run the parent can't finish until the inner call returns, so neither ever does. It's the same with any limit when every running child spawns one more level.
(Scratch test: a copy of "limits parallel subagents…" where the second call uses sessionID: child and Effect.timeoutOption("2 seconds"). It returns None. Happy to share it.)
Two possible fixes:
- only count top-level subagents (depth 1) against the limit, or
- let a child that already holds a permit run its own subagents without taking another one.
A small UX point too: while it waits, the inner call already reports status: "running". Showing "queued" until it gets a permit would make it clear the slowdown is the limit, not a stuck model.
# Conflicts: # packages/client/src/promise/generated/types.ts # packages/core/src/config/normalize.ts # packages/core/test/tool-subagent.test.ts # packages/schema/src/config/experimental.ts
|
Head is 78a07d5. The merge. The deadlock. Nested calls take no permit and run under the permit of the subagent they descend from. A second permit is a deadlock rather than a wait: the ancestor holds its permit until it settles, and it cannot settle while the tool call it waits on is this one. The double count. Continuing a running child joins its job, so it takes no permit and the child counts once. Both hang on the previous head and pass on this one. Background calls still wait before they return, which is intended. Admission is what starts the child, so the permit is held before The queued status is not here. |
balabommablock-cpu
left a comment
balabommablock-cpu
left a comment
There was a problem hiding this comment.
Re-tested at 78a07d5. Both fixes hold for me: the 14 tests in tool-subagent.test.ts pass, including the nested and steer cases.
I also tried two ways the permit could leak (scratch tests, not pushed):
- Failing child. With
subagent_concurrency: 1, three calls in a row whose child fails ("No model is available"). All three return the error; none waits. So the permit comes back on failure. - Cancel while queued. First background child holds the permit. I interrupted the second call while it waited, then started a third. When I released the first, the third started, and the peak stayed at 1. No leak.
One small thing from case 2, not blocking: the cancelled call has already created its child session, so an empty, never-prompted child is left in the parent's list. With a limit, a call can wait a long time, so "cancel while queued" becomes a normal thing to do. Creating the child after getting the permit, or cleaning it up on interrupt, would avoid the empty entry. Happy for that to be a follow-up.
|
I've actually been able to reliably solve this via prompting so not sure it warrants a PR and config change. I'm happy to close at this stage unless people think this can be genuinely useful. |
|
I've noticed that the agent becomes very obsessed with subagents where they are not needed, due to the fact that I ask not to use more than one subagent at once. So I would prefer to change how tools are handled, rather than changing the way of thinking. |
Issue for this PR
Closes #27110
Type of change
What does this PR do?
Adds
experimental.subagent_concurrency, a cap on how many subagents execute at the same time; unset keeps today's no limit.This is a port of the v1 attempt (Jieiku/opencode@101be29). v1 wrapped the task tool's inline run effect; v2 subagents do not run inline. The child executes from prompt admission to job settlement, so a call that starts a run takes the permit before admitting the child's prompt, and a forked fiber releases it at settlement. The permit therefore spans foreground and background calls alike: a background subagent holds it past the tool's return, and a foreground call that gets backgrounded keeps it until the child settles. A
catchCausegives it back when admission fails before the job exists, so a failed call cannot erode the limit.Two calls take no permit, and the reason is the same in both: a permit marks a run, and neither call starts one.
The limit therefore counts top-level subagents;
packages/web/src/content/docs/config.mdxsays so where it describes the option.How did you verify your code works?
packages/core/test/tool-subagent.test.ts, against the gated fake execution: two background calls keep peak execution at one and the second prompt out of the inbox until the first settles; a nested call from a running child gets through atsubagent_depth: 2withsubagent_concurrency: 1; a steer reaches a running child while that child still holds its permit. The nested and steer cases both hang (30s test timeout) on the previous head and pass on this one.bun run checkpasses (lint + typecheck), andtypecheck:tscpasses inpackages/schema,packages/coreandpackages/clienton the merged head.The branch is
devmerged in and the four conflicts resolved: two keys landing at the same spot inexperimental.ts,config/normalize.tsand the generated promiseConfigEntry, plus the test file's import block, which upstream moved to@opencode/ai/effect.packages/clientis regenerated, not hand-edited.Screenshots / recordings
Not a UI change.
Checklist