mirror of
https://github.com/morluto/rea.git
synced 2026-11-14 10:43:30 +00:00
fix(setup): preserve cleanup ownership and resource reports (#6920)
* fix(setup): report host-owned cleanup resources alongside the primary's
SetupHost.close documents that it reports the resources a setup host still
owns, but its contract returned only a string. runSetup therefore built the
ProviderCleanupError from the primary failure's cleanupResources alone, so a
resource the host failed to release reached the projection only inside a
free-text reason while the error asks the caller to review the reported local
resources.
Return a SetupCloseReport instead. closeSetupResources absorbs the structured
resources of any cause that reports them, through
ArtifactReaderFailure.cleanupObservation, and leaves opaque failures
message-only so no resource name is invented. runSetup then unions the host's
resources with the primary's before building the typed cleanup error.
Two boundary tests read the close result directly: the skill-cleanup case now
asserts the retained backup path in the report reason rather than in a bare
string, and the shared ownership helper takes an opaque close result because it
only ever awaits undefined from either host it is given.
Validation: RED then green in src/application/Setup.recovery.test.ts, where
the new case previously received only the primary resource; 116 tests across
the seven Setup and cleanup-ownership suites pass; tsc --noEmit over the
changed set, its setup callers, and both boundary tests exits 0; oxlint
reports 0 errors; oxfmt --check is clean; verify:module-boundaries passes.
The two failures in src/setupEnvironment.test.ts reproduce identically on
pristine bded9c628 with this change stashed.
* fix(setup): preserve cleanup ownership through host reports
* fix(setup): bind finalization reports to cleanup owners
* fix(setup): retain live resources in returned cleanup views
* fix(setup): preserve completed cleanup and exact installer resources
* fix(setup): retain delegated cleanup across authority changes
* fix(setup): compose callback and source cleanup authorities
* fix(setup): retain producer cleanup failure reasons
* fix(cli): render structured setup cleanup resources
* fix(setup): separate failure evidence from cleanup ownership
* fix(setup): normalize close failures and unify lease binding
* fix(setup): preserve direct lease resource attribution
---------
Co-authored-by: gensokyo-dev <342580529+gensokyo-dev@users.noreply.github.com>
This commit is contained in:
1 parent
bdf15b9bd5
commit
98ec00f406
23 files changed
+1543
-188
No files matched your search
@@ -17,6 +17,7 @@ import { describe, expect, it } from "vitest";
|
||||
import { z } from "zod";
|
||||
import { parse as parseToml } from "smol-toml";
|
||||
|
||||
import type { SetupResult } from "../../../src/application/SetupTypes.js";
|
||||
import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
|
||||
import { cliTest } from "../../support/cli/cliFixture.js";
|
||||
|
||||
@@ -492,6 +493,43 @@ describe("interactive setup completion", () => {
|
||||
);
|
||||
},
|
||||
);
|
||||
it("prints retained cleanup resources without relying on paths in remediation", async () => {
|
||||
const resources = ["/fixture/owned backup", "/fixture/临时目录"];
|
||||
const result: SetupResult = {
|
||||
status: "needs_human",
|
||||
code: "cleanup_failed",
|
||||
plannedActions: [],
|
||||
appliedActions: [],
|
||||
clients: {},
|
||||
clientStates: [],
|
||||
doctor: {
|
||||
healthy: true,
|
||||
environment_healthy: true,
|
||||
availableProviders: [],
|
||||
scope: {
|
||||
mode: "explicit",
|
||||
clients: [],
|
||||
providers: [],
|
||||
skill: false,
|
||||
target: null,
|
||||
},
|
||||
},
|
||||
remediation: "Owned descriptor remains busy",
|
||||
cleanup: { reason: "Owned descriptor remains busy", resources },
|
||||
};
|
||||
const { stderr } = await execute(
|
||||
process.execPath,
|
||||
[
|
||||
"--input-type=module",
|
||||
"--eval",
|
||||
'import { renderInteractiveSetupResult } from "./dist/cli/interactiveSetup.js";\n' +
|
||||
`renderInteractiveSetupResult(${JSON.stringify(result)}, {});`,
|
||||
],
|
||||
{ cwd: process.cwd(), env: { ...process.env, NO_COLOR: "1" } },
|
||||
);
|
||||
expect(stderr).toContain(result.remediation);
|
||||
for (const resource of resources) expect(stderr).toContain(resource);
|
||||
});
|
||||
});
|
||||
|
||||
interface JourneyStep {
|
||||
|
||||
@@ -15,8 +15,11 @@ import { afterEach, describe, expect, it } from "vitest";
|
||||
|
||||
import {
|
||||
installMacHopperBundle,
|
||||
systemMacHopperInstallHost,
|
||||
type MacHopperBundleInstallOperations,
|
||||
} from "../../../src/application/MacHopper.js";
|
||||
import { SetupCleanupOwner } from "../../../src/application/SetupCleanupOwner.js";
|
||||
import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js";
|
||||
|
||||
const roots: string[] = [];
|
||||
|
||||
@@ -176,7 +179,10 @@ describe("macOS Hopper replacement ownership", () => {
|
||||
},
|
||||
(path) => (stagingRoot = path),
|
||||
),
|
||||
(cleanup) => retained.push(cleanup),
|
||||
(cleanup, resources) => {
|
||||
expect(resources).toEqual([stagingRoot]);
|
||||
retained.push(cleanup);
|
||||
},
|
||||
);
|
||||
|
||||
expect(result).toMatchObject({
|
||||
@@ -233,7 +239,10 @@ describe("macOS Hopper replacement ownership", () => {
|
||||
},
|
||||
(path) => (stagingRoot = path),
|
||||
),
|
||||
(cleanup) => rollbackRetained.push(cleanup),
|
||||
(cleanup, resources) => {
|
||||
expect(resources).toEqual([stagingRoot]);
|
||||
rollbackRetained.push(cleanup);
|
||||
},
|
||||
);
|
||||
|
||||
expect(result).toMatchObject({
|
||||
@@ -273,3 +282,39 @@ describe("macOS Hopper replacement ownership", () => {
|
||||
await expect(access(stagingRoot)).rejects.toMatchObject({ code: "ENOENT" });
|
||||
});
|
||||
});
|
||||
|
||||
it("reports only retained Mac cleanup roots and clears the live report after exact retries", async () => {
|
||||
const home = await temporaryRoot();
|
||||
const temporary = join(home, "owned-dmg");
|
||||
const stage = join(home, "owned-stage");
|
||||
await Promise.all([mkdir(temporary), mkdir(stage)]);
|
||||
const host = systemMacHopperInstallHost(home, { HOME: home });
|
||||
let allowTemporary = false;
|
||||
let allowStage = false;
|
||||
let temporaryCalls = 0;
|
||||
host.retainCleanup?.(async () => {
|
||||
temporaryCalls++;
|
||||
if (!allowTemporary) throw new Error("DMG root busy");
|
||||
await rm(temporary, { recursive: true });
|
||||
}, [temporary]);
|
||||
host.retainCleanup?.(async () => {
|
||||
if (!allowStage) throw new Error("staging root busy");
|
||||
await rm(stage, { recursive: true });
|
||||
}, [stage]);
|
||||
const cleanup = new SetupCleanupOwner([{ close: () => host.close() }]);
|
||||
const failure = await cleanup.close();
|
||||
expect(failure).toBeDefined();
|
||||
expect(projectAnalysisError(failure!).details?.resources).toEqual([
|
||||
temporary,
|
||||
stage,
|
||||
]);
|
||||
allowTemporary = true;
|
||||
expect((await cleanup.close())?.cleanupResources).toEqual([stage]);
|
||||
expect(failure?.cleanupResources).toEqual([stage]);
|
||||
await expect(access(temporary)).rejects.toMatchObject({ code: "ENOENT" });
|
||||
allowStage = true;
|
||||
expect(await cleanup.close()).toBeUndefined();
|
||||
expect(failure?.cleanupIncomplete).toBe(false);
|
||||
expect(temporaryCalls).toBe(2);
|
||||
await expect(access(stage)).rejects.toMatchObject({ code: "ENOENT" });
|
||||
});
|
||||
@@ -89,7 +89,7 @@ const preparedVsCodeSetupHost = async (prefix: string) => {
|
||||
|
||||
/** Release the held read handle and verify host cleanup retries that same owner. */
|
||||
const expectCloseReclaimsHandle = async (
|
||||
host: { close?: () => Promise<string | undefined> },
|
||||
host: { close?: () => Promise<unknown> },
|
||||
closeAttempts: readonly object[],
|
||||
release: () => void,
|
||||
) => {
|
||||
@@ -102,7 +102,7 @@ const expectCloseReclaimsHandle = async (
|
||||
|
||||
/** Give up on the assertion path: release the handle and let cleanup close it. */
|
||||
const cleanupHeldHandle = async (
|
||||
host: { close?: () => Promise<string | undefined> },
|
||||
host: { close?: () => Promise<unknown> },
|
||||
release: () => void,
|
||||
) => {
|
||||
release();
|
||||
@@ -257,6 +257,10 @@ describe("setup client configuration cleanup ownership", () => {
|
||||
(cause: unknown) => cause,
|
||||
);
|
||||
expect(failure).toBeInstanceOf(ConfigurationError);
|
||||
if (!(failure instanceof ConfigurationError))
|
||||
throw new Error(
|
||||
"Expected the configuration failure to retain cleanup authority",
|
||||
);
|
||||
if (encoding === "malformed UTF-8")
|
||||
expect(failure).toMatchObject({
|
||||
cause: {
|
||||
@@ -279,6 +283,8 @@ describe("setup client configuration cleanup ownership", () => {
|
||||
expect(await readFile(client.configPath)).toEqual(original);
|
||||
|
||||
await expectCloseReclaimsHandle(host, closeAttempts, release);
|
||||
expect(failure.cleanupIncomplete).toBe(false);
|
||||
expect(failure.cleanupResources).toEqual([]);
|
||||
} finally {
|
||||
await cleanupHeldHandle(host, release);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,132 @@
|
||||
import { mkdir, rm, stat } from "node:fs/promises";
|
||||
import { join } from "node:path";
|
||||
import { afterEach, expect, it, vi } from "vitest";
|
||||
|
||||
import {
|
||||
installLinuxHopper,
|
||||
systemLinuxHopperInstallHost,
|
||||
} from "../../../src/application/LinuxHopper.js";
|
||||
import { systemSetupHost } from "../../../src/application/SetupHost.js";
|
||||
import { runSetup } from "../../../src/application/Setup.js";
|
||||
import { createDoctorHostFixture } from "../../../src/application/Doctor.fixture.js";
|
||||
import { AnalysisError } from "../../../src/domain/analysisErrorBase.js";
|
||||
import { ConfigurationError } from "../../../src/domain/configurationErrors.js";
|
||||
import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js";
|
||||
import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
|
||||
|
||||
vi.mock("../../../src/application/LinuxHopper.js", async (load) => {
|
||||
const actual =
|
||||
await load<typeof import("../../../src/application/LinuxHopper.js")>();
|
||||
return {
|
||||
...actual,
|
||||
installLinuxHopper: vi.fn(actual.installLinuxHopper),
|
||||
systemLinuxHopperInstallHost: vi.fn(actual.systemLinuxHopperInstallHost),
|
||||
};
|
||||
});
|
||||
|
||||
afterEach(() => vi.resetAllMocks());
|
||||
|
||||
it.each(["settled", "late"] as const)(
|
||||
"retains and releases the actual temporary directory through setup's Linux cleanup task: %s",
|
||||
async (timing) => {
|
||||
const home = await createTestTempDirectory("rea-setup-linux-cleanup-");
|
||||
const temporary = join(home, "owned-installer-staging");
|
||||
await mkdir(temporary);
|
||||
let allowCleanup = false;
|
||||
const actual = await vi.importActual<
|
||||
typeof import("../../../src/application/LinuxHopper.js")
|
||||
>("../../../src/application/LinuxHopper.js");
|
||||
const child = actual.systemLinuxHopperInstallHost(
|
||||
{ HOME: home },
|
||||
async () => undefined,
|
||||
);
|
||||
vi.mocked(systemLinuxHopperInstallHost).mockReturnValue({
|
||||
...child,
|
||||
cleanup: async (path) => {
|
||||
expect(path).toBe(temporary);
|
||||
if (!allowCleanup)
|
||||
throw new Error(
|
||||
"Controlled owned temporary directory cleanup failure",
|
||||
);
|
||||
await rm(path, { recursive: true });
|
||||
},
|
||||
});
|
||||
// Only the installer outcome is simulated; no package/download action runs.
|
||||
let completeInstall: (() => void) | undefined;
|
||||
const installGate = new Promise<void>((resolve) => {
|
||||
completeInstall = resolve;
|
||||
});
|
||||
vi.mocked(installLinuxHopper).mockImplementation(async () => {
|
||||
if (timing === "late") await installGate;
|
||||
return {
|
||||
status: "failed",
|
||||
reason: "cleanup_failed",
|
||||
cleanupFailure: {
|
||||
path: temporary,
|
||||
reason: "Controlled cleanup failure",
|
||||
},
|
||||
};
|
||||
});
|
||||
const closeDoctor = vi.fn(async () => undefined);
|
||||
const host = systemSetupHost(
|
||||
createDoctorHostFixture({
|
||||
platform: "linux",
|
||||
homeDirectory: home,
|
||||
close: closeDoctor,
|
||||
}),
|
||||
{ HOME: home },
|
||||
);
|
||||
try {
|
||||
const installing = host.installHopper(false);
|
||||
const closing = timing === "late" ? host.close?.() : undefined;
|
||||
completeInstall?.();
|
||||
expect(await installing).toMatchObject({ status: "failed" });
|
||||
if (closing !== undefined)
|
||||
expect((await closing)?.cleanupResources).toEqual([temporary]);
|
||||
const primary = new ConfigurationError("Controlled state failure", {
|
||||
settings: [{ setting: "fixture", constraint: "controlled" }],
|
||||
});
|
||||
const failure = await runSetup(
|
||||
{ approved: true, structured: true, installHopper: false },
|
||||
{
|
||||
...host,
|
||||
initialSetupState: async () => {
|
||||
throw primary;
|
||||
},
|
||||
},
|
||||
).then(
|
||||
() => undefined,
|
||||
(cause: unknown) => cause,
|
||||
);
|
||||
expect(failure).toBeInstanceOf(AnalysisError);
|
||||
if (!(failure instanceof AnalysisError))
|
||||
throw new Error("Expected typed cleanup failure");
|
||||
expect(projectAnalysisError(failure)).toMatchObject({
|
||||
code: "cleanup_incomplete",
|
||||
details: { resources: [temporary] },
|
||||
});
|
||||
expect((await stat(temporary)).isDirectory()).toBe(true);
|
||||
allowCleanup = true;
|
||||
expect(await host.close?.()).toBeUndefined();
|
||||
expect(failure.cleanupIncomplete).toBe(false);
|
||||
expect(closeDoctor).toHaveBeenCalledTimes(1);
|
||||
expect(projectAnalysisError(failure)).toMatchObject(
|
||||
projectAnalysisError(primary),
|
||||
);
|
||||
expect(projectAnalysisError(failure).details).not.toHaveProperty(
|
||||
"cleanup",
|
||||
);
|
||||
expect(projectAnalysisError(failure).details).not.toHaveProperty(
|
||||
"resources",
|
||||
);
|
||||
expect(projectAnalysisError(failure).details).toHaveProperty(
|
||||
"cleanup_history",
|
||||
);
|
||||
await expect(stat(temporary)).rejects.toMatchObject({ code: "ENOENT" });
|
||||
} finally {
|
||||
completeInstall?.();
|
||||
allowCleanup = true;
|
||||
await host.close?.();
|
||||
}
|
||||
},
|
||||
);
|
||||
@@ -22,6 +22,10 @@ import {
|
||||
} from "../../../src/application/SetupSkill.js";
|
||||
import { RegularFileCleanupFailure } from "../../../src/filesystem/OwnedRegularFile.js";
|
||||
import { systemSetupHost } from "../../../src/application/SetupHost.js";
|
||||
import { runSetup } from "../../../src/application/Setup.js";
|
||||
import { AnalysisError } from "../../../src/domain/analysisErrorBase.js";
|
||||
import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js";
|
||||
import { ConfigurationError } from "../../../src/domain/configurationErrors.js";
|
||||
import {
|
||||
runUninstall,
|
||||
systemUninstallHost,
|
||||
@@ -68,7 +72,7 @@ const failFirstCloses = (path: string) => {
|
||||
};
|
||||
};
|
||||
|
||||
describe("skill regular-file cleanup ownership", () => {
|
||||
describe("setup skill backup cleanup ownership", () => {
|
||||
it.each(["first", "recovery", "verification"] as const)(
|
||||
"retains a failed %s backup read until setup host disposal",
|
||||
async (stage) => {
|
||||
@@ -106,10 +110,62 @@ describe("skill regular-file cleanup ownership", () => {
|
||||
expect(openMock).toHaveBeenCalledTimes(opensBeforeAdmission);
|
||||
expect(new Set(close.attempts).size).toBe(1);
|
||||
expect(await actualFs.readFile(manifest, "utf8")).toBe(original);
|
||||
expect(await host.close?.()).toContain(backup);
|
||||
const report = await host.close?.();
|
||||
expect(report?.cleanup?.reason).toBe(
|
||||
"test holds the skill handle open",
|
||||
);
|
||||
expect(report?.cleanupResources).toEqual([backup]);
|
||||
const primary = new ConfigurationError(
|
||||
"Controlled setup state failure",
|
||||
{
|
||||
settings: [
|
||||
{ setting: "fixture", constraint: "Controlled primary failure" },
|
||||
],
|
||||
},
|
||||
);
|
||||
const failure = await runSetup(
|
||||
{ approved: true, installHopper: false, structured: true },
|
||||
{
|
||||
...host,
|
||||
initialSetupState: async () => {
|
||||
throw primary;
|
||||
},
|
||||
},
|
||||
).then(
|
||||
() => undefined,
|
||||
(cause: unknown) => cause,
|
||||
);
|
||||
expect(failure).toBeInstanceOf(AnalysisError);
|
||||
if (!(failure instanceof AnalysisError))
|
||||
throw new Error("Expected the typed setup and host cleanup failure");
|
||||
expect(projectAnalysisError(failure)).toMatchObject({
|
||||
code: "cleanup_incomplete",
|
||||
details: {
|
||||
resources: [backup],
|
||||
diagnostics: {
|
||||
cleanup_error: { details: { resources: [backup] } },
|
||||
},
|
||||
},
|
||||
});
|
||||
expect(failure.diagnostics?.primary_error).toEqual(
|
||||
projectAnalysisError(primary),
|
||||
);
|
||||
const attemptsBeforeClose = close.attempts.length;
|
||||
close.release();
|
||||
expect(await host.close?.()).toBeUndefined();
|
||||
expect(failure.cleanupIncomplete).toBe(false);
|
||||
expect(projectAnalysisError(failure)).toMatchObject(
|
||||
projectAnalysisError(primary),
|
||||
);
|
||||
expect(projectAnalysisError(failure).details).not.toHaveProperty(
|
||||
"cleanup",
|
||||
);
|
||||
expect(projectAnalysisError(failure).details).not.toHaveProperty(
|
||||
"resources",
|
||||
);
|
||||
expect(projectAnalysisError(failure).details).toHaveProperty(
|
||||
"cleanup_history",
|
||||
);
|
||||
expect(close.attempts.length).toBeGreaterThan(attemptsBeforeClose);
|
||||
expect(close.attempts[0]?.fd).toBe(-1);
|
||||
} finally {
|
||||
@@ -120,7 +176,9 @@ describe("skill regular-file cleanup ownership", () => {
|
||||
}
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
describe("skill regular-file cleanup ownership", () => {
|
||||
it("retains a failed setup skill read until setup host disposal", async () => {
|
||||
const home = await createTestTempDirectory("rea-setup-skill-close-");
|
||||
const [destination] = skillDestinations(home, [], {}, "linux");
|
||||
|
||||
@@ -151,6 +151,7 @@ import { RegularFileCleanupFailure } from "../../src/filesystem/OwnedRegularFile
|
||||
import { OwnedFileHandle } from "../../src/filesystem/OwnedFileHandle.js";
|
||||
import type { DoctorHost } from "../../src/application/Doctor.js";
|
||||
import { systemSetupHost } from "../../src/application/SetupHost.js";
|
||||
import { AnalysisError } from "../../src/domain/analysisErrorBase.js";
|
||||
import { systemUninstallHost } from "../../src/application/Uninstall.js";
|
||||
import { ConfigurationError } from "../../src/domain/configurationErrors.js";
|
||||
import { createTestTempDirectory } from "../fixtures/temporaryDirectory.js";
|
||||
@@ -1315,7 +1316,16 @@ it.each(["setup", "uninstall"] as const)(
|
||||
).status,
|
||||
).toBe("failed");
|
||||
}
|
||||
expect(await host.close?.()).toContain("cleanup");
|
||||
const cleanupFailure = await host.close?.();
|
||||
if (operation === "setup") {
|
||||
expect(cleanupFailure).toBeInstanceOf(AnalysisError);
|
||||
const retainedRoots = (await fs.readdir(root))
|
||||
.filter((name) => name.startsWith(".rea-config-"))
|
||||
.map((name) => join(root, name));
|
||||
expect((cleanupFailure as AnalysisError).cleanupResources).toEqual(
|
||||
retainedRoots,
|
||||
);
|
||||
} else expect(cleanupFailure).toContain("cleanup");
|
||||
expect(
|
||||
(await fs.readdir(root)).filter((name) =>
|
||||
name.startsWith(".rea-config-"),
|
||||
@@ -1323,6 +1333,8 @@ it.each(["setup", "uninstall"] as const)(
|
||||
).toHaveLength(1);
|
||||
state.rejectStageRemoval = false;
|
||||
expect(await host.close?.()).toBeUndefined();
|
||||
if (cleanupFailure instanceof AnalysisError)
|
||||
expect(cleanupFailure.cleanupIncomplete).toBe(false);
|
||||
expect(
|
||||
(await fs.readdir(root)).filter((name) =>
|
||||
name.startsWith(".rea-config-"),
|
||||
|
||||
Reference in new issue
Block a user