fix(apktool): retain decode facts across interrupted read stages

Keep one typed accumulation of observed decode facts instead of constructing
partial Evidence only after string projection. Record decoded client/target
before the post-command cancellation checkpoint, parsed metadata before
manifest reads, raw manifest before interpretation, and inventory prefix
before rethrowing its exact original cause. Omit unexamined fields.

Return inventory success or exact failure plus completed prefix through
Result. Keep resource-cap stops distinct from physical failures. Attach
truncation and inventory limitations at their producing stages. Validate
complete results with the named domain schema; centralize partial Evidence
assembly in the existing failure/cleanup boundary.

Validation: six authored stage controls fail unchanged34a88d6c1; 131 focused
checks across7files, check:fast, docs:check and discovery1556 pass. Covers
manifest bytes/XML, interrupted inventory, late cleanup cancellation,
denied removal/retry and existing table interruption. Two source reviewers
clear after moving the command handoff checkpoint. No separate runtime
qualification of that precise handoff race.

Public CLI manifest failure retains metadata/raw manifest with unknown
fields omitted and original failure framing unchanged. Twelve small normal
CLI tasks retain exact replies. No speedup or memory-saving claim; failure
reply estimate189 to610cl100k tokens preserves completed Evidence. No real
Apktool, large-case memory or whole-agent qualification. Heavy tests remain
stopped.

Fixes #4882

src/apktool/ApktoolDecodedInventory.ts

@@ -1,3 +1,4 @@
+import { ok, err, type Result } from "../domain/result.js";
 import type {
   ArtifactEntry,
   ArtifactTraversalOmission,
@@ -13,19 +14,26 @@ interface StringsTableOmission {
   readonly kind: Exclude<TableEntryKind, "file">;
 }
 
+interface DecodedInventory {
+  count: number;
+  capped: boolean;
+  locales: readonly string[];
+  otherStringsTables: readonly string[];
+  unreadableStringsTables: readonly StringsTableOmission[];
+}
+
 /** Count files over bounded examined coverage, including omitted special entries. */
 export const countDecodedFiles = async (
   workspace: ApktoolWorkspace,
   root: string,
   signal: AbortSignal | undefined,
   maximumEntries: number,
-): Promise<{
-  count: number;
-  capped: boolean;
-  locales: readonly string[];
-  otherStringsTables: readonly string[];
-  unreadableStringsTables: readonly StringsTableOmission[];
-}> => {
+): Promise<
+  Result<
+    DecodedInventory,
+    { readonly cause: unknown; readonly partial: DecodedInventory }
+  >
+> => {
   let count = 0;
   let capped = false;
   const locales: string[] = [];
@@ -44,6 +52,17 @@ export const countDecodedFiles = async (
     if (table.kind === "locale") locales.push(table.locale);
     else otherStringsTables.push(match[1]!);
   };
+  const observation = (): DecodedInventory => {
+    return {
+      count,
+      capped,
+      locales: locales.sort(),
+      otherStringsTables: otherStringsTables.sort(),
+      unreadableStringsTables: unreadableStringsTables.sort((a, b) =>
+        a.path < b.path ? -1 : a.path > b.path ? 1 : 0,
+      ),
+    };
+  };
   try {
     for await (const entry of workspace.entries(
       root,
@@ -56,16 +75,9 @@ export const countDecodedFiles = async (
       if (entry.kind === "file") count += 1;
     }
   } catch (cause) {
-    if (!(cause instanceof DirectoryArtifactTraversalLimit)) throw cause;
+    if (!(cause instanceof DirectoryArtifactTraversalLimit))
+      return err({ cause, partial: observation() });
     capped = true;
   }
-  return {
-    count,
-    capped,
-    locales: locales.sort(),
-    otherStringsTables: otherStringsTables.sort(),
-    unreadableStringsTables: unreadableStringsTables.sort((a, b) =>
-      a.path < b.path ? -1 : a.path > b.path ? 1 : 0,
-    ),
-  };
+  return ok(observation());
 };

src/apktool/ApktoolProvider.test.ts

@@ -14,6 +14,7 @@ import { join } from "node:path";
 import { describe, expect, onTestFinished, test, vi } from "vitest";
 
 import { ApktoolProvider } from "./ApktoolProvider.js";
+import { ApktoolWorkspace } from "./ApktoolWorkspace.js";
 import { ApktoolResourceAnalysisService } from "../application/apktool/ApktoolResourceAnalysisService.js";
 import { projectAnalysisError } from "../domain/analysisErrorProjection.js";
 import {
@@ -236,6 +237,118 @@ it.each([
   },
 );
 
+it.each(
+  (["manifest-bytes", "manifest-xml", "inventory"] as const).flatMap((stage) =>
+    [false, true].map((denied) => [stage, denied] as const),
+  ),
+)(
+  "retains parsed metadata when %s fails before string projection (cleanup denied: %s)",
+  async (stage, denyCleanup) => {
+    const { command, root } = await writeApktoolStub(decodeStub());
+    const apk = await apkFixture(root);
+    if (stage === "inventory") {
+      const entries = ApktoolWorkspace.prototype.entries;
+      const spy = vi
+        .spyOn(ApktoolWorkspace.prototype, "entries")
+        .mockImplementationOnce(async function* (
+          this: ApktoolWorkspace,
+          ...args
+        ) {
+          for await (const entry of entries.apply(this, args)) {
+            yield entry;
+            if (entry.kind === "file")
+              throw new Error("authored enumeration failure");
+          }
+        });
+      onTestFinished(() => spy.mockRestore());
+    } else {
+      const manifestFile = join(root, "manifest-control");
+      await writeFile(
+        manifestFile,
+        stage === "manifest-bytes" ? Buffer.from([255]) : "<resources/>",
+      );
+      await writeFile(
+        command,
+        `#!/bin/sh\n${decodeStub({ manifestFile })(root)}`,
+        { mode: 0o755 },
+      );
+    }
+    const controller = new AbortController();
+    let denied = denyCleanup;
+    const provider = new ApktoolProvider({
+      environment: { REA_APKTOOL_COMMAND: command },
+      removeWorkspace: async (path) => {
+        controller.abort({ stage: "final cleanup" });
+        if (denied) throw new Error("authored staged removal denial");
+        await rm(path, { recursive: true, force: true });
+      },
+    });
+    try {
+      const result = await provider.execute(
+        {
+          operation: "decode_android_resources",
+          input: { path: apk.path, include_strings: true },
+        },
+        { signal: controller.signal },
+      );
+      if (result.ok) throw new Error("Expected staged read failure");
+      expect((result.error.cleanupPrimary ?? result.error)._tag).toBe(
+        "AnalysisProtocolError",
+      );
+      expect(controller.signal.aborted).toBe(true);
+      expect(result.error.cleanupIncomplete).toBe(denyCleanup);
+      const observation = result.error.partialObservation;
+      expect(observation).toMatchObject({
+        operation: "decode_android_resources",
+        subject: { local_path: apk.path, digest: { sha256: apk.sha256 } },
+        normalized_result: {
+          coverage: "partial",
+          metadata: {
+            version_name: "1.2.3",
+            version_code: "7",
+            min_sdk_version: "24",
+            target_sdk_version: "34",
+          },
+        },
+      });
+      const projected = projectAnalysisError(result.error).details
+        ?.partial_observation;
+      expect(projected).toEqual(observation);
+      expect(observation).toHaveProperty("normalized_result");
+      if (observation === undefined || !("normalized_result" in observation))
+        throw new Error("Expected partial Evidence");
+      expect(observation.normalized_result).not.toHaveProperty("strings");
+      if (stage !== "inventory") {
+        expect(observation.normalized_result).not.toHaveProperty(
+          "decoded_file_count",
+        );
+        expect(observation.normalized_result).not.toHaveProperty(
+          "metadata.package_name",
+        );
+      }
+      if (stage === "manifest-bytes")
+        expect(observation.normalized_result).not.toHaveProperty("manifest");
+      if (stage === "manifest-xml")
+        expect(observation.normalized_result).toHaveProperty(
+          "manifest",
+          "<resources/>",
+        );
+      if (stage === "inventory")
+        expect(observation.normalized_result).toMatchObject({
+          decoded_file_count: 1,
+          metadata: { package_name: "com.rea.apktool.probe" },
+        });
+      denied = false;
+      await provider.close();
+      expect(result.error.cleanupIncomplete).toBe(false);
+      expect(result.error.partialObservation).toEqual(observation);
+    } finally {
+      denied = false;
+      await provider.close();
+    }
+  },
+);
+
 it("inspects the apktool launcher", async () => {
   const { command } = await writeApktoolStub(
     () => `if [ "$1" = "--version" ]; then ${VERSION_BLOCK}; exit 0; fi`,

src/apktool/ApktoolProvider.ts

@@ -30,7 +30,11 @@ import {
   createAnalysisExecutionEvidence,
   type AnalysisExecution,
 } from "../application/AnalysisProvider.js";
-import type { ApktoolRequest } from "../domain/apktool/apktoolResourceAnalysis.js";
+import {
+  apktoolResultSchemas,
+  type ApktoolRequest,
+  type ApktoolDecodeResult,
+} from "../domain/apktool/apktoolResourceAnalysis.js";
 import { projectApktoolStringTables } from "./ApktoolStringTables.js";
 import {
   AnalysisCapabilityUnavailableError,
@@ -232,6 +236,13 @@ export interface ApktoolProviderOptions {
   readonly removeWorkspace?: (path: string) => Promise<void>;
 }
 
+type DecodeFacts = Pick<ApktoolDecodeResult, "client" | "target" | "locale"> &
+  Partial<
+    Omit<ApktoolDecodeResult, "metadata" | "client" | "target" | "locale">
+  > & {
+    metadata?: Partial<ApktoolDecodeResult["metadata"]>;
+  };
+
 /**
  * Apktool resource decoding over a caller-selected launcher.
  *
@@ -495,11 +506,7 @@ export class ApktoolProvider {
     operation: "decode_android_resources",
     selection: ApktoolCommandSelection,
     environment: Readonly<NodeJS.ProcessEnv>,
-    client: {
-      command: string;
-      command_source: string;
-      apktool_version: string | null;
-    },
+    client: ApktoolDecodeResult["client"],
     provider: { id: string; name: string; version: string | null },
     input: Extract<
       ApktoolRequest,
@@ -522,7 +529,12 @@ export class ApktoolProvider {
     const owner = { kind: "lease" as const, resource: root, lease };
     this.#artifacts.retain(owner);
     let result: Result<AnalysisExecution, AnalysisError>;
-    let partialExecution: AnalysisExecution | undefined;
+    let completedDecode: DecodeFacts | undefined;
+    let stoppedAt = "metadata";
+    const limitations: string[] = [
+      ...clientLimitations,
+      "Decoding skips smali (--no-src); Java-source recovery belongs to the JADX family",
+    ];
     try {
       const identity = await resources.snapshotInput(
         input.path,
@@ -558,6 +570,12 @@ export class ApktoolProvider {
       if (!decoded.ok) {
         throw decoded.error;
       }
+      completedDecode = {
+        client,
+        target: { path: input.path, bytes: identity.bytes, sha256 },
+        locale: input.locale ?? null,
+        coverage: "partial",
+      };
       throwIfCancelled(operation, signal);
       const yml = await resources.readText(
         join(workspace, "apktool.yml"),
@@ -570,27 +588,58 @@ export class ApktoolProvider {
         );
       }
       const metadata = parseApktoolYml(yml.text);
+      completedDecode.metadata = {
+        version_name: metadata.versionName,
+        version_code: metadata.versionCode,
+        min_sdk_version: metadata.minSdkVersion,
+        target_sdk_version: metadata.targetSdkVersion,
+      };
+      stoppedAt = "manifest";
       const manifestRead = await resources.readText(
         join(workspace, "AndroidManifest.xml"),
         MANIFEST_MAX_BYTES,
         signal,
       );
       const manifest = manifestRead.text;
+      completedDecode.manifest = manifest;
+      if (manifestRead.truncated) {
+        limitations.push(
+          `manifest is truncated at ${String(MANIFEST_MAX_BYTES)} bytes`,
+        );
+      }
       const packageName =
         manifest === null || manifestRead.truncated
           ? null
           : parseManifestPackage(manifest);
-      const limitations: string[] = [
-        ...clientLimitations,
-        "Decoding skips smali (--no-src); Java-source recovery belongs to the JADX family",
-      ];
-      const files = await countDecodedFiles(
+      completedDecode.metadata.package_name = packageName;
+      stoppedAt = "inventory";
+      const inventory = await countDecodedFiles(
         resources,
         workspace,
         signal,
         MAX_DECODED_ENTRIES_VISITED,
       );
+      const files = inventory.ok ? inventory.value : inventory.error.partial;
+      completedDecode.locales = [...files.locales];
+      completedDecode.decoded_file_count = files.count;
+      if (files.unreadableStringsTables.length > 0) {
+        limitations.push(
+          `String tables omitted because they are not regular files: ${files.unreadableStringsTables.map((table) => `${JSON.stringify(table.path)} (${table.kind})`).join(", ")}`,
+        );
+      }
+      if (files.otherStringsTables.length > 0) {
+        limitations.push(
+          `Other strings tables are not selected by locale; their resource configuration suffixes are preserved without inferring Android runtime matching: ${files.otherStringsTables.map((table) => JSON.stringify(table)).join(", ")}`,
+        );
+      }
+      if (files.capped) {
+        limitations.push(
+          `Decoded workspace enumeration stopped after ${String(MAX_DECODED_ENTRIES_VISITED)} entries; decoded_file_count and locales may be partial`,
+        );
+      }
+      if (!inventory.ok) throw inventory.error.cause;
       throwIfCancelled(operation, signal);
+      stoppedAt = "string projection";
 
       const projection = await projectApktoolStringTables(
         resources,
@@ -610,58 +659,21 @@ export class ApktoolProvider {
           ? "partial"
           : "complete";
 
-      if (manifestRead.truncated) {
-        limitations.push(
-          `manifest is truncated at ${String(MANIFEST_MAX_BYTES)} bytes`,
-        );
-      }
-      if (files.unreadableStringsTables.length > 0) {
-        limitations.push(
-          `String tables omitted because they are not regular files: ${files.unreadableStringsTables.map((table) => `${JSON.stringify(table.path)} (${table.kind})`).join(", ")}`,
-        );
-      }
-      if (files.otherStringsTables.length > 0) {
-        limitations.push(
-          `Other strings tables are not selected by locale; their resource configuration suffixes are preserved without inferring Android runtime matching: ${files.otherStringsTables.map((table) => JSON.stringify(table)).join(", ")}`,
-        );
-      }
-      if (files.capped) {
-        limitations.push(
-          `Decoded workspace enumeration stopped after ${String(MAX_DECODED_ENTRIES_VISITED)} entries; decoded_file_count and locales may be partial`,
-        );
-      }
-      const execution = createAnalysisExecution(
-        {
-          client,
-          target: { path: input.path, bytes: identity.bytes, sha256 },
-          metadata: {
-            version_name: metadata.versionName,
-            version_code: metadata.versionCode,
-            min_sdk_version: metadata.minSdkVersion,
-            target_sdk_version: metadata.targetSdkVersion,
-            package_name: packageName,
+      completedDecode.strings = [...projectedStrings.strings];
+      if (input.locales !== undefined)
+        completedDecode.string_tables = [...projectedStrings.tables];
+      completedDecode.coverage = coverage;
+      if (!projection.ok) throw projection.error.cause;
+      result = ok(
+        createAnalysisExecution(
+          apktoolResultSchemas.decode_android_resources.parse(completedDecode),
+          provider,
+          {
+            subject: { path: input.path, sha256, format: "apk" },
+            limitations,
           },
-          manifest,
-          strings: projectedStrings.strings,
-          locale: input.locale ?? null,
-          locales: files.locales,
-          ...(input.locales === undefined
-            ? {}
-            : { string_tables: projectedStrings.tables }),
-          decoded_file_count: files.count,
-          coverage,
-        },
-        provider,
-        {
-          subject: { path: input.path, sha256, format: "apk" },
-          limitations,
-        },
+        ),
       );
-      if (!projection.ok) {
-        partialExecution = execution;
-        throw projection.error.cause;
-      }
-      result = ok(execution);
     } catch (cause) {
       const physical = artifactFilePrimaryFailure(cause);
       const primary =
@@ -697,9 +709,24 @@ export class ApktoolProvider {
               ).bindCleanupDependencies([primary, failure])
             : primary;
       if (
-        partialExecution !== undefined &&
+        completedDecode !== undefined &&
         retained.partialObservation === undefined
-      )
+      ) {
+        limitations.push(
+          `Decode observation stopped during ${stoppedAt}; fields not yet observed are omitted`,
+        );
+        const partialExecution = createAnalysisExecution(
+          { ...completedDecode, coverage: "partial" },
+          provider,
+          {
+            subject: {
+              path: input.path,
+              sha256: completedDecode.target.sha256,
+              format: "apk",
+            },
+            limitations,
+          },
+        );
         retained.retainPartialObservation(
           createAnalysisExecutionEvidence(
             operation,
@@ -708,6 +735,7 @@ export class ApktoolProvider {
             "observed",
           ),
         );
+      }
       if (
         resources.snapshot !== undefined &&
         retained.partialObservation === undefined

src/apktool/ApktoolWorkspace.lifecycle.test.ts

@@ -250,15 +250,17 @@ it.skipIf(process.platform === "win32")(
         undefined,
         3,
       );
-      expect(result.capped).toBe(true);
-      expect(result.count).toBeLessThanOrEqual(2);
+      if (!result.ok) throw result.error.cause;
+      expect(result.value.capped).toBe(true);
+      expect(result.value.count).toBeLessThanOrEqual(2);
       const complete = await countDecodedFiles(
         workspace,
         root.path,
         undefined,
         6,
       );
-      expect(complete).toEqual({
+      if (!complete.ok) throw complete.error.cause;
+      expect(complete.value).toEqual({
         count: 2,
         capped: false,
         locales: [],
@@ -306,7 +308,8 @@ it.skipIf(process.platform === "win32")(
         undefined,
         100,
       );
-      expect(result).toEqual({
+      if (!result.ok) throw result.error.cause;
+      expect(result.value).toEqual({
         count: regular.length,
         capped: false,
         locales: ["de"],
@@ -339,6 +342,7 @@ it("preserves a directory inspection failure instead of treating its locale tabl
   const blocked = join(root.path, "res/values-fr");
   try {
     await fs.mkdir(blocked, { recursive: true });
+    await fs.writeFile(join(root.path, "known.txt"), "completed");
     await fs.writeFile(join(blocked, "strings.xml"), "<resources/>");
     vi.mocked(fs.opendir).mockImplementation(async (path, options) => {
       if (String(path) === blocked)
@@ -349,9 +353,17 @@ it("preserves a directory inspection failure instead of treating its locale tabl
     });
     await expect(
       countDecodedFiles(workspace, root.path, undefined, 100),
-    ).rejects.toMatchObject({
-      reason: "io",
-      message: expect.stringContaining("authored directory inspection denial"),
+    ).resolves.toMatchObject({
+      ok: false,
+      error: {
+        partial: { count: 1 },
+        cause: {
+          reason: "io",
+          message: expect.stringContaining(
+            "authored directory inspection denial",
+          ),
+        },
+      },
     });
   } finally {
     await workspace.close();

src/domain/apktool/apktoolResourceAnalysis.ts

@@ -191,3 +191,7 @@ export const apktoolResultSchemas = {
     coverage: z.enum(["complete", "partial"]),
   }),
 } as const;
+
+export type ApktoolDecodeResult = z.infer<
+  typeof apktoolResultSchemas.decode_android_resources
+>;