Bind merge-queue review to the queued source - #4221
kojiwakayama wants to merge 7 commits into
Conversation
Merge queue refs serialize the base commit, not the pull request head. Resolve the queued source through GitHub merge queue metadata, validate every identity boundary, and keep independent invalidation fail-closed across all matching queue refs. Constraint: Merge-group payloads do not expose the source pull request head directly. Rejected: Reuse the queue-ref suffix as the source head | live queue evidence proves it is the base commit. Confidence: high Scope-risk: moderate Reversibility: clean Directive: Do not require Automated review until a live source and merge-group canary succeeds on this implementation. Tested: Deno 2.7.7 focused gate 68 steps; fmt; repository lint; typecheck and generated artifacts; unit 4277 tests and 34124 steps; cwd suites; independent code review. Not-tested: Live mergeQueueEntry values and synthetic status propagation, pending an active queue entry.
|
You have reached your Codex usage limits for security reviews. Please try again later. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
kojiwakayama has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Warning Review limit reachedNext included review available in 50 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
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 |
Live queue evidence showed that MergeQueueEntry carries separate base, synthetic merge-group, and pull-request source commits. Validate each identity independently before publishing a successful automated-review status. Constraint: GitHub GraphQL MergeQueueEntry.headCommit is the synthetic merge-group commit, not the pull-request source commit. Rejected: Equate headCommit with pullRequest.headRefOid | would fail every valid merge-group review closed. Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep baseCommit, headCommit, and pullRequest.headRefOid as separate trust bindings. Tested: Focused automated-review gate suite, 68 steps; repository lint; repository typecheck; diff check; independent code review at 96% confidence. Not-tested: Exact synthetic merge-group execution, pending deployment of this default-branch workflow fix.
![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: fc6bf1b0a3
ℹ️ 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".
There was a problem hiding this comment.
kojiwakayama has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Live merge-queue validation corrected the final commit binding on exact head GitHub GraphQL reports three distinct immutable identities for an active queue entry:
The gate now validates all three plus the pull-request number before it can publish success. Any mismatch fails closed. Verification on Deno 2.7.7:
The remaining deployment proof is an exact synthetic merge-group canary after this default-branch workflow fix merges. |
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
A failure run for an old source head could discover a newly queued ref by pull-request number and overwrite its successful synthetic status. Bind every queue failure write to GitHub’s current base, synthetic commit, and source-head identity, including the independent workflow fallback. Constraint: Queue-ref suffixes identify the base commit, not the source head. Rejected: Recheck only the pull-request number | a force-push can reuse the same number for a different source and synthetic commit. Confidence: high Scope-risk: narrow Reversibility: clean Directive: Never publish a queue status from a PR-number-only ref match. Tested: Focused gate suite, 70 steps; repository lint; repository typecheck; diff check; independent review at 95% confidence. Not-tested: Exact synthetic merge-group execution, pending deployment of this default-branch workflow fix. Related: #4221 (comment)
There was a problem hiding this comment.
kojiwakayama has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Update after live queue invalidation hardening on exact head The latest commit adds a second guard for failure publication: a stale run for an older PR source head can no longer discover a newer queue ref by PR number and overwrite that newer synthetic commit with failure. Queue failure publication now checks:
Fresh local verification with pinned Deno 2.7.7:
The remaining proof is still the post-merge exact synthetic merge-group canary, because this workflow has to be on the default branch before GitHub will run the corrected queue resolver. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de0868c3af
ℹ️ 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".
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
A later authenticated exact-head Codex no-findings verdict now supersedes earlier findings only when GitHub timestamps or same-second timeline order prove it is newer. Repeated review and comment reconciliation signals also coalesce before runner allocation, reducing the Actions backlog without dropping source-update or merge-group events. Constraint: Ambiguous finding chronology remains fail-closed, and wakeup runs are not coalesced because cancellation is an invalidation signal. Rejected: Require a no-op source commit after a false positive | wastes CI and makes resolved same-head findings impossible to clear automatically. Rejected: Coalesce all gate and wakeup events | can drop source review requests or turn harmless supersession into fail-closed invalidation. Confidence: high Scope-risk: moderate Reversibility: clean Directive: Keep source-update and merge-group runs unique unless reconciliation becomes fully state-derived before runner allocation. Tested: deno task test:file scripts/ci/automated-review-gate.test.ts (7 suites, 71 steps) Tested: deno task lint Tested: deno task typecheck Tested: deno fmt --check and git diff --check Tested: independent code review approved at 92% confidence and security review approved at 93% confidence Not-tested: Hosted GitHub concurrency behavior requires the post-merge canary.
There was a problem hiding this comment.
kojiwakayama has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
📦 Client bundle boundary
A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in |
Merge-queue status propagation must never reuse stale source proof or publish against a superseded synthetic commit. Reconcile live pull request, base, queue-ref, and status evidence at each publication boundary, preserve exact-head verdict ordering, and coalesce replaceable reconciliation runs without cancelling fail-closed wakeups. Constraint: Human approval remains required until the merged workflow passes a real source and merge-group canary. Rejected: Trust the synthetic merge-group head alone | it does not prove the queued source, base, or current queue-ref ownership Rejected: Cancel wakeup runs with shared concurrency | cancellation itself is a fail-closed invalidation signal Confidence: high Scope-risk: moderate Reversibility: clean Directive: Do not remove live source, base, queue-ref, and final pre-publication checks without an equivalent race-proof binding. Tested: Focused automated-review gate suite, 7 suites and 80 steps Tested: Direct script typecheck, repository lint, repository typecheck, format, syntax, and diff checks Not-tested: Live merge-group propagation until this commit reaches the default branch
There was a problem hiding this comment.
kojiwakayama has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
@codex review |
|
@codex review Please review exact current head |
A queue binding can disappear after ref enumeration but before a failure status is safe to publish. Re-read the exact queue ref and skip only a moved or deleted entry; keep a still-live entry, malformed response, or API outage fail-closed. Constraint: GitHub can delete or retarget merge-queue refs during reconciliation. Rejected: Treat every unpublished failure as obsolete | a live synthetic commit could retain an older successful review status Rejected: Suppress queue-ref lookup failures | an outage is not proof that the entry disappeared Confidence: high Scope-risk: narrow Reversibility: clean Directive: An unpublished failure may be skipped only after exact live-ref evidence proves the synthetic commit is no longer active. Tested: Focused automated-review gate suite, 7 suites and 82 steps Tested: Direct typecheck, lint, format, syntax, and diff checks Not-tested: Live merge-queue ref deletion race until the workflow reaches the default branch
There was a problem hiding this comment.
💡 Codex Review
veryfront-code/scripts/ci/automated-review-gate.mjs
Lines 892 to 894 in cc9acc8
Capture the reconciliation boundary inside the publisher lock
When this event's target job runs while an older publisher for the same SHA already holds the publisher lock, the target can capture status S1, then that older publisher can write success S2 from evidence it collected before this event occurred. If this event's review job subsequently fails, latestStatus.id !== reconciliationStatusId treats S2 as a successful later reconciliation and skips invalidation, even though S2 never observed the finding or dismissal; the stale source success and any queue success copied by the older publisher can therefore remain mergeable. Capture the boundary after acquiring the shared publisher lock, or bind a success to the event/evidence epoch rather than only comparing status IDs.
AGENTS.md reference: AGENTS.md:L240-L241
ℹ️ 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".
There was a problem hiding this comment.
kojiwakayama has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
@codex review Please review exact current head |
|
@codex review Exact head: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 77735e8871
ℹ️ 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".
|
@codex address that feedback |
Summary
Testing
|
Issue-comment edits expose the current verdict body, so comment evidence now uses a valid updated timestamp while preserving the creation timestamp as a defensive fallback. Reviews and reset boundaries retain their existing clocks. Constraint: The gate runs on issue_comment edited events and evaluates current comment bodies Rejected: Apply updated_at to all evidence and boundaries | review and boundary semantics must remain unchanged Confidence: high Scope-risk: narrow Directive: Keep review evidence on submitted_at and request, base, and status boundaries on created_at Tested: PATH=/private/tmp/vf-deno-2.7.7.b6yD1m:$PATH deno task test:file scripts/ci/automated-review-gate.test.ts Tested: Deno format and lint checks with scripts/test.deno.json on both touched files Tested: git diff --check Not-tested: Full repository test suite
There was a problem hiding this comment.
kojiwakayama has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
@codex review Please review exact current head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7db655008e
ℹ️ 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".
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Closed as superseded by merged PR #4223. The merged implementation includes the equal-timestamp edited-comment fail-closed fix missing here, so merging both would be redundant and could reintroduce stale-success behavior. |
1 participant
Why
The #4161 live merge-group run proved that GitHub serializes the merge queue base commit in the queue-ref suffix. The gate treated that suffix as the pull request head, so it failed every synthetic queue commit.
What changed
The privileged workflow still executes only default-branch code loaded through the GitHub API. It does not check out fork contents.
Verification
Rollout guard
Keep the current human approval requirement and keep Automated review non-required until this change passes a live source-head plus merge-group canary. The canary is the evidence needed before removing the permanent human bottleneck.