Files
rea/tests/boundary/filesystem/setupClientConfigCleanupOwnership.test.ts
morluto 823521085a fix(setup): transfer unscoped registration cleanup ownership
Preserve original RegularFileCleanupFailure capabilities when registration
inspection or maintenance planning has no caller-supplied resource scope.
Keep scoped diagnostics with the scope that already retained the descriptor.

Cover configuration and policy sources, completed and malformed-UTF8 reads,
all four exported reader/planner boundaries, exact descriptor retry after
pathname replacement, no subsequent execution, and scoped cleanup retry.

Validation: 79 tests across four focused files; the historical baseline fails
all 16 new ownership-transfer controls with eight existing/scoped controls
passing. check:fast, format:check, test discovery and six isolated completed
public doctor comparisons passed. Scoped source review found no blocker.

Real temporary descriptors and injected close rejection on macOS arm64 /
Node24.18.0; no native Windows/Linux close faults or provider verification.

Fixes #4910
2026-10-18 07:22:14 +08:00

496 lines
17 KiB
TypeScript

import {
mkdir,
open as fsOpen,
readFile,
rename,
writeFile,
type FileHandle,
} from "node:fs/promises";
import { dirname, join } from "node:path";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import {
readClientRegistrationInspections,
readClientRegistrationStatuses,
} from "../../../src/application/ClientRegistrationStatus.js";
import {
existingMaintenanceScope,
planIntegrationMaintenance,
} from "../../../src/application/UpdateMaintenance.js";
import { ArtifactResourceScope } from "../../../src/artifacts/ArtifactResourceScope.js";
import { ok } from "../../../src/domain/result.js";
import type { DoctorHost } from "../../../src/application/Doctor.js";
import { ConfigurationError } from "../../../src/domain/configurationErrors.js";
import {
filterClientsNeedingConfigure,
systemSetupHost,
} from "../../../src/application/SetupHost.js";
import { RegularFileCleanupFailure } from "../../../src/application/RegularFileRead.js";
import { configureDetectedClients } from "../../../src/application/SetupClients.js";
import { configureClientConfiguration } from "../../../src/application/SetupClientConfiguration.js";
import { systemUninstallHost } from "../../../src/application/Uninstall.js";
import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
vi.mock("node:fs/promises", async (importOriginal) => {
const actual = await importOriginal<typeof import("node:fs/promises")>();
return { ...actual, open: vi.fn(actual.open) };
});
const openMock = vi.mocked(fsOpen);
let actualFs: typeof import("node:fs/promises");
beforeEach(async () => {
actualFs =
await vi.importActual<typeof import("node:fs/promises")>(
"node:fs/promises",
);
openMock.mockImplementation((...args) => actualFs.open(...args));
});
afterEach(() => openMock.mockReset());
const holdReadHandle = (path: string, reason: string) => {
const closeAttempts: FileHandle[] = [];
let released = false;
openMock.mockImplementation(async (...args) => {
const handle = await actualFs.open(...args);
if (String(args[0]) !== path) return handle;
const nativeClose = handle.close.bind(handle);
vi.spyOn(handle, "close").mockImplementation(async () => {
closeAttempts.push(handle);
if (!released) throw new Error(reason);
await nativeClose();
});
return handle;
});
return {
closeAttempts,
release: () => {
released = true;
},
};
};
/** Linux setup host with its VS Code client configuration directory prepared. */
const preparedVsCodeSetupHost = async (prefix: string) => {
const home = await createTestTempDirectory(prefix);
const host = systemSetupHost(
{ platform: "linux", homeDirectory: home } as DoctorHost,
{ HOME: home, XDG_CONFIG_HOME: join(home, "xdg") },
);
const client = (await host.supportedClients()).find(
({ name }) => name === "vscode",
);
if (client === undefined) throw new Error("VS Code setup client missing");
await mkdir(dirname(client.configPath), { recursive: true });
return { home, host, client };
};
/** Release the held read handle and verify host cleanup retries that same owner. */
const expectCloseReclaimsHandle = async (
host: { close?: () => Promise<string | undefined> },
closeAttempts: readonly object[],
release: () => void,
) => {
const attemptsBeforeClose = closeAttempts.length;
release();
expect(await host.close?.()).toBeUndefined();
expect(closeAttempts).toHaveLength(attemptsBeforeClose + 1);
expect(closeAttempts.at(-1)).toBe(closeAttempts[0]);
};
/** Give up on the assertion path: release the handle and let cleanup close it. */
const cleanupHeldHandle = async (
host: { close?: () => Promise<string | undefined> },
release: () => void,
) => {
release();
await host.close?.();
};
it("retains invalid UTF-8 diagnostics and the uninstall read owner when close fails", async () => {
const home = await createTestTempDirectory("rea-uninstall-utf8-close-");
const path = join(home, "client.json");
const original = Buffer.from([0xff]);
await writeFile(path, original);
const { closeAttempts, release } = holdReadHandle(
path,
"held uninstall handle",
);
const host = systemUninstallHost(home, undefined, { HOME: home }, "linux");
try {
expect(
await host.removeClient({
name: "cursor",
format: "json",
configPath: path,
}),
).toMatchObject({
status: "failed",
detail: expect.stringContaining("not valid UTF-8"),
});
expect(await readFile(path)).toEqual(original);
expect(closeAttempts).toHaveLength(3);
expect(closeAttempts.every((handle) => handle === closeAttempts[0])).toBe(
true,
);
await expectCloseReclaimsHandle(host, closeAttempts, release);
} finally {
await cleanupHeldHandle(host, release);
}
});
it("stops uninstall and retains the backup read owner when its close fails", async () => {
const home = await createTestTempDirectory("rea-uninstall-backup-close-");
const path = join(home, "client.json");
const client = { name: "cursor", format: "json", configPath: path } as const;
await writeFile(path, '{"keep":"before setup"}\n');
expect(
await configureClientConfiguration(client, {}, ["rea", "mcp"]),
).toMatchObject({ status: "configured" });
const configured = await readFile(path, "utf8");
const { closeAttempts, release } = holdReadHandle(
`${path}.rea.backup`,
"held backup handle",
);
const host = systemUninstallHost(home, undefined, { HOME: home }, "linux");
try {
expect(await host.removeClient(client)).toEqual({
name: "cursor",
status: "failed",
detail:
"Configuration could not be backed up, so no change was made. Check file permissions, then rerun uninstall.",
});
expect(await readFile(path, "utf8")).toBe(configured);
expect(closeAttempts).toHaveLength(2);
await expectCloseReclaimsHandle(host, closeAttempts, release);
} finally {
await cleanupHeldHandle(host, release);
}
});
describe("ZCode fallback cleanup ownership", () => {
it.each(["preflight", "configure"])(
"retains a failed ZCode fallback close during %s until host cleanup",
async (operation) => {
const home = await createTestTempDirectory("rea-zcode-fallback-close-");
const host = systemSetupHost(
{ platform: "linux", homeDirectory: home } as DoctorHost,
{ HOME: home },
);
const client = (await host.supportedClients()).find(
({ name }) => name === "zcode",
);
if (client?.zcodeFallbackPath === undefined)
throw new Error("ZCode fallback path missing");
const fallback = client.zcodeFallbackPath;
await mkdir(dirname(fallback), { recursive: true });
const original = '{"mcpServers":{"other":{"command":"other"}}}\n';
await writeFile(fallback, original);
const { closeAttempts, release } = holdReadHandle(
fallback,
"held fallback handle",
);
try {
const action =
operation === "preflight"
? host.inspectClientConfiguration?.(client, {}, ["rea", "mcp"])
: host.configureClient(client, {}, ["rea", "mcp"]);
if (action === undefined) throw new Error("setup preflight missing");
await expect(action).rejects.toMatchObject({
_tag: "ConfigurationError",
cause: expect.any(RegularFileCleanupFailure),
cleanup: { resources: [fallback] },
});
expect(closeAttempts).toHaveLength(2);
expect(closeAttempts[0]).toBe(closeAttempts[1]);
expect(await readFile(fallback, "utf8")).toBe(original);
await expect(readFile(client.configPath)).rejects.toMatchObject({
code: "ENOENT",
});
await expectCloseReclaimsHandle(host, closeAttempts, release);
} finally {
await cleanupHeldHandle(host, release);
}
},
);
});
describe("setup client configuration cleanup ownership", () => {
it.each([
["valid UTF-8", Buffer.from('{"servers": {"other": {}}}\n')],
["malformed UTF-8", Buffer.from([0xff])],
] as const)(
"stops configuration and retains the %s read owner until host close",
async (encoding, original) => {
const { host, client } = await preparedVsCodeSetupHost(
"rea-setup-config-close-",
);
await writeFile(client.configPath, original);
const { closeAttempts, release } = holdReadHandle(
client.configPath,
"test holds the config handle open",
);
const configure = vi.spyOn(host, "configureClient");
try {
const failure = await filterClientsNeedingConfigure(
host,
[client],
{},
["rea", "mcp"],
)
.then((detectedClients) =>
configureDetectedClients({
host,
detectedClients,
providerEnvironment: {},
command: ["rea", "mcp"],
clients: {},
appliedActions: [],
}),
)
.then(
() => undefined,
(cause: unknown) => cause,
);
expect(failure).toBeInstanceOf(ConfigurationError);
if (encoding === "malformed UTF-8")
expect(failure).toMatchObject({
cause: {
outcome: { kind: "failed", cause: expect.any(TypeError) },
},
});
expect(failure).toMatchObject({
_tag: "ConfigurationError",
cause: expect.any(RegularFileCleanupFailure),
cleanup: {
resources: [client.configPath],
reason: expect.stringContaining(
"test holds the config handle open",
),
},
});
expect(configure).not.toHaveBeenCalled();
expect(closeAttempts).toHaveLength(2);
expect(closeAttempts[0]).toBe(closeAttempts[1]);
expect(await readFile(client.configPath)).toEqual(original);
await expectCloseReclaimsHandle(host, closeAttempts, release);
} finally {
await cleanupHeldHandle(host, release);
}
},
);
it("waits for an admitted configuration read and rejects later admissions", async () => {
const { host, client } = await preparedVsCodeSetupHost(
"rea-setup-config-inflight-",
);
await writeFile(client.configPath, '{"servers": {}}\n');
let beginRead!: () => void;
const readStarted = new Promise<void>((resolve) => {
beginRead = resolve;
});
let unblockRead!: () => void;
const readGate = new Promise<void>((resolve) => {
unblockRead = resolve;
});
let heldRead = false;
openMock.mockImplementation(async (...args) => {
const handle = await actualFs.open(...args);
if (String(args[0]) !== client.configPath || heldRead) return handle;
heldRead = true;
const nativeRead = handle.read.bind(handle);
vi.spyOn(handle, "read").mockImplementation(async (...readArgs) => {
beginRead();
await readGate;
return nativeRead(...readArgs);
});
return handle;
});
let hostCloseFinished = false;
let alignment: Promise<boolean> | undefined;
try {
alignment = host.clientNeedsConfigure(client, {}, ["rea", "mcp"]);
await readStarted;
const close = host.close;
if (close === undefined) throw new Error("Setup host close is missing");
const closeResult = close().then((result) => {
hostCloseFinished = true;
return result;
});
const lateAdmission = await host
.clientNeedsConfigure(client, {}, ["rea", "mcp"])
.then(
() => undefined,
(cause: unknown) => cause,
);
expect(lateAdmission).toBeInstanceOf(ConfigurationError);
expect(lateAdmission).toMatchObject({
message: expect.stringContaining("Artifact resource scope is closed"),
});
expect(hostCloseFinished).toBe(false);
unblockRead();
await expect(alignment).resolves.toBe(true);
expect(await closeResult).toBeUndefined();
expect(hostCloseFinished).toBe(true);
} finally {
unblockRead();
await alignment?.catch(() => undefined);
await host.close?.();
}
});
});
const unscopedRegistrationReaders: readonly {
readonly name: string;
readonly read: (
home: string,
environment: NodeJS.ProcessEnv,
execute: Parameters<typeof planIntegrationMaintenance>[3],
) => Promise<unknown>;
}[] = [
{
name: "inspections",
read: (home, environment) =>
readClientRegistrationInspections(home, "/current/rea", { environment }),
},
{
name: "statuses",
read: (home, environment) =>
readClientRegistrationStatuses(home, "/current/rea", { environment }),
},
{
name: "maintenance scope",
read: (home, environment) =>
existingMaintenanceScope(home, "/current/rea", environment),
},
{
name: "maintenance plan",
read: (home, environment, execute) =>
planIntegrationMaintenance(home, "/current/rea", environment, execute),
},
];
it.each(
unscopedRegistrationReaders.flatMap((reader) =>
["configuration", "policy"].flatMap((source) =>
["completed", "failed"].map((readOutcome) => ({
...reader,
source,
readOutcome,
})),
),
),
)(
"$name transfers the exact $source owner after a $readOutcome read without a supplied scope",
async ({ read, source, readOutcome }) => {
const home = await createTestTempDirectory(
"rea-registration-unscoped-close-",
);
const config = join(
home,
source === "policy" ? ".gemini/settings.json" : ".cursor/mcp.json",
);
await mkdir(dirname(config), { recursive: true });
const registration =
'{"mcpServers":{"rea":{"command":"rea","args":["mcp"]}}}\n';
await writeFile(config, registration);
const heldPath = source === "policy" ? join(home, "system.json") : config;
const heldText = source === "policy" ? "{}\n" : registration;
await writeFile(
heldPath,
readOutcome === "failed" ? Buffer.from([0xff]) : heldText,
);
const environment = {
HOME: home,
GEMINI_CLI_SYSTEM_SETTINGS_PATH: join(home, "system.json"),
GEMINI_CLI_SYSTEM_DEFAULTS_PATH: join(home, "absent-defaults.json"),
};
const held = holdReadHandle(
heldPath,
"held standalone registration descriptor",
);
const execute = vi.fn(async () =>
ok('{"status":"planned","plannedActions":[]}'),
);
try {
const outcome = await read(home, environment, execute).then(
(value) => ({ kind: "returned" as const, value }),
(cause: unknown) => ({ kind: "threw" as const, cause }),
);
expect(outcome).toMatchObject({
kind: "threw",
cause: expect.any(RegularFileCleanupFailure),
});
if (
outcome.kind !== "threw" ||
!(outcome.cause instanceof RegularFileCleanupFailure)
)
throw new Error("Read lost its failed cleanup owner");
const failure = outcome.cause;
expect(failure.path).toBe(heldPath);
if (readOutcome === "completed")
expect(failure.outcome).toEqual({ kind: "completed", value: heldText });
else
expect(failure.outcome).toMatchObject({
kind: "failed",
cause: { _tag: "InvalidUtf8FileError", path: heldPath },
});
expect(failure.owner.handle).toBe(held.closeAttempts[0]);
await expect(failure.owner.close()).rejects.toThrow("held standalone");
await rename(heldPath, `${heldPath}.retired`);
await writeFile(heldPath, "replacement");
held.release();
await failure.owner.close();
expect(failure.owner.handle.fd).toBe(-1);
expect(new Set(held.closeAttempts).size).toBe(1);
expect(await readFile(heldPath, "utf8")).toBe("replacement");
expect(execute).not.toHaveBeenCalled();
} finally {
held.release();
for (const handle of new Set(held.closeAttempts)) await handle.close();
}
},
);
it("keeps a supplied registration scope responsible for diagnostics and cleanup retry", async () => {
const home = await createTestTempDirectory("rea-registration-scoped-close-");
const config = join(home, ".cursor/mcp.json");
await mkdir(dirname(config), { recursive: true });
await writeFile(
config,
'{"mcpServers":{"rea":{"command":"rea","args":["mcp"]}}}',
);
const held = holdReadHandle(config, "held scoped registration descriptor");
const resources = new ArtifactResourceScope();
try {
expect(
(
await readClientRegistrationStatuses(home, "/current/rea", {
environment: { HOME: home },
resources,
})
).find((status) => status.client === "cursor"),
).toMatchObject({ state: "invalid", command: [] });
expect(resources.hasPending).toBe(true);
await expect(resources.close()).rejects.toThrow();
held.release();
await resources.close();
expect(resources.hasPending).toBe(false);
expect(new Set(held.closeAttempts).size).toBe(1);
expect(held.closeAttempts[0]!.fd).toBe(-1);
} finally {
held.release();
await resources.close();
}
});