fix(observation): keep incomplete cancellation observations (#5644)

* fix(observation): keep incomplete cancellation observations Service projection treated a retained observation that failed the completed schema as a malformed completed result. Cancellation became unreadable_output and the partial was dropped. Incomplete observations now stay on the owner error, including the exact abort reason. * fix(observation): drop non-JSON incomplete observations Keep a retained partial only when projectAnalysisError can serialize it. Clear a non-JSON partial and record an AnalysisOutputError as a secondary failure so cancellation and timeout stay the owner error. Co-authored-by: morluto <morluto@users.noreply.github.com> * fix(observation): validate incomplete partials without copying jsonValueSchema.safeParse cloned a JSON-safe partial that projection copies again. Use jsonValueValidationIssue so the gate only checks. Co-authored-by: morluto <morluto@users.noreply.github.com> * test(ida): expect non-retryable unresolved procedure lookups Main stops treating invalid_request as retryable. An unchanged missing procedure stays invalid_request and is not marked retryable. Co-authored-by: morluto <morluto@users.noreply.github.com> * test(observation): share incomplete-observation assertions The new non-JSON cases repeated the same owner-error checks and pushed duplication over the 3.5% limit. One expectation helper keeps those checks without a second copy. Co-authored-by: morluto <morluto@users.noreply.github.com> * test(observation): keep the assertion helper out of the package The helper imports Vitest. Rename it to a fixture so the production build excludes it and dist does not import a dev dependency. Co-authored-by: morluto <morluto@users.noreply.github.com> * fix(observation): clear non-JSON partials on cleanup primaries Projection can return the cleanup primary after retry. Clearing only the wrapper left that primary's partial in place, so projectAnalysisError threw. The gate now clears every non-JSON partial in the chain. Co-authored-by: morluto <morluto@users.noreply.github.com> * fix(observation): freeze retained incomplete observations A JSON partial stayed a live provider object, so a later mutation could make projection throw. The gate now keeps one frozen snapshot and still drops a non-JSON partial without copying it. Co-authored-by: morluto <morluto@users.noreply.github.com> * refactor(observation): share web runtime partial retention Observe and inspect used the same promote-or-keep path. One helper owns that sequence so the two operations cannot drift. Co-authored-by: morluto <morluto@users.noreply.github.com> * fix(observation): snapshot a shared partial once A cleanup wrapper and its primary can hold the same partial object. The gate now reuses the first frozen snapshot for that object. Co-authored-by: morluto <morluto@users.noreply.github.com> * fix(observation): parse a kept partial once jsonValueSchema.safeParse validates and copies only a JSON partial. The gate freezes that copy and no longer walks the same value first. Co-authored-by: morluto <morluto@users.noreply.github.com> * test(observation): share repeated observation fixtures The incomplete-observation tests repeated the same provider setup and an empty Electron coverage tail, which pushed duplication over 3.5%. One coverage fixture and shared assertions bring the tree back under the threshold. Co-authored-by: morluto <morluto@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: morluto <morluto@users.noreply.github.com>
2026-10-26 14:25:49 -04:00 · 2026-10-21 04:17:46 +08:00
parent 6b101ff23b
commit de4579e993
15 changed files with 915 additions and 304 deletions
@@ -1,6 +1,5 @@
 import { expect, it } from "vitest";

-import { classifyBrowserCompleteness } from "../../domain/browserCompleteness.js";
 import {
  AnalysisCancelledError,
  AnalysisOutputError,
@@ -12,14 +11,27 @@ import {
  electronTargetListSchema,
  inspectElectronPageInputSchema,
 } from "../../domain/javascript/electronObservation.js";
+import { emptyElectronPageCoverage } from "../../domain/javascript/electronPageInspection.fixture.js";
+import type { AnalysisError } from "../../domain/analysisErrorBase.js";
 import { err, ok } from "../../domain/result.js";
 import type { ElectronObservationPort } from "./ElectronObservationPort.js";
 import {
  inspectElectronPage,
  listElectronTargets,
 } from "./ElectronObservationService.js";
+import {
+  expectDroppedPartial,
+  expectKeptPartial,
+  expectOwnerError,
+} from "../ownerObservationExpectation.fixture.js";

 const identity = { id: "electron-fixture", name: "Fixture", version: "1" };
+const pageProvider = (failure: AnalysisError): ElectronObservationPort => ({
+  identity: () => identity,
+  listTargets: async () =>
+    err(new AnalysisCancelledError("list_electron_targets")),
+  inspectPage: async () => err(failure),
+});
 const input = inspectElectronPageInputSchema.parse({
  cdp_endpoint: "http://127.0.0.1:9222",
  target_id: "electron-page",
@@ -40,33 +52,7 @@ const inspection = electronPageInspectionSchema.parse({
    file_path: "/app/index.html",
    attached: false,
  },
-  capture_window: {
-    started_at: "2026-01-01T00:00:00.000Z",
-    ended_at: "2026-01-01T00:00:00.000Z",
-    observation_ms: 0,
-  },
-  completeness: classifyBrowserCompleteness({
-    policyFilteredSections: new Set(),
-    attachLimitedSections: new Set(),
-    truncatedSections: new Set(),
-    unavailableSections: new Set(),
-    excluded: [],
-    droppedEvents: {
-      scripts: 0,
-      network_requests: 0,
-      console_events: 0,
-      websocket_connections: 0,
-      websocket_frames: 0,
-      webmcp_tools: 0,
-      timeline_events: 0,
-    },
-  }),
-  frames: [],
-  dom: { total_nodes: 0, nodes: [] },
-  scripts: { total: 0, items: [] },
-  resources: [],
-  workers: [],
-  limitations: [],
+  ...emptyElectronPageCoverage(),
 });

 it("converts a retained raw Electron inspection into partial Evidence", async () => {
@@ -77,13 +63,7 @@ it("converts a retained raw Electron inspection into partial Evidence", async ()
      cleanup: "transport-closed",
    },
  });
-  const provider: ElectronObservationPort = {
-    identity: () => identity,
-    listTargets: async () =>
-      err(new AnalysisCancelledError("list_electron_targets")),
-    inspectPage: async () => err(failure),
-  };
-  const result = await inspectElectronPage(provider, input);
+  const result = await inspectElectronPage(pageProvider(failure), input);
  expect(result.ok).toBe(false);
  if (!result.ok) {
    expect(result.error).toBe(failure);
@@ -116,13 +96,7 @@ it("converts a raw inspection retained by provider cleanup failure", async () =>
      },
    },
  );
-  const provider: ElectronObservationPort = {
-    identity: () => identity,
-    listTargets: async () =>
-      err(new AnalysisCancelledError("list_electron_targets")),
-    inspectPage: async () => err(failure),
-  };
-  const result = await inspectElectronPage(provider, input);
+  const result = await inspectElectronPage(pageProvider(failure), input);
  expect(result.ok).toBe(false);
  if (!result.ok) {
    expect(result.error).toBe(failure);
@@ -244,3 +218,43 @@ it.each(["list", "inspect"] as const)(
    expect(providerCalls).toBe(0);
  },
 );
+
+it("keeps cancellation when the retained Electron inspection is incomplete", async () => {
+  const reason = { requested: "stop before page inspection completed" };
+  const partial = {
+    kind: "electron-page-observation",
+    inspection: { target_id: "electron-page" },
+    cleanup: "incomplete",
+  };
+  const failure = new AnalysisCancelledError("inspect_electron_page", {
+    cause: reason,
+    partialObservation: partial as never,
+  });
+  const result = await inspectElectronPage(pageProvider(failure), input);
+  expectKeptPartial(expectOwnerError(result, failure, reason), partial);
+});
+
+it.each([
+  { inspection: { requests: Number.NaN }, cleanup: "incomplete" as const },
+  {
+    inspection: { scripts: [undefined] },
+    cleanup: "transport-closed" as const,
+  },
+])(
+  "drops a non-JSON Electron inspection and keeps cancellation primary",
+  async (unsafe) => {
+    const reason = { requested: "stop before page inspection completed" };
+    const failure = new AnalysisCancelledError("inspect_electron_page", {
+      cause: reason,
+      partialObservation: {
+        kind: "electron-page-observation",
+        ...unsafe,
+      } as never,
+    });
+    const result = await inspectElectronPage(pageProvider(failure), input);
+    expectDroppedPartial(
+      expectOwnerError(result, failure, reason),
+      "Electron provider retained a non-JSON incomplete observation.",
+    );
+  },
+);
@@ -18,7 +18,10 @@ import {
  electronPageInspectionSchema,
  electronTargetListSchema,
 } from "../../domain/javascript/electronObservation.js";
-import { completeProviderEvidence } from "../ProviderOperationExecution.js";
+import {
+  completeProviderEvidence,
+  keepIncompleteObservation,
+} from "../ProviderOperationExecution.js";

 /** List local file pages exposed by the explicitly selected endpoint. */
 export const listElectronTargets = async (
@@ -79,34 +82,32 @@ export const inspectElectronPage = async (
      partial.kind === "electron-page-observation"
    ) {
      const report = electronPageInspectionSchema.safeParse(partial.inspection);
-      if (!report.success) {
-        if (result.error instanceof AnalysisCancelledError)
-          return err(
-            new AnalysisOutputError(
-              "inspect_electron_page",
-              "Electron provider retained a malformed completed page inspection.",
-              { cause: report.error },
-            ),
-          );
+      if (report.success) {
+        const inspection = {
+          ...report.data,
+          limitations: [
+            ...report.data.limitations,
+            partial.cleanup === "transport-closed"
+              ? "REA closed its CDP transport; this does not imply target shutdown."
+              : "CDP transport cleanup remains incomplete.",
+          ],
+        } satisfies ElectronPageInspection;
+        result.error.partialObservation = createElectronEvidence(
+          "inspect_electron_page",
+          input,
+          inspection,
+          ready.value.identity(),
+        );
        return result;
      }
-      const inspection = {
-        ...report.data,
-        limitations: [
-          ...report.data.limitations,
-          partial.cleanup === "transport-closed"
-            ? "REA closed its CDP transport; this does not imply target shutdown."
-            : "CDP transport cleanup remains incomplete.",
-        ],
-      } satisfies ElectronPageInspection;
-      result.error.partialObservation = createElectronEvidence(
-        "inspect_electron_page",
-        input,
-        inspection,
-        ready.value.identity(),
-      );
    }
-    return result;
+    return err(
+      keepIncompleteObservation(
+        result.error,
+        "inspect_electron_page",
+        "Electron provider retained a non-JSON incomplete observation.",
+      ),
+    );
  }
  const report = electronPageInspectionSchema.safeParse(result.value);
  if (!report.success)
@@ -14,6 +14,35 @@ import {
  listJavaScriptRuntimeTargets,
  observeJavaScriptRuntime,
 } from "./JavaScriptRuntimeObservationService.js";
+import {
+  expectDroppedPartial,
+  expectKeptPartial,
+  expectOutputError,
+  expectOwnerError,
+  expectPreemptedCancellation,
+} from "../ownerObservationExpectation.fixture.js";
+
+const observeCancelled = async (partial: unknown) => {
+  const reason = { requested: "stop before runtime observation completed" };
+  const failure = new AnalysisCancelledError("observe_javascript_runtime", {
+    cause: reason,
+    partialObservation: partial as never,
+  });
+  const result = await observeJavaScriptRuntime(
+    {
+      identity: () => ({ id: "test", name: "Test", version: "1" }),
+      listTargets: async () =>
+        err(new AnalysisCancelledError("list_javascript_runtime_targets")),
+      observe: async () => err(failure),
+    },
+    {
+      inspector_endpoint: "http://127.0.0.1:9229",
+      target_id: "target",
+      observation_ms: 0,
+    },
+  );
+  return { result, failure, reason };
+};

 it("preserves failed Inspector cleanup when cancellation occurs during observation", async () => {
  const controller = new AbortController();
@@ -126,11 +155,7 @@ it("does not let late cancellation hide an invalid completed Inspector report",
    },
    { inspector_endpoint: "http://127.0.0.1:9229" },
  );
-  expect(output).toMatchObject({
-    ok: false,
-    error: { _tag: "AnalysisOutputError" },
-  });
-  if (!output.ok) expect(output.error).toBeInstanceOf(AnalysisOutputError);
+  expectOutputError(output);

  const controller = new AbortController();
  const late = await listJavaScriptRuntimeTargets(
@@ -147,11 +172,7 @@ it("does not let late cancellation hide an invalid completed Inspector report",
    { inspector_endpoint: "http://127.0.0.1:9229" },
    { signal: controller.signal },
  );
-  expect(late).toMatchObject({
-    ok: false,
-    error: { _tag: "AnalysisOutputError" },
-  });
-  if (!late.ok) expect(late.error).toBeInstanceOf(AnalysisOutputError);
+  expectOutputError(late);
 });

 it("converts a raw runtime observation retained by cleanup failure", async () => {
@@ -309,11 +330,23 @@ it.each(["list", "observe"] as const)(
            { signal: controller.signal },
          );

-    expect(result.ok).toBe(false);
-    if (!result.ok) {
-      expect(result.error).toBeInstanceOf(AnalysisCancelledError);
-      expect(result.error.cause).toBe(reason);
-    }
-    expect(providerCalls).toBe(0);
+    expectPreemptedCancellation(result, reason, providerCalls);
+  },
+);
+
+it("keeps cancellation when the retained runtime observation is incomplete", async () => {
+  const partial = { runtime: "partial", scripts: [] };
+  const { result, failure, reason } = await observeCancelled(partial);
+  expectKeptPartial(expectOwnerError(result, failure, reason), partial);
+});
+
+it.each([{ scripts: [undefined] }, { elapsed_ms: Number.NaN }])(
+  "drops a non-JSON runtime partial and keeps cancellation primary",
+  async (partial) => {
+    const { result, failure, reason } = await observeCancelled(partial);
+    expectDroppedPartial(
+      expectOwnerError(result, failure, reason),
+      "Inspector retained a non-JSON incomplete observation.",
+    );
  },
 );
@@ -13,7 +13,10 @@ import type {
  ObserveJavaScriptRuntimeInput,
 } from "../../domain/javascript/javascriptRuntimeObservation.js";
 import { err, ok, type Result } from "../../domain/result.js";
-import { completeProviderEvidence } from "../ProviderOperationExecution.js";
+import {
+  completeProviderEvidence,
+  keepIncompleteObservation,
+} from "../ProviderOperationExecution.js";
 import {
  javascriptRuntimeObservationSchema,
  javascriptRuntimeTargetListSchema,
@@ -76,15 +79,7 @@ export const observeJavaScriptRuntime = async (
      !("kind" in partial)
    ) {
      const report = javascriptRuntimeObservationSchema.safeParse(partial);
-      if (!report.success && result.error instanceof AnalysisCancelledError)
-        return err(
-          new AnalysisOutputError(
-            "observe_javascript_runtime",
-            "Inspector retained a malformed completed runtime observation.",
-            { cause: report.error },
-          ),
-        );
-      if (report.success)
+      if (report.success) {
        result.error.partialObservation =
          createJavaScriptRuntimeObservationEvidence(
            "observe_javascript_runtime",
@@ -92,8 +87,16 @@ export const observeJavaScriptRuntime = async (
            report.data,
            ready.value.identity(),
          );
+        return result;
+      }
    }
-    return result;
+    return err(
+      keepIncompleteObservation(
+        result.error,
+        "observe_javascript_runtime",
+        "Inspector retained a non-JSON incomplete observation.",
+      ),
+    );
  }
  const report = javascriptRuntimeObservationSchema.safeParse(result.value);
  if (!report.success)