Skip to content

fix(daemon): escape underscores in Claude CWD path encoding#38

Merged
artyomsv merged 1 commit into
masterfrom
fix/claude-session-cwd-encoding
Jun 5, 2026
Merged

fix(daemon): escape underscores in Claude CWD path encoding#38
artyomsv merged 1 commit into
masterfrom
fix/claude-session-cwd-encoding

Conversation

@artyomsv

@artyomsv artyomsv commented Jun 5, 2026

Copy link
Copy Markdown
Owner

Summary

Two fixes for the silent Claude Code session-restore regression diagnosed from the production daemon log: after a daemon restart, every claude-code pane respawned with `--continue` instead of `--resume <session_id>` even though the snapshotted ids were valid.

Diagnosis (from `/.quil/quild.log` + `/.claude/projects/`)

The respawn args ended with `--continue`. `claudeResumeTemplate` only emits `--continue` when both probe branches fail: hook-recorded id AND preassigned id. Both probes route through `claudeSessionFileExists` → `escapeClaudeCWD(cwd)` → `os.Stat(...).jsonl`. The probe path was being built as `/.claude/projects/-Users-Foo_Bar/...` while Claude actually writes to `/.claude/projects/-Users-Foo-Bar/...` — underscore vs dash. Every macOS user whose home dir contains an underscore was silently hit; Windows users weren't because their test paths had no underscores.

Changes

`escapeClaudeCWD` — the bug

Extended the `strings.NewReplacer` to also map `_` → `-`, matching Claude Code's per-project directory naming under `~/.claude/projects/`. Confirmed empirically against the real directories on macOS (Jun 2026).

Three regression rows added to `TestEscapeClaudeCWD`:

  • `/Users/Artjoms_Stukans/Projects/crypto-finance` → `-Users-Artjoms-Stukans-Projects-crypto-finance`
  • Single-underscore segment
  • Multiple underscores

Three negative rows pin the conservative scope (`.`, space, uppercase preserved) so if Claude is later observed encoding any of those, the replacer and these assertions get extended together — the assumption shift is reviewable as one diff.

`Daemon.refreshPluginStateFromHooks()` — defense-in-depth

Per the user's F1 → Stop daemon idea: `Stop()` now calls a new helper before the final snapshot to copy each pane's live SessionStart-hook-recorded session id into `PluginState["session_id"]`. Without this, `workspace.json` always carries the original preassigned id and is stale after any `/clear`, `/resume`, or compaction. If the hook file is later lost (sessions dir wiped, plugin uninstalled, manual cleanup) the restore probe has nothing to fall back to.

  • Only `claude-code` and `opencode` panes are touched (terminal panes have no session-id concept)
  • Empty / error hook reads preserve the existing `PluginState["session_id"]` — clobbering with `""` would force `--continue` even when a usable preassigned id is still on disk
  • Lazy-allocates `PluginState` when nil
  • `PluginMu.Lock()` per pane for race-safety against any future call site that runs concurrently with the PTY output goroutine. At the current call site (after `d.server.Stop()` + `d.collectorWG.Wait()`) no goroutine can mutate panes, so the lock is belt-and-braces

Documentation

  • `.claude/CLAUDE.md` — appended one sentence to the existing claude-code session-id rotation bullet documenting both the encoding fix and the shutdown refresh
  • `CHANGELOG.md` — `### Fixed` entry for the encoder bug, `### Internal` entry for the shutdown helper

Test plan

  • `bash scripts/dev.sh test` — all 16 packages pass
  • `bash scripts/dev.sh test-race` — race detector clean (1.1 s total)
  • `bash scripts/dev.sh vet` — clean
  • Both new tests are explicitly not `t.Parallel()` — they mutate the package-level `readHookSessionIDFn` / `readOpencodeSessionIDFn` vars. Comment + `saveHookStubs(t)` helper documents the constraint
  • Smoke test on real macOS dev binary: kill the daemon, restart with the patched build, verify every claude-code pane respawns with `--resume ` in the daemon log (not `--continue`)
  • Verify `workspace.json` after F1 → Stop daemon carries the live (post-`/clear`) session ids, not the original preassigned ones

Test coverage added

  • `TestEscapeClaudeCWD` — 6 new rows (3 regression + 3 negative scope-pinning)
  • `TestDaemon_RefreshPluginStateFromHooks` — happy path with 4 panes (claude, opencode, terminal-skip, nil-PluginState)
  • `TestDaemon_RefreshPluginStateFromHooks_EmptyHookIDPreservesExisting` — table-driven across empty-string-no-error and error-read branches

Considered and dropped

  • Generalize to `[^A-Za-z0-9-] → "-"` — defensible to defer until empirically forced. No concrete Claude-encoded path with dots or spaces observed; generalizing now would silently change behavior for paths Claude may NOT remap. The negative test rows lock in the conservative scope so a future change is reviewable.
  • Plugin-registry-based dispatch in `refreshPluginStateFromHooks` — out of scope. Mirrors the existing stringly-typed dispatch in `resumeTemplateFor`. Future refactor could route both through a `PanePlugin.SessionIDReader` capability.
  • Path-traversal defense-in-depth on `escapeClaudeCWD` — `pane.CWD` is already validated via `EvalSymlinks` + `os.Stat` at attach time, the use is read-only `os.Stat`, and the daemon process can already reach any directory the user can. Not exploitable.

claudeSessionFileExists silently returned false for every macOS user
whose home dir contains an underscore, so claudeResumeTemplate's
on-disk probe never accepted the recorded session id and the resume
template fell through to --continue. Result: after a daemon restart,
each claude-code pane opened Claude's most-recent-by-CWD session
instead of the snapshotted one, forcing manual recovery.

Cause: escapeClaudeCWD mapped /, \, and : to '-' but left '_' alone.
Claude Code also maps '_' to '-', so /Users/Foo_Bar lives under
~/.claude/projects/-Users-Foo-Bar/ while Quil was probing
~/.claude/projects/-Users-Foo_Bar/.

Fix: extend the strings.NewReplacer to also handle '_'. Confirmed
against real on-disk directories. Three regression rows added to
TestEscapeClaudeCWD plus three negative rows pinning the deliberately
conservative scope ('.', ' ', uppercase preserved) — if Claude is
later observed encoding any of those, extend the replacer and update
the assertions in the same diff so the shift is reviewable.

Also: Daemon.Stop() now calls refreshPluginStateFromHooks() before the
final snapshot, copying the live SessionStart-hook-recorded session id
into PluginState["session_id"] for every claude-code and opencode
pane. Without this, workspace.json carries the original preassigned
id forever and is stale after any /clear, /resume, or compaction; if
the hook file is later lost (sessions dir wiped, plugin uninstalled)
the restore probe falls back to --continue.

Tests cover the happy path (claude + opencode + terminal-pane-skip +
lazy nil-PluginState allocation), the empty/error hook read preserves
existing values (table-driven across both branches), and three
underscore-bearing path encodings plus three negative cases.

Concurrency: refreshPluginStateFromHooks runs after server stop and
collectorWG.Wait() in Stop(), so no goroutine can mutate panes; the
per-pane PluginMu.Lock() is kept for race-safety against any future
call site. Race detector passes.
@artyomsv artyomsv merged commit 4e58a6d into master Jun 5, 2026
5 checks passed
@artyomsv artyomsv deleted the fix/claude-session-cwd-encoding branch June 5, 2026 14:42
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