morluto/rea

Fork 0

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

[GH-ISSUE #4885] [Bug] SIGINT or SIGTERM during the initial target open kills REA without closing its session #3035

New issue

Closed

opened 2026-10-31 00:35:12 +00:00 by bsakaguchi · 6 comments

bsakaguchi commented

2026-10-31 00:35:12 +00:00

Owner

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

Before submitting

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

Area

MCP or agent integration

Observed behavior

REA's shutdown handlers are installed only after the initial target opens and the MCP transport starts. A SIGINT or SIGTERM during a slow HOPPER_TARGET_PATH open is handled by Node's default handler, so REA exits without calling session.close().

In a test that blocks the initial open's provider health call, no shutdown handler was registered while the open was pending. In a test that blocks optional-provider loading during transport start, the same holds.

Reproduction

Set HOPPER_TARGET_PATH to a target whose provider takes a while to start.
Start rea as an MCP server.
Send SIGINT or SIGTERM before the initial open completes.
REA exits through the default signal handler. session.close() does not run, and provider children started for the open are not closed by REA.

Expected behavior

The run() docstring in src/main.ts says EOF and process signals close bridge resources. A signal or stdin EOF during startup should abort the initial open and close the session, even if the transport never started. #4884 covers SIGINT and SIGTERM. Stdin EOF during startup is not covered: nothing reads process.stdin before the transport starts, so the early end and close listeners do not fire.

Impact and scope

Provider children are spawned with detached: platform !== "win32" in src/process/ProviderProcess.ts, so on Linux and macOS they can outlive REA after a default signal exit. This only affects the window between session creation and transport start; signals after startup are handled.

REA version

6.5.0, revision 8ab502439

Runtime and platform

Node 24. Observed through the run() dependency seams on Windows 11. The orphaned-child outcome is source-derived for Linux and macOS.

Target and artifact context

Any initial target set with HOPPER_TARGET_PATH. Synthetic fixture targets only.

Interface

MCP

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

registerShutdown was not called while the initial open was pending. A transport handle that finished starting after a would-be shutdown request was closed 0 times.

Typed outcome or recovery expected

The initial open returns cancellation, the session closes with retained provider documents, the transport is never started or is closed once if it already started, and REA exits 0 after clean cleanup or 1 with the existing shutdown-failure message.

Analysis or suspected cause

Source-derived. run() in src/main.ts awaits openInitialTarget (session.open(config.hopperTargetPath, { targetKind }), no signal) and startMcpTransport, and only then calls createShutdown, which calls registerProcessShutdown (src/main/processShutdown.ts) for SIGINT, SIGTERM, and stdin end/close.

Additional context

Fix is in https://github.com/morluto/rea/pull/4884.

Originally created by @Adolanium on GitHub (Oct 17, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4885 ### 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 MCP or agent integration ### Observed behavior REA's shutdown handlers are installed only after the initial target opens and the MCP transport starts. A SIGINT or SIGTERM during a slow `HOPPER_TARGET_PATH` open is handled by Node's default handler, so REA exits without calling `session.close()`. In a test that blocks the initial open's provider `health` call, no shutdown handler was registered while the open was pending. In a test that blocks optional-provider loading during transport start, the same holds. ### Reproduction 1. Set `HOPPER_TARGET_PATH` to a target whose provider takes a while to start. 2. Start `rea` as an MCP server. 3. Send SIGINT or SIGTERM before the initial open completes. 4. REA exits through the default signal handler. `session.close()` does not run, and provider children started for the open are not closed by REA. ### Expected behavior The `run()` docstring in `src/main.ts` says EOF and process signals close bridge resources. A signal or stdin EOF during startup should abort the initial open and close the session, even if the transport never started. #4884 covers SIGINT and SIGTERM. Stdin EOF during startup is not covered: nothing reads `process.stdin` before the transport starts, so the early `end` and `close` listeners do not fire. ### Impact and scope Provider children are spawned with `detached: platform !== "win32"` in `src/process/ProviderProcess.ts`, so on Linux and macOS they can outlive REA after a default signal exit. This only affects the window between session creation and transport start; signals after startup are handled. ### REA version 6.5.0, revision `8ab502439` ### Runtime and platform Node 24. Observed through the `run()` dependency seams on Windows 11. The orphaned-child outcome is source-derived for Linux and macOS. ### Target and artifact context Any initial target set with `HOPPER_TARGET_PATH`. Synthetic fixture targets only. ### Interface MCP ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence `registerShutdown` was not called while the initial open was pending. A transport handle that finished starting after a would-be shutdown request was closed 0 times. ### Typed outcome or recovery expected The initial open returns cancellation, the session closes with retained provider documents, the transport is never started or is closed once if it already started, and REA exits 0 after clean cleanup or 1 with the existing shutdown-failure message. ### Analysis or suspected cause Source-derived. `run()` in `src/main.ts` awaits `openInitialTarget` (`session.open(config.hopperTargetPath, { targetKind })`, no signal) and `startMcpTransport`, and only then calls `createShutdown`, which calls `registerProcessShutdown` (`src/main/processShutdown.ts`) for SIGINT, SIGTERM, and stdin `end`/`close`. ### Additional context Fix is in https://github.com/morluto/rea/pull/4884.

bsakaguchi closed this issue

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

bsakaguchi commented

2026-10-31 00:35:14 +00:00

Author

Owner

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

Agreed — installing shutdown handlers only after the initial open and transport start leaves a real window where signals fall through to Node's default handler and provider children (spawned detached on Linux/macOS) can outlive the process. Registering them early with a guard so they abort the pending open and close the session is the right fix; #4884's scope (SIGINT/SIGTERM) covers the common case.

One thing worth folding in while the lifecycle is being touched: the same window affects stdin EOF, since nothing reads process.stdin before the transport starts. If the early signal handler can also subscribe to stdin end/close (or the MCP server binds stdin at startup rather than after open), the startup abort path becomes uniform for both signals and EOF. That closes the whole "process interrupted during startup" surface instead of just the signal half.

@XinwangTao commented on GitHub (Oct 18, 2026): Agreed — installing shutdown handlers only after the initial open and transport start leaves a real window where signals fall through to Node's default handler and provider children (spawned detached on Linux/macOS) can outlive the process. Registering them early with a guard so they abort the pending open and close the session is the right fix; #4884's scope (SIGINT/SIGTERM) covers the common case. One thing worth folding in while the lifecycle is being touched: the same window affects stdin EOF, since nothing reads process.stdin before the transport starts. If the early signal handler can also subscribe to stdin end/close (or the MCP server binds stdin at startup rather than after open), the startup abort path becomes uniform for both signals and EOF. That closes the whole "process interrupted during startup" surface instead of just the signal half.

bsakaguchi commented

2026-10-31 00:35:14 +00:00

Author

Owner

@Adolanium commented on GitHub (Oct 18, 2026):

Thanks. #4884 already registers the stdin end and close listeners together with the signal handlers before the initial open (registerProcessShutdown in src/main/processShutdown.ts). They can't observe EOF during startup, though: nothing reads stdin until the MCP transport starts, and attaching an end listener doesn't switch the stream to flowing mode. A comment in src/main.ts records this.

Making EOF visible earlier would mean reading stdin before the transport exists. Any MCP messages the client sends during startup would then have to be buffered and handed to the transport, so I left it out of #4884's scope. Once the transport reads stdin, the existing listeners catch EOF and run the same shutdown path. The gap is limited to the open itself: if the client disconnects mid-open, REA finishes the open and then shuts down cleanly. If maintainers want startup EOF to abort the open too, I'd rather track that as a separate change.

@Adolanium commented on GitHub (Oct 18, 2026): Thanks. #4884 already registers the stdin `end` and `close` listeners together with the signal handlers before the initial open (`registerProcessShutdown` in `src/main/processShutdown.ts`). They can't observe EOF during startup, though: nothing reads stdin until the MCP transport starts, and attaching an `end` listener doesn't switch the stream to flowing mode. A comment in `src/main.ts` records this. Making EOF visible earlier would mean reading stdin before the transport exists. Any MCP messages the client sends during startup would then have to be buffered and handed to the transport, so I left it out of #4884's scope. Once the transport reads stdin, the existing listeners catch EOF and run the same shutdown path. The gap is limited to the open itself: if the client disconnects mid-open, REA finishes the open and then shuts down cleanly. If maintainers want startup EOF to abort the open too, I'd rather track that as a separate change.

bsakaguchi commented

2026-10-31 00:35:14 +00:00

Author

Owner

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

That's a fair scoping call — attaching an end listener without switching the stream to flowing mode is indeed not enough to observe EOF, and buffering pre-transport MCP messages would complicate #4884 for a narrow edge case. The residual gap (client disconnects mid-open, REA finishes the open then shuts down cleanly) is acceptable: the session still closes, just later than an ideal abort. A separate change could add a startup flag like --expect-stdin-eof or read stdin into a buffer only when the client opts in; the docstring note in src/main.ts you added should be enough to keep the window discoverable until then.

@XinwangTao commented on GitHub (Oct 18, 2026): That's a fair scoping call — attaching an end listener without switching the stream to flowing mode is indeed not enough to observe EOF, and buffering pre-transport MCP messages would complicate #4884 for a narrow edge case. The residual gap (client disconnects mid-open, REA finishes the open then shuts down cleanly) is acceptable: the session still closes, just later than an ideal abort. A separate change could add a startup flag like --expect-stdin-eof or read stdin into a buffer only when the client opts in; the docstring note in src/main.ts you added should be enough to keep the window discoverable until then.

bsakaguchi commented

2026-10-31 00:35:15 +00:00

Author

Owner

@Varshavia commented on GitHub (Oct 30, 2026):

Checked current main (c5f50e80d; source-only, scenario not replayed): startup stdin appears to be owned now. src/main/StdioStartupInput.ts handles stdin during startup and src/main/eagerRuntime.ts uses it, which addresses the stdin EOF gap described here. The closing comment on #4884 attributes the fix to 2a4c70e3b ("fix(mcp): own cancellation and raw input throughout startup"). 2a4c70e3b is in main's history; I did not review its diff. This looks resolved on main and may be closable.

@Varshavia commented on GitHub (Oct 30, 2026): Checked current `main` (c5f50e80d; source-only, scenario not replayed): startup stdin appears to be owned now. `src/main/StdioStartupInput.ts` handles stdin during startup and `src/main/eagerRuntime.ts` uses it, which addresses the stdin EOF gap described here. The closing comment on #4884 attributes the fix to 2a4c70e3b ("fix(mcp): own cancellation and raw input throughout startup"). 2a4c70e3b is in main's history; I did not review its diff. This looks resolved on `main` and may be closable.

bsakaguchi commented

2026-10-31 00:35:15 +00:00

Author

Owner

@Adolanium commented on GitHub (Oct 30, 2026):

Thanks for checking. Confirmed on main: 2a4c70e3b registers the SIGINT/SIGTERM shutdown before the initial open, and ownStdioStartupInput reads and buffers stdin during startup, so EOF now stops startup and the buffered bytes are handed to the transport. tests/process-global/mainStartupCancellation.test.ts covers both. Closing as resolved.

@Adolanium commented on GitHub (Oct 30, 2026): Thanks for checking. Confirmed on main: 2a4c70e3b registers the SIGINT/SIGTERM shutdown before the initial open, and `ownStdioStartupInput` reads and buffers stdin during startup, so EOF now stops startup and the buffered bytes are handed to the transport. `tests/process-global/mainStartupCancellation.test.ts` covers both. Closing as resolved.

bsakaguchi referenced this issue

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

[PR #3035] [CLOSED] fix(javascript): stop overflowing the stack on large module factories #5571

bsakaguchi referenced this issue

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

[GH-ISSUE #3582] [Tracker] Ready to close or merge: issues and pull requests (October 2026) #1330

bsakaguchi commented

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

Author

Owner

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

Confirmed — registering SIGINT/SIGTERM shutdown before the initial open closes the orphan-window gap, and ownStdioStartupInput addressing the stdin EOF case during startup was exactly the follow-up this issue needed. mainStartupCancellation.test.ts covering both paths gives good confidence. Thanks for closing this out.

@XinwangTao commented on GitHub (Oct 31, 2026): Confirmed — registering SIGINT/SIGTERM shutdown before the initial open closes the orphan-window gap, and ownStdioStartupInput addressing the stdin EOF case during startup was exactly the follow-up this issue needed. mainStartupCancellation.test.ts covering both paths gives good confidence. Thanks for closing this out.

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

No description provided.

Rows
Columns