refactor(agent): compare tool names through a single helper

Wire-name aliases previously leaked into engine-name comparisons and broke tracking; each new alias required hunting every callsite. Both sides now resolve through canonicalToolName first, so future projections only touch the alias tables.

Author: TheGreatAxios

src/agent/canonical-tool-name.test.ts

@@ -0,0 +1,125 @@
+import { describe, expect, test } from "bun:test";
+import { join } from "node:path";
+import { isSameTool } from "./canonical-tool-name.js";
+
+describe("isSameTool", () => {
+  test.each([
+    // Engine id against itself.
+    ["read_file", "read_file"],
+    ["manage_tasks", "manage_tasks"],
+    // Default-profile wire names the model actually emits.
+    ["read", "read_file"],
+    ["read_file", "read"],
+    ["edit", "edit_file"],
+    ["bash", "run_shell"],
+    ["run_shell", "bash"],
+    ["todowrite", "manage_tasks"],
+    ["manage_tasks", "todowrite"],
+    // Hidden gpt-profile aliases dispatch onto the same engine.
+    ["shell", "run_shell"],
+    ["shell", "bash"],
+    ["update_plan", "manage_tasks"],
+    ["wait", "wait_agents"],
+    // Muse Spark default./doubled prefixes collapse first.
+    ["default.read", "read_file"],
+    ["default.read_file", "read"],
+    ["read.read", "read_file"],
+    ["default.bash", "run_shell"],
+    // Alias lookup falls back to lowercase, matching engineToolName.
+    ["READ", "read_file"],
+    ["Bash", "run_shell"],
+    ["WAIT", "wait_agents"],
+  ])("isSameTool(%j, %j) is true", (a, b) => {
+    expect(isSameTool(a, b)).toBe(true);
+    expect(isSameTool(b, a)).toBe(true);
+  });
+
+  test.each([
+    ["read", "edit"],
+    ["read_file", "write_file"],
+    ["bash", "read"],
+    ["todowrite", "wait_agents"],
+    ["update_plan", "read_file"],
+    ["wait", "run_shell"],
+    ["submit_plan", "manage_tasks"],
+    ["present", "manage_tasks"],
+    ["submit_output", "manage_tasks"],
+    ["mcp__acme__read", "read_file"],
+  ])("isSameTool(%j, %j) is false", (a, b) => {
+    expect(isSameTool(a, b)).toBe(false);
+    expect(isSameTool(b, a)).toBe(false);
+  });
+});
+
+// Callsites that compare incoming wire names must go through isSameTool (or
+// the canonical modules themselves). A new raw `name === "<tool>"` compare in
+// one of these files reintroduces the #1252 breakage on the next wire rename,
+// so this test fails until the callsite is converted.
+describe("tool-name comparison regression guard", () => {
+  const CALLSITES = [
+    "src/agent/director.ts",
+    "src/subagent/poll-exempt.ts",
+    "src/tui/mcp-view.ts",
+    "src/tui/runtime-bridge.ts",
+    "src/tui/stall-watchdog.ts",
+    "src/tui/tool-execution-watchdog.ts",
+    "src/tui/turns-to-blocks.ts",
+  ];
+
+  const TOOL_LITERALS = new Set([
+    "read_file",
+    "write_file",
+    "edit_file",
+    "delete_file",
+    "run_shell",
+    "search_files",
+    "manage_tasks",
+    "use_skill",
+    "web_fetch",
+    "web_search",
+    "ask_operator",
+    "ask_director",
+    "wait_agents",
+    "spawn_agent",
+    "submit_output",
+    "submit_plan",
+    "present",
+    "list_agents",
+    "read",
+    "write",
+    "edit",
+    "delete",
+    "bash",
+    "shell",
+    "glob",
+    "todowrite",
+    "skill",
+    "webfetch",
+    "websearch",
+    "question",
+    "update_plan",
+    "wait",
+  ]);
+
+  test("no raw tool-name ===/!== compares outside the canonical modules", async () => {
+    const root = join(import.meta.dirname, "..", "..");
+    const violations: string[] = [];
+    for (const file of CALLSITES) {
+      const text = await Bun.file(join(root, file)).text();
+      for (const [index, line] of text.split("\n").entries()) {
+        const match = /(?:\?\.name|\.name)\s*(===|!==)\s*(["'])([^"']+)\2/.exec(
+          line,
+        );
+        if (match?.[3] !== undefined && TOOL_LITERALS.has(match[3])) {
+          violations.push(`${file}:${index + 1}: ${line.trim()}`);
+        }
+        const canonical =
+          /canonicalToolName\(([^)]+)\)\s*(===|!==)\s*["']/.exec(line);
+        if (canonical !== null) {
+          violations.push(`${file}:${index + 1}: ${line.trim()}`);
+        }
+      }
+    }
+    expect(violations).toEqual([]);
+  });
+});

src/agent/canonical-tool-name.ts

@@ -20,6 +20,17 @@ export function canonicalToolName(requested: string): string {
   return engineToolName(baseToolName(requested));
 }
 
+// Canonical comparison for tool names arriving on the wire. Either side may
+// be an engine id, a profile wire name (read/bash/…), a hidden alias
+// (shell/update_plan/wait), or a default./doubled prefixed form — both sides
+// resolve through canonicalToolName first, so a rename or per-family
+// projection only touches the alias tables, never the callsites. Case
+// handling matches engineToolName (alias lookup falls back to lowercase;
+// unknown names compare verbatim).
+export function isSameTool(a: string, b: string): boolean {
+  return canonicalToolName(a) === canonicalToolName(b);
+}
+
 // Grant coverage across the alias→engine collapse. Comparisons run in native
 // key space (both sides resolve onto the engine id first); a raw alias name is
 // never compared against a native id. Pure renames

src/agent/director.test.ts

@@ -1013,29 +1013,42 @@ describe("open-task termination guard", () => {
     expect(hasInfer(actions)).toBe(false);
   });
 
-  test("a task change emits the updated task list on the chat event", async () => {
-    const director = createChatDirector("base", [], {});
-    const actions = actionsArray(
-      await director.decide(
-        manageTasksEvent("doing"),
-        stubReactorState,
-        stubReactorCapabilities,
-      ),
-    );
-    expect(
-      actions.filter(
-        (a) =>
-          a.type === "emit" &&
-          (a as { eventType?: string }).eventType === CHAT_TASKS_CHANGED_EVENT,
-      ),
-    ).toEqual([
-      {
-        type: "emit",
-        eventType: CHAT_TASKS_CHANGED_EVENT,
-        data: { tasks: [{ id: "t1", title: "work", status: "doing" }] },
-      },
-    ]);
-  });
+  test.each(["manage_tasks", "todowrite", "update_plan"] as const)(
+    "a task change via %s emits the updated task list on the chat event",
+    async (name) => {
+      const director = createChatDirector("base", [], {});
+      const actions = actionsArray(
+        await director.decide(
+          makeInferenceDoneEvent([
+            {
+              id: "m",
+              name,
+              args: {
+                action: "create",
+                tasks: [{ id: "t1", title: "work", status: "doing" }],
+              },
+            },
+          ]),
+          stubReactorState,
+          stubReactorCapabilities,
+        ),
+      );
+      expect(
+        actions.filter(
+          (a) =>
+            a.type === "emit" &&
+            (a as { eventType?: string }).eventType ===
+              CHAT_TASKS_CHANGED_EVENT,
+        ),
+      ).toEqual([
+        {
+          type: "emit",
+          eventType: CHAT_TASKS_CHANGED_EVENT,
+          data: { tasks: [{ id: "t1", title: "work", status: "doing" }] },
+        },
+      ]);
+    },
+  );
 
   test("stops nudging and lets the turn end after the cap of content-free attempts", async () => {
     const director = createChatDirector("base", [], {});

src/agent/director.ts

@@ -1,4 +1,4 @@
-import { canonicalToolName } from "./canonical-tool-name.js";
+import { isSameTool } from "./canonical-tool-name.js";
 import {
   DefaultDirector,
   type ExtendedInferenceOptions,
@@ -447,7 +447,7 @@ function applyManageTasksToolCall(
   tasks: Task[],
   block: { name: string; arguments: unknown },
 ): Task[] | null {
-  if (canonicalToolName(block.name) !== "manage_tasks") return null;
+  if (!isSameTool(block.name, "manage_tasks")) return null;
   const taskArgs = parseManageTasksArgs(block.arguments);
   return taskArgs !== null ? applyManageTasks(tasks, taskArgs) : null;
 }
@@ -1174,8 +1174,7 @@ class ChatDirectorImpl extends DefaultDirector {
       }
       for (const block of event.turn.content) {
         if (block.type !== "tool_call") continue;
-        const engineName = canonicalToolName(block.name);
-        if (engineName === "manage_tasks") {
+        if (isSameTool(block.name, "manage_tasks")) {
           const next = applyManageTasksToolCall(this.tasks, block);
           if (next !== null) {
             this.tasks = next;
@@ -1185,18 +1184,21 @@ class ChatDirectorImpl extends DefaultDirector {
               }),
             );
           }
-        } else if (engineName === "read_file" || engineName === "edit_file") {
+        } else if (
+          isSameTool(block.name, "read_file") ||
+          isSameTool(block.name, "edit_file")
+        ) {
           const pathResult = PathArgSchema(block.arguments);
           const path = pathResult instanceof type.errors ? "" : pathResult.path;
           if (isCodeFile(path)) this.lspTriggerCalls.add(block.id);
         }
-        if (block.name === "submit_output") {
+        if (isSameTool(block.name, "submit_output")) {
           this.workflowCalls.set(block.id, {
             name: block.name,
             args: block.arguments,
           });
         }
-        if (engineName === "ask_operator") {
+        if (isSameTool(block.name, "ask_operator")) {
           this.askOperatorCalls.add(block.id);
         }
       }

src/subagent/poll-exempt.test.ts

@@ -30,6 +30,15 @@ describe("isPollOnlyPendingBatch", () => {
     ).toBe(true);
   });
 
+  test("gpt wire alias wait is exempt like wait_agents", () => {
+    expect(
+      isPollOnlyPendingBatch(
+        [call("wait")],
+        [result(waitContent(["running"], true))],
+      ),
+    ).toBe(true);
+  });
+
   test("live wait statuses without timeout are exempt", () => {
     for (const status of ["running", "queued", "awaiting_director"]) {
       expect(

src/subagent/poll-exempt.ts

@@ -1,4 +1,4 @@
-import { canonicalToolName } from "../agent/canonical-tool-name.js";
+import { isSameTool } from "../agent/canonical-tool-name.js";
 import { type } from "arktype";
 import type { ToolCall, ToolResult } from "@intx/types/runtime";
 import { isLiveWaitStatus, type WaitJSONStatus } from "./lifecycle.js";
@@ -41,7 +41,7 @@ export function isPollOnlyPendingBatch(
   return calls.every((call, index) => {
     const result = results[index];
     if (result === undefined) return false;
-    if (canonicalToolName(call.name) !== "wait_agents") return false;
+    if (!isSameTool(call.name, "wait_agents")) return false;
     const payload = resultPayload(result);
     if (payload === undefined) return false;
     return isWaitAgentsPending(payload);

src/tui/mcp-view.ts

@@ -15,7 +15,7 @@
  * for the pagination cursor.
  */
 
-import { canonicalToolName } from "../agent/canonical-tool-name.js";
+import { isSameTool } from "../agent/canonical-tool-name.js";
 import {
   fg as fgChunk,
   bold as boldChunk,
@@ -502,7 +502,7 @@ const USE_SKILL_TOOL = "use_skill";
  * future wording) simply does not collapse.
  */
 function loadedSkillName(name: string, content: string): string | undefined {
-  if (canonicalToolName(name) !== USE_SKILL_TOOL) return undefined;
+  if (!isSameTool(name, USE_SKILL_TOOL)) return undefined;
   return /^Skill "([^"]+)"/.exec(content)?.[1];
 }
 

src/tui/runtime-bridge.ts

@@ -6,7 +6,7 @@
  * later waves can bind real agent APIs). Not the production CLI entry.
  */
 
-import { canonicalToolName } from "../agent/canonical-tool-name.js";
+import { canonicalToolName, isSameTool } from "../agent/canonical-tool-name.js";
 import {
   cancelItem,
   createSessionQueue,
@@ -1009,7 +1009,7 @@ function applyToolCall(
   raw: Extract<BridgeInboundEvent, { type: "tool_call" }>,
 ): void {
   const event = { ...raw, name: canonicalToolName(raw.name) };
-  if (event.name === MANAGE_TASKS_TOOL_NAME) {
+  if (isSameTool(raw.name, MANAGE_TASKS_TOOL_NAME)) {
     // Remembered so the matching result is dropped too — suppressing only the
     // call would leave its result to land as an unpaired row. Checklist lives
     // on the task panel; spawn_agent dispatches paint live transcript rows instead.
@@ -1043,7 +1043,10 @@ function applyToolCall(
       if (bag.turn.blockedGateCount > 0) bag.gatedToolCalls.add(event.callId);
     }
   }
-  if (event.callId !== undefined && event.name === SPAWN_AGENT_TOOL_NAME) {
+  if (
+    event.callId !== undefined &&
+    isSameTool(raw.name, SPAWN_AGENT_TOOL_NAME)
+  ) {
     bag.taskCallIds.add(event.callId);
     bag.spawnProgressRows.set(event.callId, index);
   }
@@ -1084,7 +1087,7 @@ function applyToolResult(
     // spawn_agent's immediate running JSON is not the end of the worker —
     // keep the row in taskCallIds / spawnProgressRows until the session
     // leaves the running set (see syncAgentProgress).
-    if (event.name !== SPAWN_AGENT_TOOL_NAME) {
+    if (!isSameTool(raw.name, SPAWN_AGENT_TOOL_NAME)) {
       bag.taskCallIds.delete(event.callId);
       bag.spawnProgressRows.delete(event.callId);
     }

src/tui/stall-watchdog.test.ts

@@ -76,6 +76,21 @@ describe("shouldAbortForStall", () => {
       }),
     ).toBe(false);
   });
+
+  test.each(["wait_agents", "wait"] as const)(
+    "in-flight %s is stall-bounded under any wire name",
+    (currentToolName) => {
+      // streamingType "tool" isolates the stall-bounded branch: an ordinary
+      // tool run with this shape is not a stall (see above).
+      expect(
+        shouldAbortForStall({
+          ...base,
+          streamingType: "tool",
+          currentToolName,
+        }),
+      ).toBe(true);
+    },
+  );
 });
 
 // Awaiting the model's next response with no tokens yet — set right after

src/tui/stall-watchdog.ts

@@ -1,4 +1,4 @@
-import { canonicalToolName } from "../agent/canonical-tool-name.js";
+import { isSameTool } from "../agent/canonical-tool-name.js";
 import type { TurnStatus } from "./chrome-state.js";
 
 // How long the run can be continuously awaiting a response with no new content
@@ -76,8 +76,7 @@ function silentPastThreshold(
  */
 function isStallBoundedToolName(name: string | null | undefined): boolean {
   if (name === null || name === undefined) return false;
-  const engine = canonicalToolName(name);
-  return engine === "wait_agents" || engine === "ask_director";
+  return isSameTool(name, "wait_agents") || isSameTool(name, "ask_director");
 }
 
 function isStallBoundedInFlightTool(args: ShouldAbortForStallArgs): boolean {