Skip to content

feat(pr-pipeline): let a trusted author's gate-path PR clear on a passed security review - #10708

Merged
kyle-sexton merged 19 commits into
mainfrom
fix/gate-hold-clears-for-trusted-author
Oct 29, 2026
Merged

kyle-sexton merged 19 commits into
mainfrom
fix/gate-hold-clears-for-trusted-author

Conversation

@kyle-sexton

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

Copy link
Copy Markdown
Member

No related issue: owner direction 2026-10-27 that hold-floor PRs auto-merge unless HIGH/CRITICAL

Summary

A gate-class hold-floor path now clears on a passed security review, the way an enforcement path does, but only when all of these hold:

  • the pull request was opened by a person on the trusted-actor list (a User, never the lanes App or any other bot);
  • the gate paths it changes were touched only by that person's own commits;
  • pr-verify passed the head being judged.

trust-class paths, paths a configured denied-path glob also matches, and lanes App pull requests stay person-held. A lane's fix commit on a gate path, or commits that cannot be read, keep the hold too, so a lane changing its own powers is still merged by a person.

Fix

  • babysit_risk.py: adds AUTHOR_REVIEW_FLOOR (the gate globs) and RiskInput.author_reviewed. A path in that set that no trust or configured glob matches rates review-path: instead of high-path:, so a clean security review bound to the head clears it. Any value other than a frozenset of strings lifts nothing.
  • babysit_merge_policy.py: author_reviewed_paths computes that set from the PR author, the trusted-actor list's people and foreign_paths, and returns nothing when pr-verify did not pass or the author is not a listed person. assess passes the trusted people through.
  • babysit_merge.py: score_unattended computes the set only on a complete file listing, with verified meaning the verifier reader ran.
  • docs/conventions/pr-pipeline/README.md states the new clear condition under the hold-floor classes; changelog fragments added.

Verification

Run on head efb51b4, which merges main 9d2beb6 (#10681, scripts/check-affected.sh only, clean merge) into 981f125 (itself a merge of main f714f1e). Each suite was also run on 981f125 first, with the same counts.

  • uvx pytest plugins/source-control/skills/babysit-prs/scripts/tests/ -q: exit 0; 3369 passed, 5 skipped, 4109 subtests passed.
  • python3 plugins/source-control/skills/babysit-prs/scripts/tests/test_merge_holds_sync.py: exit 0; 15 tests, OK.
  • node --test .github/scripts/pr-merge/*.test.mjs: exit 0; 617 tests, 617 pass, 0 fail.
  • scripts/validate-plugins.sh --only "source-control": exit 0; plugin contracts validated; 583 mod tests pass, 0 fail, across 8 test files; all plugin manifests and the catalog validated.
  • Ready verify gate (scripts/check-affected.sh --tests) on efb51b4: pass, 14 passed, 2 skipped.

Left out: no live babysit run against a real trusted-author gate-path PR; the unit tests cover the policy and risk paths.

Related

N/A

🤖 Generated with Claude Code

kyle-sexton and others added 4 commits October 28, 2026 13:29
…sed security review

A pull request a person on the trusted-actor list opened now clears a
`gate` floor path that only that person's own commits touched, on a head
pr-verify passed, once the security review binds to the head with no open
finding, as an `enforcement` path clears. Lanes App and bot pull requests,
a lane's fix commit on a gate path, unreadable commits or lists, and every
`trust` path still wait for a person.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…for-trusted-author

# Conflicts:
#	docs/conventions/pr-pipeline/README.md
#	plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py
#	plugins/source-control/skills/babysit-prs/scripts/tests/test_merge_holds_sync.py
@kyle-sexton
kyle-sexton marked this pull request as ready for review October 28, 2026 18:09
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 28, 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-28T18:11:56.436405Z efb51b4 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

github-actions Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

claude-review reviewed 6ce56e9d93317dbee4405ab0e5259cfb37ec32e2: 1 finding(s), 0 as inline comments.

Incremental review of 83c4d86..6ce56e9. The change moves enable-auto-merge.mjs, superseded.mjs, clear-escalations.mjs, gate-check.mjs and head-ref-map.mjs from gate to trust. That takes them out of the trusted-author lift, which closes a real gap: an owner-signed PR could otherwise have weakened a hold on a clean security review. The change is correct and hardens the gate. I read the two class tables, the new test and the docs, and found one doc/comment inconsistency. I did not run any tests.

Checked

  • The two class tables agree. babysit_risk.FLOOR_CLASSES and the merge-holds.mjs HOLD_PATH_CLASSES copy carry the same five reclassifications.
  • The new counts match the table. The merge-holds.mjs block has 14 gate rows left (13 + 5 = 18 trust, 19 − 5 = 14 gate), enforcement stays at 43, and the total stays 75. HIGH_FLOOR (trust plus gate) stays 32, so test_the_person_floor_is_the_same_list is unaffected.
  • AUTHOR_REVIEW_FLOOR is derived from the gate class, so the five scripts drop out of it automatically. test_the_author_lift_is_the_gate_class_and_the_lanes_app_never_gets_it still holds. The new test asserts the exclusion and that the path rates high-path: even after a clean review.
  • test_every_pr_merge_script_is_held_for_a_person accepts any PERSON_CLASSES member, so it passes with trust.
  • The existing floor tests that list these five paths only check that they are held, not their class, so the reclassification doesn't break them.
  • The change does not lower any protection and has no cost impact. Nothing in it reaches the critical-security or cost hold classes.

Finding (advisory): two comments still describe the old class split

  • The HOLD_PATH_CLASSES header comment in .github/scripts/pr-merge/merge-holds.mjs was not updated. Its trust list (L247-L256) stops at "this module, and scripts/sync-hold-floor.py". It defines gate as "the other pr-merge scripts and the rate guard". After this change five pr-merge scripts are trust. The same comment in babysit_risk.py was updated in this commit, so the two copies of the comment now disagree. The comment sits outside the generated BEGIN/END block, so sync-hold-floor.py will not repair it.
  • The README gate bullet (L939-L942) still says gate covers "every other script of .github/scripts/pr-merge/". Elsewhere the same README now lists the five as trust, so "every other" reads as ambiguous. Saying "every other script of .github/scripts/pr-merge/ that the trust bullet does not list" would fix it.
  • Neither comment affects behavior. A reader auditing the classes from these comments would believe those five scripts still take the author lift.

No inline comment is attached, because neither location is on a changed line.

@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 efb51b407b344f264619c0e5333dc2f92f48b0b7; the summary is in this lane's pull request comment.

claude-lane: claude-security-review

@github-actions

github-actions Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

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

No security issues found in the incremental change since 83c4d86.

The change only tightens holds. It reclassifies five hold-applying pr-merge scripts from gate to trust in the floor tables: enable-auto-merge.mjs, superseded.mjs, clear-escalations.mjs, gate-check.mjs and head-ref-map.mjs. A trust path never takes the trusted-author lift, so an owner-signed PR that weakens one of these scripts now waits for a person. Before, it could clear on a passed security review.

What I checked:

  • Tables agree. babysit_risk.py:261-266 and merge-holds.mjs:278-283 carry the same five trust rows. Each file has 18 trust entries, which matches the test's updated counts (trust 13 to 18, gate 19 to 14, enforcement 43 unchanged).
  • Nothing widens. The change adds no entry to AUTHOR_REVIEW_FLOOR, and it doesn't change who is trusted, what a token may do, or the kill switch. The new test asserts that all five paths are outside AUTHOR_REVIEW_FLOOR and hold after a clean security review.
  • No leftover gate assertion in the tests. The test in test_babysit_risk.py that checks the floor holds with the switch off still lists these paths, which is correct. The test_merge_holds_sync.py change is a docstring only.
  • Docs and changelog fragments match the code.

I did not run the test suites.

critical-findings: 0

kyle-sexton and others added 6 commits October 28, 2026 14:15
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The author lift clears the whole `gate` floor class on a trusted
author's own pull request after a clean security review. Agents commit
as the trusted owner, so a pull request widening a lane token's scope
through resolve-config (effect-grants.json, lane-rules.json,
activity-rules.json, the broker's signer code) could merge with no
person. Reclassify .github/actions/resolve-config/** from `gate` to
`trust`: both classes hold for a person, and every other reader
(HIGH_FLOOR, merge-holds.mjs HOLD_PATH_FLOOR, hold-paths,
hold_patterns) treats them alike, so only the lift changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…for-trusted-author

# Conflicts:
#	plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_risk.py
…mments

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 cfaaa2d6a05e16ebbee7dfd250d7f46628bbc201; the summary is in this lane's pull request comment.

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: 1 inline finding(s) on cfaaa2d6a05e16ebbee7dfd250d7f46628bbc201; the summary is in this lane's pull request comment.

claude-lane: claude-review

melodic-automation-lanes Bot and others added 3 commits October 28, 2026 19:20
Lane-Activity: pr-unblock/resolve-conflicts
…for-trusted-author

# Conflicts:
#	plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_risk.py
…s-for-trusted-author' into fix/gate-hold-clears-for-trusted-author

@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 fcd3629631601bcf3b3b1f339a8d4a9bfb0a458a; the summary is in this lane's pull request comment.

claude-lane: claude-security-review

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor

Unattended merge: risk HIGH at head 0140ff1cd13cc37eeac02d58eb9bbed83d672313. 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:.github/scripts/pr-merge/merge-holds.mjs: .github/scripts/pr-merge/merge-holds.mjs
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_merge_policy.py: plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py
HIGH high-path:plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py: plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py
MED findings:claude-review: 1 open, reported only
MED findings:claude-security-review: 3 open below CRITICAL, reported only

kyle-sexton and others added 2 commits October 28, 2026 19:00
docs/conventions/pr-pipeline.yaml sets the activity effect values and lane
slots the token broker reads, so it scopes lane tokens as resolve-config
does: classify it trust, so a trusted author's own pull request no longer
clears it on a security review alone. Bring the HOLD_PATH_CLASSES comment
in merge-holds.mjs in line with the trust class of resolve-config.

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 11abd85bf15b8094e4c53de9c10d8303e0e27be6; the summary is in this lane's pull request comment.

claude-lane: claude-security-review

kyle-sexton and others added 2 commits October 28, 2026 19:10
… lift

An agent signs commits as the trusted owner, so an owner-signed pull
request could widen the author lift or drop a floor row on a clean
security review, and the next pull request would take the wider lift.
Classify the code that computes the lift and the hold tables trust:
babysit_risk.py, babysit_review_evidence.py, babysit_merge_policy.py,
babysit_merge.py, merge-holds.mjs and scripts/sync-hold-floor.py.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Oct 29, 2026
Lane-Activity: pr-unblock/resolve-conflicts
@gitguardian

gitguardian Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
While these secrets were previously flagged, we no longer have a reference to the
specific commits where they were detected. Once a secret has been leaked into a git
repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@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 83c4d86c6fddbce95acf740dfda8b00af3a78dc0; the summary is in this lane's pull request comment.

claude-lane: claude-security-review

…author lift

enable-auto-merge.mjs, superseded.mjs, clear-escalations.mjs, gate-check.mjs
and head-ref-map.mjs apply and enforce the holds. Agents commit as the
trusted owner, so a gate-class lift on them would let an owner-signed PR
weaken a hold on a clean security review, and the weakened script would
judge every later PR. They move to trust, as merge-holds.mjs did.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 29, 2026
Merged via the queue into main with commit 266d444 Oct 29, 2026
41 checks passed
@kyle-sexton
kyle-sexton deleted the fix/gate-hold-clears-for-trusted-author branch October 29, 2026 02:11
@github-actions

Copy link
Copy Markdown
Contributor

Post-merge mutation testing

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

File Killed Survived Timeout No coverage Invalid Not run
.github/scripts/pr-merge/merge-holds.mjs 0 0 0 0 0 1
plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py 0 0 0 0 0 1
plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py 0 0 0 0 0 2
plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py 0 0 0 0 0 3

The run's summary has the full report.

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.

1 participant