From 23f984b1a884baefc3c44e4ab697f6eff61dffb6 Mon Sep 17 00:00:00 2001 From: gensokyo-dev <342580529+gensokyo-dev@users.noreply.github.com> Date: Sun, 18 Oct 2026 20:46:25 +0800 Subject: [PATCH] fix(tests): restore Android prerequisite verification Supply a real retained resource owner to every Android verifier prerequisite check, sharing the existing Ghidra verifier cleanup scope. Match recovery checks to typed Java or archive failures and applicable remediation, preserving exact close retry authority and primary failures. Assisted-by: Reimu (gensokyo-skills:reimu-incident-triage) --- docs/android-analysis.md | 8 ++ .../lib/ghidra-installation-inspection.mjs | 29 +---- .../lib/jadx-configuration-inspection.d.mts | 11 ++ scripts/lib/jadx-configuration-inspection.mjs | 17 +++ scripts/lib/verifier-file-resources.d.mts | 6 + scripts/lib/verifier-file-resources.mjs | 27 ++++ scripts/verify/android/analysis.mjs | 8 +- scripts/verify/android/failure-recovery.mjs | 56 +++++++-- scripts/verify/android/windows-lifecycle.mjs | 10 +- .../process/verifierFileResources.test.ts | 118 +++++++++++++++++- 10 files changed, 242 insertions(+), 48 deletions(-) create mode 100644 scripts/lib/jadx-configuration-inspection.d.mts create mode 100644 scripts/lib/jadx-configuration-inspection.mjs diff --git a/docs/android-analysis.md b/docs/android-analysis.md index 49642d195..50de2ad6c 100644 --- a/docs/android-analysis.md +++ b/docs/android-analysis.md @@ -363,6 +363,14 @@ export REA_ANDROID_TEST_APK="$PWD/_reference/apk-integration/ApiDemos-debug.apk" npm run verify:android ``` +The main lane and Windows Java-selection checks validate the selected JDK and +engine archive through `validateVerifierJadxConfiguration`. This shared verifier +boundary supplies the production validator's resource owner and waits for +retained cleanup before proceeding. Failed closes retain their exact owners for +retry; prerequisite failures must not be mistaken for completed bridge tests. +Recovery checks match the typed Java or archive failure and its applicable +remediation. + The lane also uses the selected full JDK's `jlink` to create a disposable JRE without the compiler module. It checks real Java startup failures, missing selectors, recovery after failures, and CLI cancellation with owned cleanup. diff --git a/scripts/lib/ghidra-installation-inspection.mjs b/scripts/lib/ghidra-installation-inspection.mjs index ad0b0bf62..e87822984 100644 --- a/scripts/lib/ghidra-installation-inspection.mjs +++ b/scripts/lib/ghidra-installation-inspection.mjs @@ -1,28 +1,9 @@ -import { ArtifactResourceScope } from "../../dist/artifacts/ArtifactResourceScope.js"; -import { AnalysisCleanupLease } from "../../dist/artifacts/AnalysisCleanupLease.js"; import { inspectGhidraInstallation } from "../../dist/ghidra/GhidraInstallation.js"; -import { closeVerifierResources } from "./verifier-cleanup.mjs"; +import { withVerifierResourceOwner } from "./verifier-file-resources.mjs"; /** Complete one prerequisite inspection and release its exact retained owners. */ -export async function inspectVerifierGhidraInstallation(options, host) { - const resources = new ArtifactResourceScope(); - const resourceOwner = { - retain(owner, resource) { - resources.retain({ - kind: "lease", - resource, - lease: AnalysisCleanupLease.forCloseOwner(owner, resource), - }); - }, - run: (operation) => resources.run(operation), - }; - let primaryError; - try { - return await inspectGhidraInstallation({ ...options, resourceOwner }, host); - } catch (cause) { - primaryError = cause; - throw cause; - } finally { - await closeVerifierResources([() => resources.close()], primaryError); - } +export function inspectVerifierGhidraInstallation(options, host) { + return withVerifierResourceOwner((resourceOwner) => + inspectGhidraInstallation({ ...options, resourceOwner }, host), + ); } diff --git a/scripts/lib/jadx-configuration-inspection.d.mts b/scripts/lib/jadx-configuration-inspection.d.mts new file mode 100644 index 000000000..3e7cfee65 --- /dev/null +++ b/scripts/lib/jadx-configuration-inspection.d.mts @@ -0,0 +1,11 @@ +import type { validateJadxConfiguration } from "../../src/android/JadxConfiguration.js"; + +type ConfigurationArguments = Parameters; + +/** Validate prerequisites with a real owner, joining cleanup before completion. */ +export function validateVerifierJadxConfiguration( + configuration: ConfigurationArguments[0], + environment: ConfigurationArguments[1], + operation: ConfigurationArguments[2], + options?: Omit, +): ReturnType; diff --git a/scripts/lib/jadx-configuration-inspection.mjs b/scripts/lib/jadx-configuration-inspection.mjs new file mode 100644 index 000000000..c2131b85a --- /dev/null +++ b/scripts/lib/jadx-configuration-inspection.mjs @@ -0,0 +1,17 @@ +import { validateJadxConfiguration } from "../../dist/android/JadxConfiguration.js"; +import { withVerifierResourceOwner } from "./verifier-file-resources.mjs"; + +/** Validate Java and the engine archive before beginning real verification. */ +export function validateVerifierJadxConfiguration( + configuration, + environment, + operation, + options = {}, +) { + return withVerifierResourceOwner((resourceOwner) => + validateJadxConfiguration(configuration, environment, operation, { + ...options, + resourceOwner, + }), + ); +} diff --git a/scripts/lib/verifier-file-resources.d.mts b/scripts/lib/verifier-file-resources.d.mts index e47d043e4..82df77d65 100644 --- a/scripts/lib/verifier-file-resources.d.mts +++ b/scripts/lib/verifier-file-resources.d.mts @@ -1,4 +1,10 @@ import type { ArtifactResourceScope } from "../../src/artifacts/ArtifactResourceScope.js"; +import type { FileCloseResourceOwner } from "../../src/application/AnalysisProvider.js"; + +/** Complete a prerequisite operation, including retained cleanup on failure. */ +export function withVerifierResourceOwner( + operation: (owner: FileCloseResourceOwner) => Promise, +): Promise; /** Hash one verifier input and transfer failed-close ownership to its scope. */ export function hashVerifierFile( diff --git a/scripts/lib/verifier-file-resources.mjs b/scripts/lib/verifier-file-resources.mjs index 42f4264fe..4536d9bae 100644 --- a/scripts/lib/verifier-file-resources.mjs +++ b/scripts/lib/verifier-file-resources.mjs @@ -1,4 +1,31 @@ import { hashStableFile } from "../../dist/filesystem/StableFileHash.js"; +import { ArtifactResourceScope } from "../../dist/artifacts/ArtifactResourceScope.js"; +import { AnalysisCleanupLease } from "../../dist/artifacts/AnalysisCleanupLease.js"; +import { closeVerifierResources } from "./verifier-cleanup.mjs"; + +/** Finish a prerequisite probe and release its exact retained cleanup owners. */ +export async function withVerifierResourceOwner(operation) { + const resources = new ArtifactResourceScope(); + const resourceOwner = { + retain(owner, resource) { + resources.retain({ + kind: "lease", + resource, + lease: AnalysisCleanupLease.forCloseOwner(owner, resource), + }); + }, + run: (work) => resources.run(work), + }; + let primaryError; + try { + return await operation(resourceOwner); + } catch (cause) { + primaryError = cause; + throw cause; + } finally { + await closeVerifierResources([() => resources.close()], primaryError); + } +} /** Hash one verifier input while retaining any descriptor whose close fails. */ export function hashVerifierFile(path, resources) { diff --git a/scripts/verify/android/analysis.mjs b/scripts/verify/android/analysis.mjs index baf875545..515e7300e 100644 --- a/scripts/verify/android/analysis.mjs +++ b/scripts/verify/android/analysis.mjs @@ -14,10 +14,8 @@ import { verifyAndroidCliCancellation } from "./cli-cancellation.mjs"; import { verifyAndroidMetadata } from "./metadata.mjs"; import { verifyAndroidClassSearch } from "./class-search.mjs"; import { verifyAndroidFailureRecovery } from "./failure-recovery.mjs"; -import { - resolveJadxConfigurationInputs, - validateJadxConfiguration, -} from "../../../dist/android/JadxConfiguration.js"; +import { resolveJadxConfigurationInputs } from "../../../dist/android/JadxConfiguration.js"; +import { validateVerifierJadxConfiguration } from "../../lib/jadx-configuration-inspection.mjs"; import { verifyWindowsAndroidLifecycle } from "./windows-lifecycle.mjs"; const repository = resolve(dirname(fileURLToPath(import.meta.url)), "../../.."); @@ -43,7 +41,7 @@ const configuration = await resolveJadxConfigurationInputs( process.env, "inspect_android_package", ); -await validateJadxConfiguration( +await validateVerifierJadxConfiguration( configuration, process.env, "inspect_android_package", diff --git a/scripts/verify/android/failure-recovery.mjs b/scripts/verify/android/failure-recovery.mjs index 8e1816aed..c1fac49c6 100644 --- a/scripts/verify/android/failure-recovery.mjs +++ b/scripts/verify/android/failure-recovery.mjs @@ -17,6 +17,7 @@ import { delimiter, dirname, isAbsolute, join } from "node:path"; import { Client } from "@modelcontextprotocol/client"; import { StdioClientTransport } from "@modelcontextprotocol/client/stdio"; import { TOOL_CONTRACTS } from "../../../dist/contracts/toolContracts.js"; +import { analysisErrorProjectionSchema } from "../../../dist/contracts/errorSchemas.js"; /** Real unusable Java runtimes and selectors must lead to applicable recovery. */ export async function verifyAndroidFailureRecovery({ @@ -103,18 +104,46 @@ export async function verifyAndroidFailureRecovery({ label: "JRE without compiler", env: { ...environment, JAVA_HOME: jre }, observation: "jdk.compiler", + diagnostics: { + failure_kind: "java_runtime", + failure_code: "runtime_missing", + jdk_compiler: false, + }, + message: /full JDK/u, + remediation: /java --list-modules/u, }, { label: "missing java", env: { ...withoutJavaHome, PATH: empty }, observation: process.platform === "win32" ? "Cannot find java.exe" : "ENOENT", + diagnostics: { + failure_kind: "java_runtime", + failure_code: "runtime_missing", + ...(process.platform === "win32" + ? {} + : { process_error_code: "ENOENT" }), + }, + message: + process.platform === "win32" + ? /Cannot find java\.exe/u + : /selected Java runtime/u, + remediation: + process.platform === "win32" + ? /JAVA_HOME|PATH/u + : /selected launcher/u, }, { label: "invalid engine archive with a working JDK", env: { ...environment, REA_JADX_MCP_JAR: invalidJar }, observation: "Cannot inspect REA_JADX_MCP_JAR", - jarFailure: true, + diagnostics: { + failure_kind: "jar", + failure_code: "version_unresolved", + }, + message: /REA_JADX_MCP_JAR/u, + remediation: /jadx-headless-mcp/u, + forbiddenGuidance: /Select a full JDK/u, }, ]; if (process.platform === "darwin") { @@ -126,6 +155,12 @@ export async function verifyAndroidFailureRecovery({ label: "macOS Java stub", env: { ...withoutJavaHome, PATH: "/usr/bin:/bin" }, observation: "Unable to locate a Java Runtime", + diagnostics: { + failure_kind: "java_probe", + failure_code: "runtime_unavailable", + }, + message: /selected Java runtime/u, + remediation: /selected launcher/u, }); } } @@ -145,23 +180,20 @@ export async function verifyAndroidFailureRecovery({ assert.fail(`${scenario.label} unexpectedly succeeded`); } catch (cause) { assert.equal(cause.code, 1); - failure = JSON.parse(cause.stdout); + failure = analysisErrorProjectionSchema.parse(JSON.parse(cause.stdout)); } assert.equal(failure.code, "capability_unavailable"); assert.ok( JSON.stringify(failure).includes(scenario.observation), JSON.stringify(failure), ); - assert.match( - failure.message, - scenario.jarFailure ? /REA_JADX_MCP_JAR/u : /full JDK/u, - ); - assert.match( - failure.remediation.action, - scenario.jarFailure ? /jadx-headless-mcp/u : /java --list-modules/u, - ); - if (scenario.jarFailure) - assert.doesNotMatch(failure.message, /Select a full JDK/u); + assert.ok(failure.details.diagnostics); + for (const [key, value] of Object.entries(scenario.diagnostics)) + assert.deepEqual(failure.details.diagnostics[key], value); + assert.match(failure.message, scenario.message); + assert.match(failure.remediation.action, scenario.remediation); + if (scenario.forbiddenGuidance) + assert.doesNotMatch(failure.message, scenario.forbiddenGuidance); assert.doesNotMatch(JSON.stringify(failure), /rea doctor/u); await verifyAndroidReadiness({ entrypoint, diff --git a/scripts/verify/android/windows-lifecycle.mjs b/scripts/verify/android/windows-lifecycle.mjs index d2b281c60..b1ad8d830 100644 --- a/scripts/verify/android/windows-lifecycle.mjs +++ b/scripts/verify/android/windows-lifecycle.mjs @@ -13,10 +13,8 @@ import { dirname, join } from "node:path"; import { setTimeout as delay } from "node:timers/promises"; import { Client } from "@modelcontextprotocol/client"; import { StdioClientTransport } from "@modelcontextprotocol/client/stdio"; -import { - resolveJadxConfigurationInputs, - validateJadxConfiguration, -} from "../../../dist/android/JadxConfiguration.js"; +import { resolveJadxConfigurationInputs } from "../../../dist/android/JadxConfiguration.js"; +import { validateVerifierJadxConfiguration } from "../../lib/jadx-configuration-inspection.mjs"; import { verifyWindowsGuardianIsolation } from "./windows-guardian-isolation.mjs"; /** Verify real Windows cancellation, disconnect and abrupt CLI owner exit. */ @@ -45,7 +43,7 @@ async function verifyJavaSelection(environment) { environment, "inspect_android_package", ); - await validateJadxConfiguration( + await validateVerifierJadxConfiguration( selected, environment, "inspect_android_package", @@ -65,7 +63,7 @@ async function verifyJavaSelection(environment) { pathEnvironment, "inspect_android_package", ); - await validateJadxConfiguration( + await validateVerifierJadxConfiguration( onPath, pathEnvironment, "inspect_android_package", diff --git a/tests/boundary/process/verifierFileResources.test.ts b/tests/boundary/process/verifierFileResources.test.ts index 4c8bfbf28..8c1471ee9 100644 --- a/tests/boundary/process/verifierFileResources.test.ts +++ b/tests/boundary/process/verifierFileResources.test.ts @@ -1,13 +1,15 @@ import { spawnSync } from "node:child_process"; import { createHash } from "node:crypto"; -import { readFile, readdir, writeFile } from "node:fs/promises"; +import { chmod, readFile, readdir, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { expect, it } from "vitest"; import ts from "typescript6"; import type { inspectVerifierGhidraInstallation } from "../../../scripts/lib/ghidra-installation-inspection.mjs"; +import type { validateVerifierJadxConfiguration } from "../../../scripts/lib/jadx-configuration-inspection.mjs"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { writeJadxJarInventory } from "../../fixtures/android/jadxJar.js"; it("retains and retries the exact descriptor after a verifier hash close failure", async () => { const directory = await createTestTempDirectory("rea-verifier-file-"); @@ -134,6 +136,120 @@ it("Ghidra verifier probes await the shared owned inspection contract", async () expect(violations).toEqual([]); }); +it("Android verifier probes await the shared owned validation contract", async () => { + const paths = [ + "scripts/verify/android/analysis.mjs", + "scripts/verify/android/windows-lifecycle.mjs", + ]; + const probes = new Set(); + const violations: { path: string; reason: string }[] = []; + for (const path of paths) { + const source = ts.createSourceFile( + path, + await readFile(path, "utf8"), + ts.ScriptTarget.Latest, + true, + ts.ScriptKind.JS, + ); + const visit = (node: ts.Node): void => { + if ( + ts.isCallExpression(node) && + ts.isIdentifier(node.expression) && + [ + "validateJadxConfiguration", + "validateVerifierJadxConfiguration", + ].includes(node.expression.text) + ) { + probes.add(path); + if (!ts.isAwaitExpression(node.parent)) + violations.push({ path, reason: "validation was not awaited" }); + if (node.expression.text !== "validateVerifierJadxConfiguration") + violations.push({ path, reason: "validation has no cleanup owner" }); + } + ts.forEachChild(node, visit); + }; + visit(source); + } + expect([...probes].sort()).toEqual(paths.toSorted()); + expect(violations).toEqual([]); +}); + +it + .skipIf(process.platform === "win32") + .each(["clean", "retry-close", "retained-close"])( + "owns Android verifier prerequisite finalization: %s", + async (mode) => { + const root = await createTestTempDirectory("rea-verifier-jadx-owner-"); + const jar = join(root, "engine.jar"); + await writeJadxJarInventory(jar); + const java = join(root, "java"); + await writeFile( + java, + "#!/bin/sh\nprintf 'java.base@21\\njdk.compiler@21\\n'\n", + ); + await chmod(java, 0o700); + const configuration = { + java, + jar, + jvmArguments: [], + } satisfies Parameters[0]; + const script = String.raw` + import assert from 'node:assert/strict'; + import { validateVerifierJadxConfiguration } from ${JSON.stringify(new URL("../../../scripts/lib/jadx-configuration-inspection.mjs", import.meta.url).href)}; + import { OwnedFileHandle } from ${JSON.stringify(new URL("../../../dist/filesystem/OwnedFileHandle.js", import.meta.url).href)}; + import { ArtifactResourceRetryFailure } from ${JSON.stringify(new URL("../../../dist/artifacts/ArtifactResourceScope.js", import.meta.url).href)}; + const [serializedConfiguration, mode] = process.argv.slice(1); + const configuration = JSON.parse(serializedConfiguration); + const handles = new Set(); + let attempts = 0, denied = true; + const originalClose = OwnedFileHandle.prototype.close; + OwnedFileHandle.prototype.close = async function () { + handles.add(this.handle); attempts++; + if ((mode === 'retry-close' && attempts === 1) || (mode === 'retained-close' && denied)) throw new Error('controlled archive close refusal'); + return originalClose.call(this); + }; + let failure; + try { + await validateVerifierJadxConfiguration(configuration, process.env, 'inspect_android_package'); + } catch (cause) { failure = cause; } + try { + if (mode === 'clean') { assert.equal(failure, undefined); assert.equal(attempts, 1); } + else if (mode === 'retry-close') { + assert.ok(failure); assert.ok(attempts >= 2); + assert.ok(!(failure instanceof AggregateError), 'Successful retry reported unresolved cleanup'); + } else { + assert.ok(failure instanceof AggregateError); + assert.equal(failure.errors.length, 2, 'Primary and cleanup failures must both survive'); + const cleanup = failure.errors.find(error => error instanceof ArtifactResourceRetryFailure); + assert.ok(cleanup); assert.ok(cleanup.cleanupLeases.length > 0); + denied = false; + for (const lease of cleanup.cleanupLeases) await lease.release(); + } + assert.equal(handles.size, 1); + for (const handle of handles) assert.equal(handle.fd, -1); + console.log(JSON.stringify({ mode, verified: true })); + } finally { + denied = false; + OwnedFileHandle.prototype.close = originalClose; + for (const handle of handles) await handle.close(); + } + `; + const result = spawnSync( + process.execPath, + [ + "--input-type=module", + "-e", + script, + JSON.stringify(configuration), + mode, + ], + { encoding: "utf8", timeout: 15_000 }, + ); + expect(result.status, `${result.stdout}\n${result.stderr}`).toBe(0); + expect(JSON.parse(result.stdout.trim())).toEqual({ mode, verified: true }); + }, +); + it("firmware verifier loads its current hash API before reporting unavailable prerequisites", () => { const result = spawnSync( process.execPath,