Skip to content

feat(source-control): merge MED lane PRs on review pass and cut the path floor to external effects - #9280

Merged
kyle-sexton merged 9 commits into
mainfrom
feat/babysit-guardrail-cut
Oct 23, 2026
Merged

kyle-sexton merged 9 commits into
mainfrom
feat/babysit-guardrail-cut

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Oct 23, 2026 •

Copy link
Copy Markdown
Member

Closes #9270

Summary

The owner's 2026-10-22 direction, given to this session as the guardrail-cut task, sets this PR's scope: the review lanes are the only gate, and the merge gate holds only what has an effect outside the repository or its review. It supersedes the 2026-10-18 floor decision. #9270 (merging MED lane PRs on a review pass) is one part of it.

  • A MED score merges unattended when every reason is a class or size reason (class-contradicted, blast-radius-missing, lines-over, files-over, no-patch:*). Unbound or missing review evidence, an open claude-review finding, an unread file listing, class-missing/class-above-rung (the rung), the cost hold and every HIGH reason still hold, and every policy hold (do-not-merge, needs-human, body hold, wait comment, untrusted origin) is unchanged. Such a MED gets no reasons comment and no risk escalation (A14).
  • The security lane flags, never blocks: only a CRITICAL finding holds (HIGH, owner). The gate reads a severity: line on each inline comment and a critical-findings: line in the summary, which /review:security-review now writes; a finding whose severity it cannot read counts as CRITICAL.
  • The HIGH path floor is cut to **/settings*.json, **/.gitmodules, **/CODEOWNERS, **/package.json, **/package-lock.json, **/requirements*.txt, .github/workflows/release-*.yml, the nine id-token: write workflows, and .github/actions/check-trusted-trigger/**. The unsafe-path, symlink, submodule and mode rules stay.
  • Cut with their tests: high-capability, high-script, the test-only and eval-data exemptions, eval-scaffold-review-required, base-side declarations, FLOOR_ADDITIONS, floor_extensions, and merge.diff-check.floor-paths / floor-script-scopes with their schema and README rows. manifest-review-required now holds only executable plugin.json keys and a marketplace plugin added, removed, renamed or re-sourced.
  • NEVER_FLOOR_EXEMPT is the floor; INSTRUCTION_PATH_RE, its thread-scope.mjs twin and feedback-thread.sh name only submodule, code-owner, settings and package-manifest files.
  • work-class: structural scores as C3 in the local merge gate and the promotion evidence (audit D10); untrusted-provenance stays C5.

Fix

  • babysit_risk.py: new HIGH_FLOOR, holds() and MERGEABLE_MED, blocking() and LaneEvidence.blocking_findings; the manifest check runs on its own.
  • babysit_review_evidence.py: a CRITICAL count beside the open-finding total for the security lane.
  • babysit_merge.py and babysit_whole_review.py: holds() replaces each LOW-only test.
  • babysit_merge_policy.py: config keys, readers and the floor exemption lists.

In the local babysit lane, where the C2 contradiction check stays on, a removed, renamed or schema-changing C2 diff, a C1-labelled PR, and at rung C3 an unrated C3 or structural PR now merge unattended too. That matches the owner direction; the pr-merge lane is unaffected (its class is always C2 and the check is off).

Release PRs (the release App's, both lanes skipped by design) land through their own queue path in release-publish-plugins.yml, not through this gate.

Verification

  • python3 -m unittest discover -s tests -t . in babysit-prs/scripts: 2485 tests OK.
  • node --test in .github/scripts/pr-unblock (544 pass), .github/scripts/apply-patch (104 pass) and babysit-loop/scripts (124 pass); feedback-thread.test.sh passes.
  • scripts/run-ruff.sh check: clean.
  • New tests: a MED on a path reason with both lanes clean enqueues; an open claude-review finding, an unbound lane and a cost hold refuse; non-CRITICAL security findings add no reason and unreadable severity counts as CRITICAL; the KEEP set holds and the cut paths score LOW under the live config.

Related

🤖 Generated with Claude Code

https://claude.ai/code/session_014jM255zWtpKVsuYWs5jToN

kyle-sexton and others added 7 commits October 22, 2026 22:38
…ath floor to external effects

A MED score merges unattended when every reason is a class or size reason
(#9270); unbound or missing review evidence, an open claude-review finding,
an unread listing, the rung and every HIGH reason still hold. The security
lane holds only on a CRITICAL finding, read from severity lines the
security-review skill now writes. The HIGH path floor keeps only settings,
submodules, code owners, package manifests and lockfiles, and the release
and id-token workflows; the capability, script, eval, declaration and
non-executable manifest checks go, with floor-paths and floor-script-scopes.
A structural work class merges as scoped.

Closes #9270

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014jM255zWtpKVsuYWs5jToN
Scope update: #9276 moves very-high security escalation into the review
lanes, so this change adds no severity parsing. Every open
claude-security-review finding still scores HIGH, as on main.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014jM255zWtpKVsuYWs5jToN
Restores the security lane's CRITICAL-only count: the claude-security-review
lane flags, and only a CRITICAL finding, or one whose severity the gate
cannot read, holds the merge for the owner.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014jM255zWtpKVsuYWs5jToN
Takes #9278's apply-patch tests, which drop the instruction-path refusal.
Brings feedback-thread.sh's instruction set in line with the narrowed
INSTRUCTION_PATH, and states what the HIGH path floor holds in the
patch-refusal paragraph.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014jM255zWtpKVsuYWs5jToN
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014jM255zWtpKVsuYWs5jToN
@kyle-sexton
kyle-sexton marked this pull request as ready for review October 23, 2026 03:02
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

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-23T03:05:54.420331Z 6d5ca05 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.

Comment thread docs/conventions/pr-pipeline.yaml Resolved
@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

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

Incremental security review of d6e4a0b against 6d5ca05: no security findings.

I read all seven changed files in .claude-lane/incremental.diff. The instruction-exception register was not consulted. I judged the one deleted rule by consequence instead (see the last bullet).

  • manifest <field>: no plugin.json or marketplace.json change in this increment, so there are no manifest lines.
  • New summary-comment reading in babysit_review_evidence.py (_summary_comments, _open_findings):
    • Only comments from claude or claude[bot] are read, so a PR author cannot post a comment that lowers the critical-findings count.
    • An untagged comment from the other lane is only read if it lands inside the security job's time window and carries a critical-findings: line. A lane-tag line is required for the other lane's comments to be excluded.
    • Only the security lane's skill emits that line (plugins/review/skills/security-review/SKILL.md:221), so an honest code-review comment will not carry one.
    • The reduction still needs exactly one critical-findings: line in the window, and it is capped by min(int(line), own). A second line, a missing job window or a mismatched count falls back to counting every summary finding as CRITICAL.
    • A prompt-injected code-review lane could in theory add such a line when the security summary has none. It would need the same bot identity and the same window, and an injected security lane could already do the same, so I did not report it.
  • babysit_risk.py holding() and babysit_whole_review.py wanted():
    • holds() is now defined as bool(holding()) with the same predicate, so merge-hold behaviour is unchanged.
    • wanted() now checks only the reasons that actually hold, which is the intended fix and is covered by the new test.
  • HIGH_FLOOR and its comment: the floor still contains .github/actions/check-trusted-trigger/** and the release, id-token and merge workflows, so the outside-org hold the comment describes is still enforced.
  • Removed floor-paths and floor-script-scopes: the config-side deletion was in the earlier-reviewed scope, and this increment only documents it as breaking. The floor stays fixed in code and the diff changes no globs.
  • README and changelog: the README change swaps a path for the /review:security-review skill name, which is not an instruction deletion. The changelog bump to major is not a security matter.
  • Instruction-surface deletions: none in this increment. The only deleted instruction-like text is the floor-paths and floor-script-scopes config, covered above.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6d5ca059e4

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread docs/conventions/pr-pipeline/README.md Resolved
@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

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

Incremental review of d6e4a0b against 6d5ca05: no findings.

Read .claude-lane/incremental.diff and .claude-lane/pr-context.md, and read _open_findings in babysit_review_evidence.py around the change. I did not run the test suite, so the 2483-test result in the PR body is author-claimed and unverified.

  • _summary_comments and the summary-line window in babysit_review_evidence.py: the covering review's own critical-findings: line is now read from the head marker (which carries the summary) and from Claude App summary comments posted inside that review's job window. The change stays conservative. A reduction applies only when exactly one line is found, and min(line, own) caps it at what the review reported. A summary comment outside the job window, or a second line, leaves the full count blocking. The new test covers all three shapes: one line in the window, two lines, and one line outside the window.
  • holding()/holds() and wanted(): wanted() now checks the reasons that actually hold the merge rather than every reason. Reasons that don't hold, such as blast-radius-missing, no longer stop a whole review from clearing the open findings that do hold. The two test edits cover both directions.
  • babysit_risk.py docstring and comment: documentation only, with no behaviour change.
  • Changelog fragment: raising the bump from minor to major is consistent with removing the two keys. The schema sets additionalProperties: false at the top level, and the only remaining mentions of the keys are in historical changelogs. The nested merge.diff-check object is not confirmed.
  • Docs: the README now cites /review:security-review instead of a file path, which matches the cross-plugin citation rule.

Manifest checks: this increment changes no plugin.json or marketplace.json, so no manifest lines apply.

kyle-sexton and others added 2 commits October 22, 2026 23:11
- Read `critical-findings:` from the review's summary comment as well as
  the head marker, so the older producer shape counts its line.
- Test only the holding reasons for a whole-review request.
- State what the HIGH path floor holds and why, and cite
  /review:security-review by name.
- The pr-pipeline fragment is a major bump with a Breaking line for the
  removed floor-paths and floor-script-scopes keys.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014jM255zWtpKVsuYWs5jToN
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014jM255zWtpKVsuYWs5jToN
@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 23, 2026
Merged via the queue into main with commit b08b6be Oct 23, 2026
52 of 53 checks passed
@kyle-sexton
kyle-sexton deleted the feat/babysit-guardrail-cut branch October 23, 2026 03:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(source-control): merge MED-risk lane PRs unattended when both review lanes pass

1 participant