fix: defer built-in tool overrides until session lifecycle

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+- Actually defer built-in tool override registration (`read`/`grep`/`find`/`ls`/`bash`/`edit`/`write`) until `session_start` / `before_agent_start`, and re-evaluate ownership against the full tool graph before registering. This prevents hard startup conflicts when another extension owns `bash` (e.g. `pi-patty-bg-tasks`), which previously raced because overrides were still registered at extension load time despite README claims of deferred discovery.
+
+
 ## [0.5.0] - 2026-07-03
 
 ### Added

README.md

@@ -322,7 +322,18 @@ If another extension is already rendering one of the built-in tools:
 2. Run `/reload`
 3. Use `/tool-display show` to confirm the effective ownership state
 
-Built-in tool overrides (including `bash`) are registered with deferred ownership discovery — the extension discovers which tools it owns via `pi.getAllTools()` during `before_agent_start` before overriding, preventing conflicts with other extensions that also register overrides.
+Built-in tool overrides (including `bash`) are **queued at load** and only registered during `session_start` / `before_agent_start`. Before registering, the extension re-checks `pi.getAllTools()` and yields any tool already owned by a non-builtin source (for example `pi-patty-bg-tasks` owning `bash`). Immediate load-time registration is intentionally avoided so package order cannot create a hard `Tool "bash" conflicts with ...` diagnostic.
+
+If you still prefer an explicit config, set:
+
+```json
+{
+  "registerToolOverrides": {
+    "bash": false
+  }
+}
+```
+
 
 ### Config not loading
 

src/tool-overrides.ts

@@ -1616,20 +1616,40 @@ export function registerToolDisplayOverrides(
     return false;
   };
 
+  // Built-in overrides are collected at load time and applied after all
+  // extensions have registered tools. Immediate registration races with
+  // packages like pi-patty-bg-tasks that also own `bash`, producing a hard
+  // "Tool \"bash\" conflicts with ..." diagnostic at startup.
+  const pendingBuiltInToolOverrides = new Map<BuiltInToolOverrideName, () => void>();
+
   const registerIfOwned = (
     toolName: BuiltInToolOverrideName,
     register: () => void,
   ): void => {
     if (
       registeredBuiltInToolOverrides.has(toolName) ||
-      !getConfig().registerToolOverrides[toolName] ||
-      isExternallyOwnedBuiltInTool(toolName)
+      pendingBuiltInToolOverrides.has(toolName) ||
+      !getConfig().registerToolOverrides[toolName]
     ) {
       return;
     }
 
-    register();
-    registeredBuiltInToolOverrides.add(toolName);
+    pendingBuiltInToolOverrides.set(toolName, register);
+  };
+
+  const registerDeferredBuiltInToolOverrides = (): void => {
+    for (const [toolName, register] of pendingBuiltInToolOverrides) {
+      if (
+        registeredBuiltInToolOverrides.has(toolName) ||
+        !getConfig().registerToolOverrides[toolName] ||
+        isExternallyOwnedBuiltInTool(toolName)
+      ) {
+        continue;
+      }
+
+      register();
+      registeredBuiltInToolOverrides.add(toolName);
+    }
   };
 
   function createBuiltinToolBase(toolName: keyof BuiltInTools) {
@@ -2110,11 +2130,13 @@ export function registerToolDisplayOverrides(
 
   pi.on("session_start", async () => {
     clearWriteExecutionMeta(writeExecutionMetaByToolCallId);
+    registerDeferredBuiltInToolOverrides();
     registerMcpToolOverrides();
     scheduleMcpToolOverrideDiscovery();
   });
   pi.on("before_agent_start", async () => {
     clearWriteExecutionMeta(writeExecutionMetaByToolCallId);
+    registerDeferredBuiltInToolOverrides();
     registerMcpToolOverrides();
     scheduleMcpToolOverrideDiscovery();
   });

tests/reload-behavior.test.ts

@@ -144,27 +144,37 @@ test("1: after reload, new lifecycle handlers are registered", () => {
 // 2. Tool override restoration
 // ---------------------------------------------------------------------------
 
-test("2: built-in tool overrides are re-registered on reload", () => {
-  const { api, capturedTools } = createApiStub();
+test("2: built-in tool overrides are re-registered on reload", async () => {
+  const { api, capturedTools, capturedHandlers } = createApiStub();
+  const fireLifecycle = async () => {
+    for (const handler of capturedHandlers) {
+      if (handler.event === "session_start") {
+        await handler.handler({}, { ui: { theme: {}, notify: () => {} } });
+      }
+    }
+  };
 
-  // First call
+  // First call — tools register only after session_start.
   toolDisplayExtension(api);
+  assert.equal(
+    capturedTools.filter((t) => ["find", "ls", "write", "bash"].includes(t.name)).length,
+    0,
+    "built-ins are deferred until session lifecycle",
+  );
+  await fireLifecycle();
   const firstTools = capturedTools.map((t) => t.name);
-  assert.ok(firstTools.includes("find"), "find registered on first call");
+  assert.ok(firstTools.includes("find"), "find registered after session_start");
 
-  // Simulate reload
+  // Simulate reload + lifecycle
   const countBeforeReload = capturedTools.length;
   toolDisplayExtension(api);
+  await fireLifecycle();
   const countAfterReload = capturedTools.length;
-
-  // Each call to registerToolDisplayOverrides registers the same built-in
-  // tools again (find, ls, write immediately; read/grep/edit/bash deferred).
   assert.ok(
     countAfterReload >= countBeforeReload + 3,
-    "at least 3 tools re-registered on reload",
+    "built-in tools re-registered on reload after lifecycle",
   );
 
-  // Verify tool names appear multiple times, meaning they were re-registered
   const toolNameCounts = new Map<string, number>();
   for (const tool of capturedTools) {
     toolNameCounts.set(tool.name, (toolNameCounts.get(tool.name) ?? 0) + 1);
@@ -211,30 +221,36 @@ test("2: re-registered tools have renderCall and renderResult functions after re
   }
 });
 
-test("2: built-in tool overrides register before lifecycle events and re-register on reload", async () => {
+test("2: built-in tool overrides register on lifecycle events and re-register on reload", async () => {
   const { api, registeredTools, eventHandlers } = createExtensionApiStub();
 
   registerToolDisplayOverrides(api, () => DEFAULT_TOOL_DISPLAY_CONFIG);
-  const firstImmediate = registeredTools.map((t) => t.name);
+  assert.equal(registeredTools.length, 0, "no built-ins registered before lifecycle events");
 
-  for (const toolName of ["read", "edit", "grep", "bash"] as const) {
-    assert.ok(firstImmediate.includes(toolName), `${toolName} registered before lifecycle events`);
+  await eventHandlers.before_agent_start?.();
+  const firstLifecycle = registeredTools.map((t) => t.name);
+  for (const toolName of ["read", "edit", "grep", "bash", "find", "ls", "write"] as const) {
+    assert.ok(firstLifecycle.includes(toolName), `${toolName} registered on before_agent_start`);
   }
+  const countAfterLifecycle = registeredTools.length;
 
-  const countBeforeLifecycle = registeredTools.length;
   await eventHandlers.before_agent_start?.();
   assert.equal(
     registeredTools.length,
-    countBeforeLifecycle,
+    countAfterLifecycle,
     "before_agent_start does not duplicate already registered built-ins",
   );
 
   registerToolDisplayOverrides(api, () => DEFAULT_TOOL_DISPLAY_CONFIG);
-  const countAfterReload = registeredTools.length;
-
+  assert.equal(
+    registeredTools.length,
+    countAfterLifecycle,
+    "reload queues built-ins but does not register them until lifecycle",
+  );
+  await eventHandlers.before_agent_start?.();
   assert.ok(
-    countAfterReload >= countBeforeLifecycle + 7,
-    "built-in display overrides re-register during reload initialization",
+    registeredTools.length >= countAfterLifecycle + 7,
+    "built-in display overrides re-register on lifecycle after reload",
   );
 });
 
@@ -710,30 +726,37 @@ test("8: session_start handler can be invoked after reload without errors", asyn
 // 9. Double reload safety
 // ---------------------------------------------------------------------------
 
-test("9: calling toolDisplayExtension three times (double reload) is safe", () => {
-  const { api, capturedTools, capturedCommands } = createApiStub();
+test("9: calling toolDisplayExtension three times (double reload) is safe", async () => {
+  const { api, capturedTools, capturedCommands, capturedHandlers } = createApiStub();
+  const fireLifecycle = async () => {
+    for (const handler of capturedHandlers) {
+      if (handler.event === "session_start") {
+        await handler.handler({}, { ui: { theme: {}, notify: () => {} } });
+      }
+    }
+  };
 
   // First call
   toolDisplayExtension(api);
+  await fireLifecycle();
   const afterFirst = { tools: capturedTools.length, cmds: capturedCommands.length };
 
   // First reload
   toolDisplayExtension(api);
+  await fireLifecycle();
   const afterSecond = { tools: capturedTools.length, cmds: capturedCommands.length };
 
   // Second reload (double reload)
   assert.doesNotThrow(() => toolDisplayExtension(api));
+  await fireLifecycle();
   const afterThird = { tools: capturedTools.length, cmds: capturedCommands.length };
 
-  // Each call adds more registrations (no deduplication in the stub)
+  // Each call adds more registrations after lifecycle (no deduplication in the stub)
   assert.ok(afterThird.tools > afterSecond.tools, "tools registered on third call");
   assert.ok(afterThird.cmds > afterSecond.cmds, "commands registered on third call");
 
   // Verify all tool registrations have renderCall/renderResult
   for (const tool of capturedTools) {
-    if (tool.name === "read" || tool.name === "edit" || tool.name === "grep") {
-      continue; // Deferred tools
-    }
     if (tool.renderCall !== undefined) {
       assert.equal(
         typeof tool.renderCall,

tests/tool-overrides-registration.test.ts

@@ -92,12 +92,15 @@ test("registerToolDisplayOverrides copies built-in prompt metadata onto overridd
 	const { api, registeredTools, eventHandlers } = createExtensionApiStub();
 
 	registerToolDisplayOverrides(api, () => DEFAULT_TOOL_DISPLAY_CONFIG);
+	// Built-in overrides are deferred until session/agent lifecycle so other
+	// extensions can claim ownership first (e.g. pi-patty-bg-tasks for bash).
+	assert.deepEqual(registeredTools.map((tool) => tool.name).sort(), []);
+	await eventHandlers.before_agent_start?.();
+
 	assert.deepEqual(
 		registeredTools.map((tool) => tool.name).sort(),
 		["bash", "edit", "find", "grep", "ls", "read", "write"],
 	);
-	await eventHandlers.before_agent_start?.();
-
 	assert.equal(registeredTools.length, 7);
 
 	const byName = new Map(registeredTools.map((tool) => [tool.name, tool]));
@@ -128,20 +131,49 @@ test("registerToolDisplayOverrides copies built-in prompt metadata onto overridd
 	assert.equal(byName.get("bash")?.promptGuidelines, undefined);
 });
 
-test("registerToolDisplayOverrides registers built-in display renderers during extension load for pre-bind history rendering", () => {
-	const { api, registeredTools } = createExtensionApiStub();
+test("registerToolDisplayOverrides defers built-in display renderers until session lifecycle", async () => {
+	const { api, registeredTools, eventHandlers } = createExtensionApiStub();
 
 	registerToolDisplayOverrides(api, () => DEFAULT_TOOL_DISPLAY_CONFIG);
+	assert.equal(registeredTools.length, 0, "must not register built-ins during extension load");
+
+	await eventHandlers.session_start?.();
 
 	const byName = new Map(registeredTools.map((tool) => [tool.name, tool]));
 	for (const name of ["read", "grep", "find", "ls", "bash", "edit", "write"] as const) {
 		const registeredTool = byName.get(name);
-		assert.ok(registeredTool, `expected '${name}' to be available before session_start`);
-		assert.equal(typeof registeredTool.renderCall, "function", `${name} has renderCall before session_start`);
-		assert.equal(typeof registeredTool.renderResult, "function", `${name} has renderResult before session_start`);
+		assert.ok(registeredTool, `expected '${name}' after session_start`);
+		assert.equal(typeof registeredTool.renderCall, "function", `${name} has renderCall after session_start`);
+		assert.equal(typeof registeredTool.renderResult, "function", `${name} has renderResult after session_start`);
 	}
 });
 
+test("defers built-in bash override when another extension owns bash after load", async () => {
+	// Simulate load-order: tool-display evaluates first (no external bash yet),
+	// then pi-patty-bg-tasks registers bash before session_start/before_agent_start.
+	const externalTools: unknown[] = [];
+	const { api, registeredTools, eventHandlers } = createExtensionApiStub(externalTools);
+
+	registerToolDisplayOverrides(api, () => DEFAULT_TOOL_DISPLAY_CONFIG);
+	assert.equal(registeredTools.some((tool) => tool.name === "bash"), false);
+
+	externalTools.push({
+		name: "bash",
+		sourceInfo: {
+			source: "package",
+			path: "node_modules/pi-patty-bg-tasks/src/tools/bash.ts",
+		},
+	});
+
+	await eventHandlers.session_start?.();
+	await eventHandlers.before_agent_start?.();
+
+	const registeredNames = new Set(registeredTools.map((tool) => tool.name));
+	assert.equal(registeredNames.has("bash"), false, "must yield bash to external owner");
+	assert.equal(registeredNames.has("read"), true);
+	assert.equal(registeredNames.has("write"), true);
+});
+
 test("registerToolDisplayOverrides clones built-in parameter schemas so Pi TUI keeps extension renderers active", async () => {
 	const { api, registeredTools, eventHandlers } = createExtensionApiStub();