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
9 changes: 9 additions & 0 deletions .console/log.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
## 2026-06-13 — fix(reviewer): GC orphaned PR-review state files

state/pr_reviews/<repo>-<n>.json is unlinked by _merge_and_done / _close_and_requeue, but only when
THIS watcher terminates the PR. PRs merged/closed by other means (manual gh merge, another host, or
while the watcher was down/stale) leave their state files behind forever (observed: 73 files, ~41 for
already-terminal OC PRs from May). Added _prune_orphan_state_files, called each _poll_once after a
SUCCESSFUL list_open_prs: any state file for that repo whose PR isn't in the open set is pruned. A
false prune is self-healing (next poll re-discovers the open PR and re-creates state). +2 tests.

## 2026-06-13 — fix(spec-hygiene): active.json projects only active campaigns (campaign GC)

_rebuild_active_projection wrote every campaign — incl. complete/cancelled — to state/campaigns/
Expand Down
35 changes: 35 additions & 0 deletions src/operations_center/entrypoints/pr_review_watcher/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,35 @@ def _state_path(oc_root: Path, repo_key: str, pr_number: int) -> Path:
return oc_root / _STATE_SUBDIR / f"{_state_key(repo_key, pr_number)}.json"


def _prune_orphan_state_files(oc_root: Path, repo_key: str, open_numbers: set[int]) -> None:
"""Delete review-state files for PRs that are no longer open.

The per-merge/close unlinks in _merge_and_done / _close_and_requeue only fire
when THIS watcher terminates a PR. PRs merged or closed by any other means
(a manual ``gh pr merge``, another host, or while this watcher was down/stale)
leave their state/pr_reviews/<repo>-<n>.json behind, and they accumulate
forever. Callers MUST pass a set built from a SUCCESSFUL list_open_prs — any
state file for this repo whose PR is not in that set is for a terminated PR.
A false prune (PR open but missing from a partial fetch) is self-healing: the
next poll re-discovers the open PR and re-creates its state.
"""
state_dir = oc_root / _STATE_SUBDIR
if not state_dir.is_dir():
return
prefix = f"{repo_key}-"
for f in state_dir.glob(f"{repo_key}-*.json"):
if not f.stem.startswith(prefix):
continue
num_part = f.stem[len(prefix):]
if not num_part.isdigit() or int(num_part) in open_numbers:
continue
try:
f.unlink(missing_ok=True)
logger.info("pr_review_watcher: pruned orphan review-state %s (PR not open)", f.name)
except Exception as exc:
logger.debug("pr_review_watcher: prune failed for %s — %s", f.name, exc)


def _load_state(path: Path) -> dict:
try:
return json.loads(path.read_text(encoding="utf-8"))
Expand Down Expand Up @@ -2300,6 +2329,12 @@ def _poll_once(oc_root: Path, config_path: Path, settings) -> None:
logger.warning("pr_review_watcher: failed to list PRs %s/%s — %s", owner, repo, exc)
continue

# GC leftover review-state for PRs that terminated outside this watcher's
# merge/close path (manual merge, another host, or while it was down).
_prune_orphan_state_files(
oc_root, repo_key, {int(p["number"]) for p in open_prs if p.get("number") is not None}
)

# Build the worklist (discover + load state) before processing, so the
# sweep can be ordered. A single slow PR (a multi-pass fix battle) must
# not push a merge-ready PR to the back of the sweep — or off it entirely
Expand Down
34 changes: 34 additions & 0 deletions tests/test_pr_review_watcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -2190,3 +2190,37 @@ def test_phase1_untruncated_diff_skips_file_list(tmp_path: Path) -> None:
)

gh.list_pr_files.assert_not_called()


def test_prune_orphan_state_files(tmp_path: Path) -> None:
"""Only state files for PRs not in the open set (and matching this repo, with a
numeric suffix) are pruned; other repos and non-numeric files are untouched."""
sub = tmp_path / "state" / "pr_reviews"
sub.mkdir(parents=True)
for name in (
"OperationsCenter-100.json", # open → keep
"OperationsCenter-101.json", # terminal → prune
"OperationsCenter-102.json", # terminal → prune
"OtherRepo-100.json", # different repo → untouched
"OperationsCenter-readme.json", # non-numeric → untouched
):
(sub / name).write_text("{}", encoding="utf-8")

watcher._prune_orphan_state_files(tmp_path, "OperationsCenter", {100})

remaining = {p.name for p in sub.iterdir()}
assert remaining == {
"OperationsCenter-100.json",
"OtherRepo-100.json",
"OperationsCenter-readme.json",
}


def test_prune_orphan_state_files_empty_open_set_prunes_all_for_repo(tmp_path: Path) -> None:
"""No open PRs for the repo (all merged) → all its state files pruned."""
sub = tmp_path / "state" / "pr_reviews"
sub.mkdir(parents=True)
(sub / "OperationsCenter-1.json").write_text("{}", encoding="utf-8")
(sub / "OperationsCenter-2.json").write_text("{}", encoding="utf-8")
watcher._prune_orphan_state_files(tmp_path, "OperationsCenter", set())
assert list(sub.iterdir()) == []
Loading