From 2dd46d2c3df3889774ce40073503fe26e6863182 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Tue, 30 Jun 2026 17:51:27 -0400 Subject: [PATCH] Fix chat action scoping and ADE CLI guidance --- apps/ade-cli/src/adeRpcServer.test.ts | 66 +++++++++++++++++++ apps/ade-cli/src/adeRpcServer.ts | 30 +++++++++ .../ade-cli-control-plane/SKILL.md | 6 ++ docs/ARCHITECTURE.md | 2 +- docs/features/agents/README.md | 1 + docs/features/chat/README.md | 7 +- 6 files changed, 109 insertions(+), 3 deletions(-) diff --git a/apps/ade-cli/src/adeRpcServer.test.ts b/apps/ade-cli/src/adeRpcServer.test.ts index 806bace32..de90ebd21 100644 --- a/apps/ade-cli/src/adeRpcServer.test.ts +++ b/apps/ade-cli/src/adeRpcServer.test.ts @@ -2774,6 +2774,72 @@ describe("adeRpcServer", () => { data: "continue\n", chatSessionId: "chat-1", }); + + const deniedChatRead = await callTool(handler, "run_ade_action", { + domain: "chat", + action: "readTranscript", + args: { sessionId: "chat-2", limit: 10 }, + }); + expect(deniedChatRead.isError).toBe(true); + expect(fixture.runtime.agentChatService.getChatTranscript).not.toHaveBeenCalled(); + + const deniedChatSend = await callTool(handler, "run_ade_action", { + domain: "chat", + action: "sendMessage", + args: { sessionId: "chat-2", text: "cross-chat write" }, + }); + expect(deniedChatSend.isError).toBe(true); + expect(fixture.runtime.agentChatService.sendMessage).not.toHaveBeenCalled(); + + const ownChatRead = await callTool(handler, "run_ade_action", { + domain: "chat", + action: "readTranscript", + args: { limit: 10 }, + }); + expect(ownChatRead?.isError).toBeUndefined(); + expect(fixture.runtime.agentChatService.getChatTranscript).toHaveBeenCalledWith({ + sessionId: "chat-1", + limit: 10, + }); + + const ownChatSend = await callTool(handler, "run_ade_action", { + domain: "chat", + action: "sendMessage", + args: { text: "own-chat write" }, + }); + expect(ownChatSend?.isError).toBeUndefined(); + expect(fixture.runtime.agentChatService.sendMessage).toHaveBeenCalledWith({ + sessionId: "chat-1", + text: "own-chat write", + }); + }); + + it("keeps explicit chat ADE actions available to unbound external CLI callers", async () => { + const fixture = createRuntime(); + const handler = createAdeRpcRequestHandler({ runtime: fixture.runtime, serverVersion: "test" }); + await initialize(handler, { callerId: "external-cli", role: "external" }); + + const read = await callTool(handler, "run_ade_action", { + domain: "chat", + action: "readTranscript", + args: { sessionId: "chat-2", limit: 10 }, + }); + expect(read?.isError).toBeUndefined(); + expect(fixture.runtime.agentChatService.getChatTranscript).toHaveBeenCalledWith({ + sessionId: "chat-2", + limit: 10, + }); + + const send = await callTool(handler, "run_ade_action", { + domain: "chat", + action: "sendMessage", + args: { sessionId: "chat-2", text: "external write" }, + }); + expect(send?.isError).toBeUndefined(); + expect(fixture.runtime.agentChatService.sendMessage).toHaveBeenCalledWith({ + sessionId: "chat-2", + text: "external write", + }); }); it("invokes review.startRun through ADE actions without dropping unlimited budgets", async () => { diff --git a/apps/ade-cli/src/adeRpcServer.ts b/apps/ade-cli/src/adeRpcServer.ts index d070ab2ee..d3d61902a 100644 --- a/apps/ade-cli/src/adeRpcServer.ts +++ b/apps/ade-cli/src/adeRpcServer.ts @@ -2251,6 +2251,10 @@ function ptyAccessDenied(method: string): never { throw new JsonRpcError(JsonRpcErrorCode.methodNotFound, `Unsupported PTY method: ${method}`); } +function chatAccessDenied(method: string): never { + throw new JsonRpcError(JsonRpcErrorCode.methodNotFound, `Unsupported chat method: ${method}`); +} + function listPtySessionsForAuthorization(runtime: AdeRuntime): TerminalSessionSummary[] { try { const rows = runtime.ptyService.list({}); @@ -2503,6 +2507,26 @@ function scopeTerminalAdeActionArgs( } } +function scopeChatAdeActionArgs( + session: SessionState, + action: string, + chatArgs: Record, +): Record { + const method = `run_ade_action:chat.${action}`; + if (action !== "readTranscript" && action !== "sendMessage") return chatArgs; + + const scopedArgs = { ...chatArgs }; + const callerChatSessionId = asOptionalTrimmedString(session.identity.chatSessionId); + if (session.identity.role === "external" && !callerChatSessionId) return scopedArgs; + + const requestedSessionId = asOptionalTrimmedString(scopedArgs.sessionId); + if (!callerChatSessionId || (requestedSessionId && requestedSessionId !== callerChatSessionId)) { + chatAccessDenied(method); + } + if (!requestedSessionId) scopedArgs.sessionId = callerChatSessionId; + return scopedArgs; +} + async function runCtoOperatorBridgeTool( runtime: AdeRuntime, session: SessionState, @@ -3401,6 +3425,12 @@ async function runTool(args: { action, requireObjectArgsForScopedAdeAction(domain, action, argsList, hasScalarArg, rawObjectArgs), ); + } else if (!callerIsCto && domain === "chat" && (action === "readTranscript" || action === "sendMessage")) { + scopedObjectArgs = scopeChatAdeActionArgs( + session, + action, + requireObjectArgsForScopedAdeAction(domain, action, argsList, hasScalarArg, rawObjectArgs), + ); } if (!scopedResultHandled) { if (argsList) { diff --git a/apps/desktop/resources/agent-skills/ade-cli-control-plane/SKILL.md b/apps/desktop/resources/agent-skills/ade-cli-control-plane/SKILL.md index fefec7c44..5dfe7d3af 100644 --- a/apps/desktop/resources/agent-skills/ade-cli-control-plane/SKILL.md +++ b/apps/desktop/resources/agent-skills/ade-cli-control-plane/SKILL.md @@ -9,6 +9,12 @@ description: Use this skill when an agent needs to inspect or operate ADE itself Use normal shell commands for local repo edits, tests, and Git inspection. Use `ade` when you need ADE state or ADE-owned services: lanes, chats, PR metadata, proof/artifacts, managed terminals, App Control, iOS Simulator, browser, settings, project secrets, usage, updates, or service actions. +Do not route ordinary repo commands through ADE chat-attached terminals. Those +terminals exist so ADE Work chats can expose long-running process logs or let a +user inspect/control a chat-owned shell. In a tracked CLI session, run normal +shell commands through the CLI's own shell/tooling; use `ade terminal ...` only +to inspect or control an existing ADE-owned terminal session. + ## First checks 1. Run `ade doctor --text` when the ADE environment is unclear. diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 12d03a221..2565d2390 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -131,7 +131,7 @@ Product positioning and workflows live in [`docs/PRD.md`](../docs/PRD.md). This **Session identity.** The runtime resolves caller role from ADE context env vars and command flags. Role vocabulary: `cto`, `orchestrator`, `agent`, `external`, `evaluator`. -**Action surface.** First-class command families cover lanes (including `ade lanes link-linear-issue` / `detach-linear-issue` for post-creation Linear issue linking, and `ade lanes create-from-linear` / `batch-create-from-linear` to spin up one or many issue lanes — optionally launching an agent chat with `--start-chat`), git, diffs, files, PRs, runs, shells, chats (including `ade chat create --prompt` for a persistent Work chat followed by an initial `chat.sendMessage`, `ade chat read ` for recent transcript messages, `ade chat create --from-linear-issue `, and `ade chat attach-linear-issue` / `detach-linear-issue` / `linear-issues` for session-scoped issue attachment), agents, CTO, Linear (the write bridge an attached CLI agent uses: `ade linear attach` / `detach` / `issues` / `issue` / `comment` / `set-state` / `assign` / `label`, with `--this-session` resolving the issue id from `$ADE_LINEAR_ISSUE_IDS` so a launched agent needs no Linear token — see [features/linear-integration/README.md](./features/linear-integration/README.md#session-scoped-issue-attachment-and-cli-context-injection)), tests, proof, settings, the iOS Simulator (`ade ios-sim` / `ade ios` / `ade simulator` — see [features/ios-simulator/README.md](./features/ios-simulator/README.md)), the Cursor Cloud bridge (`ade cursor cloud agents | runs | artifacts | repos | models | me` — talks directly to `@cursor/sdk` without going through the ADE runtime endpoint), the App Control bridge for Electron apps (`ade app-control` / `ade app` / `ade electron` — `launch`, `connect`, `stop`, `status`, `screenshot`, `snapshot`, `inspect`, `select`, `click`, `type`, `scroll`, `key`, `targets`, `attach`, `logs`, `terminal write`, `terminal signal` — see [features/computer-use/app-control.md](./features/computer-use/app-control.md)), the chat-scoped terminal (`ade terminal list` / `read` / `write` / `signal` / `active`), and a generic `ade actions run ` escape hatch for every registered ADE service action. The chat action surface includes `chat.createSession`, `chat.sendMessage` (returns an accepted acknowledgement while provider dispatch continues asynchronously), `chat.readTranscript`, and model-catalog actions. The action allow-list adds three domains for these surfaces: `app_control` (every public method on `AppControlService`), `terminal` (`list`, `read`, `write`, `signal`, `activeForChat` against `ptyService`), and named iOS Simulator actions for launch, live view, inspection, input, and Preview Lab workflows. +**Action surface.** First-class command families cover lanes (including `ade lanes link-linear-issue` / `detach-linear-issue` for post-creation Linear issue linking, and `ade lanes create-from-linear` / `batch-create-from-linear` to spin up one or many issue lanes — optionally launching an agent chat with `--start-chat`), git, diffs, files, PRs, runs, shells, chats (including `ade chat create --prompt` for a persistent Work chat followed by an initial `chat.sendMessage`, `ade chat read ` for recent transcript messages, `ade chat create --from-linear-issue `, and `ade chat attach-linear-issue` / `detach-linear-issue` / `linear-issues` for session-scoped issue attachment), agents, CTO, Linear (the write bridge an attached CLI agent uses: `ade linear attach` / `detach` / `issues` / `issue` / `comment` / `set-state` / `assign` / `label`, with `--this-session` resolving the issue id from `$ADE_LINEAR_ISSUE_IDS` so a launched agent needs no Linear token — see [features/linear-integration/README.md](./features/linear-integration/README.md#session-scoped-issue-attachment-and-cli-context-injection)), tests, proof, settings, the iOS Simulator (`ade ios-sim` / `ade ios` / `ade simulator` — see [features/ios-simulator/README.md](./features/ios-simulator/README.md)), the Cursor Cloud bridge (`ade cursor cloud agents | runs | artifacts | repos | models | me` — talks directly to `@cursor/sdk` without going through the ADE runtime endpoint), the App Control bridge for Electron apps (`ade app-control` / `ade app` / `ade electron` — `launch`, `connect`, `stop`, `status`, `screenshot`, `snapshot`, `inspect`, `select`, `click`, `type`, `scroll`, `key`, `targets`, `attach`, `logs`, `terminal write`, `terminal signal` — see [features/computer-use/app-control.md](./features/computer-use/app-control.md)), the chat-scoped terminal (`ade terminal list` / `read` / `write` / `signal` / `active`), and a generic `ade actions run ` escape hatch for every registered ADE service action. The chat action surface includes `chat.createSession`, `chat.sendMessage` (returns an accepted acknowledgement while provider dispatch continues asynchronously), `chat.readTranscript`, and model-catalog actions; session-bound non-CTO callers are restricted to their own chat for `chat.sendMessage` and `chat.readTranscript`. The action allow-list adds three domains for these surfaces: `app_control` (every public method on `AppControlService`), `terminal` (`list`, `read`, `write`, `signal`, `activeForChat` against `ptyService`), and named iOS Simulator actions for launch, live view, inspection, input, and Preview Lab workflows. **Proof subcommands** — `ade proof capture` (alias of `screenshot`), `ade proof attach `, `ade proof record`, `ade proof launch`, `ade proof interact`, `ade proof list/status/environment/ingest`. `attach` infers the artifact kind from the file extension and routes through `ingest_computer_use_artifacts` with `backendStyle: "manual"`. Capture-style commands set `preferHeadless: true` on the plan so the connection layer drops to headless mode unless `--socket` is explicitly requested. All proof subcommands accept `--owner-kind` / `--owner-id` (with `chat` and `pr` aliases) to layer an explicit owner on top of the inferred session identity. diff --git a/docs/features/agents/README.md b/docs/features/agents/README.md index 5b51e1b3c..8f206c064 100644 --- a/docs/features/agents/README.md +++ b/docs/features/agents/README.md @@ -20,6 +20,7 @@ those surfaces. | `apps/desktop/src/main/services/agentTools/agentToolsService.ts` | Detects external CLI tools on PATH. | | `apps/ade-cli/src/cli.ts` | Agent-focused `ade` command surface and text/JSON output formatters. Includes the `ade ios-sim` (alias `ade ios`, `ade simulator`) family — see [iOS Simulator feature](../ios-simulator/README.md), the `ade --socket app-control ...` driver for live Electron apps, and the `ade --socket browser ...` driver for the in-app browser. The browser CLI covers tabs/navigation (`browser panel`, `open`, `new-tab`, `switch`, `close`), agent sessions (`browser session start`, `browser sessions`, `browser session `, `--browser-session `), hidden-tab observations/actions (`observe --map`, `click/fill/clear-field/press/wait --handle`, `trace`, `proof`), and selection / inspect commands. `ade secrets list|get|set|delete` is the typed surface for encrypted project-scoped ADE secrets that agents may read when the user names a secret. `ade chat create --provider codex --model --reasoning-effort --no-fast --permissions full-auto --prompt "..."` starts a persistent Work chat with explicit provider settings and then sends the prompt as the first message; `--print-config`/`--dry-run` prints the resolved create payload, provider permission mapping, and any follow-up send before launching. `ade chat read --text` reads recent transcript messages after create/send. `ade agent spawn` remains the legacy lane-scoped CLI-session launcher for Codex/Claude and deliberately rejects reasoning-effort flags; use `ade chat create` for Work chats or `ade shell start-cli ... --reasoning-effort ` for tracked CLI sessions when a launch must pin reasoning. `ade shell start --lane --chat-session ` (or `ADE_CHAT_SESSION_ID` from the env) attaches a tracked shell to an existing chat so `ade --socket terminal read --chat-session "$ADE_CHAT_SESSION_ID" --text` resolves to it. `ade lanes link-linear-issue --linear-issue-json '{...}'` (aliases `link-linear`, `linear-link`) links one or more Linear issues to an existing lane with optional `--role`, `--source`, `--include-in-pr`/`--no-include-in-pr`, and `--close-on-merge` flags. | | `apps/ade-cli/src/adeRpcServer.ts` | Private ADE action RPC: registers actions, handles JSON-RPC, applies session-identity-based filtering, builds lane-scoped ADE guidance / `ADE_AGENT_SKILLS_DIRS` for worker CLI launches, and returns GitHub + ADE PR URLs from PR creation tools when available. | +| `apps/desktop/resources/agent-skills/ade-cli-control-plane/SKILL.md` | Agent-facing ADE CLI control-plane guidance. It tells agents when to use normal shell commands vs. `ade`, distinguishes persistent Work chats from tracked CLI sessions, and keeps chat-attached terminals scoped to chat-visible logs/control rather than ordinary CLI command execution. | | `apps/desktop/src/main/services/cli/adeCliService.ts` | Desktop-side install / status / uninstall surface for the `ade` launcher. Owns the install-target path resolution and the optional shell-rc PATH append. | | `apps/desktop/src/shared/adeCliGuidance.ts` | Canonical agent-prompt guidance builder for finding and using `ade`, reading Agent Skills on demand, naming the bundled ADE skills, using socket-backed live surfaces, registering proof, and cleaning up started processes. Injected into Work chats, CLI launches, ADE Code/TUI sessions, CTO/worker agents, and mobile-started runtime work. | | `apps/desktop/src/shared/agentSkillRoots.ts` | Resolves and formats Agent Skill roots injected into prompts and CLI environments: lane/current-working-directory ancestors, user homes, inherited `ADE_AGENT_SKILLS_DIRS`, packaged ADE resources, and source fallbacks across `.cursor`, `.claude`, `.agents`, `.ade`, and `.codex` skill directories. | diff --git a/docs/features/chat/README.md b/docs/features/chat/README.md index ce493edcc..3cb55d981 100644 --- a/docs/features/chat/README.md +++ b/docs/features/chat/README.md @@ -353,8 +353,11 @@ See the detail docs for the specifics: --prompt` uses this same follow-up send after the session is created, and `ade chat read ` calls `chat.readTranscript` to inspect recent transcript messages for chat sessions only; shell/terminal transcript reads - stay on the terminal/session surfaces. Interactive chat sends are not - wall-clock bounded by the service; the turn runs until the provider + stay on the terminal/session surfaces. When invoked through the generic ADE + action bridge by a session-bound non-CTO caller, `chat.sendMessage` and + `chat.readTranscript` are scoped to that caller's own chat session. + Interactive chat sends are not wall-clock bounded by the service; the turn + runs until the provider completes or the user/app interrupts it. The blocking `runSessionTurn` helper used by automation has a 5 min default RPC timeout unless the caller passes `timeoutMs: null`; background/headless chat launches opt out.