fix(ci): guard main-CI incident closing references (#8447) - #8451
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 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 |
github-actions
Bot
added
the
review-bot-auto
🔍 PR Contract Check SummaryOverall Status: 🟢 PASSED
ℹ️ Info
This check is mechanized. Please resolve any blockers to pass CI. |
|
review-claim: codex-followup-20260905 @ 8877afb until 2026-09-04T23:48Z |
|
review-claim: released @ 8877afb |
|
review-claim: cycle1-pr8451 @ 0c6a1c5 until 2026-09-05T00:07Z |
ll7
left a comment
ll7
left a comment
There was a problem hiding this comment.
Exact-head implementation review
- Pull request: #8451
- Reviewed head:
2443cf8eb4a27c107ede2956b4cbfd92a9650252 - Review base:
origin/main=e7b6f9bfde9f80ca65d62bb1d85c681946e2a3dd - Scope: main-CI incident closing-discipline guard across the PR contract check, merge-queue gate, and single-account merge receipt.
The implementation correctly keeps ordinary Refs references non-closing, recognizes the canonical incident marker/compatibility label, scans both the PR body and paginated commit messages, and fails closed when issue or commit metadata is unavailable. The live merge-gate recheck is carried through the receipt path, and the new receipt tests bind the audit result to the same current metadata.
No actionable implementation finding was identified in this bounded exact-head review. The review is not merge approval: the live gate currently reports closing_discipline_status=passed, staleness_verdict=fresh, resolved review threads, and clear reviewer requests, but remains blocked by the missing merge-ready label, pending hosted CI, missing exact-head gate verdict, and missing reconciled metadata verdict.
Local proof on the exact head:
192 passedacrosstests/validation/test_pr_contract_check.py,tests/dev/test_merge_queue_gate.py, andtests/dev/test_single_account_merge_receipt.py(one existing soft slow-test notice; no failure).- Ruff check and format check passed for all six changed Python files.
- Python compilation, docs evidence integrity, and
git diff --checkpassed.
Disposition: keep the PR non-merge-ready until the current hosted checks complete and the exact-head gate/metadata evidence is rebuilt. No benchmark, research, safety, or paper-facing claim is made.
|
review-claim: cycle1-pr8451-refresh @ 2443cf8 until 2026-09-04T23:22Z |
|
review-claim: autopilot-cycle3-pr8451 @ 4d4902b until 2026-09-05T00:50:32Z |
ll7
left a comment
ll7
left a comment
There was a problem hiding this comment.
Exact-head implementation review
Reviewed PR #8451 at exact head 4d4902b50f204900517a68393f67c5f140eec735 against current
origin/main 1cb110f36cbcc96f781c3e5e8f99b13e60a0cb3e.
Intended contract
The change correctly keeps canonical main-CI incident issues open until the scheduled reconciler
proves the two-consecutive-decisive-green-run criterion. It applies the same semantic closing
reference check to PR bodies and paginated commit messages, fails closed for unavailable or empty
commit metadata, and binds merge-gate and single-account receipt decisions to the live result.
The docs and focused tests cover the stated behavior.
Findings
No verified P0-P3 implementation findings. The scope is support/tooling only; no research,
benchmark, metric, provenance, or paper-facing claim is admitted.
Validation
tests/validation/test_pr_contract_check.py tests/dev/test_merge_queue_gate.py tests/dev/test_single_account_merge_receipt.py: 192 passed, 1 skipped.- Final
scripts/dev/pr_ready_check.shthrough the initialized shared all-extras environment: passed, including the core readiness lane, contract/follow-up/domain checks, TODO-docstring ratchet, broad-exception ratchet, and clean-tree stamp. - Base-sensitive selector: 22-test subset passed at the exact head.
- Scoped Ruff check/format, byte-compilation,
git diff --check, and conflict-free merge-tree: passed. - Hosted checks: 33 successful; exact-head
changed-coverage-gateis still in progress; no failure observed.
The single-account internal-review waiver applies only to the lack of a distinct review identity;
it does not waive hosted checks, branch protection, current-base proof, or any lifecycle gate.
Disposition
Keep open and do not apply merge-ready while changed-coverage-gate is pending. Re-run the
exact-head live gate and final receipt after that check reaches a terminal success and re-read the
PR head/body/claims immediately before any further mutation.
exact-head: 4d4902b
base-policy: current-base @ 4d4902b
pr-metadata: reconciled @ 22c8fd6d8ebabff0436a750c27e0b3f3efb922bb77834880d3961f9eee6bf08e
gate-verdict: blocked @ 4d4902b
merge-ready: no
ll7
left a comment
ll7
left a comment
There was a problem hiding this comment.
Exact-head self-review
Reviewed PR #8451 at exact head 4d4902b50f204900517a68393f67c5f140eec735 against current
origin/main 1cb110f36cbcc96f781c3e5e8f99b13e60a0cb3e.
Intended contract and findings
The implementation matches the stated lifecycle contract: canonical main-CI incident issues stay
open for the scheduled two-consecutive-decisive-green-run reconciler; semantic closing references
are checked in both the PR body and paginated commit messages; unavailable or empty commit metadata
fails closed; and merge-queue and single-account receipt decisions bind to the live result.
Documentation and regression tests cover the new behavior. No verified P0-P3 implementation
findings remain.
This is a support/tooling/docs-only change. It makes no research, benchmark, metric, provenance, or
paper-facing claim, so no domain approval or downstream evidence propagation is required.
Validation and gates
- Focused contract suite: 192 passed, 1 skipped.
- Final
scripts/dev/pr_ready_check.shthrough the initialized shared all-extras environment: passed, including core readiness, PR contract/follow-up/domain checks, TODO-docstring and broad-exception ratchets, and clean-tree stamp. - Base-sensitive selector: exact 22-test subset passed at this head.
- Scoped Ruff check/format, byte-compilation,
git diff --check, and conflict-free merge-tree: passed. - Hosted checks at this exact head: 35 successful, 2 intentional skips, 0 pending/failures; changed-coverage and CodeQL are terminal success.
- Review threads: none unresolved; requested reviewers: none.
The single-account internal-review waiver covers only the absence of a distinct review identity. It
does not waive hosted checks, branch protection, current-base proof, or lifecycle gates.
Disposition
The proof bar is complete. Apply merge-ready with the exact head below, then perform the guarded
single-account receipt and final compare-and-swap merge preflight immediately before merging.
exact-head: 4d4902b
base-policy: current-base @ 4d4902b
pr-metadata: reconciled @ 22c8fd6d8ebabff0436a750c27e0b3f3efb922bb77834880d3961f9eee6bf08e
gate-verdict: accepted @ 4d4902b
merge-ready: yes
ll7
left a comment
ll7
left a comment
There was a problem hiding this comment.
Exact-head implementation review
Reviewed PR #8451 at exact head 4d4902b50f204900517a68393f67c5f140eec735 against current
origin/main 1cb110f36cbcc96f781c3e5e8f99b13e60a0cb3e in a clean linked worktree. Hosted checks
currently have 33 successful results, one pending changed-coverage check, and two skips.
Blocking finding
- P1 — canonical write-boundary bypass: the closing-discipline recheck is present in
merge_queue_gate._evaluate_live()andbuild_live_evidence(), but
apply_guarded_merge()still accepts a receipt whose gate audit only saysstatus: success.
Thestacked_prsmerge-cascade path constructs exactly that shape from its check-run summary and
forwards it to the write boundary. A local stubbed proof reached the merge PUT without a verified
closing-discipline result.
The empty/whitespace commit-source repair and declaration-parser repair are otherwise covered by
the 193 focused tests. The receipt schema and developer/workflow documentation should also expose
the new gate-audit closing fields when the write-boundary repair lands.
Validation
- Focused contract/merge-gate/receipt suite:
193 passed. - Stacked-PR suite:
23 passed. - Ruff 0.16.5 check/format, compilation, diff check, gate self-test, docs integrity, YAML parsing,
and receipt-schema validation passed in the review lane.
Decision: request changes and keep the PR unlabelled/unmerged. The P1 must be fixed at the canonical
write boundary and covered for both direct and stacked guarded merges before any merge admission.
No research, benchmark, metric, release, or paper-facing claim is made.
gate-verdict: blocked @ 4d4902b
base-policy: ordinary-cas @ 4d4902b
pr-metadata: reconciled @ 22c8fd6d8ebabff0436a750c27e0b3f3efb922bb77834880d3961f9eee6bf08e
## Summary Close the remaining post-merge safety gap from #8447: every guarded merge must carry a fresh, validated closing-discipline result at the final write boundary. This follow-up makes the canonical single-account receipt authoritative for both direct and stacked merge paths. ## Linked Issues - Refs #8447 ## Stack / Dependency - Base dependency: none; based directly on current `origin/main` after #8451. - Safe to review independently: yes. ## What Changed - Require `apply_guarded_merge()` to re-evaluate current closing-discipline evidence immediately before the merge write and fail closed when the evidence is missing, stale, or blocked. - Carry the same structured audit through `stacked_prs` receipts, including explicit source, status, and reason fields. - Constrain the receipt schema and document the final write-boundary contract. - Add regression coverage for direct and stacked paths, ordinary compare-and-swap behavior, and unavailable or blocked closing evidence. ## Why It Matters The merged #8451 guard protected merge-queue admission and receipt construction, but an exact-head review found that the stacked caller could still provide a superficially successful check summary to `apply_guarded_merge()` without a current closing-discipline result. This follow-up closes that last bypass at the irreversible merge boundary. ## Research / Evidence Notes Not applicable — support/tooling/docs-only safety correction; no research or benchmark claim. ## Validation / Proof - Focused receipt, merge-gate, and stacked-PR suites: `220 passed` in a fresh linked-worktree environment. - Ruff, format, compilation, documentation-integrity, authority validation, and merge-queue self-tests passed. - `git diff --check` passed. ## Risks / Rollback - Risk: unavailable or ambiguous GitHub metadata can temporarily block a merge. This is intentional fail-closed behavior at the write boundary; retry after evidence is available. - Rollback: revert this self-contained follow-up commit. ## Docs / Provenance - Updated `docs/dev_guide_reference.md` and the canonical receipt/stacked-PR tooling contract. - No external artifact, benchmark result, publication, or scientific data was changed. ## Downstream Propagation Not applicable — support/tooling change; no research claim or durable evidence artifact. ## Follow-Up / Residual Scope No deferred work remains within this bounded write-boundary correction. The original incident reconciliation remains governed by the scheduled evidence-based closer. <!-- pr-contract:v2 change_class: tooling linked_issues: closes: [] relates: [8447] deferred_work: status: none issues: [] reason: "" evidence: applicability: na tier: null result: na domain_approval: required: false status: not_required domains: [] note: "NA - support/tooling safety correction; no experimental claim." performance: claimed: false exact_head: be00d0d -->
1 participant
Summary
Prevent native GitHub closing references from bypassing the evidence-based completion rule for
main continuous-integration (CI) incidents. The blocking PR contract check now leaves those
issues open for the scheduled reconciler to close after the two-green criterion is met, and
merge admission repeats that lifecycle check against current metadata.
Linked Issues
Stack / Dependency
What Changed
scripts/ci/pr_contract_check.pyand fail closed when asemantic
Closes,Fixes, orResolvesreference targets a canonical main-CI incident.repository references while ignoring qualified references to other repositories. Empty or
whitespace-only commit metadata is treated as unavailable evidence and fails closed.
GitHub-URL forms.
merge-queue gate; record the result in the machine-readable audit and fail closed when it is
blocked or unavailable.
guarded merges cannot bypass the merge-queue lifecycle check.
ll7-main-red-incident:v1body marker and compatibility label, with focusedtests for semantic blocking,
Refsallowance, metadata failure, empty-source rejection, andsummary reporting.
closer and to document that cancelled or superseded runs are neutral.
Why It Matters
Issue #8414 demonstrated that a repair PR containing a native closing reference for that issue could close the incident before
its stated two-consecutive-green-run criterion was satisfied. This guard prevents that lifecycle
bypass while preserving ordinary issue and epic closing behavior, including when the reference is
carried by a commit message or when incident metadata changes after the PR check ran.
Research / Evidence Notes
Not applicable — support/tooling/docs-only change; no research or benchmark claim.
Validation / Proof
tests/validation/test_pr_contract_check.py,tests/dev/test_merge_queue_gate.py, andtests/dev/test_single_account_merge_receipt.py: 193 passed in a fresh linked-worktreeenvironment.
ruff checkandruff format --checkpassed for all six changed Python files.python -m py_compile scripts/ci/pr_contract_check.py scripts/dev/merge_queue_gate.py scripts/dev/single_account_merge_receipt.pypassed.git diff --checkpassed.pysocialforceenvironmentissue tracked in friction: make stale shared-venv recovery worktree-safe and deterministic #8443; validation used an independent fresh linked-worktree environment.
Risks / Rollback
This is intentional fail-closed behavior for lifecycle safety; retry after metadata is available.
Docs / Provenance
.github/PULL_REQUEST_TEMPLATE/pr_default.mdanddocs/dev_guide_reference.md.incident identity or closure path is introduced.
Downstream Propagation
Not applicable — support/tooling change; no research claim or durable evidence artifact.
Follow-Up / Residual Scope
No deferred work remains for this scoped recurrence guard. Existing PR-readiness signal-timing
friction remains tracked by issues #8347, #8368, and #8443.
Reviewer Notes
Closes,Fixes, andResolvesare blocked for marker- or label-identified incidentsin both PR bodies and commit messages.
and that declaration warnings accept colon, qualified, and URL forms.
performs evidence-backed issue closure.
commit metadata.