From c21f1f09ce34877195c4bec1cabec7513cb88523 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Wed, 9 Sep 2026 17:35:04 +0100 Subject: [PATCH] fix: restore MCP status after task switches (#3547) (#3552) --- apps/web/lib/state/hydration/hydrator.test.ts | 35 ++ apps/web/lib/state/hydration/hydrator.ts | 31 +- .../mcp-attachment-reconciliation.ts | 155 +++++++++ .../lib/state/slices/session/session-slice.ts | 19 ++ .../session/set-task-sessions-mcp.test.ts | 303 ++++++++++++++++++ docs/plans/restore-session-mcp-status/plan.md | 102 ++++++ ...task-01-restore-session-list-mcp-status.md | 100 ++++++ .../requirements/mcp-session-observability.md | 5 +- .../mcp-session-observability.md | 42 ++- 9 files changed, 789 insertions(+), 3 deletions(-) create mode 100644 apps/web/lib/state/slices/session-runtime/mcp-attachment-reconciliation.ts create mode 100644 apps/web/lib/state/slices/session/set-task-sessions-mcp.test.ts create mode 100644 docs/plans/restore-session-mcp-status/plan.md create mode 100644 docs/plans/restore-session-mcp-status/task-01-restore-session-list-mcp-status.md diff --git a/apps/web/lib/state/hydration/hydrator.test.ts b/apps/web/lib/state/hydration/hydrator.test.ts index b42f43d4cd..d637f30c6d 100644 --- a/apps/web/lib/state/hydration/hydrator.test.ts +++ b/apps/web/lib/state/hydration/hydrator.test.ts @@ -649,6 +649,41 @@ describe("hydrateState — session runtime model state", () => { expect(systemResult.system).toEqual(defaultState.system); }); }); + +describe("hydrateState — route MCP status freshness", () => { + it("keeps newer live MCP state when force-merging an older route snapshot", () => { + const routeHistory: MCPAttachmentHistory = { + version: 1, + current: { + attachment_attempt_id: "attempt-route", + started_at: "2026-06-10T00:00:00.000Z", + servers: [{ name: "route", status: "connected" }], + }, + }; + const liveHistory: MCPAttachmentHistory = { + version: 1, + current: { + attachment_attempt_id: "attempt-live", + started_at: "2026-06-11T00:00:00.000Z", + updated_at: "2026-06-11T00:01:00.000Z", + servers: [{ name: "live", status: "active" }], + }, + }; + const result = produce(makeAppDraft(), (draft: Draft) => { + draft.sessionMcpStatus.bySessionId["session-1"] = liveHistory; + hydrateState( + draft, + { + sessionMcpStatus: { bySessionId: { "session-1": routeHistory } }, + } as unknown as Partial, + { activeSessionId: "session-1", forceMergeSessionId: "session-1" }, + ); + }); + + expect(result.sessionMcpStatus.bySessionId["session-1"]).toEqual(liveHistory); + }); +}); + it.each([true, false])( "turn hydration marks a session only when it is force-merged", (forceMerge) => { diff --git a/apps/web/lib/state/hydration/hydrator.ts b/apps/web/lib/state/hydration/hydrator.ts index eb976de1e7..bda9b2b019 100644 --- a/apps/web/lib/state/hydration/hydrator.ts +++ b/apps/web/lib/state/hydration/hydrator.ts @@ -15,6 +15,11 @@ import { reconcileActiveTurnAfterHydrationDraft, seedSettledSessionBoundaries, } from "@/lib/state/slices/session/turn-actions"; +import type { MCPAttachmentHistory } from "@/lib/state/slices/session-runtime/types"; +import { + readMcpAttachmentHistory, + shouldReplaceMcpAttachmentHistory, +} from "@/lib/state/slices/session-runtime/mcp-attachment-reconciliation"; import { preserveOmittedExecutorFields } from "@/lib/kanban/map-task"; import { deepMerge, mergeSessionMap, mergeLoadingState } from "./merge-strategies"; @@ -453,6 +458,28 @@ function hydrateSessionRuntime( mergeSessionMap(target.bySessionId, source.bySessionId, activeSessionId, forceMergeSessionId); }; + /** Hydrate MCP history without allowing a stale forced route snapshot to regress live evidence. */ + const mergeHydratedMcpStatus = (source: Record | undefined): void => { + if (!source) return; + const target = draft.sessionMcpStatus.bySessionId as unknown as Record< + string, + MCPAttachmentHistory + >; + for (const [sessionId, rawHistory] of Object.entries(source)) { + const shouldForceMerge = forceMergeSessionId === sessionId; + if (!shouldForceMerge && sessionId === activeSessionId) continue; + + const incoming = readMcpAttachmentHistory(rawHistory); + if (!incoming) continue; + + const existing = target[sessionId]; + if (!shouldForceMerge && existing) continue; + if (shouldReplaceMcpAttachmentHistory(existing, incoming)) { + target[sessionId] = incoming; + } + } + }; + if (state.terminal) deepMerge(draft.terminal, state.terminal); if (state.shell) { mergeSessionMap( @@ -482,7 +509,9 @@ function hydrateSessionRuntime( Object.assign(draft.environmentIdBySessionId, state.environmentIdBySessionId); } mergeBySession("sessionModels"); - mergeBySession("sessionMcpStatus"); + mergeHydratedMcpStatus( + state.sessionMcpStatus?.bySessionId as Record | undefined, + ); if (state.agents) deepMerge(draft.agents, state.agents); mergeBySession("prepareProgress"); } diff --git a/apps/web/lib/state/slices/session-runtime/mcp-attachment-reconciliation.ts b/apps/web/lib/state/slices/session-runtime/mcp-attachment-reconciliation.ts new file mode 100644 index 0000000000..c5c32b589f --- /dev/null +++ b/apps/web/lib/state/slices/session-runtime/mcp-attachment-reconciliation.ts @@ -0,0 +1,155 @@ +import { parseStrictRfc3339Timestamp } from "@/lib/utils/strict-timestamp"; +import type { + MCPAttachmentHistory, + MCPAttachmentServer, + MCPAttachmentStatus, + MCPToolSummary, +} from "./types"; + +const MCP_ATTACHMENT_STATUS_VALUES: readonly MCPAttachmentStatus[] = [ + "unknown", + "delivered", + "connected", + "active", + "failed", + "filtered", + "unavailable", +]; + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +function isNonEmptyString(value: unknown): value is string { + return typeof value === "string" && value.trim().length > 0; +} + +function isOptionalString(value: unknown): boolean { + return value === undefined || value === null || typeof value === "string"; +} + +function isOptionalBoolean(value: unknown): boolean { + return value === undefined || value === null || typeof value === "boolean"; +} + +function isOptionalNonNegativeInteger(value: unknown): boolean { + return ( + value === undefined || + value === null || + (typeof value === "number" && Number.isInteger(value) && value >= 0) + ); +} + +function isOptionalTimestamp(value: unknown): boolean { + return ( + value === undefined || + value === null || + (typeof value === "string" && parseStrictRfc3339Timestamp(value) !== null) + ); +} + +function isOptionalStringFields( + value: Record, + fields: readonly string[], +): boolean { + return fields.every((field) => isOptionalString(value[field])); +} + +function isMcpAttachmentSource(value: unknown): boolean { + return value === undefined || value === null || value === "kandev" || value === "profile"; +} + +function isMcpAttachmentStatus(value: unknown): value is MCPAttachmentStatus { + return ( + typeof value === "string" && MCP_ATTACHMENT_STATUS_VALUES.includes(value as MCPAttachmentStatus) + ); +} + +function isValidMcpToolSummary(value: unknown): value is MCPToolSummary { + if (!isRecord(value) || !isNonEmptyString(value.name)) return false; + return ( + isOptionalString(value.description) && + isOptionalBoolean(value.input_schema_truncated) && + isOptionalNonNegativeInteger(value.estimated_tokens) + ); +} + +function isValidMcpAttachmentServerFields(value: Record): boolean { + return ( + isMcpAttachmentSource(value.source) && + isOptionalStringFields(value, [ + "transport", + "target", + "reason_code", + "summary", + "connection_id", + "tool_token_estimator", + ]) && + isOptionalTimestamp(value.tools_listed_at) && + isOptionalNonNegativeInteger(value.tool_count) && + isOptionalBoolean(value.tool_catalog_truncated) && + (value.tools === undefined || + value.tools === null || + (Array.isArray(value.tools) && value.tools.every(isValidMcpToolSummary))) + ); +} + +function isValidMcpAttachmentServer(value: unknown): value is MCPAttachmentServer { + return ( + isRecord(value) && + isNonEmptyString(value.name) && + isMcpAttachmentStatus(value.status) && + isValidMcpAttachmentServerFields(value) + ); +} + +function isValidMcpAttachmentAttempt(value: unknown): value is MCPAttachmentHistory["current"] { + if (!isRecord(value)) return false; + if (!isNonEmptyString(value.attachment_attempt_id)) return false; + if ( + typeof value.started_at !== "string" || + parseStrictRfc3339Timestamp(value.started_at) === null + ) { + return false; + } + if (!isOptionalTimestamp(value.updated_at)) return false; + return ( + value.servers === undefined || + value.servers === null || + (Array.isArray(value.servers) && value.servers.every(isValidMcpAttachmentServer)) + ); +} + +function isValidMcpAttachmentHistory(value: unknown): value is MCPAttachmentHistory { + if (!isRecord(value) || value.version !== 1 || !isValidMcpAttachmentAttempt(value.current)) { + return false; + } + return ( + value.previous === undefined || + value.previous === null || + (Array.isArray(value.previous) && value.previous.every(isValidMcpAttachmentAttempt)) + ); +} + +/** Returns the validated persisted MCP projection, or null for unsupported data. */ +export function readMcpAttachmentHistory(value: unknown): MCPAttachmentHistory | null { + return isValidMcpAttachmentHistory(value) ? value : null; +} + +function mcpHistoryFreshness(history: unknown): bigint | null { + if (!isRecord(history) || !isRecord(history.current)) return null; + const timestamp = history.current.updated_at ?? history.current.started_at; + return typeof timestamp === "string" ? parseStrictRfc3339Timestamp(timestamp) : null; +} + +/** Whether an incoming valid history is newer than the existing live evidence. */ +export function shouldReplaceMcpAttachmentHistory( + existing: unknown, + incoming: MCPAttachmentHistory, +): boolean { + if (!existing) return true; + const incomingFreshness = mcpHistoryFreshness(incoming); + if (incomingFreshness === null) return false; + const existingFreshness = mcpHistoryFreshness(existing); + return existingFreshness === null || incomingFreshness > existingFreshness; +} diff --git a/apps/web/lib/state/slices/session/session-slice.ts b/apps/web/lib/state/slices/session/session-slice.ts index d48ec821da..a9803a87a5 100644 --- a/apps/web/lib/state/slices/session/session-slice.ts +++ b/apps/web/lib/state/slices/session/session-slice.ts @@ -19,6 +19,10 @@ import { purgeSessionRuntimeState } from "@/lib/state/slices/session-runtime/ses import { mergeTaskSession } from "./session-merge"; import { syncEnvironmentMapping, syncPrepareProgress } from "./session-environment-sync"; import type { SessionRuntimeSliceState } from "@/lib/state/slices/session-runtime/types"; +import { + readMcpAttachmentHistory, + shouldReplaceMcpAttachmentHistory, +} from "@/lib/state/slices/session-runtime/mcp-attachment-reconciliation"; import { getPlanLastSeen, setPlanLastSeen } from "@/lib/local-storage"; import { getWalkthroughLastSeen, @@ -122,6 +126,17 @@ function mergeTaskSessionSnapshot( }; } +function reconcileMcpAttachmentHistory( + draft: SessionSliceState & SessionRuntimeSliceState, + session: TaskSession, +): void { + const incoming = readMcpAttachmentHistory(session.metadata?.mcp_attachment_state); + if (!incoming) return; + const existing = draft.sessionMcpStatus.bySessionId[session.id]; + if (!shouldReplaceMcpAttachmentHistory(existing, incoming)) return; + draft.sessionMcpStatus.bySessionId[session.id] = incoming; +} + // Settled states are defined once in turn-actions (SETTLED_SESSION_STATES / // isSettledSessionState); this file must use the shared predicate so every // settled-boundary path — hydration seeding, session updates, and the WS @@ -608,6 +623,10 @@ function buildTaskSessionReconciliationActions(set: ImmerSet) { draft.taskSessions.items[session.id] = session; syncEnvironmentMapping(draft, session.id, session.task_environment_id); syncPrepareProgress(draft, session); + reconcileMcpAttachmentHistory( + draft as unknown as SessionSliceState & SessionRuntimeSliceState, + session, + ); reconcileActiveTurnForIdleSession(draft, session); } }), diff --git a/apps/web/lib/state/slices/session/set-task-sessions-mcp.test.ts b/apps/web/lib/state/slices/session/set-task-sessions-mcp.test.ts new file mode 100644 index 0000000000..6e5c73cbf4 --- /dev/null +++ b/apps/web/lib/state/slices/session/set-task-sessions-mcp.test.ts @@ -0,0 +1,303 @@ +import { describe, expect, it } from "vitest"; +import { create } from "zustand"; +import { immer } from "zustand/middleware/immer"; +import { createSessionSlice } from "./session-slice"; +import { createSessionRuntimeSlice } from "../session-runtime/session-runtime-slice"; +import type { SessionSlice } from "./types"; +import type { MCPAttachmentHistory, SessionRuntimeSlice } from "../session-runtime/types"; +import { sessionId as toSessionId, taskId as toTaskId, type TaskSession } from "@/lib/types/http"; + +type CombinedSlice = SessionSlice & SessionRuntimeSlice; + +function makeStore() { + return create()( + immer((set) => ({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ...(createSessionSlice as any)(set), + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ...(createSessionRuntimeSlice as any)(set), + })), + ); +} + +const TASK_ID = toTaskId("task-1"); +const SESSION_ID = toSessionId("session-1"); +const SIBLING_SESSION_ID = toSessionId("session-2"); +const SESSION_TIMESTAMP = "2026-04-20T00:00:00Z"; +const MIDDLE_TIMESTAMP = "2026-04-20T00:01:00Z"; +const LATEST_TIMESTAMP = "2026-04-20T00:02:00Z"; + +function makeSession(id: string, metadata?: Record): TaskSession { + return { + id: toSessionId(id), + task_id: TASK_ID, + state: "RUNNING", + started_at: SESSION_TIMESTAMP, + updated_at: SESSION_TIMESTAMP, + ...(metadata ? { metadata } : {}), + }; +} + +function makeHistory({ + attemptId = "attempt-persisted", + startedAt = "2026-04-20T00:00:00Z", + updatedAt, +}: { + attemptId?: string; + startedAt?: string; + updatedAt?: string; +} = {}): MCPAttachmentHistory { + const current: MCPAttachmentHistory["current"] = { + attachment_attempt_id: attemptId, + started_at: startedAt, + servers: [{ name: "kandev", status: "active" }], + }; + if (updatedAt !== undefined) current.updated_at = updatedAt; + return { version: 1, current }; +} + +function withCurrentPatch(patch: Record): Record { + const history = makeHistory(); + return { ...history, current: { ...history.current, ...patch } }; +} + +describe("setTaskSessionsForTask MCP history", () => { + it.each([ + ["started_at", makeHistory()], + [ + "updated_at", + makeHistory({ + startedAt: MIDDLE_TIMESTAMP, + updatedAt: LATEST_TIMESTAMP, + }), + ], + ])("restores valid attachment history using %s freshness", (_freshness, history) => { + const store = makeStore(); + + store + .getState() + .setTaskSessionsForTask( + TASK_ID, + [makeSession("session-1", { mcp_attachment_state: history })], + {}, + ); + + expect(store.getState().sessionMcpStatus.bySessionId[SESSION_ID]).toEqual(history); + }); +}); + +describe("setTaskSessionsForTask MCP history freshness", () => { + it.each([ + ["older", makeHistory({ attemptId: "attempt-older", startedAt: MIDDLE_TIMESTAMP })], + [ + "equal", + makeHistory({ + attemptId: "attempt-equal", + startedAt: LATEST_TIMESTAMP, + updatedAt: LATEST_TIMESTAMP, + }), + ], + ])("keeps newer live evidence when the incoming snapshot is %s", (_age, incoming) => { + const store = makeStore(); + const live = makeHistory({ + attemptId: "attempt-live", + startedAt: LATEST_TIMESTAMP, + updatedAt: LATEST_TIMESTAMP, + }); + store.getState().setSessionMCPStatus(SESSION_ID, live); + + store + .getState() + .setTaskSessionsForTask( + TASK_ID, + [makeSession("session-1", { mcp_attachment_state: incoming })], + {}, + ); + + expect(store.getState().sessionMcpStatus.bySessionId[SESSION_ID]).toEqual(live); + }); + + it("replaces older stored history with a strictly newer snapshot", () => { + const store = makeStore(); + const stored = makeHistory({ + attemptId: "attempt-stored", + startedAt: SESSION_TIMESTAMP, + updatedAt: MIDDLE_TIMESTAMP, + }); + const incoming = makeHistory({ + attemptId: "attempt-new", + startedAt: SESSION_TIMESTAMP, + updatedAt: LATEST_TIMESTAMP, + }); + store.getState().setSessionMCPStatus(SESSION_ID, stored); + + store + .getState() + .setTaskSessionsForTask( + TASK_ID, + [makeSession("session-1", { mcp_attachment_state: incoming })], + {}, + ); + + expect(store.getState().sessionMcpStatus.bySessionId[SESSION_ID]).toEqual(incoming); + }); + + it("replaces stored history when its freshness timestamp is invalid", () => { + const store = makeStore(); + const stored = makeHistory({ attemptId: "attempt-invalid", startedAt: "not-a-timestamp" }); + const incoming = makeHistory({ attemptId: "attempt-valid", startedAt: MIDDLE_TIMESTAMP }); + store.getState().setSessionMCPStatus(SESSION_ID, stored); + + store + .getState() + .setTaskSessionsForTask( + TASK_ID, + [makeSession("session-1", { mcp_attachment_state: incoming })], + {}, + ); + + expect(store.getState().sessionMcpStatus.bySessionId[SESSION_ID]).toEqual(incoming); + }); +}); + +describe("setTaskSessionsForTask MCP history isolation", () => { + it("updates only the session that owns restored history", () => { + const store = makeStore(); + const ownerHistory = makeHistory({ + attemptId: "attempt-owner", + startedAt: MIDDLE_TIMESTAMP, + }); + const siblingHistory = makeHistory({ + attemptId: "attempt-sibling", + startedAt: LATEST_TIMESTAMP, + }); + store.getState().setSessionMCPStatus(SIBLING_SESSION_ID, siblingHistory); + + store + .getState() + .setTaskSessionsForTask( + TASK_ID, + [ + makeSession("session-1", { mcp_attachment_state: ownerHistory }), + makeSession("session-2"), + ], + {}, + ); + + expect(store.getState().sessionMcpStatus.bySessionId[SESSION_ID]).toEqual(ownerHistory); + expect(store.getState().sessionMcpStatus.bySessionId[SIBLING_SESSION_ID]).toEqual( + siblingHistory, + ); + }); + + it("accepts additive fields in a valid history", () => { + const store = makeStore(); + const base = makeHistory(); + const history = { + ...base, + report_version: "backend-1", + current: { + ...base.current, + execution_id: "execution-1", + servers: [{ ...base.current.servers![0], server_revision: 3 }], + }, + }; + + store + .getState() + .setTaskSessionsForTask( + TASK_ID, + [makeSession("session-1", { mcp_attachment_state: history })], + {}, + ); + + expect(store.getState().sessionMcpStatus.bySessionId[SESSION_ID]).toEqual(history); + }); +}); + +describe("setTaskSessionsForTask MCP history validation", () => { + it("treats null optional fields as absent", () => { + const store = makeStore(); + const base = makeHistory(); + const history = { + ...base, + current: { + ...base.current, + updated_at: null, + servers: [ + { + ...base.current.servers![0], + source: null, + transport: null, + target: null, + reason_code: null, + summary: null, + connection_id: null, + tools_listed_at: null, + tool_count: null, + tools: null, + tool_catalog_truncated: null, + tool_token_estimator: null, + }, + ], + }, + previous: null, + }; + + store + .getState() + .setTaskSessionsForTask( + TASK_ID, + [makeSession("session-1", { mcp_attachment_state: history })], + {}, + ); + + expect(store.getState().sessionMcpStatus.bySessionId[SESSION_ID]).toEqual(history); + }); + + it.each([ + ["unsupported version", { ...makeHistory(), version: 2 }], + ["missing current attempt", { version: 1 }], + ["empty attempt identity", withCurrentPatch({ attachment_attempt_id: " " })], + ["invalid started_at", withCurrentPatch({ started_at: "2026-02-30T00:00:00Z" })], + ["invalid updated_at", withCurrentPatch({ updated_at: "tomorrow" })], + [ + "invalid server timestamp", + withCurrentPatch({ + servers: [{ name: "kandev", status: "active", tools_listed_at: "later" }], + }), + ], + ["empty server name", withCurrentPatch({ servers: [{ name: " ", status: "active" }] })], + [ + "unknown server status", + withCurrentPatch({ servers: [{ name: "kandev", status: "online" }] }), + ], + ["non-array previous attempts", { ...makeHistory(), previous: {} }], + [ + "invalid previous attempt", + { ...makeHistory(), previous: [{ ...makeHistory().current, attachment_attempt_id: "" }] }, + ], + ])("ignores %s metadata and preserves existing statuses", (_reason, invalidHistory) => { + const store = makeStore(); + const ownerHistory = makeHistory({ attemptId: "attempt-owner" }); + const siblingHistory = makeHistory({ attemptId: "attempt-sibling" }); + store.getState().setSessionMCPStatus(SESSION_ID, ownerHistory); + store.getState().setSessionMCPStatus(SIBLING_SESSION_ID, siblingHistory); + + store + .getState() + .setTaskSessionsForTask( + TASK_ID, + [ + makeSession("session-1", { mcp_attachment_state: invalidHistory }), + makeSession("session-2"), + ], + {}, + ); + + expect(store.getState().sessionMcpStatus.bySessionId[SESSION_ID]).toEqual(ownerHistory); + expect(store.getState().sessionMcpStatus.bySessionId[SIBLING_SESSION_ID]).toEqual( + siblingHistory, + ); + }); +}); diff --git a/docs/plans/restore-session-mcp-status/plan.md b/docs/plans/restore-session-mcp-status/plan.md new file mode 100644 index 0000000000..e60140c09e --- /dev/null +++ b/docs/plans/restore-session-mcp-status/plan.md @@ -0,0 +1,102 @@ +--- +created: 2026-09-09 +status: done +requirements: + - REQ-PLATFORM-MCP-SESSION-OBSERVABILITY-001 +system_design: + - ../../specs/platform/system-design/mcp-session-observability.md +legacy_specs: [] +--- + +# Implementation Plan: Restore Session MCP Status + +## Overview + +Issue #3547 leaves persisted MCP status unavailable after some task switches. +The repair adds validated, freshness-aware restoration to session-list and +forced task-detail route reconciliation. One frontend work order owns the code +and regression tests. + +## Confirmed root cause + +`setTaskSessionsForTask` merges the returned session rows, but it does not read +`metadata.mcp_attachment_state`. Boot hydration and live WebSocket events can +populate `sessionMcpStatus.bySessionId`, but forced route hydration could also +replace newer live MCP evidence with an older response. + +The new `set-task-sessions-mcp.test.ts` file will contain the permanent +regression test. Its restoration case fails on the current code because the +status entry remains undefined. + +## Scope + +### In scope + +- Restore valid version-1 MCP attachment history from task session metadata. +- Reject malformed and unsupported histories. +- Keep newer live evidence when a delayed session-list snapshot arrives. +- Keep newer live evidence when a delayed forced route snapshot arrives. +- Keep every sibling session status unchanged. + +### Out of scope + +- Backend persistence or response changes. +- Changes to WebSocket event production. +- Changes to the MCP status layout or user-facing copy. +- Changes to MCP evidence semantics or schema versioning. + +## Technical approach + +### Runtime validation and freshness + +Add shared validation and freshness helpers in the session-runtime slice. Reuse +`parseStrictRfc3339Timestamp` for all timestamps in the frontend projection. +Accept additive fields and JSON `null` for optional values, but reject invalid +required fields and unknown status values. + +During `setTaskSessionsForTask`, inspect each merged session's +`metadata.mcp_attachment_state`. Compare the current attempt's `updated_at`, +or its `started_at` fallback, with the stored history. Apply only a strictly +newer valid snapshot, unless no valid stored history exists. + +Update only `sessionMcpStatus.bySessionId[session.id]` in the same Immer +transaction. Metadata absence or rejection leaves the existing entry and all +sibling entries unchanged. Use the same comparator during forced task-detail +route hydration so its response cannot regress a newer live entry. + +## Tests + +| Acceptance criterion | Evidence | +| --- | --- | +| `AC-PLATFORM-MCP-SESSION-OBSERVABILITY-001.10` | `set-task-sessions-mcp.test.ts` proves restoration with and without `updated_at`. | +| `AC-PLATFORM-MCP-SESSION-OBSERVABILITY-001.11` | Session-list and route-hydration tests prove that older and equal snapshots do not replace live evidence. | +| `AC-PLATFORM-MCP-SESSION-OBSERVABILITY-001.12` | Table tests cover malformed shapes, timestamps, names, statuses, and versions. A sibling-isolation test protects unrelated state. | + +## E2E tests + +No new Playwright test is required. This repair only normalizes data in an +existing store and does not change a rendered interaction. Existing desktop +and mobile MCP status tests prove that both surfaces consume this shared state. + +Relevant existing files: + +- `apps/web/e2e/tests/chat/mcp-status.spec.ts` +- `apps/web/e2e/tests/chat/mobile-mcp-status.spec.ts` + +## Work orders + +- [x] [Task 01: Restore MCP status from session lists](task-01-restore-session-list-mcp-status.md) + +## Verification results + +- `cd apps && pnpm --filter @kandev/web exec vitest run lib/state/slices/session/set-task-sessions-mcp.test.ts lib/state/hydration/hydrator.test.ts` passed (46 tests). +- `cd apps/web && pnpm run typecheck` passed. +- Targeted ESLint passed for the session slice, reconciliation helper, hydrator, and both regression tests. +- Related session reconciliation tests passed (3 files, 57 tests). + +## Risks + +- Excessive validation can reject additive safe fields. The guard validates + known fields and permits unknown optional fields. +- Timestamp ties can hide live details. Equal timestamps retain the current + store entry. diff --git a/docs/plans/restore-session-mcp-status/task-01-restore-session-list-mcp-status.md b/docs/plans/restore-session-mcp-status/task-01-restore-session-list-mcp-status.md new file mode 100644 index 0000000000..3fb1d1d417 --- /dev/null +++ b/docs/plans/restore-session-mcp-status/task-01-restore-session-list-mcp-status.md @@ -0,0 +1,100 @@ +--- +id: "01-restore-session-list-mcp-status" +title: "Restore MCP status from session lists" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +requirements: + - REQ-PLATFORM-MCP-SESSION-OBSERVABILITY-001 +acceptance_criteria: + - AC-PLATFORM-MCP-SESSION-OBSERVABILITY-001.10 + - AC-PLATFORM-MCP-SESSION-OBSERVABILITY-001.11 + - AC-PLATFORM-MCP-SESSION-OBSERVABILITY-001.12 +system_design: + - ../../specs/platform/system-design/mcp-session-observability.md +--- + +# Task 01: Restore MCP Status From Session Lists + +## Summary + +Restore persisted MCP attachment history during task session-list and forced +task-detail route reconciliation. Reject invalid metadata and prevent delayed +snapshots from replacing newer live evidence. + +## In scope + +- Validate the version-1 frontend projection in + `metadata.mcp_attachment_state`. +- Compare incoming and stored current-attempt timestamps with the strict + RFC3339 parser. +- Reuse the validation and freshness comparator during forced task-detail + route hydration. +- Update only the MCP status entry for the owning session. +- Add focused regression tests for restoration, rejection, freshness, route + ordering, JSON null optionals, and sibling isolation. + +## Out of scope + +- Backend, persistence, or wire-contract changes. +- WebSocket handler changes. +- Component, layout, localization, or mobile interaction changes. + +## Acceptance + +- A valid persisted history appears in the owning session's MCP status entry. +- Equal or older snapshots never replace newer live evidence. +- Invalid metadata does not change the owning entry or a sibling entry. + +## Verification + +```bash +cd apps && pnpm --filter @kandev/web exec vitest run lib/state/slices/session/set-task-sessions-mcp.test.ts lib/state/hydration/hydrator.test.ts +cd apps/web && pnpm run typecheck +cd apps && pnpm --filter @kandev/web exec eslint lib/state/slices/session/session-slice.ts lib/state/slices/session-runtime/mcp-attachment-reconciliation.ts lib/state/hydration/hydrator.ts lib/state/slices/session/set-task-sessions-mcp.test.ts lib/state/hydration/hydrator.test.ts +``` + +## Files likely touched + +- `apps/web/lib/state/slices/session/session-slice.ts` +- `apps/web/lib/state/slices/session-runtime/mcp-attachment-reconciliation.ts` +- `apps/web/lib/state/hydration/hydrator.ts` +- `apps/web/lib/state/slices/session/set-task-sessions-mcp.test.ts` +- `apps/web/lib/state/hydration/hydrator.test.ts` + +## Dependencies + +None. + +## Risks + +- The metadata type is `unknown` at runtime. A partial guard can admit invalid + nested values. +- Backend timestamps can use RFC3339 nanosecond precision. Millisecond parsing + can misorder live and persisted evidence. + +## Parallelism + +`sequential` + +## Inputs + +- `REQ-PLATFORM-MCP-SESSION-OBSERVABILITY-001` and its new acceptance criteria. +- The session MCP observability system design and ADR. +- Existing strict timestamp parsing and session-list reconciliation patterns. + +## Results + +Implemented shared validated hydration of `metadata.mcp_attachment_state` during +session-list reconciliation and freshness-guarded forced route hydration. +Valid version-1 histories restore the owning session, additive fields and JSON +null optionals remain supported, malformed histories are ignored, and older or +equal snapshots cannot replace newer live evidence. + +Verification passed: + +- `cd apps && pnpm --filter @kandev/web exec vitest run lib/state/slices/session/set-task-sessions-mcp.test.ts lib/state/hydration/hydrator.test.ts` (46 tests) +- `cd apps/web && pnpm run typecheck` +- Targeted ESLint for the session slice, reconciliation helper, hydrator, and both regression tests. +- Related session reconciliation tests (3 files, 57 tests) diff --git a/docs/specs/platform/requirements/mcp-session-observability.md b/docs/specs/platform/requirements/mcp-session-observability.md index 67b0124b97..2f52429d2a 100644 --- a/docs/specs/platform/requirements/mcp-session-observability.md +++ b/docs/specs/platform/requirements/mcp-session-observability.md @@ -2,7 +2,7 @@ status: active system: platform created: 2026-07-30 -updated: 2026-08-30 +updated: 2026-09-09 owners: - Kandev --- @@ -29,6 +29,9 @@ The chat toolbar currently derives its MCP list from agent-profile configuration - **AC-PLATFORM-MCP-SESSION-OBSERVABILITY-001.7:** **GIVEN** an Active Kandev server, **WHEN** a precise-pointer user hovers or focuses the MCP trigger, **THEN** the tooltip shows a green Kandev row and its Active status. - **AC-PLATFORM-MCP-SESSION-OBSERVABILITY-001.8:** **GIVEN** a precise-pointer user, **WHEN** they click the MCP trigger, **THEN** a wide dialog lists the active session's MCP servers. - **AC-PLATFORM-MCP-SESSION-OBSERVABILITY-001.9:** **GIVEN** a modern stateless MCP request, **WHEN** Kandev records protocol evidence, **THEN** the evidence belongs to the current attachment attempt without a fabricated connection ID or a connection-closed event at the end of the HTTP request. +- **AC-PLATFORM-MCP-SESSION-OBSERVABILITY-001.10:** **GIVEN** a session list contains valid version-1 attachment history, **WHEN** the client loads the task, **THEN** the status surface restores it without a live event. +- **AC-PLATFORM-MCP-SESSION-OBSERVABILITY-001.11:** **GIVEN** the client holds newer live evidence, **WHEN** an equal or older session snapshot arrives, **THEN** the status surface retains the live evidence. +- **AC-PLATFORM-MCP-SESSION-OBSERVABILITY-001.12:** **GIVEN** session-list attachment history is malformed or unsupported, **WHEN** the client loads the task, **THEN** the client ignores it and retains other session statuses. ## System design diff --git a/docs/specs/platform/system-design/mcp-session-observability.md b/docs/specs/platform/system-design/mcp-session-observability.md index 4ee89db7d2..4dd018f677 100644 --- a/docs/specs/platform/system-design/mcp-session-observability.md +++ b/docs/specs/platform/system-design/mcp-session-observability.md @@ -4,7 +4,7 @@ system: platform requirements: - REQ-PLATFORM-MCP-SESSION-OBSERVABILITY-001 created: 2026-07-30 -updated: 2026-08-30 +updated: 2026-09-09 owners: - Kandev --- @@ -124,6 +124,46 @@ stable reason code plus a bounded sanitized summary. Raw ACP JSONL logging remains a development-only diagnostic and is not enabled by this feature. +## Frontend session-list reconciliation + +Task-detail boot state restores attachment history during initial navigation. +Later task switches use `ListTaskSessionSummariesResponse`, which includes +public session metadata. Both paths supply the same persisted history, and +forced task-detail route hydration uses the same freshness-aware merge. + +`setTaskSessionsForTask` reads `metadata.mcp_attachment_state` in its existing +Immer transaction. A runtime guard accepts only the known frontend projection: + +- The value is an object with schema version `1`. +- The current attempt has a nonempty `attachment_attempt_id`. +- Each attempt has a valid RFC3339 `started_at` value. +- Optional `updated_at` and `tools_listed_at` values are valid RFC3339 values + when present. A JSON `null` value is treated as absent. +- Each server has a nonempty name and a known attachment status. +- Optional previous attempts satisfy the same attempt and server rules. + +The guard permits unknown optional fields from the backend report. This rule +keeps the frontend compatible with additive safe metadata. + +The current attempt supplies the freshness value. The client uses `updated_at` +when that field exists. Otherwise, the client uses `started_at`. + +A valid incoming history initializes an empty store entry. It also replaces +stored history with an invalid freshness value. For two valid values, only a +strictly newer incoming value replaces the stored history. Equal values retain +the stored history because it can contain live evidence from a WebSocket event. +The same comparator applies when a forced task-detail route hydration merges a +snapshot into the active session, so an equal or older route response cannot +regress live evidence. + +The reconciliation paths change only `sessionMcpStatus.bySessionId` for the +session that owns the accepted history. Invalid or absent metadata does not +clear live evidence. It does not change a sibling session. + +Desktop and mobile surfaces already read the shared session-runtime slice. +This repair does not change layout, navigation, touch behavior, or viewport +behavior. + ## Kandev tool catalog After Kandev serves `tools/list`, the current attachment report includes a