[kyle-sexton]

No related issue: stale security count holding #10817

Summary

The merge gate's security count held #10817 on one IMPORTANT summary finding that only the PR's first whole review reported. That review's own summary said critical-findings: 0, and two later incremental reviews found nothing. babysit_review_evidence._open_findings read only the covering review's critical-findings: line, so any summary finding in an earlier chain link counted as blocking, and only a clean whole review could ever clear it. This contradicts babysit_risk.py, where only CRITICAL security findings hold.

There was a second cause. The lane edits one reviewed-head marker comment in place per head (ci-workflows publish.cjs writeMarker), so an earlier review's summary survives only in that comment's edit history.

babysit_review_evidence.py and babysit_merge.py are trust paths, so a person merges this PR.

Fix

• babysit_merge._attach_edits adds userContentEdits to the GraphQL edits query it already sends. No new API call. Each edit node's diff holds the full body after that edit.
• A new function, babysit_review_evidence._marker_revisions, lists every body the lane's marker held and when it was written. An unedited marker gives one body, at its creation time.
• A marker yields nothing when any of these is true:
  • its history is missing or truncated (totalCount differs from the node count);
  • a revision was deleted or has no diff;
  • a revision was written by anyone but the marker's author (the same Bot and login test _edited_by_another uses).
• In _open_findings, an earlier chain tree's summary findings count as min(line, reported) only when exactly one review of that tree reported summary findings, and exactly one critical-findings: line appears across two sources:
  • marker revisions that name that review's head and were written inside its job window;
  • summary comments posted inside that window.
• Otherwise every finding still counts, so a summary that can't be read never counts as non-blocking. The covering review's existing rule is unchanged, and the history is never mixed into it.
• _carried_blocking passes the same input, so carried counts follow the same rule.

Option chosen: recover each count from the marker's edit history. It needs no change in ci-workflows and no extra API call. The completion record and the job output carry no critical count, so the edit history is the only place the earlier count exists today. A durable per-review record written by the lane would also work, but it would mean changing the producer.

Verification

• New tests in SeverityTests. A whole review with one summary finding is followed by a clean incremental review:
  • earlier critical-findings: 0 gives 0 blocking;
  • critical-findings: 1 gives 1;
  • an over-count is capped at the findings reported;
  • no line, or two lines, gives 1;
  • each kind of unreadable history gives 1: absent, truncated, deleted revision, null diff, another editor, revision outside the job window;
  • a revision that names another head is not read;
  • the covering review's own line still reads when its marker carries history.
• The new critical-findings: 0 case fails on origin/main and passes with the fix.
• python3 -m unittest discover in plugins/source-control/skills/babysit-prs/scripts: 3425 tests OK (5 skipped).
• python3 -I scripts/sync-hold-floor.py --check: exit 0.
• scripts/run-ruff.sh check passes on the changed files.
• Checked against live data for feat(user-interface): offer to install a better design route on the user's yes #10817 (read-only):
  • the marker comment's edit history has 3 revisions, all written by github-actions (Bot);
  • the 4d1b8d0 revision has exactly one anchored critical-findings: 0 line;
  • that revision was written at 01:13:53Z, inside the security-review job window of run 40798989398 (01:13:49Z to 01:13:55Z).

Related

• Held PR: #10817
• Marker writer: melodic-software/ci-workflows .github/actions/publish-review/publish.cjs (writeMarker, countedSummary)

🤖 Generated with Claude Code

[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-29T03:15:18.927493Z	d953b99	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 d953b99a5f246d8ed1c7468986a402a64c3eb19f: 0 finding(s), 0 as inline comments.

Code review: PR #10850

Result: no findings. I read the whole diff and the surrounding code in babysit_review_evidence.py (_open_findings, _edited_by_another, _summary_comments, the marker and severity regexes) and babysit_merge.py (_attach_edits). I ran no tests or commands. The test counts, ruff result and live-data check for #10817 in the PR body are the author's claims, and I have not confirmed them.

What I checked

• Which earlier links get a recovered count. The new loop in _open_findings runs only over chain_trees[1:], so it never touches the covering review's tree or its existing <= 1 rule. Revisions are used only in that loop, which matches the description's "never mixed into the covering review".
• The size of the reduction. It applies only when exactly one review of the tree reported summary findings. In that case summary_max[tree] equals reported, so summary -= reported - min(line, reported) takes off only the over-count and can't go below the line's own count.
• Unreadable history.
  • A userContentEdits of null, a totalCount that differs from the node count, a deleted or diff-less revision, or a non-lane editor makes _marker_revisions yield nothing for that comment.
  • With nothing to read, the loop finds no line, so every finding still counts.
  • Zero or two or more critical-findings: lines also fall back to counting everything.
• Revisions from another head. The loop reads only revisions whose first line names the reporting review's own head sha and lane, written inside its job window.
• Who wrote a revision. Each revision's editor goes through the same _edited_by_another test as the live comment. So a human edit of the marker's history can't lower the count.
• _carried_blocking. It receives the same input, so carried counts follow the same rule.
• _attach_edits. The new history key is None when GraphQL returns no userContentEdits, and _marker_revisions treats that as unreadable. A failed GraphQL read still returns early, as before, so no edit record is attached and bind() holds those comments unproven.
• Tests. They cover the cases the PR lists, and the two-line, truncated, deleted, null-diff, other-editor, out-of-window and other-head cases all assert a count of 1. The _attach_edits unit test was updated for the new history key.
• Changelog fragment. bump: patch fits a bug fix to the merge gate's count.

Residual notes (not findings)

• The posted summary comments for an earlier link are filtered by job window only, not by head sha. The covering-review rule has the same property. A second same-lane summary comment inside the window would make two lines, which fails closed, so I judge this acceptable.
• The change relies on userContentEdits.nodes[].diff holding the full body after each edit and on the history including the original body. The author reports checking this against feat(user-interface): offer to install a better design route on the user's yes #10817's live marker, and I could not check it myself.
• babysit_review_evidence.py and babysit_merge.py are trusted paths. I read their whole changes and what they gate. The change only ever lowers a blocking count, and only from a lane-authored, bot-edited, in-window, single-line record, so it fails closed. The PR body says a person merges it.
• Cost: no billing change. The only extra spend is a larger payload on a GraphQL query the gate already sends.

[github-actions]

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

No security findings.

critical-findings: 0

Scope read: the whole diff, plus _open_findings, _summary_comments, _edited_by_another, the marker and severity regexes, and _attach_edits in the head files. Two changed files are trusted paths: babysit_review_evidence.py and babysit_merge.py. No plugin manifests changed (no manifest <field> lines). The .changes entry is inert. PR text was read as data and contained no embedded instructions.

Trust control: weakened merge hold. The change lets an earlier review's summary findings count as min(line, reported) instead of in full. I checked it as a narrowing of a merge hold and found no path that bypasses it:

• Who reaches it: a PR author can only influence what the lane writes about their own PR, and the covering review already trusts its own critical-findings: line the same way (babysit_review_evidence.py:1378-1390). The change adds no new trust in the author.
• Marker revisions are lane-only:
  • _marker_revisions (babysit_review_evidence.py:1108-1137) requires the REST login github-actions[bot] and a lane= match.
  • It runs _edited_by_another on every revision, so any non-Bot or different-login editor drops the whole marker.
  • A deleted revision, a null diff, an absent or unreadable history, or totalCount != len(nodes) (the query reads first:100) all yield nothing, so the count stays in full.
• Backdating: the timestamp-window test (start <= at <= end) uses GitHub-set editedAt and creation times. Nobody can backdate a body into an earlier review's job window after the fact.
• Fail-closed shape:
  • It applies only when exactly one review of the tree reported summary findings (summary_reviews == 1).
  • It applies only when exactly one anchored critical-findings: line appears across the named revisions and the in-window summary comments.
  • The reduction is capped at reported.
  • An unknown job window skips it.
  • Any ambiguity leaves every finding counted.
• Gates that still run: the covering review's rule is unchanged and the history is never mixed into it. Open and unresolved threads, missing inline findings, and the thread-level severity: rule are untouched. babysit_risk.py still holds only CRITICAL, which is the semantics this change aligns to.
• _attach_edits: the added GraphQL fields come from the same query, and a missing userContentEdits becomes history=None, which fails closed. No new token scope or API call.

Residual risk, not a finding:

• The "posted" source reuses _summary_comments, which treats an untagged Claude App comment as any lane's. If a marker's history were unreadable and the lane's own line were absent, a Claude App comment in an earlier job window with a single critical-findings: 0 line could be the only line read. That needs a trusted actor to trigger the App inside a seconds-long window, and the covering review has the same property, so I don't rate it.
• A model that understates criticality in an earlier review's line is an existing limit of the line-based design, not something this diff adds.

Spend: the change adds no new API call or charge, and the GraphQL query is larger only through userContentEdits(first:100) on lane comments. No hold.

[melodic-automation-lanes]

Unattended merge: risk HIGH at head d953b99a5f246d8ed1c7468986a402a64c3eb19f. Only LOW, or MED on class, size and reported review-finding reasons alone, merges without a human. A floor path holds it: a human merges it in person, never an agent or a lane.

HIGH high-path:plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py: plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py
HIGH high-path:plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py: plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py

[melodic-automation-lanes]

Unattended merge: risk HIGH at head d953b99a5f246d8ed1c7468986a402a64c3eb19f. Only LOW, or MED on class, size and reported review-finding reasons alone, merges without a human. A floor path holds it: a human merges it in person, never an agent or a lane.

HIGH high-path:plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py: plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py
HIGH high-path:plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py: plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py

This refusal has stood unchanged at this head, so a person is needed: the pr-merge lane added the human-gated label.