fix(runtime): pass the target runtime dir from install and sync too - #140
Conversation
#139 made the self-overlay guard opt-in via options.runtimeDir. launch.ts passes it; `cue install` and `cue sync` do not — and both rebuild runtimes through prepareRuntime(). So either command run from inside a cue session still took CLAUDE_CONFIG_DIR, its own runtime dir, as the overlay source and reproduced #137's self-referential symlinks. The guard was off exactly where the materialization happens. Both callers now pass the dir they are about to write: runtimeDirFor(profile.name, agent) in install.ts, matching prepareRuntime's own `runtimeKey ?? profile.name` default, and runtimeDirFor(key, agent) in sync.ts, where `key` is already the runtimeKey passed two lines below. This was raised on #139 and committed there as 91c6601d, but never reached the remote before that PR merged — main got the narrowing alone. Confirmed on origin/main afterwards: isSelfOverlaySource present, `runtimeDir:` in install.ts/sync.ts absent. Cherry-picked here onto the post-#139 main. Also carries the comment answering #139's LOW: the wiring test asserts not.toBe(target) rather than a concrete fall-through path because os.homedir() reads the passwd entry, not $HOME, so a temp-HOME fixture does not pin the branch (measured — it still resolved the real ~/.claude). No fall-through branch can return the target, so the assertion holds anywhere and stays mutation-proof. Full suite 33 fail on branch and base, failing sets identical both ways.
NagyVikt
left a comment
NagyVikt
left a comment
There was a problem hiding this comment.
🛡️ GitGuardex code-assist
1 finding(s) — 🔵 1 low
✅ Merge gate: pass — no blocking findings (blocks on high/critical).
| Severity | Location | Finding |
|---|---|---|
| 🔵 low | src/commands/install.ts:254 |
The new install/sync wiring ships without a regression test, which is the exact gap that let this bug reach main in the first place. |
Provider claude · commit 5e3f878
| // The dir prepareRuntime() is about to write — it defaults | ||
| // runtimeKey to profile.name. Passing it keeps a `cue install` run | ||
| // from inside a cue session from overlaying that runtime onto itself. | ||
| runtimeDir: runtimeDirFor(profile.name, agent), |
There was a problem hiding this comment.
Note
🔵 LOW · tests
The new install/sync wiring ships without a regression test, which is the exact gap that let this bug reach main in the first place. runtime-install.test.ts covers isSelfOverlaySource and pickClaudeCredentialsSource({runtimeDir}) at the unit level, but nothing asserts that materializeProfile() (install.ts:254) and the sync loop (sync.ts:152) actually pass a runtimeDir — src/commands/install.test.ts and
Why this matters
src/commands/sync.test.ts contain no runtimeDir assertion. #139 wired the guard into launch.ts only and the suite stayed green; deleting either line here would likewise stay green and silently resurrect #137's self-referential symlinks. A test that sets CLAUDE_CONFIG_DIR to runtimeDirFor(<profile>, "claude-code") and asserts the rebuilt runtime's credentials are not sourced from itself (or spies on resolveClaudeCredentialsSource for a non-undefined runtimeDir) would pin both call sites; verified correct otherwise — runtimeDirFor(profile.name, agent) matches materializeRuntime's runtimeKey ?? profile.name default, and sync's key is the runtimeKey passed two lines below.
1 participant
Summary
Test plan