[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]

The pr-verify verifier failed head ed21dfbbc006529e494a7c242db0e9fbb41382e3. What it wrote, as data:

Summary: The PR closes #9478 and #9493 but does not finish #9478. #9493 looks delivered: the JS and Python readers, plus ci-status, now count a closing-issue or Refs-issue cost decision only when it names `#N`. A decision covers the head of an unedited github-actions[bot] cost marker posted before it, or a later head whose only changes are base merges. Every failed read keeps the hold, and the JS/Python parity test covers the rule. ci-status's cost-hold lift now reads the one plain `Refs:` issue, which is #9478's first fix, and tests cover it. #9478 also asks to correct the stale comment at `.github/workflows/work-unblock.yml:38-39`, and the diff does not touch that file. Nothing touches the takeover set.
- `.github/workflows/work-unblock.yml:38-39`: #9478 asks to fix this comment, which still says report-outcome sets the Refs needs-human hold. Since #9405, report-outcome no longer sets it, and work-unblock only lifts holds that already exist. The diff leaves the file unchanged. Fix the comment here, or change `Closes #9478` to a `Refs: #9478` line that names this part as remaining work.
- `.github/scripts/pr-merge/merge-holds.mjs:costDecision`: #9493's done-when names other ways to bind a decision: newer than the last push or naming the head SHA, and an issue linked before the comment. This PR binds through the newest cost-hold marker's `sha=` plus a `#N` mention instead. The PR body ties that rule to the owner's Q17 (c) and Q18 (d) on #9436, which are outside the spec files. It still closes #9493's exploit path, so I don't treat it as blocking. A human should confirm that the base-merge carry matches what the owner intends.
- `.github/scripts/pr-merge/merge-holds.mjs:costMarker`: the decided head trusts any github-actions[bot] comment whose first line is a `claude-lane-hold` marker. Any workflow running with GITHUB_TOKEN can post as that account. That is a trust assumption worth a human look: if the lanes App can get a workflow to post a forged marker before the owner decides, it picks the decided head. The overall change is still much tighter than the old rule, which bound no head at all.
- `plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py:cost_marker`: `found` is annotated as a 3-tuple but holds a 4-tuple (made, sha, node, created_at). Fix the annotation so a type checker accepts it.
- `.github/workflows/pr-require-checks.yml`: the Node setup steps move ahead of the cost-hold step, and the deleted-comment read moves earlier. Two bash tests now expect the message 'covering this head' where they expected 'was deleted'. The step still fails closed, but the message for a deletion inside the window is now less specific.

[github-actions]

claude-review reviewed 2b219f4c7e1311626e409a7f693537e0b5838551: 0 finding(s), 0 as inline comments.

Incremental review of the three files changed since ed21dfb. I found nothing to flag. I read the code and tests and did not run any of them.

• babysit_merge_policy.py (deleted_since): The added isinstance(count, bool) check is correct. In Python True is an int, so filteredCount: True would have passed as a count of 1. JS Number.isInteger(true) is already false, so this brings Python in line with merge-holds.mjs. The sync test case "a deleted-comment count that is not a number" pins it.
• test_merge_holds_sync.py: The new case has filteredCount: True with one listed node. That is what separates the bool guard from the count > len(nodes) check, so the test fails without the fix.
• ci-hold-cost-decision.test.sh: The new case is consistent with the workflow in .github/workflows/pr-require-checks.yml.
  • The marker is at 13:17 and the deletion at 13:16, so deletedSince (made >= marker.at) does not block the carry.
  • The label was removed at 13:15, so the deletion falls after $since and the verdict step in pr-require-checks.yml returns "a comment was deleted since then". That matches the expected no no no "was deleted".
  • The earlier 13:09 case sits before $since, so it still carries. Together the two cases pin both sides of the window check.

The bool-guard line in babysit_merge_policy.py is long (about 125 characters). The repo has no Python line-length config (no match in pyproject.toml, ruff.toml or similar), so I did not flag it.

No hold applies. The change adds no model spend and touches no trust-gate files. Closing-issue criteria are unchanged by this increment.

[github-actions]

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

No security issues found in the incremental change since ed21dfb.

• babysit_merge_policy.py (deleted_since): the deleted-comment count check now also rejects a JSON boolean (isinstance(count, bool)). In Python bool is a subclass of int, so True used to pass as a count of 1. That let a malformed filteredCount slip through the "lists fewer deletions than it counts" fail-closed guard. The new check fails closed, which narrows the attack surface. It also matches the JS reader, where Number.isInteger(true) is false.
• test_merge_holds_sync.py: adds a case for a boolean filteredCount. This is test-only.
• ci-hold-cost-decision.test.sh: adds a case where a deletion falls inside the window but before the newest marker. It expects the label to stay held. This is test-only and pins the fail-closed behavior.

No manifest files, instruction surfaces or trust-gate paths changed in this range, so there are no manifest <field> lines.

critical-findings: 0