Skip to content

fix(source-control): hold the timeout and model-leg shapes the cost probe still passed - #10451

Closed
melodic-automation-lanes[bot] wants to merge 8 commits into
mainfrom
work-resolve-issue/issue-9241-40458109523-1
Closed

melodic-automation-lanes[bot] wants to merge 8 commits into
mainfrom
work-resolve-issue/issue-9241-40458109523-1

Conversation

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor

Closes #9241

Summary

Opened by the work-resolve-issue lane. The run's own summary:

Builds the owner's recorded decision on #9241: the cost-review-required hold in babysit_risk.py now catches the held shapes. Timeouts raised, set to an expression, or removed are compared per job, so a raise beside a moved higher timeout now holds. Local reusable workflows and composite actions count as model legs, and model inputs and variables hold, as do model flags in action.yml and .github/scripts files. The open items from the 2026-10-22 comment are covered too: whole-context and folded secret references, and runner labels behind a matrix or input expression. Moves are accepted, and triggers, guards and file paths are left to the review lanes, as the pr-pipeline README now says.

Not verified

  • The Python unit tests (test_babysit_risk_cost.py, test_babysit_risk.py, test_babysit_merge_unattended.py) were not executed: this lane runs no interpreter and check-affected ran without --tests. They were only hand-traced by two review subagents, and the final round of fixes (after the second trace)
  • Residual gaps the reviews named, left open: step timeouts within one job share one path, so swapping two steps' values in a job nets to zero; a timeout dropped while its job is renamed; a timeout removed from a one-line top-level flow mapping; expression feeds beyond matrix, inputs and env (such as
  • CI-only gates check-affected does not run here (actionlint, gitleaks, line-citations, purged-em-dashes and the other whole-repo scanners it lists) are left to the PR's CI.

Fix

  • plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py: Model legs: _MODEL_CODE (action.yml and .github/scripts/**, with a rename-in hold), _MODEL_FLAG, model:/max-turns: keys in _COST_WORKFLOW_LINE, and local uses: ./ in _MODEL_ACTION and _MODEL_ACTION_NAME. Timeouts: _raised_timeouts re
  • plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_risk_cost.py: new ProbeShapes class with probe rows for each held timeout and model-leg shape, the 30-to-200 raise beside a removed and re-added 300, the secret shapes, runner expressions, the accepted in-file secret move, and nega
  • docs/conventions/pr-pipeline/README.md: the Cost hold bullet lists the new shapes. It names the accepted move (a secrets reference moved within one file) and says triggers, guards, matrix growth, workflow subdirectories, spend logic in code, other infrastructure formats and non-Claude model actions
  • .changes/source-control/fix-9241-cost-probe-shapes-3c9e1a7b.md: minor-bump changelog fragment for the source-control plugin.
  • .changes/pr-pipeline/fix-9241-cost-probe-shapes-8d24f6e0.md: minor-bump changelog fragment for the pr-pipeline convention.

Verification

The runner ran each check from the base copy against 18f0629:

  • scripts/check-changelog-fragments.sh --check: passed

The run reports:

  • /home/runner/work/claude-code-plugins/claude-code-plugins/.lane/checks/bin/check-changelog-fragments: All 9 changelog fragment(s) under .changes/ are valid.
  • /home/runner/work/claude-code-plugins/claude-code-plugins/.lane/checks/bin/check-affected: 11 passed, 6 skipped over 5 changed files (ruff over the changed .py files is among its checks; skips: shellcheck no shell, typos not installed, changed-skills and three validate-plugins steps that run the che
  • Fresh-context review subagent, round 1: hand-traced all test_babysit_risk_cost.py tests and the neighbouring cost tests as passing. It reported gaps: removal when the parent key is gone, comparison against the largest lost value, expression feeds, values below a key or behind an alias, secrets netti
  • Fresh-context review subagent, round 2 (on the revised code): hand-traced all 64 tests in test_babysit_risk_cost.py plus test_babysit_merge_unattended.py's test_a_raised_timeout_holds_only_on_a_model_workflow as passing and found no input that raises. Its super-linear paths (the _SECRET_REF whitespa
  • grep for babysit_risk.py:<line> citations across the repository: none, so no line citation was shifted.
  • The lane run: https://github.com/melodic-software/claude-code-plugins/actions/runs/40458109523

Left out: the items under Not verified.

Related

#9241

Lane-Activity: work-resolve-issue/resolve-issue

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

claude-lane: claude-security-review

@github-actions

github-actions Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

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

No security issues found in the incremental change (the changelog fragment, babysit_risk.py and test_babysit_risk_cost.py).

babysit_risk.py is a trust-control path: it holds merges for cost-review-required. The change touches only _net_added (plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py:975-990). That function now ignores comment text in the removed lines. A line starting with # is skipped, and a trailing \s#… is cut off before matching. A removed comment therefore no longer offsets a newly added secrets reference, model leg or model package run.

The edit only shrinks what is subtracted from the added matches. The net-gain count can only grow, so the hold fires on strictly more inputs and never fewer. The added-line side and the other checks are unchanged. A mis-split on a removed line, such as a # inside a quoted string, also errs toward holding. The _net_added call sites at lines 1268, 1272 and 1277 pass the same arguments as before. The new tests cover a removed whole-line comment and a removed trailing comment against an added secrets reference, and a removed commented uses: ./ against an added one. Each asserts the hold fires. The changelog fragment adds one descriptive line.

The first two items in the changelog fragment's "Not verified" list could not be confirmed here. The Python tests were not executed, and I did not run them either. This lane has no shell, so I read the diff only. The manifest check does not apply: no plugin.json or marketplace.json changed. The PR context text contained no embedded instructions.

critical-findings: 0

Spend: this change spends no credit and no billed resource.

@github-actions github-actions Bot added the needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. label Oct 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

claude-security-review holds this pull request for a person (critical-security) at 18f06290ff27debc16ebf7e6e729b6948669ba05.

babysit_risk.py is a merge-hold control. The new _SECRET_REF no longer matches a bare secrets before ) when the prefix is &&, || or similar (for example toJSON(a && secrets)), which the base pattern held.

claude-lane: claude-security-review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

claude-lane: claude-review

@github-actions

github-actions Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

claude-review reviewed 35a5f82c87189683188b02a055939c668628ad78: 0 finding(s), 0 as inline comments.

Incremental review of the change since 223ef48, covering babysit_risk.py, test_babysit_risk_cost.py and the changelog fragment. I found nothing to block or flag.

What changed. _net_added (plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py:975) now skips removed lines that start with #. It also cuts a removed line at its first whitespace-preceded # before matching. So a removed comment, whole-line or trailing, can no longer offset a newly added secrets reference, model leg or model package run.

Why it holds up. I read the function and its three call sites (_MODEL_ACTION, _MODEL_PACKAGE, _SECRET_REF). I did not run the tests. The change can only shrink what the removed side offsets. So its only possible failure mode is an extra hold, never a missed one. Two cases can do that:

  • A # inside a quoted string, preceded by whitespace, followed by the matched text.
  • A #-led line in a .mjs or .ts file under .github/scripts/.

Both are rare, and a person decides each hold.

Tests. The new cases cover a removed whole-line comment and a removed trailing comment against an added secrets.* reference, and a removed commented uses: ./... against an added live one. There is no test for the trailing-comment variant of the model leg or for the model package run. The changelog sentence names both, but the shared _net_added code path is already covered by the secrets cases, so I did not flag it.

Cost. This change spends nothing and touches no billing.

kyle-sexton and others added 2 commits October 27, 2026 04:21
…_REF

The widened pattern required `${{`, `(`, `,`, `!` or line start before a
bare `secrets`, so `toJSON(a && secrets)` and `(x || secrets)`, which the
base pattern held, no longer held. Restore the base `secrets` before `)`
alternative and accept `&&` and `||` operands, so the pattern matches every
shape the base did plus the new ones.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

claude-lane: claude-security-review

@github-actions

Copy link
Copy Markdown
Contributor

claude-security-review holds this pull request for a person (critical-security) at bbf77d4cdc147c4fa370ba322c529000c99f5a0c.

babysit_risk.py (a merge-hold trust control): the new line-level _TIMEOUT regex counts commented-out timeout-minutes lines, so in the patch-only fallback (file text unread) a removed comment can offset a real timeout raise on a model workflow and the cost-review-required hold does not fire. The old anchored regex did not allow this.

claude-lane: claude-security-review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

claude-lane: claude-review

… comments

The per-path timeout pass held a job renamed with its timeout unchanged
(the new path had no base counterpart) and a step deleted with its own
timeout (the job's shared step list got shorter). A new path whose values
a vanished job's same key held is now a rename, not a gain, and the step
count a job loses offsets the step timeouts it loses, so a step kept with
its timeout removed still holds.

_COST_WORKFLOW_LINE and the flag patterns now skip added `#` comment
lines, so a comment naming `model:` or `--model` does not hold; a real
`model:` key still does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

claude-lane: claude-review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

claude-lane: claude-security-review

@github-actions

Copy link
Copy Markdown
Contributor

claude-security-review holds this pull request for a person (critical-security) at f436ef9eacc24c728fafc13f1c4e79a5e546faf2.

babysit_risk.py is a merge-hold control. The new step-count discount in _raised_timeouts (lines 1137-1139) lets a timeout removal (default 360 min) go unheld when an unrelated step is deleted in the same job, which narrows the cost hold.

claude-lane: claude-security-review

@kyle-sexton

Copy link
Copy Markdown
Member

Triage of the claude-security-review holds on this PR (read-only verification at head f436ef9). None is a real HIGH or CRITICAL, but two are real gaps worth fixing here before merge:

  • F1 (MEDIUM, the current hold): the step-count discount in _raised_timeouts lets a removed step timeout pass when an unrelated step in the same job is also deleted. Reproduced at head: the bypass case returns [], the control returns timeout ... removed (babysit_risk.py:1137-1139). Fix: key each step timeout by its step index (or name/id) in _timeouts instead of a shared steps path, drop the step-count subtraction at :1138-1139, and add a test where the step with the timeout stays and a different step is deleted; it must hold.
  • F3 (LOW): in the patch-only fallback, a patch that adds a commented # timeout-minutes line offsets a real timeout raise (reproduced). Fix: in _raised_timeouts :1112-1115, skip lines whose lstrip() starts with # and strip # tails before _TIMEOUT.findall.
  • F2 (_SECRET_REF with &&/||): already fixed at head (:885-886).

kyle-sexton pushed a commit that referenced this pull request Oct 27, 2026
… with positives

A weakened merge hold now has to show a security impact like any other
finding, and a bypassed gate counts only when it guards one, so a
narrowed cost hold (the #10451 shape) is no longer CRITICAL. Code a
manifest change starts running for users with no opt-in still meets the
bar by itself, which resolves a conflict the default-on case exposed.

Adds a #10451-shape negative and four positives for the rewritten rules:
a broker effect widened to write, a dropped CHANGES_REQUESTED veto, a
default-on marketplace entry, and pull_request_target running fork code
with a write token.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kyle-sexton
kyle-sexton previously approved these changes Oct 27, 2026

@kyle-sexton kyle-sexton left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Owner approval of head f436ef9.

@kyle-sexton kyle-sexton removed the needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. label Oct 27, 2026
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

pr-unblock / address-feedback finished round 1 of max 4 on baa574d.

@melodic-automation-lanes melodic-automation-lanes Bot added the needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. label Oct 27, 2026
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

The pr-unblock lane stopped on this pull request and needs a person: a review finding is uncertain or deferred, so a person decides it.

  • Reason: feedback-uncertain
  • Resume key: baa574d91b8895a6dae73cc6bcf394eb0eecccd2 (the head the lane stopped on)

To resume, deal with the reason above, remove the needs-human label (and do-not-merge, if set), then dispatch the pr-unblock workflow with pr-number=10451 and activity=address-feedback:

gh workflow run pr-unblock.yml -R melodic-software/claude-code-plugins -f pr-number=10451 -f activity=address-feedback

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

Unattended merge: native auto-merge leaves this pull request unarmed at head baa574d91b8895a6dae73cc6bcf394eb0eecccd2. A person must clear this reason, so the pr-merge lane added the needs-human label: fix the reason, then remove the label, and the next scheduled run judges it again.

it changes plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py, a hold path

kyle-sexton
kyle-sexton previously approved these changes Oct 27, 2026

@kyle-sexton kyle-sexton left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Owner approval of head baa574d: Q33; head after the base merge.

@kyle-sexton kyle-sexton removed the needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. label Oct 27, 2026
kyle-sexton added a commit that referenced this pull request Oct 27, 2026
… with positives

A weakened merge hold now has to show a security impact like any other
finding, and a bypassed gate counts only when it guards one, so a
narrowed cost hold (the #10451 shape) is no longer CRITICAL. Code a
manifest change starts running for users with no opt-in still meets the
bar by itself, which resolves a conflict the default-on case exposed.

Adds a #10451-shape negative and four positives for the rewritten rules:
a broker effect widened to write, a dropped CHANGES_REQUESTED veto, a
default-on marketplace entry, and pull_request_target running fork code
with a write token.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

Unattended merge: native auto-merge leaves this pull request unarmed at head baa574d91b8895a6dae73cc6bcf394eb0eecccd2. A person must clear this reason, so the pr-merge lane added the needs-human label: fix the reason, then remove the label, and the next scheduled run judges it again.

pr-merge's gate holds it on its risk score (security-finding: 2 open)

@melodic-automation-lanes melodic-automation-lanes Bot added the needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. label Oct 27, 2026
@kyle-sexton kyle-sexton added the do-not-merge Hard merge gate: do not merge while applied. label Oct 27, 2026
kyle-sexton and others added 2 commits October 27, 2026 15:11
… found

A deleted step's timeout no longer serves as the floor for raising
another step, the unread-text fallback counts only key-position
timeouts, and a hash-led script line naming a model holds. Each case has
a test that fails without the fix.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

The pr-merge lane escalated this pull request, and its head has moved (head-moved): its escalation names head baa574d91b8895a6dae73cc6bcf394eb0eecccd2, and the head is now 223ef483f615e9d699e507c498eab0e821376356. pr-merge removed needs-human so it judges 223ef483f615e9d699e507c498eab0e821376356 afresh, and escalates again if its refusal still holds. To keep a person on it, add needs-human back: a label a person adds is never removed.

@melodic-automation-lanes melodic-automation-lanes Bot removed the needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. label Oct 27, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

claude-lane: claude-security-review

…r model reference

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

claude-lane: claude-review

@kyle-sexton kyle-sexton removed the do-not-merge Hard merge gate: do not merge while applied. label Oct 27, 2026
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

Unattended merge: native auto-merge leaves this pull request unarmed at head 35a5f82c87189683188b02a055939c668628ad78. A person must clear this reason, so the pr-merge lane added the needs-human label: fix the reason, then remove the label, and the next scheduled run judges it again.

it changes plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py, a hold path

@melodic-automation-lanes melodic-automation-lanes Bot added the needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. label Oct 27, 2026

@kyle-sexton kyle-sexton left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Owner approval of head 35a5f82.

@kyle-sexton kyle-sexton removed the needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. label Oct 27, 2026
@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 27, 2026
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

Unattended merge: native auto-merge leaves this pull request unarmed at head 35a5f82c87189683188b02a055939c668628ad78. A person must clear this reason, so the pr-merge lane added the needs-human label: fix the reason, then remove the label, and the next scheduled run judges it again.

pr-merge's gate holds it on its risk score (security-finding: 1 open)

@melodic-automation-lanes melodic-automation-lanes Bot added the needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. label Oct 27, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 27, 2026
@kyle-sexton

Copy link
Copy Markdown
Member

Superseded under the owner's standing direction §3 (model usage is not cost; public-repo standard-runner minutes are free per https://docs.github.com/en/billing/concepts/product-billing/github-actions). Real-charge timeout holds on non-public repos are kept by the cost-hold PR (branch fix/cost-hold-real-charges-only).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pr-merge cost hold: LOW-rated shapes S&C's probe still passes

1 participant