morluto/rea

Fork 0

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

[GH-ISSUE #3927] [Bug] Android inspections classify successful provider completion after cancellation as success #1535

New issue

Closed

opened 2026-10-16 21:06:49 +00:00 by bsakaguchi · 4 comments

bsakaguchi commented

2026-10-16 21:06:49 +00:00

Owner

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

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

Observed CLI output from the interrupted package inspection contained successful inspect_android_package Evidence and no cancelled error code. The cancellation wrapper set process.exitCode to 130. The ordinary control returned successful Evidence with process.exitCode unset. The fixture observed this combination:

{"cancellation":true,"stdout_kind":"Evidence","error_code":null,"exit_code":130}
{"cancellation":false,"stdout_kind":"Evidence","error_code":null,"exit_code":0}

For each of the five MCP operations, the observed provider signal was aborted, the application result had ok: true, the server constructed a result with no error flag, the partial-error delivery path was not called, and the session retained the Evidence. Each normal MCP control succeeded. The SDK rejected the cancelled client promise. Suppression of the eventual reply is source-derived from the transport's documented SDK behavior. Evidence retention alone is not the defect; the cancelled request's success classification is.

The five MCP cases and their controls passed assertions of this defective behavior in both focused runs. The CLI case and its control passed in the second run. The sibling service's existing six-test file also passed. Real JADX has its own late signal check, so these simulated-boundary results do not establish how often a real engine reaches this condition.

Reproduction

Use the baseline checkout with its locked dependencies installed. Save the following independent verification fixture as src/application/android/cancellationVerification.test.ts. It drives the real CLI dispatcher and replaces only the Android provider factory. The synthetic APK is local test input, and the callback invocation triggers REA's installed cancellation listener without sending an operating-system signal to the test runner.

import { expect, it, vi } from "vitest";
import { join } from "node:path";
import { writeOrderedZip } from "../../../tests/fixtures/artifactEntryOrder.js";
import { createAnalysisExecution } from "../AnalysisProvider.js";
import type { AndroidAnalysisPort } from "./AndroidAnalysisPort.js";
import { ok } from "../../domain/result.js";
import { parseEvidence } from "../../domain/evidence.js";

it("shows cancellation status alongside successful Android output", async () => {
  const composition = await import("../../composition/android.js");
  const { createCli } = await import("../../cli.js");
  const apk = join(process.env.REA_VERIFICATION_ROOT!, "input.apk");
  await writeOrderedZip(apk, ["AndroidManifest.xml", "classes.dex"]);
  const observation = {
    engine: { name: "fixture", version: "1", artifact_sha256: "a".repeat(64),
      source_revision: null, worker_count: 1, heap_limit_mib: null },
    package_name: "example", version_name: null, version_code: null,
    min_sdk: null, target_sdk: null, permissions: [],
    manifest: { status: "complete", text: "", reported_total_bytes: 0 },
    class_count: 1, resource_count: 0, signature_verification: "not_performed",
  };
  let cancel = true;
  let signalAborted = false;
  const provider: AndroidAnalysisPort = {
    inspectAvailability: async () => ({
      status: "available", code: null, reason: null, diagnostics: {},
    }),
    close: async () => {},
    execute: async (_target, _request, options) => {
      if (cancel) {
        const listener = process.listeners("SIGINT").at(-1);
        if (listener === undefined) throw new Error("Missing interrupt listener");
        Reflect.apply(listener, process, []);
        signalAborted = options?.signal?.aborted === true;
      }
      return ok(createAnalysisExecution(observation, {
        id: "jadx", name: "Simulated JADX", version: "1",
      }));
    },
  };
  const factory = vi.spyOn(composition, "createAndroidAnalysisProvider")
    .mockReturnValue(provider);
  const initialExitCode = process.exitCode;
  try {
    for (const interrupted of [true, false]) {
      cancel = interrupted;
      process.exitCode = undefined;
      let stdout = "";
      await createCli({}).serve(["inspect-android-package", apk, "--json"], {
        env: {}, stdout: text => { stdout += text; }, exit: () => {},
      });
      const output = JSON.parse(stdout);
      expect(parseEvidence(output).operation).toBe("inspect_android_package");
      expect(output.code).toBeUndefined();
      if (interrupted) {
        expect(signalAborted).toBe(true);
        expect(process.exitCode).toBe(130);
      } else expect(process.exitCode).toBeUndefined();
      console.log(JSON.stringify({ cancellation: interrupted,
        stdout_kind: "Evidence", error_code: output.code ?? null,
        exit_code: process.exitCode ?? 0 }));
    }
  } finally {
    factory.mockRestore();
    process.exitCode = initialExitCode;
  }
});

Run only that fixture with an isolated home, using the repository's pinned Node.js on PATH:

work=$(mktemp -d)
mkdir -p "$work/home"
HOME="$work/home" REA_VERIFICATION_ROOT="$work" npx --no-install vitest run src/application/android/cancellationVerification.test.ts --maxWorkers=1

Observe the cancelled invocation emit the Evidence-shaped JSON described below while its signal is already aborted. The second invocation supplies the normal control. Remove the verification fixture and the exact directory held in work after inspection.

Expected behavior

When the request signal aborts before a provider's successful execution returns, the operation should report cancelled. Completed facts should remain available as partial observation Evidence with their operation, provider and parameters preserved. Ordinary calls whose signals remain active should still return successful Evidence. This is the documented cancellation contract and the existing policy tested for sibling single-request services.

Impact and scope

A bounded cancellation condition produces the wrong structured operation outcome. The package-inspection CLI integration emitted success-shaped JSON with process.exitCode 130. All five registered MCP operations showed the server-side success classification, and their ordinary controls succeeded. CLI callers can honor the nonzero signal-derived status; cancelled MCP SDK callers already stop waiting. No data loss or incorrect Android facts was demonstrated. Real-engine incidence is unmeasured.

REA version

rea 6.5.0 at source commit 4692bb3330

Runtime and platform

Node 24.18.0; macOS arm64

Target and artifact context

Synthetic APK ZIP and complete operation-specific simulated Android port results. No real JADX, Java, device or external service ran.

Interface

CLI

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

Expected source: docs/mcp-contracts.md:418-456 defines request cancellation, partial Evidence and direct CLI signal semantics; src/application/ProviderOperationExecution.ts:67-75 and src/application/apktool/ApktoolResourceAnalysisService.test.ts:87-119 confirm successful late provider output becomes a cancelled result with partialObservation.
Failure source: src/application/android/AndroidAnalysisService.ts:27-57 accepts successful execution without a post-provider signal check; src/cli/androidCommands.ts:19-45 forwards the real cancellation signal and emits the successful service result. The executed fixture observed the resulting success-shaped CLI JSON with cancellation status 130.
Supported MCP path: src/server/createServer.ts:642-648 constructs the service; src/server/registerAndroidTools.ts:25-94 binds the five named tools; src/server/contractToolHandler.ts:60-90 forwards cancellation and records/delivers success. All five paths were exercised through the SDK and actual input/output validation.
Observation limit: src/server/EvidenceMcpServer.ts:226-240 documents SDK reply suppression after cancellation; src/android/JadxProvider.ts:460-472 includes a real-provider signal guard. The report makes no claim that the cancelled MCP client received success or that real JADX reproduced the race.

All cited source files are unchanged at current upstream main 22ec8bdb2f7d4626587f23b1eed4a207df600386.

Typed outcome or recovery expected

Repeat the same successful-after-abort provider condition. The interrupted CLI must emit the structured cancelled result while preserving its conventional cancellation status and completed observation. Each registered MCP operation must produce a cancelled application outcome, carry the completed Evidence as a partial observation, and use the partial-error recording path; reply suppression may still prevent a cancelled SDK client from receiving that reply. The same never-aborted inputs must continue to return successful Evidence. Preserve the original operation, parameters, producing identity and operation-specific confidence in retained facts. The test fixture above asserts the current failure and must be changed to assert those restored outcomes when checking a fix.

Analysis or suspected cause

Source-derived: AndroidAnalysisService.execute accepts successful provider output without a post-provider cancellation guard at src/application/android/AndroidAnalysisService.ts:27-57. The CLI and MCP handlers then follow their success paths. Real JADX has its own guard at src/android/JadxProvider.ts:460-472, so the controlled-port observations do not establish real-engine frequency.

Additional context

Related #3923 concerns firmware in a separate service. Either service can be restored independently; this report concerns Android only. The MCP SDK rejected the cancelled client wait, and this report does not claim that client received a successful MCP reply.

Originally created by @coygeek on GitHub (Oct 15, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/3927 ### 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 Observed CLI output from the interrupted package inspection contained successful `inspect_android_package` Evidence and no cancelled error code. The cancellation wrapper set `process.exitCode` to 130. The ordinary control returned successful Evidence with `process.exitCode` unset. The fixture observed this combination: ```json {"cancellation":true,"stdout_kind":"Evidence","error_code":null,"exit_code":130} {"cancellation":false,"stdout_kind":"Evidence","error_code":null,"exit_code":0} ``` For each of the five MCP operations, the observed provider signal was aborted, the application result had `ok: true`, the server constructed a result with no error flag, the partial-error delivery path was not called, and the session retained the Evidence. Each normal MCP control succeeded. The SDK rejected the cancelled client promise. Suppression of the eventual reply is source-derived from the transport's documented SDK behavior. Evidence retention alone is not the defect; the cancelled request's success classification is. The five MCP cases and their controls passed assertions of this defective behavior in both focused runs. The CLI case and its control passed in the second run. The sibling service's existing six-test file also passed. Real JADX has its own late signal check, so these simulated-boundary results do not establish how often a real engine reaches this condition. ### Reproduction 1. Use the baseline checkout with its locked dependencies installed. Save the following independent verification fixture as `src/application/android/cancellationVerification.test.ts`. It drives the real CLI dispatcher and replaces only the Android provider factory. The synthetic APK is local test input, and the callback invocation triggers REA's installed cancellation listener without sending an operating-system signal to the test runner. ```ts import { expect, it, vi } from "vitest"; import { join } from "node:path"; import { writeOrderedZip } from "../../../tests/fixtures/artifactEntryOrder.js"; import { createAnalysisExecution } from "../AnalysisProvider.js"; import type { AndroidAnalysisPort } from "./AndroidAnalysisPort.js"; import { ok } from "../../domain/result.js"; import { parseEvidence } from "../../domain/evidence.js"; it("shows cancellation status alongside successful Android output", async () => { const composition = await import("../../composition/android.js"); const { createCli } = await import("../../cli.js"); const apk = join(process.env.REA_VERIFICATION_ROOT!, "input.apk"); await writeOrderedZip(apk, ["AndroidManifest.xml", "classes.dex"]); const observation = { engine: { name: "fixture", version: "1", artifact_sha256: "a".repeat(64), source_revision: null, worker_count: 1, heap_limit_mib: null }, package_name: "example", version_name: null, version_code: null, min_sdk: null, target_sdk: null, permissions: [], manifest: { status: "complete", text: "", reported_total_bytes: 0 }, class_count: 1, resource_count: 0, signature_verification: "not_performed", }; let cancel = true; let signalAborted = false; const provider: AndroidAnalysisPort = { inspectAvailability: async () => ({ status: "available", code: null, reason: null, diagnostics: {}, }), close: async () => {}, execute: async (_target, _request, options) => { if (cancel) { const listener = process.listeners("SIGINT").at(-1); if (listener === undefined) throw new Error("Missing interrupt listener"); Reflect.apply(listener, process, []); signalAborted = options?.signal?.aborted === true; } return ok(createAnalysisExecution(observation, { id: "jadx", name: "Simulated JADX", version: "1", })); }, }; const factory = vi.spyOn(composition, "createAndroidAnalysisProvider") .mockReturnValue(provider); const initialExitCode = process.exitCode; try { for (const interrupted of [true, false]) { cancel = interrupted; process.exitCode = undefined; let stdout = ""; await createCli({}).serve(["inspect-android-package", apk, "--json"], { env: {}, stdout: text => { stdout += text; }, exit: () => {}, }); const output = JSON.parse(stdout); expect(parseEvidence(output).operation).toBe("inspect_android_package"); expect(output.code).toBeUndefined(); if (interrupted) { expect(signalAborted).toBe(true); expect(process.exitCode).toBe(130); } else expect(process.exitCode).toBeUndefined(); console.log(JSON.stringify({ cancellation: interrupted, stdout_kind: "Evidence", error_code: output.code ?? null, exit_code: process.exitCode ?? 0 })); } } finally { factory.mockRestore(); process.exitCode = initialExitCode; } }); ``` 2. Run only that fixture with an isolated home, using the repository's pinned Node.js on PATH: ```sh work=$(mktemp -d) mkdir -p "$work/home" HOME="$work/home" REA_VERIFICATION_ROOT="$work" npx --no-install vitest run src/application/android/cancellationVerification.test.ts --maxWorkers=1 ``` 3. Observe the cancelled invocation emit the Evidence-shaped JSON described below while its signal is already aborted. The second invocation supplies the normal control. Remove the verification fixture and the exact directory held in `work` after inspection. ### Expected behavior When the request signal aborts before a provider's successful execution returns, the operation should report `cancelled`. Completed facts should remain available as partial observation Evidence with their operation, provider and parameters preserved. Ordinary calls whose signals remain active should still return successful Evidence. This is the documented cancellation contract and the existing policy tested for sibling single-request services. ### Impact and scope A bounded cancellation condition produces the wrong structured operation outcome. The package-inspection CLI integration emitted success-shaped JSON with `process.exitCode` 130. All five registered MCP operations showed the server-side success classification, and their ordinary controls succeeded. CLI callers can honor the nonzero signal-derived status; cancelled MCP SDK callers already stop waiting. No data loss or incorrect Android facts was demonstrated. Real-engine incidence is unmeasured. ### REA version rea 6.5.0 at source commit 4692bb33300c7cb77b095b6cc0c33da861c1b860 ### Runtime and platform Node 24.18.0; macOS arm64 ### Target and artifact context Synthetic APK ZIP and complete operation-specific simulated Android port results. No real JADX, Java, device or external service ran. ### Interface CLI ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence - Expected source: `docs/mcp-contracts.md:418-456` defines request cancellation, partial Evidence and direct CLI signal semantics; `src/application/ProviderOperationExecution.ts:67-75` and `src/application/apktool/ApktoolResourceAnalysisService.test.ts:87-119` confirm successful late provider output becomes a cancelled result with partialObservation. - Failure source: `src/application/android/AndroidAnalysisService.ts:27-57` accepts successful execution without a post-provider signal check; `src/cli/androidCommands.ts:19-45` forwards the real cancellation signal and emits the successful service result. The executed fixture observed the resulting success-shaped CLI JSON with cancellation status 130. - Supported MCP path: `src/server/createServer.ts:642-648` constructs the service; `src/server/registerAndroidTools.ts:25-94` binds the five named tools; `src/server/contractToolHandler.ts:60-90` forwards cancellation and records/delivers success. All five paths were exercised through the SDK and actual input/output validation. - Observation limit: `src/server/EvidenceMcpServer.ts:226-240` documents SDK reply suppression after cancellation; `src/android/JadxProvider.ts:460-472` includes a real-provider signal guard. The report makes no claim that the cancelled MCP client received success or that real JADX reproduced the race. All cited source files are unchanged at current upstream main `22ec8bdb2f7d4626587f23b1eed4a207df600386`. ### Typed outcome or recovery expected Repeat the same successful-after-abort provider condition. The interrupted CLI must emit the structured cancelled result while preserving its conventional cancellation status and completed observation. Each registered MCP operation must produce a cancelled application outcome, carry the completed Evidence as a partial observation, and use the partial-error recording path; reply suppression may still prevent a cancelled SDK client from receiving that reply. The same never-aborted inputs must continue to return successful Evidence. Preserve the original operation, parameters, producing identity and operation-specific confidence in retained facts. The test fixture above asserts the current failure and must be changed to assert those restored outcomes when checking a fix. ### Analysis or suspected cause Source-derived: `AndroidAnalysisService.execute` accepts successful provider output without a post-provider cancellation guard at `src/application/android/AndroidAnalysisService.ts:27-57`. The CLI and MCP handlers then follow their success paths. Real JADX has its own guard at `src/android/JadxProvider.ts:460-472`, so the controlled-port observations do not establish real-engine frequency. ### Additional context Related #3923 concerns firmware in a separate service. Either service can be restored independently; this report concerns Android only. The MCP SDK rejected the cancelled client wait, and this report does not claim that client received a successful MCP reply.

bsakaguchi commented

2026-10-16 21:06:50 +00:00

Author

Owner

@XinwangTao commented on GitHub (Oct 15, 2026):

Confirmed: when the caller has cancelled, a provider that happens to finish before the abort takes effect must not be classified as an ordinary success — the exit code 130 plus success-shaped Evidence combination misleads automation into trusting a cancelled result. Cancellation state should be checked before the result is classified as ok, and the cancelled reply should carry an explicit cancelled/partial-error code even if Evidence was retained.

@XinwangTao commented on GitHub (Oct 15, 2026): Confirmed: when the caller has cancelled, a provider that happens to finish before the abort takes effect must not be classified as an ordinary success — the exit code 130 plus success-shaped Evidence combination misleads automation into trusting a cancelled result. Cancellation state should be checked before the result is classified as ok, and the cancelled reply should carry an explicit cancelled/partial-error code even if Evidence was retained.

bsakaguchi commented

2026-10-16 21:06:50 +00:00

Author

Owner

@yankikucuk commented on GitHub (Oct 15, 2026):

Working on this. Source-confirmed: AndroidAnalysisService.execute checked the request signal only before dispatch, so an engine that finished after the abort was classified as success (Evidence on stdout, exit 130). The service now rechecks the signal once the Evidence exists and returns the typed cancelled error with that Evidence as partial_observation, matching the shared provider operation execution and the Apktool sibling; active-signal requests are unchanged. A unit test with a late-completing port pins both cases. Draft PR follows shortly.

@yankikucuk commented on GitHub (Oct 15, 2026): Working on this. Source-confirmed: `AndroidAnalysisService.execute` checked the request signal only before dispatch, so an engine that finished after the abort was classified as success (Evidence on stdout, exit 130). The service now rechecks the signal once the Evidence exists and returns the typed `cancelled` error with that Evidence as `partial_observation`, matching the shared provider operation execution and the Apktool sibling; active-signal requests are unchanged. A unit test with a late-completing port pins both cases. Draft PR follows shortly.

bsakaguchi commented

2026-10-16 21:06:50 +00:00

Author

Owner

@XinwangTao commented on GitHub (Oct 16, 2026):

yankikucuk, rechecking the signal after Evidence exists is the right shape — classification must reflect cancellation state at completion time, not just at dispatch, and carrying the Evidence as partial_observation preserves it without misleading automation. The late-completing port test pinning both cases is exactly what this needs. Will review the draft.

@XinwangTao commented on GitHub (Oct 16, 2026): yankikucuk, rechecking the signal after Evidence exists is the right shape — classification must reflect cancellation state at completion time, not just at dispatch, and carrying the Evidence as partial_observation preserves it without misleading automation. The late-completing port test pinning both cases is exactly what this needs. Will review the draft.

bsakaguchi closed this issue

2026-10-17 21:02:10 +00:00

bsakaguchi commented

2026-10-17 21:02:12 +00:00

Author

Owner

@morluto commented on GitHub (Oct 17, 2026):

Reconciled at 98886addd: this was addressed by 1b97b0f370 (#4274), which routes Android Evidence through completeProviderEvidence after assembly. An aborted completion returns AnalysisCancelledError with the completed Evidence, caller parameters and producing provider; method confidence remains derived.

Re-ran the existing focused controls: 12 tests across androidLateCompletion.test.ts, androidLateCompletionMcp.test.ts, and providerCompletion.test.ts passed. The Android controls exercise all five operations; the MCP control observes canonical partial-error recording because the SDK suppresses the cancelled client reply. These use the authored JADX protocol fixture, not a real JVM/JADX engine. The existing CLI cancellation wrapper coverage was inspected but not rerun here; no new exact CLI reproduction claim.

No additional implementation was needed. Closing this stale report against the existing fix.

@morluto commented on GitHub (Oct 17, 2026): Reconciled at 98886addd: this was addressed by 1b97b0f3707b1cf78ac98bd4ae81638b33cc286f (#4274), which routes Android Evidence through `completeProviderEvidence` after assembly. An aborted completion returns `AnalysisCancelledError` with the completed Evidence, caller parameters and producing provider; method confidence remains derived. Re-ran the existing focused controls: 12 tests across `androidLateCompletion.test.ts`, `androidLateCompletionMcp.test.ts`, and `providerCompletion.test.ts` passed. The Android controls exercise all five operations; the MCP control observes canonical partial-error recording because the SDK suppresses the cancelled client reply. These use the authored JADX protocol fixture, not a real JVM/JADX engine. The existing CLI cancellation wrapper coverage was inspected but not rerun here; no new exact CLI reproduction claim. No additional implementation was needed. Closing this stale report against the existing fix.

bsakaguchi referenced this issue

2026-10-17 21:12:11 +00:00

[PR #1535] [MERGED] fix(reference): join internal symlink targets with the inventory NFC normalizer #2952

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#1535

No description provided.

Rows
Columns