Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- `brigade mcp sync --user-scope` (and `brigade operator sync-mcp --user-scope`) no longer writes stdio MCP servers into a user-wide client config silently: interactive runs show the destination, stdio count, and the servers-times-sessions process formula and ask for confirmation, non-interactive and `--json` runs require `--allow-global-stdio`, and plan/sync items now carry `transport` and `scope`. (#349)

### Fixed
- `brigade run` no longer dies on the first unparsable plan when the chef's final
message is prose. The corrective plan turn now restates the output contract
("reply with the JSON plan object and nothing else") alongside the parse error,
and orchestrator seats that launch in a harness plan mode (claude, cursor, grok
under read-only) are told not to write a plan, design, or context file: the
failed write is what let user-level hooks replace the plan JSON with hook
rebuttal prose. Retries stay bounded at one correction. (#518)
- Agent Pantry version parsing stays non-throwing and bounded for arbitrarily
long numeric segments: the parser accepts ASCII-numeric semver triples only,
enforces a conservative per-segment digit bound, catches `int()` conversion
Expand Down
49 changes: 48 additions & 1 deletion src/brigade/aboyeur.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,23 @@
BRIEF_BUDGET_BYTES = 6000
NOOP_DETAIL = "no-op"

# A plan-mode seat has no write tool, so any file it tries to create fails, and a
# failed write is what invites harness hooks to hijack the seat's final message
# (#518). Say the quiet part in the prompt: the plan lives in the reply, nowhere else.
NO_PLAN_FILE_RULE = (
"- Do not write, create, or edit any file, including a plan, design, or context file. "
"This seat runs with every write tool hidden: the write fails, and the failure can "
"replace your plan with tool or hook commentary. The plan belongs in this reply only."
)

# The corrective turn is the last chance before the run dies on an unparsable
# plan, so it restates the output contract instead of only naming the parse error.
PLAN_JSON_ONLY_RULE = (
"Reply with the JSON plan object and nothing else: no prose, no preamble, no explanation, "
"no tool-failure or hook commentary, nothing before or after the object. "
'If no worker is useful, reply with exactly {"assignments": []}.'
)


@dataclass(frozen=True)
class CodeGraphBrief:
Expand Down Expand Up @@ -367,6 +384,7 @@ def build_plan_prompt(
drift_impact: DriftImpactBrief | None = None,
evidence: EvidenceBrief | None = None,
route: RouteBrief | None = None,
no_file_writes: bool = False,
) -> str:
worker_lines = "\n".join(
f"- {agent.name}: cli={agent.cli}; "
Expand All @@ -380,6 +398,7 @@ def build_plan_prompt(
note = f"\nCorrection needed: {corrective_note}\n" if corrective_note else ""
policy = f"\n\n{_read_only_rules()}\n" if read_only else ""
capability_rule = "- Assign only workers with read_only_capable=true.\n" if read_only else ""
no_write_rule = f"\n{NO_PLAN_FILE_RULE}" if no_file_writes else ""
route_section = ""
route_rule = ""
if route is not None and route.attached and route.text:
Expand All @@ -404,6 +423,7 @@ def build_plan_prompt(
f"{capability_rule}"
"- Use zero assignments only if no worker is useful."
f"{route_rule}"
f"{no_write_rule}"
f"{policy}"
)
return _prepend_optional_briefs(prompt, code_graph=code_graph, drift_impact=drift_impact, evidence=evidence)
Expand Down Expand Up @@ -805,6 +825,22 @@ def _unknown_covers(route: RouteBrief | None, assignments: list[Assignment]) ->
return unknown_covers(route, assignments)


def _orchestrator_hides_write_tools(
roster: Roster,
*,
read_only: bool,
sandbox_read_only: bool | None,
sandbox: str | None,
) -> bool:
"""True when the orchestrator seat launches without any file-write tool."""
orchestrator = roster.agents.get(roster.orchestrator)
if orchestrator is None:
return False
# _run_orchestrator resolves read-only the same way before building argv.
effective_read_only = read_only if sandbox_read_only is None else sandbox_read_only
return agents.hides_write_tools(orchestrator.cli, read_only=effective_read_only, sandbox=sandbox)


def plan(
task: str,
roster: Roster,
Expand All @@ -821,6 +857,12 @@ def plan(
process_registry: proc.ProcessRegistry | None = None,
) -> list[Assignment]:
transport = codex_transport or roster.codex_transport
no_file_writes = _orchestrator_hides_write_tools(
roster,
read_only=read_only,
sandbox_read_only=sandbox_read_only,
sandbox=sandbox,
)
first = _call_with_process_registry(
_run_orchestrator,
roster,
Expand All @@ -832,6 +874,7 @@ def plan(
drift_impact=drift_impact,
evidence=evidence,
route=route,
no_file_writes=no_file_writes,
),
cwd=cwd,
read_only=read_only,
Expand All @@ -855,18 +898,21 @@ def plan(
)
except ValueError as exc:
_record_plan_attempt(attempts, stage="initial", result=first, parse_error=str(exc))
# Schema-force the retry: the parse error alone left a seat whose final
# message had been hijacked by hooks with nothing to correct toward (#518).
second = _call_with_process_registry(
_run_orchestrator,
roster,
build_plan_prompt(
task,
roster,
corrective_note=str(exc),
corrective_note=f"{exc} {PLAN_JSON_ONLY_RULE}",
read_only=read_only,
code_graph=code_graph,
drift_impact=drift_impact,
evidence=evidence,
route=route,
no_file_writes=no_file_writes,
),
cwd=cwd,
read_only=read_only,
Expand Down Expand Up @@ -919,6 +965,7 @@ def plan(
drift_impact=drift_impact,
evidence=evidence,
route=route,
no_file_writes=no_file_writes,
),
cwd=cwd,
read_only=read_only,
Expand Down
15 changes: 15 additions & 0 deletions src/brigade/agents.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,21 @@ class _GrokFinal:
_CLAUDE_DISALLOWED_READ_ONLY = "Task,Agent,Bash,Edit,Write,NotebookEdit,WebSearch,WebFetch,mcp__*"


# Read-only argv for these CLIs is a harness plan mode: every file-write tool is
# hidden, but the harness still offers its own plan-file affordance and the model
# reaches for it. #518: the chef's plan-file write failed for exactly that reason,
# user-level hooks fired on the failed write, and the hook rebuttal replaced the
# plan JSON in the seat's final message. Prompts for these seats must say so.
_PLAN_MODE_CLIS = frozenset({"claude", "cursor", "grok"})


def hides_write_tools(cli_ref: str, *, read_only: bool = False, sandbox: str | None = None) -> bool:
"""Return True when this seat launches with every file-write tool hidden."""
if not (read_only or sandbox == "read-only"):
return False
return cli_ref.split(":", 1)[0] in _PLAN_MODE_CLIS


class UnsupportedSandboxError(ValueError):
"""A builder rejected the launch because the sandbox cannot be enforced.

Expand Down
89 changes: 89 additions & 0 deletions tests/test_aboyeur.py
Original file line number Diff line number Diff line change
Expand Up @@ -3094,6 +3094,95 @@ def fake_run_agent(cli_ref, prompt, timeout=600.0, cwd=None, read_only=False):
assert not (output_dir / "plan.json").exists()


def _plan_mode_roster(orchestrator_cli: str) -> Roster:
return Roster(
orchestrator="chef",
agents={
"chef": Agent("chef", orchestrator_cli, "plan and synthesize"),
"coder": Agent("coder", "codex", "write code"),
},
max_workers=1,
)


def test_plan_retry_schema_forces_json_after_prose(monkeypatch):
# #518: the chef's first turn was hook-rebuttal prose, not a plan. The retry
# must name the parse error and demand JSON only, or the second turn is prose too.
prompts: list[str] = []
replies = [
"I can't write a memory handoff this session: no Write/Edit tool is enabled.",
json.dumps({"assignments": [{"stage": 1, "worker": "coder", "task": "implement it"}]}),
]

def fake_run_agent(cli_ref, prompt, **kwargs):
prompts.append(prompt)
return agents.AgentResult(text=replies[len(prompts) - 1], ok=True)

monkeypatch.setattr(aboyeur.agents, "run_agent", fake_run_agent)

attempts: list[dict[str, object]] = []
assignments = aboyeur.plan("build feature", _roster(), attempts=attempts)

assert [assignment.worker for assignment in assignments] == ["coder"]
assert [attempt["stage"] for attempt in attempts] == ["initial", "correction"]
assert [attempt["parsed"] for attempt in attempts] == [False, True]
assert "plan is not valid JSON" in str(attempts[0]["parse_error"])

retry = prompts[1]
assert "plan is not valid JSON" in retry
assert aboyeur.PLAN_JSON_ONLY_RULE in retry


def test_plan_fails_after_two_prose_replies(monkeypatch):
# The schema-force retry is bounded: two prose turns end the run instead of
# looping a seat whose final message keeps getting hijacked.
calls: list[str] = []

def fake_run_agent(cli_ref, prompt, **kwargs):
calls.append(prompt)
return agents.AgentResult(text="No Brigade handoff tool surfaced, so I am stopping here.", ok=True)

monkeypatch.setattr(aboyeur.agents, "run_agent", fake_run_agent)

attempts: list[dict[str, object]] = []
with pytest.raises(RuntimeError, match="orchestrator returned an invalid plan"):
aboyeur.plan("build feature", _roster(), attempts=attempts)

assert len(calls) == 2
assert [attempt["stage"] for attempt in attempts] == ["initial", "correction"]
assert [attempt["parsed"] for attempt in attempts] == [False, False]


def test_plan_mode_orchestrator_is_told_not_to_write_a_plan_file(monkeypatch):
# A read-only claude seat launches with every write tool hidden, so the
# harness plan-file write it reaches for fails and trips user-level hooks.
prompts: list[str] = []

def fake_run_agent(cli_ref, prompt, **kwargs):
prompts.append(prompt)
return agents.AgentResult(text=json.dumps({"assignments": []}), ok=True)

monkeypatch.setattr(aboyeur.agents, "run_agent", fake_run_agent)

aboyeur.plan("build feature", _plan_mode_roster("claude"), read_only=True)

assert aboyeur.NO_PLAN_FILE_RULE in prompts[0]


def test_write_capable_orchestrator_keeps_the_plan_prompt_unchanged(monkeypatch):
prompts: list[str] = []

def fake_run_agent(cli_ref, prompt, **kwargs):
prompts.append(prompt)
return agents.AgentResult(text=json.dumps({"assignments": []}), ok=True)

monkeypatch.setattr(aboyeur.agents, "run_agent", fake_run_agent)

aboyeur.plan("build feature", _plan_mode_roster("codex"))

assert aboyeur.NO_PLAN_FILE_RULE not in prompts[0]


def test_plan_timeout_writes_terminal_timeout_receipt(monkeypatch, tmp_path):
def fake_run_agent(cli_ref, prompt, **kwargs):
return agents.AgentResult(
Expand Down
12 changes: 12 additions & 0 deletions tests/test_agents.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,18 @@ def test_claude_read_only_sandbox_variant_matches_read_only_flag():
]


def test_hides_write_tools_reports_plan_mode_seats():
# #518: plan-mode seats hide every write tool, so a prompt that leads the
# model toward a plan-file write produces a failed write, not a file.
assert agents.hides_write_tools("claude", read_only=True) is True
assert agents.hides_write_tools("claude", sandbox="read-only") is True
assert agents.hides_write_tools("grok", read_only=True) is True
assert agents.hides_write_tools("cursor", read_only=True) is True
assert agents.hides_write_tools("claude", sandbox="danger-full-access") is False
assert agents.hides_write_tools("codex", read_only=True) is False
assert agents.hides_write_tools("ollama:llama3.3", read_only=True) is False


def test_claude_write_run_uses_skip_permissions_and_disallows_subagents():
# Contract: only an explicit --sandbox danger-full-access request may add
# --dangerously-skip-permissions. A write run with no explicit sandbox must
Expand Down
Loading