Skip to content

fix(source-control): take babysit head pushers from the branch activity API - #11072

Closed
melodic-automation-lanes[bot] wants to merge 4 commits into
mainfrom
work-resolve-issue/issue-7354-40960981621-1
Closed

melodic-automation-lanes[bot] wants to merge 4 commits into
mainfrom
work-resolve-issue/issue-7354-40960981621-1

Conversation

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor

Closes #7354

Summary

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

The unattended merge gate built its head-pusher exclusion only from commit author and committer logins. GitHub links those through a commit email the pusher chooses, so a write collaborator could push a commit carrying the agent identity's email and then count as an independent resolver of the lane's review threads. _lane_evidence now also reads the head branch's repository activity (owner decision D6, option b) and excludes every actor that moved the branch. It refuses with head-pusher-unproven when the newest move did not bring the head, when a move has no actor, or when the pinned head is missing from the commits list (the hardening item).

Not verified

  • Python unit tests (test_babysit_merge_unattended.py and the babysit-prs suite) were not executed: this lane runs no interpreter or test runner. They are left to the PR's CI. The review subagent traced the changed tests by hand instead.
  • ruff lint/format was not run (no runner here). I checked line lengths by hand and all are within 88. Left to the PR's CI.
  • tidy: the ai-slop pass ran only as a manual read (it found no tells), because the lane cannot run ai-slop's detect.sh. The simplify, comment and docs-hygiene passes ran and needed no edits.
  • The repo-wide activity history has no page cap in this read (fetch_paginated_api follows every page). A branch name with a very long reused history costs more reads; the merge policy's branch_activity caps at 20 pages for its own reads.

Fix

  • plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py: new _head_pushers reads the actors of the head branch's activity (newest first, deletions skipped) and refuses push-history / push-actor. _lane_evidence adds those actors to excluded, using the read token, refuses `head-no
  • plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_unattended.py: HeadPushers serves an activity fixture and adds tests for head-not-listed, push-history, push-actor, and a collaborator spoofing the agent email who is no longer an independent human. LaneEvidenceReadToken, Lan
  • plugins/source-control/skills/babysit-prs/reference/safety.md: the unattended-merge step 4 describes activity-based head pushers and the new refusal codes, with a pointer to the list-repository-activities docs.
  • .changes/source-control/babysit-head-pushers-activity-7354a1c0.md: patch changelog fragment (Fixed).

Verification

The runner ran each check from the base copy against 91103c8:

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

The run reports:

  • read https: //docs.github.com/en/rest/repos/repos?apiVersion=2022-11-28#list-repository-activities: GET /repos/{owner}/{repo}/activity takes ref as refs/heads/BRANCH or BRANCH. direction defaults to desc and per_page maxes at 100, with Link-header cursors. activity_type is push, force_push, branch_c
  • check-changelog-fragments: all 9 fragments valid (it first failed on the fragment name; I renamed the file to the <slug>-<8 hex> form)
  • check-affected: 12 passed, 6 skipped (typos not installed; validate-plugins sub-checks and changed-skills run code from the checked tree), 0 failed
  • cant-fail-scan --file test_babysit_merge_unattended.py --lines <changed ranges>: no can't-fail tests found, exit 0
  • fresh-context review subagent over the whole diff: it confirmed two test bugs (the fixture HEAD is "a"*40, so two new cases never refused) and a dead try/except in CandidateReadsOnce. I fixed all three. It traced the production code and the other modified tests as correct and found no other caller o
  • The lane run: https://github.com/melodic-software/claude-code-plugins/actions/runs/40960981621

Left out: the items under Not verified.

Related

#7354

Lane-Activity: work-resolve-issue/resolve-issue
@github-actions

github-actions Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

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

Incremental security review of 5571c35 (since 50c8eca): no security findings.

The only change since the last review is in plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge.py, in the lanes_listed test helper. The fixture now sets "commits": 1. The fetch_paginated_api mock now returns a commit list and a push-activity record, so the head's pushers count as proven. It previously returned [].

I read this only as a test fixture and checked no other file. It is not on a trusted path, because only the production babysit-prs/scripts/*.py modules are listed. It changes no production code, trust control, token scope, kill switch, merge hold or workflow, and handles no secrets. The fixture actors are the literal login someone. The change makes the test model the proven-pusher path more faithfully, and it does not weaken any gate. The skip gate (no security-relevant surface) applies.

critical-findings: 0

@github-actions

github-actions Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

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

Incremental review of test_babysit_merge.py (changes since 50c8eca): no findings.

The change makes LaneRunsReadTests.lanes_listed supply the head-pusher evidence that _lane_evidence now requires. I checked it against the code by reading it, not by running the tests:

  • "commits": 1 matches the single commit row the mock returns, so total > len(commits) is false (babysit_merge.py:3165).
  • The mocked commit row has sha == HEAD and linked author and committer logins, so the head-not-listed and unlinked-commit checks pass (babysit_merge.py:3170-3182).
  • The mocked activity row has a non-deletion activity_type, after == HEAD and an actor login. That satisfies _head_pushers, which raises HeadPusherUnproven otherwise (babysit_merge.py:3098-3118).
  • The endpoint matchers (/commits?per_page=100, /activity?) match the URLs built at babysit_merge.py:3160 and 3196. Timeline and comments fall through to [].
  • The paginated(endpoint, label, **_) signature accepts the env_overrides keyword the real calls pass.

The tests still exercise what their docstring describes: which lanes' runs get listed. Spend: no cost or hold implications.

@melodic-automation-lanes

melodic-automation-lanes Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor Author

Unattended merge: native auto-merge leaves this pull request unarmed at head 5571c35a9c4cd589554296cf90884426a198f63b. The lanes clear this reason on their own; the next scheduled run judges it again.

a pr-unblock lane stopped its head (pr-unblock/address-feedback is failure), which ci-status fails
  in the merge queue

@melodic-automation-lanes melodic-automation-lanes Bot added needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. and removed needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. labels Oct 29, 2026
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

The unattended merge path armed native auto-merge at head 91103c89949cfdea00f728ad8d46647b03fac84b.

Lane-Activity: pr-unblock/resolve-conflicts
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

A lane run on this pull request's head failed because the token broker refused it as workflow-modified: .github/workflows/pr-unblock.yml changed on main after the run's event, and a rerun keeps the old file. pr-merge pushes an empty commit so the next pr-unblock run uses main's files. It does this once per version of that file, so if a run fails the same way again, a person looks at it. The push starts no run itself: the conflict-resolution run starts at the next push to main or half-hourly pr-unblock-dispatch tick, or when a listed person runs it by hand, and an address-feedback run can start after both review lanes review the new head, if an unresolved thread is left for it.

The token broker refused a lane run on the previous head as workflow-modified (.github/workflows/pr-unblock.yml). This commit changes no file.

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.

The pr-verify verifier failed head 5571c35a9c4cd589554296cf90884426a198f63b. What it wrote, as data:

What it wrote (27 lines)
Summary: Issue #7354 asked for option (b) from owner decision D6: take head pushers from the branch
  activity API, plus the head-in-list hardening. The diff does that, and the code looks correct and
  fails closed. But the owner's latest trusted comment on #7354 (2026-10-29 23:11, after this PR
  opened) closed the issue as "hardening deferred". It says hardening against the organization's own
  actors waits until open issues and open PRs both reach 0, and returns then "as configuration"
  (owner decisions Q15/Q16). This PR delivers now, as hard-coded unattended-merge gate behavior, the
  hardening the owner put off and asked to come back in a different form. The spec's current intent
  is not met, so this should not merge as-is.
- spec-text.json #7354 last comment: the owner closed the issue as "hardening deferred" until open
  issues = 0 and open PRs = 0, to return "as configuration". This PR still says `Closes #7354` and
  hard-codes the hardening. Fix: hold or close this PR until the reopen trigger fires, or get a new
  owner decision that overrides Q15/Q16. When the work resumes, make the activity-based pusher
  exclusion configurable, as the owner asked, not always on.
- pr body: the Python tests and ruff were not run by the lane. They are left to CI, so the changed
  test counts (`3 + 2 * 4`, `1 + 3 + 2 * 4`) have only been traced by hand.
- plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py:3193: the activity read follows
  every page with no cap. A reused branch name with a long history costs unbounded reads. The PR
  body says the merge policy's own branch_activity read stops at 20 pages. When this returns, apply
  a similar cap and refuse with `head-pusher-unproven` past it.
- plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py:3193: `repos/{repo}/activity`
  is read on the base repo. For a cross-repo (fork) head, the activity is empty or belongs to a
  same-named base branch, so the gate refuses with push-history. That fails closed, but no test or
  doc line covers or states it.
- plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py:3196: the activity read uses
  the verdict reader's token (`read_env`). The PR does not confirm that this token can read
  repository activity. A permission error would block every unattended merge rather than refuse with
  a clear reason. Confirm the token's scope or map the error to a `head-pusher-unproven` reason.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

pr-unblock / address-feedback: UNCERTAIN, left open

Evidence: plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py:3193, plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py:3196

The main finding is about scope, not code. The verifier reports that the owner closed issue 7354 as hardening deferred until open issues and open PRs both reach 0, and wants it to come back later as configuration (owner decisions Q15/Q16). This PR adds the activity-based head-pusher exclusion now, always on, inside the unattended-merge gate (babysit_merge.py:3193-3201, through _head_pushers at :3098). The code can't settle whether to hold or close this PR, make the exclusion configurable, or get a new owner decision that overrides Q15/Q16. That is the owner's call on shared merge infrastructure, so this lane made no edit. The verifier framed the follow-ups as work for when the issue reopens, so none was applied here. Each one is confirmed in the code: (1) the activity read at :3195 goes through fetch_paginated_api with no page cap; (2) it reads repos/{repo}/activity on the base repo, so a fork head fails closed with head-pusher-unproven:push-history, and no test or doc covers that; (3) it uses the verdict reader token (read_env, :3149), and a permission error raises instead of mapping to a head-pusher-unproven reason. A person needs to decide whether this PR goes ahead before those are fixed.

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

pr-unblock / address-feedback finished round 1 of max 4 on 5571c35.

@melodic-automation-lanes

melodic-automation-lanes Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor Author

The pr-unblock lane stopped address-feedback on this pull request: the pull request has an unresolved review thread the lane does not take (a reviewer it does not act on, a review of an older head, or a thread it already answered), so nothing will resolve it. The lane sets no label: the run ends in a failing pr-unblock/address-feedback status on the head the lane gated.

When a retry on this head can change the outcome, the lane runs one on its own, up to its per-head cap, and edits this comment if it stops again; a run that does not stop clears the status. When no retry can (a review thread the lane does not take or settle, or a failing check after the retries are spent), a person resolves the threads or gets the checks green, then dispatches the workflow below: that run finds the cause gone and clears the status. On a pull request the lanes App opened, the dispatchers start that run once per stop; on any other, a person does. A new commit to the branch starts clean.

To run it again on this head now, dispatch the pr-unblock workflow:

gh workflow run pr-unblock.yml -R melodic-software/claude-code-plugins -f pr-number=11072 -f activity=address-feedback

@kyle-sexton kyle-sexton added the do-not-merge Hard merge gate: do not merge while applied. label Oct 30, 2026
@kyle-sexton

Copy link
Copy Markdown
Member

Closing: its issue #7354 was closed as not planned by the owner (2026-10-29, hardening deferred). Reopen #7354 to revive this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge Hard merge gate: do not merge while applied.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

babysit unattended merge: head-pusher exclusion trusts commit email linkage

1 participant