Skip to content

review-marker.sh keys reviewer markers to the session worktree, not the reviewed one #565

Closed

Description

@EricAndrechek

What

.claude/hooks/review-marker.sh keys a reviewer marker to the session's working directory, not the worktree the reviewer actually examined. A review of branch B, run from a session whose cwd is a worktree on branch A, writes tmp/<reviewer>-passed-<A's HEAD> — attesting code nobody reviewed, while the reviewed branch gets nothing.

This is the #454 failure mode with a concrete reproduction.

Where

.claude/hooks/review-marker.sh:86:

head_sha=$(git rev-parse HEAD 2>/dev/null || echo "")

That resolves against the hook's cwd — the session's worktree. Nothing in the hook consults the path the subagent was told to work in. .claude/hooks/agent-bash-gate.sh:118 reads the same way on the checking side, so the two agree with each other and are wrong together.

Observed

While driving the 7-branch stack (2026-09-04): the session's worktree was .worktrees/pre-chtypes-prep on stack/4-seams (#553 @ c807b460). Two reviewers were dispatched against .worktrees/stack5-reorder on stack/5-positional-wire (#554 @ 0fb4197c), with the worktree path stated explicitly in the prompt. Both reviewed the right code and returned verdicts. The ship_it wrote:

tmp/pre-push-reviewer-passed-c807b4600f3e63d4c99c2e55b03bd62915f431f3

— #553's HEAD. #554 got no marker at all, and had to fall back to scripts/skip-pre-push-review.sh to record a review that genuinely happened.

No harm landed in this instance: #553 had already earned its own markers from reviews that did examine its code, so the stray write re-touched an already-valid file. That is luck, not a property of the mechanism.

Why it matters

The gate's purpose is to make "this exact HEAD was reviewed" checkable. As written it means "some review finished while this session's worktree was on this HEAD". Any session with a worktree open on branch A can mint A's markers from a review of anything else — including a review that never looked at A. Since the parallel-agent worktree pattern in ~/.claude/CLAUDE.md is the normal way work gets split here, this is the common case, not an edge one.

Options

  1. Have the hook resolve HEAD in the reviewed worktree — the subagent's prompt names the path, or the reviewer could echo the SHA it reviewed in a parseable line the hook consumes (symmetric with how it already parses VERDICT:).
  2. Have the reviewer emit REVIEWED-SHA: <sha> and refuse to write a marker when it is absent or disagrees with the session's HEAD — fail closed rather than mislabel.
  3. Refuse to write any marker when git rev-parse --show-toplevel differs from the reviewed path, and say so, so the operator reaches for the audited skip deliberately rather than discovering the gap later.

(2) or (3) seem preferable to (1): both make the mismatch loud instead of silently attributing a review to the wrong commit.

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/infraCI, build, deploy, Docker, releasebugSomething isn't working

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions