Phase 2: Refactor History cluster to use MessageEntry

[cliffhall]

Summary

• HistoryEntry: accepts entry: MessageEntry instead of 12 flat props; derives method/target/status from embedded JSON-RPC objects; fixes Collapse animation (same bug as TaskCard PR Phase 2: Refactor Tasks cluster to MCP SDK types #1217); drops childEntries (not part of MessageEntry)
• HistoryControls: types onMethodFilterChange as (string | undefined) instead of (string)
• HistoryListPanel: accepts MessageEntry[] + pinnedIds: Set<string> instead of two separate HistoryEntryProps[] arrays; passes onReplay/onTogglePin by entry ID instead of pre-bound callbacks
• HistoryScreen: accepts MessageEntry[] + pinnedIds + ID-based callbacks; collapses the two-array model into a single list with pinned overlay

Deleted local types

Old type	Replaced by
HistoryEntryProps as data shape	MessageEntry from core/mcp/types.ts
HistoryChildEntry	Dropped (not part of MessageEntry; child requests are separate entries)
Flat timestamp: string, method, target, status, durationMs, parameters, response	Derived from MessageEntry.message (JSONRPCRequest) and MessageEntry.response

Additional fixes

• Collapse animation: removed {isExpanded && <Collapse>} wrapper (same fix as TaskCard in PR Phase 2: Refactor Tasks cluster to MCP SDK types #1217)
• Timestamps: now Date objects (via MessageEntry.timestamp) instead of ISO strings

Test plan

• npm run format — passes
• npm run lint — passes
• npm run build (includes tsc -b) — passes
• Visually verify Storybook stories for HistoryEntry, HistoryControls, HistoryListPanel, HistoryScreen
• Verify ConnectedView HistoryActive story renders correctly

🤖 Generated with Claude Code

[cliffhall]

@claude review

[claude]

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

---

PR Review: Phase 2 — Refactor History cluster to use MessageEntry

• Gather context and read changed files
• Review HistoryEntry component
• Review HistoryControls component
• Review HistoryListPanel component
• Review HistoryScreen component
• Review ConnectedView stories
• Review test coverage and stories
• Summarize findings

---

This is a clean, well-scoped refactoring. The structural direction is correct: components now receive MessageEntry and derive their display properties locally, callbacks are threaded with IDs instead of pre-bound, and the two-array pinned/unpinned model collapses into a single list + Set<string>. A few items worth addressing:

---

Issues

1. extractMethod is duplicated in two files

HistoryEntry.tsx:66-71 and HistoryListPanel.tsx:52-57 define identical functions. If the logic for deriving a method label from a MessageEntry needs to change (e.g. to handle direction: "notification"), it has to be updated in two places. Extract to a shared util — e.g. a helper in core/mcp/ or a history-utils.ts beside these components. Fix this →

// HistoryEntry.tsx:66
function extractMethod(entry: MessageEntry): string {
  if ("method" in entry.message) {
    return entry.message.method;
  }
  return "response";
}

// HistoryListPanel.tsx:52 — identical
function extractMethod(entry: MessageEntry): string {
  if ("method" in entry.message) {
    return entry.message.method;
  }
  return "response";
}

2. Response content excluded from search

matchesFilters in HistoryListPanel.tsx:68 now searches over ${method} ${entry.id} ${JSON.stringify(entry.message)} — the old version included entry.response in the searchable text. If a user searches for an error message (e.g. "Permission denied") that appears only in a response, they'll get no results. Intentional? If so, worth a comment; if not, entry.response should be re-included.

// HistoryListPanel.tsx:68 — response content is no longer searched
const searchable =
  `${method} ${entry.id} ${JSON.stringify(entry.message)}`.toLowerCase();
// was: `${entry.method} ${entry.target ?? ""} ${entry.timestamp} ${responseText}`

3. Collapse always renders a "Request:" block — even when entry has no params

HistoryEntry.tsx:147-173: the <Collapse> now unconditionally renders a "Request:" ContentViewer with the full raw JSON-RPC message. Entries with no params (e.g. tools/list) will show the bare {"jsonrpc":"2.0","id":4,"method":"tools/list"} object. That's more information than the old "Parameters" block but also more cluttered for simple requests. Worth deciding whether to still gate on "params" in entry.message for the request display.

---

Minor Notes

value ?? undefined is correct but slightly odd (HistoryControls.tsx:41). null ?? undefined evaluates to undefined, so the conversion from Mantine's string | null to string | undefined is correct. An alternative that reads more clearly: value !== null ? value : undefined. Not blocking.

formatTimestamp uses full ISO (HistoryEntry.tsx:58-60): date.toISOString() produces "2026-03-17T10:30:00.000Z". This was the same representation as before (the parent passed an ISO string), so no regression, but a locale-friendly HH:mm:ss or relative format would be more readable in a dense history list. Nice-to-have for later.

Set<string> stability: HistoryListPanelProps.pinnedIds is a Set<string>. In the live app (not stories), the consumer must ensure the Set reference changes when the contents change (e.g. via useState + spread-to-new-Set on toggle) or React won't see the update. This is a standard React concern, not a bug here, but worth noting in the implementation guide for whoever wires up the state.

---

What Works Well

• The Collapse animation fix is correct — removing {isExpanded && <Collapse>} and using <Collapse in={isExpanded}> directly is the right fix (matches PR Phase 2: Refactor Tasks cluster to MCP SDK types #1217).
• extractStatus → "pending" | "success" | "error" is a genuine improvement; "Pending" badge for in-flight requests is much better UX than the old binary.
• Props threaded with IDs (onReplay(id) / onTogglePin(id)) is a good inversion — parents no longer pre-bind per-entry callbacks.
• The HistoryControlsProps.onMethodFilterChange: (string | undefined) fix is correct and the resulting onMethodFilterChange={setMethodFilter} passthrough in HistoryScreen is cleaner.
• Storybook fixtures are well-structured realistic JSON-RPC payloads.

[cliffhall]

Thanks for the review — all three issues addressed in 986659a:

1. Duplicated extractMethod — Extracted to clients/web/src/components/groups/historyUtils.ts, imported by both HistoryEntry and HistoryListPanel.

2. Response content excluded from search — Fixed. matchesFilters now includes JSON.stringify(entry.response) in the searchable text, so searching for error messages like "Permission denied" works again.

3. Collapse always renders Request block — Fixed. Changed to only show a "Parameters:" block when entry.message.params exists, so simple requests like tools/list don't show a cluttered raw JSON-RPC object. This matches the old "Parameters" behavior more closely.