Skip to content

fix(ci): reject malformed PR commit metadata (#8447) - #8457

Merged
ll7 merged 2 commits into
mainfrom
codex/fix-stack-closing-discipline-20260905
Sep 5, 2026
Merged

fix(ci): reject malformed PR commit metadata (#8447)#8457
ll7 merged 2 commits into
mainfrom
codex/fix-stack-closing-discipline-20260905

Conversation

@ll7

@ll7 ll7 commented Sep 4, 2026

Copy link
Copy Markdown
Owner

Summary

Fail closed when the paginated GitHub pull-request commit endpoint returns malformed metadata. The semantic-closing check must not treat a non-empty null, non-string, or otherwise invalid value as an available commit list.

Linked Issues

Stack / Dependency

What Changed

  • Require each paginated commit record to contain an object with a string commit message through a guarded gh --jq expression.
  • Encode each message at the CLI boundary, then strictly decode base64 and UTF-8 in Python before returning semantic-closing input.
  • Treat malformed, empty, or undecodable output as unavailable evidence so the existing fail-closed check path blocks it.
  • Add regression coverage for valid multiline output and non-empty malformed output.

Why It Matters

PR #8456 now enforces live closing evidence at the guarded merge boundary. This follow-up protects the commit-evidence source used by that discipline: malformed successful-looking command output must not be accepted as a commit list. The change does not alter merge authority or benchmark behavior.

Research / Evidence Notes

Not applicable — support/tooling-only change; no experimental, benchmark, or paper-facing claim.

Validation / Proof

  • tests/validation/test_pr_contract_check.py: 61 passed.
  • Ruff check and format checks passed for both changed files.
  • git diff --check passed.
  • Live paginated PR-commit probe successfully decoded the current commit list.

Risks / Rollback

  • Risk: malformed or temporarily unavailable GitHub metadata now blocks semantic-closing evaluation. This is intentional fail-closed behavior; retry after the source is available.
  • Rollback: revert this self-contained parser/test commit.

Docs / Provenance

  • No documentation, benchmark artifacts, model files, or scientific provenance claims are changed.

Downstream Propagation

Not applicable — support/tooling-only change with no research claim or durable evidence artifact.

Follow-Up / Residual Scope

No deferred work remains in this bounded input-validation correction. PR #8456 remains the owner of the live merge-boundary closing-discipline contract.

Reviewer Notes

  • Verify valid multiline and Unicode commit messages round-trip through the paginated API query.
  • Verify blank, whitespace-padded, invalid-base64, invalid-UTF-8, and non-empty malformed output is rejected and cannot reach semantic-closing evaluation.
  • Verify the PR remains independent of PR fix(ci): enforce live closing evidence at merge boundary #8456's already-merged merge-boundary implementation.

@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: c6e26a17-8e44-4678-b4cb-6008463594b1


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the review-bot-auto Managed label that routes code-bearing PRs to CodeRabbit. label Sep 4, 2026
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

🔍 PR Contract Check Summary

Overall Status: 🟢 PASSED

Check Status Description
1. Closes-discipline ✅ PASSED Demand Refs #N for epic issues and main-CI incidents
2. Closure declaration ⚠️ WARNING Require Closes/Refs for title issues
3. State-refresh-only ✅ PASSED Reject docs/context state updates
4. Evidence hygiene ✅ PASSED Checks markers and provenance fields
5. Evidence writer usage ✅ PASSED Require the shared marked writer path
6. Successor discipline ⚠️ WARNING Require successor statement on multi-PR issues
7. Worker-lane label ⚪ None Label PRs from cheap worker lane
8. Placeholder docstring ratchet ✅ PASSED Reject NEW TODO/empty docstrings in added diff lines

⚠️ Warnings

ℹ️ Info

  • No worker-lane provenance detected.

This check is mechanized. Please resolve any blockers to pass CI.

@ll7
ll7 force-pushed the codex/fix-stack-closing-discipline-20260905 branch from ba3f923 to be6f987 Compare September 5, 2026 00:11
@ll7 ll7 changed the title fix(dev): require closing evidence for stacked merges (#8447) fix(ci): reject malformed PR commit metadata (#8447) Sep 5, 2026

@ll7 ll7 left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Exact-head review for PR #8457

Disposition: BLOCKED — not merge-ready at this head.

Reviewed exact head: be6f987d1a3b24d6fc05291a06f111c940c7cb0e
Current origin/main: 98fd479e9391f27e61d837ca1ed9e86d4ad5fbf7

The narrow malformed PR-commit metadata hardening is directionally correct: focused tests pass
(54 passed), exact Ruff 0.16.5 check/format, compilation, diff checks, live paginated probing,
and the PR body contract pass. However, the parser currently filters blank lines and surrounding
whitespace before strict decoding. Mixed malformed/empty output can therefore still return a
non-None commit list, e.g. a blank line followed by a valid encoded message. This violates the
fail-closed contract.

Please reject blank or whitespace-bearing lines before decoding while preserving the normal single
trailing newline, and add regression coverage for mixed blank output plus invalid base64/UTF-8 and
valid multiline messages. Keep the PR blocked until the new exact head is reviewed and hosted checks
settle.

@ll7

ll7 commented Sep 5, 2026

Copy link
Copy Markdown
Owner Author

review-claim: autopilot-cycle5-pr8457 @ be6f987 until 2026-09-05T01:56:33Z

Exact-head review claim for the narrowed commit-metadata validation follow-up. The claim is time-bounded and does not authorize merge until the live receipt and final write-boundary gates are independently re-read.

@ll7 ll7 left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Exact-head self-review

Disposition: ACCEPTED — implementation and hosted checks are green; merge admission remains
subject to the canonical single-account receipt and final expected-head/base recheck.

Reviewed exact head: 6434ab56fcad62f0969b6cf315482f44523b03a1
Current origin/main: 98fd479e9391f27e61d837ca1ed9e86d4ad5fbf7

The requested parser repair is present and scoped to the malformed paginated PR-commit metadata
contract. It rejects blank or whitespace-bearing records, invalid base64, invalid UTF-8, empty
decoded messages, and mixed malformed/valid output while preserving valid multiline and Unicode
messages. Fresh exact-head validation passed: 61 validation tests, 83 merge-queue compatibility
tests, Ruff check/format, compilation, git diff --check, live paginated probes, and PR body
contract checks. Hosted checks are terminal with 32 successes and 2 intentional skips; the exact
head is mergeable and no actionable review-thread or requested-reviewer blocker is present.

This is support/tooling-only work with no research, benchmark, release, legal, or external-input
claim. The earlier exact-head P2 finding was fixed and re-reviewed. Single-account internal-review
waiver applies; it does not waive CI, exact-head, metadata, or final CAS requirements.

pr-metadata: reconciled @ 3c2e004bdcdca5219502088509ee6f4f6f9716b4e93945d1b41d84b8b4f307da
gate-verdict: accepted @ 6434ab56fcad62f0969b6cf315482f44523b03a1

@ll7 ll7 added the merge-ready Verified by the PR gate and ready to merge label Sep 5, 2026
@ll7
ll7 merged commit 820356c into main Sep 5, 2026
36 of 39 checks passed
@ll7
ll7 deleted the codex/fix-stack-closing-discipline-20260905 branch September 5, 2026 00:44
ll7 added a commit that referenced this pull request Sep 5, 2026
## Summary

Close the remaining merge-lifecycle bypass identified after PR #8456:
every merge-capable helper
must route through the canonical single-account receipt owner, including
the shell wrapper and
stacked-PR coordinator.

## Linked Issues

- Refs #8447

## Stack / Dependency

- Base dependency: none; this branch starts from current `origin/main`
after PR #8457.
- Safe to review independently: yes.

## What Changed

- Remove native `gh pr merge` and direct REST merge writers from
`scripts/dev/gh_pr_merge.sh`;
the wrapper now delegates exact-head report/apply operations to the
receipt owner and fails closed.
- Extend the merge-authority scan and transport policy to discover shell
writers as well as Python
callers, including split REST endpoint construction and workflow merge
commands.
- Preserve structural dual-read compatibility for pre-follow-up v1
receipts while requiring fresh
  closing evidence for active merges.
- Bind stacked readiness evidence to the exact PR head, body metadata,
and source evidence, and add
a regression proving a changed final live-evidence snapshot prevents any
merge write.
- Narrow the source-branch cleanup contract to the guarded post-merge
path and document that merge
  authority remains solely with the receipt owner.
- Update the authority fixture, receipt schema, docs, and focused
regression tests.

## Why It Matters

PR #8456 repaired the canonical receipt boundary but left the shell
dispatcher able to issue a
native or direct REST merge independently. That made the repository's
claim that all mutating paths
were guarded too strong. This follow-up closes that bypass and makes the
authority inventory
executable across the supported helper surface.

## Research / Evidence Notes

Not applicable — support/tooling safety correction; no research,
benchmark, planner-ranking,
publication, or paper-facing claim.

## Validation / Proof

- Focused merge/transport/receipt/stacked/API-contract suites: `140
passed` in a fresh linked
  worktree based on current `origin/main`.
- Merge-queue gate regression suite: `83 passed`.
- Shell syntax, authority self-test, transport-policy audit,
Ruff/format, compilation, and
  `git diff --check` passed.
- Prepublication state was captured and checked ready at exact head
  `65c6adaed5fe96ed81bc68f7ba38d4cdd95d736e` with current base
  `820356c7f30388292c43e7a5870d39697bae7b99`.

## Risks / Rollback

- Risk: malformed, stale, or unavailable merge evidence now blocks the
write path. This is
intentional fail-closed behavior; retry only after the evidence is
refreshed.
- Rollback: revert this self-contained lifecycle-safety correction.

## Docs / Provenance

- Updated the canonical GitHub transport and merge-authority
documentation and fixtures.
- No external artifact, benchmark result, publication, or scientific
data was changed.

## Downstream Propagation

Not applicable — support/tooling safety correction with no research
claim or durable evidence artifact.

## Follow-Up / Residual Scope

The parent incident #8447 remains open until the complete lifecycle
contract is independently
verified after this follow-up lands. No separate research or benchmark
follow-up is implied.

<!-- 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 merge-lifecycle safety correction; no
experimental claim."
performance:
  claimed: false
exact_head: 65c6ada
-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-ready Verified by the PR gate and ready to merge review-bot-auto Managed label that routes code-bearing PRs to CodeRabbit.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant