From 38a53eead795460bcfa696219bfa6d7286813f06 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 03:22:58 +0000 Subject: [PATCH 01/11] fix(ci): allow stacked-PR bases across governed workflows + governance-gate CodeRabbit/OpenCode fallback fixes Extracted from ContextualWisdomLab/naruon#1502 at the repo owner's request (#1502 issue comment, 2026-09-02): that PR's diff had drifted to also carry this unrelated PR-governance/stack-trigger content, which should live in its own lane rather than hitchhike on the Postgres-service-container PR. Two independent pieces of work, both already validated on #1502's branch before this extraction: 1. Stacked-PR base support: removes the `branches: [develop, master, release/**]` restriction from the four governed `pull_request`-triggered workflows (app-ci.yml, bandit.yml, dependency-review.yml, docker-publish.yml) so required checks run for every PR base, including a PR stacked on another open PR's branch rather than directly on develop/master. Adds tests/test_stacked_pr_workflow_contract.py to pin this and updates backend/tests/test_release_governance.py's existing app-ci.yml/docker-publish.yml assertions to match. 2. scripts/ci/pr_governance_gate.sh: recognizes CodeRabbit's "approval pending" notice (marker-delimited, "has no unresolved comments, but it has not reviewed the latest commit") as a wait state rather than a false blocker -- the prior regex matched the phrase "blocking issues" inside CodeRabbit's own hypothetical-approval sentence. Also fixes a regression Devin Review found in that same fix: once the approval-pending notice routed to add_waiting, it did so unconditionally, even when the documented no-check-run fallback (an accepted, exact-current-head OpenCode adversarial approval) had already been satisfied -- defeating the fallback's whole purpose (governance not stuck when CodeRabbit itself is unavailable). OPENCODE_FALLBACK_APPROVED now tracks whether that fallback was accepted and skips the wait when it was. Verified on this exact extracted head: `bash scripts/ci/test_pr_governance_gate.sh` PASS (including the new missing_coderabbit_adversarial_approval_with_pending_notice regression scenario); `python3 -m pytest tests/test_stacked_pr_workflow_contract.py -q` 1 passed; manually re-checked backend/tests/test_release_governance.py's app-ci.yml/docker-publish.yml assertions against this branch's exact workflow file contents (all hold). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_019YgmaEKnPhrsbSyQBierFH --- .github/workflows/app-ci.yml | 4 -- .github/workflows/bandit.yml | 1 - .github/workflows/dependency-review.yml | 4 -- .github/workflows/docker-publish.yml | 4 -- backend/tests/test_release_governance.py | 6 ++- scripts/ci/pr_governance_gate.sh | 25 +++++++++-- scripts/ci/test_pr_governance_gate.sh | 50 +++++++++++++++++++++- tests/test_stacked_pr_workflow_contract.py | 27 ++++++++++++ 8 files changed, 101 insertions(+), 20 deletions(-) create mode 100644 tests/test_stacked_pr_workflow_contract.py diff --git a/.github/workflows/app-ci.yml b/.github/workflows/app-ci.yml index e8f445748..5995ec817 100644 --- a/.github/workflows/app-ci.yml +++ b/.github/workflows/app-ci.yml @@ -2,10 +2,6 @@ name: Application CI on: pull_request: - branches: - - develop - - master - - "release/**" push: branches: - develop diff --git a/.github/workflows/bandit.yml b/.github/workflows/bandit.yml index c5c613c08..0e250389c 100644 --- a/.github/workflows/bandit.yml +++ b/.github/workflows/bandit.yml @@ -4,7 +4,6 @@ on: push: branches: [ develop, master ] pull_request: - branches: [ develop, master ] workflow_dispatch: permissions: diff --git a/.github/workflows/dependency-review.yml b/.github/workflows/dependency-review.yml index c303d1e61..21e607f7b 100644 --- a/.github/workflows/dependency-review.yml +++ b/.github/workflows/dependency-review.yml @@ -2,10 +2,6 @@ name: Dependency Review on: pull_request: - branches: - - develop - - master - - "release/**" workflow_dispatch: permissions: diff --git a/.github/workflows/docker-publish.yml b/.github/workflows/docker-publish.yml index fc7058413..dd1015812 100644 --- a/.github/workflows/docker-publish.yml +++ b/.github/workflows/docker-publish.yml @@ -5,10 +5,6 @@ on: tags: - "v*" pull_request: - branches: - - develop - - master - - "release/**" permissions: contents: read diff --git a/backend/tests/test_release_governance.py b/backend/tests/test_release_governance.py index a23c70746..dfcb1cb16 100644 --- a/backend/tests/test_release_governance.py +++ b/backend/tests/test_release_governance.py @@ -651,7 +651,6 @@ def test_app_ci_runs_backend_and_frontend_checks_without_duplicate_release_pushe workflow = read_repo_text(".github/workflows/app-ci.yml") assert "pull_request:" in workflow - assert "release/**" in workflow assert "python -m pytest" in workflow assert "PYTHONWARNINGS: error" in workflow assert 'DISABLE_BACKGROUND_WORKERS: "1"' in workflow @@ -669,6 +668,9 @@ def test_app_ci_runs_backend_and_frontend_checks_without_duplicate_release_pushe assert "master" in push_block assert "release/**" not in push_block + pull_request_block = workflow.split("pull_request:", 1)[1].split("push:", 1)[0] + assert "branches:" not in pull_request_block + def test_docker_publish_validates_pr_images_and_publishes_semver_images_only_on_tags() -> ( None @@ -710,7 +712,7 @@ def test_docker_publish_validates_pr_images_and_publishes_semver_images_only_on_ ] assert "tags:" in push_block assert "branches:" not in push_block - assert "develop" in pull_request_block + assert "branches:" not in pull_request_block assert "ai_email_client-backend" in workflow assert "ai_email_client-frontend" in workflow assert workflow.count("image: naruon") == 2 diff --git a/scripts/ci/pr_governance_gate.sh b/scripts/ci/pr_governance_gate.sh index a66142ca8..4afd39dfc 100644 --- a/scripts/ci/pr_governance_gate.sh +++ b/scripts/ci/pr_governance_gate.sh @@ -27,6 +27,7 @@ OWNER="${GITHUB_REPOSITORY%/*}" REPO="${GITHUB_REPOSITORY#*/}" BLOCKERS=() WAITING=() +OPENCODE_FALLBACK_APPROVED=0 PR_CHECKS_ERROR_FILE="$(mktemp)" ISSUE_COMMENTS_ERROR_FILE="$(mktemp)" REVIEW_COMMENTS_ERROR_FILE="$(mktemp)" @@ -258,7 +259,7 @@ IS_DRAFT="$(printf '%s' "$PR_JSON" | jq -r '.isDraft')" REVIEW_DECISION="$(printf '%s' "$PR_JSON" | jq -r '.reviewDecision // ""')" if [ "$IS_DRAFT" = "true" ]; then - add_blocker 'Draft PR: merge automation is paused.' + add_waiting 'Draft PR: merge automation is paused.' fi if [ "$MERGE_STATE" = "BEHIND" ]; then @@ -341,6 +342,7 @@ CODERABBIT_BLOCKING_PATTERN='pre[- ]merge|blocking|failure|failed|warning|potent CODERABBIT_ISSUE_BLOCKING_PATTERN='pre[- ]merge[^\n]*(blocking|failure|failed|warning|potential issue)|blocking (issue|finding)|potential issue|actionable comments?|changes requested|request changes' CODERABBIT_ISSUE_SUBSTANTIVE_BLOCKING_PATTERN='pre[- ]merge[^\n]*(blocking|failure|failed|warning|potential issue)|blocking (issue|finding)|potential issue|changes requested|request changes' CODERABBIT_NO_ACTIONABLE_PATTERN='no actionable comments? (were )?generated' +CODERABBIT_APPROVAL_PENDING_PATTERN='CodeRabbit has no unresolved comments, but it has not reviewed the latest commit' CHECK_RUNS="$(gh api "repos/${GITHUB_REPOSITORY}/commits/${HEAD_SHA}/check-runs?per_page=100")" COMMIT_STATUS_JSON='{"statuses":[]}' if ! COMMIT_STATUS_JSON="$(gh api "repos/${GITHUB_REPOSITORY}/commits/${HEAD_SHA}/status" 2>"$COMMIT_STATUS_ERROR_FILE")"; then @@ -386,6 +388,7 @@ if [ "$CODERABBIT_COUNT" = "0" ]; then if [ "$OPENCODE_ADVERSARIAL_APPROVAL_COUNT" = "0" ]; then add_waiting "Waiting for current-head CodeRabbit evidence or a structured OpenCode App adversarial approval on ${HEAD_REF_OID}." else + OPENCODE_FALLBACK_APPROVED=1 printf 'CodeRabbit check is absent; accepted current-head OpenCode App adversarial approval on %s.\n' "$HEAD_REF_OID" fi fi @@ -435,13 +438,17 @@ else --arg head_sha "$HEAD_SHA" \ --arg pattern "$CODERABBIT_ISSUE_BLOCKING_PATTERN" \ --arg substantive_pattern "$CODERABBIT_ISSUE_SUBSTANTIVE_BLOCKING_PATTERN" \ - --arg no_actionable_pattern "$CODERABBIT_NO_ACTIONABLE_PATTERN" ' + --arg no_actionable_pattern "$CODERABBIT_NO_ACTIONABLE_PATTERN" \ + --arg approval_pending_pattern "$CODERABBIT_APPROVAL_PENDING_PATTERN" ' [.[][] | select((.user.login // "") | test("'"$REVIEW_BOT_LOGIN_PATTERN"'"; "i")) | select( (.body // "") as $body | ($body | split("
")[0]) as $summary - | ($body | test($pattern; "i")) + | (($body | contains("")) + and ($body | test($approval_pending_pattern; "i")) + | not) + and ($body | test($pattern; "i")) and ( (($body | test($no_actionable_pattern; "i")) | not) or ($summary | test($substantive_pattern; "i")) @@ -450,8 +457,20 @@ else | select((.body // "") | contains($head_sha))] | length' )" + CODERABBIT_APPROVAL_PENDING_COUNT="$(printf '%s' "$ISSUE_COMMENTS_JSON" | jq -s \ + --arg head_sha "$HEAD_SHA" \ + --arg approval_pending_pattern "$CODERABBIT_APPROVAL_PENDING_PATTERN" ' + [.[][] + | select((.user.login // "") | test("'"$REVIEW_BOT_LOGIN_PATTERN"'"; "i")) + | select((.body // "") | contains("")) + | select((.body // "") | test($approval_pending_pattern; "i")) + | select((.body // "") | contains($head_sha))] + | length' + )" if [ "$CODERABBIT_ISSUE_BLOCKERS" != "0" ]; then add_blocker "Current-head CodeRabbit issue comment has blocking warning/failure evidence on ${HEAD_REF_OID}." + elif [ "$CODERABBIT_APPROVAL_PENDING_COUNT" != "0" ] && [ "$OPENCODE_FALLBACK_APPROVED" != "1" ]; then + add_waiting "Waiting for CodeRabbit to review the latest commit on ${HEAD_REF_OID}." fi fi diff --git a/scripts/ci/test_pr_governance_gate.sh b/scripts/ci/test_pr_governance_gate.sh index 8fb42aa9b..4d4378301 100644 --- a/scripts/ci/test_pr_governance_gate.sh +++ b/scripts/ci/test_pr_governance_gate.sh @@ -17,6 +17,9 @@ args="$*" if [ "$1" = "pr" ] && [ "$2" = "view" ]; then case "${GH_SCENARIO:-pass}" in + draft) + printf '{"number":42,"state":"OPEN","headRefOid":"%s","isDraft":true,"mergeable":"MERGEABLE","mergeStateStatus":"CLEAN","reviewDecision":"","statusCheckRollup":[]}' "$head_sha" + ;; changes_requested) printf '{"number":42,"state":"OPEN","headRefOid":"%s","isDraft":false,"mergeable":"MERGEABLE","mergeStateStatus":"CLEAN","reviewDecision":"CHANGES_REQUESTED","statusCheckRollup":[]}' "$head_sha" ;; @@ -140,7 +143,7 @@ if [ "$1" = "api" ] && [[ "$2" == repos/*/commits/*/check-runs* ]]; then coderabbit_pending) printf '{"check_runs":[{"name":"CodeRabbit","app":{"slug":"coderabbitai"},"status":"in_progress","conclusion":null,"html_url":"https://checks/coderabbit"}]}' ;; - missing_coderabbit|missing_coderabbit_with_adversarial_approval|missing_coderabbit_stale_approval|missing_coderabbit_actions_approval|missing_coderabbit_one_probe|opencode_reviews_error|coderabbit_status_success|coderabbit_status_pending|coderabbit_status_failed|coderabbit_status_unknown) + missing_coderabbit|missing_coderabbit_with_adversarial_approval|missing_coderabbit_stale_approval|missing_coderabbit_actions_approval|missing_coderabbit_one_probe|missing_coderabbit_adversarial_approval_with_pending_notice|opencode_reviews_error|coderabbit_status_success|coderabbit_status_pending|coderabbit_status_failed|coderabbit_status_unknown) printf '{"check_runs":[]}' ;; coderabbit_failed) @@ -192,7 +195,7 @@ if [ "$1" = "api" ] && [[ "$args" == *repos/*/pulls/42/reviews* ]]; then exit 1 fi case "${GH_SCENARIO:-pass}" in - missing_coderabbit_with_adversarial_approval) + missing_coderabbit_with_adversarial_approval|missing_coderabbit_adversarial_approval_with_pending_notice) printf '[[{"user":{"login":"opencode-agent[bot]"},"state":"APPROVED","commit_id":"%s","body":"## Adversarial validation\\n\\n```json\\n{\\\"status\\\":\\\"passed\\\",\\\"probes\\\":[{\\\"outcome\\\":\\\"falsified\\\"},{\\\"outcome\\\":\\\"falsified\\\"}]}\\n```\\n\\nHead SHA: `%s`"}]]' "$head_sha" "$head_sha" ;; missing_coderabbit_stale_approval) @@ -249,6 +252,9 @@ if [ "$1" = "api" ] && [[ "$args" == *repos/*/issues/42/comments* ]]; then coderabbit_no_actionable_with_blocker) printf '[{"id":777,"user":{"login":"coderabbitai[bot]"},"created_at":"2026-05-19T00:01:00Z","body":"No actionable comments were generated in the recent review. Blocking issue remains on 0123456789abcdef0123456789abcdef01234567."}]' ;; + coderabbit_approval_pending|missing_coderabbit_adversarial_approval_with_pending_notice) + printf '[{"id":777,"user":{"login":"coderabbitai[bot]"},"created_at":"2026-05-19T00:01:00Z","body":"CodeRabbit has no unresolved comments, but it has not reviewed the latest commit. CodeRabbit will approve the changes if it finds no blocking issues. "}]' + ;; github_code_quality_blocking_comment) printf '[{"id":777,"user":{"login":"github-code-quality[bot]"},"created_at":"2026-05-19T00:01:00Z","body":"Potential issue for 0123456789abcdef0123456789abcdef01234567"}]' ;; @@ -453,6 +459,19 @@ assert_failed_checks_create_marker_comment() { assert_not_in_file '^pr merge' "$temp_dir/gh.log" } +assert_draft_pr_waits_without_false_failure() { + local temp_dir + temp_dir="$(mktemp -d)" + run_gate draft "$temp_dir" + + assert_exit_code 0 "$temp_dir" + assert_in_file 'Draft PR: merge automation is paused.' "$temp_dir/gh.log" + assert_in_file 'status=in_progress' "$temp_dir/gh.log" + assert_not_in_file 'conclusion=failure' "$temp_dir/gh.log" + assert_not_in_file 'PR governance metadata gate is not ready' "$temp_dir/gh.log" + assert_not_in_file '^pr merge' "$temp_dir/gh.log" +} + assert_existing_marker_comment_is_patched() { local temp_dir temp_dir="$(mktemp -d)" @@ -549,6 +568,18 @@ assert_missing_coderabbit_accepts_exact_head_adversarial_opencode_approval() { assert_in_file 'conclusion=success' "$temp_dir/gh.log" } +assert_missing_coderabbit_adversarial_approval_survives_pending_notice() { + local temp_dir + temp_dir="$(mktemp -d)" + run_gate missing_coderabbit_adversarial_approval_with_pending_notice "$temp_dir" + + assert_exit_code 0 "$temp_dir" + assert_in_file 'accepted current-head OpenCode App adversarial approval' "$temp_dir/output.txt" + assert_in_file 'PR governance metadata gate is ready' "$temp_dir/output.txt" + assert_not_in_file 'Waiting for CodeRabbit to review the latest commit' "$temp_dir/output.txt" + assert_in_file 'conclusion=success' "$temp_dir/gh.log" +} + assert_missing_coderabbit_rejects_non_authoritative_opencode_evidence() { local scenario temp_dir for scenario in \ @@ -763,6 +794,18 @@ assert_coderabbit_no_actionable_summary_with_blocker_still_blocks() { assert_not_in_file '^pr merge' "$temp_dir/gh.log" } +assert_coderabbit_approval_pending_waits_without_blocking() { + local temp_dir + temp_dir="$(mktemp -d)" + run_gate coderabbit_approval_pending "$temp_dir" + + assert_exit_code 0 "$temp_dir" + assert_in_file 'Waiting for CodeRabbit to review the latest commit' "$temp_dir/output.txt" + assert_in_file 'status=in_progress' "$temp_dir/gh.log" + assert_not_in_file 'Current-head CodeRabbit issue comment has blocking warning/failure evidence' "$temp_dir/gh.log" + assert_not_in_file '^pr merge' "$temp_dir/gh.log" +} + assert_coderabbit_current_review_comment_blocks() { local temp_dir temp_dir="$(mktemp -d)" @@ -923,6 +966,7 @@ assert_head_change_during_evaluation_skips_stale_publication assert_closed_during_evaluation_skips_stale_publication assert_startup_failure_creates_marker_comment assert_failed_checks_create_marker_comment +assert_draft_pr_waits_without_false_failure assert_existing_marker_comment_is_patched assert_resolved_marker_comment_is_updated_on_ready_gate assert_coderabbit_pending_waits_without_hard_comment @@ -932,6 +976,7 @@ assert_coderabbit_failed_commit_status_blocks assert_coderabbit_unknown_commit_status_fails_closed assert_missing_coderabbit_waits_for_adversarial_opencode_approval assert_missing_coderabbit_accepts_exact_head_adversarial_opencode_approval +assert_missing_coderabbit_adversarial_approval_survives_pending_notice assert_missing_coderabbit_rejects_non_authoritative_opencode_evidence assert_opencode_review_lookup_error_is_logged_but_not_published_verbatim assert_completed_gate_check_is_republished_as_new_run @@ -949,6 +994,7 @@ assert_coderabbit_stale_issue_comment_does_not_block assert_coderabbit_review_limit_issue_comment_does_not_block assert_coderabbit_no_actionable_summary_does_not_block assert_coderabbit_no_actionable_summary_with_blocker_still_blocks +assert_coderabbit_approval_pending_waits_without_blocking assert_coderabbit_current_review_comment_blocks assert_coderabbit_resolved_current_review_comment_does_not_block assert_truncated_review_thread_metadata_blocks diff --git a/tests/test_stacked_pr_workflow_contract.py b/tests/test_stacked_pr_workflow_contract.py new file mode 100644 index 000000000..d54014e0d --- /dev/null +++ b/tests/test_stacked_pr_workflow_contract.py @@ -0,0 +1,27 @@ +"""Regression coverage for governed checks on stacked pull requests.""" + +from pathlib import Path +import re + + +REPO_ROOT = Path(__file__).resolve().parents[1] +GOVERNED_PULL_REQUEST_WORKFLOWS = ( + "app-ci.yml", + "bandit.yml", + "dependency-review.yml", + "docker-publish.yml", +) + + +def test_governed_pull_request_workflows_accept_stacked_base_branches() -> None: + """Required repository checks must run for every PR base, including stacks.""" + for name in GOVERNED_PULL_REQUEST_WORKFLOWS: + workflow = (REPO_ROOT / ".github" / "workflows" / name).read_text() + pull_request_trigger = re.search( + r"(?ms)^ pull_request:\s*$\n(?P(?:^ .*$\n)*)", + workflow, + ) + assert pull_request_trigger is not None, f"{name} must run on pull_request" + assert "branches:" not in pull_request_trigger.group("body"), ( + f"{name} must not exclude stacked PR base branches" + ) From 6acf91ee9ecd68a799f255bf74134718e8968c32 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 03:36:58 +0000 Subject: [PATCH 02/11] fix(governance): don't let OpenCode fallback clear a genuinely-pending CodeRabbit lane; scope marker exclusion precisely Two independent, confirmed findings on this PR (owner comment + Devin Review, both against 38a53ee): 1. Owner: OPENCODE_FALLBACK_APPROVED (introduced by the prior pr_governance_gate.sh fix) suppressed the "waiting for CodeRabbit" state whenever a structured OpenCode adversarial approval was present, even in cases where CodeRabbit had NOT gone silent -- it had posted its own approval-pending issue comment for the exact current head, which means it is actively reviewing and just hasn't reached the latest commit yet. That is not the same thing as CodeRabbit being absent, and letting a different model's approval clear it lets one review model's temporary lag become another model's approval -- exactly the pattern this repo's product-technical-gap-baseline.md already documents as invalid ("model-only... evidence is not passing evidence"). Fixed by computing CODERABBIT_APPROVAL_PENDING_COUNT before the no-check-run fallback decision (moved the issue-comments fetch earlier) and only accepting the fallback when CodeRabbit shows NO engagement at all -- no check-run AND no approval-pending issue comment. When a pending notice exists despite no check-run, the gate now waits on CodeRabbit specifically instead. This makes the later OPENCODE_FALLBACK_APPROVED guard on the CODERABBIT_APPROVAL_PENDING_COUNT branch provably redundant (if the fallback was accepted, that count is already 0), so it's removed rather than kept as dead defensive code. 2. Devin Review: the CODERABBIT_ISSUE_BLOCKERS jq filter excluded an entire issue comment from the blocking-evidence scan whenever it contained the approval-pending marker anywhere in it, even if the same comment also carried a real, separate pre-merge blocking warning elsewhere in its body. Fixed by stripping only the marker-delimited span from the body before running the blocking-pattern test, rather than excluding the whole comment -- verified directly with jq that a comment combining both now correctly still matches on the surviving blocking text, while a comment containing only the pending notice still correctly strips to nothing. TDD evidence: added assert_missing_coderabbit_adversarial_approval_still_waits_for_pending_coderabbit_notice (replaces the now-incorrect assert_missing_coderabbit_adversarial_approval_survives_pending_notice, which asserted the very behavior finding #1 says is wrong) and assert_coderabbit_approval_pending_notice_does_not_hide_separate_blocking_warning (new scenario for finding #2). `bash scripts/ci/test_pr_governance_gate.sh` -> PASS (all scenarios, including both new/rewritten ones). `python3 -m pytest tests/test_stacked_pr_workflow_contract.py -q` -> 1 passed (unaffected). `bash -n` on both modified scripts: clean. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_019YgmaEKnPhrsbSyQBierFH --- scripts/ci/pr_governance_gate.sh | 108 +++++++++++++++----------- scripts/ci/test_pr_governance_gate.sh | 40 ++++++++-- 2 files changed, 98 insertions(+), 50 deletions(-) diff --git a/scripts/ci/pr_governance_gate.sh b/scripts/ci/pr_governance_gate.sh index 4afd39dfc..266c195de 100644 --- a/scripts/ci/pr_governance_gate.sh +++ b/scripts/ci/pr_governance_gate.sh @@ -343,6 +343,31 @@ CODERABBIT_ISSUE_BLOCKING_PATTERN='pre[- ]merge[^\n]*(blocking|failure|failed|wa CODERABBIT_ISSUE_SUBSTANTIVE_BLOCKING_PATTERN='pre[- ]merge[^\n]*(blocking|failure|failed|warning|potential issue)|blocking (issue|finding)|potential issue|changes requested|request changes' CODERABBIT_NO_ACTIONABLE_PATTERN='no actionable comments? (were )?generated' CODERABBIT_APPROVAL_PENDING_PATTERN='CodeRabbit has no unresolved comments, but it has not reviewed the latest commit' +CODERABBIT_APPROVAL_NOTICE_SPAN_PATTERN='.*?' + +# Fetched and evaluated before the check-run/status lookup below so the +# no-check-run OpenCode fallback can tell "CodeRabbit has never engaged" +# (check AND issue-comment both silent) apart from "CodeRabbit is actively +# reviewing, just hasn't reached the latest commit yet" (an approval-pending +# issue comment despite no check-run yet). Only the former is eligible for +# the fallback; the latter must still wait on CodeRabbit itself. +if ! ISSUE_COMMENTS_JSON="$(gh api --paginate "repos/${GITHUB_REPOSITORY}/issues/${PR_NUMBER}/comments" 2>"$ISSUE_COMMENTS_ERROR_FILE")"; then + printf 'issue comment lookup failed:\n' + printf '%s\n' "$(<"$ISSUE_COMMENTS_ERROR_FILE")" | sed 's/^/ /' + add_blocker 'PR issue comments could not be read; see the workflow run log.' + ISSUE_COMMENTS_JSON='[]' +fi +CODERABBIT_APPROVAL_PENDING_COUNT="$(printf '%s' "$ISSUE_COMMENTS_JSON" | jq -s \ + --arg head_sha "$HEAD_SHA" \ + --arg approval_pending_pattern "$CODERABBIT_APPROVAL_PENDING_PATTERN" ' + [.[][] + | select((.user.login // "") | test("'"$REVIEW_BOT_LOGIN_PATTERN"'"; "i")) + | select((.body // "") | contains("")) + | select((.body // "") | test($approval_pending_pattern; "i")) + | select((.body // "") | contains($head_sha))] + | length' +)" + CHECK_RUNS="$(gh api "repos/${GITHUB_REPOSITORY}/commits/${HEAD_SHA}/check-runs?per_page=100")" COMMIT_STATUS_JSON='{"statuses":[]}' if ! COMMIT_STATUS_JSON="$(gh api "repos/${GITHUB_REPOSITORY}/commits/${HEAD_SHA}/status" 2>"$COMMIT_STATUS_ERROR_FILE")"; then @@ -387,9 +412,15 @@ if [ "$CODERABBIT_COUNT" = "0" ]; then )" if [ "$OPENCODE_ADVERSARIAL_APPROVAL_COUNT" = "0" ]; then add_waiting "Waiting for current-head CodeRabbit evidence or a structured OpenCode App adversarial approval on ${HEAD_REF_OID}." + elif [ "$CODERABBIT_APPROVAL_PENDING_COUNT" != "0" ]; then + # CodeRabbit has no check-run yet but has posted its own approval-pending + # issue comment for this exact head: it is actively engaged, not absent, + # so a different model's approval must not substitute for its own + # terminal verdict. Wait for CodeRabbit specifically. + add_waiting "Waiting for CodeRabbit to review the latest commit on ${HEAD_REF_OID}." else OPENCODE_FALLBACK_APPROVED=1 - printf 'CodeRabbit check is absent; accepted current-head OpenCode App adversarial approval on %s.\n' "$HEAD_REF_OID" + printf 'CodeRabbit check and issue-comment evidence are both absent; accepted current-head OpenCode App adversarial approval on %s.\n' "$HEAD_REF_OID" fi fi else @@ -429,49 +460,38 @@ else fi fi -if ! ISSUE_COMMENTS_JSON="$(gh api --paginate "repos/${GITHUB_REPOSITORY}/issues/${PR_NUMBER}/comments" 2>"$ISSUE_COMMENTS_ERROR_FILE")"; then - printf 'issue comment lookup failed:\n' - printf '%s\n' "$(<"$ISSUE_COMMENTS_ERROR_FILE")" | sed 's/^/ /' - add_blocker 'PR issue comments could not be read; see the workflow run log.' -else - CODERABBIT_ISSUE_BLOCKERS="$(printf '%s' "$ISSUE_COMMENTS_JSON" | jq -s \ - --arg head_sha "$HEAD_SHA" \ - --arg pattern "$CODERABBIT_ISSUE_BLOCKING_PATTERN" \ - --arg substantive_pattern "$CODERABBIT_ISSUE_SUBSTANTIVE_BLOCKING_PATTERN" \ - --arg no_actionable_pattern "$CODERABBIT_NO_ACTIONABLE_PATTERN" \ - --arg approval_pending_pattern "$CODERABBIT_APPROVAL_PENDING_PATTERN" ' - [.[][] - | select((.user.login // "") | test("'"$REVIEW_BOT_LOGIN_PATTERN"'"; "i")) - | select( - (.body // "") as $body - | ($body | split("
")[0]) as $summary - | (($body | contains("")) - and ($body | test($approval_pending_pattern; "i")) - | not) - and ($body | test($pattern; "i")) - and ( - (($body | test($no_actionable_pattern; "i")) | not) - or ($summary | test($substantive_pattern; "i")) - ) - ) - | select((.body // "") | contains($head_sha))] - | length' - )" - CODERABBIT_APPROVAL_PENDING_COUNT="$(printf '%s' "$ISSUE_COMMENTS_JSON" | jq -s \ - --arg head_sha "$HEAD_SHA" \ - --arg approval_pending_pattern "$CODERABBIT_APPROVAL_PENDING_PATTERN" ' - [.[][] - | select((.user.login // "") | test("'"$REVIEW_BOT_LOGIN_PATTERN"'"; "i")) - | select((.body // "") | contains("")) - | select((.body // "") | test($approval_pending_pattern; "i")) - | select((.body // "") | contains($head_sha))] - | length' - )" - if [ "$CODERABBIT_ISSUE_BLOCKERS" != "0" ]; then - add_blocker "Current-head CodeRabbit issue comment has blocking warning/failure evidence on ${HEAD_REF_OID}." - elif [ "$CODERABBIT_APPROVAL_PENDING_COUNT" != "0" ] && [ "$OPENCODE_FALLBACK_APPROVED" != "1" ]; then - add_waiting "Waiting for CodeRabbit to review the latest commit on ${HEAD_REF_OID}." - fi +# CODERABBIT_APPROVAL_PENDING_COUNT was already computed above (before the +# check-run/status lookup), from the same ISSUE_COMMENTS_JSON fetched there. +# Only the blocking-evidence scan runs here: it strips just the marker- +# delimited approval-pending span from each comment body before testing for +# blocking language, rather than excluding the whole comment whenever that +# marker is present anywhere in it -- a comment can legitimately carry both +# the boilerplate pending notice and a separate, real blocking finding, and +# the latter must still be caught. +CODERABBIT_ISSUE_BLOCKERS="$(printf '%s' "$ISSUE_COMMENTS_JSON" | jq -s \ + --arg head_sha "$HEAD_SHA" \ + --arg pattern "$CODERABBIT_ISSUE_BLOCKING_PATTERN" \ + --arg substantive_pattern "$CODERABBIT_ISSUE_SUBSTANTIVE_BLOCKING_PATTERN" \ + --arg no_actionable_pattern "$CODERABBIT_NO_ACTIONABLE_PATTERN" \ + --arg notice_span_pattern "$CODERABBIT_APPROVAL_NOTICE_SPAN_PATTERN" ' + [.[][] + | select((.user.login // "") | test("'"$REVIEW_BOT_LOGIN_PATTERN"'"; "i")) + | select( + ((.body // "") | gsub($notice_span_pattern; ""; "s")) as $body + | ($body | split("
")[0]) as $summary + | ($body | test($pattern; "i")) + and ( + (($body | test($no_actionable_pattern; "i")) | not) + or ($summary | test($substantive_pattern; "i")) + ) + ) + | select((.body // "") | contains($head_sha))] + | length' +)" +if [ "$CODERABBIT_ISSUE_BLOCKERS" != "0" ]; then + add_blocker "Current-head CodeRabbit issue comment has blocking warning/failure evidence on ${HEAD_REF_OID}." +elif [ "$CODERABBIT_APPROVAL_PENDING_COUNT" != "0" ]; then + add_waiting "Waiting for CodeRabbit to review the latest commit on ${HEAD_REF_OID}." fi if ! REVIEW_COMMENTS_JSON="$(gh api --paginate "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}/comments" 2>"$REVIEW_COMMENTS_ERROR_FILE")"; then diff --git a/scripts/ci/test_pr_governance_gate.sh b/scripts/ci/test_pr_governance_gate.sh index 4d4378301..4f852d378 100644 --- a/scripts/ci/test_pr_governance_gate.sh +++ b/scripts/ci/test_pr_governance_gate.sh @@ -255,6 +255,9 @@ if [ "$1" = "api" ] && [[ "$args" == *repos/*/issues/42/comments* ]]; then coderabbit_approval_pending|missing_coderabbit_adversarial_approval_with_pending_notice) printf '[{"id":777,"user":{"login":"coderabbitai[bot]"},"created_at":"2026-05-19T00:01:00Z","body":"CodeRabbit has no unresolved comments, but it has not reviewed the latest commit. CodeRabbit will approve the changes if it finds no blocking issues. "}]' ;; + coderabbit_approval_pending_with_separate_blocking_warning) + printf '[{"id":777,"user":{"login":"coderabbitai[bot]"},"created_at":"2026-05-19T00:01:00Z","body":"CodeRabbit has no unresolved comments, but it has not reviewed the latest commit. CodeRabbit will approve the changes if it finds no blocking issues. \\n\\nSeparately: Pre-merge blocking warning for 0123456789abcdef0123456789abcdef01234567."}]' + ;; github_code_quality_blocking_comment) printf '[{"id":777,"user":{"login":"github-code-quality[bot]"},"created_at":"2026-05-19T00:01:00Z","body":"Potential issue for 0123456789abcdef0123456789abcdef01234567"}]' ;; @@ -568,16 +571,40 @@ assert_missing_coderabbit_accepts_exact_head_adversarial_opencode_approval() { assert_in_file 'conclusion=success' "$temp_dir/gh.log" } -assert_missing_coderabbit_adversarial_approval_survives_pending_notice() { +assert_missing_coderabbit_adversarial_approval_still_waits_for_pending_coderabbit_notice() { + # CodeRabbit has no check-run yet, but its own approval-pending issue + # comment shows it is actively reviewing this exact head -- it is not + # absent, so a structured OpenCode adversarial approval must not + # substitute for CodeRabbit's own terminal verdict. Governance must stay + # non-terminal (waiting), never publish conclusion=success, on this + # combination. local temp_dir temp_dir="$(mktemp -d)" run_gate missing_coderabbit_adversarial_approval_with_pending_notice "$temp_dir" assert_exit_code 0 "$temp_dir" - assert_in_file 'accepted current-head OpenCode App adversarial approval' "$temp_dir/output.txt" - assert_in_file 'PR governance metadata gate is ready' "$temp_dir/output.txt" - assert_not_in_file 'Waiting for CodeRabbit to review the latest commit' "$temp_dir/output.txt" - assert_in_file 'conclusion=success' "$temp_dir/gh.log" + assert_in_file 'Waiting for CodeRabbit to review the latest commit' "$temp_dir/output.txt" + assert_not_in_file 'accepted current-head OpenCode App adversarial approval' "$temp_dir/output.txt" + assert_not_in_file 'PR governance metadata gate is ready' "$temp_dir/output.txt" + assert_not_in_file 'conclusion=success' "$temp_dir/gh.log" + assert_in_file 'status=in_progress' "$temp_dir/gh.log" +} + +assert_coderabbit_approval_pending_notice_does_not_hide_separate_blocking_warning() { + # A CodeRabbit issue comment can legitimately carry both the boilerplate + # approval-pending notice and a separate, genuine pre-merge blocking + # warning in the same body. Excluding the whole comment from the + # blocking-evidence scan whenever the pending-notice marker appears + # anywhere in it would hide that second, real finding -- only the + # marker-delimited span itself should be exempted. + local temp_dir + temp_dir="$(mktemp -d)" + run_gate coderabbit_approval_pending_with_separate_blocking_warning "$temp_dir" + + assert_exit_code 0 "$temp_dir" + assert_in_file 'Current-head CodeRabbit issue comment has blocking warning/failure evidence' "$temp_dir/gh.log" + assert_in_file 'conclusion=failure' "$temp_dir/gh.log" + assert_not_in_file '^pr merge' "$temp_dir/gh.log" } assert_missing_coderabbit_rejects_non_authoritative_opencode_evidence() { @@ -976,7 +1003,7 @@ assert_coderabbit_failed_commit_status_blocks assert_coderabbit_unknown_commit_status_fails_closed assert_missing_coderabbit_waits_for_adversarial_opencode_approval assert_missing_coderabbit_accepts_exact_head_adversarial_opencode_approval -assert_missing_coderabbit_adversarial_approval_survives_pending_notice +assert_missing_coderabbit_adversarial_approval_still_waits_for_pending_coderabbit_notice assert_missing_coderabbit_rejects_non_authoritative_opencode_evidence assert_opencode_review_lookup_error_is_logged_but_not_published_verbatim assert_completed_gate_check_is_republished_as_new_run @@ -995,6 +1022,7 @@ assert_coderabbit_review_limit_issue_comment_does_not_block assert_coderabbit_no_actionable_summary_does_not_block assert_coderabbit_no_actionable_summary_with_blocker_still_blocks assert_coderabbit_approval_pending_waits_without_blocking +assert_coderabbit_approval_pending_notice_does_not_hide_separate_blocking_warning assert_coderabbit_current_review_comment_blocks assert_coderabbit_resolved_current_review_comment_does_not_block assert_truncated_review_thread_metadata_blocks From ae0f09df7572d3bd090e9b6bb9fffd287f67b33c Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 03:38:47 +0000 Subject: [PATCH 03/11] test(ci): also reject branches-ignore in stacked-PR trigger contracts CodeRabbit review on this PR: the stacked-PR base-support contract tests only asserted the absence of a branches: filter on each governed workflow's pull_request trigger, not branches-ignore: -- which can just as effectively exclude a stacked PR's base branch. Added the matching assertion at all three flagged locations: tests/test_stacked_pr_workflow_contract.py's generic loop (covers all four governed workflows) and the two per-file checks in backend/tests/test_release_governance.py (app-ci.yml, docker-publish.yml). Verified: python3 -m pytest tests/test_stacked_pr_workflow_contract.py -q -> 1 passed. Manually re-checked both new backend/tests/test_release_governance.py assertions directly against the actual workflow files (neither app-ci.yml nor docker-publish.yml's pull_request block contains branches-ignore:, so both hold). ruff check: clean. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_019YgmaEKnPhrsbSyQBierFH --- backend/tests/test_release_governance.py | 2 ++ tests/test_stacked_pr_workflow_contract.py | 6 ++++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/backend/tests/test_release_governance.py b/backend/tests/test_release_governance.py index dfcb1cb16..35ae2c496 100644 --- a/backend/tests/test_release_governance.py +++ b/backend/tests/test_release_governance.py @@ -670,6 +670,7 @@ def test_app_ci_runs_backend_and_frontend_checks_without_duplicate_release_pushe pull_request_block = workflow.split("pull_request:", 1)[1].split("push:", 1)[0] assert "branches:" not in pull_request_block + assert "branches-ignore:" not in pull_request_block def test_docker_publish_validates_pr_images_and_publishes_semver_images_only_on_tags() -> ( @@ -713,6 +714,7 @@ def test_docker_publish_validates_pr_images_and_publishes_semver_images_only_on_ assert "tags:" in push_block assert "branches:" not in push_block assert "branches:" not in pull_request_block + assert "branches-ignore:" not in pull_request_block assert "ai_email_client-backend" in workflow assert "ai_email_client-frontend" in workflow assert workflow.count("image: naruon") == 2 diff --git a/tests/test_stacked_pr_workflow_contract.py b/tests/test_stacked_pr_workflow_contract.py index d54014e0d..2bd05d4f6 100644 --- a/tests/test_stacked_pr_workflow_contract.py +++ b/tests/test_stacked_pr_workflow_contract.py @@ -22,6 +22,8 @@ def test_governed_pull_request_workflows_accept_stacked_base_branches() -> None: workflow, ) assert pull_request_trigger is not None, f"{name} must run on pull_request" - assert "branches:" not in pull_request_trigger.group("body"), ( - f"{name} must not exclude stacked PR base branches" + body = pull_request_trigger.group("body") + assert "branches:" not in body, f"{name} must not exclude stacked PR base branches" + assert "branches-ignore:" not in body, ( + f"{name} must not exclude stacked PR base branches via branches-ignore" ) From e03c805c2a47ae9f67aedee1998b61d43a6a8f7d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 12:09:48 +0000 Subject: [PATCH 04/11] docs(governance): fix stale PR-base/push-branch scoping in merge-gate-policy Devin Review flagged docs/development/merge-gate-policy.md as stale against this PR's own change: it still said Application CI runs "on pull requests to master and release/**" with push checks "scoped to master", but this PR removes app-ci.yml's pull_request: branches: restriction entirely (so every PR base runs required checks, including one stacked on another open PR's branch) and push already covers both develop and master. Updated the policy text to match the actual workflow triggers. No test pins the old wording (checked backend/tests/test_release_governance.py's only assertion against this file, which is unrelated -- "GitHub Checks integration stays disabled"); verified that assertion still passes and tests/test_stacked_pr_workflow_contract.py still passes. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_019YgmaEKnPhrsbSyQBierFH --- docs/development/merge-gate-policy.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/development/merge-gate-policy.md b/docs/development/merge-gate-policy.md index 5875d21a6..b705aa8d5 100644 --- a/docs/development/merge-gate-policy.md +++ b/docs/development/merge-gate-policy.md @@ -9,8 +9,9 @@ awaited by default. - Required status checks must pass on the current head SHA. - Application CI must run backend pytest and frontend test/lint/build checks on - pull requests to `master` and `release/**`, while release-branch pushes must - not create duplicate check noise; push checks are scoped to `master`. + every pull request, including one stacked on another open PR's branch, not + just `develop`/`master`/`release/**` — required checks must not exclude a PR + base; push checks are scoped to `develop` and `master`. - The robot-review gate prefers CodeRabbit evidence. When the current head has CodeRabbit check-run evidence, it satisfies the gate only when current-head blocking findings, warnings, and failures are fixed, rebutted with evidence, From 27ab83b57e94dc84bf5f8c8708d5b2b7a962304f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 12:19:02 +0000 Subject: [PATCH 05/11] chore(governance): remove unused OPENCODE_FALLBACK_APPROVED flag CodeRabbit correctly flagged this as dead code: it's assigned when CodeRabbit check-run and issue-comment evidence are both absent (so an OpenCode adversarial approval is accepted as a fallback), but nothing downstream ever reads it. Verified this doesn't change gate behavior -- the later wait-state check re-examines CODERABBIT_APPROVAL_PENDING_COUNT directly (the same value, computed once and reused, not recomputed), which is already guaranteed to be "0" whenever this flag would have been set, so the wait branch it would have needed to suppress can't trigger anyway. No test (scripts/ci/test_pr_governance_gate.sh included) references this variable. Verified: bash -n syntax check clean, full scripts/ci/test_pr_governance_gate.sh suite passes, and tests/test_stacked_pr_workflow_contract.py passes. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_019YgmaEKnPhrsbSyQBierFH --- scripts/ci/pr_governance_gate.sh | 2 -- 1 file changed, 2 deletions(-) diff --git a/scripts/ci/pr_governance_gate.sh b/scripts/ci/pr_governance_gate.sh index 266c195de..910e040fc 100644 --- a/scripts/ci/pr_governance_gate.sh +++ b/scripts/ci/pr_governance_gate.sh @@ -27,7 +27,6 @@ OWNER="${GITHUB_REPOSITORY%/*}" REPO="${GITHUB_REPOSITORY#*/}" BLOCKERS=() WAITING=() -OPENCODE_FALLBACK_APPROVED=0 PR_CHECKS_ERROR_FILE="$(mktemp)" ISSUE_COMMENTS_ERROR_FILE="$(mktemp)" REVIEW_COMMENTS_ERROR_FILE="$(mktemp)" @@ -419,7 +418,6 @@ if [ "$CODERABBIT_COUNT" = "0" ]; then # terminal verdict. Wait for CodeRabbit specifically. add_waiting "Waiting for CodeRabbit to review the latest commit on ${HEAD_REF_OID}." else - OPENCODE_FALLBACK_APPROVED=1 printf 'CodeRabbit check and issue-comment evidence are both absent; accepted current-head OpenCode App adversarial approval on %s.\n' "$HEAD_REF_OID" fi fi From 550798ccafebea4b1a9a65018e63b9661ff25a53 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 4 Sep 2026 21:49:53 +0900 Subject: [PATCH 06/11] fix(governance): honor OpenCode fallback evidence When no CodeRabbit check exists, accept a qualifying exact-head structured OpenCode approval even if a CodeRabbit pending issue notice is present. Preserve blocking-warning detection and set -u safety. Assisted-by: OpenAI Codex Signed-off-by: Seongho Bae --- scripts/ci/pr_governance_gate.sh | 11 +++-------- scripts/ci/test_pr_governance_gate.sh | 21 ++++++++------------- 2 files changed, 11 insertions(+), 21 deletions(-) diff --git a/scripts/ci/pr_governance_gate.sh b/scripts/ci/pr_governance_gate.sh index 910e040fc..bfd813f42 100644 --- a/scripts/ci/pr_governance_gate.sh +++ b/scripts/ci/pr_governance_gate.sh @@ -391,6 +391,7 @@ CODERABBIT_STATUS_MATCHES="$(printf '%s' "$COMMIT_STATUS_JSON" | jq ' CODERABBIT_CHECK_COUNT="$(printf '%s' "$CODERABBIT_MATCHES" | jq 'length')" CODERABBIT_STATUS_COUNT="$(printf '%s' "$CODERABBIT_STATUS_MATCHES" | jq 'length')" CODERABBIT_COUNT=$((CODERABBIT_CHECK_COUNT + CODERABBIT_STATUS_COUNT)) +OPENCODE_ADVERSARIAL_APPROVAL_COUNT=0 if [ "$CODERABBIT_COUNT" = "0" ]; then if ! OPENCODE_REVIEWS_JSON="$(gh api --paginate --slurp "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}/reviews" 2>"$OPENCODE_REVIEWS_ERROR_FILE")"; then printf 'OpenCode review lookup failed:\n' @@ -411,14 +412,8 @@ if [ "$CODERABBIT_COUNT" = "0" ]; then )" if [ "$OPENCODE_ADVERSARIAL_APPROVAL_COUNT" = "0" ]; then add_waiting "Waiting for current-head CodeRabbit evidence or a structured OpenCode App adversarial approval on ${HEAD_REF_OID}." - elif [ "$CODERABBIT_APPROVAL_PENDING_COUNT" != "0" ]; then - # CodeRabbit has no check-run yet but has posted its own approval-pending - # issue comment for this exact head: it is actively engaged, not absent, - # so a different model's approval must not substitute for its own - # terminal verdict. Wait for CodeRabbit specifically. - add_waiting "Waiting for CodeRabbit to review the latest commit on ${HEAD_REF_OID}." else - printf 'CodeRabbit check and issue-comment evidence are both absent; accepted current-head OpenCode App adversarial approval on %s.\n' "$HEAD_REF_OID" + printf 'CodeRabbit check evidence is absent; accepted current-head OpenCode App adversarial approval on %s.\n' "$HEAD_REF_OID" fi fi else @@ -488,7 +483,7 @@ CODERABBIT_ISSUE_BLOCKERS="$(printf '%s' "$ISSUE_COMMENTS_JSON" | jq -s \ )" if [ "$CODERABBIT_ISSUE_BLOCKERS" != "0" ]; then add_blocker "Current-head CodeRabbit issue comment has blocking warning/failure evidence on ${HEAD_REF_OID}." -elif [ "$CODERABBIT_APPROVAL_PENDING_COUNT" != "0" ]; then +elif [ "$CODERABBIT_APPROVAL_PENDING_COUNT" != "0" ] && [ "$OPENCODE_ADVERSARIAL_APPROVAL_COUNT" = "0" ]; then add_waiting "Waiting for CodeRabbit to review the latest commit on ${HEAD_REF_OID}." fi diff --git a/scripts/ci/test_pr_governance_gate.sh b/scripts/ci/test_pr_governance_gate.sh index 4f852d378..47d60e1b9 100644 --- a/scripts/ci/test_pr_governance_gate.sh +++ b/scripts/ci/test_pr_governance_gate.sh @@ -571,23 +571,18 @@ assert_missing_coderabbit_accepts_exact_head_adversarial_opencode_approval() { assert_in_file 'conclusion=success' "$temp_dir/gh.log" } -assert_missing_coderabbit_adversarial_approval_still_waits_for_pending_coderabbit_notice() { - # CodeRabbit has no check-run yet, but its own approval-pending issue - # comment shows it is actively reviewing this exact head -- it is not - # absent, so a structured OpenCode adversarial approval must not - # substitute for CodeRabbit's own terminal verdict. Governance must stay - # non-terminal (waiting), never publish conclusion=success, on this - # combination. +assert_missing_coderabbit_adversarial_approval_overrides_pending_notice() { + # With no CodeRabbit check-run, its issue-comment notice is not gate + # evidence. The exact-head structured OpenCode approval satisfies the + # documented fallback even while that notice remains present. local temp_dir temp_dir="$(mktemp -d)" run_gate missing_coderabbit_adversarial_approval_with_pending_notice "$temp_dir" assert_exit_code 0 "$temp_dir" - assert_in_file 'Waiting for CodeRabbit to review the latest commit' "$temp_dir/output.txt" - assert_not_in_file 'accepted current-head OpenCode App adversarial approval' "$temp_dir/output.txt" - assert_not_in_file 'PR governance metadata gate is ready' "$temp_dir/output.txt" - assert_not_in_file 'conclusion=success' "$temp_dir/gh.log" - assert_in_file 'status=in_progress' "$temp_dir/gh.log" + assert_in_file 'accepted current-head OpenCode App adversarial approval' "$temp_dir/output.txt" + assert_in_file 'PR governance metadata gate is ready' "$temp_dir/output.txt" + assert_in_file 'conclusion=success' "$temp_dir/gh.log" } assert_coderabbit_approval_pending_notice_does_not_hide_separate_blocking_warning() { @@ -1003,7 +998,7 @@ assert_coderabbit_failed_commit_status_blocks assert_coderabbit_unknown_commit_status_fails_closed assert_missing_coderabbit_waits_for_adversarial_opencode_approval assert_missing_coderabbit_accepts_exact_head_adversarial_opencode_approval -assert_missing_coderabbit_adversarial_approval_still_waits_for_pending_coderabbit_notice +assert_missing_coderabbit_adversarial_approval_overrides_pending_notice assert_missing_coderabbit_rejects_non_authoritative_opencode_evidence assert_opencode_review_lookup_error_is_logged_but_not_published_verbatim assert_completed_gate_check_is_republished_as_new_run From e058f3ead35f9a19d3c3b20c6ab5fc04d2e2cbb2 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 6 Sep 2026 20:57:06 +0900 Subject: [PATCH 07/11] fix(ci): preserve multiline review wait semantics Retain all predecessor governance changes. Move the existing four-workflow regression into the CI-collected backend tree without dropping assertions; correct the shared jq notice span after a multiline-fixture RED reproduction. Co-Authored-By: Codex Signed-off-by: Seongho Bae --- .../test_stacked_pr_workflow_contract.py | 2 +- docs/development/merge-gate-policy.md | 21 +++++++++++++++++++ scripts/ci/pr_governance_gate.sh | 2 +- scripts/ci/test_pr_governance_gate.sh | 4 ++-- 4 files changed, 25 insertions(+), 4 deletions(-) rename {tests => backend/tests}/test_stacked_pr_workflow_contract.py (95%) diff --git a/tests/test_stacked_pr_workflow_contract.py b/backend/tests/test_stacked_pr_workflow_contract.py similarity index 95% rename from tests/test_stacked_pr_workflow_contract.py rename to backend/tests/test_stacked_pr_workflow_contract.py index 2bd05d4f6..0929d4aca 100644 --- a/tests/test_stacked_pr_workflow_contract.py +++ b/backend/tests/test_stacked_pr_workflow_contract.py @@ -4,7 +4,7 @@ import re -REPO_ROOT = Path(__file__).resolve().parents[1] +REPO_ROOT = Path(__file__).resolve().parents[2] GOVERNED_PULL_REQUEST_WORKFLOWS = ( "app-ci.yml", "bandit.yml", diff --git a/docs/development/merge-gate-policy.md b/docs/development/merge-gate-policy.md index b705aa8d5..a53a63482 100644 --- a/docs/development/merge-gate-policy.md +++ b/docs/development/merge-gate-policy.md @@ -66,6 +66,27 @@ awaited by default. ## Evidence commands +### Multiline robot notices and collected regressions (2026-09-06) + +At predecessor `550798ccafebea4b1a9a65018e63b9661ff25a53`, a multiline +CodeRabbit approval notice could be mistaken for a substantive blocker even +after an exact-head OpenCode fallback was accepted. The existing fake-GitHub +shell harness reproduced this after adding realistic line breaks: it published +a failure instead of the expected ready state. The common notice-removal +expression now uses jq's `m` flag so dot matches newlines; `s` only changes +anchor semantics. Only the non-greedy marker-delimited notice is removed; +separate blocking warnings must remain blockers. No approval or review rule +is weakened and no external GitHub mutation occurs in this harness. + +Run `bash scripts/ci/test_pr_governance_gate.sh` for the full metadata scenarios. +The four-workflow stacked-base regression lives in +`backend/tests/test_stacked_pr_workflow_contract.py`, where Application CI +collects it; the root-level copy was moved, not discarded or duplicated. +Local harness evidence is not a hosted approval or protected merge. + +Reference: jqlang. (n.d.). *Regular expressions*. In *jq 1.7 manual*. +https://jqlang.org/manual/v1.7/#regular-expressions + Use the same head SHA across all checks: ```bash diff --git a/scripts/ci/pr_governance_gate.sh b/scripts/ci/pr_governance_gate.sh index bfd813f42..764af9abf 100644 --- a/scripts/ci/pr_governance_gate.sh +++ b/scripts/ci/pr_governance_gate.sh @@ -470,7 +470,7 @@ CODERABBIT_ISSUE_BLOCKERS="$(printf '%s' "$ISSUE_COMMENTS_JSON" | jq -s \ [.[][] | select((.user.login // "") | test("'"$REVIEW_BOT_LOGIN_PATTERN"'"; "i")) | select( - ((.body // "") | gsub($notice_span_pattern; ""; "s")) as $body + ((.body // "") | gsub($notice_span_pattern; ""; "m")) as $body | ($body | split("
")[0]) as $summary | ($body | test($pattern; "i")) and ( diff --git a/scripts/ci/test_pr_governance_gate.sh b/scripts/ci/test_pr_governance_gate.sh index 47d60e1b9..1891520cf 100644 --- a/scripts/ci/test_pr_governance_gate.sh +++ b/scripts/ci/test_pr_governance_gate.sh @@ -253,10 +253,10 @@ if [ "$1" = "api" ] && [[ "$args" == *repos/*/issues/42/comments* ]]; then printf '[{"id":777,"user":{"login":"coderabbitai[bot]"},"created_at":"2026-05-19T00:01:00Z","body":"No actionable comments were generated in the recent review. Blocking issue remains on 0123456789abcdef0123456789abcdef01234567."}]' ;; coderabbit_approval_pending|missing_coderabbit_adversarial_approval_with_pending_notice) - printf '[{"id":777,"user":{"login":"coderabbitai[bot]"},"created_at":"2026-05-19T00:01:00Z","body":"CodeRabbit has no unresolved comments, but it has not reviewed the latest commit. CodeRabbit will approve the changes if it finds no blocking issues. "}]' + printf '[{"id":777,"user":{"login":"coderabbitai[bot]"},"created_at":"2026-05-19T00:01:00Z","body":"\\nCodeRabbit has no unresolved comments, but it has not reviewed the latest commit. \\nCodeRabbit will approve the changes if it finds no blocking issues. \\n"}]' ;; coderabbit_approval_pending_with_separate_blocking_warning) - printf '[{"id":777,"user":{"login":"coderabbitai[bot]"},"created_at":"2026-05-19T00:01:00Z","body":"CodeRabbit has no unresolved comments, but it has not reviewed the latest commit. CodeRabbit will approve the changes if it finds no blocking issues. \\n\\nSeparately: Pre-merge blocking warning for 0123456789abcdef0123456789abcdef01234567."}]' + printf '[{"id":777,"user":{"login":"coderabbitai[bot]"},"created_at":"2026-05-19T00:01:00Z","body":"\\nCodeRabbit has no unresolved comments, but it has not reviewed the latest commit. \\nCodeRabbit will approve the changes if it finds no blocking issues. \\n\\n\\nSeparately: Pre-merge blocking warning for 0123456789abcdef0123456789abcdef01234567."}]' ;; github_code_quality_blocking_comment) printf '[{"id":777,"user":{"login":"github-code-quality[bot]"},"created_at":"2026-05-19T00:01:00Z","body":"Potential issue for 0123456789abcdef0123456789abcdef01234567"}]' From fac3437c03d928e45632763530a4f130dfe505fd Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 6 Sep 2026 21:32:10 +0900 Subject: [PATCH 08/11] fix: preserve successful review evidence over stale notices Repair review 3939597997 with separate success-check and success-status regressions. Correct the notice-only fixture and stale fallback commentary; preserve all predecessor delta and substantive blockers. No deletion, restack, or policy weakening. Co-authored-by: Codex Signed-off-by: Seongho Bae --- docs/development/merge-gate-policy.md | 12 ++++++++++++ scripts/ci/pr_governance_gate.sh | 12 +++++------- scripts/ci/test_pr_governance_gate.sh | 20 +++++++++++++++++--- 3 files changed, 34 insertions(+), 10 deletions(-) diff --git a/docs/development/merge-gate-policy.md b/docs/development/merge-gate-policy.md index a53a63482..6ad74eac3 100644 --- a/docs/development/merge-gate-policy.md +++ b/docs/development/merge-gate-policy.md @@ -84,6 +84,18 @@ The four-workflow stacked-base regression lives in collects it; the root-level copy was moved, not discarded or duplicated. Local harness evidence is not a hosted approval or protected merge. +Review finding #3939597997 exposed a second stale-notice path at +`e058f3ead35f9a19d3c3b20c6ab5fc04d2e2cbb2`: a successful current-head +CodeRabbit check or status was overwritten by an `in_progress` governance +result solely because its issue comment still carried the pending notice. +The new success-check fixture failed the expected ready assertion; the fake +publisher explicitly emitted `in_progress`. Pending notices now add a wait +only when CodeRabbit check/status evidence is absent and no qualifying +OpenCode approval exists. Existing pending/failed check handling and separate +substantive-comment blockers remain authoritative. The full shell harness +covers success checks and success statuses with stale notices separately; +the notice-only waiting fixture now correctly contains no check evidence. + Reference: jqlang. (n.d.). *Regular expressions*. In *jq 1.7 manual*. https://jqlang.org/manual/v1.7/#regular-expressions diff --git a/scripts/ci/pr_governance_gate.sh b/scripts/ci/pr_governance_gate.sh index 764af9abf..2565e84f3 100644 --- a/scripts/ci/pr_governance_gate.sh +++ b/scripts/ci/pr_governance_gate.sh @@ -344,12 +344,10 @@ CODERABBIT_NO_ACTIONABLE_PATTERN='no actionable comments? (were )?generated' CODERABBIT_APPROVAL_PENDING_PATTERN='CodeRabbit has no unresolved comments, but it has not reviewed the latest commit' CODERABBIT_APPROVAL_NOTICE_SPAN_PATTERN='.*?' -# Fetched and evaluated before the check-run/status lookup below so the -# no-check-run OpenCode fallback can tell "CodeRabbit has never engaged" -# (check AND issue-comment both silent) apart from "CodeRabbit is actively -# reviewing, just hasn't reached the latest commit yet" (an approval-pending -# issue comment despite no check-run yet). Only the former is eligible for -# the fallback; the latter must still wait on CodeRabbit itself. +# Fetch comments once for both pending notices and substantive blockers. +# Without CodeRabbit check/status evidence, an exact-head structured OpenCode +# approval may satisfy the fallback even when a pending notice exists. +# Separate findings still block; a notice without either evidence source waits. if ! ISSUE_COMMENTS_JSON="$(gh api --paginate "repos/${GITHUB_REPOSITORY}/issues/${PR_NUMBER}/comments" 2>"$ISSUE_COMMENTS_ERROR_FILE")"; then printf 'issue comment lookup failed:\n' printf '%s\n' "$(<"$ISSUE_COMMENTS_ERROR_FILE")" | sed 's/^/ /' @@ -483,7 +481,7 @@ CODERABBIT_ISSUE_BLOCKERS="$(printf '%s' "$ISSUE_COMMENTS_JSON" | jq -s \ )" if [ "$CODERABBIT_ISSUE_BLOCKERS" != "0" ]; then add_blocker "Current-head CodeRabbit issue comment has blocking warning/failure evidence on ${HEAD_REF_OID}." -elif [ "$CODERABBIT_APPROVAL_PENDING_COUNT" != "0" ] && [ "$OPENCODE_ADVERSARIAL_APPROVAL_COUNT" = "0" ]; then +elif [ "$CODERABBIT_COUNT" = "0" ] && [ "$CODERABBIT_APPROVAL_PENDING_COUNT" != "0" ] && [ "$OPENCODE_ADVERSARIAL_APPROVAL_COUNT" = "0" ]; then add_waiting "Waiting for CodeRabbit to review the latest commit on ${HEAD_REF_OID}." fi diff --git a/scripts/ci/test_pr_governance_gate.sh b/scripts/ci/test_pr_governance_gate.sh index 1891520cf..93250c373 100644 --- a/scripts/ci/test_pr_governance_gate.sh +++ b/scripts/ci/test_pr_governance_gate.sh @@ -143,7 +143,7 @@ if [ "$1" = "api" ] && [[ "$2" == repos/*/commits/*/check-runs* ]]; then coderabbit_pending) printf '{"check_runs":[{"name":"CodeRabbit","app":{"slug":"coderabbitai"},"status":"in_progress","conclusion":null,"html_url":"https://checks/coderabbit"}]}' ;; - missing_coderabbit|missing_coderabbit_with_adversarial_approval|missing_coderabbit_stale_approval|missing_coderabbit_actions_approval|missing_coderabbit_one_probe|missing_coderabbit_adversarial_approval_with_pending_notice|opencode_reviews_error|coderabbit_status_success|coderabbit_status_pending|coderabbit_status_failed|coderabbit_status_unknown) + missing_coderabbit|missing_coderabbit_with_adversarial_approval|missing_coderabbit_stale_approval|missing_coderabbit_actions_approval|missing_coderabbit_one_probe|missing_coderabbit_adversarial_approval_with_pending_notice|coderabbit_approval_pending|coderabbit_status_success_with_pending_notice|opencode_reviews_error|coderabbit_status_success|coderabbit_status_pending|coderabbit_status_failed|coderabbit_status_unknown) printf '{"check_runs":[]}' ;; coderabbit_failed) @@ -170,7 +170,7 @@ fi if [ "$1" = "api" ] && [[ "$2" == repos/*/commits/*/status ]]; then case "${GH_SCENARIO:-pass}" in - coderabbit_status_success) + coderabbit_status_success|coderabbit_status_success_with_pending_notice) printf '{"statuses":[{"context":"CodeRabbit","state":"success","description":"Review approved","created_at":"2026-07-29T01:54:41Z","updated_at":"2026-07-29T01:54:41Z"}]}' ;; coderabbit_status_pending) @@ -252,7 +252,7 @@ if [ "$1" = "api" ] && [[ "$args" == *repos/*/issues/42/comments* ]]; then coderabbit_no_actionable_with_blocker) printf '[{"id":777,"user":{"login":"coderabbitai[bot]"},"created_at":"2026-05-19T00:01:00Z","body":"No actionable comments were generated in the recent review. Blocking issue remains on 0123456789abcdef0123456789abcdef01234567."}]' ;; - coderabbit_approval_pending|missing_coderabbit_adversarial_approval_with_pending_notice) + coderabbit_approval_pending|missing_coderabbit_adversarial_approval_with_pending_notice|coderabbit_check_success_with_pending_notice|coderabbit_status_success_with_pending_notice) printf '[{"id":777,"user":{"login":"coderabbitai[bot]"},"created_at":"2026-05-19T00:01:00Z","body":"\\nCodeRabbit has no unresolved comments, but it has not reviewed the latest commit. \\nCodeRabbit will approve the changes if it finds no blocking issues. \\n"}]' ;; coderabbit_approval_pending_with_separate_blocking_warning) @@ -816,6 +816,19 @@ assert_coderabbit_no_actionable_summary_with_blocker_still_blocks() { assert_not_in_file '^pr merge' "$temp_dir/gh.log" } +assert_coderabbit_success_overrides_stale_pending_notice() { + local scenario temp_dir + for scenario in coderabbit_check_success_with_pending_notice coderabbit_status_success_with_pending_notice; do + temp_dir="$(mktemp -d)" + run_gate "$scenario" "$temp_dir" + assert_exit_code 0 "$temp_dir" + assert_in_file 'PR governance metadata gate is ready' "$temp_dir/output.txt" + assert_in_file 'conclusion=success' "$temp_dir/gh.log" + assert_not_in_file 'Waiting for CodeRabbit to review the latest commit' "$temp_dir/output.txt" + assert_not_in_file '^pr merge' "$temp_dir/gh.log" + done +} + assert_coderabbit_approval_pending_waits_without_blocking() { local temp_dir temp_dir="$(mktemp -d)" @@ -1016,6 +1029,7 @@ assert_coderabbit_stale_issue_comment_does_not_block assert_coderabbit_review_limit_issue_comment_does_not_block assert_coderabbit_no_actionable_summary_does_not_block assert_coderabbit_no_actionable_summary_with_blocker_still_blocks +assert_coderabbit_success_overrides_stale_pending_notice assert_coderabbit_approval_pending_waits_without_blocking assert_coderabbit_approval_pending_notice_does_not_hide_separate_blocking_warning assert_coderabbit_current_review_comment_blocks From 179229d5fcb9a78588319e106dcec7dada0ed598 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 6 Sep 2026 22:33:46 +0900 Subject: [PATCH 09/11] fix(ci): authenticate review evidence and reject warning outputs --- docs/development/merge-gate-policy.md | 20 ++++++++++++ scripts/ci/pr_governance_gate.sh | 15 +++++---- scripts/ci/test_pr_governance_gate.sh | 44 ++++++++++++++++++++++++--- 3 files changed, 66 insertions(+), 13 deletions(-) diff --git a/docs/development/merge-gate-policy.md b/docs/development/merge-gate-policy.md index 6ad74eac3..317a57a78 100644 --- a/docs/development/merge-gate-policy.md +++ b/docs/development/merge-gate-policy.md @@ -47,6 +47,12 @@ awaited by default. resolution via GITHUB_PATH. - Authoritative `Review skipped` evidence counts only when the same check output carries no blocking warning/failure language alongside it. +- A review-like check name or status context is not publisher authentication. + Check evidence requires the `coderabbitai` or `github-code-quality` App slug; + status evidence requires the corresponding exact `[bot]` creator and Bot type. + Missing or unrelated publishers cannot replace the OpenCode fallback. + Successful and skipped check conclusions still block when their own output + contains the blocking warning/failure evidence described above. - `reviewDecision=CHANGES_REQUESTED` is a blocker until requested changes are addressed or superseded on the current head. - Blocker comments use the idempotent @@ -96,6 +102,20 @@ substantive-comment blockers remain authoritative. The full shell harness covers success checks and success statuses with stale notices separately; the notice-only waiting fixture now correctly contains no check evidence. +Independent readiness review at `fac3437c03d928e45632763530a4f130dfe505fd` +identified two pre-existing metadata risks, not regressions introduced by the +stale-notice repair: name-only publisher matching and success/skipped results +bypassing output inspection. Six isolated fake-GitHub scenarios each emitted +an incorrect success before the fix: an unrelated App check or status creator, +each with and without the pending notice, and success/skipped checks carrying +a pre-merge blocking warning. Publisher authentication now precedes evidence +selection, and output inspection precedes conclusion acceptance. The real +current-head CodeRabbit status creator was checked as `coderabbitai[bot]`, Bot. +These are local metadata-gate findings; no protected-branch bypass or malicious +publication in production was established. Keep the full harness, authoritative +positive cases, fallback and separate-comment blockers intact when repairing +this boundary; do not grant another publisher access to silence a wait state. + Reference: jqlang. (n.d.). *Regular expressions*. In *jq 1.7 manual*. https://jqlang.org/manual/v1.7/#regular-expressions diff --git a/scripts/ci/pr_governance_gate.sh b/scripts/ci/pr_governance_gate.sh index 2565e84f3..7a44e79f7 100644 --- a/scripts/ci/pr_governance_gate.sh +++ b/scripts/ci/pr_governance_gate.sh @@ -377,11 +377,13 @@ CODERABBIT_MATCHES="$(printf '%s' "$CHECK_RUNS" | jq ' | select( .app.slug == "coderabbitai" or .app.slug == "github-code-quality" - or (.name | test("CodeRabbit|coderabbit|GitHub Code Quality|github-code-quality"; "i")) )]' )" CODERABBIT_STATUS_MATCHES="$(printf '%s' "$COMMIT_STATUS_JSON" | jq ' [.statuses[] + | select(.creator.type == "Bot") + | select((.creator.login // "" | ascii_downcase) as $login + | $login == "coderabbitai[bot]" or $login == "github-code-quality[bot]") | select((.context // "") | test("CodeRabbit|coderabbit|GitHub Code Quality|github-code-quality"; "i"))] | group_by((.context // "") | ascii_downcase) | map(sort_by(.updated_at // .created_at // "") | last) @@ -420,15 +422,12 @@ else CODERABBIT_FAILED="$(printf '%s' "$CODERABBIT_MATCHES" | jq --arg pattern "$CODERABBIT_BLOCKING_PATTERN" ' [.[] | select(.status == "completed") + | ([.output.title, .output.summary, .output.text] | map(. // "") | join("\n")) as $check_output | select((.conclusion // "") as $conclusion - | if $conclusion == "success" or $conclusion == "skipped" then false + | if $check_output | test($pattern; "i") then true + elif $conclusion == "success" or $conclusion == "skipped" then false elif $conclusion == "neutral" then - # Skip evidence only counts when the output carries no blocking - # language alongside it. - (([.output.title, .output.summary, .output.text] | map(. // "") | join("\n")) as $neutral_output - | (($neutral_output | test("Review skipped"; "i")) - and (($neutral_output | test($pattern; "i")) | not)) - | not) + ($check_output | test("Review skipped"; "i") | not) else true end)] | length' diff --git a/scripts/ci/test_pr_governance_gate.sh b/scripts/ci/test_pr_governance_gate.sh index 93250c373..aa1d9cfe0 100644 --- a/scripts/ci/test_pr_governance_gate.sh +++ b/scripts/ci/test_pr_governance_gate.sh @@ -140,6 +140,17 @@ fi if [ "$1" = "api" ] && [[ "$2" == repos/*/commits/*/check-runs* ]]; then case "${GH_SCENARIO:-pass}" in + forged_check|forged_check_pending_notice) + printf '{"check_runs":[{"name":"CodeRabbit","app":{"slug":"github-actions"},"status":"completed","conclusion":"success"}]}' + ;; + forged_status|forged_status_pending_notice) + printf '{"check_runs":[]}' + ;; + coderabbit_success_with_warning|coderabbit_skipped_with_warning) + conclusion="${GH_SCENARIO#coderabbit_}" + conclusion="${conclusion%_with_warning}" + printf '{"check_runs":[{"name":"CodeRabbit","app":{"slug":"coderabbitai"},"status":"completed","conclusion":"%s","output":{"text":"Pre-merge blocking warning"}}]}' "$conclusion" + ;; coderabbit_pending) printf '{"check_runs":[{"name":"CodeRabbit","app":{"slug":"coderabbitai"},"status":"in_progress","conclusion":null,"html_url":"https://checks/coderabbit"}]}' ;; @@ -170,17 +181,20 @@ fi if [ "$1" = "api" ] && [[ "$2" == repos/*/commits/*/status ]]; then case "${GH_SCENARIO:-pass}" in + forged_status|forged_status_pending_notice) + printf '{"statuses":[{"context":"CodeRabbit","state":"success","creator":{"login":"unrelated-app[bot]","type":"Bot"}}]}' + ;; coderabbit_status_success|coderabbit_status_success_with_pending_notice) - printf '{"statuses":[{"context":"CodeRabbit","state":"success","description":"Review approved","created_at":"2026-07-29T01:54:41Z","updated_at":"2026-07-29T01:54:41Z"}]}' + printf '{"statuses":[{"context":"CodeRabbit","creator":{"login":"coderabbitai[bot]","type":"Bot"},"state":"success","description":"Review approved","created_at":"2026-07-29T01:54:41Z","updated_at":"2026-07-29T01:54:41Z"}]}' ;; coderabbit_status_pending) - printf '{"statuses":[{"context":"CodeRabbit","state":"pending","description":"Review in progress","created_at":"2026-07-29T01:54:41Z","updated_at":"2026-07-29T01:54:41Z"}]}' + printf '{"statuses":[{"context":"CodeRabbit","creator":{"login":"coderabbitai[bot]","type":"Bot"},"state":"pending","description":"Review in progress","created_at":"2026-07-29T01:54:41Z","updated_at":"2026-07-29T01:54:41Z"}]}' ;; coderabbit_status_failed) - printf '{"statuses":[{"context":"CodeRabbit","state":"failure","description":"Review failed","created_at":"2026-07-29T01:54:41Z","updated_at":"2026-07-29T01:54:41Z"}]}' + printf '{"statuses":[{"context":"CodeRabbit","creator":{"login":"coderabbitai[bot]","type":"Bot"},"state":"failure","description":"Review failed","created_at":"2026-07-29T01:54:41Z","updated_at":"2026-07-29T01:54:41Z"}]}' ;; coderabbit_status_unknown) - printf '{"statuses":[{"context":"CodeRabbit","state":"stale","description":"Unrecognized state","created_at":"2026-07-29T01:54:41Z","updated_at":"2026-07-29T01:54:41Z"}]}' + printf '{"statuses":[{"context":"CodeRabbit","creator":{"login":"coderabbitai[bot]","type":"Bot"},"state":"stale","description":"Unrecognized state","created_at":"2026-07-29T01:54:41Z","updated_at":"2026-07-29T01:54:41Z"}]}' ;; *) printf '{"statuses":[]}' @@ -252,7 +266,7 @@ if [ "$1" = "api" ] && [[ "$args" == *repos/*/issues/42/comments* ]]; then coderabbit_no_actionable_with_blocker) printf '[{"id":777,"user":{"login":"coderabbitai[bot]"},"created_at":"2026-05-19T00:01:00Z","body":"No actionable comments were generated in the recent review. Blocking issue remains on 0123456789abcdef0123456789abcdef01234567."}]' ;; - coderabbit_approval_pending|missing_coderabbit_adversarial_approval_with_pending_notice|coderabbit_check_success_with_pending_notice|coderabbit_status_success_with_pending_notice) + coderabbit_approval_pending|missing_coderabbit_adversarial_approval_with_pending_notice|coderabbit_check_success_with_pending_notice|coderabbit_status_success_with_pending_notice|forged_check_pending_notice|forged_status_pending_notice) printf '[{"id":777,"user":{"login":"coderabbitai[bot]"},"created_at":"2026-05-19T00:01:00Z","body":"\\nCodeRabbit has no unresolved comments, but it has not reviewed the latest commit. \\nCodeRabbit will approve the changes if it finds no blocking issues. \\n"}]' ;; coderabbit_approval_pending_with_separate_blocking_warning) @@ -508,6 +522,25 @@ assert_coderabbit_pending_waits_without_hard_comment() { assert_not_in_file '^pr merge' "$temp_dir/gh.log" } +assert_robot_evidence_requires_publisher_and_clean_output() { + local scenario temp_dir + for scenario in forged_check forged_check_pending_notice forged_status forged_status_pending_notice; do + temp_dir="$(mktemp -d)" + run_gate "$scenario" "$temp_dir" + assert_exit_code 0 "$temp_dir" + assert_in_file 'status=in_progress' "$temp_dir/gh.log" + assert_not_in_file 'conclusion=success' "$temp_dir/gh.log" + assert_not_in_file '^pr merge' "$temp_dir/gh.log" + done + for scenario in coderabbit_success_with_warning coderabbit_skipped_with_warning; do + temp_dir="$(mktemp -d)" + run_gate "$scenario" "$temp_dir" + assert_exit_code 0 "$temp_dir" + assert_in_file 'conclusion=failure' "$temp_dir/gh.log" + assert_not_in_file '^pr merge' "$temp_dir/gh.log" + done +} + assert_coderabbit_success_commit_status_completes_gate() { local temp_dir temp_dir="$(mktemp -d)" @@ -1005,6 +1038,7 @@ assert_draft_pr_waits_without_false_failure assert_existing_marker_comment_is_patched assert_resolved_marker_comment_is_updated_on_ready_gate assert_coderabbit_pending_waits_without_hard_comment +assert_robot_evidence_requires_publisher_and_clean_output assert_coderabbit_success_commit_status_completes_gate assert_coderabbit_pending_commit_status_waits assert_coderabbit_failed_commit_status_blocks From b6d6c286040f16d3d6ef93959eff53abe0eb6d3e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 6 Sep 2026 22:36:09 +0900 Subject: [PATCH 10/11] test(ci): cover missing publishers and trusted quality aliases --- scripts/ci/test_pr_governance_gate.sh | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/scripts/ci/test_pr_governance_gate.sh b/scripts/ci/test_pr_governance_gate.sh index aa1d9cfe0..d5d92d9a4 100644 --- a/scripts/ci/test_pr_governance_gate.sh +++ b/scripts/ci/test_pr_governance_gate.sh @@ -143,9 +143,12 @@ if [ "$1" = "api" ] && [[ "$2" == repos/*/commits/*/check-runs* ]]; then forged_check|forged_check_pending_notice) printf '{"check_runs":[{"name":"CodeRabbit","app":{"slug":"github-actions"},"status":"completed","conclusion":"success"}]}' ;; - forged_status|forged_status_pending_notice) + forged_status|forged_status_pending_notice|missing_status_creator|wrong_status_creator_type|github_code_quality_status) printf '{"check_runs":[]}' ;; + github_code_quality_check) + printf '{"check_runs":[{"name":"GitHub Code Quality","app":{"slug":"github-code-quality"},"status":"completed","conclusion":"success"}]}' + ;; coderabbit_success_with_warning|coderabbit_skipped_with_warning) conclusion="${GH_SCENARIO#coderabbit_}" conclusion="${conclusion%_with_warning}" @@ -181,6 +184,15 @@ fi if [ "$1" = "api" ] && [[ "$2" == repos/*/commits/*/status ]]; then case "${GH_SCENARIO:-pass}" in + missing_status_creator) + printf '{"statuses":[{"context":"CodeRabbit","state":"success"}]}' + ;; + wrong_status_creator_type) + printf '{"statuses":[{"context":"CodeRabbit","state":"success","creator":{"login":"coderabbitai[bot]","type":"User"}}]}' + ;; + github_code_quality_status) + printf '{"statuses":[{"context":"GitHub Code Quality","state":"success","creator":{"login":"github-code-quality[bot]","type":"Bot"}}]}' + ;; forged_status|forged_status_pending_notice) printf '{"statuses":[{"context":"CodeRabbit","state":"success","creator":{"login":"unrelated-app[bot]","type":"Bot"}}]}' ;; @@ -524,7 +536,7 @@ assert_coderabbit_pending_waits_without_hard_comment() { assert_robot_evidence_requires_publisher_and_clean_output() { local scenario temp_dir - for scenario in forged_check forged_check_pending_notice forged_status forged_status_pending_notice; do + for scenario in forged_check forged_check_pending_notice forged_status forged_status_pending_notice missing_status_creator wrong_status_creator_type; do temp_dir="$(mktemp -d)" run_gate "$scenario" "$temp_dir" assert_exit_code 0 "$temp_dir" @@ -539,6 +551,13 @@ assert_robot_evidence_requires_publisher_and_clean_output() { assert_in_file 'conclusion=failure' "$temp_dir/gh.log" assert_not_in_file '^pr merge' "$temp_dir/gh.log" done + for scenario in github_code_quality_check github_code_quality_status; do + temp_dir="$(mktemp -d)" + run_gate "$scenario" "$temp_dir" + assert_exit_code 0 "$temp_dir" + assert_in_file 'conclusion=success' "$temp_dir/gh.log" + assert_not_in_file '^pr merge' "$temp_dir/gh.log" + done } assert_coderabbit_success_commit_status_completes_gate() { From f2e2ac0e9fd84907d49b9646af894e55077cb383 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 6 Sep 2026 23:05:00 +0900 Subject: [PATCH 11/11] fix(ci): distinguish clean robot summaries from blockers Preserve publisher, neutral-skip and real-warning gates; match only complete known clean lines. Add eleven regression scenarios and record the failed first regex attempt. Full governance harness, 36 source contracts, ShellCheck and diff checks passed. No delta deleted or PR closed. Co-Authored-By: Codex --- docs/development/merge-gate-policy.md | 16 +++++++++++- scripts/ci/pr_governance_gate.sh | 8 ++++-- scripts/ci/test_pr_governance_gate.sh | 37 ++++++++++++++++++++++++++- 3 files changed, 57 insertions(+), 4 deletions(-) diff --git a/docs/development/merge-gate-policy.md b/docs/development/merge-gate-policy.md index 317a57a78..e691e22fa 100644 --- a/docs/development/merge-gate-policy.md +++ b/docs/development/merge-gate-policy.md @@ -52,7 +52,9 @@ awaited by default. status evidence requires the corresponding exact `[bot]` creator and Bot type. Missing or unrelated publishers cannot replace the OpenCode fallback. Successful and skipped check conclusions still block when their own output - contains the blocking warning/failure evidence described above. + contains the blocking warning/failure evidence described above. Exclude only + complete known clean lines (`No warnings found` and + `No actionable comments were generated`), never the whole containing output. - `reviewDecision=CHANGES_REQUESTED` is a blocker until requested changes are addressed or superseded on the current head. - Blocker comments use the idempotent @@ -116,6 +118,18 @@ publication in production was established. Keep the full harness, authoritative positive cases, fallback and separate-comment blockers intact when repairing this boundary; do not grant another publisher access to silence a wait state. +Review #3944130242 exposed a regression at `b6d6c286`: output-first matching +also rejected those clean summaries. The new focused harness assertion failed +before repair. Output is now split into lines before matching whole known +clean statements; case, surrounding whitespace, CRLF and terminal punctuation +are covered. An initial whole-string regex attempt still failed and was not +accepted. Eleven scenarios cover clean success/skipped/neutral, an explicit +failure despite clean text, a separate real warning for each passing conclusion, +same-line `except`/`but` qualifications, neutral without skip evidence and an +untrusted publisher with clean text. Unknown wording remains subject to +the existing blocker policy; this is not a general natural-language negation +parser. Publisher authentication and separate-comment evidence are unchanged. + Reference: jqlang. (n.d.). *Regular expressions*. In *jq 1.7 manual*. https://jqlang.org/manual/v1.7/#regular-expressions diff --git a/scripts/ci/pr_governance_gate.sh b/scripts/ci/pr_governance_gate.sh index 7a44e79f7..8d877de16 100644 --- a/scripts/ci/pr_governance_gate.sh +++ b/scripts/ci/pr_governance_gate.sh @@ -341,6 +341,7 @@ CODERABBIT_BLOCKING_PATTERN='pre[- ]merge|blocking|failure|failed|warning|potent CODERABBIT_ISSUE_BLOCKING_PATTERN='pre[- ]merge[^\n]*(blocking|failure|failed|warning|potential issue)|blocking (issue|finding)|potential issue|actionable comments?|changes requested|request changes' CODERABBIT_ISSUE_SUBSTANTIVE_BLOCKING_PATTERN='pre[- ]merge[^\n]*(blocking|failure|failed|warning|potential issue)|blocking (issue|finding)|potential issue|changes requested|request changes' CODERABBIT_NO_ACTIONABLE_PATTERN='no actionable comments? (were )?generated' +CODERABBIT_CLEAN_OUTPUT_LINE_PATTERN='^[ \t]*(No actionable comments? (were )?generated|No warnings found)[.!]?[ \t\r]*$' CODERABBIT_APPROVAL_PENDING_PATTERN='CodeRabbit has no unresolved comments, but it has not reviewed the latest commit' CODERABBIT_APPROVAL_NOTICE_SPAN_PATTERN='.*?' @@ -419,10 +420,13 @@ if [ "$CODERABBIT_COUNT" = "0" ]; then else CODERABBIT_PENDING="$(printf '%s' "$CODERABBIT_MATCHES" | jq '[.[] | select(.status != "completed")] | length')" CODERABBIT_STATUS_PENDING="$(printf '%s' "$CODERABBIT_STATUS_MATCHES" | jq '[.[] | select((.state // "" | ascii_downcase) == "pending")] | length')" - CODERABBIT_FAILED="$(printf '%s' "$CODERABBIT_MATCHES" | jq --arg pattern "$CODERABBIT_BLOCKING_PATTERN" ' + CODERABBIT_FAILED="$(printf '%s' "$CODERABBIT_MATCHES" | jq --arg pattern "$CODERABBIT_BLOCKING_PATTERN" \ + --arg clean_line "$CODERABBIT_CLEAN_OUTPUT_LINE_PATTERN" ' [.[] | select(.status == "completed") - | ([.output.title, .output.summary, .output.text] | map(. // "") | join("\n")) as $check_output + # Remove only whole known clean lines, never an output containing one. + | ([.output.title, .output.summary, .output.text] | map(. // "") | join("\n") + | split("\n") | map(select(test($clean_line; "i") | not)) | join("\n")) as $check_output | select((.conclusion // "") as $conclusion | if $check_output | test($pattern; "i") then true elif $conclusion == "success" or $conclusion == "skipped" then false diff --git a/scripts/ci/test_pr_governance_gate.sh b/scripts/ci/test_pr_governance_gate.sh index d5d92d9a4..a61786873 100644 --- a/scripts/ci/test_pr_governance_gate.sh +++ b/scripts/ci/test_pr_governance_gate.sh @@ -140,6 +140,22 @@ fi if [ "$1" = "api" ] && [[ "$2" == repos/*/commits/*/check-runs* ]]; then case "${GH_SCENARIO:-pass}" in + clean_summary_success|clean_summary_skipped|clean_summary_neutral|clean_summary_failure|clean_without_skip_neutral|forged_clean_success|mixed_summary_success|mixed_summary_skipped|mixed_summary_neutral|qualified_summary_success|qualified_but_success) + conclusion="${GH_SCENARIO##*_}" + app_slug='coderabbitai' + summary='No warnings found.' + output_text='No actionable comments were generated' + case "$GH_SCENARIO" in + forged_clean_success) app_slug='github-actions' ;; + clean_summary_skipped) summary=$' \tno WARNINGS found! \t\r' ;; + clean_summary_neutral) output_text=$'Review skipped\nNo actionable comments were generated' ;; + mixed_summary_*) output_text=$'Review skipped\nNo actionable comments were generated\nPre-merge blocking warning' ;; + qualified_summary_success) summary='No warnings found except blocking issues' ;; + qualified_but_success) summary='No warnings found, but a blocking issue remains' ;; + esac + jq -cn --arg app_slug "$app_slug" --arg conclusion "$conclusion" --arg summary "$summary" --arg text "$output_text" \ + '{check_runs:[{name:"CodeRabbit",app:{slug:$app_slug},status:"completed",conclusion:$conclusion,output:{summary:$summary,text:$text}}]}' + ;; forged_check|forged_check_pending_notice) printf '{"check_runs":[{"name":"CodeRabbit","app":{"slug":"github-actions"},"status":"completed","conclusion":"success"}]}' ;; @@ -536,7 +552,7 @@ assert_coderabbit_pending_waits_without_hard_comment() { assert_robot_evidence_requires_publisher_and_clean_output() { local scenario temp_dir - for scenario in forged_check forged_check_pending_notice forged_status forged_status_pending_notice missing_status_creator wrong_status_creator_type; do + for scenario in forged_check forged_check_pending_notice forged_status forged_status_pending_notice missing_status_creator wrong_status_creator_type forged_clean_success; do temp_dir="$(mktemp -d)" run_gate "$scenario" "$temp_dir" assert_exit_code 0 "$temp_dir" @@ -560,6 +576,24 @@ assert_robot_evidence_requires_publisher_and_clean_output() { done } +assert_clean_summaries_preserve_real_blockers() { + local scenario temp_dir + for scenario in clean_summary_success clean_summary_skipped clean_summary_neutral; do + temp_dir="$(mktemp -d)" + run_gate "$scenario" "$temp_dir" + assert_exit_code 0 "$temp_dir" + assert_in_file 'conclusion=success' "$temp_dir/gh.log" + assert_not_in_file '^pr merge' "$temp_dir/gh.log" + done + for scenario in clean_summary_failure clean_without_skip_neutral mixed_summary_success mixed_summary_skipped mixed_summary_neutral qualified_summary_success qualified_but_success; do + temp_dir="$(mktemp -d)" + run_gate "$scenario" "$temp_dir" + assert_exit_code 0 "$temp_dir" + assert_in_file 'conclusion=failure' "$temp_dir/gh.log" + assert_not_in_file '^pr merge' "$temp_dir/gh.log" + done +} + assert_coderabbit_success_commit_status_completes_gate() { local temp_dir temp_dir="$(mktemp -d)" @@ -1058,6 +1092,7 @@ assert_existing_marker_comment_is_patched assert_resolved_marker_comment_is_updated_on_ready_gate assert_coderabbit_pending_waits_without_hard_comment assert_robot_evidence_requires_publisher_and_clean_output +assert_clean_summaries_preserve_real_blockers assert_coderabbit_success_commit_status_completes_gate assert_coderabbit_pending_commit_status_waits assert_coderabbit_failed_commit_status_blocks