diff --git a/docs/client-configuration.md b/docs/client-configuration.md index 9cdc4e62d3..d5fa7c2f24 100644 --- a/docs/client-configuration.md +++ b/docs/client-configuration.md @@ -52,10 +52,16 @@ in the file the installed app reads: `%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 + another publisher, an `AnthropicPBC.Claude_` package, 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. + The reported `AnthropicPBC.Claude_fnn82j28hfe8t` build can use the `Claude-3p` + deployment directory. REA recognizes that package as an unresolved location; + it does not assume that it reads the legacy AppData file or automatically + register in `Claude-3p`. See [#4432](https://github.com/morluto/rea/issues/4432) + for the reported build and manual registration evidence. Quit Claude Desktop fully before approving setup; closing its window is not enough. A running Claude Desktop can rewrite its configuration from memory and diff --git a/src/application/ClaudeDesktopConfigLocation.test.ts b/src/application/ClaudeDesktopConfigLocation.test.ts index 84d8ca8bc9..5790da3e4f 100644 --- a/src/application/ClaudeDesktopConfigLocation.test.ts +++ b/src/application/ClaudeDesktopConfigLocation.test.ts @@ -12,6 +12,7 @@ import { packagePrivateStorageFamily, redirectedWriteBlocker, type MsixPackageView, + windowsAppDataRoots, } from "./MsixPackageStorage.js"; const home = join("/profile", "a"); @@ -38,6 +39,15 @@ const privateDirectory = dirname(privateConfig); const privatePreferences = join(privateDirectory, "Preferences"); const appDataDirectory = dirname(appDataConfig); +it.each([undefined, ""])( + "uses default AppData roots only for unset or empty values: %s", + (value) => { + expect( + windowsAppDataRoots(home, { APPDATA: value, LOCALAPPDATA: value }), + ).toEqual({ roaming: env.APPDATA, local: env.LOCALAPPDATA }); + }, +); + /** Package storage holding `privateFiles`; realpath decides the process view. */ const packagedFileSystem = ( realpath: ClaudeDesktopLocationFileSystem["realpath"], @@ -153,6 +163,22 @@ describe("Claude Desktop MSIX process view", () => { }); describe("unresolved Claude Desktop package identity", () => { + it.each([ + "Notes.AnthropicPBC.Claude_fnn82j28hfe8t", + "AnthropicPBC.Claude_fnn82j28hfe8t.backup", + "AnthropicPBCxClaude_fnn82j28hfe8t", + ])("does not treat %s as a Claude package family", (name) => { + const location = resolveWindowsClaudeDesktopLocation(home, env, { + readdir: () => [name], + lstat: () => file, + realpath: (path) => path, + }); + expect(location).toEqual({ + ok: true, + value: { configPath: appDataConfig, markerPath: appDataDirectory }, + }); + }); + it("reports unreadable package storage instead of assuming AppData", () => { const location = resolveWindowsClaudeDesktopLocation(home, env, { readdir: () => { diff --git a/src/application/ClaudeDesktopConfigLocation.ts b/src/application/ClaudeDesktopConfigLocation.ts index 899aadbbb3..0a0208126e 100644 --- a/src/application/ClaudeDesktopConfigLocation.ts +++ b/src/application/ClaudeDesktopConfigLocation.ts @@ -19,8 +19,9 @@ const PROCESS_VIEW_PROBES = 8; */ 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; +/** Observed Claude package names plus Windows' 13-character publisher ID. */ +const CLAUDE_PACKAGE_FAMILY = + /^(?:AnthropicPBC\.)?Claude_[0-9a-hjkmnp-tv-z]{13}$/iu; /** The configuration Claude Desktop reads and the evidence that it is installed. */ export interface ClaudeDesktopLocation { @@ -52,8 +53,8 @@ const systemFileSystem: ClaudeDesktopLocationFileSystem = { * 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. + * otherwise; a new registration goes to the private copy. Other Claude + * package identities and deployments leave the reader unknown. */ export const resolveWindowsClaudeDesktopLocation = ( home: string, @@ -84,7 +85,7 @@ export const resolveWindowsClaudeDesktopLocation = ( 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.`, + message: `MSIX package storage ${foreign.map((name) => join(packagesDirectory, name)).join(", ")} has an unrecognized Claude Desktop package identity or deployment (the supported identity is ${CLAUDE_DESKTOP_PACKAGE_FAMILY}). REA cannot establish which ${CONFIG_FILE} that package reads and does not fall back to ${appDataConfigPath}. Quit Claude Desktop fully and register REA in the configuration the installed app reads.`, }); const family = families[0]; if (family === undefined) diff --git a/src/application/MsixPackageStorage.ts b/src/application/MsixPackageStorage.ts index 098b2f1db7..9eb3dede08 100644 --- a/src/application/MsixPackageStorage.ts +++ b/src/application/MsixPackageStorage.ts @@ -30,8 +30,8 @@ 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"), + roaming: env.APPDATA || join(home, "AppData", "Roaming"), + local: env.LOCALAPPDATA || join(home, "AppData", "Local"), }); /** The per-user root of MSIX package data containers. */ @@ -165,6 +165,14 @@ export const redirectedWriteBlocker = ( 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.`; }; +/** An unavailable read-target observation does not establish a target change. */ +interface MsixReadTargetInspectionFailure { + readonly _tag: "MsixReadTargetInspectionFailure"; + readonly path: string; + readonly detail: string; + readonly cause: unknown; +} + /** * Whether the package began reading its private copy after the AppData file * was chosen, for example by saving its settings when it quit. @@ -172,16 +180,23 @@ export const redirectedWriteBlocker = ( export const packageReadTargetChanged = async ( view: MsixPackageView, configPath: string, -): Promise => { - if (configPath !== view.appDataConfigPath) return false; +): Promise> => { + if (configPath !== view.appDataConfigPath) return ok(false); try { await lstat(view.privateConfigPath); - return true; + return ok(true); } catch (cause: unknown) { - return !( + if ( cause instanceof Error && "code" in cause && (cause.code === "ENOENT" || cause.code === "ENOTDIR") - ); + ) + return ok(false); + return err({ + _tag: "MsixReadTargetInspectionFailure", + path: view.privateConfigPath, + detail: cause instanceof Error ? cause.message : String(cause), + cause, + }); } }; diff --git a/src/application/SetupClientConfiguration.ts b/src/application/SetupClientConfiguration.ts index 9b68031af1..7ca8d94178 100644 --- a/src/application/SetupClientConfiguration.ts +++ b/src/application/SetupClientConfiguration.ts @@ -230,11 +230,23 @@ const persistClientConfiguration = async ({ }: 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" }; + if (client.msixPackage !== undefined) { + const target = await packageReadTargetChanged( + client.msixPackage, + client.configPath, + ); + if (!target.ok) + return { + status: "failed", + reason: "target_unavailable", + failure: { + kind: "inspect", + path: target.error.path, + detail: target.error.detail, + }, + }; + if (target.value) return { status: "failed", reason: "target_changed" }; + } const backupPath = original === undefined ? undefined : `${client.configPath}.rea.backup`; let recoveryBackupPath: string | undefined; diff --git a/src/application/SetupClients.ts b/src/application/SetupClients.ts index 5ceccb21a8..04d0261f0c 100644 --- a/src/application/SetupClients.ts +++ b/src/application/SetupClients.ts @@ -11,6 +11,8 @@ export const failedConfigurationMessage = ( ): 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 === "target_unavailable") + return `The client configuration read target could not be established because ${result.failure.path} could not be inspected: ${result.failure.detail}. Repair access to that path, then rerun setup.`; 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/SetupSkill.ts b/src/application/SetupSkill.ts index eb7b4b138f..9a5e200e47 100644 --- a/src/application/SetupSkill.ts +++ b/src/application/SetupSkill.ts @@ -445,7 +445,7 @@ const inspectCanonicalSkill = async ( } } if (clientIds !== undefined || rootHasFiles || rootIssues.length > 0) { - installedDestination = true; + installedDestination ||= rootHasFiles; canonical = canonical && rootCanonical; issues.push(...rootIssues); } @@ -646,7 +646,6 @@ export const readInstalledSkillIdentity = async ( ? inspection.value.manifest : inspection.error.manifest; const issues = inspection.ok ? [] : inspection.error.issues; - if (content === undefined && issues.length === 0) return undefined; const toolCount = content === undefined ? undefined diff --git a/src/application/SetupTypes.ts b/src/application/SetupTypes.ts index afebad211a..4ddf376b15 100644 --- a/src/application/SetupTypes.ts +++ b/src/application/SetupTypes.ts @@ -42,6 +42,15 @@ export type ClientConfigurationResult = | "write" | "readback"; } + | { + readonly status: "failed"; + readonly reason: "target_unavailable"; + readonly failure: { + readonly kind: "inspect"; + readonly path: string; + readonly detail: string; + }; + } | { readonly status: "failed"; readonly reason: "rollback"; diff --git a/tests/acceptance/setup/registrationRuntimeEnvironmentCli.test.ts b/tests/acceptance/setup/registrationRuntimeEnvironmentCli.test.ts index 04f32480fd..07a8b2bc71 100644 --- a/tests/acceptance/setup/registrationRuntimeEnvironmentCli.test.ts +++ b/tests/acceptance/setup/registrationRuntimeEnvironmentCli.test.ts @@ -106,10 +106,11 @@ cliTest.for([ ]) : JSON.stringify({ mcpServers: { rea: entry } }); }; + const valueMarker = "reaNulFixtureValue_6c04e2"; const original = registration( scenario.position === "value" - ? { CALLER_SETTING: "private\0value" } - : { ["CALLER\0SETTING"]: "private value" }, + ? { CALLER_SETTING: `${valueMarker}\0value` } + : { ["CALLER\0SETTING"]: `${valueMarker} value` }, ); await writeFile(config, original); const doctor = await cli.run({ @@ -153,7 +154,7 @@ cliTest.for([ appliedActions: [], remediation: expect.stringContaining("NUL"), }); - expect(setup.stdout + doctor.stdout).not.toContain("private"); + expect(setup.stdout + doctor.stdout).not.toContain(valueMarker); expect(await readFile(config, "utf8")).toBe(original); await expect(access(`${config}.rea.backup`)).rejects.toMatchObject({ code: "ENOENT", @@ -323,9 +324,11 @@ cliTest( HOME: home, USERPROFILE: home, XDG_CONFIG_HOME: join(home, ".config"), - // Select an absent launcher so host-installed Hopper cannot make this + // Select absent provider locations so installed tools cannot make this // configuration merge test plan an unrelated provider-environment edit. HOPPER_LAUNCHER_PATH: join(home, "missing-hopper"), + GHIDRA_INSTALL_DIR: join(home, "missing-ghidra"), + JAVA_HOME: join(home, "missing-java"), }; const run = (args: readonly string[]) => cli.run({ arguments: [...args, "--json"], cwd: home, environment }); diff --git a/tests/acceptance/setup/unqualifiedDoctorSkill.test.ts b/tests/acceptance/setup/unqualifiedDoctorSkill.test.ts index 014467dc3f..36a1a63f21 100644 --- a/tests/acceptance/setup/unqualifiedDoctorSkill.test.ts +++ b/tests/acceptance/setup/unqualifiedDoctorSkill.test.ts @@ -1,4 +1,4 @@ -import { mkdir, readFile, writeFile } from "node:fs/promises"; +import { mkdir, readFile, rm, writeFile } from "node:fs/promises"; import { dirname, join } from "node:path"; import { expect } from "vitest"; @@ -80,19 +80,49 @@ cliTest( identity: { skill: { state: "stale" } }, }); + const reference = join( + dirname(skill), + "references/connection-and-recovery.md", + ); + const retained = await readFile(reference); + await rm(skill); + for (const arguments_ of [ + ["doctor", "--skill"], + ["doctor", "--client", "claude_code", "--skill"], + ]) { + const partial = await run(arguments_); + expect(partial.exitCode, partial.stdout).toBe(1); + expect(partial.json).toMatchObject({ + healthy: false, + identity: { + skill: { + state: "stale", + installed_version: null, + installed_tool_count: null, + }, + }, + }); + expect(await readFile(reference)).toEqual(retained); + } + const emptyHome = await createTestTempDirectory( "rea-unqualified-doctor-empty-", ); - const empty = await cli.run({ - arguments: ["doctor", "--skill", "--json"], - cwd: emptyHome, - environment: { HOME: emptyHome, USERPROFILE: emptyHome }, - }); - expect(empty.exitCode).toBe(1); - expect(empty.json).toMatchObject({ - healthy: false, - identity: { skill: { state: "missing" } }, - }); + for (const arguments_ of [ + ["doctor", "--skill"], + ["doctor", "--client", "claude_code", "--skill"], + ]) { + const empty = await cli.run({ + arguments: [...arguments_, "--json"], + cwd: emptyHome, + environment: { HOME: emptyHome, USERPROFILE: emptyHome }, + }); + expect(empty.exitCode).toBe(1); + expect(empty.json).toMatchObject({ + healthy: false, + identity: { skill: { state: "missing" } }, + }); + } const foreignHome = await createTestTempDirectory( "rea-unqualified-doctor-foreign-skill-", diff --git a/tests/boundary/filesystem/claudeDesktopClientConfiguration.test.ts b/tests/boundary/filesystem/claudeDesktopClientConfiguration.test.ts index b411aeb4be..b0ffb1cce0 100644 --- a/tests/boundary/filesystem/claudeDesktopClientConfiguration.test.ts +++ b/tests/boundary/filesystem/claudeDesktopClientConfiguration.test.ts @@ -1,8 +1,11 @@ import { access, + chmod, mkdir, readFile, + readdir, rm, + stat, symlink, writeFile, } from "node:fs/promises"; @@ -18,6 +21,7 @@ import { inspectClientConfiguration, } from "../../../src/application/SetupClientConfiguration.js"; import { detectClients } from "../../../src/application/SetupHost.js"; +import { failedConfigurationMessage } from "../../../src/application/SetupClients.js"; import { supportedClients, type SetupClient, @@ -166,31 +170,128 @@ describe("Windows Claude Desktop configuration location", () => { }); }); - 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: {} }); + it.each([ + "Claude_abcdefghjkmnp", + "AnthropicPBC.Claude_fnn82j28hfe8t", + "ANTHROPICPBC.CLAUDE_FNN82J28HFE8T", + ])( + "reports unresolved package %s consistently across the registration lifecycle", + async (family) => { + const { home, env, appDataConfig } = await windowsProfile(); + const foreign = join(env.LOCALAPPDATA, "Packages", family); + await mkdir(foreign, { recursive: true }); + const appDataOriginal = { mcpServers: { rea: reaEntry } }; + const deploymentConfig = join(env.LOCALAPPDATA, "Claude-3p", CONFIG); + const deploymentOriginal = { + deploymentMode: "3p", + mcpServers: { other: { command: "other" } }, + }; + await writeJson(appDataConfig, appDataOriginal); + await writeJson(deploymentConfig, deploymentOriginal); - const client = claudeDesktop(home, env); + 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" }); + expect(client.configPath).toBe(foreign); + expect(client.configPathError).toContain( + "unrecognized Claude Desktop package identity or deployment", + ); + 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" }); + await expect( + configureClientConfiguration(client, {}, command), + ).resolves.toEqual({ status: "failed", reason: "path" }); + const uninstall = await runUninstall( + false, + systemUninstallHost(home, undefined, env, "win32"), + ); + expect(uninstall.status).toBe("failed"); + expect(uninstall.items).toContainEqual( + expect.objectContaining({ name: "claude_desktop", status: "failed" }), + ); + expect(await readJson(appDataConfig)).toEqual(appDataOriginal); + expect(await readJson(deploymentConfig)).toEqual(deploymentOriginal); + expect(await readdir(dirname(appDataConfig))).toEqual([CONFIG]); + expect(await readdir(dirname(deploymentConfig))).toEqual([CONFIG]); + }, + ); +}); + +it("preserves explicit AppData root whitespace throughout setup, doctor and uninstall", async () => { + const { home, env, appDataConfig: otherConfig } = await windowsProfile(); + const selected = { + APPDATA: `${env.APPDATA} `, + LOCALAPPDATA: `${env.LOCALAPPDATA} `, + }; + const appDataConfig = join(selected.APPDATA, "Claude", CONFIG); + const privateConfig = join( + selected.LOCALAPPDATA, + "Packages", + CLAUDE_DESKTOP_PACKAGE_FAMILY, + "LocalCache", + "Roaming", + "Claude", + CONFIG, + ); + const original = { mcpServers: { other: { command: "other-server" } } }; + const otherOriginal = { + mcpServers: { rea: reaEntry }, + preferences: { preserve: true }, + }; + await writeJson(otherConfig, otherOriginal); + await writeJson(appDataConfig, original); + let client = claudeDesktop(home, selected); + expect(client.configPath).toBe(appDataConfig); + await expect( + configureClientConfiguration(client, {}, command), + ).resolves.toEqual({ + status: "configured", + backupPath: `${appDataConfig}.rea.backup`, }); + await writeJson(privateConfig, original); + client = claudeDesktop(home, selected); + expect(client.configPath).toBe(privateConfig); + expect(client.msixPackage?.virtualizedRoots).toEqual([ + selected.APPDATA, + selected.LOCALAPPDATA, + ]); + await expect( + configureClientConfiguration(client, {}, command), + ).resolves.toEqual({ + status: "configured", + backupPath: `${privateConfig}.rea.backup`, + }); + const status = ( + await readClientRegistrationStatuses(home, "unused", { + platform: "win32", + environment: selected, + }) + ).find(({ client: name }) => name === "claude_desktop"); + expect(status).toMatchObject({ + state: "aligned", + config_path: privateConfig, + }); + const uninstall = await runUninstall( + false, + systemUninstallHost(home, undefined, selected, "win32"), + ); + expect(uninstall.status).toBe("complete"); + expect(await readJson(appDataConfig)).toEqual(original); + expect(await readJson(privateConfig)).toEqual(original); + expect(await readJson(otherConfig)).toEqual(otherOriginal); + expect(await readJson(`${appDataConfig}.rea.backup`)).toEqual(original); + expect(await readJson(`${privateConfig}.rea.backup`)).toEqual(original); }); describe("MSIX Claude Desktop registration lifecycle", () => { @@ -317,6 +418,46 @@ describe("MSIX Claude Desktop registration lifecycle", () => { code: "ENOENT", }); }); + + it.skipIf(process.platform === "win32" || process.getuid?.() === 0)( + "keeps an inaccessible private-copy recheck unknown and blocks backup and write", + async () => { + const { home, env, appDataConfig, privateConfig } = + await windowsProfile(); + const privateDirectory = dirname(privateConfig); + await writeJson(join(privateDirectory, "Preferences"), {}); + await writeJson(appDataConfig, { preferences: { theme: "light" } }); + const original = await readFile(appDataConfig); + const client = claudeDesktop(home, env); + expect(client.configPath).toBe(appDataConfig); + expect(client.msixPackage?.processView).toBe("direct"); + expect( + await inspectClientConfiguration(client, {}, command), + ).toMatchObject({ status: "update" }); + const mode = (await stat(privateDirectory)).mode & 0o777; + await chmod(privateDirectory, 0); + try { + const result = await configureClientConfiguration(client, {}, command); + expect(result).toMatchObject({ + status: "failed", + reason: "target_unavailable", + failure: { + kind: "inspect", + path: privateConfig, + detail: expect.stringContaining("EACCES"), + }, + }); + if (result.status !== "failed") + throw new Error("Expected a blocked update"); + expect(failedConfigurationMessage(result)).toContain(privateConfig); + expect(failedConfigurationMessage(result)).toContain("Repair access"); + expect(await readFile(appDataConfig)).toEqual(original); + expect(await readdir(dirname(appDataConfig))).toEqual([CONFIG]); + } finally { + await chmod(privateDirectory, mode); + } + }, + ); }); describe("MSIX Claude Desktop uninstall", () => {