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

merged5

main

fix/issue-26620

View on GitHubTwo-way sync enabledPR last imported 12h ago

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 (9e09926d97) 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 pingdotgg/t3code#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 (9e09926d97)	After (51c60ebe00)
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 (9e09926d97)	After (51c60ebe00)
Approval prompt
After Approve

The after run used 51c60ebe00; 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)

1	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')\""}}}}
2	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"}]}}
3	2026-10-28T20:36:44.884Z T3->agent {"jsonrpc":"2.0","id":1000,"result":{"outcome":{"outcome":"selected","optionId":"allow_once"}}}
4	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"}}
5	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."}}]}}
6	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."}}]}}}
7	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)

1	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')\""}}}}
2	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"}]}}
3	2026-10-28T20:37:42.167Z T3->agent {"jsonrpc":"2.0","id":1000,"result":{"outcome":{"outcome":"selected","optionId":"allow_once"}}}
4	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"}}
5	2026-10-28T20:37:42.184Z T3->agent {"jsonrpc":"2.0","id":1001,"result":{"terminalId":"t3-term-1"}}
6	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"}}
7	2026-10-28T20:37:42.217Z T3->agent {"jsonrpc":"2.0","id":1002,"result":{"exitCode":0,"signal":null}}
8	2026-10-28T20:37:42.218Z agent->T3 {"id":1003,"method":"terminal/release","params":{"sessionId":"id-only-session-1793219852734","terminalId":"t3-term-1"}}
9	2026-10-28T20:37:42.220Z T3->agent {"jsonrpc":"2.0","id":1003,"result":{}}
10	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}}}}
11	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)

1	#!/usr/bin/env node
2	// Scratch ACP v1 agent for pingdotgg/t3code#26620: announces an execute tool_call, asks for
3	// permission by toolCallId only (as Devin does), then runs the command through terminal/create.
4	import { createInterface } from "node:readline";
5	import { appendFileSync } from "node:fs";
6
7	const LOG = process.env.ID_ONLY_AGENT_LOG;
8	const log = (dir, msg) => LOG && appendFileSync(LOG, `${new Date().toISOString()} ${dir} ${JSON.stringify(msg)}\n`);
9	const send = (msg) => {
10	log(">>", msg);
11	process.stdout.write(JSON.stringify({ jsonrpc: "2.0", ...msg }) + "\n");
12	};
13	let nextId = 1000;
14	const pending = new Map();
15	const request = (method, params) =>
16	new Promise((resolve) => {
17	const id = nextId++;
18	pending.set(id, resolve);
19	send({ id, method, params });
20	});
21	const notify = (sessionId, update) => send({ method: "session/update", params: { sessionId, update } });
22
23	const COMMAND = "python3";
24	const ARGS = ["-c", "from pathlib import Path; Path('approval-probe.txt').write_text('ACP_APPROVAL_OK\\n')"];
25	const COMMAND_LINE = `${COMMAND} -c "from pathlib import Path; Path('approval-probe.txt').write_text('ACP_APPROVAL_OK\\n')"`;
26	let turn = 0;
27	const cwd = "/tmp/issue-lead-26620-r1/project";
28
29	async function prompt(id, params) {
30	const sessionId = params.sessionId;
31	const toolCallId = `call-${++turn}`;
32	notify(sessionId, {
33	sessionUpdate: "tool_call",
34	toolCallId,
35	title: COMMAND_LINE,
36	kind: "execute",
37	status: "pending",
38	rawInput: { command: COMMAND_LINE },
39	});
40	const permission = await request("session/request_permission", {
41	sessionId,
42	toolCall: { toolCallId },
43	options: [
44	{ optionId: "allow_once", name: "Allow once", kind: "allow_once" },
45	{ optionId: "reject_once", name: "Reject", kind: "reject_once" },
46	],
47	});
48	const selected = permission.result?.outcome?.outcome === "selected" ? permission.result.outcome.optionId : null;
49	let text;
50	if (selected !== "allow_once") {
51	notify(sessionId, { sessionUpdate: "tool_call_update", toolCallId, status: "failed", rawOutput: { error: "rejected" } });
52	text = `Permission outcome: ${JSON.stringify(permission.result ?? permission.error)}`;
53	} else {
54	const created = await request("terminal/create", { sessionId, command: COMMAND, args: ARGS, cwd });
55	if (created.error) {
56	notify(sessionId, {
57	sessionUpdate: "tool_call_update",
58	toolCallId,
59	status: "failed",
60	content: [{ type: "content", content: { type: "text", text: "Failed to create terminal. Please try again." } }],
61	});
62	text = `terminal/create failed: ${created.error.message}`;
63	} else {
64	const terminalId = created.result.terminalId;
65	const exit = await request("terminal/wait_for_exit", { sessionId, terminalId });
66	await request("terminal/release", { sessionId, terminalId });
67	notify(sessionId, {
68	sessionUpdate: "tool_call_update",
69	toolCallId,
70	status: "completed",
71	rawOutput: { exitCode: exit.result?.exitCode ?? null },
72	});
73	text = `terminal exited ${JSON.stringify(exit.result ?? exit.error)}`;
74	}
75	}
76	notify(sessionId, { sessionUpdate: "agent_message_chunk", content: { type: "text", text } });
77	send({ id, result: { stopReason: "end_turn" } });
78	}
79
80	createInterface({ input: process.stdin }).on("line", (line) => {
81	if (!line.trim()) return;
82	const msg = JSON.parse(line);
83	log("<<", msg);
84	if (msg.method === undefined && pending.has(msg.id)) {
85	const resolve = pending.get(msg.id);
86	pending.delete(msg.id);
87	resolve(msg);
88	return;
89	}
90	switch (msg.method) {
91	case "initialize":
92	return send({
93	id: msg.id,
94	result: { protocolVersion: 1, agentCapabilities: { loadSession: false }, authMethods: [] },
95	});
96	case "session/new":
97	return send({ id: msg.id, result: { sessionId: `id-only-session-${Date.now()}` } });
98	case "session/prompt":
99	return void prompt(msg.id, msg.params);
100	case "session/cancel":
101	return;
102	default:
103	if (msg.id !== undefined) send({ id: msg.id, error: { code: -32601, message: `Method not found: ${msg.method}` } });
104	}
105	});

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.

Checks

24 successful · 16 skipped

All checks passed

Added 5 commits19h ago

test(acp): reproduce #26620 ID-only permission losing the tracked command1adfa5c
Merge remote-tracking branch 'origin/main' into fix/issue-2662027ae771
fix(acp): only a tracked execute kind grants terminal execution51c60eb
fix(acp): explicitly cleared permission fields are not refilled from the tracked call553a9cb
fix(acp): ID-only permission requests use the tracked tool call801d933

MmacroscopeappBotapproved these changes19h ago

MmacroscopeappBot19h ago

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.}

CcoderabbitaiBot19h ago

Review in Change Stack →

No actionable comments were generated in the recent review.

External image Recent review info

External image 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

External image Commits

Reviewing files that changed from the base of the PR and between 9826dbb9904b2591bfbf60d1317486b59132f56b and 553a9cbc6d0b54c0c1bd748de2156e96a634724f.

External image 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: External image 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: External image 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: ^Passed 4

Check name	Status	Explanation
Description check	Passed	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	Passed	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	Passed	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	Passed	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.}

Ggithub-actionsBot19h ago

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.