diff --git a/docs/llm-workflow-runbooks.md b/docs/llm-workflow-runbooks.md index 5c427f1..a5a51b2 100644 --- a/docs/llm-workflow-runbooks.md +++ b/docs/llm-workflow-runbooks.md @@ -312,6 +312,39 @@ Required behavior (fail closed, issue #258): Doc-contract tests: `tests/test_shell_spawn_hard_stop_docs.py`. +## Subagent Tool-Budget Guardrails + +General-purpose subagents tasked with **deterministic MCP work** (for example a +single `gitea_commit_files` call) have expanded to 100–122 tool calls, +WebFetch/Playwright fallbacks, and throwaway helper-script generation instead of +calling the native MCP tool once (observed during #152 closure, issue #259). + +**Default budgets** (fail closed when exceeded): + +| Task class | Max tool calls | Max wall time | +|------------|----------------|---------------| +| Single-step MCP mutation (`commit_files`, `create_pr`, `lock_issue`) | 15 | 5 minutes | +| Review / merge queue inspection | 40 | 15 minutes | +| Exploration / codebase search (non-mutating) | 60 | 20 minutes | + +**Required behavior:** + +1. **Main session first.** When the active author profile allows + `gitea.repo.commit` and `gitea_commit_files` is visible, the main session + must call it directly — do not delegate commit authority to a subagent + (see #260). +2. **Native MCP before fallback.** After a shell spawn failure (#258), attempt + the native MCP tool once before any alternate path. Shell unavailability + never authorizes WebFetch, Playwright, or manual base64 encoding. +3. **No retry spirals.** Never resume a failed subagent into a larger retry + loop or spawn a second subagent for the same deterministic step. Stop and + emit a recovery report instead. +4. **Forbidden detours** when `gitea_commit_files` is available: WebFetch, + Playwright/browser automation, manual LLM-generated base64, and ad-hoc + `_encode_*` / `_emit_*` helper scripts left in the repo. + +Doc-contract tests: `tests/test_subagent_tool_budget_docs.py`. + ## Branch worktree isolation All LLM implementation and review work happens in an isolated branch worktree diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index c500bc4..3f99bd9 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -3874,6 +3874,16 @@ _GUIDE_RULES = { "paths (e.g. gitea_commit_files) for remaining mutations. Shell " "unavailability never authorizes WebFetch/browser/manual-encoding " "fallbacks (#258)."), + "subagent_tool_budget": ( + "General-purpose subagents on deterministic MCP tasks must stay within " + "budget: single-step mutations (commit_files, create_pr, lock_issue) " + "max 15 tool calls / 5 min; review/merge queue inspection max 40 / " + "15 min. The main session with gitea.repo.commit must call " + "gitea_commit_files directly — no subagent delegation (#260). After " + "shell spawn failure (#258), attempt native MCP once; never authorize " + "WebFetch/Playwright/manual base64 when gitea_commit_files is " + "available. Never resume a failed subagent into a larger retry loop " + "(#259)."), "subagent_delegation": ( "Deterministic write workflows (issue claim, branch creation, code " "edits, commits, PR creation, review, merge, cleanup) run inline in " diff --git a/skills/llm-project-workflow/SKILL.md b/skills/llm-project-workflow/SKILL.md index a47cb6b..0ef7db1 100644 --- a/skills/llm-project-workflow/SKILL.md +++ b/skills/llm-project-workflow/SKILL.md @@ -106,6 +106,31 @@ Implementation: `(fix|feat|docs|chore)/issue--` Review: `review/pr--` +## Subagent Tool-Budget Guardrails + +General-purpose subagents on **single-step MCP tasks** (for example +`gitea_commit_files`) must not expand into 100+ tool-call retry spirals with +WebFetch/Playwright/manual-encoding fallbacks (issue #259). + +Default budgets (stop when exceeded): + +- **Single-step MCP mutation** (`commit_files`, `create_pr`, `lock_issue`): + 15 tool calls, 5 minutes wall time. +- **Review / merge queue inspection**: 40 tool calls, 15 minutes. +- **Non-mutating exploration**: 60 tool calls, 20 minutes. + +Rules: + +1. When the main session has `gitea.repo.commit`, call `gitea_commit_files` + directly — do not delegate commit to a subagent (#260). +2. After shell spawn failure (#258), attempt the native MCP tool once before + any fallback; shell unavailability never authorizes WebFetch/Playwright/ + manual base64. +3. Never resume a failed subagent into a larger retry loop or spawn a second + subagent for the same deterministic step — stop and report. +4. When `gitea_commit_files` is available, forbid WebFetch, Playwright, + manual encoding, and ad-hoc `_encode_*` / `_emit_*` helpers in the repo. + Worktree folder: branch with `/` replaced by `-` under `branches/`. Helpers: `scripts/worktree-start`, `scripts/worktree-review`, diff --git a/tests/test_operator_guide.py b/tests/test_operator_guide.py index fd62809..a5159c6 100644 --- a/tests/test_operator_guide.py +++ b/tests/test_operator_guide.py @@ -134,7 +134,8 @@ class TestControlPlaneGuide(GuideTestBase): "merge_confirmation", "redaction", "separation", "profile_switching", "identity_verification", "work_selection", "global_worktree", - "shell_spawn_hard_stop"): + "shell_spawn_hard_stop", "subagent_tool_budget", + "subagent_delegation"): self.assertIn(key, rules) self.assertIn("MERGE PR", json.dumps(rules["merge_confirmation"])) self.assertTrue(rules["hard_stops"]) diff --git a/tests/test_subagent_tool_budget_docs.py b/tests/test_subagent_tool_budget_docs.py new file mode 100644 index 0000000..8b603ba --- /dev/null +++ b/tests/test_subagent_tool_budget_docs.py @@ -0,0 +1,89 @@ +"""Doc-contract checks for subagent tool-budget guardrails (#259). + +Single-step MCP tasks must not expand into 100+ tool-call retry spirals with +WebFetch/Playwright/manual-encoding fallbacks. These tests pin budget defaults, +native-MCP-first rules, and forbidden detours in the runbooks, portable skill, +and operator guide. +""" +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent + +RUNBOOK = REPO_ROOT / "docs" / "llm-workflow-runbooks.md" +SKILL = REPO_ROOT / "skills" / "llm-project-workflow" / "SKILL.md" + + +def _normalize(text: str) -> str: + """Collapse whitespace so phrase checks survive markdown line wrapping.""" + return " ".join(text.split()) + + +def _runbook_text() -> str: + return _normalize(RUNBOOK.read_text(encoding="utf-8")) + + +def _skill_text() -> str: + return _normalize(SKILL.read_text(encoding="utf-8")) + + +def test_runbook_has_subagent_tool_budget_section(): + text = _runbook_text() + assert "## Subagent Tool-Budget Guardrails" in text + for phrase in ( + "15", + "5 minutes", + "40", + "15 minutes", + "gitea_commit_files", + "WebFetch", + "Playwright", + "no retry spirals", + ): + assert phrase.lower() in text.lower(), f"runbook missing phrase: {phrase!r}" + + +def test_runbook_forbids_subagent_commit_delegation(): + text = _runbook_text() + for phrase in ( + "do not delegate commit authority to a subagent", + "main session first", + "native MCP before fallback", + ): + assert phrase.lower() in text.lower(), f"runbook missing: {phrase!r}" + + +def test_runbook_rejects_fallback_detours_when_mcp_commit_available(): + lower = _runbook_text().lower() + for forbidden in ("webfetch", "playwright", "manual llm-generated base64"): + assert forbidden in lower, f"runbook must name forbidden detour: {forbidden!r}" + assert "_encode_" in lower + assert "gitea_commit_files" in lower + + +def test_skill_doc_declares_subagent_tool_budget_guardrails(): + text = _skill_text() + assert "## Subagent Tool-Budget Guardrails" in text + for phrase in ( + "15 tool calls", + "5 minutes", + "gitea_commit_files", + "WebFetch", + "never resume a failed subagent", + ): + assert phrase.lower() in text.lower(), f"SKILL.md missing phrase: {phrase!r}" + + +def test_operator_guide_rules_include_subagent_tool_budget(): + import sys + + sys.path.insert(0, str(REPO_ROOT)) + import gitea_mcp_server + + rule = gitea_mcp_server._GUIDE_RULES["subagent_tool_budget"] + for phrase in ( + "15 tool calls", + "gitea_commit_files", + "WebFetch", + "retry loop", + ): + assert phrase in rule, f"operator guide rule missing: {phrase!r}" \ No newline at end of file