Skip to content

fix: defer built-in tool overrides until session lifecycle - #34

Open
RaviTharuma wants to merge 1 commit into
MasuRii:mainfrom
RaviTharuma:fix/defer-builtin-tool-ownership
Open

fix: defer built-in tool overrides until session lifecycle#34
RaviTharuma wants to merge 1 commit into
MasuRii:mainfrom
RaviTharuma:fix/defer-builtin-tool-ownership

Conversation

@RaviTharuma

Copy link
Copy Markdown

Summary

  • Built-in tool overrides (read/grep/find/ls/bash/edit/write) were still registered at extension load time.
  • That races with packages that also own bash (notably pi-patty-bg-tasks) and produces a hard Pi startup diagnostic:
    Error: Failed to load extension ".../pi-patty-bg-tasks/index.ts": Tool "bash" conflicts with .../pi-tool-display/index.ts
    
  • README already claimed deferred ownership discovery (Built-in tool override ownership check is load-order-dependent, causing hard crash when another extension owns read/edit/etc. #29 / ownership docs), but the implementation still called registerIfOwned(...) immediately during load.
  • This PR queues built-in overrides at load and only registers them from session_start / before_agent_start after re-checking pi.getAllTools(). If a non-builtin owner already claimed the tool, pi-tool-display yields.

Local workaround (already documented)

{
  "registerToolOverrides": {
    "bash": false
  }
}

Test plan

  • npx tsx --test tests/tool-overrides-registration.test.ts
  • Updated reload tests for deferred registration (tests/reload-behavior.test.ts)
  • New regression: late external bash ownership (pi-patty-bg-tasks load order)
  • With both packages enabled and registerToolOverrides.bash: true, Pi starts without a tool conflict diagnostic once this lands

Fixes coexistence with pi-patty-bg-tasks without requiring package reordering.

Built-in tool overrides (including bash) were still registered at extension
load time. That races with packages like pi-patty-bg-tasks that also own
bash and produces a hard Pi startup conflict:

  Tool "bash" conflicts with .../pi-tool-display/index.ts

Queue built-in overrides at load and only register them from session_start /
before_agent_start after re-checking pi.getAllTools() ownership. Yield any
tool already owned by a non-builtin source.

Updates README ownership docs and adds regression coverage for late bash
ownership (pi-patty-bg-tasks load order).
@RaviTharuma

Copy link
Copy Markdown
Author

Tracked by #35 (repro with pi-patty-bg-tasks). Companion note on the other package: patty-io/pi-patty-bg-tasks#3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant