mirror of
https://github.com/morluto/rea.git
synced 2026-10-18 20:10:13 -05:00
fix(artifacts): retain canonical cleanup leases for scope owners
Pair each retry owner with its exact lease and reuse that lease for release and recovery. Remove variant-based lease inference. Validation: isolated scope foundation passed 23 selected tests; final candidate passed the broader focused suite and static checks. Real-provider behavior was not qualified.
This commit is contained in:
1 parent
8a247fc70f
commit
daa641aa3c
2 files changed
+64
-14
No files matched your search
@@ -1,3 +1,6 @@
|
||||
import { 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";
|
||||
@@ -154,3 +157,41 @@ it("retains a failed source analyzer until shutdown can verify its release", asy
|
||||
releaseAllowed = true;
|
||||
await scope.close();
|
||||
});
|
||||
|
||||
it("retains a canonical retry lease for a plain cleanup capability", async () => {
|
||||
const scope = new ArtifactResourceScope();
|
||||
let denied = true;
|
||||
let closes = 0;
|
||||
const owner = {
|
||||
kind: "lease",
|
||||
resource: "plain cleanup owner",
|
||||
lease: {
|
||||
release: async () => {
|
||||
closes++;
|
||||
if (denied) throw new Error("close denied");
|
||||
},
|
||||
},
|
||||
} as const;
|
||||
try {
|
||||
expect(await scope.release(owner)).toMatchObject({ kind: "failed" });
|
||||
const retry: unknown = await scope
|
||||
.retryCleanup()
|
||||
.catch((cause: unknown) => cause);
|
||||
if (!(retry instanceof ArtifactResourceRetryFailure))
|
||||
throw new Error("Expected exact failed owners");
|
||||
expect(retry.cleanupLeases).toHaveLength(1);
|
||||
const failure = new ProviderCleanupError("fixture", [owner.resource], {
|
||||
reason: "close denied",
|
||||
});
|
||||
bindArtifactFileCleanup(retry, failure);
|
||||
expect(failure.cleanupIncomplete).toBe(true);
|
||||
denied = false;
|
||||
await scope.close();
|
||||
expect(failure.cleanupIncomplete).toBe(false);
|
||||
expect(failure.cleanupResources).toEqual([]);
|
||||
expect(closes).toBe(3);
|
||||
} finally {
|
||||
denied = false;
|
||||
await scope.close();
|
||||
}
|
||||
});
|
||||
@@ -58,28 +58,26 @@ export type ArtifactCleanupAttempt =
|
||||
|
||||
/** Retry failure carrying the exact retained owners that did not release. */
|
||||
export class ArtifactResourceRetryFailure extends ArtifactReaderFailure {
|
||||
readonly cleanupLeases: readonly AnalysisCleanupLease[] | undefined;
|
||||
readonly failedOwners: readonly ArtifactResourceOwner[];
|
||||
readonly cleanupLeases: readonly AnalysisCleanupLease[];
|
||||
constructor(
|
||||
readonly failedOwners: readonly ArtifactResourceOwner[],
|
||||
failures: readonly {
|
||||
readonly owner: ArtifactResourceOwner;
|
||||
readonly lease: AnalysisCleanupLease;
|
||||
}[],
|
||||
message: string,
|
||||
options: ConstructorParameters<typeof ArtifactReaderFailure>[2],
|
||||
) {
|
||||
super("unavailable", message, options);
|
||||
const leases = failedOwners.flatMap((owner) =>
|
||||
owner.kind === "lease" && owner.lease instanceof AnalysisCleanupLease
|
||||
? [owner.lease]
|
||||
: [],
|
||||
);
|
||||
this.cleanupLeases =
|
||||
leases.length > 0 && leases.length === failedOwners.length
|
||||
? [...new Set(leases)]
|
||||
: undefined;
|
||||
this.failedOwners = failures.map(({ owner }) => owner);
|
||||
this.cleanupLeases = [...new Set(failures.map(({ lease }) => lease))];
|
||||
}
|
||||
}
|
||||
|
||||
/** 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 #active = new Set<Promise<unknown>>();
|
||||
#draining: Promise<void> | undefined;
|
||||
#closed = false;
|
||||
@@ -134,7 +132,7 @@ export class ArtifactResourceScope {
|
||||
/** Attempt cleanup now and retain the same owner if it fails. */
|
||||
async release(owner: ArtifactResourceOwner): Promise<ArtifactCleanupAttempt> {
|
||||
try {
|
||||
await cleanup(owner);
|
||||
await this.#lease(owner).release();
|
||||
this.#pending.delete(owner);
|
||||
return { kind: "released" };
|
||||
} catch (cause: unknown) {
|
||||
@@ -171,7 +169,7 @@ export class ArtifactResourceScope {
|
||||
}[] = [];
|
||||
for (const owner of this.#pending) {
|
||||
try {
|
||||
await cleanup(owner);
|
||||
await this.#lease(owner).release();
|
||||
this.#pending.delete(owner);
|
||||
} catch (cause: unknown) {
|
||||
failures.push({ owner, cause });
|
||||
@@ -190,7 +188,7 @@ export class ArtifactResourceScope {
|
||||
...new Set(observations.flatMap(({ resources }) => resources)),
|
||||
];
|
||||
throw new ArtifactResourceRetryFailure(
|
||||
failures.map(({ owner }) => owner),
|
||||
failures.map(({ owner }) => ({ owner, lease: this.#lease(owner) })),
|
||||
`Artifact cleanup remains incomplete: ${resources.join(", ")}`,
|
||||
{
|
||||
cause: new AggregateError(
|
||||
@@ -204,6 +202,17 @@ export class ArtifactResourceScope {
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
#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 cleanup = async (owner: ArtifactResourceOwner): Promise<void> => {
|
||||
|
||||
Reference in new issue
Block a user