From 461eb161282237ad3135f6d8f8fbbc560285548e Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Fri, 16 Oct 2026 05:44:20 +0800 Subject: [PATCH] fix(adb): retain cancellation at screen publication Stop before publishing when screencap returns after cancellation. If the file has already committed, preserve its path, digest and dimensions in cancellation Evidence. Reuse the execution-to-Evidence owner and remove a duplicate check. Validated both sides of publication using a valid PNG and real temporary filesystem writes; provider and process replies are simulated. Refs #4251. --- docs/adb-device-analysis.md | 3 + src/adb/AdbProvider.ts | 43 ++++--- .../adbScreenPublicationCancellation.test.ts | 119 ++++++++++++++++++ 3 files changed, 147 insertions(+), 18 deletions(-) create mode 100644 tests/boundary/providers/adb/adbScreenPublicationCancellation.test.ts diff --git a/docs/adb-device-analysis.md b/docs/adb-device-analysis.md index 539a6f847f..b18e7b4fbe 100644 --- a/docs/adb-device-analysis.md +++ b/docs/adb-device-analysis.md @@ -175,6 +175,9 @@ and during shutdown. - Cancellation during a package pull preserves caller-selected output. The error includes partial observations identifying completed artifacts and the interrupted local path; remaining APKs were not attempted. +- Screen capture checks cancellation before publishing its exclusive output. + If cancellation arrives after publication, the error retains the committed + file's path, digest and dimensions; the file remains available as evidence. - Bugreports collect into isolated staging and publish with exclusive creation; filename collisions select the next numeric suffix. If publication fails, the completed archive remains in staging. The error's partial observation gives diff --git a/src/adb/AdbProvider.ts b/src/adb/AdbProvider.ts index ea15a1fd1e..fc6ce5282e 100644 --- a/src/adb/AdbProvider.ts +++ b/src/adb/AdbProvider.ts @@ -2379,6 +2379,8 @@ export class AdbProvider { signal, ); if (!captured.ok) return captured; + if (signal?.aborted === true) + return err(new AnalysisCancelledError(operation)); const dimensions = parsePngDimensions(captured.value.stdout); if (dimensions.width === null) return err( @@ -2400,25 +2402,30 @@ export class AdbProvider { const sha256 = createHash("sha256") .update(captured.value.stdout) .digest("hex"); - return ok( - createAnalysisExecution( - { - client, - serial: input.serial, - local_path: localPath, - bytes: captured.value.stdout.byteLength, - sha256, - width: dimensions.width, - height: dimensions.height, - }, - provider, - { - limitations: [ - "The capture is one frame at read time; no recording or interaction occurred", - ], - }, - ), + const execution = createAnalysisExecution( + { + client, + serial: input.serial, + local_path: localPath, + bytes: captured.value.stdout.byteLength, + sha256, + width: dimensions.width, + height: dimensions.height, + }, + provider, + { + limitations: [ + "The capture is one frame at read time; no recording or interaction occurred", + ], + }, ); + if (signal?.aborted) + return err( + new AnalysisCancelledError(operation, { + partialObservation: executionEvidence(execution, operation, input), + }), + ); + return ok(execution); } private async collectBugreport( diff --git a/tests/boundary/providers/adb/adbScreenPublicationCancellation.test.ts b/tests/boundary/providers/adb/adbScreenPublicationCancellation.test.ts new file mode 100644 index 0000000000..72aa620cda --- /dev/null +++ b/tests/boundary/providers/adb/adbScreenPublicationCancellation.test.ts @@ -0,0 +1,119 @@ +import { createHash } from "node:crypto"; +import { readFile, writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { afterEach, expect, onTestFinished, test, vi } from "vitest"; + +import { AdbProvider } from "../../../../src/adb/AdbProvider.js"; +import { parseEvidence } from "../../../../src/domain/evidence.js"; +import { + execFileBytes, + execFileOutput, +} from "../../../../src/process/ExecFileOutput.js"; +import { createTestTempDirectory } from "../../../fixtures/temporaryDirectory.js"; + +vi.mock( + "../../../../src/process/ExecFileOutput.js", + async (importOriginal) => ({ + ...(await importOriginal< + typeof import("../../../../src/process/ExecFileOutput.js") + >()), + execFileBytes: vi.fn(), + execFileOutput: vi.fn(), + }), +); +vi.mock("node:fs/promises", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, writeFile: vi.fn(actual.writeFile) }; +}); + +afterEach(() => vi.restoreAllMocks()); + +const fixture = async () => { + const root = await createTestTempDirectory("rea-adb-screen-cancel-"); + const binary = join(root, "adb"); + const stdout = `Android Debug Bridge version 1.0.41\nVersion 34.0.5-stub\nInstalled as ${binary}\n`; + vi.mocked(execFileOutput).mockResolvedValue({ + stdout, + stderr: "", + stdoutBytes: Buffer.byteLength(stdout), + stderrBytes: 0, + }); + const provider = new AdbProvider({ environment: { REA_ADB_PATH: binary } }); + onTestFinished(() => provider.close()); + return { + root, + provider, + png: Buffer.from( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+jv4sAAAAASUVORK5CYII=", + "base64", + ), + }; +}; + +test("does not publish a screen capture cancelled before the commit point", async () => { + const f = await fixture(); + const controller = new AbortController(); + vi.mocked(execFileBytes).mockImplementationOnce(async () => { + controller.abort(); + return { stdout: f.png, stderr: Buffer.alloc(0) }; + }); + const output = join(f.root, "captures"); + const result = await f.provider.execute( + { + operation: "capture_adb_screen", + input: { serial: "device", output_directory: output }, + }, + { signal: controller.signal }, + ); + expect(result.ok).toBe(false); + if (result.ok) throw new Error("expected cancellation"); + expect(result.error._tag).toBe("AnalysisCancelledError"); + await expect(readFile(join(output, "screen.png"))).rejects.toMatchObject({ + code: "ENOENT", + }); + await f.provider.close(); +}); + +test("retains a committed screen capture receipt when cancellation arrives after commit", async () => { + const f = await fixture(); + const controller = new AbortController(); + vi.mocked(execFileBytes).mockResolvedValueOnce({ + stdout: f.png, + stderr: Buffer.alloc(0), + }); + const actualFs = + await vi.importActual( + "node:fs/promises", + ); + vi.mocked(writeFile).mockImplementationOnce(async (...args) => { + await actualFs.writeFile(...args); + controller.abort(); + }); + const output = join(f.root, "captures"); + const result = await f.provider.execute( + { + operation: "capture_adb_screen", + input: { serial: "device", output_directory: output }, + }, + { signal: controller.signal }, + ); + expect(result.ok).toBe(false); + if (result.ok) throw new Error("expected cancellation"); + expect(result.error._tag).toBe("AnalysisCancelledError"); + const evidence = parseEvidence(result.error.partialObservation); + expect(evidence).toMatchObject({ + provider: { id: "adb" }, + operation: "capture_adb_screen", + confidence: "observed", + parameters: { serial: "device", output_directory: output }, + normalized_result: { + local_path: join(output, "screen.png"), + bytes: f.png.byteLength, + sha256: createHash("sha256").update(f.png).digest("hex"), + width: 1, + height: 1, + }, + }); + expect(await readFile(join(output, "screen.png"))).toEqual(f.png); + await f.provider.close(); +});