Phase 2: Refactor Tasks cluster to MCP SDK types

[cliffhall]

Summary

• TaskCard: accepts task: Task instead of 14 flat props; derives all display from SDK Task fields (taskId, status, ttl, createdAt, lastUpdatedAt, statusMessage); drops method, target, numeric progress, error, completedAt, elapsed which don't exist on the SDK Task type
• TaskControls: types statusFilter as TaskStatus | undefined; adds runtime validation guard on Select onChange
• TaskListPanel: accepts Task[] instead of TaskCardProps[]; passes onCancel(taskId) through to each TaskCard; uses task.taskId as key
• TasksScreen: accepts Task[] + onCancel(taskId) callback; types internal statusFilter as TaskStatus | undefined
• Updates ConnectedView TasksActive story

Deleted/simplified types

Old	Replaced by
TaskCardProps as data shape (14 flat fields)	Task from SDK (7 fields)
TaskStatus re-export from TaskCard	Import directly from @modelcontextprotocol/sdk/types.js
string status filter	TaskStatus | undefined

Notable changes

The SDK Task type is narrower than the previous TaskCardProps — it doesn't carry method, target, numeric progress, or error. These were application-level concepts that the original component invented; the real MCP Task schema uses statusMessage for progress/error context. Net result: -122 lines.

Test plan

• npm run format — passes
• npm run lint — passes
• npm run build (includes tsc -b) — passes
• Visually verify Storybook stories for TaskCard, TaskControls, TaskListPanel, TasksScreen
• Verify ConnectedView TasksActive story renders correctly

🤖 Generated with Claude Code

[cliffhall]

@claude review

[claude]

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

---

PR Review: Phase 2 — Refactor Tasks cluster to MCP SDK types

• Gather context and read changed files
• Review TaskCard changes
• Review TaskControls changes
• Review TaskListPanel changes
• Review TasksScreen changes
• Review ConnectedView story changes
• Post final review

---

Overall this is a clean, well-scoped refactor — the type surface shrinks significantly and the component API is more honest about what the SDK actually provides. A few issues worth addressing before merge:

---

Bug: Collapse animation never plays

TaskCard.tsx lines 149–168:

{isExpanded && (
  <Collapse in={isExpanded}>
    ...
  </Collapse>
)}

The outer {isExpanded && ...} conditional mounts the Collapse only when isExpanded is already true, so the expand animation is immediately short-circuited (the element is mounted fully visible). The collapse animation on close also never plays — the element is unmounted before Collapse can animate it out.

Fix: remove the outer conditional and let Collapse drive everything:

<Collapse in={isExpanded}>
  ...
</Collapse>

Fix this →

---

UX: statusMessage invisible for collapsed completed/failed/cancelled tasks

TaskCard.tsx lines 145–166:

{statusMessage && isActive && (
  <StatusMessageText>{statusMessage}</StatusMessageText>
)}
// ... in expanded section only:
{statusMessage && !isActive && (
  ...
)}

When a task is failed or cancelled and the card is collapsed, the statusMessage (which contains the error reason) is hidden entirely — the only visible state information is the TaskStatusBadge in the header. A user looking at a collapsed failed task has no way to see why it failed without expanding the card first.

Consider showing statusMessage for non-active tasks in the summary row unconditionally (or at least for failed), not just inside the Collapse section.

---

Minor: "Status" detail field duplicates the badge

Lines 122–124 render a plain-text status detail row alongside TaskStatusBadge in the header. This is redundant — the badge already communicates the status with formatting. The detail row adds noise without adding information. Consider removing it or replacing it with something not already shown (e.g. a human-readable duration since creation).

---

ttl null-guard should be !== undefined

Line 133: {ttl !== null && (...)} — ttl is an optional field on the SDK Task type (ttl?: number), meaning its absent value is undefined, not null. The guard ttl !== null is true when ttl is undefined, which could render a "undefinedms" TTL row.

The correct guard is ttl !== undefined (or the loose ttl != null which catches both). Since the build passes, the SDK may type it as number | null — worth confirming, but if it is optional (?), this is a real bug.

Fix this →

---

Missing tests

Per AGENTS.md, all new/modified code requires tests with ≥90% coverage. This PR has no test additions or updates for TaskCard, TaskControls, TaskListPanel, or TasksScreen. The Storybook stories cover visual regression but not logic (filtering, expand/collapse state, cancel callback wiring, matchesFilters function). Tests should be added before merge.

---

Nits

• TaskControls.tsx line 52: STATUS_OPTIONS.includes(value as TaskStatus) — the guard is technically correct for type narrowing, but since Mantine's Select can only emit values from its data array, the only non-TaskStatus value it can return is null (already filtered by the outer value &&). The guard is harmless but slightly over-defensive.
• TaskListPanel.tsx: matchesFilters is a clean, well-extracted helper — nice.
• Removing the re-export of TaskStatus from TaskCard in favor of direct SDK imports is the right call.

[cliffhall]

Thanks for the review — all actionable items addressed in 3d6280e:

1. Bug: Collapse animation never plays — Fixed. Removed the outer {isExpanded && ...} wrapper so Collapse controls its own mount/unmount animation.

2. UX: statusMessage invisible for collapsed completed/failed tasks — Fixed. statusMessage now renders for all tasks (not just active ones), so failed/cancelled tasks show their error reason without needing to expand.

3. Minor: redundant "Status" detail row — Removed. The TaskStatusBadge in the header already communicates status.

4. ttl null-guard — Fixed. Changed ttl !== null to ttl != null (loose inequality). Confirmed the SDK types Task.ttl as ZodOptional<ZodNumber> (number | undefined), so the strict !== null was indeed wrong.

5. Missing tests — Per the plan (v2_ux_interfaces_plan.md, "Non-goals" section): "Adding tests beyond what Storybook gives us. Vitest unit tests for the refactored components are a follow-up — the AGENTS.md 90% coverage rule will be addressed in its own pass once the interface dust settles." Tests are explicitly deferred to after the interface refactor completes.