shifathosn
/
rea
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(setup): retain regular-file cleanup ownership through workflows 

Thread the existing resource scope through skill and client configuration reads. Keep failed-close handles owned, wait for admitted reads, and surface unresolved cleanup alongside doctor, uninstall and update results.

Validated focused setup/doctor/update and filesystem boundary checks, temporary-file hygiene, packaged setup/update/uninstall workflows, and static checks. No external provider installation was performed.
This commit is contained in:
morluto 2026-10-12 05:56:26 +08:00
parent 9c8511409c
commit dcfc5d403a
15 changed files with 707 additions and 143 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
src/application/ClientConfigurationFiles.ts
View File

@ -10,6 +10,7 @@ import {
} from "./ClientConfigurationDocument.js";
import type { SetupClient } from "./SupportedClients.js";
import { clientRegistrationEnvironment } from "./ClientRegistrationEnvironment.js";
import type { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js";
/** A named source failure that setup can explain without losing the bad path. */
export interface ClientConfigurationFileError {
@ -22,9 +23,15 @@ export interface ClientConfigurationFileError {
/** Preserve named read failures and distinguish a missing source from an empty file. */
export const readConfigurationText = async (
path: string,
resources?: ArtifactResourceScope,
): Promise<Result<string | undefined, ClientConfigurationFileError>> => {
try {
return ok(await readRegularFileText(path));
return ok(
await readRegularFileText(
path,
resources === undefined ? {} : { resources },
),
);
} catch (cause: unknown) {
if (cause instanceof Error && "code" in cause && cause.code === "ENOENT")
return ok(undefined);
@ -43,10 +50,11 @@ export const readConfigurationText = async (
export const readConfigurationFile = async (
path: string,
format: SetupClient["format"],
resources?: ArtifactResourceScope,
): Promise<
Result<ClientConfigurationDocument | undefined, ClientConfigurationFileError>
> => {
const source = await readConfigurationText(path);
const source = await readConfigurationText(path, resources);
if (!source.ok) return source;
const content = source.value;
if (content === undefined) return ok(undefined);
@ -64,12 +72,17 @@ export const readConfigurationFile = async (
/** Read every applicable user file; malformed lower-priority files also stop setup. */
export const readClientConfigurationFiles = async (
client: SetupClient,
resources?: ArtifactResourceScope,
): Promise<
Result<readonly ClientConfigurationDocument[], ClientConfigurationFileError>
> => {
const documents: ClientConfigurationDocument[] = [];
for (const path of client.configPaths ?? [client.configPath]) {
const document = await readConfigurationFile(path, client.format);
const document = await readConfigurationFile(
path,
client.format,
resources,
);
if (!document.ok) return document;
if (document.value !== undefined) {
const environment = clientRegistrationEnvironment(document.value);

14
src/application/ClientRegistrationStatus.ts
View File

@ -26,6 +26,7 @@ import {
} from "./SupportedClients.js";
import type { SetupClient } from "./SupportedClients.js";
import { compareUnicodeCodePoints } from "../domain/unicodeCodePointOrder.js";
import type { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js";
interface ClientRegistrationStatusBase {
readonly client: string;
@ -78,6 +79,7 @@ export const readClientRegistrationStatuses = async (
options: {
readonly platform?: NodeJS.Platform;
readonly environment?: NodeJS.ProcessEnv;
readonly resources?: ArtifactResourceScope;
} = {},
): Promise<readonly ClientRegistrationStatus[]> => {
const statuses: ClientRegistrationStatus[] = [];
@ -110,7 +112,10 @@ export const readClientRegistrationStatuses = async (
continue;
}
try {
const files = await readClientConfigurationFiles(client);
const files = await readClientConfigurationFiles(
client,
options.resources,
);
if (!files.ok) {
statuses.push({
client: client.name,
@ -128,7 +133,12 @@ export const readClientRegistrationStatuses = async (
);
continue;
}
const policy = await readClientPolicyBlock(client, parsed);
const policy = await readClientPolicyBlock(
client,
parsed,
undefined,
options.resources,
);
if (!policy.ok) {
statuses.push({
client: client.name,

79
src/application/Doctor.ts
View File

@ -1,5 +1,6 @@
import { constants } from "node:fs";
import { access, readdir, stat } from "node:fs/promises";
import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js";
import { homeDirectoryFromEnvironment } from "../config/homeDirectory.js";
import { join } from "node:path";
@ -100,6 +101,7 @@ export interface DoctorHost {
installedSkillIdentity?(
clientIds: readonly string[],
): Promise<InstalledSkillIdentity | undefined>;
close?(): Promise<string | undefined>;
clientRegistrations?(): Promise<readonly ClientRegistrationStatus[]>;
ilspyCmdVersion?(path: string): Promise<string | undefined>;
}
@ -155,7 +157,41 @@ interface DoctorIdentity {
*/
export const runDoctor = async (
target?: string,
host: DoctorHost = systemDoctorHost(),
selectedHost?: DoctorHost,
requestedScope?: DoctorScope,
): Promise<DoctorReport> =>
selectedHost === undefined
? runDoctorOwned(target, systemDoctorHost(), requestedScope)
: inspectDoctor(target, selectedHost, requestedScope);
/** Inspect with a host whose lifetime belongs to this operation. */
export const runDoctorOwned = async (
target: string | undefined,
host: DoctorHost,
requestedScope?: DoctorScope,
): Promise<DoctorReport> => {
let report: DoctorReport;
try {
report = await inspectDoctor(target, host, requestedScope);
} catch (cause: unknown) {
const cleanup = await closeDoctorHost(host);
if (cleanup !== undefined)
throw new AggregateError(
[cause, new Error(cleanup)],
"Doctor failed and its resource cleanup remains incomplete.",
{ cause },
);
throw cause;
}
const cleanup = await closeDoctorHost(host);
return cleanup === undefined
? report
: doctorReportWithCleanup(report, cleanup);
};
const inspectDoctor = async (
target: string | undefined,
host: DoctorHost,
requestedScope?: DoctorScope,
): Promise<DoctorReport> => {
const scope = normalizeDoctorScope(requestedScope, target);
@ -197,6 +233,36 @@ export const runDoctor = async (
};
};
const closeDoctorHost = async (
host: DoctorHost,
): Promise<string | undefined> => {
try {
return await host.close?.();
} catch (cause: unknown) {
return `Doctor host cleanup failed: ${cause instanceof Error ? cause.message : String(cause)}`;
}
};
const doctorReportWithCleanup = (
report: DoctorReport,
detail: string,
): DoctorReport => {
const check: DoctorCheck = {
name: "doctor-resource-cleanup",
ok: false,
classification: "config_drift",
detail,
remediation: "Resolve the reported cleanup failure, then rerun rea doctor.",
};
return {
...report,
healthy: false,
environment_healthy: false,
checks: [...report.checks, check],
scope_checks: [...report.scope_checks, check],
};
};
const collectDoctorIdentity = async (
host: DoctorHost,
scope: DoctorScopeReport,
@ -307,6 +373,7 @@ export interface SystemDoctorHostOptions {
export const systemDoctorHost = (
options: SystemDoctorHostOptions = {},
): DoctorHost => {
const resources = new ArtifactResourceScope();
const platform = options.platform ?? process.platform;
const architecture = options.architecture ?? process.arch;
const environment = options.environment ?? process.env;
@ -446,11 +513,21 @@ export const systemDoctorHost = (
clientIds,
environment,
platform,
resources,
),
close: async () => {
try {
await resources.close();
return undefined;
} catch (cause: unknown) {
return `Doctor resource cleanup failed: ${cause instanceof Error ? cause.message : String(cause)}`;
}
},
clientRegistrations: () =>
readClientRegistrationStatuses(homeDirectory, undefined, {
platform,
environment,
resources,
}),
};
};

14
src/application/GeminiClientSettings.ts
View File

@ -4,6 +4,7 @@ import { stripComments } from "jsonc-parser";
import { z } from "zod";
import { err, ok, type Result } from "../domain/result.js";
import { PRODUCT_IDENTITY } from "../identity.js";
import type { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js";
import { npxRegistrationCommand } from "./ClientRegistrationIdentity.js";
import {
clientConfigurationValuesEqual,
@ -61,6 +62,7 @@ const normalizedPath = async (
const workspaceTrusted = async (
settings: NonNullable<SetupClient["geminiSettings"]>,
sources: readonly SettingsSource[],
resources?: ArtifactResourceScope,
): Promise<Result<boolean, ClientConfigurationFileError>> => {
let enabled = true;
for (const { path, document } of sources) {
@ -70,7 +72,10 @@ const workspaceTrusted = async (
}
if (settings.trustOverride !== undefined) return ok(settings.trustOverride);
if (!enabled) return ok(true);
const text = await readConfigurationText(settings.trustedFoldersPath);
const text = await readConfigurationText(
settings.trustedFoldersPath,
resources,
);
if (!text.ok) return text;
if (text.value === undefined) return ok(false);
let rules: z.output<typeof trustRulesSchema>;
@ -110,6 +115,7 @@ export const readClientPolicyBlock = async (
client: SetupClient,
userDocument?: ClientConfigurationDocument,
desiredRegistration?: unknown,
resources?: ArtifactResourceScope,
): Promise<Result<string | undefined, ClientConfigurationFileError>> => {
if (client.name !== "gemini_cli" || client.geminiSettings === undefined)
return ok(undefined);
@ -124,7 +130,7 @@ export const readClientPolicyBlock = async (
const parsed =
path === client.configPath && userDocument !== undefined
? ok(userDocument)
: await readConfigurationFile(path, "json");
: await readConfigurationFile(path, "json", resources);
if (!parsed.ok) return parsed;
if (parsed.value !== undefined)
sources.push({ path, document: parsed.value });
@ -143,10 +149,10 @@ export const readClientPolicyBlock = async (
".gemini",
"settings.json",
);
const document = await readConfigurationFile(path, "json");
const document = await readConfigurationFile(path, "json", resources);
if (!document.ok) return document;
if (document.value !== undefined) {
const trusted = await workspaceTrusted(settings, sources);
const trusted = await workspaceTrusted(settings, sources, resources);
if (!trusted.ok) return trusted;
if (trusted.value) {
const source = { path, document: document.value };

26
src/application/RegularFileRead.ts
View File

@ -16,6 +16,7 @@ import type { AnalysisCleanupObservation } from "../domain/analysisErrorBase.js"
export interface RegularFileReadOptions {
readonly signal?: AbortSignal | undefined;
readonly symlinks?: "follow" | "reject";
readonly resources?: ArtifactResourceScope;
}
export type RegularFileReadOutcome<Value> =
@ -68,6 +69,15 @@ export const withRegularFile = async <Value>(
path: string,
read: (handle: FileHandle, stats: Stats) => Promise<Value>,
options: RegularFileReadOptions = {},
): Promise<Value> =>
options.resources === undefined
? withRegularFileOwned(path, read, options)
: options.resources.run(() => withRegularFileOwned(path, read, options));
const withRegularFileOwned = async <Value>(
path: string,
read: (handle: FileHandle, stats: Stats) => Promise<Value>,
options: RegularFileReadOptions,
): Promise<Value> => {
const { signal } = options;
let handle: FileHandle;
@ -78,12 +88,15 @@ export const withRegularFile = async <Value>(
});
} catch (cause: unknown) {
if (!(cause instanceof RegularFileAdmissionFailure)) throw cause;
throw new RegularFileCleanupFailure(
const failure = new RegularFileCleanupFailure(
path,
cause.owner,
{ kind: "failed", cause: cause.cause },
cause.cleanupCause,
);
if (options.resources === undefined) throw failure;
await retryRegularFileCleanup(failure, options.resources);
throw cause.cause;
}
const owner = new OwnedFileHandle(handle);
let outcome: RegularFileReadOutcome<Value>;
@ -108,7 +121,16 @@ export const withRegularFile = async <Value>(
try {
await owner.close();
} catch (cleanupCause: unknown) {
throw new RegularFileCleanupFailure(path, owner, outcome, cleanupCause);
const failure = new RegularFileCleanupFailure(
path,
owner,
outcome,
cleanupCause,
);
if (options.resources === undefined) throw failure;
const cleanup = await retryRegularFileCleanup(failure, options.resources);
if (cleanup.outcome.kind === "failed") throw cleanup.outcome.cause;
return cleanup.outcome.value;
}
if (outcome.kind === "failed") throw outcome.cause;
return outcome.value;

29
src/application/SetupHost.ts
View File

@ -122,7 +122,7 @@ export const systemSetupHost = (
nodeVersion: process.versions.node,
macosVersion: () => doctorHost.macosVersion(),
linuxDistribution: readLinuxDistribution,
close: () => closeSetupResources(resources, macHopperHost),
close: () => closeSetupResources(resources, macHopperHost, doctorHost),
initialSetupState: async (
scope?: DoctorScope,
): Promise<SetupInitialState> => {
@ -181,9 +181,17 @@ export const systemSetupHost = (
clientIds,
environment,
platform,
resources,
),
installSkill: (clientIds) =>
installCanonicalSkill(homeDirectory, clientIds, environment, platform),
installCanonicalSkill(
homeDirectory,
clientIds,
environment,
platform,
undefined,
resources,
),
doctor: (scope) => runDoctor(undefined, doctorHost, scope),
};
};
@ -226,11 +234,14 @@ const clientConfigurationOperations = (resources: ArtifactResourceScope) => ({
const closeSetupResources = async (
resources: ArtifactResourceScope,
macHopperHost: ReturnType<typeof systemMacHopperInstallHost> | undefined,
doctorHost: DoctorHost,
): Promise<string | undefined> => {
const [resourceCleanup, hopperCleanup] = await Promise.allSettled([
resources.close(),
Promise.resolve().then(() => macHopperHost?.close()),
]);
const [resourceCleanup, hopperCleanup, doctorCleanup] =
await Promise.allSettled([
resources.close(),
Promise.resolve().then(() => macHopperHost?.close()),
Promise.resolve().then(() => doctorHost.close?.()),
]);
const failures: string[] = [];
if (hopperCleanup.status === "rejected")
failures.push(
@ -238,6 +249,12 @@ const closeSetupResources = async (
);
else if (hopperCleanup.value !== undefined)
failures.push(hopperCleanup.value);
if (doctorCleanup.status === "rejected")
failures.push(
`Doctor host cleanup failed: ${errorMessage(doctorCleanup.reason)}`,
);
else if (doctorCleanup.value !== undefined)
failures.push(doctorCleanup.value);
if (resourceCleanup.status === "rejected")
failures.push(errorMessage(resourceCleanup.reason));
return failures.length === 0 ? undefined : failures.join("; ");

84
src/application/SetupSkill.ts
View File

@ -4,13 +4,18 @@ import { dirname, join } from "node:path";
import writeFileAtomic from "write-file-atomic";
import { err, ok, type Result } from "../domain/result.js";
import { readBoundedFileBytes } from "../process/BoundedFileBytes.js";
import type { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js";
import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js";
import { PRODUCT_IDENTITY } from "../identity.js";
import {
NonRegularFileReadError,
RegularFileChangedError,
} from "../filesystem/RegularFile.js";
import { withRegularFile } from "./RegularFileRead.js";
import {
RegularFileCleanupFailure,
withRegularFile,
} from "./RegularFileRead.js";
import { clientSkillDirectories } from "./SupportedClients.js";
export const MANAGED_SKILL_FILES = [
@ -27,15 +32,21 @@ export const isManagedSkillManifest = (content: string): boolean =>
content.startsWith(`---\nname: ${PRODUCT_IDENTITY.skillName}\n`) ||
content.startsWith(`---\r\nname: ${PRODUCT_IDENTITY.skillName}\r\n`);
export const readSkillFile = (path: string): Promise<string> =>
readSkillFileState(path).then(({ content }) => content);
export const readSkillFile = (
path: string,
resources?: ArtifactResourceScope,
): Promise<string> =>
readSkillFileState(path, resources).then(({ content }) => content);
interface SkillFileState {
readonly content: string;
readonly mode: number;
}
const readSkillFileState = async (path: string): Promise<SkillFileState> =>
const readSkillFileState = async (
path: string,
resources?: ArtifactResourceScope,
): Promise<SkillFileState> =>
withRegularFile(
path,
async (handle, stats) => {
@ -49,7 +60,10 @@ const readSkillFileState = async (path: string): Promise<SkillFileState> =>
throw new RegularFileChangedError(path);
return { content: bytes.toString("utf8"), mode: stats.mode & 0o7777 };
},
{ symlinks: "reject" },
{
symlinks: "reject",
...(resources === undefined ? {} : { resources }),
},
);
interface CanonicalSkillFile {
@ -173,6 +187,7 @@ export const existingSkillDestinations = async (
home: string,
environment: Readonly<NodeJS.ProcessEnv> = {},
platform: NodeJS.Platform = process.platform,
resources?: ArtifactResourceScope,
): Promise<readonly SkillDestination[]> => {
const existing: SkillDestination[] = [];
for (const destination of skillDestinations(
@ -185,11 +200,16 @@ export const existingSkillDestinations = async (
const root = await lstat(destination.path);
if (root.isSymbolicLink() || !root.isDirectory()) continue;
const manifest = join(destination.path, "SKILL.md");
const content = await readSkillFile(manifest);
const content = await readSkillFile(manifest, resources);
if (isManagedSkillManifest(content)) existing.push(destination);
} catch (cause: unknown) {
if (cause instanceof Error && "code" in cause && cause.code === "ENOENT")
continue;
if (
cause instanceof RegularFileCleanupFailure ||
cause instanceof ArtifactReaderFailure
)
throw cause;
if (cause instanceof NonRegularFileReadError) continue;
throw cause;
}
@ -199,9 +219,10 @@ export const existingSkillDestinations = async (
const readOptionalSkillState = async (
path: string,
resources?: ArtifactResourceScope,
): Promise<SkillFileState | undefined> => {
try {
return await readSkillFileState(path);
return await readSkillFileState(path, resources);
} catch (cause: unknown) {
if (cause instanceof Error && "code" in cause && cause.code === "ENOENT")
return undefined;
@ -214,6 +235,7 @@ const canonicalSkillFiles = async (
clientIds: readonly string[],
environment: Readonly<NodeJS.ProcessEnv> = {},
platform: NodeJS.Platform = process.platform,
resources?: ArtifactResourceScope,
): Promise<readonly CanonicalSkillFile[]> =>
Promise.all(
skillDestinations(home, clientIds, environment, platform).flatMap(
@ -229,7 +251,7 @@ const canonicalSkillFiles = async (
),
"utf8",
),
original: await readOptionalSkillState(destination),
original: await readOptionalSkillState(destination, resources),
};
}),
),
@ -241,12 +263,24 @@ export const canonicalSkillNeedsInstall = async (
clientIds: readonly string[] = [],
environment: Readonly<NodeJS.ProcessEnv> = {},
platform: NodeJS.Platform = process.platform,
resources?: ArtifactResourceScope,
): Promise<boolean> => {
try {
return (
await canonicalSkillFiles(home, clientIds, environment, platform)
await canonicalSkillFiles(
home,
clientIds,
environment,
platform,
resources,
)
).some(({ content, original }) => original?.content !== content);
} catch (cause: unknown) {
if (
cause instanceof RegularFileCleanupFailure ||
cause instanceof ArtifactReaderFailure
)
throw cause;
// Unreadable skill state fails open to install so setup can repair it.
void cause;
return true;
@ -277,7 +311,6 @@ const restoreSkillFiles = async (
return failures;
};
/** Transactionally install or upgrade the canonical REA skill and references. */
/** Transactionally install the canonical skill bundle and optional references. */
export const installCanonicalSkill = async (
home: string,
@ -285,6 +318,7 @@ export const installCanonicalSkill = async (
environment: Readonly<NodeJS.ProcessEnv> = {},
platform: NodeJS.Platform = process.platform,
operations: SkillInstallFileOperations = systemSkillInstallFileOperations,
resources?: ArtifactResourceScope,
): Promise<SkillInstallResult> => {
let attempted: CanonicalSkillFile[] = [];
let stage: "prepare" | "backup" | "write" | "readback" = "prepare";
@ -295,6 +329,7 @@ export const installCanonicalSkill = async (
clientIds,
environment,
platform,
resources,
);
const changed = canonical.filter(
({ content, original }) => original?.content !== content,
@ -323,7 +358,7 @@ export const installCanonicalSkill = async (
stage = "readback";
for (const { destination, content } of changed) {
failingPath = destination;
if ((await readSkillFile(destination)) !== content)
if ((await readSkillFile(destination, resources)) !== content)
throw new Error(`skill readback mismatch: ${destination}`);
}
return ok("installed");
@ -351,6 +386,7 @@ export const readInstalledSkillIdentity = async (
clientIds: readonly string[],
environment: Readonly<NodeJS.ProcessEnv> = {},
platform: NodeJS.Platform = process.platform,
resources?: ArtifactResourceScope,
): Promise<InstalledSkillIdentity | undefined> => {
const destinations = skillDestinations(
home,
@ -361,22 +397,38 @@ export const readInstalledSkillIdentity = async (
let content: string | undefined;
for (const destination of destinations) {
try {
content = await readSkillFile(join(destination.path, "SKILL.md"));
content = await readSkillFile(
join(destination.path, "SKILL.md"),
resources,
);
break;
} catch {
} catch (cause: unknown) {
if (
cause instanceof RegularFileCleanupFailure ||
cause instanceof ArtifactReaderFailure
)
throw cause;
// Another selected copy may provide metadata; canonical validation below
// still marks missing or unreadable copies as stale.
}
}
if (content === undefined) return undefined;
const toolCount = /^\s{2}tool_count:\s*(\d+)\s*$/mu.exec(content)?.[1];
return {
canonical: !(await canonicalSkillNeedsInstall(
let canonical: boolean;
try {
canonical = !(await canonicalSkillNeedsInstall(
home,
clientIds,
environment,
platform,
)),
resources,
));
} catch (cause: unknown) {
if (!(cause instanceof ArtifactReaderFailure)) throw cause;
canonical = false;
}
return {
canonical,
version: /^\s{2}version:\s*"([^"]+)"\s*$/mu.exec(content)?.[1] ?? null,
toolCount: toolCount === undefined ? null : Number.parseInt(toolCount, 10),
};

111
src/application/Uninstall.ts
View File

@ -24,6 +24,7 @@ import {
import { isOwnedClientRegistrationCommand } from "./ClientRegistrationIdentity.js";
import { resolveClientConfigTransactionPath } from "./ClientConfigPath.js";
import { readRegularFileText } from "./RegularFileRead.js";
import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js";
import {
manualRegistrationRemediation,
supportedClients,
@ -50,9 +51,11 @@ export interface UninstallFileSystem {
remove(path: string, recursive?: boolean): Promise<void>;
}
const systemFileSystem: UninstallFileSystem = {
readText: (path) => readRegularFileText(path),
readRegularText: readSkillFile,
const systemFileSystem = (
resources: ArtifactResourceScope,
): UninstallFileSystem => ({
readText: (path) => readRegularFileText(path, { resources }),
readRegularText: (path) => readSkillFile(path, resources),
copy: (source, destination) =>
copyFile(source, destination, fsConstants.COPYFILE_EXCL),
writeText: (path, contents) =>
@ -60,7 +63,7 @@ const systemFileSystem: UninstallFileSystem = {
stat: (path) => lstat(path),
realpath: (path) => realpath(path),
remove: (path, recursive = true) => rm(path, { recursive }),
};
});
/** One explicitly classified uninstall action. */
export interface UninstallItem {
@ -83,6 +86,7 @@ export interface UninstallHost {
removeClient(client: SetupClient): Promise<UninstallItem>;
removeSkill(): Promise<UninstallItem>;
purgeData(): Promise<readonly UninstallItem[]>;
close?(): Promise<string | undefined>;
}
/**
@ -95,35 +99,66 @@ export const runUninstall = async (
purgeData: boolean,
host: UninstallHost = systemUninstallHost(),
): Promise<UninstallResult> => {
const clients = await host.clients();
const blocked: UninstallItem[] = [];
for (const client of clients) {
const blocking = await host.inspectClient(client);
if (blocking !== undefined) blocked.push(blocking);
let result: UninstallResult | undefined;
let primaryFailure: unknown;
let failed = false;
try {
const clients = await host.clients();
const blocked: UninstallItem[] = [];
for (const client of clients) {
const blocking = await host.inspectClient(client);
if (blocking !== undefined) blocked.push(blocking);
}
const items: UninstallItem[] =
blocked.length > 0
? [
...blocked,
item(
"uninstall",
"skipped",
"No REA registration, skill, or data was removed. Repair each failed configuration, then rerun uninstall.",
),
]
: await removeAll(clients, purgeData, host);
items.push({
name: "analysis_engine",
status: "retained",
detail: "Hopper is not owned by REA uninstall.",
});
const reported = collapseUninstallItems(items);
result = {
status: reported.some(({ status }) => status === "failed")
? "failed"
: "complete",
items: reported,
};
} catch (cause: unknown) {
failed = true;
primaryFailure = cause;
}
const items: UninstallItem[] =
blocked.length > 0
? [
...blocked,
item(
"uninstall",
"skipped",
"No REA registration, skill, or data was removed. Repair each failed configuration, then rerun uninstall.",
),
]
: await removeAll(clients, purgeData, host);
items.push({
name: "analysis_engine",
status: "retained",
detail: "Hopper is not owned by REA uninstall.",
});
const reported = collapseUninstallItems(items);
return {
status: reported.some(({ status }) => status === "failed")
? "failed"
: "complete",
items: reported,
};
let cleanupFailure: string | undefined;
try {
cleanupFailure = await host.close?.();
} catch (cause: unknown) {
cleanupFailure = `Uninstall resource cleanup failed: ${cause instanceof Error ? cause.message : String(cause)}`;
}
if (failed) {
if (cleanupFailure !== undefined)
throw new AggregateError(
[primaryFailure, new Error(cleanupFailure)],
"Uninstall failed and its resource cleanup remains incomplete.",
{ cause: primaryFailure },
);
throw primaryFailure;
}
if (result === undefined)
throw new Error("Uninstall result was not produced.");
if (cleanupFailure === undefined) return result;
const items = collapseUninstallItems([
...result.items,
item("uninstall_cleanup", "failed", cleanupFailure),
]);
return { status: "failed", items };
};
/**
@ -157,12 +192,14 @@ const removeAll = async (
/** Create uninstall effects contained to detected client configs and REA-owned paths. */
export const systemUninstallHost = (
selectedHome: string | undefined = undefined,
fileSystem: UninstallFileSystem = systemFileSystem,
selectedFileSystem: UninstallFileSystem | undefined = undefined,
environment: Readonly<NodeJS.ProcessEnv> = process.env,
platform: NodeJS.Platform = process.platform,
): UninstallHost => {
const home =
selectedHome ?? homeDirectoryFromEnvironment(environment, platform);
const resources = new ArtifactResourceScope();
const fileSystem = selectedFileSystem ?? systemFileSystem(resources);
return {
clients: () =>
Promise.resolve(
@ -185,6 +222,14 @@ export const systemUninstallHost = (
removeSkill: () =>
removeManagedSkills(home, fileSystem, environment, platform),
purgeData: () => purgeManagedData(join(home, ".rea"), fileSystem),
close: async () => {
try {
await resources.close();
return undefined;
} catch (cause: unknown) {
return `Uninstall resource cleanup failed: ${cause instanceof Error ? cause.message : String(cause)}`;
}
},
};
};

18
src/application/Update.test.ts
View File

@ -18,6 +18,7 @@ import {
type NpmInstallationHost,
} from "./UpdateRuntime.js";
import type { IntegrationMaintenance } from "./UpdateMaintenance.js";
import { isUpdateFailure } from "./UpdateFailure.js";
class FakeUpdateHost implements UpdateHost {
release: Result<string, ReleaseLookupFailure> = ok("3.3.0");
@ -37,6 +38,7 @@ class FakeUpdateHost implements UpdateHost {
verifiedInstallation: UpdateInstallation | undefined;
maintenanceInspected = false;
output: UpdateOutput | undefined;
cleanupFailure: string | undefined;
installation = (): Promise<UpdateInstallation | undefined> => {
this.installationInspected = true;
@ -63,6 +65,8 @@ class FakeUpdateHost implements UpdateHost {
this.maintenanceInspected = true;
return Promise.resolve(this.maintenance);
};
close = (): Promise<string | undefined> =>
Promise.resolve(this.cleanupFailure);
}
describe("REA update", () => {
@ -79,6 +83,20 @@ describe("REA update", () => {
expect(host.maintenanceInspected).toBe(false);
});
it("preserves update status while marking incomplete resource cleanup as failure", async () => {
const host = new FakeUpdateHost();
host.cleanupFailure = "Regular file remains open: /fixture/skill/SKILL.md";
const result = await runUpdate("3.3.0", host);
expect(result).toMatchObject({
status: "current",
currentVersion: "3.3.0",
cleanupFailure: host.cleanupFailure,
});
expect(isUpdateFailure(result)).toBe(true);
});
it("reports an unknown current installation honestly", async () => {
const host = new FakeUpdateHost();
host.installationResult = undefined;

39
src/application/Update.ts
View File

@ -49,10 +49,11 @@ export interface UpdateHost {
planMaintenance(
installation: UpdateInstallation,
): Promise<IntegrationMaintenance>;
close?(): Promise<string | undefined>;
}
/** Verified update outcome, separate from the unapplied integration plan. */
export type UpdateResult =
export type UpdateResult = (
| {
readonly status: "current";
readonly currentVersion: string;
@ -80,7 +81,8 @@ export type UpdateResult =
| "verification";
readonly installedVersion?: string;
readonly remediation: string;
};
}
) & { readonly cleanupFailure?: string };
/** Resolve the exact package-manager command for the identified installation and release. */
export const updateInstallCommand = (
@ -141,6 +143,39 @@ export const runUpdate = async (
currentVersion: string,
host: UpdateHost,
output: UpdateOutput = "human",
): Promise<UpdateResult> => {
let result: UpdateResult | undefined;
let primaryFailure: unknown;
let failed = false;
try {
result = await runUpdateWorkflow(currentVersion, host, output);
} catch (cause: unknown) {
failed = true;
primaryFailure = cause;
}
let cleanupFailure: string | undefined;
try {
cleanupFailure = await host.close?.();
} catch (cause: unknown) {
cleanupFailure = `Update resource cleanup failed: ${cause instanceof Error ? cause.message : String(cause)}`;
}
if (failed) {
if (cleanupFailure !== undefined)
throw new AggregateError(
[primaryFailure, new Error(cleanupFailure)],
"Update failed and its resource cleanup remains incomplete.",
{ cause: primaryFailure },
);
throw primaryFailure;
}
if (result === undefined) throw new Error("Update result was not produced.");
return cleanupFailure === undefined ? result : { ...result, cleanupFailure };
};
const runUpdateWorkflow = async (
currentVersion: string,
host: UpdateHost,
output: UpdateOutput,
): Promise<UpdateResult> => {
const installation = await host.installation();
const release = await host.latestVersion(installation);

1
src/application/UpdateFailure.ts
View File

@ -2,6 +2,7 @@ import type { UpdateResult } from "./Update.js";
/** Whether the update command left the requested release unapplied. */
export const isUpdateFailure = (result: UpdateResult): boolean => {
if (result.cleanupFailure !== undefined) return true;
const status = result.status;
switch (status) {
case "current":

23
src/application/UpdateMaintenance.ts
View File

@ -14,6 +14,7 @@ import { existingSkillDestinations } from "./SetupSkill.js";
import type { SetupAction } from "./SetupTypes.js";
import type { Result } from "../domain/result.js";
import { readRegularFileText } from "./RegularFileRead.js";
import type { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js";
/** Existing REA integrations selected for maintenance, without discovering new targets. */
export interface MaintenanceScope {
@ -58,9 +59,11 @@ export const existingMaintenanceScope = async (
home: string,
entryPoint: string,
environment: Readonly<NodeJS.ProcessEnv>,
resources?: ArtifactResourceScope,
): Promise<MaintenanceScope> => {
const registrations = await readClientRegistrationStatuses(home, entryPoint, {
environment,
...(resources === undefined ? {} : { resources }),
});
const supported = supportedClients(home, process.platform, environment);
const clients: string[] = [];
@ -73,7 +76,10 @@ export const existingMaintenanceScope = async (
const client = supported.find(({ name }) => name === registration.client);
if (client === undefined) continue;
const parsed = parseClientConfiguration(
await readRegularFileText(client.configPath),
await readRegularFileText(
client.configPath,
resources === undefined ? {} : { resources },
),
client.format,
);
const enabled = z
@ -91,7 +97,12 @@ export const existingMaintenanceScope = async (
)
clients.push(client.name);
}
const skillDestinations = await existingSkillDestinations(home, environment);
const skillDestinations = await existingSkillDestinations(
home,
environment,
process.platform,
resources,
);
return {
clients,
skill: skillDestinations.length > 0,
@ -105,9 +116,15 @@ export const planIntegrationMaintenance = async (
entryPoint: string,
environment: Readonly<NodeJS.ProcessEnv>,
execute: (command: readonly string[]) => Promise<Result<string, string>>,
resources?: ArtifactResourceScope,
): Promise<IntegrationMaintenance> => {
try {
const scope = await existingMaintenanceScope(home, entryPoint, environment);
const scope = await existingMaintenanceScope(
home,
entryPoint,
environment,
resources,
);
const skillDestinations = scope.skillDestinations ?? [];
if (scope.clients.length === 0 && !scope.skill)
return { status: "current", plannedActions: [] };

145
src/application/UpdateRuntime.ts
View File

@ -6,6 +6,7 @@ import spawn from "cross-spawn";
import { lt, valid } from "semver";
import { PRODUCT_IDENTITY } from "../identity.js";
import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js";
import { err, ok, type Result } from "../domain/result.js";
import { safeParseJson } from "../domain/safeJson.js";
import {
@ -292,74 +293,86 @@ export const systemUpdateHost = (
packageRoot = fileURLToPath(new URL("../..", import.meta.url)),
home: string | undefined = undefined,
environment: Readonly<NodeJS.ProcessEnv> = process.env,
): UpdateHost => ({
installation: async () =>
(await detectBunInstallation(
packageRoot,
bunGlobalDirectory(environment, process.platform),
{ canonicalPath: realpath, isFile },
)) ??
detectNpmInstallation(packageRoot, {
canonicalPath: realpath,
globalRoot: () =>
requireCommandOutput(["npm", "root", "--global"], environment),
globalPrefix: () =>
requireCommandOutput(["npm", "prefix", "--global"], environment),
}),
latestVersion: async (installation) => {
const context = packageManagerContext(installation, environment);
if (installation?.manager === "bun") {
const version = await runUpdateCommand(
["bun", "--version"],
): UpdateHost => {
const resources = new ArtifactResourceScope();
return {
installation: async () =>
(await detectBunInstallation(
packageRoot,
bunGlobalDirectory(environment, process.platform),
{ canonicalPath: realpath, isFile },
)) ??
detectNpmInstallation(packageRoot, {
canonicalPath: realpath,
globalRoot: () =>
requireCommandOutput(["npm", "root", "--global"], environment),
globalPrefix: () =>
requireCommandOutput(["npm", "prefix", "--global"], environment),
}),
latestVersion: async (installation) => {
const context = packageManagerContext(installation, environment);
if (installation?.manager === "bun") {
const version = await runUpdateCommand(
["bun", "--version"],
context.environment,
"structured",
context.workingDirectory,
);
if (!version.ok)
return err({ kind: "unavailable", detail: version.error });
const bunVersion = valid(version.value);
// bun info was introduced in 1.2.17; older releases reserve the command.
if (bunVersion === null || lt(bunVersion, "1.2.17"))
return err({
kind: "unsupported-package-manager",
detail: `REA updates for Bun global installations require Bun 1.2.17 or later; bun --version returned ${JSON.stringify(version.value)}. Upgrade Bun manually, then rerun rea update.`,
});
}
return npmReleaseLookup(
await runUpdateCommand(
installation?.manager === "bun"
? BUN_LATEST_VERSION_COMMAND
: npmLatestVersionCommand(installation),
context.environment,
"structured",
context.workingDirectory,
),
);
},
installVersion: async (installation, version, output) => {
const context = packageManagerContext(installation, environment);
const result = await runUpdateCommand(
updateInstallCommand(installation, version),
context.environment,
"structured",
output,
context.workingDirectory,
);
if (!version.ok)
return err({ kind: "unavailable", detail: version.error });
const bunVersion = valid(version.value);
// bun info was introduced in 1.2.17; older releases reserve the command.
if (bunVersion === null || lt(bunVersion, "1.2.17"))
return err({
kind: "unsupported-package-manager",
detail: `REA updates for Bun global installations require Bun 1.2.17 or later; bun --version returned ${JSON.stringify(version.value)}. Upgrade Bun manually, then rerun rea update.`,
});
}
return npmReleaseLookup(
await runUpdateCommand(
installation?.manager === "bun"
? BUN_LATEST_VERSION_COMMAND
: npmLatestVersionCommand(installation),
context.environment,
"structured",
context.workingDirectory,
return result.ok ? ok(undefined) : result;
},
installedVersion: (installation) =>
runUpdateCommand(
[
process.execPath,
join(installation.packageRoot, "scripts", "rea.mjs"),
"--version",
],
environment,
),
);
},
installVersion: async (installation, version, output) => {
const context = packageManagerContext(installation, environment);
const result = await runUpdateCommand(
updateInstallCommand(installation, version),
context.environment,
output,
context.workingDirectory,
);
return result.ok ? ok(undefined) : result;
},
installedVersion: (installation) =>
runUpdateCommand(
[
process.execPath,
planMaintenance: (installation) =>
planIntegrationMaintenance(
home ?? homeDirectoryFromEnvironment(environment, process.platform),
join(installation.packageRoot, "scripts", "rea.mjs"),
"--version",
],
environment,
),
planMaintenance: (installation) =>
planIntegrationMaintenance(
home ?? homeDirectoryFromEnvironment(environment, process.platform),
join(installation.packageRoot, "scripts", "rea.mjs"),
environment,
(command) => runUpdateCommand(command, environment),
),
});
environment,
(command) => runUpdateCommand(command, environment),
resources,
),
close: async () => {
try {
await resources.close();
return undefined;
} catch (cause: unknown) {
return `Update resource cleanup failed: ${cause instanceof Error ? cause.message : String(cause)}`;
}
},
};
};

11
src/cli/setupCommands.ts
View File

@ -120,11 +120,12 @@ const registerDoctorCommand = (
}),
run: ({ options }) =>
logCliCommand(logger, "doctor", async () => {
const [{ runDoctor }, { createSystemDoctorHost }] = await Promise.all([
import("../application/Doctor.js"),
import("../doctorRuntime.js"),
]);
return runDoctor(
const [{ runDoctorOwned }, { createSystemDoctorHost }] =
await Promise.all([
import("../application/Doctor.js"),
import("../doctorRuntime.js"),
]);
return runDoctorOwned(
options.target,
createSystemDoctorHost(environment),
doctorScope(options),

237
tests/boundary/filesystem/setupSkillCleanupOwnership.test.ts Normal file
View File

@ -0,0 +1,237 @@
import {
mkdir,
open as fsOpen,
writeFile,
type FileHandle,
} from "node:fs/promises";
import { dirname, join } from "node:path";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { ArtifactResourceScope } from "../../../src/artifacts/ArtifactResourceScope.js";
import { PRODUCT_IDENTITY } from "../../../src/identity.js";
import { createDoctorHostFixture } from "../../../src/application/Doctor.fixture.js";
import {
runDoctorOwned,
type DoctorHost,
} from "../../../src/application/Doctor.js";
import {
readSkillFile,
skillDestinations,
} from "../../../src/application/SetupSkill.js";
import { systemSetupHost } from "../../../src/application/SetupHost.js";
import {
runUninstall,
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 failFirstCloses = (path: string) => {
const attempts: FileHandle[] = [];
let allowClose = 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 () => {
attempts.push(handle);
if (!allowClose) throw new Error("test holds the skill handle open");
await nativeClose();
});
return handle;
});
return {
attempts,
release: () => {
allowClose = true;
},
};
};
describe("skill regular-file cleanup ownership", () => {
it("keeps setup skill reads owned until setup host disposal", async () => {
const home = await createTestTempDirectory("rea-setup-skill-close-");
const [destination] = skillDestinations(home, []);
if (destination === undefined) throw new Error("Shared skill root missing");
const manifest = join(destination.path, "SKILL.md");
await mkdir(dirname(manifest), { recursive: true });
await writeFile(manifest, "stale managed skill\n");
const close = failFirstCloses(manifest);
const host = systemSetupHost(
{ platform: "linux", homeDirectory: home } as DoctorHost,
{ HOME: home },
);
try {
expect(await host.skillNeedsInstall([])).toBe(true);
expect(close.attempts.length).toBeGreaterThan(0);
expect(new Set(close.attempts).size).toBe(1);
const attemptsBeforeClose = close.attempts.length;
close.release();
expect(await host.close?.()).toBeUndefined();
expect(close.attempts.length).toBeGreaterThan(attemptsBeforeClose);
expect(new Set(close.attempts).size).toBe(1);
expect(close.attempts[0]?.fd).toBe(-1);
} finally {
close.release();
await host.close?.();
}
});
it("waits for an admitted skill read before its resource scope closes", async () => {
const home = await createTestTempDirectory("rea-setup-skill-inflight-");
const [destination] = skillDestinations(home, []);
if (destination === undefined) throw new Error("Shared skill root missing");
const manifest = join(destination.path, "SKILL.md");
await mkdir(dirname(manifest), { recursive: true });
await writeFile(manifest, "stale managed skill\n");
const resources = new ArtifactResourceScope();
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]) !== manifest || 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;
});
try {
const reading = readSkillFile(manifest, resources);
await readStarted;
let closed = false;
const closing = resources.close().then(() => {
closed = true;
});
await Promise.resolve();
expect(closed).toBe(false);
unblockRead();
expect(await reading).toBe("stale managed skill\n");
await closing;
expect(closed).toBe(true);
} finally {
unblockRead();
await resources.close();
}
});
});
describe("skill cleanup diagnostics", () => {
it("preserves a doctor report and cleanup diagnostic when host close still fails", async () => {
const home = await createTestTempDirectory("rea-doctor-skill-close-");
const [destination] = skillDestinations(home, []);
if (destination === undefined) throw new Error("Shared skill root missing");
const manifest = join(destination.path, "SKILL.md");
await mkdir(dirname(manifest), { recursive: true });
await writeFile(manifest, "managed skill fixture\n");
const close = failFirstCloses(manifest);
const resources = new ArtifactResourceScope();
const host = createDoctorHostFixture({
installedSkillIdentity: async () => {
await readSkillFile(manifest, resources);
return { version: null, toolCount: null, canonical: true };
},
close: async () => {
try {
await resources.close();
return undefined;
} catch (cause: unknown) {
return cause instanceof Error ? cause.message : String(cause);
}
},
});
try {
const report = await runDoctorOwned(undefined, host, { skill: true });
expect(report.healthy).toBe(false);
expect(report.identity?.skill.installed_version).toBeNull();
expect(report.checks).toContainEqual(
expect.objectContaining({
name: "doctor-resource-cleanup",
ok: false,
detail: expect.stringContaining(manifest),
}),
);
expect(close.attempts.length).toBeGreaterThan(0);
expect(new Set(close.attempts).size).toBe(1);
const attemptsBeforeClose = close.attempts.length;
close.release();
expect(await host.close?.()).toBeUndefined();
expect(close.attempts.length).toBeGreaterThan(attemptsBeforeClose);
expect(new Set(close.attempts).size).toBe(1);
expect(close.attempts[0]?.fd).toBe(-1);
} finally {
close.release();
await host.close?.();
}
});
it("returns uninstall changes with a failed cleanup item for a retained handle", async () => {
const home = await createTestTempDirectory("rea-uninstall-skill-close-");
const [destination] = skillDestinations(home, []);
if (destination === undefined) throw new Error("Shared skill root missing");
const manifest = join(destination.path, "SKILL.md");
await mkdir(dirname(manifest), { recursive: true });
await writeFile(
manifest,
`---\nname: ${PRODUCT_IDENTITY.skillName}\n---\nmanaged fixture\n`,
);
const close = failFirstCloses(manifest);
const host = systemUninstallHost(home, undefined, { HOME: home });
try {
const result = await runUninstall(false, host);
expect(result.status).toBe("failed");
expect(result.items).toContainEqual(
expect.objectContaining({
name: "uninstall_cleanup",
status: "failed",
detail: expect.stringContaining(manifest),
}),
);
expect(close.attempts.length).toBeGreaterThan(0);
expect(new Set(close.attempts).size).toBe(1);
const attemptsBeforeClose = close.attempts.length;
close.release();
expect(await host.close?.()).toBeUndefined();
expect(close.attempts.length).toBeGreaterThan(attemptsBeforeClose);
expect(new Set(close.attempts).size).toBe(1);
expect(close.attempts[0]?.fd).toBe(-1);
} finally {
close.release();
await host.close?.();
}
});
});