Skip to content

fix(cli): stop forwarding a literal 'default' permission mode to Claude - #1733

Open
f-liva wants to merge 2 commits into
slopus:mainfrom
f-liva:fix/claude-default-permission-mode-shadowing
Open

fix(cli): stop forwarding a literal 'default' permission mode to Claude#1733
f-liva wants to merge 2 commits into
slopus:mainfrom
f-liva:fix/claude-default-permission-mode-shadowing

Conversation

@f-liva

@f-liva f-liva commented Aug 28, 2026

Copy link
Copy Markdown

Fixes the half of #1695 that lives in the CLI.

The problem

mapToClaudeMode returns the literal 'default' unchanged, so claudeRemote.ts puts it into sdkOptions.permissionMode and the child is spawned with an explicit flag:

claude --output-format stream-json … --permission-mode default --settings /…/session-hook-1866005.json

A CLI flag outranks the settings file, so permissions.defaultMode in ~/.claude/settings.json is overridden. On my machine that is "auto"; the session lands in default instead and asks about nearly every tool call. permissions.allow still applies — I verified that a rule-matching ls runs unprompted — so the prompting is entirely the lost defaultMode.

That makes forwarding the literal strictly worse than sending nothing: an omitted field would have let the user's own configuration stand.

Measured on one session, all Bash calls, none matching an allow rule:

$ grep -c "Permission request sent" ~/.happy/logs/2026-08-28-16-58-56-pid-1866005.log
11

This is the same root cause @chaehyun2 traced in the issue thread; I hit it independently on a second machine and their analysis matches what I see here.

Why fix it in the CLI

4cdb8b1 already taught the app to omit the field for Default, and mapToClaudeMode's own doc comment states the contract: "Undefined is a meaningful value, not a missing one: it is how 'Default' reaches the SDK, which then applies Claude's own configuration."

But the CLI still honors the literal if it receives one, and it does receive one — any app build predating that commit sends it on every spawn and resume. Normalizing at the SDK boundary makes the contract hold regardless of which client is on the other end, which is the point of having a boundary.

The change

  • mapToClaudeMode maps Claude's 'default' to undefined, alongside the existing undefined case.
  • Codex's safe-yolo and read-only still map to a literal 'default'. For them "ask first" is the policy the user picked, not the absence of one, so they must not fall back to whatever defaultMode the user configured for Claude — that could be more permissive than what they chose.
  • The three overload signatures collapse into one, since PermissionMode no longer guarantees a defined result.
  • PermissionHandler.handleModeChange guards on the mapped value rather than the raw one, so switching to Default no longer pushes undefined into a live query. Its comment already described this behavior ("Only a concrete mode is pushed"); the guard now matches it.

Tests

src/claude/utils/permissionMode.test.ts — the 'default' pass-through expectation is replaced by its inverse, plus a case pinning the Codex ask-first modes so a later simplification cannot fold them into the same branch.

$ npx vitest run src/claude/utils/
 Test Files  14 passed (14)
      Tests  186 passed (186)
$ npx tsc --noEmit
(clean)

Full suite: 814 passed, 12 failing — all pre-existing and unrelated (difftastic/ripgrep binaries absent in my sandbox, and the Codex/daemon integration suites needing the docker environment). None touch the permission path.

Not included

Two adjacent things I saw while tracing, left alone to keep this reviewable:

  1. runClaude.ts adds --dangerously-skip-permissions only when the initial mode is bypass, so a mid-session switch to YOLO logs Cannot set permission mode to bypassPermissions because the session was not launched with --dangerously-skip-permissions. Happy's own handler auto-approves anyway via isClaudeBypassEquivalent, so the toggle works in practice and only the SDK-level sync is a no-op.
  2. In claudeRemoteLauncher.ts the replayed pending batch returns without restoring modeHash/mode, which the restart detector below depends on.

@chaehyun2 mentioned having both on a fork — happy to defer to their PR for those.

Generated with Claude Code
via Happy

`mapToClaudeMode` passed the literal 'default' through unchanged, so the
Claude child was spawned with an explicit `--permission-mode default`. A
CLI flag outranks the settings file, so a user whose
`permissions.defaultMode` is `auto` silently lost it and was prompted on
nearly every tool call — a worse outcome than sending no mode at all.

Claude's `default` is the ambient "no override" value, the same thing
the app expresses by omitting the field, so it now maps to undefined and
the SDK applies the user's own configuration. Codex's `safe-yolo` and
`read-only` still map to a literal 'default': for them asking is the
policy that was picked, not the absence of one.

Normalizing at the SDK boundary rather than in the app also makes the
CLI robust against client builds that predate 4cdb8b1 and still send
the literal spelling on every spawn and resume.

`handleModeChange` now guards on the mapped value, so switching to
Default no longer tries to push undefined into a live query — matching
the behavior its own comment already described.

Refs slopus#1695

Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
The daemon looks up a session for `resume-happy-session` in two places:
`pidToTrackedSession` for live sessions, and `sessionIdToFinishedSession`
for ones that have already exited. Only `onChildExited` ever populates the
second map, and it fires exclusively for processes the daemon spawned
itself.

Sessions started from a terminal register through the local webhook, so
they are tracked but are not daemon children. When such a process dies,
the only thing that notices is the heartbeat's stale-PID sweep, which
dropped the entry from `pidToTrackedSession` without preserving it.
From that moment `resume-happy-session` failed with "Session <id> is not
tracked by this daemon", and stayed broken until a daemon restart
repopulated `sessionIdToFinishedSession` from `~/.happy/sessions.json`.

Route the stale-PID sweep through `onChildExited` so both exit paths
preserve the session the same way.

Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
@f-liva

f-liva commented Aug 31, 2026

Copy link
Copy Markdown
Author

Pushed a second, related daemon fix onto this branch (d0a8b05) — happy to split it into its own PR if you'd rather keep them separate.

Terminal-started sessions become unresumable once they exit.

resume-happy-session resolves a session through findTrackedSessionById, which looks at pidToTrackedSession (live) and then sessionIdToFinishedSession (exited). The second map is only ever written by onChildExited, and that runs solely for processes the daemon spawned.

A session started from a terminal registers via onHappySessionWebhook — it is tracked, and its encryption data is persisted — but it is not a daemon child. When its process dies, the only code that notices is the heartbeat's stale-PID sweep, which did a bare pidToTrackedSession.delete(pid). The session then existed in neither map, and resume from the app failed with:

Session <id> is not tracked by this daemon. It may have been started before the daemon or on another machine.

It stayed broken until the next daemon restart, which repopulates sessionIdToFinishedSession from ~/.happy/sessions.json — which is why the bug looks intermittent.

The fix routes the stale-PID sweep through onChildExited so both exit paths preserve the session identically.

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.

1 participant