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
496 lines
17 KiB
TypeScript
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();
|
|
}
|
|
});
|