Skip to content

check-review-execution: give background-agent stubs their own failure_kind, so they are not retried #551

Description

@d-morrison

Split out of #532, which carries it forward from gha#392's closing "Suggested direction". #550 ships #532's other half (the mechanical deny rule) and deliberately does not attempt this one — see the blocker below.

The ask

gha#392 asked:

Worth checking whether the guard can also treat "final message names background agents" as a distinct outcome from a generic stub, since the retry is unlikely to help and the spend is large.

Today check-review-execution.sh classifies this as failure_kind=stub whenever the denial count is low, which marks it retryable. A same-prompt retry of a run that just demonstrated it ignores the synchronous-only instruction is not obviously more likely to recover than gha#198's high-denial pattern is — and gha#536's tally in #532 bears that out: 8 stub attempts across 3 PRs, only 2 recovered on a manual re-run.

Since #548, a distinct kind is cheap to add and immediately visible: it gets its own failure_kind, its own exclusion from the stub-retry, and its own advice in the posted PR comment ("a same-prompt retry is unlikely to recover this") instead of a silent retry and a second charge.

Why #550 did not do it, specifically

The detection must key on a structured field, not prose. That is this repo's own standing rule — classify-gemini-failure.sh's header records why (gha#380 finding 1), and the mid-run quota check keys on api_error_status rather than the result message for the same reason. Matching "background agents" in the final text is exactly the shape that rule forbids.

The structured signal would be a tool_use block naming Agent/Task with run_in_background not false. I could not confirm that field's presence or shape from real evidence, and building a detector on an unverified field is how a check ends up silently matching nothing:

  • The one fixture of this shape, stub-pr171-waiting-background-agents.json, carries only assistant text and no tool_use blocks at all — so it cannot answer the question, and inferring the real file's shape from it is the trap fixtures-are-not-evidence names.
  • The real artifact exists and has not expired (claude-review-execution-32347489886-2-attempt1, on Morrison-Lab/ai-config, expires 2026-09-03), but this session's network policy denies the blob host: connect_rejected, "gateway answered 403 to CONNECT (policy denial or upstream failure)", for productionresultssa14.blob.core.windows.net. Confirmed against the proxy's own status endpoint, not inferred from the curl failure.

What whoever picks this up should do first

Download that artifact from a session that can reach blob storage, and answer one question: does a backgrounded Agent/Task call appear as a tool_use block, and what exactly does its input carry for run_in_background? Two sub-cases matter, because the parameter defaults to true:

  • passed explicitly as true
  • omitted entirely

A detector has to catch both, so run_in_background != false is likely the right test rather than == true. Once confirmed, commit a fixture built from the real shape and the rest is mechanical.

One caution on over-matching

"Any Agent/Task tool_use plus no verdict" is tempting and would need no field introspection at all — but it also matches a synchronous fan-out that stubbed for an unrelated reason, and that case is the gha#185 low-denial stub which genuinely does recover on retry sometimes. Making it non-retryable would be a behaviour regression. The field is what separates the two, which is why it is worth confirming rather than working around.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions