Detect bots by account type in claude-review instead of enumerating them per caller

[kyle-sexton]

claude-review.yml decides which PRs to skip by enumerating bot authors by name. Every consumer caller overrides the defaults to add each App as it appears, so introducing a new bot means editing seven callers — and forgetting one means that bot's PRs silently consume review budget.

This is the same maintenance tax the pr-issue-linkage bot exemption deliberately avoided by keying on the author's account type instead of an author list. The two workflows should reach the same answer the same way.

Why this was split out

Deliberately fenced out of the pr-issue-linkage change rather than bundled:

• Different stakes. A missed review-skip wastes review budget. A missed linkage exemption blocks a merge outright. The gate fix should not wait on the lower-stakes one.
• Different blast radius. Changing the input contract pulls every consumer caller into the diff, on top of the pin sweep those repositories already owe.

Scope

• Replace the author-name enumeration with account-type detection in the reusable workflow.
• Decide the fate of the existing per-caller override inputs — removed outright, or retained as a narrowing escape hatch. Removal is a breaking input-contract change and needs the caller cleanup sequenced with it.
• Update the workflow's tests and the README section describing the inputs.
• Drop the now-redundant overrides from each consumer caller.

Open question

Are there bots whose PRs should still be reviewed? Account-type detection skips every bot uniformly, where the current list is per-bot opt-in. Confirm no consumer depends on that distinction before switching.

[kyle-sexton]

This comment was written by an AI agent (gate-audit lane), on the operator's behalf.

Gate challenge result: stays gated. Unanimous 3/3 (this agent, a fresh-context Fable panelist, a fresh-context Codex panelist): a breaking input-contract change to a reusable workflow with 7 consumers, plus an explicitly unresolved behavioral question (which bots should still be reviewed) that the issue itself says must be confirmed with consumers first — cross-cutting, human-call.

Decision-ready summary — RECOMMENDED: adopt account-type detection but retain the per-caller override input as a narrowing escape hatch (don't remove it). This gets the maintenance-tax win (no more per-caller bot-name enumeration for the common case) without forcing a synchronized breaking change across 7 callers, and preserves room for the rare bot whose PRs should still be reviewed without deciding that question by fiat today.

Alternative: remove the override outright once account-type detection lands — cleaner end state, but requires the issue's own open question ("does any consumer depend on the per-bot distinction?") to be answered across all 7 consumers before it's safe, and sequences a breaking caller update everywhere at once.

[kyle-sexton]

Status: still open — the supersession depends on a component that has not shipped

docs/topics/claude-review-lanes/PLAN.md maps this issue as "superseded by caller-component normalization + skip-actors defaults; evaluate account-type detection in the component."

What has actually landed

The reusable now carries a centralized default (ci-workflows main, .github/workflows/claude-review.yml:150):

default: dependabot[bot],claude[bot],melodic-ai[bot],melodic-standards-sync[bot]

That addresses the "instead of enumerating them per caller" half — the list lives in one place and callers inherit it. melodic-software/standards#286 makes this explicit: its component deliberately does not override skip-actors, and documents that a caller which overrides the input owns the whole exclusion list, self-trigger ban included.

What has not

• feat(workflows): Claude lane incident aggregator (Phase 4, #238) #286 is still OPEN, so no consumer inherits the default yet. Every live caller still pins a pre-normalization ci-workflows revision (five pin 90f1c54, claude-code-plugins pins e295107).
• Account-type detection — the actual subject of this issue — has not been evaluated. Centralizing an enumerated list is not the same as detecting bots by account type; it removes the per-caller duplication but keeps the enumeration.

Trigger to close

1. melodic-software/standards#286 merges and the caller components sync to consumers, so the inherited default is live fleet-wide; then
2. someone evaluates account-type detection in the component and records the outcome — adopt it, or reject it with a reason.

Step 2 is the part that decides this issue. Step 1 alone only makes the enumeration inherited rather than copied.

[kyle-sexton]

Trigger 1 is now satisfied; trigger 2 still decides this issue. Still open

Recording state change since the comment above, which set two triggers to close.

Trigger 1 — met. melodic-software/standards#286 merged 2026-07-29T16:12:20Z, so the sync-managed caller components exist and the centralized skip-actors default can reach consumers. Note the rollout is not finished: claude-code-plugins, for one, still pins 66073e5 (2026-07-26) on its security caller, predating cf666f6.

Trigger 2 — not met, and it is the deciding one. No evaluation of account-type detection in the component has been performed or recorded — neither adopted nor rejected-with-a-reason. Centralizing an enumerated bot list is not the same as detecting bots by account type, which is this issue's actual subject.

PLAN.md Phase 3f directs comment-closing this issue as superseded by component normalization. Per the analysis above, that mapping is premature: normalization delivers the "not per caller" half only. Left open pending trigger 2.

This comment was written by an AI agent (claude-review-lanes Phase 3f issue sweep), on the operator's behalf.

[kyle-sexton]

Account-type detection: evaluated — REJECT, with reasons

This is trigger 2 from the 2026-07-29T15:41Z comment: "someone evaluates account-type
detection in the component and records the outcome — adopt it, or reject it with a
reason." Recording the outcome. Rejected. Reasons below, each with the evidence
it rests on.

This evaluates the mechanism on its merits, not the incumbent's right to stay. Two
findings would have supported adoption and did not survive checking: the founding
analogy (§1) and the measured cost (§6).

---

1. The founding premise is inverted — the cited precedent decided the opposite way

This issue's opening argument is that account-type detection is "the same maintenance
tax the pr-issue-linkage bot exemption deliberately avoided by keying on the author's
account type instead of an author list," and that "the two workflows should reach the
same answer the same way."

pr-issue-linkage does not key on account type. It keys on exact login equality,
and it rejected the pattern approach explicitly. From the input's own description at
main (.github/workflows/pr-issue-linkage.yml):

Matched by exact equality against github.event.pull_request.user.login — never a
*[bot] pattern, so an unknown future bot is not silently skipped.

PR #171, which shipped that input, says the same thing in its summary and pairs it with
a dedicated regression test (unlisted bot login is not suffix-matched). The timing
explains the mismatch rather than excusing it: this issue was filed 2026-07-20T15:34Z
and PR #171 merged 2026-07-20T19:46Z, about four hours later. The issue was written
against an intended shape that the implementation then deliberately did not take.

Weight this correctly. The two lanes have different stakes, which this issue itself
names: a missed linkage exemption blocks a merge, a missed review-skip wastes budget.
So #171 is not proof the two must match. It is evidence of how this org reasons about
failure direction when a bot-classification predicate is on the table — and that
reasoning, applied here, points the same way (§3).

2. There is no account-type surface in the if: that is both available and reliable

No context property exposes account type. The Actions contexts reference lists
exactly three actor properties — github.actor, github.actor_id,
github.triggering_actor. There is no github.actor_type. Type detection therefore
has to read the event payload, which makes it event-shape-dependent in a reusable
workflow that deliberately runs on non-pull_request events (the current job if:
guards github.event_name != 'pull_request' precisely so other events still run).

github.event.sender is not guaranteed. The webhook payload reference states that
most payloads include sender, and warns explicitly that you "shouldn't assume
sender always identifies the person who caused an event" — it can be populated with
the ghost placeholder when GitHub cannot resolve a user. On a payload without
sender, github.event.sender.type evaluates to the empty string, and any naive
predicate silently classifies the run as whatever the empty string maps to.

github.actor is not the PR author. Documented: github.actor is "the username of
the user that triggered the initial workflow run… If the workflow run is a re-run, this
value may differ from github.triggering_actor." Confirmed live on this org's runs — a
re-run of a melodic-standards-sync[bot] PR reports actor=melodic-standards-sync[bot]
(type Bot) with triggering_actor=kyle-sexton (type User):

pr-issue-linkage | actor=melodic-standards-sync[bot](Bot) | trig=kyle-sexton(User) | pull_request_target
pr-title         | actor=melodic-standards-sync[bot](Bot) | trig=kyle-sexton(User) | pull_request

So a type check on github.triggering_actor flips to User on any human re-run, and a
type check on sender follows whoever fired the event action — reopened and
ready_for_review (two of this lane's three trigger types) are commonly fired by a
human on a bot-authored PR. Keying on the author instead
(github.event.pull_request.user.type) is a different predicate than the one in place
today, not a drop-in swap.

endsWith('[bot]') is not a sound proxy for the type. A Bot-type account whose
login does not carry the suffix exists right now:

$ gh api users/copilot-swe-agent%5Bbot%5D --jq '"\(.login)\t\(.type)\t\(.id)"'
Copilot Bot 198982749

(Reproduction note: that account resolves only through the copilot-swe-agent[bot]
path — gh api users/Copilot returns 404. The point stands either way: the login field
GitHub reports for a live Bot account is Copilot, with no [bot] suffix.)

For completeness, the REST pulls schema documents user.type as a required string
and does not enumerate its permitted values, so == 'Bot' is matching against a field
whose value set is not part of the documented contract.

3. Failure direction inverts — and the blast radius is a required check

This is the reason that decides the issue.

claude-security-review.yml carries the identical skip-actors input, the
identical default (dependabot[bot],claude[bot],melodic-ai[bot],melodic-standards-sync[bot]),
and the identical predicate:

&& !contains(format(',{0},', inputs.skip-actors), format(',{0},', github.actor))

That lane is the one whose check is designed to be required. The README is explicit
about what that means:

Every legitimate non-run reads as success to a ruleset. Four are name-stable job-level
skips: a fork PR, an out-of-scope PR, a skip-listed actor, a kill-switched lane.

Now compare how each design fails:

• Name list (incumbent). Goes stale → the unlisted bot's PR gets reviewed.
  Wasted budget on an advisory lane. Fails toward more review, and the waste is
  visible and self-correcting.
• Type detection. A new bot appears → its PR is silently not reviewed, and on
  the security lane that non-run reports green to a required check. Fails toward less
  coverage, silently, on exactly the agent-authored PRs that most warrant review —
  melodic-ai[bot], codex-connector[bot], and any future coding agent.

A stale name list costs runner minutes. A blanket type skip converts "no security review
happened" into a passing required check. Those are not the same class of error, and the
cheaper failure belongs on the side that fails more often.

Answering this issue's own open question directly — "Are there bots whose PRs should
still be reviewed?" — yes, and the set is growing. melodic-ai[bot] and
codex-connector[bot] author substantive code. They are skipped today for a specific
reason (the self-trigger ban: an agent must not trigger its own review lane), not
because bot-authored code is exempt from review. Any future agent-authored PR from an
account this org has not banned should be reviewed, and type detection removes the
ability to say so.

4. The hybrid does not remove the maintenance tax — it moves it to the worse-failing side

The 2026-07-27 gate-audit comment's RECOMMENDED shape was: adopt type detection, retain
the per-caller override as a narrowing escape hatch. Engaging that directly, because it
is the live proposal on this thread and not the blanket version.

It does not eliminate the list. It inverts it — from an enumerated list of bots to
skip into an enumerated list of bots to review. Both lists need maintenance when a
new bot appears. The difference is what happens when the maintainer forgets:

	list goes stale	consequence
Skip-list (today)	new bot not listed	its PR is reviewed — budget wasted, visible
Review-allowlist (hybrid)	new bot not listed	its PR is not reviewed — silent, and green on the required check

The hybrid keeps the same annual maintenance and buys a strictly worse failure mode. It
is not a middle ground; it is the type-detection failure mode with extra configuration.

5. Dependabot's skip is not a statement about bot-ness

Type detection would also flatten a distinction the security lane documents at length.
dependabot[bot] is in the list for a credential reason, not a bot reason:

A Dependabot-triggered run reads from the separate Dependabot secrets store, never
from Actions or organization secrets… Unless the consumer mirrors
CLAUDE_CODE_OAUTH_TOKEN into that store, the review cannot authenticate, and because
this lane FAILS CLOSED the required check then blocks every Dependabot PR until an
operator provisions it.

That entry encodes an operational precondition with a specific remedy (mirror the
secret, then you may remove it). "It is a Bot" cannot express that, cannot be lifted
per-consumer once the secret is mirrored, and would silently absorb the one list entry
that has a documented exit condition.

6. The cost this issue was filed to eliminate has already been eliminated

Measured rather than asserted. Every claude-review-lane workflow run in Actions
retention (2026-07-12 → 2026-07-29) across ci-workflows, standards,
claude-code-plugins, provisioning, github-iac, and dotfiles whose actor was
melodic-standards-sync[bot], melodic-ai[bot], or claude[bot], resolved at
job level (run-level conclusions are misleading here — a run reports success when
the selector job succeeds and the review job skips):

• 62 distinct bot-actor review runs
• 49 review jobs skipped
• 13 review jobs actually executed — 12 × melodic-standards-sync[bot] in
  ci-workflows, 1 × melodic-ai[bot] in claude-code-plugins

All 13 predate centralization. The last one ran 2026-07-27T00:03:43Z, about 12.6
hours before cf666f6 (2026-07-27T12:41:05Z) replaced the dependabot[bot]-only
default with the current four names. Zero bot-actor review jobs have executed since.

The lifetime cost of the enumerated list, fleet-wide, is 13 advisory reviews — and the
leak is already closed. Adoption would be paying the risks in §2–§5 to fix a leak that
measurement shows has stopped.

7. What is still open is a pin/override problem, not a predicate problem

The residual drift is real but orthogonal. claude-code-plugins pins e295107 (whose
default was still dependabot[bot] alone) and overrides the input:

skip-actors: "dependabot[bot],melodic-standards-sync[bot]"

That override is missing claude[bot] and melodic-ai[bot] — the exact per-caller
staleness this issue describes, live today. The fix is finishing the pin sweep and
dropping the override so the caller inherits the centralized default (which is what
standards#286's component already does). Type detection would ship through the same
pin bump and would not reach that repo any sooner.

---

Verdict

REJECT. Keep the enumerated skip-actors list, centralized in the reusable and
inherited by callers.

Durable reasons, in the order they bind:

1. Blanket type detection silently converts "not reviewed" into a green required
   security check; the enumerated list fails toward wasted budget instead
   (§3).
2. The hybrid does not avoid this — it inverts the list into a review-allowlist whose
   staleness fails in that same silent direction (§4).
3. No reliable in-if: type surface exists: no actor_type context, sender not
   guaranteed, github.actor ≠ author, and endsWith('[bot]') already has a live
   counterexample (§2).
4. dependabot[bot]'s entry encodes a secret-store precondition that account type
   cannot express (§5).
5. Measured benefit is zero — 13 wasted reviews lifetime, none since 2026-07-27 (§6).

This reject is not permanent, and here is the trigger that would reopen it. If a
future GitHub Actions release adds a first-class actor-type context (an
github.actor_type, or a documented, enumerated type on a guaranteed payload field),
reasons §2 and part of §3 weaken materially. Reasons §3-required-check, §4, and §5 are
design facts about this fleet and would still need answering — most likely by scoping
any type check to the advisory claude-review lane and leaving claude-security-review
on the enumerated list. That asymmetry is worth remembering rather than rediscovering.

Issue state — deliberately left open, and why

The 2026-07-29T15:41Z comment set two triggers to close. Trigger 1 (standards#286
merged) is recorded as met. This comment satisfies trigger 2: the evaluation is
performed and the outcome is recorded as rejected-with-reason.

Not closing it anyway. This issue carries needs-human, whose definition is
"Human-in-the-loop required; autonomous sessions must not resolve items carrying this."
The 2026-07-27 gate-audit comment reached the same conclusion independently
("stays gated. Unanimous 3/3… cross-cutting, human-call"). An autonomous session
closing it would resolve a human-gated item, so the label outranks the close trigger it
would otherwise satisfy. Closing this issue is a human action, and it is now unblocked
— both triggers are met.

No follow-up work item filed: the verdict adopts no change. The §7 pin/override drift in
claude-code-plugins is flagged here for the operator rather than filed, since it
belongs to the ongoing pin-rollout arc and not to this issue's subject.

This comment was written by an AI agent (account-type evaluation, trigger 2), on the
operator's behalf.