Skip to content

Review agent should read PR issue comments to detect known build/artifact failures before approving #3906

Description

@fullsend-ai-retro

What happened

On conforma/cli PR #3380, Renovate created a security dependency bump (oras-go v2.6.0→v2.6.1) targeting release-v0.8. Three seconds after PR creation (06:17:48Z), Renovate posted an issue comment warning: "Artifact update problem — Renovate failed to update an artifact related to this branch. You probably do not want to merge this PR as-is." The warning included specific details: go mod tidy failed because knative.dev/pkg@latest no longer contains packages that transitive dependencies (via tektoncd) require.

The review agent started at 06:20:53Z, completed at 06:24:58Z, and submitted an APPROVED review with body "Looks good to me." It then applied ready-for-merge, dependencies, and go labels. The PR was auto-closed by Renovate 7 days later without being merged.

The identical pattern occurred on sibling PR #3379 (release-v0.7): same artifact failure warning from Renovate, same "Looks good to me" approval from the review agent, same ready-for-merge label.

Meanwhile, on PR #3378 (main branch, same update, no artifact failure), the review agent produced a substantive Comment verdict with a medium-severity finding about tools/go.mod not being updated — a finding that a human maintainer later acknowledged as a good catch and created PR #3391 to address.

What could go better

The review agent's pr-review skill (step 2: Fetch PR context) reads PR metadata, the diff, and linked issues, but does NOT read existing PR issue comments. This means contextual signals from other bots — artifact failure warnings from Renovate, build failure reports from CI bots, or advisory comments from other tools — are completely invisible to the review agent.

In this case, Renovate's warning was posted as a PR issue comment 3 seconds after PR creation, well before the review started. The review agent had access to this information via gh api repos/{owner}/{repo}/issues/{number}/comments but its process does not include this step. The result was an approval on a PR that the creating bot itself warned should not be merged.

The contrast with PR #3378 (main branch) is instructive: when the artifact update succeeded and the diff was complete, the review agent produced a useful review. The quality gap is specifically about missing contextual signals from PR comments, not about the agent's analytical capability.

Confidence: High that this is the root cause. The pr-review SKILL.md step 2 explicitly lists what context is fetched, and PR issue comments are not included. High that this is a recurring pattern — any Renovate PR with artifact failures will exhibit the same blind spot. Medium confidence on the implementation approach — reading all PR comments could add noise; the solution should be targeted at bot warning detection.

Proposed change

Add a new step to the pr-review skill (between current steps 2 and 3) that reads existing PR issue comments and scans for known failure/warning patterns:

  1. In skills/pr-review/SKILL.md, after step 2 (Fetch PR context), add step 2b:

    ### 2b. Scan existing PR comments for build/artifact failure signals
    
    Fetch PR issue comments:
    gh api "repos/${REPO_FULL_NAME}/issues/${PR_NUMBER}/comments" --jq '.[].body'
    
    Scan for known failure patterns from dependency management bots:
    - Renovate: "Artifact update problem" or "failed to update an artifact"
    - Dependabot: "Dependabot cannot update" or "compatibility score"
    - General: "go mod tidy" failures, lockfile generation failures
    
    If a bot-authored comment contains an explicit warning that the PR 
    should not be merged as-is, record this as a blocking signal. Do NOT
    approve the PR. Instead, use comment-only verdict with a finding:
    - category: build-integrity
    - severity: high
    - description: reference the specific bot warning
    
  2. To limit token cost, apply these constraints:

    • Only read comments from bot accounts (user type Bot)
    • Only scan the first 10 comments (failure warnings are typically posted immediately)
    • Only scan for known warning patterns, not general commentary
  3. In the verdict determination logic (step 7), treat the presence of an unresolved artifact failure warning as a hard block on approve verdict, similar to how protected-path findings block approval.

This change belongs in fullsend-ai/fullsend (or fullsend-ai/agents) at the pr-review skill level since it applies universally to all repos — any repo using Renovate, Dependabot, or similar tools benefits.

Validation criteria

  1. On the next 5 Renovate PRs that include an "Artifact update problem" warning comment, the review agent should NOT approve. Instead it should produce a comment-only or changes-requested verdict referencing the artifact failure.
  2. On Renovate PRs without artifact warnings (clean dependency bumps), the review agent should continue to approve normally — no false positives from the comment scanning.
  3. The ready-for-merge label should NOT be applied to any PR where the review agent detects an unresolved artifact failure warning.
  4. Token cost increase from reading bot comments should be minimal (< 5% increase per review) since the scan is limited to bot-authored comments with pattern matching.
  5. Testable by replaying the review agent on PR chore(deps): update dependency vitest to v4.1.10 #3380's state: the agent should detect Renovate's artifact warning and decline to approve.

Generated by retro agent from conforma/cli#3380

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    Status
    Todo

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions