fix(review): stop claude -p swallowing the configured skill as a slash command

6e76d2c·

peyton-alt·1w ago·2 files·+74 added/-2 removed

The composed prompt leads with the configured skill invocation line (e.g. /pr-review-toolkit:review-pr). claude -p expands a leading slash token as a command — observed live in two runs: it resolved to the BUILT-IN /review skill with the entire composed prompt as its arguments, interpolating the configured skill name where a PR number belongs. The configured skill never ran; the built-in maximum-breadth PR-review harness ran instead (its diff-proportional line-by-line finder alone took 11.4 of the run's 15 minutes on a 102KB scope).

Prefix a prose preamble whenever the composed prompt starts with "/", instructing the agent to invoke the skill lines via the Skill tool — expansion is impossible once the first byte isn't a slash. Add Skill to the reviewer allowlist so headless invocation is pre-approved.

Co-Authored-By: Claude Fable 5 noreply@anthropic.com

Sessions

5ffb689d299fView transcript

Changes

2

• cmd/entire/cli/agent/claudecode

• Mreviewer.go+18/-2

  • Mreviewer_test.go+56

47 unmodified lines

48
49
50
51
51
52
53
54
3 unmodified lines

58
59
60
61
61
62
63
64
5 unmodified lines

70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91

47 unmodified lines

// modify the repo. --allowedTools ADDS to the user's own permission config;
// it cannot revoke anything.
var reviewToolAllowlist = []string{
    "Read", "Grep", "Glob", "Task", "TodoWrite",
    "Read", "Grep", "Glob", "Task", "TodoWrite", "Skill",
    "Bash(git diff:*)", "Bash(git log:*)", "Bash(git show:*)",
    "Bash(git status:*)", "Bash(git blame:*)", "Bash(git rev-parse:*)",
    "Bash(git merge-base:*)", "Bash(git ls-files:*)", "Bash(git branch:*)",
3 unmodified lines

// buildReviewCmd builds the exec.Cmd for a claude review run.
// Exposed at package level for test inspection of argv and env.
func buildReviewCmd(ctx context.Context, cfg reviewtypes.RunConfig) *exec.Cmd {
    prompt := review.ComposeReviewPrompt(cfg)
    prompt := guardSlashExpansion(review.ComposeReviewPrompt(cfg))
    args := []string{
        "-p", prompt,
        "--output-format", "stream-json", "--verbose",
5 unmodified lines

return cmd
}

// guardSlashExpansion prevents claude -p from interpreting the prompt as a
// slash command. The composed prompt leads with the configured skill line
// (e.g. "/pr-review-toolkit:review-pr"); print mode expands a leading slash
// token as a command invocation — observed to resolve to the BUILT-IN
// /review skill with the entire prompt (skill name included) as its
// arguments, so the configured skill never ran and the built-in PR-review
// harness treated the skill name as a PR number. A prose preamble makes
// expansion impossible while telling the agent to invoke the skill lines
// properly via the Skill tool (which the allowlist pre-approves).
func guardSlashExpansion(prompt string) string {
    if !strings.HasPrefix(prompt, "/") {
        return prompt
    }
    return "Perform the review below. Lines starting with "/" name configured review skills: invoke each with the Skill tool and follow it.\n\n" + prompt
}

// parseClaudeOutput converts claude's --output-format stream-json --verbose
// stdout into a stream of Events. Each stdout line is one JSON envelope:
//   - {"type":"system",...}      session metadata / hooks; swallowed

Mcmd/entire/cli/agent/claudecode/reviewer.go+18/-2

443 unmodified lines

444
445
446
447
448
449
450
451
452
453
454
455
456
457
458
459
460
461
462
463
464
465
466
467
468
469
470
471
472
473
474
475
476
477
478
479
480
481
482
483
484
485
486
487
488
489
490
491
492
493
494
495
496
497
498
499
500
501
502

443 unmodified lines

}
    }
}

// TestReviewer_PromptNeverStartsWithSlash guards against claude -p's
// slash-command expansion swallowing the composed prompt. Observed live: a
// prompt beginning with the configured skill line
// "/pr-review-toolkit:review-pr" was expanded by the harness as a slash
// command — it resolved to the BUILT-IN /review skill with the entire
// composed prompt (skill name included) as its arguments, so the configured
// skill never ran and the built-in PR-review harness interpolated the skill
// name where a PR number belongs. The reviewer must deliver a prompt that
// cannot trigger expansion, while still instructing the agent to invoke the
// configured skills via the Skill tool.
func TestReviewer_PromptNeverStartsWithSlash(t *testing.T) {
    t.Parallel()
    cmd := buildReviewCmd(context.Background(), reviewtypes.RunConfig{
        Skills: []string{"/pr-review-toolkit:review-pr"},
    })
    prompt := cmd.Args[2]
    if strings.HasPrefix(prompt, "/") {
        t.Fatalf("prompt starts with %q — claude -p will expand it as a slash command:\n%s", "/", prompt)
    }
    if !strings.Contains(prompt, "/pr-review-toolkit:review-pr") {
            t.Errorf("prompt lost the configured skill invocation:\n%s", prompt)
    }
    if !strings.Contains(prompt, "Skill tool") {
            t.Errorf("prompt missing instruction to invoke skills via the Skill tool:\n%s", prompt)
    }
}

// TestReviewer_PromptWithoutLeadingSlashUnchanged verifies the guard only
// fires when needed: a prompt that already starts with prose is passed
// through without the skill-invocation preamble.
func TestReviewer_PromptWithoutLeadingSlashUnchanged(t *testing.T) {
    t.Parallel()
    cmd := buildReviewCmd(context.Background(), reviewtypes.RunConfig{
        PromptOverride: "Review the change described below.",
    })
    if got := cmd.Args[2]; got != "Review the change described below." {
            t.Errorf("prose prompt was modified: %q", got)
    }
}

// TestReviewer_AllowlistIncludesSkillTool verifies the child can actually
// invoke the configured skills headlessly.
func TestReviewer_AllowlistIncludesSkillTool(t *testing.T) {
    t.Parallel()
    cmd := buildReviewCmd(context.Background(), reviewtypes.RunConfig{Skills: []string{"/x"}})
    for i, arg := range cmd.Args {
        if arg == "--allowedTools" {
            if !strings.Contains(cmd.Args[i+1], "Skill") {
                    t.Errorf("allowlist missing Skill tool: %q", cmd.Args[i+1])
            }
            return
        }
    }
    t.Fatal("--allowedTools not found")
}

Mcmd/entire/cli/agent/claudecode/reviewer_test.go+56