[GH-ISSUE #6047] [Bug] Android snapshot close failures erase primary errors and lose descriptor ownership #3692

New issue

Closed

opened 2026-10-31 00:43:07 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 00:43:07 +00:00

Owner

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

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

A controlled destination-close EIO during Android APK snapshot finalization replaces the primary copy-change error with generic execution_failure. A close failure after an otherwise clean snapshot copy does the same. Through the real AndroidAnalysisService and JadxProvider, both fault cases leave the real destination descriptor open after provider.close(), while the returned error has cleanupIncomplete: false. The provider still owns the workspace and attempts its removal; the confirmed missing owner is the descriptor.

Reproduction

Executed a focused production-boundary test using the repository's createJadxProtocolFixture. Real APK/JAR admission, file handles, snapshot copying, error projection and provider retirement run. The fixture supplies an inert Java prerequisite probe; no JVM/JADX analysis starts. A wrapper intercepts only the exclusive destination open for target.apk. Appending to the actual source immediately after its initial stat triggers copy-change validation. The close wrapper then injects persistent EIO without releasing the actual descriptor. Each handle is explicitly reclaimed by the test afterward.

Save as tests/boundary/process/androidSnapshotClose.test.ts and run node node_modules/vitest/vitest.mjs run tests/boundary/process/androidSnapshotClose.test.ts --maxWorkers 1. The final execution passed all three cases.

import { type FileHandle } from 'node:fs/promises';
import { expect, test, vi } from 'vitest';
const state = vi.hoisted(() => ({
  enabled: false, change: false, fail: false, source: '', closeCalls: 0,
  handles: [] as { handle: FileHandle; close: () => Promise<void> }[],
}));
vi.mock('node:fs/promises', async (importOriginal) => {
  const actual = await importOriginal<typeof import('node:fs/promises')>();
  return { ...actual, open: async (...args: Parameters<typeof actual.open>) => {
    const handle = await actual.open(...args);
    if (state.enabled && String(args[0]).endsWith('/target.apk') && args[1] === 'wx') {
      if (state.change) await actual.appendFile(state.source, 'changed');
      const close = handle.close.bind(handle);
      state.handles.push({ handle, close });
      handle.close = async () => {
        state.closeCalls++;
        if (state.fail) throw Object.assign(new Error('controlled snapshot close EIO'), { code: 'EIO' });
        return close();
      };
    }
    return handle;
  }};
});
import { createJadxProtocolFixture, verifyJadxFixtureCleanup } from '../../fixtures/android/jadx.js';
import { projectAnalysisError } from '../../../src/domain/analysisErrorProjection.js';

for (const mode of ['changed-control', 'changed-close-failure', 'completed-close-failure']) {
  test(mode, async () => {
    const fixture = await createJadxProtocolFixture();
    state.source = fixture.apk;
    state.change = mode !== 'completed-close-failure';
    state.fail = mode !== 'changed-control';
    state.enabled = true;
    state.closeCalls = 0;
    state.handles = [];
    try {
      const result = await fixture.service.execute('inspect_android_package', { path: fixture.apk });
      expect(result.ok).toBe(false);
      if (result.ok) throw new Error('expected failure');
      const projected = projectAnalysisError(result.error);
      console.log('SNAPSHOT_RESULT', mode, JSON.stringify(projected));
      if (mode === 'changed-control') {
        expect(projected.code).toBe('invalid_request');
        expect(result.error.cause?.constructor.name).toBe('AndroidSnapshotChangedError');
        expect(state.handles[0]?.handle.fd).toBe(-1);
      } else {
        expect(projected.code).toBe('execution_failure');
        expect(result.error.cause).toMatchObject({ code: 'EIO', message: 'controlled snapshot close EIO' });
        expect(result.error.cleanupIncomplete).not.toBe(true);
        expect(state.handles[0]?.handle.fd).toBeGreaterThanOrEqual(0);
      }
      expect(fixture.launches).toHaveLength(0);
      const closeCallsBeforeShutdown = state.closeCalls;
      await fixture.provider.close();
      console.log('SNAPSHOT_CLEANUP', mode, JSON.stringify({ closeCalls: state.closeCalls, fdOpen: state.handles[0]?.handle.fd !== -1, cleanupIncomplete: result.error.cleanupIncomplete }));
      if (state.fail) {
        expect(state.closeCalls).toBe(closeCallsBeforeShutdown);
        expect(state.handles[0]?.handle.fd).toBeGreaterThanOrEqual(0);
      }
    } finally {
      state.enabled = false;
      state.fail = false;
      for (const owned of state.handles) await owned.close();
      await fixture.provider.close();
      await verifyJadxFixtureCleanup(fixture.launches);
    }
  });
}

Expected behavior

Keep the operation outcome and unresolved descriptor owner together. A close failure must preserve the primary copy-change diagnosis or the completed copy facts, expose typed cleanup state, and remain available to provider cleanup/retry. The lifecycle contract explicitly requires this. OwnedRegularFile already retains outcome and owner when close rejects, and JadxFailure handles its typed cleanup composition. The stable-file diagnosis should remain identifiable even when cleanup also fails; changing the existing standalone copy-change classification is a separate question.

Impact and scope

P2: a failed snapshot close can erase the reason admission failed and leave a live descriptor without a retry owner in a long-running process. Restarting the process releases its descriptors; callers can stabilize the input before another attempt. No spontaneous filesystem EIO frequency was measured. The executed cases fault only the target snapshot destination. Source-handle and engine-JAR close failures share the same producer but were not separately injected. No analysis result, target code execution, or permanent snapshot-file leak was demonstrated.

REA version

rea-agents 6.7.0; source revision 5241597594.

Runtime and platform

Node.js v26.12.0; macOS 27.0.1 arm64; Vitest 5.0.3 with locked dependencies. Source service/provider boundary, real filesystem, controlled close rejection and simulated Java prerequisite probe.

Target and artifact context

The repository's generated APK/JAR protocol fixtures. The APK is appended during its private snapshot copy for two cases; the third copy remains stable. No Android device or real decompiler is used.

Interface

Other

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

Observed final focused run: 3 tests passed. Selected facts:

Case	Projected code	Destination after provider shutdown	Cleanup incomplete
Source appended; close succeeds	`invalid_request` with internal `AndroidSnapshotChangedError`	Closed	false
Source appended; close rejects	`execution_failure`, diagnostics reason `controlled snapshot close EIO`	Still open	false
Clean copy; close rejects	`execution_failure`, diagnostics reason `controlled snapshot close EIO`	Still open	false

The two fault results retain the EIO as their direct cause, without the earlier change error. Finalization called the destination close wrapper twice in this Node runtime; provider shutdown made no further close attempt. The tests verify the actual FileHandle.fd remains open, then close it explicitly. The engine launcher was never called in any case.

Typed outcome or recovery expected

A composed primary/cleanup failure carrying exact descriptor ownership, preserving either the original operation diagnosis or completed snapshot information. Mark unresolved cleanup accurately and retry the retained owner before discarding its workspace or accepting replacement work.

Analysis or suspected cause

copyAndroidSnapshot closes raw FileHandles in finally. A rejection from the awaited close replaces the pending primary exception. The target mapping and engine mapping recognize only AndroidSnapshotChangedError, so the close error passes through instead. JadxProvider calls both snapshots during admission and captures their failure before retirement, but raw snapshot handles never enter its retained descriptor set. The control and fault outputs establish this failure through a callable application operation, not solely an isolated helper.

Additional context

Read #5140 (process/capture FilesystemSnapshot), #4266 (shared stable hashing), #4598 (digest/cancellation provenance), #4835 (JADX retirement composition), #1441 (fixture teardown diagnostics), and #4240 (CLI composition). They affect different owners or already-fixed shared helpers. PR #4274 changes retirement and ADB input snapshots, not AndroidTargetSnapshot; #4503/#4493/#1960/#4345 address other producers or tests. Inspected #4887's relevant diff: the snapshot producer is unchanged. #6024 adds deadline guidance. #5978 concerns Apktool's separate copying implementation; #5971 and #5981 concern input/probe diagnostics. Fetched upstream main 6d356b8434; the snapshot producer remains unchanged. Public CLI/MCP transport and naturally occurring OS close failures were not exercised.

Originally created by @coygeek on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/6047 ### 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 A controlled destination-close EIO during Android APK snapshot finalization replaces the primary copy-change error with generic `execution_failure`. A close failure after an otherwise clean snapshot copy does the same. Through the real AndroidAnalysisService and JadxProvider, both fault cases leave the real destination descriptor open after `provider.close()`, while the returned error has `cleanupIncomplete: false`. The provider still owns the workspace and attempts its removal; the confirmed missing owner is the descriptor. ### Reproduction Executed a focused production-boundary test using the repository's `createJadxProtocolFixture`. Real APK/JAR admission, file handles, snapshot copying, error projection and provider retirement run. The fixture supplies an inert Java prerequisite probe; no JVM/JADX analysis starts. A wrapper intercepts only the exclusive destination open for `target.apk`. Appending to the actual source immediately after its initial stat triggers copy-change validation. The close wrapper then injects persistent EIO without releasing the actual descriptor. Each handle is explicitly reclaimed by the test afterward. Save as `tests/boundary/process/androidSnapshotClose.test.ts` and run `node node_modules/vitest/vitest.mjs run tests/boundary/process/androidSnapshotClose.test.ts --maxWorkers 1`. The final execution passed all three cases. ```ts import { type FileHandle } from 'node:fs/promises'; import { expect, test, vi } from 'vitest'; const state = vi.hoisted(() => ({ enabled: false, change: false, fail: false, source: '', closeCalls: 0, handles: [] as { handle: FileHandle; close: () => Promise<void> }[], })); vi.mock('node:fs/promises', async (importOriginal) => { const actual = await importOriginal<typeof import('node:fs/promises')>(); return { ...actual, open: async (...args: Parameters<typeof actual.open>) => { const handle = await actual.open(...args); if (state.enabled && String(args[0]).endsWith('/target.apk') && args[1] === 'wx') { if (state.change) await actual.appendFile(state.source, 'changed'); const close = handle.close.bind(handle); state.handles.push({ handle, close }); handle.close = async () => { state.closeCalls++; if (state.fail) throw Object.assign(new Error('controlled snapshot close EIO'), { code: 'EIO' }); return close(); }; } return handle; }}; }); import { createJadxProtocolFixture, verifyJadxFixtureCleanup } from '../../fixtures/android/jadx.js'; import { projectAnalysisError } from '../../../src/domain/analysisErrorProjection.js'; for (const mode of ['changed-control', 'changed-close-failure', 'completed-close-failure']) { test(mode, async () => { const fixture = await createJadxProtocolFixture(); state.source = fixture.apk; state.change = mode !== 'completed-close-failure'; state.fail = mode !== 'changed-control'; state.enabled = true; state.closeCalls = 0; state.handles = []; try { const result = await fixture.service.execute('inspect_android_package', { path: fixture.apk }); expect(result.ok).toBe(false); if (result.ok) throw new Error('expected failure'); const projected = projectAnalysisError(result.error); console.log('SNAPSHOT_RESULT', mode, JSON.stringify(projected)); if (mode === 'changed-control') { expect(projected.code).toBe('invalid_request'); expect(result.error.cause?.constructor.name).toBe('AndroidSnapshotChangedError'); expect(state.handles[0]?.handle.fd).toBe(-1); } else { expect(projected.code).toBe('execution_failure'); expect(result.error.cause).toMatchObject({ code: 'EIO', message: 'controlled snapshot close EIO' }); expect(result.error.cleanupIncomplete).not.toBe(true); expect(state.handles[0]?.handle.fd).toBeGreaterThanOrEqual(0); } expect(fixture.launches).toHaveLength(0); const closeCallsBeforeShutdown = state.closeCalls; await fixture.provider.close(); console.log('SNAPSHOT_CLEANUP', mode, JSON.stringify({ closeCalls: state.closeCalls, fdOpen: state.handles[0]?.handle.fd !== -1, cleanupIncomplete: result.error.cleanupIncomplete })); if (state.fail) { expect(state.closeCalls).toBe(closeCallsBeforeShutdown); expect(state.handles[0]?.handle.fd).toBeGreaterThanOrEqual(0); } } finally { state.enabled = false; state.fail = false; for (const owned of state.handles) await owned.close(); await fixture.provider.close(); await verifyJadxFixtureCleanup(fixture.launches); } }); } ``` ### Expected behavior Keep the operation outcome and unresolved descriptor owner together. A close failure must preserve the primary copy-change diagnosis or the completed copy facts, expose typed cleanup state, and remain available to provider cleanup/retry. [The lifecycle contract explicitly requires this](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/tool-design.md#L132-L140). [OwnedRegularFile already retains outcome and owner when close rejects](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/filesystem/OwnedRegularFile.ts#L119-L198), and [JadxFailure handles its typed cleanup composition](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/android/JadxFailure.ts#L115-L143). The stable-file diagnosis should remain identifiable even when cleanup also fails; changing the existing standalone copy-change classification is a separate question. ### Impact and scope P2: a failed snapshot close can erase the reason admission failed and leave a live descriptor without a retry owner in a long-running process. Restarting the process releases its descriptors; callers can stabilize the input before another attempt. No spontaneous filesystem EIO frequency was measured. The executed cases fault only the target snapshot destination. Source-handle and engine-JAR close failures share the same producer but were not separately injected. No analysis result, target code execution, or permanent snapshot-file leak was demonstrated. ### REA version rea-agents 6.7.0; source revision 52415975942a4b9c53a817915b313ccc404f280e. ### Runtime and platform Node.js v26.12.0; macOS 27.0.1 arm64; Vitest 5.0.3 with locked dependencies. Source service/provider boundary, real filesystem, controlled close rejection and simulated Java prerequisite probe. ### Target and artifact context The repository's generated APK/JAR protocol fixtures. The APK is appended during its private snapshot copy for two cases; the third copy remains stable. No Android device or real decompiler is used. ### Interface Other ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence Observed final focused run: 3 tests passed. Selected facts: | Case | Projected code | Destination after provider shutdown | Cleanup incomplete | | --- | --- | --- | --- | | Source appended; close succeeds | `invalid_request` with internal `AndroidSnapshotChangedError` | Closed | false | | Source appended; close rejects | `execution_failure`, diagnostics reason `controlled snapshot close EIO` | Still open | false | | Clean copy; close rejects | `execution_failure`, diagnostics reason `controlled snapshot close EIO` | Still open | false | The two fault results retain the EIO as their direct cause, without the earlier change error. Finalization called the destination close wrapper twice in this Node runtime; provider shutdown made no further close attempt. The tests verify the actual `FileHandle.fd` remains open, then close it explicitly. The engine launcher was never called in any case. ### Typed outcome or recovery expected A composed primary/cleanup failure carrying exact descriptor ownership, preserving either the original operation diagnosis or completed snapshot information. Mark unresolved cleanup accurately and retry the retained owner before discarding its workspace or accepting replacement work. ### Analysis or suspected cause [copyAndroidSnapshot closes raw FileHandles in finally](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/android/AndroidTargetSnapshot.ts#L130-L142). A rejection from the awaited close replaces the pending primary exception. [The target mapping](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/android/AndroidTargetSnapshot.ts#L32-L41) and [engine mapping](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/android/AndroidTargetSnapshot.ts#L58-L66) recognize only AndroidSnapshotChangedError, so the close error passes through instead. [JadxProvider calls both snapshots during admission](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/android/JadxProvider.ts#L513-L527) and [captures their failure before retirement](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/android/JadxProvider.ts#L588-L618), but raw snapshot handles never enter its retained descriptor set. The control and fault outputs establish this failure through a callable application operation, not solely an isolated helper. ### Additional context Read #5140 (process/capture FilesystemSnapshot), #4266 (shared stable hashing), #4598 (digest/cancellation provenance), #4835 (JADX retirement composition), #1441 (fixture teardown diagnostics), and #4240 (CLI composition). They affect different owners or already-fixed shared helpers. PR #4274 changes retirement and ADB input snapshots, not AndroidTargetSnapshot; #4503/#4493/#1960/#4345 address other producers or tests. Inspected #4887's relevant diff: the snapshot producer is unchanged. #6024 adds deadline guidance. #5978 concerns Apktool's separate copying implementation; #5971 and #5981 concern input/probe diagnostics. Fetched upstream main 6d356b84344659085f2b912857b5e27fe0160c9b; the snapshot producer remains unchanged. Public CLI/MCP transport and naturally occurring OS close failures were not exercised.

bsakaguchi closed this issue

2026-10-31 00:43:07 +00:00

bsakaguchi referenced this issue

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

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