From 895b351158a49df151b8053d6cb01f743dc25268 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 06:16:13 +0000 Subject: [PATCH 1/3] fix(scheduler): fail closed on unknown mergeStateStatus in change-request gate _clean_change_request_body() only fails closed when mergeStateStatus is a known-dirty value, so a missing/empty mergeStateStatus (the REST mergeable_state fallback can return null right after a push) was silently treated as CLEAN and let the autofix path proceed on unverified merge state. Match the fail-closed pattern already used by needs_conflict_resolution() and pr_auto_rebase.py's is_clean/is_dirty/is_behind_base. --- scripts/ci/pr_review_fix_scheduler.py | 2 +- tests/test_pr_review_fix_scheduler.py | 30 +++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/scripts/ci/pr_review_fix_scheduler.py b/scripts/ci/pr_review_fix_scheduler.py index 33bd7ca1bb..2268e99624 100755 --- a/scripts/ci/pr_review_fix_scheduler.py +++ b/scripts/ci/pr_review_fix_scheduler.py @@ -155,7 +155,7 @@ def latest_current_head_opencode_review(pr: dict[str, Any]) -> dict[str, Any] | def _clean_change_request_body(pr: dict[str, Any]) -> str | None: """Return normalized exact-head OpenCode review text for a clean PR.""" merge_state = str(pr.get("mergeStateStatus") or "").upper() - if merge_state and merge_state not in {"CLEAN", "HAS_HOOKS"}: + if merge_state not in {"CLEAN", "HAS_HOOKS"}: return None review = latest_current_head_opencode_review(pr) if review is None: diff --git a/tests/test_pr_review_fix_scheduler.py b/tests/test_pr_review_fix_scheduler.py index bd836379dc..92be849021 100644 --- a/tests/test_pr_review_fix_scheduler.py +++ b/tests/test_pr_review_fix_scheduler.py @@ -130,6 +130,36 @@ def test_change_request_requires_current_head_opencode_review(): assert not fix.change_request_is_autofixable(stale_review_pr) +def test_change_request_gates_fail_closed_on_unknown_merge_state(): + """A missing/empty mergeStateStatus must not be treated as CLEAN. + + The GraphQL schema guarantees a non-null mergeStateStatus enum, but the + REST fallback path (used when GraphQL is unavailable) can compute an + empty string when GitHub's REST mergeable_state is still null right + after a push. That must fail closed like every other unrecognized + merge state, not fall through as if the PR were confirmed clean. + """ + head = "a" * 40 + body = "Actionable source-backed finding with suggested diff." + review = { + "state": "CHANGES_REQUESTED", + "author": {"login": "opencode-agent"}, + "commit": {"oid": head}, + "body": body, + } + + for merge_state in ("", "UNKNOWN"): + pr = make_pr(headRefOid=head, mergeStateStatus=merge_state, reviews={"nodes": [review]}) + assert fix._clean_change_request_body(pr) is None, merge_state + assert not fix.change_request_is_autofixable(pr), merge_state + assert not fix.change_request_requires_rca(pr), merge_state + + pr_missing_key = make_pr(headRefOid=head, reviews={"nodes": [review]}) + del pr_missing_key["mergeStateStatus"] + assert fix._clean_change_request_body(pr_missing_key) is None + assert not fix.change_request_is_autofixable(pr_missing_key) + + def test_process_queue_dispatches_same_repo_current_head(monkeypatch, capsys): """The queue path dispatches one same-repository autofix.""" pr = make_pr() From 8b8e24a898acff651cd8006ccee0e48d9f200892 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 7 Sep 2026 13:05:52 +0900 Subject: [PATCH 2/3] test(scheduler): reject unknown merge state in autofix gate --- tests/test_pr_review_fix_scheduler.py | 31 +++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/test_pr_review_fix_scheduler.py b/tests/test_pr_review_fix_scheduler.py index 6b9bd91e0c..bd5a75ea7d 100644 --- a/tests/test_pr_review_fix_scheduler.py +++ b/tests/test_pr_review_fix_scheduler.py @@ -483,6 +483,37 @@ def test_change_request_requires_current_head_opencode_review(): assert not fix.change_request_is_autofixable(stale_review_pr) +def test_change_request_gates_fail_closed_on_unknown_merge_state(): + """Unknown or missing merge state cannot authorize automatic repair.""" + head = "a" * 40 + body = "Actionable source-backed finding with suggested diff." + current_review = { + "state": "CHANGES_REQUESTED", + "author": {"login": "opencode-agent"}, + "commit": {"oid": head}, + "body": body, + } + + for merge_state in ("", "UNKNOWN"): + pull_request = make_pr( + headRefOid=head, + mergeStateStatus=merge_state, + reviews={"nodes": [current_review]}, + ) + assert fix._clean_change_request_body(pull_request) is None + assert not fix.change_request_is_autofixable(pull_request) + assert not fix.change_request_requires_rca(pull_request) + + pull_request = make_pr( + headRefOid=head, + reviews={"nodes": [current_review]}, + ) + del pull_request["mergeStateStatus"] + assert fix._clean_change_request_body(pull_request) is None + assert not fix.change_request_is_autofixable(pull_request) + assert not fix.change_request_requires_rca(pull_request) + + def test_process_queue_dispatches_same_repo_current_head(monkeypatch, capsys): """The queue path dispatches one same-repository autofix.""" pr = make_pr() From ccef01023ed997023a38ac160e2ff37fb9d93a28 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 7 Sep 2026 13:05:57 +0900 Subject: [PATCH 3/3] fix(scheduler): fail closed on unknown merge state --- CHANGELOG.md | 4 +++ .../unknown-merge-state-fail-closed.md | 33 +++++++++++++++++++ docs/product-technical-gap-baseline.md | 6 ++++ scripts/ci/pr_review_fix_scheduler.py | 2 +- 4 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 docs/doctoring/unknown-merge-state-fail-closed.md mode change 100755 => 100644 scripts/ci/pr_review_fix_scheduler.py diff --git a/CHANGELOG.md b/CHANGELOG.md index bf192f6a9e..c5a98a7e65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +### Change-request autofix fails closed on unknown merge state + +- The shared change-request gate now requires `mergeStateStatus` to be exactly `CLEAN` or `HAS_HOOKS`. Missing, empty, and unknown REST fallback values cannot authorize autofix or RCA dispatch. Proposed in ContextualWisdomLab/.github#1492. + ### Failed-check finding names the Strix sandbox instead of the gateway - `opencode-review-dispatch.yml`'s `emit_strix_provider_failure_finding` rendered one fixed finding for every `STRIX_PROVIDER_UNAVAILABLE` line, whose Root cause read "The contextual-orchestrator gateway or its discovered provider pool was unavailable for this run". `#1953` had just given the Strix sandbox bootstrap failure its own second verdict token (`STRIX_SANDBOX_UNAVAILABLE`) precisely because that attribution is wrong for it -- the sandbox container never reaches its Caido proxy, so the run dies before the gateway serves anything -- and this consumer re-applied the wrong attribution one step downstream, into the review findings and the failure census. The emitter now branches on the second token: a sandbox verdict gets a finding that names Strix's sandbox, says the verdict does not name the gateway, and tells the reader not to change gateway or provider configuration on its strength. A `STRIX_PROVIDER_UNAVAILABLE` line without the token keeps its existing text verbatim, so the gateway class has no regression surface. No test covered this finding text at all before (`gateway or its discovered provider pool` matched nothing under `tests/`); `tests/test_opencode_dispatch_strix_sandbox_finding.py` now runs the production emitter from the published run block and pins both directions plus the no-signal case. Refs #1953, #1935. diff --git a/docs/doctoring/unknown-merge-state-fail-closed.md b/docs/doctoring/unknown-merge-state-fail-closed.md new file mode 100644 index 0000000000..6f748a5ef4 --- /dev/null +++ b/docs/doctoring/unknown-merge-state-fail-closed.md @@ -0,0 +1,33 @@ +# Unknown merge state does not authorize change-request automation + +검토 기준일: **2026-09-07** + +## Problem + +GitHub can temporarily report an unresolved mergeability state after a push. +The REST fallback normalizes that condition to an empty value. Treating the +empty value as clean allows automatic change-request handling without positive +mergeability evidence. + +## Decision + +The shared gate accepts only `CLEAN` and `HAS_HOOKS`. Missing, empty, or +unknown values return no clean review body, so neither autofix nor RCA dispatch +is authorized. Known dirty states retain the same behavior. + +## Verification contract + +`test_change_request_gates_fail_closed_on_unknown_merge_state` covers empty, +unknown, and absent values across the normalized body, autofix, and RCA entry +points. Hosted exact-head checks remain mandatory. + +## Status + +**Proposed** in ContextualWisdomLab/.github#1492. Protected `main` remains the +release authority. + +## Reference + +GitHub. (n.d.). *REST API endpoints for pull requests*. GitHub Docs. Retrieved +September 7, 2026, from +https://docs.github.com/en/rest/pulls/pulls diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index 1cc9e20313..f181347f5a 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -7,6 +7,12 @@ 이 문서는 제품·기술·운영 Gap을 현재 문서와 현재 GitHub 상태에 묶어 두는 기준선이다. 새 작업은 먼저 이 문서의 Gap ID를 PR 설명과 테스트 증거에 연결하고, PR의 정확한 exact HEAD·Checks·리뷰를 다시 수집한 뒤 구현한다. 표의 상태는 작성 시점의 관측값이므로, 병합 판단에는 재사용하지 않는다. 이 인벤토리는 스냅샷이며 merge authorization이 아니다. +### 2026-09-07 unknown merge-state fail-closed amendment + +- **Gap:** an empty or absent REST fallback `mergeStateStatus` can pass the change-request gate as if mergeability were proven. +- **Action:** ContextualWisdomLab/.github#1492 requires `CLEAN` or `HAS_HOOKS` explicitly before autofix or RCA classification. +- **Status:** Proposed; exact-head hosted Checks, independent review, ordinary protected integration, and post-merge current-main verification remain required. + ## 1. 근거와 범위 ### 1.1 우선순위가 높은 근거 diff --git a/scripts/ci/pr_review_fix_scheduler.py b/scripts/ci/pr_review_fix_scheduler.py old mode 100755 new mode 100644 index bc2868c5a4..66c0109e05 --- a/scripts/ci/pr_review_fix_scheduler.py +++ b/scripts/ci/pr_review_fix_scheduler.py @@ -203,7 +203,7 @@ def latest_current_head_opencode_review(pr: dict[str, Any]) -> dict[str, Any] | def _clean_change_request_body(pr: dict[str, Any]) -> str | None: """Return normalized exact-head OpenCode review text for a clean PR.""" merge_state = str(pr.get("mergeStateStatus") or "").upper() - if merge_state and merge_state not in {"CLEAN", "HAS_HOOKS"}: + if merge_state not in {"CLEAN", "HAS_HOOKS"}: return None review = latest_current_head_opencode_review(pr) if review is None: