morluto/rea

Fork 0

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

[GH-ISSUE #5163] [Bug] MCP close_binary drops process cleanup failure when session close also fails #3173

New issue

Closed

opened 2026-10-31 00:37:30 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-31 00:37:30 +00:00

Owner

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

Before submitting

Searched open and closed reports for close_binary cleanup and process_capture_resources; related #5144 concerns stale recovery status, rather than loss of a second failed child.
Evidence contains only controlled temporary resources and diagnostics.

Area

Process capture or runtime observation / MCP lifecycle.

Observed behavior

At dfe8378cf5, a public in-memory SDK close_binary request returned only a typed session-close invalid_request failure while a separate real process-capture temporary root remained owned after cleanup denial. The response omitted cleanup_incomplete and the pending resource. The root was subsequently released through the owning scope after restoring real removal.

Reproduction and evidence

A production-server regression uses createServer, the official SDK client and linked in-memory transports. A test session supplies a controlled AnalysisInputError close result; a narrow retry seam seeds the actual ProcessCaptureResourceScope with a real mkdtemp root, denies its actual removal, then delegates to the original retry method. No analysis engine or target process is opened. Both the clean baseline and current #5144 candidate reproduce the mixed-child loss. Test: tests/boundary/mcp/processCleanupRetryMcp.test.ts (new regression, not yet committed when reported).

Smallest result:

{"code":"invalid_request","details":{"operation":"close_binary","issues":[{"path":["session"],"reason":"invalid_value","message":"primary session-close diagnostic"}]}}

Expected concurrent fact: cleanup remains incomplete for temporary_root.

Root cause / expected behavior

registerCloseLifecycleTool awaits both children, but when session close fails it only logs process cleanup failure and returns the first child. Compose the original typed session failure and exact cleanup dependency, retaining all known facts. A later successful owner retry should clear current cleanup uncertainty while preserving the primary diagnostic and historical response.

Scope and limitations

Observed public SDK/in-memory MCP result and actual temporary-root retention/removal on macOS arm64, Node24.18.0. Session failure and removal denial are controlled seams; no real-provider teardown, natural denial, stdio transport or affected user application is claimed. Natural frequency and real-engine impact: NOT_ENOUGH_INFO.

Proposed validation

Keep separate session-only, cleanup-only, both-failed and successful-retry controls, verifying live dependency resolution and immutable historical wire results. Related source branches where session close rejects should preserve the second child as well.

Originally created by @morluto on GitHub (Oct 19, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5163 ### Before submitting - [x] Searched open and closed reports for `close_binary` cleanup and `process_capture_resources`; related #5144 concerns stale recovery status, rather than loss of a second failed child. - [x] Evidence contains only controlled temporary resources and diagnostics. ### Area Process capture or runtime observation / MCP lifecycle. ### Observed behavior At dfe8378cf5a727e3d93d47a5986233175c6ef898, a public in-memory SDK `close_binary` request returned only a typed session-close `invalid_request` failure while a separate real process-capture temporary root remained owned after cleanup denial. The response omitted `cleanup_incomplete` and the pending resource. The root was subsequently released through the owning scope after restoring real removal. ### Reproduction and evidence A production-server regression uses `createServer`, the official SDK client and linked in-memory transports. A test session supplies a controlled `AnalysisInputError` close result; a narrow retry seam seeds the actual `ProcessCaptureResourceScope` with a real `mkdtemp` root, denies its actual removal, then delegates to the original retry method. No analysis engine or target process is opened. Both the clean baseline and current #5144 candidate reproduce the mixed-child loss. Test: `tests/boundary/mcp/processCleanupRetryMcp.test.ts` (new regression, not yet committed when reported). Smallest result: ```json {"code":"invalid_request","details":{"operation":"close_binary","issues":[{"path":["session"],"reason":"invalid_value","message":"primary session-close diagnostic"}]}} ``` Expected concurrent fact: cleanup remains incomplete for `temporary_root`. ### Root cause / expected behavior `registerCloseLifecycleTool` awaits both children, but when session close fails it only logs process cleanup failure and returns the first child. Compose the original typed session failure and exact cleanup dependency, retaining all known facts. A later successful owner retry should clear current cleanup uncertainty while preserving the primary diagnostic and historical response. ### Scope and limitations Observed public SDK/in-memory MCP result and actual temporary-root retention/removal on macOS arm64, Node24.18.0. Session failure and removal denial are controlled seams; no real-provider teardown, natural denial, stdio transport or affected user application is claimed. Natural frequency and real-engine impact: NOT_ENOUGH_INFO. ### Proposed validation Keep separate session-only, cleanup-only, both-failed and successful-retry controls, verifying live dependency resolution and immutable historical wire results. Related source branches where session close rejects should preserve the second child as well.

bsakaguchi closed this issue

2026-10-31 00:37:30 +00:00

bsakaguchi commented

2026-10-31 00:37:30 +00:00

Author

Owner

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

Fixed on main in 974487f00 together with #5144 and #5166.

close_binary starts both cleanup children before joining their results. Returned, rejected and synchronous typed session failures now follow one Result boundary. If process cleanup also fails, the result composes both typed failures and binds live status to their exact cleanup dependencies. The MCP adapter preserves the scope's typed retry error instead of replacing it with an unbound generic resource label.

Public SDK regressions use the real server and in-memory transport, a controlled typed session failure, and an actual temporary root with controlled removal denial. They verify exact resource attribution, primary issue diagnostics, completed retry, composed-error resolution and unchanged historical wire results. Cleanup-only, primary-only after recovery, and successful-close controls pass. No native engine is opened, and no stdio or natural provider-failure claim is made.

Validation: the clean baseline loses resource attribution in both tested close cases; final mixed-close controls cover returned, rejected and synchronous typed failures. The broader run passed 156 tests/16 files, then 72 affected tests/7 files after final ownership hardening. Static/architecture, docs and discovery checks passed. Unexpected untyped failures retain their existing generic exception path; this change covers the declared typed failure contract.

@morluto commented on GitHub (Oct 19, 2026): Fixed on main in [`974487f00`](https://github.com/morluto/rea/commit/974487f00f2d68912c7c2c9e033a2760a343a82e) together with #5144 and #5166. `close_binary` starts both cleanup children before joining their results. Returned, rejected and synchronous typed session failures now follow one Result boundary. If process cleanup also fails, the result composes both typed failures and binds live status to their exact cleanup dependencies. The MCP adapter preserves the scope's typed retry error instead of replacing it with an unbound generic resource label. Public SDK regressions use the real server and in-memory transport, a controlled typed session failure, and an actual temporary root with controlled removal denial. They verify exact resource attribution, primary issue diagnostics, completed retry, composed-error resolution and unchanged historical wire results. Cleanup-only, primary-only after recovery, and successful-close controls pass. No native engine is opened, and no stdio or natural provider-failure claim is made. Validation: the clean baseline loses resource attribution in both tested close cases; final mixed-close controls cover returned, rejected and synchronous typed failures. The broader run passed 156 tests/16 files, then 72 affected tests/7 files after final ownership hardening. Static/architecture, docs and discovery checks passed. Unexpected untyped failures retain their existing generic exception path; this change covers the declared typed failure contract.

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

No description provided.

Rows
Columns