fix(watch,review): role guard for start_watch_role + 406 diff-too-large fallback#216
Closed
ProtocolWarden wants to merge 3 commits into
Closed
fix(watch,review): role guard for start_watch_role + 406 diff-too-large fallback#216ProtocolWarden wants to merge 3 commits into
ProtocolWarden wants to merge 3 commits into
Conversation
… loops `start_watch_role` had no validation for the role argument; any unknown role (e.g. "all") fell through to the board_worker.main else-branch with --role <unknown>, which argparse rejected with exit code 2. The bash wrapper then restarted the process every 30s indefinitely, producing a silent crash loop with no operational watcher. Added an explicit guard that emits a diagnostic and returns 1 immediately. Fixed a live crash loop (PID 590871, role="all", 333+ cycles, 5+ hours) that prior cycles misclassified as historical. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fix all custodian violations in the observer/alert module introduced when PR #213 merged without a pre-commit audit pass: - alert_channels: remove async from notify() (no await inside); fix G004 f-string logging; update message to include condition name - alert_validation/exporters/structured_logging: fix G004 f-string logging; add encoding= to open() calls; add ensure_ascii=False to json.dump(s) calls; fix DTZ007 naive strptime → strptime with +0000%z - metrics: add MetricUnit.COUNT usage so D6 (never-constructed) resolves - docs/README.md: link two orphan coverage-gating docs (DC7) - tests: convert async def notify tests to sync (matching non-async impl); fix test_health_check_degraded_error_rate threshold (3% → not CRITICAL) - tests/test_alert_validation/test_validation_metrics_exporter: add missing assert statements for T2 findings Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…file list Root cause: GitHub returns 406 when PR diff exceeds the API size limit. get_pr_diff() silently swallowed HTTPStatusError and returned "", causing the review watcher to log "empty diff" and skip the PR permanently. Fix: Detect 406 explicitly in get_pr_diff() and fall back to the files API (_pr_diff_too_large_summary), returning a file-list summary prefixed with [DIFF_TOO_LARGE]. The review watcher now logs a distinct warning and proceeds with the partial diff rather than skipping the PR entirely. Incidental: ruff-format alignment fixes inherited in both touched files. Affected repo: OperationsCenter Gate/check fixed: PR review watcher now processes large-diff PRs (#214) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Owner
Author
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three fixes from watchdog cycle 9+ (2026-06-01/02):
start_watch_roleto prevent crash loops — adds validation guard; unknown roles now emit diagnostic and return 1 instead of restarting every 30s (root cause:watch-allinvoked with role="all", ran crash loop for 5+ hours)get_pr_diff— fall back to file-list summary so review watcher can still process oversized PRsNote
This branch diverged from main before PR #214 merged; observer fixes in 4c74e1a overlap with PR #214's custodian fix. Net-new contributions vs main: role guard (scripts/operations-center.sh) and 406 fallback (github_pr.py + pr_review_watcher).
Test plan
tests/unit/er000_phase0_golden/: 15/15 passed (confirmed pre-push)tests/unit/observer/: 181/181 passed (confirmed pre-push)🤖 Generated with Claude Code