fix(orchestrator): route review-before-finalize tasks to synchronous delegation

[M3gA-Mind]

For "draft a bio, then have a second agent critique it before you finalize", the orchestrator dispatched the critique via fire-and-forget spawn_async_subagent. The turn finalized immediately and the critique ran as a separate autonomous_task ~10 min later — so the parent never honored "before you finalize" and burned a wasted detached run.

Why it mis-routed: the orchestrator has no synchronous spawn_subagent; its awaited primitives are spawn_parallel_agents (collects results before returning), blocking delegate_* specialists, and spawn_async_subagent + wait_subagent. Neither the async tool description nor the prompt named the result-gating ("review/critique/verify before finalize") pattern, so a gating step slipped through as "background work".

Fix (guidance-only, no routing refactor):

• orchestrator/prompt.md: new "Result-gating tasks run synchronously (hard rule)" — such work must run in the same turn via a blocking delegate_*, spawn_parallel_agents, or wait_subagent; never fire-and-forget.
• spawn_async_subagent tool description(): explicitly excludes result-gating tasks and points to the awaited alternatives.
• Added a prompt regression test.

Closes #4681

Summary by CodeRabbit

• Bug Fixes

  • Improved guidance for task delegation so important verification or approval steps complete before the assistant finalizes a response.
  • Prevents premature answers when a follow-up result is required to confirm correctness.

• Tests

  • Added coverage to ensure the assistant’s routing guidance stays consistent for these gated tasks.

[coderabbitai]

📝 Walkthrough

Walkthrough

Updates the spawn_async_subagent tool description and orchestrator prompt to explicitly disallow using fire-and-forget async spawning for tasks whose results must gate the final answer, directing use toward synchronous/awaited alternatives, and adds a regression test verifying the prompt rule text.

Changes

Result-gating delegation rule

Layer / File(s)	Summary
Tool description and prompt rule src/openhuman/agent_orchestration/tools/spawn_async_subagent.rs, src/openhuman/agent_registry/agents/orchestrator/prompt.md	Tool description and orchestrator prompt now state that spawn_async_subagent must not be used when a sub-agent's result must gate the final answer, pointing to synchronous/awaited alternatives (spawn_parallel_agents, wait_subagent, blocking delegation).
Regression test src/openhuman/agent_registry/agents/orchestrator/prompt.rs	New test asserts the embedded prompt text contains the result-gating rule and names the synchronous primitives.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

• tinyhumansai/openhuman#4230: Modifies spawn_async_subagent's outcome handling, directly tied to the same tool's result behavior being clarified here.

Suggested labels: bug, agent

Poem

A rabbit hops, then pauses to see,
"Wait for the critique before you flee!"
No more fire-and-forget for gating fate,
Now we wait, synchronously, in state.
🐇✅ Finalize only when it's checked and true!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: routing review-before-finalize work to synchronous delegation.
Linked Issues check	✅ Passed	The changes implement the requested routing guidance so result-gating tasks use awaited delegation instead of fire-and-forget async spawn.
Out of Scope Changes check	✅ Passed	The PR stays focused on orchestrator guidance and regression coverage for result-gating delegation, with no obvious unrelated changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

src/openhuman/agent_registry/agents/orchestrator/prompt.rs (1)

381-397: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Second assertion is weakly coupled to the actual rule change.

The first assert! (Line 388) meaningfully pins the new rule text. But spawn_parallel_agents and wait_subagent already appear elsewhere in prompt.md prior to this change (e.g. the pre-existing "Async background sub-agents" section), so the second assertion (Lines 393-396) would still pass even if the new rule text omitted these primitives entirely or named different tools. It doesn't actually verify these terms occur within/near the new rule.

Consider tightening this to check the terms appear within the rule's paragraph specifically (e.g. by locating the substring starting at "Result-gating tasks run synchronously" and asserting the alternatives are within that slice).

♻️ Proposed tightening

-        assert!(
-            ARCHETYPE.contains("Result-gating tasks run synchronously"),
-            "orchestrator prompt must carry the result-gating delegation rule"
-        );
-        // It must steer such tasks to a synchronous/awaited primitive rather
-        // than fire-and-forget `spawn_async_subagent`.
-        assert!(
-            ARCHETYPE.contains("spawn_parallel_agents") && ARCHETYPE.contains("wait_subagent"),
-            "the rule must name the synchronous/awaited alternatives"
-        );
+        let rule_start = ARCHETYPE
+            .find("Result-gating tasks run synchronously")
+            .expect("orchestrator prompt must carry the result-gating delegation rule");
+        let rule_slice = &ARCHETYPE[rule_start..];
+        // It must steer such tasks to a synchronous/awaited primitive rather
+        // than fire-and-forget `spawn_async_subagent`, within the rule itself.
+        assert!(
+            rule_slice.contains("spawn_parallel_agents") && rule_slice.contains("wait_subagent"),
+            "the rule must name the synchronous/awaited alternatives"
+        );

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/openhuman/agent_registry/agents/orchestrator/prompt.rs` around lines 381
- 397, The test in prompt_routes_result_gating_tasks_to_synchronous_delegation
is too loosely coupled because it checks ARCHETYPE globally for
spawn_parallel_agents and wait_subagent, which may pass due to unrelated
existing prompt text. Tighten the assertion by isolating the paragraph/substring
that begins with the new "Result-gating tasks run synchronously" rule in
ARCHETYPE, then verify the alternative primitives appear within that same slice;
keep the existing function name and regression intent while making the check
specific to the updated rule text.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/openhuman/agent_registry/agents/orchestrator/prompt.rs`:
- Around line 381-397: The test in
prompt_routes_result_gating_tasks_to_synchronous_delegation is too loosely
coupled because it checks ARCHETYPE globally for spawn_parallel_agents and
wait_subagent, which may pass due to unrelated existing prompt text. Tighten the
assertion by isolating the paragraph/substring that begins with the new
"Result-gating tasks run synchronously" rule in ARCHETYPE, then verify the
alternative primitives appear within that same slice; keep the existing function
name and regression intent while making the check specific to the updated rule
text.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b410925e-1069-4679-82cd-9214576d8788

📥 Commits

Reviewing files that changed from the base of the PR and between c38d4c9 and 5f2e3eb.

📒 Files selected for processing (3)

• src/openhuman/agent_orchestration/tools/spawn_async_subagent.rs
• src/openhuman/agent_registry/agents/orchestrator/prompt.md
• src/openhuman/agent_registry/agents/orchestrator/prompt.rs

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5f2e3eb8cd

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Allow single-agent waits for gated reviews

For single result-gating requests like the #4681 repro (“draft a bio, then have a second agent critique it before finalizing”), this guidance leaves no valid awaited path: spawn_parallel_agents only accepts two or more independent tasks (its schema/validator rejects fewer than 2), and arbitrary prose critiques often have no matching delegate_* specialist. Since wait_subagent can collect a just-spawned async child in the same turn, making it usable only after the model has already used async can still push the orchestrator into a tool error or into skipping the requested second-agent review.

Useful? React with 👍 / 👎.

[M3gA-Mind]

CI + review pass — LGTM.

Reviewed origin/main...HEAD. The fix correctly routes result-gating work (review/critique/verify/approve before finalize) away from fire-and-forget spawn_async_subagent and toward a synchronous/awaited path — via a 'Result-gating tasks run synchronously (hard rule)' section in the orchestrator prompt.md plus a matching caveat in the spawn_async_subagent tool description. The reasoning is accurate (there is no synchronous spawn_subagent tool) and every named awaited alternative is real: blocking delegate_* specialists, spawn_parallel_agents (collects results before returning), and spawn_async_subagent+wait_subagent (confirmed registered). The Rust change is behavior-neutral (a description string + a new test — no logic paths touched), and the added regression test is valid (ARCHETYPE = include_str!("prompt.md"); the asserted substrings are present) and passes. No regressions, sensible, follows conventions; no fix needed.

Checks: cargo check --lib ✓ (clean compile), cargo fmt --check ✓; GitHub CI green — Rust Quality (fmt, clippy), Rust Core Coverage (runs the new test), Markdown Link Check, PR Submission Checklist, CodeRabbit all pass (10/10, 5 skipped as not-applicable).