Skip to content

fix(server): ACP auto-approvals allow once instead of always - #13802

Merged
juliusmarminge merged 1 commit into
t3code/codex-turn-mappingfrom
t3code/acp-auto-approve-once
Sep 26, 2026
Merged

juliusmarminge merged 1 commit into
t3code/codex-turn-mappingfrom
t3code/acp-auto-approve-once

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Sep 26, 2026 •

Copy link
Copy Markdown
Member

When T3's runtime policy approves an ACP permission prompt itself, it answers with the agent's allow_always option whenever one is offered. Agents may keep that answer beyond the session, so each auto-approved prompt can become a lasting grant outside T3's policy.

The recorded grok_monitor fixture shows it. Under Full access, Grok's monitor prompt offers always-allow ("Yes, and don't ask again for bash commands") and allow-once, and T3 answered always-allow. Grok records that as a project-wide grant in ~/.grok/sessions/<cwd>/permission_*.toml (crates/codegen/xai-grok-workspace/src/permission/grants.rs record_prompt_outcome, at f0e3be1). The grant then applies to that project in any later Grok session, including a Supervised thread or the Grok TUI.

What changed

selectAutoApprovedPermissionOption in AcpAdapterV2 prefers allow_once and falls back to allow_always only when the agent offers no once option. A policy approval covers one request, which is what allow_once means in the ACP spec. The change is generic ACP with no per-agent code.

Verification

  • grok_monitor replay fixture: the recorded T3 answer (an expect_outbound frame) now reads allow-once, and output.ts asserts it. I hand-edited that one outbound frame rather than re-recording, because a fresh live recording today took a different, shorter path (Grok ended the monitor in-turn), which would change the fixture's scenario. Everything Grok sent is unchanged.
    • Without the fix: the replay stalls at the permission response (T3 sends always-allow, the transcript expects allow-once) and times out.
    • With the fix: passes.
  • vp test run src/orchestration-v2/testkit/OrchestratorReplayFixtures.integration.test.ts -t "(registry|cursor|antigravity|grok)": 23 passed. The registry fixture already answers allow-once when it is offered first.
  • vp test run src/orchestration-v2/Adapters/{Grok,Acp,AcpRegistry,Antigravity,Cursor}AdapterV2.test.ts src/provider/acp/AcpClientPolicy.test.ts: 169 passed.
  • vp exec tsc --noEmit -p . in apps/server: no errors. knip --workspace apps/server --exports: clean. vp lint on touched files: one pre-existing warning on an untouched import.
  • Not run: repo-wide checks.

Related: #13796 (the user-facing "Always allow this session" on Grok bash prompts).

Model: Claude Opus 5.5 (Claude Code)

🤖 Generated with Claude Code


Devin Review

When the runtime policy approves an ACP permission prompt itself, the
adapter answered with the agent's `allow_always` option whenever one was
offered. Agents may persist that answer beyond the session: Grok saves its
bash and monitor `always-allow` as a project-wide grant, so every prompt
T3 auto-approved turned into a lasting allow outside T3's policy.

Answer with `allow_once`, and use `allow_always` only when the agent offers
no once option. The grok_monitor fixture's recorded answer is updated to
the option T3 now sends, and it asserts the choice.

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:S 10-29 changed lines (additions + deletions). labels Sep 26, 2026
@github-actions

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: 9c9171e · 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 26, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — The PR narrowly changes ACP auto-approved permission responses from persistent allow-always grants to single-request allow-once grants, with focused replay coverage. Because this changes authorization and permission lifetime behavior, it warrants human review.

You can add or adjust custom eligibility rules. Learn more.

@juliusmarminge
juliusmarminge merged commit 98400c6 into t3code/codex-turn-mapping Sep 26, 2026
24 of 25 checks passed
@juliusmarminge
juliusmarminge deleted the t3code/acp-auto-approve-once branch September 26, 2026 18:14
juliusmarminge added a commit that referenced this pull request Sep 26, 2026
Brings in the V2 bug-hunt fixes merged since this branch was cut
(#13541, #13775, #13786, #13793, #13796, #13802). No conflicts.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 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