From d4e233946a39574e36a0d927b13eb77528ad5dc5 Mon Sep 17 00:00:00 2001
From: morluto <76467478+morluto@users.noreply.github.com>
Date: Mon, 19 Oct 2026 01:56:58 +0800
Subject: [PATCH] fix(errors): retain secondary failures without replacing
 primary causes

Give the typed error owner a separate retained secondary-failure collection. Preserve exact direct causes, including caller cancellation reasons and absent causes, while keeping existing diagnostic and live cleanup ownership projections.

Fixes #5059
---
 src/domain/analysisErrorBase.ts                    | 11 ++++++-----
 src/domain/analysisErrorProjection.test.ts         | 14 ++++++++++++++
 .../mcp/reverseEngineeringPartialMcp.test.ts       | 12 +++++++++---
 .../adb/adbInputSnapshotOwnership.test.ts          | 11 ++++++-----
 4 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/src/domain/analysisErrorBase.ts b/src/domain/analysisErrorBase.ts
index 86e6a9939..e9993705d 100644
--- a/src/domain/analysisErrorBase.ts
+++ b/src/domain/analysisErrorBase.ts
@@ -79,6 +79,7 @@ export abstract class AnalysisError extends Error {
   cleanupIncomplete: boolean;
   #ownedCleanupResources: readonly string[];
   #cleanupResourceOwner: AnalysisCleanupResourceOwner | undefined;
+  #secondaryFailures: AnalysisError[] = [];
   constructor(message: string, options?: AnalysisErrorOptions) {
     super(message, options);
     this.#cleanupPrimary = options?.cleanupPrimary;
@@ -104,6 +105,10 @@ export abstract class AnalysisError extends Error {
   get cleanupPrimary(): AnalysisError | undefined {
     return this.#cleanupPrimary;
   }
+  /** Later typed failures retained without changing the primary error cause. */
+  get secondaryFailures(): readonly AnalysisError[] {
+    return this.#secondaryFailures;
+  }
   readonly cleanupReport: ProcessCaptureCleanupReport | undefined = undefined;
 
   /** Live dependencies own current resource attribution; cleanup keeps history. */
@@ -142,12 +147,8 @@ export abstract class AnalysisError extends Error {
     // the primary error's live cleanup status and original output.
     Object.assign(this, {
       diagnostics: structuredClone({ ...this.diagnostics, ...diagnostics }),
-      cause: new AggregateError(
-        this.cause === undefined ? [failure] : [this.cause, failure],
-        "Analysis failed and a later lifecycle operation also failed.",
-        { cause: this.cause },
-      ),
     });
+    this.#secondaryFailures.push(failure);
     return this;
   }
 
diff --git a/src/domain/analysisErrorProjection.test.ts b/src/domain/analysisErrorProjection.test.ts
index 7b18fe4dd..cc8fac59b 100644
--- a/src/domain/analysisErrorProjection.test.ts
+++ b/src/domain/analysisErrorProjection.test.ts
@@ -839,7 +839,9 @@ describe("analysis error projection: evidence files", () => {
 });
 
 it("retains the primary failure and both cleanup reports after a secondary failure", () => {
+  const callerReason = { caller: "stop" };
   const primary = new AnalysisCancelledError("execute_rizin_command", {
+    cause: callerReason,
     capturedOutput: {
       stdout: "partial",
       stderr: "diagnostic",
@@ -855,6 +857,8 @@ it("retains the primary failure and both cleanup reports after a secondary failu
   });
   expect(enriched).toBe(primary);
   expect(enriched._tag).toBe("AnalysisCancelledError");
+  expect(enriched.cause).toBe(callerReason);
+  expect(enriched.secondaryFailures).toEqual([secondary]);
   expect(projectAnalysisError(enriched)).toMatchObject({
     code: "cleanup_incomplete",
     details: {
@@ -867,6 +871,16 @@ it("retains the primary failure and both cleanup reports after a secondary failu
   });
 });
 
+it("retains secondary failures when the primary error has no cause", () => {
+  const primary = new AnalysisCancelledError("execute_rizin_command");
+  const secondary = new AnalysisOutputError("open_binary", "cannot rebind");
+
+  primary.retainSecondaryFailure(secondary, {});
+
+  expect(primary.cause).toBeUndefined();
+  expect(primary.secondaryFailures).toEqual([secondary]);
+});
+
 it.each([false, true])(
   "preserves producer capability recovery with retryable=%s",
   (retryable) => {
diff --git a/tests/boundary/mcp/reverseEngineeringPartialMcp.test.ts b/tests/boundary/mcp/reverseEngineeringPartialMcp.test.ts
index 6a2b20409..cf9eebeda 100644
--- a/tests/boundary/mcp/reverseEngineeringPartialMcp.test.ts
+++ b/tests/boundary/mcp/reverseEngineeringPartialMcp.test.ts
@@ -186,10 +186,14 @@ it("delivers failed Rizin execution after external admission", async () => {
 
 it("preserves the Rizin operation error when analysis retirement also fails", async () => {
   const { server, handler } = createReverseEngineeringMcpHarness();
+  const reason = { stage: "operation-input" };
+  const primary = new AnalysisInputError("execute_rizin_command", {
+    cause: reason,
+  });
+  const retirement = new AnalysisInputError("rebind");
   const service = {
     retryCleanup: async () => ok(null),
-    executeRizinCommand: async () =>
-      err(new AnalysisInputError("execute_rizin_command")),
+    executeRizinCommand: async () => err(primary),
   } as unknown as ReverseEngineeringService;
   const observedOperationResult = vi.fn();
   registerReverseEngineeringTools(
@@ -203,7 +207,7 @@ it("preserves the Rizin operation error when analysis retirement also fails", as
     async (_path, _signal, operation, rebindFailure) => {
       const result = await operation();
       observedOperationResult(result);
-      return err(rebindFailure(result, new AnalysisInputError("rebind")));
+      return err(rebindFailure(result, retirement));
     },
   );
   const invoke = handler("execute_rizin_command");
@@ -221,6 +225,8 @@ it("preserves the Rizin operation error when analysis retirement also fails", as
     };
   };
 
+  expect(primary.cause).toBe(reason);
+  expect(primary.secondaryFailures).toEqual([retirement]);
   expect(response.isError).toBe(true);
   expect(observedOperationResult).toHaveBeenCalledWith(
     expect.objectContaining({ ok: false }),
diff --git a/tests/boundary/providers/adb/adbInputSnapshotOwnership.test.ts b/tests/boundary/providers/adb/adbInputSnapshotOwnership.test.ts
index d3050a1de..c661fcc44 100644
--- a/tests/boundary/providers/adb/adbInputSnapshotOwnership.test.ts
+++ b/tests/boundary/providers/adb/adbInputSnapshotOwnership.test.ts
@@ -347,11 +347,12 @@ test.each([
     expect(result.error.cleanupIncomplete).toBe(false);
     expect(result.error.cleanupResources).toEqual([]);
     expect(sourceCloseAttempts).toBe(2);
-    const cause = result.error.cause;
-    expect(cause).toBeInstanceOf(AggregateError);
-    const secondary = (cause as AggregateError).errors.find(
-      (entry) => entry instanceof AnalysisOutputError,
-    );
+    expect(result.error.cause).toMatchObject({
+      message: "controlled snapshot read failure",
+    });
+    expect(result.error.secondaryFailures).toHaveLength(1);
+    const secondary = result.error.secondaryFailures[0];
+    expect(secondary).toBeInstanceOf(AnalysisOutputError);
     expect(secondary?.cause).toBeInstanceOf(AdbInputSnapshotFailure);
     expect(
       (secondary?.cause as AdbInputSnapshotFailure).cleanupCause,