Skip to content

fix(claude): Plan mode asks before writes in Auto and Full access - #17963

Open
maxwellyoung wants to merge 2 commits into
pingdotgg:mainfrom
maxwellyoung:fix/claude-plan-write-approval-v2
Open

maxwellyoung wants to merge 2 commits into
pingdotgg:mainfrom
maxwellyoung:fix/claude-plan-write-approval-v2

Conversation

@maxwellyoung

@maxwellyoung maxwellyoung commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Problem

In Auto or Full access with Plan mode on, Claude's native plan mode routes file edits and file-modifying shell commands to T3's canUseTool callback. That callback only raises approvals for Supervised and Auto-accept edits, so for Auto and Full access it answers allow and the write goes through with no prompt. Reported and confirmed in #15503.

Change

  • Plan-mode query policies always install the permission callback.
  • In Plan mode, callback requests go through the existing approval flow instead of being allowed outright.
  • With an explicit approvalPolicy: never, the callback denies the request instead of silently allowing the write.

Read-only tools Claude approves before the callback, ExitPlanMode proposed-plan capture, and Full access outside Plan keep their current behavior.

Scope and approval

Fixes #15503. This follows the fix area in the maintainer triage (#15503 (comment)): treat edit and shell-write callback requests in Auto + Plan and Full access + Plan the way Supervised and Auto-accept edits already do. It does not change default Auto escalation (#15360). Backend only, two files.

Verification

  • cd apps/server && vp test run src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts
    • Original adapter with the new tests: 16 fail, 201 pass. The 14 new control cases pass on both.
    • With the fix: 217/217 pass.
  • The regressions drive the adapter's real permission callback and approval handling against a mocked native SDK query, modeled on the existing Auto-accept edits test. A simulated allowed-tool executor writes a real temp file; pending and denied requests leave it unchanged.
  • Aborting the turn while a Plan-mode approval is pending denies the write, and a late approval for that request fails with no pending request, so the file stays unchanged (Auto and Full access, Edit and Bash). This path predates the PR and Supervised mode uses it too; the new cases pin it for the requests this PR starts routing there.
  • Scoped vp check on both files: 0 errors, 1 warning on an unchanged layer declaration. vp run --filter t3 typecheck passes.
  • Not checked: an authenticated Claude CLI run in the full app.

Prepared with gpt-6.1-sol through Codex, rebased and verified with Claude Opus 5.5 through Claude Code in T3 Code.

🤖 Generated with Claude Code

@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 Oct 11, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR changes production Claude permission enforcement for Plan-mode file edits and shell commands, including whether those operations are allowed to execute. The implementation is tightly scoped and well tested, but the authorization-sensitive runtime impact warrants human review.

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

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Oct 11, 2026
@maxwellyoung
maxwellyoung force-pushed the fix/claude-plan-write-approval-v2 branch from 0cff6e7 to d518b40 Compare October 11, 2026 01:19
@coderabbitai

coderabbitai Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f8b7489d-1acb-40f3-a2df-a00d7f9ac813

📥 Commits

Reviewing files that changed from the base of the PR and between d518b40 and b2fa07b.


📒 Files selected for processing (1)
  • apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts

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



📝 Walkthrough

Walkthrough

Plan-mode tool requests now use the permission callback in all runtime modes. When approvals are disabled, the callback denies the request. Tests cover approval decisions, Full-access behavior outside Plan mode, and proposed-plan capture.

Changes

Plan-mode approval handling

Layer / File(s) Summary
Plan-mode permission policy
apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts
The query policy always installs the permission callback in Plan mode. When approvals are disabled, the callback denies the tool request with a specific message and tool-use ID.
Approval and plan-capture tests
apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts
Tests cover Edit and Bash approval requests across runtime modes, acceptance and cancellation, denial when approvals are disabled, Full-access behavior outside Plan mode, and proposed-plan capture. The harness records runtime-request and plan-update events.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: juliusmarminge


Merge Risk | ⚪ Minimal · up to b2fa0

Merge Risk: ⚪ Minimal · up to b2fa0

The Plan-mode approval change is ready to merge after normal checks; no unresolved issue was identified.

Security Architecture Review

Security architecture risk: 🔵 Low · up to b2fa0

The change strengthens approval requirements for planned file edits and shell commands without increasing tool privileges. Risk is low, with cancellation during an interrupted turn not demonstrated end to end.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The protected outcomes include model-requested file changes and shell execution through the existing Claude process. The change adds an approval control, not filesystem or privilege confinement; effective command reach remains dependent on the existing execution and sandbox policy, rather than the temporary workspace used by the tests.

Trust Boundaries and Controls

  • observed — The adapter denies requests without an active turn. For approval-gated requests, the decision controls the callback’s original tool input and tool-use identity. Generated request identity includes the provider turn, although the response handler itself resolves by request ID without independently checking current-turn ownership. This request lifecycle predates the PR and is now used by additional Plan-mode requests.

Resilience and Maintainability Implications

  • observed — Signal cancellation resolves pending approval as cancel and removes its entry. Actual turn interruption checks turn identity, calls SDK interrupt and close, and waits for closure; pending approvals are not explicitly resolved there or in turn finalization. The new abort tests directly trigger the callback signal, leaving production interrupt-to-signal delivery unproven. This reliance is pre-existing, not an established new bypass.

Pre-merge checks | Passed 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description check Passed The description covers the problem, change, scope and approval, verification results, limitations, test commands, and agent details. It aligns with the repository template and PR objectives.
Title check Passed The title clearly and concisely identifies the Claude Plan mode fix and the affected Auto and Full access modes.
Linked Issues check Passed Issue #15503 requires Plan-mode writes to avoid silent authorization. The PR installs the Claude permission callback for Plan mode, routes callback requests through the approval flow, and denies reque…
Out of Scope Changes check Passed The source change is limited to Claude query-policy and Plan-mode callback handling in ClaudeAdapterV2.ts. The test change adds harness event capture and regression cases for the affected approval b…

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • 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.

@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 11, 2026 — with ChatGPT Codex Connector
@macroscopeapp
macroscopeapp Bot dismissed their stale review October 11, 2026 06:25

Dismissing prior approval to re-evaluate b2fa07b

@maxwellyoung

Copy link
Copy Markdown
Contributor Author

@coderabbitai Thanks for raising pending approvals on interruption. I checked the path: awaitClaudeApprovalDecision races the decision against the callback's abort signal and resolves to cancel, which becomes a deny. Its ensuring drops the request from pendingRuntimeRequests, so a later respondToRuntimeRequest fails with "No pending Claude runtime request" and can't allow the write. Supervised mode already uses this path; this PR just routes Auto and Full access Plan writes into it.

b2fa07b adds regressions for that case: Auto and Full access, Edit and Bash, abort while the approval is pending. Each one gets a deny, the late accept is rejected, and the file is unchanged. Adapter suite: 217/217 with the fix. The original adapter fails 16, the 4 new cases among them, because it allows the write without asking.

@coderabbitai

coderabbitai Bot commented Oct 11, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
fd -t f '^ClaudeAdapterV2(\.test)?\.ts$'
ast-grep outline apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts --items all --match 'awaitClaudeApprovalDecision|respondToRuntimeRequest|canUseToolEffect' --view expanded
ast-grep outline apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts --items all --match 'plan|Plan|abort|Abort' --view expanded
rg -n -C 18 'awaitClaudeApprovalDecision|No pending Claude runtime request|pendingRuntimeRequests\.delete' apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts
rg -n -C 30 'late accept|abort while|abort.*pending|pending.*abort|file is unchanged|controller\.abort|abortController\.abort' apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts

Length of output: 11463


@maxwellyoung Thanks for the clarification and regression tests. The inspected code confirms that abort cancels the approval and cleanup removes the pending request. A later acceptance cannot authorize the write.

The four new cases cover Auto and Full access with both Edit and Bash. They check denial, rejected late acceptance, and unchanged file contents. I did not rerun the suite.

The existing cancellation path already handles my concern. I withdraw the finding.


✏️ Learnings added
Learnt from: maxwellyoung
URL: https://github.com/pingdotgg/t3code/pull/17963

Timestamp: 2026-10-11T06:25:48.584Z
Learning: In apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts, awaitClaudeApprovalDecision races the approval decision against the callback AbortSignal and returns cancel on abort. Its caller uses Effect.ensuring to remove the request from pendingRuntimeRequests, and respondToRuntimeRequest rejects later responses to that request. Auto and Full access Plan-mode writes use this existing approval cancellation path. Regression tests in apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts cover pending approval aborts for Edit and Bash in both runtime modes, including denial, rejected late acceptance, and unchanged file contents.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

You are interacting with an AI system.

This branch has not been deployed

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

Labels

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews 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.

[Bug]: Claude Auto + Plan silently approves native plan-mode write requests (V2)

2 participants