feat: allow custom subagents to delegate tasks via task()#1908
feat: allow custom subagents to delegate tasks via task()#1908Stranmor wants to merge 3 commits intocode-yeongyu:devfrom
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 11 files
Confidence score: 2/5
- Potential deadlock in
src/features/background-agent/spawner.tswhen nested agents share a low-limit concurrency key and both try to acquire it, which can block parent/child execution - Severity is high (8/10) with plausible user-facing hangs, so merge risk is elevated despite being a single issue
- Pay close attention to
src/features/background-agent/spawner.ts- nested concurrency acquisition can stall agents
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/features/background-agent/spawner.ts">
<violation number="1" location="src/features/background-agent/spawner.ts:143">
P1: Potential deadlock when nested agents share concurrency key with low limits. Parent acquires slot and invokes task tool (now enabled), child attempts to acquire same key but blocks because parent holds it while waiting for child completion.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| call_omo_agent: true, | ||
| question: false, | ||
| }, | ||
| tools: buildSubagentTools(input.agent), |
There was a problem hiding this comment.
P1: Potential deadlock when nested agents share concurrency key with low limits. Parent acquires slot and invokes task tool (now enabled), child attempts to acquire same key but blocks because parent holds it while waiting for child completion.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/background-agent/spawner.ts, line 143:
<comment>Potential deadlock when nested agents share concurrency key with low limits. Parent acquires slot and invokes task tool (now enabled), child attempts to acquire same key but blocks because parent holds it while waiting for child completion.</comment>
<file context>
@@ -140,12 +140,7 @@ export async function startTask(
- call_omo_agent: true,
- question: false,
- },
+ tools: buildSubagentTools(input.agent),
parts: [{ type: "text", text: input.prompt }],
},
</file context>
|
@Stranmor Thanks for the PR! There are a couple of items to address before we can merge:
Could you take a look at these issues? Once the conflicts are resolved and the deadlock concern is addressed (or explained if it is not a real issue), we can proceed with the review. Thanks! |
|
[sisyphus-bot] Triage Assessment for PR #1908SummaryThis PR enables custom subagents defined in
Current Status:
|
| Criteria | Status |
|---|---|
| CI Checks | ✅ All passing (3/3) |
| Review Decision | ❌ Not approved |
| Merge Conflicts | ❌ Conflicts exist (needs resolution) |
| AI Review Issues | |
| Draft Status | ✅ Not a draft |
Critical Issues Requiring Attention
1. Merge Conflicts
The branch has conflicts that must be resolved before merge. Please rebase on the latest dev branch.
2. Deadlock Risk (High Severity)
As identified by @cubic-dev-ai, there is a potential deadlock in src/features/background-agent/spawner.ts (around line 143):
- Scenario: Nested agents sharing a concurrency key with low limits
- Risk: Parent acquires slot → invokes task tool → child attempts same key → blocks because parent holds it while waiting for child
- Impact: User-facing hangs (severity 8/10)
Recommendation: Review the concurrency acquisition logic in spawner.ts. Consider:
- Separate concurrency keys for parent/child relationships
- Timeout mechanisms for slot acquisition
- Documentation warning users about low concurrency limits with nested agents
Assessment
This is a feature PR (not a bugfix), so automatic merge is not applicable. The changes look architecturally sound for enabling custom subagent delegation, but the deadlock concern should be addressed or documented before merge.
Next Steps:
- Resolve merge conflicts
- Address or respond to the deadlock concern from cubic review
- Request re-review from maintainers
This assessment was generated by sisyphus-bot. Please reach out if you have questions!
Replace hardcoded `task: false` in all subagent spawning paths with `buildSubagentTools()` — a centralized function that respects AGENT_RESTRICTIONS. Built-in agents (explore, librarian, oracle, metis, momus, sisyphus-junior) keep `task: false` via their AGENT_RESTRICTIONS entries. Custom/user-defined agents (not in AGENT_RESTRICTIONS) now get `task: true`, enabling multi-level agent delegation (e.g. a critic agent spawning a breaker agent). This unblocks workflows where specialized review agents need to orchestrate sub-pipelines without routing through the top-level orchestrator.
Custom agents defined in oh-my-opencode.json (e.g. breaker, type-sentinel) were silently dropped because they were passed as overrides to createBuiltinAgents() which only processes known builtin names. Additionally, the global config.permission.task = 'deny' blocked custom agents from calling task() since they were never whitelisted. Changes: - applyAgentConfig: add custom agents from pluginConfig.agents that aren't already registered (both sisyphus-enabled and disabled paths) - applyToolConfig: grant task='allow' to all agents not explicitly blocked by AGENT_RESTRICTIONS
AgentOverridesSchema used strict z.object() with only hardcoded builtin agent names. Zod strips unknown keys during safeParse, silently dropping any custom agents (breaker, critic, type-sentinel, etc.) from the parsed config. Adding .catchall(AgentOverrideConfigSchema) preserves unknown agent keys while still validating them against the same schema.
Stranmor
force-pushed
the
feat/configurable-subagent-task-access
branch
from
c78aab8 to
fa40c25
Compare
|
Thanks for the review and for highlighting the deadlock concern! I've looked into how the custom agent permissions interact with the concurrency model. While the theoretical deadlock exists, here are a few findings:
Additionally, I've resolved the recent merge conflicts and rebased the branch on the latest |
|
@Stranmor @code-yeongyu I have this MR, which is heavily related. I believe we should try to merge these under the same release asap if possible |
|
@brandonwebb-vista Thanks for flagging this — I've reviewed #2299 and they're definitely complementary (this PR handles task() permissions for custom agents, yours handles dynamic agent discovery/resolution). Coordinating for the same release makes sense to avoid a window where one half works without the other. Happy to sync on merge order — I'd suggest yours (#2299) lands first since it wires up agent loading, then this one layers on the task delegation permissions. Let me know if that works or if you see a different dependency order. |
code-yeongyu
added
triage:feature
4 participants
Summary
Custom subagents defined in
oh-my-opencode.jsoncannot usetask()to delegate work to other agents. This PR fixes three independent issues that collectively prevent the pipeline from working.Problem
When a user defines custom agents (e.g.,
critic,breaker,type-sentinel) inoh-my-opencode.json, those agents:task()—buildSubagentTools()hardcodestask: falsefor background tasks and only allowstaskfor plan-family agents in sync flowsapplyAgentConfig()passes custom agents as overrides tocreateBuiltinAgents(), which only processes known builtin namesapplyToolConfig()setsconfig.permission.task = "deny"globally, then only whitelists hardcoded agentsChanges
Commit 1 (
agent-tool-restrictions.ts): CheckAGENT_RESTRICTIONSinstead of hardcodingtask: false. 4 code paths fixed.Commit 2 (
agent-config-handler.ts,tool-config-handler.ts): Register custom agents from config. Granttask: "allow"to non-restricted agents.