fix(runtime): make subagent spawn selection explicit and recoverable - #5534
Conversation
Expose callable catalog arguments, ignore inactive selector fields only in explicit modes, and provide actionable errors without changing legacy routing or parallel scheduling. Generated-by: Codex
Report the selected identity field when an explicit spawn mode lacks its selector, retaining legacy guidance for callers without a mode. Cover recovery with and without an inactive selector present. Generated-by: Codex
Astro-Han
left a comment
There was a problem hiding this comment.
Reviewed cb589961. The fix shape is right and matches an existing in-repo precedent: stream-graph-supervisor-tools.ts already solves this exact ambiguity with a target_kind discriminator + preprocess field discard, so agent_spawn converging on the same idiom is the correct move rather than a new mechanism. I walked the full preprocess matrix (every target_kind × selector and executor_mode × executor_id combination): legacy precedence is preserved (subagent_id wins when target_kind is absent), executor_id survives when executor_mode is absent (existing plugin callers unaffected), and the impl still resolves on surviving-field presence rather than re-reading the discriminator — so a preset spawn without target_kind can't fall through to the profile path. spawn_args templates omit task, so a verbatim copy fails required-field validation rather than spawning a garbage-task child — the safer failure direction. The unknown-preset recovery path correctly distinguishes "built-in profile id passed as preset" and hands back a usable corrected call. Tests pin the reported incident shape.
Remaining findings are all minor:
P3 — spawn_args is emitted on view=catalog entries regardless of availability. In projectAgentList the catalog view keeps unavailable presets/profiles (by design, for diagnosis) but now also attaches a copyable spawn_args template to them. A model operating from the catalog view can copy a template that is guaranteed to fail downstream. It fails closed with an actionable unavailable message, so this is cosmetic; omitting spawn_args on non-available entries would make the contract airtight.
P3 — target_kind vocabularies are disjoint across sibling tools. agent_spawn accepts profile|preset; update_agent_graph uses new_agent|new_preset|existing_operator, and the graph/swarm prompts explicitly teach target_kind=new_preset. A model carrying the graph vocabulary into agent_spawn gets a plain enum error — self-correcting, but the shared field name invites the mistake. Worth a one-line note in the description ("graph target_kind values are not valid here") or accepting as-is.
P3 (pre-existing, more traveled now) — second-resolution TOCTOU produces the least actionable error. The tool resolves the preset against a fresh listChildAgents snapshot, then resolveChildSessionSelector → subagentCatalog.resolve re-fetches presets independently. A concurrent settings edit between the two yields Subagent preset "x" profile changed during spawn (session-manager.ts) — the only message on this path that names neither the field nor a remedy; the model can only retry identically. Not introduced by this PR, but spawn_args steering makes the copy-then-spawn path more common, so the window gets more traffic. A retry the same spawn_args hint in that message would close it cheaply.
Approved — the reported failure mode is addressed at the schema boundary where it originates, and the discarded-field semantics are explicit rather than implicit.
|
@Astro-Han Thanks for the detailed review. I addressed the two concrete recovery/contract follow-ups in d79140d:
I kept the sibling tools' Validation:
|
Summary
agent_listcan return no presets while the built-inimplementationprofile is available. A model nevertheless usedsubagent_id="implementation"andexecutor_id="default", causing three parallel spawn calls to fail before any child started. Improving descriptions alone was insufficient: in a live follow-up, the model kept filling inactive fields with placeholder values and exhausted 11 unsuccessful calls.spawn_argsfromagent_list, and give actionable selector-error feedback. Missing-selector errors identify the required field for the selected mode, so following the feedback does not add a field that will be discarded.target_kind=profile|presetandexecutor_mode=inherit|plugin. Discard inactive fields before validation, while strictly validating the selected branch. Explicit plugin mode requires an executor ID.No scheduler or concurrency-limit changes. Retries remain new calls chosen by the model after tool feedback; this does not add automatic runtime retries.
Verification
Real-model startup, recovery, and parallelism
Manual validation on 2026-09-20 used the actual Maka runtime-host/CLI path and
gpt-5.6-solthroughcodex-subscriptionfor both parent and children (parent thinking: high). Each run used an isolated state root and a temporary Git repository. Two Implementation children each read one fixture file in a separate worktree; no mock model or stub child session was used.For the recovery case, the first batch deliberately selected
target_kind=preset,subagent_id=implementation,executor_mode=plugin, andexecutor_id=default. Both calls failed before creating a child. After receiving the error, the model issued both corrected calls together in its next batch, 9.667 s later:{ "target_kind": "profile", "profile": "implementation", "executor_mode": "inherit", "isolation": "worktree", "write_back": "patch" }taskis omitted above for brevity. Both successful calls shared one model step in each run. All four successful children made two real model requests each, used onlyRead, and returned the expected fixture values (ALPHA=17,BETA=29). The parent did not read the files itself. The recovery run created exactly two children, with no duplicate launches.These samples demonstrate successful startup and parallel correction when parallel work is requested; they do not establish a general success rate or guarantee that the model will always choose parallel calls. Same-batch tools still settle before the next model request, so a slow sibling can delay model-driven correction. These live runs used the local development build; the isolated PR worktree checks below validate the two-file patch separately.
Follow-up: Local Read and Web Research
Additional real-model runs used
gpt-5.6-soland an isolated runtime-host state root:ALPHA=17using onlyRead.unavailable / missing_tools, and no child was created. Search is disabled in the local configuration, no Tavily credential is configured, and the current Maka native-search capability resolver returnsnullforopenai-codex. This is an availability/configuration limitation; successful live startup or search is not claimed.Automated checks
node --test packages/runtime/dist/__tests__/subagent-tools.test.js: 27 passed, 0 failed in the isolated PR worktree. The two mode-specific missing-selector recovery tests both failed before the review follow-up fix and passed afterward, covering absent selectors and populated inactive fields. The earlier 25-test suite produced 20 passed, 5 failed against the original spawn implementation and 25 passed with the initial fix.npm run lint,npm run format:check,npx knip --workspace apps/desktop,npx knip --workspace packages/ui, andgit diff --check: passed.npm run buildandnpm run typecheck: attempted but not passing in the isolated worktree. Runtime compilation reports existing TS7006 errors inmodel-factory-thinking.test.tsat lines 1037, 1081, and 1129, andopenai-responses-plaintext-reasoning.test.tsat line 549. Restoring the original spawn implementation reproduced the same four errors. Root typecheck additionally reports missing downstream build outputs after the build stops. TypeScript emitted the runtime output used by the focused tests despite its nonzero exit.AI use
Select exactly one:
Tool(s) and scope: OpenAI Codex implemented the runtime fix and regression tests, ran the automated and real-model checks, and prepared this PR description. The commit includes a
Generated-by: Codextrailer.Checklist
Does this PR entail a change in behavior?