[kyle-sexton]

Closes #9493
Closes #9478

Summary

This PR applies the owner's cost decisions Q17 (c) and Q18 (d) on #9436. It reads them the same way ci-workflows v0.65.4's apply-review-hold step does (ci-workflows#877, 31b44f3f).

• Which comments count. An owner **Decision recorded.** Cost: comment counts when the body links it and it sits either on PR N itself or on PR N's linked issue (the closing issue, else the one plain Refs: issue). An issue comment must also name #N and be newer than the PR. The decision is the newest qualifying comment on the PR, else the newest on the issue.
• What a decision covers. It covers the head it decided. It covers a later head only when every change since then is a merge of the base. Any other commit needs a new decision.
• The issues closed. A decision that does not name PR N no longer lifts PR N's hold, which closes fix(pr-merge): bind a cost decision to the pull request head it approves #9493. ci-status's lift now reads the Refs issue too (fix(pr-pipeline): let ci-status's cost-hold lift read a Refs issue #9478).

Fix

• Decided head. This is the sha= in the newest comment github-actions[bot] posted on PR N before the decision whose first line is <!-- claude-lane-hold lane=<lane> sha=<40hex> criteria=... --> with cost among the criteria (costMarker / cost_marker). That marker counts only if it was never edited: its GraphQL lastEditedAt is null, read the same way as the decision's edit check. Otherwise anyone with write access could edit the comment and move its sha= to the current head. If the marker is missing, edited, or its edit state cannot be read, nothing carries, even at the same head, and no older marker is used instead.
• Deleted comments. Nothing carries when any comment on the PR was deleted at or after the chosen marker was posted (deletedSince / deleted_since, a COMMENT_DELETED_EVENT timeline read), or when that read fails or lists fewer deletions than it counts. The check runs before the same-head shortcut. Without it, deleting the newest marker would make an older marker's head the decided head.
• Carry test.
  • At the decided head, the decision carries with no compare read.
  • Otherwise the readers compare <base branch>...<decided head> and <base branch>...<head> (three-dot). The decision carries only when both list the same filenames, each with the same status, previous_filename and patch (sameNetChange / same_net_change).
  • Nothing carries on 300 or more files, a file with no patch (so a pure rename never carries), a filename listed twice, an unusable base branch name, or any failed read.
• The three readers.
  • babysit_merge_policy.cost_decision implements the rule in Python.
  • merge-holds.mjs costDecision implements it in JS. It reads baseRefName in its pull query, and costDecision now takes the head SHA (enable-auto-merge.mjs passes headRefOid).
  • ci-status's cost-hold step in pr-require-checks.yml does its own reads through read_api. It runs the base commit's merge-holds.mjs costMarker and sameNetChange over them, as it already ran refsIssue, so the JS logic exists once.
• Live head. ci-status's cost-hold step keeps the hold when the PR's live head is not the event's head, so a late or re-run event cannot lift the label off a newer head.
• Unchanged. The author, App, edit and time checks, the #N pattern, the Refs-target checks, and fail-closed handling on every read.
• Bootstrap. ci-status runs the base commit's module. Until this merges, the base merge-holds.mjs has no costMarker, so on this PR the step cannot carry anything and the hold stays.

Agreement with the review lanes' hold step

• Same rules. Marker author and shape, the never-edited marker check (Lane Funding is adding it to hold.cjs), the cutoff before the decision, a tie between two markers going to the first listed, the same-head short-circuit, the three-dot compare against the base branch name with the same name check, the compare fields, and every no-carry case.
• Still different here.
  • These readers need the decision to be linked from the body. The hold step reads every comment.
  • The hold step counts only its own lane's markers. These readers take any lane's.
  • Python counts an admin as the owner, as before. The others use the trusted list.

Verification

• node --test .github/scripts/pr-merge/*.test.mjs .github/scripts/pr-verify/resolve-pr.test.mjs: 368 pass.
• Python, full babysit suite (python3 -m unittest discover in babysit-prs/scripts/tests): 2840 tests OK.
  • test_babysit_merge_cost.py has 25 tests.
  • test_merge_holds_sync.py checks that JS and Python give the same answer on every carry case.
• Bash: scripts/ci-hold-cost-decision.test.sh 139 pass; scripts/ci-lanes-pr-hold.test.sh 128 pass.
• New cases:
  • a base merge carries, and a commit of the PR's own re-holds;
  • a rebase with the same net change carries in any file order, and one that drops a file re-holds;
  • a conflict resolution, rename or status change re-holds;
  • 300 files, a patchless file or pure rename, a duplicate filename, unusable base names and failed reads do not carry, and 299 files and a release/2.x base do carry;
  • with no marker nothing carries, even at the same head;
  • at the decided head there is no compare read;
  • the decided head is the newest qualifying marker before the decision, and markers that come after the decision, are written by a person or another bot, are not on the first line, or lack cost are ignored;
  • the newest decision on the PR is chosen over the issue's, and only it may carry;
  • an edited marker, or one whose edit state is null, has no lastEditedAt key or cannot be read, carries nothing, both at its own head and after a base merge, and an older unedited marker does not stand in. The JS/Python sync test covers the same cases.
• Against the previous head 6c3b6ae: with the new tests overlaid on the old code, JS fails 11, Python 15 and bash 23. The two pure-function JS tests fail only because the symbols are missing; the rest fail on behavior.
• Marker edit check, against the previous head 7276090: the new tests fail there in JS (4), Python (6 subtests), the sync test (1) and bash (6).
• nice bash scripts/check-affected.sh --uncommitted: 12 passed, 4 skipped.
• Deleted-comment and live-head checks: new JS, Python, sync and bash cases (a deletion after the marker blocks at its own head and after a base merge, an earlier one does not, an unreadable or overflowing list blocks, a moved live head keeps the hold). Against the code before them, the bash test fails 5 and the sync test fails on behavior.

Related

• fix(pr-pipeline): carry an owner-lifted claude-review cost hold forward across base-only merges #9436, the cost-decision rule.
• fix(pr-pipeline): hold native lane PRs on marketplace and cost paths #9488, whose security review found the gap.
• feat(source-control): read a Refs: issue as the linked issue for pr-merge eligibility #9405, after which a Refs: PR is merge-eligible.

🤖 Generated with Claude Code

[kyle-sexton]

Held with do-not-merge until the owner decides Q17 in his own words. Option (b) loosens a cost hold, and the owner did not pick (b) himself (#9436 (comment)). His written constraint on #9436 (#9436 (comment)) is 'Do not suppress cost outright once any decision exists'. AGENTS.md asks first when a change alters what agents may do unattended and the owner has not decided it. The #N alignment and the Refs-target check in this PR are tightenings and can ship separately if the owner picks another option.

[kyle-sexton]

Hold lifted. The owner decided Q17 (c), #9436 (comment), and Q18 (d), #9436 (comment). This head (1ef5787) implements both: a cost decision carries only when the change since the decided head is base merges only, the decided head comes from an unedited github-actions[bot] cost marker, and any read failure keeps the hold. Next: the ready flow, then a trust-gate review.

[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-24T07:52:32.105826Z	ed21dfb	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.

[github-actions]

claude-review reviewed 149f3214a61e68fcc719d3946378f769aa98e8d0: 0 finding(s), 0 as inline comments.

No findings. I read the full diff (11 files) and the PR context. I did not run any tests.

Checked by reading code (no execution):

• The three readers apply the same rules: costDecision in merge-holds.mjs, cost_decision in babysit_merge_policy.py, and the cost-hold step in pr-require-checks.yml.
  • Marker regex and cutoff: all three require the marker on the first line, authored by github-actions[bot], with cost among the criteria and created before the decision. Of two markers at the same time, the first listed wins.
  • Order of checks: the edit check on the marker, then the deleted-comment check, then the same-head shortcut, then the compare.
  • Compare and decision choice: all three use the same branch-name guard, the same 300-file and no-patch fail-closed cases, and the same choice of the newest decision on the pull request, else the newest on the issue.
• enable-auto-merge.mjs can pass pull.headRefOid because PULLS_QUERY already selects it (line 148).
• merge-holds.mjs imports only node: builtins, so running it from the one-file sparse base checkout works.
• refsIssue already exists at the base commit, so the new workflow code can call it.
• When the base merge-holds.mjs has no costMarker yet, holds_js exits nonzero and carries returns 1, so the hold stays. This is the bootstrap behaviour the PR describes.
• The new "head moved since this event" check in the workflow compares .head.sha from the REST pull request with HEAD_SHA, which the step already sets.

Closing issues:

• fix(pr-pipeline): let ci-status's cost-hold lift read a Refs issue #9478: the cost-hold lift now reads the Refs: issue and checks it is an issue rather than a pull request. There is a bash test, and the work-unblock.yml comment is corrected.
• fix(pr-merge): bind a cost decision to the pull request head it approves #9493: the PR binds a decision to the pull request's head through the marker and base-merge rule that owner decisions Q17(c) and Q18(d) on fix(pr-pipeline): carry an owner-lifted claude-review cost hold forward across base-only merges #9436 set. It does not use the "newer than the last push, or names the head SHA" wording in the issue text. Q17/Q18 are later owner decisions that the PR cites, so I did not count the difference as a finding.

Manifests: the diff changes no plugin.json or marketplace.json, so no manifest items apply. The changelog fragment is bump: patch for a fix, which matches the change.

Not independently verified: the test counts and the "fails against old code" results in the PR description are the author's claims. I did not run them.

Hold: none. The change adds no model spend, no trusted-actor or CODEOWNERS changes, and no CRITICAL security finding.

[github-actions]

claude-security-review reviewed 149f3214a61e68fcc719d3946378f769aa98e8d0: 0 finding(s), 0 as inline comments.

No security findings in #9569 at head 149f321.

critical-findings: 0

I read the full diff, covering the JS reader (merge-holds.mjs, enable-auto-merge.mjs), the Python reader (babysit_merge_policy.py), the ci-status cost-hold step in pr-require-checks.yml, the tests and the changelog entry. I also read the PR context.

What I checked:

• Decision binding. A decision on the linked issue now has to name #<pr>, come after the PR was opened, and sit on the closing issue or the one Refs: issue. The Refs: target is read and rejected if it is a pull request. The #N pattern is identical in the JS, Python and jq readers. Author, App, edit and trusted-actor checks are unchanged.
• Head binding.
  • The decided head comes from the newest github-actions[bot] cost marker before the decision. A bot-only login check means a person cannot forge one.
  • The marker must be unedited, and no comment on the PR may have been deleted since it was posted. That check runs before the same-head shortcut, so deleting a newer marker cannot promote an older head.
  • A later head carries only if sameNetChange holds for the base...decided and base...head compares. It fails closed on 300 or more files, a missing patch, a duplicate filename, a bad branch name, or a failed read.
  • The hold stays if the marker is missing, edited, or unreadable.
• ci-status step.
  • It now runs merge-holds.mjs with node, taken from the base-commit sparse checkout (base-config/, pinned to pull_request.base.sha), not from PR code.
  • The module imports only node: built-ins, so the single sparse-checkout file is enough.
  • The function name passed to holds_js is a fixed literal. The PR body goes in through a temp file, not shell interpolation.
  • If the checkout fails, BASE_CONFIG is empty and the step keeps the hold.
  • The new live-head check (.head.sha must equal HEAD_SHA) stops a late or re-run event from lifting the label off a newer head.
• Instruction surface. No standing instruction, hook allowlist or deny rule is deleted or softened. The only prose changes are comment updates in work-unblock.yml and the workflow comments.
• Manifests. No plugin.json or marketplace.json is changed, so there are no manifest <field> lines.
• Trust-gate paths. The diff touches no trusted-actors, check-trusted-trigger, CODEOWNERS or .gitmodules file, so there is no hold.
• Actions hardening. The new checkout and setup-node steps use SHA-pinned actions and persist-credentials: false. Anything further on workflow hardening is left to zizmor.

I did not run the code or the tests; this review is by reading the diff.