From f1d1e09ce37b0a46e76a5f46d5ee9cfbe21c8cac Mon Sep 17 00:00:00 2001 From: Etan Joseph Heyman Date: Mon, 10 Aug 2026 19:30:52 +0300 Subject: [PATCH 1/2] fix: refuse agent delivery to exited panes Co-Authored-By: cmuxlayerCodex running gpt-5.6-sol --- src/agent-discovery.ts | 4 ++ src/server.ts | 41 +++++++++-- tests/server-agent-tools.test.ts | 118 ++++++++++++++++++++++++++++++- tests/v2-interact-kill.test.ts | 2 +- 4 files changed, 158 insertions(+), 7 deletions(-) diff --git a/src/agent-discovery.ts b/src/agent-discovery.ts index 753dfae0..2f01318d 100644 --- a/src/agent-discovery.ts +++ b/src/agent-discovery.ts @@ -3,6 +3,7 @@ import type { AgentState, CliType } from "./agent-types.js"; import type { CmuxReadScreenResult, CmuxSurface, + ParsedControlPlaneState, ParsedScreenStatus, } from "./types.js"; @@ -13,6 +14,7 @@ export interface DiscoveredAgent { surface_title: string; workspace_id?: string | null; cli: CliType | "unknown"; + control_state: ParsedControlPlaneState; parsed_status: ParsedScreenStatus | null; model: string | null; token_count: number | null; @@ -132,6 +134,7 @@ export class AgentDiscovery { surface_title: surface.title, workspace_id: workspaceId, cli, + control_state: parsed.control_state, parsed_status: parsed.status, model: parsed.model, token_count: parsed.token_count, @@ -150,6 +153,7 @@ export class AgentDiscovery { surface_title: surface.title, workspace_id: workspaceId, cli: "unknown", + control_state: "unknown", parsed_status: null, model: null, token_count: null, diff --git a/src/server.ts b/src/server.ts index e8fb2fd0..5a50f5a8 100644 --- a/src/server.ts +++ b/src/server.ts @@ -9338,16 +9338,46 @@ export function createServer(opts?: CreateServerOptions): McpServer { } route = reresolved; } + // Agent-path delivery requires a live agent TUI. A crashed CLI leaves its + // terminal surface alive at a bare shell; typing a routed message there + // executes fleet text as shell input. Fresh discovery validates the + // stable UUID/ref binding around read-screen, so readable shell evidence + // can fail closed before any terminal mutation. Raw surface/command/key + // modes bypass this helper and remain available for deliberate recovery. + const normalizedUuid = (value: string | null | undefined): string | null => + value?.trim().toLowerCase() || null; + const assertAgentRouteHasTui = async (candidateRoute: typeof route) => { + discovery.invalidate(); + const freshOccupant = (await discovery.scan(true)).find((entry) => + candidateRoute.surface_uuid + ? normalizedUuid(entry.surface_uuid) === + normalizedUuid(candidateRoute.surface_uuid) + : entry.surface_id === candidateRoute.surface_id, + ); + if ( + freshOccupant && + !freshOccupant.read_error && + freshOccupant.control_state === "shell" + ) { + throw new Error( + `Agent "${args.agent_id}" exited / no agent currently initiated on ` + + `surface ${candidateRoute.surface_id} (control_state=${freshOccupant.control_state}, ` + + `agent_type=${freshOccupant.cli}); refusing routed agent delivery. ` + + `Use send_to mode=surface, command, or key for deliberate raw terminal input.`, + ); + } + return freshOccupant; + }; + const freshOccupant = await assertAgentRouteHasTui(route); + // Identity guard: a live surface ref may have been RECYCLED — a crashed // agent's pane reused by a different agent. If the live surface now hosts // a known CLI that differs from this agent's recorded CLI, refuse rather - // than delivering to the new occupant. Fails OPEN when the live CLI is - // unknown/unreadable so a parse miss never blocks a healthy relay. + // than delivering to the new occupant. Fresh shell evidence was already + // refused above; other unknown/unreadable evidence remains inconclusive. const expectedCli = engine.getAgentState(args.agent_id)?.cli; if (requiresMutableRefGuards && expectedCli) { - const cachedOccupant = (await discovery.scan(false)).find( - (entry) => entry.surface_id === route.surface_id, - ); + const cachedOccupant = freshOccupant; const isForeign = (occ: typeof cachedOccupant): boolean => Boolean( occ && @@ -9407,6 +9437,7 @@ export function createServer(opts?: CreateServerOptions): McpServer { // landed, following a moved UUID would split one logical message across // terminals, so route changes fail closed instead. route = await engine.resolveAgentIoRoute(args.agent_id); + await assertAgentRouteHasTui(route); const deliveryRoute = route; const assertDeliveryRouteCurrent = async (): Promise => { const current = await engine.resolveAgentIoRoute(args.agent_id); diff --git a/tests/server-agent-tools.test.ts b/tests/server-agent-tools.test.ts index 56f18bff..fa9716b4 100644 --- a/tests/server-agent-tools.test.ts +++ b/tests/server-agent-tools.test.ts @@ -143,7 +143,7 @@ function makeLifecycleExec(opts?: { } if (text.includes("Claude")) { activeCli = "claude"; - readyText = "What can I help you with?\n>"; + readyText = "Claude Code\nWhat can I help you with?\n>"; } if (text.includes("Cursor")) { activeCli = "cursor"; @@ -6995,6 +6995,60 @@ codex> expect(engine.getAgentState(agentId)?.state).toBe("idle"); }); + it.each(["send_to", "send_to_agent"] as const)( + "%s refuses routed delivery when the agent pane has fallen back to a bare shell", + async (toolName) => { + let showBareShell = false; + const base = makeLifecycleExec({ + surfaceUuid: "11111111-2222-4333-8444-555555555555", + }); + const exec: ExecFn = vi.fn().mockImplementation(async (cmd, args) => { + if (showBareShell && args.includes("read-screen")) { + return { + stdout: JSON.stringify({ + surface: "surface:new", + text: "etan@mac cmuxlayer %", + lines: 1, + scrollback_used: false, + }), + stderr: "", + }; + } + return base(cmd, args); + }); + const server = createLifecycleServer(exec); + const spawn = (server as any)._registeredTools["spawn_agent"]; + const sendTo = (server as any)._registeredTools[toolName]; + const spawnResult = await spawn.handler( + { repo: "test", model: "sonnet", cli: "claude" }, + {} as any, + ); + const agentId = parseToolResult(spawnResult).agent_id as string; + const engine = (server as any)._registeredTools["interact"]._engine; + const registry = engine.getRegistry(); + const exited = engine.stateMgr.updateRecord(agentId, { + state: "error", + error: "Agent CLI exited", + }); + registry.set(agentId, exited); + showBareShell = true; + exec.mockClear(); + + const result = await sendTo.handler( + { agent_id: agentId, text: "Etan routed message", press_enter: false }, + {} as any, + ); + + expect(result.isError).toBe(true); + expect(parseToolResult(result).error).toMatch( + /exited \/ no agent currently initiated/i, + ); + expect( + exec.mock.calls.filter(([, args]) => args.includes("send")), + ).toEqual([]); + }, + ); + it.each(["send_to", "send_to_agent"] as const)( "RC3: %s delivers to an error-state agent whose surface is alive", async (toolName) => { @@ -7149,6 +7203,68 @@ codex> ); }); + it("send_to rechecks for a bare shell after its final agent route resolution", async () => { + const stableUuid = "11111111-2222-4333-8444-555555555555"; + const routeClient = makeUuidRouteClient([ + { + ref: "surface:agent", + id: stableUuid, + workspace_ref: "workspace:1", + }, + ]); + routeClient.setScreenText( + "OpenAI Codex\nModel: gpt-5.5\nWorking (1s - esc to interrupt)", + ); + const record = makeServerAgentRecord({ + agent_id: "uuid-route-rebinds-to-shell", + surface_id: "surface:agent", + surface_uuid: stableUuid, + workspace_id: "workspace:1", + state: "ready", + repo: "cmuxlayer", + cli: "codex", + }); + const server = await createUuidRouteServer(routeClient, record); + const engine = testLifecycleEngine(server) as any; + const originalResolveAgentIoRoute = + engine.resolveAgentIoRoute.bind(engine); + let resolveCount = 0; + vi.spyOn(engine, "resolveAgentIoRoute").mockImplementation( + async (agentId: string) => { + resolveCount += 1; + if (resolveCount === 2) { + routeClient.setLiveSurfaces([ + { + ref: "surface:shell", + id: stableUuid, + workspace_ref: "workspace:1", + }, + ]); + routeClient.setScreenText("etan@mac cmuxlayer %"); + } + return originalResolveAgentIoRoute(agentId); + }, + ); + routeClient.client.send.mockClear(); + routeClient.sendCalls.length = 0; + + const result = await registeredTestTool(server, "send_to").handler( + { + agent_id: record.agent_id, + text: "must not execute after route rebind", + press_enter: false, + }, + {}, + ); + + expect(result.isError).toBe(true); + expect(parseToolResult(result).error).toMatch( + /exited \/ no agent currently initiated/i, + ); + expect(routeClient.sendCalls).toEqual([]); + expect(routeClient.client.send).not.toHaveBeenCalled(); + }); + 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"; diff --git a/tests/v2-interact-kill.test.ts b/tests/v2-interact-kill.test.ts index 9a7eaadd..44ae862f 100644 --- a/tests/v2-interact-kill.test.ts +++ b/tests/v2-interact-kill.test.ts @@ -129,7 +129,7 @@ function makeSpawnReadyExec(opts?: { closeKeepsSurface?: boolean }): ExecFn { text: agentMessageSubmitted ? "Claude Code\n✻ Working\n" : launchSent - ? "What can I help you with?\n>" + ? "Claude Code\nWhat can I help you with?\n>" : "$ ", lines: 20, scrollback_used: false, From f70244289e7e093ec829507e9dbc25e2723126cd Mon Sep 17 00:00:00 2001 From: Etan Joseph Heyman Date: Mon, 10 Aug 2026 19:52:01 +0300 Subject: [PATCH 2/2] fix: scope shell guard to target pane Co-Authored-By: cmuxlayerCodex running gpt-5.6-sol --- src/agent-discovery.ts | 151 +++++++++++++++++++++---------- src/server.ts | 30 ++---- tests/server-agent-tools.test.ts | 73 +++++++++++++++ 3 files changed, 185 insertions(+), 69 deletions(-) diff --git a/src/agent-discovery.ts b/src/agent-discovery.ts index 2f01318d..9e8a1cc8 100644 --- a/src/agent-discovery.ts +++ b/src/agent-discovery.ts @@ -98,6 +98,107 @@ export class AgentDiscovery { return this.deps.observerIdProvider?.()?.trim() || null; } + private async scanSurface(surface: CmuxSurface): Promise { + const workspaceId = + typeof surface.workspace_ref === "string" ? surface.workspace_ref : null; + try { + const screen = await this.deps.readScreen(surface.ref, { + lines: 30, + workspace: workspaceId ?? undefined, + }); + const parsed = parseScreen(screen.text); + const cli = + parsed.agent_type === "unknown" + ? "unknown" + : (parsed.agent_type as CliType); + + return { + surface_id: surface.ref, + surface_uuid: surface.id ?? null, + surface_title: surface.title, + workspace_id: workspaceId, + cli, + control_state: parsed.control_state, + parsed_status: parsed.status, + model: parsed.model, + token_count: parsed.token_count, + context_pct: parsed.context_pct, + has_agent: cli !== "unknown", + read_error: false, + }; + } catch (error) { + console.warn( + `[AgentDiscovery] Failed to scan surface ${surface.ref} (${surface.title})`, + error, + ); + return { + surface_id: surface.ref, + surface_uuid: surface.id ?? null, + surface_title: surface.title, + workspace_id: workspaceId, + cli: "unknown", + control_state: "unknown", + parsed_status: null, + model: null, + token_count: null, + context_pct: null, + has_agent: false, + read_error: true, + }; + } + } + + async scanTarget(target: { + surface_id: string; + surface_uuid?: string | null; + }): Promise { + const observerId = this.getObserverId(); + const uuidKey = (value: string | null | undefined): string | null => + value?.trim().toLowerCase() || null; + const expectedUuid = uuidKey(target.surface_uuid); + const matchesTarget = (surface: CmuxSurface): boolean => + expectedUuid + ? uuidKey(surface.id) === expectedUuid + : surface.ref === target.surface_id; + + const initialMatches = (await this.deps.listSurfaces()) + .filter((surface) => surface.type === "terminal") + .filter(matchesTarget); + if (initialMatches.length !== 1) return null; + + const initial = initialMatches[0]; + const result = await this.scanSurface(initial); + const completedObserverId = this.getObserverId(); + if (completedObserverId !== observerId) { + throw new Error( + `Surface observer changed during target discovery (${observerId ?? "unknown"} -> ${completedObserverId ?? "unknown"})`, + ); + } + + const completedMatches = (await this.deps.listSurfaces()) + .filter((surface) => surface.type === "terminal") + .filter(matchesTarget); + const completed = completedMatches[0]; + if ( + completedMatches.length !== 1 || + completed?.ref !== initial.ref || + (completed.workspace_ref ?? null) !== (initial.workspace_ref ?? null) + ) { + throw new SurfaceBindingChangedDuringDiscoveryError( + `Target surface binding changed during discovery for ${initial.ref}` + + `${initial.id ? ` (UUID ${initial.id})` : ""}; refusing stale screen evidence`, + ); + } + + const validatedObserverId = this.getObserverId(); + if (validatedObserverId !== observerId) { + throw new Error( + `Surface observer changed during target discovery (${observerId ?? "unknown"} -> ${validatedObserverId ?? "unknown"})`, + ); + } + return result; + } + async scan(force = false): Promise { const observerScoped = typeof this.deps.observerIdProvider === "function"; @@ -114,55 +215,7 @@ export class AgentDiscovery { (surface) => surface.type === "terminal", ); const result = await Promise.all( - surfaces.map(async (surface): Promise => { - const workspaceId = - typeof surface.workspace_ref === "string" ? surface.workspace_ref : null; - try { - const screen = await this.deps.readScreen(surface.ref, { - lines: 30, - workspace: workspaceId ?? undefined, - }); - const parsed = parseScreen(screen.text); - const cli = - parsed.agent_type === "unknown" - ? "unknown" - : (parsed.agent_type as CliType); - - return { - surface_id: surface.ref, - surface_uuid: surface.id ?? null, - surface_title: surface.title, - workspace_id: workspaceId, - cli, - control_state: parsed.control_state, - parsed_status: parsed.status, - model: parsed.model, - token_count: parsed.token_count, - context_pct: parsed.context_pct, - has_agent: cli !== "unknown", - read_error: false, - }; - } catch (error) { - console.warn( - `[AgentDiscovery] Failed to scan surface ${surface.ref} (${surface.title})`, - error, - ); - return { - surface_id: surface.ref, - surface_uuid: surface.id ?? null, - surface_title: surface.title, - workspace_id: workspaceId, - cli: "unknown", - control_state: "unknown", - parsed_status: null, - model: null, - token_count: null, - context_pct: null, - has_agent: false, - read_error: true, - }; - } - }), + surfaces.map((surface) => this.scanSurface(surface)), ); const completedObserverId = this.getObserverId(); diff --git a/src/server.ts b/src/server.ts index 5a50f5a8..6f71244f 100644 --- a/src/server.ts +++ b/src/server.ts @@ -9340,20 +9340,13 @@ export function createServer(opts?: CreateServerOptions): McpServer { } // Agent-path delivery requires a live agent TUI. A crashed CLI leaves its // terminal surface alive at a bare shell; typing a routed message there - // executes fleet text as shell input. Fresh discovery validates the - // stable UUID/ref binding around read-screen, so readable shell evidence - // can fail closed before any terminal mutation. Raw surface/command/key - // modes bypass this helper and remain available for deliberate recovery. - const normalizedUuid = (value: string | null | undefined): string | null => - value?.trim().toLowerCase() || null; + // executes fleet text as shell input. Target-scoped discovery validates + // only this route's stable UUID/ref binding around read-screen, so + // unrelated pane churn cannot block a healthy relay. Raw + // surface/command/key modes bypass this helper and remain available for + // deliberate recovery. const assertAgentRouteHasTui = async (candidateRoute: typeof route) => { - discovery.invalidate(); - const freshOccupant = (await discovery.scan(true)).find((entry) => - candidateRoute.surface_uuid - ? normalizedUuid(entry.surface_uuid) === - normalizedUuid(candidateRoute.surface_uuid) - : entry.surface_id === candidateRoute.surface_id, - ); + const freshOccupant = await discovery.scanTarget(candidateRoute); if ( freshOccupant && !freshOccupant.read_error && @@ -9387,13 +9380,10 @@ export function createServer(opts?: CreateServerOptions): McpServer { occ.cli !== expectedCli, ); if (isForeign(cachedOccupant)) { - // Confirm against a FRESH scan before refusing. discovery.scan(false) - // serves a 2s cache that can predate the current occupant; refusing - // on it alone would false-refuse a healthy relay. - discovery.invalidate(); - const freshOccupant = (await discovery.scan(true)).find( - (entry) => entry.surface_id === route.surface_id, - ); + // Confirm against another target-scoped fresh read before refusing; + // one parse alone can be transient, while a fleet-wide scan would + // couple this route to unrelated pane churn. + const freshOccupant = await discovery.scanTarget(route); if (isForeign(freshOccupant)) { throw new Error( `Agent "${args.agent_id}" (${expectedCli}) no longer occupies ` + diff --git a/tests/server-agent-tools.test.ts b/tests/server-agent-tools.test.ts index fa9716b4..757c38c1 100644 --- a/tests/server-agent-tools.test.ts +++ b/tests/server-agent-tools.test.ts @@ -7265,6 +7265,79 @@ codex> expect(routeClient.client.send).not.toHaveBeenCalled(); }); + it("send_to ignores unrelated surface churn while the target agent stays healthy", async () => { + const targetUuid = "11111111-2222-4333-8444-555555555555"; + const routeClient = makeUuidRouteClient([ + { + ref: "surface:agent", + id: targetUuid, + workspace_ref: "workspace:1", + }, + { + ref: "surface:other", + id: "aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee", + workspace_ref: "workspace:1", + }, + ]); + routeClient.setScreenText( + "OpenAI Codex\nModel: gpt-5.5\nWorking (1s - esc to interrupt)", + ); + const record = makeServerAgentRecord({ + agent_id: "healthy-target-during-unrelated-churn", + surface_id: "surface:agent", + surface_uuid: targetUuid, + workspace_id: "workspace:1", + state: "ready", + repo: "cmuxlayer", + cli: "codex", + }); + const server = await createUuidRouteServer(routeClient, record); + const engine = testLifecycleEngine(server) as any; + const originalResolveAgentIoRoute = + engine.resolveAgentIoRoute.bind(engine); + let resolveCount = 0; + vi.spyOn(engine, "resolveAgentIoRoute").mockImplementation( + async (agentId: string) => { + const route = await originalResolveAgentIoRoute(agentId); + resolveCount += 1; + if (resolveCount === 1) { + moveUuidRouteAfterNextSurfaceSnapshot(routeClient, [ + { + ref: "surface:agent", + id: targetUuid, + workspace_ref: "workspace:1", + }, + { + ref: "surface:replacement", + id: "99999999-8888-4777-8666-555555555555", + workspace_ref: "workspace:1", + }, + ]); + } + return route; + }, + ); + routeClient.client.send.mockClear(); + routeClient.sendCalls.length = 0; + + const result = await registeredTestTool(server, "send_to").handler( + { + agent_id: record.agent_id, + text: "deliver despite foreign pane churn", + press_enter: false, + }, + {}, + ); + + expect(result.isError).not.toBe(true); + expect(routeClient.sendCalls).toEqual([ + { + surface: "surface:agent", + text: "deliver despite foreign pane churn", + }, + ]); + }); + 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";