fix(artifacts): bind inventory and extraction cleanup to exact owners
Preserve typed failed attempts through root admission, hashing, inventory, nested JavaScript readers and extraction rollback. Remove the intermediate cause/resource records that discarded canonical cleanup capabilities. Resolve original provider errors after confirmed release while preserving primary failures, partial observations and invalidated-handle uncertainty. Fixes #4928
This commit is contained in:
1 parent
2582c267af
commit
bbfe6edd60
7 files changed
+38
-57
No files matched your search
@@ -1,3 +1,4 @@
|
||||
import type { ArtifactCleanupAttempt } from "../ArtifactResourceScope.js";
|
||||
import { lstat, realpath } from "node:fs/promises";
|
||||
|
||||
import { compareUnicodeCodePoints } from "../../domain/unicodeCodePointOrder.js";
|
||||
@@ -245,13 +246,9 @@ const materializeSelection = async ({
|
||||
localReaderOwner = undefined;
|
||||
const closeAttempt = await input.resourceScope.release(readerOwner);
|
||||
if (closeAttempt.kind === "failed")
|
||||
throw ArtifactReaderFailure.withCleanup(
|
||||
closeAttempt.cause,
|
||||
ArtifactReaderFailure.cleanupObservation(
|
||||
closeAttempt.cause,
|
||||
readerOwner.resource,
|
||||
),
|
||||
);
|
||||
throw input.resourceScope.cleanupFailure({ kind: "completed" }, [
|
||||
closeAttempt,
|
||||
]);
|
||||
extracted.sort((left, right) =>
|
||||
compareUnicodeCodePoints(left.relative_path, right.relative_path),
|
||||
);
|
||||
@@ -265,19 +262,11 @@ const materializeSelection = async ({
|
||||
output,
|
||||
input.outputRoot,
|
||||
);
|
||||
if (cleanupFailures.length > 0) {
|
||||
const observations = cleanupFailures.map(
|
||||
({ cause: cleanupCause, resource }) =>
|
||||
ArtifactReaderFailure.cleanupObservation(cleanupCause, resource),
|
||||
);
|
||||
throw ArtifactReaderFailure.withCleanup(
|
||||
cause,
|
||||
{
|
||||
reason: observations.map(({ reason }) => reason).join("; "),
|
||||
resources: [
|
||||
...new Set(observations.flatMap(({ resources }) => resources)),
|
||||
],
|
||||
},
|
||||
const [firstCleanup, ...remainingCleanup] = cleanupFailures;
|
||||
if (firstCleanup !== undefined) {
|
||||
throw input.resourceScope.cleanupFailure(
|
||||
{ kind: "failed", cause },
|
||||
[firstCleanup, ...remainingCleanup],
|
||||
output?.published === true && completed !== undefined
|
||||
? { kind: "artifact-extraction", extraction: completed }
|
||||
: undefined,
|
||||
@@ -346,12 +335,14 @@ const releaseExtractionOwners = async (
|
||||
readerOwner: ArtifactResourceOwner | undefined,
|
||||
output: SafeOutputTree | undefined,
|
||||
outputRoot: string,
|
||||
): Promise<{ readonly cause: unknown; readonly resource: string }[]> => {
|
||||
const failures: { readonly cause: unknown; readonly resource: string }[] = [];
|
||||
): Promise<Extract<ArtifactCleanupAttempt, { readonly kind: "failed" }>[]> => {
|
||||
const failures: Extract<
|
||||
ArtifactCleanupAttempt,
|
||||
{ readonly kind: "failed" }
|
||||
>[] = [];
|
||||
if (readerOwner !== undefined) {
|
||||
const attempt = await resourceScope.release(readerOwner);
|
||||
if (attempt.kind === "failed")
|
||||
failures.push({ cause: attempt.cause, resource: readerOwner.resource });
|
||||
if (attempt.kind === "failed") failures.push(attempt);
|
||||
}
|
||||
if (output !== undefined) {
|
||||
const owner: ArtifactResourceOwner = {
|
||||
@@ -360,8 +351,7 @@ const releaseExtractionOwners = async (
|
||||
resource: outputRoot,
|
||||
};
|
||||
const attempt = await resourceScope.release(owner);
|
||||
if (attempt.kind === "failed")
|
||||
failures.push({ cause: attempt.cause, resource: owner.resource });
|
||||
if (attempt.kind === "failed") failures.push(attempt);
|
||||
}
|
||||
return failures;
|
||||
};
|
||||
|
||||
@@ -66,13 +66,9 @@ export const classifyAndHashRoot = async (
|
||||
};
|
||||
const cleanupAttempt = await options.resourceScope.release(owner);
|
||||
if (cleanupAttempt.kind === "failed")
|
||||
throw ArtifactReaderFailure.withCleanup(
|
||||
cleanupAttempt.cause,
|
||||
ArtifactReaderFailure.cleanupObservation(
|
||||
cleanupAttempt.cause,
|
||||
owner.resource,
|
||||
),
|
||||
);
|
||||
throw options.resourceScope.cleanupFailure({ kind: "completed" }, [
|
||||
cleanupAttempt,
|
||||
]);
|
||||
}
|
||||
return { format: result.format, digest: result.digest };
|
||||
});
|
||||
@@ -136,13 +132,7 @@ export const classifyAndHashRootForInventory = async (
|
||||
if (outcome.kind === "failed" || outcome.value.rootSource === undefined) {
|
||||
const cleanupAttempt = await options.resourceScope.release(owner);
|
||||
if (cleanupAttempt.kind === "failed")
|
||||
throw ArtifactReaderFailure.withCleanup(
|
||||
outcome.kind === "failed" ? outcome.cause : cleanupAttempt.cause,
|
||||
ArtifactReaderFailure.cleanupObservation(
|
||||
cleanupAttempt.cause,
|
||||
owner.resource,
|
||||
),
|
||||
);
|
||||
throw options.resourceScope.cleanupFailure(outcome, [cleanupAttempt]);
|
||||
}
|
||||
if (outcome.kind === "failed") throw outcome.cause;
|
||||
return outcome.value;
|
||||
|
||||
@@ -55,10 +55,9 @@ export const openRootArtifact = async (
|
||||
handle: cause.owner,
|
||||
});
|
||||
if (cleanup.kind === "failed")
|
||||
throw ArtifactReaderFailure.withCleanup(
|
||||
failure,
|
||||
ArtifactReaderFailure.cleanupObservation(cleanup.cause, resource),
|
||||
);
|
||||
throw resourceScope.cleanupFailure({ kind: "failed", cause: failure }, [
|
||||
cleanup,
|
||||
]);
|
||||
}
|
||||
throw failure;
|
||||
}
|
||||
@@ -91,10 +90,7 @@ export const hashStableRootArtifact = async (
|
||||
handle: owner,
|
||||
});
|
||||
if (cleanup.kind === "failed")
|
||||
throw ArtifactReaderFailure.withCleanup(
|
||||
outcome.kind === "failed" ? outcome.cause : cleanup.cause,
|
||||
ArtifactReaderFailure.cleanupObservation(cleanup.cause, resource),
|
||||
);
|
||||
throw resourceScope.cleanupFailure(outcome, [cleanup]);
|
||||
if (outcome.kind === "failed") throw outcome.cause;
|
||||
return outcome.value;
|
||||
};
|
||||
|
||||
@@ -173,12 +173,11 @@ const cleanupInventoryOwners = async ({
|
||||
for (const owner of owners) {
|
||||
const attempt = await options.resourceScope.release(owner);
|
||||
if (attempt.kind === "released") continue;
|
||||
const primary =
|
||||
cleanupFailure ??
|
||||
(outcome.kind === "failed" ? outcome.cause : attempt.cause);
|
||||
cleanupFailure = ArtifactReaderFailure.withCleanup(
|
||||
primary,
|
||||
ArtifactReaderFailure.cleanupObservation(attempt.cause, owner.resource),
|
||||
cleanupFailure = options.resourceScope.cleanupFailure(
|
||||
cleanupFailure === undefined
|
||||
? outcome
|
||||
: { kind: "failed", cause: cleanupFailure },
|
||||
[attempt],
|
||||
outcome.kind === "completed"
|
||||
? { kind: "artifact-inventory", inventory: outcome.snapshot }
|
||||
: undefined,
|
||||
|
||||
@@ -306,9 +306,11 @@ const visitReader = async (
|
||||
});
|
||||
if (cleanup.kind === "failed") {
|
||||
failure = {
|
||||
cause: ArtifactReaderFailure.withCleanup(
|
||||
failure === undefined ? cleanup.cause : failure.cause,
|
||||
ArtifactReaderFailure.cleanupObservation(cleanup.cause, resource),
|
||||
cause: context.resources.cleanupFailure(
|
||||
failure === undefined
|
||||
? { kind: "completed" }
|
||||
: { kind: "failed", cause: failure.cause },
|
||||
[cleanup],
|
||||
),
|
||||
};
|
||||
}
|
||||
|
||||
@@ -87,6 +87,8 @@ it.each([false, true])(
|
||||
invalidated ? attemptsBeforeRecovery : attemptsBeforeRecovery + 1,
|
||||
);
|
||||
expect(descriptor?.fd).toBe(-1);
|
||||
if (!invalidated && !inventory.ok)
|
||||
expect(inventory.error.cleanupIncomplete).toBe(false);
|
||||
if (invalidated) expect(closeAttempts).toBe(1);
|
||||
},
|
||||
);
|
||||
@@ -97,6 +97,8 @@ it("retains a failed reader close, blocks new work, and retries the same reader"
|
||||
expect(await client.close()).toMatchObject({ ok: true });
|
||||
expect(closeOwners).toHaveLength(5);
|
||||
expect(closeOwners[4]).toBe(closeOwners[1]);
|
||||
if (!extraction.ok) expect(extraction.error.cleanupIncomplete).toBe(false);
|
||||
expect(incompleteClose.error.cleanupIncomplete).toBe(false);
|
||||
const closed = await client.execute("inventory_artifact", {});
|
||||
expect(closed).toMatchObject({
|
||||
ok: false,
|
||||
|
||||
Reference in new issue
Block a user