fix(sync): renew the sealed delivery Gate trigger - #3388
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Warning Review limit reachedNext included review available in 9 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 95 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (2)
Comment |
GitHub Actions
Inactive
Automated Status SummaryHead SHA: cb8f0a5
Coverage Overview
Updated automatically; will refresh on subsequent CI/Docker completions. Keepalive checklistScopeNo scope information available Tasks
Acceptance criteria
|
There was a problem hiding this comment.
🟢 Approval recommended
The change is narrowly scoped, matches the stated incident/behavior, and is backed by explicit contract tests for the key success and failure cases.
Pull request overview
This PR updates Maint 71’s “begin stable delivery review” step to reliably re-trigger consumer Gates that only listen for labeled events by ensuring any stale sync:delivery-ready label is removed before the PR body is updated to the “reviewing” delivery state. This prevents a prior delivery generation’s leftover label from causing sealing to add an already-present label (and therefore emit no fresh label event with the sealed body).
Changes:
- Reordered the review-start mutations to: add the staging hold label → remove any stale
sync:delivery-readylabel (ignore 404) → persist the updated PR body. - Added a contract test covering: stale label removal, absent label (404) being non-blocking, and permission failure (403) preventing review state advancement.
File summaries
| File | Description |
|---|---|
.github/scripts/maint71_merge_sync_prs.js |
Clears stale sync:delivery-ready before persisting “reviewing” state so later sealing reliably emits a fresh labeled event. |
.github/scripts/__tests__/sync_pr_merge_contract.test.js |
Adds a real-execution contract test to lock in the new label-clearing behavior and failure-mode handling. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
A previous delivery generation can leave
sync:delivery-readyon a stable candidate PR. Sealing then adds an already-present label, so consumers whose Gate listens forlabeledbut not body edits receive no fresh event with the sealed body. This stalled trip-planner#1790; refreshing the label produced a fresh required Gate run (33933089879) with a passing delivery seal; its Python matrix separately exposed a packaging-test race, while rerunning the older run retained its unsealed event payload.Maint 71 now establishes the staging hold, clears the ready label, and only then persists the start of review. Missing labels are harmless; other removal failures stop before advancing the review state. Sealing retains the staging merge hold and adds the ready label as before.
Validation: 82 contract tests pass, including real execution with a stale label, absent label (404), and permission failure (403) that must not advance state;
git diff --checkpasses.