morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[GH-ISSUE #6079] [Bug] pull_adb_file omits the acquired-file receipt after a digest read failure #3716

New issue

Closed

opened 2026-10-31 00:43:19 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

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

Owner

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

Before submitting

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

Area

Evidence, artifacts, or reconstruction

Observed behavior

After a successful single-file pull writes real output, an injected physical digest-read EIO returns AnalysisOutputError / unreadable_output without partialObservation or projected details.partial_observation. The file still contains the transferred bytes. The producing catch block already constructs the acquired-file observation but omits it from this fallback error.

This was executed through AdbDeviceAnalysisService and the production AdbProvider, using simulated ADB process responses and real host files. No device or real ADB server was used.

Reproduction

With the source checkout and locked dependencies, save this focused test as tests/process-global/adbDigest.repro.test.ts and run node node_modules/vitest/vitest.mjs run tests/process-global/adbDigest.repro.test.ts --maxWorkers=1. The process port supplies version/pull responses as in the existing ADB boundary tests. Only the selected acquired file's real descriptor read is faulted; cleanup succeeds and no cancellation is sent.

import { writeFile, readFile } from 'node:fs/promises';
import { join } from 'node:path';
import { afterEach, expect, it, vi } from 'vitest';
import { createTestTempDirectory } from '../fixtures/temporaryDirectory.js';
import { AdbProvider } from '../../src/adb/AdbProvider.js';
import { AdbDeviceAnalysisService } from '../../src/application/adb/AdbDeviceAnalysisService.js';
import * as regularFile from '../../src/filesystem/RegularFile.js';
import { execFileBytes, execFileOutput } from '../../src/process/ExecFileOutput.js';
import { projectAnalysisError } from '../../src/domain/analysisErrorProjection.js';
vi.mock('../../src/process/ExecFileOutput.js', async (importOriginal) => ({
  ...(await importOriginal<typeof import('../../src/process/ExecFileOutput.js')>()),
  execFileBytes: vi.fn(), execFileOutput: vi.fn(),
}));
afterEach(() => vi.restoreAllMocks());
it.each([false, true])('ADB physical digest read failure=%s', async fail => {
  const root = await createTestTempDirectory('rea-adb-read-');
  const path = join(root, 'payload.bin');
  vi.mocked(execFileOutput).mockResolvedValue({stdout: 'Android Debug Bridge version 1.0.41\nVersion 34.0.5-stub\n', stderr: '', stdoutBytes: 66, stderrBytes: 0});
  vi.mocked(execFileBytes).mockImplementation(async (_binary, args) => {
    if (!args.includes('pull')) throw new Error('unexpected command');
    await writeFile(args.at(-1)!, 'pulled bytes');
    return {stdout: Buffer.alloc(0), stderr: Buffer.alloc(0)};
  });
  const physical = Object.assign(new Error('controlled physical digest EIO'), {code:'EIO'});
  const nativeOpen = regularFile.openRegularFile;
  if (fail) vi.spyOn(regularFile, 'openRegularFile').mockImplementation(async (selected, options) => {
    const handle = await nativeOpen(selected, options);
    if (selected === path) vi.spyOn(handle, 'read').mockRejectedValueOnce(physical);
    return handle;
  });
  const provider = new AdbProvider({environment:{REA_ADB_PATH:join(root, 'adb')}});
  try {
    const result = await new AdbDeviceAnalysisService(provider).execute('pull_adb_file', {serial:'device', device_path:'/sdcard/payload.bin', output_directory:root});
    expect(await readFile(path, 'utf8')).toBe('pulled bytes');
    if (fail) {
      if (result.ok) throw new Error('expected failure');
      const projected = projectAnalysisError(result.error);
      expect(result.error._tag).toBe('AnalysisOutputError');
      expect(result.error.partialObservation).toBeUndefined();
      expect(projected.details?.partial_observation).toBeUndefined();
      console.log('ADB_DIGEST', JSON.stringify({fail, tag:result.error._tag, code:projected.code, partial:projected.details?.partial_observation ?? null, fileRetained:true}));
    } else {
      if (!result.ok) throw result.error;
      expect(result.value.normalized_result).toMatchObject({bytes:12, local_path:path});
      console.log('ADB_DIGEST', JSON.stringify({fail, succeeded:true, fileRetained:true}));
    }
  } finally { await provider.close(); }
});

Expected behavior

Retain the known acquired-file facts on the digest error: provider, operation, client, serial, selected device path, local output path, and digest_status: "unknown". AdbAcquiredFilePartialObservation explicitly covers digest-verification failures, and the lifecycle contract requires preserving completed observations.

Impact and scope

P2: callers lose the structured acquisition receipt although output remains in the caller-selected directory. This breaks automated handling of unverified acquisitions. The controlled test independently reads the retained bytes; users can locate the explicit output and examine or hash it separately. No file corruption, device mutation, or data loss was observed. Spontaneous read-failure frequency and device behavior remain unknown.

REA version

rea-agents 6.7.0; source revision 5241597594.

Runtime and platform

Node.js v26.12.0; macOS 27.0.1 arm64; locked Vitest 5.0.3. Simulated ADB process port, production application/provider/filesystem boundaries, real output file and controlled read EIO. The pinned Node 24.18.0 runtime was not exercised.

Target and artifact context

A simulated device file selected as /sdcard/payload.bin. The pull response writes 12 bytes, pulled bytes, into the caller-selected output directory.

Interface

Other

Earliest failure stage

Capture or observation

Relevant output or evidence

The no-fault control returns successful Evidence with the correct 12-byte length. With only digest read EIO injected, the observed selected fields are:

{"error_tag":"AnalysisOutputError","code":"unreadable_output","partial_observation":null,"file_retained":true}

Here null records an absent field, rather than a field returned by REA. Both the internal error property and projected field are undefined. Reading the actual output afterwards returns pulled bytes. The final focused run passed eight related lifecycle controls; these two exercise this path.

Typed outcome or recovery expected

Keep the typed output failure and attach the already-constructed acquired-file partial observation with digest status unknown. Do not assert a verified digest or convert the physical failure to cancellation.

Analysis or suspected cause

The single-file catch constructs partialObservation and uses it for cleanup/cancellation cases, then creates the ordinary AnalysisOutputError with only cause. The analogous bugreport fallback retains its observation. The correction belongs at the single-file producer; the generic error projection cannot reconstruct the omitted acquisition.

Additional context

Fetched upstream main f86206535f; the single-file fallback is unchanged. ADB listing/device-side checksum changes on that main are unrelated; the newer runtime was not rebuilt. #4598 fixes physical-failure provenance versus late cancellation, #6046 concerns all-failed APK aggregation, #4643 concerns serial/path attribution, and #4276 concerns raw-partial recovery after SDK cancellation. This failure has no cancellation and needs its existing observation passed to the fallback constructor. PR #4274 does not change that omission. Concurrent file-change failures follow the same source fallback but were not separately executed.

Originally created by @coygeek on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/6079 ### 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 Evidence, artifacts, or reconstruction ### Observed behavior After a successful single-file pull writes real output, an injected physical digest-read EIO returns AnalysisOutputError / `unreadable_output` without `partialObservation` or projected `details.partial_observation`. The file still contains the transferred bytes. The producing catch block already constructs the acquired-file observation but omits it from this fallback error. This was executed through AdbDeviceAnalysisService and the production AdbProvider, using simulated ADB process responses and real host files. No device or real ADB server was used. ### Reproduction With the source checkout and locked dependencies, save this focused test as `tests/process-global/adbDigest.repro.test.ts` and run `node node_modules/vitest/vitest.mjs run tests/process-global/adbDigest.repro.test.ts --maxWorkers=1`. The process port supplies version/pull responses as in the existing ADB boundary tests. Only the selected acquired file's real descriptor read is faulted; cleanup succeeds and no cancellation is sent. ```ts import { writeFile, readFile } from 'node:fs/promises'; import { join } from 'node:path'; import { afterEach, expect, it, vi } from 'vitest'; import { createTestTempDirectory } from '../fixtures/temporaryDirectory.js'; import { AdbProvider } from '../../src/adb/AdbProvider.js'; import { AdbDeviceAnalysisService } from '../../src/application/adb/AdbDeviceAnalysisService.js'; import * as regularFile from '../../src/filesystem/RegularFile.js'; import { execFileBytes, execFileOutput } from '../../src/process/ExecFileOutput.js'; import { projectAnalysisError } from '../../src/domain/analysisErrorProjection.js'; vi.mock('../../src/process/ExecFileOutput.js', async (importOriginal) => ({ ...(await importOriginal<typeof import('../../src/process/ExecFileOutput.js')>()), execFileBytes: vi.fn(), execFileOutput: vi.fn(), })); afterEach(() => vi.restoreAllMocks()); it.each([false, true])('ADB physical digest read failure=%s', async fail => { const root = await createTestTempDirectory('rea-adb-read-'); const path = join(root, 'payload.bin'); vi.mocked(execFileOutput).mockResolvedValue({stdout: 'Android Debug Bridge version 1.0.41\nVersion 34.0.5-stub\n', stderr: '', stdoutBytes: 66, stderrBytes: 0}); vi.mocked(execFileBytes).mockImplementation(async (_binary, args) => { if (!args.includes('pull')) throw new Error('unexpected command'); await writeFile(args.at(-1)!, 'pulled bytes'); return {stdout: Buffer.alloc(0), stderr: Buffer.alloc(0)}; }); const physical = Object.assign(new Error('controlled physical digest EIO'), {code:'EIO'}); const nativeOpen = regularFile.openRegularFile; if (fail) vi.spyOn(regularFile, 'openRegularFile').mockImplementation(async (selected, options) => { const handle = await nativeOpen(selected, options); if (selected === path) vi.spyOn(handle, 'read').mockRejectedValueOnce(physical); return handle; }); const provider = new AdbProvider({environment:{REA_ADB_PATH:join(root, 'adb')}}); try { const result = await new AdbDeviceAnalysisService(provider).execute('pull_adb_file', {serial:'device', device_path:'/sdcard/payload.bin', output_directory:root}); expect(await readFile(path, 'utf8')).toBe('pulled bytes'); if (fail) { if (result.ok) throw new Error('expected failure'); const projected = projectAnalysisError(result.error); expect(result.error._tag).toBe('AnalysisOutputError'); expect(result.error.partialObservation).toBeUndefined(); expect(projected.details?.partial_observation).toBeUndefined(); console.log('ADB_DIGEST', JSON.stringify({fail, tag:result.error._tag, code:projected.code, partial:projected.details?.partial_observation ?? null, fileRetained:true})); } else { if (!result.ok) throw result.error; expect(result.value.normalized_result).toMatchObject({bytes:12, local_path:path}); console.log('ADB_DIGEST', JSON.stringify({fail, succeeded:true, fileRetained:true})); } } finally { await provider.close(); } }); ``` ### Expected behavior Retain the known acquired-file facts on the digest error: provider, operation, client, serial, selected device path, local output path, and `digest_status: "unknown"`. [AdbAcquiredFilePartialObservation](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/domain/adb/adbDeviceAnalysis.ts#L809-L839) explicitly covers digest-verification failures, and [the lifecycle contract](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/tool-design.md#L119-L136) requires preserving completed observations. ### Impact and scope P2: callers lose the structured acquisition receipt although output remains in the caller-selected directory. This breaks automated handling of unverified acquisitions. The controlled test independently reads the retained bytes; users can locate the explicit output and examine or hash it separately. No file corruption, device mutation, or data loss was observed. Spontaneous read-failure frequency and device behavior remain unknown. ### REA version rea-agents 6.7.0; source revision 52415975942a4b9c53a817915b313ccc404f280e. ### Runtime and platform Node.js v26.12.0; macOS 27.0.1 arm64; locked Vitest 5.0.3. Simulated ADB process port, production application/provider/filesystem boundaries, real output file and controlled read EIO. The pinned Node 24.18.0 runtime was not exercised. ### Target and artifact context A simulated device file selected as /sdcard/payload.bin. The pull response writes 12 bytes, `pulled bytes`, into the caller-selected output directory. ### Interface Other ### Earliest failure stage Capture or observation ### Relevant output or evidence The no-fault control returns successful Evidence with the correct 12-byte length. With only digest read EIO injected, the observed selected fields are: ```json {"error_tag":"AnalysisOutputError","code":"unreadable_output","partial_observation":null,"file_retained":true} ``` Here `null` records an absent field, rather than a field returned by REA. Both the internal error property and projected field are undefined. Reading the actual output afterwards returns `pulled bytes`. The final focused run passed eight related lifecycle controls; these two exercise this path. ### Typed outcome or recovery expected Keep the typed output failure and attach the already-constructed acquired-file partial observation with digest status unknown. Do not assert a verified digest or convert the physical failure to cancellation. ### Analysis or suspected cause The [single-file catch](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/adb/AdbProvider.ts#L2351-L2375) constructs partialObservation and uses it for cleanup/cancellation cases, then creates the ordinary AnalysisOutputError with only cause. The [analogous bugreport fallback](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/adb/AdbProvider.ts#L3162-L3179) retains its observation. The correction belongs at the single-file producer; the generic error projection cannot reconstruct the omitted acquisition. ### Additional context Fetched upstream main f86206535f8ec71014781f9d432a84b01e183e5d; the single-file fallback is unchanged. ADB listing/device-side checksum changes on that main are unrelated; the newer runtime was not rebuilt. #4598 fixes physical-failure provenance versus late cancellation, #6046 concerns all-failed APK aggregation, #4643 concerns serial/path attribution, and #4276 concerns raw-partial recovery after SDK cancellation. This failure has no cancellation and needs its existing observation passed to the fallback constructor. PR #4274 does not change that omission. Concurrent file-change failures follow the same source fallback but were not separately executed.

bsakaguchi closed this issue

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

bsakaguchi commented

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

Author

Owner

@coygeek commented on GitHub (Oct 24, 2026):

At d87e73f2db50be72d33a4b3c7199af35518b441c, the ordinary post-pull digest failure now passes the already-built acquired-file receipt to AnalysisOutputError (catch and fallback); shared projection emits partial_observation. The receipt keeps the selected paths, client/serial and digest_status unknown.

I replayed the report's physical digest-read failure with restoration assertions, plus its successful-read control. Both pass: the acquired file remains present, and the EIO failure now retains the internal acquired-file receipt and the same projected partial_observation, including digest_status: "unknown".

I ran the cited regression files on this revision; all passed as part of the focused 28-file, 603-test run. Provider-facing checks here use controlled fixtures; this does not claim a run with the real analysis engine. Closing as completed on main.

@coygeek commented on GitHub (Oct 24, 2026): At `d87e73f2db50be72d33a4b3c7199af35518b441c`, the ordinary post-pull digest failure now passes the already-built acquired-file receipt to AnalysisOutputError ([catch and fallback](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/adb/AdbProvider.ts#L2424)); [shared projection](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/domain/analysisErrorProjection.ts#L127) emits partial_observation. The receipt keeps the selected paths, client/serial and digest_status unknown. I replayed the report's physical digest-read failure with restoration assertions, plus its successful-read control. Both pass: the acquired file remains present, and the EIO failure now retains the internal acquired-file receipt and the same projected `partial_observation`, including `digest_status: "unknown"`. I ran the cited regression files on this revision; all passed as part of the focused 28-file, 603-test run. Provider-facing checks here use controlled fixtures; this does not claim a run with the real analysis engine. Closing as completed on main.

bsakaguchi referenced this issue

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

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

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#3716

No description provided.

Rows
Columns