feat(#1366): phase 1 — sub-agents are now synchronous tool delegations#1368
Merged
feat(#1366): phase 1 — sub-agents are now synchronous tool delegations#1368
Conversation
Drops the bg-spawn dispatch path from #1163 and reverts the public sub-agent surface to synchronous delegation (codex ExecCell / gemini SubagentGroupDisplay shape). InvokeAgent and SpawnAgent now drive the sub-agent's inference loop inline on the parent's task and return its final answer as ToolCallResult.output — same shape as every other tool. No env-var / runtime config (per DESIGN.md P1: "compile decisions in, don't configure them at runtime"). The swap is unconditional. A draft that gated the change behind KODA_SUBAGENT_SYNC was rejected and abandoned (closed #1367). Code change is two lines: inline_only=false → true at the InvokeAgent and SpawnAgent callsites in tool_dispatch.rs. Everything else is the ripple effect: - InvokeAgent tool description rewritten for sync semantics (BLOCKS / final answer / no task_id-WaitForMail-auto-drain). - 4 description-pinning tests rewritten or deleted; new test_invoke_agent_description_documents_sync_dispatch_model forbids the async-era ghost terms as a regression guard. - 6 e2e cases in e2e_agent_test.rs migrated from collect_bg_events_after(..).find_map(AgentStatus::Completed) to events.iter().find_map(EngineEvent::ToolCallResult). - 2 e2e cases in e2e_sub_agent_trust_test.rs lose their #1321/ #1323/#1327 deflake waits — the race they guarded against (parent-loop termination vs bg-agent completion) does not exist on the sync path. - bg_agent_iter_counter_advances_via_status_channel test deleted (pinned the bg path's Running { iter: n } heartbeat, now unreachable from the public dispatcher). Phases 2–5 of #1366 will delete the now-dead bg infrastructure (WaitForMail, mailbox bridge, ChildAgentActivity, BgRegistry, activity-pill UI, child-activity overlay — ~5,000 LOC). Verified - 1394 koda-core lib tests pass - 656 koda-cli lib tests pass - All koda-core integration suites pass - cargo clippy --workspace --lib --tests --all-features clean - cargo fmt --check clean Net diff: +224 / −468 (−244 LOC). Refs: #1366
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 1 of #1366 — sub-agents are now synchronous tool delegations
What this PR does
Drops the bg-spawn dispatch path from #1163 and reverts the public sub-agent surface to synchronous delegation (codex
ExecCell/ geminiSubagentGroupDisplayshape).InvokeAgentandSpawnAgentnow drive the sub-agent's inference loop inline on the parent's task and return its final answer asToolCallResult.output— same shape as every other tool call.The dispatcher change is two lines:
inline_only=false→trueat the InvokeAgent and SpawnAgent callsites intool_dispatch.rs. Everything else is the ripple effect.Why no env var
Per DESIGN.md P1 ("compile decisions in, don't configure them at runtime"), the sync swap is unconditional. A previous draft (#1367) gated the change behind
KODA_SUBAGENT_SYNC=1so the bg path could be tested side-by-side; that draft was rejected as a principle violation and abandoned. Default behavior IS the new behavior, full stop.Tool-result shape change (BREAKING for the LLM contract)
ToolCallResult.outputtask_idconfirmation stringWaitForMailor wait for auto-drain on a future iterationToolCallResult.outputACP / headless wire format is unchanged.
EngineEvent::SubAgentStartstill fires at dispatch time. The tool description was rewritten to reflect the new shape.Test migrations
e2e_agent_test.rscollect_bg_events_after(...).find_map(AgentStatus::Completed)→events.iter().find_map(EngineEvent::ToolCallResult). The pre-#1163 wait-free assertion shape returns.e2e_agent_test.rsbg_agent_iter_counter_advances_via_status_channeldeleted — pinned the bg path'sRunning { iter: n }heartbeat, now unreachable from the public dispatcher.e2e_sub_agent_trust_test.rs#1321/#1323/#1327deflake waits — the race they guarded against (parent-loop termination vs bg-agent completion) does not exist on the sync path.tools/agent.rstest_invoke_agent_description_documents_spawn_only_model→ renamed/rewritten astest_invoke_agent_description_documents_sync_dispatch_model. Asserts onsynchronously/BLOCKS/final answerand forbidstask_id/WaitForMail/auto-drain/in the backgroundas a regression guard.tools/agent.rstest_invoke_agent_top_level_documents_drain_semanticsandtest_invoke_agent_description_discourages_immediate_waitdeleted — drain semantics and the WaitForMail anti-pattern only existed because of bg-spawn.tools/agent.rstest_invoke_agent_description_explains_error_marker_conventionextended to also recognize the dispatcher'sErrarm format ("Error invoking sub-agent: {chain}") as the second structural-failure shape.tools/agent.rstest_invoke_agent_schema_does_not_carry_background_paramkept — schema is unchanged.Verification
cargo test -p koda-core --libcargo test -p koda-core --tests(all integration)cargo test -p koda-cli --libcargo clippy --workspace --lib --tests --all-featurescargo fmt --checkDiff stats
Net −244 LOC even before the deletion phases.
Phase roadmap (from #1366)
WaitForMailtool + mailbox bridgeChildAgentActivityevent family +BgRegistry+run_bg_agentSubAgentBlock(codex ExecCell-style) +SubAgentGroup(gemini parallel-N) widgetsRisk
The behavior change is intentional and load-bearing for the epic. The migrated tests cover every previously-asserted concern (sub-agent output reaches the parent, grace turn fires, preflight bails, max_turns marker surfaces, file writes execute, destructive bash blocks). Wire format is unchanged.
The deleted
bg_agent_iter_counter_advances_via_status_channeltest pinned a code path that's now dead from the public surface; phase 2/3 deletes the underlying code.Refs: #1366
Closes: (none — phase 1 is foundational; the epic stays open through phase 5)