Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-08-07
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# agent-claude-credentials-source-install-sync-2026-08-07-13-47 (minimal / T1)

Branch: `agent/claude/credentials-source-install-sync-2026-08-07-13-47`

Completes #139, whose review raised a HIGH that its own merge did not carry.

## The HIGH

#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:

- `install.ts` — `runtimeDirFor(profile.name, agent)`, matching
`prepareRuntime`'s own `runtimeKey ?? profile.name` default.
- `sync.ts` — `runtimeDirFor(key, agent)`; `key` is already the `runtimeKey`
passed to `prepareRuntime` two lines below.

## Why it is a separate PR

The fix was committed on #139's branch as `91c6601d` but never reached the
remote before the PR merged — main got only the narrowing commit. Verified after
the fact on `origin/main`: `isSelfOverlaySource` present (3 hits), `runtimeDir:`
in install.ts / sync.ts absent (0). Cherry-picked here onto the post-#139 main.

## Also carried

The comment on #139's LOW finding, explaining why the wiring test asserts
`not.toBe(target)` rather than a concrete fall-through path: `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 on any machine and stays
mutation-proof.

## Verification

- `bun run typecheck` — clean.
- `bun run lint` — warnings pre-existing; zero in the touched files.
- `bun test src/lib/runtime-install.test.ts` — 23 pass / 0 fail.
- `bun test` (full), branch vs base in the same worktree and shell — failing set
compared both directions.

## Cleanup

- [ ] Run: `gx branch finish --branch agent/claude/credentials-source-install-sync-2026-08-07-13-47 --base main --via-pr --gate-review --review-provider claude --wait-for-merge --cleanup`
- [ ] Record PR URL + `MERGED` state in the completion handoff.
- [ ] Confirm sandbox worktree is gone (`git worktree list`, `git branch -a`).
8 changes: 7 additions & 1 deletion src/commands/install.ts
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,13 @@ async function materializeProfile(profileName: string, agent: AnyAgent, force: b
profile,
agent,
credentialsSource: agent === "claude-code"
? await resolveClaudeCredentialsSource({ healFromRuntime: false })
? await resolveClaudeCredentialsSource({
healFromRuntime: false,
// 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),

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 runtimeDirsrc/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.

})
: undefined,
});
return { profile: profileName, agent, targetDir: result.runtimeDir, status: result.rebuilt ? "rebuilt" : "cached" };
Expand Down
9 changes: 8 additions & 1 deletion src/commands/sync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {
prepareRuntime,
resolveClaudeCredentialsSource,
runtimeAgentSubdir,
runtimeDirFor,
} from "../lib/runtime-install";

const RUNTIME_ROOT = join(configDir(), "runtime");
Expand Down Expand Up @@ -143,7 +144,13 @@ export async function run(args: string[]): Promise<number> {
runtimeKey: key,
credentialsSource:
agent === "claude-code"
? await resolveClaudeCredentialsSource({ healFromRuntime: false })
// `key` is the runtimeKey passed above, so this is exactly the
// dir this iteration will write — a `cue sync` run from inside a
// cue session must not overlay that runtime onto itself.
? await resolveClaudeCredentialsSource({
healFromRuntime: false,
runtimeDir: runtimeDirFor(key, agent),
})
: undefined,
});
rebuilt = rebuilt || out.rebuilt;
Expand Down
5 changes: 5 additions & 0 deletions src/lib/runtime-install.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,11 @@ describe("pickClaudeCredentialsSource", () => {
else process.env.CLAUDE_CONFIG_DIR = original;
});

// Asserted as "not the target" rather than a concrete path: which fall-through
// branch wins (~/.claude, or an authmux profile) depends on the machine, but
// none of them can return `target` — only the rejected `CLAUDE_CONFIG_DIR`
// early-return could. `os.homedir()` reads the passwd entry, not $HOME, so
// there is no cheap way to pin the branch without a test-only injection point.
test("refuses CLAUDE_CONFIG_DIR when it is the dir being rebuilt", async () => {
process.env.CLAUDE_CONFIG_DIR = target;
expect(await pickClaudeCredentialsSource({ runtimeDir: target })).not.toBe(target);
Expand Down
Loading