feat(#1366): phase 1 — sub-agent sync-dispatch scaffold + KODA_SUBAGENT_SYNC kill switch#1367
Closed
lijunzh wants to merge 1 commit into
Closed
feat(#1366): phase 1 — sub-agent sync-dispatch scaffold + KODA_SUBAGENT_SYNC kill switch#1367lijunzh wants to merge 1 commit into
lijunzh wants to merge 1 commit into
Conversation
…NT_SYNC kill switch Adds the public sync-dispatch entry point (`execute_sub_agent_sync`) and a runtime kill switch (`KODA_SUBAGENT_SYNC=1`) so the rest of the #1366 epic can land incrementally without disturbing the legacy bg-spawn path from #1163. The wrapper is intentionally a thin call to `execute_sub_agent(..., inline_only=true)`, promoting the existing recursion-guard branch to a public symbol. This keeps phase 1 purely additive — phase 2 will be a one-line callsite swap in tool_dispatch to flip the default. Default behavior unchanged (env var unset → legacy bg-spawn path). With KODA_SUBAGENT_SYNC=1 the FG inference path runs synchronously and returns the sub-agent's final output as the tool result, no task_id round-trip via WaitForMail. Tests - New `sync_dispatch_tests::subagent_sync_env_var_parsing` pins the truthy/falsy/unset matrix (single #[test] because cargo parallelizes and the env slot is process-global). - 1397 koda-core lib tests pass with flag unset (default behavior). - 12 e2e_agent_test cases pass with flag unset. - With flag SET, 6/12 e2e cases fail because they assert on bg-spawn registry terminal-status events the FG path deliberately doesn't emit (sub_agent_dispatch.rs:1075 documents this). Migrating those assertions is phase 2 work, not scope for this PR. Wire format unchanged — ACP / headless clients unaffected. Refs: #1366
Owner
Author
|
Closing — env-var kill switch violates DESIGN.md P1 ("compile decisions in, don't configure them at runtime"). Redoing as a single commit that swaps the callsite directly + migrates the 6 e2e tests in the same PR. New PR incoming. |
lijunzh
added a commit
that referenced
this pull request
May 10, 2026
#1368) 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
1 participant
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-agent sync-dispatch scaffold
What this PR does
Adds the public sync-dispatch entry point and a runtime kill switch so the rest of the #1366 epic can land incrementally without disturbing the legacy bg-spawn path from #1163.
execute_sub_agent_sync()— new public symbol insub_agent_dispatch.rs. Thin wrapper overexecute_sub_agent(..., inline_only=true), promoting the existing recursion-guard branch to a first-class API.subagent_sync_enabled()+KODA_SUBAGENT_SYNCenv var — runtime kill switch. Default unset → legacy bg-spawn (no behavior change). Set to1/true→ new sync streaming dispatch.tool_dispatch.rscallsite gating — bothInvokeAgentandSpawnAgentbranch on the env var. Both paths coexist for the migration window.Why this shape
SubAgentStart,ChildTaskUpdate { is_background: bool }, andChildAgentActivity(Bug review: 9 issues found in debug-bundle session 5c4087a8 (sub-agent visibility, context budgeting, trust UX) #1232 PR-A0). The FG path already wires through the parent'sEngineSink. The work for Epic: lean sub-agent rewrite — fg-sync delegation, kill mailbox/bg/pill #1366 is mostly deletion + UI redesign, not new engine plumbing.docs/. That directory is reserved for user-facing docs. The dev design-of-record lives in Epic: lean sub-agent rewrite — fg-sync delegation, kill mailbox/bg/pill #1366 itself.Behavior
KODA_SUBAGENT_SYNC0/false(default)Background agent 'X' started (agent:N). Results will be injected when complete.1/true/TRUE/TrueTest status
KODA_SUBAGENT_SYNC=1)koda-corelib (1397 tests)e2e_agent_test(12 tests)agent_mailbox_e2e_testtool_wiring_testnew_tools_testcargo fmt --checkcargo clippy -p koda-coreThe 6 e2e failures with the flag SET are expected and out of scope for phase 1: they assert on the bg-spawn registry's terminal-status
ChildTaskUpdateevents that the FG path deliberately does not emit (documented atsub_agent_dispatch.rs:1075). Migrating those assertions to the sync-path event shape is phase 2 work.Phase roadmap (from #1366)
WaitForMail+ bg-dispatch pathSubAgentBlockwidget (codex ExecCell-inspired)SubAgentGroupcontainer (gemini-style parallel-N)Manual smoke test
Risk
Very low. Default code path is byte-identical to pre-PR behavior — same callsite, same
inline_only=false, same legacy bg-spawn branch. The new symbol is unreachable without explicit opt-in via env var.Refs: #1366