Web UI polish: footer (version/copyright), toolbar + monitoring column tweaks

[cliffhall]

Closes #1642
Closes #1639

A batch of web-UI polish (umbrella #1642), including the version display (#1639). Stacked on #1637 (reuses the shared readInspectorVersion() reader); base is set to 1636-v2-publish-bundle so the diff is clean — retarget to v2/main once #1637 merges.

⚠️ Targets v2/main indirectly (non-default branch), so the Closes keywords are cross-references only — close #1642 and #1639 and move their board cards to Done manually on merge, per AGENTS.md.

What's in it

• Version display (Web: show clients/web version in lower-right corner (grey) #1639) — the Inspector version (root package.json) shown in the footer. Backend reads it once via the shared readInspectorVersion() and sends it on GET /api/config (new InitialConfigPayload.version); the browser reads it via a useInspectorVersion hook and renders a VersionBadge.
• Footer copyright — grey "Copyright © Model Context Protocol a Series of LF Projects, LLC." bottom-right; version bottom-left. Both vertically centered in the bottom xl margin band and aligned to the content's left/right margins (versionBadge / copyrightBadge Text theme variants).
• Toolbar — list expand/collapse toggle moved right of "Add Servers"; monitoring column close-icon restyled to match the open-icon PinColumnButton (subtle Button, brand color).
• Monitoring column animation — its screens now use the same fade-up cross-fade as the primary pane, via a shared ScreenStage element (with a fill variant for the column's height).
• Open-monitoring-column button — on the server list, when connected + monitor data exists + column closed, an "open monitoring column" button appears right of the list toggle (reuses PinColumnButton).

Verification

Each change verified live in the running app (screenshots in the thread): footer alignment, toolbar order, the connected-state open-column button (right of the toggle, brand-colored), and the monitoring-column cross-fade (outgoing/incoming screens overlap mid-switch). New hooks/components unit- and Storybook-tested. npm run ci green (per-file ≥90 coverage gate).

🤖 Generated with Claude Code

[cliffhall]

Live verification — v2.0.0 rendered grey in the lower-right corner of the running prod --web app (from the root package.json via GET /api/config):

• position: fixed, color rgb(134,142,150), 12px, pointer-events: none
• 10px from the right edge, 6px from the bottom

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 2m 27s —— View job

---

Review — PR #1641 (Web UI polish)

• Read the diff against origin/1636-v2-publish-bundle
• Review version display (backend + hook + badge)
• Review footer/copyright + theme changes
• Review toolbar / monitoring column / ScreenStage changes
• Check tests & coverage
• Post review feedback

Overall this is clean, well-documented work. The ScreenStage extraction is a nice de-duplication, the legacy-backend tolerance on the version path is thoughtful, and the Mantine-first conventions (theme variants for the badges, --inspector-* tokens, .withProps() subcomponents, ActionIcon-over-Button) are followed throughout. Test coverage is thorough — every VersionBadge/CopyrightBadge/ScreenStage branch and all six useInspectorVersion fetch branches are exercised, and the two toolbar tests (ServerListControls, TaskListPanel) that relied on the now-removed mantine-Button-root class were correctly re-anchored to aria-labels. Nothing here blocks merge; a few observations below.

Suggestions (non-blocking)

1. readInspectorVersion() at module load can crash the backend on a cosmetic failure — clients/web/server/web-server-config.ts:22

const inspectorVersion = readInspectorVersion(import.meta.url);

This runs at top-level import time, and readInspectorVersion throws if it can't locate the root manifest (core/node/version.ts:63). So a failure to resolve a purely cosmetic version label would take down the entire web backend at startup. The manifest is always present in the dev tree and the published tarball, so this is low-severity — but the failure mode is disproportionate to the feature, and the client already degrades gracefully when version is absent (the whole point of the optional field + VersionBadge returning null). Wrapping the read so a resolution failure yields undefined would keep the backend up and just hide the badge:

function readVersionSafely(): string | undefined {
  try {
    return readInspectorVersion(import.meta.url);
  } catch {
    return undefined; // cosmetic — hide the badge rather than crash the server
  }
}
const inspectorVersion = readVersionSafely();

Fix this →

2. Three independent GET /api/config fetches on load — clients/web/src/App.tsx

useSandboxUrl, useServerListWritable, and now useInspectorVersion each fire their own GET /api/config. The new hook's docstring explicitly acknowledges it mirrors the existing two, so this is consistent with the established pattern rather than a regression — but it's three requests for one static payload. Not for this PR, but a shared useInitialConfig hook (fetch once, expose version/sandboxUrl/writable) would be a worthwhile follow-up. Might be worth a tracking issue on the board.

3. Stale docstring on MonitoringScreen — clients/web/src/components/groups/MonitoringScreen/MonitoringScreen.tsx:38-43

The class comment still says it "renders whichever embedded screen node the caller supplies for the active tab," but the body now maps all tabs into ScreenStages (only the active one mounts, via Transition). Same for the value prop comment ("keys into screens to pick what renders below"). Worth a one-line update so the doc matches the cross-fade behavior.

Minor notes

• useInspectorVersion returns loading, which App.tsx doesn't consume — fine, it keeps the hook symmetric with its siblings and is covered by tests.
• ServerListControls renders the open-monitor PinColumnButton purely on onOpenMonitor truthiness (no internal serverCount guard like ListToggle has). Correct in practice since InspectorView's canOpenMonitor requires connected (⇒ a server exists), and gating in the parent is the right call for a dumb component — just noting the asymmetry.
  · 1639-web-version-display

[cliffhall]

Thanks for the thorough review! Addressed all three in a26cbac:

1. Backend crash on cosmetic version-read failure — fixed. Added a non-throwing readInspectorVersionSafe() in core/node/version.ts and switched the web backend to it, so a manifest-resolution failure now hides the badge (version omitted from /api/config) instead of taking down the backend at startup. Put the wrapper in core rather than inline so both paths are unit-tested (resolves → version; unresolvable → undefined); version.ts stays at 100% coverage.

2. Three /api/config fetches — agreed it's worth consolidating into a shared useInitialConfig. Out of scope here (established pattern, not a regression), so I filed it as a tracking issue: #1643 (v2, on the board).

3. Stale MonitoringScreen docstrings — fixed. Updated both the class comment and the value prop comment to reflect that it now wraps every screen in a ScreenStage (only the active tab mounts, cross-fading), matching the primary pane.

Minor notes (loading unused / open-monitor gated only on onOpenMonitor) — left as-is per your reasoning; both are intentional.

npm run ci green. Re-requesting review.

@claude review

[claude]

Claude finished @cliffhall's task in 47s —— View job

---

Re-review — PR #1641 (follow-up commit a26cbac6)

• Review readInspectorVersionSafe() in core/node/version.ts + backend switch
• Review MonitoringScreen docstring fix
• Confirm tests cover the new safe-read paths
• Post review feedback

All three points from the prior review are resolved cleanly. LGTM. 👍

1. Backend crash on cosmetic version-read failure — resolved. readInspectorVersionSafe() (core/node/version.ts:76) wraps the throwing reader in a try/catch → undefined, and the web backend now calls it (clients/web/server/web-server-config.ts:25). The undefined flows cleanly into version: inspectorVersion at line 110 because InitialConfigPayload.version is optional (core/mcp/remote/node/server.ts:99), so the field is simply omitted and the client's existing "no version → hide badge" path takes over. Putting the wrapper in core (rather than inline) was the right call — it keeps both branches unit-tested and version.ts at 100%. The two new tests (version.test.ts:73,78) cover the resolves-→-version and unresolvable-→-undefined paths, and the comment on the temp-dir test explains why it's unresolvable, which is a nice touch.

2. Three /api/config fetches — tracked as #1643. Correctly deferred; it's an established-pattern consolidation, not a regression in this PR. Good to have it on the board.

3. Stale MonitoringScreen docstrings — resolved. Both the class comment and the value prop comment (MonitoringScreen.tsx:9,42) now describe the wrap-each-screen-in-ScreenStage / only-active-tab-mounts / cross-fade behavior, matching the primary pane. Docs match the code now.

Minor notes from the previous pass (unused loading, open-monitor gated only on onOpenMonitor) were intentionally left as-is with sound reasoning — agreed on both.

Nothing outstanding on my end — the branch is in good shape (Closes #1642, #1639; remember the manual issue-close + board-move on merge since it targets a non-default branch, per AGENTS.md).
· 1639-web-version-display

[cliffhall]

GitHub auto-closed this PR when its stacked base branch 1636-v2-publish-bundle was deleted on #1637's merge. The work (same head branch/commits, all review feedback addressed) was re-targeted to v2/main and merged via #1644. Nothing lost.