fix(web): key tool selection by row identity so duplicate tool names are individually inspectable

[cliffhall]

Closes#2001

#1961 fixed the rendering half of the duplicate-tool-name problem — sidebar rows are keyed by source position, so filtering no longer orphans a child. But everything downstream of that key still identified a tool by tool.name, so the second copy rendered and then could not be opened:

• ToolControls marked selected={tool.name === selectedName}, so both duplicates highlighted at once, and its onClick guard (if (tool.name !== selectedName)) made clicking the other copy a no-op.
• ToolsScreen's tools.find((t) => t.name === selectedToolName) always resolved to the first match, so the detail panel could never show a later copy even if selection had changed.

What changed

Selection is now keyed by the same stable per-row identity the list already computed for its React key, rather than by name.

• utils/toolUtils.ts — the keying moves here and becomes the shared vocabulary: toolRowKey(name, sourceIndex) (was a private rowKey in ToolControls) plus findToolByRowKey(tools, key), which compares computed keys rather than parsing one, so the two can't drift.
• ToolControls — selectedName → selectedKey, and onSelectTool emits the row key. Highlight and the click guard both compare keys, so each copy is independently selectable.
• ToolsScreen — ToolsUiState.selectedToolName → selectedToolKey, and both lookups (the detail-panel tool and the form-defaults seed) go through findToolByRowKey. Renamed through screenUiState.ts and App.tsx's onToolsUiChange.
• The wire identity is untouched.onCallTool still sends selectedTool.name — the duplicated name genuinely is the protocol identity; only the UI identity needed to be unique.

One adjacent fix while here: excluded (SEP-2243) rows render into the sameStack as the list rows but index within their own list, so an excluded tool sharing a name and index with a listed one collided in the React key space. They now carry an excluded: prefix.

Tests

• toolUtils.test.ts — toolRowKey distinguishes same-named tools; findToolByRowKey resolves each duplicate to its own entry, and returns undefined for no key, a stale key, and a right-name/wrong-position key.
• ToolControls.test.tsx — with the leading get_weather selected, exactly one row is highlighted and clicking the trailing one reports "2:get_weather".
• ToolsScreen.test.tsx — clicking the second copy renders its schema (its field present, the first copy's absent), and Execute calls onCallTool("get_weather", …) with the protocol name.

Verification

npm run ci green. Screenshots below are the repro from the issue: test-servers/configs/duplicate-tool-names-http.json, default (legacy) era — select the leading get_weather, then click the trailing one.

Before — both copies highlighted, detail panel stuck on the first

The trailing row is clicked, but get_weather (the leading copy) is what the panel shows, and both rows read as selected.

After — the trailing copy is selected and inspectable

Only get_weather (duplicate) is highlighted, and the panel shows that row's tool.

[Copilot]

Pull request overview

Keys tool selection by row identity so duplicate names are independently inspectable while preserving protocol names for calls.

Changes:

• Adds shared row-key generation and lookup utilities.
• Threads row keys through controls, screen state, and App state.
• Adds duplicate-name regression tests and updates snapshots/stories.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
clients/web/src/utils/toolUtils.ts	Adds row-key utilities.
clients/web/src/utils/toolUtils.test.ts	Tests duplicate-key resolution.
clients/web/src/lib/oauthResume.test.ts	Updates persisted UI-state fixtures.
clients/web/src/components/screens/ToolsScreen/ToolsScreen.tsx	Resolves selections by row key.
clients/web/src/components/screens/ToolsScreen/ToolsScreen.test.tsx	Tests duplicate inspection and execution.
clients/web/src/components/screens/screenUiState.ts	Renames default selection state.
clients/web/src/components/groups/ToolControls/ToolControls.tsx	Selects and keys individual rows.
clients/web/src/components/groups/ToolControls/ToolControls.test.tsx	Tests independent duplicate selection.
clients/web/src/components/groups/ToolControls/ToolControls.stories.tsx	Updates selected-row story data.
clients/web/src/App.tsx	Tracks selection changes by row key.
clients/web/src/App.test.tsx	Updates App test state and mocks.

---

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

[cliffhall]

Response to Copilot review (round 1)

Mirroring the inline replies at PR level, since inline threads go hidden once the fix is pushed.

1. ToolsUiState is serialized in the version-1 OAuth resume snapshot — migrate or bump the version — addressed, differently.

A snapshot lives for exactly one redirect, so a legacy one is only ever read when the app is upgraded mid-flight; and the old selectedToolName can't be mapped to a row key at restore time regardless, because the tools list isn't fetched until after reconnect. Bumping the version gives the reader nothing extra to do.

What was worth fixing is that the blind as ToolsUiState cast carried the legacy field through as a stray property that would be re-serialized on the next redirect. restoreTabUiFromSnapshot now routes the Tools entry through normalizeToolsUi, which drops it explicitly with the reasoning in a comment. The degradation is now intentional and bounded: the selection is lost, search / formValues / runAsTask restore intact, and the screen opens on its "Select a tool" placeholder. Added the legacy-snapshot test that was asked for.

2. A stale key can resolve into a newer tools snapshot — not changing, with reasons.

This is the staleness the code already had: the previous lookup was tools.find((t) => t.name === selectedToolName), which after a refresh resolved to whatever tool now sits first under that name — different definition, same carried-over form values, same uncleared result. Keying by index:name inherits that rather than introducing it.

Resetting selection on every new tools snapshot is the part to push back on: persisting selection across a refresh is exactly what the lifted ui state exists for (#1414/#1417), and a list_changed auto-refresh that silently cleared the panel and the user's form input would be the worse bug. A list-generation identity is the same reset wearing a hat — it would invalidate the selection on every refresh.

The residual case (a server that reorders duplicate definitions so a same-named tool lands at the same index with a different schema) renders the schema genuinely in the current list at that position; only the form values are stale. If it ever shows up in practice it wants real per-tool identity — a schema digest — not a selection reset.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

Suppressed comments (1)

clients/web/src/components/screens/ToolsScreen/ToolsScreen.tsx:46

• The authoritative V2 specs now contradict this state shape: specification/v2_ux_interfaces.md:649,719,722,726 still defines selectedName/selectedToolName, emits names from onSelectTool, and prescribes tools.find(...), while specification/v2_storage.md:307 still stores selectedToolName. Please update those sections to document row-key UI identity and the separate protocol name used for tools/call, otherwise future work is explicitly directed back to the bug fixed here.

 /**
* The selected row's `toolRowKey` — its position in the list plus its name,
* not the name alone. A `tools/list` may repeat a name, and identifying the
* selection by name highlighted every copy while the detail panel always
* resolved the first, so the later copies could not be inspected (#2001).
* The protocol name a `tools/call` sends still comes from the resolved tool.
*/
selectedToolKey?: string;

[cliffhall]

Response to Copilot review (round 2)

No new inline comments. The one suppressed comment was a fair catch and is now fixed in 3b995f6.

The V2 specs still prescribed the buggy shape — fixed.

specification/v2_ux_interfaces.md still documented ToolControls taking selectedName, ToolsScreen holding selectedToolName, and deriving the detail panel from tools.find(t => t.name === selectedToolName); specification/v2_storage.md still stored selectedToolName. Left alone, they'd direct the next person straight back into this bug. All four now describe row-key identity — toolRowKey / findToolByRowKey — and ToolControls gained an explicit "Row identity" bullet spelling out the split: onSelectTool carries the row key (UI identity), onCallTool carries the protocol name (wire identity).

v2_ux_interfaces_plan.md deliberately keeps its selectedToolName references — it's a historical record of a migration that has already happened, not a target-state doc.

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.