Merge pull request 'docs: add subagent tool-budget guardrails for MCP tasks (Closes #259)' (#284) from docs/issue-259-subagent-tool-budget into master
This commit was merged in pull request #284.
This commit is contained in:
@@ -312,6 +312,39 @@ Required behavior (fail closed, issue #258):
|
|||||||
|
|
||||||
Doc-contract tests: `tests/test_shell_spawn_hard_stop_docs.py`.
|
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
|
## Branch worktree isolation
|
||||||
|
|
||||||
All LLM implementation and review work happens in an isolated branch worktree
|
All LLM implementation and review work happens in an isolated branch worktree
|
||||||
|
|||||||
@@ -3874,6 +3874,16 @@ _GUIDE_RULES = {
|
|||||||
"paths (e.g. gitea_commit_files) for remaining mutations. Shell "
|
"paths (e.g. gitea_commit_files) for remaining mutations. Shell "
|
||||||
"unavailability never authorizes WebFetch/browser/manual-encoding "
|
"unavailability never authorizes WebFetch/browser/manual-encoding "
|
||||||
"fallbacks (#258)."),
|
"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": (
|
"subagent_delegation": (
|
||||||
"Deterministic write workflows (issue claim, branch creation, code "
|
"Deterministic write workflows (issue claim, branch creation, code "
|
||||||
"edits, commits, PR creation, review, merge, cleanup) run inline in "
|
"edits, commits, PR creation, review, merge, cleanup) run inline in "
|
||||||
|
|||||||
@@ -106,6 +106,31 @@ Implementation: `(fix|feat|docs|chore)/issue-<number>-<short-description>`
|
|||||||
|
|
||||||
Review: `review/pr-<number>-<short-description>`
|
Review: `review/pr-<number>-<short-description>`
|
||||||
|
|
||||||
|
## 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/`.
|
Worktree folder: branch with `/` replaced by `-` under `branches/`.
|
||||||
|
|
||||||
Helpers: `scripts/worktree-start`, `scripts/worktree-review`,
|
Helpers: `scripts/worktree-start`, `scripts/worktree-review`,
|
||||||
|
|||||||
@@ -134,7 +134,8 @@ class TestControlPlaneGuide(GuideTestBase):
|
|||||||
"merge_confirmation", "redaction", "separation",
|
"merge_confirmation", "redaction", "separation",
|
||||||
"profile_switching", "identity_verification",
|
"profile_switching", "identity_verification",
|
||||||
"work_selection", "global_worktree",
|
"work_selection", "global_worktree",
|
||||||
"shell_spawn_hard_stop"):
|
"shell_spawn_hard_stop", "subagent_tool_budget",
|
||||||
|
"subagent_delegation"):
|
||||||
self.assertIn(key, rules)
|
self.assertIn(key, rules)
|
||||||
self.assertIn("MERGE PR", json.dumps(rules["merge_confirmation"]))
|
self.assertIn("MERGE PR", json.dumps(rules["merge_confirmation"]))
|
||||||
self.assertTrue(rules["hard_stops"])
|
self.assertTrue(rules["hard_stops"])
|
||||||
|
|||||||
@@ -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}"
|
||||||
Reference in New Issue
Block a user