Fix App Control shell hijacking chat CLI terminal routing

[cursor]

Bug and impact

App Control shell steals chat CLI terminal routing (significant breakage)

When App Control is launched from a chat, it creates a toolType: "shell" PTY with the parent chatSessionId (for sidebar nesting). That PTY was registered in activeTerminalByChatSession, overwriting the agent CLI binding. Subsequent activeForChat, reattachChatCli, and resolveTerminalId calls could target the App Control / dev-server shell instead of the Claude/Codex/Cursor CLI.

Concrete trigger: Open a chat with a live agent CLI, launch App Control from that chat, then use "Use ADE terminal" (Codex) or any path that calls activeForChat + terminal.write — the resume command is injected into the Electron/npm shell, not the agent CLI. Ctrl+C and signals can kill the controlled app.

Root cause

activeTerminalByChatSession treated every PTY with a chatSessionId equally. App Control intentionally sets chatSessionId for UI nesting, but must not become the canonical chat-CLI routing target.

Fix

• Partition live PTYs into chat-CLI (claude-chat, codex-chat, etc.) vs auxiliary (shell, App Control).
• Only chat-CLI sessions update activeTerminalByChatSession.
• activeForChat / reattachChatCli prefer chat-CLI; auxiliary shells are a fallback for log/reveal flows.
• resolveTerminalId (read/write/preview) routes only to auxiliary terminals.

Validation

• vitest run src/main/services/pty/ptyService.test.ts -t "chat terminal contract" (22 tests)
• New regression: prefers chat CLI over a newer App Control shell with the same chatSessionId

Does not overlap open draft PRs #363–#367 (sync/CRR/orchestration/binding fixes).

  

  Open in ADE  ·  cursor/critical-correctness-bugs-8d53 branch  ·  PR #371

Greptile Summary

This PR fixes a routing conflict where App Control shells (with toolType: "shell") were overwriting the active chat-CLI entry in activeTerminalByChatSession, causing agent resume commands to be injected into dev-server PTYs instead of the Claude/Codex CLI.

• Introduces a second map, activeAuxiliaryTerminalByChatSession, and a set of predicate helpers (isChatCliRoutingEntry, isAuxiliaryRoutingEntry) to partition PTYs by role: only claude-chat, codex-chat, cursor, opencode-chat, and droid-chat sessions own the chat-CLI route, while shells and App Control entries own the auxiliary route.
• resolveTerminalId (used by readTerminal, writeTerminal, signalTerminal) now targets only the auxiliary map, so tool-call writes to chatSessionId always land in the App Control shell rather than the agent CLI; activeForChat and reattachChatCli preference chat-CLI and fall back to auxiliary only for display/log flows.
• Two regression tests are added covering the preference ordering and the per-operation routing split.

Confidence Score: 5/5

The routing split is well-scoped and the two-map invariant is maintained at every creation, disposal, and lookup site.

All write/promote paths use toolTypeHint from the in-memory PTY entry rather than the session-service field, avoiding the field-mismatch pitfall. Disposal handlers correctly guard on map-ownership before evicting, and activeEntryFromMap self-heals stale entries. The regression tests cover both the preference ordering and the per-operation routing split.

No files require special attention.

Important Files Changed

Filename	Overview
apps/desktop/src/main/services/pty/ptyService.ts	Core routing split: adds activeAuxiliaryTerminalByChatSession, isChatCliRoutingEntry/isAuxiliaryRoutingEntry predicates, and partitioned promote/lookup helpers; rewrites resolveTerminalId, activeForChat, and reattachChatCli fast path to use the new two-map scheme.
apps/desktop/src/main/services/pty/ptyService.test.ts	Adds two new regression tests covering the chat-CLI preference ordering and the per-operation routing split for write/signal.
docs/features/terminals-and-sessions/pty-and-processes.md	Documentation updated to describe the chat-CLI vs auxiliary PTY partitioning and the routing rules for activeForChat, reattachChatCli, and terminal.read/write/signal.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant ptyService
    participant chatCliMap as activeTerminalByChatSession
    participant auxMap as activeAuxiliaryTerminalByChatSession
    participant AppCtrl as App Control PTY (shell)
    participant ChatCLI as Chat CLI PTY (claude-chat)

    Note over ptyService: PTY Creation - partitioned routing
    Caller->>ptyService: "create({ toolType: claude-chat, chatSessionId })"
    ptyService->>chatCliMap: set(chatSessionId, chatCli.sessionId)
    Caller->>ptyService: "create({ toolType: shell, chatSessionId })"
    ptyService->>auxMap: set(chatSessionId, appCtrl.sessionId)

    Note over ptyService: activeForChat - prefers chat-CLI
    Caller->>ptyService: "activeForChat({ chatSessionId })"
    ptyService->>chatCliMap: activeChatCliEntryFor(chatSessionId)
    ptyService-->>Caller: ChatTerminalSession (chatCli)

    Note over ptyService: resolveTerminalId - auxiliary only
    Caller->>ptyService: "writeTerminal({ chatSessionId, data })"
    ptyService->>auxMap: activeAuxiliaryEntryFor(chatSessionId)
    ptyService->>AppCtrl: pty.write(data)
    Note over ChatCLI: chatPty.write NOT called
    ptyService-->>Caller: ok: true

Loading

_{Reviews (4): Last reviewed commit: "ship: align action audit with main" | Re-trigger Greptile}

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
ade	Ignored	Preview	May 28, 2026 8:24am

[arul28]

@copilot review but do not make fixes

[Copilot]

@copilot review but do not make fixes

Reviewed the latest commit set for correctness/regression risk only, and I did not find any actionable issues. I did not apply any fixes. I also attempted targeted validation (npm --prefix apps/desktop run test -- src/main/services/pty/ptyService.test.ts -t "chat terminal contract"), but this sandbox is missing vitest in PATH (sh: 1: vitest: not found).

[cursor]

PR Review

Scope: 5 file(s), +263 / −43
Verdict: Minor issues

This PR splits chat-scoped PTY routing into separate chat-CLI and auxiliary active maps so App Control shells no longer hijack activeForChat / reattachChatCli, while terminal.read / write / signal keep targeting auxiliary shells. It also wraps ade/actions/call in operation audit bookkeeping. The routing change is well covered by new tests; the audit wrapper has one accuracy gap.

---

🐛 Functionality

[Medium] Failed run_ade_action results can be audited as succeeded

File: apps/ade-cli/src/adeRpcServer.ts:L5262-L5268
Issue: callActionWithAudit always calls operationService.finish with status: "succeeded" when callAction returns without throwing. run_ade_action (and other tools) often return domain failures inside the payload instead of throwing, so operations can be recorded as successful even when the invoked action failed.
Repro: Call ade/actions/call with name: "run_ade_action" for a domain action that returns { ok: false, error: "..." } (or similar) without throwing; observe operationService.finish gets status: "succeeded" and resultStatus: "success".
Fix: Before finishing, inspect the return value (for run_ade_action, check result / nested ok flags) and use status: "failed" with an error patch when the payload indicates failure; only use "succeeded" when the action actually succeeded.

---

Notes

• The dual-map routing in ptyService.ts matches the updated pty-and-processes.md contract and the new regression tests (chat CLI preferred for activeForChat / reattachChatCli, auxiliary for read / write / signal) look like the right fix for the hijacking bug.
• Callers that used activeForChat to surface auxiliary output (e.g. dev-server logs) while a chat CLI is also live may now correctly get the chat CLI; if that UX matters, a follow-up API or flag is cleaner than overloading activeForChat.

  

_{Sent by Cursor Automation: BUGBOT in Versic}

[arul28]

@copilot review but do not make fixes

[capy-ai]

Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews.

[arul28]

@copilot review but do not make fixes

[arul28]

@copilot review but do not make fixes