Skip to content

feat(server): ACP registry agents run in their native permission mode - #13629

Closed
juliusmarminge wants to merge 9 commits into
t3code/codex-turn-mappingfrom
v2/acp-native-modes
Closed

juliusmarminge wants to merge 9 commits into
t3code/codex-turn-mappingfrom
v2/acp-native-modes

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Sep 25, 2026 •

Copy link
Copy Markdown
Member

Registry agents all ran in whatever permission mode they started in, and T3 answered their permission prompts by its own policy. A full-access thread on codex-acp still ran codex in its default agent mode. An approval-required thread on claude-acp was only as strict as the prompts claude chose to send. The mode picker could also select any agent mode after the runtime mode had been applied, so a stored pick could loosen the policy.

Layer 4 of the ACP stack ("rely on the agent's own sandboxes and permission models rather than implementing our own, like Claude and Codex"). Stacked on #13623.

What changed

  • AcpRegistryPermissionModes.ts maps each T3 runtime mode to the agent's own mode. Ids come from each agent's source (snapshots of the current registry versions):

    agent approval-required auto-accept-edits auto full-access
    codex-acp (src/AgentMode.ts) read-only workspace-write agent agent-full-access
    claude-acp (src/session-mode.ts) default acceptEdits auto bypassPermissions
    gemini (ApprovalMode) default autoEdit default yolo
    qwen-code (setMode modeMap) default auto-edit auto yolo
    goose (GooseMode) approve smart_approve smart_approve auto
    mistral-vibe (agents/models.py) ask accept-edits smart-approve auto-approve
  • The registry flavor returns that mode from sessionModeForPolicy. When a policy mode applies, stored mode selections (the synthetic _t3/session-mode or a category: "mode" config option) are dropped, so an old pick can't override the thread's runtime mode.

  • Fails closed. Suppose the agent doesn't advertise the mapped mode, rejects it (for example qwen's untrusted-folder -32003), or reports a different mode after the switch. If the thread is in approval-required, auto-accept-edits or auto, the session doesn't open. The error names the agent and the mode, for example "Codex could not switch to its 'read-only' mode for this thread's permission mode: it refused it. Choose another permission mode or update the agent." The agent's own error text and reported mode id stay out of the message and log annotations; the rejection is kept as the error cause. Without this, an agent that keeps a looser mode (Goose starts in auto) would never send session/request_permission, and the thread's promise would silently break. Full access only warns, because any mode the agent stays in is stricter.

  • runtime.setMode picks the transport by what the session advertises:

    • With a mode config option it sends session/set_config_option and takes the mode from the agent's answer instead of assuming it switched.
    • If that call is refused with method-not-found or invalid-params and the session also advertises modes, it falls back to session/set_mode. Goose advertises a mode config option (category Mode) once a provider and model are configured (goose/src/acp/response_builder.rs:245-276, 308-314), but the only handler I could find is on_set_mode (server.rs:2512).
    • With only modes (gemini-cli) it sends session/set_mode. effect-acp gains agent.setSessionMode, v1-only like setSessionModel; on v2 it fails method-not-found.
  • Mapped agents report runtimePolicy.enforcement: "native". That's accurate because a stricter-than-full-access session only exists once the agent is confirmed in the mapped mode, and registry agents have had client fs off since refactor(server): ACP client fs and terminals are opt-in per flavor #13623. Unmapped agents stay "client-boundary".

  • For mapped agents the mode picker is left out where the option descriptors are built (acpProviderOptionDescriptors with omitModes), by category: "mode" rather than by id. Any agent mode option (mode, permission-mode, …) and the synthetic modes descriptor are hidden, so web and mobile show no mode picker the runtime mode would overrule. Other options (reasoning, fast mode) stay.

Decisions (override if you disagree)

  • The table follows the maintainer's defaults. Where an agent has no matching mode, it maps to the stricter neighbour: gemini auto → default (gemini has no classifier mode); goose auto-accept-edits → smart_approve (goose has no edits-only mode). smart_approve may auto-run commands Goose's classifier deems read-only, which is looser than "ask before commands".
  • A stricter thread fails to open rather than running an agent that ignored its mode. Full access proceeds with a warning.
  • Unmapped registry agents (Devin and the rest) keep their own default mode, and T3 answers their prompts by policy.
  • The picker is hidden rather than shown locked. ProviderOptionDescriptor has no read-only state, and adding one means changes to contracts plus web and mobile, so that's left for a follow-up. The thread's runtime mode control is the one place to change it.
  • enforcement is kept (not deleted), and "native" is set only for mapped registry agents. Grok maps its runtime mode at launch (fix(server): V2 Grok launches in the thread's permission mode #13616) and could claim "native" too; that is left to the Grok owner.

Verification

In apps/server unless noted, with TMPDIR under /home:

  • vp test run src/orchestration-v2/Adapters/AcpRegistryAdapterV2.test.ts. native permission modes drives the real registry adapter against scripted ACP v1 agents (acp-replay-agent.ts). Each script must be consumed exactly, with no missing or extra frames.
    • switches each mapped agent to its own mode covers one mapping per agent: codex-acp approval-required → set_config_option read-only; claude-acp auto-accept-edits → acceptEdits; gemini full-access → session/set_mode yolo; qwen-code auto → auto; goose approval-required → set_config_option approve, refused -32601, then session/set_mode approve; mistral-vibe auto-accept-edits → accept-edits. A stored agent-full-access pick is never sent, and each session reports native.
    • refuses to open a stricter thread when the agent cannot switch: under approval-required, a mode that isn't advertised, a rejected mode (-32003), and a read-back mismatch each fail with the named message. Under full access, a mode that isn't advertised and a rejected mode both proceed.
    • leaves unmapped agents in their own mode: the stored pick is applied and the session reports client-boundary.
    • Six mutations each fail these tests: removing the advertised-mode guard, the strict failure, the rejection catch, or the goose fallback; ignoring the read-back; and making full access strict.
  • vp test run src/provider/acp/AcpRegistryProbe.test.ts: leaves out the mode picker for agents whose mode follows the permission mode checks that a permission-mode option (category mode) and the synthetic modes descriptor are hidden for mapped agents and kept for unmapped ones. Filtering by id only fails it.
  • vp test run src/provider/acp/AcpRegistryPermissionModes.test.ts: unmapped and prototype-key agent ids get no mode.
  • packages/effect-acp: vp test run src/client.test.ts passes 29/29. The v1 flow sends session/set_mode, and a v2 session gets -32601.
  • Broader run: the ACP adapters, both registry and Antigravity drivers, AcpJsonRpcConnection, and the full replay suite. 297 passed.
  • vp exec tsc --noEmit -p . (apps/server and packages/effect-acp): clean. vp run knip:check: clean. vp lint on the touched files: only pre-existing warnings.
  • Not run: live registry agents (none installed here).

Model: Claude Opus 5.5 (Claude Code)

🤖 Generated with Claude Code


Devin Review

juliusmarminge and others added 3 commits September 25, 2026 02:05
The V2 ACP runtime input carried no runtime mode, so GrokAdapterV2 always
launched `grok agent stdio` and every thread ran in Grok's ask mode. The
ACP runtime input now carries the session's runtime policy and Grok maps it
to its launch flags. Auto-accept edits launches asking (Grok's agent ignores
`--permission-mode acceptEdits`) and T3's ACP policy approves edit prompts
while asking for everything else.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The generic ACP fs handlers registered after Antigravity's own and replaced
them (effect-acp keeps the last handler per method), so Antigravity could
read and write any path the T3 server can. The flavor now supplies its
handlers through `clientFileSystem`, and the adapter serves them behind the
runtime policy guard instead of the unconfined generic ones.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…f the server cwd

A session policy without a cwd made the server's own cwd Antigravity's
containment root. Pass the nullable policy cwd through and allow only the
attachments dir in that case.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Sep 25, 2026
* through `session/set_mode`, or the `mode` config option.
*/
export function acpRegistryIsModeOptionDescriptor(descriptor: ProviderOptionDescriptor): boolean {
return descriptor.id === ACP_SESSION_MODE_OPTION_ID || descriptor.id === "mode";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium acp/AcpRegistryPermissionModes.ts:77

Mapped registry agents leave mode pickers such as permission-mode visible even though their stored selection is ignored, so users can choose a mode that has no effect. acpRegistryIsModeOptionDescriptor only recognizes the literal id "mode"; classify descriptors by their category: "mode" instead so arbitrary SessionConfigIds are handled.

Suggested change
return descriptor.id === ACP_SESSION_MODE_OPTION_ID || descriptor.id === "mode";
return descriptor.id === ACP_SESSION_MODE_OPTION_ID || descriptor.category === "mode";
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/provider/acp/AcpRegistryPermissionModes.ts around line 77:

Mapped registry agents leave mode pickers such as `permission-mode` visible even though their stored selection is ignored, so users can choose a mode that has no effect. `acpRegistryIsModeOptionDescriptor` only recognizes the literal id `"mode"`; classify descriptors by their `category: "mode"` instead so arbitrary `SessionConfigId`s are handled.

: AcpProviderCapabilitiesV2,
...(nativePermissionModes
? {
sessionModeForPolicy: (policy: ProviderAdapterV2RuntimePolicy) =>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High Adapters/AcpRegistryAdapterV2.ts:159

Mapped agents are reported as runtimePolicy.enforcement: "native" even when the live session does not support the mode returned by sessionModeForPolicy, so setup skips setMode and leaves the agent in its default permission mode. An upgraded agent that removes or renames a mode such as read-only can therefore run an approval-required thread without the requested sandbox/approval policy or the previous client-boundary fallback. Advertise native enforcement only when the session confirms the mapped mode, otherwise retain the fallback path.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/orchestration-v2/Adapters/AcpRegistryAdapterV2.ts around line 159:

Mapped agents are reported as `runtimePolicy.enforcement: "native"` even when the live session does not support the mode returned by `sessionModeForPolicy`, so setup skips `setMode` and leaves the agent in its default permission mode. An upgraded agent that removes or renames a mode such as `read-only` can therefore run an approval-required thread without the requested sandbox/approval policy or the previous client-boundary fallback. Advertise native enforcement only when the session confirms the mapped mode, otherwise retain the fallback path.

Comment thread apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts Outdated
@github-actions

github-actions Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ No successful main baseline artifact is available yet. This run establishes the initial measurement.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire — 4.9 KiB — 6.8 KiB ✅
Codex Thread snapshot wire — 3.7 KiB — 4.9 KiB ✅
Codex Live turn WebSocket wire — 1.2 KiB — 2.0 KiB ✅
Codex Live turn WebSocket decoded — 20.4 KiB — 29.3 KiB ✅
Codex Live turn messages — 2 — 8 ✅
Claude Total thread wire — 4.9 KiB — 6.8 KiB ✅
Claude Thread snapshot wire — 3.7 KiB — 4.9 KiB ✅
Claude Live turn WebSocket wire — 1.2 KiB — 2.0 KiB ✅
Claude Live turn WebSocket decoded — 20.8 KiB — 29.3 KiB ✅
Claude Live turn messages — 2 — 8 ✅

Baseline: unavailable · PR result: 6e062f3 · 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: 106.1 KiB
  • Claude decoded thread snapshot: 106.4 KiB

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

@macroscopeapp

macroscopeapp Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR substantially changes production permission enforcement, native agent modes, filesystem and terminal capability routing, and Grok launch behavior across shared ACP infrastructure. The effective defaults for existing sessions change, and unresolved findings concern mode visibility and ensuring unsupported native modes fail closed.

Not approved because:

  • 2 blocking correctness issues found at or above your repo's Minimum Blocking Severity

Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more.

Comment thread apps/server/src/provider/acp/AcpSessionRuntime.ts Outdated
Comment thread apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts Outdated
Comment thread apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts Outdated
juliusmarminge and others added 6 commits September 25, 2026 10:01
…nt workspace

Two escapes from Antigravity's workspace containment:

- The check resolved only the parent directory, so a symlink inside the
  workspace pointing outside it could be read or overwritten through. The
  final target is now canonicalized before the check, and a link that
  exists but cannot be resolved (dangling) is refused rather than written
  through.
- The allowed roots came from the policy the session opened with. They now
  come from the policy active when the request arrives, the one the policy
  guard already checks, so a session carried into a turn for another
  workspace cannot reach the previous one.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Every ACP agent was offered client fs, so agents like Grok routed their file
reads and writes through T3 instead of their own permission model. Now only
Antigravity (its own workspace-contained handlers) gets client fs and only
Devin gets client terminals; everyone else reads, writes and runs commands
itself and asks through session/request_permission. The unconfined generic
fs handlers are gone, so a stray fs request gets method-not-found.

The Grok recorder pins the clientCapabilities T3 advertises, and the two
Grok fixtures that carried fs frames are re-recorded live against grok
1.0.41: Grok asks once for its own write and sends no fs or terminal
requests.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The mock agent can now send fs/write_text_file and fs/read_text_file on
every prompt regardless of what the client advertised. A generic flavor
without clientFileSystem answers both with method-not-found and nothing is
written. A registry test pins client terminals to Devin and client fs to no
registry agent.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Mapped registry agents (codex-acp, claude-acp, gemini, qwen-code, goose,
mistral-vibe) now switch to their own permission mode for the thread's
runtime mode, so they enforce it with their own sandbox and approvals. The
mode is applied only when the session advertises it, read back from the
agent's answer, and a mismatch is logged. Those sessions report
`enforcement: "native"`, their mode picker is hidden, and a stored mode pick
can no longer override the policy.

ACP v1 agents that expose modes without a mode config option (gemini-cli,
goose) get `session/set_mode`, which effect-acp now sends on v1 sessions.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…er mode

A mapped agent that does not advertise, refuses, or does not report the
native mode for the thread's runtime mode now fails the session open with a
message naming the agent and mode, unless the thread is in full access,
where any mode the agent stays in is stricter and the mismatch only warns.
Otherwise an agent like Goose, which starts in `auto`, would run
approval-required threads without ever asking.

A rejected `session/set_config_option` for the mode (method-not-found or
invalid params) falls back to `session/set_mode` when the agent also
advertises modes, which Goose may need.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…of mode errors

The mode picker for mapped registry agents was hidden only when the agent
named its option `mode`; an option with another id and category `mode`
(like `permission-mode`) stayed visible and did nothing. Mode options are
now left out by category where the descriptors are built.

The session-open error and warning for a mode that did not apply no longer
include the agent's error text or reported mode id; the rejection stays as
the error cause.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@juliusmarminge
juliusmarminge force-pushed the v2/acp-client-fs-opt-in branch 2 times, most recently from 6cb5ed0 to c8ec40f Compare September 25, 2026 23:05
Base automatically changed from v2/acp-client-fs-opt-in to t3code/codex-turn-mapping September 25, 2026 23:10
@juliusmarminge

Copy link
Copy Markdown
Member Author

Superseded by #13724: registry agents stay on the plain ACP spec (no per-agent mode table), per maintainer decision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant