diff --git a/src/agent-engine.ts b/src/agent-engine.ts index 0b807182..c3a6ff83 100644 --- a/src/agent-engine.ts +++ b/src/agent-engine.ts @@ -5178,14 +5178,20 @@ export class AgentEngine { if (!agent) { throw new Error(`Agent not found: ${agentId}`); } - const resumeCommand = agent.cli_session_id - ? buildResumeCommand( + let resumeCommand: string | undefined; + if (agent.cli_session_id) { + try { + resumeCommand = buildResumeCommand( agent.cli, agent.repo, agent.cli_session_id, agent.launcher_name, - ) - : undefined; + ); + } catch { + // Terminal I/O depends on the stable surface binding, not optional + // resume metadata. A damaged legacy repo field must not disable send. + } + } return { agent_id: agent.agent_id, surface_id: agent.surface_id, diff --git a/src/agent-registry.ts b/src/agent-registry.ts index f15da29b..0de8a1f4 100644 --- a/src/agent-registry.ts +++ b/src/agent-registry.ts @@ -1399,7 +1399,6 @@ export class AgentRegistry { return null; } const agentId = record.agent_id; - const repo = inferRepoFromTitle(discoveredEntry.surface_title) || record.repo; const model = discoveredEntry.model ?? record.model; const workspaceId = discoveredEntry.workspace_id ?? null; const surfaceUuid = discoveredEntry.surface_uuid ?? null; @@ -1409,7 +1408,6 @@ export class AgentRegistry { const explicitRole = this.explicitRoleFor(discoveredEntry); const patch: Partial = {}; - if (repo !== record.repo) patch.repo = repo; if (model !== record.model) patch.model = model; if ((record.workspace_id ?? null) !== workspaceId) { patch.workspace_id = workspaceId; diff --git a/src/format.ts b/src/format.ts index 3bea81f1..4028e2b6 100644 --- a/src/format.ts +++ b/src/format.ts @@ -159,8 +159,12 @@ export function formatReadScreen( return result.join("\n"); } -export function formatListAgents(agents: PublicAgent[], count: number): string { - if (count === 0) { +export function formatListAgents( + agents: PublicAgent[], + count: number, + skippedAgents: Array<{ agent_id: string; error: string }> = [], +): string { + if (count === 0 && skippedAgents.length === 0) { return "\u250c\u2500 cmux agents\n\u2502 No agents running.\n\u2514\u2500"; } @@ -168,12 +172,16 @@ export function formatListAgents(agents: PublicAgent[], count: number): string { lines.push( `\u250c\u2500 cmux agents \u2500 ${count} agent${count !== 1 ? "s" : ""}`, ); - lines.push( - `\u2502 ${pad("ID", 20)} ${pad("Repo", 16)} ${pad("State", 8)} ${pad("Model", 18)} ${pad("Session", 14)}`, - ); - lines.push( - `\u251c${"─".repeat(20)}${"─".repeat(17)}${"─".repeat(9)}${"─".repeat(19)}${"─".repeat(14)}`, - ); + if (count > 0) { + lines.push( + `\u2502 ${pad("ID", 20)} ${pad("Repo", 16)} ${pad("State", 8)} ${pad("Model", 18)} ${pad("Session", 14)}`, + ); + lines.push( + `\u251c${"─".repeat(20)}${"─".repeat(17)}${"─".repeat(9)}${"─".repeat(19)}${"─".repeat(14)}`, + ); + } else { + lines.push("\u2502 No healthy agent rows."); + } for (const a of agents) { const id = pad(truncate(a.agent_id, 18), 20); @@ -184,6 +192,13 @@ export function formatListAgents(agents: PublicAgent[], count: number): string { lines.push(`\u2502 ${id} ${repo} ${state} ${model} ${session}`); } + if (skippedAgents.length > 0) { + const skippedIds = skippedAgents.map((agent) => agent.agent_id).join(", "); + lines.push( + `\u2502 \u26a0 skipped ${skippedAgents.length} invalid agent row${skippedAgents.length === 1 ? "" : "s"}: ${skippedIds}`, + ); + } + lines.push("\u2514\u2500"); return lines.join("\n"); } diff --git a/src/server.ts b/src/server.ts index 7529d0f1..77d73110 100644 --- a/src/server.ts +++ b/src/server.ts @@ -526,6 +526,12 @@ export interface DeliveryRecord { completed_at?: string; error?: string; failed_chunk?: number; + /** Internal UUID guard; omitted from public delivery snapshots. */ + stableSurfaceIdentity?: string | null; + /** Ref-only provenance captured before an asynchronous write starts. */ + surfaceObserverIdentity?: string | null; + beforeMutation?: () => Promise; + lockKey?: string; } class DeliveryError extends Error { @@ -916,6 +922,9 @@ function toMinimalSurface( typeof surface.workspace_ref === "string" ? surface.workspace_ref : "", }; + if (typeof surface.id === "string") { + minimal.id = surface.id; + } if (typeof surface.pane_ref === "string") { minimal.pane_ref = surface.pane_ref; } @@ -2274,6 +2283,11 @@ export interface CmuxServerContext { surfaceWriteLivenessCandidates: Set; surfacePtyDeadSince: Map; readScreenInflight: Map>; + /** First-seen stable identities for caller-visible mutable surface refs. */ + capturedSurfaceUuidByRef: Map; + /** Refs observed with more than one UUID in one observer epoch are unsafe. */ + ambiguousCapturedSurfaceRefs: Set; + capturedSurfaceObserverEpoch: string | null; codexRolloutFillProvider: CodexRolloutFillProvider; surfaceWriteLiveness: SurfaceWriteLivenessTracker; enableClaudeChannels: boolean; @@ -2410,6 +2424,9 @@ export function createServerContext( surfaceWriteLivenessCandidates: new Set(), surfacePtyDeadSince: new Map(), readScreenInflight: new Map(), + capturedSurfaceUuidByRef: new Map(), + ambiguousCapturedSurfaceRefs: new Set(), + capturedSurfaceObserverEpoch: null, codexRolloutFillProvider: opts?.codexRolloutFillProvider ?? makeCodexRolloutFillProvider(), surfaceWriteLiveness: @@ -2456,6 +2473,9 @@ export function createServerContext( context.lifecycleAgentInputDeliverer = null; context.lifecycleAgentInputDelivererReadyListeners.clear(); context.originalLaunchCommandsBySurface.clear(); + context.capturedSurfaceUuidByRef.clear(); + context.ambiguousCapturedSurfaceRefs.clear(); + context.capturedSurfaceObserverEpoch = null; context.lifecycleStarted = false; context.lifecycleStartPromise = null; context.lifecycleStartError = null; @@ -2645,6 +2665,7 @@ export function createServer(opts?: CreateServerOptions): McpServer { let lifecycleSeatManifestPublisher: (input: { agentId?: string; surfaceId?: string; + surfaceUuid?: string; tabName?: string; model?: string; }) => Promise = async () => {}; @@ -3316,9 +3337,18 @@ export function createServer(opts?: CreateServerOptions): McpServer { failedChunk?: number, ) => { if (status === "delivered") { - recordSurfaceWriteSuccess(record.surface); + recordSurfaceWriteSuccess( + record.surface, + record.stableSurfaceIdentity, + record.surfaceObserverIdentity, + ); } else if (status === "failed") { - recordSurfaceWriteFailure(record.surface, error); + recordSurfaceWriteFailure( + record.surface, + error, + record.stableSurfaceIdentity, + record.surfaceObserverIdentity, + ); } record.status = status; record.completed_at = new Date().toISOString(); @@ -3329,7 +3359,7 @@ export function createServer(opts?: CreateServerOptions): McpServer { if (activeDeliveryBySurface.get(record.surface) === record.delivery_id) { activeDeliveryBySurface.delete(record.surface); } - releaseSurfaceWrite(record.surface, record.delivery_id); + releaseSurfaceWrite(record.lockKey ?? record.surface, record.delivery_id); pruneCompletedDeliveryHistory(record.surface); }; @@ -3628,6 +3658,8 @@ export function createServer(opts?: CreateServerOptions): McpServer { surface: string; workspace?: string; rename_to_task?: string; + stableSurfaceIdentity?: string | null; + beforeMutation?: () => Promise; }) => { if (!opts.rename_to_task) { return; @@ -3639,11 +3671,15 @@ export function createServer(opts?: CreateServerOptions): McpServer { const surface = surfaces.surfaces.find((s) => s.ref === opts.surface); const currentTitle = surface?.title ?? ""; const newTitle = replaceTaskSuffix(currentTitle, opts.rename_to_task); + await opts.beforeMutation?.(); await client.renameTab(opts.surface, newTitle, { workspace: opts.workspace, }); await lifecycleSeatManifestPublisher({ surfaceId: opts.surface, + ...(opts.stableSurfaceIdentity + ? { surfaceUuid: opts.stableSurfaceIdentity } + : {}), tabName: newTitle, }); }; @@ -3865,6 +3901,7 @@ export function createServer(opts?: CreateServerOptions): McpServer { verify_submit?: boolean; allow_recovery_enter_retry?: boolean; submit_verify_timeout_ms?: number; + stableSurfaceIdentity?: string | null; beforeMutation?: () => Promise; }): Promise<{ bytes: number; @@ -3948,6 +3985,8 @@ export function createServer(opts?: CreateServerOptions): McpServer { surface: opts.surface, workspace: opts.workspace, rename_to_task: opts.rename_to_task, + stableSurfaceIdentity: opts.stableSurfaceIdentity, + beforeMutation: opts.beforeMutation, }); if (opts.source_event) { @@ -4584,6 +4623,7 @@ export function createServer(opts?: CreateServerOptions): McpServer { const deliverBootPrompt = async (opts: { surface: string; + stableSurfaceIdentity?: string | null; workspace?: string; cli?: CliType; prompt?: string; @@ -4704,6 +4744,7 @@ export function createServer(opts?: CreateServerOptions): McpServer { toolName: "boot_prompt", workspace: deliveryRoute.workspace, observePtyWrite: true, + stableSurfaceIdentity: opts.stableSurfaceIdentity, }, ); return { @@ -5158,7 +5199,14 @@ export function createServer(opts?: CreateServerOptions): McpServer { }; const startBackgroundDelivery = (record: DeliveryRecord) => { - acquireSurfaceWrite(record.surface, record.delivery_id); + // Preserve the backend owner that accepted the asynchronous write. Reading + // the observer after completion could attribute old-backend evidence to a + // new backend that reused the same mutable ref. + record.surfaceObserverIdentity = context.surfaceObserverId; + record.lockKey = record.stableSurfaceIdentity + ? `uuid:${record.stableSurfaceIdentity.toLowerCase()}` + : record.surface; + acquireSurfaceWrite(record.lockKey, record.delivery_id); deliveries.set(record.delivery_id, record); latestDeliveryBySurface.set(record.surface, record.delivery_id); activeDeliveryBySurface.set(record.surface, record.delivery_id); @@ -5174,8 +5222,10 @@ export function createServer(opts?: CreateServerOptions): McpServer { chunk_delay_ms: record.chunk_delay_ms, press_enter: record.press_enter, rename_to_task: record.rename_to_task, + stableSurfaceIdentity: record.stableSurfaceIdentity, source_event: "send_input", verify_submit: record.verify_submit, + beforeMutation: record.beforeMutation, onChunkDelivered: (sentChunks) => { record.sent_chunks = sentChunks; }, @@ -5260,6 +5310,223 @@ export function createServer(opts?: CreateServerOptions): McpServer { surfaceObserverEpochProvider(), ); + const resetCapturedSurfaceIdentitiesForObserver = (): string | null => { + const observerEpoch = context.surfaceObserverEpoch; + if (context.capturedSurfaceObserverEpoch !== observerEpoch) { + context.capturedSurfaceUuidByRef.clear(); + context.ambiguousCapturedSurfaceRefs.clear(); + context.capturedSurfaceObserverEpoch = observerEpoch; + } + return observerEpoch; + }; + + const captureSurfaceIdentities = ( + surfaceIdByRef: ReadonlyMap, + observedEpoch: string | null, + ): void => { + const currentEpoch = context.surfaceObserverEpoch; + if (!observedEpoch || currentEpoch !== observedEpoch) return; + if (context.capturedSurfaceObserverEpoch !== observedEpoch) { + context.capturedSurfaceUuidByRef.clear(); + context.ambiguousCapturedSurfaceRefs.clear(); + context.capturedSurfaceObserverEpoch = observedEpoch; + } + for (const [surfaceRef, surfaceUuid] of surfaceIdByRef) { + const capturedUuid = context.capturedSurfaceUuidByRef.get(surfaceRef); + if (!capturedUuid) { + // A ref is a caller-visible handle for the first UUID observed there. + // Never overwrite it with a later occupant after refs renumber/recycle. + context.capturedSurfaceUuidByRef.set(surfaceRef, surfaceUuid); + } else if (capturedUuid.toLowerCase() !== surfaceUuid.toLowerCase()) { + context.ambiguousCapturedSurfaceRefs.add(surfaceRef); + } + } + }; + + const findSurfaceRefByUuid = ( + topology: SurfaceTopologySnapshot, + surfaceUuid: string, + ): string | null => { + const uuidKey = surfaceUuid.trim().toLowerCase(); + return ( + [...topology.surfaceRefById].find( + ([observedUuid]) => observedUuid.trim().toLowerCase() === uuidKey, + )?.[1] ?? null + ); + }; + + type RawSurfaceMutationRoute = { + surface: string; + workspace?: string; + stableSurfaceIdentity: string | null; + assertCurrent: () => Promise; + }; + + /** + * Bind a caller-visible mutable ref to a stable UUID before terminal I/O. + * Old/ref-only cmux clients retain compatibility, but once UUID evidence has + * been captured the route always fails closed if that UUID is absent. + */ + const resolveRawSurfaceMutationRoute = async ( + requestedSurface: string, + requestedWorkspace: string | undefined, + operation: string, + ): Promise => { + const explicitWorkspace = requestedWorkspace + ? normalizeWorkspaceRefAlias(requestedWorkspace) + : undefined; + const assertExplicitWorkspace = ( + observedWorkspace: string | undefined, + ): void => { + if ( + explicitWorkspace && + normalizeWorkspaceRefAlias(observedWorkspace ?? "") !== + explicitWorkspace + ) { + throw new Error( + `Stable surface binding for ${requestedSurface} belongs to ` + + `${observedWorkspace ?? "an unknown workspace"}, not the caller's ` + + `explicit workspace ${explicitWorkspace}; refusing ${operation}.`, + ); + } + }; + resetCapturedSurfaceIdentitiesForObserver(); + const capturedUuid = context.capturedSurfaceUuidByRef.get(requestedSurface); + const registryUuids = new Set( + stateMgr + .listStates() + .filter((record) => record.surface_id === requestedSurface) + .map((record) => record.surface_uuid?.trim()) + .filter((uuid): uuid is string => Boolean(uuid)), + ); + const registryUuid = registryUuids.size === 1 ? [...registryUuids][0] : null; + const expectedUuid = capturedUuid ?? registryUuid; + const topologyObserverEpoch = context.surfaceObserverEpoch; + const topology = await collectSurfaceTopology(); + + if (topology?.complete === true) { + const uuidTargetRef = findSurfaceRefByUuid(topology, requestedSurface); + captureSurfaceIdentities( + topology.surfaceIdByRef, + topologyObserverEpoch, + ); + const currentUuidAtRequestedRef = + topology.surfaceIdByRef.get(requestedSurface) ?? null; + if ( + (expectedUuid && + currentUuidAtRequestedRef && + expectedUuid.toLowerCase() !== + currentUuidAtRequestedRef.toLowerCase()) || + context.ambiguousCapturedSurfaceRefs.has(requestedSurface) + ) { + throw new Error( + `Mutable surface ref ${requestedSurface} was observed for multiple stable UUIDs; ` + + `refusing ${operation}. Re-address by agent_id or stable surface UUID.`, + ); + } + const stableUuid = + expectedUuid ?? + (uuidTargetRef ? requestedSurface : null) ?? + topology.surfaceIdByRef.get(requestedSurface) ?? + null; + + if (stableUuid) { + const currentRef = findSurfaceRefByUuid(topology, stableUuid); + if (!currentRef) { + throw new Error( + `Stable surface UUID ${stableUuid} captured for ${requestedSurface} ` + + `is no longer live; refusing ${operation} rather than using a recycled ref.`, + ); + } + const observedWorkspace = topology.workspaceBySurface.get(currentRef); + assertExplicitWorkspace(observedWorkspace); + const workspace = observedWorkspace ?? explicitWorkspace; + const assertCurrent = async (): Promise => { + const current = await collectSurfaceTopology(); + const currentRefForUuid = + current?.complete === true + ? findSurfaceRefByUuid(current, stableUuid) + : null; + const currentWorkspace = currentRefForUuid + ? current?.workspaceBySurface.get(currentRefForUuid) + : null; + if ( + !current || + current.complete !== true || + currentRefForUuid !== currentRef || + (currentWorkspace ?? null) !== (workspace ?? null) + ) { + throw new Error( + `Stable surface UUID ${stableUuid} changed or disappeared during ` + + `${operation}; refusing terminal mutation.`, + ); + } + }; + return { + surface: currentRef, + workspace, + stableSurfaceIdentity: stableUuid, + assertCurrent, + }; + } + + if ( + topology.surfaceIdByRef.size > 0 || + topology.surfaceRefById.size > 0 + ) { + throw new Error( + `Fresh topology did not provide a stable surface UUID for ` + + `${requestedSurface}; refusing ${operation}.`, + ); + } + + if (!topology.workspaceBySurface.has(requestedSurface)) { + throw new Error( + `Fresh topology does not contain ${requestedSurface}; refusing ${operation} ` + + `rather than trusting an absent mutable ref.`, + ); + } + + const workspace = + topology.workspaceBySurface.get(requestedSurface) ?? explicitWorkspace; + assertExplicitWorkspace(workspace); + return { + surface: requestedSurface, + workspace, + stableSurfaceIdentity: null, + assertCurrent: async () => { + const current = await collectSurfaceTopology(); + if ( + current?.complete !== true || + current.surfaceIdByRef.size !== 0 || + !current.workspaceBySurface.has(requestedSurface) + ) { + throw new Error( + `Ref-only surface ${requestedSurface} is no longer uniquely live; ` + + `refusing ${operation}.`, + ); + } + }, + }; + } + + if (expectedUuid) { + throw new Error( + `Stable surface UUID ${expectedUuid} captured for ${requestedSurface} ` + + `could not be resolved in fresh topology; refusing ${operation}.`, + ); + } + + // Compatibility for pre-UUID/mock connectors that cannot produce a + // complete topology. No stable claim has been made, so preserve ref I/O. + return { + surface: requestedSurface, + workspace: explicitWorkspace, + stableSurfaceIdentity: null, + assertCurrent: async () => {}, + }; + }; + const readScreenSnapshotKey = (opts: { surface: string; workspace?: string; @@ -5642,6 +5909,7 @@ export function createServer(opts?: CreateServerOptions): McpServer { ANNOTATIONS.readOnly, async (args) => { try { + const listingObserverEpoch = context.surfaceObserverEpoch; const workspaces = await client.listWorkspaces(); const targetWorkspaceRefs = args.workspace ? [args.workspace] @@ -5703,6 +5971,19 @@ export function createServer(opts?: CreateServerOptions): McpServer { }), ); const surfaceGroups = surfaceGroupsByWorkspace.flat(); + const surfacesWithStableIds = enrichSurfaceIdsFromPanes( + panesByWorkspace.map(({ workspaceRef, panes }) => ({ + ref: workspaceRef, + panes, + })), + surfaceGroups, + ); + const stableIdByRef = new Map( + surfacesWithStableIds.flatMap((surface) => + surface.id ? [[surface.ref, surface.id] as const] : [], + ), + ); + captureSurfaceIdentities(stableIdByRef, listingObserverEpoch); const uniqueSurfaceEntries: Array<{ group: { workspace_ref: string; @@ -5735,6 +6016,9 @@ export function createServer(opts?: CreateServerOptions): McpServer { uniqueSurfaceEntries.map(async ({ group, surface }) => { const enrichedSurface: Record = { ...surface, + ...(surface.id || !stableIdByRef.has(surface.ref) + ? {} + : { id: stableIdByRef.get(surface.ref) }), workspace_ref: group.workspace_ref, window_ref: group.window_ref, pane_ref: group.pane_ref, @@ -6632,16 +6916,31 @@ export function createServer(opts?: CreateServerOptions): McpServer { ANNOTATIONS.mutating, async (args) => { try { - await assertSurfaceMutationAllowed("move_surface", args.surface); - const result = await client.moveSurface({ - surface: args.surface, - pane: args.pane, - workspace: args.workspace, - before: args.before, - after: args.after, - index: args.index, - focus: args.focus, - }); + const route = await resolveRawSurfaceMutationRoute( + args.surface, + undefined, + "move_surface", + ); + const result = await withSurfaceWrite( + route.surface, + async () => { + await route.assertCurrent(); + return client.moveSurface({ + surface: route.surface, + pane: args.pane, + workspace: args.workspace, + before: args.before, + after: args.after, + index: args.index, + focus: args.focus, + }); + }, + { + toolName: "move_surface", + workspace: route.workspace, + stableSurfaceIdentity: route.stableSurfaceIdentity, + }, + ); // F8: slim, phone-readable confirmation — drop the verbose passthrough. const data = { surface: result.surface, @@ -6728,9 +7027,14 @@ export function createServer(opts?: CreateServerOptions): McpServer { chunkTerminalInput(sanitizedText, effectiveChunkSize), ) : [sanitizedText]; + const route = await resolveRawSurfaceMutationRoute( + args.surface, + args.workspace, + "send_input", + ); const targetRecord = resolveLatestSurfaceAgentRecord( stateMgr, - args.surface, + route.surface, ); assertInteractiveMultilineInputAllowed({ tool: "send_input", @@ -6746,13 +7050,14 @@ export function createServer(opts?: CreateServerOptions): McpServer { if (args.background) { await assertSurfaceMutationAllowed( "send_input", - args.surface, - args.workspace, + route.surface, + route.workspace, ); + await route.assertCurrent(); const record: DeliveryRecord = { delivery_id: randomUUID(), - surface: args.surface, - workspace: args.workspace, + surface: route.surface, + workspace: route.workspace, status: "delivering", total_chunks: chunks.length, sent_chunks: 0, @@ -6765,10 +7070,12 @@ export function createServer(opts?: CreateServerOptions): McpServer { retry_count: 0, rename_to_task: args.rename_to_task, started_at: new Date().toISOString(), + stableSurfaceIdentity: route.stableSurfaceIdentity, + beforeMutation: route.assertCurrent, }; startBackgroundDelivery(record); - const identity = resolveTargetIdentity(stateMgr, args.surface); + const identity = resolveTargetIdentity(stateMgr, route.surface); const data = { ...identity, delivered: false, @@ -6788,28 +7095,32 @@ export function createServer(opts?: CreateServerOptions): McpServer { } const delivery = await withSurfaceWrite( - args.surface, + route.surface, async () => { + await route.assertCurrent(); return deliverInputChunks({ - surface: args.surface, - workspace: args.workspace, + surface: route.surface, + workspace: route.workspace, chunks, chunk_size: effectiveChunkSize, chunk_delay_ms: SEND_INPUT_CHUNK_DELAY_MS, press_enter: args.press_enter, rename_to_task: args.rename_to_task, + stableSurfaceIdentity: route.stableSurfaceIdentity, source_event: "send_input", verify_submit: shouldVerifySubmit, + beforeMutation: route.assertCurrent, }); }, { toolName: "send_input", - workspace: args.workspace, + workspace: route.workspace, observePtyWrite: true, + stableSurfaceIdentity: route.stableSurfaceIdentity, }, ); - const identity = resolveTargetIdentity(stateMgr, args.surface); + const identity = resolveTargetIdentity(stateMgr, route.surface); const data = { ...identity, delivered: true, @@ -6916,31 +7227,40 @@ export function createServer(opts?: CreateServerOptions): McpServer { sanitizedCommand.length > SEND_INPUT_CHUNK_THRESHOLD ? chunkTerminalInput(sanitizedCommand, SEND_INPUT_CHUNK_THRESHOLD) : [sanitizedCommand]; + const route = await resolveRawSurfaceMutationRoute( + args.surface, + args.workspace, + "send_command", + ); const targetRecord = resolveLatestSurfaceAgentRecord( stateMgr, - args.surface, + route.surface, ); const shouldVerifySubmit = !!targetRecord && INTERACTIVE_AGENT_STATES.has(targetRecord.state); const delivery = await withSurfaceWrite( - args.surface, + route.surface, async () => { + await route.assertCurrent(); return deliverInputChunks({ - surface: args.surface, - workspace: args.workspace, + surface: route.surface, + workspace: route.workspace, chunks, chunk_size: SEND_INPUT_CHUNK_THRESHOLD, chunk_delay_ms: SEND_INPUT_CHUNK_DELAY_MS, press_enter: true, + stableSurfaceIdentity: route.stableSurfaceIdentity, source_event: "send_command", verify_submit: bootPromptPath ? false : shouldVerifySubmit, + beforeMutation: route.assertCurrent, }); }, { toolName: "send_command", - workspace: args.workspace, + workspace: route.workspace, observePtyWrite: true, + stableSurfaceIdentity: route.stableSurfaceIdentity, }, ); @@ -6948,22 +7268,29 @@ export function createServer(opts?: CreateServerOptions): McpServer { Awaited> | undefined; if (bootPromptPath && launcherCli) { bootPromptDelivery = await deliverBootPrompt({ - surface: args.surface, - workspace: args.workspace, + surface: route.surface, + stableSurfaceIdentity: route.stableSurfaceIdentity, + workspace: route.workspace, cli: launcherCli, boot_prompt_path: bootPromptPath, timeout_ms: args.boot_prompt_timeout_ms, + resolveRoute: async () => { + await route.assertCurrent(); + return { surface: route.surface, workspace: route.workspace }; + }, onUpdateShellRelaunch: () => sendLauncherCommandToSurface({ - surface: args.surface, - workspace: args.workspace, + surface: route.surface, + stableSurfaceIdentity: route.stableSurfaceIdentity, + workspace: route.workspace, command: sanitizedCommand, relaunch: true, + assertSurfaceBindingCurrent: route.assertCurrent, }), }); } - const identity = resolveTargetIdentity(stateMgr, args.surface); + const identity = resolveTargetIdentity(stateMgr, route.surface); const data = { ...identity, command: sanitizedCommand, @@ -7037,18 +7364,30 @@ export function createServer(opts?: CreateServerOptions): McpServer { async (args) => { try { const key = normalizeKeyName(args.key); - await withSurfaceWrite( + const route = await resolveRawSurfaceMutationRoute( args.surface, + args.workspace, + "send_key", + ); + await withSurfaceWrite( + route.surface, async () => { - await sendKeyWithRetry(args.surface, key, args.workspace); + await route.assertCurrent(); + await sendKeyWithRetry( + route.surface, + key, + route.workspace, + route.assertCurrent, + ); }, { toolName: "send_key", - workspace: args.workspace, + workspace: route.workspace, observePtyWrite: true, + stableSurfaceIdentity: route.stableSurfaceIdentity, }, ); - const data = { surface: args.surface, key }; + const data = { surface: route.surface, key }; return okFormatted(formatOk("send_key", data), data); } catch (e) { return err(e); @@ -7242,29 +7581,42 @@ export function createServer(opts?: CreateServerOptions): McpServer { ANNOTATIONS.mutating, async (args) => { try { + const route = await resolveRawSurfaceMutationRoute( + args.surface, + args.workspace, + "rename_tab", + ); let finalTitle = args.title; if (args.preserve_prefix) { const surfaces = await client.listPaneSurfaces({ - workspace: args.workspace, + workspace: route.workspace, }); - const surface = surfaces.surfaces.find((s) => s.ref === args.surface); + const surface = surfaces.surfaces.find((s) => s.ref === route.surface); const currentTitle = surface?.title ?? ""; finalTitle = replaceTaskSuffix(currentTitle, args.title); } await withSurfaceWrite( - args.surface, + route.surface, async () => { - await client.renameTab(args.surface, finalTitle, { - workspace: args.workspace, + await route.assertCurrent(); + await client.renameTab(route.surface, finalTitle, { + workspace: route.workspace, }); }, - { toolName: "rename_tab", workspace: args.workspace }, + { + toolName: "rename_tab", + workspace: route.workspace, + stableSurfaceIdentity: route.stableSurfaceIdentity, + }, ); await lifecycleSeatManifestPublisher({ - surfaceId: args.surface, + surfaceId: route.surface, + ...(route.stableSurfaceIdentity + ? { surfaceUuid: route.stableSurfaceIdentity } + : {}), tabName: finalTitle, }); - const data = { surface: args.surface, title: finalTitle }; + const data = { surface: route.surface, title: finalTitle }; return okFormatted(formatOk("rename_tab", data), data); } catch (e) { return err(e); @@ -7379,7 +7731,7 @@ export function createServer(opts?: CreateServerOptions): McpServer { // 10. close_surface server.tool( "close_surface", - "Close a surface (terminal or browser pane). SAFETY: if the surface still backs a live agent (not done/error), the close is REFUSED unless force:true, and the response includes a fresh read of the pane so you can confirm for yourself whether it is really finished before destroying it. Browser panes and surfaces with no tracked agent close normally.", + "Close a surface (terminal or browser pane). SAFETY: if the surface still backs a live agent (not done/error), the close is REFUSED unless force:true, and the response includes a fresh read of the pane so you can confirm for yourself whether it is really finished before destroying it. force:true bypasses only the live-agent lifecycle gate; it never bypasses stable surface identity checks. Browser panes and surfaces with no tracked agent close normally.", { surface: z.string().describe("Target surface ref"), workspace: z.string().optional().describe("Target workspace ref"), @@ -7388,17 +7740,23 @@ export function createServer(opts?: CreateServerOptions): McpServer { .optional() .default(false) .describe( - "Close even when the backing agent is still live (not done/error). Without this, a live agent's surface is protected and the response returns the current pane contents instead of closing.", + "Close even when the backing agent is still live (not done/error). This never bypasses stable surface identity checks. Without force, a live agent's surface is protected and the response returns the current pane contents instead of closing.", ), }, ANNOTATIONS.destructive, async (args) => { try { - await assertSurfaceMutationAllowed( - "close_surface", + const route = await resolveRawSurfaceMutationRoute( args.surface, args.workspace, + "close_surface", + ); + await assertSurfaceMutationAllowed( + "close_surface", + route.surface, + route.workspace, ); + await route.assertCurrent(); let staleRegistryDoneConsolidated: | { agent_id: string; @@ -7421,15 +7779,18 @@ export function createServer(opts?: CreateServerOptions): McpServer { .listStates() .find( (record) => - record.surface_id === args.surface && + ((route.stableSurfaceIdentity && record.surface_uuid + ? record.surface_uuid.toLowerCase() === + route.stableSurfaceIdentity.toLowerCase() + : record.surface_id === route.surface)) && !TERMINAL_AGENT_STATES.has(record.state), ); if (backingAgent) { let screenText = "(unable to read pane)"; let screenParsed: ReturnType | null = null; try { - const screen = await client.readScreen(args.surface, { - workspace: args.workspace, + const screen = await client.readScreen(route.surface, { + workspace: route.workspace, lines: 40, }); screenText = screen.text; @@ -7466,7 +7827,7 @@ export function createServer(opts?: CreateServerOptions): McpServer { // If consolidation fails, keep the fail-safe refusal path. appendCloseEvent({ event: "close_surface", - target: `${args.surface} (agent ${backingAgent.agent_id})`, + target: `${route.surface} (agent ${backingAgent.agent_id})`, caller: resolveCloseCaller("close_surface"), force: args.force ?? false, reason: `refused: agent still live (${backingAgent.state}), registry consolidation failed`, @@ -7474,11 +7835,11 @@ export function createServer(opts?: CreateServerOptions): McpServer { }); return err( new Error( - `Refused to close ${args.surface}: agent ${backingAgent.agent_id} is "${backingAgent.state}" (still live) and registry consolidation failed. Pass force:true to close anyway. Current pane contents follow in screen/structuredContent.`, + `Refused to close ${route.surface}: agent ${backingAgent.agent_id} is "${backingAgent.state}" (still live) and registry consolidation failed. Pass force:true to close anyway. Current pane contents follow in screen/structuredContent.`, ), { refused: true, - surface: args.surface, + surface: route.surface, agent_id: backingAgent.agent_id, state: backingAgent.state, screen: screenText, @@ -7490,13 +7851,16 @@ export function createServer(opts?: CreateServerOptions): McpServer { .listStates() .find( (record) => - record.surface_id === args.surface && + ((route.stableSurfaceIdentity && record.surface_uuid + ? record.surface_uuid.toLowerCase() === + route.stableSurfaceIdentity.toLowerCase() + : record.surface_id === route.surface)) && !TERMINAL_AGENT_STATES.has(record.state), ); if (remainingLiveAgent) { appendCloseEvent({ event: "close_surface", - target: `${args.surface} (agent ${remainingLiveAgent.agent_id})`, + target: `${route.surface} (agent ${remainingLiveAgent.agent_id})`, caller: resolveCloseCaller("close_surface"), force: args.force ?? false, reason: `refused: agent still live (${remainingLiveAgent.state}) after stale registry consolidation`, @@ -7504,11 +7868,11 @@ export function createServer(opts?: CreateServerOptions): McpServer { }); return err( new Error( - `Refused to close ${args.surface}: agent ${remainingLiveAgent.agent_id} is "${remainingLiveAgent.state}" (still live) after stale registry consolidation. Pass force:true to close anyway. Current pane contents follow in screen/structuredContent.`, + `Refused to close ${route.surface}: agent ${remainingLiveAgent.agent_id} is "${remainingLiveAgent.state}" (still live) after stale registry consolidation. Pass force:true to close anyway. Current pane contents follow in screen/structuredContent.`, ), { refused: true, - surface: args.surface, + surface: route.surface, agent_id: remainingLiveAgent.agent_id, state: remainingLiveAgent.state, screen: screenText, @@ -7521,7 +7885,7 @@ export function createServer(opts?: CreateServerOptions): McpServer { } else { appendCloseEvent({ event: "close_surface", - target: `${args.surface} (agent ${backingAgent.agent_id})`, + target: `${route.surface} (agent ${backingAgent.agent_id})`, caller: resolveCloseCaller("close_surface"), force: args.force ?? false, reason: `refused: agent still live (${backingAgent.state})`, @@ -7529,11 +7893,11 @@ export function createServer(opts?: CreateServerOptions): McpServer { }); return err( new Error( - `Refused to close ${args.surface}: agent ${backingAgent.agent_id} is "${backingAgent.state}" (still live). Pass force:true to close anyway. Current pane contents follow in screen/structuredContent.`, + `Refused to close ${route.surface}: agent ${backingAgent.agent_id} is "${backingAgent.state}" (still live). Pass force:true to close anyway. Current pane contents follow in screen/structuredContent.`, ), { refused: true, - surface: args.surface, + surface: route.surface, agent_id: backingAgent.agent_id, state: backingAgent.state, screen: screenText, @@ -7548,11 +7912,11 @@ export function createServer(opts?: CreateServerOptions): McpServer { ReturnType | undefined; try { - const identified = args.workspace + const identified = route.workspace ? null - : await client.identify(args.surface); + : await client.identify(route.surface); const workspace = - args.workspace ?? + route.workspace ?? identified?.caller?.workspace_ref ?? identified?.focused?.workspace_ref; if (workspace) { @@ -7581,7 +7945,7 @@ export function createServer(opts?: CreateServerOptions): McpServer { panes.panes, paneSurfaces, workerSurfaceIds, - args.surface, + route.surface, ); } } catch { @@ -7590,15 +7954,28 @@ export function createServer(opts?: CreateServerOptions): McpServer { const collapsePane = closePolicy?.collapsePane ?? false; const observedSurface = await findSurfaceByRef( - args.surface, - args.workspace, + route.surface, + route.workspace, ); - const requestedSurfaceKey = args.surface.toLowerCase(); + const requestedSurfaceKey = + route.stableSurfaceIdentity?.toLowerCase() ?? + route.surface.toLowerCase(); const observedSurfaceUuid = observedSurface?.id?.toLowerCase(); - await client.closeSurface(args.surface, { - workspace: args.workspace, - collapsePane, - }); + await withSurfaceWrite( + route.surface, + async () => { + await route.assertCurrent(); + await client.closeSurface(route.surface, { + workspace: route.workspace, + collapsePane, + }); + }, + { + toolName: "close_surface", + workspace: route.workspace, + stableSurfaceIdentity: route.stableSurfaceIdentity, + }, + ); for (const record of stateMgr.listStates()) { // Stable identity wins whenever cmux exposes it. On a ref-only or // unavailable observation, preserve the explicit close intent by @@ -7607,9 +7984,9 @@ export function createServer(opts?: CreateServerOptions): McpServer { ? record.surface_uuid.toLowerCase() === requestedSurfaceKey || record.surface_uuid.toLowerCase() === observedSurfaceUuid || (observedSurfaceUuid === undefined && - record.surface_id === args.surface) + record.surface_id === route.surface) : observedSurfaceUuid === undefined && - record.surface_id === args.surface; + record.surface_id === route.surface; if (!matchesClosedSurface) { continue; } @@ -7630,7 +8007,7 @@ export function createServer(opts?: CreateServerOptions): McpServer { } appendCloseEvent({ event: "close_surface", - target: args.surface, + target: route.surface, caller: resolveCloseCaller("close_surface"), force: args.force ?? false, reason: staleRegistryDoneConsolidated @@ -7639,7 +8016,7 @@ export function createServer(opts?: CreateServerOptions): McpServer { refused: false, }); const data = { - surface: args.surface, + surface: route.surface, pane: closePolicy?.pane ?? undefined, collapse_pane: collapsePane, stale_registry_done_consolidated: staleRegistryDoneConsolidated, @@ -8324,9 +8701,12 @@ export function createServer(opts?: CreateServerOptions): McpServer { try { const existing = input.agentId ? engine.getAgentState(input.agentId) - : (registry - .list() - .find((record) => record.surface_id === input.surfaceId) ?? null); + : (registry.list().find((record) => + input.surfaceUuid + ? record.surface_uuid?.toLowerCase() === + input.surfaceUuid.toLowerCase() + : record.surface_id === input.surfaceId, + ) ?? null); if (!existing) return; const updated = @@ -8692,6 +9072,7 @@ export function createServer(opts?: CreateServerOptions): McpServer { chunk_size: SEND_INPUT_CHUNK_THRESHOLD, chunk_delay_ms: SEND_INPUT_CHUNK_DELAY_MS, press_enter: args.press_enter, + stableSurfaceIdentity: deliveryRoute.surface_uuid, source_event: args.source_event, source_agent: args.agent_id, // Verify every relay to an interactive agent — not just long ones. @@ -10264,30 +10645,57 @@ export function createServer(opts?: CreateServerOptions): McpServer { const requestedState = args.state; const buildListAgentsResponse = async (records: AgentRecord[]) => { const topology = await collectSurfaceTopology(); - const enrichedAgents = await Promise.all( + const rows = await Promise.all( records.map(async (agent) => { - const health = await evaluateServerAgentHealth( - agent, - { - ...healthTopologyOverrides(agent, topology), - }, - topology, - ); - return { - ...toPublicAgent(agent), - state: health.reconciled_state ?? agent.state, - health, - }; + try { + const health = await evaluateServerAgentHealth( + agent, + { + ...healthTopologyOverrides(agent, topology), + }, + topology, + ); + return { + agent: { + ...toPublicAgent(agent), + state: health.reconciled_state ?? agent.state, + health, + }, + skipped: null, + }; + } catch (error) { + return { + agent: null, + skipped: { + agent_id: agent.agent_id, + error: + error instanceof Error ? error.message : String(error), + }, + }; + } }), ); + const enrichedAgents = rows.flatMap((row) => + row.agent ? [row.agent] : [], + ); + const skippedAgents = rows.flatMap((row) => + row.skipped ? [row.skipped] : [], + ); const agents = requestedState ? enrichedAgents.filter((agent) => agent.state === requestedState) : enrichedAgents; const data = { agents: agents as unknown as Record[], count: agents.length, + ...(skippedAgents.length > 0 + ? { skipped_agents: skippedAgents } + : {}), }; - const formatted = formatListAgents(agents, agents.length); + const formatted = formatListAgents( + agents, + agents.length, + skippedAgents, + ); return okFormatted(formatted, data); }; diff --git a/tests/painpoint-e2e.test.ts b/tests/painpoint-e2e.test.ts index 12f48b61..1c17d84f 100644 --- a/tests/painpoint-e2e.test.ts +++ b/tests/painpoint-e2e.test.ts @@ -774,7 +774,33 @@ describe("Phase 10 painpoint e2e replay", () => { }; } if (command === "list-workspaces") { - return { stdout: JSON.stringify({ workspaces: [] }), stderr: "" }; + return { + stdout: JSON.stringify({ + workspaces: [{ ref: "workspace:1", title: "workspace" }], + }), + stderr: "", + }; + } + if (command === "list-panes") { + return { + stdout: JSON.stringify({ + workspace_ref: "workspace:1", + window_ref: "window:1", + panes: [{ ref: "pane:1", surface_refs: ["surface:agent-bg"] }], + }), + stderr: "", + }; + } + if (command === "list-pane-surfaces") { + return { + stdout: JSON.stringify({ + workspace_ref: "workspace:1", + window_ref: "window:1", + pane_ref: "pane:1", + surfaces: [{ ref: "surface:agent-bg", type: "terminal" }], + }), + stderr: "", + }; } return { stdout: "{}", stderr: "" }; }; diff --git a/tests/server-agent-tools.test.ts b/tests/server-agent-tools.test.ts index c6b02ff3..4bd7b86e 100644 --- a/tests/server-agent-tools.test.ts +++ b/tests/server-agent-tools.test.ts @@ -769,6 +769,7 @@ type UuidRouteSurface = { ref: string; id?: string; workspace_ref: string; + title?: string; }; function makeUuidRouteClient(initialSurfaces: UuidRouteSurface[]) { @@ -776,6 +777,7 @@ function makeUuidRouteClient(initialSurfaces: UuidRouteSurface[]) { let screenText = "gpt-5.5 xhigh - 99% left - ~/Gits/cmuxlayer\ncodex> "; const sendCalls: Array<{ surface: string; text: string }> = []; + const pasteCalls: Array<{ surface: string; text: string }> = []; const surfacesForWorkspace = (workspace?: string) => liveSurfaces.filter( (surface) => !workspace || surface.workspace_ref === workspace, @@ -827,7 +829,7 @@ function makeUuidRouteClient(initialSurfaces: UuidRouteSurface[]) { pane_ref: opts?.pane ?? `pane:${opts?.workspace ?? "1"}`, surfaces: surfacesForWorkspace(opts?.workspace).map((surface, index) => ({ ...surface, - title: "cmuxlayerCodex", + title: surface.title ?? "cmuxlayerCodex", type: "terminal", index, selected: index === 0, @@ -844,6 +846,11 @@ function makeUuidRouteClient(initialSurfaces: UuidRouteSurface[]) { send: vi.fn().mockImplementation(async (surface: string, text: string) => { sendCalls.push({ surface, text }); }), + pasteText: vi + .fn() + .mockImplementation(async (surface: string, text: string) => { + pasteCalls.push({ surface, text }); + }), sendKey: vi.fn().mockResolvedValue(undefined), log: vi.fn().mockResolvedValue(undefined), setStatus: vi.fn().mockResolvedValue(undefined), @@ -856,6 +863,12 @@ function makeUuidRouteClient(initialSurfaces: UuidRouteSurface[]) { closeSurface: vi.fn().mockImplementation(async (surface: string) => { liveSurfaces = liveSurfaces.filter((candidate) => candidate.ref !== surface); }), + moveSurface: vi.fn().mockImplementation(async (opts: { surface: string }) => ({ + surface: opts.surface, + pane: null, + workspace: null, + })), + renameTab: vi.fn().mockResolvedValue(undefined), notify: vi.fn(), listStatus: vi.fn().mockResolvedValue([]), identify: vi.fn().mockResolvedValue({}), @@ -865,6 +878,7 @@ function makeUuidRouteClient(initialSurfaces: UuidRouteSurface[]) { return { client, sendCalls, + pasteCalls, setLiveSurfaces(next: UuidRouteSurface[]) { liveSurfaces = next; }, @@ -4301,6 +4315,131 @@ describe("agent lifecycle tool handlers", () => { }); }); + it("list_agents returns healthy rows when one persisted repo is corrupt", async () => { + const routeClient = makeUuidRouteClient([ + { + ref: "surface:healthy", + id: "11111111-2222-4333-8444-555555555555", + workspace_ref: "workspace:1", + }, + { + ref: "surface:corrupt", + id: "aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee", + workspace_ref: "workspace:1", + }, + ]); + const server = createTrackedServer({ + client: routeClient.client as any, + stateDir: TEST_DIR, + lifecycleInitializer: async () => {}, + disableSpawnPreflight: true, + sessionIdentityResolver: () => null, + }); + await serverContexts.at(-1)?.lifecycleStartPromise; + const engine = testLifecycleEngine(server); + const healthy = makeServerAgentRecord({ + agent_id: "healthy-agent", + surface_id: "surface:healthy", + surface_uuid: "11111111-2222-4333-8444-555555555555", + workspace_id: "workspace:1", + state: "ready", + repo: "cmuxlayer", + cli_session_id: "healthy-session", + }); + const corrupt = makeServerAgentRecord({ + agent_id: "corrupt-agent", + surface_id: "surface:corrupt", + surface_uuid: "aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee", + workspace_id: "workspace:1", + state: "ready", + repo: "brainlayerClaude [surface:199]", + cli_session_id: "corrupt-session", + }); + for (const record of [healthy, corrupt]) { + engine.stateMgr.writeState(record); + engine.getRegistry().set(record.agent_id, record); + } + vi.spyOn(engine.getRegistry(), "listMerged").mockResolvedValue([ + healthy, + corrupt, + ]); + + const result = await registeredTestTool(server, "list_agents").handler( + {}, + {} as any, + ); + const parsed = parseToolResult(result) as { + ok: boolean; + agents: Array<{ agent_id: string }>; + skipped_agents?: Array<{ agent_id: string; error: string }>; + }; + + expect(parsed.ok).toBe(true); + expect(parsed.agents).toEqual([ + expect.objectContaining({ agent_id: "healthy-agent" }), + ]); + expect(parsed.skipped_agents).toEqual([ + expect.objectContaining({ + agent_id: "corrupt-agent", + error: expect.stringMatching(/repo|launcher/i), + }), + ]); + expect(result.content[0]?.text).toMatch(/skipped.*corrupt-agent/i); + }); + + it("send_to keeps registry repo ownership when a title contains a surface suffix", async () => { + const stableUuid = "11111111-2222-4333-8444-555555555555"; + const routeClient = makeUuidRouteClient([ + { + ref: "surface:199", + id: stableUuid, + workspace_ref: "workspace:1", + title: "brainlayerClaude [surface:199]", + }, + ]); + routeClient.setScreenText("Claude Code\n> "); + const record = makeServerAgentRecord({ + agent_id: "auto-claude-11111111-2222-4333-8444-555555555555", + surface_id: "surface:199", + surface_uuid: stableUuid, + workspace_id: "workspace:1", + state: "ready", + repo: "brainlayer", + cli: "claude", + model: "sonnet", + cli_session_id: "claude-session", + task_summary: "(auto-discovered)", + }); + const server = await createUuidRouteServer(routeClient, record); + const listResult = await registeredTestTool(server, "list_agents").handler( + {}, + {} as any, + ); + expect(parseToolResult(listResult)).toMatchObject({ + ok: true, + agents: [expect.objectContaining({ repo: "brainlayer" })], + }); + routeClient.client.send.mockClear(); + routeClient.sendCalls.length = 0; + + const result = await registeredTestTool(server, "send_to").handler( + { + agent_id: record.agent_id, + text: "keep going", + press_enter: false, + }, + {} as any, + ); + + expect(result.isError).toBeFalsy(); + expect(routeClient.sendCalls).toEqual([ + { surface: "surface:199", text: "keep going" }, + ]); + expect(testLifecycleEngine(server).getAgentState(record.agent_id)?.repo).toBe( + "brainlayer", + ); + }); + it("list_agents surfaces a collapsed monitor on its owning agent health", async () => { const registryPath = join(TEST_DIR, "monitor-registry.json"); const watchedFile = join(TEST_DIR, "collab.md"); @@ -6353,6 +6492,449 @@ codex> ); }); + it("raw send_to refuses an ambiguous numeric ref after it is recycled", async () => { + const originalUuid = "11111111-2222-4333-8444-555555555555"; + const otherUuid = "aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee"; + const routeClient = makeUuidRouteClient([ + { + ref: "surface:230", + id: originalUuid, + workspace_ref: "workspace:1", + }, + { + ref: "surface:219", + id: otherUuid, + workspace_ref: "workspace:1", + }, + ]); + const server = createTrackedServer({ + client: routeClient.client as any, + stateDir: TEST_DIR, + lifecycleInitializer: async () => {}, + }); + await registeredTestTool(server, "list_surfaces").handler({}, {} as any); + routeClient.setLiveSurfaces([ + { + ref: "surface:236", + id: originalUuid, + workspace_ref: "workspace:1", + }, + { + ref: "surface:230", + id: otherUuid, + workspace_ref: "workspace:1", + }, + ]); + routeClient.client.send.mockClear(); + routeClient.sendCalls.length = 0; + + const result = await registeredTestTool(server, "send_to").handler( + { + mode: "surface", + target: "surface:230", + text: "follow the captured UUID", + press_enter: false, + }, + {} as any, + ); + + expect(result.isError).toBe(true); + expect(parseToolResult(result).error).toMatch(/ambiguous|recycled|multiple/i); + expect(routeClient.sendCalls).toEqual([]); + }); + + it("raw send_to follows the captured UUID when the old ref is vacated", async () => { + const originalUuid = "11111111-2222-4333-8444-555555555555"; + const routeClient = makeUuidRouteClient([ + { + ref: "surface:230", + id: originalUuid, + workspace_ref: "workspace:1", + }, + ]); + const server = createTrackedServer({ + client: routeClient.client as any, + stateDir: TEST_DIR, + lifecycleInitializer: async () => {}, + }); + await registeredTestTool(server, "list_surfaces").handler({}, {} as any); + routeClient.setLiveSurfaces([ + { + ref: "surface:236", + id: originalUuid, + workspace_ref: "workspace:1", + }, + ]); + routeClient.client.send.mockClear(); + routeClient.sendCalls.length = 0; + + const result = await registeredTestTool(server, "send_to").handler( + { + mode: "surface", + target: "surface:230", + text: "follow the captured UUID", + press_enter: false, + }, + {} as any, + ); + + expect(result.isError).toBeFalsy(); + expect(routeClient.sendCalls).toEqual([ + { surface: "surface:236", text: "follow the captured UUID" }, + ]); + }); + + it("raw send_to refuses to follow a UUID outside the caller's explicit workspace", async () => { + const originalUuid = "11111111-2222-4333-8444-555555555555"; + const routeClient = makeUuidRouteClient([ + { + ref: "surface:230", + id: originalUuid, + workspace_ref: "workspace:1", + }, + ]); + const server = createTrackedServer({ + client: routeClient.client as any, + stateDir: TEST_DIR, + lifecycleInitializer: async () => {}, + }); + await registeredTestTool(server, "list_surfaces").handler({}, {} as any); + routeClient.setLiveSurfaces([ + { + ref: "surface:236", + id: originalUuid, + workspace_ref: "workspace:2", + }, + ]); + routeClient.client.send.mockClear(); + routeClient.sendCalls.length = 0; + + const result = await registeredTestTool(server, "send_to").handler( + { + mode: "surface", + target: "surface:230", + workspace: "workspace:1", + text: "must stay workspace-scoped", + press_enter: false, + }, + {} as any, + ); + + expect(result.isError).toBe(true); + expect(parseToolResult(result).error).toMatch(/explicit workspace|workspace:1/i); + expect(routeClient.sendCalls).toEqual([]); + }); + + it("background ref-only delivery attributes failure to its start observer", async () => { + vi.useFakeTimers(); + try { + const routeClient = makeUuidRouteClient([ + { + ref: "surface:230", + workspace_ref: "workspace:1", + }, + ]); + const tracker = new SurfaceWriteLivenessTracker({ now: () => 1_000 }); + let observerOwner = "cmux:/tmp/observer-old.sock"; + routeClient.client.send.mockImplementation(async () => { + observerOwner = "cmux:/tmp/observer-new.sock"; + throw Object.assign(new Error("broken pipe"), { code: "EPIPE" }); + }); + const server = createTrackedServer({ + client: routeClient.client as any, + stateDir: TEST_DIR, + skipAgentLifecycle: true, + surfaceWriteLiveness: tracker, + surfaceObserverOwnerIdProvider: () => observerOwner, + surfaceObserverEpochProvider: () => "stable-test-epoch", + }); + + const accepted = await registeredTestTool(server, "send_input").handler( + { + surface: "surface:230", + text: "fail after observer capture", + background: true, + press_enter: false, + }, + {} as any, + ); + expect(parseToolResult(accepted)).toMatchObject({ status: "delivering" }); + + await vi.advanceTimersByTimeAsync(1); + + expect( + tracker.observe( + "surface:230", + null, + "cmux:/tmp/observer-old.sock", + ), + ).toMatchObject({ consecutive_broken_pipe_failures: 1 }); + expect( + tracker.observe( + "surface:230", + null, + "cmux:/tmp/observer-new.sock", + ), + ).toBeNull(); + } finally { + vi.useRealTimers(); + } + }); + + it("raw send_to refuses an absent ref in complete ref-only topology", async () => { + const routeClient = makeUuidRouteClient([ + { + ref: "surface:other", + workspace_ref: "workspace:1", + }, + ]); + const server = createTrackedServer({ + client: routeClient.client as any, + stateDir: TEST_DIR, + lifecycleInitializer: async () => {}, + }); + + const result = await registeredTestTool(server, "send_to").handler( + { + mode: "surface", + target: "surface:missing", + text: "must not reach a later occupant", + press_enter: false, + }, + {} as any, + ); + + expect(result.isError).toBe(true); + expect(parseToolResult(result).error).toMatch(/fresh topology|not live/i); + expect(routeClient.sendCalls).toEqual([]); + }); + + it("list_surfaces discards UUID captures when the observer changes mid-list", async () => { + const staleUuid = "11111111-2222-4333-8444-555555555555"; + const currentUuid = "aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee"; + const routeClient = makeUuidRouteClient([ + { + ref: "surface:230", + id: staleUuid, + workspace_ref: "workspace:1", + }, + ]); + let observerEpoch = "test:1"; + const listPaneSurfaces = + routeClient.client.listPaneSurfaces.getMockImplementation()!; + routeClient.client.listPaneSurfaces.mockImplementationOnce(async (opts) => { + const snapshot = await listPaneSurfaces(opts); + observerEpoch = "test:2"; + return snapshot; + }); + const server = createTrackedServer({ + client: routeClient.client as any, + stateDir: TEST_DIR, + lifecycleInitializer: async () => {}, + surfaceObserverEpochProvider: () => observerEpoch, + }); + + const listed = await registeredTestTool(server, "list_surfaces").handler( + {}, + {} as any, + ); + expect(parseToolResult(listed)).toMatchObject({ + surfaces: [expect.objectContaining({ ref: "surface:230", id: staleUuid })], + }); + routeClient.setLiveSurfaces([ + { + ref: "surface:230", + id: currentUuid, + workspace_ref: "workspace:1", + }, + ]); + routeClient.client.send.mockClear(); + routeClient.sendCalls.length = 0; + + const result = await registeredTestTool(server, "send_to").handler( + { + mode: "surface", + target: "surface:230", + text: "current observer only", + press_enter: false, + }, + {} as any, + ); + + expect(result.isError).toBeFalsy(); + expect(routeClient.sendCalls).toEqual([ + { surface: "surface:230", text: "current observer only" }, + ]); + }); + + it.each(["move_surface", "rename_tab"])( + "%s refuses a recycled raw ref", + async (toolName) => { + const originalUuid = "11111111-2222-4333-8444-555555555555"; + const replacementUuid = "aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee"; + const routeClient = makeUuidRouteClient([ + { + ref: "surface:230", + id: originalUuid, + workspace_ref: "workspace:1", + }, + ]); + const server = createTrackedServer({ + client: routeClient.client as any, + stateDir: TEST_DIR, + lifecycleInitializer: async () => {}, + }); + await registeredTestTool(server, "list_surfaces").handler({}, {} as any); + routeClient.setLiveSurfaces([ + { + ref: "surface:236", + id: originalUuid, + workspace_ref: "workspace:1", + }, + { + ref: "surface:230", + id: replacementUuid, + workspace_ref: "workspace:1", + }, + ]); + + const args = + toolName === "move_surface" + ? { surface: "surface:230", pane: "pane:destination" } + : { surface: "surface:230", title: "must not rename replacement" }; + const result = await registeredTestTool(server, toolName).handler( + args, + {} as any, + ); + + expect(result.isError).toBe(true); + expect(parseToolResult(result).error).toMatch(/ambiguous|recycled|multiple/i); + expect(routeClient.client.moveSurface).not.toHaveBeenCalled(); + expect(routeClient.client.renameTab).not.toHaveBeenCalled(); + }, + ); + + it("raw send_to refuses when the UUID captured for a numeric ref is gone", async () => { + const originalUuid = "11111111-2222-4333-8444-555555555555"; + const replacementUuid = "aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee"; + const routeClient = makeUuidRouteClient([ + { + ref: "surface:230", + id: originalUuid, + workspace_ref: "workspace:1", + }, + ]); + const server = createTrackedServer({ + client: routeClient.client as any, + stateDir: TEST_DIR, + lifecycleInitializer: async () => {}, + }); + await registeredTestTool(server, "list_surfaces").handler({}, {} as any); + routeClient.setLiveSurfaces([ + { + ref: "surface:230", + id: replacementUuid, + workspace_ref: "workspace:1", + }, + ]); + routeClient.client.send.mockClear(); + routeClient.sendCalls.length = 0; + + const result = await registeredTestTool(server, "send_to").handler( + { + mode: "surface", + target: "surface:230", + text: "must not reach the replacement", + press_enter: false, + }, + {} as any, + ); + + expect(result.isError).toBe(true); + expect(parseToolResult(result).error).toMatch(/stable surface UUID|stale/i); + expect(routeClient.sendCalls).toEqual([]); + expect(routeClient.client.send).not.toHaveBeenCalled(); + }); + + it("raw send_to pastes multiline text through the captured stable UUID", async () => { + const originalUuid = "11111111-2222-4333-8444-555555555555"; + const routeClient = makeUuidRouteClient([ + { + ref: "surface:230", + id: originalUuid, + workspace_ref: "workspace:1", + }, + ]); + const server = createTrackedServer({ + client: routeClient.client as any, + stateDir: TEST_DIR, + lifecycleInitializer: async () => {}, + }); + await registeredTestTool(server, "list_surfaces").handler({}, {} as any); + routeClient.setLiveSurfaces([ + { + ref: "surface:236", + id: originalUuid, + workspace_ref: "workspace:1", + }, + ]); + routeClient.client.pasteText.mockClear(); + routeClient.pasteCalls.length = 0; + + const result = await registeredTestTool(server, "send_to").handler( + { + mode: "surface", + target: "surface:230", + text: "first line\nsecond line", + press_enter: false, + allow_long_inline: true, + }, + {} as any, + ); + + expect(result.isError).toBeFalsy(); + expect(routeClient.pasteCalls).toEqual([ + { surface: "surface:236", text: "first line\nsecond line" }, + ]); + }); + + it.each([undefined, "workspace:1"])( + "close_surface uses the same stable binding with workspace=%s", + async (workspace) => { + const routeClient = makeUuidRouteClient([ + { + ref: "surface:230", + id: "11111111-2222-4333-8444-555555555555", + workspace_ref: "workspace:1", + }, + ]); + routeClient.client.closeSurface.mockImplementation( + async (_surface: string, opts?: { workspace?: string }) => { + if (!opts?.workspace) { + throw new Error("Workspace not found"); + } + }, + ); + const server = createTrackedServer({ + client: routeClient.client as any, + stateDir: TEST_DIR, + skipAgentLifecycle: true, + }); + await registeredTestTool(server, "list_surfaces").handler({}, {} as any); + + const result = await registeredTestTool(server, "close_surface").handler( + { surface: "surface:230", ...(workspace ? { workspace } : {}) }, + {} as any, + ); + + expect(result.isError).toBeFalsy(); + expect(routeClient.client.closeSurface).toHaveBeenCalledWith( + "surface:230", + expect.objectContaining({ workspace: "workspace:1" }), + ); + }, + ); + it("records managed send failures against the stable UUID instead of its mutable ref", async () => { const stableUuid = "11111111-2222-4333-8444-555555555555"; const otherUuid = "aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee"; diff --git a/tests/server.test.ts b/tests/server.test.ts index e0cd783f..06c23b88 100644 --- a/tests/server.test.ts +++ b/tests/server.test.ts @@ -1464,6 +1464,7 @@ describe("tool handler integration", () => { ]); expect(parsed.surfaces[0]).toEqual({ ref: "surface:1", + id: "surface-uuid-1", workspace_ref: "workspace:1", pane_ref: "pane:1", column: 0, @@ -1476,6 +1477,7 @@ describe("tool handler integration", () => { }); expect(parsed.surfaces[1]).toEqual({ ref: "surface:2", + id: "surface-uuid-2", workspace_ref: "workspace:1", pane_ref: "pane:2", column: 1, @@ -2674,28 +2676,32 @@ describe("tool handler integration", () => { {} as any, ); - // Should try to resolve mode scope, fail open when no workspace is known, - // preflight the screen, send text, and press enter. Raw uncached surfaces do - // not get submit_verified:true from prompt clearing alone. - expect(mockExec).toHaveBeenCalledTimes(4); + // Stable identity resolution runs first, then mode scope, screen preflight, + // text delivery, and Return. Legacy topology has no UUID to retain. + expect(mockExec).toHaveBeenCalledTimes(5); expect(mockExec).toHaveBeenNthCalledWith( 1, "cmux", - expect.arrayContaining(["identify", "--surface", "surface:1"]), + expect.arrayContaining(["list-workspaces"]), ); expect(mockExec).toHaveBeenNthCalledWith( 2, "cmux", - expect.arrayContaining(["read-screen"]), + expect.arrayContaining(["identify", "--surface", "surface:1"]), ); expect(mockExec).toHaveBeenNthCalledWith( 3, "cmux", - expect.arrayContaining(["send"]), + expect.arrayContaining(["read-screen"]), ); expect(mockExec).toHaveBeenNthCalledWith( 4, "cmux", + expect.arrayContaining(["send"]), + ); + expect(mockExec).toHaveBeenNthCalledWith( + 5, + "cmux", expect.arrayContaining(["send-key"]), ); expect(mockExec).not.toHaveBeenCalledWith( @@ -2819,25 +2825,30 @@ describe("tool handler integration", () => { {} as any, ); - expect(mockExec).toHaveBeenCalledTimes(4); + expect(mockExec).toHaveBeenCalledTimes(5); expect(mockExec).toHaveBeenNthCalledWith( 1, "cmux", - expect.arrayContaining(["identify", "--surface", "surface:6"]), + expect.arrayContaining(["list-workspaces"]), ); expect(mockExec).toHaveBeenNthCalledWith( 2, "cmux", - expect.arrayContaining(["read-screen", "--surface", "surface:6"]), + expect.arrayContaining(["identify", "--surface", "surface:6"]), ); expect(mockExec).toHaveBeenNthCalledWith( 3, "cmux", - expect.arrayContaining(["send", "--surface", "surface:6"]), + expect.arrayContaining(["read-screen", "--surface", "surface:6"]), ); expect(mockExec).toHaveBeenNthCalledWith( 4, "cmux", + expect.arrayContaining(["send", "--surface", "surface:6"]), + ); + expect(mockExec).toHaveBeenNthCalledWith( + 5, + "cmux", expect.arrayContaining(["send-key", "--surface", "surface:6", "return"]), ); expect(mockExec).not.toHaveBeenCalledWith( @@ -4041,7 +4052,30 @@ describe("tool handler integration", () => { } if (args.includes("list-workspaces")) { return Promise.resolve({ - stdout: JSON.stringify({ workspaces: [] }), + stdout: JSON.stringify({ + workspaces: [{ ref: "workspace:1", title: "workspace" }], + }), + stderr: "", + }); + } + if (args.includes("list-panes")) { + return Promise.resolve({ + stdout: JSON.stringify({ + workspace_ref: "workspace:1", + window_ref: "window:1", + panes: [{ ref: "pane:1", surface_refs: ["surface:1"] }], + }), + stderr: "", + }); + } + if (args.includes("list-pane-surfaces")) { + return Promise.resolve({ + stdout: JSON.stringify({ + workspace_ref: "workspace:1", + window_ref: "window:1", + pane_ref: "pane:1", + surfaces: [{ ref: "surface:1", type: "terminal" }], + }), stderr: "", }); } @@ -4501,7 +4535,30 @@ describe("tool handler integration", () => { } if (args.includes("list-workspaces")) { return Promise.resolve({ - stdout: JSON.stringify({ workspaces: [] }), + stdout: JSON.stringify({ + workspaces: [{ ref: "workspace:1", title: "workspace" }], + }), + stderr: "", + }); + } + if (args.includes("list-panes")) { + return Promise.resolve({ + stdout: JSON.stringify({ + workspace_ref: "workspace:1", + window_ref: "window:1", + panes: [{ ref: "pane:1", surface_refs: ["surface:1"] }], + }), + stderr: "", + }); + } + if (args.includes("list-pane-surfaces")) { + return Promise.resolve({ + stdout: JSON.stringify({ + workspace_ref: "workspace:1", + window_ref: "window:1", + pane_ref: "pane:1", + surfaces: [{ ref: "surface:1", type: "terminal" }], + }), stderr: "", }); } @@ -4629,7 +4686,30 @@ describe("tool handler integration", () => { } if (args.includes("list-workspaces")) { return Promise.resolve({ - stdout: JSON.stringify({ workspaces: [] }), + stdout: JSON.stringify({ + workspaces: [{ ref: "workspace:1", title: "workspace" }], + }), + stderr: "", + }); + } + if (args.includes("list-panes")) { + return Promise.resolve({ + stdout: JSON.stringify({ + workspace_ref: "workspace:1", + window_ref: "window:1", + panes: [{ ref: "pane:1", surface_refs: ["surface:agent-bg"] }], + }), + stderr: "", + }); + } + if (args.includes("list-pane-surfaces")) { + return Promise.resolve({ + stdout: JSON.stringify({ + workspace_ref: "workspace:1", + window_ref: "window:1", + pane_ref: "pane:1", + surfaces: [{ ref: "surface:agent-bg", type: "terminal" }], + }), stderr: "", }); } @@ -10976,7 +11056,17 @@ describe("tool handler integration", () => { listPanes: vi.fn().mockResolvedValue({ workspace_ref: "workspace:1", window_ref: "window:1", - panes: [{ ref: "pane:1" }], + panes: [ + { + ref: "pane:1", + index: 0, + focused: true, + surface_count: 1, + surface_refs: ["surface:worker-crash-recoverable"], + surface_ids: [stableSurfaceUuid], + selected_surface_ref: "surface:worker-crash-recoverable", + }, + ], }), listPaneSurfaces: vi.fn().mockResolvedValue({ workspace_ref: "workspace:1", @@ -11016,7 +11106,7 @@ describe("tool handler integration", () => { rmSync(stateDir, { recursive: true, force: true }); }); - it("close_surface does not make a stale UUID owner terminal when its ref was recycled", async () => { + it("close_surface refuses a recycled ref instead of closing its new occupant", async () => { const stateDir = processScopedTmpDir( "cmuxlayer-close-surface-recycled-ref-terminal", ); @@ -11065,7 +11155,17 @@ describe("tool handler integration", () => { listPanes: vi.fn().mockResolvedValue({ workspace_ref: "workspace:1", window_ref: "window:1", - panes: [{ ref: "pane:1" }], + panes: [ + { + ref: "pane:1", + index: 0, + focused: true, + surface_count: 1, + surface_refs: ["surface:recycled-worker"], + surface_ids: ["bbbbbbbb-cccc-4ddd-8eee-ffffffffffff"], + selected_surface_ref: "surface:recycled-worker", + }, + ], }), listPaneSurfaces: vi.fn().mockResolvedValue({ workspace_ref: "workspace:1", @@ -11095,8 +11195,9 @@ describe("tool handler integration", () => { {} as any, ); - expect(result.isError).not.toBe(true); - expect(mockClient.closeSurface).toHaveBeenCalled(); + expect(result.isError).toBe(true); + expect(result.structuredContent?.error).toMatch(/stable surface UUID/i); + expect(mockClient.closeSurface).not.toHaveBeenCalled(); expect(stateMgr.readState("worker-stale-ref-owner")).toMatchObject({ user_killed: false, crash_recover: true, @@ -11125,9 +11226,12 @@ describe("tool handler integration", () => { {} as any, ); - expect(degradedResult.isError).not.toBe(true); + expect(degradedResult.isError).toBe(true); + expect(degradedResult.structuredContent?.error).toMatch( + /could not be resolved in fresh topology/i, + ); expect(stateMgr.readState("worker-degraded-ref-owner")).toMatchObject({ - user_killed: true, + user_killed: false, crash_recover: true, });