diff --git a/src/domain/analysisErrorBase.ts b/src/domain/analysisErrorBase.ts index 593c943a60..541c6b7677 100644 --- a/src/domain/analysisErrorBase.ts +++ b/src/domain/analysisErrorBase.ts @@ -70,7 +70,8 @@ export interface AnalysisCleanupResourceOwner { /** Base class for expected analysis, provider, and session failures. */ export abstract class AnalysisError extends Error { readonly #cleanupPrimary: AnalysisError | undefined; - readonly #cleanupDependencies = new Set(); + // Undefined means unbound; a bound empty set records confirmed completion. + #cleanupDependencies: Set | undefined; #cleanupDependents: WeakRef[] = []; readonly diagnostics: Readonly> | undefined; readonly capturedOutput: AnalysisCapturedOutput | undefined; @@ -107,7 +108,7 @@ export abstract class AnalysisError extends Error { /** Live dependencies own current resource attribution; cleanup keeps history. */ get cleanupResources(): readonly string[] { - if (this.#cleanupDependencies.size === 0) + if (this.#cleanupDependencies === undefined) return ( this.#cleanupResourceOwner?.cleanupResources ?? this.#ownedCleanupResources @@ -152,13 +153,10 @@ export abstract class AnalysisError extends Error { /** Keep a composed error incomplete until all explicitly owned source errors resolve. */ bindCleanupDependencies(dependencies: readonly AnalysisError[]): this { + const pending = (this.#cleanupDependencies ??= new Set()); for (const dependency of dependencies) { - if ( - !dependency.cleanupIncomplete || - this.#cleanupDependencies.has(dependency) - ) - continue; - this.#cleanupDependencies.add(dependency); + if (!dependency.cleanupIncomplete || pending.has(dependency)) continue; + pending.add(dependency); dependency.#cleanupDependents = [ ...dependency.#cleanupDependents.filter( (reference) => reference.deref() !== undefined, @@ -166,19 +164,19 @@ export abstract class AnalysisError extends Error { new WeakRef(this), ]; } - if (this.#cleanupDependencies.size === 0) this.resolveCleanup(); + if (pending.size === 0) this.resolveCleanup(); return this; } /** Clear live cleanup status only after its owning resource confirms retry success. */ resolveCleanup(): this { if ( - [...this.#cleanupDependencies].some( + [...(this.#cleanupDependencies ?? [])].some( (dependency) => dependency.cleanupIncomplete, ) ) return this; - this.#cleanupDependencies.clear(); + this.#cleanupDependencies?.clear(); this.cleanupIncomplete = false; for (const reference of this.#cleanupDependents) reference.deref()?.resolveCleanup(); diff --git a/src/domain/analysisErrorCleanup.test.ts b/src/domain/analysisErrorCleanup.test.ts index 481fc77df1..41ce5abcfa 100644 --- a/src/domain/analysisErrorCleanup.test.ts +++ b/src/domain/analysisErrorCleanup.test.ts @@ -16,6 +16,26 @@ import { import { ProviderCleanupError } from "./providerCleanupError.js"; import { createEvidence } from "./evidence.js"; +it("keeps dependency-owned resources empty after the final owner releases", () => { + const file = new ProviderCleanupError("fixture", ["file-owner"], {}); + const trace = new ProviderCleanupError("fixture", ["trace-owner"], {}); + const composed = analysisErrorWithCleanupFailure( + file, + trace, + ).bindCleanupDependencies([file, trace]); + expect(new Set(composed.cleanupResources)).toEqual( + new Set(["file-owner", "trace-owner"]), + ); + file.resolveCleanup(); + expect(composed.cleanupResources).toEqual(["trace-owner"]); + trace.resolveCleanup(); + expect(composed.cleanupIncomplete).toBe(false); + expect(composed.cleanupResources).toEqual([]); + expect(new Set(composed.cleanup?.resources)).toEqual( + new Set(["file-owner", "trace-owner"]), + ); +}); + it("keeps primary diagnostics separate from historical cleanup diagnostics with the same keys", () => { const primary = new AnalysisInputError("inspect", { diagnostics: {