Skip to content

review:code-reviewer resolves an unfindable base to HEAD and reviews an empty diff without saying so #4245

Closed

Description

@kyle-sexton

Observed in .claude/plugins/data/plugin-quality-melodic-software/evidence/c7ff503b-19e2-40d2-b99e-38c747f7fc0f/review-code-reviewer/20260919T205610Z/audit-notes.md, findings F3 and F5 (auditor severity: IMPORTANT, confidence high; mapped to medium per the run contract). It leads the security block on ordering, not on priority, because the failure state is reachable through the plugin's own dispatcher.

Where: agents/code-reviewer.md step 2, the git diff "$(git merge-base ... || echo HEAD)" block. Reproduced twice in scratch repos:

  • No origin remote, clean tree, two commits on feature: the chain resolves to the literal HEAD and git diff HEAD emits 0 lines.
  • A real git clone --depth 1 -b feature file://<bare>, the shape actions/checkout produces by default: the ls-remote --symref fallback finds and fetches main, but git merge-base FETCH_HEAD HEAD fails on shallow history with no shared ancestor, the origin/main fallback has no such ref, and the chain again resolves to HEAD with 0 diff lines.

The reviewer then gets an empty change set with no error and no way to tell "nothing changed" from "I could not find a base", so the output is a clean bill of health on an unreviewed change. The author knew of the adjacent case (the inline comment mentions shallow or single-branch clones lacking origin/$PR_BASE), but the mitigation does not cover the merge-base failure shallowness causes.

skills/fanout/context/run-everything-mode.md line ~51 sets const REVIEW_DIFF = 'HEAD' as the unsubstituted placeholder, commented "Substituted at dispatch by the main thread", and interpolates it into the agent prompt as git diff HEAD: the exact string F3 fails open to. Because the prompt supplies the command, the agent's step-2 chain is bypassed.

Not exercised in the evidence run: the session supplied git diff origin/main...HEAD, so step 2 never fired. The finding rests on the auditor's reproduction, not the packet.

Cheapest fix: replace the terminal || echo HEAD with a sentinel and add one line to step 2: an unresolved base is reported as such and the change set treated as unresolved, not empty. Same for fanout: make the placeholder '<UNRESOLVED>' so a missed substitution fails loudly. Fuller fix: add git rev-parse --is-shallow-repository to the block and, when true, instruct git fetch --unshallow --filter=blob:none before resolving.

Verification: re-run both reproductions. Each must name the base as unresolved and decline to grade, not return clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_01F7GFS5autRMSaea6kSoXzx

Activity

  1. added
    priority: mediumReal value, no hard deadline; normal backlog flow.
    area: securitySecurity-relevant: vulnerability, hardening, or disclosure follow-up.
    agent-readyFully specified and briefed; eligible for autonomous pickup from the frontier.
    work-class: scopedA briefed fix or small feature; blast radius bounded by the brief, tests exist.
    on Sep 19, 2026
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

    agent-readyFully specified and briefed; eligible for autonomous pickup from the frontier.area: securitySecurity-relevant: vulnerability, hardening, or disclosure follow-up.priority: mediumReal value, no hard deadline; normal backlog flow.work-class: scopedA briefed fix or small feature; blast radius bounded by the brief, tests exist.

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions