fix(pr-pipeline): carry an owner-lifted claude-review cost hold forward across base-only merges #9436

Activity

1. github-actions

   added

   needs-triageNot yet classified. Floor until a type and one priority tier are set.Not yet classified. Floor until a type and one priority tier are set.

   on Oct 23, 2026
2. github-actions commented on Oct 23, 2026

   

   github-actionsBot

   on Oct 23, 2026 – with GitHub Actions

   Contributor

   More actions

   Proposed work class: none

   Thanks for the report. Automated triage handed this issue to a maintainer.

   Unconfirmed model suggestion: work-class: scoped
   Basis: automated intake triage; a maintainer confirms or changes it.

   _{Automated triage. A maintainer reviews it.}
3. github-actions

   added

   needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.Human-in-the-loop required; autonomous sessions must not resolve items carrying this.

   and removed

   needs-triageNot yet classified. Floor until a type and one priority tier are set.Not yet classified. Floor until a type and one priority tier are set.

   on Oct 23, 2026
4. github-actions

   added the

   Bug

   issue type on Oct 23, 2026
5. melodic-automation-lanes

   added

   status: needs-decisionAwaiting a human or maintainer judgment call.Awaiting a human or maintainer judgment call.

   on Oct 23, 2026
6. kyle-sexton commented on Oct 24, 2026

   

   kyle-sexton

   on Oct 24, 2026

   MemberAuthor

   More actions

   Second instance, and the cause.

   #9509: the owner recorded the cost decision (issuecomment-6376016538, 22:30:12Z) and removed needs-human. The next head, 07fb37d, changed only cancel-in-progress and its docs, and claude-review held it again for cost at 23:07:32Z. The model's own hold text says: "The commit under review only changes cancel-in-progress and its docs, and adds no further cost."

   Cause: the hold step remembers nothing about lifts. In ci-workflows v0.65.0 (d02b8697), pr-review-cli.yml's "Apply the hold" step (.github/actions/apply-review-hold/hold.cjs) only adds the label and marker whenever the counted leg returns a hold. Its documented contract is "Add-only ... It never removes a label" (pr-review-cli.yml:2282-2285). Its inputs carry no prior hold marker, label removal or **Decision recorded.** Cost: comment. The model re-judges the whole PR diff on every head, so a schedule still in the diff holds again.

   Constraint for the fix: #9493 binds a cost decision to the head it approves. Carry a lift forward only when the cost-relevant diff is unchanged since the lifted head, as this issue's Fix says. Do not suppress cost outright once any decision exists. The fix lands in ci-workflows (apply-review-hold), then a re-pin here.
7. kyle-sexton

   closed this as completedin ci-workflows#875on Oct 24, 2026
8. kyle-sexton commented on Oct 24, 2026

   

   kyle-sexton

   on Oct 24, 2026

   MemberAuthor

   More actions

   Q17 (how a cost decision binds) is settled as option (b). That supersedes the head-binding constraint in #9436 (comment).

   How it was settled: on 2026-10-24, in the Automation Lanes session (c736de2c), the owner was given the open questions, Q17 among them, and answered verbatim: "least amount of friction, highest throughput (safely, within constraints) - go with whatever that is", and "only necessary things that would prevent an actual risk/attacker". Under that direction the session picked (b), the lower-friction option, which binding cost approvals to every head does not weaken against an attacker. The owner did not name (b) itself; it was chosen under the direction quoted above.

   The rule under (b):

   • A cost decision binds to a pull request and covers every head of it. A new head does not need a new decision.
   • The decision counts when it is posted by the owner on the pull request itself, or on the pull request's linked issue (its closing issue, else its one plain Refs: issue) with text that names the pull request as #N. A decision on an issue that does not name the pull request does not carry over to it; this closes the fix(pr-merge): bind a cost decision to the pull request head it approves #9493 path.
   • The author, App, edit and created-after-the-PR checks are unchanged.

   Readers: #9569 (merge gate, babysit and ci-status lift) and ci-workflows v0.65.2/v0.65.3 apply-review-hold (the review-lane hold step, melodic-software/ci-workflows#875).
9. kyle-sexton commented on Oct 24, 2026

   

   kyle-sexton

   on Oct 24, 2026

   MemberAuthor

   More actions

   The owner decided Q17 (how far a cost decision reaches) on 2026-10-24 at ~06:10Z, in the Automation Lanes session. Option (c) was put to him as the recommendation, and he answered verbatim: "go w/ recommended".

   This replaces the (b) record at #9436 (comment). The session picked (b) under his general direction, without his own choice.

   The rule under (c):

   • A cost decision covers later heads of the same pull request only while the cost-relevant paths are unchanged. These are the paths whose change makes a review lane hold for cost: new model-run triggers, dispatch caps, the model and effort settings of lane jobs. A later head that changes any of them needs a new decision.
   • A head that differs from the decided one only by a base merge, or by changes outside those paths, keeps the decision. This is what ends the re-hold after a base-only merge.
   • These rules from (b) still apply: a decision counts when an unedited owner comment carries it, posted on the pull request itself or on its linked issue (the closing issue, else the one plain Refs: issue) with text that names the pull request as #N. The author, App, edit and time checks are unchanged.
   • It stays within the owner's earlier constraint on this issue (fix(pr-pipeline): carry an owner-lifted claude-review cost hold forward across base-only merges #9436 (comment)): "Do not suppress cost outright once any decision exists".

   Readers to update: #9569 (merge gate, babysit, ci-status lift) and the ci-workflows apply-review-hold step that #9589 pins.
10. kyle-sexton commented on Oct 24, 2026

    

    kyle-sexton

    on Oct 24, 2026

    MemberAuthor

    More actions

    The owner decided Q18 on 2026-10-24 at about 06:2xZ, in the Lane Funding session. Q18 asked which change keeps a cost decision on a later head, which is how the Q17 (c) rule recorded at #9436 (comment) gets applied. He was shown (d) as the recommendation, with (b) and (a) as alternatives, and answered verbatim: "go w/ recomendations". That answer reached this session as Lane Funding's relay; I did not see it first-hand.

    Rule (d): a cost decision carries to a later head of the same pull request only when every change since the decided head is a merge of the base. Any other commit, including a change outside known cost files, needs a new decision. No list of cost files is involved, so no list can miss a file. Example: #9561's added spend lived entirely in scripts a workflow calls, where no path or line rule would have caught it.

    Shared mechanics, the same in both readers:

    • The decided head is the sha= in the latest bot-authored claude-lane-hold ... criteria=cost marker created before the decision comment.
    • Base-merge-only test: the pull request's own net change (its three-dot diff against the base) is identical at the decided head and at the current head. This holds across merges and rebases of the base, and it catches a hand edit made while resolving a conflict.
    • Any read failure, or a missing marker, means no carry, so the hold stays.

    Readers: #9569 (merge gate, babysit, ci-status lift) and ci-workflows v0.65.4 apply-review-hold (re-pinned by #9589).
11. kyle-sexton

    added a commit that references this issue on Oct 24, 2026

    fix(pr-merge): carry a cost decision only across base merges

    ...

    70d048b