Repository navigation
Replace permission modes with approval presets - #797
Merged
Merged
Conversation
This was referenced Jul 22, 2026
SawyerHood
added a commit
that referenced
this pull request
Jul 23, 2026
Fixes the silent-drop failure mode reported in #853, plus the 0.33.0 change behind that report's first error. ## The silent drop had a mechanism @amadad correctly identified that `handleLine` drops any request that fails schema validation (`if (!request) return`), turning every mismatch into an undebuggable 30s timeout. But that alone does not explain a *well-formed* `turn/start` vanishing. This does: Both directions number their outgoing requests with a plain integer counter from 1 — `runtime.ts:230` (`nextRequestId = 1`) and `bridge.ts:298` (`toolCallRequestIdCounter = 0`, then `+= 1` per forwarded tool call and per permission request). They share one id space on a bidirectional channel. Meanwhile `decodeBridgeJsonRpcResponse` accepted an inbound **request** as a success response: the response schemas are non-strict `z.object`s, so the extra `method`/`params` are allowed, and `result: z.unknown()` also matches a *missing* key. Since `handleLine` checks the response branch first, a daemon request whose id collided with an outstanding tool call or permission request settled that pending promise with garbage and returned **before reaching the request decoder** — no reply, no log, caller times out 30s later. That matches the reported forensics exactly: bytes consumed off the socketpair (`Recv-Q`/`Send-Q` 0), bridge idle in `epoll_wait`, handler never invoked. Fixed by discriminating on `method`, which is what separates the two per the JSON-RPC spec and what the daemon side already does (`parseJsonRpcLine` checks `!parsedMethod`). This also fixes the pi and acp bridges, which share the decoder and the same counter pattern. ## Remaining decode failures are now loud Both of @amadad's suggestions, implemented: - `decodeClaudeCodeJsonRpcRequest` returns a discriminated result instead of `null`, so `handleLine` answers **-32602** carrying the zod issue summary (e.g. `params.workflowsEnabled: expected boolean, received undefined`) and logs to stderr, which the runtime already captures into provider diagnostics. - Unknown methods get **-32601**. The known-method set is derived from the schema union rather than hardcoded, so it cannot drift. ## Sandbox degrades instead of aborting The approval-preset migration in #797 rewrote `readonly` **and** `workspace-write` defaults to `accept-edits` (`0078_permission_modes.sql`), and `accept-edits` enables the Claude SDK OS sandbox — which 0.0.32's `readonly` mapping did not. The SDK defaults `failIfUnavailable` to `true`, so an install that previously defaulted to `readonly` lost every session on a host without `bubblewrap`. That is the "sandbox deps missing" error in the report. Now `failIfUnavailable: false`. `autoAllowBashIfSandboxed` only auto-approves while the sandbox is actually active, so degrading costs the session its auto-allow and falls back to bb's own `canUseTool` gating rather than running unsandboxed. Worth deciding separately whether the headless Linux install should declare `bubblewrap`/`socat` as dependencies, since a degraded sandbox is a quieter posture than the operator may expect. ## Two things I did *not* change - **Empty `inputGroups` group.** The bridge requires `.min(1)` per group and the adapter maps groups through `stripClaudePlanCommandInput`, so I expected a `/plan`-only group to empty and take the turn down. It cannot: `removeCommandMentionsFromPromptInput` is a `.map` that rewrites entry text and never drops entries. I implemented the fix, found the premise wrong, and reverted it. If such a group ever arrives from upstream, it is now a `-32602` naming `inputGroups`. - **Renumbering the bridge's outgoing ids.** Once requests and responses are discriminated correctly, colliding ids are harmless — each side matches responses against its own outgoing requests, so the directions are independent id spaces. Renumbering would have changed `callId: call-${requestId}`, which is carried in tool-call payloads, for no correctness gain. ## Testing `turbo run test typecheck lint --filter=@bb/agent-runtime` — 798 tests, clean. Each new test was verified to fail against the unfixed code: - a `turn/start` whose id collides with a pending `AskUserQuestion` is still dispatched, and the pending request is not settled by it - a schema-invalid `turn/start` gets `-32602` naming the offending field; an unknown method gets `-32601` - `decodeClaudeCodeJsonRpcRequest` reports `unknown_method` and `invalid_params` distinctly No `HOST_DAEMON_PROTOCOL_VERSION` bump: the bridge ships inside the daemon bundle, and nothing on the server/daemon wire changed. I cannot prove from here that the id collision is @amadad's exact trigger — that needs a turn with a tool call or permission request outstanding when the daemon sends its next request. It is the only mechanism I found that reproduces their forensics, and the `-32602` path is what will pin it definitively on their machine: the next failure names a field instead of timing out. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation
Known unrelated failure