Skip to content

fix(run): schema-force the plan turn and stop plan-file writes in hidden-write seats - #520

Merged
solomonneas merged 1 commit into
mainfrom
fix/518-chef-plan-schema-force
Jul 25, 2026
Merged

fix(run): schema-force the plan turn and stop plan-file writes in hidden-write seats#520
solomonneas merged 1 commit into
mainfrom
fix/518-chef-plan-schema-force

Conversation

@solomonneas

@solomonneas solomonneas commented Jul 25, 2026

Copy link
Copy Markdown
Collaborator

Closes #518

What happened

Two consecutive brigade run failures on the same task died with
orchestrator returned an invalid plan: plan is not valid JSON. A third run in the
same session, same roster, different repo, planned and completed normally.

Repro evidence (run ids): 20260725-014242-7a7b15af and 20260725-015900-d38906ed
failed; 20260725-014248-48ba749e is the control.

In both failures plan-attempts.json recorded ok: true, parsed: false for both the
initial and the corrective turn: the seat call succeeded and the payload was prose.
The prose in both cases was the chef arguing with a harness memory-handoff gate, and
both texts reference a Write of a code-graph context file that failed because no
write tool was enabled. The control run's chef never attempted that write.

Read-only claude, cursor, and grok seats launch in a harness plan mode with every
write tool hidden. Brigade never asked the chef to persist anything; the harness's own
plan-file affordance did. The failed write tripped user-level hooks, and the hook
rebuttal became the seat's final message.

The fix

Schema-force the plan turn. The corrective turn already existed but passed only
the raw parse error, which gives a seat mid-rebuttal nothing to correct toward. It now
restates the output contract next to the error: reply with the JSON plan object and
nothing else, no prose, no preamble, no tool-failure or hook commentary, and
{"assignments": []} if no worker is useful. Retries stay bounded at one correction,
so a second prose turn still fails the run with the existing error rather than looping
a hijacked seat.

Remove the trigger. agents.hides_write_tools() answers whether a seat launches
without file-write tools (claude, cursor, grok under read-only). When the orchestrator
is such a seat, the plan prompt says the plan lives in the reply and no plan, design,
or context file may be written. The existing read-only rule ("do not modify files")
did not cover the harness's own plan artifact.

Tests

  • prose then valid JSON: the plan succeeds, plan-attempts.json records
    initial (unparsed, with the parse error) then correction (parsed), and the retry
    prompt carries both the parse error and the JSON-only contract.
  • prose twice: fails with the existing orchestrator returned an invalid plan, two
    seat calls, both attempts recorded.
  • a read-only claude orchestrator gets the no-plan-file rule; a codex orchestrator's
    plan prompt is unchanged.
  • hides_write_tools unit coverage across plan-mode and write-capable seats.

Verification

brigade work verify run --target . --command "./scripts/verify" --capture brigade-work
-> completed, exit 0, 4203 passed / 3 skipped, coverage 82.67%.

Note for review

Worker and synthesis prompts run on the same plan-mode seats and could take the same
no-plan-file rule. Left out of scope here: only the plan turn parses the seat's final
message as structured output, so only the plan turn fails the run outright.

Summary by CodeRabbit

  • Bug Fixes

    • Improved plan-mode handling when write tools are unavailable.
    • Plans now require valid JSON after an invalid or prose response, with retries limited to prevent repeated failures.
    • Added safeguards to prevent attempts to write plan files in read-only mode.
  • Tests

    • Added coverage for JSON-only retry behavior, retry limits, and read-only tool detection across supported command-line interfaces.

…den-write seats

The chef seat runs read-only, which for claude, cursor, and grok means a harness
plan mode with every write tool hidden. The chef reached for the harness's own
plan-file affordance, the write failed, and the failure tripped user-level hooks
whose rebuttal prose replaced the plan JSON in the seat's final message. Both
plan attempts recorded ok: true with parsed: false, and the run died on
"orchestrator returned an invalid plan".

Two changes. The corrective plan turn now restates the output contract next to
the parse error instead of passing the error alone, so a seat mid-rebuttal has
something to correct toward; retries stay bounded at one correction. And plan
prompts for seats that launch without write tools say the plan lives in the
reply and no file may be written, so the trigger never fires. A generic
"do not modify files" rule did not cover the harness's own plan artifact.

Closes #518

Co-authored-by: Claude <noreply@anthropic.com>
@solomonneas solomonneas added the coderabbit-review Request one CodeRabbit review after CI is green label Jul 25, 2026
@coderabbitai

coderabbitai Bot commented Jul 25, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Changes

Plan-mode output safeguards

Layer / File(s) Summary
Seat detection and prompt rules
src/brigade/agents.py, src/brigade/aboyeur.py
Adds plan-mode CLI detection and conditional rules preventing plan-file writes and requiring JSON-only correction responses.
Planner retry flow
src/brigade/aboyeur.py
Propagates write-tool visibility through planning prompts and retries invalid initial plan responses with the JSON-only rule.
Validation coverage
tests/test_agents.py, tests/test_aboyeur.py
Tests CLI detection, JSON retry behavior, bounded failures, and read-only versus write-capable prompt rules.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant plan
  participant agents.hides_write_tools
  participant orchestrator
  plan->>agents.hides_write_tools: determine read-only write-tool visibility
  agents.hides_write_tools-->>plan: return no_file_writes
  plan->>orchestrator: send plan prompt with applicable rules
  orchestrator-->>plan: return plan response
  plan->>plan: parse JSON plan
  plan->>orchestrator: retry with PLAN_JSON_ONLY_RULE after parse failure
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main fix: schema-forcing plan output and preventing plan-file writes in hidden-write seats.
Linked Issues check ✅ Passed The PR matches #518 by retrying non-JSON plan output and avoiding plan-file writes when write tools are hidden.
Out of Scope Changes check ✅ Passed The changes and tests stay focused on the reported plan-response and write-tool-hiding issue.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/518-chef-plan-schema-force

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
tests/test_aboyeur.py (1)

3156-3183: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Exercise the actual writable plan-mode and sandbox-override paths.

Line 3181 uses codex, so it cannot catch accidental rule injection for writable claude. Add a writable-claude case and a sandbox_read_only=True case to validate _orchestrator_hides_write_tools integration.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_aboyeur.py` around lines 3156 - 3183, Update the plan-mode tests
around test_plan_mode_orchestrator_is_told_not_to_write_a_plan_file and
test_write_capable_orchestrator_keeps_the_plan_prompt_unchanged to exercise
writable claude and sandbox_read_only=True scenarios. Add assertions that
writable claude does not receive NO_PLAN_FILE_RULE, while sandbox_read_only=True
causes the orchestrator’s write-tool hiding behavior and corresponding rule
injection, validating _orchestrator_hides_write_tools integration.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/test_aboyeur.py`:
- Around line 3156-3183: Update the plan-mode tests around
test_plan_mode_orchestrator_is_told_not_to_write_a_plan_file and
test_write_capable_orchestrator_keeps_the_plan_prompt_unchanged to exercise
writable claude and sandbox_read_only=True scenarios. Add assertions that
writable claude does not receive NO_PLAN_FILE_RULE, while sandbox_read_only=True
causes the orchestrator’s write-tool hiding behavior and corresponding rule
injection, validating _orchestrator_hides_write_tools integration.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: escoffier-labs/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a125a67e-481f-4bd8-936f-d72db9935c97

📥 Commits

Reviewing files that changed from the base of the PR and between e8a1766 and f00f21b.

⛔ Files ignored due to path filters (1)
  • CHANGELOG.md is excluded by !**/CHANGELOG*
📒 Files selected for processing (4)
  • src/brigade/aboyeur.py
  • src/brigade/agents.py
  • tests/test_aboyeur.py
  • tests/test_agents.py

@solomonneas
solomonneas merged commit ee1ae34 into main Jul 25, 2026
28 checks passed
@solomonneas
solomonneas deleted the fix/518-chef-plan-schema-force branch July 25, 2026 02:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

coderabbit-review Request one CodeRabbit review after CI is green

Projects

None yet

Development

Successfully merging this pull request may close these issues.

run: chef plan output hijacked by harness hooks when a code-graph plan-file write fails

1 participant