diff --git a/CHANGELOG.d/20260911-zero-diff-close-lineage.md b/CHANGELOG.d/20260911-zero-diff-close-lineage.md new file mode 100644 index 0000000000..4544a26844 --- /dev/null +++ b/CHANGELOG.d/20260911-zero-diff-close-lineage.md @@ -0,0 +1,6 @@ +## Fixed + +- PR review merge scheduler의 zero-diff 자동 종료가 base-to-head 비교만으로 + 유효한 미병합 변경을 닫지 않도록 커밋 lineage와 파일 변경 근거를 다시 + 확인합니다. lineage가 누락·잘림·오류이거나 커밋 단위 변경이 남아 있으면 + fail-closed wait 상태로 보존합니다. diff --git a/docs/pr-review-and-merge-procedure.md b/docs/pr-review-and-merge-procedure.md index e34c3957cb..8f8dc29f3c 100644 --- a/docs/pr-review-and-merge-procedure.md +++ b/docs/pr-review-and-merge-procedure.md @@ -89,6 +89,15 @@ the existing review, or merges until a new exact-head OpenCode approval exists. ## Do-not-merge and DIRTY / CONFLICTING repair +## Zero-diff cleanup requires lineage evidence + +The scheduler does not close a non-draft pull request solely because the fresh +base-to-head comparison reports zero changed files. Before that destructive +cleanup, it reads the complete pull-request commit lineage and each commit's +changed-file list. Missing, truncated, malformed, or non-empty lineage fails +closed to a wait decision. This preserves a valid unmerged delta when a later +forward commit reverted it without a verified successor inheriting the work. + The `update_branch` path is deliberately not used for `DIRTY` or `CONFLICTING` PRs. GitHub cannot synthesize a safe conflict resolution for the author, so the merge scheduler must give the author a repair path instead diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index 1cc9e20313..f382a0ea93 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -2753,6 +2753,19 @@ asserts the literal cron string (currently exactly these two files) in the same that changes the cron value, per this repo's own "contract tests pin workflows AND prose" convention already stated in `CLAUDE.md`. +## Item 42: zero-diff PR cleanup required commit-lineage evidence — 2026-09-11 + +The merge scheduler previously treated a fresh base-to-head comparison with +zero changed files as sufficient authority to close a non-draft pull request. +That predicate is unsafe when a later forward commit removes a still-valid +unmerged delta: the terminal tree is empty, but the PR's commit lineage proves +that the lane carried real work and no verified successor necessarily owns it. +The scheduler now reads the bounded commit list and each commit's changed-file +list immediately before zero-diff cleanup. Missing, truncated, malformed, or +non-empty lineage returns a wait decision and never closes the PR. Existing +stale-run cancellation keeps its narrower live-head API contract and does not +pay this lineage cost outside the destructive zero-diff boundary. + ## Item 4 fresh evidence: gateway 500 after a 649.5s "connecting" phase with `served_model=unknown` — 2026-09-03 **Status:** A live, current instance of item 4's still-open telemetry complaint, distinct from the already-resolved html4tree/900-second caller-repair-deadline case above (that mechanism was removed by PR #1672). Recorded here from a fresh, exact job log. Two distinct defects were found in the one error line below, both root-caused and both with a fix proposed but not yet merged: a caller-owned phase-mislabeling bug (this repository's own `scripts/ci/noema_review_gate.py`, see below) and a gateway-owned attribution gap (`contextual-orchestrator`'s `_invoke` failover loop, relayed to and fixed by the peer session with deep context in that repo, see below). diff --git a/scripts/ci/pr_review_merge_scheduler_core.py b/scripts/ci/pr_review_merge_scheduler_core.py index 4df4dac3de..efb45a62eb 100644 --- a/scripts/ci/pr_review_merge_scheduler_core.py +++ b/scripts/ci/pr_review_merge_scheduler_core.py @@ -3448,7 +3448,7 @@ def cancel_one(run_id: str) -> tuple[str, str | None]: def _fresh_open_pr_for_cancellation(repo: str, number: int) -> dict[str, Any]: - """Return fresh open PR authority, including explicitly identified draft state.""" + """Return fresh open PR authority for stale-run cancellation.""" payload = gh_api_json(f"repos/{repo}/pulls/{number}") if not isinstance(payload, dict) or str(payload.get("state") or "").lower() != "open": raise ValueError(f"PR #{number} in {repo} is not a resolvable open pull request") @@ -3458,6 +3458,32 @@ def _fresh_open_pr_for_cancellation(repo: str, number: int) -> dict[str, Any]: return payload +def _fresh_commit_lineage_for_close(repo: str, number: int) -> list[dict[str, Any]]: + """Return complete per-commit file evidence before a zero-diff close.""" + commits = gh_api_json(f"repos/{repo}/pulls/{number}/commits?per_page=100") + if not isinstance(commits, list) or len(commits) >= 100: + raise ValueError(f"PR #{number} in {repo} has incomplete commit lineage") + lineage: list[dict[str, Any]] = [] + for commit in commits: + if not isinstance(commit, dict): + raise ValueError(f"PR #{number} in {repo} has malformed commit lineage") + sha = str(commit.get("sha") or "") + validate_git_sha(sha) + detail = gh_api_json(f"repos/{repo}/commits/{sha}") + if not isinstance(detail, dict) or not isinstance(detail.get("files"), list): + raise ValueError(f"PR #{number} in {repo} has incomplete commit file lineage") + lineage.append({"sha": sha, "files": detail["files"]}) + return lineage + + +def _zero_diff_close_authorized(fresh_pr: dict[str, Any]) -> bool: + """Allow empty-PR cleanup only when complete commit lineage is also empty.""" + lineage = fresh_pr.get("commit_lineage") + if not isinstance(lineage, list): + return False + return all(isinstance(commit, dict) and not commit.get("files") for commit in lineage) + + def _fresh_active_run_for_cancellation(run_repo: str, run_id: str) -> dict[str, Any]: """Return fresh active workflow-run evidence immediately before cancellation.""" payload = gh_api_json(f"repos/{run_repo}/actions/runs/{run_id}") @@ -4227,6 +4253,21 @@ def inspect_pr( return Decision(number, "wait", "empty PR candidate metadata is incomplete") if fresh_pr["draft"] or fresh_changed_files != 0: return Decision(number, "skip", "empty PR candidate no longer eligible") + if "commit_lineage" not in fresh_pr: + try: + fresh_pr["commit_lineage"] = _fresh_commit_lineage_for_close(repo, number) + except (RuntimeError, ValueError, json.JSONDecodeError): + return Decision( + number, + "wait", + "empty PR candidate has incomplete commit lineage", + ) + if not _zero_diff_close_authorized(fresh_pr): + return Decision( + number, + "wait", + "empty PR candidate has non-empty or incomplete commit lineage", + ) if not dry_run: try: run( diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index ba47b89c8d..c42d1cbe7d 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -190,6 +190,7 @@ def test_inspect_pr_closes_only_fresh_non_draft_empty_pull_request(monkeypatch): "draft": False, "changed_files": 0, "head": {"sha": head_sha}, + "commit_lineage": [], }, ) monkeypatch.setattr(sched, "run", lambda args: calls.append(args) or "") @@ -218,6 +219,7 @@ def test_inspect_pr_classifies_empty_pull_request_without_closing_in_dry_run(mon "draft": False, "changed_files": 0, "head": {"sha": head_sha}, + "commit_lineage": [], }, ) monkeypatch.setattr(sched, "run", lambda args: calls.append(args) or "") @@ -249,6 +251,7 @@ def fake_run(args): "draft": False, "changed_files": 0, "head": {"sha": head_sha}, + "commit_lineage": [], }, ) monkeypatch.setattr(sched, "run", fake_run) @@ -264,6 +267,33 @@ def fake_run(args): assert calls == [["gh", "pr", "close", "1", "--repo", "owner/repo"]] +def test_inspect_pr_preserves_zero_diff_candidate_with_disappeared_commit_delta(monkeypatch): + """A reverted valid commit must not be mistaken for an intentionally empty PR.""" + head_sha = "a" * 40 + candidate = make_pr(headRefOid=head_sha, files={"totalCount": 0, "nodes": []}) + calls = [] + monkeypatch.setattr( + sched, + "_fresh_open_pr_for_cancellation", + lambda _repo, _number: { + "draft": False, + "changed_files": 0, + "head": {"sha": head_sha}, + "commit_lineage": [{"sha": "b" * 40, "files": [{"filename": "tests/regression.py"}]}], + }, + ) + monkeypatch.setattr(sched, "run", lambda args: calls.append(args) or "") + monkeypatch.setattr( + sched, "recover_current_head_startup_failures", lambda repo, pr, *, dry_run: [] + ) + + decision = inspect(candidate, dry_run=False) + + assert decision.action == "wait" + assert "commit lineage" in decision.reason + assert calls == [] + + @pytest.mark.parametrize( "fresh", ( diff --git a/tests/test_zero_diff_close_lineage_contract.py b/tests/test_zero_diff_close_lineage_contract.py new file mode 100644 index 0000000000..44af98425a --- /dev/null +++ b/tests/test_zero_diff_close_lineage_contract.py @@ -0,0 +1,36 @@ +"""Contract tests for lineage-aware zero-diff pull-request cleanup.""" + +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[1] +CORE = ROOT / "scripts" / "ci" / "pr_review_merge_scheduler_core.py" +PROCEDURE = ROOT / "docs" / "pr-review-and-merge-procedure.md" +BASELINE = ROOT / "docs" / "product-technical-gap-baseline.md" + + +def test_zero_diff_cleanup_requires_commit_lineage_and_fails_closed() -> None: + """A base-to-head empty diff must not be the scheduler's sole close authority.""" + source = CORE.read_text(encoding="utf-8") + + assert "_fresh_commit_lineage_for_close" in source + assert "incomplete commit lineage" in source + assert "_zero_diff_close_authorized" in source + assert '"wait"' in source[source.index("_zero_diff_close_authorized") :] + + +def test_governance_procedure_names_lineage_as_zero_diff_close_evidence() -> None: + """Operator guidance must preserve the lineage guard against reverted work.""" + procedure = PROCEDURE.read_text(encoding="utf-8") + + assert "Zero-diff" in procedure + assert "commit lineage" in procedure + assert "fails\nclosed" in procedure + + +def test_product_gap_baseline_records_the_lineage_guard() -> None: + """The product baseline must retain the incident and its bounded repair.""" + baseline = BASELINE.read_text(encoding="utf-8") + + assert "Item 42: zero-diff PR cleanup required commit-lineage evidence" in baseline + assert "non-empty lineage returns a wait decision" in baseline