[GH-ISSUE #6049] [Bug] Hopper regex search returns success after cancellation during worker termination #3694

New issue

Closed

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

bsakaguchi commented

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

Owner

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

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 Hopper regex-search CLI returned exit 0 and successful search_strings Evidence after SIGINT aborted its request signal during the final matching-worker termination await. The real worker was terminated before the result returned. The uninterrupted control produced the same match. This was a controlled teardown interleaving with the repository’s simulated Hopper socket bridge and the real regex worker.

Reproduction

The CLI request exercised was rea search sample fixture --provider hopper --mode regex --json. Save the following as tests/process-global/hopperRegexTeardownCancellation.test.ts in a built checkout and run:

node node_modules/vitest/vitest.mjs run tests/process-global/hopperRegexTeardownCancellation.test.ts --maxWorkers 1

The termination wrapper calls the real Worker.terminate() first, emits SIGINT before awaiting its completion, and still awaits the original promise. Matching and worker shutdown remain real. Only Hopper engine admission/protocol are supplied by the repository fixture; the target is never executed. Both cases below were executed in a combined focused run with all 12 controls passing their observed-behavior assertions.

import { copyFile, mkdtemp, rm } from "node:fs/promises";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { afterEach, expect, it, vi } from "vitest";
import type { BridgeLauncher, BridgeSession } from "../../src/hopper/BridgeLauncher.js";
import { HopperFixtureLauncher } from "../boundary/providers/hopper/hopperClient.fixture.js";
import { serveCli } from "../boundary/cli/cliServeHarness.js";
import { waitForExit } from "../support/process/processFixture.js";
import { Worker } from "node:worker_threads";
import { HopperRegexSearch } from "../../src/hopper/HopperRegexSearch.js";

const fixture = vi.hoisted(() => ({ launcher: undefined as BridgeLauncher | undefined }));
vi.mock("../../src/hopper/BridgeLauncher.js", async (original) => ({
  ...(await original<typeof import("../../src/hopper/BridgeLauncher.js")>()),
  HopperApplicationLauncher: class implements BridgeLauncher {
    launch(session: BridgeSession) {
      if (!fixture.launcher) throw new Error("No fixture launcher");
      return fixture.launcher.launch(session);
    }
  },
}));
const exitCode = process.exitCode;
afterEach(() => { vi.restoreAllMocks(); process.exitCode = exitCode; });

it.each([false, true])("Hopper regex public CLI caller abort=%s during worker termination", async (cancel) => {
  const root = await mkdtemp(join(tmpdir(), "rea-regex-cancel-"));
  const launcher = new HopperFixtureLauncher();
  fixture.launcher = launcher;
  const terminate = Worker.prototype.terminate;
  let terminated = false;
  let signal: AbortSignal | undefined;
  const execute = HopperRegexSearch.prototype.execute;
  vi.spyOn(HopperRegexSearch.prototype, "execute").mockImplementation(function(operation, parameters, options) {
    signal = options?.signal;
    return execute.call(this, operation, parameters, options);
  });
  vi.spyOn(Worker.prototype, "terminate").mockImplementation(async function() {
    const pending = terminate.call(this);
    if (cancel) process.emit("SIGINT");
    const code = await pending;
    terminated = true;
    return code;
  });
  try {
    const target = join(root, "sample"); await copyFile("/usr/bin/true", target);
    const run = await serveCli(["search", target, "fixture", "--provider", "hopper", "--mode", "regex", "--json"],
      { HOME: root, HOPPER_LAUNCHER_PATH: process.execPath, REA_LOG_LEVEL: "silent" });
    const output = JSON.parse(run.stdout);
    expect(output.operation).toBe("search_strings");
    expect(output.normalized_result).toEqual([{ address: "0x1000", value: "fixture" }]);
    expect(signal?.aborted).toBe(cancel);
    expect(terminated).toBe(true);
    console.log(JSON.stringify({ case: "hopper-regex-cli", cancel, aborted: signal?.aborted, terminated,
      exit: process.exitCode ?? run.exitCode, operation: output.operation, code: output.code ?? null }));
  } finally {
    fixture.launcher = undefined;
    for (const child of launcher.processes) {
      if (child.exitCode === null && child.signalCode === null) child.kill("SIGTERM");
      await waitForExit(child, 5000);
    }
    await rm(root, { recursive: true, force: true });
  }
});

Expected behavior

Return typed cancellation, retaining the match observation, when the request signal is already aborted before the final teardown await returns. The lifecycle contract requires cancellation through awaited cleanup and a final signal check before success. Lifecycle testing guidance explicitly distinguishes this phase from active-execution cancellation. No existing provider failure is present in this reproduction.

Impact and scope

P2: a canceled regex observation is reported as completed, so a caller can consume a successful match response after cancellation. A caller that retains its own interruption state can discard that response. No worker leak or target mutation was observed. Runtime verification covers search_strings; search_procedures shares this same source path but was not separately executed. The natural timing-window duration/frequency, actual Hopper and MCP transport were not tested.

REA version

rea-agents 6.7.0; source revision 52415975942a4b9c53a817915b313ccc404f280e. Fetched upstream 6d356b84344659085f2b912857b5e27fe0160c9b leaves the relevant producing code unchanged; that newer revision was source-compared, not executed.

Runtime and platform

macOS 27.0.1 (26A434), arm64; Node.js v26.12.0; locked Vitest 5.0.3. The contributor Node 24.18.0 pin was not exercised. The Hopper socket fixture was used; no real Hopper engine was launched.

Target and artifact context

A system native executable supplies target admission; authored provider facts come from repository fixtures. The target bytes are never executed.

Interface

CLI

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

Observed public CLI subset:

{"cancel":false,"aborted":false,"terminated":true,"exit":0,"operation":"search_strings","code":null}
{"cancel":true,"aborted":true,"terminated":true,"exit":0,"operation":"search_strings","code":null}

Both responses contained normalized_result: [{"address":"0x1000","value":"fixture"}]. Direct HopperRegexSearch controls also returned ok in both cases with real worker termination completed. Simulated Hopper processes were awaited and fixture directories removed.

Typed outcome or recovery expected

Return the existing AnalysisCancelledError for the accepted search operation, with the caller reason and available partial observation. Retain the current guarantee that the worker has terminated before returning. An actual matching/output error should keep its precedence.

Analysis or suspected cause

Source-derived: RegexMatcher removes its abort listener as soon as the worker posts a terminal message, then awaits termination in finally. HopperRegexSearch checks the signal at earlier boundaries but does not check after that matcher await. HopperProvider wraps the match in a successful execution without a later cancellation check. Restoring abort reasons at existing cancellation constructors does not close this completion gap.

Additional context

Related #5986 concerns invalid regex correction metadata, #4680 concerns inventory-failure operation identity, and #5714 concerns caller reasons in HopperRequestQueue. #4860 and PR #4862 concern synchronous IDA regex matching and move it to the worker; they do not fix this post-match teardown boundary. #340 and #4227 request broader validation/design work. No duplicate with the same producer and fix was found.

Related but separate: #6048 concerns native value tracing accepting a final successful provider reply after cancellation, at a different producing boundary. #5996 concerns IDA replacing an existing provider failure with late cancellation; this report has no prior failure. Neither preserving abort reasons at existing cancellation exits nor changing IDA failure precedence fixes this successful matcher-teardown path.

Originally created by @coygeek on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/6049 ### 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 Hopper regex-search CLI returned exit 0 and successful `search_strings` Evidence after SIGINT aborted its request signal during the final matching-worker termination await. The real worker was terminated before the result returned. The uninterrupted control produced the same match. This was a controlled teardown interleaving with the repository’s simulated Hopper socket bridge and the real regex worker. ### Reproduction The CLI request exercised was `rea search sample fixture --provider hopper --mode regex --json`. Save the following as `tests/process-global/hopperRegexTeardownCancellation.test.ts` in a built checkout and run: ```sh node node_modules/vitest/vitest.mjs run tests/process-global/hopperRegexTeardownCancellation.test.ts --maxWorkers 1 ``` The termination wrapper calls the real `Worker.terminate()` first, emits SIGINT before awaiting its completion, and still awaits the original promise. Matching and worker shutdown remain real. Only Hopper engine admission/protocol are supplied by the repository fixture; the target is never executed. Both cases below were executed in a combined focused run with all 12 controls passing their observed-behavior assertions. ```ts import { copyFile, mkdtemp, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, expect, it, vi } from "vitest"; import type { BridgeLauncher, BridgeSession } from "../../src/hopper/BridgeLauncher.js"; import { HopperFixtureLauncher } from "../boundary/providers/hopper/hopperClient.fixture.js"; import { serveCli } from "../boundary/cli/cliServeHarness.js"; import { waitForExit } from "../support/process/processFixture.js"; import { Worker } from "node:worker_threads"; import { HopperRegexSearch } from "../../src/hopper/HopperRegexSearch.js"; const fixture = vi.hoisted(() => ({ launcher: undefined as BridgeLauncher | undefined })); vi.mock("../../src/hopper/BridgeLauncher.js", async (original) => ({ ...(await original<typeof import("../../src/hopper/BridgeLauncher.js")>()), HopperApplicationLauncher: class implements BridgeLauncher { launch(session: BridgeSession) { if (!fixture.launcher) throw new Error("No fixture launcher"); return fixture.launcher.launch(session); } }, })); const exitCode = process.exitCode; afterEach(() => { vi.restoreAllMocks(); process.exitCode = exitCode; }); it.each([false, true])("Hopper regex public CLI caller abort=%s during worker termination", async (cancel) => { const root = await mkdtemp(join(tmpdir(), "rea-regex-cancel-")); const launcher = new HopperFixtureLauncher(); fixture.launcher = launcher; const terminate = Worker.prototype.terminate; let terminated = false; let signal: AbortSignal | undefined; const execute = HopperRegexSearch.prototype.execute; vi.spyOn(HopperRegexSearch.prototype, "execute").mockImplementation(function(operation, parameters, options) { signal = options?.signal; return execute.call(this, operation, parameters, options); }); vi.spyOn(Worker.prototype, "terminate").mockImplementation(async function() { const pending = terminate.call(this); if (cancel) process.emit("SIGINT"); const code = await pending; terminated = true; return code; }); try { const target = join(root, "sample"); await copyFile("/usr/bin/true", target); const run = await serveCli(["search", target, "fixture", "--provider", "hopper", "--mode", "regex", "--json"], { HOME: root, HOPPER_LAUNCHER_PATH: process.execPath, REA_LOG_LEVEL: "silent" }); const output = JSON.parse(run.stdout); expect(output.operation).toBe("search_strings"); expect(output.normalized_result).toEqual([{ address: "0x1000", value: "fixture" }]); expect(signal?.aborted).toBe(cancel); expect(terminated).toBe(true); console.log(JSON.stringify({ case: "hopper-regex-cli", cancel, aborted: signal?.aborted, terminated, exit: process.exitCode ?? run.exitCode, operation: output.operation, code: output.code ?? null })); } finally { fixture.launcher = undefined; for (const child of launcher.processes) { if (child.exitCode === null && child.signalCode === null) child.kill("SIGTERM"); await waitForExit(child, 5000); } await rm(root, { recursive: true, force: true }); } }); ``` ### Expected behavior Return typed cancellation, retaining the match observation, when the request signal is already aborted before the final teardown await returns. The [lifecycle contract](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/tool-design.md#L101-L107) requires cancellation through awaited cleanup and a final signal check before success. [Lifecycle testing guidance](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/testing.md#L268-L275) explicitly distinguishes this phase from active-execution cancellation. No existing provider failure is present in this reproduction. ### Impact and scope P2: a canceled regex observation is reported as completed, so a caller can consume a successful match response after cancellation. A caller that retains its own interruption state can discard that response. No worker leak or target mutation was observed. Runtime verification covers search_strings; search_procedures shares this same source path but was not separately executed. The natural timing-window duration/frequency, actual Hopper and MCP transport were not tested. ### REA version rea-agents 6.7.0; source revision `52415975942a4b9c53a817915b313ccc404f280e`. Fetched upstream `6d356b84344659085f2b912857b5e27fe0160c9b` leaves the relevant producing code unchanged; that newer revision was source-compared, not executed. ### Runtime and platform macOS 27.0.1 (26A434), arm64; Node.js v26.12.0; locked Vitest 5.0.3. The contributor Node 24.18.0 pin was not exercised. The Hopper socket fixture was used; no real Hopper engine was launched. ### Target and artifact context A system native executable supplies target admission; authored provider facts come from repository fixtures. The target bytes are never executed. ### Interface CLI ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence Observed public CLI subset: ```json {"cancel":false,"aborted":false,"terminated":true,"exit":0,"operation":"search_strings","code":null} {"cancel":true,"aborted":true,"terminated":true,"exit":0,"operation":"search_strings","code":null} ``` Both responses contained `normalized_result: [{"address":"0x1000","value":"fixture"}]`. Direct HopperRegexSearch controls also returned `ok` in both cases with real worker termination completed. Simulated Hopper processes were awaited and fixture directories removed. ### Typed outcome or recovery expected Return the existing `AnalysisCancelledError` for the accepted search operation, with the caller reason and available partial observation. Retain the current guarantee that the worker has terminated before returning. An actual matching/output error should keep its precedence. ### Analysis or suspected cause Source-derived: [RegexMatcher](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/process/RegexMatcher.ts#L74-L176) removes its abort listener as soon as the worker posts a terminal message, then awaits termination in `finally`. [HopperRegexSearch](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/hopper/HopperRegexSearch.ts#L110-L140) checks the signal at earlier boundaries but does not check after that matcher await. [HopperProvider](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/hopper/HopperProvider.ts#L349-L373) wraps the match in a successful execution without a later cancellation check. Restoring abort reasons at existing cancellation constructors does not close this completion gap. ### Additional context Related #5986 concerns invalid regex correction metadata, #4680 concerns inventory-failure operation identity, and #5714 concerns caller reasons in HopperRequestQueue. #4860 and PR #4862 concern synchronous IDA regex matching and move it to the worker; they do not fix this post-match teardown boundary. #340 and #4227 request broader validation/design work. No duplicate with the same producer and fix was found. Related but separate: [#6048](https://github.com/morluto/rea/issues/6048) concerns native value tracing accepting a final successful provider reply after cancellation, at a different producing boundary. [#5996](https://github.com/morluto/rea/issues/5996) concerns IDA replacing an existing provider failure with late cancellation; this report has no prior failure. Neither preserving abort reasons at existing cancellation exits nor changing IDA failure precedence fixes this successful matcher-teardown path.

bsakaguchi closed this issue

2026-10-31 00:43:09 +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