fix(acp): ID-only command approvals keep the tracked command and grant the terminal by juliusmarminge · Pull Request #26647 · pingdotgg/t3code

juliusmarminge · 2026-10-28T20:53:14Z

Problem

An ACP agent can send session/request_permission with only a toolCallId after it has already announced that tool call in the turn. Devin from the ACP Registry does this. T3 read only the request payload, so in Supervised mode the approval prompt showed Session <session id> instead of the command. The request's kind was also unknown, so accepting it recorded no command grant. Devin's next terminal/create was then refused, and the command never ran ("Failed to create terminal").

Reproduced on main (9e09926) in the web dev server, with a scripted ACP v1 agent registered as devin (Executable override) in Supervised mode. The agent announces an execute tool_call, sends an ID-only permission request, then calls terminal/create once the request is approved.

Change

When a permission request leaves out the title, raw input or kind, the ACP adapter now copies them from the tool call that the active turn of the same session is already tracking (withTrackedPermissionToolCall). This happens once, before anything else reads the request, so the prompt, the policy disposition and the grant all see the same request. Fields the request does send are kept, including fields it explicitly clears with null. A tracked kind is copied only when it is execute: other and custom kinds map to command approvals, so copying them would let an approved non-execute tool open terminals. An untracked tool call is left unchanged and still gets no execute grant.

Scope and approval

Fixes #26620

Verification

AcpAdapterV2.test.ts: the new table test fails on main and passes here.
- grants an ID-only execute approval its tracked command: on main the prompt is Session mock-session-1 and terminalCreated is false.
- does not grant execution to an ID-only approval of another tool: a tracked other kind is still refused.
- The existing does not turn an unknown permission approval into an execute grant passes unchanged.
runtimeModel.test.ts: keeps an explicit permission request kind over the tracked tool call's kind and keeps explicitly cleared permission request fields over the tracked tool call (a kind: null request stays unknown).
AcpAdapterV2, AcpRegistryAdapterV2, runtimeModel and clientPolicy tests pass (145 tests). vp run check:changed passes.
Real app, web dev server, fresh databases (T3CODE_DEV_SEED=0), Supervised mode, scripted agent as devin, 1280×800, light theme.

	Before (`9e09926`)	After (`51c60eb`)
Approval prompt	`Session id-only-session-…`	the full `python3 -c …` command
After Approve	`terminal/create` refused with "requires approval for terminal/create"; no `approval-probe.txt`	terminal exits 0; `approval-probe.txt` contains `ACP_APPROVAL_OK`

Captures, 1280×800, light theme:

	Before (`9e09926`)	After (`51c60eb`)
Approval prompt
After Approve

The after run used 51c60eb; the later head only changes how explicit null fields are handled, which this ID-only flow doesn't send. No render code changed; only the prompt text and the grant change.

Protocol transcript, before (agent ↔ T3)

2026-10-28T20:36:37.157Z agent->T3 {"method":"session/update","params":{"sessionId":"id-only-session-1793219795759","update":{"sessionUpdate":"tool_call","toolCallId":"call-1","title":"python3 -c \"from pathlib import Path; Path('approval-probe.txt').write_text('ACP_APPROVAL_OK\\n')\"","kind":"execute","status":"pending","rawInput":{"command":"python3 -c \"from pathlib import Path; Path('approval-probe.txt').write_text('ACP_APPROVAL_OK\\n')\""}}}}
2026-10-28T20:36:37.157Z agent->T3 {"id":1000,"method":"session/request_permission","params":{"sessionId":"id-only-session-1793219795759","toolCall":{"toolCallId":"call-1"},"options":[{"optionId":"allow_once","name":"Allow once","kind":"allow_once"},{"optionId":"reject_once","name":"Reject","kind":"reject_once"}]}}
2026-10-28T20:36:44.884Z T3->agent {"jsonrpc":"2.0","id":1000,"result":{"outcome":{"outcome":"selected","optionId":"allow_once"}}}
2026-10-28T20:36:44.884Z agent->T3 {"id":1001,"method":"terminal/create","params":{"sessionId":"id-only-session-1793219795759","command":"python3","args":["-c","from pathlib import Path; Path('approval-probe.txt').write_text('ACP_APPROVAL_OK\\n')"],"cwd":"/tmp/issue-lead-26620-r1/project"}}
2026-10-28T20:36:44.887Z T3->agent {"jsonrpc":"2.0","id":1001,"error":{"_tag":"Cause","code":-32603,"message":"The active T3 runtime policy requires approval for terminal/create. Request permission with session/request_permission before retrying.","data":[{"_tag":"Fail","error":{"code":-32603,"message":"The active T3 runtime policy requires approval for terminal/create. Request permission with session/request_permission before retrying."}}]}}
2026-10-28T20:36:44.887Z agent->T3 {"method":"session/update","params":{"sessionId":"id-only-session-1793219795759","update":{"sessionUpdate":"tool_call_update","toolCallId":"call-1","status":"failed","content":[{"type":"content","content":{"type":"text","text":"Failed to create terminal. Please try again."}}]}}}
2026-10-28T20:36:44.887Z agent->T3 {"method":"session/update","params":{"sessionId":"id-only-session-1793219795759","update":{"sessionUpdate":"agent_message_chunk","content":{"type":"text","text":"terminal/create failed: The active T3 runtime policy requires approval for terminal/create. Request permission with session/request_permission before retrying."}}}}

Protocol transcript, after (agent ↔ T3)

2026-10-28T20:37:33.242Z agent->T3 {"method":"session/update","params":{"sessionId":"id-only-session-1793219852734","update":{"sessionUpdate":"tool_call","toolCallId":"call-1","title":"python3 -c \"from pathlib import Path; Path('approval-probe.txt').write_text('ACP_APPROVAL_OK\\n')\"","kind":"execute","status":"pending","rawInput":{"command":"python3 -c \"from pathlib import Path; Path('approval-probe.txt').write_text('ACP_APPROVAL_OK\\n')\""}}}}
2026-10-28T20:37:33.242Z agent->T3 {"id":1000,"method":"session/request_permission","params":{"sessionId":"id-only-session-1793219852734","toolCall":{"toolCallId":"call-1"},"options":[{"optionId":"allow_once","name":"Allow once","kind":"allow_once"},{"optionId":"reject_once","name":"Reject","kind":"reject_once"}]}}
2026-10-28T20:37:42.167Z T3->agent {"jsonrpc":"2.0","id":1000,"result":{"outcome":{"outcome":"selected","optionId":"allow_once"}}}
2026-10-28T20:37:42.167Z agent->T3 {"id":1001,"method":"terminal/create","params":{"sessionId":"id-only-session-1793219852734","command":"python3","args":["-c","from pathlib import Path; Path('approval-probe.txt').write_text('ACP_APPROVAL_OK\\n')"],"cwd":"/tmp/issue-lead-26620-r1/project"}}
2026-10-28T20:37:42.184Z T3->agent {"jsonrpc":"2.0","id":1001,"result":{"terminalId":"t3-term-1"}}
2026-10-28T20:37:42.184Z agent->T3 {"id":1002,"method":"terminal/wait_for_exit","params":{"sessionId":"id-only-session-1793219852734","terminalId":"t3-term-1"}}
2026-10-28T20:37:42.217Z T3->agent {"jsonrpc":"2.0","id":1002,"result":{"exitCode":0,"signal":null}}
2026-10-28T20:37:42.218Z agent->T3 {"id":1003,"method":"terminal/release","params":{"sessionId":"id-only-session-1793219852734","terminalId":"t3-term-1"}}
2026-10-28T20:37:42.220Z T3->agent {"jsonrpc":"2.0","id":1003,"result":{}}
2026-10-28T20:37:42.220Z agent->T3 {"method":"session/update","params":{"sessionId":"id-only-session-1793219852734","update":{"sessionUpdate":"tool_call_update","toolCallId":"call-1","status":"completed","rawOutput":{"exitCode":0}}}}
2026-10-28T20:37:42.220Z agent->T3 {"method":"session/update","params":{"sessionId":"id-only-session-1793219852734","update":{"sessionUpdate":"agent_message_chunk","content":{"type":"text","text":"terminal exited {\"exitCode\":0,\"signal\":null}"}}}}

Scratch ACP agent used for the runtime reproduction (not committed)

#!/usr/bin/env node
// Scratch ACP v1 agent for pingdotgg/t3code#26620: announces an execute tool_call, asks for
// permission by toolCallId only (as Devin does), then runs the command through terminal/create.
import { createInterface } from "node:readline";
import { appendFileSync } from "node:fs";

const LOG = process.env.ID_ONLY_AGENT_LOG;
const log = (dir, msg) => LOG && appendFileSync(LOG, `${new Date().toISOString()} ${dir} ${JSON.stringify(msg)}\n`);
const send = (msg) => {
  log(">>", msg);
  process.stdout.write(JSON.stringify({ jsonrpc: "2.0", ...msg }) + "\n");
};
let nextId = 1000;
const pending = new Map();
const request = (method, params) =>
  new Promise((resolve) => {
    const id = nextId++;
    pending.set(id, resolve);
    send({ id, method, params });
  });
const notify = (sessionId, update) => send({ method: "session/update", params: { sessionId, update } });

const COMMAND = "python3";
const ARGS = ["-c", "from pathlib import Path; Path('approval-probe.txt').write_text('ACP_APPROVAL_OK\\n')"];
const COMMAND_LINE = `${COMMAND} -c "from pathlib import Path; Path('approval-probe.txt').write_text('ACP_APPROVAL_OK\\n')"`;
let turn = 0;
const cwd = "/tmp/issue-lead-26620-r1/project";

async function prompt(id, params) {
  const sessionId = params.sessionId;
  const toolCallId = `call-${++turn}`;
  notify(sessionId, {
    sessionUpdate: "tool_call",
    toolCallId,
    title: COMMAND_LINE,
    kind: "execute",
    status: "pending",
    rawInput: { command: COMMAND_LINE },
  });
  const permission = await request("session/request_permission", {
    sessionId,
    toolCall: { toolCallId },
    options: [
      { optionId: "allow_once", name: "Allow once", kind: "allow_once" },
      { optionId: "reject_once", name: "Reject", kind: "reject_once" },
    ],
  });
  const selected = permission.result?.outcome?.outcome === "selected" ? permission.result.outcome.optionId : null;
  let text;
  if (selected !== "allow_once") {
    notify(sessionId, { sessionUpdate: "tool_call_update", toolCallId, status: "failed", rawOutput: { error: "rejected" } });
    text = `Permission outcome: ${JSON.stringify(permission.result ?? permission.error)}`;
  } else {
    const created = await request("terminal/create", { sessionId, command: COMMAND, args: ARGS, cwd });
    if (created.error) {
      notify(sessionId, {
        sessionUpdate: "tool_call_update",
        toolCallId,
        status: "failed",
        content: [{ type: "content", content: { type: "text", text: "Failed to create terminal. Please try again." } }],
      });
      text = `terminal/create failed: ${created.error.message}`;
    } else {
      const terminalId = created.result.terminalId;
      const exit = await request("terminal/wait_for_exit", { sessionId, terminalId });
      await request("terminal/release", { sessionId, terminalId });
      notify(sessionId, {
        sessionUpdate: "tool_call_update",
        toolCallId,
        status: "completed",
        rawOutput: { exitCode: exit.result?.exitCode ?? null },
      });
      text = `terminal exited ${JSON.stringify(exit.result ?? exit.error)}`;
    }
  }
  notify(sessionId, { sessionUpdate: "agent_message_chunk", content: { type: "text", text } });
  send({ id, result: { stopReason: "end_turn" } });
}

createInterface({ input: process.stdin }).on("line", (line) => {
  if (!line.trim()) return;
  const msg = JSON.parse(line);
  log("<<", msg);
  if (msg.method === undefined && pending.has(msg.id)) {
    const resolve = pending.get(msg.id);
    pending.delete(msg.id);
    resolve(msg);
    return;
  }
  switch (msg.method) {
    case "initialize":
      return send({
        id: msg.id,
        result: { protocolVersion: 1, agentCapabilities: { loadSession: false }, authMethods: [] },
      });
    case "session/new":
      return send({ id: msg.id, result: { sessionId: `id-only-session-${Date.now()}` } });
    case "session/prompt":
      return void prompt(msg.id, msg.params);
    case "session/cancel":
      return;
    default:
      if (msg.id !== undefined) send({ id: msg.id, error: { code: -32601, message: `Method not found: ${msg.method}` } });
  }
});

Not checked: a real Devin binary (needs a Devin account) and the desktop and mobile clients. The change is server-side, and they render the same approval data.

Implemented with Claude Opus 5.5 (Claude Code) and reviewed with GPT-6.1-Sol (Codex), orchestrated in T3 Code.

…mand An ACP agent in Supervised mode reports its execute tool call, then asks session/request_permission with only the toolCallId. The approval prompt falls back to "Session <id>" and approving it records no execute grant, so the agent's terminal/create is refused. These tests reproduce #26620. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

ACP allows session/request_permission to name its tool call by ID only. Devin does this after announcing the execute call, so the prompt showed "Session <id>", the kind was unknown, and approving recorded no command grant, which made the agent's terminal/create fail. The permission handler now fills the omitted title, kind and rawInput from the tool call the active turn tracks for that toolCallId in the same session, before the prompt, disposition, flavor hooks and grant read the request. Fields the request sends still win, and an untracked toolCallId is left unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…the tracked call Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

macroscopeapp · 2026-10-28T20:56:34Z

Approvability

Verdict: Approved at 553a9cb

Macroscope's review found this PR approvable — ID-only ACP approvals now show the tracked command and enable terminal execution through the existing approval flow. Session matching, execute-only inheritance and unchanged approval scopes keep the correction narrowly bounded.

^{You can add or adjust custom eligibility rules. Learn more.}

coderabbitai · 2026-10-28T20:59:53Z

No actionable comments were generated in the recent review.

Recent review info

Run configuration

Source mode: Auto
Repository: pingdotgg/t3code
Branch: fix/issue-26620
File: .coderabbit.config.ts
Commit: 553a9cb
Layers: Repository TS config
Review profile: CHILL
Plan: Team
Run ID: fd4f5ec4-05ed-463d-830f-6d493480aec9

Commits

Reviewing files that changed from the base of the PR and between 9826dbb and 553a9cb.

Files selected for processing: 5

apps/server/scripts/acp-mock-agent.ts
apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts
packages/provider-acp/src/server/adapter.ts
packages/provider-acp/src/server/runtimeModel.test.ts
packages/provider-acp/src/server/runtimeModel.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

Walkthrough

The ACP adapter now enriches matching active-session permission requests with omitted fields from the tracked tool call. Explicit native values remain authoritative, and only an omitted kind on a tracked execute call is inferred as execute. Tests cover ID-only approvals and terminal outcomes for execute and other.

Changes: ACP permission context

Layer / File(s)	Summary
Permission field enrichment `packages/provider-acp/src/server/runtimeModel.ts`, `packages/provider-acp/src/server/runtimeModel.test.ts`	`withTrackedPermissionToolCall` fills undefined title and raw input from tracked state. It sets an undefined kind to `execute` only when the tracked call is an execute call. Tests check that explicit values, including `null`, remain unchanged.
Active-session permission wiring `packages/provider-acp/src/server/adapter.ts`	The permission handler enriches requests when their session matches the active session and tracked tool-call state is available. Other requests use the native parameters unchanged.
Approval outcome test scenario `apps/server/scripts/acp-mock-agent.ts`, `apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts`	The mock agent can emit a pending `git status` tool call and wait. Adapter tests check the prompt and terminal-creation outcome for ID-only approvals with `execute` and `other` kinds.

Priority: Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related issues

pingdotgg/t3code#26620 — Reports the same ID-only ACP permission request losing tracked command context and blocking terminal execution.

Possibly related PRs

pingdotgg/t3code#26624 — Enriches ID-only permission requests from the active turn’s tracked tool call; this PR also preserves explicit nulls and limits execute-kind inference to tracked execute calls.
pingdotgg/t3code#20145 — Addresses command-approval presentation; this PR supplies tracked ACP title and raw input for ID-only approvals.

Suggested reviewers: t3dotgg, stienswout

Merge Risk: Minimal · up to 553a9

The ID-only approval path appears to preserve the intended terminal permission distinction. No merge-blocking issue was identified.

Pre-merge checks: 4

Check name	Status	Explanation
Description check		The description clearly covers the problem, change, scope, approval reference, focused verification, observed results, limitations, screenshots, and agent attribution. It matches the required template…
Title check		The title clearly identifies the ACP ID-only approval fix and its primary outcome: preserving the tracked command and granting terminal execution. It is concise and specific.
Linked Issues check		Issue `#26620` requires ID-only permission requests to use the tracked command, authorize a tracked execute request, and deny execution for unknown or non-execute tools. The adapter enriches matching re…
Out of Scope Changes check		The changes stay within issue `#26620`. The adapter and runtime-model changes implement request enrichment. The ACP mock-agent option and added tests provide coverage for the reported protocol cases. No…

Full details: Description check

Explanation

The description clearly covers the problem, change, scope, approval reference, focused verification, observed results, limitations, screenshots, and agent attribution. It matches the required template and explains the cross-component fix.

Full details: Linked Issues check

Explanation

Issue #26620 requires ID-only permission requests to use the tracked command, authorize a tracked execute request, and deny execution for unknown or non-execute tools. The adapter enriches matching requests before policy and grant processing. The helper preserves request fields, including explicit null values, and copies kind only for tracked execute calls. The added tests cover the execute, other-kind, unknown-kind, explicit-kind, and explicit-null cases.

Full details: Out of Scope Changes check

Explanation

The changes stay within issue #26620. The adapter and runtime-model changes implement request enrichment. The ACP mock-agent option and added tests provide coverage for the reported protocol cases. No unrelated production behavior is identified.

Finishing Touches

📝 Generate docstrings

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

github-actions · 2026-10-28T20:59:58Z

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider	Metric	Main baseline	This PR	Impact	PR ceiling
Codex	Total thread wire	5.0 KiB	5.0 KiB	0 B (0.0%)	6.8 KiB	✅
Codex	Thread snapshot wire	3.8 KiB	3.8 KiB	0 B (0.0%)	4.9 KiB	✅
Codex	Live turn WebSocket wire	1.2 KiB	1.2 KiB	0 B (0.0%)	2.0 KiB	✅
Codex	Live turn WebSocket decoded	21.6 KiB	21.6 KiB	0 B (0.0%)	24.4 KiB	✅
Codex	Live turn messages	2	2	0 (0.0%)	8	✅
Claude	Total thread wire	5.0 KiB	5.0 KiB	0 B (0.0%)	6.8 KiB	✅
Claude	Thread snapshot wire	3.8 KiB	3.8 KiB	0 B (0.0%)	4.9 KiB	✅
Claude	Live turn WebSocket wire	1.3 KiB	1.3 KiB	0 B (0.0%)	2.0 KiB	✅
Claude	Live turn WebSocket decoded	21.9 KiB	21.9 KiB	0 B (0.0%)	24.4 KiB	✅
Claude	Live turn messages	2	2	0 (0.0%)	8	✅

Baseline: 9826dbb · PR result: 553a9cb · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

Codex decoded thread snapshot: 108.5 KiB
Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

juliusmarminge and others added 5 commits October 28, 2026 20:04

fix(acp): only a tracked execute kind grants terminal execution

51c60eb

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Merge remote-tracking branch 'origin/main' into fix/issue-26620

27ae771

fix(acp): explicitly cleared permission fields are not refilled from …

553a9cb

…the tracked call Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 28, 2026

macroscopeapp Bot approved these changes Oct 28, 2026

View reviewed changes

juliusmarminge added bugfix Fix PR opened by an issue lead; enters the PR loop pr:needs-julius Needs Julius: route gate, unverifiable surface, or blocked labels Oct 28, 2026

juliusmarminge merged commit b287102 into main Oct 29, 2026
40 checks passed

juliusmarminge deleted the fix/issue-26620 branch October 29, 2026 00:17

Navigation Menu

Repository navigation

fix(acp): ID-only command approvals keep the tracked command and grant the terminal - #26647

fix(acp): ID-only command approvals keep the tracked command and grant the terminal#26647
juliusmarminge merged 5 commits into
mainfrom
fix/issue-26620

juliusmarminge commented Oct 28, 2026

Uh oh!

macroscopeapp Bot commented Oct 28, 2026

Uh oh!

coderabbitai Bot commented Oct 28, 2026

Uh oh!

github-actions Bot commented Oct 28, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

juliusmarminge commented Oct 28, 2026

Problem

Change

Scope and approval

Verification

Uh oh!

macroscopeapp Bot commented Oct 28, 2026

Approvability

Uh oh!

coderabbitai Bot commented Oct 28, 2026

Uh oh!

github-actions Bot commented Oct 28, 2026

Thread transfer impact

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant