fix(ci): fail closed on incomplete review evaluation - #147
Conversation
Signed-off-by: Tapish Khandelwal <tapishkhandelwal13@gmail.com>
Signed-off-by: Tapish Khandelwal <tapishkhandelwal13@gmail.com>
There was a problem hiding this comment.
💡 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".
Signed-off-by: Tapish Khandelwal <tapishkhandelwal13@gmail.com>
Signed-off-by: Tapish Khandelwal <tapishkhandelwal13@gmail.com>
Signed-off-by: Tapish Khandelwal <tapishkhandelwal13@gmail.com>
There was a problem hiding this comment.
💡 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".
| issue_comment: | ||
| types: [created, edited, deleted] |
There was a problem hiding this comment.
Keep deleted findings blocking the gate
When a Codex PR-level finding is deleted, this deleted event runs only after the comment has disappeared from pullRequest.comments, so the evaluator sees no blocking comment and publishes a successful PR-head check even though no fixed, stale, rejected, or follow-up disposition was recorded. A maintainer can therefore clear the gate by deleting the finding; inspect the deleted event payload and keep matching automated findings blocking unless a durable disposition exists.
AGENTS.md reference: AGENTS.md:L92-L93
Useful? React with 👍 / 👎.
Signed-off-by: Tapish Khandelwal <tapishkhandelwal13@gmail.com>
Signed-off-by: Tapish Khandelwal <tapishkhandelwal13@gmail.com>
Signed-off-by: Tapish Khandelwal <tapishkhandelwal13@gmail.com>
| uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 | ||
| with: | ||
| fetch-depth: 1 | ||
| ref: ${{ github.event.repository.default_branch }} | ||
| persist-credentials: false | ||
|
|
||
| - name: Set up Node.js | ||
| uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 | ||
| with: | ||
| node-version: 22.13.0 |
There was a problem hiding this comment.
Linked follow-up: #149. The reported schedule route does not execute this checkout because review-gate is gated by github.event_name != 'schedule'; the separate scheduled-review-gate job checks out default-branch code and never installs PR content. The workflow_dispatch route can execute a selected PR head with pnpm caching, but only after an actor with repository write access dispatches it, and that job has no write permissions. Per the Round 3 stop rule, this pre-existing manual-dispatch/cache boundary is recorded for isolated treatment rather than adding machinery or making a second push here.
There was a problem hiding this comment.
💡 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".
| "--strict-head-review", | ||
| "--required-review-author", | ||
| "chatgpt-codex-connector", | ||
| "--allow-missing-head-review", | ||
| "--expected-head-oid", |
There was a problem hiding this comment.
Include workflow preflight in scheduled gate verdicts
In .github/workflows/review-gate.yml, the event-backed required Review gate includes dependency installation and pnpm workflow:preflight, but scheduled reconciliation reaches this invocation and evaluates only check-pr-review-gate.mjs before publishing the same required check name. If the event run fails because the PR deletes required template content, breaks its lockfile, or otherwise fails preflight, the next schedule can publish success for that same head and supersede the failure for branch protection. Make the scheduled verdict include a trusted equivalent of every condition in the required job, or publish it under a separate required context.
AGENTS.md reference: AGENTS.md:L21-L22
Useful? React with 👍 / 👎.
| "--strict-head-review", | ||
| "--required-review-author", | ||
| "chatgpt-codex-connector", | ||
| "--allow-missing-head-review", | ||
| "--expected-head-oid", |
There was a problem hiding this comment.
Preserve the head-review wait before publishing success
When a PR is opened or synchronized immediately before the 15-minute schedule and Codex has not reviewed the new head yet, this scheduled path passes --allow-missing-head-review without the event workflow's --wait-head-review-ms 180000; the evaluator therefore uses its zero-wait default and publishes a successful required check immediately. That removes the documented grace period and can make the new head mergeable before the automated review arrives. Preserve the wait or publish a non-success verdict while the required head review is still absent.
AGENTS.md reference: AGENTS.md:L56-L59
Useful? React with 👍 / 👎.
| const selected = eligible.slice(0, maxPrs); | ||
| if (eligible.length > maxPrs) { | ||
| console.warn( | ||
| `Review gate schedule cap hit: processing ${maxPrs} of ${eligible.length} eligible pull requests.`, | ||
| ); |
There was a problem hiding this comment.
Rotate capped reconciliation across all open PRs
When more than 25 eligible same-repository, non-draft PRs are open, every scheduled run rebuilds the same list and slices the same prefix, with no cursor or rotation. PRs beyond that prefix are therefore never reconciled while the first 25 remain open, so a late finding can leave an older successful check in place—or a correctly resolved finding can leave a stale failure indefinitely. Rotate the starting point, persist a cursor, or process all eligible PRs across runs.
AGENTS.md reference: AGENTS.md:L92-L93
Useful? React with 👍 / 👎.
Summary
minimizedReason === "resolved"predicate.issue_commenttrigger, its PR-head evaluator coupling, and the comment-specific publisher job.Review gatecontext to each captured PR head.Root Cause / Decision Record
Round 2 let an
issue_commentevaluation execute PR-authored scripts before a privileged job published that result as trusted. It also lacked publish retry and could not observe Hide → Resolved. The event mechanism is removed rather than extended.The scheduled job checks out
github.event.repository.default_branch, never checks out or installs a PR tree, and executes only the default branch's API-only evaluator and publisher. Exit 0 maps tosuccess, exit 1 tofailure, and exit 2 toaction_required.Schedule:
*/15 * * * *. Fifteen minutes keeps late-finding exposure in minutes, not hours, while avoiding continuous Actions/API churn. GitHub may delay scheduled starts, so this is nominal rather than an instant-clear promise. Hide → Resolved is picked up by the next scheduled run.Work is capped at 25 eligible PRs per run; cap hits are logged. Only open, non-draft, same-repository PRs are evaluated.
Permission and trigger boundary:
contents: readandpull-requests: read.scheduled-review-gatereceiveschecks: write.pull_request,pull_request_review, andpull_request_review_commenttrigger definitions are unchanged.pull_request_targetorissue_commentpath exists.Fork limitation:
Deleted-finding disposition:
Scope
Net removal:
Pack Workflow Preflight
pnpm workflow:preflightpassed from clean head01d6b2464d682123ca702cf14caa6ae9fa010bc4.origin/masterat7428f51was merged as5e409c3; no rebase was used.Sanchika Adoption Gate
@sanchika/*packages or copied Sanchika guidance.sanchika/docs/adoption-pack.mdor ComplyEaze and Axal completion evidence.Privacy And Data-Flow Impact
Sensitive Surface Review
checks: write.Chrome Web Store Impact
Verification
All final commands exited 0:
pnpm install --frozen-lockfilenode scripts/run-dependency-audit.mjs— no known vulnerabilitiespnpm exec wxt preparepnpm exec vitest run— 105 files / 1705 testspnpm exec tsc --noEmitpnpm exec eslint . --max-warnings 0pnpm exec prettier --check .pnpm exec actionlintpnpm workflow:preflightgit diff --checkpnpm exec wxt buildnode scripts/verify-extension-package.mjs .output/chrome-mv3pnpm exec wxt zipnode scripts/verify-extension-zip.mjsVitest aggregate footer:
The first exact ZIP verification attempt correctly failed because ignored
.outputheld both the prior 0.5.0 and new 0.5.1 ZIPs. The stale generated 0.5.0 ZIP was removed; the rerun and headed-browser check passed.PR #144 A/B with the exact requested command and expected head
a89d806224ced0aee9d5f6cd3f36139baf289fb7:7428f51: exit 0,PR review gate passed for lamemustafa/pack#144.5318064584, and prints the next-scheduled-run remedy.Scheduled live boundary:
Artifact Evidence
01d6b2464d682123ca702cf14caa6ae9fa010bc4..output/complyeazepack-0.5.1-chrome.zip.0ced11a01372bee6d6aeff7eadfdfc2ec9071a30ce7c01109e407002ad74ffd6.PR Review Follow-Up
ebd14c5; event removed; scheduled job checks out default branchebd14c5; shared retry helper; transient-success/exhaustion testsebd14c5; 15-minute schedule and corrected remedyReview gatecheck on captured eligible head44ce29b; Resolved clears; Off-topic and Outdated blockScreenshots
Not applicable; no UI changes.
DCO
Signed-off-by:trailers.