fix(ci): stacked-PR base support + governance-gate CodeRabbit/OpenCode fallback fixes - #1531
Conversation
…e-gate CodeRabbit/OpenCode fallback fixes Extracted from #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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019YgmaEKnPhrsbSyQBierFH
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughPull-request workflows now run for all target branches. The governance gate authenticates CodeRabbit evidence, evaluates blocking output, and handles draft and approval-pending states. Frontend and Python CI dependency pins were updated with security validation. ChangesPull-request workflow and governance
Dependency security pins
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to No concrete merge-blocking risk remains after the governance and dependency updates. Sequence Diagram(s)sequenceDiagram
participant GitHubPR as GitHub PR metadata
participant GitHubChecks as GitHub checks and statuses
participant GitHubComments as GitHub issue comments
participant OpenCodeReviews as OpenCode reviews
participant GovernanceGate as pr_governance_gate.sh
GitHubPR->>GovernanceGate: provide draft and merge state
GitHubChecks->>GovernanceGate: provide CodeRabbit publisher and output
GitHubComments->>GovernanceGate: provide approval-pending notices
OpenCodeReviews->>GovernanceGate: provide current-head adversarial approval
GovernanceGate->>GitHubChecks: publish waiting, failure, or success status
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Current-source review found one governance-contract violation that needs repair on this external branch before it can be treated as merge-ready. In Please repair this on #1531 itself, test-first: preserve the useful parsing fix that distinguishes CodeRabbit's marker-delimited approval-pending notice from an actual blocking finding, but keep that notice in |
…on fix with #1503 Owner's request on this PR (2026-09-02): this branch had accumulated unrelated content over a long, CI-congestion-heavy session. Narrows it back to its stated purpose -- the dependency-root CI-enablement slice (Postgres service container + the minimum fix needed to make the newly real-executing tests pass) -- and converges the overlapping migration fix with #1503's independently-derived, more complete implementation so the two branches don't carry conflicting historical-migration semantics. Moved out entirely (zero diff vs develop now), extracted to #1531: - .github/workflows/{bandit,dependency-review,docker-publish}.yml and app-ci.yml's `pull_request: branches:` removal (stacked-PR base support) - tests/test_stacked_pr_workflow_contract.py - backend/tests/test_release_governance.py's stacked-PR assertions - scripts/ci/pr_governance_gate.sh + scripts/ci/test_pr_governance_gate.sh (CodeRabbit approval-pending + OpenCode fallback governance-gate fixes) Converged with #1503 (adopted its exact implementation, verified byte-identical via diff against its branch): - backend/alembic/versions/0011_email_read_state.py: #1503 independently found and fixed the same relation "emails" does not exist crash, more completely -- guards on column existence (not just table existence) and checks both "email_records" and legacy "emails", vs. this branch's simpler has_table-only guard. Adopting it here means #1503's later rebase onto this PR (once merged) is a clean no-op on this file instead of a conflict between two divergent fixes for the same bug. - backend/scripts/bootstrap_db.py: same convergence for execute_schema_backfill's legacy-index handling. - backend/tests/test_alembic_migrations.py: swapped this branch's test_email_read_state_legacy_table_guard_is_reversible (asserted the old implementation's exact shape) for #1503's test_email_read_state_guards_both_legacy_and_current_table_names, which asserts the now-shared implementation instead. backend/alembic/versions/0001_initial_control_plane.py, backend/tests/test_bootstrap_db.py, and backend/tests/test_data_api.py needed no changes -- diffed identically against #1503's branch already (both PRs independently reached the exact same is_read raw-SQL fix and 0001 refactor while investigating the same real-Postgres failures). Net result: 10 files changed vs. develop (down from 18), matching the "dependency-root CI-enablement slice" scope the owner asked for. Verification (real-Postgres re-run blocked by this sandbox having no usable Docker daemon -- ExpressJS-style hashes/lint/isolated-assertion checks substitute; the point of this very PR is to make GitHub's own CI runners do the authoritative check): - python3 -m py_compile on every touched Python file: clean. - python3 -m ruff check on every touched backend file: all checks passed. - backend/tests/test_alembic_migrations.py's three targeted test functions executed directly (bypassing this sandbox's unrelated conftest.py/cryptography import breakage): all 3 pass. - git diff against develop for every "moved out" file: zero lines, confirming a clean revert. - git diff against #1503's branch for the converged migration/bootstrap files: zero lines, confirming byte-identical adoption. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019YgmaEKnPhrsbSyQBierFH
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…g 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 <!-- approval_notice_start/end --> 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019YgmaEKnPhrsbSyQBierFH
|
Fixed both findings on this branch (exact head Your finding: confirmed — Devin's finding (same root file, different bug): the blocker-scan jq filter excluded an entire CodeRabbit issue comment whenever it contained the approval-pending marker anywhere, even if the same comment also carried a separate real blocking warning. Fixed by stripping only the marker-delimited Regression coverage: replaced the now-incorrect I did not weaken the separate formal GitHub approval/ruleset requirement — this script's Claude-Session: https://claude.ai/code/session_019YgmaEKnPhrsbSyQBierFH Generated by Claude Code |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019YgmaEKnPhrsbSyQBierFH
|
Fixed CodeRabbit's Claude-Session: https://claude.ai/code/session_019YgmaEKnPhrsbSyQBierFH Generated by Claude Code |
|
Replying to Devin's newest finding on Devin's description of the mechanism is accurate: yes, a genuine CodeRabbit approval-pending notice now makes the gate wait rather than accept the OpenCode fallback, and yes, that wait persists for as long as CodeRabbit doesn't produce terminal evidence for that head (its own rate limits included — ironically, CodeRabbit just hit exactly that on this PR a few minutes ago). But that's not an oversight; it's the explicit, reasoned trade-off the owner asked for in this comment's predecessor (the earlier finding on The liveness concern (an indefinite wait if CodeRabbit's lane never resolves) is real, but the owner's own instruction names its intended relief valve: a separately authoritative deterministic policy explicitly classifying the CodeRabbit lane as unavailable, not a blanket fallback re-opened inside this same conditional. That's a bigger, deliberate design decision outside this PR's scope — flagging it for the owner rather than unilaterally deciding it. Claude-Session: https://claude.ai/code/session_019YgmaEKnPhrsbSyQBierFH Generated by Claude Code |
seonghobae
added
documentation
ChatGPT Codex Connector
|
PR governance metadata gate is not ready for
|
…-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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019YgmaEKnPhrsbSyQBierFH
|
Addressed the governance gate's blockers on this cycle's wake:
@coderabbitai review Generated by Claude Code Generated by Claude Code |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019YgmaEKnPhrsbSyQBierFH
|
CodeRabbit's Generated by Claude Code Generated by Claude Code |
|
|
|
Re-checked this cycle's metadata-gate wake — all four listed blockers are stale or infra, not new findings:
@coderabbitai review
No further code change is needed on this PR's own diff; will keep watching for CodeRabbit's fresh review and for Generated by Claude Code Generated by Claude Code |
|
✅ Action performedReview finished.
|
|
None of this touches Generated by Claude Code Generated by Claude Code |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 550798ccaf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Normally merge full migration prerequisite #1503 at 19d5860, including #1565; preserve #1562/#1531/#1554 history and current #1531 base. Record decision and original missing-emails/skip failures in doctoring before commit. No delta discarded, no copied central workflow, no gate bypass. Add hardened task-only DB lifecycle, isolated bootstrap/child settings, actual pytest collection/xfail guard, cancellation-safe scoped cleanup and redacted artifacts. Full candidate: 1871 passed and 2 explicitly unconfigured live API skips. Independent signal tests passed. Revalidate this exact committed head before protected integration.
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 <noreply@openai.com> Signed-off-by: Seongho Bae <me@seonghobae.me>
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 <noreply@openai.com> Signed-off-by: Seongho Bae <me@seonghobae.me>
|
Exact current head b6d6c28 preserves fac3437 and adds ordinary commits 179229d (shared publisher/output repair) and b6d6c28 (edge fixtures). Independent nine-file readiness review identified two pre-existing metadata risks: name-only CodeRabbit evidence and success/skipped conclusions bypassing warning inspection. Main independently confirmed both in source. Six isolated pre-fix fake-GitHub scenarios each incorrectly emitted success: forged App check/status publisher with/without a pending notice, and success/skipped check output carrying a blocking warning. Repair requires an allowed App slug or exact Bot status creator before selecting evidence, and checks blocking output before accepting the conclusion. Existing fallback, neutral-skip and separate-comment rejection remain intact. Ten focused publisher/output cases pass, including absent creator, correct login with User type rejection, and github-code-quality check/status positive coverage. Post-commit full fake-GitHub harness: PASS, exit 0. Post-commit source contracts: 36 passed, zero skips, 0.25s. ShellCheck and git diff --check passed. Local/remote heads match and worktree is clean. Independent source re-review found no new blocking finding; it is not a GitHub approval. Actual Edge Markdown Preview visual inspection at this exact head covered the full new policy bullet and RCA paragraph, with readable wrapping and no observed overlap/clipping in those regions. Screenshots retained locally under naruon-gate-b6d6-visual-eLO7dv (publisher_policy.png, publisher_rca.png). No whole-document, mobile or product UI claim. Draft remains open. Hosted checks/reviews need this new head. CI #1562 and SMTP #1417 still retain prior heads 0648eac/cc2c4cbc; their earlier local DB/security receipts do not certify a future restack. Complete non-force delta propagation, AGENTS owner guidance and Gap refresh remain next steps. No review was self-resolved, no protection changed and no merge claimed. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b6d6c28604
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
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 <noreply@openai.com>
Preserve existing1244 and1571 dependency repairs before governance adoption. Normal merge retains all1531 review-gate delta; no history rewrite or PR closure. Parent dependency scan passed expanded Medium-plus scope; hosted gates and UI-test diagnostics remain separate.
|
선행 보안 수정 상속 완료: bd0135de778a01a51deae536084447ce2481280c는 기존 f2e2ac0e와 #1244의156a816c를 일반 병합했습니다. 4개 dependency 파일은156a와 byte-identical이며 리뷰 게이트 delta는 보존됐습니다. PR base를 기존 #1244 branch로 변경해 선행 수정 순서를 명시했습니다. 전체 governance harness exit0/PASS,36 source contracts -W error/ShellCheck/diff exit0. tracked archive의 expanded Trivy MEDIUM/HIGH/CRITICAL fixable(vuln,secret,misconfig) exit0,11language manifests/0findings. JSON SHA2562710b3768d5b95b6dd15d19b637e846876fdf41fc93dd85f0fe9b93fbfcc1bc6. 이는 localhead검증이며 새 hosted checks/review와 보호 병합은 별도입니다. #1244의 기존 React act 경고와 최신 Visual Inspection 미완료도 유지합니다. Force/PRclose/반복Ready 없이 기존 변경을 통합했습니다. |
2 participants
Current authority — 2026-09-07
develop@042b0c70531b229af3acbd0421a2f23098d848b3chore/aiohttp-3.14.3-maintainer@156a816c3e799bc8cc2cf87e5e1a2ffb8cc1c78fchore/stacked-pr-governance-gate-supportbd0135de778a01a51deae536084447ce2481280cReady and current-head robot approval do not authorize merge while the prerequisite and required hosted evidence remain incomplete.
Bounded governance contract
This PR remains the Naruon-owned stacked-PR trigger and PR-governance metadata lane.
branches:andbranches-ignore:filters.coderabbitaiorgithub-code-qualityApp slug; commit-status evidence requires Bot type plus exact supported bot identity.CHANGES_REQUESTEDremain fail closed.Intervening delta adoption and P1 repair
The previous authority stopped at
b6d6c286...; later normal descendants were read and adopted rather than treated as a race.f2e2ac0e9fd84907d49b9646af894e55077cb383repairs the remaining P1: broad output matching had classified complete clean CodeRabbit lines such asNo warnings foundandNo actionable comments were generatedas blockers. The fix removes only whole anchored clean lines before blocker matching; real warnings, qualified clean text, explicit failure, neutral-without-skip evidence, and forged publishers still fail closed. Eleven regression scenarios cover the boundary.Current merge commit
bd0135de778a01a51deae536084447ce2481280cthen normally inherited combined dependency prerequisite #1244/#1571 as its second parent. It keeps governance history as first-parent ancestry and does not make dependency files part of this PR's effective delta. No force push, destructive rebase, predecessor closure, or source copy was used.Review state
The final clean-summary P1 thread is resolved and fresh thread inventory is zero unresolved. CodeRabbit submitted
APPROVEDon 2026-09-06 14:40:28Z after exact headbd0135de...existed. This remains qualifying current-head independent robot-review evidence for this lane; older submissions are predecessor history only.Exact-head hosted evidence
The previously queued repository workflows are now terminal on unchanged exact head
bd0135de...:34039482640— success34039482620— success34039482600— success34039482619— success34039482904— success34039482713— failureThe CodeQL run reproduces the shared verdict-path failure rather than a governance-source CodeQL finding. Language detection succeeded; JavaScript/TypeScript, Actions, and Python compatibility jobs each completed
Request current-head CodeQL scan dispatchsuccessfully and then failed atRelease runner or enforce current-head CodeQL verdict. This remains a canonical.githubcontrol-plane repair, not grounds for a Naruon-local authorization expansion, synthetic status, dummy commit, workflow copy, or gate weakening.Stack and owner boundary
#1244 remains the direct security prerequisite and must land normally first. #1562 and #1417 remain downstream governance/runtime consumers. After each prerequisite lands, descendants must adopt the protected ancestry normally and regenerate their own exact-head evidence.
The current Calendar stack (#1245 → #1488 → #1569) now also provides a live non-default-base trigger acceptance case: after #1488 and #1569 were repaired/restacked without force, both exact heads produced zero PR workflow runs. #1562 remains the canonical stacked-base workflow prerequisite; do not copy its workflow changes into Calendar PRs.
Shared organization CodeQL/Strix/review execution defects remain
.githubowner concerns. The product Gap ledger remains owned by #1557.Merge boundary
Merge Gate: FAIL. This head has qualifying current-head review and five repository workflow families GREEN, but CodeQL is not GREEN and #1244 is unintegrated. Once #1244 lands, reconcile this branch through normal ancestry without dropping governance delta and regenerate all then-live evidence on the resulting exact head. No self-approval, review dismissal/fabrication, admin bypass, dummy/no-op requeue commit, force-push, destructive rebase, predecessor-evidence transfer, synthesized status, or ruleset weakening.