mirror of
https://github.com/morluto/rea
synced 2026-10-25 22:08:11 -04:00
fix(doctor): preserve unresolved client configuration evidence
Use entry metadata to discover registration evidence independently of target readability. Preserve failed metadata inspection as a named invalid diagnostic instead of treating all lookup failures as absence. Remove the duplicate boolean existence wrapper; configuration parsing remains the read owner. Cover native permission-denied targets and metadata, dangling configuration entries, maintenance exclusion, and public doctor output without mutation. Validation: 73 focused tests across four files, check:fast, format:check, and six isolated completed public doctor cost comparisons passed on macOS arm64 / Node24.18.0. Native Windows/Linux permissions were not verified; POSIX permission controls skip unsupported hosts and root. Fixes #4905
This commit is contained in:
@@ -6,7 +6,8 @@ import {
|
||||
effectiveClientServer,
|
||||
type ClientConfigurationDocument,
|
||||
} from "./ClientConfigurationDocument.js";
|
||||
import { access } from "node:fs/promises";
|
||||
import { lstat } from "node:fs/promises";
|
||||
import { err, ok, type Result } from "../domain/result.js";
|
||||
import { isAbsolute } from "node:path";
|
||||
|
||||
import { z } from "zod";
|
||||
@@ -146,7 +147,18 @@ export const readClientRegistrationInspections = async (
|
||||
});
|
||||
continue;
|
||||
}
|
||||
if (!(await clientEvidencePresent(client))) continue;
|
||||
const presence = await clientEvidencePresent(client);
|
||||
if (!presence.ok) {
|
||||
record({
|
||||
client: client.name,
|
||||
config_path: presence.error.path,
|
||||
command: [],
|
||||
state: "invalid",
|
||||
remediation: presence.error.detail,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
if (!presence.value) continue;
|
||||
const manualRemediation = manualRegistrationRemediation(client.name);
|
||||
if (client.format === "unsupported") {
|
||||
if (manualRemediation !== undefined)
|
||||
@@ -559,22 +571,28 @@ const unavailableStatus = (
|
||||
remediation: remediation(client),
|
||||
});
|
||||
|
||||
const clientEvidencePresent = async (client: SetupClient): Promise<boolean> => {
|
||||
for (const path of clientEvidencePaths(client))
|
||||
if (await exists(path)) return true;
|
||||
return false;
|
||||
};
|
||||
|
||||
const exists = async (path: string | undefined): Promise<boolean> => {
|
||||
if (path === undefined) return false;
|
||||
try {
|
||||
await access(path);
|
||||
return true;
|
||||
} catch (cause: unknown) {
|
||||
// best-effort cleanup: optional registration probing; absence means unregistered.
|
||||
void cause;
|
||||
return false;
|
||||
/** Entry presence is independent of whether its target can currently be read. */
|
||||
const clientEvidencePresent = async (
|
||||
client: SetupClient,
|
||||
): Promise<
|
||||
Result<boolean, { readonly path: string; readonly detail: string }>
|
||||
> => {
|
||||
let unavailable:
|
||||
| { readonly path: string; readonly detail: string }
|
||||
| undefined;
|
||||
for (const path of clientEvidencePaths(client)) {
|
||||
try {
|
||||
await lstat(path);
|
||||
return ok(true);
|
||||
} catch (cause: unknown) {
|
||||
if (!isMissing(cause))
|
||||
unavailable ??= {
|
||||
path,
|
||||
detail: `Client configuration evidence at ${path} could not be inspected: ${cause instanceof Error ? cause.message : String(cause)}. Check file permissions and rerun rea doctor.`,
|
||||
};
|
||||
}
|
||||
}
|
||||
return unavailable === undefined ? ok(false) : err(unavailable);
|
||||
};
|
||||
|
||||
const isMissing = (cause: unknown): boolean =>
|
||||
|
||||
@@ -1,4 +1,11 @@
|
||||
import { mkdir, readFile, readdir, writeFile } from "node:fs/promises";
|
||||
import {
|
||||
chmod,
|
||||
mkdir,
|
||||
readFile,
|
||||
readdir,
|
||||
symlink,
|
||||
writeFile,
|
||||
} from "node:fs/promises";
|
||||
import { dirname, join } from "node:path";
|
||||
import { expect } from "vitest";
|
||||
|
||||
@@ -165,3 +172,56 @@ for (const scenario of cases)
|
||||
]);
|
||||
},
|
||||
);
|
||||
|
||||
cliTest.skipIf(process.platform === "win32" || process.getuid?.() === 0)(
|
||||
"doctor reports an unreadable configuration symlink without changing it",
|
||||
async ({ cli }) => {
|
||||
const home = await createTestTempDirectory(
|
||||
"rea-doctor-denied-registration-",
|
||||
);
|
||||
const locked = join(home, "owned-denied");
|
||||
const target = join(locked, "client.json");
|
||||
await mkdir(locked);
|
||||
const original =
|
||||
'{"mcpServers":{"rea":{"command":"rea","args":["mcp"]}}}\n';
|
||||
await writeFile(target, original);
|
||||
const config = join(home, ".claude.json");
|
||||
await symlink(target, config);
|
||||
await chmod(locked, 0);
|
||||
try {
|
||||
const result = await cli.run({
|
||||
arguments: [
|
||||
"doctor",
|
||||
"--client",
|
||||
"claude_code",
|
||||
"--skill=false",
|
||||
"--json",
|
||||
],
|
||||
cwd: home,
|
||||
environment: {
|
||||
HOME: home,
|
||||
USERPROFILE: home,
|
||||
XDG_CONFIG_HOME: join(home, ".config"),
|
||||
},
|
||||
});
|
||||
expect(result.exitCode).toBe(1);
|
||||
expect(result.json).toMatchObject({
|
||||
healthy: false,
|
||||
identity: {
|
||||
registrations: [
|
||||
expect.objectContaining({
|
||||
client: "claude_code",
|
||||
config_path: config,
|
||||
state: "invalid",
|
||||
command: [],
|
||||
}),
|
||||
],
|
||||
},
|
||||
});
|
||||
} finally {
|
||||
await chmod(locked, 0o700);
|
||||
}
|
||||
expect(await readFile(target, "utf8")).toBe(original);
|
||||
expect(await readdir(home)).toEqual([".claude.json", "owned-denied"]);
|
||||
},
|
||||
);
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { mkdir, writeFile } from "node:fs/promises";
|
||||
import { chmod, mkdir, symlink, writeFile } from "node:fs/promises";
|
||||
import { join, relative, resolve } from "node:path";
|
||||
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
@@ -7,6 +7,7 @@ import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
|
||||
import { clearClientLocationEnvironment } from "../../fixtures/clientEnvironment.js";
|
||||
|
||||
import { readClientRegistrationStatuses } from "../../../src/application/ClientRegistrationStatus.js";
|
||||
import { existingMaintenanceScope } from "../../../src/application/UpdateMaintenance.js";
|
||||
import { PRODUCT_IDENTITY } from "../../../src/identity.js";
|
||||
|
||||
beforeEach(clearClientLocationEnvironment);
|
||||
@@ -439,3 +440,86 @@ describe("Node-wrapped registration policy", () => {
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it.skipIf(process.platform === "win32" || process.getuid?.() === 0)(
|
||||
"reports an inaccessible configuration target instead of omitting its registration",
|
||||
async () => {
|
||||
const home = await createTestTempDirectory(
|
||||
"rea-registration-denied-target-",
|
||||
);
|
||||
const locked = join(home, "owned-denied");
|
||||
await mkdir(locked);
|
||||
const target = join(locked, "client.json");
|
||||
await writeFile(
|
||||
target,
|
||||
JSON.stringify({
|
||||
mcpServers: { rea: { command: "rea", args: ["mcp"] } },
|
||||
}),
|
||||
);
|
||||
const path = join(home, ".claude.json");
|
||||
await symlink(target, path);
|
||||
await chmod(locked, 0);
|
||||
try {
|
||||
const statuses = await readClientRegistrationStatuses(
|
||||
home,
|
||||
"/current/rea",
|
||||
);
|
||||
expect(
|
||||
statuses.find((status) => status.client === "claude_code"),
|
||||
).toMatchObject({
|
||||
config_path: path,
|
||||
state: "invalid",
|
||||
command: [],
|
||||
remediation: expect.any(String),
|
||||
});
|
||||
expect(
|
||||
(await existingMaintenanceScope(home, "/current/rea", process.env))
|
||||
.clients,
|
||||
).not.toContain("claude_code");
|
||||
} finally {
|
||||
await chmod(locked, 0o700);
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it.skipIf(process.platform === "win32" || process.getuid?.() === 0)(
|
||||
"reports an unresolved configuration location when metadata lookup is denied",
|
||||
async () => {
|
||||
const home = await createTestTempDirectory(
|
||||
"rea-registration-denied-metadata-",
|
||||
);
|
||||
const locked = join(home, "owned-denied");
|
||||
const directory = join(locked, "claude");
|
||||
await mkdir(directory, { recursive: true });
|
||||
vi.stubEnv("CLAUDE_CONFIG_DIR", directory);
|
||||
await chmod(locked, 0);
|
||||
try {
|
||||
const statuses = await readClientRegistrationStatuses(
|
||||
home,
|
||||
"/current/rea",
|
||||
);
|
||||
expect(
|
||||
statuses.find((status) => status.client === "claude_code"),
|
||||
).toMatchObject({
|
||||
state: "invalid",
|
||||
command: [],
|
||||
remediation: expect.stringContaining("EACCES"),
|
||||
});
|
||||
} finally {
|
||||
await chmod(locked, 0o700);
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it("reports a dangling configuration entry as missing without losing the detected client", async () => {
|
||||
const home = await createTestTempDirectory(
|
||||
"rea-registration-dangling-config-",
|
||||
);
|
||||
const path = join(home, ".claude.json");
|
||||
await symlink(join(home, "absent.json"), path);
|
||||
expect(
|
||||
(await readClientRegistrationStatuses(home)).find(
|
||||
(status) => status.client === "claude_code",
|
||||
),
|
||||
).toMatchObject({ config_path: path, state: "missing", command: [] });
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user