[GH-ISSUE #4840] [Bug] Provider stop discards diagnostics between process exit and output close #3008

New issue

Closed

opened 2026-10-31 00:34:46 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-31 00:34:46 +00:00

Owner

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

Before submitting

Searched open and closed supervisor/output reports. Related #3517 and lifecycle tracker #4227; no matching shared stop/output-close report found.
Removed credentials and private target data.

Area

Process capture or runtime observation

Observed behavior

At source 3fa972af7, ProviderProcessSupervisor.stop() disposes stdout/stderr/close listeners as soon as process exit or verified process cleanup is observed. Exit does not establish output closure. Diagnostics delivered between exit and close are permanently lost, and a later waitForOutputClose() cannot observe the removed close listener. Snapshots can then omit an incomplete-output marker.

Source: supervisor finalization. Node 24 documents that stdio may still be open at exit: https://nodejs.org/download/release/v24.18.0/docs/api/child_process.html#event-exit.

Reproduction

Controlled production-boundary tests, no real provider: npm run test:local -- src/process/ProviderProcessOutput.test.ts with three added regressions at the stated baseline. All three fail: final stderr is empty; late bytes/incompleteness are missing; cleanup retry retains earlier bytes but loses its final tail. Existing nine tests pass. Fixtures use EventEmitter/PassThrough, explicit exit/close barriers and fake timers for bounded waiting.

Expected behavior

Keep capture attached through bounded output finalization, bound additional retained bytes while draining, and explicitly mark incomplete output if it cannot settle. Preserve verified process cleanup as a separate fact; missing pipe closure must not invent failed process cleanup. Retain ownership/capture when process cleanup fails and is retried. Borrowed streams must not be killed or destroyed.

Impact and scope

Source consumers include Android/Ghidra retirement and command/worker failure cleanup. Android additionally snapshots startup errors before retirement; correcting the shared producer alone will not update that earlier snapshot. Actual real-provider failure frequency is NOT_ENOUGH_INFO. No Windows/native-provider behavior is established by these controls.

REA version

6.5.0 source 3fa972af7c.

Runtime and platform

Node 24.18.0, macOS arm64; controlled lifecycle fixtures.

Target and artifact context

No target execution, APK, private binary or provider required.

Interface

Other: shared process supervision consumed by CLI/MCP workflows.

Earliest failure stage

Cleanup, cancellation, or lifecycle

Originally created by @morluto on GitHub (Oct 17, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4840 ### Before submitting - [x] Searched open and closed supervisor/output reports. Related #3517 and lifecycle tracker #4227; no matching shared stop/output-close report found. - [x] Removed credentials and private target data. ### Area Process capture or runtime observation ### Observed behavior At source 3fa972af7, `ProviderProcessSupervisor.stop()` disposes stdout/stderr/close listeners as soon as process exit or verified process cleanup is observed. Exit does not establish output closure. Diagnostics delivered between exit and close are permanently lost, and a later `waitForOutputClose()` cannot observe the removed close listener. Snapshots can then omit an incomplete-output marker. Source: [supervisor finalization](https://github.com/morluto/rea/blob/3fa972af7c7cb6cf014ffc426bdf7130dc6f2fd2/src/process/ProviderProcess.ts#L561). Node 24 documents that stdio may still be open at exit: https://nodejs.org/download/release/v24.18.0/docs/api/child_process.html#event-exit. ### Reproduction Controlled production-boundary tests, no real provider: `npm run test:local -- src/process/ProviderProcessOutput.test.ts` with three added regressions at the stated baseline. All three fail: final stderr is empty; late bytes/incompleteness are missing; cleanup retry retains earlier bytes but loses its final tail. Existing nine tests pass. Fixtures use EventEmitter/PassThrough, explicit exit/close barriers and fake timers for bounded waiting. ### Expected behavior Keep capture attached through bounded output finalization, bound additional retained bytes while draining, and explicitly mark incomplete output if it cannot settle. Preserve verified process cleanup as a separate fact; missing pipe closure must not invent failed process cleanup. Retain ownership/capture when process cleanup fails and is retried. Borrowed streams must not be killed or destroyed. ### Impact and scope Source consumers include Android/Ghidra retirement and command/worker failure cleanup. Android additionally snapshots startup errors before retirement; correcting the shared producer alone will not update that earlier snapshot. Actual real-provider failure frequency is NOT_ENOUGH_INFO. No Windows/native-provider behavior is established by these controls. ### REA version 6.5.0 source 3fa972af7c7cb6cf014ffc426bdf7130dc6f2fd2. ### Runtime and platform Node 24.18.0, macOS arm64; controlled lifecycle fixtures. ### Target and artifact context No target execution, APK, private binary or provider required. ### Interface Other: shared process supervision consumed by CLI/MCP workflows. ### Earliest failure stage Cleanup, cancellation, or lifecycle

bsakaguchi closed this issue

2026-10-31 00:34:46 +00:00

bsakaguchi commented

2026-10-31 00:34:47 +00:00

Author

Owner

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

Fixed on main in 7016951394.

Owned stop now bounds additional retained diagnostics to 64 KiB before awaiting cleanup, preserves that bound/capture through failed cleanup and retries, and waits up to one second for output close after verified stop. Missing closure, early capture disposal and stream read failures remain explicit as incomplete output. Verified process cleanup keeps its own disposition. Output timeout releases owned event-loop references; borrowed process/stream reference state is preserved.

Validation: 257 tests passed across 13 focused files, five skipped. Coverage includes shared process output/commands, Ghidra and Android boundaries, and simulated Windows ownership. Two Windows helper tests initially could not import the locally missing declared tsx dependency; locked npm ci restored it and both suites passed (24 tests, two skips). Static/docs checks and commit/push hooks passed.

The original three controlled regressions fail on the baseline. The real POSIX inherited-stderr test uses a release pipe after leader exit: baseline drops the final ten bytes; candidate retains them and observes output closure. Additional controls cover held cleanup, bounded retention/drain, retry, read errors and borrowed authority. No native Windows or real provider startup-failure reproduction is claimed.

Six real MCP comparison workflows on a tiny authored APK, three known-name inspections each, discovery excluded: baseline 3fa972af7 versus final commit, two alternating warm-repeat medians. Elapsed 2.461/2.302 s; reaped-family CPU 3.789/3.532 s; sampled process-family RSS 852,516,864/848,904,192 bytes; cl100k output estimate 9,625/9,633 tokens. Normalized facts matched. No speedup claim: two warm repeats, no true cold/input/discovery/whole-agent measurement; sampled RSS may miss transient/reparented processes. This measures normal completed supervision, not failed cleanup or the one-second drain-timeout path. Final source hashes match the measured commit. All six measured roots were absent, no matching processes remained, and the owned baseline worktree/dependency link was removed.

Remaining consumer work: Android still copies startup diagnostics before retirement. It needs a frozen primary failure and terminal output presentation; moving all classification after cleanup would risk replacing a physical failure with a later cancellation. That distinct source finding is queued for its own report/fix. Heavy profiling and stress stayed stopped.

@morluto commented on GitHub (Oct 17, 2026): Fixed on main in 701695139490777ece7a3ccb8508b6215a11c0fe. Owned stop now bounds additional retained diagnostics to 64 KiB before awaiting cleanup, preserves that bound/capture through failed cleanup and retries, and waits up to one second for output close after verified stop. Missing closure, early capture disposal and stream read failures remain explicit as incomplete output. Verified process cleanup keeps its own disposition. Output timeout releases owned event-loop references; borrowed process/stream reference state is preserved. Validation: 257 tests passed across 13 focused files, five skipped. Coverage includes shared process output/commands, Ghidra and Android boundaries, and simulated Windows ownership. Two Windows helper tests initially could not import the locally missing declared tsx dependency; locked npm ci restored it and both suites passed (24 tests, two skips). Static/docs checks and commit/push hooks passed. The original three controlled regressions fail on the baseline. The real POSIX inherited-stderr test uses a release pipe after leader exit: baseline drops the final ten bytes; candidate retains them and observes output closure. Additional controls cover held cleanup, bounded retention/drain, retry, read errors and borrowed authority. No native Windows or real provider startup-failure reproduction is claimed. Six real MCP comparison workflows on a tiny authored APK, three known-name inspections each, discovery excluded: baseline 3fa972af7 versus final commit, two alternating warm-repeat medians. Elapsed 2.461/2.302 s; reaped-family CPU 3.789/3.532 s; sampled process-family RSS 852,516,864/848,904,192 bytes; cl100k output estimate 9,625/9,633 tokens. Normalized facts matched. No speedup claim: two warm repeats, no true cold/input/discovery/whole-agent measurement; sampled RSS may miss transient/reparented processes. This measures normal completed supervision, not failed cleanup or the one-second drain-timeout path. Final source hashes match the measured commit. All six measured roots were absent, no matching processes remained, and the owned baseline worktree/dependency link was removed. Remaining consumer work: Android still copies startup diagnostics before retirement. It needs a frozen primary failure and terminal output presentation; moving all classification after cleanup would risk replacing a physical failure with a later cancellation. That distinct source finding is queued for its own report/fix. Heavy profiling and stress stayed stopped.

bsakaguchi referenced this issue

2026-10-31 01:07:52 +00:00

[PR #3081] [CLOSED] fix(ghidra): discover an installed Ghidra from a launcher on PATH #5600

bsakaguchi referenced this issue

2026-11-01 21:10:57 +00:00

[GH-ISSUE #3796] [Tracker] Open feature requests by area (October 2026) #1471