Split out of #442 (comment: #442 (comment)).
ai-code-review.yml's own header comment already names this gap:
Scope of the fallback: it covers agents that are unavailable at dispatch
time --- no API key/token secret configured, or the agent's review workflow
file missing or disabled in the caller repo. It does NOT cover an agent that
dispatches fine and then fails partway through its own run, because
gh workflow run returns as soon as the run is queued and reports no run id
to wait on. Failing over on a runtime failure needs the dispatched run to be
located and awaited; tracked separately rather than papered over here.
That comment says "tracked separately" but cites no issue --- this is that
issue.
Why this matters
ucdavis/bcs#612 hit it live, across three pushes, while #442 was still
being diagnosed:
| head |
agent dispatched |
outcome |
98719439 |
claude-review |
cancelled by the next push |
d7d1ca75 |
antigravity-review |
failure (the #442 token bug) |
4ae6f46e |
antigravity-review |
failure (the #442 token bug) |
One PR consumed three automatic review dispatches and got zero automatic
reviews --- the only review it received was a manual dispatch of Claude. The
check goes red, which reads as an ordinary CI failure rather than "your
reviewer never ran," so the gap is easy to miss unless someone is watching
closely.
#442 fixed the specific token bug that caused those two failures, but the
selector's own behavior --- no fallthrough once an agent is dispatched and
then fails --- is unchanged and will reproduce with any other cause of a
post-dispatch failure (quota exhaustion, an upstream outage, a future
regression in one agent's own workflow).
What to decide
Per #442's own comment on this:
If that is the intended behaviour it is worth saying so; if not, a
dispatched-but-failed agent probably wants the same fallthrough.
Either:
- Extend the fallthrough to cover a dispatched-but-failed agent (needs the
dispatched run to be located and awaited, since gh workflow run returns
as soon as the run is queued with no run id to poll), or
- Decide the current one-shot-per-agent behavior is intentional and say so
explicitly in the header comment, replacing "tracked separately" with a
reasoned "not planned" and a pointer to this issue for context.
Not proposing an implementation here --- the polling/run-location mechanism
needs its own design.
Split out of #442 (comment: #442 (comment)).
ai-code-review.yml's own header comment already names this gap:That comment says "tracked separately" but cites no issue --- this is that
issue.
Why this matters
ucdavis/bcs#612hit it live, across three pushes, while #442 was stillbeing diagnosed:
98719439claude-reviewd7d1ca75antigravity-review4ae6f46eantigravity-reviewOne PR consumed three automatic review dispatches and got zero automatic
reviews --- the only review it received was a manual dispatch of Claude. The
check goes red, which reads as an ordinary CI failure rather than "your
reviewer never ran," so the gap is easy to miss unless someone is watching
closely.
#442 fixed the specific token bug that caused those two failures, but the
selector's own behavior --- no fallthrough once an agent is dispatched and
then fails --- is unchanged and will reproduce with any other cause of a
post-dispatch failure (quota exhaustion, an upstream outage, a future
regression in one agent's own workflow).
What to decide
Per #442's own comment on this:
Either:
dispatched run to be located and awaited, since
gh workflow runreturnsas soon as the run is queued with no run id to poll), or
explicitly in the header comment, replacing "tracked separately" with a
reasoned "not planned" and a pointer to this issue for context.
Not proposing an implementation here --- the polling/run-location mechanism
needs its own design.