feat(experimental): derive client toggles from ClientCapabilities

[cliffhall]

Closes #1240.

Summary

Adopts Option B from the design question. The component now accepts the SDK record shape directly:

clientExperimental: ClientCapabilities["experimental"]

instead of a parallel clientToggles: ClientExperimentalToggle[] array of UI metadata.

Toggle metadata (label, description) lives in a static CLIENT_EXPERIMENTAL_TOGGLE_METADATA map inside the component file. Iteration is over Object.keys(clientExperimental ?? {}), so presence in the record == enabled. Unknown keys (in the record but not in the metadata map) render with the raw capability key as the label and no description, per AC.

The onToggleClientCapability(name: string, enabled: boolean) callback signature is unchanged.

Semantic note

Because only keys present in the record render, this panel can only signal disabling a currently-enabled client toggle (the checkbox starts checked; unchecking fires (name, false)). Adding new toggles isn't a UI affordance here — that's a client-init concern. This matches the SDK schema, where experimental is Record<string, object> with no per-key boolean field.

Story coverage

• NoClientToggles — clientExperimental undefined.
• AllKnownClientToggles — both metadata-map keys present.
• MixedKnownAndUnknownToggles — one known key + experimental/futureFeatureXyz rendering with the raw key.
• Existing server-side stories (WithServerCaps, NoServerCaps, WithResponse, WithErrorResponse, WithHistory) continue to pass; the meta default now seeds clientExperimental with one batched-requests toggle so each story shows the section populated.

Test plan

• npm run validate passes.
• npx vitest run --project=storybook src/components/groups/ExperimentalFeaturesPanel — 8/8 stories pass.
• git grep ClientExperimentalToggle clients/web/src — zero matches.
• Visual diff in Storybook: client section renders one row per record key with the indented description for known toggles.

Out of scope

• Server-side capability rendering (unchanged; uses the existing ServerCapabilities['experimental'] prop correctly).
• The JSON-RPC tester portion of the panel.
• Adding metadata for capabilities beyond the two seeded.

🤖 Generated with Claude Code

[cliffhall]

Updated ac5ee8de to render the full toggle universe rather than only the keys present in the record.

What changed

The strict-AC version iterated Object.keys(clientExperimental ?? {}), so:

• Every checkbox started checked (presence == enabled).
• Unchecking removed the key from the record → row disappeared on next render.
• No UI path to re-enable a toggle you'd just turned off.
• The metadata map's labels/descriptions only ever surfaced for keys already in the record.

The refined version computes the rendered set as the union of metadata-map keys and record keys, and derives each row's checked state from name in clientExperimental. So:

• Known toggles always render, regardless of record state.
• Toggling is fully reversible from the UI — uncheck-then-recheck just flips the boolean.
• Unknown record keys (in record but not in the metadata map) still surface with a raw-key label, per the AC.
• The callback signature is unchanged; the parent still removes the key from the record on (name, false) and adds it on (name, true).

Story changes

Stories renamed to describe state instead of record shape:

• NoClientToggles → NoneEnabled (record undefined → both metadata-map toggles render unchecked)
• AllKnownClientToggles → AllKnownEnabled
• MixedKnownAndUnknownToggles → MixedKnownAndUnknown

Coverage matches the AC's three cases. 8/8 stories pass; validate clean.

AC deviation note

The original AC said "Component iterates Object.keys(clientExperimental ?? {})." That phrasing was provisional (the issue's design-question section flagged the dynamic-vs-static tension). I'll add a comment on #1240 capturing the refinement so the AC reads correctly post-merge.

[cliffhall]

@claude review

[claude]

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

---

Code Review

• Gather context and read changed files
• Analyze ExperimentalFeaturesPanel.tsx changes
• Analyze ExperimentalFeaturesPanel.stories.tsx changes
• Post comprehensive review feedback

---

Overall this is a clean, well-reasoned change. Removing the parallel ClientExperimentalToggle[] array in favor of ClientCapabilities["experimental"] eliminates an unnecessary abstraction, and the "full toggle universe" refinement makes the UI genuinely reversible. A few observations below.

---

ExperimentalFeaturesPanel.tsx

enabled derivation (line 216)

const enabled = clientExperimental?.[name] !== undefined;

This is correct. ClientCapabilities["experimental"] is Record<string, object> | undefined, so values can only be object (never undefined), and optional chaining on the undefined case returns undefined. The semantics match exactly.

Worth noting: the PR comment mentions deriving from name in clientExperimental, but the implementation uses the !== undefined form instead. The latter is slightly more precise (avoids prototype-chain false positives for inherited properties), so this is actually the better choice.

getClientToggleNames — no deduplication risk, but worth a glance (lines 148–157)

const known = Object.keys(CLIENT_EXPERIMENTAL_TOGGLE_METADATA);
const unknown = recordKeys.filter(name => !(name in CLIENT_EXPERIMENTAL_TOGGLE_METADATA));
return [...known, ...unknown];

Deduplication is handled correctly: unknown excludes anything in the metadata map. No issues.

Missing empty-state for client section

The server section shows <Text c="dimmed">No experimental capabilities</Text> when serverEntries.length === 0. The client section has no equivalent guard. When CLIENT_EXPERIMENTAL_TOGGLE_METADATA contains entries (as it does now) this is fine — the known keys always render unchecked. But the asymmetry could become visible if the metadata map is ever empty AND the record is empty/undefined: the section would silently render nothing between the title and divider. A small defensive empty-state would make it consistent. Not a blocker given the current metadata always provides entries.

Fix this →

gap={4} raw number (line 218)

<Stack key={name} gap={4}>

Mantine's spacing system accepts named tokens ("xs" = 10px by default) or raw numbers (px). 4 is intentionally tight here to keep the checkbox and its description visually grouped. This works, but using a theme spacing token (e.g., gap={2} in Mantine's 4px-base scale, or gap="xs") would be more consistent with theme-driven spacing. Minor.

ClientToggleMetadata interface not exported

Correct — it's an internal implementation detail. No change needed.

---

ExperimentalFeaturesPanel.stories.tsx

Story coverage is solid:

Story	Scenario
NoneEnabled	clientExperimental: undefined — both known toggles render unchecked
AllKnownEnabled	Both metadata-map keys present — both checked
MixedKnownAndUnknown	One known + one unknown key — unknown renders with raw key as label
Default meta.args	Seeds clientExperimental with batchRequests so other stories show section populated

The story renames (NoClientToggles → NoneEnabled, etc.) are an improvement — describing toggle state rather than record shape is more semantically accurate after the "full toggle universe" shift.

One minor observation: MixedKnownAndUnknown uses experimental/futureFeatureXyz as the unknown key. That's a perfectly readable test value. The story effectively demonstrates the AC's "unknown keys render with the raw capability key as the label" requirement.

---

Summary

Strengths:

• Clean elimination of ClientExperimentalToggle in favor of the SDK type directly
• Toggle reversibility is genuinely fixed by the "full universe" approach
• getClientToggleNames union logic is correct and dedup-safe
• Story coverage hits all three AC cases

Suggestions:

• Add a client-section empty-state (defensive, matches server-section parity)
• Consider a theme spacing token instead of gap={4} (cosmetic)

Neither suggestion is a blocker. The logic is sound and the implementation is well-structured. LGTM.
|

[cliffhall]

Thanks for the second pass. Walking through the two suggestions:

Client-section empty-state — declining. The empty branch is unreachable in practice: the section is only empty when CLIENT_EXPERIMENTAL_TOGGLE_METADATA is empty and clientExperimental is empty/undefined. The metadata map is a hard-coded constant in this file with two entries, so the only way to hit the empty state is to deliberately strip the map — at which point "No experimental capabilities" would be a misleading message anyway (the truth is "the inspector currently models no client toggles," not "the client has none enabled"). AGENTS.md's stance here is "Don't add error handling, fallbacks, or validation for scenarios that can't happen" — this fits that mold. The server-section empty-state exists because serverExperimental is genuinely caller-supplied and can legitimately be empty at runtime; the client-section's universe is partly local, so the analogy doesn't carry.

gap={4} raw number — declining. Raw px gap values are established convention in this codebase: gap={2} appears in PromptListItem, ResourceSubscribedItem, TaskCard (×3), ToolListItem; p={36} in ContentViewer. Mantine's theme spacing scale starts at xs = 10px, which is too loose for a checkbox stacked over its description — you actually need sub-token values to express tight grouping, and the codebase's pattern is to drop to a number rather than introduce a xxs-style theme token for one-off cases. Happy to revisit if a sweep introduces a tighter token, but not for this PR.

Also noted but no action: the name in clientExperimental phrasing in my prior comment vs. the clientExperimental?.[name] !== undefined form in the code — you're right that the latter is slightly more precise (avoids prototype-chain false positives), so the implementation is intentional and the comment was a slight imprecision. I'll keep the code as-is.

No code changes; ready to merge if you're happy.