morluto/rea

Fork 0

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

[GH-ISSUE #4512] [Bug] Process capture can attempt PTY launch after cancellation during executable sampling #1794

New issue

Closed

opened 2026-10-16 21:10:18 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-16 21:10:18 +00:00

Owner

Originally created by @dakjdakd on GitHub (Oct 16, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/4512

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

The focused fix and regression test are implemented on fix/process-pre-spawn-cancellation, commit 1fd8a884e. I am working on the corresponding pull request alongside this report.

Process capture can reach its PTY launch call after the caller has cancelled during the final executable metadata read or descriptor close. The cancellation arrives before any target has been started, but startCaptureRuntime continues from await observeSelectedExecutable(...) into spawn(...) without checking the signal again.

I reproduced the launch attempt through the exported production captureProcessScenario boundary. The executable was opened, read, inspected, and closed using real filesystem operations. The native PTY entry point was replaced with a throwing sentinel, so no target process was created. On the affected source, both late-cancellation cases called that sentinel once. Earlier cancellation controls did not call it.

This report concerns the decision to initiate execution. It does not claim that the sampler's completed digest is invalid, that cleanup leaked a real process, or that a real target caused damage. A native Linux/macOS capture and public CLI/MCP cancellation were not executed for this reproduction.

Reproduction

The regression is tests/process-global/processPrelaunchCancellation.test.ts in the proposed fix. It exercises the actual capture harness rather than reproducing its launch logic in a separate helper.

Parse a scenario selecting process.execPath, with arguments ["--eval", "process.exit(0)"] and the checkout as its working directory. Supply an AbortController.signal and an empty host environment.
Preserve actual node:fs/promises.open, executable reads, descriptor stat, and descriptor close. Wrap the returned real handle only to synchronize an abort with a chosen completion boundary.
For the first affected case, abort after the final real descriptor stat resolves. This is the second descriptor metadata inspection in the sampler, after the executable bytes have been read.
For the second affected case, abort after the real descriptor close resolves. The descriptor has already been released when cancellation is raised.
Invoke captureProcessScenario with the existing platform and cleanup seams. Native ownership preparation/baseline discovery are replaced with controlled host responses; PTY spawn is intercepted and throws before creating a process. The fixture checks that interception is installed before entering the harness.
Observe whether the launch sentinel was called and verify descriptor release and temporary-root removal.

Run the new regression together with the existing early-metadata cancellation control:

node node_modules/vitest/vitest.mjs run \
  --project process-global --project process-boundary \
  tests/process-global/processPrelaunchCancellation.test.ts \
  tests/boundary/process/selectedExecutableCancellation.test.ts \
  --reporter=verbose

The deterministic observations on the affected base were:

Cancellation point	Launch sentinel calls	Interpretation
No cancellation	1	The ordinary path reaches the launch boundary.
Immediately after executable open	0	Existing earlier cancellation checks stop startup.
After final descriptor stat	1	Startup continues after cancellation during sample finalization.
After descriptor close	1	Startup continues after cancellation during sample finalization.
Before the capture call	0	Admission rejects an already-cancelled request.

The uninterrupted case deliberately ends at the throwing sentinel; it establishes reachability, not a successful native capture. The two affected boundaries reproduced under their explicit fault-injection schedules. Their frequency under normal filesystem timing is NOT_ENOUGH_INFO; no timing-based incidence estimate is available.

Expected behavior

Recheck the caller's signal after the awaited executable sampling has fully settled and before launching the selected program. If the signal is already aborted, take the existing typed cancellation/startup-cleanup path and make zero PTY launch attempts.

The lifecycle guidance separates target admission, active execution, and resource release, and requires cancellation to remain authoritative through awaited finalization. An executable sample establishes file facts; it should not independently authorize a later launch after the requesting operation has been cancelled.

Completed file observations should remain available. In particular, obtaining a complete SHA-256 before cancellation does not make that digest unknown. Preventing the subsequent launch should not require changing the sampler's return value or discarding the bytes it already observed.

Impact and scope

The observed scope is the shared process-capture startup path. Both CLI and MCP consumers use this harness, so the source path is shared; adapter-specific cancellation delivery was not tested here.

The practical concern is that a request cancelled during its last awaited prerequisite can still cross the execution boundary. Once started, a selected program may begin work before later cancellation monitoring and cleanup run. That consequence follows from the call order; the fixture intentionally prevents execution and therefore does not demonstrate a particular target action, a process leak, or an isolation failure.

The window is narrow but deterministic to exercise. Earlier checks cannot close it because they run before an awaited final metadata read and descriptor release. No additional permission, target format, provider, or transport option is involved.

REA version

Source revision 180ba03bfddd5916251e81e97a500b1ede7a9eea (rea-agents 6.5.0).

Runtime and platform

Node.js v24.18.0, Windows x64, Vitest v5.0.3. The test uses the harness's controlled Linux platform/ownership seams to exercise portable startup logic; it does not establish native Windows PTY support or a real Linux execution result.

Target and artifact context

The local Node executable supplies real executable bytes and metadata. No extracted application, private target, external service, or credential is required.

Interface

Other: exported production captureProcessScenario boundary. CLI/MCP impact is source-derived.

Earliest failure stage

Cleanup, cancellation, or lifecycle: executable sampling has completed, immediately before PTY launch.

Relevant output or evidence

The affected caller awaits observeSelectedExecutable and proceeds to spawn. In the sampler, the last ordinary cancellation check precedes the final awaited descriptor stat; the finally block then awaits descriptor close.

Separate direct-sampler controls confirm that both late schedules retain the correct complete SHA-256 with signal.aborted === true and a closed descriptor. The caller must therefore make its own execution decision after consuming that completed sample.

Typed outcome or recovery expected

The existing ProcessCaptureError cancellation outcome, with cleanup completed and no target PID acquired. Retain existing startup resource cleanup; do not start a target solely to discover cancellation in a later monitoring phase.

Analysis or suspected cause

The final metadata and close awaits introduce cancellation opportunities after the sampler's last check. There is no intervening signal check at the consuming launch boundary. A single assertNotCancelled(options.signal) after the sampling await closes these schedules without changing descriptor ownership or completed-digest semantics.

Additional context

#992 checks cancellation after initial metadata admission, before constructing a read stream with an already-aborted signal. Its regression does not cover final descriptor stat/close followed by PTY launch.
#4379 / #4443 address finalization cancellation in the shared RegularFileRead / OwnedRegularFile owner and preserve completed file facts. This sampler is independent; this report targets its downstream launch decision.
#3263 tests cancellation during an obstructed filesystem-root lookup in a final capture snapshot. It does not change executable sampling or the PTY launch gate.
#1011's provider-startup cancellation follow-up concerns ownership preparation in ProviderProcess.ts; #4005 concerns cancellation after target exit during capture cleanup. Neither covers this prelaunch boundary.

These related fixes establish useful controls, but I found no focused report or pending change for this final sampling-to-launch window.

Originally created by @dakjdakd on GitHub (Oct 16, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4512 ### 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 The focused fix and regression test are implemented on [`fix/process-pre-spawn-cancellation`](https://github.com/dakjdakd/rea/tree/fix/process-pre-spawn-cancellation), commit [`1fd8a884e`](https://github.com/dakjdakd/rea/commit/1fd8a884e57849de4bf09981cf8fb60eb7df7b23). I am working on the corresponding pull request alongside this report. Process capture can reach its PTY launch call after the caller has cancelled during the final executable metadata read or descriptor close. The cancellation arrives before any target has been started, but `startCaptureRuntime` continues from `await observeSelectedExecutable(...)` into `spawn(...)` without checking the signal again. I reproduced the launch **attempt** through the exported production `captureProcessScenario` boundary. The executable was opened, read, inspected, and closed using real filesystem operations. The native PTY entry point was replaced with a throwing sentinel, so no target process was created. On the affected source, both late-cancellation cases called that sentinel once. Earlier cancellation controls did not call it. This report concerns the decision to initiate execution. It does not claim that the sampler's completed digest is invalid, that cleanup leaked a real process, or that a real target caused damage. A native Linux/macOS capture and public CLI/MCP cancellation were not executed for this reproduction. ### Reproduction The regression is `tests/process-global/processPrelaunchCancellation.test.ts` in the proposed fix. It exercises the actual capture harness rather than reproducing its launch logic in a separate helper. 1. Parse a scenario selecting `process.execPath`, with arguments `["--eval", "process.exit(0)"]` and the checkout as its working directory. Supply an `AbortController.signal` and an empty host environment. 2. Preserve actual `node:fs/promises.open`, executable reads, descriptor `stat`, and descriptor `close`. Wrap the returned real handle only to synchronize an abort with a chosen completion boundary. 3. For the first affected case, abort after the final real descriptor `stat` resolves. This is the second descriptor metadata inspection in the sampler, after the executable bytes have been read. 4. For the second affected case, abort after the real descriptor `close` resolves. The descriptor has already been released when cancellation is raised. 5. Invoke `captureProcessScenario` with the existing platform and cleanup seams. Native ownership preparation/baseline discovery are replaced with controlled host responses; PTY `spawn` is intercepted and throws before creating a process. The fixture checks that interception is installed before entering the harness. 6. Observe whether the launch sentinel was called and verify descriptor release and temporary-root removal. Run the new regression together with the existing early-metadata cancellation control: ```sh node node_modules/vitest/vitest.mjs run \ --project process-global --project process-boundary \ tests/process-global/processPrelaunchCancellation.test.ts \ tests/boundary/process/selectedExecutableCancellation.test.ts \ --reporter=verbose ``` The deterministic observations on the affected base were: | Cancellation point | Launch sentinel calls | Interpretation | | --- | ---: | --- | | No cancellation | 1 | The ordinary path reaches the launch boundary. | | Immediately after executable open | 0 | Existing earlier cancellation checks stop startup. | | After final descriptor stat | 1 | Startup continues after cancellation during sample finalization. | | After descriptor close | 1 | Startup continues after cancellation during sample finalization. | | Before the capture call | 0 | Admission rejects an already-cancelled request. | The uninterrupted case deliberately ends at the throwing sentinel; it establishes reachability, not a successful native capture. The two affected boundaries reproduced under their explicit fault-injection schedules. Their frequency under normal filesystem timing is `NOT_ENOUGH_INFO`; no timing-based incidence estimate is available. ### Expected behavior Recheck the caller's signal after the awaited executable sampling has fully settled and before launching the selected program. If the signal is already aborted, take the existing typed cancellation/startup-cleanup path and make zero PTY launch attempts. The [lifecycle guidance](https://github.com/morluto/rea/blob/180ba03bfddd5916251e81e97a500b1ede7a9eea/docs/tool-design.md#own-the-lifecycle) separates target admission, active execution, and resource release, and requires cancellation to remain authoritative through awaited finalization. An executable sample establishes file facts; it should not independently authorize a later launch after the requesting operation has been cancelled. Completed file observations should remain available. In particular, obtaining a complete SHA-256 before cancellation does not make that digest unknown. Preventing the subsequent launch should not require changing the sampler's return value or discarding the bytes it already observed. ### Impact and scope The observed scope is the shared process-capture startup path. Both CLI and MCP consumers use this harness, so the source path is shared; adapter-specific cancellation delivery was not tested here. The practical concern is that a request cancelled during its last awaited prerequisite can still cross the execution boundary. Once started, a selected program may begin work before later cancellation monitoring and cleanup run. That consequence follows from the call order; the fixture intentionally prevents execution and therefore does not demonstrate a particular target action, a process leak, or an isolation failure. The window is narrow but deterministic to exercise. Earlier checks cannot close it because they run before an awaited final metadata read and descriptor release. No additional permission, target format, provider, or transport option is involved. ### REA version Source revision `180ba03bfddd5916251e81e97a500b1ede7a9eea` (`rea-agents` 6.5.0). ### Runtime and platform Node.js v24.18.0, Windows x64, Vitest v5.0.3. The test uses the harness's controlled Linux platform/ownership seams to exercise portable startup logic; it does not establish native Windows PTY support or a real Linux execution result. ### Target and artifact context The local Node executable supplies real executable bytes and metadata. No extracted application, private target, external service, or credential is required. ### Interface Other: exported production `captureProcessScenario` boundary. CLI/MCP impact is source-derived. ### Earliest failure stage Cleanup, cancellation, or lifecycle: executable sampling has completed, immediately before PTY launch. ### Relevant output or evidence The affected [caller](https://github.com/morluto/rea/blob/180ba03bfddd5916251e81e97a500b1ede7a9eea/src/process/capture/ProcessHarness.ts#L271) awaits `observeSelectedExecutable` and proceeds to `spawn`. In the [sampler](https://github.com/morluto/rea/blob/180ba03bfddd5916251e81e97a500b1ede7a9eea/src/process/capture/ProcessCaptureLifecycle.ts#L551), the last ordinary cancellation check precedes the final awaited descriptor `stat`; the `finally` block then awaits descriptor close. Separate direct-sampler controls confirm that both late schedules retain the correct complete SHA-256 with `signal.aborted === true` and a closed descriptor. The caller must therefore make its own execution decision after consuming that completed sample. ### Typed outcome or recovery expected The existing `ProcessCaptureError` cancellation outcome, with cleanup completed and no target PID acquired. Retain existing startup resource cleanup; do not start a target solely to discover cancellation in a later monitoring phase. ### Analysis or suspected cause The final metadata and close awaits introduce cancellation opportunities after the sampler's last check. There is no intervening signal check at the consuming launch boundary. A single `assertNotCancelled(options.signal)` after the sampling await closes these schedules without changing descriptor ownership or completed-digest semantics. ### Additional context - #992 checks cancellation after **initial** metadata admission, before constructing a read stream with an already-aborted signal. Its regression does not cover final descriptor stat/close followed by PTY launch. - #4379 / #4443 address finalization cancellation in the shared `RegularFileRead` / `OwnedRegularFile` owner and preserve completed file facts. This sampler is independent; this report targets its downstream launch decision. - #3263 tests cancellation during an obstructed filesystem-root lookup in a final capture snapshot. It does not change executable sampling or the PTY launch gate. - #1011's provider-startup cancellation follow-up concerns ownership preparation in `ProviderProcess.ts`; #4005 concerns cancellation after target exit during capture cleanup. Neither covers this prelaunch boundary. These related fixes establish useful controls, but I found no focused report or pending change for this final sampling-to-launch window.

bsakaguchi closed this issue

2026-10-31 00:32:55 +00:00

bsakaguchi commented

2026-10-31 00:32:56 +00:00

Author

Owner

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

Fixed on main in d6ca7b3f20.

The shared launch consumer now checks cancellation immediately before PTY spawn, after executable sampling and descriptor cleanup have settled. The sampler keeps its completed file facts. The shared typed cancellation constructor now retains the exact caller reason, and admission, normalization, active-exit and finalization consumers pass that fact through directly.

Five controlled capture-API schedules cover uninterrupted launch reachability, pre-admission abort, post-open abort, final metadata completion and post-close abort. Executable reads/stats/closes are real; PTY is a throwing sentinel, so these tests create no target. A separate real Node capture cancels at terminal cleanup reporting and retains the completed exit, cleanup facts and exact object reason. Unchanged d3627 baseline:5 failures/1 pass across the two new files; final focused gate:114 passed across11 files, including real CLI signals, MCP cancellation, process cleanup/finalization and progress. Full npm run check, docs:check (5), and discovery (1608 files) passed.

Six paired compiled SDK workflows (actual controlled Node capture, whole207-tool catalog, stable filesystem and retained unknown) preserved catalog and semantic observation hashes. Warm repeated medians: elapsed3.537→3.411s, CPU4.261→4.180s, sampled process-family RSS889.9→889.5MB. Request estimate108 cl100k tokens; full SDK reply estimate814174→814103 reflects retained ambient capture variation. Small sample, no speedup/model-context/true-cold claim; fault and other-host costs unmeasured.

Adapted #4836 at014afa38929d46d58067b617c707759cd753cf8d after reviewing its full diff; contributor credited in the commit, PR not merged. Native Windows qualification was not performed. The separately verified snapshot/sampler failed-close ownership problem remains open as #5140; execution admission and reason provenance do not repair that lifecycle ownership.

@morluto commented on GitHub (Oct 19, 2026): Fixed on main in d6ca7b3f20a273823c50d6253f0b6f34f3876f53. The shared launch consumer now checks cancellation immediately before PTY spawn, after executable sampling and descriptor cleanup have settled. The sampler keeps its completed file facts. The shared typed cancellation constructor now retains the exact caller reason, and admission, normalization, active-exit and finalization consumers pass that fact through directly. Five controlled capture-API schedules cover uninterrupted launch reachability, pre-admission abort, post-open abort, final metadata completion and post-close abort. Executable reads/stats/closes are real; PTY is a throwing sentinel, so these tests create no target. A separate real Node capture cancels at terminal cleanup reporting and retains the completed exit, cleanup facts and exact object reason. Unchanged d3627 baseline:5 failures/1 pass across the two new files; final focused gate:114 passed across11 files, including real CLI signals, MCP cancellation, process cleanup/finalization and progress. Full npm run check, docs:check (5), and discovery (1608 files) passed. Six paired compiled SDK workflows (actual controlled Node capture, whole207-tool catalog, stable filesystem and retained unknown) preserved catalog and semantic observation hashes. Warm repeated medians: elapsed3.537→3.411s, CPU4.261→4.180s, sampled process-family RSS889.9→889.5MB. Request estimate108 cl100k tokens; full SDK reply estimate814174→814103 reflects retained ambient capture variation. Small sample, no speedup/model-context/true-cold claim; fault and other-host costs unmeasured. Adapted #4836 at014afa38929d46d58067b617c707759cd753cf8d after reviewing its full diff; contributor credited in the commit, PR not merged. Native Windows qualification was not performed. The separately verified snapshot/sampler failed-close ownership problem remains open as #5140; execution admission and reason provenance do not repair that lifecycle ownership.

bsakaguchi referenced this issue

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

[PR #1785] [MERGED] fix(ida): report a procedure IDA cannot resolve as invalid input #4815

bsakaguchi referenced this issue

2026-10-31 00:56:33 +00:00

[PR #1906] [MERGED] fix(ida): preserve non-x86 call edges and workflow uncertainty #4898