Skip to content

fix: refuse agent delivery to exited panes #389

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
EtanHey merged 2 commits into from
Aug 10, 2026
Merged

fix: refuse agent delivery to exited panes #389

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f1d1e09
fix: refuse agent delivery to exited panes
EtanHey Aug 10, 2026
f702442
fix: scope shell guard to target pane
EtanHey Aug 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
151 changes: 104 additions & 47 deletions src/agent-discovery.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type { AgentState, CliType } from "./agent-types.js";
import type {
CmuxReadScreenResult,
CmuxSurface,
ParsedControlPlaneState,
ParsedScreenStatus,
} from "./types.js";

Expand All @@ -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;
Expand Down Expand Up @@ -96,6 +98,107 @@ export class AgentDiscovery {
return this.deps.observerIdProvider?.()?.trim() || null;
}

private async scanSurface(surface: CmuxSurface): Promise<DiscoveredAgent> {
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<DiscoveredAgent | null> {
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())

@macroscopeapp macroscopeapp Bot Aug 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High src/agent-discovery.ts:178

scanTarget validates surface identity by surface.ref when target.surface_uuid is absent, but ref is mutable and can be recycled to a different UUID while keeping the same ref and workspace. If the surface is rebound during scanSurface, the validation passes and returns stale screen evidence for the old occupant — which can route keystrokes to the new occupant. Compare the initial and completed stable surface.id values whenever either side provides a UUID, not just when target.surface_uuid is present.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent: 
In file @src/agent-discovery.ts around line 178:

`scanTarget` validates surface identity by `surface.ref` when `target.surface_uuid` is absent, but `ref` is mutable and can be recycled to a different UUID while keeping the same ref and workspace. If the surface is rebound during `scanSurface`, the validation passes and returns stale screen evidence for the old occupant — which can route keystrokes to the new occupant. Compare the initial and completed stable `surface.id` values whenever either side provides a UUID, not just when `target.surface_uuid` is present.

.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<DiscoveredAgent[]> {
const observerScoped =
typeof this.deps.observerIdProvider === "function";
Expand All @@ -112,53 +215,7 @@ export class AgentDiscovery {
(surface) => surface.type === "terminal",
);
const result = await Promise.all(
surfaces.map(async (surface): Promise<DiscoveredAgent> => {
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,
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",
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();
Expand Down
45 changes: 33 additions & 12 deletions src/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9338,16 +9338,39 @@ 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. 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) => {

@macroscopeapp macroscopeapp Bot Aug 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High src/server.ts:9349

The final assertAgentRouteHasTui(route) check is separated from the first text mutation by several awaited operations (resolveAgentIoRoute, assertDeliveryRouteCurrent, and assertDeliveryTargetIsSafe inside deliverInputChunks). If the agent exits to a bare shell after the TUI check but before deliverInputChunks sends text — while the same surface UUID/ref remains bound — assertDeliveryTargetIsSafe reads the screen but does not reject control_state === "shell", so deliverInputChunks types the fleet message into the shell. This is the exact exited-pane command-execution race the assertAgentRouteHasTui guard was added to prevent. Consider making assertDeliveryTargetIsSafe reject control_state === "shell" so the shell-exit race is covered at the final mutation boundary, or moving the TUI check immediately before the first client.send/client.pasteText call.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent: 
In file @src/server.ts around line 9349:

The final `assertAgentRouteHasTui(route)` check is separated from the first text mutation by several awaited operations (`resolveAgentIoRoute`, `assertDeliveryRouteCurrent`, and `assertDeliveryTargetIsSafe` inside `deliverInputChunks`). If the agent exits to a bare shell after the TUI check but before `deliverInputChunks` sends text — while the same surface UUID/ref remains bound — `assertDeliveryTargetIsSafe` reads the screen but does not reject `control_state === "shell"`, so `deliverInputChunks` types the fleet message into the shell. This is the exact exited-pane command-execution race the `assertAgentRouteHasTui` guard was added to prevent. Consider making `assertDeliveryTargetIsSafe` reject `control_state === "shell"` so the shell-exit race is covered at the final mutation boundary, or moving the TUI check immediately before the first `client.send`/`client.pasteText` call.

const freshOccupant = await discovery.scanTarget(candidateRoute);
if (
freshOccupant &&
!freshOccupant.read_error &&
freshOccupant.control_state === "shell"
Comment on lines +9350 to +9353

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Detect shell prompts beneath stale TUI banners

When an exited CLI leaves recognizable TUI text in the last 30 screen lines—for example, Claude Code followed by the restored shell prompt—this condition does not fire. detectAgentType scans the entire buffer, and inferControlState classifies an idle known agent as ready before considering the trailing shell prompt, so the routed text can still execute as a shell command. Determine shell fallback from current trailing prompt evidence rather than requiring the whole-screen parse to equal shell.

Useful? React with 👍 / 👎.

) {
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 &&
Expand All @@ -9357,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 ` +
Expand Down Expand Up @@ -9407,6 +9427,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);
Comment on lines 9429 to +9430

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Recheck the TUI after acquiring the surface write lock

This final shell check still occurs before withSurfaceWrite acquires the per-surface lock. If another routed write is finishing concurrently, this call can observe the agent TUI, the first write can then submit an exit-triggering command and release the lock, and this call can acquire the lock and type into the resulting shell; the only check inside the critical section validates that the registry route is unchanged. Repeat the TUI check inside the locked callback immediately before mutation so concurrent cmuxlayer deliveries cannot reopen the shell-injection path.

Useful? React with 👍 / 👎.

const deliveryRoute = route;
const assertDeliveryRouteCurrent = async (): Promise<void> => {
const current = await engine.resolveAgentIoRoute(args.agent_id);
Expand Down
Loading
Loading