fix(artifacts): carry exact cleanup authority through scope failures

Return the canonical owner and lease from failed releases. Compose initial
cleanup failures from explicit operation outcomes and failed attempts, and
carry their authority through the shared artifact failure contract.

Keep independent unknown cleanup unresolved. Distinguish operation failures
from completed work whose close rejected. Let the scope own current child
resource labels without replacing its canonical retry lease.

Cover mixed owners, partial release, cancellation provenance, typed close
rejections and independent unknown cleanup through the producing boundaries.
This commit is contained in:
morluto committed 2026-10-18 08:44:41 +08:00
1 parent 2183c9598c
commit 2582c267af
7 files changed
+433 -55

No files matched your search

+11 -17
View File
@@ -7,11 +7,13 @@ import {
RegularFileCancelled,
RegularFileCleanupFailure,
} from "../filesystem/OwnedRegularFile.js";
import { ArtifactReaderFailure } from "./ArtifactReader.js";
import {
ArtifactResourceRetryFailure,
type ArtifactResourceOwner,
type ArtifactResourceScope,
ArtifactReaderFailure,
artifactPrimaryCleanupLeases,
} from "./ArtifactReader.js";
import type {
ArtifactResourceOwner,
ArtifactResourceScope,
} from "./ArtifactResourceScope.js";
import {
AnalysisCleanupLease,
@@ -23,7 +25,7 @@ import type { AnalysisError } from "../domain/analysisErrorBase.js";
export class ArtifactFileCleanupFailure<
Value = unknown,
> extends ArtifactReaderFailure {
readonly cleanupLeases: readonly AnalysisCleanupLease[] | undefined;
override readonly cleanupLeases: readonly AnalysisCleanupLease[] | undefined;
readonly fileCleanupFailure: RegularFileCleanupFailure<Value>;
constructor(
fileCleanupFailure: RegularFileCleanupFailure<Value>,
@@ -78,13 +80,9 @@ export class ArtifactFileCleanupFailure<
);
this.fileCleanupFailure = fileCleanupFailure;
const inherited =
primary instanceof ArtifactFileCleanupFailure ||
primary instanceof ArtifactResourceRetryFailure
? primary.cleanupLeases
: primary instanceof ArtifactReaderFailure &&
primary.cleanup !== undefined
? undefined
: [];
fileCleanupFailure.outcome.kind === "failed"
? artifactPrimaryCleanupLeases(fileCleanupFailure.outcome.cause)
: [];
this.cleanupLeases =
inherited === undefined
? undefined
@@ -150,11 +148,7 @@ export const bindArtifactFileCleanup = (
error: AnalysisError,
): void => {
const leases =
cause instanceof ArtifactFileCleanupFailure
? cause.cleanupLeases
: cause instanceof ArtifactResourceRetryFailure
? cause.cleanupLeases
: undefined;
cause instanceof ArtifactReaderFailure ? cause.cleanupLeases : undefined;
if (leases !== undefined) new AnalysisCleanupLeaseGroup(error, leases);
};
+43 -2
View File
@@ -1,3 +1,5 @@
import { AnalysisError } from "../domain/analysisErrorBase.js";
import type { AnalysisCleanupLease } from "./AnalysisCleanupLease.js";
import type { Readable } from "node:stream";
import type {
ArtifactCommand,
@@ -15,6 +17,8 @@ export type ArtifactReaderBoundary = "file-size" | "capacity" | "memory";
export interface ArtifactReaderFailureOptions extends ErrorOptions {
readonly cancellation?: { readonly reason: unknown } | undefined;
readonly cleanup?: AnalysisCleanupObservation | undefined;
/** Exact authority for all reported cleanup; undefined leaves ownership unknown. */
readonly cleanupLeases?: readonly AnalysisCleanupLease[] | undefined;
readonly partialObservation?: AnalysisPartialObservation | undefined;
/** The constrained resource bound for a limit failure. */
readonly boundary?: ArtifactReaderBoundary;
@@ -138,6 +142,7 @@ export interface ArtifactReader {
export class ArtifactReaderFailure extends Error {
readonly cancellation: { readonly reason: unknown } | undefined;
readonly cleanup: AnalysisCleanupObservation | undefined;
readonly cleanupLeases: readonly AnalysisCleanupLease[] | undefined;
readonly partialObservation: AnalysisPartialObservation | undefined;
/** The constrained bound for a `limit` reason, when the reader knows it. */
readonly boundary: ArtifactReaderBoundary | undefined;
@@ -167,20 +172,38 @@ export class ArtifactReaderFailure extends Error {
? (options?.cancellation ?? { reason: options?.cause })
: undefined;
this.cleanup = options?.cleanup;
this.cleanupLeases =
options?.cleanupLeases ?? (this.cleanup === undefined ? [] : undefined);
this.partialObservation = options?.partialObservation;
this.boundary = options?.boundary;
}
/** Preserve the primary failure while attaching cleanup and any complete observation. */
/** Preserve the primary failure and observation. Supplied leases must cover all merged cleanup. */
static withCleanup(
cause: unknown,
cleanup: AnalysisCleanupObservation,
partialObservation?: AnalysisPartialObservation,
cleanupLeases?: readonly AnalysisCleanupLease[],
): ArtifactReaderFailure {
const primary =
cause instanceof ArtifactReaderFailure
? cause
: new ArtifactReaderFailure("io", errorMessage(cause), { cause });
: new ArtifactReaderFailure("io", errorMessage(cause), {
cause,
...(cause instanceof AnalysisError
? {
partialObservation: cause.partialObservation,
...(cause.cleanupIncomplete
? {
cleanup: ArtifactReaderFailure.cleanupObservation(
cause,
[],
),
}
: {}),
}
: {}),
});
const mergedObservation = partialObservation ?? primary.partialObservation;
return new ArtifactReaderFailure(
primary.reason,
@@ -189,6 +212,9 @@ export class ArtifactReaderFailure extends Error {
cause: primary,
cancellation: primary.cancellation,
cleanup: combineCleanup(primary.cleanup, cleanup),
cleanupLeases:
cleanupLeases ??
(cleanup === primary.cleanup ? primary.cleanupLeases : undefined),
// The constrained bound survives cleanup wrapping, or a file-size
// refusal would be reported as a memory constraint once a close fails.
...(primary.boundary === undefined
@@ -209,6 +235,11 @@ export class ArtifactReaderFailure extends Error {
): AnalysisCleanupObservation {
if (cause instanceof ArtifactReaderFailure && cause.cleanup !== undefined)
return cause.cleanup;
if (cause instanceof AnalysisError && cause.cleanupIncomplete)
return {
reason: cause.cleanup?.reason ?? cause.message,
resources: [...cause.cleanupResources],
};
return {
reason: errorMessage(cause),
resources: typeof resource === "string" ? [resource] : [...resource],
@@ -229,3 +260,13 @@ const combineCleanup = (
const errorMessage = (cause: unknown): string =>
cause instanceof Error ? cause.message : String(cause);
/** Read explicit source authority; unrelated domain cleanup remains unresolved. */
export const artifactPrimaryCleanupLeases = (
cause: unknown,
): readonly AnalysisCleanupLease[] | undefined =>
cause instanceof ArtifactReaderFailure
? cause.cleanupLeases
: cause instanceof AnalysisError && cause.cleanupIncomplete
? undefined
: [];
+280 -3
View File
@@ -1,11 +1,21 @@
import { bindArtifactFileCleanup } from "./ArtifactFileOperation.js";
import { open, writeFile } from "node:fs/promises";
import { join } from "node:path";
import { createTestTempDirectory } from "../../tests/fixtures/temporaryDirectory.js";
import {
withArtifactFileHandle,
bindArtifactFileCleanup,
} from "./ArtifactFileOperation.js";
import { ArtifactResourceRetryFailure } from "./ArtifactResourceScope.js";
import { ProviderCleanupError } from "../domain/providerCleanupError.js";
import { expect, it } from "vitest";
import { err } from "../domain/result.js";
import { AnalysisOutputError } from "../domain/analysisErrorCore.js";
import type { ArtifactReader } from "./ArtifactReader.js";
import {
ArtifactReaderFailure,
type ArtifactReader,
} from "./ArtifactReader.js";
import { translateArtifactFailure } from "./ArtifactProvider.js";
import { ArtifactResourceScope } from "./ArtifactResourceScope.js";
const deferred = <T>() => {
@@ -138,9 +148,10 @@ it("retains a failed source analyzer until shutdown can verify its release", asy
},
},
};
expect(await scope.release(owner)).toEqual({
expect(await scope.release(owner)).toMatchObject({
kind: "failed",
cause: failure,
owner,
});
let admitted = false;
await expect(
@@ -195,3 +206,269 @@ it("retains a canonical retry lease for a plain cleanup capability", async () =>
await scope.close();
}
});
it("binds initial mixed cleanup to only its unresolved owners", async () => {
const scope = new ArtifactResourceScope();
let denyFirst = true,
denySecond = true;
const first = {
kind: "lease" as const,
resource: "first",
lease: {
async release() {
if (denyFirst) throw new Error("first denied");
},
},
};
const second = {
kind: "lease" as const,
resource: "second",
lease: {
async release() {
if (denySecond) throw new Error("second denied");
},
},
};
try {
const a = await scope.release(first),
b = await scope.release(second);
if (a.kind !== "failed" || b.kind !== "failed")
throw new Error("Expected owned cleanup failures");
const initial = scope.cleanupFailure({ kind: "completed" }, [a, b]);
expect(initial.cleanupLeases).toEqual([a.lease, b.lease]);
const error = translateArtifactFailure("inventory_artifact", initial);
denyFirst = false;
const retry = await scope.retryCleanup().catch((cause: unknown) => cause);
expect(retry).toBeInstanceOf(ArtifactResourceRetryFailure);
if (!(retry instanceof ArtifactResourceRetryFailure))
throw new Error("Expected failed second owner");
expect(retry.cleanupLeases).toEqual([b.lease]);
expect(error.cleanupIncomplete).toBe(true);
expect(error.cleanupResources).toEqual(["second"]);
denySecond = false;
await scope.retryCleanup();
expect(error.cleanupIncomplete).toBe(false);
expect(error.cleanupResources).toEqual([]);
} finally {
denyFirst = false;
denySecond = false;
await scope.close();
}
});
it.each(["owned-close", "independent-primary"] as const)(
"keeps the scope of unknown cleanup explicit: %s",
async (kind) => {
const scope = new ArtifactResourceScope();
let denied = true;
const unknown = new ArtifactReaderFailure("io", "unknown cleanup", {
cleanup: { resources: ["unverified"], reason: "no capability" },
});
const owner = {
kind: "lease" as const,
resource: "known owner",
lease: {
async release() {
if (denied) throw unknown;
},
},
};
try {
const attempt = await scope.release(owner);
if (attempt.kind !== "failed") throw new Error("Expected owned failure");
const outcome =
kind === "owned-close"
? { kind: "completed" as const }
: { kind: "failed" as const, cause: unknown };
const initial = scope.cleanupFailure(outcome, [attempt]);
const error = translateArtifactFailure("inventory_artifact", initial);
expect(error.cleanupIncomplete).toBe(true);
denied = false;
await scope.close();
expect(attempt.lease.released).toBe(true);
expect(error.cleanupIncomplete).toBe(kind === "independent-primary");
expect(initial.cleanupLeases === undefined).toBe(
kind === "independent-primary",
);
} finally {
denied = false;
await scope.close();
}
},
);
it.each([undefined, null, { reason: "opaque reason" }])(
"retains cancellation provenance through initial cleanup and recovery: %j",
async (reason) => {
const scope = new ArtifactResourceScope();
let denied = true;
const owner = {
kind: "lease" as const,
resource: "cancelled reader",
lease: {
async release() {
if (denied) throw new Error("denied");
},
},
};
try {
const attempt = await scope.release(owner);
if (attempt.kind !== "failed") throw new Error("Expected owned failure");
const primary = new ArtifactReaderFailure(
"cancelled",
"primary cancelled",
{ cancellation: { reason } },
);
const initial = scope.cleanupFailure({ kind: "failed", cause: primary }, [
attempt,
]);
expect(initial.reason).toBe("cancelled");
expect(initial.cancellation?.reason).toBe(reason);
const error = translateArtifactFailure("inventory_artifact", initial);
expect(error.cause).toBe(reason);
denied = false;
await scope.close();
expect(error.cleanupIncomplete).toBe(false);
expect(error.cause).toBe(reason);
} finally {
denied = false;
await scope.close();
}
},
);
it("does not clear independent domain cleanup when a new scope owner releases", async () => {
const scope = new ArtifactResourceScope();
let denied = true;
const primary = new ProviderCleanupError("independent", ["other resource"], {
reason: "unknown authority",
});
const owner = {
kind: "lease" as const,
resource: "new owner",
lease: {
async release() {
if (denied) throw new Error("denied");
},
},
};
try {
const attempt = await scope.release(owner);
if (attempt.kind !== "failed") throw new Error("Expected owned failure");
const initial = scope.cleanupFailure({ kind: "failed", cause: primary }, [
attempt,
]);
const error = translateArtifactFailure("inventory_artifact", initial);
denied = false;
await scope.close();
expect(attempt.lease.released).toBe(true);
expect(primary.cleanupIncomplete).toBe(true);
expect(error.cleanupIncomplete).toBe(true);
expect(initial.cleanupLeases).toBeUndefined();
expect(initial.cleanup?.resources).toContain("other resource");
} finally {
denied = false;
await scope.close();
}
});
it.each(["completed", "failed"] as const)(
"preserves physical file ownership separately from a typed close rejection after %s work",
async (outcome) => {
const directory = await createTestTempDirectory(
"rea-scope-file-authority-",
);
const path = join(directory, "payload");
await writeFile(path, "selected bytes");
const file = await open(path, "r");
const close = file.close.bind(file);
const scope = new ArtifactResourceScope();
let denied = true;
const primary = new ProviderCleanupError(
"independent",
["independent resource"],
{ reason: "unverified primary" },
);
const closeCause = new ProviderCleanupError("file-close", [path], {
reason: "close denied",
});
file.close = async () => {
if (denied) throw closeCause;
await close();
};
try {
const failure = await withArtifactFileHandle(
path,
file,
scope,
undefined,
async () => {
if (outcome === "failed") throw primary;
return 42;
},
).catch((cause: unknown) => cause);
if (!(failure instanceof ArtifactReaderFailure))
throw new Error("Expected physical file failure");
const error = translateArtifactFailure("inventory_artifact", failure);
expect(error.cleanupIncomplete).toBe(true);
denied = false;
await scope.close();
expect(file.fd).toBe(-1);
expect(error.cleanupIncomplete).toBe(outcome === "failed");
expect(failure.cleanupLeases === undefined).toBe(outcome === "failed");
} finally {
denied = false;
await scope.close();
await close();
}
},
);
it.each(["artifact", "domain"] as const)(
"keeps initial %s composite labels current when a later close retains only one child",
async (kind) => {
const scope = new ArtifactResourceScope();
let children = ["first child", "second child"];
const owner = {
kind: "lease" as const,
resource: "composite owner",
lease: {
async release() {
if (children.length > 0) {
if (kind === "domain")
throw new ProviderCleanupError("composite", [...children], {
reason: "held children",
});
throw new ArtifactReaderFailure("io", "child close denied", {
cleanup: { reason: "held children", resources: [...children] },
});
}
},
},
};
try {
const attempt = await scope.release(owner);
if (attempt.kind !== "failed")
throw new Error("Expected retained children");
const error = translateArtifactFailure(
"inventory_artifact",
scope.cleanupFailure({ kind: "completed" }, [attempt]),
);
expect(error.cleanupResources).toEqual(children);
children = ["second child"];
await expect(scope.retryCleanup()).rejects.toMatchObject({
cleanupLeases: [attempt.lease],
});
expect(error.cleanupIncomplete).toBe(true);
expect(error.cleanupResources).toEqual(children);
expect(scope.cleanupResources).toEqual(children);
children = [];
await scope.close();
expect(error.cleanupIncomplete).toBe(false);
expect(error.cleanupResources).toEqual([]);
} finally {
children = [];
await scope.close();
}
},
);
+86 -18
View File
@@ -1,10 +1,14 @@
import type { AnalysisPartialObservation } from "../domain/analysisErrorBase.js";
import type { JavaScriptSourceAnalysisPort } from "../domain/javascript/javascriptSourceAnalysis.js";
import type { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js";
import type { OwnedDirectoryHandle } from "../filesystem/OwnedDirectoryHandle.js";
import type { OwnedStagedOutput } from "../filesystem/OwnedStagedOutput.js";
import type { ArtifactReader } from "./ArtifactReader.js";
import { ArtifactReaderFailure } from "./ArtifactReader.js";
import {
ArtifactReaderFailure,
artifactPrimaryCleanupLeases,
} from "./ArtifactReader.js";
import type { SafeOutputTree } from "./SafeOutputTree.js";
import type { PrivateRuntimeRoot } from "../process/PrivateRuntimeRoot.js";
import { AnalysisCleanupLease } from "./AnalysisCleanupLease.js";
@@ -54,12 +58,17 @@ export type ArtifactResourceOwner =
export type ArtifactCleanupAttempt =
| { readonly kind: "released" }
| { readonly kind: "failed"; readonly cause: unknown };
| {
readonly kind: "failed";
readonly cause: unknown;
readonly owner: ArtifactResourceOwner;
readonly lease: AnalysisCleanupLease;
};
/** Retry failure carrying the exact retained owners that did not release. */
export class ArtifactResourceRetryFailure extends ArtifactReaderFailure {
readonly failedOwners: readonly ArtifactResourceOwner[];
readonly cleanupLeases: readonly AnalysisCleanupLease[];
override readonly cleanupLeases: readonly AnalysisCleanupLease[];
constructor(
failures: readonly {
readonly owner: ArtifactResourceOwner;
@@ -77,7 +86,13 @@ export class ArtifactResourceRetryFailure extends ArtifactReaderFailure {
/** Caller-owned scope that retains failed artifact cleanup for a later close. */
export class ArtifactResourceScope {
readonly #pending = new Set<ArtifactResourceOwner>();
readonly #leases = new WeakMap<ArtifactResourceOwner, AnalysisCleanupLease>();
readonly #leases = new WeakMap<
ArtifactResourceOwner,
{
readonly lease: AnalysisCleanupLease;
resources: readonly string[];
}
>();
readonly #active = new Set<Promise<unknown>>();
#draining: Promise<void> | undefined;
#closed = false;
@@ -90,13 +105,12 @@ export class ArtifactResourceScope {
get cleanupResources(): readonly string[] {
return [
...new Set(
[...this.#pending].flatMap((owner) =>
owner.kind === "lease" && owner.lease instanceof AnalysisCleanupLease
? owner.lease.released
? []
: (owner.lease.cleanupResources ?? [owner.resource])
: [owner.resource],
),
[...this.#pending].flatMap((owner) => {
const lease = this.#lease(owner);
return lease.released
? []
: (lease.cleanupResources ?? [owner.resource]);
}),
),
];
}
@@ -137,10 +151,44 @@ export class ArtifactResourceScope {
return { kind: "released" };
} catch (cause: unknown) {
this.#pending.add(owner);
return { kind: "failed", cause };
this.#observeCleanupFailure(owner, cause);
return { kind: "failed", cause, owner, lease: this.#lease(owner) };
}
}
/** Compose initial cleanup authority without confusing it with an independent primary. */
cleanupFailure(
outcome:
| { readonly kind: "completed" }
| { readonly kind: "failed"; readonly cause: unknown },
failures: readonly [
Extract<ArtifactCleanupAttempt, { readonly kind: "failed" }>,
...Extract<ArtifactCleanupAttempt, { readonly kind: "failed" }>[],
],
partialObservation?: AnalysisPartialObservation,
): ArtifactReaderFailure {
const observations = failures.map(({ cause, owner }) =>
ArtifactReaderFailure.cleanupObservation(cause, owner.resource),
);
const primary =
outcome.kind === "failed" ? outcome.cause : failures[0].cause;
const inherited =
outcome.kind === "failed" ? artifactPrimaryCleanupLeases(primary) : [];
return ArtifactReaderFailure.withCleanup(
primary,
{
reason: observations.map(({ reason }) => reason).join("; "),
resources: [
...new Set(observations.flatMap(({ resources }) => resources)),
],
},
partialObservation,
inherited === undefined
? undefined
: [...new Set([...inherited, ...failures.map(({ lease }) => lease)])],
);
}
/** Adopt an owner after another boundary's cleanup attempt failed. */
retain(owner: ArtifactResourceOwner): void {
this.#pending.add(owner);
@@ -172,6 +220,7 @@ export class ArtifactResourceScope {
await this.#lease(owner).release();
this.#pending.delete(owner);
} catch (cause: unknown) {
this.#observeCleanupFailure(owner, cause);
failures.push({ owner, cause });
}
}
@@ -203,15 +252,34 @@ export class ArtifactResourceScope {
);
}
/** This boundary owns the latest parsed child cleanup facts for a composite owner. */
#observeCleanupFailure(owner: ArtifactResourceOwner, cause: unknown): void {
const retained = this.#leases.get(owner);
if (retained !== undefined)
retained.resources = [
...ArtifactReaderFailure.cleanupObservation(cause, owner.resource)
.resources,
];
}
#lease(owner: ArtifactResourceOwner): AnalysisCleanupLease {
if (owner.kind === "lease" && owner.lease instanceof AnalysisCleanupLease)
return owner.lease;
let lease = this.#leases.get(owner);
if (lease === undefined) {
lease = new AnalysisCleanupLease(() => cleanup(owner), [owner.resource]);
this.#leases.set(owner, lease);
}
return lease;
const previous = this.#leases.get(owner);
if (previous !== undefined) return previous.lease;
const retained: {
readonly lease: AnalysisCleanupLease;
resources: readonly string[];
} = {
resources: [owner.resource],
lease: new AnalysisCleanupLease(() => cleanup(owner), {
get cleanupResources() {
return retained.resources;
},
}),
};
this.#leases.set(owner, retained);
return retained.lease;
}
}
+1 -5
View File
@@ -1,5 +1,4 @@
import type { AnalysisCleanupLease } from "../artifacts/AnalysisCleanupLease.js";
import { ArtifactResourceRetryFailure } from "../artifacts/ArtifactResourceScope.js";
import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js";
import {
ArtifactFileCleanupFailure,
@@ -44,10 +43,7 @@ export const firmwareFailure = (
{ operation, cause },
);
bindArtifactFileCleanup(cause, cleanup);
if (
!(cause instanceof ArtifactFileCleanupFailure) &&
!(cause instanceof ArtifactResourceRetryFailure)
)
if (cause.cleanupLeases === undefined)
treeCleanupLease?.bindCleanupError(cleanup);
if (cause instanceof ArtifactFileCleanupFailure) {
const outcome = cause.fileCleanupFailure.outcome;
+2 -9
View File
@@ -7,10 +7,7 @@ import {
artifactFileCancellationCause,
} from "../artifacts/ArtifactFileOperation.js";
import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js";
import {
ArtifactResourceScope,
ArtifactResourceRetryFailure,
} from "../artifacts/ArtifactResourceScope.js";
import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js";
import { readStableArtifact } from "../artifacts/readStableArtifact.js";
import {
AnalysisError,
@@ -282,11 +279,7 @@ export class ArtifactAnalysisResources {
},
{ operation, cause },
);
let leases =
cause instanceof ArtifactFileCleanupFailure ||
cause instanceof ArtifactResourceRetryFailure
? cause.cleanupLeases
: undefined;
let leases = cause.cleanupLeases;
if (leases === undefined) {
// An unreported capability cannot be recovered by observing an empty scope.
// Retain the unresolved source failure and refuse replacement admission.
@@ -117,11 +117,20 @@ it.each(readers)(
expect(causeChainContains(primary, statFailure)).toBe(true);
const release = await scope.release(owner);
expect(release).toEqual({ kind: "failed", cause: closeFailure });
expect(release).toMatchObject({
kind: "failed",
cause: closeFailure,
owner,
});
if (release.kind !== "failed")
throw new Error("Expected owned cleanup failure");
expect(release.cause).toBe(closeFailure);
expect(release.lease.released).toBe(false);
expect(descriptor?.fd).toBe(-1);
await expect(scope.close()).rejects.toMatchObject({
reason: "unavailable",
cleanup: { reason: closeFailure.message, resources: [path] },
cleanupLeases: [release.lease],
});
// Native close invalidated the fd before rejecting. Retrying the same owner
// must remain failed without another syscall or a false successful release.