[GH-ISSUE #5978] [Bug] Apktool classifies an APK changed during snapshot copying as invalid_request #3646

New issue

Closed

opened 2026-10-31 00:42:41 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 00:42:41 +00:00

Owner

Originally created by @coygeek on GitHub (Oct 21, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/5978

Before submitting

I searched the open and closed issues for a related report.
I removed credentials, tokens, private paths, and sensitive target data.

Area

Native analysis or decompilation

Observed behavior

The public decode service returns invalid_request, category invalid_input, retryable: false, and schema-correction guidance when the selected APK changes during its private snapshot copy. There are no rejected fields to correct. The internal cause identifies the file changing during copying, but the public result tells the caller to inspect the input schema. The decoder is never started.

Reproduction

A focused production-boundary test executed the real ApktoolResourceAnalysisService, ApktoolProvider, workspace and file reads. An inert launcher passes version admission. A wrapper around the real chunk reader appends to the actual source after the first chunk is read; this controls the race without fabricating a domain error. No real Apktool or JVM is used.

Save the following as src/apktool/apkCopyClassification.test.ts and run node node_modules/vitest/vitest.mjs run src/apktool/apkCopyClassification.test.ts --maxWorkers 1 in a checkout with locked dependencies installed. The test asserts the current incorrect result and passed once. The temporary-directory prefix below is portable; all behavior and assertions match the executed test.

import { appendFile, chmod, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises';
import { join } from 'node:path';
import { tmpdir } from 'node:os';
import { expect, test, vi } from 'vitest';

const state = vi.hoisted(() => ({ source: '', changed: false }));
vi.mock('../artifacts/readArtifactFileChunks.js', async (importOriginal) => {
  const actual = await importOriginal<typeof import('../artifacts/readArtifactFileChunks.js')>();
  return { ...actual, readArtifactFileChunks: async function* (...args: Parameters<typeof actual.readArtifactFileChunks>) {
    for await (const chunk of actual.readArtifactFileChunks(...args)) {
      if (!state.changed) {
        state.changed = true;
        await appendFile(state.source, 'changed-during-copy');
      }
      yield chunk;
    }
  }};
});
import { ApktoolProvider } from './ApktoolProvider.js';
import { ApktoolResourceAnalysisService } from '../application/apktool/ApktoolResourceAnalysisService.js';
import { projectAnalysisError } from '../domain/analysisErrorProjection.js';

test('changed admitted APK reaches public decode service as invalid_request', async () => {
  const root = await mkdtemp(join(tmpdir(), 'rea-apk-copy-'));
  const command = join(root, 'apktool');
  const marker = join(root, 'decoded-marker');
  const source = join(root, 'input.apk');
  await writeFile(source, 'APK fixture');
  await writeFile(command, `#!/bin/sh\nif [ "$1" = "--version" ]; then printf '3.0.3\\n'; exit 0; fi\nprintf called > '${marker}'\nexit 1\n`);
  await chmod(command, 0o755);
  state.source = source;
  state.changed = false;
  const provider = new ApktoolProvider({ environment: { REA_APKTOOL_COMMAND: command } });
  try {
    const result = await new ApktoolResourceAnalysisService(provider).execute('decode_android_resources', { path: source });
    expect(state.changed).toBe(true);
    expect(result.ok).toBe(false);
    if (result.ok) throw new Error('expected failed acquisition');
    const error = projectAnalysisError(result.error);
    console.log('CHANGED_APK', JSON.stringify(error));
    expect(error.code).toBe('invalid_request');
    expect(error.retryable).toBe(false);
    expect(error.details).toBeUndefined();
    expect((result.error.cause as Error).message).toContain('changed while being copied');
    await expect(readFile(marker)).rejects.toMatchObject({ code: 'ENOENT' });
  } finally {
    await provider.close();
    await rm(root, { recursive: true, force: true });
  }
});

The source fixture need not be a valid APK because rejection occurs before decoder invocation. The absent decoder marker confirms this boundary.

Expected behavior

Use the established acquisition-change outcome: artifact_changed, category integrity_mismatch, retryable: true, with selected path/reason and guidance to wait for a stable file. AnalysisArtifactChangedError defines precisely this condition; the ADB private input snapshot uses it for the same copy-time change.

Impact and scope

P2: the machine-readable classification marks a transient acquisition race as non-retryable caller input failure and directs recovery toward schema correction. Waiting for the input writer to finish and repeating the request is a practical workaround. Refusal still prevents decoding unstable input. Runtime frequency under natural concurrent writes is unknown; only a controlled append during copying was exercised. No corrupted success result or partial decode was observed.

REA version

rea-agents 6.7.0; source 5241597594.

Runtime and platform

Node.js v24.22.0; macOS 27.0.1 arm64; one focused Vitest 5.0.3 test using locked source dependencies.

Target and artifact context

A small local APK-named byte fixture and inert version-reporting launcher. The source is appended during snapshot acquisition. No application, device or real decoder is executed.

Interface

Other

Earliest failure stage

Capture or observation

Relevant output or evidence

Observed projected error from the service:

{
  "code": "invalid_request",
  "category": "invalid_input",
  "message": "Analysis input is invalid. Check the arguments and try again.",
  "retryable": false,
  "remediation": {
    "action": "Read the declared input schema or CLI usage for decode_android_resources; if using MCP and the catalog may be stale, call tools/list. Correct the listed fields, then retry. Do not repeat the unchanged call."
  },
  "recovery": {"kind":"inspect_input_contract","operation":"decode_android_resources"}
}

The same test asserted that the internal cause contains changed while being copied and that the decode marker does not exist. One test passed; the temporary test was removed after execution.

Typed outcome or recovery expected

An AnalysisArtifactChangedError for decode_android_resources, preserving the selected APK path and acquisition-change reason. The existing projection already supplies retryability and stable-file recovery. Keep unstable-input refusal and pre-decode cleanup.

Analysis or suspected cause

Source-derived cause: both snapshot stability checks throw AnalysisInputError. The catch preserves an admitted failure, and decode invokes snapshotInput before runApktool. AnalysisInputError is defined as application parsing failure. The artifact-change projection includes the stable-artifact boundary and is retryable. The executed append tests final stability validation; the earlier copied-byte overflow branch is source-derived.

Additional context

Read related #3340 (decode refusal target path), #4768 (successful Evidence subject), #4812 (file/workspace finalization), #4880/#4882 (completed decode facts lost after output-read failures), #3347 (version deadline) and #5177 / PR #5180 (launcher execute permissions). Their fixes do not change pre-decode snapshot-change classification. Checked upstream main 25e1a24a3e; the relevant workspace/provider path remains unchanged. No CLI/MCP transport, real decoder or uncontrolled writer timing was exercised.

Originally created by @coygeek on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5978 ### Before submitting - [x] I searched the open and closed issues for a related report. - [x] I removed credentials, tokens, private paths, and sensitive target data. ### Area Native analysis or decompilation ### Observed behavior The public decode service returns `invalid_request`, category `invalid_input`, `retryable: false`, and schema-correction guidance when the selected APK changes during its private snapshot copy. There are no rejected fields to correct. The internal cause identifies the file changing during copying, but the public result tells the caller to inspect the input schema. The decoder is never started. ### Reproduction A focused production-boundary test executed the real `ApktoolResourceAnalysisService`, `ApktoolProvider`, workspace and file reads. An inert launcher passes version admission. A wrapper around the real chunk reader appends to the actual source after the first chunk is read; this controls the race without fabricating a domain error. No real Apktool or JVM is used. Save the following as `src/apktool/apkCopyClassification.test.ts` and run `node node_modules/vitest/vitest.mjs run src/apktool/apkCopyClassification.test.ts --maxWorkers 1` in a checkout with locked dependencies installed. The test asserts the current incorrect result and passed once. The temporary-directory prefix below is portable; all behavior and assertions match the executed test. ```ts import { appendFile, chmod, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; import { expect, test, vi } from 'vitest'; const state = vi.hoisted(() => ({ source: '', changed: false })); vi.mock('../artifacts/readArtifactFileChunks.js', async (importOriginal) => { const actual = await importOriginal<typeof import('../artifacts/readArtifactFileChunks.js')>(); return { ...actual, readArtifactFileChunks: async function* (...args: Parameters<typeof actual.readArtifactFileChunks>) { for await (const chunk of actual.readArtifactFileChunks(...args)) { if (!state.changed) { state.changed = true; await appendFile(state.source, 'changed-during-copy'); } yield chunk; } }}; }); import { ApktoolProvider } from './ApktoolProvider.js'; import { ApktoolResourceAnalysisService } from '../application/apktool/ApktoolResourceAnalysisService.js'; import { projectAnalysisError } from '../domain/analysisErrorProjection.js'; test('changed admitted APK reaches public decode service as invalid_request', async () => { const root = await mkdtemp(join(tmpdir(), 'rea-apk-copy-')); const command = join(root, 'apktool'); const marker = join(root, 'decoded-marker'); const source = join(root, 'input.apk'); await writeFile(source, 'APK fixture'); await writeFile(command, `#!/bin/sh\nif [ "$1" = "--version" ]; then printf '3.0.3\\n'; exit 0; fi\nprintf called > '${marker}'\nexit 1\n`); await chmod(command, 0o755); state.source = source; state.changed = false; const provider = new ApktoolProvider({ environment: { REA_APKTOOL_COMMAND: command } }); try { const result = await new ApktoolResourceAnalysisService(provider).execute('decode_android_resources', { path: source }); expect(state.changed).toBe(true); expect(result.ok).toBe(false); if (result.ok) throw new Error('expected failed acquisition'); const error = projectAnalysisError(result.error); console.log('CHANGED_APK', JSON.stringify(error)); expect(error.code).toBe('invalid_request'); expect(error.retryable).toBe(false); expect(error.details).toBeUndefined(); expect((result.error.cause as Error).message).toContain('changed while being copied'); await expect(readFile(marker)).rejects.toMatchObject({ code: 'ENOENT' }); } finally { await provider.close(); await rm(root, { recursive: true, force: true }); } }); ``` The source fixture need not be a valid APK because rejection occurs before decoder invocation. The absent decoder marker confirms this boundary. ### Expected behavior Use the established acquisition-change outcome: `artifact_changed`, category `integrity_mismatch`, `retryable: true`, with selected path/reason and guidance to wait for a stable file. [AnalysisArtifactChangedError defines precisely this condition](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/domain/analysisErrorCore.ts#L204-L215); [the ADB private input snapshot uses it for the same copy-time change](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/adb/AdbInputSnapshot.ts#L124-L132). ### Impact and scope P2: the machine-readable classification marks a transient acquisition race as non-retryable caller input failure and directs recovery toward schema correction. Waiting for the input writer to finish and repeating the request is a practical workaround. Refusal still prevents decoding unstable input. Runtime frequency under natural concurrent writes is unknown; only a controlled append during copying was exercised. No corrupted success result or partial decode was observed. ### REA version rea-agents 6.7.0; source 52415975942a4b9c53a817915b313ccc404f280e. ### Runtime and platform Node.js v24.22.0; macOS 27.0.1 arm64; one focused Vitest 5.0.3 test using locked source dependencies. ### Target and artifact context A small local APK-named byte fixture and inert version-reporting launcher. The source is appended during snapshot acquisition. No application, device or real decoder is executed. ### Interface Other ### Earliest failure stage Capture or observation ### Relevant output or evidence Observed projected error from the service: ```json { "code": "invalid_request", "category": "invalid_input", "message": "Analysis input is invalid. Check the arguments and try again.", "retryable": false, "remediation": { "action": "Read the declared input schema or CLI usage for decode_android_resources; if using MCP and the catalog may be stale, call tools/list. Correct the listed fields, then retry. Do not repeat the unchanged call." }, "recovery": {"kind":"inspect_input_contract","operation":"decode_android_resources"} } ``` The same test asserted that the internal cause contains `changed while being copied` and that the decode marker does not exist. One test passed; the temporary test was removed after execution. ### Typed outcome or recovery expected An `AnalysisArtifactChangedError` for `decode_android_resources`, preserving the selected APK path and acquisition-change reason. The existing projection already supplies retryability and stable-file recovery. Keep unstable-input refusal and pre-decode cleanup. ### Analysis or suspected cause Source-derived cause: [both snapshot stability checks throw AnalysisInputError](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/apktool/ApktoolWorkspace.ts#L83-L120). [The catch preserves an admitted failure](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/apktool/ApktoolWorkspace.ts#L133-L141), and [decode invokes snapshotInput before runApktool](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/apktool/ApktoolProvider.ts#L542-L570). [AnalysisInputError is defined as application parsing failure](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/domain/analysisErrorCore.ts#L64-L76). The artifact-change [projection includes the stable-artifact boundary](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/domain/analysisErrorProjection.ts#L311-L317) and [is retryable](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/domain/analysisErrorProjection.ts#L594-L600). The executed append tests final stability validation; the earlier copied-byte overflow branch is source-derived. ### Additional context Read related #3340 (decode refusal target path), #4768 (successful Evidence subject), #4812 (file/workspace finalization), #4880/#4882 (completed decode facts lost after output-read failures), #3347 (version deadline) and #5177 / PR #5180 (launcher execute permissions). Their fixes do not change pre-decode snapshot-change classification. Checked upstream main 25e1a24a3ed3671a0148757555f18d16c29d936a; the relevant workspace/provider path remains unchanged. No CLI/MCP transport, real decoder or uncontrolled writer timing was exercised.

bsakaguchi closed this issue

2026-10-31 00:42:41 +00:00

bsakaguchi referenced this issue

2026-11-01 21:10:58 +00:00

[GH-ISSUE #3796] [Tracker] Open feature requests by area (October 2026) #1471