diff --git a/docs/client-configuration.md b/docs/client-configuration.md index e27866f7d..1a85dbdfa 100644 --- a/docs/client-configuration.md +++ b/docs/client-configuration.md @@ -33,6 +33,71 @@ schema constraints; REA still validates complete canonical inputs. Full-profile `gpt-5.4` metadata is also verified locally. Live model API acceptance and Windows remain unverified; see [native client verification](testing.md). +## Claude Desktop + +Setup registers `mcpServers.rea` in `claude_desktop_config.json`: under +`~/Library/Application Support/Claude` on macOS, under +`$XDG_CONFIG_HOME/Claude` (default `~/.config/Claude`) on Linux, and on Windows +in the file the installed app reads: + +- Without Claude Desktop's MSIX package storage, setup uses + `%APPDATA%\Claude\claude_desktop_config.json`. +- The MSIX build (package family `Claude_pzs8sxrjxfjjc`, installed under + `C:\Program Files\WindowsApps\Claude__x64__pzs8sxrjxfjjc`) + virtualizes AppData writes. Once it has saved its configuration, it reads + the package-private copy + `%LOCALAPPDATA%\Packages\Claude_pzs8sxrjxfjjc\LocalCache\Roaming\Claude\claude_desktop_config.json` + and ignores the `%APPDATA%` file. Setup, doctor and uninstall use that + private copy when it exists. Before the package has saved one, they use the + `%APPDATA%` file it still reads; with neither file, setup creates the + private copy. +- If `%LOCALAPPDATA%\Packages` holds a `Claude_` package from + another publisher, or cannot be read, REA cannot establish which file Claude + Desktop reads. Setup, doctor and uninstall report that instead of falling + back to `%APPDATA%`. Doctor then reports `claude_desktop` as invalid, and + uninstall stops before removing anything for any client. + +Quit Claude Desktop fully before approving setup; closing its window is not +enough. A running Claude Desktop can rewrite its configuration from memory and +drop the registration. This was observed with the Windows MSIX build. Start +Claude Desktop again after setup. If the package saves its private copy after +setup planned to write the `%APPDATA%` file, setup makes no change and asks you +to rerun it. + +A process that the MSIX app started, such as a terminal in Claude Desktop's +Code tab, sees the package's private files at their `%APPDATA%` paths even +though it reports no package identity. Setup compares one of those files at +both paths. When it finds that redirected view, the app is running, so setup +and uninstall refuse to change Claude Desktop's configuration; rerun them from +a terminal that Claude Desktop did not start. If the package has no private +file to compare, setup and uninstall cannot rule that out and also refuse; open +Claude Desktop once if it has never run, quit it fully, then rerun. Doctor +reads the same file from either kind of process. + +The same process also writes everything under `%APPDATA%` and +`%LOCALAPPDATA%` into the package's private storage, where other apps never +read it. From that redirected view, setup and uninstall refuse to change other +clients' files there (VS Code, Trae, Devin and Hermes) and skill directories +there. `npm install -g` run from such a process installs into the package's +`LocalCache\Roaming\npm`, which other processes cannot see at the `%APPDATA%` +path. Setup refuses to register a launcher stored in any MSIX package's +private storage. Install REA from an ordinary terminal and rerun setup there. + +Uninstall removes REA's entry from both the private copy and the `%APPDATA%` +file, including registrations that earlier releases wrote to `%APPDATA%`. + +REA recognizes only the MSIX package. A non-MSIX Claude Desktop installed +beside it, which would read the `%APPDATA%` file, is not detected. + +The package identity, both file locations, the redirected view, and these +setup and doctor results were observed on Windows 11 with the MSIX build +2.31226.1.0, using read-only `doctor` and `setup --dry-run` runs both inside +and outside the app. On that host, setup run from an ordinary terminal with +the app quit wrote the package-private copy; after the app restarted, the +entry was still present, doctor reported it aligned, and Claude Desktop's MCP +log showed REA starting and answering `tools/list`. Uninstall against the real +app is unverified. + ## Claude Code Claude Code normally defers MCP tool definitions through tool search. It diff --git a/docs/installation.md b/docs/installation.md index 9285883e3..bb0adfbc9 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -20,7 +20,9 @@ npx rea-agents setup Choose the agents to configure, review the proposed paths and changes, and approve the plan. npm's package-download approval and REA's setup approval are separate. Setup asks before changing agent configuration or installing Hopper. -Restart the agents named in the completion message. +Restart the agents named in the completion message. Quit Claude Desktop fully +before approving, because a running app can overwrite the registration; on +Windows, see [where Claude Desktop reads its configuration](client-configuration.md#claude-desktop). For the latest published release, use `npx rea-agents@latest setup`. [Package-runner and global-installation options](#package-runners-and-global-installation) diff --git a/scripts/verify-package-environment.mjs b/scripts/verify-package-environment.mjs index cd84655b2..e57f7784b 100644 --- a/scripts/verify-package-environment.mjs +++ b/scripts/verify-package-environment.mjs @@ -76,6 +76,7 @@ export async function verifyPackageEnvironment({ HOME: home, USERPROFILE: home, APPDATA: join(home, "AppData", "Roaming"), + LOCALAPPDATA: join(home, "AppData", "Local"), CLAUDE_CONFIG_DIR: home, CODEX_HOME: codexDir, COPILOT_HOME: join(home, ".copilot"), diff --git a/scripts/verify-package-update.mjs b/scripts/verify-package-update.mjs index 5998ab77f..8d8582b44 100644 --- a/scripts/verify-package-update.mjs +++ b/scripts/verify-package-update.mjs @@ -98,6 +98,7 @@ export async function verifyPackageUpdate({ HOME: home, USERPROFILE: home, APPDATA: join(home, "AppData", "Roaming"), + LOCALAPPDATA: join(home, "AppData", "Local"), CLAUDE_CONFIG_DIR: home, CODEX_HOME: join(home, ".codex"), COPILOT_HOME: join(home, ".copilot"), diff --git a/scripts/verify-published-package.mjs b/scripts/verify-published-package.mjs index 8c0281d56..8a5eb48d8 100644 --- a/scripts/verify-published-package.mjs +++ b/scripts/verify-published-package.mjs @@ -118,6 +118,7 @@ async function verifyPublishedUpdate({ version: targetVersion, canaryRoot }) { HOME: home, USERPROFILE: home, APPDATA: join(home, "AppData", "Roaming"), + LOCALAPPDATA: join(home, "AppData", "Local"), CLAUDE_CONFIG_DIR: home, CODEX_HOME: join(home, ".codex"), COPILOT_HOME: join(home, ".copilot"), diff --git a/scripts/verify-windows-package.mjs b/scripts/verify-windows-package.mjs index 7ac204868..425b5cbd3 100644 --- a/scripts/verify-windows-package.mjs +++ b/scripts/verify-windows-package.mjs @@ -110,6 +110,7 @@ try { HOME: home, USERPROFILE: home, APPDATA: join(home, "AppData", "Roaming"), + LOCALAPPDATA: join(home, "AppData", "Local"), CLAUDE_CONFIG_DIR: home, CODEX_HOME: join(home, ".codex"), XDG_CONFIG_HOME: join(home, ".config"), diff --git a/src/application/ClaudeDesktopConfigLocation.test.ts b/src/application/ClaudeDesktopConfigLocation.test.ts new file mode 100644 index 000000000..84d8ca8bc --- /dev/null +++ b/src/application/ClaudeDesktopConfigLocation.test.ts @@ -0,0 +1,341 @@ +import { basename, dirname, join, sep } from "node:path"; + +import { describe, expect, it } from "vitest"; + +import { + CLAUDE_DESKTOP_PACKAGE_FAMILY, + resolveWindowsClaudeDesktopLocation, + type ClaudeDesktopLocationFileSystem, +} from "./ClaudeDesktopConfigLocation.js"; +import { + packagePrivateLauncherBlocker, + packagePrivateStorageFamily, + redirectedWriteBlocker, + type MsixPackageView, +} from "./MsixPackageStorage.js"; + +const home = join("/profile", "a"); +const env = { + APPDATA: join(home, "AppData", "Roaming"), + LOCALAPPDATA: join(home, "AppData", "Local"), +}; +const packages = join(env.LOCALAPPDATA, "Packages"); +const appDataConfig = join(env.APPDATA, "Claude", "claude_desktop_config.json"); +const privateConfig = join( + packages, + CLAUDE_DESKTOP_PACKAGE_FAMILY, + "LocalCache", + "Roaming", + "Claude", + "claude_desktop_config.json", +); + +const failure = (code: string): Error => + Object.assign(new Error(`${code}: injected`), { code }); + +const file = { isFile: () => true }; +const privateDirectory = dirname(privateConfig); +const privatePreferences = join(privateDirectory, "Preferences"); +const appDataDirectory = dirname(appDataConfig); + +/** Package storage holding `privateFiles`; realpath decides the process view. */ +const packagedFileSystem = ( + realpath: ClaudeDesktopLocationFileSystem["realpath"], + privateFiles: readonly string[] = [privateConfig], +): ClaudeDesktopLocationFileSystem => ({ + readdir: (path) => { + if (path === packages) return [CLAUDE_DESKTOP_PACKAGE_FAMILY]; + if (path === privateDirectory) + return privateFiles.map((name) => basename(name)); + throw failure("ENOENT"); + }, + lstat: (path) => { + if (!privateFiles.includes(path)) throw failure("ENOENT"); + return file; + }, + realpath, +}); + +describe("Claude Desktop MSIX process view", () => { + it("recognizes a process that sees the private copy at the AppData path", () => { + const location = resolveWindowsClaudeDesktopLocation( + home, + env, + packagedFileSystem((path) => + path === appDataConfig ? privateConfig : path, + ), + ); + expect(location).toEqual({ + ok: true, + value: expect.objectContaining({ + configPath: privateConfig, + msixPackage: expect.objectContaining({ processView: "redirected" }), + }), + }); + }); + + it("treats an absent AppData file as an unredirected process", () => { + const location = resolveWindowsClaudeDesktopLocation( + home, + env, + packagedFileSystem((path) => { + if (path === appDataConfig) throw failure("ENOENT"); + return path; + }), + ); + expect(location.ok && location.value.msixPackage?.processView).toBe( + "direct", + ); + }); + + it("recognizes redirection through another private file before the configuration exists", () => { + const location = resolveWindowsClaudeDesktopLocation( + home, + env, + packagedFileSystem( + (path) => + path === join(appDataDirectory, "Preferences") + ? privatePreferences + : path, + [privatePreferences], + ), + ); + expect(location.ok && location.value.configPath).toBe(privateConfig); + expect(location.ok && location.value.msixPackage?.processView).toBe( + "redirected", + ); + }); + + it("continues after a listed private file disappears", () => { + const fs = packagedFileSystem( + (path) => + path === join(appDataDirectory, "Preferences") + ? privatePreferences + : path, + [privatePreferences], + ); + const location = resolveWindowsClaudeDesktopLocation(home, env, { + ...fs, + readdir: (path) => + path === privateDirectory + ? ["vanished", "Preferences"] + : fs.readdir(path), + }); + expect(location.ok && location.value.msixPackage?.processView).toBe( + "redirected", + ); + }); + + it("keeps the view unknown when the package has no private files", () => { + const location = resolveWindowsClaudeDesktopLocation( + home, + env, + packagedFileSystem((path) => path, []), + ); + expect(location.ok && location.value.msixPackage?.processView).toBe( + "unknown", + ); + }); + + it("keeps the view unknown when the AppData path cannot be resolved", () => { + const location = resolveWindowsClaudeDesktopLocation( + home, + env, + packagedFileSystem((path) => { + if (path === appDataConfig) throw failure("EACCES"); + return path; + }), + ); + expect(location.ok && location.value.msixPackage?.processView).toBe( + "unknown", + ); + }); +}); + +describe("unresolved Claude Desktop package identity", () => { + it("reports unreadable package storage instead of assuming AppData", () => { + const location = resolveWindowsClaudeDesktopLocation(home, env, { + readdir: () => { + throw failure("EACCES"); + }, + lstat: () => file, + realpath: (path) => path, + }); + expect(location).toEqual({ + ok: false, + error: { + path: packages, + message: expect.stringContaining("could not be inspected"), + }, + }); + }); + + it("reports an uninspectable private copy instead of choosing a file", () => { + const location = resolveWindowsClaudeDesktopLocation(home, env, { + readdir: () => [CLAUDE_DESKTOP_PACKAGE_FAMILY], + lstat: () => { + throw failure("EPERM"); + }, + realpath: (path) => path, + }); + expect(location.ok).toBe(false); + expect(!location.ok && location.error.path).toBe(privateConfig); + }); + + it("matches the package family case-insensitively, as Windows does", () => { + const location = resolveWindowsClaudeDesktopLocation(home, env, { + readdir: () => [CLAUDE_DESKTOP_PACKAGE_FAMILY.toUpperCase()], + lstat: () => { + throw failure("ENOENT"); + }, + realpath: (path) => path, + }); + expect(location.ok && location.value.msixPackage?.packageFamilyName).toBe( + CLAUDE_DESKTOP_PACKAGE_FAMILY.toUpperCase(), + ); + }); +}); + +describe("package-private REA launchers", () => { + const launcher = join( + env.APPDATA, + "npm", + "node_modules", + "rea-agents", + "scripts", + "rea.mjs", + ); + const stored = join( + packages, + CLAUDE_DESKTOP_PACKAGE_FAMILY, + "LocalCache", + "Roaming", + "npm", + "node_modules", + "rea-agents", + "scripts", + "rea.mjs", + ); + + it("names the package that privately stores a path", () => { + expect(packagePrivateStorageFamily(stored, packages)).toBe( + CLAUDE_DESKTOP_PACKAGE_FAMILY, + ); + expect(packagePrivateStorageFamily(launcher, packages)).toBeUndefined(); + expect( + packagePrivateStorageFamily( + join(packages, CLAUDE_DESKTOP_PACKAGE_FAMILY, "LocalState", "x"), + packages, + ), + ).toBeUndefined(); + }); + + it("blocks a launcher that only the package's processes can see", () => { + const blocker = packagePrivateLauncherBlocker( + [join("/Program Files", "nodejs", "node.exe"), launcher, "mcp"], + packages, + (path) => (path === launcher ? stored : path), + ); + expect(blocker).toContain(stored); + expect(blocker).toContain(CLAUDE_DESKTOP_PACKAGE_FAMILY); + expect(blocker).toContain("npm install -g rea-agents"); + }); + + it("compares the launcher with the canonical package storage path", () => { + const shortProfilePackages = join( + "/profile", + "A~1", + "AppData", + "Local", + "Packages", + ); + const blocker = packagePrivateLauncherBlocker( + [launcher, "mcp"], + shortProfilePackages, + (path) => + path === shortProfilePackages + ? packages + : path === launcher + ? stored + : path, + ); + expect(blocker).toContain(CLAUDE_DESKTOP_PACKAGE_FAMILY); + }); + + it("allows ordinary and unresolvable launchers", () => { + expect( + packagePrivateLauncherBlocker( + ["cmd.exe", "/d", "/c", "npx", "-y", "rea-agents@6.4.0", "mcp"], + packages, + () => { + throw new Error("package runners are not resolved"); + }, + ), + ).toBeUndefined(); + expect( + packagePrivateLauncherBlocker([launcher, "mcp"], packages, () => { + throw failure("ENOENT"); + }), + ).toBeUndefined(); + }); +}); + +describe("writes from a redirected AppData view", () => { + const view: MsixPackageView = { + packageFamilyName: CLAUDE_DESKTOP_PACKAGE_FAMILY, + privateConfigPath: privateConfig, + appDataConfigPath: appDataConfig, + virtualizedRoots: [env.APPDATA, env.LOCALAPPDATA], + processView: "redirected", + }; + const vscode = join(env.APPDATA, "Code", "User", "mcp.json"); + const hermes = join(env.LOCALAPPDATA, "hermes", "config.yaml"); + const codex = join(home, ".codex", "config.toml"); + + it("refuses only targets under the package's virtualized roots", () => { + const blocker = redirectedWriteBlocker( + view, + [vscode, codex, hermes], + "setup", + ); + expect(blocker).toContain(`${vscode}, ${hermes}`); + expect(blocker).not.toContain(codex); + expect(blocker).toContain("Rerun setup from a terminal"); + expect(redirectedWriteBlocker(view, [codex], "setup")).toBeUndefined(); + }); + + it("compares aliases through the existing parent of a new destination", () => { + const alias = join(home, "skill-alias"); + const target = join(alias, "skills", "reverse-engineer-anything"); + const blocker = redirectedWriteBlocker( + view, + [target], + "uninstall", + (path) => { + if (path === alias) return join(env.APPDATA, "dsh"); + if (path.startsWith(`${alias}${sep}`)) throw failure("ENOENT"); + return path; + }, + ); + expect(blocker).toContain(target); + expect(blocker).toContain("Rerun uninstall"); + }); + + it("keeps canonicalization failures visible instead of assuming a target is outside AppData", () => { + expect( + redirectedWriteBlocker(view, [codex], "setup", () => { + throw failure("EACCES"); + }), + ).toContain("Cannot establish the MSIX write location"); + }); + + it("allows every target without positive evidence of redirection", () => { + for (const processView of ["direct", "unknown"] as const) + expect( + redirectedWriteBlocker({ ...view, processView }, [vscode], "uninstall"), + ).toBeUndefined(); + expect( + redirectedWriteBlocker(undefined, [vscode], "setup"), + ).toBeUndefined(); + }); +}); diff --git a/src/application/ClaudeDesktopConfigLocation.ts b/src/application/ClaudeDesktopConfigLocation.ts new file mode 100644 index 000000000..899aadbbb --- /dev/null +++ b/src/application/ClaudeDesktopConfigLocation.ts @@ -0,0 +1,199 @@ +import { lstatSync, readdirSync, realpathSync } from "node:fs"; +import { join } from "node:path"; + +import { err, ok, type Result } from "../domain/result.js"; +import { + windowsAppDataRoots, + type MsixPackageView, +} from "./MsixPackageStorage.js"; + +const CONFIG_FILE = "claude_desktop_config.json"; + +/** Private files compared at most when looking for a redirected AppData view. */ +const PROCESS_VIEW_PROBES = 8; + +/** + * Claude Desktop's MSIX package family: package name `Claude` and the + * publisher ID Windows derives from `CN="Anthropic, PBC", O="Anthropic, PBC", + * L=San Francisco, S=California, C=US, …`. + */ +export const CLAUDE_DESKTOP_PACKAGE_FAMILY = "Claude_pzs8sxrjxfjjc"; + +/** `Claude_` plus a 13-character Crockford base32 MSIX publisher ID. */ +const CLAUDE_PACKAGE_FAMILY = /^Claude_[0-9a-hjkmnp-tv-z]{13}$/iu; + +/** The configuration Claude Desktop reads and the evidence that it is installed. */ +export interface ClaudeDesktopLocation { + readonly configPath: string; + readonly markerPath: string; + readonly msixPackage?: MsixPackageView; +} + +/** Unresolved Claude Desktop identity, reported instead of guessing a file. */ +export interface ClaudeDesktopLocationFailure { + readonly path: string; + readonly message: string; +} + +/** Synchronous probes; each throws for failures other than absence. */ +export interface ClaudeDesktopLocationFileSystem { + readonly readdir: (path: string) => readonly string[]; + readonly lstat: (path: string) => { isFile(): boolean }; + readonly realpath: (path: string) => string; +} + +const systemFileSystem: ClaudeDesktopLocationFileSystem = { + readdir: (path) => readdirSync(path), + lstat: (path) => lstatSync(path), + realpath: (path) => realpathSync.native(path), +}; + +/** + * Resolve the `claude_desktop_config.json` that Windows Claude Desktop reads. + * Without its MSIX package storage, that is the AppData file. With it, the + * package reads its private copy when one exists and the AppData file + * otherwise; a new registration goes to the private copy. A `Claude_*` + * package from another publisher leaves the reader unknown. + */ +export const resolveWindowsClaudeDesktopLocation = ( + home: string, + env: Readonly, + fileSystem: ClaudeDesktopLocationFileSystem = systemFileSystem, +): Result => { + const roots = windowsAppDataRoots(home, env); + const appDataDirectory = join(roots.roaming, "Claude"); + const appDataConfigPath = join(appDataDirectory, CONFIG_FILE); + const packagesDirectory = join(roots.local, "Packages"); + let families: readonly string[]; + try { + families = fileSystem + .readdir(packagesDirectory) + .filter((name) => CLAUDE_PACKAGE_FAMILY.test(name)); + } catch (cause: unknown) { + if (!isMissing(cause)) + return err({ + path: packagesDirectory, + message: `MSIX package storage ${packagesDirectory} could not be inspected, so REA cannot establish whether Claude Desktop reads ${appDataConfigPath} or a package-private copy: ${errorMessage(cause)}. Repair access to that directory, then rerun.`, + }); + families = []; + } + const foreign = families.filter( + (name) => + name.toLowerCase() !== CLAUDE_DESKTOP_PACKAGE_FAMILY.toLowerCase(), + ); + if (foreign.length > 0) + return err({ + path: join(packagesDirectory, foreign[0] ?? ""), + message: `MSIX package storage ${foreign.map((name) => join(packagesDirectory, name)).join(", ")} uses Claude Desktop's package name with an unrecognized publisher (Claude Desktop is ${CLAUDE_DESKTOP_PACKAGE_FAMILY}). REA cannot establish which ${CONFIG_FILE} that package reads and does not fall back to ${appDataConfigPath}. Remove the unrecognized package, or register REA in the configuration it reads.`, + }); + const family = families[0]; + if (family === undefined) + return ok({ configPath: appDataConfigPath, markerPath: appDataDirectory }); + const container = join(packagesDirectory, family); + const privateDirectory = join(container, "LocalCache", "Roaming", "Claude"); + const privateConfigPath = join(privateDirectory, CONFIG_FILE); + const privateCopy = present(fileSystem, privateConfigPath); + if (!privateCopy.ok) return privateCopy; + const appDataCopy = privateCopy.value + ? ok(false) + : present(fileSystem, appDataConfigPath); + if (!appDataCopy.ok) return appDataCopy; + return ok({ + configPath: + privateCopy.value || !appDataCopy.value + ? privateConfigPath + : appDataConfigPath, + markerPath: container, + msixPackage: { + packageFamilyName: family, + privateConfigPath, + appDataConfigPath, + virtualizedRoots: [roots.roaming, roots.local], + processView: processView( + fileSystem, + privateDirectory, + appDataDirectory, + privateCopy.value, + ), + }, + }); +}; + +/** + * A process the package started sees the package's private files at their + * AppData paths; any other process sees the real AppData files or nothing. + * Claude Desktop saves its user data there while it runs, so the + * configuration, or any other private file, can show which view this is. + */ +const processView = ( + fileSystem: ClaudeDesktopLocationFileSystem, + privateDirectory: string, + appDataDirectory: string, + privateConfig: boolean, +): MsixPackageView["processView"] => { + let probes: readonly string[]; + try { + probes = privateConfig + ? [CONFIG_FILE] + : fileSystem + .readdir(privateDirectory) + .filter((name) => { + try { + return fileSystem.lstat(join(privateDirectory, name)).isFile(); + } catch (cause: unknown) { + // A candidate may vanish or become unreadable while listed. + void cause; + return false; + } + }) + .slice(0, PROCESS_VIEW_PROBES); + } catch (cause: unknown) { + // No private file can be listed, so no view can be compared. + void cause; + return "unknown"; + } + let view: MsixPackageView["processView"] = "unknown"; + for (const name of probes) { + let privateTarget: string; + try { + privateTarget = fileSystem.realpath(join(privateDirectory, name)); + } catch (cause: unknown) { + // A private file that vanished cannot be compared; try another. + void cause; + continue; + } + try { + if (fileSystem.realpath(join(appDataDirectory, name)) === privateTarget) + return "redirected"; + view = "direct"; + } catch (cause: unknown) { + if (!isMissing(cause)) return "unknown"; + view = "direct"; + } + } + return view; +}; + +const present = ( + fileSystem: ClaudeDesktopLocationFileSystem, + path: string, +): Result => { + try { + fileSystem.lstat(path); + return ok(true); + } catch (cause: unknown) { + if (isMissing(cause)) return ok(false); + return err({ + path, + message: `${path} could not be inspected, so REA cannot establish which ${CONFIG_FILE} Claude Desktop's MSIX package reads: ${errorMessage(cause)}. Repair access to that path, then rerun.`, + }); + } +}; + +const isMissing = (cause: unknown): boolean => + cause instanceof Error && + "code" in cause && + (cause.code === "ENOENT" || cause.code === "ENOTDIR"); + +const errorMessage = (cause: unknown): string => + cause instanceof Error ? cause.message : String(cause); diff --git a/src/application/ClientRegistrationStatus.ts b/src/application/ClientRegistrationStatus.ts index caeba4fb6..d107c5f32 100644 --- a/src/application/ClientRegistrationStatus.ts +++ b/src/application/ClientRegistrationStatus.ts @@ -155,7 +155,7 @@ export const readClientRegistrationInspections = async ( } const parsed = effectiveClientConfiguration(files.value); if (parsed === undefined) { - record(unavailableStatus(client.name, client.configPath, "missing")); + record(unavailableStatus(client, "missing")); continue; } const policy = await readClientPolicyBlock( @@ -191,7 +191,7 @@ export const readClientRegistrationInspections = async ( }); continue; } - record(unavailableStatus(client.name, client.configPath, "missing")); + record(unavailableStatus(client, "missing")); continue; } const registration = parseRegistration(raw, client); @@ -202,8 +202,7 @@ export const readClientRegistrationInspections = async ( const activation = registrationActivation(registration, client, parsed); record( configuredStatus( - client.name, - client.configPath, + client, command, activation === "enabled" && registrationAligned( @@ -222,11 +221,7 @@ export const readClientRegistrationInspections = async ( ); } catch (cause: unknown) { record( - unavailableStatus( - client.name, - client.configPath, - isMissing(cause) ? "missing" : "invalid", - ), + unavailableStatus(client, isMissing(cause) ? "missing" : "invalid"), ); } } @@ -352,36 +347,45 @@ const parseRegistration = ( return registration; }; -const remediation = - "Run rea setup to refresh this registration, then restart the client."; +/** A client that rewrites its file from memory must be quit before setup. */ +const remediation = (client: SetupClient): string => { + if (client.quitBeforeSetup !== true) + return "Run rea setup to refresh this registration, then restart the client."; + const name = client.displayName ?? client.name; + return `Quit ${name} fully, run rea setup to refresh this registration, then start ${name} again. A running ${name} can rewrite its configuration from memory and drop the registration.`; +}; const configuredStatus = ( - client: string, - configPath: string, + client: SetupClient, command: RegistrationCommand, state: "aligned" | "stale", policyRemediation?: string, ): ClientRegistrationStatus => state === "aligned" - ? { client, config_path: configPath, command, state, remediation: null } - : { - client, - config_path: configPath, + ? { + client: client.name, + config_path: client.configPath, command, state, - remediation: policyRemediation ?? remediation, + remediation: null, + } + : { + client: client.name, + config_path: client.configPath, + command, + state, + remediation: policyRemediation ?? remediation(client), }; const unavailableStatus = ( - client: string, - configPath: string, + client: SetupClient, state: "missing" | "invalid", ): ClientRegistrationStatus => ({ - client, - config_path: configPath, + client: client.name, + config_path: client.configPath, command: [], state, - remediation, + remediation: remediation(client), }); const clientEvidencePresent = async (client: SetupClient): Promise => { diff --git a/src/application/MsixPackageStorage.ts b/src/application/MsixPackageStorage.ts new file mode 100644 index 000000000..098b2f1db --- /dev/null +++ b/src/application/MsixPackageStorage.ts @@ -0,0 +1,187 @@ +import { lstat } from "node:fs/promises"; +import { realpathSync } from "node:fs"; +import { basename, dirname, isAbsolute, join, relative, sep } from "node:path"; +import { err, ok, type Result } from "../domain/result.js"; + +/** + * How one MSIX-packaged client reads its AppData configuration. A package + * with write virtualization saves AppData files to its private + * `LocalCache\Roaming` copy, then reads that copy whenever it exists and the + * ordinary AppData file otherwise. + */ +export interface MsixPackageView { + readonly packageFamilyName: string; + /** The package-private copy, which shadows the AppData file once it exists. */ + readonly privateConfigPath: string; + /** The AppData path that unpackaged processes read and write. */ + readonly appDataConfigPath: string; + /** Roots whose writes the package redirects for processes it starts. */ + readonly virtualizedRoots: readonly string[]; + /** + * `redirected`: this process sees one of the package's private files at its + * AppData path, so the running package started it. `direct`: it does not. + * `unknown`: no private file could be compared. + */ + readonly processView: "redirected" | "direct" | "unknown"; +} + +/** Windows' roaming and local application data roots; empty means unset. */ +export const windowsAppDataRoots = ( + home: string, + env: Readonly, +): { readonly roaming: string; readonly local: string } => ({ + roaming: env.APPDATA?.trim() || join(home, "AppData", "Roaming"), + local: env.LOCALAPPDATA?.trim() || join(home, "AppData", "Local"), +}); + +/** The per-user root of MSIX package data containers. */ +export const msixPackagesDirectory = ( + home: string, + env: Readonly, +): string => join(windowsAppDataRoots(home, env).local, "Packages"); + +/** Whether `path` is `root` or inside it, using the host's path comparison. */ +const pathWithin = (path: string, root: string): boolean => { + const inner = relative(root, path); + return ( + inner === "" || + (inner !== ".." && !inner.startsWith(`..${sep}`) && !isAbsolute(inner)) + ); +}; + +/** + * Name the package whose private `LocalCache` holds a canonical path. Its + * processes see those files at virtual AppData locations; other processes do + * not, and Windows removes them with the package. + */ +export const packagePrivateStorageFamily = ( + canonicalPath: string, + packagesDirectory: string, +): string | undefined => { + if (!pathWithin(canonicalPath, packagesDirectory)) return undefined; + const [family, storage] = relative(packagesDirectory, canonicalPath).split( + /[\\/]/u, + ); + return family !== undefined && + family !== "" && + storage?.toLowerCase() === "localcache" + ? family + : undefined; +}; + +/** + * Explain why setup must not register a launcher that exists only in one + * package's private storage. Other clients could not start it, and doctor + * outside that package could not verify it. + */ +export const packagePrivateLauncherBlocker = ( + command: readonly string[], + packagesDirectory: string, + canonicalPath: (path: string) => string = realpathSync.native, +): string | undefined => { + // Compare canonical forms: a short-name, junctioned or substituted profile + // spells the same directory differently. + let packages = packagesDirectory; + try { + packages = canonicalPath(packagesDirectory); + } catch (cause: unknown) { + // Without package storage, the spelled path is the only comparison. + void cause; + } + for (const part of command) { + if (!isAbsolute(part)) continue; + let resolved: string; + try { + resolved = canonicalPath(part); + } catch (cause: unknown) { + // A launcher that cannot be resolved is not evidence of package storage. + void cause; + continue; + } + const family = packagePrivateStorageFamily(resolved, packages); + if (family !== undefined) + return `The REA launcher ${part} is stored at ${resolved}, inside the private storage of MSIX package ${family}. Only processes started by that app can see it at ${part}, and Windows removes it with the package, so other clients could not start this registration. Install REA from a terminal that the app did not start (for example with \`npm install -g rea-agents\`), then rerun setup from that terminal.`; + } + return undefined; +}; + +/** + * Explain why this process must not write `targets`: the running package + * started it, so writes under its virtualized roots land in that package's + * private storage, which the targets' owners never read. + */ +const canonicalDestination = ( + path: string, + canonicalPath: (path: string) => string, +): Result => { + let ancestor = path; + const missing: string[] = []; + while (true) { + try { + return ok(join(canonicalPath(ancestor), ...missing)); + } catch (cause: unknown) { + if ( + cause instanceof Error && + "code" in cause && + (cause.code === "ENOENT" || cause.code === "ENOTDIR") && + dirname(ancestor) !== ancestor + ) { + missing.unshift(basename(ancestor)); + ancestor = dirname(ancestor); + continue; + } + return err({ + path, + detail: cause instanceof Error ? cause.message : String(cause), + }); + } + } +}; + +export const redirectedWriteBlocker = ( + view: MsixPackageView | undefined, + targets: readonly string[], + operation: "setup" | "uninstall", + canonicalPath: (path: string) => string = realpathSync.native, +): string | undefined => { + if (view?.processView !== "redirected" || targets.length === 0) + return undefined; + const roots: string[] = []; + const redirected: string[] = []; + for (const root of view.virtualizedRoots) { + const resolved = canonicalDestination(root, canonicalPath); + if (!resolved.ok) + return `Cannot establish the MSIX write location for ${resolved.error.path}: ${resolved.error.detail}. Rerun ${operation} from a terminal that this app did not start.`; + roots.push(resolved.value); + } + for (const target of targets) { + const resolved = canonicalDestination(target, canonicalPath); + if (!resolved.ok) + return `Cannot establish the MSIX write location for ${resolved.error.path}: ${resolved.error.detail}. Rerun ${operation} from a terminal that this app did not start.`; + if (roots.some((root) => pathWithin(resolved.value, root))) + redirected.push(target); + } + if (redirected.length === 0) return undefined; + return `This process was started by MSIX package ${view.packageFamilyName}, which redirects its writes under ${view.virtualizedRoots.join(" and ")} into the package's private storage. Changes to ${redirected.join(", ")} would not reach the files other apps read. Rerun ${operation} from a terminal that this app did not start.`; +}; + +/** + * Whether the package began reading its private copy after the AppData file + * was chosen, for example by saving its settings when it quit. + */ +export const packageReadTargetChanged = async ( + view: MsixPackageView, + configPath: string, +): Promise => { + if (configPath !== view.appDataConfigPath) return false; + try { + await lstat(view.privateConfigPath); + return true; + } catch (cause: unknown) { + return !( + cause instanceof Error && + "code" in cause && + (cause.code === "ENOENT" || cause.code === "ENOTDIR") + ); + } +}; diff --git a/src/application/Setup.configuration.test.ts b/src/application/Setup.configuration.test.ts index 4ba11f622..3bd08cd4f 100644 --- a/src/application/Setup.configuration.test.ts +++ b/src/application/Setup.configuration.test.ts @@ -1,9 +1,43 @@ import { describe, expect, it } from "vitest"; -import { resolve } from "node:path"; +import { join, resolve } from "node:path"; import { FakeSetupHost, options } from "./Setup.fixture.js"; import { runSetup } from "./Setup.js"; import { setupRegistrationCommand } from "./SetupHost.js"; +import type { MsixPackageView } from "./MsixPackageStorage.js"; +import type { SetupClient } from "./SupportedClients.js"; + +// Host joins keep containment checks meaningful on POSIX test hosts too. +const PROFILE = String.raw`C:\Users\a`; +const ROAMING = join(PROFILE, "AppData", "Roaming"); +const LOCAL = join(PROFILE, "AppData", "Local"); +const PRIVATE_CONFIG = join( + LOCAL, + "Packages", + "Claude_pzs8sxrjxfjjc", + "LocalCache", + "Roaming", + "Claude", + "claude_desktop_config.json", +); +const APPDATA_CONFIG = join(ROAMING, "Claude", "claude_desktop_config.json"); + +/** Claude Desktop as setup resolves it from its MSIX package storage. */ +const msixClaudeDesktop = ( + processView: MsixPackageView["processView"], +): SetupClient => ({ + name: "claude_desktop", + displayName: "Claude Desktop", + configPath: PRIVATE_CONFIG, + quitBeforeSetup: true, + msixPackage: { + packageFamilyName: "Claude_pzs8sxrjxfjjc", + privateConfigPath: PRIVATE_CONFIG, + appDataConfigPath: APPDATA_CONFIG, + virtualizedRoots: [ROAMING, LOCAL], + processView, + }, +}); describe("setup workflow", () => { it("omits an aligned managed skill from an otherwise empty plan", async () => { @@ -293,3 +327,85 @@ describe("setup workflow action selection", () => { expect(result.remediation).not.toContain("internal_client_key"); }); }); + +describe("MSIX Claude Desktop setup", () => { + it("discloses which file MSIX Claude Desktop reads and that it must be quit", async () => { + const host = new FakeSetupHost("win32"); + host.hopper = join(PROFILE, "Hopper"); + host.skill = "unchanged"; + host.clients = [msixClaudeDesktop("direct")]; + + const result = await runSetup( + { ...options(false), clientIds: ["claude_desktop"], dryRun: true }, + host, + ); + + expect(result.status).toBe("planned"); + const [action] = result.plannedActions; + expect(action?.target).toBe(PRIVATE_CONFIG); + expect(action?.detail).toContain( + `Claude Desktop is installed as MSIX package Claude_pzs8sxrjxfjjc, which reads this package-private file instead of ${APPDATA_CONFIG}.`, + ); + expect(action?.detail).toContain( + "Quit Claude Desktop fully before approving; closing its window is not enough.", + ); + }); + + it("refuses AppData writes from a process the MSIX package started", async () => { + const host = new FakeSetupHost("win32"); + host.hopper = join(PROFILE, "Hopper"); + host.skill = "unchanged"; + const vscode = join(ROAMING, "Code", "User", "mcp.json"); + host.clients = [ + msixClaudeDesktop("redirected"), + { name: "vscode", displayName: "VS Code", configPath: vscode }, + { + name: "codex", + displayName: "Codex", + configPath: join(PROFILE, ".codex", "config.toml"), + }, + ]; + + const blocked = await runSetup( + { ...options(true), clientIds: ["vscode", "codex"] }, + host, + ); + expect(blocked.status).toBe("needs_human"); + expect(blocked.remediation).toContain( + `Changes to ${vscode} would not reach the files other apps read.`, + ); + expect(host.configurations).toBe(0); + + const homeOnly = await runSetup( + { ...options(true), clientIds: ["codex"] }, + host, + ); + expect(homeOnly.status).toBe("ready"); + expect(host.configurations).toBe(1); + }); + + it("refuses registrations whose launcher only one MSIX package can see", async () => { + const host = new FakeSetupHost("win32"); + host.hopper = join(PROFILE, "Hopper"); + host.registrationCommandBlocker = "Install REA outside the package."; + host.clients = [ + { name: "codex", displayName: "Codex", configPath: "/codex.toml" }, + ]; + + const blocked = await runSetup( + { ...options(true), clientIds: ["codex"] }, + host, + ); + expect(blocked.status).toBe("needs_human"); + expect(blocked.remediation).toBe("Install REA outside the package."); + expect(host.configurations).toBe(0); + expect(host.skillInstalls).toBe(0); + + const skillOnly = await runSetup( + { ...options(true), clientIds: [], installSkill: true }, + host, + ); + expect(skillOnly.status).toBe("ready"); + expect(host.skillInstalls).toBe(1); + }); +}); diff --git a/src/application/Setup.fixture.ts b/src/application/Setup.fixture.ts index a99897494..3c7c21d35 100644 --- a/src/application/Setup.fixture.ts +++ b/src/application/Setup.fixture.ts @@ -25,6 +25,7 @@ export class FakeSetupHost implements SetupHost { return skillDestinations(this.homeDirectory, clientIds); } readonly registrationCommand: readonly string[]; + registrationCommandBlocker?: string; nodeVersion = "24.18.0"; version: string | undefined = "14.5"; distribution: LinuxDistribution | undefined; diff --git a/src/application/SetupClientConfiguration.ts b/src/application/SetupClientConfiguration.ts index 981b829a4..a868c6177 100644 --- a/src/application/SetupClientConfiguration.ts +++ b/src/application/SetupClientConfiguration.ts @@ -29,6 +29,7 @@ import { PRODUCT_IDENTITY } from "../identity.js"; import { npxRegistrationCommand } from "./ClientRegistrationIdentity.js"; import { MCP_STARTUP_POLICY } from "../mcpStartupPolicy.js"; import { resolveClientConfigTransactionPath } from "./ClientConfigPath.js"; +import { packageReadTargetChanged } from "./MsixPackageStorage.js"; import { readRegularFile, readRegularFileText, @@ -40,7 +41,10 @@ import type { ClientConfigurationResult, SetupProviderEnvironment, } from "./SetupTypes.js"; -import type { SetupClient } from "./SupportedClients.js"; +import { + registrationWriteBlocker, + type SetupClient, +} from "./SupportedClients.js"; export class ClientConfigurationCleanupRollbackFailure extends RegularFileCleanupFailure { constructor( @@ -187,6 +191,13 @@ const persistClientConfiguration = async ({ desired, fallbackServers, }: ClientConfigurationUpdate): Promise => { + // The plan chose this file; an MSIX package may have switched to its + // private copy since, for example when it quit. + if ( + client.msixPackage !== undefined && + (await packageReadTargetChanged(client.msixPackage, client.configPath)) + ) + return { status: "failed", reason: "target_changed" }; const backupPath = original === undefined ? undefined : `${client.configPath}.rea.backup`; let recoveryBackupPath: string | undefined; @@ -358,6 +369,25 @@ export const inspectClientConfiguration = async ( client: SetupClient, providerEnvironment: SetupProviderEnvironment, command: readonly string[], +): Promise => { + const inspection = await inspectClientDocument( + client, + providerEnvironment, + command, + ); + if (inspection.status !== "create" && inspection.status !== "update") + return inspection; + // A change the running client would overwrite is not a persisted registration. + const blocker = registrationWriteBlocker(client, "setup"); + return blocker === undefined + ? inspection + : { status: "invalid", remediation: blocker }; +}; + +const inspectClientDocument = async ( + client: SetupClient, + providerEnvironment: SetupProviderEnvironment, + command: readonly string[], ): Promise => { if (client.configPathError !== undefined) return { status: "invalid", remediation: client.configPathError }; diff --git a/src/application/SetupClients.ts b/src/application/SetupClients.ts index d86d04b9e..5ceccb21a 100644 --- a/src/application/SetupClients.ts +++ b/src/application/SetupClients.ts @@ -9,6 +9,8 @@ import type { SetupClient } from "./SupportedClients.js"; export const failedConfigurationMessage = ( result: Extract, ): string => { + if (result.reason === "target_changed") + return "The client began reading a different configuration file after the plan was shown, so no change was made. Rerun setup to review the new target."; if (result.reason === "path") return "Agent configuration path could not be safely verified. Check its permissions and, if it is a symbolic link, verify that the link resolves to a regular file owned by the current user, then rerun setup."; if (result.reason === "backup") diff --git a/src/application/SetupHost.ts b/src/application/SetupHost.ts index 7ac6828fd..022cecc62 100644 --- a/src/application/SetupHost.ts +++ b/src/application/SetupHost.ts @@ -44,6 +44,10 @@ import { retryRegularFileCleanup, } from "./RegularFileRead.js"; import { providerRegistrationEnvironment } from "./SetupRegistrationEnvironment.js"; +import { + msixPackagesDirectory, + packagePrivateLauncherBlocker, +} from "./MsixPackageStorage.js"; import type { SetupHost, SetupInitialState, @@ -66,6 +70,31 @@ export const setupRegistrationCommand = ( ] : [resolve(process.argv[1] ?? PRODUCT_IDENTITY.cliBinary), "mcp"]; +/** + * The launcher setup registers, and why it must not be registered when it + * exists only inside one MSIX package's private storage. + */ +const setupRegistration = ( + platform: NodeJS.Platform, + homeDirectory: string, + environment: Readonly, +): Pick => { + const registrationCommand = setupRegistrationCommand( + platform, + invokingPackageRunner(environment, platform), + ); + const registrationCommandBlocker = + platform === "win32" + ? packagePrivateLauncherBlocker( + registrationCommand, + msixPackagesDirectory(homeDirectory, environment), + ) + : undefined; + return registrationCommandBlocker === undefined + ? { registrationCommand } + : { registrationCommand, registrationCommandBlocker }; +}; + export const filterClientsNeedingConfigure = async ( host: SetupHost, detectedClients: readonly SetupClient[], @@ -117,10 +146,7 @@ export const systemSetupHost = ( homeDirectory, skillDestinations: (clientIds) => skillDestinations(homeDirectory, clientIds, environment, platform), - registrationCommand: setupRegistrationCommand( - platform, - invokingPackageRunner(environment, platform), - ), + ...setupRegistration(platform, homeDirectory, environment), nodeVersion: process.versions.node, macosVersion: () => doctorHost.macosVersion(), linuxDistribution: readLinuxDistribution, diff --git a/src/application/SetupPlan.ts b/src/application/SetupPlan.ts index a78697c80..35db28de4 100644 --- a/src/application/SetupPlan.ts +++ b/src/application/SetupPlan.ts @@ -8,12 +8,18 @@ import { import type { SkillDestination } from "./SetupSkill.js"; import { macHopperInstallDisclosure } from "./MacHopper.js"; import type { + ClientConfigurationInspection, SetupAction, SetupClientState, SetupHost, SetupProviderEnvironment, } from "./SetupTypes.js"; -import type { SetupClient } from "./SupportedClients.js"; +import { + appDataRedirection, + registrationPersistenceDetail, + type SetupClient, +} from "./SupportedClients.js"; +import { redirectedWriteBlocker } from "./MsixPackageStorage.js"; import type { DoctorReport } from "./Doctor.js"; import { compareUnicodeCodePoints } from "../domain/unicodeCodePointOrder.js"; @@ -161,12 +167,58 @@ export const planSetupActions = async (input: { blocker: `${invalid.client.displayName ?? invalid.client.name}: ${invalid.inspection.remediation}`, }; } - const clientPlans = inspections.flatMap(({ client, inspection }) => + const clientPlans = plannedClientWrites( + inspections, + input.discovery.installHopper, + ); + const plannedActions = setupPlan({ + platform: input.host.platform, + homeDirectory: input.host.homeDirectory, + installHopper: input.discovery.installHopper, + installSkill: input.installSkill, + skillClientIds: input.skillClientIds, + skillDestinations: input.host.skillDestinations(input.skillClientIds), + clients: clientPlans, + providerEnvironment: input.providerEnvironment, + ...(input.discovery.linuxPackageFamily === undefined + ? {} + : { linuxPackageFamily: input.discovery.linuxPackageFamily }), + command: input.command, + }); + const blocker = + (clientPlans.length > 0 + ? input.host.registrationCommandBlocker + : undefined) ?? + redirectedWriteBlocker( + appDataRedirection(input.discovery.clients), + plannedActions + .filter(({ kind }) => kind !== "install_hopper") + .map(({ target }) => target), + "setup", + ); + return { + plannedActions, + selectedClients, + clientsToConfigure: selectedClients.filter((client) => + clientPlans.some(({ client: planned }) => planned.name === client.name), + ), + ...(blocker === undefined ? {} : { blocker }), + }; +}; + +/** Selected clients whose registration setup writes, with the disclosed operation. */ +const plannedClientWrites = ( + inspections: readonly { + readonly client: SetupClient; + readonly inspection: ClientConfigurationInspection; + }[], + installHopper: boolean, +) => + inspections.flatMap(({ client, inspection }) => client.format !== "unsupported" && (inspection.status === "create" || inspection.status === "update" || - (input.discovery.installHopper && - inspection.status === "already_current")) + (installHopper && inspection.status === "already_current")) ? [ { client, @@ -182,27 +234,6 @@ export const planSetupActions = async (input: { ] : [], ); - return { - plannedActions: setupPlan({ - platform: input.host.platform, - homeDirectory: input.host.homeDirectory, - installHopper: input.discovery.installHopper, - installSkill: input.installSkill, - skillClientIds: input.skillClientIds, - skillDestinations: input.host.skillDestinations(input.skillClientIds), - clients: clientPlans, - providerEnvironment: input.providerEnvironment, - ...(input.discovery.linuxPackageFamily === undefined - ? {} - : { linuxPackageFamily: input.discovery.linuxPackageFamily }), - command: input.command, - }), - selectedClients, - clientsToConfigure: selectedClients.filter((client) => - clientPlans.some(({ client: planned }) => planned.name === client.name), - ), - }; -}; /** Describe every selected setup mutation before user approval. */ const setupPlan = (input: { @@ -259,7 +290,7 @@ const setupPlan = (input: { label: client.displayName ?? client.name, target: client.configPath, detail: clientConfigurationDetail( - client.displayName ?? client.name, + client, input.command, input.providerEnvironment, input.installHopper, @@ -303,7 +334,7 @@ const majorVersion = (version: string): number => Number.parseInt(version.split(".")[0] ?? "0", 10); const clientConfigurationDetail = ( - client: string, + client: SetupClient, command: readonly string[], environment: SetupProviderEnvironment, installHopper: boolean, @@ -320,5 +351,6 @@ const clientConfigurationDetail = ( ? " If this plan installs Hopper, REA will add HOPPER_LAUNCHER_PATH using the launcher path reported by that installation." : " The HOPPER_LAUNCHER_PATH value shown above is the current fallback; if this plan installs Hopper, REA will replace it with the launcher path reported by that installation." : ""; - return `Add the REA MCP registration for ${client}; preserve unrelated configuration. The backup path reflects the configuration inspected for this plan; if that file changes before execution, setup will select a recovery path for its current contents. Command: ${command.map((part) => JSON.stringify(part)).join(" ")}.${environmentDetail}${hopperDetail}`; + const persistence = registrationPersistenceDetail(client); + return `Add the REA MCP registration for ${client.displayName ?? client.name}; preserve unrelated configuration. The backup path reflects the configuration inspected for this plan; if that file changes before execution, setup will select a recovery path for its current contents. Command: ${command.map((part) => JSON.stringify(part)).join(" ")}.${environmentDetail}${hopperDetail}${persistence === undefined ? "" : ` ${persistence}`}`; }; diff --git a/src/application/SetupTypes.ts b/src/application/SetupTypes.ts index 241e0f6dd..f0cf61c59 100644 --- a/src/application/SetupTypes.ts +++ b/src/application/SetupTypes.ts @@ -25,7 +25,12 @@ export type ClientConfigurationResult = } | { readonly status: "failed"; - readonly reason: "path" | "backup" | "write" | "readback"; + readonly reason: + | "path" + | "target_changed" + | "backup" + | "write" + | "readback"; } | { readonly status: "failed"; @@ -55,6 +60,8 @@ export interface SetupHost { readonly homeDirectory: string; skillDestinations(clientIds: readonly string[]): readonly SkillDestination[]; readonly registrationCommand: readonly string[]; + /** Why no client may be registered with `registrationCommand` from this host. */ + readonly registrationCommandBlocker?: string; readonly nodeVersion: string; macosVersion(): Promise; linuxDistribution(): Promise; diff --git a/src/application/SupportedClients.ts b/src/application/SupportedClients.ts index 0a7352627..8407da4c0 100644 --- a/src/application/SupportedClients.ts +++ b/src/application/SupportedClients.ts @@ -7,6 +7,8 @@ import { resolveHermesClientHome, } from "./HermesClientHome.js"; import { compareUnicodeCodePoints } from "../domain/unicodeCodePointOrder.js"; +import { resolveWindowsClaudeDesktopLocation } from "./ClaudeDesktopConfigLocation.js"; +import type { MsixPackageView } from "./MsixPackageStorage.js"; /** One supported client configuration location. */ export interface SetupClient { @@ -29,6 +31,10 @@ export interface SetupClient { /** Same-scope fallback ZCode stops loading after its first native server. */ readonly zcodeFallbackPath?: string; readonly markerPath?: string; + /** MSIX package storage that decides which configuration the client reads. */ + readonly msixPackage?: MsixPackageView; + /** A running client can rewrite its configuration from memory. */ + readonly quitBeforeSetup?: boolean; readonly format?: | "json" | "toml" @@ -61,12 +67,35 @@ interface ClientPathContext { readonly env: Readonly; } -interface ClientDefinition { +/** One resolved configuration location and its detection evidence. */ +interface ClientLocation { + readonly configPath: string; + readonly markerPath: string; + readonly msixPackage?: MsixPackageView; +} + +type ClientDefinition = ClientDefinitionBase & + ( + | { + readonly configPath: ClientPath; + readonly markerPath: ClientPath; + readonly location?: undefined; + } + | { + /** Resolve configuration and marker together when they share probes. */ + readonly location: ( + context: ClientPathContext, + ) => Result; + readonly configPath?: undefined; + readonly markerPath?: undefined; + } + ); + +interface ClientDefinitionBase { readonly name: string; readonly displayName: string; - readonly configPath: ClientPath; - readonly markerPath: ClientPath; readonly format: NonNullable; + readonly quitBeforeSetup?: boolean; readonly zcodeFallbackPath?: (context: ClientPathContext) => string; readonly skillPath?: ClientPath; readonly configPaths?: (context: ClientPathContext) => readonly string[]; @@ -111,16 +140,20 @@ const traeUserDirectory = ({ return join(xdgConfigHome({ home, platform, env }), "Trae", "User"); }; -const claudeDesktopDirectory = ({ - home, - platform, - env, -}: ClientPathContext): string => { - if (platform === "win32") - return join(env.APPDATA ?? join(home, "AppData", "Roaming"), "Claude"); - if (platform === "linux") - return join(xdgConfigHome({ home, platform, env }), "Claude"); - return join(home, "Library", "Application Support", "Claude"); +/** Windows packaging decides which file Claude Desktop reads; see its resolver. */ +const claudeDesktopLocation = ( + context: ClientPathContext, +): Result => { + if (context.platform === "win32") + return resolveWindowsClaudeDesktopLocation(context.home, context.env); + const directory = + context.platform === "linux" + ? join(xdgConfigHome(context), "Claude") + : join(context.home, "Library", "Application Support", "Claude"); + return ok({ + configPath: join(directory, "claude_desktop_config.json"), + markerPath: directory, + }); }; const claudeCodeConfigDirectory = ({ home, env }: ClientPathContext): string => @@ -211,6 +244,55 @@ export const manualRegistrationRemediation = ( ? GROK_BOT_MANUAL_REGISTRATION_REMEDIATION : undefined; +/** + * Facts a setup plan must disclose so an approved registration persists: + * which file an MSIX package reads, and that a running client can overwrite it. + */ +export const registrationPersistenceDetail = ( + client: SetupClient, +): string | undefined => { + const name = client.displayName ?? client.name; + const msix = client.msixPackage; + const packageDetail = + msix === undefined + ? undefined + : client.configPath === msix.privateConfigPath + ? `${name} is installed as MSIX package ${msix.packageFamilyName}, which reads this package-private file instead of ${msix.appDataConfigPath}.` + : `${name} is installed as MSIX package ${msix.packageFamilyName}, which reads this file until it saves its own copy at ${msix.privateConfigPath}.`; + const quitDetail = + client.quitBeforeSetup === true + ? `Quit ${name} fully before approving; closing its window is not enough. A running ${name} can rewrite this file from memory and drop the registration. Start it again after setup.` + : undefined; + const details = [packageDetail, quitDetail].filter( + (detail): detail is string => detail !== undefined, + ); + return details.length === 0 ? undefined : details.join(" "); +}; + +/** + * Why this process must not change an MSIX client's configuration. A process + * the running app started sees its redirected AppData, and the app would + * overwrite the change from memory; an unknown view cannot rule that out. + */ +export const registrationWriteBlocker = ( + client: SetupClient, + operation: "setup" | "uninstall", +): string | undefined => { + const msix = client.msixPackage; + if (msix === undefined || msix.processView === "direct") return undefined; + const name = client.displayName ?? client.name; + return msix.processView === "redirected" + ? `This process was started from ${name}'s MSIX package ${msix.packageFamilyName}: its AppData is redirected into that package, so ${name} is running and would overwrite ${msix.privateConfigPath} from memory. Quit ${name} fully, then rerun ${operation} from a terminal that ${name} did not start.` + : `REA cannot tell whether ${name}'s MSIX package ${msix.packageFamilyName} started this process, because none of its files under ${dirname(msix.privateConfigPath)} could be compared with ${dirname(msix.appDataConfigPath)}. A running ${name} would overwrite the change. Open ${name} once if it has never run, quit it fully, then rerun ${operation} from a terminal that ${name} did not start.`; +}; + +/** The MSIX package view, if any, that redirects this process's AppData writes. */ +export const appDataRedirection = ( + clients: readonly SetupClient[], +): MsixPackageView | undefined => + clients.find(({ msixPackage }) => msixPackage?.processView === "redirected") + ?.msixPackage; + const OMP_PROFILE_NAME = /^[a-z0-9][a-z0-9._-]{0,63}$/u; const OMP_RESERVED_PROFILE = /^(?:CON|PRN|AUX|NUL|COM[0-9]|LPT[0-9])(?:\..*)?$/iu; @@ -408,9 +490,8 @@ export const SUPPORTED_CLIENT_DEFINITIONS = [ { name: "claude_desktop", displayName: "Claude Desktop", - configPath: (context: ClientPathContext) => - join(claudeDesktopDirectory(context), "claude_desktop_config.json"), - markerPath: claudeDesktopDirectory, + location: claudeDesktopLocation, + quitBeforeSetup: true, format: "json", }, { @@ -627,6 +708,19 @@ const resolvePath = ( return typeof resolved === "string" ? ok(resolved) : resolved; }; +const resolveLocation = ( + definition: ClientDefinition, + context: ClientPathContext, +): Result => { + if (definition.location !== undefined) return definition.location(context); + const config = resolvePath(definition.configPath, context); + if (!config.ok) return config; + const marker = resolvePath(definition.markerPath, context); + return marker.ok + ? ok({ configPath: config.value, markerPath: marker.value }) + : marker; +}; + /** Describe every client location that setup, doctor, or uninstall may inspect. */ export const supportedClients = ( home: string, @@ -637,27 +731,16 @@ export const supportedClients = ( const definitions: readonly ClientDefinition[] = SUPPORTED_CLIENT_DEFINITIONS; return definitions.map((definition) => { try { - const config = resolvePath(definition.configPath, context); - const marker = config.ok - ? resolvePath(definition.markerPath, context) - : config; - if (!config.ok) + const location = resolveLocation(definition, context); + if (!location.ok) return { name: definition.name, displayName: definition.displayName, format: definition.format, - configPath: config.error.path, - configPathError: config.error.message, + configPath: location.error.path, + configPathError: location.error.message, }; - if (!marker.ok) - return { - name: definition.name, - displayName: definition.displayName, - format: definition.format, - configPath: marker.error.path, - configPathError: marker.error.message, - }; - const configPath = config.value; + const { configPath, markerPath, msixPackage } = location.value; return { name: definition.name, displayName: definition.displayName, @@ -675,7 +758,11 @@ export const supportedClients = ( ...(definition.zcodeFallbackPath === undefined ? {} : { zcodeFallbackPath: definition.zcodeFallbackPath(context) }), - markerPath: marker.value, + markerPath, + ...(msixPackage === undefined ? {} : { msixPackage }), + ...(definition.quitBeforeSetup === true + ? { quitBeforeSetup: true } + : {}), format: definition.format, }; } catch (cause: unknown) { diff --git a/src/application/Uninstall.ts b/src/application/Uninstall.ts index a7f051a49..a4561c45e 100644 --- a/src/application/Uninstall.ts +++ b/src/application/Uninstall.ts @@ -32,10 +32,16 @@ import { } from "./RegularFileRead.js"; import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; import { + appDataRedirection, manualRegistrationRemediation, + registrationWriteBlocker, supportedClients, type SetupClient, } from "./SupportedClients.js"; +import { + redirectedWriteBlocker, + type MsixPackageView, +} from "./MsixPackageStorage.js"; export { isUninstallFailure } from "./UninstallFailure.js"; interface ManagedPathStats { @@ -93,6 +99,7 @@ export interface UninstallHost { /** Report a configuration that cannot be read safely, before any removal. */ inspectClient(client: SetupClient): Promise; removeClient(client: SetupClient): Promise; + inspectSkill?(): Promise; removeSkill(): Promise; purgeData(): Promise; close?(): Promise; @@ -118,6 +125,8 @@ export const runUninstall = async ( const blocking = await host.inspectClient(client); if (blocking !== undefined) blocked.push(blocking); } + const skillBlocker = await host.inspectSkill?.(); + if (skillBlocker !== undefined) blocked.push(skillBlocker); const items: UninstallItem[] = blocked.length > 0 ? [ @@ -209,27 +218,70 @@ export const systemUninstallHost = ( selectedHome ?? homeDirectoryFromEnvironment(environment, platform); const resources = new ArtifactResourceScope(); const fileSystem = selectedFileSystem ?? systemFileSystem(resources); + // Set with the client list: the package view that redirects this + // process's AppData writes, if any. + let redirection: MsixPackageView | undefined; + const clientWriteBlocker = (client: SetupClient): string | undefined => + registrationWriteBlocker(client, "uninstall") ?? + redirectedWriteBlocker(redirection, [client.configPath], "uninstall"); + const skillWriteBlocker = (path: string): string | undefined => + redirectedWriteBlocker(redirection, [path], "uninstall"); return { - clients: () => - Promise.resolve( - supportedClients(home, platform, environment).flatMap((client) => - client.configPaths === undefined - ? [client] - : client.configPaths.map((configPath) => ({ - ...client, - configPath, - })), + clients: () => { + const clients = supportedClients(home, platform, environment); + redirection = appDataRedirection(clients); + return Promise.resolve( + clients.flatMap((client) => + uninstallConfigPaths(client).map((configPath) => ({ + ...client, + configPath, + })), ), - ), + ); + }, inspectClient: async (client) => { const read = await readClientConfiguration(client, fileSystem); - return read.kind === "item" && read.item.status === "failed" - ? read.item - : undefined; + if (read.kind === "item") + return read.item.status === "failed" ? read.item : undefined; + const removal = clientRegistrationRemoval(read.parsed, client); + if (!removal.removeNative && removal.legacyPath === undefined) + return undefined; + // A removal the running package would revert, or redirect into its + // private storage, would leave the registration in place. + const blocker = clientWriteBlocker(client); + return blocker === undefined + ? undefined + : item(client.name, "failed", blocker); + }, + removeClient: (client) => + removeClient(client, fileSystem, () => clientWriteBlocker(client)), + inspectSkill: async () => { + for (const destination of skillDestinations( + home, + undefined, + environment, + platform, + )) { + const inspected = await inspectManagedSkill( + destination.path, + destination.client, + fileSystem, + ); + if (inspected?.status === "failed") return inspected; + if (inspected !== undefined) continue; + const blocker = skillWriteBlocker(destination.path); + if (blocker !== undefined) return item("skill", "failed", blocker); + } + return undefined; }, - removeClient: (client) => removeClient(client, fileSystem), removeSkill: () => - removeManagedSkills(home, fileSystem, environment, platform), + removeManagedSkills( + home, + fileSystem, + environment, + platform, + skillWriteBlocker, + ), purgeData: () => purgeManagedData(join(home, ".rea"), fileSystem), close: async () => { try { @@ -242,6 +294,36 @@ export const systemUninstallHost = ( }; }; +/** + * Every user file that can hold REA's registration for one client. An MSIX + * package can read either its private copy or the AppData file, and earlier + * setups wrote the AppData file, so both lose REA's entry. + */ +const uninstallConfigPaths = (client: SetupClient): readonly string[] => [ + ...new Set([ + ...(client.configPaths ?? [client.configPath]), + ...(client.msixPackage === undefined + ? [] + : [ + client.msixPackage.privateConfigPath, + client.msixPackage.appDataConfigPath, + ]), + ]), +]; + +/** Whether removal would change this configuration. */ +const registrationPresent = (parsed: ClientConfigurationDocument): boolean => { + const name = PRODUCT_IDENTITY.mcpServerKey; + return ( + Object.hasOwn(parsed.servers, name) || + // Beside a native OpenCode V2 table, OpenCode also loads a V1 `mcp.rea`. + legacyClientServerPath(parsed, name) !== undefined || + // Harness ownership is the stable entry id. A stale or malformed launcher + // still belongs to that id, and so does a direct patch that projects no row. + (parsed.dialect === "dsh" && dshDocument(parsed.document).carriesEntry) + ); +}; + type ClientConfigurationRead = | { readonly kind: "item"; readonly item: UninstallItem } | { @@ -308,21 +390,47 @@ const itemRead = (result: UninstallItem): ClientConfigurationRead => ({ item: result, }); +/** + * Share the exact ownership and applicability decision with preflight. A + * removable legacy entry carries its document path, so removal edits exactly + * the entry this decision admitted. + */ +const clientRegistrationRemoval = ( + parsed: ClientConfigurationDocument, + client: SetupClient, +): { + readonly removeNative: boolean; + readonly legacyPath: readonly string[] | undefined; +} => { + const name = PRODUCT_IDENTITY.mcpServerKey; + if ( + parsed.dialect === "dsh" && + dshDocument(parsed.document).hasUnknownStructure + ) + return { removeNative: false, legacyPath: undefined }; + const legacyPath = legacyClientServerPath(parsed, name); + return { + removeNative: + (parsed.dialect === "dsh" && dshDocument(parsed.document).carriesEntry) || + (Object.hasOwn(parsed.servers, name) && + isOwnedRegistration(parsed.servers[name], client)), + legacyPath: + legacyPath !== undefined && + isOwnedRegistration(parsed.legacyServers[name], client) + ? legacyPath + : undefined, + }; +}; + const removeClient = async ( client: SetupClient, fileSystem: UninstallFileSystem, + writeBlocker?: () => string | undefined, ): Promise => { const read = await readClientConfiguration(client, fileSystem); if (read.kind === "item") return read.item; const { transactionPath, original, parsed } = read; const name = PRODUCT_IDENTITY.mcpServerKey; - // Beside a native OpenCode V2 table, OpenCode also loads a V1 `mcp.rea`. - const legacyPath = legacyClientServerPath(parsed, name); - const hasNative = Object.hasOwn(parsed.servers, name); - // Harness ownership is the stable entry id. A stale or malformed launcher - // still belongs to that id, and so does a direct patch that projects no row. - const carriesDshEntry = - parsed.dialect === "dsh" && dshDocument(parsed.document).carriesEntry; if ( parsed.dialect === "dsh" && dshDocument(parsed.document).hasUnknownStructure @@ -332,20 +440,21 @@ const removeClient = async ( "retained", "A Cordis expression may produce registration rows, so the patch was retained without changes.", ); - if (!hasNative && legacyPath === undefined && !carriesDshEntry) + if (!registrationPresent(parsed)) return item(client.name, "skipped", "REA registration is absent."); - const removeNative = - carriesDshEntry || - (hasNative && isOwnedRegistration(parsed.servers[name], client)); - const removeLegacy = - legacyPath !== undefined && - isOwnedRegistration(parsed.legacyServers[name], client); + const { removeNative, legacyPath } = clientRegistrationRemoval( + parsed, + client, + ); + const removeLegacy = legacyPath !== undefined; if (!removeNative && !removeLegacy) return item( client.name, "retained", "The rea-named registration is not REA-owned.", ); + const blocker = writeBlocker?.(); + if (blocker !== undefined) return item(client.name, "failed", blocker); const remaining = { ...parsed.servers }; if (removeNative) delete remaining[name]; const document = withClientServers( @@ -355,7 +464,7 @@ const removeClient = async ( ); const editedPaths = [ ...(removeNative ? [clientServerPath(parsed, name)] : []), - ...(removeLegacy ? [legacyPath] : []), + ...(legacyPath === undefined ? [] : [legacyPath]), ]; const backupPath = `${client.configPath}.rea.backup`; try { @@ -516,12 +625,13 @@ const removeManagedSkills = async ( fileSystem: UninstallFileSystem, environment: Readonly, platform: NodeJS.Platform, + writeBlocker?: (path: string) => string | undefined, ): Promise => { // Every destination shares one summary item, so each result is labeled by // the directory it refers to. const results = await Promise.all( skillDestinations(home, undefined, environment, platform).map(({ path }) => - removeManagedSkill(path, path, fileSystem), + removeManagedSkill(path, path, fileSystem, writeBlocker), ), ); const failed = results.find(({ status }) => status === "failed"); @@ -552,11 +662,11 @@ const removeManagedSkills = async ( ); }; -const removeManagedSkill = async ( +const inspectManagedSkill = async ( path: string, name: string, fileSystem: UninstallFileSystem, -): Promise => { +): Promise => { let root: ManagedPathStats; try { root = await fileSystem.stat(path); @@ -594,6 +704,20 @@ const removeManagedSkill = async ( if (!isManagedSkillManifest(content)) return item(name, "retained", "Skill manifest is not REA-owned."); + return undefined; +}; + +const removeManagedSkill = async ( + path: string, + name: string, + fileSystem: UninstallFileSystem, + writeBlocker?: (path: string) => string | undefined, +): Promise => { + const inspection = await inspectManagedSkill(path, name, fileSystem); + if (inspection !== undefined) return inspection; + const blocker = writeBlocker?.(path); + if (blocker !== undefined) return item(name, "failed", blocker); + const manifestName = "SKILL.md"; const otherFiles = MANAGED_SKILL_FILES.filter( (relativePath) => relativePath !== manifestName, diff --git a/tests/boundary/filesystem/claudeDesktopClientConfiguration.test.ts b/tests/boundary/filesystem/claudeDesktopClientConfiguration.test.ts new file mode 100644 index 000000000..b411aeb4b --- /dev/null +++ b/tests/boundary/filesystem/claudeDesktopClientConfiguration.test.ts @@ -0,0 +1,412 @@ +import { + access, + mkdir, + readFile, + rm, + symlink, + writeFile, +} from "node:fs/promises"; +import { dirname, join } from "node:path"; + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { CLAUDE_DESKTOP_PACKAGE_FAMILY } from "../../../src/application/ClaudeDesktopConfigLocation.js"; +import { readClientRegistrationStatuses } from "../../../src/application/ClientRegistrationStatus.js"; +import { npxRegistrationCommand } from "../../../src/application/ClientRegistrationIdentity.js"; +import { + configureClientConfiguration, + inspectClientConfiguration, +} from "../../../src/application/SetupClientConfiguration.js"; +import { detectClients } from "../../../src/application/SetupHost.js"; +import { + supportedClients, + type SetupClient, +} from "../../../src/application/SupportedClients.js"; +import { + runUninstall, + systemUninstallHost, +} from "../../../src/application/Uninstall.js"; +import { clearClientLocationEnvironment } from "../../fixtures/clientEnvironment.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; + +/** The Windows registration setup writes, independent of the test host. */ +const command = npxRegistrationCommand("win32"); +const CONFIG = "claude_desktop_config.json"; + +const roots: string[] = []; +beforeEach(clearClientLocationEnvironment); +afterEach(async () => { + vi.unstubAllEnvs(); + await Promise.all( + roots.splice(0).map((root) => rm(root, { recursive: true, force: true })), + ); +}); + +/** A Windows profile with separate roaming and local AppData roots. */ +const windowsProfile = async () => { + const home = await createTestTempDirectory("rea-claude-desktop-"); + roots.push(home); + const env = { + APPDATA: join(home, "AppData", "Roaming"), + LOCALAPPDATA: join(home, "AppData", "Local"), + }; + const appDataConfig = join(env.APPDATA, "Claude", CONFIG); + const container = join( + env.LOCALAPPDATA, + "Packages", + CLAUDE_DESKTOP_PACKAGE_FAMILY, + ); + const privateConfig = join( + container, + "LocalCache", + "Roaming", + "Claude", + CONFIG, + ); + return { home, env, appDataConfig, container, privateConfig }; +}; + +const claudeDesktop = ( + home: string, + env: Readonly>, +): SetupClient => { + const client = supportedClients(home, "win32", env).find( + ({ name }) => name === "claude_desktop", + ); + if (client === undefined) throw new Error("claude_desktop is not supported"); + return client; +}; + +const writeJson = async (path: string, value: unknown): Promise => { + await mkdir(dirname(path), { recursive: true }); + await writeFile(path, `${JSON.stringify(value, null, 2)}\n`); +}; + +const readJson = async (path: string): Promise> => + JSON.parse(await readFile(path, "utf8")) as Record; + +const reaEntry = { command: command[0], args: command.slice(1) }; + +describe("Windows Claude Desktop configuration location", () => { + it("uses AppData when no MSIX package storage exists", async () => { + const { home, env, appDataConfig } = await windowsProfile(); + await mkdir( + join(env.LOCALAPPDATA, "Packages", "Contoso.App_8wekyb3d8bbwe"), + { + recursive: true, + }, + ); + + const client = claudeDesktop(home, env); + + expect(client.configPath).toBe(appDataConfig); + expect(client.markerPath).toBe(dirname(appDataConfig)); + expect(client.msixPackage).toBeUndefined(); + expect(client.quitBeforeSetup).toBe(true); + }); + + it("uses the package-private copy that the MSIX package reads", async () => { + const { home, env, appDataConfig, container, privateConfig } = + await windowsProfile(); + await writeJson(privateConfig, { preferences: { theme: "dark" } }); + await writeJson(appDataConfig, { mcpServers: {} }); + + const client = claudeDesktop(home, env); + + expect(client.configPath).toBe(privateConfig); + expect(client.markerPath).toBe(container); + expect(client.msixPackage).toEqual({ + packageFamilyName: CLAUDE_DESKTOP_PACKAGE_FAMILY, + privateConfigPath: privateConfig, + appDataConfigPath: appDataConfig, + virtualizedRoots: [env.APPDATA, env.LOCALAPPDATA], + processView: "direct", + }); + expect( + (await detectClients(home, "win32", env)).map(({ name }) => name), + ).toContain("claude_desktop"); + }); + + it("uses AppData while the MSIX package has no private copy", async () => { + const { home, env, appDataConfig, container, privateConfig } = + await windowsProfile(); + await mkdir(container, { recursive: true }); + await writeJson(appDataConfig, { preferences: {} }); + + const client = claudeDesktop(home, env); + + expect(client.configPath).toBe(appDataConfig); + expect(client.msixPackage).toMatchObject({ + privateConfigPath: privateConfig, + processView: "unknown", + }); + // No private file shows whether the running package started this process. + const inspection = await inspectClientConfiguration(client, {}, command); + expect(inspection.status).toBe("invalid"); + expect(inspection.status === "invalid" && inspection.remediation).toContain( + "Open Claude Desktop once if it has never run, quit it fully, then rerun setup", + ); + }); + + it("creates the private copy when the MSIX package has no configuration", async () => { + const { home, env, privateConfig } = await windowsProfile(); + await writeJson(join(dirname(privateConfig), "Local State"), {}); + + const client = claudeDesktop(home, env); + expect(client.configPath).toBe(privateConfig); + expect(client.msixPackage?.processView).toBe("direct"); + await expect( + inspectClientConfiguration(client, {}, command), + ).resolves.toEqual({ status: "create" }); + await expect( + configureClientConfiguration(client, {}, command), + ).resolves.toEqual({ status: "configured" }); + expect(await readJson(privateConfig)).toEqual({ + mcpServers: { rea: reaEntry }, + }); + }); + + it("reports an unrecognized Claude package instead of falling back to AppData", async () => { + const { home, env, appDataConfig } = await windowsProfile(); + const foreign = join(env.LOCALAPPDATA, "Packages", "Claude_abcdefghjkmnp"); + await mkdir(foreign, { recursive: true }); + await writeJson(appDataConfig, { mcpServers: {} }); + + const client = claudeDesktop(home, env); + + expect(client.configPath).toBe(foreign); + expect(client.configPathError).toContain("unrecognized publisher"); + expect(client.configPathError).toContain(CLAUDE_DESKTOP_PACKAGE_FAMILY); + expect( + (await detectClients(home, "win32", env)).map(({ name }) => name), + ).not.toContain("claude_desktop"); + const status = ( + await readClientRegistrationStatuses(home, "unused", { + platform: "win32", + environment: env, + }) + ).find(({ client: name }) => name === "claude_desktop"); + expect(status).toMatchObject({ state: "invalid", config_path: foreign }); + await expect( + inspectClientConfiguration(client, {}, command), + ).resolves.toMatchObject({ status: "invalid" }); + }); +}); + +describe("MSIX Claude Desktop registration lifecycle", () => { + it("registers in the private copy and reports an AppData-only entry as missing", async () => { + const { home, env, appDataConfig, privateConfig } = await windowsProfile(); + const privateOriginal = { + coworkUserFilesPath: "C:\\Users\\a\\Claude", + preferences: { menuBarEnabled: false }, + mcpServers: { other: { command: "other", args: ["serve"] } }, + }; + await writeJson(privateConfig, privateOriginal); + // Earlier setups wrote AppData, which the package never reads. + await writeJson(appDataConfig, { mcpServers: { rea: reaEntry } }); + const appDataOriginal = await readFile(appDataConfig, "utf8"); + const status = async () => + ( + await readClientRegistrationStatuses(home, "unused", { + platform: "win32", + environment: env, + }) + ).find(({ client }) => client === "claude_desktop"); + + expect(await status()).toMatchObject({ + state: "missing", + config_path: privateConfig, + }); + expect((await status())?.remediation).toContain( + "Quit Claude Desktop fully", + ); + + const client = claudeDesktop(home, env); + await expect( + inspectClientConfiguration(client, {}, command), + ).resolves.toEqual({ + status: "update", + backupPath: `${privateConfig}.rea.backup`, + }); + await expect( + configureClientConfiguration(client, {}, command), + ).resolves.toEqual({ + status: "configured", + backupPath: `${privateConfig}.rea.backup`, + }); + + expect(await readJson(privateConfig)).toEqual({ + ...privateOriginal, + mcpServers: { + ...privateOriginal.mcpServers, + rea: reaEntry, + }, + }); + expect(await readJson(`${privateConfig}.rea.backup`)).toEqual( + privateOriginal, + ); + expect(await readFile(appDataConfig, "utf8")).toBe(appDataOriginal); + expect(await status()).toMatchObject({ + state: "aligned", + config_path: privateConfig, + }); + await expect( + configureClientConfiguration(client, {}, command), + ).resolves.toEqual({ status: "unchanged" }); + }); + + it("refuses to write from a process the running package started", async () => { + const { home, env, privateConfig } = await windowsProfile(); + await writeJson(privateConfig, { preferences: {} }); + const resolved = claudeDesktop(home, env); + if (resolved.msixPackage === undefined) + throw new Error("expected MSIX package storage"); + // Only a packaged process sees AppData redirected; model that observation. + const client: SetupClient = { + ...resolved, + msixPackage: { ...resolved.msixPackage, processView: "redirected" }, + }; + + const inspection = await inspectClientConfiguration(client, {}, command); + + expect(inspection.status).toBe("invalid"); + expect(inspection.status === "invalid" && inspection.remediation).toContain( + "Quit Claude Desktop fully, then rerun setup from a terminal that Claude Desktop did not start", + ); + expect(await readJson(privateConfig)).toEqual({ preferences: {} }); + + await configureClientConfiguration(resolved, {}, command); + await expect( + inspectClientConfiguration(client, {}, command), + ).resolves.toEqual({ status: "already_current" }); + }); + + it("refuses a write once the package reads a newer private copy", async () => { + const { home, env, appDataConfig, privateConfig } = await windowsProfile(); + await writeJson(join(dirname(privateConfig), "Preferences"), {}); + await writeJson(appDataConfig, { preferences: {} }); + const client = claudeDesktop(home, env); + expect(client.configPath).toBe(appDataConfig); + // Quitting the app after the plan was shown can save its private copy. + await writeJson(privateConfig, { preferences: {} }); + + await expect( + configureClientConfiguration(client, {}, command), + ).resolves.toEqual({ status: "failed", reason: "target_changed" }); + expect(await readJson(appDataConfig)).toEqual({ preferences: {} }); + expect(await readJson(privateConfig)).toEqual({ preferences: {} }); + }); + + it("registers in AppData while the package has no private copy", async () => { + const { home, env, appDataConfig, privateConfig } = await windowsProfile(); + await writeJson(join(dirname(privateConfig), "Preferences"), {}); + await writeJson(appDataConfig, { preferences: { theme: "light" } }); + + const client = claudeDesktop(home, env); + expect(client.configPath).toBe(appDataConfig); + expect(client.msixPackage?.processView).toBe("direct"); + await expect( + configureClientConfiguration(client, {}, command), + ).resolves.toMatchObject({ status: "configured" }); + + expect(await readJson(appDataConfig)).toEqual({ + preferences: { theme: "light" }, + mcpServers: { rea: reaEntry }, + }); + await expect(access(privateConfig)).rejects.toMatchObject({ + code: "ENOENT", + }); + }); +}); + +describe("MSIX Claude Desktop uninstall", () => { + it("refuses uninstall when the package view is unknown", async () => { + const { home, env, appDataConfig, container } = await windowsProfile(); + await mkdir(container, { recursive: true }); + await writeJson(appDataConfig, { mcpServers: { rea: reaEntry } }); + + const result = await runUninstall( + false, + systemUninstallHost(home, undefined, env, "win32"), + ); + + expect(result.status).toBe("failed"); + expect(result.items).toContainEqual( + expect.objectContaining({ + name: "claude_desktop", + status: "failed", + detail: expect.stringContaining("rerun uninstall"), + }), + ); + expect(await readJson(appDataConfig)).toEqual({ + mcpServers: { rea: reaEntry }, + }); + }); + + it("retains an unrelated rea-named entry when the package view is unknown", async () => { + const { home, env, appDataConfig, container } = await windowsProfile(); + await mkdir(container, { recursive: true }); + const unrelated = { mcpServers: { rea: { command: "unrelated-server" } } }; + await writeJson(appDataConfig, unrelated); + const result = await runUninstall( + false, + systemUninstallHost(home, undefined, env, "win32"), + ); + expect(result.status).toBe("complete"); + expect(await readJson(appDataConfig)).toEqual(unrelated); + }); + + it("blocks managed skill removal through redirected AppData before any removal", async () => { + const { home, env, appDataConfig, privateConfig } = await windowsProfile(); + await writeJson(join(dirname(privateConfig), "Preferences"), {}); + await mkdir(dirname(dirname(appDataConfig)), { recursive: true }); + await symlink(dirname(privateConfig), dirname(appDataConfig), "dir"); + const agents = join(env.APPDATA, "dsh", "agents"); + const manifest = join( + agents, + "skills", + "reverse-engineer-anything", + "SKILL.md", + ); + await mkdir(dirname(manifest), { recursive: true }); + await writeFile( + manifest, + "---\nname: reverse-engineer-anything\n---\nmanaged fixture\n", + ); + const environment = { ...env, DSH_AGENTS_HOME: agents }; + const result = await runUninstall( + false, + systemUninstallHost(home, undefined, environment, "win32"), + ); + expect(result.status).toBe("failed"); + expect(result.items).toContainEqual( + expect.objectContaining({ + name: "skill", + status: "failed", + detail: expect.stringContaining("redirects its writes"), + }), + ); + expect(await readFile(manifest, "utf8")).toContain("managed fixture"); + }); + + it("removes REA from both the private copy and AppData on uninstall", async () => { + const { home, env, appDataConfig, privateConfig } = await windowsProfile(); + await writeJson(privateConfig, { + preferences: {}, + mcpServers: { other: { command: "other" }, rea: reaEntry }, + }); + await writeJson(appDataConfig, { mcpServers: { rea: reaEntry } }); + + const result = await runUninstall( + false, + systemUninstallHost(home, undefined, env, "win32"), + ); + + expect(result.status).toBe("complete"); + expect(await readJson(privateConfig)).toEqual({ + preferences: {}, + mcpServers: { other: { command: "other" } }, + }); + expect(await readJson(appDataConfig)).toEqual({ mcpServers: {} }); + }); +}); diff --git a/tests/process-global/updateMaintenance.test.ts b/tests/process-global/updateMaintenance.test.ts index ae057bab2..171761478 100644 --- a/tests/process-global/updateMaintenance.test.ts +++ b/tests/process-global/updateMaintenance.test.ts @@ -25,6 +25,7 @@ beforeEach(async () => { HOME: home, USERPROFILE: home, APPDATA: join(home, "AppData", "Roaming"), + LOCALAPPDATA: join(home, "AppData", "Local"), CLAUDE_CONFIG_DIR: home, CODEX_HOME: join(home, ".codex"), COPILOT_HOME: join(home, ".copilot"),