morluto/rea

Fork 0

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

[GH-ISSUE #4253] [Bug] One-shot MCP shutdown listeners prevent same-event cleanup retries #1677

New issue

Closed

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

bsakaguchi commented

2026-10-16 21:08:54 +00:00

Owner

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

Area

MCP or agent integration

Observed behavior

A failed shutdown leaves its cleanup owners retryable, but the production shutdown registration uses once for each process/stdin event. Repeating the same event does not invoke cleanup again. Other shutdown event types may still have an unconsumed listener; this report does not claim that every alternate retry path disappears after one event.

Reproduction

Baseline: REA 6.5.0, 0e1f0bf2d06943e59253b8a66923a1ae6005e81e; Node 24.18.0, macOS arm64.

Extracted the exact production once/off registration from src/main.ts:29-39 into a named registration function, without changing its behavior. A production-boundary test connects that registration and createShutdown to Node EventEmitters, makes the first transport close reject, emits SIGINT, awaits the completed failure, then emits SIGINT again. It observes one close attempt and one shutdown-failure diagnostic. It does not manually reinvoke the stored callback.

Executed: npm run test:local -- src/server/shutdownRegistration.test.ts with pinned Node 24. The characterization passes by asserting the defective observation. The extracted function and test will accompany the fix PR. No OS signal was sent and no real stdio transport failure was injected; this is a production-registration boundary reproduction.

Expected behavior

Keep shutdown event registration available while cleanup is incomplete. Repeated shutdown requests should retry unresolved owners, preserve already-closed owners, and remove registrations only after confirmed cleanup. Exercise each registered event and concurrent requests.

Impact and scope

A repeated request from the same shutdown source cannot retry retained cleanup. Actual subprocess termination/leak frequency is unmeasured. The existing lifecycle implementation itself supports retries: src/main/shutdown.ts:47-74 resets a rejected attempt and preserves successful owners. Its tests call the request callback directly, which misses production listener consumption.

Analysis or suspected cause

The event source owns registration lifetime, while cleanup completion owns retry lifetime. One-shot event wiring ends that registration before cleanup success is known. run() discards the returned lifecycle handle, so the production path relies on the registrations for subsequent requests.

Relevant evidence and dedupe

Source: src/main.ts, src/main/shutdown.ts, tests/boundary/mcp/shutdown.test.ts. Searches across open/closed shutdown, SIGINT, EOF and cleanup issues found no focused duplicate; #147 concerns older handler removal and #4227 is the broad lifecycle audit.

Originally created by @morluto on GitHub (Oct 15, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4253 ### Area MCP or agent integration ### Observed behavior A failed shutdown leaves its cleanup owners retryable, but the production shutdown registration uses `once` for each process/stdin event. Repeating the same event does not invoke cleanup again. Other shutdown event types may still have an unconsumed listener; this report does not claim that every alternate retry path disappears after one event. ### Reproduction Baseline: REA 6.5.0, `0e1f0bf2d06943e59253b8a66923a1ae6005e81e`; Node 24.18.0, macOS arm64. Extracted the exact production `once`/`off` registration from `src/main.ts:29-39` into a named registration function, without changing its behavior. A production-boundary test connects that registration and `createShutdown` to Node EventEmitters, makes the first transport close reject, emits SIGINT, awaits the completed failure, then emits SIGINT again. It observes one close attempt and one shutdown-failure diagnostic. It does not manually reinvoke the stored callback. Executed: `npm run test:local -- src/server/shutdownRegistration.test.ts` with pinned Node 24. The characterization passes by asserting the defective observation. The extracted function and test will accompany the fix PR. No OS signal was sent and no real stdio transport failure was injected; this is a production-registration boundary reproduction. ### Expected behavior Keep shutdown event registration available while cleanup is incomplete. Repeated shutdown requests should retry unresolved owners, preserve already-closed owners, and remove registrations only after confirmed cleanup. Exercise each registered event and concurrent requests. ### Impact and scope A repeated request from the same shutdown source cannot retry retained cleanup. Actual subprocess termination/leak frequency is unmeasured. The existing lifecycle implementation itself supports retries: `src/main/shutdown.ts:47-74` resets a rejected attempt and preserves successful owners. Its tests call the request callback directly, which misses production listener consumption. ### Analysis or suspected cause The event source owns registration lifetime, while cleanup completion owns retry lifetime. One-shot event wiring ends that registration before cleanup success is known. `run()` discards the returned lifecycle handle, so the production path relies on the registrations for subsequent requests. ### Relevant evidence and dedupe Source: `src/main.ts`, `src/main/shutdown.ts`, `tests/boundary/mcp/shutdown.test.ts`. Searches across open/closed shutdown, SIGINT, EOF and cleanup issues found no focused duplicate; #147 concerns older handler removal and #4227 is the broad lifecycle audit.

bsakaguchi

2026-10-16 21:08:54 +00:00

closed this issue
added the
triage

bug
labels

bsakaguchi commented

2026-10-16 21:08:56 +00:00

Author

Owner

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

Fixed on main in 8873b3977. The shutdown registration owner keeps same-event signal listeners available until cleanup succeeds, allowing a failed owner to be retried.

Production-registration tests cover repeated events and mixed cleanup outcomes; these are controlled event tests, not a real OS-signal integration claim.

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

@morluto commented on GitHub (Oct 16, 2026): Fixed on `main` in [8873b3977](https://github.com/morluto/rea/commit/8873b39770e89eae22b7693bc492d7e423b22bea). The shutdown registration owner keeps same-event signal listeners available until cleanup succeeds, allowing a failed owner to be retried. Production-registration tests cover repeated events and mixed cleanup outcomes; these are controlled event tests, not a real OS-signal integration claim. Hosted main CI is still pending; this closure is based on the implemented fix and the scoped regression evidence.

bsakaguchi referenced this issue

2026-10-31 00:54:38 +00:00

[PR #1677] [CLOSED] fix(native): report missing and non-file PE resource targets as input errors #4746

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

No description provided.

Rows
Columns