Skip to content

fix(ci): fail closed on incomplete review evaluation - #147

Merged
lamemustafa merged 11 commits into
masterfrom
tapish-codex/review-gate
Aug 18, 2026
Merged

fix(ci): fail closed on incomplete review evaluation#147
lamemustafa merged 11 commits into
masterfrom
tapish-codex/review-gate

Conversation

@lamemustafa

@lamemustafa lamemustafa commented Aug 17, 2026

Copy link
Copy Markdown
Owner

Summary

  • Preserves the Round 1–3 evaluator core: fail-closed API evaluation, retry, distinct blocking/evaluation exits, exact minimizedReason === "resolved", author matching, and PR-level finding detection.
  • Keeps the event-backed job named Review gate; it still checks out the PR head and runs install, workflow preflight, and the review evaluator under pull-request event isolation.
  • Publishes trusted scheduled reconciliation as a separate Review gate (scheduled) context so it cannot overwrite a preflight failure from Review gate.
  • Removes manual dispatch of a selected PR. workflow_dispatch now takes the trusted default-branch, API-only reconciliation route and never checks out, installs, caches, or executes PR-authored content.
  • Structurally separates PR-family triggers into review-gate.yml and trusted schedule/manual triggers into review-gate-reconcile.yml, so CodeQL no longer taints the PR-head cache from privileged triggers.
  • Preserves the three-minute current-head review wait and rotates the bounded four-PR selection without durable state.

Root Cause / Decision Record

The scheduled evaluator checks less than the event-backed job: it cannot honestly attest to PR-tree install or pnpm workflow:preflight without executing PR-authored content. Publishing both verdicts as Review gate let a scheduled success supersede an event/preflight failure. Round 4 therefore separates the contexts instead of duplicating privileged work.

Repository-admin action required: add the exact Review gate (scheduled) context to branch protection as a required check. Until an administrator does that, scheduled verdicts are advisory only. The existing required Review gate context remains unchanged.

The previous workflow_dispatch input selected a PR head and admitted that content to checkout, pnpm caching, install, and preflight. That capability is removed. A manual dispatch now reconciles eligible open same-repository PRs from trusted default-branch code using GitHub APIs only; it cannot manually run PR install/preflight.

Schedule: */15 * * * *. The reconciler excludes closed PRs, drafts, and forks, logs cap hits, processes at most four eligible PRs per run, and rotates the starting point by the 15-minute slot. Four keeps the worst-case sequence of four three-minute review waits within the 14-minute job limit.

Permission and trust boundary:

  • Workflow-level permissions remain contents: read and pull-requests: read.
  • Only scheduled-review-gate receives checks: write.
  • review-gate.yml has only pull_request, pull_request_review, and pull_request_review_comment; it contains the PR-head checkout/install/preflight job and no write permission.
  • review-gate-reconcile.yml has only schedule and workflow_dispatch; it contains the default-branch API-only publisher and the sole checks: write permission.
  • No pull_request_target or issue_comment path exists.

Deleted-finding persistence remains out of scope. The finding was rejected with reasoning and linked to #148; its thread is resolved without adding durable deleted-comment state.

Scope

  • Workflow: physically separate PR-family and trusted schedule/manual trigger sets; cap reduced to four.
  • Scripts: distinct scheduled context, preserved current-head wait, stateless capped rotation.
  • Tests: context names, dispatch isolation, eligibility/cap behavior, rotation, wait, and all Round 1–3 evaluator/publisher regressions.
  • Out of scope: src/**, extension behavior, manifest permissions, portal flows, dependencies, release claims, and durable deleted-comment state.

Final PR diff against origin/master: 8 files, +874/−81. The authorized structural follow-up commit 2942bb7 changes 3 files, +93/−75, moving the existing trusted job rather than adding evaluator machinery.

Pack Workflow Preflight

  • pnpm workflow:preflight passed from clean head 2942bb7e4f1916bd0a628f71ba9d0543793ccc54.
  • This PR was opened from a Pack branch, not master.
  • Latest master Pack AGENTS guidance was checked.
  • Required Pack privacy/review/verification checklist remains visible.
  • origin/master remains 7428f51; no Round 4 merge was needed and no rebase was used.

Sanchika Adoption Gate

  • This PR does not consume @sanchika/* packages or copied Sanchika guidance.
  • This PR does not require sanchika/docs/adoption-pack.md or ComplyEaze and Axal completion evidence.
  • This PR does not import parent or Sanchika source paths.

Privacy And Data-Flow Impact

  • No new browser or host permissions, extension network calls, analytics, telemetry, credentials, session material, GST files, or taxpayer data.
  • The only write is a repository-internal GitHub check run from trusted default-branch code.
  • Fork PRs are skipped and logged.

Sensitive Surface Review

  • Portal target binding, downloads, and service-worker behavior are unchanged.
  • No taxpayer identifiers, portal HTML, raw portal URLs, downloaded files, or workstation paths are in the diff.
  • No PR-authored code runs in the job with checks: write.
  • Schedule and manual dispatch cannot reach PR checkout, pnpm cache, install, or preflight.

Chrome Web Store Impact

  • No listing, permission, privacy declaration, or release-posture change.
  • Build/ZIP evidence is local verification only, not store-submission or release-readiness sign-off.
  • PR title uses Conventional Commits.

Verification

Round 4 dispatch guard red before fix — exit 1: tests/extension/ci-workflow.test.ts expected the explicit PR-family guard, but the workflow still used github.event_name != 'schedule', leaving workflow_dispatch admitted.

Final commands exited 0:

  • pnpm exec vitest run
  • pnpm exec tsc --noEmit
  • pnpm exec eslint . --max-warnings 0
  • pnpm exec prettier --check .
  • pnpm exec actionlint
  • pnpm workflow:preflight
  • git diff --check
  • pnpm exec wxt build
  • node scripts/verify-extension-package.mjs .output/chrome-mv3
  • pnpm exec wxt zip
  • node scripts/verify-extension-zip.mjs

The first full-suite run exited 1 because tests/scripts/export-chrome-web-store-assets.test.ts exceeded its existing 5-second timeout: 104 files / 1706 tests passed and 1 file / 1 test failed. The isolated file then passed 3/3 in 4.25 seconds without a retry/timeout code change. The final clean structural-split head exited 0:

 Test Files  105 passed (105)
      Tests  1707 passed (1707)
   Start at  05:51:24
   Duration  181.77s (transform 2.95s, setup 0ms, import 21.87s, tests 135.80s, environment 11ms)

Exact PR #144 A/B command with expected head a89d806224ced0aee9d5f6cd3f36139baf289fb7:

  • Protected base 7428f51: exit 0, PR review gate passed for lamemustafa/pack#144.
  • Updated head 2942bb7: exit 1, names PR-level comment 5318064584, and prints the next-scheduled-run remedy.

Package evidence:

  • Pack WXT extension package verification passed.
  • ZIP SHA-256: 6e39feae3a021b05250966e1f30c0f2de98b70fecd8a3672927a65931ac22341.
  • Exact ZIP verification passed, including Chromium network-denial evidence.

CodeQL after the authorized structural push:

Artifact Evidence

  • Clean pushed head: 2942bb7e4f1916bd0a628f71ba9d0543793ccc54.
  • Local ignored ZIP: .output/complyeazepack-0.5.1-chrome.zip.
  • No generated package, ZIP, checksum, profile, or downloaded file is committed.

PR Review Follow-Up

  • Latest-head CodeQL, Pack CI, title, and GitGuardian checks completed successfully.
  • No current-head Codex review object arrived during the three-minute gate wait; this PR does not claim review cleanliness.
  • The deleted-finding thread is rejected, linked to Track deleted Codex PR findings with durable review-gate state #148, and resolved.
  • Scheduled-context, review-wait, and capped-rotation findings are fixed and resolved with evidence.
  • CodeQL cache-poisoning finding is fixed by the authorized structural split; alert docs: tighten public issue data warnings #4 reports fixed and its threads auto-resolved.
  • The authorized structural follow-up push was made at 2942bb7 after the initial single-push boundary was explicitly lifted.
  • Fresh P1 thread discussion_r3799815762 remains open: branch protection still requires only Review gate; an administrator must add Review gate (scheduled).
Thread/comment Disposition Commit or evidence
Scheduled verdict supersedes event/preflight failure fixed a2354c3; separate Review gate (scheduled) context; admin branch-protection action disclosed
Schedule omits current-head review wait fixed a2354c3; 180000 ms wait and 10000 ms poll preserved
Capped reconciliation starves later PRs fixed a2354c3; stateless slot rotation and bounded cap; regression coverage
CodeQL cache poisoning through manual dispatch fixed 2942bb7; trusted triggers moved to a separate workflow; PR CodeQL check passes and alert #4 reports fixed
Keep deleted findings blocking rejected / linked follow-up #148; durable state remains out of scope
Scheduled context is not yet required open / external admin action Live branch protection currently requires only Review gate; add exact context Review gate (scheduled) before treating scheduled verdicts as blocking

This update does not claim the PR is review-clean; current-head CI and automated review remain authoritative.

Screenshots

Not applicable; no UI changes.

DCO

  • All new commits include Signed-off-by: trailers.

Signed-off-by: Tapish Khandelwal <tapishkhandelwal13@gmail.com>
Signed-off-by: Tapish Khandelwal <tapishkhandelwal13@gmail.com>
@lamemustafa
lamemustafa marked this pull request as ready for review August 17, 2026 22:13

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4b9899dbb9

ℹ️ 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".

Comment thread .github/workflows/review-gate.yml Outdated
Comment thread scripts/check-pr-review-gate.mjs Outdated
Signed-off-by: Tapish Khandelwal <tapishkhandelwal13@gmail.com>
Signed-off-by: Tapish Khandelwal <tapishkhandelwal13@gmail.com>
Signed-off-by: Tapish Khandelwal <tapishkhandelwal13@gmail.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1806e9478b

ℹ️ 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".

Comment thread .github/workflows/review-gate.yml Outdated
Comment thread scripts/publish-review-gate-check.mjs Outdated
Comment thread .github/workflows/review-gate.yml Outdated
Comment thread .github/workflows/review-gate.yml Outdated
Signed-off-by: Tapish Khandelwal <tapishkhandelwal13@gmail.com>
Signed-off-by: Tapish Khandelwal <tapishkhandelwal13@gmail.com>
Signed-off-by: Tapish Khandelwal <tapishkhandelwal13@gmail.com>
Comment thread .github/workflows/review-gate.yml Fixed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 01d6b2464d

ℹ️ 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".

Comment thread scripts/publish-review-gate-check.mjs
Comment thread scripts/publish-review-gate-check.mjs
Comment thread scripts/publish-review-gate-check.mjs Outdated
Signed-off-by: Tapish Khandelwal <tapishkhandelwal13@gmail.com>
Signed-off-by: Tapish Khandelwal <tapishkhandelwal13@gmail.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 598f13e492

ℹ️ 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".

Comment thread scripts/publish-review-gate-check.mjs
Comment thread .github/workflows/review-gate.yml Fixed
Signed-off-by: Tapish Khandelwal <tapishkhandelwal13@gmail.com>
@lamemustafa
lamemustafa merged commit 677f231 into master Aug 18, 2026
11 of 13 checks passed
@lamemustafa
lamemustafa deleted the tapish-codex/review-gate branch August 18, 2026 04:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants