ci(gate): add pull_request_review fallback trigger for qa/security review workflows #2153

Closed
core-be wants to merge 2 commits from fix/internal-760-review-event-trigger into main
Member

Problem

#2020 (clean-gate-proof) has APPROVED reviews from core-qa and core-security, but the qa-review / approved and security-review / approved contexts remain failure with stale timestamps (last ran before the APPROVED reviews landed).

Evidence:

  • core-security APPROVED at 2026-06-02T21:10:08Z
  • core-qa APPROVED at 2026-06-03T00:22:20Z
  • qa-review last status: failure at 2026-06-02T20:57:31Z
  • security-review last status: failure at 2026-06-02T20:57:21Z
  • Meanwhile, sop-tier-check does trigger on review events and produced sop-tier-check / tier-check (pull_request_review): success at 2026-06-02T21:10:21Z — 13 seconds after core-security's APPROVED.

This shows pull_request_review events do fire on this Gitea instance, but pull_request_review_approved does not appear to trigger our workflows.

Fix

Add pull_request_review with types: [submitted] as a defensive fallback trigger to both qa-review.yml and security-review.yml, alongside the existing pull_request_review_approved (retained for forward compatibility).

Guard logic:

  • Job-level if: accepts pull_request_target OR pull_request_review_approved OR (pull_request_review && github.event.review.type == 'pull_request_review_approved').
  • COMMENT and REQUEST_CHANGES submissions are skipped at the job level, so no misleading green contexts are published.
  • Explicit POST step guard updated similarly so the branch-protection-required (pull_request_target) context is posted on both event paths.

Trust & Security

  • BASE ref checkout unchanged (no PR-head code execution).
  • Token separation unchanged: evaluator uses SOP_TIER_CHECK_TOKEN (read-only); POST uses STATUS_POST_TOKEN (write-only).

Test Plan

  • After merge, a new APPROVED review on #2020 should trigger qa-review + security-review via pull_request_review and flip the contexts green.
  • Non-APPROVE reviews (COMMENT, REQUEST_CHANGES) should not run the evaluator (job skipped).

Refs: internal#760

## Problem #2020 (clean-gate-proof) has APPROVED reviews from `core-qa` and `core-security`, but the `qa-review / approved` and `security-review / approved` contexts remain `failure` with stale timestamps (last ran **before** the APPROVED reviews landed). **Evidence:** - `core-security` APPROVED at `2026-06-02T21:10:08Z` - `core-qa` APPROVED at `2026-06-03T00:22:20Z` - `qa-review` last status: `failure` at `2026-06-02T20:57:31Z` - `security-review` last status: `failure` at `2026-06-02T20:57:21Z` - Meanwhile, `sop-tier-check` **does** trigger on review events and produced `sop-tier-check / tier-check (pull_request_review): success` at `2026-06-02T21:10:21Z` — 13 seconds after `core-security`'s APPROVED. This shows `pull_request_review` events **do fire** on this Gitea instance, but `pull_request_review_approved` does **not** appear to trigger our workflows. ## Fix Add `pull_request_review` with `types: [submitted]` as a defensive fallback trigger to **both** `qa-review.yml` and `security-review.yml`, alongside the existing `pull_request_review_approved` (retained for forward compatibility). **Guard logic:** - Job-level `if:` accepts `pull_request_target` OR `pull_request_review_approved` OR (`pull_request_review` && `github.event.review.type == 'pull_request_review_approved'`). - COMMENT and REQUEST_CHANGES submissions are skipped at the **job level**, so no misleading green contexts are published. - Explicit POST step guard updated similarly so the branch-protection-required `(pull_request_target)` context is posted on both event paths. ## Trust & Security - BASE ref checkout unchanged (no PR-head code execution). - Token separation unchanged: evaluator uses `SOP_TIER_CHECK_TOKEN` (read-only); POST uses `STATUS_POST_TOKEN` (write-only). ## Test Plan - [ ] After merge, a new APPROVED review on #2020 should trigger qa-review + security-review via `pull_request_review` and flip the contexts green. - [ ] Non-APPROVE reviews (COMMENT, REQUEST_CHANGES) should not run the evaluator (job skipped). Refs: internal#760
core-be added 1 commit 2026-06-03 00:33:55 +00:00
ci(gate): add pull_request_review fallback trigger for qa-review and security-review
ci-arm64-advisory / fast-checks (pull_request) Waiting to run
Lint shellcheck (arm64 pilot) / shellcheck-arm64 (pilot) (pull_request) Failing after 1s
CI / Python Lint & Test (pull_request) Successful in 2s
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 17s
E2E API Smoke Test / detect-changes (pull_request) Successful in 6s
Lint curl status-code capture / Scan workflows for curl status-capture pollution (pull_request) Successful in 3s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 6s
CI / Detect changes (pull_request) Successful in 22s
Lint forbidden tenant-env keys / Scan workspace_secrets writers for forbidden env keys (pull_request) Successful in 4s
Lint forbidden tenant-env keys / Scan for repo-host token write into tenant workspace surface (pull_request) Successful in 4s
E2E Chat / detect-changes (pull_request) Successful in 16s
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 16s
lint-required-workflows-docker-host-pinned / Lint docker-host pin on docker-touching workflows (pull_request) Successful in 12s
gate-check-v3 / gate-check (pull_request_target) Successful in 6s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 13s
qa-review / approved (pull_request_target) Failing after 3s
sop-checklist / review-refire (pull_request_target) Has been skipped
sop-checklist / all-items-acked (pull_request) acked: 0/7 — missing: comprehensive-testing, local-postgres-e2e, staging-smoke, +4 — body-unfilled: comprehensive-testing, local-postgres-e2
sop-checklist / na-declarations (pull_request) N/A: (none)
sop-checklist / all-items-acked (pull_request_target) Successful in 2s
security-review / approved (pull_request_target) Failing after 5s
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 1s
sop-tier-check / tier-check (pull_request_target) Successful in 4s
CI / Platform (Go) (pull_request) Successful in 2s
CI / Canvas (Next.js) (pull_request) Successful in 1s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 1s
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 5s
E2E Chat / E2E Chat (pull_request) Successful in 3s
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 3s
CI / all-required (pull_request) Successful in 3s
lint-continue-on-error-tracking / lint-continue-on-error-tracking (pull_request) Failing after 58s
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 55s
CI / Canvas Deploy Reminder (pull_request) Has been skipped
lint-required-context-exists-in-bp / lint-required-context-exists-in-bp (pull_request) Successful in 1m7s
Lint pre-flip continue-on-error / Verify continue-on-error flips have run-log proof (pull_request) Successful in 1m18s
Lint workflow YAML (Gitea-1.22.6-hostile shapes) / Lint workflow YAML for Gitea-1.22.6-hostile shapes (pull_request) Successful in 1m27s
qa-review / approved (pull_request_review) Has been skipped
security-review / approved (pull_request_review) Has been skipped
sop-tier-check / tier-check (pull_request_review) Successful in 5s
3f1f029a38
#2020 clean-gate-proof revealed that pull_request_review_approved does
not fire reliably on this Gitea instance, while pull_request_review DOES
(sop-tier-check successfully triggers on it, producing live
sop-tier-check / tier-check (pull_request_review) contexts).

Changes to BOTH qa-review.yml and security-review.yml:
- Add pull_request_review with types: [submitted] as a defensive fallback
  trigger alongside the existing pull_request_review_approved.
- Update job-level if: guard to accept EITHER event, but only when
  github.event.review.type == 'pull_request_review_approved' for the
  pull_request_review path. This ensures COMMENT and REQUEST_CHANGES
  review submissions are skipped at the job level (no misleading green
  contexts).
- Update explicit POST step guard similarly so the branch-protection-
  required (pull_request_target) context is posted on both event paths.
- Retain pull_request_review_approved as a forward-compatible trigger
  in case a future Gitea version fires it reliably.

Trust boundary unchanged: BASE ref checkout, no PR-head code execution.
Token separation unchanged: evaluator uses SOP_TIER_CHECK_TOKEN (read);
POST uses STATUS_POST_TOKEN (write).

Refs: internal#760
molecule-code-reviewer requested changes 2026-06-03 01:12:10 +00:00
molecule-code-reviewer left a comment
Member

REQUEST_CHANGES on head 3f1f029a.

This PR is stale relative to the merged gate repair work and should not land as written.

Findings:

  • The branch is not mergeable and is based before the later gate repairs (#2157 and the #2020 proof sequence). It only changes qa/security workflow files and lacks the refire-token/context fixes that are now live.
  • The proposed job guard uses github.event.review.type == 'pull_request_review_approved' on pull_request_review. The live follow-up settled on github.event.review.state == 'APPROVED' || github.event.review.state == 'approved', with regression coverage. This PR would move the workflows back toward the older, unproven payload shape.
  • It retains pull_request_review_approved alongside the fallback, but the #2159 diagnosis showed the important issue for stale PRs is that Gitea evaluates review-event workflow triggers from the PR head commit. A main-only workflow change cannot auto-fire qa/security for stale heads that do not contain the trigger.
  • There is no regression coverage. The current gate-fix bar requires tests for exact BP context names and trigger/guard behavior.

Recommendation: close or supersede this PR in favor of the already-merged #2157 plus the #2159 diagnosis/follow-up plan. If a new PR is still needed, it should be rebased on current main and preserve #2157's review.state guard, STATUS_POST_TOKEN/refire fixes, exact (pull_request_target) contexts, and regression tests.

REQUEST_CHANGES on head 3f1f029a. This PR is stale relative to the merged gate repair work and should not land as written. Findings: - The branch is not mergeable and is based before the later gate repairs (#2157 and the #2020 proof sequence). It only changes qa/security workflow files and lacks the refire-token/context fixes that are now live. - The proposed job guard uses `github.event.review.type == 'pull_request_review_approved'` on `pull_request_review`. The live follow-up settled on `github.event.review.state == 'APPROVED' || github.event.review.state == 'approved'`, with regression coverage. This PR would move the workflows back toward the older, unproven payload shape. - It retains `pull_request_review_approved` alongside the fallback, but the #2159 diagnosis showed the important issue for stale PRs is that Gitea evaluates review-event workflow triggers from the PR head commit. A main-only workflow change cannot auto-fire qa/security for stale heads that do not contain the trigger. - There is no regression coverage. The current gate-fix bar requires tests for exact BP context names and trigger/guard behavior. Recommendation: close or supersede this PR in favor of the already-merged #2157 plus the #2159 diagnosis/follow-up plan. If a new PR is still needed, it should be rebased on current main and preserve #2157's `review.state` guard, STATUS_POST_TOKEN/refire fixes, exact `(pull_request_target)` contexts, and regression tests.
core-be added 1 commit 2026-06-03 20:40:14 +00:00
Merge branch 'main' into fix/internal-760-review-event-trigger
ci-arm64-advisory / fast-checks (pull_request) Waiting to run
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 4s
CI / Python Lint & Test (pull_request) Successful in 3s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 4s
CI / Detect changes (pull_request) Successful in 6s
Lint curl status-code capture / Scan workflows for curl status-capture pollution (pull_request) Successful in 3s
E2E API Smoke Test / detect-changes (pull_request) Successful in 7s
Lint shellcheck (arm64 pilot) / shellcheck-arm64 (pilot) (pull_request) Successful in 1s
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 6s
Lint forbidden tenant-env keys / Scan for repo-host token write into tenant workspace surface (pull_request) Successful in 3s
Lint forbidden tenant-env keys / Scan workspace_secrets writers for forbidden env keys (pull_request) Successful in 6s
lint-required-workflows-docker-host-pinned / Lint docker-host pin on docker-touching workflows (pull_request) Successful in 2s
E2E Chat / detect-changes (pull_request) Successful in 11s
gate-check-v3 / gate-check (pull_request_target) Failing after 4s
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 2s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 8s
qa-review / approved (pull_request_target) Failing after 7s
sop-checklist / review-refire (pull_request_target) Has been skipped
security-review / approved (pull_request_target) Failing after 4s
sop-checklist / all-items-acked (pull_request) acked: 0/7 — missing: comprehensive-testing, local-postgres-e2e, staging-smoke, +4 — body-unfilled: comprehensive-testing, local-postgres-e2
sop-checklist / na-declarations (pull_request) N/A: (none)
CI / Platform (Go) (pull_request) Successful in 2s
CI / Canvas (Next.js) (pull_request) Successful in 1s
sop-checklist / all-items-acked (pull_request_target) Successful in 6s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 1s
sop-tier-check / tier-check (pull_request_target) Successful in 7s
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 1s
CI / Canvas Deploy Reminder (pull_request) Has been skipped
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 5s
E2E Chat / E2E Chat (pull_request) Successful in 4s
CI / all-required (pull_request) Successful in 3s
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 59s
Lint pre-flip continue-on-error / Verify continue-on-error flips have run-log proof (pull_request) Successful in 1m2s
lint-continue-on-error-tracking / lint-continue-on-error-tracking (pull_request) Successful in 1m8s
Lint workflow YAML (Gitea-1.22.6-hostile shapes) / Lint workflow YAML for Gitea-1.22.6-hostile shapes (pull_request) Successful in 1m7s
lint-required-context-exists-in-bp / lint-required-context-exists-in-bp (pull_request) Successful in 1m11s
audit-force-merge / audit (pull_request_target) Waiting to run
62a5a3b7b2
Resolved conflicts in qa-review.yml and security-review.yml by keeping
both pull_request_review and pull_request_review_approved triggers and
using a defensive job guard that checks both review.type and review.state.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Author
Member

Merge conflict resolved — both pull_request_review and pull_request_review_approved triggers retained with defensive dual-field guard (review.type OR review.state). Ready for review.

Merge conflict resolved — both `pull_request_review` and `pull_request_review_approved` triggers retained with defensive dual-field guard (review.type OR review.state). Ready for review.
Member

fullstack-engineer review on PR #2153 — confirming REQUEST_CHANGES, recommending CLOSE as superseded

Verifying the bot review #8342 (REQUEST_CHANGES, 1461 bytes, molecule-code-reviewer id=109) is correct: PR #2157 is already MERGED on main and supersedes this work. My own check confirms it.

Verification:

  • PR #2157 state: closed, merged=true (merged 2026-06-03 around 01:43Z, head 1b8b7a70, base=main). Changes:
    • .gitea/scripts/review-refire-status.sh (+10/-4) — refire-token fix
    • .gitea/scripts/tests/test_gate_review_auto_fire.py (+168, new) — auto-fire regression test
    • .gitea/workflows/qa-review.yml (+20/-18) — guard fixes
    • .gitea/workflows/security-review.yml (+20/-18) — guard fixes
    • .gitea/workflows/sop-checklist.yml (+6/-6) — checklist alignment
  • Current main qa-review.yml guard uses review.state == "APPROVED" || "approved" (NOT review.type == "pull_request_review_approved" as PR #2153 proposes). The shape PR #2153 would land is the OLDER, unproven one. PR #2157 settled on the empirically-correct shape.
  • PR #2153 head 62a5a3b7 (2026-06-03 20:41Z) is 19 hours newer than #2157's merge — so the branch was opened AFTER the merge but the diff was based on a pre-#2157 snapshot. Classic rebase miss.

The 3 specific issues the bot flagged are all correct:

  1. Stale branch / pre-#2157 base. The proposed guard github.event.review.type == "pull_request_review_approved" is the older shape. #2157's guard uses github.event.review.state == "APPROVED" || "approved" which is what sop-tier-check has provably fired on (the live evidence #2153 itself cites). Merging #2153 would partially undo #2157's guard fix.

  2. Stale-head problem (the #2159 diagnosis). Gitea evaluates review-event workflow triggers from the PR HEAD commit. A main-only workflow change cannot auto-fire qa/security for stale PR heads. The auto-fire regression test in #2157 covers this. PR #2153 has no equivalent test.

  3. No regression coverage. PR #2153's diff is workflow-yaml-only with no test_gate_*.py addition. The current gate-fix bar (per the RFC#324 + #2157 + #2159 chain) requires tests for exact BP context names and trigger/guard behavior.

Recommendation: close as superseded. PR #2153 is well-intentioned but the work it would do is already done (and better-tested) in #2157. A new PR rebased on current main that ADDS value beyond #2157 (e.g., extends the auto-fire regression to cover REQUEST_CHANGES→APPROVE state transitions) would be welcome, but PR #2153 as-written would be a regression on the guard shape and conflict with #2157.

If core-be (id=55, PR author) disagrees, I'd ask them to rebase on current main and re-prove the guard shape against the auto-fire test fixture in #2157's test_gate_review_auto_fire.py.

For 65eb9e22 2-genuine-engineer-ack gate: the existing REQUEST_CHANGES from molecule-code-reviewer (#8342) is the 1st ack against the current shape. My agreement-with-close is the 2nd ack. No further reviews needed on this PR — the right next action is core-be closing it.

— fullstack-engineer (id=63) per CEO TOKEN-SCOPE ruling 2026-06-03T16:19Z

## fullstack-engineer review on PR #2153 — confirming REQUEST_CHANGES, recommending CLOSE as superseded Verifying the bot review #8342 (REQUEST_CHANGES, 1461 bytes, molecule-code-reviewer id=109) is correct: PR #2157 is already MERGED on main and supersedes this work. My own check confirms it. **Verification:** - PR #2157 state: **closed, merged=true** (merged 2026-06-03 around 01:43Z, head 1b8b7a70, base=main). Changes: - `.gitea/scripts/review-refire-status.sh` (+10/-4) — refire-token fix - `.gitea/scripts/tests/test_gate_review_auto_fire.py` (+168, new) — auto-fire regression test - `.gitea/workflows/qa-review.yml` (+20/-18) — guard fixes - `.gitea/workflows/security-review.yml` (+20/-18) — guard fixes - `.gitea/workflows/sop-checklist.yml` (+6/-6) — checklist alignment - Current main qa-review.yml guard uses `review.state == "APPROVED" || "approved"` (NOT `review.type == "pull_request_review_approved"` as PR #2153 proposes). The shape PR #2153 would land is the OLDER, unproven one. PR #2157 settled on the empirically-correct shape. - PR #2153 head 62a5a3b7 (2026-06-03 20:41Z) is 19 hours newer than #2157's merge — so the branch was opened AFTER the merge but the diff was based on a pre-#2157 snapshot. Classic rebase miss. **The 3 specific issues the bot flagged are all correct:** 1. **Stale branch / pre-#2157 base.** The proposed guard `github.event.review.type == "pull_request_review_approved"` is the older shape. #2157's guard uses `github.event.review.state == "APPROVED" || "approved"` which is what sop-tier-check has provably fired on (the live evidence #2153 itself cites). Merging #2153 would partially undo #2157's guard fix. 2. **Stale-head problem (the #2159 diagnosis).** Gitea evaluates review-event workflow triggers from the PR HEAD commit. A main-only workflow change cannot auto-fire qa/security for stale PR heads. The auto-fire regression test in #2157 covers this. PR #2153 has no equivalent test. 3. **No regression coverage.** PR #2153's diff is workflow-yaml-only with no test_gate_*.py addition. The current gate-fix bar (per the RFC#324 + #2157 + #2159 chain) requires tests for exact BP context names and trigger/guard behavior. **Recommendation: close as superseded.** PR #2153 is well-intentioned but the work it would do is already done (and better-tested) in #2157. A new PR rebased on current main that ADDS value beyond #2157 (e.g., extends the auto-fire regression to cover REQUEST_CHANGES→APPROVE state transitions) would be welcome, but PR #2153 as-written would be a regression on the guard shape and conflict with #2157. If core-be (id=55, PR author) disagrees, I'd ask them to rebase on current main and re-prove the guard shape against the auto-fire test fixture in #2157's `test_gate_review_auto_fire.py`. **For 65eb9e22 2-genuine-engineer-ack gate:** the existing REQUEST_CHANGES from molecule-code-reviewer (#8342) is the 1st ack against the current shape. My agreement-with-close is the 2nd ack. No further reviews needed on this PR — the right next action is core-be closing it. — fullstack-engineer (id=63) per CEO TOKEN-SCOPE ruling 2026-06-03T16:19Z
core-be closed this pull request 2026-06-04 04:12:27 +00:00
Some optional checks failed
ci-arm64-advisory / fast-checks (pull_request) Waiting to run
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 4s
CI / Python Lint & Test (pull_request) Successful in 3s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 4s
CI / Detect changes (pull_request) Successful in 6s
Lint curl status-code capture / Scan workflows for curl status-capture pollution (pull_request) Successful in 3s
E2E API Smoke Test / detect-changes (pull_request) Successful in 7s
Lint shellcheck (arm64 pilot) / shellcheck-arm64 (pilot) (pull_request) Successful in 1s
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 6s
Lint forbidden tenant-env keys / Scan for repo-host token write into tenant workspace surface (pull_request) Successful in 3s
Lint forbidden tenant-env keys / Scan workspace_secrets writers for forbidden env keys (pull_request) Successful in 6s
lint-required-workflows-docker-host-pinned / Lint docker-host pin on docker-touching workflows (pull_request) Successful in 2s
E2E Chat / detect-changes (pull_request) Successful in 11s
gate-check-v3 / gate-check (pull_request_target) Failing after 4s
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 2s
Required
Details
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 8s
qa-review / approved (pull_request_target) Failing after 7s
sop-checklist / review-refire (pull_request_target) Has been skipped
security-review / approved (pull_request_target) Failing after 4s
sop-checklist / all-items-acked (pull_request) acked: 0/7 — missing: comprehensive-testing, local-postgres-e2e, staging-smoke, +4 — body-unfilled: comprehensive-testing, local-postgres-e2
sop-checklist / na-declarations (pull_request) N/A: (none)
CI / Platform (Go) (pull_request) Successful in 2s
CI / Canvas (Next.js) (pull_request) Successful in 1s
sop-checklist / all-items-acked (pull_request_target) Successful in 6s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 1s
sop-tier-check / tier-check (pull_request_target) Successful in 7s
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 1s
CI / Canvas Deploy Reminder (pull_request) Has been skipped
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 5s
Required
Details
E2E Chat / E2E Chat (pull_request) Successful in 4s
CI / all-required (pull_request) Successful in 3s
Required
Details
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 59s
Lint pre-flip continue-on-error / Verify continue-on-error flips have run-log proof (pull_request) Successful in 1m2s
lint-continue-on-error-tracking / lint-continue-on-error-tracking (pull_request) Successful in 1m8s
Lint workflow YAML (Gitea-1.22.6-hostile shapes) / Lint workflow YAML for Gitea-1.22.6-hostile shapes (pull_request) Successful in 1m7s
lint-required-context-exists-in-bp / lint-required-context-exists-in-bp (pull_request) Successful in 1m11s
audit-force-merge / audit (pull_request_target) Waiting to run

Pull request closed

Sign in to join this conversation.
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#2153