From e3570b444fce736c240a39bdbf36fe58d6b7db13 Mon Sep 17 00:00:00 2001 From: Renn F Date: Fri, 15 May 2026 05:01:18 +0200 Subject: [PATCH] fix(orchestrator): briefing renders ToolSearch directive + current verb names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Smoke-8 follow-up. Two issues in _write_agent_briefing: 1. _build_tool_load_block was scraping role prompts for a "## Load on spawn" section that doesn't exist in any role file. Returned "" for every role → no ToolSearch directive in the briefing. Combined with weak models skipping the system-prompt-layer directive (#144), the agent's first action was Edit → "not enabled in this context." Fix: per-role tool list lives in the orchestrator (mirrors factories._base.py). Pre-renders the directive directly. developer and documenter get Edit + Write; QA/PMs/board get the common read-only set. 7 tests pin the contract. 2. The briefing's "Terminal tools (how to exit cleanly)" section still listed pre-gateway verb names: roboco_agent_idle, roboco_task_substitute, roboco_task_escalate, roboco_task_submit_qa, _qa_pass/fail, _docs_complete, _complete. Same rename pattern as #145's _TERMINAL_TOOLS set. Updated to: i_am_idle, i_am_blocked, unclaim, i_am_done, pass, fail, i_documented, complete, submit_up, escalate_up, escalate_to_ceo. The agent now reads the same directive in two places (system prompt + session briefing) — the second touch point catches weak models that skip the first. --- roboco/runtime/orchestrator.py | 78 +++++++++++++---- .../runtime/test_briefing_tool_load_block.py | 83 +++++++++++++++++++ 2 files changed, 143 insertions(+), 18 deletions(-) create mode 100644 tests/unit/runtime/test_briefing_tool_load_block.py diff --git a/roboco/runtime/orchestrator.py b/roboco/runtime/orchestrator.py index d70f45cd..b013238d 100644 --- a/roboco/runtime/orchestrator.py +++ b/roboco/runtime/orchestrator.py @@ -2175,23 +2175,61 @@ class AgentOrchestrator: _TOOL_LOAD_CACHE: ClassVar[dict[str, str]] = {} + # Per-role built-in tools that must be activated via ToolSearch before + # use. Mirrors the system-prompt layer's _ROLE_BUILTIN_TOOLS in + # roboco/agents/factories/_base.py — kept in sync because the briefing + # and the system prompt are independent code paths. Smoke-8 evidence: + # weak models skip the system-prompt directive; the briefing block is + # the second touch point that gets them to actually run ToolSearch. + _COMMON_BUILTIN_TOOLS: ClassVar[tuple[str, ...]] = ( + "Read", + "Bash", + "Grep", + "Glob", + "Task", + "TodoWrite", + ) + _ROLE_BUILTIN_TOOLS: ClassVar[dict[str, tuple[str, ...]]] = { + "developer": (*_COMMON_BUILTIN_TOOLS, "Edit", "Write"), + "documenter": (*_COMMON_BUILTIN_TOOLS, "Edit", "Write"), + "qa": _COMMON_BUILTIN_TOOLS, + "main_pm": _COMMON_BUILTIN_TOOLS, + "cell_pm": _COMMON_BUILTIN_TOOLS, + "product_owner": _COMMON_BUILTIN_TOOLS, + "head_marketing": _COMMON_BUILTIN_TOOLS, + "auditor": _COMMON_BUILTIN_TOOLS, + } + def _build_tool_load_block(self, role: str) -> str: """Build the mandatory first-action ToolSearch directive. - Weak models consistently skip the role prompt's `Load on spawn - (one ToolSearch select: call)` line — then trip on "Edit exists - but is not enabled in this context" when they try to edit a - file, or "No such tool available: mcp__roboco-flow__…" when - they try to act. Hoisting the directive into the briefing's - first block (with the exact query string inline) pulls the - bootstrap into the prompt-most-salient position. - - Returns empty string if we can't locate the role file — the - briefing still works without it. + Weak models consistently skip the system-prompt's directive (added + in #144) and call Edit/Write directly, hitting "Edit exists but is + not enabled in this context." Hoisting the same directive into the + briefing (which is the first user-visible message after spawn) is + the second touch point. Pre-rendered from the per-role tool list + — no role-file scrape required. """ if role in self._TOOL_LOAD_CACHE: return self._TOOL_LOAD_CACHE[role] - block = self._read_tool_load_from_role_prompt(role) + tools = self._ROLE_BUILTIN_TOOLS.get(role) + if not tools: + block = "" + else: + tool_list = ",".join(tools) + block = ( + "## First action required\n" + "\n" + "Before any other tool call you MUST run this ToolSearch.\n" + "Built-in tools (Edit, Write, Read, etc.) are deferred until\n" + "you activate them. Skipping this step makes Edit calls fail\n" + 'with "Edit exists but is not enabled in this context" — ' + "wasting your budget and stalling the task.\n" + "\n" + "Copy this verbatim as your first action:\n" + "\n" + f'```\nToolSearch(query="select:{tool_list}")\n```\n"\n"\n' + ) self._TOOL_LOAD_CACHE[role] = block return block @@ -2311,15 +2349,19 @@ class AgentOrchestrator: f"- **Workspace:** `{workspace_path}`\n" f"{task_block}" "\n## Terminal tools (how to exit cleanly)\n" - "- `roboco_agent_idle()` — no work remaining\n" - "- `roboco_task_substitute(reason=...)` — release the task\n" - "- `roboco_task_escalate(reason=...)` — escalate up the chain\n" - "- `roboco_task_pause(checkpoint=...)` — save progress, resume later\n" - "- Role handoffs: `roboco_task_submit_qa()`, `_qa_pass/fail()`, " - "`_docs_complete()`, `_complete()`\n" + "- `i_am_idle()` — no work remaining (every role)\n" + "- `i_am_blocked(task_id, reason, ...)` — stuck (developer)\n" + "- `unclaim(task_id)` — release a claim back to the pool\n" + "- Role handoffs:\n" + " - developer → `i_am_done(task_id, notes)` (submit for QA)\n" + " - qa → `pass(task_id, notes)` / `fail(task_id, issues)`\n" + " - documenter → `i_documented(task_id, notes, files)`\n" + " - cell_pm → `complete(task_id, notes)` / `submit_up(...)`" + " / `escalate_up(...)`\n" + " - main_pm → `complete(...)` / `escalate_to_ceo(...)`\n" "\n" "A Stop without a terminal tool will be rejected; a second Stop\n" - "auto-substitutes with `reason='stopped_without_transition'`.\n" + "auto-substitutes the task so it can be picked up elsewhere.\n" "\n" "## Budget\n" f"Soft-warn at {settings.agent_tool_call_warn} tool calls, " diff --git a/tests/unit/runtime/test_briefing_tool_load_block.py b/tests/unit/runtime/test_briefing_tool_load_block.py new file mode 100644 index 00000000..ec9e40ed --- /dev/null +++ b/tests/unit/runtime/test_briefing_tool_load_block.py @@ -0,0 +1,83 @@ +"""Smoke-8: briefing's _build_tool_load_block emits the ToolSearch directive +without depending on a "## Load on spawn" section in the role file. + +The previous implementation scraped role prompts for that marker; since +no role file had it, the function returned "" and the briefing showed +agents no tool-load instruction. Combined with weak models skipping the +system-prompt directive, the agent went straight to Edit and hit "not +enabled in this context." + +Fix: per-role tool list lives in the orchestrator (mirrors the +factories layer). Pre-renders the directive directly — no file scrape. +""" + +from __future__ import annotations + +import re +from unittest.mock import patch + +from roboco.runtime.orchestrator import AgentOrchestrator + + +def _orch() -> AgentOrchestrator: + with patch.object(AgentOrchestrator, "__init__", return_value=None): + orch = AgentOrchestrator.__new__(AgentOrchestrator) + orch._TOOL_LOAD_CACHE = {} + return orch + + +def test_developer_directive_includes_edit_and_write() -> None: + block = _orch()._build_tool_load_block("developer") + assert "First action required" in block + assert "ToolSearch" in block + assert "Edit" in block + assert "Write" in block + + +def test_documenter_directive_includes_edit_and_write() -> None: + block = _orch()._build_tool_load_block("documenter") + assert "Edit" in block + assert "Write" in block + + +def test_qa_directive_excludes_edit_and_write() -> None: + block = _orch()._build_tool_load_block("qa") + assert "First action required" in block + # The ToolSearch line itself: pull the comma list to be precise about + # which tools are listed (substring match would catch "TodoWrite"). + match = re.search(r'select:([^"]+)"', block) + assert match is not None + tools = match.group(1).split(",") + assert "Edit" not in tools + assert "Write" not in tools + assert "Read" in tools + assert "Bash" in tools + + +def test_pm_directives_exclude_edit_and_write() -> None: + for role in ("main_pm", "cell_pm", "product_owner", "head_marketing", "auditor"): + block = _orch()._build_tool_load_block(role) + match = re.search(r'select:([^"]+)"', block) + assert match is not None + tools = match.group(1).split(",") + assert "Edit" not in tools, f"{role} must not list Edit" + assert "Write" not in tools, f"{role} must not list Write" + + +def test_unknown_role_returns_empty() -> None: + """No directive for unknown roles (defensive).""" + block = _orch()._build_tool_load_block("nonexistent") + assert block == "" + + +def test_directive_warns_about_failure_mode() -> None: + block = _orch()._build_tool_load_block("developer") + assert "Edit exists but is not enabled" in block + + +def test_role_cache_works() -> None: + orch = _orch() + first = orch._build_tool_load_block("developer") + # Second call hits the cache (same return). + second = orch._build_tool_load_block("developer") + assert first is second # same string object