Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions .github/workflows/app-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,6 @@ name: Application CI

on:
pull_request:
Comment thread
seonghobae marked this conversation as resolved.
branches:
- develop
- master
- "release/**"
push:
branches:
- develop
Expand Down
1 change: 0 additions & 1 deletion .github/workflows/bandit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ on:
push:
branches: [ develop, master ]
pull_request:
branches: [ develop, master ]
workflow_dispatch:

permissions:
Expand Down
4 changes: 0 additions & 4 deletions .github/workflows/dependency-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,6 @@ name: Dependency Review

on:
pull_request:
branches:
- develop
- master
- "release/**"
workflow_dispatch:

permissions:
Expand Down
4 changes: 0 additions & 4 deletions .github/workflows/docker-publish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@ on:
tags:
- "v*"
pull_request:
Comment thread
seonghobae marked this conversation as resolved.
branches:
- develop
- master
- "release/**"

permissions:
contents: read
Expand Down
8 changes: 6 additions & 2 deletions backend/tests/test_release_governance.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -669,6 +668,10 @@ 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
assert "branches-ignore:" not in pull_request_block


def test_docker_publish_validates_pr_images_and_publishes_semver_images_only_on_tags() -> (
None
Expand Down Expand Up @@ -710,7 +713,8 @@ 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 "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
Expand Down
29 changes: 29 additions & 0 deletions backend/tests/test_stacked_pr_workflow_contract.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
"""Regression coverage for governed checks on stacked pull requests."""

from pathlib import Path
import re


REPO_ROOT = Path(__file__).resolve().parents[2]
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:
Comment thread
seonghobae marked this conversation as resolved.
"""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<body>(?:^ .*$\n)*)",
workflow,
)
assert pull_request_trigger is not None, f"{name} must run on pull_request"
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"
)
72 changes: 70 additions & 2 deletions docs/development/merge-gate-policy.md
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -46,6 +47,14 @@ 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. 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
Expand All @@ -65,6 +74,65 @@ 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.

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.

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.

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

Use the same head SHA across all checks:

```bash
Expand Down
109 changes: 71 additions & 38 deletions scripts/ci/pr_governance_gate.sh
Comment thread
seonghobae marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,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.'
Comment thread
seonghobae marked this conversation as resolved.
fi

if [ "$MERGE_STATE" = "BEHIND" ]; then
Expand Down Expand Up @@ -341,6 +341,31 @@ 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='<!-- approval_notice_start -->.*?<!-- approval_notice_end -->'

# 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/^/ /'
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("<!-- approval_notice_start -->"))
| 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
Expand All @@ -353,18 +378,21 @@ 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)
')"
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'
Expand All @@ -386,24 +414,24 @@ 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
printf 'CodeRabbit check is 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
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")
# 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 $conclusion == "success" or $conclusion == "skipped" then false
| if $check_output | test($pattern; "i") then true
Comment thread
seonghobae marked this conversation as resolved.
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'
Expand All @@ -426,33 +454,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" '
[.[][]
| select((.user.login // "") | test("'"$REVIEW_BOT_LOGIN_PATTERN"'"; "i"))
| select(
(.body // "") as $body
| ($body | split("<details>")[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}."
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; ""; "m")) as $body
| ($body | split("<details>")[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_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

if ! REVIEW_COMMENTS_JSON="$(gh api --paginate "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}/comments" 2>"$REVIEW_COMMENTS_ERROR_FILE")"; then
Expand Down
Loading
Loading