[kyle-sexton]

No related issue: owner re-lift churn measured on #10224

Summary

On #10224, a lanes App pull request that changes a hold path, the owner lifted the hold on head f3de238112 at 23:26Z by removing needs-human and approved it at 23:31Z. At 23:28Z the pr-unblock resolve-conflicts lane merged main into the branch (head bad82b1b56), and at 23:45Z enable-auto-merge labeled it needs-human again: "it changes .github/actions/resolve-config/live-config.test.mjs, a hold path".

A hold path was lifted only by a trusted person's approving review of the exact head (merge-holds.mjs pathLifters, babysit_merge_policy.path_lifters), so any base merge after the approval sent the pull request back to the owner.

Fix

The owner's decision on this pull request: on gate and trust hold paths, only an approving review lifts the hold, bound to its commit, and carried only across a verified merge of the base. A label removal lifts no hold-path hold, and a dismissed approval never counts. ci-status's separate review-lane hold rule, where a label removal approves, is unchanged.

• Which approvals count (liftsOf, lifts_of): each trusted person's newest review that approves, requests changes or was dismissed; it lifts only when it approves, and then its commit. A dismissed approval never counts, whoever dismissed it.

• The carry (baseMergeCarry, base_merge_carry): the base branch is read once by its full ref (baseTip, base_tip: GET /repos/{repo}/git/ref/heads/{base}), and every compare names its tip commit, so no tag of the same name stands in for it. An approval of head H covers head H' when both hold, read from the compares <tip>...H and <tip>...H':

  • Every commit H' adds is a merge whose parents other than the first the base holds (baseMergesOnly over mergedParents; baseParents reads each compare/<parent>...<tip>, ahead or identical), and H is among H''s commits. A crafted merge commit as a second parent is not a base commit.
  • The net change is the same at H and H' over every file (sameNetChange: same files, status, previous_filename and patch). A merge that writes any change of its own, to a hold path or any other file, carries nothing. An honest merge after the base changed one of the pull request's files also carries nothing, and needs a new approval.

  The owner's cost decision carries through the same function, so the cost hold reads the base the same way.

• What a carried approval lifts: the hold path and the trusted-actor list widening, nothing else. Only an approving review of the exact head lifts the verifier, issue-link and pr-verify FAIL holds.

Where the rule runs:

• ci-status (merge-holds.mjs main) calls liftersOf only when a hold path or widening holds and no approval of the head is in the discussion file. liftersOf reads the pull request itself and lifts nothing for a head that moved or is in a fork, then reads the base ref, the reviews and the compares with the step's GH_TOKEN.
• enable-auto-merge calls the same liftersOf, with the pull request's own base rather than a fixed main, so a pull request with a carried approval is armed instead of labeled again.
• The gate's risk score (lane_risk_refusal, carried_lifters) drops high-path: reasons on the same approvals, and lifts nothing for a head in a fork.
• The cost hold step in ci-status resolves the base branch's tip by its full ref, compares against it, reads each merged parent against it (mergedParents), and calls the base's baseMergeCarry. Until this pull request merges, a base without it keeps the hold.

Limit under the current ruleset. Ruleset 24475076 on main sets dismiss_stale_reviews_on_push: true, so the push of a base merge dismisses every approval of the earlier head, and a dismissed approval never counts. On #10224 the owner's 23:31:57Z approval of f3de238112 reads as DISMISSED. While that setting holds, an approval does not carry over a base merge in this repository, and the #10224 churn stays; the carry applies where stale approvals are not dismissed. The cost-decision carry does not depend on reviews and is unaffected.

Residual: mode-only changes. The compare API reports no file mode, so a merge that only flips a file's mode (making a script executable, say) reads as the same net change and carries an approval. Closing this needs tree reads, which this change does not add.

Truncated compares. A hold-path carry also requires the head compare to list as many files as the pull request's changed_files (read with the pull request liftersOf and carried_lifters already re-read), so a compare GitHub cut short fails closed. That GitHub truncates a compare below its 300-file cap is UNVERIFIED. The cost carry does not have changed_files in hand without another read, so it relies on sameNetChange's 300-file cap alone.

The lane-change vet in the cost carry (noticedHead, _noticed_head) also compares against the base's tip commit, resolved once, never the branch name.

Security reviews of earlier versions found that they counted a label removal at the exact head as an approval, bound a removal to a head pushed while the person was reviewing, checked only a merge's parent count, compared only hold-path files (so a merge could rewrite scripts/helper.sh and still carry), admitted any file the base had touched, named the base by branch in every compare, and lacked the fork check in Python. The owner's decision removes the label-removal path and the stale-dismissal rule; the parent check, the whole-net-change rule, the tip-commit base and the fork check are in.

#10224's own shape no longer carries under the whole-net-change rule. Replayed read-only against main's tip when enable-auto-merge relabeled it (23:45Z, 0218059955): the net change at bad82b1b56 differs from f3de238112's in two files outside the hold paths (.github/actions/collect-base-activity/collect-base-activity.test.mjs and plugins/code-tidying/.claude-plugin/plugin.json), so sameNetChange is false. It fails closed and needs a new approval, as decided. Combined with the ruleset limit above, this change does not remove the #10224 churn; it closes the base-merge hole safely.

Verification

Adversarial cases. Every one holds, in JS (merge-holds.test.mjs through main), and in JS and Python together (test_merge_holds_sync.py):

• a label removal before a base merge, or while the head is the head, with the label and push history served
• an approval GitHub dismissed as stale on the merge's push, alone or with a label removal
• an approval later dismissed; an approval followed by a request for changes; an approval then a dismissal
• an approval by a person off the trusted-actor list, or by a bot account
• a merge that edits or adds a hold path
• a merge that rewrites scripts/helper.sh, which the approved head already changed
• a merge that adds a file the base touched, with content of its own
• the base took in one of the approved head's files
• a merge whose second parent the base does not hold (a crafted merge of base commits); parents that are not commit objects
• a non-merge commit after the approval; a merge of a branch other than the base
• a force push that rewrites the approved head; a commit list the compare cuts at 250
• a head in a fork; a pull request whose head moved; an unreadable pull request
• a base ref that does not resolve to a commit, or does not read
• unreadable reviews or compares

Each fix's tests fail against the version before it: under cbb68549, the scripts/helper.sh rewrite and the added base-touched file carry (baseMergeCarry returns true), and the tip-commit path assertions fail. The positive cases assert that the base is read once by git/ref/heads/main and every carry compare names the tip commit. No reader reads issue events, push history or the tracker config for a hold-path lift.

Positive cases carry: an approval of an earlier head followed by a base merge, with or without a non-hold file in the net change that the merge leaves alone.

Suites, before the merge of main:

• merge-holds.test.mjs: 46 pass. enable-auto-merge.test.mjs: 143 pass.
• babysit-prs scripts/tests: 3332 passed.
• ci-hold-cost-decision.test.sh: 157. ci-hold-human-gated: 49. ci-hold-lifted-by: 35. ci-hold-merge-group: 67. ci-lanes-pr-hold: 220.

Related

• Follow-up, outside this change: babysit_verify_evidence.py reads the trusted-actor list by the base branch's name (around line 1400), with the same tag-shadowing exposure this change closes for compares.

• feat(pr-pipeline): tidy a small single-area change inside resolve-issue before its one push #10224: the measured churn.

• fix(pr-merge): keep a cost hold when a newer lane run was deleted #10515 (merged): the cost hold's push-history check.

• This changes gate files (merge-holds.mjs, enable-auto-merge.mjs, pr-require-checks.yml, babysit_merge_policy.py), so the security lane may hold it for a person.

• The cost carry now also requires every commit since the decided head to be a merge of commits the base holds, so a rebase no longer carries a cost decision. This only tightens that hold.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-28T04:05:50.356754Z	4425153	Draft marked ready

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 44251530bf

ℹ️ 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".

[github-actions]

claude-review: 2 inline finding(s) on 44251530bf08e5e833cbf0f08a85a463e95fb5ce; the summary is in this lane's pull request comment.

claude-lane: claude-review

[github-actions]

claude-review reviewed 450899334971a1543de747084b1035aa56b9d874: 0 finding(s), 0 as inline comments.

No blocking or advisory findings. I read the whole diff (.claude-lane/pr.diff, 14 files) and the PR context, and checked how the cost step's holds_js helper passes arguments and what fromRest returns. I ran no tests, so the PR's suite counts are author-claimed and unverified.

The PR has no closing issue, so there are no acceptance criteria to check. It touches trusted paths (merge-holds.mjs, enable-auto-merge.mjs, pr-require-checks.yml, babysit_merge_policy.py, babysit_verify_evidence.py). I read each of those changes in full and found nothing that weakens a gate:

• Fail-closed reads: every unreadable or unresolved input (the base ref, the compares, the reviews, the pull request) lifts nothing. A fork head and a head that moved since it was read also lift nothing.
• Merge-only carry: a carry needs every added commit to be a merge whose non-first parents are ancestors of the base tip, with the approved head among the listed commits. It also needs the same net change over every file. A commit list the compare cuts at 250 carries nothing.
• Verifier holds: a carried approval lifts only the hold path and the trusted-actor widening. The verifier, issue-link and pr-verify FAIL holds still need an approval of the exact head (approved rather than lifters in main).
• Matching Python and JS: lifts_of/liftsOf, base_merge_carry/baseMergeCarry and carried_lifters/liftersOf are written the same way. test_merge_holds_sync.py checks that they agree.
• Cost-hold step: holds_js passes parsed JSON files to the named export, so the new mergedParents and baseMergeCarry calls and the JSON array of base-held parents line up. A base without these exports makes the cost decision fail closed, as the PR says.

Notes, not counted as findings:

• Known residual (documented in the PR and code): the compare API reports no file mode, so a merge that only flips a file's mode reads as the same net change. The gate's high-path:** mode and symlink holds are still not lifted by an approval.
• Ruleset limit (stated in the PR): with dismiss_stale_reviews_on_push on main, an approval is dismissed on the base-merge push, so the feat(pr-pipeline): tidy a small single-area change inside resolve-issue before its one push #10224 churn stays until that setting changes. The PR says this too.
• Test noise: test_babysit_lane_reviews.py and test_babysit_merge_cost.py carry formatter-only rewraps unrelated to the change.
• Spend: the change adds a few extra GitHub REST reads (pull request, base ref, reviews, compares) per held pull request in ci-status and enable-auto-merge. It adds no paid plan, billing setting or billed resource, so there is no cost hold.

[github-actions]

claude-security-review: 1 inline finding(s) on 44251530bf08e5e833cbf0f08a85a463e95fb5ce; the summary is in this lane's pull request comment.

claude-lane: claude-security-review

[github-actions]

claude-security-review reviewed 450899334971a1543de747084b1035aa56b9d874: 1 finding(s), 1 as inline comments.

Security review: PR #10596, carry a hold-path lift over a later base merge

critical-findings: 0

Scope read. I read the whole diff and the PR context. The trusted-path files in it are merge-holds.mjs, enable-auto-merge.mjs, the ci-status cost-hold steps in pr-require-checks.yml, and babysit_merge_policy.py and babysit_verify_evidence.py (the last two for the risk-score lift). I did not run any of the code or its tests.

What the change does. A trusted person's approving review of an earlier head can now lift the hold-path hold and the trusted-actor-list widening hold on a later head. The cost decision carries by the same rule. This weakens a merge hold, so I checked what still stops a bypass.

Checks that held

• Who the approver is. Only a trusted-list User id counts. A bot or off-list id does not (liftsOf, lifts_of).
• Which review counts. Each person's newest APPROVED, CHANGES_REQUESTED or DISMISSED review decides. A dismissed approval or a later change request cancels the lift.
• How the commits are checked.
  • Every commit the new head adds must be a merge.
  • Every merged non-first parent must be an ancestor of the base tip, read as compare/<parent>...<tip> being ahead or identical.
  • The approved head must be among the listed commits, so a force push carries nothing.
  • Both commit lists must be read whole. A history of 250 or more commits carries nothing.
• How the tip is read. The base tip is read once by its full ref (git/ref/heads/<base>) and checked as 40-hex. Every compare names that tip commit, so a tag of the same name cannot stand in.
• What the net-change rule covers. The net change (sameNetChange) is compared over every file, with status, previous_filename and patch. A merge that edits any file, hold path or not, carries nothing. A file with no patch, a duplicate filename, or 300 or more files carries nothing.
• Compare truncation. In both JS and Python, the head compare's file count must equal the PR's changed_files, so a truncated compare carries nothing.
• Which head is judged. liftersOf and carried_lifters re-read the PR and lift nothing if the head moved or lives in a fork. Every failed read returns an empty lifter list, so reads fail closed.
• What a carried approval does not lift. In main, the verifier and issue-link holds and the pr-verify FAIL holds use only approved, the exact-head approvals. A carried approval lifts only the path hold and the widening hold.
• Base without the new functions. The ci-status cost step runs merge-holds.mjs from the base. A base that lacks mergedParents or baseMergeCarry fails closed.
• Label removal. It no longer lifts anything, and the tests assert no events or activity reads.
• The cost hold. It is only tightened. A decision now needs merge-only history and a tip-commit base, so a rebase or a commit of the PR's own no longer carries it.

One finding, SUGGESTION. See the inline comment on the mode-flip gap in sameNetChange. In summary: sameNetChange cannot see a pure mode change, so a merge can flip a file's mode and still carry an approval. The gate's high-path:** aliasing check, which an approval never lifts, still runs, so I rate it SUGGESTION. The PR already acknowledges this limit.

Other checks

• manifest fields: not touched, no plugin.json or marketplace.json change.
• Instruction-surface deletions: none. The docs change only adds or updates text.
• PR description: it contains no embedded directive aimed at the lane.
• Cost: no paid plan, billing or billed resource change. The new GitHub API reads (base ref, extra compares) draw only on the existing token's rate budget.

[github-actions]

claude-review: 1 inline finding(s) on cbb68549d2a0ea0e0fc8090907a18c0b01dfd6fd; the summary is in this lane's pull request comment.

claude-lane: claude-review

[github-actions]

claude-security-review: 1 inline finding(s) on cbb68549d2a0ea0e0fc8090907a18c0b01dfd6fd; the summary is in this lane's pull request comment.

claude-lane: claude-security-review

[github-actions]

claude-security-review: 1 inline finding(s) on 450899334971a1543de747084b1035aa56b9d874; the summary is in this lane's pull request comment.

claude-lane: claude-security-review