fix(dsh-plugin): register browser tools one by one so a name collision cannot take the whole suite down (#444)

[Jay0130-a]

Summary

When another plugin already owns a browser_* tool name, registerBrowserTools fails all six registrations instead of the one that collided, and the failure is invisible on a desktop host.

registerBrowserTools did:

const disposers = BROWSER_TOOL_SPECS.map((spec) =>
  deps.ctx.tools.register(defineBrowserTool(spec, definitions)),
).filter(/* ... */);

The host tool registry rejects a duplicate name by throwing. Array.prototype.map therefore aborts at the first rejection, so:

• Collision on spec 0 (browser_session) — the case in [dsh plugin 0.3.2] All six browser_* tools silently fail to register when dsh-builtin-browser is mounted: first spec "browser_session" collides in the global tool scope #444, on a profile that also mounts dsh-builtin-browser — means the other five specs are never even attempted. 0 of 6 tools register, while the plugin itself looks healthy (skill loads, CLI/daemon/extension chain fine).
• Collision on a later spec is quieter but worse: the specs before it did register, yet their disposers are discarded together with the pending map entries, so unloading the plugin can never release them.
• Either way the only trace is the console.warn inside armLazyTools, which an Electron desktop host does not surface — from the user's side the plugin "does nothing".

Fixes #444.

Change

packages/dsh-plugin-browserskill/src/browser-tools.ts registers the specs one at a time:

• every tool whose name is still free is registered, so a collision costs one tool instead of the suite;
• the disposers of the tools that did register are collected and returned, so ctx.effect teardown releases them;
• each conflict is reported with the offending tool name, e.g. 1 of 6 browser tools failed to register — another tool already owns the name in this scope: browser_session ("browser_session" is already registered).

When nothing could register, it still throws, so armLazyTools keeps its existing "keep the invocation proof and retry at the next lifecycle boundary" behaviour. The eager (lazyTools: false) path still fails loudly.

I deliberately did not rename any tool: that changes the public tool contract and is the maintainers' call. This PR only makes the failure survivable and diagnosable.

Test plan

Four cases added to packages/dsh-plugin-browserskill/tests/tools.test.ts, driving the real registerBrowserTools with a register stub that throws for chosen names:

case	assertion
collision on spec 0	the other five register; one warning naming browser_session
two collisions	the remaining four register; warning names both
collision on a later spec	dispose() still releases every tool that registered
all six collide	throws with every name in the message

Mutation check — with the source change reverted, all four fail (4 failed | 70 skipped), including expected [Function] to throw error matching /browser_session.*browser_assist/s but got 'duplicate'.

$ pnpm exec vitest run
Test Files  1 failed | 22 passed (23)
     Tests  2 failed | 496 passed (498)

$ pnpm exec tsc --noEmit
(clean)

The 2 remaining failures are in tests/runner.test.ts (settles after the kill grace…, settles killAll and killFor…) and are pre-existing on Windows — they reproduce on a clean tree with this branch stashed (2 failed | 28 passed). The client suites need the repo's build:css step first; with it run, they pass.

• Added/updated tests
• Updated CHANGELOG.md

[iuyo5678]

Superseded by #446, which resolves #444. Thanks for identifying the registration and cleanup issues