fix(apktool): retain completed facts after locale read failures

Return completed string projections with the exact failure cause in Result.
Force partial coverage and disclose the interrupted selection and remaining
unattempted selections without inventing table status or counts.

Assemble execution once from success or partial projection. Preserve the
original failure through existing ownership/cleanup classification, then
retain completed Evidence before snapshot fallback. Existing producer
observations retain precedence. Remove the throw-only projection contract.

Validation: four authored filesystem controls fail unchanged f1edf0da0;
99 focused checks across six files, check:fast and docs:check pass. Covers
late cancellation, denied workspace cleanup and retry, opaque cancellation
between tables, exact physical cause, and CLI/MCP contracts. Public CLI
failure retains completed facts; original failure framing is unchanged.
Two source reviews found no blocker.

Twelve small normal-path CLI tasks preserve exact replies. Measurements
show no speedup; failure reply grows to retain Evidence (cl100k estimate
189 to 696). No real decoder, whole-agent, or large-case memory claim.
Heavy tests and profiling remain stopped.

Fixes #4880
This commit is contained in:
morluto committed 2026-10-18 04:29:41 +08:00
1 parent f1edf0da0c
commit 34a88d6c16
4 files changed
+320 -160

No files matched your search

+85 -5
View File
@@ -168,6 +168,74 @@ const apkFixture = async (
};
};
it.each([
[false, false],
[true, false],
[false, true],
[true, true],
])(
"retains completed decode facts when a later locale read fails (cleanup denied: %s, late cancellation: %s)",
async (denyCleanup, lateCancellation) => {
const { command, root } = await writeApktoolStub((path) =>
decodeStub()(path).replace(
" printf 'I: Using Apktool on stub",
" printf '\\377' > \"$5/res/values-de/strings.xml\"\n printf 'I: Using Apktool on stub",
),
);
const apk = await apkFixture(root);
let denied = denyCleanup;
const controller = new AbortController();
const provider = new ApktoolProvider({
environment: { REA_APKTOOL_COMMAND: command },
removeWorkspace: async (path) => {
if (lateCancellation) controller.abort({ stage: "cleanup" });
if (denied) throw new Error("authored 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, locales: ["de"] },
},
{ signal: controller.signal },
);
if (result.ok) throw new Error("Expected invalid UTF-8 failure");
expect((result.error.cleanupPrimary ?? result.error).message).toContain(
"UTF-8",
);
expect(result.error.cleanupIncomplete).toBe(denyCleanup);
const observation = {
operation: "decode_android_resources",
parameters: { path: apk.path, locales: ["de"] },
subject: { local_path: apk.path, digest: { sha256: apk.sha256 } },
normalized_result: {
metadata: { package_name: "com.rea.apktool.probe" },
manifest: `${REAL_MANIFEST}\n`,
strings: [
{ name: "api_endpoint", value: "https://api.example.com/v1" },
{ name: "app_name", value: "ApktoolProbe" },
],
string_tables: [{ locale: null, status: "present", total_count: 2 }],
coverage: "partial",
},
};
expect(result.error.partialObservation).toMatchObject(observation);
expect(projectAnalysisError(result.error)).toMatchObject({
details: { partial_observation: observation },
});
denied = false;
await provider.close();
expect(result.error.cleanupIncomplete).toBe(false);
expect(result.error.partialObservation).toMatchObject(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`,
@@ -1077,11 +1145,23 @@ it.each(["decode", "projection"] as const)(
expect(result.error.capturedOutput?.stderr).toContain(
"Could not decode the APK",
);
expect(result.error.partialObservation).toMatchObject({
kind: "file-snapshot-hash",
path: apk.path,
sha256: apk.sha256,
});
expect(result.error.partialObservation).toMatchObject(
stage === "decode"
? {
kind: "file-snapshot-hash",
path: apk.path,
sha256: apk.sha256,
}
: {
operation: "decode_android_resources",
subject: { local_path: apk.path, digest: { sha256: apk.sha256 } },
normalized_result: {
coverage: "partial",
metadata: { package_name: "com.rea.apktool.probe" },
strings: [],
},
},
);
} finally {
await provider.close();
}
+47 -28
View File
@@ -522,6 +522,7 @@ 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;
try {
const identity = await resources.snapshotInput(
input.path,
@@ -591,12 +592,15 @@ export class ApktoolProvider {
);
throwIfCancelled(operation, signal);
const projectedStrings = await projectApktoolStringTables(
const projection = await projectApktoolStringTables(
resources,
workspace,
input,
signal,
);
const projectedStrings = projection.ok
? projection.value
: projection.error.partial;
limitations.push(...projectedStrings.limitations);
const coverage =
projectedStrings.coverage === "partial" ||
@@ -626,35 +630,38 @@ export class ApktoolProvider {
`Decoded workspace enumeration stopped after ${String(MAX_DECODED_ENTRIES_VISITED)} entries; decoded_file_count and locales may be partial`,
);
}
result = ok(
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,
},
manifest,
strings: projectedStrings.strings,
locale: input.locale ?? null,
locales: files.locales,
...(input.locales === undefined
? {}
: { string_tables: projectedStrings.tables }),
decoded_file_count: files.count,
coverage,
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,
},
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 =
@@ -689,6 +696,18 @@ export class ApktoolProvider {
operation,
).bindCleanupDependencies([primary, failure])
: primary;
if (
partialExecution !== undefined &&
retained.partialObservation === undefined
)
retained.retainPartialObservation(
createAnalysisExecutionEvidence(
operation,
input,
partialExecution,
"observed",
),
);
if (
resources.snapshot !== undefined &&
retained.partialObservation === undefined
+60 -22
View File
@@ -21,7 +21,7 @@ const textRead = (text: string) => ({
describe("physical Apktool string facts", () => {
it("reads one physical table for two caller-selected aliases", async () => {
const paths: string[] = [];
const result = await projectApktoolStringTables(
const outcome = await projectApktoolStringTables(
{
async readText(path) {
paths.push(path);
@@ -31,6 +31,8 @@ describe("physical Apktool string facts", () => {
"decoded",
{ include_strings: true, locale: "pt-BR", locales: ["pt-rBR"] },
);
if (!outcome.ok) throw outcome.error.cause;
const result = outcome.value;
expect(paths).toEqual([
join("decoded", "res", "values-pt-rBR", "strings.xml"),
]);
@@ -42,7 +44,7 @@ describe("physical Apktool string facts", () => {
});
it("counts complete source entries beyond the retained table prefix", async () => {
const result = await projectApktoolStringTables(
const outcome = await projectApktoolStringTables(
{
async readText() {
return textRead(
@@ -56,6 +58,8 @@ describe("physical Apktool string facts", () => {
"decoded",
{ include_strings: true },
);
if (!outcome.ok) throw outcome.error.cause;
const result = outcome.value;
expect(result.tables[0]).toMatchObject({
total_count: MAX_STRINGS + 1,
unparsed_count: 1,
@@ -69,7 +73,7 @@ describe("physical Apktool string facts", () => {
it("does not read unexamined tables after output exhaustion", async () => {
const paths: string[] = [];
const result = await projectApktoolStringTables(
const outcome = await projectApktoolStringTables(
{
async readText(path) {
paths.push(path);
@@ -81,6 +85,8 @@ describe("physical Apktool string facts", () => {
"decoded",
{ include_strings: true, locales: ["de", "es", "it", "fr"] },
);
if (!outcome.ok) throw outcome.error.cause;
const result = outcome.value;
expect(paths).toHaveLength(4);
expect(result.tables.at(-1)).toEqual({
locale: "fr",
@@ -98,7 +104,7 @@ describe("physical Apktool string facts", () => {
it("keeps a known empty alias present after the output budget is spent", async () => {
const paths: string[] = [];
const result = await projectApktoolStringTables(
const outcome = await projectApktoolStringTables(
{
async readText(path) {
paths.push(path);
@@ -114,6 +120,8 @@ describe("physical Apktool string facts", () => {
locales: ["de", "es", "it", "fr", "en", "pt-rBR"],
},
);
if (!outcome.ok) throw outcome.error.cause;
const result = outcome.value;
expect(paths.filter((path) => path.includes("values-pt-rBR"))).toHaveLength(
1,
);
@@ -130,8 +138,37 @@ describe("physical Apktool string facts", () => {
});
describe("Apktool string projection admission", () => {
it("honors omitted strings without reading any selected table", async () => {
it("retains completed tables when cancellation arrives before the next selection", async () => {
const controller = new AbortController();
const reason = { selected: "caller" };
const paths: string[] = [];
const result = await projectApktoolStringTables(
{
async readText(path) {
paths.push(path);
controller.abort(reason);
return textRead(xml(1));
},
},
"decoded",
{ include_strings: true, locales: ["de", "fr"] },
controller.signal,
);
if (result.ok) throw new Error("Expected cancellation");
expect(result.error.cause).toBeInstanceOf(AnalysisCancelledError);
expect(result.error.cause).toHaveProperty("cause", reason);
expect(paths).toHaveLength(1);
expect(result.error.partial).toMatchObject({
coverage: "partial",
tables: [{ locale: null, total_count: 1 }],
});
expect(result.error.partial.limitations).toContain(
'String projection stopped at locale "de"; this selection and 1 remaining selections were not projected',
);
});
it("honors omitted strings without reading any selected table", async () => {
const outcome = await projectApktoolStringTables(
{
async readText() {
throw new Error("Unexpected read");
@@ -140,27 +177,26 @@ describe("Apktool string projection admission", () => {
"decoded",
{ include_strings: false, locale: "de", locales: ["fr"] },
);
expect(result).toEqual({
strings: [],
tables: [],
limitations: [],
coverage: "complete",
expect(outcome).toEqual({
ok: true,
value: { strings: [], tables: [], limitations: [], coverage: "complete" },
});
});
it("preserves the exact physical read failure", async () => {
const cause = new Error("Physical read and close failed");
await expect(
projectApktoolStringTables(
{
async readText() {
throw cause;
},
const result = await projectApktoolStringTables(
{
async readText() {
throw cause;
},
"decoded",
{ include_strings: true },
),
).rejects.toBe(cause);
},
"decoded",
{ include_strings: true },
);
if (result.ok) throw new Error("Expected read failure");
expect(result.error.cause).toBe(cause);
expect(result.error.partial.coverage).toBe("partial");
});
it("preserves opaque cancellation before any table read", async () => {
@@ -175,7 +211,9 @@ describe("Apktool string projection admission", () => {
{ include_strings: true },
AbortSignal.abort(reason),
);
await expect(pending).rejects.toBeInstanceOf(AnalysisCancelledError);
await expect(pending).rejects.toHaveProperty("cause", reason);
const result = await pending;
if (result.ok) throw new Error("Expected cancellation");
expect(result.error.cause).toBeInstanceOf(AnalysisCancelledError);
expect(result.error.cause).toHaveProperty("cause", reason);
});
});
+128 -105
View File
@@ -1,4 +1,5 @@
import { join } from "node:path";
import { ok, err, type Result } from "../domain/result.js";
import {
AGGREGATE_STRINGS_XML_MAX_BYTES,
@@ -124,9 +125,19 @@ export const projectApktoolStringTables = async (
workspace: string,
input: Selection,
signal?: AbortSignal,
): Promise<ApktoolStringProjection> => {
): Promise<
Result<
ApktoolStringProjection,
{ readonly cause: unknown; readonly partial: ApktoolStringProjection }
>
> => {
if (!input.include_strings)
return { strings: [], tables: [], limitations: [], coverage: "complete" };
return ok({
strings: [],
tables: [],
limitations: [],
coverage: "complete",
});
const primary = input.locale ?? null;
const locales = [...new Set([primary, ...(input.locales ?? [])])];
const physical = new Map<string, PhysicalTable>();
@@ -135,118 +146,130 @@ export const projectApktoolStringTables = async (
let xmlBytes = 0;
let outputEntries = 0;
let aggregateCapped = false;
for (const [index, locale] of locales.entries()) {
if (signal?.aborted)
throw new AnalysisCancelledError("decode_android_resources", {
cause: signal.reason,
});
const directory =
locale === null
? "values"
: `values-${androidLocaleDirectorySuffix(locale)}`;
const label =
locale === null
? "default res/values/strings.xml"
: `res/${directory}/strings.xml`;
let fact = physical.get(directory);
if (fact === undefined) {
const budget = Math.min(
STRINGS_XML_MAX_BYTES,
AGGREGATE_STRINGS_XML_MAX_BYTES - xmlBytes,
const projection = (): ApktoolStringProjection => {
if (aggregateCapped)
limitations.add(
"Aggregate string table projection stopped due to resource limits",
);
if (budget <= 0 || outputEntries >= MAX_AGGREGATE_STRINGS) {
return {
strings: tables[0]?.strings ?? [],
tables,
limitations: [...limitations],
coverage: tables.some((table) => table.coverage === "partial")
? "partial"
: "complete",
};
};
let pendingIndex = 0;
try {
for (const [index, locale] of locales.entries()) {
pendingIndex = index;
if (signal?.aborted)
throw new AnalysisCancelledError("decode_android_resources", {
cause: signal.reason,
});
const directory =
locale === null
? "values"
: `values-${androidLocaleDirectorySuffix(locale)}`;
const label =
locale === null
? "default res/values/strings.xml"
: `res/${directory}/strings.xml`;
let fact = physical.get(directory);
if (fact === undefined) {
const budget = Math.min(
STRINGS_XML_MAX_BYTES,
AGGREGATE_STRINGS_XML_MAX_BYTES - xmlBytes,
);
if (budget <= 0 || outputEntries >= MAX_AGGREGATE_STRINGS) {
tables.push({
locale,
status: "skipped",
strings: [],
total_count: null,
unparsed_count: null,
coverage: "partial",
});
limitations.add(
`Projection for ${label} skipped: aggregate resource limit reached`,
);
aggregateCapped = true;
continue;
}
const read = await readTable(
resources,
join(workspace, "res", directory, "strings.xml"),
budget,
signal,
);
fact = read.fact;
xmlBytes += read.bytes;
if (fact.status === "oversized" && budget < STRINGS_XML_MAX_BYTES)
aggregateCapped = true;
physical.set(directory, fact);
}
if (fact.status !== "present") {
tables.push({
locale,
status: "skipped",
status: fact.status,
strings: [],
total_count: null,
unparsed_count: null,
coverage: "partial",
total_count: fact.totalCount,
unparsed_count: fact.unparsedCount,
coverage: fact.status === "missing" ? "complete" : "partial",
});
limitations.add(
unavailableTableLimitation(fact, directory, locale, input),
);
continue;
}
// Batch output includes the primary strings twice, in the legacy field and
// in string_tables. Charge that representation before selecting entries.
const copies = index === 0 && input.locales !== undefined ? 2 : 1;
const remaining = Math.max(0, MAX_AGGREGATE_STRINGS - outputEntries);
const selected = fact.entries.slice(0, Math.floor(remaining / copies));
outputEntries += selected.length * copies;
const skipped = selected.length === 0 && fact.totalCount > 0;
const limited = selected.length < Math.min(fact.totalCount, MAX_STRINGS);
if (limited) aggregateCapped = true;
tables.push({
locale,
status: skipped ? "skipped" : "present",
strings: selected,
// Output exhaustion cannot erase counts from an already examined table.
total_count: fact.totalCount,
unparsed_count: fact.unparsedCount,
coverage:
selected.length < fact.totalCount || fact.unparsedCount > 0
? "partial"
: "complete",
});
if (skipped)
limitations.add(
`Projection for ${label} skipped: aggregate resource limit reached`,
);
aggregateCapped = true;
continue;
}
const read = await readTable(
resources,
join(workspace, "res", directory, "strings.xml"),
budget,
signal,
);
fact = read.fact;
xmlBytes += read.bytes;
if (fact.status === "oversized" && budget < STRINGS_XML_MAX_BYTES)
aggregateCapped = true;
physical.set(directory, fact);
if (fact.unparsedCount > 0)
limitations.add(
input.locales === undefined || locale === null
? `String entries omitted due to unsupported markup or missing names: ${String(fact.unparsedCount)}`
: `String entries omitted in locale ${locale} due to unsupported markup or missing names: ${String(fact.unparsedCount)}`,
);
if (limited && !skipped)
limitations.add(
`Locale ${locale ?? "default"} strings reports ${String(selected.length)} entries due to aggregate strings limit`,
);
else if (fact.totalCount > MAX_STRINGS && !skipped)
limitations.add(
input.locales === undefined || locale === null
? `strings reports the first ${String(selected.length)} of ${String(fact.totalCount)} entries`
: `Locale ${locale} strings reports the first ${String(selected.length)} of ${String(fact.totalCount)} entries`,
);
}
if (fact.status !== "present") {
tables.push({
locale,
status: fact.status,
strings: [],
total_count: fact.totalCount,
unparsed_count: fact.unparsedCount,
coverage: fact.status === "missing" ? "complete" : "partial",
});
limitations.add(
unavailableTableLimitation(fact, directory, locale, input),
);
continue;
}
// Batch output includes the primary strings twice, in the legacy field and
// in string_tables. Charge that representation before selecting entries.
const copies = index === 0 && input.locales !== undefined ? 2 : 1;
const remaining = Math.max(0, MAX_AGGREGATE_STRINGS - outputEntries);
const selected = fact.entries.slice(0, Math.floor(remaining / copies));
outputEntries += selected.length * copies;
const skipped = selected.length === 0 && fact.totalCount > 0;
const limited = selected.length < Math.min(fact.totalCount, MAX_STRINGS);
if (limited) aggregateCapped = true;
tables.push({
locale,
status: skipped ? "skipped" : "present",
strings: selected,
// Output exhaustion cannot erase counts from an already examined table.
total_count: fact.totalCount,
unparsed_count: fact.unparsedCount,
coverage:
selected.length < fact.totalCount || fact.unparsedCount > 0
? "partial"
: "complete",
});
if (skipped)
limitations.add(
`Projection for ${label} skipped: aggregate resource limit reached`,
);
if (fact.unparsedCount > 0)
limitations.add(
input.locales === undefined || locale === null
? `String entries omitted due to unsupported markup or missing names: ${String(fact.unparsedCount)}`
: `String entries omitted in locale ${locale} due to unsupported markup or missing names: ${String(fact.unparsedCount)}`,
);
if (limited && !skipped)
limitations.add(
`Locale ${locale ?? "default"} strings reports ${String(selected.length)} entries due to aggregate strings limit`,
);
else if (fact.totalCount > MAX_STRINGS && !skipped)
limitations.add(
input.locales === undefined || locale === null
? `strings reports the first ${String(selected.length)} of ${String(fact.totalCount)} entries`
: `Locale ${locale} strings reports the first ${String(selected.length)} of ${String(fact.totalCount)} entries`,
);
}
if (aggregateCapped)
} catch (cause) {
limitations.add(
"Aggregate string table projection stopped due to resource limits",
`String projection stopped at locale ${JSON.stringify(locales[pendingIndex])}; this selection and ${String(locales.length - pendingIndex - 1)} remaining selections were not projected`,
);
return {
strings: tables[0]?.strings ?? [],
tables,
limitations: [...limitations],
coverage: tables.some((table) => table.coverage === "partial")
? "partial"
: "complete",
};
return err({ cause, partial: { ...projection(), coverage: "partial" } });
}
return ok(projection());
};