Skip to content

fix(reviewer): refuse to review against an unclean OC source tree - #251

Merged
ProtocolWarden merged 1 commit into
mainfrom
fix/reviewer-clean-tree-guard
Jun 7, 2026
Merged

fix(reviewer): refuse to review against an unclean OC source tree#251
ProtocolWarden merged 1 commit into
mainfrom
fix/reviewer-clean-tree-guard

Conversation

@ProtocolWarden

Copy link
Copy Markdown
Owner

The outage this fixes

On 2026-06-07, a git merge-conflict marker in the shared live checkout's cxrp_mapper.py made the reviewer's planning subprocess crash with SyntaxError at import time — for every PR, because planning imports operations_center from oc_root/src. Verdicts silently failed for ~4 hours (11:00–15:15 UTC). Consequences: #245/#246 had to be hand-merged, #247 sat green-but-unmerged. The marker was transient working-tree pollution from a concurrent watchdog session (never committed).

The reviewer logged planning produced no JSONno verdictreviewer unavailable — a misleading message that hid an environment problem as a per-PR review failure, and burned each PR's no-verdict budget.

The fix

  • Pre-flight guard (_oc_source_conflict_markers): one cheap git grep for conflict markers in tracked src/**/*.py; fail-open if git is unavailable.
  • Distinct classification (OCSourceTreeUncleanError): an unclean tree is an ENVIRONMENT failure, not a PR-quality one. It is not charged to the PR's no-verdict budget (an env problem would otherwise exhaust the budget and park a good PR), the log names the exact dirty files, and persistent uncleanliness escalates with reason=oc_source_tree_unclean instead of reviewer unavailable.
  • Both review-pass and fix-pass call sites handle it.

Scope note

This is the contained fix — it converts a silent multi-hour outage into a loud, specific, self-healing skip. The deeper isolation (run planning against a clean dedicated worktree at the merge ref, not the shared mutable checkout) is filed as WO-6 in .console/task.md because it needs the live pipeline to validate and can't be tested offline.

Verification

  • pytest tests/test_pr_review_watcher.py → 42 passed (8 new)
  • ruff + ty clean
  • Guard run against the live tree → [] (clean, as expected post-incident)

🤖 Generated with Claude Code

The planning subprocess imports operations_center from oc_root/src, so a git
conflict marker left in a tracked source file by a concurrent session crashes
it with SyntaxError at import — for EVERY PR, not just the one under review.
On 2026-06-07 a marker in cxrp_mapper.py silently blocked all verdicts for
~4h (#245/#246 hand-merged, #247 stuck green).

Pre-flight the tree for conflict markers and raise OCSourceTreeUncleanError, a
distinct ENVIRONMENT failure: it is not charged to the PR's no-verdict budget
(an env problem would otherwise exhaust the budget and park a good PR), the log
names the exact dirty files, and persistent uncleanliness escalates with
reason=oc_source_tree_unclean rather than a misleading 'reviewer unavailable'.
Guard is cheap (one git grep) and fail-open. 8 new tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ProtocolWarden
ProtocolWarden merged commit a8ffd23 into main Jun 7, 2026
15 checks passed
@ProtocolWarden
ProtocolWarden deleted the fix/reviewer-clean-tree-guard branch June 7, 2026 19:53
ProtocolWarden added a commit that referenced this pull request Jun 7, 2026
Every OC PR appends an entry at the top of .console/log.md, so each merge to
main turned every other open PR CONFLICTING on log.md (this session: #247,
#249, #250 all conflicted after #248/#251 merged). A merge=union driver makes
git keep both sides' appended lines automatically — no conflict — on every
local merge/rebase the loop runs.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant