docs(bump-callers): mark the detect-unreviewed-merge fleet seeded (BE-6473) - #156
Conversation
…-6473) The roster secret is now seeded, so this fleet is live like the other seven. Replace the pre-seed prose with the standard post-migration wording: an empty roster no longer has a benign reading, it always means a clobbered secret to restore from the canonical callers.json, and ALLOW_EMPTY stays false. Also refresh the two caller-facing docs that still told consumers their pins move by hand, and note in the entrypoint header why this repo itself is not in its own roster (it is deliberately not self-enrolled in the detector).
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 39 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Comment |
There was a problem hiding this comment.
🔍 Cursor Review — Consolidated panel
Triggered by @mattmillerai.
✅ No high-signal findings.
Panel: 8/8 reviewers contributed findings.
|
Review: the doc sweep is right and complete. One thing gives me pause — the fleet this now describes as live has never run. The wording change is correct everywhere it lands, and I checked the sweep is total: no The one substantive concern: (The query works —
One Two small things:
Public-repo note. This repo is public, and the diff carries |
ELI-5
The bumper that keeps every consumer repo's
detect-unreviewed-mergepin up to date has been sitting deliberately switched off, because its list of consumers included two repos whose names would have been printed in this public repo's run log. That leak is fixed (rosters are secrets now), so the list has been loaded into the secret and the fleet is on. This PR is the paperwork: it deletes the "still unseeded, expect a red run" prose everywhere it appears and replaces it with the normal wording the other fleets use.What changed
The functional half of this ticket is not a code change — the workflow already binds
CALLERS_JSON: ${{ secrets.DETECT_UNREVIEWED_MERGE_CALLERS }}(the secrets migration did that). The deliverable was seeding the secret, which is done. So this diff is documentation only:.github/workflows/bump-detect-unreviewed-merge-callers.yml— dropped the pre-seed paragraphs (STILL UNSEEDED,READ THIS BEFORE ACTING ON A RED RUN) and the matchingALLOW_EMPTYenv comment. Replaced with the standard post-migration prose: roster in a repo secret, masked everywhere, and an empty roster now always means a clobbered secret to restore from the canonicalcallers.json— never a reason to flipALLOW_EMPTY.ALLOW_EMPTY: "false"is unchanged..github/bump-callers/README.md— the fleet's table row is nownon-empty (hard-fails if empty), matching the auto-label fleet; the "Why the detect-unreviewed-merge roster is not seeded yet" section is deleted.AGENTS.md— the workflows bullet now says the fleet is live and its roster seeded/secret-stored. The conventions bullet already listed the secret, so it needed no change. Kept line-neutral (see caveats).docs/callers/detect-unreviewed-merge.md,docs/callers/README.md— not enumerated in the ticket, but both still told consumers the roster was unseeded and their pins moved by hand. Leaving them would have shipped self-contradicting public docs.Judgment calls — please read, two ticket premises were stale
1. The roster is seeded with 11 entries, not the 12 the ticket lists. The 12th entry was this repo's own self-caller. That file no longer exists: it was deleted by #148 (
chore(ci): unenroll this repo from its own unreviewed-merge detector), which landed after the ticket was written.AGENTS.mdindependently says "Do not re-add aci-detect-unreviewed-merge.ymlcaller."This is load-bearing, not cosmetic. The ticket's reasoning for that entry was that the canonical filename would land it in "bump_repo's warn-and-succeed 404 branch" — but BE-6471 (#134) changed that branch: a 404 is now recorded in
NOPINand fails the job at the end. So seeding the entry as written would have hard-failed the fleet's first run, the exact opposite of the acceptance criterion. I added anNB:to the entrypoint header explaining why this repo is absent from its own roster, so nobody "fixes" it back.Evidence for the absence (checked by every available path rather than assumed): the path 404s on the default branch via the contents API, it is absent from
git ls-tree origin/main,git lognames the PR that deleted it, andAGENTS.mdforbids re-adding it.I verified the other 11 entries live before seeding — every one resolves 200 on its default branch and pins the reusable at a full 40-hex SHA, so none can trip the 404 hard-fail.
2. I did not run the post-merge dispatch verification, because its stated expectation is false. The ticket expects "12 already-pinned skips, the correct no-op". The 11 live callers are all pinned at the SHA the ticket names, but the reusable has moved twice since (
3b5886b,d0c6271), somainis far ahead of that pin.bump-callers.shonly skips onalready at <NEW_SHA>, so a dispatch would not be a no-op — it would open ~11 real bump PRs across 11 repos. That is the fleet working as designed and is presumably wanted soon, but it is a wide outward action on a premise that turned out to be wrong, so it is your call, not one to take unattended. Nothing about merging this PR triggers it: the fleet's push filter watches onlydetect-unreviewed-merge.yml, which this diff does not touch. Dispatch it when you want the pins to move.Unmet criterion
The canonical
callers.jsonwas not committed to the private ops repo. The ticket says to put it "alongside the other fleets' rosters (find them there)" — there are none. Nocallers.jsonexists in any Comfy-Org repo I can reach (org-wide code search, plus the file trees of the two plausible ops repos). The private-ops-repo roster home appears to be a convention the workflow headers describe but that was never actually created. Rather than invent a location and split the source of truth, I left it undone. The roster JSON is otherwise unrecorded, so this is worth closing soon — it is the only readback path, since there is nogh secret get.Verification
bash .github/bump-callers/tests/test_bump_callers.sh— 368 passed, 0 failed. This includes the phase-1 static guard, which covers this entrypoint automatically through itsbump-*-callers.ymlglob (8 files, floor>= 8) — no extension needed.shellcheck -x .github/bump-callers/bump-callers.sh .github/bump-callers/tests/test_bump_callers.sh— clean.vars.binding reintroduced.DETECT_UNREVIEWED_MERGE_CALLERSvariable existed, so there was nothing to delete.Caveats
AGENTS.mdis 225 lines, already over the checker's 200-line hard ceiling onmain— pre-existing, not introduced here, and not caught in CI because this repo has no self-caller foragents-md-integrity.yml. I kept my edit exactly line-neutral rather than widen the gap. Worth a separate trim.No caller repo names appear in this PR, the commit message, or any file in the diff — the roster exists only in the secret.