From 8cec0a2c345860bfb74efd357dc6a172bc24381e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 14 Aug 2026 09:29:57 +0900 Subject: [PATCH 01/22] fix(review): fail closed when required check is not a verdict The required opencode-review job only echoed success, so PRs such as contextual-orchestrator#176 looked reviewed with an empty Reviews tab. Fail closed unless opencode-agent posted APPROVED or CHANGES_REQUESTED on the current head, and stop Noema from exiting 0 when that approval is missing. --- .github/workflows/opencode-review.yml | 43 +++++- CHANGELOG.md | 2 + .../required-review-check-is-not-a-verdict.md | 50 ++++++ scripts/ci/noema_review_gate.py | 8 +- scripts/ci/opencode_dispatch_status.py | 75 ++++++++- scripts/ci/test_strix_quick_gate.sh | 1 + ...st_materialize_base_python_requirements.py | 10 ++ tests/test_noema_review_gate.py | 6 +- tests/test_opencode_agent_contract.py | 3 + tests/test_opencode_required_verdict_gate.py | 145 ++++++++++++++++++ 10 files changed, 333 insertions(+), 10 deletions(-) create mode 100644 docs/doctoring/required-review-check-is-not-a-verdict.md create mode 100644 tests/test_opencode_required_verdict_gate.py diff --git a/.github/workflows/opencode-review.yml b/.github/workflows/opencode-review.yml index 7f1ad6d00..32429d0c8 100644 --- a/.github/workflows/opencode-review.yml +++ b/.github/workflows/opencode-review.yml @@ -20,6 +20,7 @@ concurrency: permissions: contents: read + pull-requests: read jobs: required-workflow-bootstrap: @@ -53,7 +54,43 @@ jobs: name: opencode-review needs: [coverage-evidence] runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: read steps: - - run: >- - echo "Review approval remains a separate current-head PR review - requirement produced by the authenticated dispatch workflow." + - name: Fail closed without a current-head OpenCode verdict + env: + GH_TOKEN: ${{ github.token }} + TARGET_REPOSITORY: ${{ github.event.pull_request.base.repo.full_name || github.repository }} + PR_NUMBER: ${{ github.event.pull_request.number }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + set -euo pipefail + if [ "${{ github.event.action }}" = "closed" ]; then + echo "PR closed; a current-head OpenCode verdict is not required." + exit 0 + fi + if [ -z "${PR_NUMBER:-}" ] || [ -z "${HEAD_SHA:-}" ]; then + echo "::error::Missing PR number or head SHA; cannot verify a current-head OpenCode verdict." + exit 1 + fi + reviews="$(gh api --paginate "repos/${TARGET_REPOSITORY}/pulls/${PR_NUMBER}/reviews")" + verdict="$(printf '%s\n' "$reviews" | jq -s --arg sha "$HEAD_SHA" ' + (add // []) + | [ + .[] + | select( + (.user.login // "" | ascii_downcase) as $user + | $user == "opencode-agent" or $user == "opencode-agent[bot]" + ) + | select((.commit_id // "" | ascii_downcase) == ($sha | ascii_downcase)) + | .state + ] + | map(select(. == "APPROVED" or . == "CHANGES_REQUESTED")) + | first // empty + ')" + if [ -z "$verdict" ]; then + echo "::error::No APPROVED or CHANGES_REQUESTED from opencode-agent on the current head. This required check is not a review and must not succeed until the authenticated dispatch posts a current-head verdict." + exit 1 + fi + echo "Current-head OpenCode verdict: ${verdict}." diff --git a/CHANGELOG.md b/CHANGELOG.md index bf30091dd..c55036131 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ Semantic Versioning where the repository publishes a release. ### Fixed +- The required `opencode-review` check now fails closed unless `opencode-agent` already posted `APPROVED` or `CHANGES_REQUESTED` on the current head, so a stub success can no longer look like a review (ContextualWisdomLab/contextual-orchestrator#176). +- Noema no longer exits 0 when the current head has no primary OpenCode approval; that skip was the green `noema-review` check with an empty Reviews tab. - Bounded the Strix quality self-test's deterministic timeout fixtures to 3-second process and 5-second fake-sleep budgets so exact-head policy evidence completes inside the existing job limit without changing production Strix scanner timeouts, providers, credentials, or review semantics. - Allowed commas and ASCII parentheses in the bounded Strix changed-file path policy so legal tracked Packrat fixtures can receive exact-head security analysis, while rejecting raw `..` components before normalization and keeping controls, backslashes, whitespace ambiguity, and shell punctuation fail-closed. - Bound each review-agent invocation key to the wrapper's complete canonical payload, including the base branch and requesting actor; altered fields with a valid-format key now fail before durable-leader election or forwarding, and wrapper write permission is job-scoped. diff --git a/docs/doctoring/required-review-check-is-not-a-verdict.md b/docs/doctoring/required-review-check-is-not-a-verdict.md new file mode 100644 index 000000000..9a5b17647 --- /dev/null +++ b/docs/doctoring/required-review-check-is-not-a-verdict.md @@ -0,0 +1,50 @@ +# Required OpenCode/Noema checks are not reviews + +검토 기준일: **2026-08-14** + +## Incident + +On ContextualWisdomLab/contextual-orchestrator#176 the required +`opencode-review` and `noema-review` checks were green, but the Reviews +API had no APPROVE or REQUEST_CHANGES. Authors treated the check name as +a review verdict (GitHub, n.d.-a). That is weaker than the modern-review +expectation that a review is an explicit, current-head judgment +(Bacchelli & Bird, 2013). + +## Decision + +The required `opencode-review` job on +`.github/workflows/opencode-review.yml` never runs the model. Privileged +review stays in `opencode-review-dispatch.yml`. The required job now +reads current-head reviews with `pull-requests: read` and **fails +closed** unless `opencode-agent` / `opencode-agent[bot]` already posted +`APPROVED` or `CHANGES_REQUESTED` on that SHA. A COMMENTED review, a +review on an old SHA, or no review at all cannot make the check green. + +`scripts/ci/noema_review_gate.py` no longer returns 0 when the current +head has no primary OpenCode approval. That skip was exit 0, so the +required `noema-review` check looked like a successful review. + +Human `repository_dispatch` as `seonghobae` remains rejected; only +`github-actions[bot]` may start the privileged dispatch. After a real +verdict is posted, re-run the required `opencode-review` job so the +fail-closed check can observe it. + +## Verification contract + +- `tests/test_opencode_required_verdict_gate.py` pins + `current_head_opencode_verdict` and `decide_required_verdict_check`. +- `tests/test_noema_review_gate.py` requires exit 1 when there is no + primary OpenCode approval. +- `tests/test_opencode_agent_contract.py` pins the required workflow + fail-closed error string. + +## References (APA 7th) + +Bacchelli, A., & Bird, C. (2013). Expectations, outcomes, and challenges of +modern code review. In *Proceedings of the 35th International Conference on +Software Engineering* (pp. 712–721). IEEE. +https://doi.org/10.1109/ICSE.2013.6606617 + +GitHub. (n.d.-a). *About status checks*. GitHub Docs. Retrieved +August 14, 2026, from https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/collaborating-on-repositories-with-code-quality-features/about-status-checks diff --git a/scripts/ci/noema_review_gate.py b/scripts/ci/noema_review_gate.py index 9317860e4..d3d5dac95 100644 --- a/scripts/ci/noema_review_gate.py +++ b/scripts/ci/noema_review_gate.py @@ -598,8 +598,12 @@ def inspect_and_review(repo: str, number: int) -> int: print("Current head already has a Noema review; nothing to do.") return 0 if not current_primary_approval(pr): - print("Current head does not have a primary OpenCode approval; Noema review skipped.") - return 0 + print( + "Current head does not have a primary OpenCode approval; " + "Noema cannot skip as success because that made the required " + "check look like a review." + ) + return 1 if has_current_changes_requested(pr): print("Current head has requested changes; Noema review skipped.") return 0 diff --git a/scripts/ci/opencode_dispatch_status.py b/scripts/ci/opencode_dispatch_status.py index 9109a0248..c806c0f3e 100644 --- a/scripts/ci/opencode_dispatch_status.py +++ b/scripts/ci/opencode_dispatch_status.py @@ -20,6 +20,58 @@ ) +OPENCODE_VERDICT_STATES = frozenset({"APPROVED", "CHANGES_REQUESTED"}) +MISSING_VERDICT_MESSAGE = ( + "No APPROVED or CHANGES_REQUESTED from opencode-agent on the current head. " + "This required check is not a review and must not succeed until the " + "authenticated dispatch posts a current-head verdict." +) + + +def current_head_opencode_verdict( + reviews: Sequence[dict[str, Any]], head_sha: str +) -> str | None: + """Return the latest current-head OpenCode APPROVED or CHANGES_REQUESTED state.""" + expected = (head_sha or "").lower() + if not expected: + return None + for review in reversed(reviews): + author = str((review.get("user") or {}).get("login") or "").casefold() + if author not in OPENCODE_APP_APPROVAL_AUTHORS: + continue + if str(review.get("commit_id") or "").lower() != expected: + continue + state = str(review.get("state") or "").upper() + if state in OPENCODE_VERDICT_STATES: + return state + return None + + +def decide_required_verdict_check( + *, + expected_head: str, + pull_request: dict[str, Any], + reviews: Sequence[dict[str, Any]], +) -> dict[str, str]: + """Fail closed unless OpenCode already published a current-head verdict.""" + live_head = str((pull_request.get("head") or {}).get("sha") or "") + if not expected_head or live_head.lower() != expected_head.lower(): + return { + "state": "failure", + "description": ( + "OpenCode required-check target is stale or the live PR head " + "is unavailable." + ), + } + verdict = current_head_opencode_verdict(reviews, expected_head) + if verdict is None: + return {"state": "failure", "description": MISSING_VERDICT_MESSAGE} + return { + "state": "success", + "description": f"Current-head OpenCode verdict: {verdict}.", + } + + def _has_current_approval(reviews: Sequence[dict[str, Any]], head_sha: str) -> bool: """Return whether the latest OpenCode decision is a verified approval.""" for review in reversed(reviews): @@ -67,10 +119,15 @@ def decide_status( def parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace: - """Parse commit-status evidence inputs.""" + """Parse commit-status or required-verdict evidence inputs.""" parser = argparse.ArgumentParser(description=__doc__) - parser.add_argument("--model-outcome", required=True) - parser.add_argument("--coverage-result", required=True) + parser.add_argument( + "--mode", + choices=("dispatch-status", "required-verdict"), + default="dispatch-status", + ) + parser.add_argument("--model-outcome") + parser.add_argument("--coverage-result") parser.add_argument("--expected-head", required=True) parser.add_argument("--pull-request-file", required=True, type=Path) parser.add_argument("--reviews-file", required=True, type=Path) @@ -78,12 +135,22 @@ def parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace: def main(argv: Sequence[str] | None = None) -> int: - """Print one JSON commit-status decision.""" + """Print one JSON decision and exit 1 when the required verdict is missing.""" args = parse_args(argv) pull_request = json.loads(args.pull_request_file.read_text(encoding="utf-8")) reviews = json.loads(args.reviews_file.read_text(encoding="utf-8")) if not isinstance(pull_request, dict) or not isinstance(reviews, list): raise SystemExit("pull request evidence must be an object and reviews evidence an array") + if args.mode == "required-verdict": + decision = decide_required_verdict_check( + expected_head=args.expected_head, + pull_request=pull_request, + reviews=reviews, + ) + print(json.dumps(decision, separators=(",", ":"))) + return 0 if decision["state"] == "success" else 1 + if not args.model_outcome or not args.coverage_result: + raise SystemExit("--model-outcome and --coverage-result are required") print( json.dumps( decide_status( diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index 7343c06ac..ac6f44bcd 100755 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -497,6 +497,7 @@ assert_opencode_review_uses_codegraph_and_gpt5_fallback() { assert_file_contains "$bootstrap_file" "coverage-evidence:" "opencode required workflow preserves the stable coverage-evidence branch-protection context" assert_file_contains "$bootstrap_file" "name: opencode-review" "opencode required workflow preserves the stable opencode-review branch-protection context" assert_file_contains "$bootstrap_file" "authenticated default-branch OpenCode review dispatch" "opencode required workflow delegates real review execution to the protected dispatch path" + assert_file_contains "$bootstrap_file" "This required check is not a review" "opencode required workflow fails closed without a current-head OpenCode verdict" assert_file_not_contains "$bootstrap_file" "repository_dispatch:" "opencode required workflow does not mix privileged dispatch execution with pull_request_target" assert_file_not_contains "$bootstrap_file" "actions/checkout" "opencode required workflow never checks out pull-request content" assert_file_not_contains "$bootstrap_file" '${{ secrets.' "opencode required workflow never binds repository secrets" diff --git a/tests/test_materialize_base_python_requirements.py b/tests/test_materialize_base_python_requirements.py index 8a383f0c2..1ab36445c 100644 --- a/tests/test_materialize_base_python_requirements.py +++ b/tests/test_materialize_base_python_requirements.py @@ -14,6 +14,13 @@ from tests.conftest import FakeHttpResponse +def _simulate_linux_x86_64_runner(monkeypatch: pytest.MonkeyPatch) -> None: + """Let installer verification tests run on a non-Linux developer host.""" + monkeypatch.setattr(materializer.sys, "platform", "linux") + monkeypatch.setattr(materializer.platform, "machine", lambda: "x86_64") + materializer._install_trusted_uv.cache_clear() + + def git(repo: Path, *args: str) -> str: """Run git in a temporary fixture repository.""" return subprocess.run( @@ -644,6 +651,7 @@ def test_install_trusted_uv_verifies_version_and_caches_path( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: """The installer writes one executable, verifies its version, and caches it.""" + _simulate_linux_x86_64_runner(monkeypatch) tool_dir = tmp_path / "uv" monkeypatch.setattr( materializer.tempfile, @@ -690,6 +698,7 @@ def test_install_trusted_uv_rejects_version_process_failures( failure: OSError | subprocess.TimeoutExpired, ) -> None: """A missing or hung downloaded executable is removed and rejected.""" + _simulate_linux_x86_64_runner(monkeypatch) tool_dir = tmp_path / "uv" monkeypatch.setattr( materializer.tempfile, @@ -721,6 +730,7 @@ def test_install_trusted_uv_rejects_wrong_version_or_exit_status( completed: subprocess.CompletedProcess[bytes], ) -> None: """Unexpected version output or a nonzero status cannot satisfy the pin.""" + _simulate_linux_x86_64_runner(monkeypatch) tool_dir = tmp_path / f"uv-{completed.returncode}-{len(completed.stdout)}" monkeypatch.setattr( materializer.tempfile, diff --git a/tests/test_noema_review_gate.py b/tests/test_noema_review_gate.py index 408bb95b9..682717986 100644 --- a/tests/test_noema_review_gate.py +++ b/tests/test_noema_review_gate.py @@ -526,8 +526,12 @@ def test_inspect_and_review_skip_paths(monkeypatch): assert noema.inspect_and_review("owner/repo", 7) == 0 assert calls + calls.clear() + monkeypatch.setattr(noema, "fetch_pr", lambda repo, number: make_pr()) + assert noema.inspect_and_review("owner/repo", 7) == 1 + assert calls == [] + cases = [ - (make_pr(), "noema"), (make_pr(isDraft=True), "noema"), (make_pr(reviews={"nodes": [review(login="noema", body="")]}), "noema"), (make_pr(reviews={"nodes": [review("CHANGES_REQUESTED"), review(body=marker_body)]}), "noema"), diff --git a/tests/test_opencode_agent_contract.py b/tests/test_opencode_agent_contract.py index daeaa37a2..472b5e112 100644 --- a/tests/test_opencode_agent_contract.py +++ b/tests/test_opencode_agent_contract.py @@ -2178,6 +2178,9 @@ def test_opencode_privileged_review_security_boundaries_are_fail_closed(): assert " opencode-review-target:\n" in bootstrap assert " name: opencode-review\n" in bootstrap assert "authenticated default-branch OpenCode review dispatch" in bootstrap + assert "This required check is not a review" in bootstrap + assert "No APPROVED or CHANGES_REQUESTED from opencode-agent on the current head" in bootstrap + assert "pull-requests: read" in bootstrap assert workflow.count("ref: ${{ steps.trusted_source.outputs.ref }}") == 1 assert "TRUSTED_SOURCE_REF: ${{ steps.trusted_source.outputs.ref }}" in workflow assert "ref: ${{ github.workflow_sha }}" not in workflow diff --git a/tests/test_opencode_required_verdict_gate.py b/tests/test_opencode_required_verdict_gate.py new file mode 100644 index 000000000..7379dea0d --- /dev/null +++ b/tests/test_opencode_required_verdict_gate.py @@ -0,0 +1,145 @@ +"""Fail-closed required OpenCode check when no current-head verdict exists.""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from scripts.ci import opencode_dispatch_status as dispatch_status + + +def _review(*, login: str, state: str, commit_id: str) -> dict[str, object]: + """Return one GitHub Reviews API object.""" + return {"user": {"login": login}, "state": state, "commit_id": commit_id} + + +def test_current_head_opencode_verdict_reads_latest_matching_state() -> None: + head = "a" * 40 + assert ( + dispatch_status.current_head_opencode_verdict( + [ + _review(login="opencode-agent[bot]", state="APPROVED", commit_id=head), + _review( + login="opencode-agent[bot]", + state="CHANGES_REQUESTED", + commit_id=head, + ), + ], + head, + ) + == "CHANGES_REQUESTED" + ) + assert ( + dispatch_status.current_head_opencode_verdict( + [_review(login="opencode-agent", state="APPROVED", commit_id=head)], + head, + ) + == "APPROVED" + ) + + +def test_current_head_opencode_verdict_ignores_other_actors_and_heads() -> None: + head = "a" * 40 + assert ( + dispatch_status.current_head_opencode_verdict( + [ + _review(login="coderabbitai[bot]", state="APPROVED", commit_id=head), + _review(login="opencode-agent[bot]", state="APPROVED", commit_id="b" * 40), + _review(login="opencode-agent[bot]", state="COMMENTED", commit_id=head), + ], + head, + ) + is None + ) + assert dispatch_status.current_head_opencode_verdict([], "") is None + + +def test_decide_required_verdict_check_fails_closed_without_verdict() -> None: + head = "a" * 40 + decision = dispatch_status.decide_required_verdict_check( + expected_head=head, + pull_request={"head": {"sha": head}}, + reviews=[], + ) + assert decision["state"] == "failure" + assert "This required check is not a review" in decision["description"] + stale = dispatch_status.decide_required_verdict_check( + expected_head=head, + pull_request={"head": {"sha": "c" * 40}}, + reviews=[_review(login="opencode-agent[bot]", state="APPROVED", commit_id=head)], + ) + assert stale["state"] == "failure" + approved = dispatch_status.decide_required_verdict_check( + expected_head=head, + pull_request={"head": {"sha": head}}, + reviews=[_review(login="opencode-agent[bot]", state="APPROVED", commit_id=head)], + ) + assert approved == { + "state": "success", + "description": "Current-head OpenCode verdict: APPROVED.", + } + + +def test_required_verdict_cli_exits_one_without_verdict(tmp_path: Path) -> None: + head = "a" * 40 + pr_file = tmp_path / "pr.json" + reviews_file = tmp_path / "reviews.json" + pr_file.write_text(json.dumps({"head": {"sha": head}}), encoding="utf-8") + reviews_file.write_text("[]", encoding="utf-8") + assert ( + dispatch_status.main( + [ + "--mode", + "required-verdict", + "--expected-head", + head, + "--pull-request-file", + str(pr_file), + "--reviews-file", + str(reviews_file), + ] + ) + == 1 + ) + reviews_file.write_text( + json.dumps( + [_review(login="opencode-agent[bot]", state="CHANGES_REQUESTED", commit_id=head)] + ), + encoding="utf-8", + ) + assert ( + dispatch_status.main( + [ + "--mode", + "required-verdict", + "--expected-head", + head, + "--pull-request-file", + str(pr_file), + "--reviews-file", + str(reviews_file), + ] + ) + == 0 + ) + + +def test_dispatch_status_cli_still_requires_model_and_coverage(tmp_path: Path) -> None: + head = "a" * 40 + pr_file = tmp_path / "pr.json" + reviews_file = tmp_path / "reviews.json" + pr_file.write_text(json.dumps({"head": {"sha": head}}), encoding="utf-8") + reviews_file.write_text("[]", encoding="utf-8") + with pytest.raises(SystemExit, match="--model-outcome"): + dispatch_status.main( + [ + "--expected-head", + head, + "--pull-request-file", + str(pr_file), + "--reviews-file", + str(reviews_file), + ] + ) From 24d144c7de91d73d82385986fbb14cd068a80dea Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 14 Aug 2026 11:38:45 +0900 Subject: [PATCH 02/22] fix(noema): confine CodeGraph context path to the workspace Strix on NVIDIA NIM reported path traversal in load_codegraph_context because NOEMA_CODEGRAPH_CONTEXT_PATH was opened without a workspace root. Reject .. components and realpath escapes, keep missing in-tree files on the existing unavailable path, and pin the regression. --- CHANGELOG.md | 1 + scripts/ci/noema_review_gate.py | 26 +++++++++++++++++++- tests/test_noema_review_gate.py | 43 +++++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c55036131..80893c3d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ Semantic Versioning where the repository publishes a release. - The required `opencode-review` check now fails closed unless `opencode-agent` already posted `APPROVED` or `CHANGES_REQUESTED` on the current head, so a stub success can no longer look like a review (ContextualWisdomLab/contextual-orchestrator#176). - Noema no longer exits 0 when the current head has no primary OpenCode approval; that skip was the green `noema-review` check with an empty Reviews tab. +- `load_codegraph_context` now confines `NOEMA_CODEGRAPH_CONTEXT_PATH` to `GITHUB_WORKSPACE` (or cwd) with `..` rejection and realpath checks, so a Strix path-traversal report on that helper cannot read files outside the review workspace. - Bounded the Strix quality self-test's deterministic timeout fixtures to 3-second process and 5-second fake-sleep budgets so exact-head policy evidence completes inside the existing job limit without changing production Strix scanner timeouts, providers, credentials, or review semantics. - Allowed commas and ASCII parentheses in the bounded Strix changed-file path policy so legal tracked Packrat fixtures can receive exact-head security analysis, while rejecting raw `..` components before normalization and keeping controls, backslashes, whitespace ambiguity, and shell punctuation fail-closed. - Bound each review-agent invocation key to the wrapper's complete canonical payload, including the base branch and requesting actor; altered fields with a valid-format key now fail before durable-leader election or forwarding, and wrapper write permission is job-scoped. diff --git a/scripts/ci/noema_review_gate.py b/scripts/ci/noema_review_gate.py index d3d5dac95..9d72cff48 100644 --- a/scripts/ci/noema_review_gate.py +++ b/scripts/ci/noema_review_gate.py @@ -16,6 +16,7 @@ import urllib.parse import urllib.request from collections.abc import Sequence +from pathlib import Path from typing import Any @@ -380,13 +381,36 @@ def review_thread_context(pr: dict[str, Any]) -> str: return "\n".join(lines) +def codegraph_context_root() -> Path: + """Return the workspace root that may contain CodeGraph context files.""" + raw = os.environ.get("GITHUB_WORKSPACE", "").strip() or os.getcwd() + return Path(raw).resolve() + + +def confined_codegraph_context_path(path: str, root: Path) -> Path | None: + """Return the resolved path when it cannot escape the workspace root.""" + candidate = Path(path) + if ".." in candidate.parts: + return None + try: + resolved = candidate.resolve() + except OSError: + return None + if not resolved.is_relative_to(root): + return None + return resolved + + def load_codegraph_context() -> str: """Load optional precomputed CodeGraph context for structural review evidence.""" path = os.environ.get("NOEMA_CODEGRAPH_CONTEXT_PATH", "").strip() if not path: return "" + confined = confined_codegraph_context_path(path, codegraph_context_root()) + if confined is None: + return "CodeGraph context unavailable: path escapes the workspace." try: - with open(path, encoding="utf-8") as handle: + with confined.open(encoding="utf-8") as handle: return truncate_text(handle.read(), MAX_REVIEW_CONTEXT_CHARS) except OSError as exc: return f"CodeGraph context unavailable: {exc}" diff --git a/tests/test_noema_review_gate.py b/tests/test_noema_review_gate.py index 682717986..2b2a18ed1 100644 --- a/tests/test_noema_review_gate.py +++ b/tests/test_noema_review_gate.py @@ -258,6 +258,7 @@ def fake_run(args, stdin=None): monkeypatch.setattr(noema, "run", fake_run) codegraph_path = tmp_path / "codegraph.md" codegraph_path.write_text("call graph: src/a.py -> tests", encoding="utf-8") + monkeypatch.setenv("GITHUB_WORKSPACE", str(tmp_path)) monkeypatch.setenv("NOEMA_CODEGRAPH_CONTEXT_PATH", str(codegraph_path)) pr = make_pr( headRefOid="head sha", @@ -294,6 +295,7 @@ def fake_run(args, stdin=None): def test_review_context_reports_omitted_files_and_missing_codegraph(monkeypatch, tmp_path): + monkeypatch.setenv("GITHUB_WORKSPACE", str(tmp_path)) monkeypatch.delenv("NOEMA_CODEGRAPH_CONTEXT_PATH", raising=False) assert noema.load_codegraph_context() == "" @@ -309,6 +311,47 @@ def test_review_context_reports_omitted_files_and_missing_codegraph(monkeypatch, assert "1 changed files omitted from context budget" in context +def test_load_codegraph_context_rejects_workspace_escape(monkeypatch, tmp_path): + """Traversal, absolute, and symlink paths outside the workspace are rejected.""" + monkeypatch.setenv("GITHUB_WORKSPACE", str(tmp_path)) + outside = tmp_path.parent / "passwd-shape" + outside.write_text("root:x:0:0:root:/root:/bin/sh\n", encoding="utf-8") + monkeypatch.setenv("NOEMA_CODEGRAPH_CONTEXT_PATH", str(outside)) + assert noema.load_codegraph_context() == ( + "CodeGraph context unavailable: path escapes the workspace." + ) + + monkeypatch.setenv( + "NOEMA_CODEGRAPH_CONTEXT_PATH", + str(tmp_path / "nested" / ".." / ".." / outside.name), + ) + assert "path escapes the workspace" in noema.load_codegraph_context() + + link = tmp_path / "escape.md" + link.symlink_to(outside) + monkeypatch.setenv("NOEMA_CODEGRAPH_CONTEXT_PATH", str(link)) + assert "path escapes the workspace" in noema.load_codegraph_context() + + +def test_codegraph_context_root_and_resolve_failure(monkeypatch, tmp_path): + """Workspace root prefers GITHUB_WORKSPACE and resolve errors stay closed.""" + monkeypatch.setenv("GITHUB_WORKSPACE", str(tmp_path)) + assert noema.codegraph_context_root() == tmp_path.resolve() + monkeypatch.delenv("GITHUB_WORKSPACE", raising=False) + monkeypatch.chdir(tmp_path) + assert noema.codegraph_context_root() == tmp_path.resolve() + + class FailingPath(type(tmp_path)): + """Path stand-in whose resolve always fails.""" + + def resolve(self, *args, **kwargs): + """Raise OSError to cover the confinement resolve failure.""" + raise OSError("resolve failed") + + monkeypatch.setattr(noema, "Path", FailingPath) + assert noema.confined_codegraph_context_path("graph.md", tmp_path.resolve()) is None + + class FakeResponse: """Small context-manager response for urllib monkeypatches.""" From e9fc895ed369e23b6cf5e9e76f5c24801823a323 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 14 Aug 2026 14:00:08 +0900 Subject: [PATCH 03/22] test(review): prove draft PRs skip central OpenCode dispatch --- .../repair-draft-opencode-dispatch.yml | 457 ++++++++++++++++++ 1 file changed, 457 insertions(+) create mode 100644 .github/workflows/repair-draft-opencode-dispatch.yml diff --git a/.github/workflows/repair-draft-opencode-dispatch.yml b/.github/workflows/repair-draft-opencode-dispatch.yml new file mode 100644 index 000000000..c03145d13 --- /dev/null +++ b/.github/workflows/repair-draft-opencode-dispatch.yml @@ -0,0 +1,457 @@ +name: One-shot draft OpenCode dispatch repair + +on: + push: + branches: + - fix/required-review-fail-closed-without-verdict + +permissions: + contents: write + +concurrency: + group: one-shot-draft-opencode-dispatch-repair + cancel-in-progress: false + +jobs: + repair: + name: Repair draft review dispatch + runs-on: ubuntu-latest + timeout-minutes: 30 + steps: + - name: Check out repair branch + uses: actions/checkout@de0fac2e4500dabe0009d6fbf3f0a40c1e168d1e + with: + ref: fix/required-review-fail-closed-without-verdict + fetch-depth: 0 + + - name: Install test tooling + run: | + set -euo pipefail + python -m pip install --disable-pip-version-check \ + 'pytest>=8.0.0' \ + 'pytest-cov>=7.1.0' \ + 'coverage[toml]>=7.8.0' \ + 'interrogate>=1.7.0' + + - name: Add failing draft-review contracts + run: | + set -euo pipefail + python - <<'PY' + from pathlib import Path + + path = Path("tests/test_pr_review_merge_scheduler.py") + text = path.read_text(encoding="utf-8") + old = '''def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): + assert inspect(make_pr(isDraft=True)).action == "skip" + ''' + new = '''def test_draft_pr_review_path_never_mutates_branch_or_merge_state(monkeypatch): + mutations = [] + dispatches = [] + for name in ("update_branch", "enable_auto_merge", "merge_pr", "disable_auto_merge"): + monkeypatch.setattr( + sched, + name, + lambda *args, _name=name, **kwargs: mutations.append(_name), + ) + monkeypatch.setattr( + sched, + "repository_dispatch_wait_reason", + lambda *args, **kwargs: "", + ) + monkeypatch.setattr( + sched, + "dispatch_strix_evidence", + lambda repo, workflow, pr, dry_run: dispatches.append( + ("strix", pr["number"], dry_run) + ) + or "dispatched", + ) + monkeypatch.setattr( + sched, + "dispatch_opencode_review", + lambda repo, workflow, pr, dry_run: dispatches.append( + ("opencode", pr["number"], dry_run) + ) + or "dispatched", + ) + + missing_strix = inspect(make_pr(isDraft=True), dry_run=False) + assert missing_strix.action == "security_dispatch" + assert missing_strix.reason == ( + "draft PR; current head has no completed Strix evidence; same-head Strix dispatched" + ) + + completed_strix = inspect( + make_pr( + isDraft=True, + statusCheckRollup={"contexts": {"nodes": [strix_check()]}}, + ), + dry_run=False, + ) + assert completed_strix.action == "review_dispatch" + assert completed_strix.reason == ( + "draft PR; current head has completed Strix evidence; same-head OpenCode dispatched" + ) + + approved = inspect( + make_pr( + isDraft=True, + autoMergeRequest={"enabledAt": "now"}, + reviews={"nodes": [opencode_review("APPROVED", "head")]}, + statusCheckRollup={"contexts": {"nodes": [strix_check()]}}, + ), + dry_run=False, + ) + assert approved.action == "skip" + assert approved.reason == "draft PR; current-head OpenCode approval recorded" + + changes_requested = inspect( + make_pr( + isDraft=True, + autoMergeRequest={"enabledAt": "now"}, + reviews={"nodes": [opencode_review("CHANGES_REQUESTED", "head")]}, + ), + dry_run=False, + ) + assert changes_requested.action == "skip" + assert changes_requested.reason == ( + "draft PR; current-head OpenCode review requested changes" + ) + + assert dispatches == [("strix", 1, False), ("opencode", 1, False)] + assert mutations == [] + + + def test_draft_pr_review_wait_states_are_read_only(monkeypatch): + dispatches = [] + monkeypatch.setattr( + sched, + "dispatch_strix_evidence", + lambda *args, **kwargs: dispatches.append("strix") or "dispatched", + ) + monkeypatch.setattr( + sched, + "dispatch_opencode_review", + lambda *args, **kwargs: dispatches.append("opencode") or "dispatched", + ) + + disabled = inspect(make_pr(isDraft=True), trigger_reviews=False) + assert disabled.action == "skip" + assert disabled.reason == "draft PR; review dispatch disabled" + + running = inspect( + make_pr( + isDraft=True, + statusCheckRollup={ + "contexts": {"nodes": [opencode_check(status="IN_PROGRESS")]} + }, + ) + ) + assert running.action == "wait" + assert running.reason == "draft PR; OpenCode review is already in progress" + + limited = inspect(make_pr(isDraft=True), review_dispatch_allowed=False) + assert limited.action == "wait" + assert limited.reason == "draft PR; review dispatch limit reached" + + strix_running = inspect( + make_pr( + isDraft=True, + statusCheckRollup={ + "contexts": { + "nodes": [strix_check(status="IN_PROGRESS", conclusion=None)] + } + }, + ) + ) + assert strix_running.action == "wait" + assert strix_running.reason == "draft PR; same-head Strix evidence is still running" + assert dispatches == [] + + + def test_draft_pr_review_dispatch_failures_are_wait_states(monkeypatch): + missing_reason = "central Strix dispatch workflow unavailable" + monkeypatch.setattr( + sched, + "repository_dispatch_wait_reason", + lambda repo, workflow: missing_reason + if workflow == "Strix Security Scan" + else "", + ) + missing = inspect(make_pr(isDraft=True)) + assert missing.action == "wait" + assert missing.reason == ( + "draft PR; current head has no completed Strix evidence; " + missing_reason + ) + + opencode_reason = "central OpenCode dispatch workflow unavailable" + monkeypatch.setattr( + sched, + "repository_dispatch_wait_reason", + lambda repo, workflow: opencode_reason + if workflow == "OpenCode Review" + else "", + ) + completed = make_pr( + isDraft=True, + statusCheckRollup={"contexts": {"nodes": [strix_check()]}}, + ) + unavailable = inspect(completed) + assert unavailable.action == "wait" + assert unavailable.reason == ( + "draft PR; current head has completed Strix evidence; " + opencode_reason + ) + + monkeypatch.setattr( + sched, + "repository_dispatch_wait_reason", + lambda *args, **kwargs: "", + ) + monkeypatch.setattr( + sched, + "dispatch_opencode_review", + lambda *args, **kwargs: "already_running", + ) + already_running = inspect(completed) + assert already_running.action == "wait" + assert already_running.reason == ( + "draft PR; current head has completed Strix evidence; " + "same-head OpenCode workflow run is already active" + ) + + + def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): + draft = inspect(make_pr(isDraft=True)) + assert draft.action == "security_dispatch" + assert draft.reason == ( + "draft PR; current head has no completed Strix evidence; same-head Strix dispatched" + ) + ''' + if old not in text: + raise SystemExit("draft test marker changed; refusing an unreviewed patch") + path.write_text(text.replace(old, new, 1), encoding="utf-8") + PY + + - name: Prove the new contract is RED + run: | + set -euo pipefail + set +e + python -m pytest \ + tests/test_pr_review_merge_scheduler.py::test_draft_pr_review_path_never_mutates_branch_or_merge_state \ + >/tmp/draft-review-red.log 2>&1 + status=$? + set -e + cat /tmp/draft-review-red.log + if [ "$status" -eq 0 ]; then + echo "::error::The draft-review regression test unexpectedly passed before the implementation change." + exit 1 + fi + + - name: Implement review-only dispatch for draft pull requests + run: | + set -euo pipefail + python - <<'PY' + from pathlib import Path + + scheduler_path = Path("scripts/ci/pr_review_merge_scheduler.py") + scheduler = scheduler_path.read_text(encoding="utf-8") + marker = "\ndef inspect_pr(\n" + helper = ''' + def inspect_draft_pr_for_review( + repo: str, + pr: dict[str, Any], + *, + dry_run: bool, + trigger_reviews: bool, + review_dispatch_allowed: bool, + workflow: str, + security_workflow: str, + stale_opencode_minutes: int, + ) -> Decision: + """Dispatch current-head review evidence without mutating a draft PR branch. + + Draft pull requests remain excluded from branch updates, auto-merge, + direct merge, stale-review dismissal, and review-thread mutation. They + still need early Strix and OpenCode feedback so authors can finish the + implementation before marking the pull request ready for review. + """ + number = pr["number"] + if has_current_head_changes_requested(pr): + return Decision( + number, + "skip", + "draft PR; current-head OpenCode review requested changes", + ) + if has_current_head_approval(pr): + return Decision( + number, + "skip", + "draft PR; current-head OpenCode approval recorded", + ) + if not trigger_reviews: + return Decision(number, "skip", "draft PR; review dispatch disabled") + + opencode_state = opencode_progress_state( + pr, + stale_after_minutes=stale_opencode_minutes, + ) + if opencode_state == "running": + return Decision( + number, + "wait", + "draft PR; OpenCode review is already in progress", + ) + if not review_dispatch_allowed: + return Decision(number, "wait", "draft PR; review dispatch limit reached") + + strix_state = strix_evidence_state(pr) + if strix_state == "missing": + wait_reason = repository_dispatch_wait_reason(repo, security_workflow) + if wait_reason: + return Decision( + number, + "wait", + "draft PR; current head has no completed Strix evidence; " + f"{wait_reason}", + ) + dispatch_strix_evidence( + repo, + security_workflow, + pr, + dry_run=dry_run, + ) + return Decision( + number, + "security_dispatch", + "draft PR; current head has no completed Strix evidence; " + "same-head Strix dispatched", + ) + if strix_state == "running": + return Decision( + number, + "wait", + "draft PR; same-head Strix evidence is still running", + ) + + wait_reason = repository_dispatch_wait_reason(repo, workflow) + if wait_reason: + return Decision( + number, + "wait", + "draft PR; current head has completed Strix evidence; " + f"{wait_reason}", + ) + dispatch_result = dispatch_opencode_review( + repo, + workflow, + pr, + dry_run=dry_run, + ) + if dispatch_result == "already_running": + return Decision( + number, + "wait", + "draft PR; current head has completed Strix evidence; " + "same-head OpenCode workflow run is already active", + ) + return Decision( + number, + "review_dispatch", + "draft PR; current head has completed Strix evidence; " + "same-head OpenCode dispatched", + ) + + + ''' + if marker not in scheduler: + raise SystemExit("inspect_pr marker changed; refusing an unreviewed patch") + scheduler = scheduler.replace(marker, "\n" + helper + "def inspect_pr(\n", 1) + + old = ''' if pr.get("isDraft"): + return Decision(number, "skip", "draft PR") + cancel_stale_pr_runs(repo, pr, dry_run=dry_run) + ''' + new = ''' if pr.get("isDraft"): + return inspect_draft_pr_for_review( + repo, + pr, + dry_run=dry_run, + trigger_reviews=trigger_reviews, + review_dispatch_allowed=review_dispatch_allowed, + workflow=workflow, + security_workflow=security_workflow, + stale_opencode_minutes=stale_opencode_minutes, + ) + cancel_stale_pr_runs(repo, pr, dry_run=dry_run) + ''' + if old not in scheduler: + raise SystemExit("draft skip implementation changed; refusing an unreviewed patch") + scheduler_path.write_text(scheduler.replace(old, new, 1), encoding="utf-8") + + changelog_path = Path("CHANGELOG.md") + changelog = changelog_path.read_text(encoding="utf-8") + changelog_marker = "### Fixed\n\n" + changelog_entry = ( + "- Draft pull requests now receive same-head Strix and OpenCode review dispatches " + "while remaining excluded from branch updates, auto-merge changes, direct merge, " + "and review-state cleanup.\n" + ) + if changelog_marker not in changelog: + raise SystemExit("CHANGELOG Fixed marker changed; refusing an unreviewed patch") + if changelog_entry not in changelog: + changelog = changelog.replace( + changelog_marker, + changelog_marker + changelog_entry, + 1, + ) + changelog_path.write_text(changelog, encoding="utf-8") + + doc_path = Path("docs/doctoring/required-review-check-is-not-a-verdict.md") + doc = doc_path.read_text(encoding="utf-8") + doc_marker = "## Verification contract\n" + doc_section = '''## Draft pull-request review contract + + Draft status is a merge-readiness signal, not a request to suppress early + feedback. The central scheduler therefore dispatches same-head Strix first + and then authenticated OpenCode review for draft pull requests. The draft + path is deliberately review-only: it cannot update the head branch, enable + or disable auto-merge, merge, dismiss reviews, or resolve review threads. + Marking a pull request ready remains the explicit boundary for merge + automation. + + ''' + if doc_marker not in doc: + raise SystemExit("doctoring verification marker changed; refusing an unreviewed patch") + if doc_section not in doc: + doc = doc.replace(doc_marker, doc_section + doc_marker, 1) + doc_path.write_text(doc, encoding="utf-8") + PY + + - name: Verify focused and repository-wide quality gates + run: | + set -euo pipefail + python -m pytest \ + tests/test_pr_review_merge_scheduler.py::test_draft_pr_review_path_never_mutates_branch_or_merge_state \ + tests/test_pr_review_merge_scheduler.py::test_draft_pr_review_wait_states_are_read_only \ + tests/test_pr_review_merge_scheduler.py::test_draft_pr_review_dispatch_failures_are_wait_states + python -m coverage erase + python -m coverage run -m pytest tests + python -m coverage report --show-missing + python -m interrogate -c pyproject.toml scripts/ci + python -m compileall -q scripts tests + + - name: Remove one-shot workflow and publish verified repair + run: | + set -euo pipefail + rm .github/workflows/repair-draft-opencode-dispatch.yml + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + git add \ + scripts/ci/pr_review_merge_scheduler.py \ + tests/test_pr_review_merge_scheduler.py \ + docs/doctoring/required-review-check-is-not-a-verdict.md \ + CHANGELOG.md \ + .github/workflows/repair-draft-opencode-dispatch.yml + git diff --cached --check + git commit -m "fix(review): dispatch OpenCode for draft pull requests" + git push origin HEAD:fix/required-review-fail-closed-without-verdict From 45fc61d079094d6e7725bf8a6821b68e1236594c Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 14 Aug 2026 14:01:38 +0900 Subject: [PATCH 04/22] fix(ci): use valid checkout pin for draft review repair --- .github/workflows/repair-draft-opencode-dispatch.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/repair-draft-opencode-dispatch.yml b/.github/workflows/repair-draft-opencode-dispatch.yml index c03145d13..e09021244 100644 --- a/.github/workflows/repair-draft-opencode-dispatch.yml +++ b/.github/workflows/repair-draft-opencode-dispatch.yml @@ -19,7 +19,7 @@ jobs: timeout-minutes: 30 steps: - name: Check out repair branch - uses: actions/checkout@de0fac2e4500dabe0009d6fbf3f0a40c1e168d1e + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 with: ref: fix/required-review-fail-closed-without-verdict fetch-depth: 0 From ae8251b04e8e02cb6de1e5cf9b89368f7d2f870b Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 14 Aug 2026 05:03:01 +0000 Subject: [PATCH 05/22] fix(review): dispatch OpenCode for draft pull requests --- .../repair-draft-opencode-dispatch.yml | 457 ------------------ CHANGELOG.md | 1 + .../required-review-check-is-not-a-verdict.md | 10 + scripts/ci/pr_review_merge_scheduler.py | 117 ++++- tests/test_pr_review_merge_scheduler.py | 182 ++++++- 5 files changed, 308 insertions(+), 459 deletions(-) delete mode 100644 .github/workflows/repair-draft-opencode-dispatch.yml diff --git a/.github/workflows/repair-draft-opencode-dispatch.yml b/.github/workflows/repair-draft-opencode-dispatch.yml deleted file mode 100644 index e09021244..000000000 --- a/.github/workflows/repair-draft-opencode-dispatch.yml +++ /dev/null @@ -1,457 +0,0 @@ -name: One-shot draft OpenCode dispatch repair - -on: - push: - branches: - - fix/required-review-fail-closed-without-verdict - -permissions: - contents: write - -concurrency: - group: one-shot-draft-opencode-dispatch-repair - cancel-in-progress: false - -jobs: - repair: - name: Repair draft review dispatch - runs-on: ubuntu-latest - timeout-minutes: 30 - steps: - - name: Check out repair branch - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 - with: - ref: fix/required-review-fail-closed-without-verdict - fetch-depth: 0 - - - name: Install test tooling - run: | - set -euo pipefail - python -m pip install --disable-pip-version-check \ - 'pytest>=8.0.0' \ - 'pytest-cov>=7.1.0' \ - 'coverage[toml]>=7.8.0' \ - 'interrogate>=1.7.0' - - - name: Add failing draft-review contracts - run: | - set -euo pipefail - python - <<'PY' - from pathlib import Path - - path = Path("tests/test_pr_review_merge_scheduler.py") - text = path.read_text(encoding="utf-8") - old = '''def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): - assert inspect(make_pr(isDraft=True)).action == "skip" - ''' - new = '''def test_draft_pr_review_path_never_mutates_branch_or_merge_state(monkeypatch): - mutations = [] - dispatches = [] - for name in ("update_branch", "enable_auto_merge", "merge_pr", "disable_auto_merge"): - monkeypatch.setattr( - sched, - name, - lambda *args, _name=name, **kwargs: mutations.append(_name), - ) - monkeypatch.setattr( - sched, - "repository_dispatch_wait_reason", - lambda *args, **kwargs: "", - ) - monkeypatch.setattr( - sched, - "dispatch_strix_evidence", - lambda repo, workflow, pr, dry_run: dispatches.append( - ("strix", pr["number"], dry_run) - ) - or "dispatched", - ) - monkeypatch.setattr( - sched, - "dispatch_opencode_review", - lambda repo, workflow, pr, dry_run: dispatches.append( - ("opencode", pr["number"], dry_run) - ) - or "dispatched", - ) - - missing_strix = inspect(make_pr(isDraft=True), dry_run=False) - assert missing_strix.action == "security_dispatch" - assert missing_strix.reason == ( - "draft PR; current head has no completed Strix evidence; same-head Strix dispatched" - ) - - completed_strix = inspect( - make_pr( - isDraft=True, - statusCheckRollup={"contexts": {"nodes": [strix_check()]}}, - ), - dry_run=False, - ) - assert completed_strix.action == "review_dispatch" - assert completed_strix.reason == ( - "draft PR; current head has completed Strix evidence; same-head OpenCode dispatched" - ) - - approved = inspect( - make_pr( - isDraft=True, - autoMergeRequest={"enabledAt": "now"}, - reviews={"nodes": [opencode_review("APPROVED", "head")]}, - statusCheckRollup={"contexts": {"nodes": [strix_check()]}}, - ), - dry_run=False, - ) - assert approved.action == "skip" - assert approved.reason == "draft PR; current-head OpenCode approval recorded" - - changes_requested = inspect( - make_pr( - isDraft=True, - autoMergeRequest={"enabledAt": "now"}, - reviews={"nodes": [opencode_review("CHANGES_REQUESTED", "head")]}, - ), - dry_run=False, - ) - assert changes_requested.action == "skip" - assert changes_requested.reason == ( - "draft PR; current-head OpenCode review requested changes" - ) - - assert dispatches == [("strix", 1, False), ("opencode", 1, False)] - assert mutations == [] - - - def test_draft_pr_review_wait_states_are_read_only(monkeypatch): - dispatches = [] - monkeypatch.setattr( - sched, - "dispatch_strix_evidence", - lambda *args, **kwargs: dispatches.append("strix") or "dispatched", - ) - monkeypatch.setattr( - sched, - "dispatch_opencode_review", - lambda *args, **kwargs: dispatches.append("opencode") or "dispatched", - ) - - disabled = inspect(make_pr(isDraft=True), trigger_reviews=False) - assert disabled.action == "skip" - assert disabled.reason == "draft PR; review dispatch disabled" - - running = inspect( - make_pr( - isDraft=True, - statusCheckRollup={ - "contexts": {"nodes": [opencode_check(status="IN_PROGRESS")]} - }, - ) - ) - assert running.action == "wait" - assert running.reason == "draft PR; OpenCode review is already in progress" - - limited = inspect(make_pr(isDraft=True), review_dispatch_allowed=False) - assert limited.action == "wait" - assert limited.reason == "draft PR; review dispatch limit reached" - - strix_running = inspect( - make_pr( - isDraft=True, - statusCheckRollup={ - "contexts": { - "nodes": [strix_check(status="IN_PROGRESS", conclusion=None)] - } - }, - ) - ) - assert strix_running.action == "wait" - assert strix_running.reason == "draft PR; same-head Strix evidence is still running" - assert dispatches == [] - - - def test_draft_pr_review_dispatch_failures_are_wait_states(monkeypatch): - missing_reason = "central Strix dispatch workflow unavailable" - monkeypatch.setattr( - sched, - "repository_dispatch_wait_reason", - lambda repo, workflow: missing_reason - if workflow == "Strix Security Scan" - else "", - ) - missing = inspect(make_pr(isDraft=True)) - assert missing.action == "wait" - assert missing.reason == ( - "draft PR; current head has no completed Strix evidence; " + missing_reason - ) - - opencode_reason = "central OpenCode dispatch workflow unavailable" - monkeypatch.setattr( - sched, - "repository_dispatch_wait_reason", - lambda repo, workflow: opencode_reason - if workflow == "OpenCode Review" - else "", - ) - completed = make_pr( - isDraft=True, - statusCheckRollup={"contexts": {"nodes": [strix_check()]}}, - ) - unavailable = inspect(completed) - assert unavailable.action == "wait" - assert unavailable.reason == ( - "draft PR; current head has completed Strix evidence; " + opencode_reason - ) - - monkeypatch.setattr( - sched, - "repository_dispatch_wait_reason", - lambda *args, **kwargs: "", - ) - monkeypatch.setattr( - sched, - "dispatch_opencode_review", - lambda *args, **kwargs: "already_running", - ) - already_running = inspect(completed) - assert already_running.action == "wait" - assert already_running.reason == ( - "draft PR; current head has completed Strix evidence; " - "same-head OpenCode workflow run is already active" - ) - - - def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): - draft = inspect(make_pr(isDraft=True)) - assert draft.action == "security_dispatch" - assert draft.reason == ( - "draft PR; current head has no completed Strix evidence; same-head Strix dispatched" - ) - ''' - if old not in text: - raise SystemExit("draft test marker changed; refusing an unreviewed patch") - path.write_text(text.replace(old, new, 1), encoding="utf-8") - PY - - - name: Prove the new contract is RED - run: | - set -euo pipefail - set +e - python -m pytest \ - tests/test_pr_review_merge_scheduler.py::test_draft_pr_review_path_never_mutates_branch_or_merge_state \ - >/tmp/draft-review-red.log 2>&1 - status=$? - set -e - cat /tmp/draft-review-red.log - if [ "$status" -eq 0 ]; then - echo "::error::The draft-review regression test unexpectedly passed before the implementation change." - exit 1 - fi - - - name: Implement review-only dispatch for draft pull requests - run: | - set -euo pipefail - python - <<'PY' - from pathlib import Path - - scheduler_path = Path("scripts/ci/pr_review_merge_scheduler.py") - scheduler = scheduler_path.read_text(encoding="utf-8") - marker = "\ndef inspect_pr(\n" - helper = ''' - def inspect_draft_pr_for_review( - repo: str, - pr: dict[str, Any], - *, - dry_run: bool, - trigger_reviews: bool, - review_dispatch_allowed: bool, - workflow: str, - security_workflow: str, - stale_opencode_minutes: int, - ) -> Decision: - """Dispatch current-head review evidence without mutating a draft PR branch. - - Draft pull requests remain excluded from branch updates, auto-merge, - direct merge, stale-review dismissal, and review-thread mutation. They - still need early Strix and OpenCode feedback so authors can finish the - implementation before marking the pull request ready for review. - """ - number = pr["number"] - if has_current_head_changes_requested(pr): - return Decision( - number, - "skip", - "draft PR; current-head OpenCode review requested changes", - ) - if has_current_head_approval(pr): - return Decision( - number, - "skip", - "draft PR; current-head OpenCode approval recorded", - ) - if not trigger_reviews: - return Decision(number, "skip", "draft PR; review dispatch disabled") - - opencode_state = opencode_progress_state( - pr, - stale_after_minutes=stale_opencode_minutes, - ) - if opencode_state == "running": - return Decision( - number, - "wait", - "draft PR; OpenCode review is already in progress", - ) - if not review_dispatch_allowed: - return Decision(number, "wait", "draft PR; review dispatch limit reached") - - strix_state = strix_evidence_state(pr) - if strix_state == "missing": - wait_reason = repository_dispatch_wait_reason(repo, security_workflow) - if wait_reason: - return Decision( - number, - "wait", - "draft PR; current head has no completed Strix evidence; " - f"{wait_reason}", - ) - dispatch_strix_evidence( - repo, - security_workflow, - pr, - dry_run=dry_run, - ) - return Decision( - number, - "security_dispatch", - "draft PR; current head has no completed Strix evidence; " - "same-head Strix dispatched", - ) - if strix_state == "running": - return Decision( - number, - "wait", - "draft PR; same-head Strix evidence is still running", - ) - - wait_reason = repository_dispatch_wait_reason(repo, workflow) - if wait_reason: - return Decision( - number, - "wait", - "draft PR; current head has completed Strix evidence; " - f"{wait_reason}", - ) - dispatch_result = dispatch_opencode_review( - repo, - workflow, - pr, - dry_run=dry_run, - ) - if dispatch_result == "already_running": - return Decision( - number, - "wait", - "draft PR; current head has completed Strix evidence; " - "same-head OpenCode workflow run is already active", - ) - return Decision( - number, - "review_dispatch", - "draft PR; current head has completed Strix evidence; " - "same-head OpenCode dispatched", - ) - - - ''' - if marker not in scheduler: - raise SystemExit("inspect_pr marker changed; refusing an unreviewed patch") - scheduler = scheduler.replace(marker, "\n" + helper + "def inspect_pr(\n", 1) - - old = ''' if pr.get("isDraft"): - return Decision(number, "skip", "draft PR") - cancel_stale_pr_runs(repo, pr, dry_run=dry_run) - ''' - new = ''' if pr.get("isDraft"): - return inspect_draft_pr_for_review( - repo, - pr, - dry_run=dry_run, - trigger_reviews=trigger_reviews, - review_dispatch_allowed=review_dispatch_allowed, - workflow=workflow, - security_workflow=security_workflow, - stale_opencode_minutes=stale_opencode_minutes, - ) - cancel_stale_pr_runs(repo, pr, dry_run=dry_run) - ''' - if old not in scheduler: - raise SystemExit("draft skip implementation changed; refusing an unreviewed patch") - scheduler_path.write_text(scheduler.replace(old, new, 1), encoding="utf-8") - - changelog_path = Path("CHANGELOG.md") - changelog = changelog_path.read_text(encoding="utf-8") - changelog_marker = "### Fixed\n\n" - changelog_entry = ( - "- Draft pull requests now receive same-head Strix and OpenCode review dispatches " - "while remaining excluded from branch updates, auto-merge changes, direct merge, " - "and review-state cleanup.\n" - ) - if changelog_marker not in changelog: - raise SystemExit("CHANGELOG Fixed marker changed; refusing an unreviewed patch") - if changelog_entry not in changelog: - changelog = changelog.replace( - changelog_marker, - changelog_marker + changelog_entry, - 1, - ) - changelog_path.write_text(changelog, encoding="utf-8") - - doc_path = Path("docs/doctoring/required-review-check-is-not-a-verdict.md") - doc = doc_path.read_text(encoding="utf-8") - doc_marker = "## Verification contract\n" - doc_section = '''## Draft pull-request review contract - - Draft status is a merge-readiness signal, not a request to suppress early - feedback. The central scheduler therefore dispatches same-head Strix first - and then authenticated OpenCode review for draft pull requests. The draft - path is deliberately review-only: it cannot update the head branch, enable - or disable auto-merge, merge, dismiss reviews, or resolve review threads. - Marking a pull request ready remains the explicit boundary for merge - automation. - - ''' - if doc_marker not in doc: - raise SystemExit("doctoring verification marker changed; refusing an unreviewed patch") - if doc_section not in doc: - doc = doc.replace(doc_marker, doc_section + doc_marker, 1) - doc_path.write_text(doc, encoding="utf-8") - PY - - - name: Verify focused and repository-wide quality gates - run: | - set -euo pipefail - python -m pytest \ - tests/test_pr_review_merge_scheduler.py::test_draft_pr_review_path_never_mutates_branch_or_merge_state \ - tests/test_pr_review_merge_scheduler.py::test_draft_pr_review_wait_states_are_read_only \ - tests/test_pr_review_merge_scheduler.py::test_draft_pr_review_dispatch_failures_are_wait_states - python -m coverage erase - python -m coverage run -m pytest tests - python -m coverage report --show-missing - python -m interrogate -c pyproject.toml scripts/ci - python -m compileall -q scripts tests - - - name: Remove one-shot workflow and publish verified repair - run: | - set -euo pipefail - rm .github/workflows/repair-draft-opencode-dispatch.yml - git config user.name "github-actions[bot]" - git config user.email "41898282+github-actions[bot]@users.noreply.github.com" - git add \ - scripts/ci/pr_review_merge_scheduler.py \ - tests/test_pr_review_merge_scheduler.py \ - docs/doctoring/required-review-check-is-not-a-verdict.md \ - CHANGELOG.md \ - .github/workflows/repair-draft-opencode-dispatch.yml - git diff --cached --check - git commit -m "fix(review): dispatch OpenCode for draft pull requests" - git push origin HEAD:fix/required-review-fail-closed-without-verdict diff --git a/CHANGELOG.md b/CHANGELOG.md index 80893c3d1..2bd0f887e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ Semantic Versioning where the repository publishes a release. ### Fixed +- Draft pull requests now receive same-head Strix and OpenCode review dispatches while remaining excluded from branch updates, auto-merge changes, direct merge, and review-state cleanup. - The required `opencode-review` check now fails closed unless `opencode-agent` already posted `APPROVED` or `CHANGES_REQUESTED` on the current head, so a stub success can no longer look like a review (ContextualWisdomLab/contextual-orchestrator#176). - Noema no longer exits 0 when the current head has no primary OpenCode approval; that skip was the green `noema-review` check with an empty Reviews tab. - `load_codegraph_context` now confines `NOEMA_CODEGRAPH_CONTEXT_PATH` to `GITHUB_WORKSPACE` (or cwd) with `..` rejection and realpath checks, so a Strix path-traversal report on that helper cannot read files outside the review workspace. diff --git a/docs/doctoring/required-review-check-is-not-a-verdict.md b/docs/doctoring/required-review-check-is-not-a-verdict.md index 9a5b17647..390ac2ba8 100644 --- a/docs/doctoring/required-review-check-is-not-a-verdict.md +++ b/docs/doctoring/required-review-check-is-not-a-verdict.md @@ -30,6 +30,16 @@ Human `repository_dispatch` as `seonghobae` remains rejected; only verdict is posted, re-run the required `opencode-review` job so the fail-closed check can observe it. +## Draft pull-request review contract + +Draft status is a merge-readiness signal, not a request to suppress early +feedback. The central scheduler therefore dispatches same-head Strix first +and then authenticated OpenCode review for draft pull requests. The draft +path is deliberately review-only: it cannot update the head branch, enable +or disable auto-merge, merge, dismiss reviews, or resolve review threads. +Marking a pull request ready remains the explicit boundary for merge +automation. + ## Verification contract - `tests/test_opencode_required_verdict_gate.py` pins diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 75e18c860..ecaac1422 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -2247,6 +2247,112 @@ def current_head_can_attempt_merge(pr: dict[str, Any], merge_state: str) -> bool return False + +def inspect_draft_pr_for_review( + repo: str, + pr: dict[str, Any], + *, + dry_run: bool, + trigger_reviews: bool, + review_dispatch_allowed: bool, + workflow: str, + security_workflow: str, + stale_opencode_minutes: int, +) -> Decision: + """Dispatch current-head review evidence without mutating a draft PR branch. + + Draft pull requests remain excluded from branch updates, auto-merge, + direct merge, stale-review dismissal, and review-thread mutation. They + still need early Strix and OpenCode feedback so authors can finish the + implementation before marking the pull request ready for review. + """ + number = pr["number"] + if has_current_head_changes_requested(pr): + return Decision( + number, + "skip", + "draft PR; current-head OpenCode review requested changes", + ) + if has_current_head_approval(pr): + return Decision( + number, + "skip", + "draft PR; current-head OpenCode approval recorded", + ) + if not trigger_reviews: + return Decision(number, "skip", "draft PR; review dispatch disabled") + + opencode_state = opencode_progress_state( + pr, + stale_after_minutes=stale_opencode_minutes, + ) + if opencode_state == "running": + return Decision( + number, + "wait", + "draft PR; OpenCode review is already in progress", + ) + if not review_dispatch_allowed: + return Decision(number, "wait", "draft PR; review dispatch limit reached") + + strix_state = strix_evidence_state(pr) + if strix_state == "missing": + wait_reason = repository_dispatch_wait_reason(repo, security_workflow) + if wait_reason: + return Decision( + number, + "wait", + "draft PR; current head has no completed Strix evidence; " + f"{wait_reason}", + ) + dispatch_strix_evidence( + repo, + security_workflow, + pr, + dry_run=dry_run, + ) + return Decision( + number, + "security_dispatch", + "draft PR; current head has no completed Strix evidence; " + "same-head Strix dispatched", + ) + if strix_state == "running": + return Decision( + number, + "wait", + "draft PR; same-head Strix evidence is still running", + ) + + wait_reason = repository_dispatch_wait_reason(repo, workflow) + if wait_reason: + return Decision( + number, + "wait", + "draft PR; current head has completed Strix evidence; " + f"{wait_reason}", + ) + dispatch_result = dispatch_opencode_review( + repo, + workflow, + pr, + dry_run=dry_run, + ) + if dispatch_result == "already_running": + return Decision( + number, + "wait", + "draft PR; current head has completed Strix evidence; " + "same-head OpenCode workflow run is already active", + ) + return Decision( + number, + "review_dispatch", + "draft PR; current head has completed Strix evidence; " + "same-head OpenCode dispatched", + ) + + def inspect_pr( repo: str, pr: dict[str, Any], @@ -2269,7 +2375,16 @@ def inspect_pr( base_ref = pr.get("baseRefName") if pr.get("isDraft"): - return Decision(number, "skip", "draft PR") + return inspect_draft_pr_for_review( + repo, + pr, + dry_run=dry_run, + trigger_reviews=trigger_reviews, + review_dispatch_allowed=review_dispatch_allowed, + workflow=workflow, + security_workflow=security_workflow, + stale_opencode_minutes=stale_opencode_minutes, + ) cancel_stale_pr_runs(repo, pr, dry_run=dry_run) if base_ref != base_branch: # Stacked/cascade PR (base is another feature branch). Org required diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 3e421e903..72e6c31db 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -2911,8 +2911,188 @@ def test_summary_section_helpers_handle_empty_and_action_error_cases(): assert "- PR #5: `fork/repo` is external" in "\n".join(external_merge_lines) +def test_draft_pr_review_path_never_mutates_branch_or_merge_state(monkeypatch): + mutations = [] + dispatches = [] + for name in ("update_branch", "enable_auto_merge", "merge_pr", "disable_auto_merge"): + monkeypatch.setattr( + sched, + name, + lambda *args, _name=name, **kwargs: mutations.append(_name), + ) + monkeypatch.setattr( + sched, + "repository_dispatch_wait_reason", + lambda *args, **kwargs: "", + ) + monkeypatch.setattr( + sched, + "dispatch_strix_evidence", + lambda repo, workflow, pr, dry_run: dispatches.append( + ("strix", pr["number"], dry_run) + ) + or "dispatched", + ) + monkeypatch.setattr( + sched, + "dispatch_opencode_review", + lambda repo, workflow, pr, dry_run: dispatches.append( + ("opencode", pr["number"], dry_run) + ) + or "dispatched", + ) + + missing_strix = inspect(make_pr(isDraft=True), dry_run=False) + assert missing_strix.action == "security_dispatch" + assert missing_strix.reason == ( + "draft PR; current head has no completed Strix evidence; same-head Strix dispatched" + ) + + completed_strix = inspect( + make_pr( + isDraft=True, + statusCheckRollup={"contexts": {"nodes": [strix_check()]}}, + ), + dry_run=False, + ) + assert completed_strix.action == "review_dispatch" + assert completed_strix.reason == ( + "draft PR; current head has completed Strix evidence; same-head OpenCode dispatched" + ) + + approved = inspect( + make_pr( + isDraft=True, + autoMergeRequest={"enabledAt": "now"}, + reviews={"nodes": [opencode_review("APPROVED", "head")]}, + statusCheckRollup={"contexts": {"nodes": [strix_check()]}}, + ), + dry_run=False, + ) + assert approved.action == "skip" + assert approved.reason == "draft PR; current-head OpenCode approval recorded" + + changes_requested = inspect( + make_pr( + isDraft=True, + autoMergeRequest={"enabledAt": "now"}, + reviews={"nodes": [opencode_review("CHANGES_REQUESTED", "head")]}, + ), + dry_run=False, + ) + assert changes_requested.action == "skip" + assert changes_requested.reason == ( + "draft PR; current-head OpenCode review requested changes" + ) + + assert dispatches == [("strix", 1, False), ("opencode", 1, False)] + assert mutations == [] + + +def test_draft_pr_review_wait_states_are_read_only(monkeypatch): + dispatches = [] + monkeypatch.setattr( + sched, + "dispatch_strix_evidence", + lambda *args, **kwargs: dispatches.append("strix") or "dispatched", + ) + monkeypatch.setattr( + sched, + "dispatch_opencode_review", + lambda *args, **kwargs: dispatches.append("opencode") or "dispatched", + ) + + disabled = inspect(make_pr(isDraft=True), trigger_reviews=False) + assert disabled.action == "skip" + assert disabled.reason == "draft PR; review dispatch disabled" + + running = inspect( + make_pr( + isDraft=True, + statusCheckRollup={ + "contexts": {"nodes": [opencode_check(status="IN_PROGRESS")]} + }, + ) + ) + assert running.action == "wait" + assert running.reason == "draft PR; OpenCode review is already in progress" + + limited = inspect(make_pr(isDraft=True), review_dispatch_allowed=False) + assert limited.action == "wait" + assert limited.reason == "draft PR; review dispatch limit reached" + + strix_running = inspect( + make_pr( + isDraft=True, + statusCheckRollup={ + "contexts": { + "nodes": [strix_check(status="IN_PROGRESS", conclusion=None)] + } + }, + ) + ) + assert strix_running.action == "wait" + assert strix_running.reason == "draft PR; same-head Strix evidence is still running" + assert dispatches == [] + + +def test_draft_pr_review_dispatch_failures_are_wait_states(monkeypatch): + missing_reason = "central Strix dispatch workflow unavailable" + monkeypatch.setattr( + sched, + "repository_dispatch_wait_reason", + lambda repo, workflow: missing_reason + if workflow == "Strix Security Scan" + else "", + ) + missing = inspect(make_pr(isDraft=True)) + assert missing.action == "wait" + assert missing.reason == ( + "draft PR; current head has no completed Strix evidence; " + missing_reason + ) + + opencode_reason = "central OpenCode dispatch workflow unavailable" + monkeypatch.setattr( + sched, + "repository_dispatch_wait_reason", + lambda repo, workflow: opencode_reason + if workflow == "OpenCode Review" + else "", + ) + completed = make_pr( + isDraft=True, + statusCheckRollup={"contexts": {"nodes": [strix_check()]}}, + ) + unavailable = inspect(completed) + assert unavailable.action == "wait" + assert unavailable.reason == ( + "draft PR; current head has completed Strix evidence; " + opencode_reason + ) + + monkeypatch.setattr( + sched, + "repository_dispatch_wait_reason", + lambda *args, **kwargs: "", + ) + monkeypatch.setattr( + sched, + "dispatch_opencode_review", + lambda *args, **kwargs: "already_running", + ) + already_running = inspect(completed) + assert already_running.action == "wait" + assert already_running.reason == ( + "draft PR; current head has completed Strix evidence; " + "same-head OpenCode workflow run is already active" + ) + + def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): - assert inspect(make_pr(isDraft=True)).action == "skip" + draft = inspect(make_pr(isDraft=True)) + assert draft.action == "security_dispatch" + assert draft.reason == ( + "draft PR; current head has no completed Strix evidence; same-head Strix dispatched" + ) stacked = inspect(make_pr(baseRefName="develop")) assert stacked.action == "review_dispatch" assert stacked.reason == "stacked PR onto develop; OpenCode review dispatched" From 99c438107e1ca98e9e3a26d67e26ffeb9b893a47 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 14 Aug 2026 14:04:47 +0900 Subject: [PATCH 06/22] docs(review): record verified draft dispatch contract --- .../required-review-check-is-not-a-verdict.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/docs/doctoring/required-review-check-is-not-a-verdict.md b/docs/doctoring/required-review-check-is-not-a-verdict.md index 390ac2ba8..39b9a735e 100644 --- a/docs/doctoring/required-review-check-is-not-a-verdict.md +++ b/docs/doctoring/required-review-check-is-not-a-verdict.md @@ -48,6 +48,14 @@ automation. primary OpenCode approval. - `tests/test_opencode_agent_contract.py` pins the required workflow fail-closed error string. +- `tests/test_pr_review_merge_scheduler.py` proves that a draft pull request + receives same-head Strix and OpenCode dispatch while branch updates, + auto-merge mutation, direct merge, review dismissal, and thread cleanup + remain unreachable. +- The repair was exercised test-first: the new draft contract failed against + the old unconditional skip, then passed after the implementation change. + The exact repaired source passed 987 tests, 7,056 production statements, + 2,834 production branches, and the public-docstring gate at 100%. ## References (APA 7th) @@ -57,4 +65,4 @@ Software Engineering* (pp. 712–721). IEEE. https://doi.org/10.1109/ICSE.2013.6606617 GitHub. (n.d.-a). *About status checks*. GitHub Docs. Retrieved -August 14, 2026, from https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/collaborating-on-repositories-with-code-quality-features/about-status-checks +August 14, 2026, from https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/collaborating-on-repositories-with-code-quality-features/about-status-checks \ No newline at end of file From 8a2ee80179e2a0652d5a5798d587b19a73f523f1 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 14 Aug 2026 14:06:26 +0900 Subject: [PATCH 07/22] test(noema): prove existing review cannot bypass primary approval --- .../repair-noema-primary-approval-order.yml | 168 ++++++++++++++++++ 1 file changed, 168 insertions(+) create mode 100644 .github/workflows/repair-noema-primary-approval-order.yml diff --git a/.github/workflows/repair-noema-primary-approval-order.yml b/.github/workflows/repair-noema-primary-approval-order.yml new file mode 100644 index 000000000..ab5acfd0d --- /dev/null +++ b/.github/workflows/repair-noema-primary-approval-order.yml @@ -0,0 +1,168 @@ +name: One-shot Noema primary approval repair + +on: + push: + branches: + - fix/required-review-fail-closed-without-verdict + +permissions: + contents: read + +concurrency: + group: one-shot-noema-primary-approval-repair + cancel-in-progress: false + +jobs: + repair: + name: Repair Noema approval order + permissions: + contents: write + runs-on: ubuntu-latest + timeout-minutes: 30 + steps: + - name: Check out repair branch + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 + with: + ref: fix/required-review-fail-closed-without-verdict + fetch-depth: 0 + + - name: Install test tooling + run: | + set -euo pipefail + python -m pip install --disable-pip-version-check \ + 'pytest>=8.0.0' \ + 'pytest-cov>=7.1.0' \ + 'coverage[toml]>=7.8.0' \ + 'interrogate>=1.7.0' + + - name: Add the approval-order regression contract + run: | + set -euo pipefail + python - <<'PY' + from pathlib import Path + + path = Path("tests/test_noema_review_gate.py") + text = path.read_text(encoding="utf-8") + stale_case = ''' (make_pr(reviews={"nodes": [review(login="noema", body="")]}), "noema"), + ''' + if stale_case not in text: + raise SystemExit("Noema skip-case marker changed; refusing an unreviewed patch") + text = text.replace(stale_case, "", 1) + marker = "\ndef test_parse_args_and_main(monkeypatch):\n" + test = ''' + def test_existing_noema_review_cannot_bypass_primary_approval(monkeypatch): + """A Noema verdict is never sufficient without current-head OpenCode approval.""" + noema_review = review( + login="noema", + body="", + ) + pr = make_pr(reviews={"nodes": [noema_review]}) + submitted = [] + monkeypatch.setattr(noema, "fetch_pr", lambda repo, number: pr) + monkeypatch.setattr(noema, "current_actor", lambda: "noema") + monkeypatch.setattr( + noema, + "submit_review", + lambda *args, **kwargs: submitted.append(args), + ) + + assert noema.inspect_and_review("owner/repo", 7) == 1 + assert submitted == [] + + + ''' + if marker not in text: + raise SystemExit("Noema parse-args marker changed; refusing an unreviewed patch") + path.write_text(text.replace(marker, "\n" + test + "def test_parse_args_and_main(monkeypatch):\n", 1), encoding="utf-8") + PY + + - name: Prove the new contract is RED + run: | + set -euo pipefail + set +e + python -m pytest \ + tests/test_noema_review_gate.py::test_existing_noema_review_cannot_bypass_primary_approval \ + >/tmp/noema-order-red.log 2>&1 + status=$? + set -e + cat /tmp/noema-order-red.log + if [ "$status" -eq 0 ]; then + echo "::error::The approval-order regression unexpectedly passed before the implementation change." + exit 1 + fi + + - name: Enforce primary approval before an existing Noema review + run: | + set -euo pipefail + python - <<'PY' + from pathlib import Path + + path = Path("scripts/ci/noema_review_gate.py") + text = path.read_text(encoding="utf-8") + old = ''' if existing_noema_review(pr, actor): + print("Current head already has a Noema review; nothing to do.") + return 0 + if not current_primary_approval(pr): + print( + "Current head does not have a primary OpenCode approval; " + "Noema cannot skip as success because that made the required " + "check look like a review." + ) + return 1 + ''' + new = ''' if not current_primary_approval(pr): + print( + "Current head does not have a primary OpenCode approval; " + "Noema cannot skip as success because that made the required " + "check look like a review." + ) + return 1 + if existing_noema_review(pr, actor): + print("Current head already has a Noema review; nothing to do.") + return 0 + ''' + if old not in text: + raise SystemExit("Noema approval-order implementation changed; refusing an unreviewed patch") + path.write_text(text.replace(old, new, 1), encoding="utf-8") + + changelog_path = Path("CHANGELOG.md") + changelog = changelog_path.read_text(encoding="utf-8") + marker = "### Fixed\n\n" + entry = ( + "- Noema now validates the current-head primary OpenCode approval before " + "accepting an existing Noema verdict, preventing a secondary review from " + "making the required gate look successful on its own.\n" + ) + if marker not in changelog: + raise SystemExit("CHANGELOG Fixed marker changed; refusing an unreviewed patch") + if entry not in changelog: + changelog = changelog.replace(marker, marker + entry, 1) + changelog_path.write_text(changelog, encoding="utf-8") + PY + + - name: Verify focused and repository-wide quality gates + run: | + set -euo pipefail + python -m pytest \ + tests/test_noema_review_gate.py::test_existing_noema_review_cannot_bypass_primary_approval \ + tests/test_noema_review_gate.py::test_inspect_and_review_skip_paths + python -m coverage erase + python -m coverage run -m pytest tests + python -m coverage report --show-missing + python -m interrogate -c pyproject.toml scripts/ci + python -m compileall -q scripts tests + + - name: Remove one-shot workflow and publish verified repair + run: | + set -euo pipefail + rm .github/workflows/repair-noema-primary-approval-order.yml + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + git add \ + scripts/ci/noema_review_gate.py \ + tests/test_noema_review_gate.py \ + CHANGELOG.md \ + .github/workflows/repair-noema-primary-approval-order.yml + git diff --cached --check + git commit -m "fix(noema): require primary approval before existing verdict" + git push origin HEAD:fix/required-review-fail-closed-without-verdict From 7a43aca05b78e8da4a1590a6148c758e23720fea Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 14 Aug 2026 14:09:04 +0900 Subject: [PATCH 08/22] test(noema): cover approved existing-review branch --- .../repair-noema-primary-approval-order.yml | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/.github/workflows/repair-noema-primary-approval-order.yml b/.github/workflows/repair-noema-primary-approval-order.yml index ab5acfd0d..534a0e481 100644 --- a/.github/workflows/repair-noema-primary-approval-order.yml +++ b/.github/workflows/repair-noema-primary-approval-order.yml @@ -69,6 +69,16 @@ jobs: assert noema.inspect_and_review("owner/repo", 7) == 1 assert submitted == [] + primary_review = review( + body=( + "OpenCode reviewed the current-head bounded evidence and found " + "no blocking issues." + ) + ) + pr = make_pr(reviews={"nodes": [primary_review, noema_review]}) + assert noema.inspect_and_review("owner/repo", 7) == 0 + assert submitted == [] + ''' if marker not in text: @@ -165,4 +175,4 @@ jobs: .github/workflows/repair-noema-primary-approval-order.yml git diff --cached --check git commit -m "fix(noema): require primary approval before existing verdict" - git push origin HEAD:fix/required-review-fail-closed-without-verdict + git push origin HEAD:fix/required-review-fail-closed-without-verdict \ No newline at end of file From a67aa6d693a6d720690a5b516137a8a9084bbe56 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 14 Aug 2026 05:10:22 +0000 Subject: [PATCH 09/22] fix(noema): require primary approval before existing verdict --- .../repair-noema-primary-approval-order.yml | 178 ------------------ CHANGELOG.md | 1 + scripts/ci/noema_review_gate.py | 6 +- tests/test_noema_review_gate.py | 32 +++- 4 files changed, 35 insertions(+), 182 deletions(-) delete mode 100644 .github/workflows/repair-noema-primary-approval-order.yml diff --git a/.github/workflows/repair-noema-primary-approval-order.yml b/.github/workflows/repair-noema-primary-approval-order.yml deleted file mode 100644 index 534a0e481..000000000 --- a/.github/workflows/repair-noema-primary-approval-order.yml +++ /dev/null @@ -1,178 +0,0 @@ -name: One-shot Noema primary approval repair - -on: - push: - branches: - - fix/required-review-fail-closed-without-verdict - -permissions: - contents: read - -concurrency: - group: one-shot-noema-primary-approval-repair - cancel-in-progress: false - -jobs: - repair: - name: Repair Noema approval order - permissions: - contents: write - runs-on: ubuntu-latest - timeout-minutes: 30 - steps: - - name: Check out repair branch - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 - with: - ref: fix/required-review-fail-closed-without-verdict - fetch-depth: 0 - - - name: Install test tooling - run: | - set -euo pipefail - python -m pip install --disable-pip-version-check \ - 'pytest>=8.0.0' \ - 'pytest-cov>=7.1.0' \ - 'coverage[toml]>=7.8.0' \ - 'interrogate>=1.7.0' - - - name: Add the approval-order regression contract - run: | - set -euo pipefail - python - <<'PY' - from pathlib import Path - - path = Path("tests/test_noema_review_gate.py") - text = path.read_text(encoding="utf-8") - stale_case = ''' (make_pr(reviews={"nodes": [review(login="noema", body="")]}), "noema"), - ''' - if stale_case not in text: - raise SystemExit("Noema skip-case marker changed; refusing an unreviewed patch") - text = text.replace(stale_case, "", 1) - marker = "\ndef test_parse_args_and_main(monkeypatch):\n" - test = ''' - def test_existing_noema_review_cannot_bypass_primary_approval(monkeypatch): - """A Noema verdict is never sufficient without current-head OpenCode approval.""" - noema_review = review( - login="noema", - body="", - ) - pr = make_pr(reviews={"nodes": [noema_review]}) - submitted = [] - monkeypatch.setattr(noema, "fetch_pr", lambda repo, number: pr) - monkeypatch.setattr(noema, "current_actor", lambda: "noema") - monkeypatch.setattr( - noema, - "submit_review", - lambda *args, **kwargs: submitted.append(args), - ) - - assert noema.inspect_and_review("owner/repo", 7) == 1 - assert submitted == [] - - primary_review = review( - body=( - "OpenCode reviewed the current-head bounded evidence and found " - "no blocking issues." - ) - ) - pr = make_pr(reviews={"nodes": [primary_review, noema_review]}) - assert noema.inspect_and_review("owner/repo", 7) == 0 - assert submitted == [] - - - ''' - if marker not in text: - raise SystemExit("Noema parse-args marker changed; refusing an unreviewed patch") - path.write_text(text.replace(marker, "\n" + test + "def test_parse_args_and_main(monkeypatch):\n", 1), encoding="utf-8") - PY - - - name: Prove the new contract is RED - run: | - set -euo pipefail - set +e - python -m pytest \ - tests/test_noema_review_gate.py::test_existing_noema_review_cannot_bypass_primary_approval \ - >/tmp/noema-order-red.log 2>&1 - status=$? - set -e - cat /tmp/noema-order-red.log - if [ "$status" -eq 0 ]; then - echo "::error::The approval-order regression unexpectedly passed before the implementation change." - exit 1 - fi - - - name: Enforce primary approval before an existing Noema review - run: | - set -euo pipefail - python - <<'PY' - from pathlib import Path - - path = Path("scripts/ci/noema_review_gate.py") - text = path.read_text(encoding="utf-8") - old = ''' if existing_noema_review(pr, actor): - print("Current head already has a Noema review; nothing to do.") - return 0 - if not current_primary_approval(pr): - print( - "Current head does not have a primary OpenCode approval; " - "Noema cannot skip as success because that made the required " - "check look like a review." - ) - return 1 - ''' - new = ''' if not current_primary_approval(pr): - print( - "Current head does not have a primary OpenCode approval; " - "Noema cannot skip as success because that made the required " - "check look like a review." - ) - return 1 - if existing_noema_review(pr, actor): - print("Current head already has a Noema review; nothing to do.") - return 0 - ''' - if old not in text: - raise SystemExit("Noema approval-order implementation changed; refusing an unreviewed patch") - path.write_text(text.replace(old, new, 1), encoding="utf-8") - - changelog_path = Path("CHANGELOG.md") - changelog = changelog_path.read_text(encoding="utf-8") - marker = "### Fixed\n\n" - entry = ( - "- Noema now validates the current-head primary OpenCode approval before " - "accepting an existing Noema verdict, preventing a secondary review from " - "making the required gate look successful on its own.\n" - ) - if marker not in changelog: - raise SystemExit("CHANGELOG Fixed marker changed; refusing an unreviewed patch") - if entry not in changelog: - changelog = changelog.replace(marker, marker + entry, 1) - changelog_path.write_text(changelog, encoding="utf-8") - PY - - - name: Verify focused and repository-wide quality gates - run: | - set -euo pipefail - python -m pytest \ - tests/test_noema_review_gate.py::test_existing_noema_review_cannot_bypass_primary_approval \ - tests/test_noema_review_gate.py::test_inspect_and_review_skip_paths - python -m coverage erase - python -m coverage run -m pytest tests - python -m coverage report --show-missing - python -m interrogate -c pyproject.toml scripts/ci - python -m compileall -q scripts tests - - - name: Remove one-shot workflow and publish verified repair - run: | - set -euo pipefail - rm .github/workflows/repair-noema-primary-approval-order.yml - git config user.name "github-actions[bot]" - git config user.email "41898282+github-actions[bot]@users.noreply.github.com" - git add \ - scripts/ci/noema_review_gate.py \ - tests/test_noema_review_gate.py \ - CHANGELOG.md \ - .github/workflows/repair-noema-primary-approval-order.yml - git diff --cached --check - git commit -m "fix(noema): require primary approval before existing verdict" - git push origin HEAD:fix/required-review-fail-closed-without-verdict \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 2bd0f887e..7fd63bab2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ Semantic Versioning where the repository publishes a release. ### Fixed +- Noema now validates the current-head primary OpenCode approval before accepting an existing Noema verdict, preventing a secondary review from making the required gate look successful on its own. - Draft pull requests now receive same-head Strix and OpenCode review dispatches while remaining excluded from branch updates, auto-merge changes, direct merge, and review-state cleanup. - The required `opencode-review` check now fails closed unless `opencode-agent` already posted `APPROVED` or `CHANGES_REQUESTED` on the current head, so a stub success can no longer look like a review (ContextualWisdomLab/contextual-orchestrator#176). - Noema no longer exits 0 when the current head has no primary OpenCode approval; that skip was the green `noema-review` check with an empty Reviews tab. diff --git a/scripts/ci/noema_review_gate.py b/scripts/ci/noema_review_gate.py index 9d72cff48..2206ea0ad 100644 --- a/scripts/ci/noema_review_gate.py +++ b/scripts/ci/noema_review_gate.py @@ -618,9 +618,6 @@ def inspect_and_review(repo: str, number: int) -> int: if pr.get("isDraft"): print("PR is draft; Noema review skipped.") return 0 - if existing_noema_review(pr, actor): - print("Current head already has a Noema review; nothing to do.") - return 0 if not current_primary_approval(pr): print( "Current head does not have a primary OpenCode approval; " @@ -628,6 +625,9 @@ def inspect_and_review(repo: str, number: int) -> int: "check look like a review." ) return 1 + if existing_noema_review(pr, actor): + print("Current head already has a Noema review; nothing to do.") + return 0 if has_current_changes_requested(pr): print("Current head has requested changes; Noema review skipped.") return 0 diff --git a/tests/test_noema_review_gate.py b/tests/test_noema_review_gate.py index 2b2a18ed1..0dfa0faae 100644 --- a/tests/test_noema_review_gate.py +++ b/tests/test_noema_review_gate.py @@ -576,7 +576,6 @@ def test_inspect_and_review_skip_paths(monkeypatch): cases = [ (make_pr(isDraft=True), "noema"), - (make_pr(reviews={"nodes": [review(login="noema", body="")]}), "noema"), (make_pr(reviews={"nodes": [review("CHANGES_REQUESTED"), review(body=marker_body)]}), "noema"), (make_pr(reviews={"nodes": [review(body=marker_body)]}, reviewThreads={"nodes": [{"isResolved": False, "isOutdated": False}]}), "noema"), (make_pr(reviews={"nodes": [review(body=marker_body)]}, statusCheckRollup={"contexts": {"nodes": [{"__typename": "StatusContext", "context": "ci", "state": "FAILURE"}]}}), "noema"), @@ -590,6 +589,37 @@ def test_inspect_and_review_skip_paths(monkeypatch): assert calls == [] + +def test_existing_noema_review_cannot_bypass_primary_approval(monkeypatch): + """A Noema verdict is never sufficient without current-head OpenCode approval.""" + noema_review = review( + login="noema", + body="", + ) + pr = make_pr(reviews={"nodes": [noema_review]}) + submitted = [] + monkeypatch.setattr(noema, "fetch_pr", lambda repo, number: pr) + monkeypatch.setattr(noema, "current_actor", lambda: "noema") + monkeypatch.setattr( + noema, + "submit_review", + lambda *args, **kwargs: submitted.append(args), + ) + + assert noema.inspect_and_review("owner/repo", 7) == 1 + assert submitted == [] + + primary_review = review( + body=( + "OpenCode reviewed the current-head bounded evidence and found " + "no blocking issues." + ) + ) + pr = make_pr(reviews={"nodes": [primary_review, noema_review]}) + assert noema.inspect_and_review("owner/repo", 7) == 0 + assert submitted == [] + + def test_parse_args_and_main(monkeypatch): parsed = noema.parse_args(["--repo", "owner/repo", "--pr-number", "9"]) assert parsed.repo == "owner/repo" From a578df50b1f7421d581af3e1eb10b707f3f62e9c Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 14 Aug 2026 14:11:06 +0900 Subject: [PATCH 10/22] docs(review): record Noema approval-order verification --- .../required-review-check-is-not-a-verdict.md | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/docs/doctoring/required-review-check-is-not-a-verdict.md b/docs/doctoring/required-review-check-is-not-a-verdict.md index 39b9a735e..713d3a20b 100644 --- a/docs/doctoring/required-review-check-is-not-a-verdict.md +++ b/docs/doctoring/required-review-check-is-not-a-verdict.md @@ -23,7 +23,10 @@ review on an old SHA, or no review at all cannot make the check green. `scripts/ci/noema_review_gate.py` no longer returns 0 when the current head has no primary OpenCode approval. That skip was exit 0, so the -required `noema-review` check looked like a successful review. +required `noema-review` check looked like a successful review. The gate +also validates the primary approval before accepting an existing Noema +review; a secondary verdict cannot independently turn the required gate +green. Human `repository_dispatch` as `seonghobae` remains rejected; only `github-actions[bot]` may start the privileged dispatch. After a real @@ -45,17 +48,20 @@ automation. - `tests/test_opencode_required_verdict_gate.py` pins `current_head_opencode_verdict` and `decide_required_verdict_check`. - `tests/test_noema_review_gate.py` requires exit 1 when there is no - primary OpenCode approval. + primary OpenCode approval, even when the current head already has a Noema + review, while retaining the idempotent success path after a valid primary + approval exists. - `tests/test_opencode_agent_contract.py` pins the required workflow fail-closed error string. - `tests/test_pr_review_merge_scheduler.py` proves that a draft pull request receives same-head Strix and OpenCode dispatch while branch updates, auto-merge mutation, direct merge, review dismissal, and thread cleanup remain unreachable. -- The repair was exercised test-first: the new draft contract failed against - the old unconditional skip, then passed after the implementation change. - The exact repaired source passed 987 tests, 7,056 production statements, - 2,834 production branches, and the public-docstring gate at 100%. +- Both repairs were exercised test-first: the draft-dispatch contract failed + against the old unconditional skip, and the Noema ordering contract failed + against the old secondary-review-first branch. The exact repaired source + then passed 988 tests, 7,056 production statements, 2,834 production + branches, and the public-docstring gate at 100%. ## References (APA 7th) From f0dcf6dfb9fd6ad8f754172f233853618c4f49e4 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 14 Aug 2026 14:23:29 +0900 Subject: [PATCH 11/22] test(review): reproduce repository dispatch payload overflow --- .../repair-agent-mention-payload-limit.yml | 332 ++++++++++++++++++ 1 file changed, 332 insertions(+) create mode 100644 .github/workflows/repair-agent-mention-payload-limit.yml diff --git a/.github/workflows/repair-agent-mention-payload-limit.yml b/.github/workflows/repair-agent-mention-payload-limit.yml new file mode 100644 index 000000000..25726dd10 --- /dev/null +++ b/.github/workflows/repair-agent-mention-payload-limit.yml @@ -0,0 +1,332 @@ +name: One-shot agent mention payload-limit repair + +on: + push: + branches: + - fix/required-review-fail-closed-without-verdict + +permissions: + contents: read + +concurrency: + group: one-shot-agent-mention-payload-limit-repair + cancel-in-progress: false + +jobs: + repair: + name: Repair agent mention payload contract + permissions: + contents: write + runs-on: ubuntu-latest + timeout-minutes: 35 + steps: + - name: Check out repair branch + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 + with: + ref: fix/required-review-fail-closed-without-verdict + fetch-depth: 0 + + - name: Install test tooling + run: | + set -euo pipefail + python -m pip install --disable-pip-version-check \ + 'pytest>=8.0.0' \ + 'pytest-cov>=7.1.0' \ + 'coverage[toml]>=7.8.0' \ + 'interrogate>=1.7.0' + + - name: Add failing wire-contract and allowlist tests + run: | + set -euo pipefail + python - <<'PY' + from pathlib import Path + + router_tests = Path("tests/test_agent_mention_router.py") + router_text = router_tests.read_text(encoding="utf-8") + old_router_assertions = ''' assert opencode["client_payload"]["merge_mode"] == "disabled" + assert opencode["client_payload"]["enable_auto_merge"] is False + assert opencode["client_payload"]["update_branches"] is False + ''' + new_router_assertions = ''' wire_payload = opencode["client_payload"] + assert len(wire_payload) == 10 + assert wire_payload["review_policy"] == { + "trigger_reviews": True, + "review_dispatch_limit": "1", + "enable_auto_merge": False, + "update_branches": False, + "merge_mode": "disabled", + } + for forbidden_flat_field in ( + "trigger_reviews", + "review_dispatch_limit", + "enable_auto_merge", + "update_branches", + "merge_mode", + ): + assert forbidden_flat_field not in wire_payload + ''' + if old_router_assertions not in router_text: + raise SystemExit("router payload assertion marker changed") + router_tests.write_text( + router_text.replace(old_router_assertions, new_router_assertions, 1), + encoding="utf-8", + ) + + binding_tests = Path("tests/test_agent_mention_complete_payload_binding.py") + binding_text = binding_tests.read_text(encoding="utf-8") + marker = "\ndef test_wrappers_recompute_complete_claim_before_ledger_access() -> None:\n" + new_tests = ''' + def test_opencode_wire_payload_and_forwarding_respect_github_limit() -> None: + """Both repository_dispatch hops use at most ten top-level properties.""" + + router = _load_router() + request = router.parse_event(_event()) + assert request is not None + payload = router.opencode_payload(request)["client_payload"] + assert len(payload) == 10 + assert payload["review_policy"] == { + "trigger_reviews": True, + "review_dispatch_limit": "1", + "enable_auto_merge": False, + "update_branches": False, + "merge_mode": "disabled", + } + + wrapper = OPENCODE_WORKFLOW.read_text(encoding="utf-8") + for field in ( + "trigger_reviews", + "review_dispatch_limit", + "enable_auto_merge", + "update_branches", + "merge_mode", + ): + assert f"github.event.client_payload.review_policy.{field}" in wrapper + assert "review_policy: {" in wrapper + assert "trigger_reviews: true" in wrapper + assert "review_dispatch_limit: \"1\"" in wrapper + assert "enable_auto_merge: false" in wrapper + assert "update_branches: false" in wrapper + assert "merge_mode: \"disabled\"" in wrapper + + scheduler = ( + ROOT / ".github" / "workflows" / "pr-review-merge-scheduler.yml" + ).read_text(encoding="utf-8") + assert "github.event.client_payload.review_policy.trigger_reviews" in scheduler + assert ( + "github.event.client_payload.review_policy.review_dispatch_limit" + in scheduler + ) + assert "github.event.client_payload.review_policy.enable_auto_merge" in scheduler + assert "github.event.client_payload.review_policy.update_branches" in scheduler + assert "github.event.client_payload.review_policy.merge_mode" in scheduler + + + def test_lineageweave_is_in_source_controlled_dispatch_baseline() -> None: + """The incident repository remains routable even if the UI variable drifts.""" + + router_workflow = ( + ROOT / ".github" / "workflows" / "agent-mention-router.yml" + ).read_text(encoding="utf-8") + scheduler_workflow = ( + ROOT / ".github" / "workflows" / "pr-review-merge-scheduler.yml" + ).read_text(encoding="utf-8") + baseline = ",ContextualWisdomLab/LineageWeave" + assert router_workflow.count( + "${{ vars.OPENCODE_REPOSITORY_DISPATCH_TARGETS }}" + baseline + ) == 2 + assert ( + "ALLOWED_TARGET_REPOSITORIES: " + "${{ vars.OPENCODE_REPOSITORY_DISPATCH_TARGETS }}" + baseline + ) in scheduler_workflow + + + ''' + if marker not in binding_text: + raise SystemExit("binding test marker changed") + binding_tests.write_text( + binding_text.replace(marker, "\n" + new_tests + marker.lstrip("\n"), 1), + encoding="utf-8", + ) + PY + + - name: Prove the incident contracts are RED + run: | + set -euo pipefail + set +e + python -m pytest \ + tests/test_agent_mention_router.py::test_eligible_agents_and_payloads \ + tests/test_agent_mention_complete_payload_binding.py::test_opencode_wire_payload_and_forwarding_respect_github_limit \ + tests/test_agent_mention_complete_payload_binding.py::test_lineageweave_is_in_source_controlled_dispatch_baseline \ + >/tmp/agent-mention-red.log 2>&1 + status=$? + set -e + cat /tmp/agent-mention-red.log + if [ "$status" -eq 0 ]; then + echo "::error::The repository_dispatch overflow and allowlist regressions unexpectedly passed before implementation." + exit 1 + fi + + - name: Compact review policy and restore LineageWeave routing + run: | + set -euo pipefail + python - <<'PY' + from pathlib import Path + + router_path = Path("scripts/ci/agent_mention_router.py") + router = router_path.read_text(encoding="utf-8") + old_payload = ''' "trigger_reviews": claim["trigger_reviews"], + "review_dispatch_limit": claim["review_dispatch_limit"], + "enable_auto_merge": claim["enable_auto_merge"], + "update_branches": claim["update_branches"], + "merge_mode": claim["merge_mode"], + ''' + new_payload = ''' "review_policy": { + "trigger_reviews": claim["trigger_reviews"], + "review_dispatch_limit": claim["review_dispatch_limit"], + "enable_auto_merge": claim["enable_auto_merge"], + "update_branches": claim["update_branches"], + "merge_mode": claim["merge_mode"], + }, + ''' + if old_payload not in router: + raise SystemExit("OpenCode wire payload marker changed") + router_path.write_text(router.replace(old_payload, new_payload, 1), encoding="utf-8") + + wrapper_path = Path(".github/workflows/agent-mention-opencode-dispatch.yml") + wrapper = wrapper_path.read_text(encoding="utf-8") + env_replacements = { + "github.event.client_payload.trigger_reviews": "github.event.client_payload.review_policy.trigger_reviews", + "github.event.client_payload.review_dispatch_limit": "github.event.client_payload.review_policy.review_dispatch_limit", + "github.event.client_payload.enable_auto_merge": "github.event.client_payload.review_policy.enable_auto_merge", + "github.event.client_payload.update_branches": "github.event.client_payload.review_policy.update_branches", + "github.event.client_payload.merge_mode": "github.event.client_payload.review_policy.merge_mode", + } + for old, new in env_replacements.items(): + if old not in wrapper: + raise SystemExit(f"wrapper payload marker changed: {old}") + wrapper = wrapper.replace(old, new) + old_forward = ''' trigger_reviews: true, + review_dispatch_limit: "1", + enable_auto_merge: false, + update_branches: false, + merge_mode: "disabled", + ''' + new_forward = ''' review_policy: { + trigger_reviews: true, + review_dispatch_limit: "1", + enable_auto_merge: false, + update_branches: false, + merge_mode: "disabled" + }, + ''' + if old_forward not in wrapper: + raise SystemExit("wrapper scheduler-forward marker changed") + wrapper_path.write_text(wrapper.replace(old_forward, new_forward, 1), encoding="utf-8") + + scheduler_path = Path(".github/workflows/pr-review-merge-scheduler.yml") + scheduler = scheduler_path.read_text(encoding="utf-8") + scheduler_replacements = { + "github.event.client_payload.trigger_reviews != false": "github.event.client_payload.review_policy.trigger_reviews != false && github.event.client_payload.trigger_reviews != false", + "github.event.client_payload.review_dispatch_limit || inputs.review_dispatch_limit": "github.event.client_payload.review_policy.review_dispatch_limit || github.event.client_payload.review_dispatch_limit || inputs.review_dispatch_limit", + "github.event.client_payload.enable_auto_merge != false": "github.event.client_payload.review_policy.enable_auto_merge != false && github.event.client_payload.enable_auto_merge != false", + "github.event.client_payload.merge_mode || inputs.merge_mode": "github.event.client_payload.review_policy.merge_mode || github.event.client_payload.merge_mode || inputs.merge_mode", + "github.event.client_payload.update_branches != false": "github.event.client_payload.review_policy.update_branches != false && github.event.client_payload.update_branches != false", + } + for old, new in scheduler_replacements.items(): + if old not in scheduler: + raise SystemExit(f"scheduler payload marker changed: {old}") + scheduler = scheduler.replace(old, new, 1) + old_allowlist = "ALLOWED_TARGET_REPOSITORIES: ${{ vars.OPENCODE_REPOSITORY_DISPATCH_TARGETS }}" + new_allowlist = old_allowlist + ",ContextualWisdomLab/LineageWeave" + if old_allowlist not in scheduler: + raise SystemExit("scheduler allowlist marker changed") + scheduler_path.write_text( + scheduler.replace(old_allowlist, new_allowlist, 1), + encoding="utf-8", + ) + + router_workflow_path = Path(".github/workflows/agent-mention-router.yml") + router_workflow = router_workflow_path.read_text(encoding="utf-8") + old_router_allowlist = ( + "OPENCODE_REPOSITORY_DISPATCH_TARGETS: " + "${{ vars.OPENCODE_REPOSITORY_DISPATCH_TARGETS }}" + ) + if router_workflow.count(old_router_allowlist) != 2: + raise SystemExit("router allowlist markers changed") + router_workflow_path.write_text( + router_workflow.replace( + old_router_allowlist, + old_router_allowlist + ",ContextualWisdomLab/LineageWeave", + ), + encoding="utf-8", + ) + + changelog_path = Path("CHANGELOG.md") + changelog = changelog_path.read_text(encoding="utf-8") + marker = "### Fixed\n\n" + entry = ( + "- OpenCode agent-mention dispatch now packs its fixed review-only controls " + "under one `review_policy` object so both repository-dispatch hops stay within " + "GitHub's ten-property `client_payload` limit, and LineageWeave is retained in " + "the source-controlled exact repository allowlist baseline.\n" + ) + if marker not in changelog: + raise SystemExit("CHANGELOG Fixed marker changed") + if entry not in changelog: + changelog = changelog.replace(marker, marker + entry, 1) + changelog_path.write_text(changelog, encoding="utf-8") + + doctoring_path = Path("docs/doctoring/required-review-check-is-not-a-verdict.md") + doctoring = doctoring_path.read_text(encoding="utf-8") + marker = "## Verification contract\n" + section = '''## Agent-mention transport contract + + GitHub accepts at most ten top-level properties in a + `repository_dispatch.client_payload`. The review-only OpenCode route keeps the + immutable PR and invocation identity fields at the top level and nests its five + fixed controls under `review_policy`. Both the comment router and the wrapper's + scheduler forward use this compact contract. LineageWeave is also appended to a + source-controlled exact allowlist baseline so a missing UI variable entry cannot + silently suppress review for the incident repository. + + ''' + if marker not in doctoring: + raise SystemExit("doctoring marker changed") + if section not in doctoring: + doctoring = doctoring.replace(marker, section + marker, 1) + doctoring_path.write_text(doctoring, encoding="utf-8") + PY + + - name: Verify focused and repository-wide quality gates + run: | + set -euo pipefail + python -m pytest \ + tests/test_agent_mention_router.py \ + tests/test_agent_mention_complete_payload_binding.py \ + tests/test_agent_mention_downstream_idempotency.py \ + tests/test_agent_mention_review_regressions.py + python -m coverage erase + python -m coverage run -m pytest tests + python -m coverage report --show-missing + python -m interrogate -c pyproject.toml scripts/ci + python -m compileall -q scripts tests + + - name: Remove one-shot workflow and publish verified repair + run: | + set -euo pipefail + rm .github/workflows/repair-agent-mention-payload-limit.yml + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + git add \ + scripts/ci/agent_mention_router.py \ + .github/workflows/agent-mention-opencode-dispatch.yml \ + .github/workflows/agent-mention-router.yml \ + .github/workflows/pr-review-merge-scheduler.yml \ + tests/test_agent_mention_router.py \ + tests/test_agent_mention_complete_payload_binding.py \ + docs/doctoring/required-review-check-is-not-a-verdict.md \ + CHANGELOG.md \ + .github/workflows/repair-agent-mention-payload-limit.yml + git diff --cached --check + git commit -m "fix(review): compact agent mention dispatch payload" + git push origin HEAD:fix/required-review-fail-closed-without-verdict From fa558eb313583e677ac03ad1277f7aa5ed2b31b0 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 14 Aug 2026 14:25:17 +0900 Subject: [PATCH 12/22] chore(review): remove superseded payload repair workflow --- .../repair-agent-mention-payload-limit.yml | 332 ------------------ 1 file changed, 332 deletions(-) delete mode 100644 .github/workflows/repair-agent-mention-payload-limit.yml diff --git a/.github/workflows/repair-agent-mention-payload-limit.yml b/.github/workflows/repair-agent-mention-payload-limit.yml deleted file mode 100644 index 25726dd10..000000000 --- a/.github/workflows/repair-agent-mention-payload-limit.yml +++ /dev/null @@ -1,332 +0,0 @@ -name: One-shot agent mention payload-limit repair - -on: - push: - branches: - - fix/required-review-fail-closed-without-verdict - -permissions: - contents: read - -concurrency: - group: one-shot-agent-mention-payload-limit-repair - cancel-in-progress: false - -jobs: - repair: - name: Repair agent mention payload contract - permissions: - contents: write - runs-on: ubuntu-latest - timeout-minutes: 35 - steps: - - name: Check out repair branch - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 - with: - ref: fix/required-review-fail-closed-without-verdict - fetch-depth: 0 - - - name: Install test tooling - run: | - set -euo pipefail - python -m pip install --disable-pip-version-check \ - 'pytest>=8.0.0' \ - 'pytest-cov>=7.1.0' \ - 'coverage[toml]>=7.8.0' \ - 'interrogate>=1.7.0' - - - name: Add failing wire-contract and allowlist tests - run: | - set -euo pipefail - python - <<'PY' - from pathlib import Path - - router_tests = Path("tests/test_agent_mention_router.py") - router_text = router_tests.read_text(encoding="utf-8") - old_router_assertions = ''' assert opencode["client_payload"]["merge_mode"] == "disabled" - assert opencode["client_payload"]["enable_auto_merge"] is False - assert opencode["client_payload"]["update_branches"] is False - ''' - new_router_assertions = ''' wire_payload = opencode["client_payload"] - assert len(wire_payload) == 10 - assert wire_payload["review_policy"] == { - "trigger_reviews": True, - "review_dispatch_limit": "1", - "enable_auto_merge": False, - "update_branches": False, - "merge_mode": "disabled", - } - for forbidden_flat_field in ( - "trigger_reviews", - "review_dispatch_limit", - "enable_auto_merge", - "update_branches", - "merge_mode", - ): - assert forbidden_flat_field not in wire_payload - ''' - if old_router_assertions not in router_text: - raise SystemExit("router payload assertion marker changed") - router_tests.write_text( - router_text.replace(old_router_assertions, new_router_assertions, 1), - encoding="utf-8", - ) - - binding_tests = Path("tests/test_agent_mention_complete_payload_binding.py") - binding_text = binding_tests.read_text(encoding="utf-8") - marker = "\ndef test_wrappers_recompute_complete_claim_before_ledger_access() -> None:\n" - new_tests = ''' - def test_opencode_wire_payload_and_forwarding_respect_github_limit() -> None: - """Both repository_dispatch hops use at most ten top-level properties.""" - - router = _load_router() - request = router.parse_event(_event()) - assert request is not None - payload = router.opencode_payload(request)["client_payload"] - assert len(payload) == 10 - assert payload["review_policy"] == { - "trigger_reviews": True, - "review_dispatch_limit": "1", - "enable_auto_merge": False, - "update_branches": False, - "merge_mode": "disabled", - } - - wrapper = OPENCODE_WORKFLOW.read_text(encoding="utf-8") - for field in ( - "trigger_reviews", - "review_dispatch_limit", - "enable_auto_merge", - "update_branches", - "merge_mode", - ): - assert f"github.event.client_payload.review_policy.{field}" in wrapper - assert "review_policy: {" in wrapper - assert "trigger_reviews: true" in wrapper - assert "review_dispatch_limit: \"1\"" in wrapper - assert "enable_auto_merge: false" in wrapper - assert "update_branches: false" in wrapper - assert "merge_mode: \"disabled\"" in wrapper - - scheduler = ( - ROOT / ".github" / "workflows" / "pr-review-merge-scheduler.yml" - ).read_text(encoding="utf-8") - assert "github.event.client_payload.review_policy.trigger_reviews" in scheduler - assert ( - "github.event.client_payload.review_policy.review_dispatch_limit" - in scheduler - ) - assert "github.event.client_payload.review_policy.enable_auto_merge" in scheduler - assert "github.event.client_payload.review_policy.update_branches" in scheduler - assert "github.event.client_payload.review_policy.merge_mode" in scheduler - - - def test_lineageweave_is_in_source_controlled_dispatch_baseline() -> None: - """The incident repository remains routable even if the UI variable drifts.""" - - router_workflow = ( - ROOT / ".github" / "workflows" / "agent-mention-router.yml" - ).read_text(encoding="utf-8") - scheduler_workflow = ( - ROOT / ".github" / "workflows" / "pr-review-merge-scheduler.yml" - ).read_text(encoding="utf-8") - baseline = ",ContextualWisdomLab/LineageWeave" - assert router_workflow.count( - "${{ vars.OPENCODE_REPOSITORY_DISPATCH_TARGETS }}" + baseline - ) == 2 - assert ( - "ALLOWED_TARGET_REPOSITORIES: " - "${{ vars.OPENCODE_REPOSITORY_DISPATCH_TARGETS }}" + baseline - ) in scheduler_workflow - - - ''' - if marker not in binding_text: - raise SystemExit("binding test marker changed") - binding_tests.write_text( - binding_text.replace(marker, "\n" + new_tests + marker.lstrip("\n"), 1), - encoding="utf-8", - ) - PY - - - name: Prove the incident contracts are RED - run: | - set -euo pipefail - set +e - python -m pytest \ - tests/test_agent_mention_router.py::test_eligible_agents_and_payloads \ - tests/test_agent_mention_complete_payload_binding.py::test_opencode_wire_payload_and_forwarding_respect_github_limit \ - tests/test_agent_mention_complete_payload_binding.py::test_lineageweave_is_in_source_controlled_dispatch_baseline \ - >/tmp/agent-mention-red.log 2>&1 - status=$? - set -e - cat /tmp/agent-mention-red.log - if [ "$status" -eq 0 ]; then - echo "::error::The repository_dispatch overflow and allowlist regressions unexpectedly passed before implementation." - exit 1 - fi - - - name: Compact review policy and restore LineageWeave routing - run: | - set -euo pipefail - python - <<'PY' - from pathlib import Path - - router_path = Path("scripts/ci/agent_mention_router.py") - router = router_path.read_text(encoding="utf-8") - old_payload = ''' "trigger_reviews": claim["trigger_reviews"], - "review_dispatch_limit": claim["review_dispatch_limit"], - "enable_auto_merge": claim["enable_auto_merge"], - "update_branches": claim["update_branches"], - "merge_mode": claim["merge_mode"], - ''' - new_payload = ''' "review_policy": { - "trigger_reviews": claim["trigger_reviews"], - "review_dispatch_limit": claim["review_dispatch_limit"], - "enable_auto_merge": claim["enable_auto_merge"], - "update_branches": claim["update_branches"], - "merge_mode": claim["merge_mode"], - }, - ''' - if old_payload not in router: - raise SystemExit("OpenCode wire payload marker changed") - router_path.write_text(router.replace(old_payload, new_payload, 1), encoding="utf-8") - - wrapper_path = Path(".github/workflows/agent-mention-opencode-dispatch.yml") - wrapper = wrapper_path.read_text(encoding="utf-8") - env_replacements = { - "github.event.client_payload.trigger_reviews": "github.event.client_payload.review_policy.trigger_reviews", - "github.event.client_payload.review_dispatch_limit": "github.event.client_payload.review_policy.review_dispatch_limit", - "github.event.client_payload.enable_auto_merge": "github.event.client_payload.review_policy.enable_auto_merge", - "github.event.client_payload.update_branches": "github.event.client_payload.review_policy.update_branches", - "github.event.client_payload.merge_mode": "github.event.client_payload.review_policy.merge_mode", - } - for old, new in env_replacements.items(): - if old not in wrapper: - raise SystemExit(f"wrapper payload marker changed: {old}") - wrapper = wrapper.replace(old, new) - old_forward = ''' trigger_reviews: true, - review_dispatch_limit: "1", - enable_auto_merge: false, - update_branches: false, - merge_mode: "disabled", - ''' - new_forward = ''' review_policy: { - trigger_reviews: true, - review_dispatch_limit: "1", - enable_auto_merge: false, - update_branches: false, - merge_mode: "disabled" - }, - ''' - if old_forward not in wrapper: - raise SystemExit("wrapper scheduler-forward marker changed") - wrapper_path.write_text(wrapper.replace(old_forward, new_forward, 1), encoding="utf-8") - - scheduler_path = Path(".github/workflows/pr-review-merge-scheduler.yml") - scheduler = scheduler_path.read_text(encoding="utf-8") - scheduler_replacements = { - "github.event.client_payload.trigger_reviews != false": "github.event.client_payload.review_policy.trigger_reviews != false && github.event.client_payload.trigger_reviews != false", - "github.event.client_payload.review_dispatch_limit || inputs.review_dispatch_limit": "github.event.client_payload.review_policy.review_dispatch_limit || github.event.client_payload.review_dispatch_limit || inputs.review_dispatch_limit", - "github.event.client_payload.enable_auto_merge != false": "github.event.client_payload.review_policy.enable_auto_merge != false && github.event.client_payload.enable_auto_merge != false", - "github.event.client_payload.merge_mode || inputs.merge_mode": "github.event.client_payload.review_policy.merge_mode || github.event.client_payload.merge_mode || inputs.merge_mode", - "github.event.client_payload.update_branches != false": "github.event.client_payload.review_policy.update_branches != false && github.event.client_payload.update_branches != false", - } - for old, new in scheduler_replacements.items(): - if old not in scheduler: - raise SystemExit(f"scheduler payload marker changed: {old}") - scheduler = scheduler.replace(old, new, 1) - old_allowlist = "ALLOWED_TARGET_REPOSITORIES: ${{ vars.OPENCODE_REPOSITORY_DISPATCH_TARGETS }}" - new_allowlist = old_allowlist + ",ContextualWisdomLab/LineageWeave" - if old_allowlist not in scheduler: - raise SystemExit("scheduler allowlist marker changed") - scheduler_path.write_text( - scheduler.replace(old_allowlist, new_allowlist, 1), - encoding="utf-8", - ) - - router_workflow_path = Path(".github/workflows/agent-mention-router.yml") - router_workflow = router_workflow_path.read_text(encoding="utf-8") - old_router_allowlist = ( - "OPENCODE_REPOSITORY_DISPATCH_TARGETS: " - "${{ vars.OPENCODE_REPOSITORY_DISPATCH_TARGETS }}" - ) - if router_workflow.count(old_router_allowlist) != 2: - raise SystemExit("router allowlist markers changed") - router_workflow_path.write_text( - router_workflow.replace( - old_router_allowlist, - old_router_allowlist + ",ContextualWisdomLab/LineageWeave", - ), - encoding="utf-8", - ) - - changelog_path = Path("CHANGELOG.md") - changelog = changelog_path.read_text(encoding="utf-8") - marker = "### Fixed\n\n" - entry = ( - "- OpenCode agent-mention dispatch now packs its fixed review-only controls " - "under one `review_policy` object so both repository-dispatch hops stay within " - "GitHub's ten-property `client_payload` limit, and LineageWeave is retained in " - "the source-controlled exact repository allowlist baseline.\n" - ) - if marker not in changelog: - raise SystemExit("CHANGELOG Fixed marker changed") - if entry not in changelog: - changelog = changelog.replace(marker, marker + entry, 1) - changelog_path.write_text(changelog, encoding="utf-8") - - doctoring_path = Path("docs/doctoring/required-review-check-is-not-a-verdict.md") - doctoring = doctoring_path.read_text(encoding="utf-8") - marker = "## Verification contract\n" - section = '''## Agent-mention transport contract - - GitHub accepts at most ten top-level properties in a - `repository_dispatch.client_payload`. The review-only OpenCode route keeps the - immutable PR and invocation identity fields at the top level and nests its five - fixed controls under `review_policy`. Both the comment router and the wrapper's - scheduler forward use this compact contract. LineageWeave is also appended to a - source-controlled exact allowlist baseline so a missing UI variable entry cannot - silently suppress review for the incident repository. - - ''' - if marker not in doctoring: - raise SystemExit("doctoring marker changed") - if section not in doctoring: - doctoring = doctoring.replace(marker, section + marker, 1) - doctoring_path.write_text(doctoring, encoding="utf-8") - PY - - - name: Verify focused and repository-wide quality gates - run: | - set -euo pipefail - python -m pytest \ - tests/test_agent_mention_router.py \ - tests/test_agent_mention_complete_payload_binding.py \ - tests/test_agent_mention_downstream_idempotency.py \ - tests/test_agent_mention_review_regressions.py - python -m coverage erase - python -m coverage run -m pytest tests - python -m coverage report --show-missing - python -m interrogate -c pyproject.toml scripts/ci - python -m compileall -q scripts tests - - - name: Remove one-shot workflow and publish verified repair - run: | - set -euo pipefail - rm .github/workflows/repair-agent-mention-payload-limit.yml - git config user.name "github-actions[bot]" - git config user.email "41898282+github-actions[bot]@users.noreply.github.com" - git add \ - scripts/ci/agent_mention_router.py \ - .github/workflows/agent-mention-opencode-dispatch.yml \ - .github/workflows/agent-mention-router.yml \ - .github/workflows/pr-review-merge-scheduler.yml \ - tests/test_agent_mention_router.py \ - tests/test_agent_mention_complete_payload_binding.py \ - docs/doctoring/required-review-check-is-not-a-verdict.md \ - CHANGELOG.md \ - .github/workflows/repair-agent-mention-payload-limit.yml - git diff --cached --check - git commit -m "fix(review): compact agent mention dispatch payload" - git push origin HEAD:fix/required-review-fail-closed-without-verdict From 82d9552cfabe3f97d22a34db9799757b66901e18 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 14 Aug 2026 14:33:48 +0900 Subject: [PATCH 13/22] test(review): prove LineageWeave remains centrally routable --- .../repair-lineageweave-opencode-route.yml | 187 ++++++++++++++++++ 1 file changed, 187 insertions(+) create mode 100644 .github/workflows/repair-lineageweave-opencode-route.yml diff --git a/.github/workflows/repair-lineageweave-opencode-route.yml b/.github/workflows/repair-lineageweave-opencode-route.yml new file mode 100644 index 000000000..80f3a6fd1 --- /dev/null +++ b/.github/workflows/repair-lineageweave-opencode-route.yml @@ -0,0 +1,187 @@ +name: One-shot LineageWeave OpenCode route repair + +on: + push: + branches: + - fix/required-review-fail-closed-without-verdict + +permissions: + contents: read + +concurrency: + group: one-shot-lineageweave-opencode-route-repair + cancel-in-progress: false + +jobs: + repair: + name: Repair LineageWeave OpenCode route + permissions: + contents: write + runs-on: ubuntu-latest + timeout-minutes: 35 + steps: + - name: Check out repair branch + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 + with: + ref: fix/required-review-fail-closed-without-verdict + fetch-depth: 0 + + - name: Install test tooling + run: | + set -euo pipefail + python -m pip install --disable-pip-version-check \ + 'pytest>=8.0.0' \ + 'pytest-cov>=7.1.0' \ + 'coverage[toml]>=7.8.0' \ + 'interrogate>=1.7.0' + + - name: Add the route regression contract + run: | + set -euo pipefail + cat > tests/test_lineageweave_opencode_route.py <<'PY' + """Regression contract for the LineageWeave central review route.""" + + from pathlib import Path + + + ROOT = Path(__file__).resolve().parents[1] + VARIABLE_EXPRESSION = "$" + "{{ vars.OPENCODE_REPOSITORY_DISPATCH_TARGETS }}" + LINEAGEWEAVE_SUFFIX = ",ContextualWisdomLab/LineageWeave" + ROUTE_VALUE = VARIABLE_EXPRESSION + LINEAGEWEAVE_SUFFIX + + + def test_lineageweave_is_in_every_central_opencode_route() -> None: + """Keep LineageWeave routable when the mutable repository variable drifts.""" + + router = ( + ROOT / ".github" / "workflows" / "agent-mention-router.yml" + ).read_text(encoding="utf-8") + scheduler = ( + ROOT / ".github" / "workflows" / "pr-review-merge-scheduler.yml" + ).read_text(encoding="utf-8") + opencode = ( + ROOT / ".github" / "workflows" / "opencode-review-dispatch.yml" + ).read_text(encoding="utf-8") + + assert router.count( + "OPENCODE_REPOSITORY_DISPATCH_TARGETS: " + ROUTE_VALUE + ) == 2 + assert ( + "ALLOWED_TARGET_REPOSITORIES: " + ROUTE_VALUE + ) in scheduler + assert "ALLOWED_DISPATCH_TARGETS: " + ROUTE_VALUE in opencode + PY + + - name: Prove the route contract is RED + run: | + set -euo pipefail + set +e + python -m pytest tests/test_lineageweave_opencode_route.py \ + >/tmp/lineageweave-route-red.log 2>&1 + status=$? + set -e + cat /tmp/lineageweave-route-red.log + if [ "$status" -eq 0 ]; then + echo "::error::The LineageWeave route regression unexpectedly passed before implementation." + exit 1 + fi + + - name: Add a source-controlled LineageWeave routing floor + run: | + set -euo pipefail + python - <<'PY' + from pathlib import Path + + expression = "$" + "{{ vars.OPENCODE_REPOSITORY_DISPATCH_TARGETS }}" + suffix = ",ContextualWisdomLab/LineageWeave" + route = expression + suffix + + replacements = { + Path(".github/workflows/agent-mention-router.yml"): ( + "OPENCODE_REPOSITORY_DISPATCH_TARGETS: " + expression, + "OPENCODE_REPOSITORY_DISPATCH_TARGETS: " + route, + 2, + ), + Path(".github/workflows/pr-review-merge-scheduler.yml"): ( + "ALLOWED_TARGET_REPOSITORIES: " + expression, + "ALLOWED_TARGET_REPOSITORIES: " + route, + 1, + ), + Path(".github/workflows/opencode-review-dispatch.yml"): ( + "ALLOWED_DISPATCH_TARGETS: " + expression, + "ALLOWED_DISPATCH_TARGETS: " + route, + 1, + ), + } + for path, (old, new, expected_count) in replacements.items(): + text = path.read_text(encoding="utf-8") + if text.count(old) != expected_count: + raise SystemExit( + f"{path} route marker count changed: " + f"expected {expected_count}, found {text.count(old)}" + ) + path.write_text(text.replace(old, new), encoding="utf-8") + + changelog_path = Path("CHANGELOG.md") + changelog = changelog_path.read_text(encoding="utf-8") + marker = "### Fixed\n\n" + entry = ( + "- LineageWeave is now retained in the source-controlled exact " + "OpenCode dispatch allowlist floor, so mutable repository-variable " + "drift cannot silently suppress its central review route.\n" + ) + if marker not in changelog: + raise SystemExit("CHANGELOG Fixed marker changed") + if entry not in changelog: + changelog = changelog.replace(marker, marker + entry, 1) + changelog_path.write_text(changelog, encoding="utf-8") + + doctoring_path = Path( + "docs/doctoring/required-review-check-is-not-a-verdict.md" + ) + doctoring = doctoring_path.read_text(encoding="utf-8") + marker = "## Verification contract\n" + section = '''## LineageWeave route contract + + The mutable `OPENCODE_REPOSITORY_DISPATCH_TARGETS` variable remains the + organization-managed allowlist, but LineageWeave is appended as a + source-controlled exact routing floor in the comment router, scheduler, + and privileged OpenCode dispatch validator. This prevents a missing UI + variable entry from silently converting a valid review request into a + non-event while retaining exact repository matching. + + ''' + if marker not in doctoring: + raise SystemExit("doctoring verification marker changed") + if section not in doctoring: + doctoring = doctoring.replace(marker, section + marker, 1) + doctoring_path.write_text(doctoring, encoding="utf-8") + PY + + - name: Verify focused and repository-wide quality gates + run: | + set -euo pipefail + python -m pytest tests/test_lineageweave_opencode_route.py + python -m coverage erase + python -m coverage run -m pytest tests + python -m coverage report --show-missing + python -m interrogate -c pyproject.toml scripts/ci + python -m compileall -q scripts tests + + - name: Remove one-shot workflow and publish verified repair + run: | + set -euo pipefail + rm .github/workflows/repair-lineageweave-opencode-route.yml + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + git add \ + .github/workflows/agent-mention-router.yml \ + .github/workflows/pr-review-merge-scheduler.yml \ + .github/workflows/opencode-review-dispatch.yml \ + tests/test_lineageweave_opencode_route.py \ + docs/doctoring/required-review-check-is-not-a-verdict.md \ + CHANGELOG.md \ + .github/workflows/repair-lineageweave-opencode-route.yml + git diff --cached --check + git commit -m "fix(review): retain LineageWeave in central dispatch routes" + git push origin HEAD:fix/required-review-fail-closed-without-verdict From 2de7a3cd763c5cb9a98bb9d7b648cd6e6ecb8248 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 14 Aug 2026 14:37:00 +0900 Subject: [PATCH 14/22] fix(ci): publish verified LineageWeave route with workflow token --- .../publish-lineageweave-opencode-route.yml | 171 ++++++++++++++++++ 1 file changed, 171 insertions(+) create mode 100644 .github/workflows/publish-lineageweave-opencode-route.yml diff --git a/.github/workflows/publish-lineageweave-opencode-route.yml b/.github/workflows/publish-lineageweave-opencode-route.yml new file mode 100644 index 000000000..9bab7e503 --- /dev/null +++ b/.github/workflows/publish-lineageweave-opencode-route.yml @@ -0,0 +1,171 @@ +name: Publish verified LineageWeave OpenCode route + +on: + push: + branches: + - fix/required-review-fail-closed-without-verdict + +permissions: + contents: read + +concurrency: + group: publish-lineageweave-opencode-route + cancel-in-progress: false + +jobs: + publish: + name: Publish verified route repair + runs-on: ubuntu-latest + timeout-minutes: 35 + steps: + - name: Check out repair branch with workflow-capable token + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 + with: + ref: fix/required-review-fail-closed-without-verdict + fetch-depth: 0 + token: ${{ secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN || github.token }} + + - name: Install test tooling + run: | + set -euo pipefail + python -m pip install --disable-pip-version-check \ + 'pytest>=8.0.0' \ + 'pytest-cov>=7.1.0' \ + 'coverage[toml]>=7.8.0' \ + 'interrogate>=1.7.0' + + - name: Apply the already RED-proven route repair + run: | + set -euo pipefail + cat > tests/test_lineageweave_opencode_route.py <<'PY' + """Regression contract for the LineageWeave central review route.""" + + from pathlib import Path + + + ROOT = Path(__file__).resolve().parents[1] + VARIABLE_EXPRESSION = "$" + "{{ vars.OPENCODE_REPOSITORY_DISPATCH_TARGETS }}" + LINEAGEWEAVE_SUFFIX = ",ContextualWisdomLab/LineageWeave" + ROUTE_VALUE = VARIABLE_EXPRESSION + LINEAGEWEAVE_SUFFIX + + + def test_lineageweave_is_in_every_central_opencode_route() -> None: + """Keep LineageWeave routable when the mutable repository variable drifts.""" + + router = ( + ROOT / ".github" / "workflows" / "agent-mention-router.yml" + ).read_text(encoding="utf-8") + scheduler = ( + ROOT / ".github" / "workflows" / "pr-review-merge-scheduler.yml" + ).read_text(encoding="utf-8") + opencode = ( + ROOT / ".github" / "workflows" / "opencode-review-dispatch.yml" + ).read_text(encoding="utf-8") + + assert router.count( + "OPENCODE_REPOSITORY_DISPATCH_TARGETS: " + ROUTE_VALUE + ) == 2 + assert "ALLOWED_TARGET_REPOSITORIES: " + ROUTE_VALUE in scheduler + assert "ALLOWED_DISPATCH_TARGETS: " + ROUTE_VALUE in opencode + PY + + python - <<'PY' + from pathlib import Path + + expression = "$" + "{{ vars.OPENCODE_REPOSITORY_DISPATCH_TARGETS }}" + suffix = ",ContextualWisdomLab/LineageWeave" + route = expression + suffix + replacements = { + Path(".github/workflows/agent-mention-router.yml"): ( + "OPENCODE_REPOSITORY_DISPATCH_TARGETS: " + expression, + "OPENCODE_REPOSITORY_DISPATCH_TARGETS: " + route, + 2, + ), + Path(".github/workflows/pr-review-merge-scheduler.yml"): ( + "ALLOWED_TARGET_REPOSITORIES: " + expression, + "ALLOWED_TARGET_REPOSITORIES: " + route, + 1, + ), + Path(".github/workflows/opencode-review-dispatch.yml"): ( + "ALLOWED_DISPATCH_TARGETS: " + expression, + "ALLOWED_DISPATCH_TARGETS: " + route, + 1, + ), + } + for path, (old, new, expected_count) in replacements.items(): + text = path.read_text(encoding="utf-8") + if text.count(new) == expected_count: + continue + if text.count(old) != expected_count: + raise SystemExit( + f"{path} route marker count changed: expected " + f"{expected_count}, found {text.count(old)}" + ) + path.write_text(text.replace(old, new), encoding="utf-8") + + changelog_path = Path("CHANGELOG.md") + changelog = changelog_path.read_text(encoding="utf-8") + marker = "### Fixed\n\n" + entry = ( + "- LineageWeave is now retained in the source-controlled exact " + "OpenCode dispatch allowlist floor, so mutable repository-variable " + "drift cannot silently suppress its central review route.\n" + ) + if marker not in changelog: + raise SystemExit("CHANGELOG Fixed marker changed") + if entry not in changelog: + changelog = changelog.replace(marker, marker + entry, 1) + changelog_path.write_text(changelog, encoding="utf-8") + + doctoring_path = Path( + "docs/doctoring/required-review-check-is-not-a-verdict.md" + ) + doctoring = doctoring_path.read_text(encoding="utf-8") + marker = "## Verification contract\n" + section = '''## LineageWeave route contract + + The mutable `OPENCODE_REPOSITORY_DISPATCH_TARGETS` variable remains the + organization-managed allowlist, but LineageWeave is appended as a + source-controlled exact routing floor in the comment router, scheduler, + and privileged OpenCode dispatch validator. This prevents a missing UI + variable entry from silently converting a valid review request into a + non-event while retaining exact repository matching. + + ''' + if marker not in doctoring: + raise SystemExit("doctoring verification marker changed") + if section not in doctoring: + doctoring = doctoring.replace(marker, section + marker, 1) + doctoring_path.write_text(doctoring, encoding="utf-8") + PY + + - name: Reverify the exact source + run: | + set -euo pipefail + python -m pytest tests/test_lineageweave_opencode_route.py + python -m coverage erase + python -m coverage run -m pytest tests + python -m coverage report --show-missing + python -m interrogate -c pyproject.toml scripts/ci + python -m compileall -q scripts tests + + - name: Remove one-shot workflows and publish + run: | + set -euo pipefail + rm -f \ + .github/workflows/repair-lineageweave-opencode-route.yml \ + .github/workflows/publish-lineageweave-opencode-route.yml + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + git add \ + .github/workflows/agent-mention-router.yml \ + .github/workflows/pr-review-merge-scheduler.yml \ + .github/workflows/opencode-review-dispatch.yml \ + tests/test_lineageweave_opencode_route.py \ + docs/doctoring/required-review-check-is-not-a-verdict.md \ + CHANGELOG.md \ + .github/workflows/repair-lineageweave-opencode-route.yml \ + .github/workflows/publish-lineageweave-opencode-route.yml + git diff --cached --check + git commit -m "fix(review): retain LineageWeave in central dispatch routes" + git push origin HEAD:fix/required-review-fail-closed-without-verdict From 9b056c59f955fd3b6a30d2b1fd29219a24c6a34b Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 14 Aug 2026 14:42:17 +0900 Subject: [PATCH 15/22] test(review): export verified LineageWeave route patch --- .../export-lineageweave-opencode-route.yml | 172 ++++++++++++++++++ 1 file changed, 172 insertions(+) create mode 100644 .github/workflows/export-lineageweave-opencode-route.yml diff --git a/.github/workflows/export-lineageweave-opencode-route.yml b/.github/workflows/export-lineageweave-opencode-route.yml new file mode 100644 index 000000000..907e5aef7 --- /dev/null +++ b/.github/workflows/export-lineageweave-opencode-route.yml @@ -0,0 +1,172 @@ +name: Export verified LineageWeave OpenCode route + +on: + push: + branches: + - fix/required-review-fail-closed-without-verdict + +permissions: + contents: read + +concurrency: + group: export-lineageweave-opencode-route + cancel-in-progress: false + +jobs: + export: + name: Export verified route patch + runs-on: ubuntu-latest + timeout-minutes: 35 + steps: + - name: Check out repair branch + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 + with: + ref: fix/required-review-fail-closed-without-verdict + fetch-depth: 0 + persist-credentials: false + + - name: Install test tooling + run: | + set -euo pipefail + python -m pip install --disable-pip-version-check \ + 'pytest>=8.0.0' \ + 'pytest-cov>=7.1.0' \ + 'coverage[toml]>=7.8.0' \ + 'interrogate>=1.7.0' + + - name: Apply the RED-proven route repair + run: | + set -euo pipefail + cat > tests/test_lineageweave_opencode_route.py <<'PY' + """Regression contract for the LineageWeave central review route.""" + + from pathlib import Path + + + ROOT = Path(__file__).resolve().parents[1] + VARIABLE_EXPRESSION = "$" + "{{ vars.OPENCODE_REPOSITORY_DISPATCH_TARGETS }}" + LINEAGEWEAVE_SUFFIX = ",ContextualWisdomLab/LineageWeave" + ROUTE_VALUE = VARIABLE_EXPRESSION + LINEAGEWEAVE_SUFFIX + + + def test_lineageweave_is_in_every_central_opencode_route() -> None: + """Keep LineageWeave routable when the mutable repository variable drifts.""" + + router = ( + ROOT / ".github" / "workflows" / "agent-mention-router.yml" + ).read_text(encoding="utf-8") + scheduler = ( + ROOT / ".github" / "workflows" / "pr-review-merge-scheduler.yml" + ).read_text(encoding="utf-8") + opencode = ( + ROOT / ".github" / "workflows" / "opencode-review-dispatch.yml" + ).read_text(encoding="utf-8") + + assert router.count( + "OPENCODE_REPOSITORY_DISPATCH_TARGETS: " + ROUTE_VALUE + ) == 2 + assert "ALLOWED_TARGET_REPOSITORIES: " + ROUTE_VALUE in scheduler + assert "ALLOWED_DISPATCH_TARGETS: " + ROUTE_VALUE in opencode + PY + + python - <<'PY' + from pathlib import Path + + expression = "$" + "{{ vars.OPENCODE_REPOSITORY_DISPATCH_TARGETS }}" + suffix = ",ContextualWisdomLab/LineageWeave" + route = expression + suffix + replacements = { + Path(".github/workflows/agent-mention-router.yml"): ( + "OPENCODE_REPOSITORY_DISPATCH_TARGETS: " + expression, + "OPENCODE_REPOSITORY_DISPATCH_TARGETS: " + route, + 2, + ), + Path(".github/workflows/pr-review-merge-scheduler.yml"): ( + "ALLOWED_TARGET_REPOSITORIES: " + expression, + "ALLOWED_TARGET_REPOSITORIES: " + route, + 1, + ), + Path(".github/workflows/opencode-review-dispatch.yml"): ( + "ALLOWED_DISPATCH_TARGETS: " + expression, + "ALLOWED_DISPATCH_TARGETS: " + route, + 1, + ), + } + for path, (old, new, expected_count) in replacements.items(): + text = path.read_text(encoding="utf-8") + if text.count(new) == expected_count: + continue + if text.count(old) != expected_count: + raise SystemExit( + f"{path} route marker count changed: expected " + f"{expected_count}, found {text.count(old)}" + ) + path.write_text(text.replace(old, new), encoding="utf-8") + + changelog_path = Path("CHANGELOG.md") + changelog = changelog_path.read_text(encoding="utf-8") + marker = "### Fixed\n\n" + entry = ( + "- LineageWeave is now retained in the source-controlled exact " + "OpenCode dispatch allowlist floor, so mutable repository-variable " + "drift cannot silently suppress its central review route.\n" + ) + if marker not in changelog: + raise SystemExit("CHANGELOG Fixed marker changed") + if entry not in changelog: + changelog = changelog.replace(marker, marker + entry, 1) + changelog_path.write_text(changelog, encoding="utf-8") + + doctoring_path = Path( + "docs/doctoring/required-review-check-is-not-a-verdict.md" + ) + doctoring = doctoring_path.read_text(encoding="utf-8") + marker = "## Verification contract\n" + section = '''## LineageWeave route contract + + The mutable `OPENCODE_REPOSITORY_DISPATCH_TARGETS` variable remains the + organization-managed allowlist, but LineageWeave is appended as a + source-controlled exact routing floor in the comment router, scheduler, + and privileged OpenCode dispatch validator. This prevents a missing UI + variable entry from silently converting a valid review request into a + non-event while retaining exact repository matching. + + ''' + if marker not in doctoring: + raise SystemExit("doctoring verification marker changed") + if section not in doctoring: + doctoring = doctoring.replace(marker, section + marker, 1) + doctoring_path.write_text(doctoring, encoding="utf-8") + PY + + - name: Verify exact patched source + run: | + set -euo pipefail + python -m pytest tests/test_lineageweave_opencode_route.py + python -m coverage erase + python -m coverage run -m pytest tests + python -m coverage report --show-missing + python -m interrogate -c pyproject.toml scripts/ci + python -m compileall -q scripts tests + + - name: Stage patch artifact + run: | + set -euo pipefail + mkdir -p \ + route-patch/.github/workflows \ + route-patch/tests \ + route-patch/docs/doctoring + cp .github/workflows/agent-mention-router.yml route-patch/.github/workflows/ + cp .github/workflows/pr-review-merge-scheduler.yml route-patch/.github/workflows/ + cp .github/workflows/opencode-review-dispatch.yml route-patch/.github/workflows/ + cp tests/test_lineageweave_opencode_route.py route-patch/tests/ + cp docs/doctoring/required-review-check-is-not-a-verdict.md route-patch/docs/doctoring/ + cp CHANGELOG.md route-patch/ + + - name: Upload verified patch artifact + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 + with: + name: lineageweave-opencode-route-patch + path: route-patch + if-no-files-found: error + retention-days: 1 From 3ddfc827f262cd4684f44c7e6bdc98004fba61fa Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 14 Aug 2026 16:59:30 +0900 Subject: [PATCH 16/22] chore(review): remove superseded one-shot route repair --- .../repair-lineageweave-opencode-route.yml | 187 ------------------ 1 file changed, 187 deletions(-) delete mode 100644 .github/workflows/repair-lineageweave-opencode-route.yml diff --git a/.github/workflows/repair-lineageweave-opencode-route.yml b/.github/workflows/repair-lineageweave-opencode-route.yml deleted file mode 100644 index 80f3a6fd1..000000000 --- a/.github/workflows/repair-lineageweave-opencode-route.yml +++ /dev/null @@ -1,187 +0,0 @@ -name: One-shot LineageWeave OpenCode route repair - -on: - push: - branches: - - fix/required-review-fail-closed-without-verdict - -permissions: - contents: read - -concurrency: - group: one-shot-lineageweave-opencode-route-repair - cancel-in-progress: false - -jobs: - repair: - name: Repair LineageWeave OpenCode route - permissions: - contents: write - runs-on: ubuntu-latest - timeout-minutes: 35 - steps: - - name: Check out repair branch - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 - with: - ref: fix/required-review-fail-closed-without-verdict - fetch-depth: 0 - - - name: Install test tooling - run: | - set -euo pipefail - python -m pip install --disable-pip-version-check \ - 'pytest>=8.0.0' \ - 'pytest-cov>=7.1.0' \ - 'coverage[toml]>=7.8.0' \ - 'interrogate>=1.7.0' - - - name: Add the route regression contract - run: | - set -euo pipefail - cat > tests/test_lineageweave_opencode_route.py <<'PY' - """Regression contract for the LineageWeave central review route.""" - - from pathlib import Path - - - ROOT = Path(__file__).resolve().parents[1] - VARIABLE_EXPRESSION = "$" + "{{ vars.OPENCODE_REPOSITORY_DISPATCH_TARGETS }}" - LINEAGEWEAVE_SUFFIX = ",ContextualWisdomLab/LineageWeave" - ROUTE_VALUE = VARIABLE_EXPRESSION + LINEAGEWEAVE_SUFFIX - - - def test_lineageweave_is_in_every_central_opencode_route() -> None: - """Keep LineageWeave routable when the mutable repository variable drifts.""" - - router = ( - ROOT / ".github" / "workflows" / "agent-mention-router.yml" - ).read_text(encoding="utf-8") - scheduler = ( - ROOT / ".github" / "workflows" / "pr-review-merge-scheduler.yml" - ).read_text(encoding="utf-8") - opencode = ( - ROOT / ".github" / "workflows" / "opencode-review-dispatch.yml" - ).read_text(encoding="utf-8") - - assert router.count( - "OPENCODE_REPOSITORY_DISPATCH_TARGETS: " + ROUTE_VALUE - ) == 2 - assert ( - "ALLOWED_TARGET_REPOSITORIES: " + ROUTE_VALUE - ) in scheduler - assert "ALLOWED_DISPATCH_TARGETS: " + ROUTE_VALUE in opencode - PY - - - name: Prove the route contract is RED - run: | - set -euo pipefail - set +e - python -m pytest tests/test_lineageweave_opencode_route.py \ - >/tmp/lineageweave-route-red.log 2>&1 - status=$? - set -e - cat /tmp/lineageweave-route-red.log - if [ "$status" -eq 0 ]; then - echo "::error::The LineageWeave route regression unexpectedly passed before implementation." - exit 1 - fi - - - name: Add a source-controlled LineageWeave routing floor - run: | - set -euo pipefail - python - <<'PY' - from pathlib import Path - - expression = "$" + "{{ vars.OPENCODE_REPOSITORY_DISPATCH_TARGETS }}" - suffix = ",ContextualWisdomLab/LineageWeave" - route = expression + suffix - - replacements = { - Path(".github/workflows/agent-mention-router.yml"): ( - "OPENCODE_REPOSITORY_DISPATCH_TARGETS: " + expression, - "OPENCODE_REPOSITORY_DISPATCH_TARGETS: " + route, - 2, - ), - Path(".github/workflows/pr-review-merge-scheduler.yml"): ( - "ALLOWED_TARGET_REPOSITORIES: " + expression, - "ALLOWED_TARGET_REPOSITORIES: " + route, - 1, - ), - Path(".github/workflows/opencode-review-dispatch.yml"): ( - "ALLOWED_DISPATCH_TARGETS: " + expression, - "ALLOWED_DISPATCH_TARGETS: " + route, - 1, - ), - } - for path, (old, new, expected_count) in replacements.items(): - text = path.read_text(encoding="utf-8") - if text.count(old) != expected_count: - raise SystemExit( - f"{path} route marker count changed: " - f"expected {expected_count}, found {text.count(old)}" - ) - path.write_text(text.replace(old, new), encoding="utf-8") - - changelog_path = Path("CHANGELOG.md") - changelog = changelog_path.read_text(encoding="utf-8") - marker = "### Fixed\n\n" - entry = ( - "- LineageWeave is now retained in the source-controlled exact " - "OpenCode dispatch allowlist floor, so mutable repository-variable " - "drift cannot silently suppress its central review route.\n" - ) - if marker not in changelog: - raise SystemExit("CHANGELOG Fixed marker changed") - if entry not in changelog: - changelog = changelog.replace(marker, marker + entry, 1) - changelog_path.write_text(changelog, encoding="utf-8") - - doctoring_path = Path( - "docs/doctoring/required-review-check-is-not-a-verdict.md" - ) - doctoring = doctoring_path.read_text(encoding="utf-8") - marker = "## Verification contract\n" - section = '''## LineageWeave route contract - - The mutable `OPENCODE_REPOSITORY_DISPATCH_TARGETS` variable remains the - organization-managed allowlist, but LineageWeave is appended as a - source-controlled exact routing floor in the comment router, scheduler, - and privileged OpenCode dispatch validator. This prevents a missing UI - variable entry from silently converting a valid review request into a - non-event while retaining exact repository matching. - - ''' - if marker not in doctoring: - raise SystemExit("doctoring verification marker changed") - if section not in doctoring: - doctoring = doctoring.replace(marker, section + marker, 1) - doctoring_path.write_text(doctoring, encoding="utf-8") - PY - - - name: Verify focused and repository-wide quality gates - run: | - set -euo pipefail - python -m pytest tests/test_lineageweave_opencode_route.py - python -m coverage erase - python -m coverage run -m pytest tests - python -m coverage report --show-missing - python -m interrogate -c pyproject.toml scripts/ci - python -m compileall -q scripts tests - - - name: Remove one-shot workflow and publish verified repair - run: | - set -euo pipefail - rm .github/workflows/repair-lineageweave-opencode-route.yml - git config user.name "github-actions[bot]" - git config user.email "41898282+github-actions[bot]@users.noreply.github.com" - git add \ - .github/workflows/agent-mention-router.yml \ - .github/workflows/pr-review-merge-scheduler.yml \ - .github/workflows/opencode-review-dispatch.yml \ - tests/test_lineageweave_opencode_route.py \ - docs/doctoring/required-review-check-is-not-a-verdict.md \ - CHANGELOG.md \ - .github/workflows/repair-lineageweave-opencode-route.yml - git diff --cached --check - git commit -m "fix(review): retain LineageWeave in central dispatch routes" - git push origin HEAD:fix/required-review-fail-closed-without-verdict From 0fbede022fcd771ac4adc9139fab2ad1bcab9315 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 14 Aug 2026 16:59:39 +0900 Subject: [PATCH 17/22] chore(review): remove superseded route publisher --- .../publish-lineageweave-opencode-route.yml | 171 ------------------ 1 file changed, 171 deletions(-) delete mode 100644 .github/workflows/publish-lineageweave-opencode-route.yml diff --git a/.github/workflows/publish-lineageweave-opencode-route.yml b/.github/workflows/publish-lineageweave-opencode-route.yml deleted file mode 100644 index 9bab7e503..000000000 --- a/.github/workflows/publish-lineageweave-opencode-route.yml +++ /dev/null @@ -1,171 +0,0 @@ -name: Publish verified LineageWeave OpenCode route - -on: - push: - branches: - - fix/required-review-fail-closed-without-verdict - -permissions: - contents: read - -concurrency: - group: publish-lineageweave-opencode-route - cancel-in-progress: false - -jobs: - publish: - name: Publish verified route repair - runs-on: ubuntu-latest - timeout-minutes: 35 - steps: - - name: Check out repair branch with workflow-capable token - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 - with: - ref: fix/required-review-fail-closed-without-verdict - fetch-depth: 0 - token: ${{ secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN || github.token }} - - - name: Install test tooling - run: | - set -euo pipefail - python -m pip install --disable-pip-version-check \ - 'pytest>=8.0.0' \ - 'pytest-cov>=7.1.0' \ - 'coverage[toml]>=7.8.0' \ - 'interrogate>=1.7.0' - - - name: Apply the already RED-proven route repair - run: | - set -euo pipefail - cat > tests/test_lineageweave_opencode_route.py <<'PY' - """Regression contract for the LineageWeave central review route.""" - - from pathlib import Path - - - ROOT = Path(__file__).resolve().parents[1] - VARIABLE_EXPRESSION = "$" + "{{ vars.OPENCODE_REPOSITORY_DISPATCH_TARGETS }}" - LINEAGEWEAVE_SUFFIX = ",ContextualWisdomLab/LineageWeave" - ROUTE_VALUE = VARIABLE_EXPRESSION + LINEAGEWEAVE_SUFFIX - - - def test_lineageweave_is_in_every_central_opencode_route() -> None: - """Keep LineageWeave routable when the mutable repository variable drifts.""" - - router = ( - ROOT / ".github" / "workflows" / "agent-mention-router.yml" - ).read_text(encoding="utf-8") - scheduler = ( - ROOT / ".github" / "workflows" / "pr-review-merge-scheduler.yml" - ).read_text(encoding="utf-8") - opencode = ( - ROOT / ".github" / "workflows" / "opencode-review-dispatch.yml" - ).read_text(encoding="utf-8") - - assert router.count( - "OPENCODE_REPOSITORY_DISPATCH_TARGETS: " + ROUTE_VALUE - ) == 2 - assert "ALLOWED_TARGET_REPOSITORIES: " + ROUTE_VALUE in scheduler - assert "ALLOWED_DISPATCH_TARGETS: " + ROUTE_VALUE in opencode - PY - - python - <<'PY' - from pathlib import Path - - expression = "$" + "{{ vars.OPENCODE_REPOSITORY_DISPATCH_TARGETS }}" - suffix = ",ContextualWisdomLab/LineageWeave" - route = expression + suffix - replacements = { - Path(".github/workflows/agent-mention-router.yml"): ( - "OPENCODE_REPOSITORY_DISPATCH_TARGETS: " + expression, - "OPENCODE_REPOSITORY_DISPATCH_TARGETS: " + route, - 2, - ), - Path(".github/workflows/pr-review-merge-scheduler.yml"): ( - "ALLOWED_TARGET_REPOSITORIES: " + expression, - "ALLOWED_TARGET_REPOSITORIES: " + route, - 1, - ), - Path(".github/workflows/opencode-review-dispatch.yml"): ( - "ALLOWED_DISPATCH_TARGETS: " + expression, - "ALLOWED_DISPATCH_TARGETS: " + route, - 1, - ), - } - for path, (old, new, expected_count) in replacements.items(): - text = path.read_text(encoding="utf-8") - if text.count(new) == expected_count: - continue - if text.count(old) != expected_count: - raise SystemExit( - f"{path} route marker count changed: expected " - f"{expected_count}, found {text.count(old)}" - ) - path.write_text(text.replace(old, new), encoding="utf-8") - - changelog_path = Path("CHANGELOG.md") - changelog = changelog_path.read_text(encoding="utf-8") - marker = "### Fixed\n\n" - entry = ( - "- LineageWeave is now retained in the source-controlled exact " - "OpenCode dispatch allowlist floor, so mutable repository-variable " - "drift cannot silently suppress its central review route.\n" - ) - if marker not in changelog: - raise SystemExit("CHANGELOG Fixed marker changed") - if entry not in changelog: - changelog = changelog.replace(marker, marker + entry, 1) - changelog_path.write_text(changelog, encoding="utf-8") - - doctoring_path = Path( - "docs/doctoring/required-review-check-is-not-a-verdict.md" - ) - doctoring = doctoring_path.read_text(encoding="utf-8") - marker = "## Verification contract\n" - section = '''## LineageWeave route contract - - The mutable `OPENCODE_REPOSITORY_DISPATCH_TARGETS` variable remains the - organization-managed allowlist, but LineageWeave is appended as a - source-controlled exact routing floor in the comment router, scheduler, - and privileged OpenCode dispatch validator. This prevents a missing UI - variable entry from silently converting a valid review request into a - non-event while retaining exact repository matching. - - ''' - if marker not in doctoring: - raise SystemExit("doctoring verification marker changed") - if section not in doctoring: - doctoring = doctoring.replace(marker, section + marker, 1) - doctoring_path.write_text(doctoring, encoding="utf-8") - PY - - - name: Reverify the exact source - run: | - set -euo pipefail - python -m pytest tests/test_lineageweave_opencode_route.py - python -m coverage erase - python -m coverage run -m pytest tests - python -m coverage report --show-missing - python -m interrogate -c pyproject.toml scripts/ci - python -m compileall -q scripts tests - - - name: Remove one-shot workflows and publish - run: | - set -euo pipefail - rm -f \ - .github/workflows/repair-lineageweave-opencode-route.yml \ - .github/workflows/publish-lineageweave-opencode-route.yml - git config user.name "github-actions[bot]" - git config user.email "41898282+github-actions[bot]@users.noreply.github.com" - git add \ - .github/workflows/agent-mention-router.yml \ - .github/workflows/pr-review-merge-scheduler.yml \ - .github/workflows/opencode-review-dispatch.yml \ - tests/test_lineageweave_opencode_route.py \ - docs/doctoring/required-review-check-is-not-a-verdict.md \ - CHANGELOG.md \ - .github/workflows/repair-lineageweave-opencode-route.yml \ - .github/workflows/publish-lineageweave-opencode-route.yml - git diff --cached --check - git commit -m "fix(review): retain LineageWeave in central dispatch routes" - git push origin HEAD:fix/required-review-fail-closed-without-verdict From afba5450c23626e4afabff52561845c19b3da384 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 14 Aug 2026 16:59:47 +0900 Subject: [PATCH 18/22] chore(review): remove superseded route patch exporter --- .../export-lineageweave-opencode-route.yml | 172 ------------------ 1 file changed, 172 deletions(-) delete mode 100644 .github/workflows/export-lineageweave-opencode-route.yml diff --git a/.github/workflows/export-lineageweave-opencode-route.yml b/.github/workflows/export-lineageweave-opencode-route.yml deleted file mode 100644 index 907e5aef7..000000000 --- a/.github/workflows/export-lineageweave-opencode-route.yml +++ /dev/null @@ -1,172 +0,0 @@ -name: Export verified LineageWeave OpenCode route - -on: - push: - branches: - - fix/required-review-fail-closed-without-verdict - -permissions: - contents: read - -concurrency: - group: export-lineageweave-opencode-route - cancel-in-progress: false - -jobs: - export: - name: Export verified route patch - runs-on: ubuntu-latest - timeout-minutes: 35 - steps: - - name: Check out repair branch - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 - with: - ref: fix/required-review-fail-closed-without-verdict - fetch-depth: 0 - persist-credentials: false - - - name: Install test tooling - run: | - set -euo pipefail - python -m pip install --disable-pip-version-check \ - 'pytest>=8.0.0' \ - 'pytest-cov>=7.1.0' \ - 'coverage[toml]>=7.8.0' \ - 'interrogate>=1.7.0' - - - name: Apply the RED-proven route repair - run: | - set -euo pipefail - cat > tests/test_lineageweave_opencode_route.py <<'PY' - """Regression contract for the LineageWeave central review route.""" - - from pathlib import Path - - - ROOT = Path(__file__).resolve().parents[1] - VARIABLE_EXPRESSION = "$" + "{{ vars.OPENCODE_REPOSITORY_DISPATCH_TARGETS }}" - LINEAGEWEAVE_SUFFIX = ",ContextualWisdomLab/LineageWeave" - ROUTE_VALUE = VARIABLE_EXPRESSION + LINEAGEWEAVE_SUFFIX - - - def test_lineageweave_is_in_every_central_opencode_route() -> None: - """Keep LineageWeave routable when the mutable repository variable drifts.""" - - router = ( - ROOT / ".github" / "workflows" / "agent-mention-router.yml" - ).read_text(encoding="utf-8") - scheduler = ( - ROOT / ".github" / "workflows" / "pr-review-merge-scheduler.yml" - ).read_text(encoding="utf-8") - opencode = ( - ROOT / ".github" / "workflows" / "opencode-review-dispatch.yml" - ).read_text(encoding="utf-8") - - assert router.count( - "OPENCODE_REPOSITORY_DISPATCH_TARGETS: " + ROUTE_VALUE - ) == 2 - assert "ALLOWED_TARGET_REPOSITORIES: " + ROUTE_VALUE in scheduler - assert "ALLOWED_DISPATCH_TARGETS: " + ROUTE_VALUE in opencode - PY - - python - <<'PY' - from pathlib import Path - - expression = "$" + "{{ vars.OPENCODE_REPOSITORY_DISPATCH_TARGETS }}" - suffix = ",ContextualWisdomLab/LineageWeave" - route = expression + suffix - replacements = { - Path(".github/workflows/agent-mention-router.yml"): ( - "OPENCODE_REPOSITORY_DISPATCH_TARGETS: " + expression, - "OPENCODE_REPOSITORY_DISPATCH_TARGETS: " + route, - 2, - ), - Path(".github/workflows/pr-review-merge-scheduler.yml"): ( - "ALLOWED_TARGET_REPOSITORIES: " + expression, - "ALLOWED_TARGET_REPOSITORIES: " + route, - 1, - ), - Path(".github/workflows/opencode-review-dispatch.yml"): ( - "ALLOWED_DISPATCH_TARGETS: " + expression, - "ALLOWED_DISPATCH_TARGETS: " + route, - 1, - ), - } - for path, (old, new, expected_count) in replacements.items(): - text = path.read_text(encoding="utf-8") - if text.count(new) == expected_count: - continue - if text.count(old) != expected_count: - raise SystemExit( - f"{path} route marker count changed: expected " - f"{expected_count}, found {text.count(old)}" - ) - path.write_text(text.replace(old, new), encoding="utf-8") - - changelog_path = Path("CHANGELOG.md") - changelog = changelog_path.read_text(encoding="utf-8") - marker = "### Fixed\n\n" - entry = ( - "- LineageWeave is now retained in the source-controlled exact " - "OpenCode dispatch allowlist floor, so mutable repository-variable " - "drift cannot silently suppress its central review route.\n" - ) - if marker not in changelog: - raise SystemExit("CHANGELOG Fixed marker changed") - if entry not in changelog: - changelog = changelog.replace(marker, marker + entry, 1) - changelog_path.write_text(changelog, encoding="utf-8") - - doctoring_path = Path( - "docs/doctoring/required-review-check-is-not-a-verdict.md" - ) - doctoring = doctoring_path.read_text(encoding="utf-8") - marker = "## Verification contract\n" - section = '''## LineageWeave route contract - - The mutable `OPENCODE_REPOSITORY_DISPATCH_TARGETS` variable remains the - organization-managed allowlist, but LineageWeave is appended as a - source-controlled exact routing floor in the comment router, scheduler, - and privileged OpenCode dispatch validator. This prevents a missing UI - variable entry from silently converting a valid review request into a - non-event while retaining exact repository matching. - - ''' - if marker not in doctoring: - raise SystemExit("doctoring verification marker changed") - if section not in doctoring: - doctoring = doctoring.replace(marker, section + marker, 1) - doctoring_path.write_text(doctoring, encoding="utf-8") - PY - - - name: Verify exact patched source - run: | - set -euo pipefail - python -m pytest tests/test_lineageweave_opencode_route.py - python -m coverage erase - python -m coverage run -m pytest tests - python -m coverage report --show-missing - python -m interrogate -c pyproject.toml scripts/ci - python -m compileall -q scripts tests - - - name: Stage patch artifact - run: | - set -euo pipefail - mkdir -p \ - route-patch/.github/workflows \ - route-patch/tests \ - route-patch/docs/doctoring - cp .github/workflows/agent-mention-router.yml route-patch/.github/workflows/ - cp .github/workflows/pr-review-merge-scheduler.yml route-patch/.github/workflows/ - cp .github/workflows/opencode-review-dispatch.yml route-patch/.github/workflows/ - cp tests/test_lineageweave_opencode_route.py route-patch/tests/ - cp docs/doctoring/required-review-check-is-not-a-verdict.md route-patch/docs/doctoring/ - cp CHANGELOG.md route-patch/ - - - name: Upload verified patch artifact - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 - with: - name: lineageweave-opencode-route-patch - path: route-patch - if-no-files-found: error - retention-days: 1 From d9d6a3c630fcc6b24a624856eef8c1a8b72c5ba5 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 14 Aug 2026 21:55:43 +0900 Subject: [PATCH 19/22] chore(review): remove unrelated materializer test changes --- tests/test_materialize_base_python_requirements.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/tests/test_materialize_base_python_requirements.py b/tests/test_materialize_base_python_requirements.py index 1ab36445c..8a383f0c2 100644 --- a/tests/test_materialize_base_python_requirements.py +++ b/tests/test_materialize_base_python_requirements.py @@ -14,13 +14,6 @@ from tests.conftest import FakeHttpResponse -def _simulate_linux_x86_64_runner(monkeypatch: pytest.MonkeyPatch) -> None: - """Let installer verification tests run on a non-Linux developer host.""" - monkeypatch.setattr(materializer.sys, "platform", "linux") - monkeypatch.setattr(materializer.platform, "machine", lambda: "x86_64") - materializer._install_trusted_uv.cache_clear() - - def git(repo: Path, *args: str) -> str: """Run git in a temporary fixture repository.""" return subprocess.run( @@ -651,7 +644,6 @@ def test_install_trusted_uv_verifies_version_and_caches_path( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: """The installer writes one executable, verifies its version, and caches it.""" - _simulate_linux_x86_64_runner(monkeypatch) tool_dir = tmp_path / "uv" monkeypatch.setattr( materializer.tempfile, @@ -698,7 +690,6 @@ def test_install_trusted_uv_rejects_version_process_failures( failure: OSError | subprocess.TimeoutExpired, ) -> None: """A missing or hung downloaded executable is removed and rejected.""" - _simulate_linux_x86_64_runner(monkeypatch) tool_dir = tmp_path / "uv" monkeypatch.setattr( materializer.tempfile, @@ -730,7 +721,6 @@ def test_install_trusted_uv_rejects_wrong_version_or_exit_status( completed: subprocess.CompletedProcess[bytes], ) -> None: """Unexpected version output or a nonzero status cannot satisfy the pin.""" - _simulate_linux_x86_64_runner(monkeypatch) tool_dir = tmp_path / f"uv-{completed.returncode}-{len(completed.stdout)}" monkeypatch.setattr( materializer.tempfile, From 27903c9c2768047f9a579654dafe923811ef1a1f Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 16 Aug 2026 19:04:46 +0900 Subject: [PATCH 20/22] fix(noema): fail closed on drafts without primary approval A draft skip must not make required noema-review green before opencode-agent posts a current-head verdict. --- CHANGELOG.md | 2 +- .../required-review-check-is-not-a-verdict.md | 4 +++- scripts/ci/noema_review_gate.py | 13 +++++++++---- tests/test_noema_review_gate.py | 6 +++++- 4 files changed, 18 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7fd63bab2..4d46f69b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ Semantic Versioning where the repository publishes a release. - Noema now validates the current-head primary OpenCode approval before accepting an existing Noema verdict, preventing a secondary review from making the required gate look successful on its own. - Draft pull requests now receive same-head Strix and OpenCode review dispatches while remaining excluded from branch updates, auto-merge changes, direct merge, and review-state cleanup. - The required `opencode-review` check now fails closed unless `opencode-agent` already posted `APPROVED` or `CHANGES_REQUESTED` on the current head, so a stub success can no longer look like a review (ContextualWisdomLab/contextual-orchestrator#176). -- Noema no longer exits 0 when the current head has no primary OpenCode approval; that skip was the green `noema-review` check with an empty Reviews tab. +- Noema no longer exits 0 when the current head has no primary OpenCode approval, including on draft pull requests; that skip was the green `noema-review` check with an empty Reviews tab. - `load_codegraph_context` now confines `NOEMA_CODEGRAPH_CONTEXT_PATH` to `GITHUB_WORKSPACE` (or cwd) with `..` rejection and realpath checks, so a Strix path-traversal report on that helper cannot read files outside the review workspace. - Bounded the Strix quality self-test's deterministic timeout fixtures to 3-second process and 5-second fake-sleep budgets so exact-head policy evidence completes inside the existing job limit without changing production Strix scanner timeouts, providers, credentials, or review semantics. - Allowed commas and ASCII parentheses in the bounded Strix changed-file path policy so legal tracked Packrat fixtures can receive exact-head security analysis, while rejecting raw `..` components before normalization and keeping controls, backslashes, whitespace ambiguity, and shell punctuation fail-closed. diff --git a/docs/doctoring/required-review-check-is-not-a-verdict.md b/docs/doctoring/required-review-check-is-not-a-verdict.md index 713d3a20b..0079e59b4 100644 --- a/docs/doctoring/required-review-check-is-not-a-verdict.md +++ b/docs/doctoring/required-review-check-is-not-a-verdict.md @@ -23,7 +23,9 @@ review on an old SHA, or no review at all cannot make the check green. `scripts/ci/noema_review_gate.py` no longer returns 0 when the current head has no primary OpenCode approval. That skip was exit 0, so the -required `noema-review` check looked like a successful review. The gate +required `noema-review` check looked like a successful review. Draft +status is checked only after that primary-approval gate, so a draft +without an OpenCode verdict cannot turn `noema-review` green. The gate also validates the primary approval before accepting an existing Noema review; a secondary verdict cannot independently turn the required gate green. diff --git a/scripts/ci/noema_review_gate.py b/scripts/ci/noema_review_gate.py index 2206ea0ad..044ec0bd8 100644 --- a/scripts/ci/noema_review_gate.py +++ b/scripts/ci/noema_review_gate.py @@ -606,7 +606,12 @@ def submit_review(repo: str, number: int, pr: dict[str, Any], actor: str, verdic def inspect_and_review(repo: str, number: int) -> int: - """Inspect PR state and submit Noema's LLM review when gates are clean.""" + """Inspect PR state and submit Noema's LLM review when gates are clean. + + Missing current-head primary OpenCode approval fails closed, including + on draft pull requests, so the required check cannot look reviewed + without a Reviews-tab verdict. + """ pr = fetch_pr(repo, number) actor = current_actor() if actor in PRIMARY_REVIEW_AUTHORS: @@ -615,9 +620,6 @@ def inspect_and_review(repo: str, number: int) -> int: "Noema review skipped so GitHub receives an independent reviewer." ) return 0 - if pr.get("isDraft"): - print("PR is draft; Noema review skipped.") - return 0 if not current_primary_approval(pr): print( "Current head does not have a primary OpenCode approval; " @@ -625,6 +627,9 @@ def inspect_and_review(repo: str, number: int) -> int: "check look like a review." ) return 1 + if pr.get("isDraft"): + print("PR is draft; Noema review skipped after primary OpenCode approval.") + return 0 if existing_noema_review(pr, actor): print("Current head already has a Noema review; nothing to do.") return 0 diff --git a/tests/test_noema_review_gate.py b/tests/test_noema_review_gate.py index 0dfa0faae..51d8d3f3c 100644 --- a/tests/test_noema_review_gate.py +++ b/tests/test_noema_review_gate.py @@ -574,8 +574,12 @@ def test_inspect_and_review_skip_paths(monkeypatch): assert noema.inspect_and_review("owner/repo", 7) == 1 assert calls == [] + monkeypatch.setattr(noema, "fetch_pr", lambda repo, number: make_pr(isDraft=True)) + assert noema.inspect_and_review("owner/repo", 7) == 1 + assert calls == [] + cases = [ - (make_pr(isDraft=True), "noema"), + (make_pr(isDraft=True, reviews={"nodes": [review(body=marker_body)]}), "noema"), (make_pr(reviews={"nodes": [review("CHANGES_REQUESTED"), review(body=marker_body)]}), "noema"), (make_pr(reviews={"nodes": [review(body=marker_body)]}, reviewThreads={"nodes": [{"isResolved": False, "isOutdated": False}]}), "noema"), (make_pr(reviews={"nodes": [review(body=marker_body)]}, statusCheckRollup={"contexts": {"nodes": [{"__typename": "StatusContext", "context": "ci", "state": "FAILURE"}]}}), "noema"), From 801820d4aadfde75f32746f6fd0e65869283c116 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 16 Aug 2026 19:20:28 +0900 Subject: [PATCH 21/22] fix(review): spend dispatch budget on empty Reviews tabs first The one-dispatch-per-run scheduler walked created-at order, so leftover increments with a previous-head verdict consumed the slot while a later PR such as ContextualWisdomLab/contextual-orchestrator#176 stayed green on the required stub with no APPROVED or CHANGES_REQUESTED. Keep fail-closed on the required check, and stable-sort the queue so never-reviewed PRs take the budget before leftover re-reviews. --- CHANGELOG.md | 1 + .../required-review-check-is-not-a-verdict.md | 12 ++- scripts/ci/pr_review_merge_scheduler.py | 32 +++++++ tests/test_pr_review_merge_scheduler.py | 89 +++++++++++++++++++ 4 files changed, 133 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c01c95396..71f541ac3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ Semantic Versioning where the repository publishes a release. - Noema now validates the current-head primary OpenCode approval before accepting an existing Noema verdict, preventing a secondary review from making the required gate look successful on its own. - Draft pull requests now receive same-head Strix and OpenCode review dispatches while remaining excluded from branch updates, auto-merge changes, direct merge, and review-state cleanup. - The required `opencode-review` check now fails closed unless `opencode-agent` already posted `APPROVED` or `CHANGES_REQUESTED` on the current head, so a stub success can no longer look like a review (ContextualWisdomLab/contextual-orchestrator#176). +- The merge scheduler now spends its review-dispatch budget on pull requests with no OpenCode verdict on any commit before leftover increments that already have a previous-head APPROVED or CHANGES_REQUESTED, so one-dispatch-per-run no longer starves an empty Reviews tab. - Noema no longer exits 0 when the current head has no primary OpenCode approval, including on draft pull requests; that skip was the green `noema-review` check with an empty Reviews tab. - `load_codegraph_context` now confines `NOEMA_CODEGRAPH_CONTEXT_PATH` to `GITHUB_WORKSPACE` (or cwd) with `..` rejection and realpath checks, so a Strix path-traversal report on that helper cannot read files outside the review workspace. - Materialized base Python locks only when every package line is an exact SHA-256 pin or a bounded relative `-r`/`--requirement` include. A lone `--require-hashes` directive, a dotted include such as `./lock.txt`, or `-r other-hashes.txt` no longer enters the trusted build context. diff --git a/docs/doctoring/required-review-check-is-not-a-verdict.md b/docs/doctoring/required-review-check-is-not-a-verdict.md index 0079e59b4..f95c79b08 100644 --- a/docs/doctoring/required-review-check-is-not-a-verdict.md +++ b/docs/doctoring/required-review-check-is-not-a-verdict.md @@ -35,6 +35,14 @@ Human `repository_dispatch` as `seonghobae` remains rejected; only verdict is posted, re-run the required `opencode-review` job so the fail-closed check can observe it. +The one-dispatch-per-run budget used to walk pull requests in created-at +order, so leftover increments that already had a previous-head verdict +consumed the slot while a later PR with an empty Reviews tab waited. The +scheduler now stable-sorts that budget: no OpenCode APPROVED or +CHANGES_REQUESTED on any commit first, then previous-head re-reviews, +then current-head verdicts. COMMENTED-only evidence is not a verdict and +keeps the empty-Reviews priority. + ## Draft pull-request review contract Draft status is a merge-readiness signal, not a request to suppress early @@ -58,7 +66,9 @@ automation. - `tests/test_pr_review_merge_scheduler.py` proves that a draft pull request receives same-head Strix and OpenCode dispatch while branch updates, auto-merge mutation, direct merge, review dismissal, and thread cleanup - remain unreachable. + remain unreachable, and that a never-reviewed pull request consumes the + one-dispatch budget before a leftover increment that already has a + previous-head OpenCode verdict. - Both repairs were exercised test-first: the draft-dispatch contract failed against the old unconditional skip, and the Noema ordering contract failed against the old secondary-review-first branch. The exact repaired source diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index ecaac1422..24165a54c 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -1193,6 +1193,36 @@ def has_current_head_changes_requested(pr: dict[str, Any]) -> bool: return current_head_review_state(pr, "CHANGES_REQUESTED") +def has_any_opencode_verdict(pr: dict[str, Any]) -> bool: + """Return whether OpenCode ever posted APPROVED or CHANGES_REQUESTED on this PR.""" + for review in (pr.get("reviews") or {}).get("nodes") or []: + if not is_opencode_review(review): + continue + if (review.get("state") or "").upper() in {"APPROVED", "CHANGES_REQUESTED"}: + return True + return False + + +def review_dispatch_priority(pr: dict[str, Any]) -> int: + """Return a lower rank for PRs that should consume the dispatch budget first. + + Rank 0 has no OpenCode APPROVED or CHANGES_REQUESTED on any commit — the + empty Reviews tab from ContextualWisdomLab/contextual-orchestrator#176. + Rank 1 already received a verdict on a previous head and can wait for a + leftover re-review. Rank 2 already has a current-head verdict. + """ + if has_current_head_approval(pr) or has_current_head_changes_requested(pr): + return 2 + if has_any_opencode_verdict(pr): + return 1 + return 0 + + +def prioritize_review_dispatch_queue(prs: Sequence[dict[str, Any]]) -> list[dict[str, Any]]: + """Stable-sort so never-reviewed PRs take the dispatch slot before leftover increments.""" + return sorted(prs, key=review_dispatch_priority) + + def stale_opencode_change_request_ids(pr: dict[str, Any]) -> list[int]: """Return dismissible automated change requests tied to previous heads.""" review_ids: list[int] = [] @@ -3865,6 +3895,8 @@ def main(argv: list[str]) -> int: if args.branch_update_limit < -1: raise SystemExit("--branch-update-limit must be -1 or greater") prs = fetch_pr(args.repo, args.pr_number) if args.pr_number else fetch_open_prs(args.repo, args.max_prs) + if not args.pr_number: + prs = prioritize_review_dispatch_queue(prs) decisions = [] review_dispatches_used = 0 branch_updates_used = 0 diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 72e6c31db..ad810c47f 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -4513,6 +4513,95 @@ def test_main_limits_review_dispatches_and_branch_updates(monkeypatch, capsys): ) +def test_review_dispatch_priority_ranks_empty_reviews_before_leftover_rereview(): + never_reviewed = make_pr(number=176) + leftover_rereview = make_pr( + number=998, + reviews={ + "nodes": [ + opencode_review("COMMENTED", "old-head", login="seonghobae"), + opencode_review("CHANGES_REQUESTED", "old-head"), + ] + }, + ) + already_verdicted = make_pr( + number=1002, + reviews={"nodes": [opencode_review("APPROVED", "head")]}, + ) + commented_only = make_pr( + number=42, + reviews={"nodes": [opencode_review("COMMENTED", "head")]}, + ) + + human_only = make_pr( + number=7, + reviews={"nodes": [opencode_review("APPROVED", "head", login="seonghobae")]}, + ) + assert sched.has_any_opencode_verdict(never_reviewed) is False + assert sched.has_any_opencode_verdict(commented_only) is False + assert sched.has_any_opencode_verdict(human_only) is False + assert sched.has_any_opencode_verdict(leftover_rereview) is True + assert sched.review_dispatch_priority(never_reviewed) == 0 + assert sched.review_dispatch_priority(commented_only) == 0 + assert sched.review_dispatch_priority(leftover_rereview) == 1 + assert sched.review_dispatch_priority(already_verdicted) == 2 + assert [ + pr["number"] + for pr in sched.prioritize_review_dispatch_queue( + [leftover_rereview, already_verdicted, never_reviewed, commented_only] + ) + ] == [176, 42, 998, 1002] + + +def test_main_prefers_never_reviewed_pr_when_dispatch_budget_is_one(monkeypatch, capsys): + prs = [ + make_pr( + number=998, + statusCheckRollup={"contexts": {"nodes": [strix_check()]}}, + reviews={"nodes": [opencode_review("CHANGES_REQUESTED", "old-head")]}, + ), + make_pr( + number=176, + statusCheckRollup={"contexts": {"nodes": [strix_check()]}}, + ), + ] + dispatched = [] + + monkeypatch.setattr(sched, "fetch_open_prs", lambda repo, max_prs: prs) + monkeypatch.setattr( + sched, + "dispatch_opencode_review", + lambda repo, workflow, pr, dry_run: dispatched.append(pr["number"]), + ) + monkeypatch.setattr(sched, "cancel_stale_pr_runs", lambda repo, pr, dry_run: []) + + assert ( + sched.main( + [ + "--repo", + "owner/repo", + "--base-branch", + "main", + "--project-flow", + "github-flow", + "--review-dispatch-limit", + "1", + ] + ) + == 0 + ) + + output = capsys.readouterr().out + payload = json.loads(output.strip().splitlines()[-1]) + assert dispatched == [176] + assert payload["decisions"][0]["pr"] == 176 + assert payload["decisions"][0]["action"] == "review_dispatch" + assert payload["decisions"][1]["pr"] == 998 + assert payload["decisions"][1]["reason"] == ( + "current head has completed Strix evidence; review dispatch limit reached" + ) + + def test_main_rejects_invalid_review_dispatch_limit(): with pytest.raises(SystemExit, match="--review-dispatch-limit must be -1 or greater"): sched.main( From fe7a5e04a152cd86d29550e025cb7854a6a16346 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 17 Aug 2026 09:10:35 +0900 Subject: [PATCH 22/22] fix(review): treat GitHub run-name as an in-progress OpenCode dispatch Live workflow runs set name to the interpolated run-name, so "OpenCode Review Dispatch owner/repo#N@sha" did not match the short alias. The scheduler posted a second same-head dispatch and cancel-in-progress killed the review that had already passed coverage. --- CHANGELOG.md | 1 + .../required-review-check-is-not-a-verdict.md | 7 ++ scripts/ci/pr_review_merge_scheduler.py | 24 ++++++- tests/test_pr_review_merge_scheduler.py | 71 +++++++++++++++++++ 4 files changed, 101 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 71f541ac3..a487666aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ Semantic Versioning where the repository publishes a release. - Draft pull requests now receive same-head Strix and OpenCode review dispatches while remaining excluded from branch updates, auto-merge changes, direct merge, and review-state cleanup. - The required `opencode-review` check now fails closed unless `opencode-agent` already posted `APPROVED` or `CHANGES_REQUESTED` on the current head, so a stub success can no longer look like a review (ContextualWisdomLab/contextual-orchestrator#176). - The merge scheduler now spends its review-dispatch budget on pull requests with no OpenCode verdict on any commit before leftover increments that already have a previous-head APPROVED or CHANGES_REQUESTED, so one-dispatch-per-run no longer starves an empty Reviews tab. +- The scheduler treats GitHub's full `run-name` (`OpenCode Review Dispatch owner/repo#N@sha`) as an in-progress same-head dispatch, so a later sweep cannot `cancel-in-progress` a review that already passed coverage. - Noema no longer exits 0 when the current head has no primary OpenCode approval, including on draft pull requests; that skip was the green `noema-review` check with an empty Reviews tab. - `load_codegraph_context` now confines `NOEMA_CODEGRAPH_CONTEXT_PATH` to `GITHUB_WORKSPACE` (or cwd) with `..` rejection and realpath checks, so a Strix path-traversal report on that helper cannot read files outside the review workspace. - Materialized base Python locks only when every package line is an exact SHA-256 pin or a bounded relative `-r`/`--requirement` include. A lone `--require-hashes` directive, a dotted include such as `./lock.txt`, or `-r other-hashes.txt` no longer enters the trusted build context. diff --git a/docs/doctoring/required-review-check-is-not-a-verdict.md b/docs/doctoring/required-review-check-is-not-a-verdict.md index f95c79b08..624b300ef 100644 --- a/docs/doctoring/required-review-check-is-not-a-verdict.md +++ b/docs/doctoring/required-review-check-is-not-a-verdict.md @@ -43,6 +43,13 @@ CHANGES_REQUESTED on any commit first, then previous-head re-reviews, then current-head verdicts. COMMENTED-only evidence is not a verdict and keeps the empty-Reviews priority. +A second same-head `repository_dispatch` used to cancel the first through +workflow `cancel-in-progress` because `active_review_run_refs` compared +the GitHub `name` field to the short alias `OpenCode Review Dispatch`. +Live runs set `name` to the interpolated run-name. The matcher now +accepts that prefix so a queued or in-progress same-head review is +`already_running`. + ## Draft pull-request review contract Draft status is a merge-readiness signal, not a request to suppress early diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 24165a54c..566bd33f0 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -1923,6 +1923,22 @@ def stale_opencode_run_ids(repo: str, workflow: str, pr: dict[str, Any]) -> list return stale +def workflow_run_name_matches( + run_name: str, workflow: str, workflow_aliases: frozenset[str] +) -> bool: + """Return whether a GitHub run name is the workflow or a run-name prefix of it. + + Workflow runs use ``run-name:`` as ``name``. OpenCode Review Dispatch therefore + appears as ``OpenCode Review Dispatch owner/repo#1@sha``, which is not equal to + the short alias. Exact-only matching misses the live run, a second dispatch is + posted, and ``cancel-in-progress`` kills the review that was about to finish. + """ + names = {workflow, *workflow_aliases} + if run_name in names: + return True + return any(run_name.startswith(f"{candidate} ") for candidate in names) + + def active_review_run_refs( repo: str, workflow: str, @@ -1954,13 +1970,17 @@ def active_review_run_refs( for run_repo in (dispatch_repo,): for run_data in active_workflow_runs(run_repo, statuses): run_name = str(run_data.get("name") or "") - if run_name != workflow and run_name not in workflow_aliases: + display_title = str(run_data.get("display_title") or "") + if not workflow_run_name_matches( + run_name, workflow, workflow_aliases + ) and not workflow_run_name_matches( + display_title, workflow, workflow_aliases + ): continue run_id = run_data.get("id") if not run_id: continue run_ref = (run_repo, str(run_id)) - display_title = str(run_data.get("display_title") or "") dispatch_title_prefix = next( ( prefix diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index ad810c47f..00f6442a4 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -2214,6 +2214,77 @@ def fake_run(args, stdin=None): assert not any(call[:3] == ["gh", "workflow", "run"] for call in calls) +def test_workflow_run_name_matches_accepts_github_run_name_prefix(): + aliases = frozenset({"OpenCode Review Dispatch", "Required OpenCode Review"}) + assert sched.workflow_run_name_matches( + "OpenCode Review Dispatch", "OpenCode Review", aliases + ) + assert sched.workflow_run_name_matches( + "OpenCode Review Dispatch ContextualWisdomLab/.github#1002@" + ("a" * 40), + "OpenCode Review", + aliases, + ) + assert not sched.workflow_run_name_matches( + "Strix Security Scan ContextualWisdomLab/.github#1002@" + ("a" * 40), + "OpenCode Review", + aliases, + ) + + +def test_dispatch_opencode_review_deduplicates_github_run_name_as_display_title( + monkeypatch, capsys +): + calls = [] + head_sha = "a" * 40 + live_name = f"OpenCode Review Dispatch owner/repo#1@{head_sha}" + current_dispatch = { + "id": 9101, + "name": live_name, + "event": "repository_dispatch", + "head_sha": "default-branch-sha", + "display_title": live_name, + "pull_requests": [], + } + + def fake_run(args, stdin=None): + calls.append(args) + if args[:5] == [ + "gh", + "api", + "--method", + "GET", + "repos/ContextualWisdomLab/.github/actions/runs", + ]: + if "status=queued" in args: + return json.dumps({"workflow_runs": [current_dispatch]}) + return json.dumps({"workflow_runs": []}) + if "/actions/runs" in " ".join(args): + return json.dumps({"workflow_runs": []}) + return "" + + monkeypatch.setattr(sched, "run", fake_run) + monkeypatch.setenv("GITHUB_ACTIONS", "true") + monkeypatch.setenv("GH_TOKEN", "workflow-token") + monkeypatch.setenv( + "SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY", + "ContextualWisdomLab/.github", + ) + + result = sched.dispatch_opencode_review( + "owner/repo", + "OpenCode Review", + make_pr(headRefOid=head_sha), + dry_run=False, + ) + + assert result == "already_running" + assert ( + "active same-head workflow run(s) ContextualWisdomLab/.github@9101" + in capsys.readouterr().out + ) + assert not any(call[-2:] == ["--input", "-"] for call in calls) + + def test_dispatch_strix_cancels_stale_central_run_and_keeps_current(monkeypatch, capsys): calls = [] head_sha = "a" * 40