Skip to content

Commit c04e0b3

Browse files
cursor[bot]cursoragentarul28
authored
Fix App Control shell hijacking chat CLI terminal routing (#371)
* Fix App Control shell hijacking chat CLI terminal routing App Control launches a shell PTY with the parent chatSessionId for sidebar nesting, but that overwrote activeTerminalByChatSession and caused activeForChat/reattachChatCli to target the Electron dev shell instead of the agent CLI — sending resume commands and signals to the wrong process. Partition routing: chat-CLI sessions own the active map; auxiliary shells are resolved separately for read/write/list vs activeForChat/reattach. Co-authored-by: Arul Sharma <arul28@users.noreply.github.com> * fix: preserve chat terminal routing * ship: iteration 1 - address review feedback * ship: align action audit with main --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Arul Sharma <arul28@users.noreply.github.com> Co-authored-by: Arul Sharma <31745423+arul28@users.noreply.github.com>
1 parent b8ec8aa commit c04e0b3

3 files changed

Lines changed: 219 additions & 40 deletions

File tree

apps/desktop/src/main/services/pty/ptyService.test.ts

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4557,6 +4557,87 @@ describe("ptyService", () => {
45574557
expect(result.ptyId).toBe(created.ptyId);
45584558
});
45594559

4560+
it("prefers chat CLI over a newer App Control shell with the same chatSessionId", async () => {
4561+
const { service } = createChatHarness();
4562+
const chatCli = await service.create({
4563+
sessionId: "chat-app-control",
4564+
allowNewSessionId: true,
4565+
laneId: "lane-1",
4566+
title: "Claude Chat",
4567+
cols: 80,
4568+
rows: 24,
4569+
chatSessionId: "chat-app-control",
4570+
tracked: true,
4571+
toolType: "claude-chat",
4572+
startupCommand: "claude --resume target",
4573+
});
4574+
const appControlShell = await service.create({
4575+
laneId: "lane-1",
4576+
title: "App Control",
4577+
cols: 80,
4578+
rows: 24,
4579+
chatSessionId: "chat-app-control",
4580+
tracked: true,
4581+
toolType: "shell",
4582+
startupCommand: "npm run dev",
4583+
});
4584+
4585+
const active = service.activeForChat({ chatSessionId: "chat-app-control" });
4586+
expect(active?.terminalId).toBe(chatCli.sessionId);
4587+
expect(active?.terminalId).not.toBe(appControlShell.sessionId);
4588+
4589+
const reattached = await service.reattachChatCli({ chatSessionId: "chat-app-control" });
4590+
expect(reattached.relaunched).toBe(false);
4591+
expect(reattached.terminalId).toBe(chatCli.sessionId);
4592+
expect(reattached.ptyId).toBe(chatCli.ptyId);
4593+
});
4594+
4595+
it("routes terminal operations to the App Control shell without making it the active chat CLI", async () => {
4596+
const { service, loadPty, sessionService } = createChatHarness();
4597+
const chatPty = createMockPty();
4598+
const appControlPty = createMockPty();
4599+
const spawn = vi.fn()
4600+
.mockReturnValueOnce(chatPty)
4601+
.mockReturnValueOnce(appControlPty);
4602+
loadPty.mockReturnValue({ spawn });
4603+
4604+
const chatCli = await service.create({
4605+
sessionId: "chat-app-control-routing",
4606+
allowNewSessionId: true,
4607+
laneId: "lane-1",
4608+
title: "Claude Chat",
4609+
cols: 80,
4610+
rows: 24,
4611+
chatSessionId: "chat-app-control-routing",
4612+
tracked: true,
4613+
toolType: "claude-chat",
4614+
startupCommand: "claude --resume target",
4615+
});
4616+
const appControlShell = await service.create({
4617+
laneId: "lane-1",
4618+
title: "App Control",
4619+
cols: 80,
4620+
rows: 24,
4621+
chatSessionId: "chat-app-control-routing",
4622+
tracked: true,
4623+
toolType: "shell",
4624+
startupCommand: "npm run dev",
4625+
});
4626+
(chatPty.write as ReturnType<typeof vi.fn>).mockClear();
4627+
(appControlPty.write as ReturnType<typeof vi.fn>).mockClear();
4628+
sessionService.readTranscriptTail.mockResolvedValueOnce("app control output");
4629+
4630+
const read = await service.readTerminal({ chatSessionId: "chat-app-control-routing" });
4631+
await service.writeTerminal({ chatSessionId: "chat-app-control-routing", data: "q\n" });
4632+
service.signalTerminal({ chatSessionId: "chat-app-control-routing", signal: "SIGINT" });
4633+
4634+
expect(read.terminalId).toBe(appControlShell.sessionId);
4635+
expect(appControlPty.write).toHaveBeenNthCalledWith(1, "q\n");
4636+
expect(appControlPty.write).toHaveBeenNthCalledWith(2, "\x03");
4637+
expect(chatPty.write).not.toHaveBeenCalled();
4638+
expect(service.activeForChat({ chatSessionId: "chat-app-control-routing" })?.terminalId).toBe(chatCli.sessionId);
4639+
});
4640+
45604641
it("relaunches a new PTY for a disposed chat-CLI session", async () => {
45614642
const { service, loadPty, sessionStore } = createChatHarness();
45624643
const created = await service.create({

apps/desktop/src/main/services/pty/ptyService.ts

Lines changed: 130 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -929,6 +929,7 @@ export function createPtyService({
929929
const exitListeners = new Set<PtyExitListener>();
930930
const terminalChatSessions = new Map<string, string>();
931931
const activeTerminalByChatSession = new Map<string, string>();
932+
const activeAuxiliaryTerminalByChatSession = new Map<string, string>();
932933
const missingResumeTargetBackfillFailures = new Map<string, { toolType: TerminalToolType | null; checkedAtMs: number }>();
933934
const claudeTitleCaptureKeys = new Set<string>();
934935
const resumeRuntimeFlights = new Map<string, Promise<PtyCreateResult>>();
@@ -2438,20 +2439,30 @@ export function createPtyService({
24382439
const finalRuntimeState = runtimeFromStatus(status);
24392440
setRuntimeState(entry.sessionId, finalRuntimeState, { touch: false });
24402441
runtimeStates.delete(entry.sessionId);
2441-
if (entry.chatSessionId && activeTerminalByChatSession.get(entry.chatSessionId) === entry.sessionId) {
2442-
const replacement = Array.from(ptys.values())
2443-
.filter((candidate) => (
2444-
candidate.sessionId !== entry.sessionId
2445-
&& !candidate.disposed
2446-
&& candidate.chatSessionId === entry.chatSessionId
2447-
))
2448-
.sort((a, b) => b.createdAt - a.createdAt)[0] ?? null;
2442+
if (
2443+
entry.chatSessionId
2444+
&& isChatCliRoutingEntry(entry)
2445+
&& activeTerminalByChatSession.get(entry.chatSessionId) === entry.sessionId
2446+
) {
2447+
const replacement = liveChatCliEntriesFor(entry.chatSessionId)[0] ?? null;
24492448
if (replacement) {
24502449
activeTerminalByChatSession.set(entry.chatSessionId, replacement.sessionId);
24512450
} else {
24522451
activeTerminalByChatSession.delete(entry.chatSessionId);
24532452
}
24542453
}
2454+
if (
2455+
entry.chatSessionId
2456+
&& isAuxiliaryRoutingEntry(entry)
2457+
&& activeAuxiliaryTerminalByChatSession.get(entry.chatSessionId) === entry.sessionId
2458+
) {
2459+
const replacement = liveAuxiliaryEntriesFor(entry.chatSessionId)[0] ?? null;
2460+
if (replacement) {
2461+
activeAuxiliaryTerminalByChatSession.set(entry.chatSessionId, replacement.sessionId);
2462+
} else {
2463+
activeAuxiliaryTerminalByChatSession.delete(entry.chatSessionId);
2464+
}
2465+
}
24552466
try {
24562467
onSessionRuntimeSignal?.({
24572468
laneId: entry.laneId,
@@ -2692,6 +2703,89 @@ export function createPtyService({
26922703
Array.from(ptys.entries()).find(([, entry]) => entry.sessionId === sessionId && !entry.disposed) ?? null
26932704
);
26942705

2706+
const isChatCliRoutingEntry = (entry: PtyEntry): boolean =>
2707+
isPersistedChatToolType(entry.toolTypeHint);
2708+
2709+
const isAuxiliaryRoutingEntry = (entry: PtyEntry): boolean =>
2710+
!isChatCliRoutingEntry(entry);
2711+
2712+
const liveChatCliEntriesFor = (chatSessionId: string): PtyEntry[] =>
2713+
Array.from(ptys.values())
2714+
.filter((entry) => (
2715+
entry.chatSessionId === chatSessionId
2716+
&& !entry.disposed
2717+
&& isChatCliRoutingEntry(entry)
2718+
))
2719+
.sort((a, b) => b.createdAt - a.createdAt);
2720+
2721+
const liveAuxiliaryEntriesFor = (chatSessionId: string): PtyEntry[] =>
2722+
Array.from(ptys.values())
2723+
.filter((entry) => (
2724+
entry.chatSessionId === chatSessionId
2725+
&& !entry.disposed
2726+
&& isAuxiliaryRoutingEntry(entry)
2727+
))
2728+
.sort((a, b) => b.createdAt - a.createdAt);
2729+
2730+
const activeEntryFromMap = (
2731+
map: Map<string, string>,
2732+
chatSessionId: string,
2733+
predicate: (entry: PtyEntry) => boolean,
2734+
): PtyEntry | null => {
2735+
const sessionId = map.get(chatSessionId) ?? null;
2736+
if (!sessionId) return null;
2737+
const live = liveEntryBySessionId(sessionId);
2738+
if (live && live[1].chatSessionId === chatSessionId && predicate(live[1])) return live[1];
2739+
map.delete(chatSessionId);
2740+
return null;
2741+
};
2742+
2743+
const activeChatCliEntryFor = (chatSessionId: string): PtyEntry | null => {
2744+
const active = activeEntryFromMap(activeTerminalByChatSession, chatSessionId, isChatCliRoutingEntry);
2745+
if (active) return active;
2746+
const replacement = liveChatCliEntriesFor(chatSessionId)[0] ?? null;
2747+
if (replacement) activeTerminalByChatSession.set(chatSessionId, replacement.sessionId);
2748+
return replacement;
2749+
};
2750+
2751+
const activeAuxiliaryEntryFor = (chatSessionId: string): PtyEntry | null => {
2752+
const active = activeEntryFromMap(activeAuxiliaryTerminalByChatSession, chatSessionId, isAuxiliaryRoutingEntry);
2753+
if (active) return active;
2754+
const replacement = liveAuxiliaryEntriesFor(chatSessionId)[0] ?? null;
2755+
if (replacement) activeAuxiliaryTerminalByChatSession.set(chatSessionId, replacement.sessionId);
2756+
return replacement;
2757+
};
2758+
2759+
const promoteActiveChatCliTerminal = (
2760+
chatSessionId: string,
2761+
sessionId: string,
2762+
toolType: TerminalToolType | null,
2763+
): void => {
2764+
if (!isPersistedChatToolType(toolType)) return;
2765+
activeTerminalByChatSession.set(chatSessionId, sessionId);
2766+
};
2767+
2768+
const promoteActiveAuxiliaryTerminal = (
2769+
chatSessionId: string,
2770+
sessionId: string,
2771+
toolType: TerminalToolType | null,
2772+
): void => {
2773+
if (isPersistedChatToolType(toolType)) return;
2774+
activeAuxiliaryTerminalByChatSession.set(chatSessionId, sessionId);
2775+
};
2776+
2777+
const promoteActiveChatTerminal = (
2778+
chatSessionId: string,
2779+
sessionId: string,
2780+
toolType: TerminalToolType | null,
2781+
): void => {
2782+
if (isPersistedChatToolType(toolType)) {
2783+
promoteActiveChatCliTerminal(chatSessionId, sessionId, toolType);
2784+
} else {
2785+
promoteActiveAuxiliaryTerminal(chatSessionId, sessionId, toolType);
2786+
}
2787+
};
2788+
26952789
const codexReadyRegion = (text: string): string => {
26962790
const lastPrompt = text.lastIndexOf("›");
26972791
if (lastPrompt < 0) return text;
@@ -2858,8 +2952,15 @@ export function createPtyService({
28582952
?? live?.[1].chatSessionId
28592953
?? summary.chatSessionId
28602954
?? null;
2861-
const activeTerminalId = chatSessionId ? activeTerminalByChatSession.get(chatSessionId) ?? null : null;
28622955
const fallbackStatus = live ? "running" : summary.status;
2956+
let active = false;
2957+
if (chatSessionId) {
2958+
if (isPersistedChatToolType(summary.toolType)) {
2959+
active = activeChatCliEntryFor(chatSessionId)?.sessionId === summary.id;
2960+
} else {
2961+
active = activeAuxiliaryEntryFor(chatSessionId)?.sessionId === summary.id;
2962+
}
2963+
}
28632964
return {
28642965
terminalId: summary.id,
28652966
ptyId: live?.[0] ?? summary.ptyId ?? null,
@@ -2871,7 +2972,7 @@ export function createPtyService({
28712972
goal: summary.goal,
28722973
status: fallbackStatus,
28732974
runtimeState: computeRuntimeState(summary.id, fallbackStatus),
2874-
active: Boolean(activeTerminalId && activeTerminalId === summary.id),
2975+
active,
28752976
startedAt: summary.startedAt,
28762977
endedAt: live ? null : summary.endedAt,
28772978
exitCode: live ? null : summary.exitCode,
@@ -2891,17 +2992,8 @@ export function createPtyService({
28912992
if (terminalId) return terminalId;
28922993
const chatSessionId = cleanOptionalId(args.chatSessionId);
28932994
if (!chatSessionId) return null;
2894-
const activeTerminalId = activeTerminalByChatSession.get(chatSessionId) ?? null;
2895-
if (activeTerminalId && liveEntryBySessionId(activeTerminalId)) return activeTerminalId;
2896-
const replacement = Array.from(ptys.values())
2897-
.filter((entry) => entry.chatSessionId === chatSessionId && !entry.disposed)
2898-
.sort((a, b) => b.createdAt - a.createdAt)[0] ?? null;
2899-
if (replacement) {
2900-
activeTerminalByChatSession.set(chatSessionId, replacement.sessionId);
2901-
return replacement.sessionId;
2902-
}
2903-
activeTerminalByChatSession.delete(chatSessionId);
2904-
return null;
2995+
// Auxiliary terminals (shell, App Control, etc.) — never route chat-CLI rows.
2996+
return activeAuxiliaryEntryFor(chatSessionId)?.sessionId ?? null;
29052997
};
29062998

29072999
const service = {
@@ -2963,7 +3055,7 @@ export function createPtyService({
29633055
if (chatSessionId) {
29643056
attachedEntry.chatSessionId = chatSessionId;
29653057
terminalChatSessions.set(existingSession.id, chatSessionId);
2966-
activeTerminalByChatSession.set(chatSessionId, existingSession.id);
3058+
promoteActiveChatTerminal(chatSessionId, existingSession.id, attachedEntry.toolTypeHint);
29673059
if (existingSession.chatSessionId !== chatSessionId) {
29683060
try { sessionService.setChatSessionId(existingSession.id, chatSessionId); } catch {}
29693061
}
@@ -3262,7 +3354,7 @@ export function createPtyService({
32623354
ptys.set(ptyId, entry);
32633355
if (chatSessionId) {
32643356
terminalChatSessions.set(sessionId, chatSessionId);
3265-
activeTerminalByChatSession.set(chatSessionId, sessionId);
3357+
promoteActiveChatTerminal(chatSessionId, sessionId, toolTypeHint);
32663358
if (existingSession && existingSession.chatSessionId !== chatSessionId) {
32673359
try { sessionService.setChatSessionId(sessionId, chatSessionId); } catch {}
32683360
}
@@ -3804,20 +3896,17 @@ export function createPtyService({
38043896
activeForChat(args: ChatTerminalActiveForChatArgs): ChatTerminalSession | null {
38053897
const chatSessionId = cleanOptionalId(args.chatSessionId);
38063898
if (!chatSessionId) return null;
3807-
const terminalId = activeTerminalByChatSession.get(chatSessionId) ?? null;
3808-
if (terminalId && liveEntryBySessionId(terminalId)) {
3809-
const session = sessionService.get(terminalId);
3810-
return session ? terminalSessionFromSummary(session) : null;
3811-
}
3812-
const replacement = Array.from(ptys.values())
3813-
.filter((entry) => entry.chatSessionId === chatSessionId && !entry.disposed)
3814-
.sort((a, b) => b.createdAt - a.createdAt)[0] ?? null;
3815-
if (!replacement) {
3816-
activeTerminalByChatSession.delete(chatSessionId);
3817-
return null;
3899+
const chatCli = activeChatCliEntryFor(chatSessionId);
3900+
if (chatCli) {
3901+
const session = sessionService.get(chatCli.sessionId);
3902+
if (session) {
3903+
promoteActiveChatCliTerminal(chatSessionId, chatCli.sessionId, session.toolType);
3904+
return terminalSessionFromSummary(session);
3905+
}
38183906
}
3819-
activeTerminalByChatSession.set(chatSessionId, replacement.sessionId);
3820-
const session = sessionService.get(replacement.sessionId);
3907+
const auxiliary = activeAuxiliaryEntryFor(chatSessionId);
3908+
if (!auxiliary) return null;
3909+
const session = sessionService.get(auxiliary.sessionId);
38213910
return session ? terminalSessionFromSummary(session) : null;
38223911
},
38233912

@@ -3827,12 +3916,13 @@ export function createPtyService({
38273916

38283917
// Fast path: an existing live PTY is already bound. Skip the dedup map to
38293918
// keep the no-op cost low.
3830-
const activeTerminalId = activeTerminalByChatSession.get(chatSessionId) ?? null;
3831-
if (activeTerminalId) {
3832-
const liveActive = liveEntryBySessionId(activeTerminalId);
3919+
const liveChatCli = activeChatCliEntryFor(chatSessionId);
3920+
if (liveChatCli) {
3921+
const liveActive = liveEntryBySessionId(liveChatCli.sessionId);
38333922
if (liveActive) {
3923+
promoteActiveChatCliTerminal(chatSessionId, liveChatCli.sessionId, liveChatCli.toolTypeHint);
38343924
return {
3835-
terminalId: activeTerminalId,
3925+
terminalId: liveChatCli.sessionId,
38363926
ptyId: liveActive[0],
38373927
pid: liveActive[1].pty.pid ?? null,
38383928
relaunched: false,

docs/features/terminals-and-sessions/pty-and-processes.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,14 @@ user wants to send a message". It only accepts tracked chat-typed
316316
sessions (`isPersistedChatToolType(toolType)`) — non-chat sessions and
317317
untracked rows are rejected with a clear error.
318318

319+
Chat-scoped PTYs are partitioned by tool type. Persisted chat tool
320+
types (`claude-chat`, `codex-chat`, `cursor`, `opencode-chat`,
321+
`droid-chat`) are the only sessions allowed to own the chat-CLI active
322+
route used by `activeForChat` and `reattachChatCli`. Auxiliary PTYs
323+
such as App Control or plain shells may carry the same `chatSessionId`
324+
so they nest under the parent chat, but they are tracked in a separate
325+
auxiliary active route for `terminal.read` / `write` / `signal`.
326+
319327
Behaviour:
320328

321329
- Fast path: if a live PTY is already bound to the chat session, the

0 commit comments

Comments
 (0)