Unify tool-name classification constants (CL-6809)

Three READ_TOOLS constants had drifted apart under the same name, one
of them (permission's auto-allow gate) security-relevant. Add
src/agent/tool-classification.ts as the single source of truth:

- AUTO_ALLOW_READ_TOOLS: derived from the director read surface minus
  run_shell/web_fetch/web_search (which have their own auto-allow
  rules) plus manage_tasks. Replaces classify.ts's inline
  READ_ONLY_TOOLS with the identical membership — no auto-allow
  behavior change.
- PATH_KEYED_READ_TOOLS: the {read_file} set that was defined
  identically in both compactor.ts and thrash.ts, now shared.
- SEARCH_QUERY_TOOLS: the {grep, search_files} base shared by
  compaction's QUERY_TOOLS (adds list_dir) and thrash's SEARCH_TOOLS
  (deliberately omits it — a repeated list_dir isn't the stuck
  read/search loop thrash watches for).

Adds a pinning test so future drift fails CI instead of spreading
silently.

Author: TheGreatAxios

src/agent/tool-classification.test.ts

@@ -0,0 +1,35 @@
+import { describe, expect, test } from "bun:test";
+import {
+  AUTO_ALLOW_READ_TOOLS,
+  PATH_KEYED_READ_TOOLS,
+  SEARCH_QUERY_TOOLS,
+} from "./tool-classification.js";
+
+// Pins membership so a future edit to any of these sets — or to the director
+// READ_TOOLS they derive from — fails CI instead of silently drifting one
+// call site out of sync with the others (CL-6809).
+describe("AUTO_ALLOW_READ_TOOLS", () => {
+  test("gates auto-allow with exactly this membership", () => {
+    expect([...AUTO_ALLOW_READ_TOOLS].sort()).toEqual(
+      ["grep", "list_dir", "lsp", "manage_tasks", "read_file", "search_files"].sort(),
+    );
+  });
+
+  test("excludes tools with their own, narrower auto-allow logic", () => {
+    for (const tool of ["run_shell", "web_fetch", "web_search"]) {
+      expect(AUTO_ALLOW_READ_TOOLS.has(tool)).toBe(false);
+    }
+  });
+});
+
+describe("PATH_KEYED_READ_TOOLS", () => {
+  test("is read_file only", () => {
+    expect([...PATH_KEYED_READ_TOOLS]).toEqual(["read_file"]);
+  });
+});
+
+describe("SEARCH_QUERY_TOOLS", () => {
+  test("is grep and search_files only", () => {
+    expect([...SEARCH_QUERY_TOOLS].sort()).toEqual(["grep", "search_files"]);
+  });
+});

src/agent/tool-classification.ts

@@ -0,0 +1,57 @@
+/**
+ * Shared tool-name classification constants (CL-6809).
+ *
+ * Three separate READ_TOOLS constants (director tool-sets, session compactor,
+ * subagent thrash tracker) had drifted to different memberships under the
+ * same name, and the permission classifier's auto-allow gate carried a fourth
+ * (READ_ONLY_TOOLS) with no declared relationship to the others. Same name,
+ * different meanings, one of them security-relevant.
+ *
+ * These are genuinely different concepts, not the same list typed four times:
+ *   - the director read surface (tool-sets.ts READ_TOOLS) is "everything a
+ *     read-only leaf may call", including run_shell and the web tools;
+ *   - the auto-allow gate is "never needs an approval prompt", a strict
+ *     subset (shell/web get their own, narrower auto-allow logic) plus
+ *     manage_tasks (side-effect-free, see classify.ts);
+ *   - compaction's re-read dedup and thrash's read tracking both care about
+ *     "read_file specifically, because its result is keyed by path" — this
+ *     one actually was the same set twice, so it is unified here.
+ * Where the concepts differ, the sets stay separate but are derived from the
+ * same base and named for what they mean, so a future difference reads as
+ * intentional instead of drift.
+ */
+
+import { READ_TOOLS as DIRECTOR_READ_TOOLS } from "./directors/tool-sets.js";
+
+/**
+ * read_file: the one read tool whose result is keyed by path, so an older
+ * result for the same path is safely superseded by a newer one. Shared by
+ * compaction's re-read dedup and thrash's read-count bookkeeping — both are
+ * asking the same question ("was this path already read?").
+ */
+export const PATH_KEYED_READ_TOOLS: ReadonlySet<string> = new Set(["read_file"]);
+
+/**
+ * grep / search_files: pattern-keyed query tools whose repeated identical
+ * call reflects current workspace state, not stale history. This is the base
+ * both compaction and thrash build on; each adds/omits list_dir for its own
+ * reason (see compactor.ts's QUERY_TOOLS and thrash.ts's SEARCH_TOOLS).
+ */
+export const SEARCH_QUERY_TOOLS: ReadonlySet<string> = new Set(["grep", "search_files"]);
+
+/**
+ * Tools that never need an approval prompt because they cannot change the
+ * workspace: the director's read surface minus run_shell/web_fetch/web_search
+ * (which get their own, narrower auto-allow rules — see
+ * isAutoAllowedShellCommand and the webfetch/websearch permission classes),
+ * plus manage_tasks (side-effect-free by the time the tool executes — see
+ * classify.ts). SECURITY-RELEVANT: this gates auto-allow. A tool added here
+ * is auto-approved everywhere; get it wrong in either direction deliberately,
+ * not by accident.
+ */
+export const AUTO_ALLOW_READ_TOOLS: ReadonlySet<string> = new Set([
+  ...DIRECTOR_READ_TOOLS.filter(
+    (tool) => tool !== "run_shell" && tool !== "web_fetch" && tool !== "web_search",
+  ),
+  "manage_tasks",
+]);

src/permission/classify.ts

@@ -17,6 +17,7 @@ import {
 import { resolveWorkspacePath } from "./path-restriction.js";
 import type { RootsProvider } from "./worktree-roots.js";
 import { isProductMutationTool, productMutationPaths } from "../agent/product-mutation-tools.js";
+import { AUTO_ALLOW_READ_TOOLS as READ_ONLY_TOOLS } from "../agent/tool-classification.js";
 
 // Read-only tools never need approval as long as they don't touch a restricted
 // path; they cannot change the workspace. `lsp` is included here even though
@@ -30,14 +31,9 @@ import { isProductMutationTool, productMutationPaths } from "../agent/product-mu
 // for a denial to prevent. Every other posix tool is consequential and
 // defaults to the "ask" tier. Catastrophic commands are denied earlier by the
 // authorization plugin, so they never reach here.
-const READ_ONLY_TOOLS = new Set([
-  "read_file",
-  "search_files",
-  "grep",
-  "list_dir",
-  "lsp",
-  "manage_tasks",
-]);
+//
+// Membership lives in tool-classification.ts (AUTO_ALLOW_READ_TOOLS, imported
+// here as READ_ONLY_TOOLS) — see CL-6809.
 
 // Tools that take a single path-like argument the gate should check against
 // restriction (outside the workspace boundary, or writes under the session state root).

src/session/compactor.ts

@@ -21,6 +21,7 @@ import type {
 } from "@intx/types/runtime";
 import { ageImageBlocks } from "./attachment-store.js";
 import type { SummaryContext } from "./summarizer.js";
+import { PATH_KEYED_READ_TOOLS, SEARCH_QUERY_TOOLS } from "../agent/tool-classification.js";
 
 // ---------------------------------------------------------------------------
 // Task boundary decision
@@ -237,19 +238,18 @@ export function compactorNoOpFloor(keepRecentTurns: number): number {
 // Minimum anchor score for a turn to be pulled forward past the summary boundary.
 const ANCHOR_SCORE_THRESHOLD = 5;
 
-// Tool names whose results are path-keyed for re-read dedup during compaction.
-const READ_TOOLS = new Set(["read_file"]);
-
 // Replayable query tools deduped by full-argument identity: a later identical
 // grep/search_files/list_dir call reflects newer workspace state, so an older
 // identical result is stale the same way an older read_file body is.
 // run_shell is deliberately excluded — the same command is not idempotent
 // (builds, tests, mutations), so an older run_shell result can be the only
-// record of a genuinely distinct outcome.
-const QUERY_TOOLS = new Set(["grep", "search_files", "list_dir"]);
+// record of a genuinely distinct outcome. Built on the shared SEARCH_QUERY_TOOLS
+// base plus list_dir, which compaction treats as replayable the same way
+// (unlike thrash's narrower SEARCH_TOOLS — see tool-classification.ts).
+const QUERY_TOOLS = new Set([...SEARCH_QUERY_TOOLS, "list_dir"]);
 
 function isReplayableResultTool(name: string): boolean {
-  return READ_TOOLS.has(name) || QUERY_TOOLS.has(name);
+  return PATH_KEYED_READ_TOOLS.has(name) || QUERY_TOOLS.has(name);
 }
 
 // Call-id index for stub rendering (name + path). Dedup keys live on `readKey`.

src/subagent/thrash.ts

@@ -15,6 +15,7 @@
  */
 
 import { isProductMutationTool, productMutationPaths } from "../agent/product-mutation-tools.js";
+import { PATH_KEYED_READ_TOOLS, SEARCH_QUERY_TOOLS } from "../agent/tool-classification.js";
 import { classifyShellFileEvidence } from "./shell-evidence.js";
 
 /** Tunable thresholds for force-report detection. */
@@ -53,8 +54,11 @@ export interface ThrashToolCallBlock {
   arguments?: unknown;
 }
 
-const READ_TOOLS = new Set(["read_file"]);
-const SEARCH_TOOLS = new Set(["grep", "search_files"]);
+// list_dir is deliberately excluded: a repeated identical listing of the same
+// directory is not the stuck read/search loop this bookkeeping watches for
+// the way a repeated read_file or grep is (see tool-classification.ts).
+const READ_TOOLS = PATH_KEYED_READ_TOOLS;
+const SEARCH_TOOLS = SEARCH_QUERY_TOOLS;
 const SHELL_TOOL = "run_shell";
 
 function parseArgs(raw: unknown): Record<string, unknown> {