morluto/rea

Fork 0

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

[GH-ISSUE #4005] [Bug] SIGINT during process capture cleanup emits successful Evidence #1591

New issue

Closed

opened 2026-10-16 21:07:35 +00:00 by bsakaguchi · 5 comments

bsakaguchi commented

2026-10-16 21:07:35 +00:00

Owner

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

Before submitting

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

Area

Process capture or runtime observation

Observed behavior

When SIGINT arrives while capture-process removes its owned capture root, the CLI waits for cleanup and exits with status 130 but emits ordinary successful capture_process_scenario Evidence. The public CLI filesystem-boundary probe below reproduced this at the reported baseline. The injected delay and signal delivery are controlled test conditions, not an observed uninstrumented cleanup delay.

Reproduction

Use the source baseline in REA version on a supported process-capture host. From the repository root, run the following public CLI probe. The preload simulates a short filesystem cleanup delay and delivers SIGINT to that same CLI process when its capture-root removal begins. It preserves the real removal, PTY capture, signal wrapper, result validation, and Evidence projection. This is a production filesystem-boundary injection, not a claim that an uninstrumented cleanup delay was observed.

node --input-type=module <<'JS'
import { spawnSync } from 'node:child_process';
import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
const work = mkdtempSync(join(tmpdir(), 'process-cancel-probe-'));
mkdirSync(join(work, 'home'));
const hook = join(work, 'cleanup-signal.mjs');
writeFileSync(hook, `
import fs from 'node:fs';
import { basename } from 'node:path';
import { syncBuiltinESMExports } from 'node:module';
const original = fs.promises.rm;
let sent = false;
fs.promises.rm = async function(path, ...args) {
  const name = basename(String(path));
  if (!sent && name.startsWith('rea-process-') && !name.startsWith('rea-process-token-')) {
    sent = true;
    process.kill(process.pid, 'SIGINT');
    await new Promise(resolve => setTimeout(resolve, 100));
  }
  return original.call(this, path, ...args);
};
syncBuiltinESMExports();
`);
const scenario = join(work, 'scenario.json');
writeFileSync(scenario, JSON.stringify({ executable: process.execPath,
  arguments: ['-e', "console.log('done')"], timeout_ms: 5000 }));
try {
  const run = spawnSync(process.execPath,
    ['--import', hook, 'scripts/rea.mjs', 'capture-process', scenario, '--json'],
    { encoding: 'utf8', timeout: 30000,
      env: { ...process.env, HOME: join(work, 'home'), TMPDIR: work } });
  if (run.error) throw run.error;
  const result = JSON.parse(run.stdout);
  console.log(JSON.stringify({ status: run.status, signal: run.signal,
    operation: result.operation, code: result.code ?? null,
    exit: result.normalized_result?.exit,
    cleanup: result.normalized_result?.cleanup }));
} finally { rmSync(work, { recursive: true, force: true }); }
JS

Expected behavior

After cleanup settles, cancellation before completion should return a typed cancelled result, retain completed observations as partial Evidence, and preserve exit status 130. The target's observed normal exit can remain in those partial facts. docs/mcp-contracts.md:418-456 includes process capture in the shared cancellation contract, describes partial observations and cancellation instead of success, and keeps CLI signal guards active through cleanup.

Impact and scope

JSON consumers can accept a success-shaped operation record after interruption if they use the Evidence alone. Both executed CLI reproductions retained status 130, so honoring the nonzero status is an observed workaround. Cleanup completed and no owned root was retained. The evidence establishes the cleanup-time window, not its frequency without instrumentation. No MCP runtime reproduction was executed.

REA version

rea 6.5.0 at source commit 4692bb3330; source freshness checked at origin/main 6fcfc5b5e0

Runtime and platform

Node 24.18.0; macOS arm64

Target and artifact context

Harmless Node command printing done; native PTY process capture. Only capture-root fs.promises.rm was delayed. The Darwin helper root was excluded. No network service or external analysis provider was used.

Interface

CLI

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

The executed self-signal hook returned:

{"status":130,"signal":null,"operation":"capture_process_scenario","code":null,"exit":{"code":0,"signal":0,"reason":"exited"},"cleanup":{"owned_process_group":"verified","temporary_root":"removed"}}

Here code: null is the probe's projection of an absent top-level error code. A separate parent driver delivering SIGINT during a 1000 ms capture-root removal delay reproduced the same normal Evidence and status 130. Both used real removal, PTY capture, signal handling, result validation, and Evidence projection; neither retained the owned root.

Source freshness checked against current origin/main 6fcfc5b5e0c063300cd5278dd8bad5b1ab1374f6. The cited production files and contract documentation are unchanged from the reproduced baseline; the source-derived failure still holds. The full runtime reproduction was not rerun for this freshness check.

Typed outcome or recovery expected

Return cancelled with completed capture facts in partial observations after cleanup, while retaining status 130. Preserve successful output without a signal and the distinct cleanup-incomplete outcome when owned cleanup fails.

Analysis or suspected cause

Source-derived: src/process/capture/ProcessHarness.ts:363-457 awaits resource cleanup and returns resolveProcessResult without receiving or rechecking the cancellation signal. captureProcessScenario returns ok(capture) after the awaited run at line 879. src/application/process/ProcessCli.ts:84-94 closes the resource scope and projects success without a final signal check. src/cli/commandCancellation.ts preserves the signal-derived process status but returns the operation value unchanged.

Additional context

Related closed #1232 concerns an interrupted CLI leaving its target alive, and closed #1233 concerns cancellation waiting through post-exit settlement. Here cleanup succeeds, but cancellation during that cleanup still publishes success. Open #3923 concerns firmware-provider completion; its affected code and operations differ. Open PR #2893 concerns CDP browser cleanup, and #3263 tests cancellation during a filesystem snapshot lookup; neither covers the process-capture final-cleanup result.

Originally created by @coygeek on GitHub (Oct 15, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4005 ### 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 Process capture or runtime observation ### Observed behavior When SIGINT arrives while `capture-process` removes its owned capture root, the CLI waits for cleanup and exits with status 130 but emits ordinary successful `capture_process_scenario` Evidence. The public CLI filesystem-boundary probe below reproduced this at the reported baseline. The injected delay and signal delivery are controlled test conditions, not an observed uninstrumented cleanup delay. ### Reproduction Use the source baseline in REA version on a supported process-capture host. From the repository root, run the following public CLI probe. The preload simulates a short filesystem cleanup delay and delivers SIGINT to that same CLI process when its capture-root removal begins. It preserves the real removal, PTY capture, signal wrapper, result validation, and Evidence projection. This is a production filesystem-boundary injection, not a claim that an uninstrumented cleanup delay was observed. ```sh node --input-type=module <<'JS' import { spawnSync } from 'node:child_process'; import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; const work = mkdtempSync(join(tmpdir(), 'process-cancel-probe-')); mkdirSync(join(work, 'home')); const hook = join(work, 'cleanup-signal.mjs'); writeFileSync(hook, ` import fs from 'node:fs'; import { basename } from 'node:path'; import { syncBuiltinESMExports } from 'node:module'; const original = fs.promises.rm; let sent = false; fs.promises.rm = async function(path, ...args) { const name = basename(String(path)); if (!sent && name.startsWith('rea-process-') && !name.startsWith('rea-process-token-')) { sent = true; process.kill(process.pid, 'SIGINT'); await new Promise(resolve => setTimeout(resolve, 100)); } return original.call(this, path, ...args); }; syncBuiltinESMExports(); `); const scenario = join(work, 'scenario.json'); writeFileSync(scenario, JSON.stringify({ executable: process.execPath, arguments: ['-e', "console.log('done')"], timeout_ms: 5000 })); try { const run = spawnSync(process.execPath, ['--import', hook, 'scripts/rea.mjs', 'capture-process', scenario, '--json'], { encoding: 'utf8', timeout: 30000, env: { ...process.env, HOME: join(work, 'home'), TMPDIR: work } }); if (run.error) throw run.error; const result = JSON.parse(run.stdout); console.log(JSON.stringify({ status: run.status, signal: run.signal, operation: result.operation, code: result.code ?? null, exit: result.normalized_result?.exit, cleanup: result.normalized_result?.cleanup })); } finally { rmSync(work, { recursive: true, force: true }); } JS ``` ### Expected behavior After cleanup settles, cancellation before completion should return a typed `cancelled` result, retain completed observations as partial Evidence, and preserve exit status 130. The target's observed normal exit can remain in those partial facts. `docs/mcp-contracts.md:418-456` includes process capture in the shared cancellation contract, describes partial observations and cancellation instead of success, and keeps CLI signal guards active through cleanup. ### Impact and scope JSON consumers can accept a success-shaped operation record after interruption if they use the Evidence alone. Both executed CLI reproductions retained status 130, so honoring the nonzero status is an observed workaround. Cleanup completed and no owned root was retained. The evidence establishes the cleanup-time window, not its frequency without instrumentation. No MCP runtime reproduction was executed. ### REA version rea 6.5.0 at source commit 4692bb33300c7cb77b095b6cc0c33da861c1b860; source freshness checked at origin/main 6fcfc5b5e0c063300cd5278dd8bad5b1ab1374f6 ### Runtime and platform Node 24.18.0; macOS arm64 ### Target and artifact context Harmless Node command printing `done`; native PTY process capture. Only capture-root `fs.promises.rm` was delayed. The Darwin helper root was excluded. No network service or external analysis provider was used. ### Interface CLI ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence The executed self-signal hook returned: ```json {"status":130,"signal":null,"operation":"capture_process_scenario","code":null,"exit":{"code":0,"signal":0,"reason":"exited"},"cleanup":{"owned_process_group":"verified","temporary_root":"removed"}} ``` Here `code: null` is the probe's projection of an absent top-level error code. A separate parent driver delivering SIGINT during a 1000 ms capture-root removal delay reproduced the same normal Evidence and status 130. Both used real removal, PTY capture, signal handling, result validation, and Evidence projection; neither retained the owned root. Source freshness checked against current `origin/main` `6fcfc5b5e0c063300cd5278dd8bad5b1ab1374f6`. The cited production files and contract documentation are unchanged from the reproduced baseline; the source-derived failure still holds. The full runtime reproduction was not rerun for this freshness check. ### Typed outcome or recovery expected Return `cancelled` with completed capture facts in partial observations after cleanup, while retaining status 130. Preserve successful output without a signal and the distinct cleanup-incomplete outcome when owned cleanup fails. ### Analysis or suspected cause Source-derived: `src/process/capture/ProcessHarness.ts:363-457` awaits resource cleanup and returns `resolveProcessResult` without receiving or rechecking the cancellation signal. `captureProcessScenario` returns `ok(capture)` after the awaited run at line 879. `src/application/process/ProcessCli.ts:84-94` closes the resource scope and projects success without a final signal check. `src/cli/commandCancellation.ts` preserves the signal-derived process status but returns the operation value unchanged. ### Additional context Related closed #1232 concerns an interrupted CLI leaving its target alive, and closed #1233 concerns cancellation waiting through post-exit settlement. Here cleanup succeeds, but cancellation during that cleanup still publishes success. Open #3923 concerns firmware-provider completion; its affected code and operations differ. Open PR #2893 concerns CDP browser cleanup, and #3263 tests cancellation during a filesystem snapshot lookup; neither covers the process-capture final-cleanup result.

bsakaguchi closed this issue

2026-10-16 21:07:36 +00:00

bsakaguchi commented

2026-10-16 21:07:37 +00:00

Author

Owner

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

Confirmed — a SIGINT that arrives during capture-root removal still yields successful capture-process Evidence while the CLI exits with status 130. The signal wrapper / cleanup path should mark the run as interrupted and emit cancelled (or at least non-success) Evidence instead of projecting a normal success, so downstream consumers don't treat an interrupted capture as a completed observation.

@XinwangTao commented on GitHub (Oct 15, 2026): Confirmed — a SIGINT that arrives during capture-root removal still yields successful capture-process Evidence while the CLI exits with status 130. The signal wrapper / cleanup path should mark the run as interrupted and emit cancelled (or at least non-success) Evidence instead of projecting a normal success, so downstream consumers don't treat an interrupted capture as a completed observation.

bsakaguchi commented

2026-10-16 21:07:37 +00:00

Author

Owner

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

Working on this. Reproduced with your preload probe on a clean build: status 130 with ordinary successful Evidence. The run now rechecks the request signal once owned cleanup has settled and resolves through the existing cancellation path, so the probe returns code cancelled with cleanup: complete and the completed capture (observed normal exit) retained in partial_observation, status 130 preserved. Failed runs and cleanup-incomplete outcomes are unchanged. Draft PR follows shortly.

@yankikucuk commented on GitHub (Oct 15, 2026): Working on this. Reproduced with your preload probe on a clean build: status 130 with ordinary successful Evidence. The run now rechecks the request signal once owned cleanup has settled and resolves through the existing cancellation path, so the probe returns code `cancelled` with `cleanup: complete` and the completed capture (observed normal exit) retained in `partial_observation`, status 130 preserved. Failed runs and cleanup-incomplete outcomes are unchanged. Draft PR follows shortly.

bsakaguchi commented

2026-10-16 21:07:38 +00:00

Author

Owner

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

Reproduced on bde139c5d using the actual public CLI, a harmless owned Node workload, and a preload seam that delivers SIGINT while real fs.promises.rm of the owned capture root is pending. Before repair, exit status was 130 but stdout was ordinary successful Evidence. The non-interrupted control returned success and removed the root.

Local repair in src/process/capture/ProcessHarness.ts threads the existing request signal into finalization and checks it after awaited cleanup and before resolving the result. Existing primary errors and cleanup-failure precedence are preserved. Normal workload exit remains an observation, not proof that the cancelled request completed successfully.

The repaired public CLI control returns success; interrupted finalization returns cancelled with the observed exit code 0 retained under details.partial_observation, cleanup complete, and the temporary root verified absent. One warm control/interrupted pair took approximately 1.739s/1.842s; no before/after performance claim is made. The regression also exercises real cleanup rather than replacing removal with a stub.

Active SIGINT/SIGTERM CLI cleanup, MCP post-exit descendant cancellation, and producer tests passed (35 tests in that focused run). No globally installed server or released package has been updated; the patch is local and uncommitted.

@morluto commented on GitHub (Oct 15, 2026): Reproduced on `bde139c5d` using the actual public CLI, a harmless owned Node workload, and a preload seam that delivers SIGINT while real `fs.promises.rm` of the owned capture root is pending. Before repair, exit status was 130 but stdout was ordinary successful Evidence. The non-interrupted control returned success and removed the root. Local repair in `src/process/capture/ProcessHarness.ts` threads the existing request signal into finalization and checks it after awaited cleanup and before resolving the result. Existing primary errors and cleanup-failure precedence are preserved. Normal workload exit remains an observation, not proof that the cancelled request completed successfully. The repaired public CLI control returns success; interrupted finalization returns `cancelled` with the observed exit code 0 retained under `details.partial_observation`, cleanup complete, and the temporary root verified absent. One warm control/interrupted pair took approximately 1.739s/1.842s; no before/after performance claim is made. The regression also exercises real cleanup rather than replacing removal with a stub. Active SIGINT/SIGTERM CLI cleanup, MCP post-exit descendant cancellation, and producer tests passed (35 tests in that focused run). No globally installed server or released package has been updated; the patch is local and uncommitted.

bsakaguchi commented

2026-10-16 21:07:38 +00:00

Author

Owner

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

Fixed on main in 0e1f0bf2d. Process capture checks cancellation after awaited cleanup and before publishing success, preserving completed capture facts as partial Evidence.

The public CLI cleanup-delay signal reproduction was verified to return cancellation with exit status 130 and removed owned roots. The interleaving is controlled, not a frequency measurement.

Hosted main CI is still pending; this closure is based on the implemented fix and scoped regression evidence.

@morluto commented on GitHub (Oct 16, 2026): Fixed on `main` in [0e1f0bf2d](https://github.com/morluto/rea/commit/0e1f0bf2d06943e59253b8a66923a1ae6005e81e). Process capture checks cancellation after awaited cleanup and before publishing success, preserving completed capture facts as partial Evidence. The public CLI cleanup-delay signal reproduction was verified to return cancellation with exit status 130 and removed owned roots. The interleaving is controlled, not a frequency measurement. Hosted main CI is still pending; this closure is based on the implemented fix and scoped regression evidence.

bsakaguchi commented

2026-10-16 21:07:39 +00:00

Author

Owner

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

Agreed with the fix direction — rechecking the request signal after awaited cleanup and before resolving is exactly the right point, since the previous code only checked cancellation before the finalization window. Keeping the observed exit code under partial_observation while emitting cancelled preserves both the signal semantics and the useful capture facts. Thanks for threading this through the shared finalizer.

@XinwangTao commented on GitHub (Oct 16, 2026): Agreed with the fix direction — rechecking the request signal after awaited cleanup and before resolving is exactly the right point, since the previous code only checked cancellation before the finalization window. Keeping the observed exit code under partial_observation while emitting cancelled preserves both the signal semantics and the useful capture facts. Thanks for threading this through the shared finalizer.

bsakaguchi referenced this issue

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

[PR #1590] [MERGED] fix(browser): keep declared scenario secrets out of redacted evidence #2991

bsakaguchi referenced this issue

2026-10-31 00:44:13 +00:00

[GH-ISSUE #6289] [Bug] Distinct localStorage code units receive the same digest and scenarios compare unchanged #3803

bsakaguchi referenced this issue

2026-10-31 00:51:42 +00:00

[GH-ISSUE #7782] fix(process): honor sensitive inputs across captured terminal projections #4640