[melodic-automation-lanes]

Closes #8638

Summary

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

babysit_review_evidence._open_findings now keys its per-head finding counts on each lane run's head_commit.tree_id rather than the head sha. An empty commit (new sha, same tree) followed by a clean whole review therefore leaves a finding open, and no longer counts as a fresh head for whole_review_clears; a reviewed run with no readable tree unbinds the lane (tree-unread). The review pass also found that a re-rolled review's critical-findings: 0 line could lower the security lane's blocking count. That line now applies only when no other review of the same tree reported a summary finding.

Not verified

• The Python unit tests (test_babysit_review_evidence.py, test_babysit_merge_unattended.py) were not run. This lane's Bash runs only the named wrappers, and check-affected runs without --tests, so no interpreter could run them. That the new tests fail before the fix and pass after was traced by readin
• Review finding not fixed: _carried_blocking reads trees for every recorded run of the security lane. One run whose head_commit is null, however old, makes the carried security blocking count None, so an unbound security lane carries no HIGH security-finding. This matches the existing fail-closed pat

Fix

• plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py: new _tree() reads a run's head_commit.tree_id (unreadable raises Unbound('tree-unread')). In _open_findings, head_max, summary_max, the per-window list for thread-less inline findings and the whole-review 'spent' test are
• plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_review_evidence.py: the run() fixture carries head_commit with a per-sha tree (tree_of) and takes tree=. Added tree parameters to two_reviews and incremental_then_whole, and the probe-fixture bind adds head_commit to each run. Th
• plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_unattended.py: LaneEvidenceReadTokenOutage's hand-built run listing now carries head_commit.tree_id, as a real listing does, so its bound-result assertion still holds.
• .changes/source-control/fix-8638-finding-closure-tree-5c1e9a07.md: new patch fragment (Fixed) for both changes. Written by hand in the script's format because scripts/new-changelog-fragment.sh refuses a detached HEAD ('HEAD is not on a branch').

Verification

The runner ran each check from the base copy against 7cfe8ba:

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

The run reports:

• read https: //docs.github.com/en/rest/actions/workflow-runs?apiVersion=2022-11-28＃list-workflow-runs-for-a-workflow: the workflow-run schema (listing and get-a-run) has a required head_commit that is null or a Simple Commit, whose tree_id is a required string. head_commit is therefore nullable, whic
• read plugins/source-control/skills/babysit-prs/scripts/babysit_gh.py collect_lane_runs: it appends {**item, 'attempts': ...} from the workflow-runs listing, so head_commit reaches bind() with no caller change.
• .lane/checks/bin/check-changelog-fragments: All 7 changelog fragment(s) under .changes/ are valid.
• .lane/checks/bin/check-affected: 11 passed, 6 skipped over 4 changed files (the skips are shellcheck with no changed shell, three validate-plugins sub-checks and changed-skills that run checked-tree code, and typos, which is not installed).
• Fresh-context review subagent over the whole diff: it confirmed the re-keying and found no breaking bug. Tracing by hand, it found that each new tree test fails on the pre-change code (gives 0 open findings) and passes after it, and that no existing bind() test breaks. It raised a pre-existing gap w
• The lane run: https://github.com/melodic-software/claude-code-plugins/actions/runs/40441446617

Left out: the items under Not verified.

Related

#8638

[github-actions]

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

No security issues found in this pull request.

critical-findings: 0

Scope. babysit_review_evidence.py is on the trusted-path list and feeds the merge-hold evidence, so I read its whole change and what it gates. No plugin.json or marketplace.json is touched, so no manifest <field> lines apply. The other three files are a changelog fragment and two test files.

Trust-control judgment (from the code alone). The change makes finding closure stricter. It does not weaken or narrow a merge hold.

• Finding counts are keyed by tree. head_max, summary_max, the thread-less inline missing count and the spent set now use head_commit.tree_id instead of the head sha. An empty commit therefore no longer counts as a fresh head, so a clean whole review of it cannot close a finding or satisfy whole_review_clears. spent now covers any review of the tree, so whole_review_clears goes false more often.
• Unreadable evidence unbinds the lane. _tree() raises Unbound("tree-unread") when head_commit or tree_id is null, missing, or not a valid sha. A sha that maps to two different trees raises Unbound("evidence-error"). Both fail closed. _carried_blocking turns the exception into None, which is the existing fail-closed behaviour: an unbound lane carries no count.
• The critical-findings: line is read less often. It now lowers the blocking count only when summary_reviews[tree] <= 1. A re-run or empty-commit review that prints critical-findings: 0 can no longer clear an earlier review's CRITICAL summary findings. This is a narrowing.
• Input source. tree_id comes from GitHub's workflow-run listing, not from PR text or a commit message. A PR author can't choose it independently of the real tree.

Edge case, not a finding. chain_trees deduplicates chain links that share a tree. If a later commit reverts to an already-reviewed tree, those links now count once (the max) instead of summing. The same code was reviewed both times, and count > 0 still blocks. I found no path where this reaches zero.

Spend. None: the diff adds no billed resources or paid settings.

[github-actions]

claude-review reviewed 7cfe8babe15bd370bc7b5f300a7b2bf1fa87cb85: 0 finding(s), 0 as inline comments.

Code review: no findings

I found nothing to block or flag. I read the diff, the PR context and the surrounding babysit_review_evidence.py (_bind_lane, _carried_blocking, _open_findings). I did not run the Python tests, because I have no shell here. The PR's own account that the new tests fail before the fix and pass after is the author's claim, and I did not confirm it. I traced the new cases by hand against the code and they hold.

Closing issue #8638, acceptance criteria

• A head counts as new only when its tree differs from the tree the finding was raised on. Met. _open_findings now keys head_max, summary_max, spent, the per-window thread-less count and cleared on head_commit.tree_id (babysit_review_evidence.py:792-828). Chain links that share a tree count once (chain_trees).
• Stay-loud test: an empty commit followed by a clean whole review leaves the finding open. Met. Three tests cover an incremental finding, a summary finding and a whole review's findings. Each puts the later run on the earlier run's tree, so head_max[tree] stays non-zero and cleared is False.
• A changed tree with a clean whole review still closes the finding. Met by test_a_clean_whole_review_of_a_changed_tree_clears_the_finding.
• A source-control changelog fragment. Present, bump: patch, and the PR reports check-changelog-fragments passed. The author notes it was written by hand because the script refuses a detached HEAD.

Other checks

• The summary_reviews <= 1 guard keeps a re-rolled review's critical-findings: 0 line from lowering the security lane's blocking count. It only makes the count more conservative, which matches the lane's fail-closed pattern. test_a_rerolled_review_of_the_same_tree_never_lowers_the_count covers both the same-sha and empty-commit cases.
• The other callers of bind() that I read build their runs with ev_tests.run, which now carries head_commit. LaneEvidenceReadTokenOutage was updated by hand. Callers whose runs are still in progress or failed never reach _tree.
• babysit_gh.collect_lane_runs copies each listed run whole ({**item, "attempts": ...}), so head_commit reaches bind() with no caller change.

Disclosed risk, not a finding

• _tree runs for every completed, reviewed lane run, not just the runs on the chain. One old run with a null head_commit therefore unbinds the lane with tree-unread. For the security lane it also leaves the carried blocking count at None. The PR body discloses this and it fails closed, so I am not counting it. If GitHub leaves head_commit null on real pull_request runs, the effect would be a stuck merge rather than a silent pass.

Spend: none. The change adds no billed resource, and hold is not set.

[melodic-automation-lanes]

Unattended merge: native auto-merge leaves this pull request unarmed at head 7cfe8babe15bd370bc7b5f300a7b2bf1fa87cb85. 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_review_evidence.py, a hold path

[kyle-sexton]

Owner approval of head 7cfe8ba: hold path babysit_review_evidence.py; security review clean.

[melodic-automation-lanes]

The unattended merge path armed native auto-merge at head 7cfe8babe15bd370bc7b5f300a7b2bf1fa87cb85.

[github-actions]

Post-merge mutation testing

• Range: 8535f1f..7366a58
• Merged pull request: #10414
• Outcome: refused

Killed 0, survived 0, timeout 0, no coverage 0, invalid 0, not run 8.
Covered-code score n/a; mutation score n/a.

File	Killed	Survived	Timeout	No coverage	Invalid	Not run
plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py	0	0	0	0	0	8

The run's summary has the full report.