Repository navigation
feat(rollup): rollup bot — select, build, verify, close-with-SHA (Refs #5737) - #5742
Conversation
…e rollup bot (Refs #5394) Agent: aprender-w5404 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…Refs #5737) Agent: aprender-w5404 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…5737) Agent: aprender-w5404 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…icting PRs (Refs #5737) Agent: aprender-w5404 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…#5737) Agent: aprender-w5404 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… and close (Refs #5737) Agent: aprender-w5018 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…5737) Agent: aprender-w5018 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
noahgift
left a comment
There was a problem hiding this comment.
Seat: aprender-w5382 (claude-sonnet-5-5, non-author) at 3f53383. Read lib.rs, close.rs, build.rs, mg.rs, gh_json.rs, main.rs and Cargo.toml in full (tests not run; CI still pending). Scope matches: crates/aprender-rollup, b=3 (MAX_ENTRIES), bisect (split/walk), close only via outcome() with a non-empty SHA. gh_json green rejects an empty rollup and an in-progress run (conclusion "" fails check_ok).
Findings, none blocking the dry-run default (verify only writes with --apply):
- mg.rs run_list_args lists merge_group runs by BRANCH only, and branch_name is stable (rollup/-). A stale red run from an earlier build of the same name keeps the verdict red; a stale green with a new head reads Green. The verdict is not bound to the head SHA, so --apply could close PRs on a run that did not test this head. Fix: pass the built head SHA and filter by headSha in the json.
- 'sha on main' is a caller-supplied string; nothing checks it (git merge-base --is-ancestor origin/main). The close guard is only as good as the operator's argument.
- verify with sha '-' and a Green run prints/applies 'Ejected from rollup, left open: rollup commit not on main' on every constituent; --apply re-runs add a duplicate comment each time.
- select() silently drops eligible PRs past the 3rd (no Reject recorded), so plan output does not say why they were left out.
Seat: APPROVE as a dry-run tool; findings 1 and 2 should land before --apply is used in anger.
noahgift
left a comment
There was a problem hiding this comment.
seat 1, aprender-pace @3f533834306cd0c30f14b8cbde20ae88d2019f16
Non-author (0 aprender-pace trailers). Read lib.rs, mg.rs, main.rs, build.rs, close.rs and the contracts/binary-debt row (tests, report.rs, gh_json.rs only skimmed by name). Scope is the new crate plus Cargo.lock/Cargo.toml/one binary-debt row. Dry run is the default; closing needs --apply.
Selection (b=3, 200 lines, opt-in label, no shared file, forbidden paths) and bisection read correctly: a green half is good, a red single is a culprit, pending is undecided and never green.
DEFECT (invariant stated in the docs, not enforced): verify takes the rollup SHA as a CLI argument and outcome() only checks it is non-empty. Nothing runs git merge-base --is-ancestor origin/main, so --apply can close constituents with a SHA that is not on main (typo, wrong branch). The module doc says 'closed only when the rollup SHA is on main'. Fix: check ancestry before Outcome::Close, and add a falsifier with a SHA that is not an ancestor.
Smaller: (a) build merges whatever pull/N/head is now, not the head that was green+approved at selection, so a push between plan and build rides in unreviewed; re-read the head sha and compare. (b) parse_verdict counts skipped/neutral as ok; a rollup run whose required jobs were all skipped reads Green. (c) the bot closes PRs; the stale-PR rule keeps closing superseded PRs with the operator, so the --apply close path needs that authorization named in the ticket.
NOT verified: tests or clippy run, CI (still running), any real gh output.
noahgift
left a comment
There was a problem hiding this comment.
seat 1, aprender-pace @8e1b7f8a5a1e8dd4a8f1b953d1730313113403b5
Re-seat after the author's fixes. Non-author (0 aprender-pace trailers). Read the delta from 3f53383 (non-test source in full; tests by diffstat only).
Fixed as claimed: (defect) Green now needs the rollup SHA to pass git merge-base --is-ancestor origin/main, else it prints 'waiting' and posts nothing. (a) build re-reads FETCH_HEAD after the fetch and ejects a PR whose head differs from the one judged at selection, before the merge. (b) green needs at least one real SUCCESS, and parse_verdict now counts only runs whose headSha equals the given head and requires a success with nothing unfinished.
Remarks (non-blocking): the ancestry check reads the local origin/main without fetching, so a stale ref errs toward 'waiting', the safe side; fetch first if it should be prompt. verify gained a required argument; callers must pass it.
NOT verified: the 45 tests, clippy, CI, any real gh output.
noahgift
left a comment
There was a problem hiding this comment.
Seat: aprender-w5382 (claude-sonnet-5-5, non-author) at 8e1b7f8. Re-read the diff 3f53383..8e1b7f8 (src files in full; tests/CI not run by me). Prior findings: (1) verdict now filters runs by headSha, all-skipped/neutral is Pending, green needs one real success; (2) 'sha on main' is checked with git merge-base --is-ancestor origin/main, and Green without it prints and posts nothing (also fixes 3, no duplicate comments); (4) OverCap rejection recorded. Plus head-binding at build: each PR head is compared with the judged head after fetch and ejected if moved. APPROVE.
Non-blocking: (a) verify's usage error string still shows the old 4-arg form. (b) merge_group runs have head_branch gh-readonly-queue/main/pr--, not the rollup branch, so run list --branch may return no runs and stay Pending forever. That fails safe (never closes) but verify may never go Green; check against a real queue entry. (c) ancestor check uses the local origin/main ref, so run git fetch origin main first or it only delays.
Agent: aprender-w5018 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
noahgift
left a comment
There was a problem hiding this comment.
seat 2, aprender-pace @9de6cb733a58f89c373cf9cbd8e842327ba30512
Re-seat. Product delta vs my seat at 8e1b7f8 is 0 lines (merge of main only). 0 aprender-pace trailers. Not verified: I did not run the tests or read CI results.
noahgift
left a comment
There was a problem hiding this comment.
aprender-w5382 seat (non-author) @9de6cb733a58f89c373cf9cbd8e842327ba30512: APPROVE on content. Verified: git diff 8e1b7f8..9de6cb7 over crates/aprender-rollup, contracts/binary-debt-v1.yaml and Cargo.toml is empty; the head is a merge of main only (parents 8e1b7f8 + e691fb7). My earlier review of 8e1b7f8 carries. CI on this head is still pending (workspace-test-shard, x86-main); this seat is not a green verdict. Not armed.
🤖 Generated with Claude Code
|
§13.11 rung 1 — quorum shadow verdict Shadow mode: this records a verdict and merges nothing. A refusal |
…) and shapes.ttl after merging main Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… (ont4b freshness gate) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Refs #5737 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Refs #5737 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…dger entry resolves (lint, readme_contract, ont4d) Refs PMAT-5737 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…MONO-011 holds without growing the shrink-only allowlist Refs PMAT-5737 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… rows Refs PMAT-5737 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PMAT-5737, rollup bot (operator priority 5; operator 12:25 Madrid relayed by aprender-57: this PR is exempt from the WIP freeze).
New crate
aprender-rollup(build tooling, publish = false):plan: reads open PRs viagh, picks up to 10 small (<=200 lines) green, approved,rollup-okPRs with no shared file and none touching workflows/contracts/Cargo.lock.build <dir>: assembles the rollup branch locally; conflicting PRs are ejected and stay open.verify <branch> <sha|-> <n,..> [--apply]: merge-group verdict; green + SHA on main closes each constituent with the SHA; red prints the bisect halves; pending is a no-op. Dry run by default.37 lib tests; clippy -D warnings clean; mutant (pending treated as good) turns a test red.
Left: automated halves rebuild loop, constituents KPI.
🤖 Generated with Claude Code