From 6eb55be4e8e685d4eacfe9ccf5d3e0308a9cd617 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sat, 17 Oct 2026 14:13:16 +0800 Subject: [PATCH] fix(kuna): preserve live cleanup in caller failures Reuse canonical typed failure attribution instead of copying cleanup markers. Preserve captured facts, typed primary outcomes and opaque cancellation reasons; bind caller projections to the exact producer cleanup owner. Refs: #4693 --- src/domain/providerCleanupError.ts | 12 +- src/kuna/KunaQueryErrors.ts | 97 ++--------- src/providers/AnalysisClientFailure.ts | 61 ++++++- .../providers/kuna/kunaQueryErrors.test.ts | 163 ++++++++++++++++++ 4 files changed, 236 insertions(+), 97 deletions(-) create mode 100644 tests/boundary/providers/kuna/kunaQueryErrors.test.ts diff --git a/src/domain/providerCleanupError.ts b/src/domain/providerCleanupError.ts index 7a8299cb74..b7fc3f1af6 100644 --- a/src/domain/providerCleanupError.ts +++ b/src/domain/providerCleanupError.ts @@ -1,6 +1,6 @@ import type { JsonValue } from "./jsonValue.js"; import { ProviderAdapterError } from "./providerAdapterError.js"; -import type { AnalysisPartialObservation } from "./analysisErrorBase.js"; +import type { AnalysisErrorOptions } from "./analysisErrorBase.js"; /** Provider resources could not be proven closed after bounded cleanup. */ export class ProviderCleanupError extends ProviderAdapterError { @@ -13,20 +13,14 @@ export class ProviderCleanupError extends ProviderAdapterError { resources: readonly string[], diagnostics: Readonly>, /** Identify the operation owning these resources; binary callers retain their default. */ - options?: ErrorOptions & { + options?: Omit & { readonly operation?: string; - readonly partialObservation?: AnalysisPartialObservation; readonly remediationAction?: string; }, ) { - const { - operation = "close_binary", - partialObservation, - ...errorOptions - } = options ?? {}; + const { operation = "close_binary", ...errorOptions } = options ?? {}; super(providerId, operation, { ...errorOptions, - ...(partialObservation === undefined ? {} : { partialObservation }), diagnostics, }); this.setCleanupResources(resources); diff --git a/src/kuna/KunaQueryErrors.ts b/src/kuna/KunaQueryErrors.ts index 536aef6a84..93c987c7eb 100644 --- a/src/kuna/KunaQueryErrors.ts +++ b/src/kuna/KunaQueryErrors.ts @@ -1,16 +1,6 @@ -import type { - AnalysisError, - AnalysisErrorOptions, -} from "../domain/analysisErrorBase.js"; -import { - AnalysisArtifactChangedError, - AnalysisCancelledError, - AnalysisOutputError, - AnalysisResourceConstraintError, - AnalysisTimeoutError, -} from "../domain/analysisErrorCore.js"; -import { ProviderAdapterError } from "../domain/providerAdapterError.js"; -import { ProviderCleanupError } from "../domain/providerCleanupError.js"; +import type { AnalysisError } from "../domain/analysisErrorBase.js"; +import { analysisClientFailureForOperation } from "../providers/AnalysisClientFailure.js"; +import { KUNA_PROVIDER_ID } from "./KunaRelease.js"; /** Keep one producer failure immutable while each waiter receives its context. */ export const kunaQueryFailureForCaller = ( @@ -23,77 +13,12 @@ export const kunaQueryFailureForCaller = ( source.operation === operation ) return source; - const diagnostics = { - ...source.diagnostics, - producer_operation: source.operation, - }; - const options: AnalysisErrorOptions & { - userMessage?: string; - remediationAction?: string; - } = { - cause: source, - diagnostics, - ...(source.capturedOutput === undefined - ? {} - : { capturedOutput: source.capturedOutput }), - ...(source.cleanup === undefined ? {} : { cleanup: source.cleanup }), - ...(source.partialObservation === undefined - ? {} - : { partialObservation: source.partialObservation }), - ...(source.userMessage === undefined - ? {} - : { userMessage: source.userMessage }), - ...("remediationAction" in source && - typeof source.remediationAction === "string" - ? { remediationAction: source.remediationAction } - : {}), - }; - // The command owner keeps the original cleanup failure and its supervisor. - // This contextual diagnostic retains that source as its cause and every resource. - if (source instanceof ProviderCleanupError) - return new ProviderCleanupError( - source.providerId, - source.cleanupResources, - diagnostics, - { - operation, - cause: source, - ...(source.partialObservation === undefined - ? {} - : { partialObservation: source.partialObservation }), - }, - ); - if (source instanceof AnalysisTimeoutError) - return new AnalysisTimeoutError(operation, source.timeoutMs, options); - if (source instanceof AnalysisResourceConstraintError) - return new AnalysisResourceConstraintError( - operation, - source.resource, - source.reason, - source.reportedLimits, - { - ...options, - ...(source.retryable === undefined - ? {} - : { retryable: source.retryable }), - }, - ); - if (source instanceof AnalysisOutputError) - return new AnalysisOutputError(operation, source.reason, { - ...options, - cleanupOnly: source.cleanupOnly, - }); - if (source instanceof ProviderAdapterError) - return new ProviderAdapterError(source.providerId, operation, options); - if (source instanceof AnalysisCancelledError) - return new AnalysisCancelledError(operation, options); - if (source instanceof AnalysisArtifactChangedError) - return new AnalysisArtifactChangedError( - operation, - source.path, - source.reason, - options, - ); - // Preserve unfamiliar error variants rather than flattening their semantics. - return source; + return analysisClientFailureForOperation( + source, + KUNA_PROVIDER_ID, + operation, + { + producer_operation: source.operation, + }, + ); }; diff --git a/src/providers/AnalysisClientFailure.ts b/src/providers/AnalysisClientFailure.ts index a3344acbb1..56b24b6102 100644 --- a/src/providers/AnalysisClientFailure.ts +++ b/src/providers/AnalysisClientFailure.ts @@ -13,8 +13,12 @@ import { AnalysisAccessDeniedError, AnalysisArtifactChangedError, AnalysisResourceConstraintError, + AnalysisOutputError, + AnalysisTimeoutError, } from "../domain/analysisErrorCore.js"; import { ProviderAdapterError } from "../domain/providerAdapterError.js"; +import { ProviderCleanupError } from "../domain/providerCleanupError.js"; +import type { JsonValue } from "../domain/jsonValue.js"; import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js"; import { bindArtifactFileCleanup } from "../artifacts/ArtifactFileOperation.js"; @@ -86,10 +90,14 @@ export const analysisClientFailureForOperation = ( cause: unknown, providerId: string, operation: string, + callerDiagnostics?: Readonly>, ): AnalysisError => { if (!(cause instanceof AnalysisError)) { const failure = new ProviderAdapterError(providerId, operation, { cause, + ...(callerDiagnostics === undefined + ? {} + : { diagnostics: callerDiagnostics }), ...(cause instanceof ArtifactReaderFailure && cause.cleanup !== undefined ? { cleanup: cause.cleanup } : {}), @@ -106,10 +114,24 @@ export const analysisClientFailureForOperation = ( !(cause instanceof AnalysisCapabilityUnavailableError) && !(cause instanceof AnalysisAccessDeniedError) && !(cause instanceof AnalysisArtifactChangedError) && + !(cause instanceof AnalysisTimeoutError) && + !(cause instanceof AnalysisOutputError) && !(cause instanceof AnalysisResourceConstraintError) ) return cause; - const context = analysisClientFailureContext(cause, providerId, operation); + const sourceContext = analysisClientFailureContext( + cause, + providerId, + operation, + ); + const context = { + ...sourceContext, + ...(callerDiagnostics === undefined + ? {} + : { + diagnostics: { ...sourceContext.diagnostics, ...callerDiagnostics }, + }), + }; let result: AnalysisError; if (cause instanceof HopperRemoteError) result = new HopperRemoteError(cause.code, cause.safeMessage, { @@ -142,6 +164,41 @@ export const analysisClientFailureForOperation = ( ? {} : { launcherFailure: cause.launcherFailure }), }); + else if (cause instanceof ProviderCleanupError) { + const { diagnostics, ...outcomeContext } = context; + result = new ProviderCleanupError( + cause.providerId, + cause.cleanupResources, + diagnostics ?? {}, + { + ...outcomeContext, + operation, + ...(cause.remediationAction === undefined + ? {} + : { remediationAction: cause.remediationAction }), + }, + ); + } else if (cause instanceof AnalysisTimeoutError) + result = new AnalysisTimeoutError(operation, cause.timeoutMs, { + ...context, + ...(cause.userMessage === undefined + ? {} + : { userMessage: cause.userMessage }), + ...(cause.remediationAction === undefined + ? {} + : { remediationAction: cause.remediationAction }), + }); + else if (cause instanceof AnalysisOutputError) + result = new AnalysisOutputError(operation, cause.reason, { + ...context, + cleanupOnly: cause.cleanupOnly, + ...(cause.userMessage === undefined + ? {} + : { userMessage: cause.userMessage }), + ...(cause.remediationAction === undefined + ? {} + : { remediationAction: cause.remediationAction }), + }); else if (cause instanceof AnalysisCancelledError) result = new AnalysisCancelledError(operation, { ...context, @@ -202,5 +259,5 @@ export const analysisClientFailureForOperation = ( }); return cause.cleanupIncomplete ? result.bindCleanupDependencies([cause]) - : result; + : result.resolveCleanup(); }; diff --git a/tests/boundary/providers/kuna/kunaQueryErrors.test.ts b/tests/boundary/providers/kuna/kunaQueryErrors.test.ts new file mode 100644 index 0000000000..de8bde40a9 --- /dev/null +++ b/tests/boundary/providers/kuna/kunaQueryErrors.test.ts @@ -0,0 +1,163 @@ +import { expect, it } from "vitest"; +import { kunaQueryFailureForCaller } from "../../../../src/kuna/KunaQueryErrors.js"; +import { + AnalysisCancelledError, + AnalysisInputError, + AnalysisOutputError, + AnalysisTimeoutError, +} from "../../../../src/domain/analysisErrorCore.js"; +import { ProviderAdapterError } from "../../../../src/domain/providerAdapterError.js"; +import { ProviderCleanupError } from "../../../../src/domain/providerCleanupError.js"; +import { analysisErrorWithCleanupFailure } from "../../../../src/domain/analysisErrorCleanup.js"; +import { projectAnalysisError } from "../../../../src/domain/analysisErrorProjection.js"; +import { createEvidence } from "../../../../src/domain/evidence.js"; + +const capturedOutput = { + stdout: "selected output", + stderr: "selected warning", + truncated: false, +}; +const partialObservation = createEvidence( + undefined, + { id: "kuna", name: "Kuna", version: null }, + { + operation: "kuna:strings", + parameters: {}, + result: [], + confidence: "observed", + }, +); +const context = { + capturedOutput, + partialObservation, + diagnostics: { selected_url: "https://example.test/?keep=1" }, + cleanup: { reason: "controlled close refusal", resources: ["one", "two"] }, +}; + +it.each([ + { + name: "cleanup", + create: () => + new ProviderCleanupError("kuna", ["one", "two"], context.diagnostics, { + ...context, + operation: "kuna:strings", + }), + }, + { + name: "adapter", + create: () => new ProviderAdapterError("kuna", "kuna:strings", context), + }, + { + name: "output", + create: () => + new AnalysisOutputError("kuna:strings", "invalid scan", { + ...context, + cleanupOnly: true, + }), + }, + { + name: "timeout", + create: () => new AnalysisTimeoutError("kuna:strings", 17, context), + }, +])( + "keeps $name caller projections bound to the exact producer cleanup state", + ({ create }) => { + const source = create(); + let resources = ["one", "two"]; + source.bindCleanupResourceOwner({ + get cleanupResources() { + return resources; + }, + }); + const callers = ["list_strings", "search_strings"].map((operation) => + kunaQueryFailureForCaller(source, operation), + ); + for (const caller of callers) { + expect(caller._tag).toBe(source._tag); + if (source instanceof ProviderCleanupError) + expect(caller).toBeInstanceOf(ProviderCleanupError); + if (source instanceof AnalysisTimeoutError) + expect(caller).toMatchObject({ timeoutMs: source.timeoutMs }); + if (source instanceof AnalysisOutputError) + expect(caller).toMatchObject({ + cleanupOnly: source.cleanupOnly, + reason: source.reason, + }); + expect(caller.capturedOutput).toEqual(capturedOutput); + expect(caller.partialObservation).toEqual(partialObservation); + expect(caller.diagnostics).toMatchObject({ + ...context.diagnostics, + producer_operation: "kuna:strings", + }); + expect(caller.cleanupIncomplete).toBe(true); + } + resources = ["two"]; + for (const caller of callers) + expect(caller.cleanupResources).toEqual(["two"]); + source.resolveCleanup(); + for (const caller of callers) expect(caller.cleanupIncomplete).toBe(false); + }, +); + +it("does not resurrect an already resolved cleanup failure", () => { + const source = new ProviderCleanupError( + "kuna", + ["owner"], + {}, + { operation: "kuna:strings" }, + ); + source.resolveCleanup(); + const caller = kunaQueryFailureForCaller(source, "search_strings"); + expect(caller.cleanupIncomplete).toBe(false); +}); + +it.each([ + null, + "caller stopped", + new ProviderCleanupError("foreign", ["foreign-owner"], {}), +])( + "preserves an opaque cancellation reason without adopting its cleanup authority: %j", + (reason) => { + const source = new AnalysisCancelledError("kuna:strings", { + cause: reason, + }); + const caller = kunaQueryFailureForCaller(source, "search_strings"); + expect(caller.cause).toBe(reason); + expect(caller.cleanupIncomplete).toBe(false); + }, +); + +it("restores a caller-attributed typed primary after its exact cleanup owner resolves", () => { + const primary = new AnalysisTimeoutError("list_procedures", 23, { + capturedOutput, + }); + const cleanup = new ProviderCleanupError("kuna", ["owner"], {}); + const source = analysisErrorWithCleanupFailure( + primary, + cleanup, + "list_procedures", + ).bindCleanupDependencies([cleanup]); + const caller = kunaQueryFailureForCaller(source, "search_procedures"); + expect(caller.cleanupPrimary).toMatchObject({ + _tag: "AnalysisTimeoutError", + operation: "search_procedures", + timeoutMs: 23, + }); + cleanup.resolveCleanup(); + expect(projectAnalysisError(caller)).toMatchObject({ + code: "provider_timeout", + details: { + operation: "search_procedures", + captured_output: capturedOutput, + }, + }); +}); + +it("keeps same-operation and unfamiliar error variants unchanged", () => { + const source = new AnalysisTimeoutError("search_strings", 19); + expect(kunaQueryFailureForCaller(source, "search_strings")).toBe(source); + const unfamiliar = new AnalysisInputError("producer"); + expect(kunaQueryFailureForCaller(unfamiliar, "search_strings")).toBe( + unfamiliar, + ); +});