From 2454bc7020ba7c0cc11a127ee38ef0d7ceacb846 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 1 Jul 2026 09:54:41 +0900 Subject: [PATCH 1/5] Guard autofix dispatch against process-only reviews --- scripts/ci/pr_review_fix_scheduler.py | 81 ++++++++++++++++++++++++++- 1 file changed, 79 insertions(+), 2 deletions(-) diff --git a/scripts/ci/pr_review_fix_scheduler.py b/scripts/ci/pr_review_fix_scheduler.py index d46360617..a553ac6f4 100755 --- a/scripts/ci/pr_review_fix_scheduler.py +++ b/scripts/ci/pr_review_fix_scheduler.py @@ -17,6 +17,8 @@ fetch_open_prs, fetch_pr, has_current_head_changes_requested, + is_opencode_review, + review_matches_current_head, run, unresolved_thread_count, ) @@ -25,6 +27,8 @@ fetch_open_prs, fetch_pr, has_current_head_changes_requested, + is_opencode_review, + review_matches_current_head, run, unresolved_thread_count, ) @@ -37,6 +41,18 @@ r"head_sha=([0-9a-fA-F]{40}) epoch=([0-9]+) -->" ) REPO_RE = re.compile(r"^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$") +NON_AUTOFIX_CHANGE_REQUEST_MARKERS = ( + "merge conflict", + "mergestatestatus `dirty`", + "mergestatestatus dirty", + "model pool exhausted", + "could not establish approval sufficiency", + "unresolved human review thread", + "failed check", + "failed-check", + "coverage-evidence", + "strix failed", +) def run_json(args: list[str]) -> Any: @@ -70,10 +86,31 @@ def same_repository_head(repo: str, pr: dict[str, Any]) -> bool: return ((pr.get("headRepository") or {}).get("nameWithOwner") or "") == repo +def latest_current_head_opencode_review(pr: dict[str, Any]) -> dict[str, Any] | None: + """Return the newest OpenCode review for the current head, if present.""" + for review in reversed((pr.get("reviews") or {}).get("nodes") or []): + if is_opencode_review(review) and review_matches_current_head(review, pr): + return review + return None + + +def change_request_is_autofixable(pr: dict[str, Any]) -> bool: + """Return whether the latest OpenCode request is safe for bot autofix.""" + merge_state = str(pr.get("mergeStateStatus") or "").upper() + if merge_state and merge_state not in {"CLEAN", "HAS_HOOKS"}: + return False + + review = latest_current_head_opencode_review(pr) + body = str((review or {}).get("body") or "").lower() + if any(marker in body for marker in NON_AUTOFIX_CHANGE_REQUEST_MARKERS): + return False + return True + + def needs_autofix(pr: dict[str, Any]) -> tuple[bool, tuple[str, ...]]: """Return whether current-head evidence justifies an autofix attempt.""" reasons: list[str] = [] - if has_current_head_changes_requested(pr): + if has_current_head_changes_requested(pr) and change_request_is_autofixable(pr): reasons.append("current-head OpenCode requested changes") unresolved = unresolved_thread_count(pr) if unresolved: @@ -260,11 +297,51 @@ def self_test() -> int: assert recent_fix_marker_exists(comments, head, 24 * 3600) assert not recent_fix_marker_exists(comments, "b" * 40, 24 * 3600) pr = { - "reviews": {"nodes": [{"state": "CHANGES_REQUESTED", "author": {"login": "opencode-agent"}, "commit": {"oid": head}}]}, + "reviews": { + "nodes": [ + { + "state": "CHANGES_REQUESTED", + "author": {"login": "opencode-agent"}, + "commit": {"oid": head}, + "body": "Actionable source-backed finding with a suggested diff.", + } + ] + }, "reviewThreads": {"nodes": []}, "headRefOid": head, + "mergeStateStatus": "CLEAN", } assert needs_autofix(pr) == (True, ("current-head OpenCode requested changes",)) + dirty_pr = {**pr, "mergeStateStatus": "DIRTY"} + assert needs_autofix(dirty_pr) == (False, ()) + model_exhausted_pr = { + **pr, + "reviews": { + "nodes": [ + { + "state": "CHANGES_REQUESTED", + "author": {"login": "opencode-agent"}, + "commit": {"oid": head}, + "body": "OpenCode could not establish approval sufficiency because the model pool exhausted.", + } + ] + }, + } + assert needs_autofix(model_exhausted_pr) == (False, ()) + unresolved_thread_pr = { + **pr, + "reviews": { + "nodes": [ + { + "state": "CHANGES_REQUESTED", + "author": {"login": "opencode-agent"}, + "commit": {"oid": head}, + "body": "OpenCode found unresolved human review thread evidence before approval.", + } + ] + }, + } + assert needs_autofix(unresolved_thread_pr) == (False, ()) print("self-test passed") return 0 From 146892fc0f012fc02e6b4a51552d48cd790f626b Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 1 Jul 2026 10:03:37 +0900 Subject: [PATCH 2/5] test: cover non-current autofix review guard --- scripts/ci/pr_review_fix_scheduler.py | 2 ++ tests/test_pr_review_fix_scheduler.py | 27 +++++++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/scripts/ci/pr_review_fix_scheduler.py b/scripts/ci/pr_review_fix_scheduler.py index a553ac6f4..590b6f3b3 100755 --- a/scripts/ci/pr_review_fix_scheduler.py +++ b/scripts/ci/pr_review_fix_scheduler.py @@ -101,6 +101,8 @@ def change_request_is_autofixable(pr: dict[str, Any]) -> bool: return False review = latest_current_head_opencode_review(pr) + if review is None: + return False body = str((review or {}).get("body") or "").lower() if any(marker in body for marker in NON_AUTOFIX_CHANGE_REQUEST_MARKERS): return False diff --git a/tests/test_pr_review_fix_scheduler.py b/tests/test_pr_review_fix_scheduler.py index d371e1a2b..dec6aa3c3 100644 --- a/tests/test_pr_review_fix_scheduler.py +++ b/tests/test_pr_review_fix_scheduler.py @@ -56,6 +56,33 @@ def test_needs_autofix_uses_current_head_evidence(): ) +def test_change_request_requires_current_head_opencode_review(): + """Autofixable change requests require an OpenCode review on the current head.""" + head = "a" * 40 + stale_head = "b" * 40 + + no_review_pr = make_pr(headRefOid=head, mergeStateStatus="CLEAN") + assert fix.latest_current_head_opencode_review(no_review_pr) is None + assert not fix.change_request_is_autofixable(no_review_pr) + + stale_review_pr = make_pr( + headRefOid=head, + mergeStateStatus="CLEAN", + reviews={ + "nodes": [ + { + "state": "CHANGES_REQUESTED", + "author": {"login": "opencode-agent"}, + "commit": {"oid": stale_head}, + "body": "Actionable source-backed finding with a suggested diff.", + } + ] + }, + ) + assert fix.latest_current_head_opencode_review(stale_review_pr) is None + assert not fix.change_request_is_autofixable(stale_review_pr) + + def test_process_queue_dispatches_same_repo_current_head(monkeypatch, capsys): """The queue path dispatches one same-repository autofix.""" pr = make_pr() From f2ca3ae6f2eea6f78c9122a5a06dd311639a478e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 1 Jul 2026 10:04:30 +0900 Subject: [PATCH 3/5] Add autofix guard regression tests --- scripts/ci/pr_review_fix_scheduler.py | 2 ++ tests/test_pr_review_fix_scheduler.py | 42 ++++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/scripts/ci/pr_review_fix_scheduler.py b/scripts/ci/pr_review_fix_scheduler.py index a553ac6f4..590b6f3b3 100755 --- a/scripts/ci/pr_review_fix_scheduler.py +++ b/scripts/ci/pr_review_fix_scheduler.py @@ -101,6 +101,8 @@ def change_request_is_autofixable(pr: dict[str, Any]) -> bool: return False review = latest_current_head_opencode_review(pr) + if review is None: + return False body = str((review or {}).get("body") or "").lower() if any(marker in body for marker in NON_AUTOFIX_CHANGE_REQUEST_MARKERS): return False diff --git a/tests/test_pr_review_fix_scheduler.py b/tests/test_pr_review_fix_scheduler.py index d371e1a2b..3b27ccb5c 100644 --- a/tests/test_pr_review_fix_scheduler.py +++ b/tests/test_pr_review_fix_scheduler.py @@ -19,6 +19,7 @@ def make_pr(**overrides): "headRefName": "feature", "headRefOid": "a" * 40, "headRepository": {"nameWithOwner": "owner/repo"}, + "mergeStateStatus": "CLEAN", "reviews": {"nodes": []}, "reviewThreads": {"nodes": []}, } @@ -44,7 +45,12 @@ def test_needs_autofix_uses_current_head_evidence(): reviews={ "nodes": [ {"state": "APPROVED", "author": {"login": "opencode-agent"}, "commit": {"oid": head}}, - {"state": "CHANGES_REQUESTED", "author": {"login": "opencode-agent"}, "commit": {"oid": head}}, + { + "state": "CHANGES_REQUESTED", + "author": {"login": "opencode-agent"}, + "commit": {"oid": head}, + "body": "Actionable source-backed finding with suggested diff.", + }, ] }, reviewThreads={"nodes": [{"id": "thread", "isResolved": False, "isOutdated": False}]}, @@ -56,6 +62,40 @@ def test_needs_autofix_uses_current_head_evidence(): ) +@pytest.mark.parametrize( + ("merge_state", "body"), + [ + ("DIRTY", "Actionable source-backed finding with suggested diff."), + ("CONFLICTING", "Actionable source-backed finding with suggested diff."), + ("CLEAN", "OpenCode could not establish approval sufficiency because the model pool exhausted."), + ("CLEAN", "OpenCode found unresolved human review thread evidence before approval."), + ("CLEAN", "Failed-check evidence reports coverage-evidence failure."), + ], +) +def test_needs_autofix_suppresses_process_only_reviews(merge_state, body): + head = "a" * 40 + pr = make_pr( + headRefOid=head, + mergeStateStatus=merge_state, + reviews={ + "nodes": [ + { + "state": "CHANGES_REQUESTED", + "author": {"login": "opencode-agent"}, + "commit": {"oid": head}, + "body": body, + }, + ] + }, + ) + + assert fix.needs_autofix(pr) == (False, ()) + + +def test_change_request_is_not_autofixable_without_review_evidence(): + assert not fix.change_request_is_autofixable(make_pr()) + + def test_process_queue_dispatches_same_repo_current_head(monkeypatch, capsys): """The queue path dispatches one same-repository autofix.""" pr = make_pr() From cbeeb85e4ce54cb5e439c350a4fa33426dc877c6 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 1 Jul 2026 10:03:37 +0900 Subject: [PATCH 4/5] test: cover non-current autofix review guard --- scripts/ci/pr_review_fix_scheduler.py | 2 + tests/test_pr_review_fix_scheduler.py | 78 +++++++++++++++++++++++++++ 2 files changed, 80 insertions(+) diff --git a/scripts/ci/pr_review_fix_scheduler.py b/scripts/ci/pr_review_fix_scheduler.py index a553ac6f4..590b6f3b3 100755 --- a/scripts/ci/pr_review_fix_scheduler.py +++ b/scripts/ci/pr_review_fix_scheduler.py @@ -101,6 +101,8 @@ def change_request_is_autofixable(pr: dict[str, Any]) -> bool: return False review = latest_current_head_opencode_review(pr) + if review is None: + return False body = str((review or {}).get("body") or "").lower() if any(marker in body for marker in NON_AUTOFIX_CHANGE_REQUEST_MARKERS): return False diff --git a/tests/test_pr_review_fix_scheduler.py b/tests/test_pr_review_fix_scheduler.py index d371e1a2b..c0e6049d2 100644 --- a/tests/test_pr_review_fix_scheduler.py +++ b/tests/test_pr_review_fix_scheduler.py @@ -56,6 +56,84 @@ def test_needs_autofix_uses_current_head_evidence(): ) +def test_change_request_requires_current_head_opencode_review(): + """Autofixable change requests require an OpenCode review on the current head.""" + head = "a" * 40 + stale_head = "b" * 40 + + no_review_pr = make_pr(headRefOid=head, mergeStateStatus="CLEAN") + assert fix.latest_current_head_opencode_review(no_review_pr) is None + assert not fix.change_request_is_autofixable(no_review_pr) + + stale_review_pr = make_pr( + headRefOid=head, + mergeStateStatus="CLEAN", + reviews={ + "nodes": [ + { + "state": "CHANGES_REQUESTED", + "author": {"login": "opencode-agent"}, + "commit": {"oid": stale_head}, + "body": "Actionable source-backed finding with a suggested diff.", + } + ] + }, + ) + assert fix.latest_current_head_opencode_review(stale_review_pr) is None + assert not fix.change_request_is_autofixable(stale_review_pr) + + +@pytest.mark.parametrize("merge_state", ["DIRTY", "CONFLICTING"]) +def test_needs_autofix_blocks_non_clean_merge_states(merge_state): + """Autofix dispatch does not try to repair merge-state blockers.""" + head = "a" * 40 + pr = make_pr( + headRefOid=head, + mergeStateStatus=merge_state, + reviews={ + "nodes": [ + { + "state": "CHANGES_REQUESTED", + "author": {"login": "opencode-agent"}, + "commit": {"oid": head}, + "body": "Actionable source-backed finding with a suggested diff.", + } + ] + }, + ) + + assert fix.needs_autofix(pr) == (False, ()) + + +@pytest.mark.parametrize( + "body", + [ + "OpenCode could not establish approval sufficiency because the model pool exhausted.", + "OpenCode found unresolved human review thread evidence before approval.", + "Failed check evidence shows coverage-evidence failed on the current head.", + ], +) +def test_needs_autofix_blocks_process_only_reviews(body): + """Process-only OpenCode requests do not trigger code autofix dispatch.""" + head = "a" * 40 + pr = make_pr( + headRefOid=head, + mergeStateStatus="CLEAN", + reviews={ + "nodes": [ + { + "state": "CHANGES_REQUESTED", + "author": {"login": "opencode-agent"}, + "commit": {"oid": head}, + "body": body, + } + ] + }, + ) + + assert fix.needs_autofix(pr) == (False, ()) + + def test_process_queue_dispatches_same_repo_current_head(monkeypatch, capsys): """The queue path dispatches one same-repository autofix.""" pr = make_pr() From 5990a1c887fdcd61c03399d31da321e78d4abaf6 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 1 Jul 2026 10:13:20 +0900 Subject: [PATCH 5/5] test: consolidate autofix guard coverage --- tests/test_pr_review_fix_scheduler.py | 53 +-------------------------- 1 file changed, 2 insertions(+), 51 deletions(-) diff --git a/tests/test_pr_review_fix_scheduler.py b/tests/test_pr_review_fix_scheduler.py index f28c2d57a..044f09a47 100644 --- a/tests/test_pr_review_fix_scheduler.py +++ b/tests/test_pr_review_fix_scheduler.py @@ -70,9 +70,11 @@ def test_needs_autofix_uses_current_head_evidence(): ("CLEAN", "OpenCode could not establish approval sufficiency because the model pool exhausted."), ("CLEAN", "OpenCode found unresolved human review thread evidence before approval."), ("CLEAN", "Failed-check evidence reports coverage-evidence failure."), + ("CLEAN", "Failed check evidence shows coverage-evidence failed on the current head."), ], ) def test_needs_autofix_suppresses_process_only_reviews(merge_state, body): + """Process-only or non-clean OpenCode requests do not dispatch autofix.""" head = "a" * 40 pr = make_pr( headRefOid=head, @@ -119,57 +121,6 @@ def test_change_request_requires_current_head_opencode_review(): assert not fix.change_request_is_autofixable(stale_review_pr) -@pytest.mark.parametrize("merge_state", ["DIRTY", "CONFLICTING"]) -def test_needs_autofix_blocks_non_clean_merge_states(merge_state): - """Autofix dispatch does not try to repair merge-state blockers.""" - head = "a" * 40 - pr = make_pr( - headRefOid=head, - mergeStateStatus=merge_state, - reviews={ - "nodes": [ - { - "state": "CHANGES_REQUESTED", - "author": {"login": "opencode-agent"}, - "commit": {"oid": head}, - "body": "Actionable source-backed finding with a suggested diff.", - } - ] - }, - ) - - assert fix.needs_autofix(pr) == (False, ()) - - -@pytest.mark.parametrize( - "body", - [ - "OpenCode could not establish approval sufficiency because the model pool exhausted.", - "OpenCode found unresolved human review thread evidence before approval.", - "Failed check evidence shows coverage-evidence failed on the current head.", - ], -) -def test_needs_autofix_blocks_process_only_reviews(body): - """Process-only OpenCode requests do not trigger code autofix dispatch.""" - head = "a" * 40 - pr = make_pr( - headRefOid=head, - mergeStateStatus="CLEAN", - reviews={ - "nodes": [ - { - "state": "CHANGES_REQUESTED", - "author": {"login": "opencode-agent"}, - "commit": {"oid": head}, - "body": body, - } - ] - }, - ) - - assert fix.needs_autofix(pr) == (False, ()) - - def test_process_queue_dispatches_same_repo_current_head(monkeypatch, capsys): """The queue path dispatches one same-repository autofix.""" pr = make_pr()