Selaa lähdekoodia

test: 添加 FileEditTool/permissions/filterToolsByDenyRules 测试

- FileEditTool/utils.test.ts: 24 tests (normalizeQuotes, stripTrailingWhitespace, findActualString, preserveQuoteStyle, applyEditToFile)
- permissions/permissions.test.ts: 13 tests (getDenyRuleForTool, getAskRuleForTool, getDenyRuleForAgent, filterDeniedAgents)
- tools.test.ts: 扩展 5 tests (filterToolsByDenyRules 过滤逻辑)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
claude-code-best 3 viikkoa sitten
vanhempi
sitoutus
a28a44f9f7

+ 59 - 1
src/__tests__/tools.test.ts

@@ -1,5 +1,6 @@
 import { describe, expect, test } from "bun:test";
-import { parseToolPreset } from "../tools";
+import { parseToolPreset, filterToolsByDenyRules } from "../tools";
+import { getEmptyToolPermissionContext } from "../Tool";
 
 describe("parseToolPreset", () => {
   test('returns "default" for "default" input', () => {
@@ -22,3 +23,60 @@ describe("parseToolPreset", () => {
     expect(parseToolPreset("custom-preset")).toBeNull();
   });
 });
+
+// ─── filterToolsByDenyRules ─────────────────────────────────────────────
+
+describe("filterToolsByDenyRules", () => {
+  const mockTools = [
+    { name: "Bash", mcpInfo: undefined },
+    { name: "Read", mcpInfo: undefined },
+    { name: "Write", mcpInfo: undefined },
+    { name: "mcp__server__tool", mcpInfo: { serverName: "server", toolName: "tool" } },
+  ];
+
+  test("returns all tools when no deny rules", () => {
+    const ctx = getEmptyToolPermissionContext();
+    const result = filterToolsByDenyRules(mockTools, ctx);
+    expect(result).toHaveLength(4);
+  });
+
+  test("filters out denied tool by name", () => {
+    const ctx = {
+      ...getEmptyToolPermissionContext(),
+      alwaysDenyRules: {
+        localSettings: ["Bash"],
+      },
+    };
+    const result = filterToolsByDenyRules(mockTools, ctx as any);
+    expect(result.find((t) => t.name === "Bash")).toBeUndefined();
+    expect(result).toHaveLength(3);
+  });
+
+  test("filters out multiple denied tools", () => {
+    const ctx = {
+      ...getEmptyToolPermissionContext(),
+      alwaysDenyRules: {
+        localSettings: ["Bash", "Write"],
+      },
+    };
+    const result = filterToolsByDenyRules(mockTools, ctx as any);
+    expect(result).toHaveLength(2);
+    expect(result.map((t) => t.name)).toEqual(["Read", "mcp__server__tool"]);
+  });
+
+  test("returns empty array when all tools denied", () => {
+    const ctx = {
+      ...getEmptyToolPermissionContext(),
+      alwaysDenyRules: {
+        localSettings: mockTools.map((t) => t.name),
+      },
+    };
+    const result = filterToolsByDenyRules(mockTools, ctx as any);
+    expect(result).toHaveLength(0);
+  });
+
+  test("handles empty tools array", () => {
+    const ctx = getEmptyToolPermissionContext();
+    expect(filterToolsByDenyRules([], ctx)).toEqual([]);
+  });
+});

+ 164 - 0
src/tools/FileEditTool/__tests__/utils.test.ts

@@ -0,0 +1,164 @@
+import { mock, describe, expect, test } from "bun:test";
+
+// Mock log.ts to cut the heavy dependency chain
+mock.module("src/utils/log.ts", () => ({
+  logError: () => {},
+  logToFile: () => {},
+  getLogDisplayTitle: () => "",
+  logEvent: () => {},
+  logMCPError: () => {},
+  logMCPDebug: () => {},
+  dateToFilename: (d: Date) => d.toISOString().replace(/[:.]/g, "-"),
+  getLogFilePath: () => "/tmp/mock-log",
+  attachErrorLogSink: () => {},
+  getInMemoryErrors: () => [],
+  loadErrorLogs: async () => [],
+  getErrorLogByIndex: async () => null,
+  captureAPIRequest: () => {},
+  _resetErrorLogForTesting: () => {},
+}));
+
+const {
+  normalizeQuotes,
+  stripTrailingWhitespace,
+  findActualString,
+  preserveQuoteStyle,
+  applyEditToFile,
+  LEFT_SINGLE_CURLY_QUOTE,
+  RIGHT_SINGLE_CURLY_QUOTE,
+  LEFT_DOUBLE_CURLY_QUOTE,
+  RIGHT_DOUBLE_CURLY_QUOTE,
+} = await import("../utils");
+
+// ─── normalizeQuotes ────────────────────────────────────────────────────
+
+describe("normalizeQuotes", () => {
+  test("converts left single curly to straight", () => {
+    expect(normalizeQuotes(`${LEFT_SINGLE_CURLY_QUOTE}hello`)).toBe("'hello");
+  });
+
+  test("converts right single curly to straight", () => {
+    expect(normalizeQuotes(`hello${RIGHT_SINGLE_CURLY_QUOTE}`)).toBe("hello'");
+  });
+
+  test("converts left double curly to straight", () => {
+    expect(normalizeQuotes(`${LEFT_DOUBLE_CURLY_QUOTE}hello`)).toBe('"hello');
+  });
+
+  test("converts right double curly to straight", () => {
+    expect(normalizeQuotes(`hello${RIGHT_DOUBLE_CURLY_QUOTE}`)).toBe('hello"');
+  });
+
+  test("leaves straight quotes unchanged", () => {
+    expect(normalizeQuotes("'hello' \"world\"")).toBe("'hello' \"world\"");
+  });
+
+  test("handles empty string", () => {
+    expect(normalizeQuotes("")).toBe("");
+  });
+});
+
+// ─── stripTrailingWhitespace ────────────────────────────────────────────
+
+describe("stripTrailingWhitespace", () => {
+  test("strips trailing spaces from lines", () => {
+    expect(stripTrailingWhitespace("hello   \nworld  ")).toBe("hello\nworld");
+  });
+
+  test("strips trailing tabs", () => {
+    expect(stripTrailingWhitespace("hello\t\nworld\t")).toBe("hello\nworld");
+  });
+
+  test("preserves leading whitespace", () => {
+    expect(stripTrailingWhitespace("  hello  \n  world  ")).toBe(
+      "  hello\n  world"
+    );
+  });
+
+  test("handles empty string", () => {
+    expect(stripTrailingWhitespace("")).toBe("");
+  });
+
+  test("handles CRLF line endings", () => {
+    expect(stripTrailingWhitespace("hello   \r\nworld  ")).toBe(
+      "hello\r\nworld"
+    );
+  });
+
+  test("handles no trailing whitespace", () => {
+    expect(stripTrailingWhitespace("hello\nworld")).toBe("hello\nworld");
+  });
+});
+
+// ─── findActualString ───────────────────────────────────────────────────
+
+describe("findActualString", () => {
+  test("finds exact match", () => {
+    expect(findActualString("hello world", "hello")).toBe("hello");
+  });
+
+  test("finds match with curly quotes normalized", () => {
+    const fileContent = `${LEFT_DOUBLE_CURLY_QUOTE}hello${RIGHT_DOUBLE_CURLY_QUOTE}`;
+    const result = findActualString(fileContent, '"hello"');
+    expect(result).not.toBeNull();
+  });
+
+  test("returns null when not found", () => {
+    expect(findActualString("hello world", "xyz")).toBeNull();
+  });
+
+  test("returns null for empty search in non-empty content", () => {
+    // Empty string is always found at index 0 via includes()
+    const result = findActualString("hello", "");
+    expect(result).toBe("");
+  });
+});
+
+// ─── preserveQuoteStyle ─────────────────────────────────────────────────
+
+describe("preserveQuoteStyle", () => {
+  test("returns newString unchanged when no normalization happened", () => {
+    expect(preserveQuoteStyle("hello", "hello", "world")).toBe("world");
+  });
+
+  test("converts straight double quotes to curly in replacement", () => {
+    const oldString = '"hello"';
+    const actualOldString = `${LEFT_DOUBLE_CURLY_QUOTE}hello${RIGHT_DOUBLE_CURLY_QUOTE}`;
+    const newString = '"world"';
+    const result = preserveQuoteStyle(oldString, actualOldString, newString);
+    expect(result).toContain(LEFT_DOUBLE_CURLY_QUOTE);
+    expect(result).toContain(RIGHT_DOUBLE_CURLY_QUOTE);
+  });
+});
+
+// ─── applyEditToFile ────────────────────────────────────────────────────
+
+describe("applyEditToFile", () => {
+  test("replaces first occurrence by default", () => {
+    expect(applyEditToFile("foo bar foo", "foo", "baz")).toBe("baz bar foo");
+  });
+
+  test("replaces all occurrences with replaceAll=true", () => {
+    expect(applyEditToFile("foo bar foo", "foo", "baz", true)).toBe(
+      "baz bar baz"
+    );
+  });
+
+  test("handles deletion (empty newString) with trailing newline", () => {
+    const result = applyEditToFile("line1\nline2\nline3\n", "line2", "");
+    expect(result).toBe("line1\nline3\n");
+  });
+
+  test("handles deletion without trailing newline", () => {
+    const result = applyEditToFile("foobar", "foo", "");
+    expect(result).toBe("bar");
+  });
+
+  test("handles no match (returns original)", () => {
+    expect(applyEditToFile("hello world", "xyz", "abc")).toBe("hello world");
+  });
+
+  test("handles empty original content with insertion", () => {
+    expect(applyEditToFile("", "", "new content")).toBe("new content");
+  });
+});

+ 165 - 0
src/utils/permissions/__tests__/permissions.test.ts

@@ -0,0 +1,165 @@
+import { mock, describe, expect, test } from "bun:test";
+
+// Mock log.ts to cut the heavy dependency chain
+mock.module("src/utils/log.ts", () => ({
+  logError: () => {},
+  logToFile: () => {},
+  getLogDisplayTitle: () => "",
+  logEvent: () => {},
+  logMCPError: () => {},
+  logMCPDebug: () => {},
+  dateToFilename: (d: Date) => d.toISOString().replace(/[:.]/g, "-"),
+  getLogFilePath: () => "/tmp/mock-log",
+  attachErrorLogSink: () => {},
+  getInMemoryErrors: () => [],
+  loadErrorLogs: async () => [],
+  getErrorLogByIndex: async () => null,
+  captureAPIRequest: () => {},
+  _resetErrorLogForTesting: () => {},
+}));
+
+// Mock slowOperations to avoid bun:bundle
+mock.module("src/utils/slowOperations.ts", () => ({
+  jsonStringify: JSON.stringify,
+  jsonParse: JSON.parse,
+  slowLogging: { enabled: false },
+  clone: (v: any) => structuredClone(v),
+  cloneDeep: (v: any) => structuredClone(v),
+  callerFrame: () => "",
+  SLOW_OPERATION_THRESHOLD_MS: 100,
+  writeFileSync_DEPRECATED: () => {},
+}));
+
+const {
+  getDenyRuleForTool,
+  getAskRuleForTool,
+  getDenyRuleForAgent,
+  filterDeniedAgents,
+} = await import("../permissions");
+
+import { getEmptyToolPermissionContext } from "../../../Tool";
+
+// ─── Helper ─────────────────────────────────────────────────────────────
+
+function makeContext(opts: {
+  denyRules?: string[];
+  askRules?: string[];
+}) {
+  const ctx = getEmptyToolPermissionContext();
+  const deny: Record<string, string[]> = {};
+  const ask: Record<string, string[]> = {};
+
+  // alwaysDenyRules stores raw rule strings — getDenyRules() calls
+  // permissionRuleValueFromString internally
+  if (opts.denyRules?.length) {
+    deny["localSettings"] = opts.denyRules;
+  }
+  if (opts.askRules?.length) {
+    ask["localSettings"] = opts.askRules;
+  }
+
+  return {
+    ...ctx,
+    alwaysDenyRules: deny,
+    alwaysAskRules: ask,
+  } as any;
+}
+
+function makeTool(name: string, mcpInfo?: { serverName: string; toolName: string }) {
+  return { name, mcpInfo };
+}
+
+// ─── getDenyRuleForTool ─────────────────────────────────────────────────
+
+describe("getDenyRuleForTool", () => {
+  test("returns null when no deny rules", () => {
+    const ctx = makeContext({});
+    expect(getDenyRuleForTool(ctx, makeTool("Bash"))).toBeNull();
+  });
+
+  test("returns matching deny rule for tool", () => {
+    const ctx = makeContext({ denyRules: ["Bash"] });
+    const result = getDenyRuleForTool(ctx, makeTool("Bash"));
+    expect(result).not.toBeNull();
+    expect(result!.ruleValue.toolName).toBe("Bash");
+  });
+
+  test("returns null for non-matching tool", () => {
+    const ctx = makeContext({ denyRules: ["Bash"] });
+    expect(getDenyRuleForTool(ctx, makeTool("Read"))).toBeNull();
+  });
+
+  test("rule with content does not match whole-tool deny", () => {
+    // getDenyRuleForTool uses toolMatchesRule which requires ruleContent === undefined
+    // Rules like "Bash(rm -rf)" only match specific invocations, not the entire tool
+    const ctx = makeContext({ denyRules: ["Bash(rm -rf)"] });
+    const result = getDenyRuleForTool(ctx, makeTool("Bash"));
+    expect(result).toBeNull();
+  });
+});
+
+// ─── getAskRuleForTool ──────────────────────────────────────────────────
+
+describe("getAskRuleForTool", () => {
+  test("returns null when no ask rules", () => {
+    const ctx = makeContext({});
+    expect(getAskRuleForTool(ctx, makeTool("Bash"))).toBeNull();
+  });
+
+  test("returns matching ask rule", () => {
+    const ctx = makeContext({ askRules: ["Write"] });
+    const result = getAskRuleForTool(ctx, makeTool("Write"));
+    expect(result).not.toBeNull();
+  });
+
+  test("returns null for non-matching tool", () => {
+    const ctx = makeContext({ askRules: ["Write"] });
+    expect(getAskRuleForTool(ctx, makeTool("Bash"))).toBeNull();
+  });
+});
+
+// ─── getDenyRuleForAgent ────────────────────────────────────────────────
+
+describe("getDenyRuleForAgent", () => {
+  test("returns null when no deny rules", () => {
+    const ctx = makeContext({});
+    expect(getDenyRuleForAgent(ctx, "Agent", "Explore")).toBeNull();
+  });
+
+  test("returns matching deny rule for agent type", () => {
+    const ctx = makeContext({ denyRules: ["Agent(Explore)"] });
+    const result = getDenyRuleForAgent(ctx, "Agent", "Explore");
+    expect(result).not.toBeNull();
+  });
+
+  test("returns null for non-matching agent type", () => {
+    const ctx = makeContext({ denyRules: ["Agent(Explore)"] });
+    expect(getDenyRuleForAgent(ctx, "Agent", "Research")).toBeNull();
+  });
+});
+
+// ─── filterDeniedAgents ─────────────────────────────────────────────────
+
+describe("filterDeniedAgents", () => {
+  test("returns all agents when no deny rules", () => {
+    const ctx = makeContext({});
+    const agents = [{ agentType: "Explore" }, { agentType: "Research" }];
+    expect(filterDeniedAgents(agents, ctx, "Agent")).toEqual(agents);
+  });
+
+  test("filters out denied agent type", () => {
+    const ctx = makeContext({ denyRules: ["Agent(Explore)"] });
+    const agents = [{ agentType: "Explore" }, { agentType: "Research" }];
+    const result = filterDeniedAgents(agents, ctx, "Agent");
+    expect(result).toHaveLength(1);
+    expect(result[0]!.agentType).toBe("Research");
+  });
+
+  test("returns empty array when all agents denied", () => {
+    const ctx = makeContext({
+      denyRules: ["Agent(Explore)", "Agent(Research)"],
+    });
+    const agents = [{ agentType: "Explore" }, { agentType: "Research" }];
+    expect(filterDeniedAgents(agents, ctx, "Agent")).toEqual([]);
+  });
+});