Skip to content

feat(source-control): adopt idle owner pull requests in the pr-merge lane - #8999

Merged
kyle-sexton merged 12 commits into
mainfrom
feat/pr-merge-adopt-idle-owner-prs
Oct 22, 2026
Merged

kyle-sexton merged 12 commits into
mainfrom
feat/pr-merge-adopt-idle-owner-prs

Conversation

@kyle-sexton

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

Copy link
Copy Markdown
Member

No related issue: owner decision Q22 (2026-10-22): idle owner PRs are adopted by the lanes

Summary

The pr-merge lane now adopts an idle owner pull request. A pull request is adopted when it is not a draft, an account in adoption.owner-ids opened it (matched by GitHub account id, never login; default the owner's id 153232337), its updatedAt and head commit committer date are both at least adoption.idle-hours old (default 24; an unreadable date is not idle), and none of the hold label, the tracker's resolved human-gated role label, or a body hold applies. Both settings are read at the default branch head; the adoption: section and its schema entries belong to #8997, and this PR falls back to the defaults while that section is absent. Adopted pull requests go through every existing gate unchanged.

Fix

  • babysit_merge_policy.adoption_refusal holds the whole predicate and returns a distinct refusal per condition. lane_eligibility calls it only for a pull request the lanes App did not open: a non-owner author still refuses not-lane-authored, an owner pull request that is not adopted refuses not-adopted:<reason>, and an adopted one is recorded as eligibility.adoption (idle-owner:<hours>h) in the decision output.
  • PipelineConfig reads adoption.idle-hours and adoption.owner-ids, and stricter_config takes the default branch head's values.
  • assess_unattended resolves the human-gated label through human_gated_label (role_labels) for the pr-merge authority and passes it in. If the label cannot be read, nothing is adopted. The body-hold regex and hold labels moved into the policy module, and babysit_merge re-exports them.
  • pr-merge.yml adds each owner account's pull requests to the candidate listing, looking up the login from the account id. A failed lookup only warns. The gate checks the id again.
  • pr-verify: the new value merge.verify-authors: lane-or-owner judges the lanes App's pull requests plus the owner account's (a User, by id), and this repository now sets it. pr-verify runs only when pr-require-checks completes, which is right after a push while the head is still fresh. So it judges owner pull requests at each ready head, and the verdict is in place once the pull request goes idle.

Verification

  • uv run --with pytest python -m pytest plugins/source-control/skills/babysit-prs/scripts/tests/: 2412 passed, 1840 subtests passed. New tests cover the adopted case and each refusal: draft, wrong id, the owner's login on another id, the owner's id on a Bot, fresh update, fresh head, an unreadable date, the hold label, the resolved and remapped human-gated label, a body hold line, invalid idle-hours, owner-ids from config, and both defaults.
  • node --test .github/scripts/pr-verify/*.test.mjs .github/scripts/pr-merge/*.test.mjs: 210 passed.
  • bash scripts/check-affected.sh --tests: one failure, check-script-contract.test.sh, because htmlhint is not installed in the fresh worktree (no npm ci). That is unrelated to this change; everything else passed. scripts/run-ruff.sh check is clean, and actionlint is clean on pr-merge.yml.

Related

🤖 Generated with Claude Code

kyle-sexton and others added 3 commits October 21, 2026 22:35
…lane

Owner decision Q22: a ready, unheld pull request an owner account opened
(matched by account id in adoption.owner-ids, default the owner's), whose
update and head commit are both adoption.idle-hours old (default 24), is
eligible for the pr-merge lane under every existing gate. The predicate is
babysit_merge_policy.adoption_refusal; the decision records
eligibility.adoption. pr-merge.yml lists the owner accounts' PRs, and
merge.verify-authors: lane-or-owner lets pr-verify judge them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
pr-verify.yml reads adoption.owner-ids from main's pr-pipeline.yaml (the
owner's id when absent) and passes it as OWNER_IDS, so lane-or-owner
judges the same accounts the pr-merge lane adopts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…dle-owner-prs

# Conflicts:
#	docs/conventions/pr-pipeline.yaml
#	plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py
@kyle-sexton

Copy link
Copy Markdown
Member Author

Decision recorded. The owner keeps Q22 (2026-10-22): idle owner PRs stay adoptable and merge unattended on the non-author verifier verdict, the same trade as Q567 and Q624. This is after weighing the OWASP Secure Coding with AI cheat sheet §14 and GitHub's Copilot risks-and-mitigations guidance that AI-changed code get a human approval before merge.

@kyle-sexton
kyle-sexton marked this pull request as ready for review October 22, 2026 13:21
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 22, 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-22T13:25:54.470192Z 51e8304 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 plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py Outdated
Comment thread plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py Outdated
@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

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

Incremental review of docs/conventions/pr-pipeline/README.md since 27f42c9: no findings.

The change adds one sentence to the trusted-spec risk paragraph (lines 1217–1224). It says an agent session that opens both the issue and the pull request as the owner can be merged against that spec once the pull request has been idle for adoption.idle-hours.

I checked it against the rest of the README and the PR context:

  • Trusted-spec rule: the sentence matches lines 1208–1214, where the pull request's author is already counted among the trusted humans and a Bot or App never is.
  • Adoption eligibility: it matches lines 788–800, which cover adoption.owner-ids, adoption.idle-hours and the idle-owner record.
  • Verify-authors: it matches the lane-or-owner text at lines 1143–1149.
  • Owner decision: it matches option B in the PR body, so the dated "owner decision" attribution is backed by the PR itself.

I ran no tests; this is a prose-only change.

@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review needs no new review of ad8c660bbc37a1ff872eb8b63c2d70209092df9c: every file in scope matches docs-only-paths.

Security review: incremental, head 27f42c9

No security findings in the changes since b278d65. No manifest files changed, and the diff deletes or softens no standing instruction, so I did not consult the instruction exception register.

What changed. read_pin(..., alone=True), used by lane_queued_at_head, now ends the "lane's own merge-queue entry keeps adoption" excuse once anyone other than the lane identity comments after the pin or submits the newest review after it. This narrows adoption of idle owner PRs, so it moves toward more refusals.

Fail-closed behaviour, checked in babysit_postmerge.py and babysit_merge_policy.py:

  • A comment by any author other than the identity at or after the newest pin returns None. An author that cannot be read (for example a deleted user) also fails the identity match and returns None.
  • A newest review by anyone but the identity, with submittedAt >= pinned_at, returns None.
  • An unreadable review list, or a timestamp that fails to parse or mixes naive and aware values, raises PostMergeReadError. lane_queued_at_head turns that into "not queued", so the pull request is refused again.
  • An edited pin, a comment thread longer than the page limit, and a missing pin still read as no pin.
  • Tests cover the review, comment and unreadable cases, and the stay-quiet case where only the identity spoke after the pin.

Residual, not reported. The query reads only the newest review (reviews(last:1)). If the lane identity itself submitted a review after a human's post-pin review, that human review would be hidden. babysit_merge_policy.py and babysit_postmerge.py contain no code that submits a review as the identity. A review by any other author, including review-lane bots, makes the newest review non-identity and so refuses, which is the safe side. _human_blockers and the other hold checks still apply to the entry independently. I found no concrete exploit path, so this is not a finding.

The changelog fragment, the pr-pipeline README and safety.md changes are prose that matches the code. The remaining diff is reformatting and test fixtures.

manifest fields: none touched.

@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: 51e8304273

ℹ️ 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 plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py Outdated
kyle-sexton and others added 4 commits October 22, 2026 09:37
The schema and README give adoption.owner-ids a default of [] (none), but
the gate, the pr-merge listing and the pr-verify resolver fell back to the
owner's id when the key was absent or null. All three now read an absent,
null or empty list as no owner ids. This repository's config sets the id
explicitly, so its behavior does not change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The schema and README bound adoption.idle-hours to 1 through 720, but the
reader accepted 0 and any larger whole number. A value outside the range now
adopts nothing with a note, and a cached default-branch config holding one
is read again.

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

The unattended enqueue posts a pin comment, and enqueues, which refreshes an
adopted owner pull request's updated_at. The next sweep then refused it
not-adopted:updated-fresh and dequeued it, so a slow queue never merged it.
A recent update no longer refuses while the lanes App's own merge-queue
entry holds the pull request and its newest unedited pin comment names the
current head. The head commit idle test and every hold still apply, and an
unreadable queue or pin is not queued.

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

Copy link
Copy Markdown
Member Author

Decision recorded. The owner chose option B for S&C's L1 (2026-10-22). A spec written by a trusted human counts as trusted even when that human also authored the pull request, so an adopted owner PR can merge unattended once every other gate passes (the non-author verifier verdict, ci-status, the HIGH floor, holds). Lane PRs are App-authored, so they still never supply their own spec. This supersedes Q23's author-spec separation for human authors.

The owner's standing intent, in their words: PRs authored by them, by the organization's GitHub Apps, or by organization members should merge automatically when it is safe and the classifier has checked them, unless the owner needs to review.

kyle-sexton and others added 2 commits October 22, 2026 11:27
… request

A linked issue written, and last edited if ever, by a User on the trusted
list's humans now counts as the spec even when that human opened the pull
request (owner decision, option B). Without it an owner pull request whose
closing issue the owner wrote always refused verify:untrusted-spec, so
idle-owner adoption merged nothing. Bots and Apps, the lanes App among them,
and untrusted accounts stay excluded, so a lane pull request never supplies
its own spec.

The gate's _trusted_actor and the resolver's trusted filter stay twins; the
PR-author parameter they no longer read is removed with its plumbing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread .github/scripts/pr-verify/resolve-pr.mjs
…speaks

A queued adopted pull request excused every fresh update while the lane's
own entry sat at the head, so a human comment asking to wait no longer
refused and dequeued it. The excuse now holds only while no author but the
agent identity has commented, or submitted the newest review, after the
lane's pin comment. An unreadable comment or review list is not excused.

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

With the pull request's author counted as a spec author, a session using
the owner's login can open both the issue and the pull request, and the
lane can merge it once it is idle. The trusted-spec paragraph now says
the owner accepts that case too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 22, 2026
Merged via the queue into main with commit 210ccfa Oct 22, 2026
52 of 53 checks passed
@kyle-sexton
kyle-sexton deleted the feat/pr-merge-adopt-idle-owner-prs branch October 22, 2026 16:13
@kyle-sexton

Copy link
Copy Markdown
Member Author

Decision recorded. The owner answered in the poteto-review session (2026-10-24, "go w/ recommendations", Q9): the 2026-10-22 option B spec-trust rule stands. A lanes App issue body and the App's intake-drafted criteria comment do not count as spec for a lane pull request. The cause behind those verify failures is fixed at intake instead: a trusted human confirms the drafted criteria (by editing the issue or posting them), and the verifier reads that confirmed text. The spec-trust part of #9557 was removed for this reason.

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