| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
cliffhall
|
ed2070f67c |
chore(web): extract App.tsx's connection lifecycle into useConnectionLifecycle
Phase 2 step 3 of the App.tsx decomposition (#2154, under #2129/#2126). Moves the whole connection cluster out of App.tsx and into clients/web/src/hooks/useConnectionLifecycle.ts: setupClientForServer (the single place a connection is configured), onToggleConnection, onDisconnect, onReauthenticateFromBanner, the deep-link auto-connect effect and its three phase latches, the connected/teardown/disconnect effects, connectErrorMessage, and the session-scoped reset a disconnect triggers. Handshake telemetry rides along as useHandshakeTelemetry, split out only because useOAuthRecovery runs first and stamps the same ref. No behavior change. App.tsx goes 3,039 -> 2,412 lines. Per #2129, resetSessionScopedUiState takes a narrow SessionResetSurface - one reset() per owner - rather than every phase-1 setter, so the hook does not end up depending on everything phase 1 extracted. The two log-level operations stay App-owned because the connect path seeds the same state. Closes #2154 Signed-off-by: cliffhall <cliff@futurescale.com> |
||
|
cliffhall
|
02a56e8e89 |
chore(web): extract App.tsx's MCP Apps wiring into useMcpApps
Step 5 of the phase-2 App.tsx decomposition (#2129, under #2126): the two sandbox bridge factories, the renderer handle, and the app-rendered elicitation controller move out of App.tsx into a hook of their own. Nearly a leaf — it reads the active client and the resource listing and calls nothing back — so `getListedResourceMeta`, `publishDocument` and `listedResourcesRef` become internal to the hook rather than App.tsx locals. The hook is called where `resources` is derived, which is what lets it own the latest-listing ref write too. `sandboxUrl` and `initialConfigSettledRef` deliberately stay in App.tsx: whether the sandbox exists decides whether the client advertises the nested MCP Apps `elicitation` capability, and that is a connection-lifecycle question rather than an Apps one. Inert by construction: no behavior change, and the extraction is covered 100% on all four dimensions plus the existing `smoke:web:app` and `smoke:web:elicit`, in both Chromium and Firefox. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hzwzS8UvBsr4yZm7o7sev Signed-off-by: cliffhall <cliff@futurescale.com> |
||
|
cliffhall
|
5f875e3393 |
chore(web): address Copilot review on #2153
- Publish `setupClientForServerRef` from a layout effect rather than during render. React runs every layout effect before any passive effect of the same commit, and the /oauth/callback effect is passive, so the seam is still published before its first read — without the render-phase ref mutation the compiler rule rejects. This was the one `react-hooks/refs` finding the PR introduced, so its suppression is gone rather than deferred to #2161 under a justification that did not describe it. - Narrow the hook's `fetchLogRef` option to `FetchRequestSource`, the `getFetchRequests` slice it actually consumes, mirroring `InspectorClientEventSource` in useProgressToasts. That removes the test harness's `as unknown as` cast, and the narrowed contract is real: it immediately rejected the loosely-typed fixture, which is now a properly typed `FetchRequestEntry`. - Make the test harness hold both refs across renders and publish them from a layout effect, as App.tsx does. The deferred-publication test previously passed because `Probe` built a fresh ref each render and the rerender also changed `servers`; it now asserts what the guard is actually for — a run that cannot exchange must not spend the single-use code — and re-triggers on the `servers` change that carries it in production. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hzwzS8UvBsr4yZm7o7sev Signed-off-by: cliffhall <cliff@futurescale.com> |
||
|
cliffhall
|
ed38a5e5fe |
chore(web): extract App.tsx's OAuth recovery cluster into useOAuthRecovery (#2153)
Phase 2 step 2 of the App.tsx decomposition (#2129, under #2126). App.tsx goes from 4,267 to 3,178 lines; no behaviour change. The new hook owns the step-up slot and its retry operation, the re-auth banner, the deferred background-tab recovery, the redirect plumbing (webOAuthStorage, sessionStorageAdapter, onBeforeOAuthRedirect, prepareOAuthRedirect), the command-scoped recovery wrappers, the four OAuth effects, the abandoned-redirect check, the /oauth/callback completion, the Connection Info OAuth details, and the clear/finalize paths. All five once-only latches move with it unchanged. Three seams could not be a straight move: - useSessionRef no longer takes pendingStepUp/pendingReauth. They are this hook's own state, so App.tsx would have had to hold them purely to hand them back; instead the owner mirrors them into the ref in an effect of its own, following the shape activeServerName already had. - setupClientForServer is injected through a ref, because it reads onBeforeOAuthRedirect and sessionStorageAdapter out of this hook and so cannot be declared before it. The callback effect reads the ref before burning its one-shot latch. - The two clear-OAuth modal adapters and onReauthenticateFromBanner stay in App.tsx: they bind App-owned state, and the latter drives a reconnect, which step 3 owns. Dropping ~1,100 lines took App.tsx under the React Compiler's bail-out, so the compiler-backed react-hooks rules now analyse it and report nine pre-existing findings that were previously invisible. Each is suppressed with a narrow justified disable naming #2161, which is filed to resolve them; the one new finding (exhaustive-deps on setReAuthBanner) is fixed rather than suppressed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hzwzS8UvBsr4yZm7o7sev Signed-off-by: cliffhall <cliff@futurescale.com> |
||
|
cliffhall
|
dc026acfea |
chore(web): drop the double cast from the useSessionRef test fixture
Addresses Copilot's review on #2152. The client stand-in was `({ tag }) as unknown as InspectorClient` — a double cast with no justification, which AGENTS.md rejects. `Object.create(InspectorClient.prototype)` gives a real prototype chain without running the constructor, so it needs only a single cast and still drags in no transport or environment. The tag went with it: every assertion compares identity, and `Object.create` already returns a distinct object per call. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MZdpUwhHf943FoprcLVzEm Signed-off-by: cliffhall <cliff@futurescale.com> |
||
|
cliffhall
|
104b3c0775 |
chore(web): collapse App.tsx's six session refs into useSessionRef (#2129)
Step 1 of phase 2 of the App.tsx decomposition, landed first and alone because it is the seam the remaining clusters are extracted onto. App.tsx kept six "latest value" refs so long-lived callbacks could read a current value without listing it as a dependency and being re-created on every change: `activeServerNameRef`, `activeServerIdRef`, `serversRef`, `inspectorClientRef`, `pendingStepUpRef` and `pendingReauthRef`, plus the three passive effects that synced them. `useSessionRef` replaces all nine with one hook returning one stable ref onto a `SessionSnapshot`. This is not a store: nothing renders off it, nothing subscribes to it, and mutating it never schedules a render. It is not the prop-to-state sync AGENTS.md forbids either — that rule is about `setState` in an effect, and a ref write produces no render at all. It is exactly the pattern the file already used, written once instead of six times. What it buys the later steps is a *stable* dependency: a hook taking `sessionRef` does not drag the other clusters into its argument list, so `useOAuthRecovery`, `useConnectionLifecycle` and `useServerCommands` can be extracted one PR at a time. Behavior is unchanged: - The snapshot is seeded from the first render's values, matching what each `useRef(value)` seeded. - `activeServerName` keeps its one non-uniform rule — it holds the last server that *was* active and is never cleared, because a transport crash clears `activeServerId` before the failure toast is raised. It is derived from `servers`/`activeServerId` rather than taking the `activeServer` memo, which is the same value by definition and is what lets the hook be called ahead of that memo. - The two pending-* syncs already ran ahead of the only effect that reads them, so merging them changes no effect's view. The other four synced below every effect that reads them, so hoisting that sync changes none either. - The sync effect is deliberately un-gated rather than carrying a dependency list: re-assigning unchanged values costs nothing and removes a place for a newly-added field to be forgotten. `sessionRef` is declared ahead of its first reader because a hook return is not provably stable to `react-hooks/exhaustive-deps` — consumers must list it, and a later declaration would put every one of them in the temporal dead zone. Listing a ref that never changes identity re-runs nothing. The inline `pendingStepUp` state shape is named `PendingStepUp` and moved beside `StepUpSource` in `utils/stepUp.ts`, so the snapshot can name it without importing from the component that owns the state. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MZdpUwhHf943FoprcLVzEm Signed-off-by: cliffhall <cliff@futurescale.com> |
||
|
cliffhall
|
2fca87c0ad |
chore(web): record the useProgressToasts return-type exception
Copilot's second pass accepted every code fix and re-raised one item: useProgressToasts is the only extracted hook that returns nothing, which contradicts #2128's "each hook returns one object" criterion, and the disagreement was recorded nowhere a reader would find it. Its ask was either a result object or an explicit exception. Taking the second: the hook subscribes, shows toasts and unsubscribes, and the only state it owns is toast-id bookkeeping the caller must not see, so `{}` would advertise an API that is not there. The criterion exists to stop a hook handing back a long list of loose values — useTabUiState's twenty-two is the case it was written against, and that one does return a bundled object. Noted in the hook's own doc block (so it reads as a decision rather than an oversight) and on #2128 itself, so the acceptance criteria and the implementation agree. Signed-off-by: cliffhall <cliff@futurescale.com> |
||
|
cliffhall
|
edac58e304 |
chore(web): address Copilot review on #2128
Seven of the eight comments, plus the three suppressed ones (the same
double-cast class):
- Narrow the toast hooks to `InspectorClientEventSource`, the client's
typed event-listener surface. A bare `EventTarget` cannot satisfy it
(its listener takes an `Event`, the typed one a `TypedEventGeneric`
with `detail`), so both tests now construct a real
`InspectorClientEventTarget` and dispatch through `dispatchTypedEvent`
rather than casting a stand-in. That failed to compile immediately and
caught two bad fixtures: `task()` was a `Pick<Task, "status">` where
the event map wants a full `Task`, and `toolCallTaskUpdated` was
dispatched with no `task` at all.
- Narrow `useExportActions`'s three store parameters to the single
clearing method each handler calls, so the test doubles are
structurally checked instead of cast away.
- Build real `MessageEntry` fixtures (`timestamp` is a `Date`, not `0`),
removing four more double casts.
- Fix a tautological assertion in useTabUiState.test: it compared a
callback to itself, so it held however the identity changed. Every
callback is now captured before the update and compared to those
references.
- Reword `resetTabUiState`'s doc: `exhaustive-deps` cannot infer
stability across a hook boundary, so the dependency IS named — the
stability buys no churn, not omission.
- Justify the one remaining double cast in place, per the type-safety
rule.
`useProgressToasts` still returns `void` rather than an object,
deliberately: it has nothing to return, and `{}` would imply an API that
is not there. Noted on the PR as the one deviation from #2128's
"returns one object" criterion.
Signed-off-by: cliffhall <cliff@futurescale.com>
|
||
|
cliffhall
|
e42968ba78 |
chore(web): extract App.tsx's leaf hooks (#2128)
Phase 1 of #2126. Six hooks move out of `clients/web/src/App.tsx`, taking its component body from 4,891 to 4,267 lines with no behavior change: - useThemeToggle — the light/dark toggle - useTabUiState — the nine per-screen `ui` objects, the active tab, the pinned-Protocol ids, and the session reset - useInspectorStores — the twelve state managers as one create/destroy lifecycle, their `core/react` hooks, and the pagination-mode selection - useExportActions — the eleven Clear / Export / Replay handlers - useProgressToasts — `notifications/progress` toasts - useTaskToasts — task-status toasts, the cancel confirmation, the taskId -> progress map, and the in-flight tool-call taskId ref Holding the stores in one slot collapses `setupClientForServer`'s and `onConfirmRemove`'s dependency arrays from twelve entries to two stable callbacks; `destroyStores` reads the live set through a ref mirror rather than the state it also writes. All six hooks are at 100% on all four coverage dimensions, no `v8 ignore` needed. Also refreshes the now-stale "~4.5k-line" description of App.tsx in AGENTS.md and the coverage `include` comment. Signed-off-by: cliffhall <cliff@futurescale.com> |
||
|
cliffhall
|
bbffafded1 |
fix: resolve the servers list outside the override's state updater (#2095)
`record` read `serversRef.current` from inside the `setRecords` updater. A functional updater must be pure -- React may defer or replay it -- so the pairing resolved at whatever moment the updater happened to run rather than when the record was written. A list read committing in between would then be paired with the record as its baseline, and the very read that should have superseded the override would instead certify it. Read the list once, at call time, and use it for both the pairing and the prune. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0177mPHAdECD18nLLCTwR5rh Signed-off-by: cliffhall <cliff@futurescale.com> |
||
|
cliffhall
|
7098da7473 |
fix: keep the pagination toggle on the last write across a server switch (#2095)
The displayed `paginatedLists` value falls back to the active `servers` entry, and that list only advances when a `GET /api/servers` succeeds — so once a list read fails it keeps describing disk as it was before the writes made since. The optimistic override papered over that, but it was a single app-wide slot cleared on every change of the *active* entry, so switching to another server and back dropped it and the UI fell back to the stale entry. Two things then disagreed, and the one the user could see was the wrong one: the toggle read `off` while disk, the write tracker, and the client just built from it all said `on`; and because the client skipped its connect-time aggregate walk, the all-pages UI rendered an aggregate that was never fetched, with no Load-next-page control to fill it. Hold the override per server instead, in `usePaginatedListsOverride`, paired with that server's entry as of when it was recorded and believed only while the list still carries that same entry object — identity, not equality, so a read reporting the value the override replaced supersedes it too. A server's override then survives a switch away and back, and is still superseded by the first successful read that rebuilds its entry. The read cannot instead move to `lastPersistedSettings.resolve(id)`: that is called during render, and it both reads refs and prunes superseded records. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0177mPHAdECD18nLLCTwR5rh Signed-off-by: cliffhall <cliff@futurescale.com> |
||
|
Cliff Hall
|
8b9839cc93 |
Merge pull request #2087 from modelcontextprotocol/v2/fix/1914-surface-post-add-refresh-failure
fix(web): surface a failed post-write server-list reload |
||
|
cliffhall
|
b1996d1093 |
Merge branch 'v2/main' into v2/fix/1914-surface-post-add-refresh-failure
Both lines changed the same two settings-write paths in App.tsx: - #2089 (v2/main) rewrote the pagination toggle and the settings-draft flush around `useLastPersistedSettings`, so each write announces itself with `begin` and confirms with `landed`/`failed`. - #1914 (this branch) made a failed post-write list reload reject with `ServerListReloadError` — the PUT landed, only reading the list back failed. Resolved by treating a `ServerListReloadError` as a *landed* write in the new model rather than a failed one: both paths now settle it exactly as the success path does (so it becomes the rollback baseline and re-applies itself when settled) and only the toast says the reload failed. Rolling it back would put the UI, the live client and the modal's seed on a value disk no longer holds — the very contradiction #2089 exists to prevent. The two suites' new tests were interleaved by the conflict and are both kept, plus one covering the combination: a modal save whose reload failed is recorded as the landed write instead of rolled back. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UL9retfvAXRgvi6EWk4SY4 Signed-off-by: cliffhall <cliff@futurescale.com> |
||
|
cliffhall
|
75f9c51146 |
fix(web): update a fixture the merge brought in to the object metadata shape
`useLastPersistedSettings.test.tsx` arrived on `v2/main` with `metadata: []` — correct when it was written, but this branch changes `InspectorServerSettings.metadata` to a JSON object (#1910). Caught by `tsc -b`, not by the tests, which pass either way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0177mPHAdECD18nLLCTwR5rh Signed-off-by: cliffhall <cliff@futurescale.com> |
||
|
cliffhall
|
db84823262 |
fix(web): apply the whole live surface, and track write outcomes per server
Review round 9 on #2091, all three findings. Every live-apply now goes through one `applyLiveServerSettings`: the settings object, the Network log size, and the roots diff together. A close with a pending debounce applies the draft to all three, so reconciling only `setServerSettings` when that save rejected left the server advertising roots that were never persisted and the log sized for them. The "this server's draft is unpersisted" flag moves from a single id in `App.tsx` into the tracker, which already orders writes by issue and keeps state per server. One id could be overwritten by a failure for a different server, or cleared by a stale success, either of which made a close apply a rejected draft. And when nothing has ever landed for a server, close falls back to the empty settings the toggle rollback uses rather than to the draft — the old fallback re-applied the very values that had just failed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UL9retfvAXRgvi6EWk4SY4 Signed-off-by: cliffhall <cliff@futurescale.com> |
||
|
cliffhall
|
a935de08dc |
fix(web): re-apply the write that settles after an overlapping one failed
Review round 5 on #2091, both findings. The other overlap order was still wrong. Write 1 (`true`) is in flight when write 2 (`false`) fails, so the rollback resolves to the stale entry's `false` — correct at that instant, nothing had landed. Write 1 then lands `true` and only touched refs, so if its own list reload had failed too, no render would ever correct the UI or the live client while disk held `true`. Writes now report both outcomes: `landed` returns whether this write is the settled state for its server — recorded, with nothing later still in flight — and `failed` takes a write out of flight so an earlier one that lands afterwards settles. The toggle re-applies the settled value to the override and the client, gated on its server still being active. Serializing per server was the alternative and was not taken: it makes the modal's debounced saves queue behind each other and changes the write path to fix a UI-reconciliation concern, and it still could not order two requests already in flight. The override reset also moves from a `useEffect` to `useValueChange`, which is what this repo requires for resetting state from render data — the entry is referentially stable between reads, and keying on it alone subsumes the persisted boolean and the server id. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UL9retfvAXRgvi6EWk4SY4 Signed-off-by: cliffhall <cliff@futurescale.com> |
||
|
cliffhall
|
ce29d8273d |
fix(web): resolve the rollback baseline at failure time, not at issue
Review round 3 on #2091. The baseline was still captured when the write went out, so overlapping toggles reproduced the original bug through the new machinery: toggle 1 PUTs `true` while its reload is pending, toggle 2 reads the baseline and sees `false`, toggle 1 lands `true`, toggle 2 fails — and its catch pushed the `false` it had closed over, contradicting both disk and the hook. `resolve` now reads the record and the list live and takes no fallback, returning `undefined` when neither has anything to say, so calling it in the failure handler answers for the state at failure time. Both the value being written and the rollback go through it, each at the moment it is needed. The hook says explicitly not to hoist the result. The regression test drives the exact order — issue 1, issue 2, land 1, fail 2 — with deferred promises rather than timing, and fails on the issue-time capture. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UL9retfvAXRgvi6EWk4SY4 Signed-off-by: cliffhall <cliff@futurescale.com> |
||
|
cliffhall
|
7fa42299c7 |
fix(web): order overlapping settings writes by issue, not by completion
Review round 2 on #2091. Recording on completion made the *last to finish* the winner, and `updateServerSettings` waits for a list read after its PUT — so an older write with a slow read finishes last and would overwrite the record with a value a newer write had already replaced on disk. Both writers can overlap: two quick toggles, or the settings modal's debounced flush. A write is now announced with `begin` when it is issued and confirmed with `landed` when it completes, and a confirmation is dropped if a later-issued write for that server has already confirmed. The high-water mark is kept apart from the record so a straggler cannot resurrect a record a fresh list read dropped. Issue order is a proxy for disk order rather than a proof of it — HTTP promises nothing about two in-flight PUTs — but it is the best signal on this side until the servers route returns a revision to order by. Noted on the hook alongside the other consequence of that gap. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UL9retfvAXRgvi6EWk4SY4 Signed-off-by: cliffhall <cliff@futurescale.com> |
||
|
cliffhall
|
e359368892 |
fix(web): key the rollback baseline per server, and pair it at completion
Review round 1 on #2091. A single record slot let a save on one server discard the baseline a later failed write on another still needed — the settings modal opens for any server, connected or not, so that sequence reproduces #2089 through the very mechanism meant to prevent it. Records are now kept per server id. The record also paired with the `servers` entry captured when the write was *issued*, so a list read landing while the write was in flight was mistaken for one that happened after it and discarded a live record. It now pairs with the entry as of completion, read through a ref (mirrored in an effect — writing a ref during render is a `react-hooks/refs` error). The residual — a read whose response predates the write but arrives after it — needs a revision or ETag `useServers` does not expose, and is documented on the hook. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UL9retfvAXRgvi6EWk4SY4 Signed-off-by: cliffhall <cliff@futurescale.com> |
||
|
cliffhall
|
0bcb7ec78a |
fix(web): roll a failed settings write back to what actually landed
The pagination toggle read its rollback baseline off the `servers` entry, which only advances on a successful `GET /api/servers`. Once a list read fails that entry describes disk as it was before every write made since, so the #1721 rollback reverted to a value nothing on disk holds — and clearing the optimistic override fell back to the same stale entry, so the UI was wrong from both directions. Remember the last settings write known to have landed (a new gated hook, `useLastPersistedSettings`) and roll back to that instead, keyed to the `servers` entry it was derived from so a fresh list read supersedes it. Both settings writers — the toggle and the settings modal's debounced persist — feed the one record. Closes #2089 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UL9retfvAXRgvi6EWk4SY4 Signed-off-by: cliffhall <cliff@futurescale.com> |
||
|
cliffhall
|
b49d4f20f5 |
fix(web): stop the remaining consumers calling a landed write a failure
Copilot review r3. Three findings, all the same root: a ServerListReloadError rejects after the write succeeded, so any consumer still reading "rejection" as "the write failed" now says something false. - `useImportClientConfig.applyEntry` gave a successfully imported server a red "Failed" badge. It now reports the intended status and puts the reload message in `detail`, which renders in red beneath it either way — so the failure stays visible without claiming the row wasn't created. - The settings-draft toast said "Failed to save settings". It now branches its title; this toast is the only signal there, since the modal has usually closed by the time a flush rejects. - An edit modal left open by a failed reload kept targeting the old id. When the row landed under the new one, `configModalTarget` went undefined and ServerConfigModal's own reset blanked the open form and cleared the error it was showing. An edit/clone modal whose target has left the list now closes — adjusted during render via `useValueChange`, so no frame paints the blanked form. That also covers the ordinary case of an external mcp.json edit removing the server being edited. The reachability here is not remote: the write is what triggers the backend's change broadcast, so the SSE-driven refresh that lands the renamed row is the likely next event. Also adds the regression test r3 asked for on the add path — the highlight batch has to be marked from the catch for the same reason the rename is. Both new App tests and the import one fail without their fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UL9retfvAXRgvi6EWk4SY4 Signed-off-by: cliffhall <cliff@futurescale.com> |
||
|
cliffhall
|
de60983ccc |
fix: surface list-fetch failures in paginated mode (#1998)
#1954 wired the list-load error off the aggregate managed stores only. With `paginatedLists` on, those stores deliberately skip their all-page walk, so their error is permanently null — while the paged stores that actually drive the sidebar had no error state at all and `loadPage` had a try/finally with no catch. A failing list showed an empty panel with no alert and no Retry, and the connect-time `void loadPage(undefined)` left an unhandled rejection. - Give PagedTools/Prompts/ResourcesState observable error state: an `errorChange` event plus `getError()`, set in a new catch in `loadPage`, cleared on the next success and on disconnect. The rejection is still re-thrown, matching ManagedListState — callers' auth-recovery wrappers key off it to detect a 401. - Catch the connect-time load's rejection (`.catch(() => {})`) now that the failure is recorded as state rather than lost. - Select the error by mode in `usePaginatedList`, the same way `items` already is, and read it in App.tsx from the pagination model. - Rename `useManagedListError` to `useListError`: both store families expose an identical `errorChange`, so the hook declares that one-event contract itself instead of importing ManagedListEventMap. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0177mPHAdECD18nLLCTwR5rh Signed-off-by: cliffhall <cliff@futurescale.com> |
||
|
Cliff Hall
|
834fd2d77d |
chore(deps): migrate to eslint 10 to clear the brace-expansion DoS advisory (#1900)
* chore(deps): migrate to eslint 10 to clear the brace-expansion DoS advisory (#1838) eslint 9 pulled `minimatch@3` → `brace-expansion@1.1.16`, which carries GHSA-mh99-v99m-4gvg (high, DoS via unbounded expansion length). It appeared in all five lockfiles — the root plus each of the four clients, none of which is an npm workspace, so all five had to move together. Deferred from the pre-2.0.0 sweep because it is dev-scope, not attacker- controlled, and a major bump across five packages was the wrong thing to attempt immediately before an irreversible publish. This is that work. ## What moved - `eslint` `^9.39.4/5` → `^10.8.0` and `@eslint/js` → `^10.0.1`, in all five manifests. - `eslint-plugin-react-hooks` `^7.0.1` → `^7.1.1` in web and tui. Required, not cosmetic: 7.0.1's peer range stops at `^9.0.0`, so installing it alongside eslint 10 fails `ERESOLVE`. - Everything else already declared eslint 10 support and stayed put: `typescript-eslint@^8.56.1` (`^8.57 || ^9 || ^10`), `eslint-plugin-react-refresh@^0.5.2` (`^9 || ^10`), `eslint-plugin-storybook@^10.2.19` (`>=8`). ## Config migration None was needed. All five configs were already flat (`eslint.config.js` with `defineConfig`/`globalIgnores` from `eslint/config`), which is the format v10 keeps and the only one it still supports. The root config's `core/` and shared- surface blocks — including the `_`-prefix `argsIgnorePattern` / `varsIgnorePattern` / `caughtErrorsIgnorePattern` — are unchanged and still gate `lint:core` and `lint:shared`. No rule was disabled, downgraded, or scoped away to make the new eslint pass. v10 surfaced 21 real violations across three rules; all 21 are fixed in the code. ### `preserve-caught-error` (new in `eslint:recommended`) — 15 sites A `throw new Error(msg)` inside a `catch` silently drops the original error. Each now passes `{ cause }`, so the underlying failure survives into the chain. Messages are unchanged, so nothing that asserts on them moves. 12 in `core/`, 2 in `clients/cli`, 1 in `test-servers/src`. ### `no-useless-assignment` (new in `eslint:recommended`) — 2 sites `clients/cli/src/cli.ts`'s `optionArgs` and `test-servers/src/test-server-oauth` 's `valid` were initialized and then unconditionally overwritten on every path. Both are now declared without the dead initializer; TypeScript's definite- assignment analysis covers the remaining paths. ### `react-hooks/set-state-in-effect` — 8 sites (web) These were false negatives the plugin fixed in 7.1.0, not a rule we newly opted into: `set-state-in-effect` has been `error` in `flat/recommended` since 7.0.1, and the web config has always extended it. Every one was the same anti-pattern — resetting or re-syncing local state from a prop inside a `useEffect`, which paints the stale value for one frame and then renders again to correct it. All eight now use a new `useValueChange(value, onChange)` hook (`clients/web/src/hooks/useValueChange.ts`), which is React's documented "adjusting state during render": compare against the previous render's value with `Object.is` and call back *during* render, so React discards the in-progress output before it reaches the DOM. - `MrtrConversation`, `ProtocolEntry`, `TaskCard` — mirror `isListExpanded`. - `NetworkEntry` — same mirror, plus the "Reveal in Network" force-open. The reveal's rAF scroll stays an effect (real external-system work); only the `setIsExpanded(true)` moved. `isExpanded` is now seeded from `isListExpanded || revealed`, since a render-time sync fires on change and so cannot cover an entry that mounts already revealed. - `PromptArgumentsForm`, `ResourceTemplatePanel` — reset on template/prompt switch. - `ServerConfigModal` — reset on open. Keyed on `opened ? initial : undefined`, which collapses "opened flipped" and "initial changed while open" into one value; `initial` is already memoized, so it cannot thrash. `useValueChange` has its own test file, and `ServerConfigModal` gains an open→edit→close→reopen test (the close leg was previously untested, which is what left the new guard uncovered). ## Advisory delta `npm audit`, per package, before → after: | Package | before | after | | ------------------- | -------------------------- | -------------------------- | | root | 9 (1 low, 6 mod, 2 high) | 8 (1 low, 6 mod, 1 high) | | `clients/web` | 19 (3 low, 6 mod, 6 high, 4 crit) | 17 (3 low, 6 mod, 4 high, 4 crit) | | `clients/cli` | 6 (2 low, 4 high) | 4 (2 low, 2 high) | | `clients/tui` | 4 (1 low, 3 high) | 2 (1 low, 1 high) | | `clients/launcher` | 3 (3 high) | 1 (1 high) | `brace-expansion` is gone from all five — `eslint@10` resolves `minimatch@10` → `brace-expansion@5.0.9`. Dropping `@eslint/eslintrc` (removed in v10) also took `js-yaml` with it, clearing GHSA-52cp-r559-cp3m (high) from all four clients as a bonus. The remainder are unrelated and tracked separately. `npm run ci` passes end to end — validate, the per-file ≥90 coverage gate, the build gate, all five smokes (including `smoke:tui`, run locally on a real TTY), and 462 Storybook tests. Also documents the effect rule in AGENTS.md and mirrors it into `.github/copilot-instructions.md`, since it is now something a reviewer cites. Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: state useValueChange's referential-stability requirement (#1838) Copilot review on #1900: the hook compares with `Object.is`, so an object or array literal rebuilt in the component body compares unequal every render. Because the callback is what updates state, that is not merely a wasted render — it is an infinite loop. Every current caller passes a primitive (`boolean`, `string`) or an already- memoized value, so nothing changes behaviorally. But the hazard was implicit, and the next caller is the one it would bite. Documented on the hook itself, and one clause added to the AGENTS.md rule with its mirror in `.github/copilot-instructions.md`. No runtime guard: the requirement is the same one a `useEffect` dependency array carries, and here the failure is loud and immediate rather than silent. `npm run ci` passes. Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(cli): stop asserting the --tool-args-json catch binding is an Error (#1838) Copilot review round 2 on #1900: the `--tool-args-json` parse handler formatted its message with `(e as Error).message`. A `catch` binding is `unknown`, so that cast is an assertion rather than a check — a thrown non-Error would render `--tool-args-json is not valid JSON: undefined`, which points the user at nothing. `JSON.parse` does only throw `SyntaxError` today, so this is not a live bug. But the cast is the kind AGENTS.md asks us not to leave unjustified, the same handler was already being touched here to add `{ cause: e }`, and every other site in this PR uses the guarded form. Now it does too. `npm run ci` passes. Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: require useValueChange's onChange to be render-pure (#1838) Copilot review round 3 on #1900: `onChange` is called during render, so it is bound by render-phase purity — and the web client mounts under `StrictMode`, which deliberately double-renders in development. A callback doing anything external would run an unpredictable number of times, and concurrent React can abandon an in-progress render entirely. Every current caller already passes a callback that only calls `setState`, so nothing changes. But "runs during render" is the whole point of the hook and the constraint that follows from it was left implicit. Stated on the hook, with the same clause in the AGENTS.md rule and its `.github/copilot-instructions.md` mirror. All three now point at `NetworkEntry` as the worked example of the split: the reveal's force-open is a state update and lives in `useValueChange`, while its `requestAnimationFrame` scroll stays a `useEffect`. `npm run ci` passes. Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Cliff Hall
|
4defa32ac6 |
chore: close a batch of quality-gate & tidy gaps (#1785 #1787 #1789 #1790 #1767) (#1792)
* chore: close a batch of quality-gate & tidy gaps (#1785 #1787 #1789 #1790 #1767) Bundles five small, related tidy/gate items into one PR: - #1785: delete the dead src/types/navigation.ts InspectorTab (0 importers; duplicated/drifted from utils/inspectorTabs InspectorTabId). src/types/ now holds only ambient .d.ts stubs. - #1787: add src/hooks/** to the web coverage `include` (it was silently outside the ≥90 gate) and add tests for the two untested hooks, useServerJsonImport and useImportClientConfig, bringing every hook to ≥90 on lines/statements/functions/branches. - #1789: widen clients/{cli,tui,launcher} format/format:check to their top-level configs; add root format:shared / format:check:shared covering test-servers/src/**, vitest.shared.mts, and the repo-root eslint.config.js. - #1790: pin prettier to an exact version (3.8.4, dropping the ^3.8.1 caret) in all five package.json files so the format gate's verdict can't shift with an in-range patch bump; lockfiles synced. - #1767: its root-file format+lint gap is closed here (format:shared + a new lint:shared via a second files block in the root eslint.config.js; web/server/ was already gated in #1788). Its larger cli/tui __tests__ typecheck piece (76 type errors) is split out to #1791. Prettier-normalized the newly-covered files (pure formatting). No runtime behavior change. `npm run ci` green. Closes #1785 Closes #1787 Closes #1789 Closes #1790 Closes #1767 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * chore: address PR #1792 review findings - #1789 gap: widen clients/web format to include .storybook and clients/ launcher format to include scripts/; reformat the 3 .storybook files (were non-conforming under the pinned prettier 3.8.4). Audited all 885 tracked source files — now fully prettier-clean. - Export VALIDATE_DEBOUNCE_MS / COLLAPSE_DELAY_MS / HIGHLIGHT_DURATION_MS from useServerJsonImport and use them in the test instead of hardcoded 300/1000/250 (so a tuned constant can't silently desync the test). - Assert the symmetric renameErrors["alpha"] alongside ["beta"] so a one-directional collision-check regression fails. - Comment the blank-rename test as exercising a UI-unreachable defensive branch (canImport gates submit). - Un-stale the two specification/v2_ux_interfaces_plan.md references to the now-removed navigation.ts InspectorTab. npm run ci green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * docs: sync AGENTS.md format-surface description with the widened globs (#1792 review r2) 1ce3b64 added .storybook to clients/web's format glob and scripts/ to clients/launcher's, but AGENTS.md:222 still described the pre-fix scope. Bring it in line and note the non-recursive top-level glob is why nested dirs are named explicitly. Docs only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * chore: handle PR #1792 round-3 follow-ups in-place (format-coverage guard, uniform globs, theme gate) Rather than defer these to tickets, fold them into this PR: - Obs 2: widen every client + root format glob to a uniform *.{ts,tsx,mts,cts,js,jsx,mjs,cjs} so a future .mts/.cjs/.tsx top-level file can't silently slip the gate. - Obs 1: add scripts/verify-format-coverage.mjs (wired into `validate`) — it parses the format:check globs out of every package.json, enumerates all tracked source files, and fails listing any not covered by a glob. The durable, standing version of #1789's one-shot audit; negative-tested. - Obs 3: bring src/theme/** under the coverage gate (21/23 files were already ≥90 transitively; add Paper.test.tsx for the one laggard, Paper.ts). src/App.tsx (~4.5k lines, ~42% branches) is converted from a silent omission into an explicit, documented exclusion (comment on the coverage `include` + AGENTS.md) — gating it is a dedicated effort, not a whitelist tweak. npm run ci green (the new guard reports 885/885 tracked source files gated). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * chore: address PR #1792 round-4 findings (guard hardening, docs, exclusion list) - Harden verify-format-coverage: only harvest format:check globs from scripts transitively reachable from `validate` (via npm run refs), so an unwired/renamed format:check no longer counts as coverage. Negative-tested — renaming clients/web format:check to an unreferenced name now fails the guard (627 files flagged) instead of passing. - Name src/main.tsx and src/index.ts (browser + bin bootstraps) alongside src/App.tsx in the coverage-include comment and AGENTS.md, so the documented exclusion list is complete rather than claiming App.tsx is the only one. - Doc sync: document verify:format-coverage (AGENTS.md + README script table + scripts/ layout line), fix the `validate` ordering (verify:format- coverage now runs before validate:core) in AGENTS.md and README, and update the quoted globs to the uniform *.{ts,tsx,mts,cts,js,jsx,mjs,cjs} / test-servers *.{ts,tsx,mts,cts}. npm run ci green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * chore: address PR #1792 round-5 findings (airtight guard + doc/config sync) - verify-format-coverage: assert the root `validate` chain invokes each client's `validate` (cd <dir> && npm run validate), closing the last reachability edge — a client dropped from the root chain now fails the guard even though its own globs still exist. Negative-tested. - eslint.config.js: widen the shared `files` block to test-servers/src/**/*.{ts,tsx,mts,cts} to match the widened lint:shared glob, so a future .mts/.cts there is lint-gated, not just prettier-gated. - AGENTS.md: scope the "uniform extension set" claim to client globs (core stays {ts,tsx}, shared {ts,tsx,mts,cts}), with verify:format-coverage as the backstop — removing the self-contradiction with the line below. npm run ci green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Cliff Hall
|
b3b04c6939 |
Thread cacheMode through managed list refresh via SDK cache-aware verbs (#1721) (#1723)
Some checks failed
Run install, format, lint, build, and test on every push / build (push) Has been cancelled
Run install, format, lint, build, and test on every push / publish (push) Has been cancelled
Run install, format, lint, build, and test on every push / publish-github-container-registry (push) Has been cancelled
* Thread cacheMode through managed list refresh via SDK cache-aware verbs (#1721) Manual/auto refresh of tools/resources/prompts now forces a cache-bypassing round trip on modern servers, delivering the raw-request cacheMode control deferred from #1628. The InspectorClient list/read path uses the SDK's single-page `client.request` verbs (for pagination debugging), which never consult the SDK response cache — so `cacheMode` was inert there. This adds a separate aggregate path that routes through the SDK's cache-aware high-level verbs, where `cacheMode` is honored: - InspectorClient: new `listAllTools/Prompts/Resources/ResourceTemplates( { cacheMode, metadata })` that call `client.listTools()` etc. (all-page aggregate + SEP-2243 x-mcp-header exclusion + cache consult/write). A shared `getCacheableRequestOptions` / `aggregateListParams` back all four. - InspectorClientProtocol: the four `listAll*` methods added. - ManagedListState: delegates page walking to the SDK verb (config `fetchPage` loop → `fetchAll`); `refresh(metadata?, cacheMode?)` threads cacheMode. A `list_changed` auto-refresh bypasses the cache (`cacheMode: "refresh"`). - Managed hooks: user-initiated `refresh()` passes `cacheMode: "refresh"`, so the existing per-tab Refresh buttons become genuinely cache-bypassing — no App wiring change. Connect-time load stays default (`'use'`). The single-page `listTools(cursor)` primitive is unchanged; the paged hooks still surface per-page `nextCursor`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * Single-page list pagination: sidebar toggle + server setting (#1721) Delivers the deferred single-page fetch UI: Tools/Resources/Prompts can be loaded one page at a time instead of auto-aggregating every page — a defensive option for servers with very large lists. - New `singlePageLists` server setting (persisted, server-wide), plumbed through ServerConfig/InspectorServerSettings, serverList convert/default, the node + remote settings paths, and the Server Settings form ("Fetch Lists One Page at a Time"). - New `ListPaginationControls` element (a "Single page" switch + "Load next page" button + "N pages loaded" status) rendered in each list sidebar; the toggle edits the setting live (optimistic) and persists it. - The paged state stores (PagedTools/Prompts/ResourcesState) now own loading: they auto-load page 1 on connect in single-page mode and track nextCursor + page count as observable state; the managed stores skip their all-page walk in single-page mode (so nothing is over-fetched) while still lighting the list-changed indicator. `usePaginatedList` selects the display source and pagination controls per mode — pure, no data-loading in React effects. - Refresh is mode-aware: reload page 1 in single-page mode, refetch the whole aggregate otherwise. Sidebars widened 340→360 so the controls don't crowd list entries. - Test server: `test-servers/configs/pagination-http.json` (12 tools/resources/ prompts, page size 4 → 3 pages each) exercises the flow end to end. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * Add pr-screenshots proving single-page pagination (#1721) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * Address @claude review: template load, double-click guard, auth recovery (#1721) - Fix: resource templates never loaded on connect in single-page mode. The connect-time all-page skip now applies only to lists with a paged counterpart (new `deferInSinglePage` flag on ManagedListConfig; false for resource templates), so templates still aggregate on connect regardless of the setting. - Guard `PagedTools/Prompts/ResourcesState.loadPage` against a concurrent load (fast double-click on "Load next page") so the same page can't be appended twice; the in-flight load owns the cursor and the second call is a no-op. - Thread ambient auth recovery through the single-page refresh, Load-next-page, and mode-switch loads (they previously bypassed it) — usePaginatedList's onRefresh/onLoadMore now return their promises so App can wrap them. - Drop the misleading "aggregated list" wording from the paged-store docs. Tests added for each; npm run coverage green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * Review round 2: revert optimistic toggle when persisting the setting fails (#1721) Non-blocking review follow-up. If updateServerSettings rejects, the optimistic `singlePageListsOverride` is now cleared and the live client setting rolled back (the effect only clears the override when the persisted value changes, which doesn't happen on failure), so the UI and client reflect what's on disk rather than the failed edit. Added an App test for the revert path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * Fix single-page Refresh not clearing list-changed indicator + UI polish (#1721) Round-3 review fix (blocking): in single-page mode a `list_changed` lights the list-changed indicator, but the single-page Refresh reloads page 1 of the paged store and bypassed `useManaged*.refresh()`, so the indicator was never cleared. Expose `clearListChanged` from useManagedTools/Prompts/Resources and call it in the single-page branch of onRefreshTools/Prompts/Resources. Tests added. UI polish (user feedback on the sidebar control): - Rename the switch label "Single page" → "Paginated". - Put "Load next page" on the same row as the switch, space-between (switch fully left, button fully right). - Move the "N pages loaded" status to its own line below the row, centered. - Hide the whole control once the list is known to be a single page (single-page mode, page 1 loaded, no nextCursor) — nothing to paginate. pr-screenshots refreshed to the new layout. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * Rename the singlePageLists setting to paginatedLists for clarity (#1721) `singlePageLists` read backwards from the "Paginated" switch label (it sounds like paginated=false). Rename the server setting to `paginatedLists` with the same value meaning — true = paginated/one-page-at-a-time, false = aggregate-all — and follow through on the related identifiers for consistency: - Setting field `paginatedLists` across ServerConfig / InspectorServerSettings, serverList convert/default, node + remote settings paths, and the form/modal callbacks (onPaginatedListsChange). - `usePaginatedList` / `ListPaginationControls` prop `singlePage` → `paginated`, `onSinglePageChange` → `onPaginatedChange`. - `ManagedListConfig.deferInSinglePage` → `deferWhenPaginated`. - App state (paginatedListsOverride, onTogglePaginatedLists, …) and all tests/fixtures + mode-referring comments. "single-page" wording that denotes the SDK's one-page `client.request` verb (inspectorClient) is intentionally left as-is. Purely a rename — no behavior change; unit + integration + cli + tui + coverage all green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * Format App.test.tsx (prettier) after the paginatedLists rename (#1721) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Cliff Hall
|
97b1a7fbff |
feat(web,core): import client configs and registry server.json (#1348) (#1511)
* feat(web,core): import client configs and registry server.json (#1348) Wire up the two server-import affordances that were previously todoNoop: - Import config: source picker (Claude Desktop / Cursor / Cline / VS Code) read on the backend via a new GET /api/import-source, plus a file-upload fallback. Per-server review with import/skip on new servers and overwrite/skip/rename on id conflicts, then a per-server outcome summary. - Import server.json: parse the MCP registry single-server format (npm/pypi/oci packages + remotes, env vars, package/runtime args, URL template vars) into a runnable config, with a file picker and debounced live validation; Add Server is disabled until validation passes. Core gains an isomorphic import strategy layer (core/mcp/import) with a strategy registry, client-config + server.json parsers, a strategy-agnostic merge, and a dependency-injected source resolver. useServers exposes importSource(). Modal headers are bold and the redundant server.json section header is folded into the modal title. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(web): collapse the server.json textarea into a File Contents disclosure Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(web): auto-collapse File Contents disclosure after content loads Once content is pasted or a file is loaded, collapse the File Contents disclosure after ~1s with Mantine's built-in accordion animation, signaling the input was accepted and surfacing the validation/env-var sections. The disclosure becomes controlled by the modal; clearing the textarea re-opens it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(web): flash File Contents hover highlight before auto-collapse Before the File Contents disclosure auto-collapses, paint it with its hover background for 250ms, then run the collapse over a 325ms animation, so the collapse reads as intentional. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(web): rename "Import server.json" to "Import from registry config" Rename the Add Servers menu entry and the modal header. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(web): rename "Import config" to "Import client config" Rename the Add Servers menu entry and the modal header. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(web,core): move import orchestration into core helpers + web hooks Make the import logic reusable by CLI/TUI and keep the modals display-only: - core/mcp/import: add parseClientConfig (mcpServers→servers fallback), resolveServerId, selectServerJsonOption, and buildServerConfigForSelection, so the full import (parse → select → build → merge) is callable from any environment. - core/storage: extract the pure validateStoreId into a Node-free store-id module (store-io re-exports it) so the isomorphic import layer can validate ids without pulling Node deps into the browser bundle. - clients/web/hooks: useServerJsonImport and useImportClientConfig own all the wizard/parse/debounce/apply state; ServerImportConfigModal and ServerImportJsonModal are now thin display shells over the dumb panel. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(web): rename "Import client config" to "Import from client config" Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(web): remove the Cancel button from the registry import modal The modal header X (and Escape) still dismiss it; drop the redundant Cancel button (and the panel onCancel prop). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(web): hide validation results + name override until content is present In the registry import modal, the Validation Results section and the Server Name override field stay hidden until something has been pasted/loaded into the textarea. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(web): make Validation Results a collapsible disclosure Wrap the registry import modal's validation results in a Mantine accordion disclosure (default open), matching the File Contents section. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(web): Server Name disclosure with read-only derived name + override Make Server Name a collapsible disclosure containing a read-only "From configuration" field (the id derived from the server.json name) and the editable Override field. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(web): scroll to and highlight a freshly-added server After a manual add/clone or an import (client config / registry), the new server's card scrolls into view and draws an animated border (Mantine Border Animate); clicking anywhere on the card dismisses it. - App tracks highlightedServerId; addServerHighlighted sets it (edits and conflict-overwrites don't). Threaded through InspectorView -> ServerListScreen -> ServerCard. - ServerCard scrolls into view on highlight and wraps in <BorderAnimate>; onClick clears the highlight. - Pin @gfazioli/mantine-border-animate@2.0.3 (Mantine 8 compatible; 2.0.4+ requires Mantine 9). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(web): highlighted card fills its cell and its controls work on first click - The BorderAnimate wrapper is display:flex, so the card was shrinking to content width while the border spanned the cell. Stretch the card with w=100% (and the wrapper). - Always render BorderAnimate and toggle show/animate instead of conditionally wrapping: swapping the element type on clear remounted the card, swallowing the click that triggered it (e.g. the connect toggle fired clear but not connect, needing a second click). Verified: one click now connects + clears. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(web,core): address review — OCI env, untrack fixture, rename validation - Untrack the stray inspector-v1-multi.server.json (accidentally committed) and gitignore root *.server.json fixtures. - core/mcp/import/serverJson: forward declared env vars into OCI containers via '-e KEY' docker args (they previously only set the docker CLI process env); add a test. Clarify buildServerConfig (destructure env out instead of spread-then-delete) and document value-vs-default env handling. - useImportClientConfig: validate 'rename' targets inline (invalid charset / collision with existing, additions, or other renames) and gate the Import button on canImport; surface the error on the field. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(web): highlight all servers from a batch import, not just the last Track highlighted servers as a set of ids instead of one. addServerHighlighted accumulates each id (a multi-server client-config import highlights them all); opening an add/import modal starts a fresh batch. Clicking a card clears only that card's highlight; only the first highlighted card scrolls into view. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Cliff Hall
|
8923a9bbf9 |
feat(web): tool progress toasts + persist all per-screen state across navigation (#1414, #1417) (#1420)
* feat(web): tool progress toasts + persist tool selection/result across navigation (#1414) Surface incoming notifications/progress as toasts so the user can watch a long-running tool's progress without leaving the tool view — the v2 replacement for v1's always-visible "Server Notifications" shelf. Toasts are keyed by progress stream and updated (not stacked) per tick so a chatty server updates one toast rather than flooding the corner. Progress notifications still land in the History tab via the existing transport-level tracking. Companion change: lift the selected tool and its form values into App.tsx alongside the already-lifted toolCallState, make ToolsScreen controlled, and drop the clear-on-unmount effect. Leaving the Tools tab and returning now preserves selection, inputs, and result. All three reset together on disconnect via resetSessionScopedUiState, preserving the #1368/#1394 behavior. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(web): persist per-screen selection/search/filter across navigation (#1417) Lift every screen's selection, form, search, and filter state out of the screen components into session-scoped state owned by App, passed down as controlled props. The screens unmount on tab switch, so this is what keeps the user's context in place when they navigate away and back within a live session. State is cleared only on disconnect (resetSessionScopedUiState) or an explicit user action — never on plain navigation. Lifted per screen: - Tools: sidebar search (selection/form/result were lifted in #1414) - Prompts: selected prompt, argument values, submitted marker, search - Resources: selected resource/template URI, originating-template marker, search, accordion open-sections - Apps: selected app, form values, search (running/maximized stay local — they're tied to the live iframe/bridge that is torn down on unmount) - Tasks: search + status filter - Logs: filter text + visible-level set - History: search + method filter - Network: filter text + visible-category set The four sidebar *Controls (Tool/Prompt/App/Resource) become controlled for search; ResourceControls also controls accordion open-sections. Level/category default maps move to logLevels.ts / fetchCategories.ts so the screen files keep a single component export. Pagination already persists (it lives in the App-level Paged*State managers), so no work was needed there. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(web): persist list scroll position across navigation + reset tests (#1417) Add useScrollMemory: a module-keyed hook that remembers a Mantine ScrollArea's offset across unmount/remount (the screens unmount on tab switch). Scroll lives in a module-scope map rather than threaded App state — it's ephemeral DOM state, and threading ~16 more props would balloon the surface for no benefit. App calls clearScrollMemory() in resetSessionScopedUiState so offsets clear on disconnect alongside the lifted selection/filter state. Wired into the four main list panels (Logs, History, Network, Tasks) and the Tools/Prompts/Apps sidebar lists. Pagination already persists via the App-level Paged*State managers. Tests: useScrollMemory unit test (save/restore, key isolation, clear), and the App disconnect test now also exercises representative lifted UI state (tool search, prompt selection, log filter) clearing on disconnect. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(web): collapse per-screen lifted state into one {screen}Ui object Replace the ~46 individual selection/search/filter props threaded App → InspectorView → screen with one `{screen}Ui` object + one `on{Screen}UiChange` setter per screen (8 pairs). Each screen reads its fields from `ui` and updates via `onUiChange({ ...ui, field })`; the in-flight result panels (toolCallState / getPromptState / readResourceState) stay separate since they're written by App's async action handlers, not the screens. The `*UiState` interfaces live with their screens; the empty-state defaults are gathered in screenUiState.ts so the screen files keep a single component export (react-refresh), mirroring logLevels.ts / fetchCategories.ts. Trims InspectorView's prop surface substantially and keeps the persistence behavior identical. Tests/stories collapse their stateful wrappers to a single ui object; added Tools/Tasks/History interaction tests to keep per-file coverage above the gate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(web): dismiss visible progress toasts on client teardown (#1414 review) The progressNotification effect cleanup cleared its id-tracking set but left on-screen "Tool progress" toasts to auto-close up to 5s later — they could linger into the next session, and the lingering toast's onClose could delete an id from the new session's set and trigger a duplicate-id re-show. Hide each tracked toast in cleanup so both gaps close. Addresses @claude review note #2. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(web): clear the tool result when a different tool is selected The Tools result panel renders `toolCallState` regardless of which tool is selected, so selecting a new tool used to leave the previous tool's result showing beneath it. Route tools UI changes through an App handler that drops `toolCallState` whenever `selectedToolName` changes; search/form edits keep the selection (and the result) intact. Tests cover both the clear-on-switch and keep-on-search paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
ed2070f67c | ||
|
|
02a56e8e89 | ||
|
|
5f875e3393 | ||
|
|
ed38a5e5fe | ||
|
|
dc026acfea | ||
|
|
104b3c0775 | ||
|
|
2fca87c0ad | ||
|
|
edac58e304 | ||
|
|
e42968ba78 | ||
|
|
bbffafded1 | ||
|
|
7098da7473 | ||
|
|
8b9839cc93 |
||
|
|
b1996d1093 | ||
|
|
75f9c51146 | ||
|
|
db84823262 | ||
|
|
a935de08dc | ||
|
|
ce29d8273d | ||
|
|
7fa42299c7 | ||
|
|
e359368892 | ||
|
|
0bcb7ec78a | ||
|
|
b49d4f20f5 | ||
|
|
de60983ccc | ||
|
|
834fd2d77d |
||
|
|
4defa32ac6 |
||
|
|
b3b04c6939 |
||
|
|
97b1a7fbff |
||
|
|
8923a9bbf9 |