diff --git a/README.md b/README.md index 90a778a..d6f2a74 100644 --- a/README.md +++ b/README.md @@ -363,6 +363,14 @@ In both modes, *where* a toggle is written depends on `$PWD` at invocation time, `claude-use check` (below) shows you which of the three would apply before you commit to a change, if you're unsure. +#### Which `claude-use` a bare command name resolves to + +`doctor`'s PATH-resolution check answers a question no other check does: is the `claude-use` your shell runs the same executable as the one producing this report? It scans PATH for the filename a bare `claude-use` would resolve to, using the same `findPathShadow` scan `shim enable` already uses for `claude`, and compares the first hit against the running executable's own PATH-visible location — collapsing the verdict back to a pass when both names turn out to be the same real file reached through a symlink. + +An earlier PATH entry winning is a **failure**, not a warning, because it invalidates the rest of the report rather than sitting alongside it: every other finding describes the binary that produced it, which in that state is not the binary your commands reach. The failure mode it exists to catch is entirely silent otherwise — a wrapper script or an abandoned install directory from an earlier channel keeps working at whatever version it was frozen at, so nothing looks broken until a config file written by the newer version trips the older one's own validation. That is not hypothetical: a hand-written wrapper from an earlier install channel, sitting ahead of `~/.local/bin` on PATH, kept re-execing a month-old binary whose copy of `IdentitySchema` predated the naming rule widening to allow `@` — so an `identity.json` a current claude-use had written was rejected outright, with nothing anywhere reporting that the running binary was not the installed one. + +The two softer verdicts are warnings rather than failures. The running executable's own directory not being on PATH at all is legitimate (an absolute-path invocation, or `npx`), and an enabled `claude` shim being shadowed still leaves the launcher reachable as `claude-use run`. + ### Debugging: `claude-use check` `claude-use check [path] [--identity ]` resolves the full cascade for the given path (default `$PWD`) and identity (default the active one), and prints the result — every entry's resolved state, which layer decided it, and which condition (if any) was evaluated and how — without touching the farm or spawning `claude` at all. This is the primary way to answer "why is X shared/hidden here" without launching a session to find out. For any `history/projects/` glob override in scope, it also flags whenever the pattern's encoded form could plausibly match more than one real path (see [Pattern matching](#pattern-matching-against-claudeprojects)), rather than resolving that ambiguity silently. @@ -375,7 +383,7 @@ It also runs three checks that don't depend on `path` at all, every time, so a r ### Debugging: `claude-use doctor` -Where `claude-use check` resolves one directory+identity's cascade in detail, `claude-use doctor` audits the whole `~/.claude-use` config graph at once — identity/directory-agnostic, no arguments needed. It validates every identity's `identity.json`, every configuration profile's own `extends` chain (catching a missing profile name or a circular `extends` before a launch would), `directory-rules.json`, `config.json`, `categories.local.json`, and `active-identity`, each against its own Zod schema and cross-referenced against each other (an identity's `defaultConfigProfile`, a directory rule's `identity`/`configProfile`, actually pointing at something real) — plus whether a real Claude Code binary is discoverable at all, whether the `claude` command shim is enabled and its recorded location still exists, and the same ambient-credential check `check` runs. One malformed file is reported as its own failure rather than aborting the rest of the audit, and the command exits non-zero if anything failed — useful as a scriptable "is everything still consistent" gate, not just an interactive debugging aid. +Where `claude-use check` resolves one directory+identity's cascade in detail, `claude-use doctor` audits the whole `~/.claude-use` config graph at once — identity/directory-agnostic, no arguments needed. It validates every identity's `identity.json`, every configuration profile's own `extends` chain (catching a missing profile name or a circular `extends` before a launch would), `directory-rules.json`, `config.json`, `categories.local.json`, and `active-identity`, each against its own Zod schema and cross-referenced against each other (an identity's `defaultConfigProfile`, a directory rule's `identity`/`configProfile`, actually pointing at something real) — plus whether a real Claude Code binary is discoverable at all, whether the `claude` command shim is enabled and its recorded location still exists, **which `claude-use` a bare command name actually resolves to** (below), and the same ambient-credential check `check` runs. One malformed file is reported as its own failure rather than aborting the rest of the audit, and the command exits non-zero if anything failed — useful as a scriptable "is everything still consistent" gate, not just an interactive debugging aid. ## Examples @@ -490,7 +498,7 @@ src/ directoryRules.ts # `claude-use rules` subcommands configure.ts # `claude-use configure` interactive picker (@clack/prompts) check.ts # `claude-use check` dry-run inspector — cascade resolution, ambient-credential/Keychain/settings-secrets diagnostics — no farm writes, no spawn - doctor.ts # `claude-use doctor` whole-tree audit — every identity/profile/extends-chain/directory-rules/config.json/categories.local.json/active-identity, aggregating rather than throwing on a broken file + doctor.ts # `claude-use doctor` whole-tree audit — every identity/profile/extends-chain/directory-rules/config.json/categories.local.json/active-identity, plus which `claude-use` PATH actually resolves to, aggregating rather than throwing on a broken file claudeShim.ts # `claude-use shim enable`/`disable` — the one explicit action that creates/removes a `claude`-named hardlink of the running executable; records claude-shim.json cli/ parsers.ts # shared CLI-flag parsing helpers (splitTopLevelCommas, parsePair, repeatable-flag collectors) @@ -617,11 +625,11 @@ the resolver's cascade and materialisation logic is exactly the kind of thing th - Conditional entries with injectable/fake mtimes, a fake resolved branch, and a fake env snapshot (never real filesystem/git/environment state, so tests aren't time-dependent, git-dependent, or slow) — a `newerThan` condition including a fresh file and excluding a stale one under the same glob, a `branch` condition applying only on a matching branch, an `env` condition applying only when the right variable is set, and a conditionally-matched subtree always being materialised rather than symlinked - A materialised directory reconciling any real (non-symlink) children written since the last resync back into `~/.claude` before re-deciding, and collapsing back into a plain symlink once its split condition no longer holds -`identityManager.ts`, `configProfiles.ts`, `directoryRules.ts`, and `configure.ts` stay thin adapters over the resolver, so most of their correctness rides on the resolver's own test coverage above. `launcher.ts` carries three separately-testable responsibilities of its own that aren't covered by the resolver's purity, and need their own coverage: translating a resolved `Map` into real filesystem side effects (creating/removing symlinks, materialising/collapsing directories, diffing against the farm's prior state, the per-identity lock and atomic-swap behaviour from [Directory rules](#directory-rules)) against a fake/in-memory filesystem; invoking the real `claude` binary via an injected `spawn` function (argv/env construction, exit-code propagation), never a real subprocess in a unit test; and the ambient-credential guard — given a fake `process.env`, refusing to proceed when any of the six named variables is set and the active identity's `allowAmbientCredential` is unset/false, proceeding when it's true, and proceeding when `CLAUDE_USE_ALLOW_AMBIENT_CREDENTIAL=1` is set for that one call regardless of the identity's own setting. +`identityManager.ts`, `configProfiles.ts`, `directoryRules.ts`, and `configure.ts` stay thin adapters over the resolver, so most of their correctness rides on the resolver's own test coverage above. The one exception is `listIdentities`, whose own tests cover a deliberate departure from the "throw a validation error and let it propagate" convention: an `identity.json` that is present but unreadable — malformed JSON, or valid JSON this version's `IdentitySchema` rejects — is reported as that one identity's own unreadable entry, so a single bad file never hides every *other* identity from `claude-use identity list` at the moment they most need to be visible. Only those two content-shaped failures are absorbed; a permission error still propagates. A wholly *absent* `identity.json` remains a silent skip rather than a problem, and both it and `doctor`'s own enumeration filter out directories whose name starts with `.`, since `IdentitySchema` requires an identity name to start with a letter or digit and a resync's own `..scratch.`/`..previous.` directories are therefore never identities to report on. `launcher.ts` carries three separately-testable responsibilities of its own that aren't covered by the resolver's purity, and need their own coverage: translating a resolved `Map` into real filesystem side effects (creating/removing symlinks, materialising/collapsing directories, diffing against the farm's prior state, the per-identity lock and atomic-swap behaviour from [Directory rules](#directory-rules)) against a fake/in-memory filesystem; invoking the real `claude` binary via an injected `spawn` function (argv/env construction, exit-code propagation), never a real subprocess in a unit test; and the ambient-credential guard — given a fake `process.env`, refusing to proceed when any of the six named variables is set and the active identity's `allowAmbientCredential` is unset/false, proceeding when it's true, and proceeding when `CLAUDE_USE_ALLOW_AMBIENT_CREDENTIAL=1` is set for that one call regardless of the identity's own setting. `check.ts`'s three always-on diagnostics get their own tests too, independent of path/cascade resolution: the ambient-credential check against a fake `process.env` (same fixture as `launcher.ts`'s guard, since they share the same detection logic); the settings-secrets advisory against a fake settings.json with populated `env`/`hooks` fields, confirming it reports counts and key names only, never values; and — since Keychain access is real OS state, not something to fake — a manual/integration-only note that the Keychain-name lookup is exercised against a real `security` call in CI on macOS runners, not unit-tested with a mock. -`doctor.ts` deliberately breaks the "throw a validation error and let it propagate" convention every other command file follows, since aggregating every check into one report — rather than aborting on the first broken file — is the whole point of the command. Its own tests cover this directly: every input (identities, configuration profiles, `directory-rules.json`, `config.json`, `categories.local.json`, `active-identity`) fed simultaneously malformed at once, asserting `runDoctor` still returns a full report with one `fail` finding per broken input rather than throwing, plus a genuine `extends` cycle correctly failing and a genuine diamond correctly not being mistaken for one. Its wiring layer sets `process.exitCode` rather than throwing or calling `process.exit()` when the report contains any failure — this is new to the codebase and not unit-tested, matching `registerCheckCommand`'s own I/O wiring being untested for the same reason. +`doctor.ts` deliberately breaks the "throw a validation error and let it propagate" convention every other command file follows, since aggregating every check into one report — rather than aborting on the first broken file — is the whole point of the command. Its own tests cover this directly: every input (identities, configuration profiles, `directory-rules.json`, `config.json`, `categories.local.json`, `active-identity`) fed simultaneously malformed at once, asserting `runDoctor` still returns a full report with one `fail` finding per broken input rather than throwing, plus a genuine `extends` cycle correctly failing and a genuine diamond correctly not being mistaken for one. The PATH-resolution section gets its own coverage for each of its three verdicts at each of the two names it reports on (a shadowed `claude-use` failing and naming both paths, a not-on-PATH executable warning instead, and an enabled-but-shadowed `claude` shim warning rather than failing), alongside `refinePathShadow`'s own unit tests for the symlink case a directory comparison alone would misreport as a shadow of itself. Its wiring layer sets `process.exitCode` rather than throwing or calling `process.exit()` when the report contains any failure — this is new to the codebase and not unit-tested, matching `registerCheckCommand`'s own I/O wiring being untested for the same reason. `claudeShim.test.ts` follows `identityManager.test.ts`'s real-temp-directory convention (a fake "own executable" file standing in for the running `claude-use` binary), rather than `doctor.ts`'s pure-function style, since `enableClaudeShim`/`disableClaudeShim` are themselves real filesystem operations, not something to keep separate from a thin wiring layer. Coverage includes the version-drift case that motivates persisting `claude-shim.json` at all (the source file overwritten in place between two `shim enable` runs, proving the marker — not the inode — is what lets the second run refresh cleanly instead of refusing), a foreign file at the target being refused without `--force` and accepted with it, and the cross-device (`EXDEV`) copy-fallback path via a small injectable `LinkFs` seam (mirroring `config/store.ts`'s own `StoreFs`/`nodeStoreFs` pattern), since a real cross-filesystem rig isn't practical in CI. One test also reproduces Homebrew's actual layout (a symlink into a separate "Cellar" directory) to confirm the shim lands next to the symlink users invoke, not buried in the directory its realpath resolves to. diff --git a/src/claudeShim.test.ts b/src/claudeShim.test.ts index aa3b9b1..7db26d8 100644 --- a/src/claudeShim.test.ts +++ b/src/claudeShim.test.ts @@ -7,6 +7,7 @@ import { ForeignClaudeEntryError, UnsupportedShimSourceError, claudeTargetFilename, + commandFilename, disableClaudeShim, enableClaudeShim, findPathShadow, @@ -50,6 +51,16 @@ describe("claudeShim", () => { }); }); + describe("commandFilename", () => { + it("adds no extension for an extensionless source, whatever the command name", () => { + expect(commandFilename("/usr/local/bin/claude-use", "claude-use")).toBe("claude-use"); + }); + + it("adds .exe for a .exe source, so doctor looks for the filename PATH would actually hold on Windows", () => { + expect(commandFilename("C:\\bin\\claude-use.exe", "claude-use")).toBe("claude-use.exe"); + }); + }); + describe("isInvokedAsClaude", () => { it("matches the extensionless POSIX name", () => { expect(isInvokedAsClaude("claude")).toBe(true); diff --git a/src/claudeShim.ts b/src/claudeShim.ts index b7faa5d..644480f 100644 --- a/src/claudeShim.ts +++ b/src/claudeShim.ts @@ -49,7 +49,12 @@ export class UnsupportedShimSourceError extends CliError { * Deliberately not "preserve whatever extension the source has" — an npm install's own file is `cli.cjs`, and a bare `claude` (not `claude.cjs`) is what makes it invocable as the expected command on POSIX, where a shebang plus the executable bit is what matters, not the filename's extension. */ export function claudeTargetFilename(ownExecutablePath: string): string { - return ownExecutablePath.toLowerCase().endsWith(".exe") ? "claude.exe" : "claude"; + return commandFilename(ownExecutablePath, "claude"); +} + +/** The same rule generalised to any command name this tool owns, so `doctor`'s PATH-resolution check can ask "what filename would a bare `claude-use` be" without restating the Windows `.exe` condition and letting the two drift apart. */ +export function commandFilename(ownExecutablePath: string, commandName: string): string { + return ownExecutablePath.toLowerCase().endsWith(".exe") ? `${commandName}.exe` : commandName; } /** diff --git a/src/doctor.test.ts b/src/doctor.test.ts index 30276a3..35affea 100644 --- a/src/doctor.test.ts +++ b/src/doctor.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import { formatDoctorReport, + refinePathShadow, runDoctor, type DoctorConfigProfileInput, type DoctorIdentityInput, @@ -22,6 +23,7 @@ function baseParams(overrides: Partial = {}): RunDoctorParams { activeIdentity: { path: "/claude-use/active-identity", raw: undefined }, binaryDiscovery: DISCOVERED_BINARY, claudeShim: { state: undefined, targetExists: false }, + pathResolution: { ownExecutablePath: "/home/u/.local/bin/claude-use", claudeUse: { status: "ok" } }, platform: "linux", ...overrides, }; @@ -112,6 +114,88 @@ describe("runDoctor: claude-shim", () => { }); }); +describe("runDoctor: path-resolution", () => { + it("passes when a bare `claude-use` reaches this running executable", () => { + const report = runDoctor(baseParams()); + const [finding] = findingsFor(report, "path-resolution"); + expect(finding?.severity).toBe("pass"); + expect(finding?.message).toContain("/home/u/.local/bin/claude-use"); + expect(report.ok).toBe(true); + }); + + it("fails when an earlier PATH entry shadows the running executable, naming both and how to fix it", () => { + const report = runDoctor( + baseParams({ + pathResolution: { + ownExecutablePath: "/home/u/.local/bin/claude-use", + claudeUse: { status: "shadowed", by: "/home/u/.dotfiles/bin/claude-use" }, + }, + }), + ); + const [finding] = findingsFor(report, "path-resolution"); + expect(finding?.severity).toBe("fail"); + expect(finding?.message).toContain("/home/u/.dotfiles/bin/claude-use"); + expect(finding?.message).toContain("/home/u/.local/bin/claude-use"); + expect(finding?.message).toContain("/home/u/.local/bin ahead of it on PATH"); + expect(report.ok).toBe(false); + }); + + it("warns, not fails, when the running executable's own directory is not on PATH at all", () => { + const report = runDoctor( + baseParams({ + pathResolution: { ownExecutablePath: "/tmp/npx-cache/claude-use", claudeUse: { status: "not-on-path" } }, + }), + ); + const [finding] = findingsFor(report, "path-resolution"); + expect(finding?.severity).toBe("warn"); + expect(report.ok).toBe(true); + }); + + it("reports nothing about `claude` when no shim is enabled, since a `claude` on PATH is then Claude Code's own binary", () => { + const report = runDoctor(baseParams()); + expect(findingsFor(report, "path-resolution").map((finding) => finding.subject)).toEqual(["claude-use"]); + }); + + it("warns, not fails, when an enabled `claude` shim is shadowed — the launcher is still reachable as `claude-use run`", () => { + const report = runDoctor( + baseParams({ + pathResolution: { + ownExecutablePath: "/home/u/.local/bin/claude-use", + claudeUse: { status: "ok" }, + claude: { status: "shadowed", by: "/opt/homebrew/bin/claude" }, + }, + }), + ); + const claudeFinding = findingsFor(report, "path-resolution").find((finding) => finding.subject === "claude"); + expect(claudeFinding?.severity).toBe("warn"); + expect(claudeFinding?.message).toContain("/opt/homebrew/bin/claude"); + expect(report.ok).toBe(true); + }); +}); + +describe("refinePathShadow", () => { + it("leaves a genuine shadow alone", () => { + const status = refinePathShadow({ status: "shadowed", by: "/a/claude-use" }, "/b/claude-use", (target) => target); + expect(status).toEqual({ status: "shadowed", by: "/a/claude-use" }); + }); + + it("collapses to ok when both names resolve to the same real file", () => { + const realpaths: Record = { "/a/claude-use": "/real/claude-use", "/b/claude-use": "/real/claude-use" }; + const status = refinePathShadow( + { status: "shadowed", by: "/a/claude-use" }, + "/b/claude-use", + (target) => realpaths[target] ?? target, + ); + expect(status).toEqual({ status: "ok" }); + }); + + it("passes through every non-shadowed status untouched", () => { + const realpath = (target: string): string => target; + expect(refinePathShadow({ status: "ok" }, "/b/claude-use", realpath)).toEqual({ status: "ok" }); + expect(refinePathShadow({ status: "not-on-path" }, "/b/claude-use", realpath)).toEqual({ status: "not-on-path" }); + }); +}); + describe("runDoctor: identity", () => { it("fails when identity.json is missing", () => { const report = runDoctor(baseParams({ identities: [identity("work", { raw: undefined })] })); diff --git a/src/doctor.ts b/src/doctor.ts index efa202e..82d06c6 100644 --- a/src/doctor.ts +++ b/src/doctor.ts @@ -4,7 +4,14 @@ import type { Command } from "commander"; import type { z } from "zod"; import { lookupKeychainService } from "./check"; -import { ClaudeShimStateSchema, resolveOwnInstallDirs, type ClaudeShimState } from "./claudeShim"; +import { + ClaudeShimStateSchema, + commandFilename, + findPathShadow, + resolveOwnInstallDirs, + type ClaudeShimState, + type PathShadowStatus, +} from "./claudeShim"; import { ConfigValidationError } from "./config/load"; import { readJson } from "./config/store"; import { @@ -14,10 +21,11 @@ import { GlobalConfigSchema, IdentitySchema, } from "./config/schema"; +import { isIdentityDirectoryName } from "./identityManager"; import { detectAmbientCredential, formatAmbientCredentialGuardMessage } from "./launcher/guard"; import type { RunPort } from "./launcher/ports"; import type { LayoutPaths } from "./paths"; -import { realFsPort, realOwnExecutablePath, realResolveClaudeBinary, realRunPort } from "./realPorts"; +import { findExecutableInDir, realFsPort, realOwnExecutablePath, realResolveClaudeBinary, realRunPort } from "./realPorts"; import { lineariseProfile, type ProfileLoader, type ProfileSource } from "./resolve/extends"; import type { DiscoveredClaudeBinary } from "./versionDiscovery"; @@ -27,6 +35,7 @@ type DoctorSection = | "ambient-credential" | "binary-discovery" | "claude-shim" + | "path-resolution" | "config-profile" | "identity" | "keychain" @@ -76,6 +85,33 @@ type DoctorBinaryDiscovery = | { readonly ok: true; readonly binary: DiscoveredClaudeBinary } | { readonly ok: false; readonly message: string }; +/** + * Where a bare command name resolves for the two names this tool owns. + * + * `ownExecutablePath` is this process's own PATH-visible location (`realOwnExecutablePath()`); `claudeUse` is `findPathShadow`'s verdict for a bare `claude-use` against the directory that executable lives in. `claude` is only populated when a shim is actually enabled — without one, a `claude` on PATH is Claude Code's own binary, which is not a shadow of anything. + */ +interface DoctorPathResolution { + readonly ownExecutablePath: string; + readonly claudeUse: PathShadowStatus; + readonly claude?: PathShadowStatus; +} + +/** + * Collapses a `shadowed` verdict back to `ok` when the shadowing entry and this executable are literally the same file reached by two names — `findPathShadow` compares *directories*, so a symlink on PATH pointing at the running executable's own real location otherwise reads as a shadow of itself. + * + * `realpath` must resolve symlinks, and must return its argument unchanged rather than throwing when the path cannot be resolved (a broken symlink, a race with an uninstall), so an unresolvable path simply stays unequal and the shadow verdict stands. + */ +export function refinePathShadow( + status: PathShadowStatus, + ownExecutablePath: string, + realpath: (target: string) => string, +): PathShadowStatus { + if (status.status !== "shadowed") { + return status; + } + return realpath(status.by) === realpath(ownExecutablePath) ? { status: "ok" } : status; +} + /** Everything `runDoctor` needs, all of it already loaded/injected — nothing in `runDoctor` itself reads a file, shells out, or touches the farm. */ export interface RunDoctorParams { readonly env: Readonly>; @@ -88,6 +124,8 @@ export interface RunDoctorParams { readonly binaryDiscovery: DoctorBinaryDiscovery; /** Whether `claude-use shim enable` has been run, and whether its recorded target still exists on disk — pre-resolved by the wiring layer, since checking a file's existence is real I/O, not a parse-shaped pure operation. */ readonly claudeShim: { readonly state: ClaudeShimState | undefined; readonly targetExists: boolean }; + /** Which executables a bare `claude-use` (and, when the shim is enabled, a bare `claude`) would actually run — pre-resolved by the wiring layer, since scanning PATH is real I/O. */ + readonly pathResolution: DoctorPathResolution; /** Runs `security find-generic-password` for the per-identity Keychain check. Omit to skip that check entirely (e.g. off macOS). */ readonly run?: RunPort; /** `process.platform` in real use; the Keychain check only ever runs when this is `"darwin"`. */ @@ -115,6 +153,65 @@ function validateJson( return { ok: true, data: result.data }; } +/** + * Reports which `claude-use` a bare command name actually runs, and — when a `claude` shim is enabled — the same for `claude`. + * + * A shadowed `claude-use` is a `fail`, not a `warn`, because it invalidates the rest of the report rather than merely sitting alongside it: every other finding here describes the binary that produced them, which by definition is not the binary the user's own commands reach. It is also a silent failure in every other respect, since the shadowing install keeps working, just at whatever version it was frozen at. Confirmed in the wild: a hand-written wrapper script from an earlier install channel sat ahead of `~/.local/bin` on PATH and kept re-execing a month-old binary, so a naming rule that had since widened kept rejecting an `identity.json` a current claude-use had written — with nothing anywhere reporting that the running binary was not the installed one. + * + * `not-on-path` is a `warn` rather than a `fail`: invoking this tool by an absolute path, or through `npx`, is a legitimate one-off, and nothing about it is inconsistent. + */ +function pushPathResolution( + push: (section: DoctorSection, severity: DoctorSeverity, message: string, subject?: string) => void, + resolution: DoctorPathResolution, +): void { + const ownDir = path.dirname(resolution.ownExecutablePath); + switch (resolution.claudeUse.status) { + case "ok": + push("path-resolution", "pass", `\`claude-use\` on PATH resolves to this running executable, ${resolution.ownExecutablePath}.`, "claude-use"); + break; + case "not-on-path": + push( + "path-resolution", + "warn", + `${ownDir} is not on PATH, so a bare \`claude-use\` does not reach ${resolution.ownExecutablePath}. ` + + "Add it to PATH, or keep invoking this executable by its full path.", + "claude-use", + ); + break; + case "shadowed": + push( + "path-resolution", + "fail", + `\`claude-use\` on PATH resolves to ${resolution.claudeUse.by}, not this running executable, ${resolution.ownExecutablePath}. ` + + "Every command you type runs that one instead, at whatever version it happens to be — including the checks in this report, which describe this executable. " + + `Remove ${resolution.claudeUse.by}, repoint it at ${resolution.ownExecutablePath}, or put ${ownDir} ahead of it on PATH.`, + "claude-use", + ); + break; + } + + if (resolution.claude === undefined) { + return; + } + switch (resolution.claude.status) { + case "ok": + push("path-resolution", "pass", "`claude` on PATH resolves to the enabled shim.", "claude"); + break; + case "not-on-path": + push("path-resolution", "warn", "The enabled `claude` shim's directory is not on PATH — add it, or use `claude-use run` instead.", "claude"); + break; + case "shadowed": + push( + "path-resolution", + "warn", + `\`claude\` on PATH resolves to ${resolution.claude.by}, not the enabled shim. ` + + "Put the shim's directory ahead of it on PATH, or run `claude-use shim disable` if you meant to launch that one directly.", + "claude", + ); + break; + } +} + /** * Audits the whole `~/.claude-use` config graph for internal consistency: every identity, every configuration profile's own `extends` chain, `directory-rules.json`, `config.json`, `categories.local.json`, `active-identity`, plus real Claude Code binary discoverability and ambient-credential exposure. * @@ -161,6 +258,8 @@ export function runDoctor(params: RunDoctorParams): DoctorReport { ); } + pushPathResolution(push, params.pathResolution); + const profileSources = new Map(); for (const entry of params.configProfiles) { const validated = validateJson(ConfigProfileSchema, entry); @@ -289,6 +388,7 @@ const SECTION_TITLES: Readonly> = { "ambient-credential": "Ambient-credential exposure", "binary-discovery": "Claude Code binary discovery", "claude-shim": "`claude` command shim", + "path-resolution": "PATH resolution", "config-profile": "Configuration profiles", identity: "Identities", keychain: "macOS Keychain", @@ -302,6 +402,7 @@ const SECTION_ORDER: readonly DoctorSection[] = [ "ambient-credential", "binary-discovery", "claude-shim", + "path-resolution", "config-profile", "identity", "keychain", @@ -341,6 +442,15 @@ export function formatDoctorReport(report: DoctorReport): string[] { return lines; } +/** `fs.realpathSync` with every failure collapsed back to the input path — see `refinePathShadow` for why an unresolvable path must stay unequal rather than abort the audit. */ +function realpathOrSelf(target: string): string { + try { + return fs.realpathSync(target); + } catch { + return target; + } +} + /** * Registers `claude-use doctor` onto `program`. * @@ -358,7 +468,7 @@ export function registerDoctorCommand(program: Command, paths: LayoutPaths): voi const identityNames = fs.existsSync(paths.identitiesDir) ? fs .readdirSync(paths.identitiesDir, { withFileTypes: true }) - .filter((entry) => entry.isDirectory()) + .filter((entry) => entry.isDirectory() && isIdentityDirectoryName(entry.name)) .map((entry) => entry.name) .sort() : []; @@ -392,6 +502,21 @@ export function registerDoctorCommand(program: Command, paths: LayoutPaths): voi const shimState = readJson(paths.claudeShimFile, ClaudeShimStateSchema); + const pathDirs = (process.env.PATH ?? "").split(path.delimiter).filter((dir) => dir !== ""); + const claudeShimShadow = + shimState === undefined + ? undefined + : refinePathShadow( + findPathShadow({ + pathDirs, + targetDir: path.dirname(shimState.targetPath), + targetFilename: path.basename(shimState.targetPath), + findExecutableInDir, + }), + shimState.targetPath, + realpathOrSelf, + ); + const report = runDoctor({ env: process.env, identities, @@ -402,6 +527,20 @@ export function registerDoctorCommand(program: Command, paths: LayoutPaths): voi activeIdentity: { path: paths.activeIdentityFile, raw: realFsPort.readFileUtf8(paths.activeIdentityFile) }, binaryDiscovery, claudeShim: { state: shimState, targetExists: shimState !== undefined && fs.existsSync(shimState.targetPath) }, + pathResolution: { + ownExecutablePath, + claudeUse: refinePathShadow( + findPathShadow({ + pathDirs, + targetDir: path.dirname(ownExecutablePath), + targetFilename: commandFilename(ownExecutablePath, "claude-use"), + findExecutableInDir, + }), + ownExecutablePath, + realpathOrSelf, + ), + ...(claudeShimShadow === undefined ? {} : { claude: claudeShimShadow }), + }, run: realRunPort, platform: process.platform, }); diff --git a/src/identityManager.test.ts b/src/identityManager.test.ts index fc7f1bd..3e974fe 100644 --- a/src/identityManager.test.ts +++ b/src/identityManager.test.ts @@ -10,6 +10,7 @@ import { IdentityNotFoundError, InvalidIdentityNameError, addIdentity, + isIdentityDirectoryName, listIdentities, readActiveIdentity, readIdentity, @@ -219,6 +220,18 @@ describe("identityManager", () => { }); }); + describe("isIdentityDirectoryName", () => { + it("accepts a name IdentitySchema itself would accept", () => { + expect(isIdentityDirectoryName("work")).toBe(true); + expect(isIdentityDirectoryName("joseph.mearman@exadev.io")).toBe(true); + }); + + it("rejects both farm-bookkeeping shapes, which an identity name could never take", () => { + expect(isIdentityDirectoryName(".work.previous.123.abc")).toBe(false); + expect(isIdentityDirectoryName(".work.scratch.123.abc")).toBe(false); + }); + }); + describe("listIdentities", () => { it("returns an empty list when no identities exist", () => { expect(listIdentities(paths)).toEqual([]); @@ -235,12 +248,65 @@ describe("identityManager", () => { expect(entries.find((entry) => entry.name === "work")?.isActive).toBe(false); }); - it("skips a directory under identitiesDir that has no valid identity.json", () => { + it("skips a directory under identitiesDir that has no identity.json at all", () => { addIdentity(paths, "work"); fs.mkdirSync(path.join(paths.identitiesDir, "not-an-identity"), { recursive: true }); const entries = listIdentities(paths); expect(entries.map((entry) => entry.name)).toEqual(["work"]); }); + + it("skips a retained superseded farm even when a crash left a readable identity.json inside it", () => { + addIdentity(paths, "work"); + const retained = path.join(paths.identitiesDir, ".work.previous.123.abc"); + fs.mkdirSync(retained, { recursive: true }); + fs.writeFileSync( + path.join(retained, "identity.json"), + JSON.stringify({ name: "work", allowAmbientCredential: false }), + "utf8", + ); + fs.mkdirSync(path.join(paths.identitiesDir, ".work.scratch.456.def"), { recursive: true }); + + expect(listIdentities(paths).map((entry) => entry.name)).toEqual(["work"]); + }); + + it("reports an identity.json this version's schema rejects as its own unreadable entry instead of aborting the whole listing", () => { + addIdentity(paths, "work"); + const rejectedDir = path.join(paths.identitiesDir, "written-by-a-newer-claude-use"); + fs.mkdirSync(rejectedDir, { recursive: true }); + fs.writeFileSync( + path.join(rejectedDir, "identity.json"), + JSON.stringify({ name: "a name a future naming rule allows!", allowAmbientCredential: false }), + "utf8", + ); + + const entries = listIdentities(paths); + expect(entries.map((entry) => entry.name)).toEqual(["work", "written-by-a-newer-claude-use"]); + + const rejected = entries.find((entry) => entry.name === "written-by-a-newer-claude-use"); + expect(rejected?.identity).toBeUndefined(); + expect(rejected?.problem).toContain("must match pattern"); + expect(rejected?.problem).not.toContain("\n"); + expect(entries.find((entry) => entry.name === "work")?.identity?.name).toBe("work"); + }); + + it("reports malformed JSON as an unreadable entry too, rather than letting the SyntaxError escape", () => { + const brokenDir = path.join(paths.identitiesDir, "broken"); + fs.mkdirSync(brokenDir, { recursive: true }); + fs.writeFileSync(path.join(brokenDir, "identity.json"), "{ not json", "utf8"); + + const entries = listIdentities(paths); + expect(entries.map((entry) => entry.name)).toEqual(["broken"]); + expect(entries[0]?.problem).toBeDefined(); + }); + + it("still marks an unreadable identity as active when active-identity names it", () => { + const brokenDir = path.join(paths.identitiesDir, "broken"); + fs.mkdirSync(brokenDir, { recursive: true }); + fs.writeFileSync(path.join(brokenDir, "identity.json"), "{ not json", "utf8"); + fs.writeFileSync(paths.activeIdentityFile, "broken", "utf8"); + + expect(listIdentities(paths)[0]?.isActive).toBe(true); + }); }); describe("setDefaultConfigProfile", () => { diff --git a/src/identityManager.ts b/src/identityManager.ts index b1115a1..1c1d29c 100644 --- a/src/identityManager.ts +++ b/src/identityManager.ts @@ -2,6 +2,7 @@ import fs from "node:fs"; import path from "node:path"; import type { Command } from "commander"; +import { ConfigValidationError } from "./config/load"; import { applyPatch, readJson, writeJsonAtomic, writeTextAtomic } from "./config/store"; import { IdentitySchema, type Identity } from "./config/schema"; import { realPromptsPort, runProfileWizard, type PromptsPort } from "./configure"; @@ -168,31 +169,76 @@ export function readActiveIdentity(paths: LayoutPaths): string | undefined { return raw === "" ? undefined : raw; } -/** One identity as reported by `listIdentities`. */ -export interface IdentityListEntry { +/** + * Whether a directory name directly under `identitiesDir` names an actual identity, rather than one of claude-use's own farm directories. + * + * `IdentitySchema` requires an identity name to start with a letter or digit, so a leading `.` can only be a resync's own bookkeeping — a `..scratch.` tree still being built, or a `..previous.` superseded farm retained for `claude-use identity resolve`. Neither is an identity, and neither should be reported as a broken one for lacking an `identity.json` a resync never put there. + */ +export function isIdentityDirectoryName(name: string): boolean { + return !name.startsWith("."); +} + +/** One identity as reported by `listIdentities`, whose `identity.json` parsed and validated cleanly. */ +interface IdentityListEntry { readonly name: string; readonly identity: Identity; readonly isActive: boolean; + readonly problem?: never; +} + +/** One identity whose `identity.json` is present but unreadable — malformed JSON, or valid JSON this version's `IdentitySchema` rejects. `problem` carries the reason, already flattened onto a single line. */ +interface UnreadableIdentityListEntry { + readonly name: string; + readonly identity?: never; + readonly isActive: boolean; + readonly problem: string; } -/** Lists every identity under `identitiesDir` that has a valid `identity.json`, marking which one (if any) is currently active. */ -export function listIdentities(paths: LayoutPaths): readonly IdentityListEntry[] { +/** Either shape `listIdentities` can report, discriminated by which of `identity`/`problem` is present rather than by a tag field — the two are never simultaneously satisfiable. */ +export type IdentityListing = IdentityListEntry | UnreadableIdentityListEntry; + +/** + * Reads one identity for `listIdentities`, converting an unreadable `identity.json` into a reportable problem string instead of throwing. + * + * Only the two failure modes a *file's own content* can produce are caught: a `SyntaxError` from `JSON.parse`, and the `ConfigValidationError` a schema violation raises. Anything else (a permission error, a directory where a file belongs) still propagates, since those are environment faults rather than one identity's data being bad. + * + * A wholly absent `identity.json` is neither — it yields `undefined`, and `listIdentities` skips the entry entirely. That is what keeps `identities/` retained superseded farms (`..previous../`, which are real directories with no `identity.json`) out of the listing. + */ +function readIdentityForListing(paths: LayoutPaths, name: string): Identity | { readonly problem: string } | undefined { + try { + return readIdentity(paths, name); + } catch (error) { + if (error instanceof ConfigValidationError || error instanceof SyntaxError) { + return { problem: error.message.replace(/\s*\n\s*/g, " ") }; + } + throw error; + } +} + +/** + * Lists every identity under `identitiesDir`, marking which one (if any) is currently active. + * + * One identity whose `identity.json` cannot be read is reported as its own `UnreadableIdentityListEntry` rather than aborting the whole listing. A single bad file blocking `identity list` outright is exactly the failure mode that hides every *other* identity from view at the moment the user most needs to see them — and the file need not even be corrupt to land here, since a name written by a newer claude-use whose naming rule has since widened is rejected outright by an older binary's own copy of `IdentitySchema`. + */ +export function listIdentities(paths: LayoutPaths): readonly IdentityListing[] { if (!fs.existsSync(paths.identitiesDir)) { return []; } const active = readActiveIdentity(paths); const names = fs .readdirSync(paths.identitiesDir, { withFileTypes: true }) - .filter((entry) => entry.isDirectory()) + .filter((entry) => entry.isDirectory() && isIdentityDirectoryName(entry.name)) .map((entry) => entry.name) .sort(); - const result: IdentityListEntry[] = []; + const result: IdentityListing[] = []; for (const name of names) { - const identity = readIdentity(paths, name); - if (identity !== undefined) { - result.push({ name, identity, isActive: name === active }); + const read = readIdentityForListing(paths, name); + if (read === undefined) { + continue; } + const isActive = name === active; + result.push("problem" in read ? { name, isActive, problem: read.problem } : { name, identity: read, isActive }); } return result; } @@ -295,6 +341,10 @@ export function registerIdentityCommand(program: Command, paths: LayoutPaths): v } for (const entry of entries) { const marker = entry.isActive ? "* " : " "; + if (entry.problem !== undefined) { + console.log(`${marker}${entry.name} [unreadable: ${entry.problem}]`); + continue; + } const defaultProfile = entry.identity.defaultConfigProfile !== undefined ? ` (default profile: ${entry.identity.defaultConfigProfile})` @@ -302,6 +352,9 @@ export function registerIdentityCommand(program: Command, paths: LayoutPaths): v const ambient = entry.identity.allowAmbientCredential ? " [allows ambient credential]" : ""; console.log(`${marker}${entry.name}${defaultProfile}${ambient}`); } + if (entries.some((entry) => entry.problem !== undefined)) { + console.log("\nRun `claude-use doctor` for the full detail on every unreadable entry."); + } }); identity