Skip to content

docs(bump-callers): mark the detect-unreviewed-merge fleet seeded (BE-6473) - #156

Merged
mattmillerai merged 2 commits into
mainfrom
matt/be-6473-seed-detect-unreviewed-merge-roster
Aug 13, 2026
Merged

docs(bump-callers): mark the detect-unreviewed-merge fleet seeded (BE-6473)#156
mattmillerai merged 2 commits into
mainfrom
matt/be-6473-seed-detect-unreviewed-merge-roster

Conversation

@mattmillerai

Copy link
Copy Markdown
Contributor

ELI-5

The bumper that keeps every consumer repo's detect-unreviewed-merge pin 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 matching ALLOW_EMPTY env 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 canonical callers.json — never a reason to flip ALLOW_EMPTY. ALLOW_EMPTY: "false" is unchanged.
  • .github/bump-callers/README.md — the fleet's table row is now non-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.mdnot 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.md independently says "Do not re-add a ci-detect-unreviewed-merge.yml caller."

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 NOPIN and 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 an NB: 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 log names the PR that deleted it, and AGENTS.md forbids 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), so main is far ahead of that pin. bump-callers.sh only skips on already 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 only detect-unreviewed-merge.yml, which this diff does not touch. Dispatch it when you want the pins to move.

Unmet criterion

The canonical callers.json was not committed to the private ops repo. The ticket says to put it "alongside the other fleets' rosters (find them there)" — there are none. No callers.json exists 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 no gh secret get.

Verification

  • bash .github/bump-callers/tests/test_bump_callers.sh368 passed, 0 failed. This includes the phase-1 static guard, which covers this entrypoint automatically through its bump-*-callers.yml glob (8 files, floor >= 8) — no extension needed.
  • shellcheck -x .github/bump-callers/bump-callers.sh .github/bump-callers/tests/test_bump_callers.sh — clean.
  • Workflow file parses as YAML; no vars. binding reintroduced.
  • Secret confirmed present on the repo. No variable-era DETECT_UNREVIEWED_MERGE_CALLERS variable existed, so there was nothing to delete.

Caveats

AGENTS.md is 225 lines, already over the checker's 200-line hard ceiling on main — pre-existing, not introduced here, and not caught in CI because this repo has no self-caller for agents-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.

…-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).
@mattmillerai mattmillerai added cursor-review Multi-model cursor review agent-coded Authored by the agent-work loop labels Aug 12, 2026
@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 39 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d032b8cd-4d8f-4f1d-9ff7-3784cbff9bc1

📥 Commits

Reviewing files that changed from the base of the PR and between 0e0ef57 and f3ffa7f.

📒 Files selected for processing (5)
  • .github/bump-callers/README.md
  • .github/workflows/bump-detect-unreviewed-merge-callers.yml
  • AGENTS.md
  • docs/callers/README.md
  • docs/callers/detect-unreviewed-merge.md

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions 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.

🔍 Cursor Review — Consolidated panel

Triggered by @mattmillerai.

✅ No high-signal findings.

Panel: 8/8 reviewers contributed findings.

@wei-hai

wei-hai commented Aug 13, 2026

Copy link
Copy Markdown

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 unseeded / not seeded / not yet seeded text survives anywhere in the tree on this branch. Picking up docs/callers/detect-unreviewed-merge.md and docs/callers/README.md even though the ticket did not list them was the right call — leaving them would have shipped two public pages contradicting the workflow header. The "12 entries → 11, the 12th was our own self-caller removed by #148" reasoning holds up against AGENTS.md, which independently says not to re-add it.

The one substantive concern: bump-detect-unreviewed-merge-callers.yml has zero runs, ever.

$ gh run list -R Comfy-Org/github-workflows -w bump-detect-unreviewed-merge-callers.yml
(nothing)

(The query works — bump-auto-label-callers.yml returns rows.) So the roster has been seeded but never read by the thing that reads it. Everything this PR now states as fact is inference from the config rather than from an observed run:

  • docs/callers/detect-unreviewed-merge.md now tells consumers "bump-...-callers.yml moves your pin for you";
  • the header says an empty roster "ALWAYS means the secret was clobbered", which is the diagnostic an on-call reads at 2am;
  • the header also asserts the roster shape parses and every entry's file resolves — and bump-callers.sh hard-fails the run on an entry naming a file that no longer exists (I confirmed that behavior at bump-callers.sh:470 + :1135), which is exactly the failure a never-exercised roster is most likely to have.

One workflow_dispatch from main costs nothing and settles all three: it prints the count and the sha256 fingerprint the header promises, and either bumps the fleet or tells you which entry is stale. I would run that before merging the docs that promise it works, rather than after.

Two small things:

  1. README.md:112 still lists DETECT_UNREVIEWED_MERGE_CALLERS in its secrets enumeration — that line is fine as-is, just flagging that I checked it for the same staleness and it needed no change.
  2. Unrelated but adjacent: bump-auto-label-callers.yml's last two runs both failed (2026-07-30, 2026-07-31). Not this PR's problem, and I have not looked into it — but this directory's whole premise is that a red bumper run is a signal someone acts on, and that one has been red for a while. Worth a separate ticket.

Public-repo note. This repo is public, and the diff carries BE-6471 / BE-6472 / BE-6473 into files and prose. That matches the existing convention in this file, so I am not asking you to change it here — but bare ticket IDs in a public repo are the kind of thing worth deciding on deliberately once rather than per-PR.

@mattmillerai
mattmillerai merged commit 6bdf40c into main Aug 13, 2026
5 checks passed
@mattmillerai
mattmillerai deleted the matt/be-6473-seed-detect-unreviewed-merge-roster branch August 13, 2026 06:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent-coded Authored by the agent-work loop cursor-review Multi-model cursor review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants