Skip to content

Commit ea78975

Browse files
authored
fix(selfhost): remove Claude reviewer tools (#5105)
1 parent b1ab5c2 commit ea78975

2 files changed

Lines changed: 25 additions & 8 deletions

File tree

src/selfhost/ai.ts

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -882,13 +882,26 @@ export function createClaudeCodeAi(parentEnv: Record<string, string | undefined>
882882
const prompt = toCliPrompt(options, systemAppend);
883883
const spawn = spawnImpl ?? (await defaultSpawn());
884884
const cwd = await isolatedCliCwd();
885-
// bypassPermissions (not "plan"): --disallowedTools already forbids every mutating/networked tool, so
886-
// nothing left needs an interactive approval prompt -- and this call has no TTY to answer one anyway.
887-
// "plan" activates Claude Code's full interactive Plan-Mode WORKFLOW (explore, draft a plan, wait for
888-
// ExitPlanMode approval), not just a permission restriction, which confused the model into treating a
889-
// one-shot review request as an interactive planning session instead of returning a JSON verdict
890-
// (#observability-plan-mode-injection-lookalike, live in ai_review_provider_unparseable_exhausted).
891-
const args = ["--print", "--output-format", "json", "--model", claudeModel, "--permission-mode", "bypassPermissions", "--effort", effort, "--disallowedTools", "Bash,Edit,Write,WebFetch,WebSearch"];
885+
// Keep bypassPermissions (not "plan") only to avoid a headless approval prompt; the actual boundary is
886+
// tool removal. --tools "" removes every built-in tool, --strict-mcp-config prevents user/home MCP config
887+
// from loading, and mcp__* is a defense-in-depth deny for CLIs that still have MCP tools available. This
888+
// avoids a brittle mutating-tool denylist while preserving the one-shot JSON review flow.
889+
const args = [
890+
"--print",
891+
"--output-format",
892+
"json",
893+
"--model",
894+
claudeModel,
895+
"--permission-mode",
896+
"bypassPermissions",
897+
"--effort",
898+
effort,
899+
"--tools",
900+
"",
901+
"--strict-mcp-config",
902+
"--disallowedTools",
903+
"mcp__*",
904+
];
892905
// A dedicated system-prompt-file channel (not textual stdin-prepending) marks repo instructions
893906
// unambiguously SYSTEM rather than author-controlled content -- see writeClaudeSystemPromptFile's doc
894907
// comment for why the textual-prepend approach this replaces was itself the bug.

test/unit/selfhost-ai.test.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1316,7 +1316,7 @@ describe("subscription CLI helpers + fail-safe", () => {
13161316
expect(capturedInput).toBe("Review this diff.");
13171317
});
13181318

1319-
it("Claude Code runs with --permission-mode bypassPermissions, not plan (#observability-plan-mode-injection-lookalike): disallowedTools already forbids every mutating tool, and 'plan' activates the interactive Plan-Mode workflow instead of just restricting permissions", async () => {
1319+
it("Claude Code disables all built-in and MCP tools while keeping bypassPermissions headless (#observability-plan-mode-injection-lookalike)", async () => {
13201320
let seen: string[] = [];
13211321
const cap: StubSpawn = async (_c, a) => {
13221322
seen = a;
@@ -1325,6 +1325,10 @@ describe("subscription CLI helpers + fail-safe", () => {
13251325
await createClaudeCodeAi({ CLAUDE_CODE_OAUTH_TOKEN: "t" }, cap).run("", { prompt: "x" });
13261326
expect(seen[seen.indexOf("--permission-mode") + 1]).toBe("bypassPermissions");
13271327
expect(seen).not.toContain("plan");
1328+
expect(seen[seen.indexOf("--tools") + 1]).toBe("");
1329+
expect(seen).toContain("--strict-mcp-config");
1330+
expect(seen[seen.indexOf("--disallowedTools") + 1]).toBe("mcp__*");
1331+
expect(seen[seen.indexOf("--disallowedTools") + 1]).not.toContain("Bash");
13281332
});
13291333

13301334
it("chat-only CLIs reject embeds so the chain routes embeddings to an embed-capable provider (Claude review + ollama embed)", async () => {

0 commit comments

Comments
 (0)