Merge pull request 'docs: add Shell Spawn Hard-Stop Rule for agent workflows (Closes #258)' (#281) from docs/issue-258-shell-spawn-hard-stop into master
This commit was merged in pull request #281.
This commit is contained in:
@@ -285,6 +285,33 @@ explicit control-checkout repair.
|
|||||||
|
|
||||||
Portable wording: [`skills/llm-project-workflow/SKILL.md`](../skills/llm-project-workflow/SKILL.md).
|
Portable wording: [`skills/llm-project-workflow/SKILL.md`](../skills/llm-project-workflow/SKILL.md).
|
||||||
|
|
||||||
|
## Shell Spawn Hard-Stop Rule
|
||||||
|
|
||||||
|
Symptom: a shell tool call returns `exit_code: -1` with empty stdout/stderr.
|
||||||
|
That is an executor spawn failure, not a command failure — the command never
|
||||||
|
ran, and retrying the identical call cannot succeed.
|
||||||
|
|
||||||
|
Required behavior (fail closed, issue #258):
|
||||||
|
|
||||||
|
1. **Probe once.** On the first spawn failure, run one trivial probe
|
||||||
|
(`echo ok` or `pwd`). If the probe also returns `exit_code: -1`, mark
|
||||||
|
shell unavailable for the session.
|
||||||
|
2. **Hard-stop at two.** After two consecutive spawn failures, stop all
|
||||||
|
further shell tool use for the session; never retry the same failing
|
||||||
|
spawn. A hundred retries produce a hundred identical failures (session
|
||||||
|
`019f382e`: 100+ tool calls stalled on a trivial encode-and-commit task).
|
||||||
|
3. **Emit a recovery report.** The report must direct the operator to:
|
||||||
|
- restart the session,
|
||||||
|
- kill hung background terminals (a hung test runner holding the
|
||||||
|
executor is a known contributor),
|
||||||
|
- prefer MCP-native paths for remaining mutations (for example
|
||||||
|
`gitea_commit_files` under `gitea.repo.commit`) instead of shell.
|
||||||
|
4. **No improvised fallbacks.** Shell unavailability never authorizes
|
||||||
|
WebFetch/browser/manual-encoding workarounds (see #260). No shell means
|
||||||
|
stop-and-report.
|
||||||
|
|
||||||
|
Doc-contract tests: `tests/test_shell_spawn_hard_stop_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
|
||||||
|
|||||||
@@ -3629,6 +3629,16 @@ _GUIDE_RULES = {
|
|||||||
"small fixes, review fixes, conflicts, emergencies). Main checkout: "
|
"small fixes, review fixes, conflicts, emergencies). Main checkout: "
|
||||||
"read-only inspect, fetch, create worktrees, post-merge stable "
|
"read-only inspect, fetch, create worktrees, post-merge stable "
|
||||||
"update, explicit repair only."),
|
"update, explicit repair only."),
|
||||||
|
"shell_spawn_hard_stop": (
|
||||||
|
"A shell result of exit_code: -1 with empty stdout/stderr is an "
|
||||||
|
"executor spawn failure, not a command failure. Probe once (echo/pwd); "
|
||||||
|
"if the probe fails, mark shell unavailable for the session. After "
|
||||||
|
"two consecutive spawn failures, hard-stop all shell use — never "
|
||||||
|
"retry the same failing spawn — and emit a recovery report: restart "
|
||||||
|
"the session, kill hung background terminals, prefer MCP-native "
|
||||||
|
"paths (e.g. gitea_commit_files) for remaining mutations. Shell "
|
||||||
|
"unavailability never authorizes WebFetch/browser/manual-encoding "
|
||||||
|
"fallbacks (#258)."),
|
||||||
}
|
}
|
||||||
|
|
||||||
_COMMON_WORKFLOWS = [
|
_COMMON_WORKFLOWS = [
|
||||||
|
|||||||
@@ -115,6 +115,26 @@ The main checkout may only be used for read-only inspection, fetching,
|
|||||||
stable-branch update after merged PRs, creating `branches/` worktrees, or
|
stable-branch update after merged PRs, creating `branches/` worktrees, or
|
||||||
explicit control-checkout repair.
|
explicit control-checkout repair.
|
||||||
|
|
||||||
|
## Shell Spawn Hard-Stop Rule
|
||||||
|
|
||||||
|
A shell tool result of `exit_code: -1` with empty stdout/stderr means the
|
||||||
|
executor failed to spawn — it is not a command failure, and retrying the same
|
||||||
|
call cannot succeed.
|
||||||
|
|
||||||
|
1. On the first spawn failure, run one trivial probe (`echo ok` or `pwd`).
|
||||||
|
If the probe also fails, mark shell unavailable for the session.
|
||||||
|
2. After two consecutive spawn failures, hard-stop all further shell tool
|
||||||
|
use for the session. Never retry the same failing spawn.
|
||||||
|
3. Stop and emit a recovery report instead of improvising fallbacks. The
|
||||||
|
report must tell the operator to: restart the session, kill hung
|
||||||
|
background terminals (a hung test runner is a known contributor), and
|
||||||
|
prefer MCP-native paths (for example `gitea_commit_files`) for any
|
||||||
|
remaining mutations.
|
||||||
|
|
||||||
|
Retry spirals are a real failure mode (issue #258: 100+ tool calls on a
|
||||||
|
trivial encode-and-commit task). The hard-stop is fail-closed: no shell means
|
||||||
|
stop-and-report, not workarounds.
|
||||||
|
|
||||||
## B. Isolated worktree rule
|
## B. Isolated worktree rule
|
||||||
|
|
||||||
**Never implement or review in the main checkout** (Global LLM Worktree Rule).
|
**Never implement or review in the main checkout** (Global LLM Worktree Rule).
|
||||||
|
|||||||
@@ -133,7 +133,8 @@ class TestControlPlaneGuide(GuideTestBase):
|
|||||||
for key in ("hard_stops", "fail_closed", "head_sha_pinning",
|
for key in ("hard_stops", "fail_closed", "head_sha_pinning",
|
||||||
"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"):
|
||||||
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,82 @@
|
|||||||
|
"""Doc-contract checks for the shell-spawn hard-stop rule (#258).
|
||||||
|
|
||||||
|
When the shell executor fails to spawn (exit_code: -1 with empty
|
||||||
|
stdout/stderr), agents must probe once, mark shell unavailable, hard-stop
|
||||||
|
after two consecutive spawn failures, and emit a recovery report instead of
|
||||||
|
retry-spiralling. These tests pin that guidance in the runbooks, the portable
|
||||||
|
skill doc, and the operator guide so it cannot silently regress.
|
||||||
|
"""
|
||||||
|
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_shell_spawn_hard_stop_section():
|
||||||
|
text = _runbook_text()
|
||||||
|
assert "## Shell Spawn Hard-Stop Rule" in text
|
||||||
|
for phrase in (
|
||||||
|
"exit_code: -1",
|
||||||
|
"empty stdout/stderr",
|
||||||
|
"trivial probe",
|
||||||
|
"two consecutive spawn failures",
|
||||||
|
"mark shell unavailable",
|
||||||
|
"recovery report",
|
||||||
|
):
|
||||||
|
assert phrase in text, f"runbook missing phrase: {phrase!r}"
|
||||||
|
|
||||||
|
|
||||||
|
def test_runbook_recovery_report_names_required_steps():
|
||||||
|
text = _runbook_text()
|
||||||
|
for phrase in (
|
||||||
|
"restart the session",
|
||||||
|
"kill hung background terminals",
|
||||||
|
"MCP-native",
|
||||||
|
):
|
||||||
|
assert phrase in text, f"runbook recovery guidance missing: {phrase!r}"
|
||||||
|
|
||||||
|
|
||||||
|
def test_runbook_forbids_retry_spirals():
|
||||||
|
text = _runbook_text()
|
||||||
|
assert "never retry the same failing spawn" in text
|
||||||
|
|
||||||
|
|
||||||
|
def test_skill_doc_declares_shell_spawn_hard_stop_rule():
|
||||||
|
text = _skill_text()
|
||||||
|
assert "## Shell Spawn Hard-Stop Rule" in text
|
||||||
|
for phrase in (
|
||||||
|
"exit_code: -1",
|
||||||
|
"two consecutive spawn failures",
|
||||||
|
"recovery report",
|
||||||
|
):
|
||||||
|
assert phrase in text, f"SKILL.md missing phrase: {phrase!r}"
|
||||||
|
|
||||||
|
|
||||||
|
def test_operator_guide_rules_include_shell_spawn_hard_stop():
|
||||||
|
import sys
|
||||||
|
|
||||||
|
sys.path.insert(0, str(REPO_ROOT))
|
||||||
|
import gitea_mcp_server
|
||||||
|
|
||||||
|
rule = gitea_mcp_server._GUIDE_RULES["shell_spawn_hard_stop"]
|
||||||
|
for phrase in (
|
||||||
|
"exit_code: -1",
|
||||||
|
"two consecutive",
|
||||||
|
"recovery report",
|
||||||
|
):
|
||||||
|
assert phrase in rule, f"operator guide rule missing: {phrase!r}"
|
||||||
Reference in New Issue
Block a user