Skip to content

Counter-evidence for review autonomy: human reviewer found all critical findings on dispatch infrastructure PR #5274 #5331

Description

@fullsend-ai-retro

What happened

On PR #5274 (fix dispatch SHA resolution for issue_comment events), the review agent ran 5 review rounds across 4 commits and produced 4 substantive findings: 1 HIGH (dispatch-sync violation — correctly caught), 1 MEDIUM (architectural-violation for gh api in workflow YAML), and 2 procedural/informational findings.

Human reviewer waynesun09 used a multi-agent review squad (4 independent agents: 2x Claude, Grok, Codex) and found 6 additional findings that the review agent completely missed:

  • HIGH: per-repo workflow_call mode not covered — The fix only worked for per-org workflow_dispatch deployment. Per-repo installations still fell back to the wrong SHA. This was the most critical finding.
  • HIGH: harness-run job doesn't pass pr-head-sha — Custom harness agents retained the bug. Filed as #5329.
  • MEDIUM: availability regression — Consolidating the fork-check into the new resolve step expanded the hard-failure surface from one stage (fix) to all stages.
  • MEDIUM: dispatch-sync-drift — Missing pr-check guard in the if: condition on reusable-dispatch.yml.
  • MEDIUM: test coverage gap — Tests exercise an unchanged function, not the actual fix.
  • MEDIUM: silent-fallback2>/dev/null || true silently reintroduces the bug under payload corruption.

Notably, the review agent saw the || true pattern in Round 1 and concluded it was "intentional and safe" — the opposite of what the human reviewer correctly identified as a latent bug (#4021 documents a prior instance of this same agent error pattern).

What could go better

The review agent's approach reviewed each file in relative isolation rather than tracing the data flow end-to-end across deployment modes. The human reviewer's advantage came from three capabilities the agent currently lacks:

  1. Cross-deployment-mode tracing: Following the SHA from its origin through dispatch, payload construction, action.yml reconciliation, and downstream CLI consumers across per-org, per-repo, and harness-run modes (partially addressed by #1275 and #1725).
  2. Blast-radius analysis for refactored code: Recognizing that consolidating a fix-only guard into a broader step expanded the failure surface (partially addressed by #925 and #2844).
  3. Challenging error-swallowing patterns: The agent defaulted to "assume the author is right" on || true rather than analyzing failure modes (addressed by #4021).

Confidence: HIGH on the gap assessment. The evidence is clear and well-documented across the PR review timeline. The human reviewer's multi-agent approach was particularly effective — using diverse models to cover different analysis angles.

Proposed change

This is a tracking issue documenting counter-evidence for review agent autonomy on dispatch/infrastructure PRs. The agent scored 4 substantive findings vs. the human's 10 (4 agent + 6 missed), with the human catching both HIGH-severity issues.

No autonomy expansion is warranted for this PR class. The following existing issues must be resolved before reconsidering:

  • #1275 — trace workflow_call chains
  • #1725 — parity check between org-mode and per-repo dispatch
  • #925 / #2844 — scope expansion and error-propagation regression detection
  • #4021 — challenge || true error-swallowing patterns
  • The test-adequacy proposal from this retro

Positive note: The agent's Round 1 HIGH finding (dispatch-sync violation between the two workflow files) was the single most valuable early review catch. The agent correctly applies AGENTS.md conventions (architectural-violation, protected-path) and turns around reviews in 14–16 minutes.

Validation criteria

Track across 3+ subsequent dispatch infrastructure PRs (changes to dispatch.yml, reusable-dispatch.yml, or action.yml). When the review agent's findings fully cover human review findings on this class of change — no missed HIGH or MEDIUM findings — file a corresponding positive-evidence tracking issue. At that point, consider whether the protected-path human-approval requirement for .github/workflows/ can be scoped to exclude changes already validated by the agent.


Generated by retro agent from #5274

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions