mirror of
https://github.com/morluto/rea.git
synced 2026-10-26 17:35:11 +00:00
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
This commit is contained in:
1 parent
abb54ce374
commit
823521085a
3 files changed
+186
-3
No files matched your search
@@ -13,6 +13,7 @@ import { isAbsolute } from "node:path";
|
||||
import { z } from "zod";
|
||||
import { valid } from "semver";
|
||||
|
||||
import { RegularFileCleanupFailure } from "./RegularFileRead.js";
|
||||
import { PRODUCT_IDENTITY } from "../identity.js";
|
||||
import { CURRENT_ENTRYPOINT_PATH } from "../process/CurrentEntrypointPath.js";
|
||||
import {
|
||||
@@ -119,7 +120,9 @@ export const readClientRegistrationStatuses = async (
|
||||
await readClientRegistrationInspections(home, currentCommandPath, options)
|
||||
).map(({ status }) => status);
|
||||
|
||||
/** Parse effective registrations and their client-specific activation once. */
|
||||
/** Parse effective registrations and their client-specific activation once.
|
||||
* A supplied scope retains failed cleanup; otherwise transfer its exact failure.
|
||||
*/
|
||||
export const readClientRegistrationInspections = async (
|
||||
home: string,
|
||||
currentCommandPath: string | undefined = CURRENT_ENTRYPOINT_PATH,
|
||||
@@ -178,6 +181,11 @@ export const readClientRegistrationInspections = async (
|
||||
options.resources,
|
||||
);
|
||||
if (!configuration.ok) {
|
||||
if (
|
||||
options.resources === undefined &&
|
||||
configuration.error.cleanupFailure !== undefined
|
||||
)
|
||||
throw configuration.error.cleanupFailure;
|
||||
record({
|
||||
client: client.name,
|
||||
config_path: configuration.error.path,
|
||||
@@ -200,6 +208,11 @@ export const readClientRegistrationInspections = async (
|
||||
options.resources,
|
||||
);
|
||||
if (!policy.ok) {
|
||||
if (
|
||||
options.resources === undefined &&
|
||||
policy.error.cleanupFailure !== undefined
|
||||
)
|
||||
throw policy.error.cleanupFailure;
|
||||
record({
|
||||
client: client.name,
|
||||
config_path: configPath,
|
||||
@@ -313,6 +326,11 @@ export const readClientRegistrationInspections = async (
|
||||
activation,
|
||||
);
|
||||
} catch (cause: unknown) {
|
||||
if (
|
||||
options.resources === undefined &&
|
||||
cause instanceof RegularFileCleanupFailure
|
||||
)
|
||||
throw cause;
|
||||
record(
|
||||
unavailableStatus(
|
||||
client,
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { z } from "zod";
|
||||
import { RegularFileCleanupFailure } from "./RegularFileRead.js";
|
||||
|
||||
import { classifyClientRegistrationCommand } from "./ClientRegistrationIdentity.js";
|
||||
import { readClientRegistrationInspections } from "./ClientRegistrationStatus.js";
|
||||
@@ -150,6 +151,9 @@ export const planIntegrationMaintenance = async (
|
||||
),
|
||||
};
|
||||
} catch (cause: unknown) {
|
||||
// Without a supplied scope, the typed failure is the caller's cleanup capability.
|
||||
if (resources === undefined && cause instanceof RegularFileCleanupFailure)
|
||||
throw cause;
|
||||
return {
|
||||
status: "unavailable",
|
||||
remediation: cause instanceof Error ? cause.message : String(cause),
|
||||
|
||||
@@ -1,8 +1,25 @@
|
||||
import { mkdir, open as fsOpen, readFile, writeFile } from "node:fs/promises";
|
||||
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 {
|
||||
@@ -34,7 +51,7 @@ beforeEach(async () => {
|
||||
afterEach(() => openMock.mockReset());
|
||||
|
||||
const holdReadHandle = (path: string, reason: string) => {
|
||||
const closeAttempts: object[] = [];
|
||||
const closeAttempts: FileHandle[] = [];
|
||||
let released = false;
|
||||
openMock.mockImplementation(async (...args) => {
|
||||
const handle = await actualFs.open(...args);
|
||||
@@ -332,3 +349,147 @@ describe("setup client configuration cleanup ownership", () => {
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
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();
|
||||
}
|
||||
});
|
||||
Reference in new issue
Block a user