Skip to content

fix(agent): allow bypassPermissions switch in sandboxed sessions#1858

Merged
VojtechBartos merged 9 commits into
mainfrom
posthog-code/fix-plan-approval-mode-switch
Apr 24, 2026
Merged

fix(agent): allow bypassPermissions switch in sandboxed sessions#1858
VojtechBartos merged 9 commits into
mainfrom
posthog-code/fix-plan-approval-mode-switch

Conversation

@VojtechBartos
Copy link
Copy Markdown
Member

Problem

Approving a plan with "Yes, bypass all permissions" in a cloud run did nothing: the agent refused to proceed, the footer stayed on Plan Mode, and the next tool call re-prompted.

Root cause

Three stacked gaps:

  1. SDK locked out of bypass. The agent launches the Claude SDK with allowDangerouslySkipPermissions: !IS_ROOT. Cloud sandboxes run as root, so this was always false — the SDK rejected every bypass switch. The UI-side gate already handled this via !IS_ROOT || IS_SANDBOX (sandbox overrides root because the blast radius is a throwaway container). The SDK gate didn't. Aligned them.

  2. Cloud UI never saw the mode change. Cloud runs receive session/update notifications through an SSE relay that only classifies task_run_state and permission_requestconfig_option_update fell through to the display log, never updating the store. The hydration path already knew to extract it; added the same scan to the live-update path.

  3. UI auto-reverted bypass → default. A SessionView effect force-reset the mode when the user's local allowBypassPermissions preference was off. Correct for local sessions; wrong for cloud (sandboxed, the agent's ALLOW_BYPASS gate already said bypass is safe, user just explicitly picked it). Skip the revert when isCloud.

Also consolidated plan-approval mode-switch handlers onto the existing applySessionMode + updateConfigOption pattern used by setSessionMode, dropping a legacy current_mode_update emit the renderer no longer listens for.

The plan-approval prompt offers "Yes, bypass all permissions" whenever
ALLOW_BYPASS is true (`!IS_ROOT || IS_SANDBOX`), but the Claude SDK session
itself was launched with allowDangerouslySkipPermissions: !IS_ROOT. In cloud
sandboxes the process runs as root with IS_SANDBOX=1, so the UI offered
bypass while the SDK always rejected it with "Cannot set permission mode to
bypassPermissions because the session was not launched with
--dangerously-skip-permissions". Match the UI gate so the two agree.

Generated-By: PostHog Code
Task-Id: 07bd3105-7746-440a-a82c-86e40803062d
applyPlanApproval and handleEnterPlanModeTool each did the mode switch
by hand — mutating session.permissionMode, calling setPermissionMode, and
emitting a legacy current_mode_update that the renderer no longer listens
for (the comment at service.ts:1032 literally says "replaces
current_mode_update"). Meanwhile setSessionMode in claude-agent.ts already
had the right pattern: applySessionMode + updateConfigOption. Collapse the
two handlers onto that pattern by exposing applySessionMode on the
ToolHandlerContext. No behavior change for surfaces still wired up; it
just eliminates one dead signal path that could race against the live one.

Generated-By: PostHog Code
Task-Id: 07bd3105-7746-440a-a82c-86e40803062d
Cloud runs deliver session/update notifications via the SSE stream as
generic log entries (cloud-task/service.ts only classifies task_run_state
and permission_request; everything else goes to pendingLogEntries).
convertStoredEntriesToEvents appends them to the display log but never
looks inside for state-updating notifications, so mid-run config changes
(e.g. plan approval switching the mode to bypassPermissions) never reach
the session store and the footer mode selector stays frozen on the
initial value.

Scan incoming entries for config_option_update and update the store the
same way parseSessionLogs already does during hydration.

Generated-By: PostHog Code
Task-Id: 07bd3105-7746-440a-a82c-86e40803062d
SessionView watched currentModeId and reset it to "default" whenever the
session landed on bypassPermissions without the local allowBypassPermissions
setting enabled. That gate makes sense for local runs (where the user's
machine is the blast radius) but is wrong for cloud runs — the agent's own
check (ALLOW_BYPASS = !IS_ROOT || IS_SANDBOX) already permits bypass in the
sandbox, and auto-reverting clobbers the user's explicit plan-approval
choice, stranding them in Plan Mode. Skip the revert when isCloud is true.

Generated-By: PostHog Code
Task-Id: 07bd3105-7746-440a-a82c-86e40803062d
@VojtechBartos VojtechBartos self-assigned this Apr 23, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 23, 2026

Prompt To Fix All With AI
This is a comment left during a code review.
Path: packages/agent/src/adapters/claude/permissions/permission-handlers.ts
Line: 174-175

Comment:
**`applySessionMode` invoked twice per plan approval**

`context.applySessionMode(optionId)` is called here, and then `context.updateConfigOption("mode", optionId)` internally calls `this.applySessionMode(resolvedValue)` again (see `claude-agent.ts` line ~974). This means `session.permissionMode` is set and `session.query.setPermissionMode` is awaited twice per approval.

The same redundancy exists in `setSessionMode`, so it's a pre-existing pattern and causes no correctness issue (the operation is idempotent). But now that both call sites go through `applySessionMode` you could consolidate by letting `updateConfigOption` be the sole entry point (it already calls `applySessionMode` internally), removing the explicit first call.

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "fix(code): don't auto-revert bypass mode..." | Re-trigger Greptile

Two copies of the same expression (!IS_ROOT || !!process.env.IS_SANDBOX)
lived in permission-options.ts and execution-mode.ts. Inconsistency
between them produced the footer/plan-approval drift we just fixed, so
collapse into a single exported constant next to IS_ROOT with a doc
comment explaining the capability-vs-containment reasoning. Both call
sites now import it instead of re-deriving.

Generated-By: PostHog Code
Task-Id: 07bd3105-7746-440a-a82c-86e40803062d
Codex's setSessionConfigOption mutated the native mode in codex-acp and
updated local sessionState silently — no ACP notification. That left
agent-server's session.permissionMode cache (populated only via
current_mode_update) stuck at the initial mode for the lifetime of the
session. shouldRelayPermissionToClient uses that cache to decide relay,
so user-initiated mode switches via the footer (e.g. full-access →
read-only) didn't update the relay decision, and subsequent edit/bash
requests fell through the stale-cache path to silent auto-approve.

Emit current_mode_update on mode changes, same pattern Claude already
uses in setSessionConfigOption.

Generated-By: PostHog Code
Task-Id: 07bd3105-7746-440a-a82c-86e40803062d
@tatoalo
Copy link
Copy Markdown
Contributor

tatoalo commented Apr 24, 2026

agent-server still listens for that notification to update session.permissionMode, right?

applySessionMode or updateConfigOption are not emitting current_mode_update. So after a cloud plan approval, the agent-server's cached mode is frozen at whatever it was before plan mode?

I tried on a plan-initiated cloud run, then approved the plan with manual approval needed but then agent did the change autonomously 👀

… changes

The agent-server only updates its cached permissionMode when it receives
a current_mode_update session update, but updateConfigOption only emitted
config_option_update. This caused the agent-server's relay decisions to
use a stale mode after plan approval, silently auto-approving tool calls.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@VojtechBartos VojtechBartos enabled auto-merge (squash) April 24, 2026 12:02
@VojtechBartos VojtechBartos merged commit f9f78da into main Apr 24, 2026
15 checks passed
@VojtechBartos VojtechBartos deleted the posthog-code/fix-plan-approval-mode-switch branch April 24, 2026 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants