[PR #6411] [MERGED] fix(lifecycle): reject session work once close has started #7340

Closed
opened 2026-11-01 21:47:11 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6411
Author: @morluto
Created: 10/22/2026
Status: ✅ Merged
Merged: 10/23/2026
Merged by: @morluto

Base: main ← Head: cursor/session-close-races-94bf


📝 Commits (2)

  • 3b609a0 fix(lifecycle): reject session work once close has started
  • efd799c fix(gdb): block console admission after cleanup starts

📊 Changes

5 files changed (+436 additions, -44 deletions)

View changed files

📝 src/application/binary/BinarySession.ts (+22 -0)
📝 src/gdb/GdbSessionManager.ts (+58 -40)
📝 src/main/shutdown.ts (+5 -2)
📝 tests/boundary/mcp/shutdown.test.ts (+124 -0)
📝 tests/boundary/providers/GdbSessionManagerCoverage.test.ts (+227 -2)

📄 Description

Summary

Fixes #6399 and #6370.

Once process shutdown or a GDB session close has started, later work must not reopen that session or repeat cleanup. A stdio tool call could still open the shared binary session after shutdown's close was queued, and a queued gdb_console command or second close_gdb_session could still run after close had started.

Problem and expected behavior

On 47353e494, createShutdown does not seal the shared BinarySession before closing it. Stdio has no prepareShutdown drain. session.close() and a later session.open() share one transition queue, so an open that arrives after the shutdown close runs after that close and starts a provider nobody closes again.

GdbSessionManager.execute() checks that the session is open when the call arrives, then waits in #serialize. When its turn starts it previously rechecked only session.exited and session.protocolError. closeAll() sets #closing immediately, and close() removes the session only inside its own turn. The child exit event can arrive after stop reports success, so exited is still false and the queued command is sent. A second close() queued behind the first sends -gdb-exit and stops the process again.

Expected: a target open that loses the race with process shutdown fails as AnalysisCancelledError (open_binary or open_bundle_resources) and leaves no active target. A gdb_console command whose turn starts after close has started fails with the existing AnalysisInputError (operation: "gdb_console", issue path session_id, reason invalid_value). A duplicate close fails with AnalysisInputError (operation: "close_gdb_session") and does not repeat -gdb-exit or stop. An ordinary close() still allows a later open().

Change and scope

  • BinarySession.beginShutdown() rejects later open and openBundleResources turns. createShutdown calls it before the transport drain and owner closes. An acquisition already inside its transition still finishes; the shutdown close queued behind it retires that provider.
  • GdbSessionManager repeats console admission checks when a queued command gets its turn, including a per-session seal set before destructive cleanup. Failed cleanup retains the session for cleanup retries but blocks console commands and reports status as closed. A disposition refusal before destructive cleanup still permits explicit console remediation. Retried cleanup skips inferior inventory and -gdb-exit; concurrent closes of an exited session still share one stop attempt. A close whose turn starts after session removal returns the existing close input error.
  • No schema, catalog, or tool-contract change. Stdio still does not drain in-flight handlers the way HTTP prepareShutdown does; the session owner refuses acquisition instead.

Contribution scope: not applicable. This is a bugfix inside the existing binary-session and GDB session lifecycles. It does not add target execution, injection, mutation, or protection recovery.

The GDB admission outcome is the same fix still open in #6369. This PR carries it with the shutdown seal so both close races land together.

Contract and boundary impact

  • Semantic owner and earliest changed stage: BinarySession target admission, invoked by createShutdown at the start of process shutdown; GdbSessionManager admission of queued execute and close.
  • Contribution scope fit and linked maintainer decision: not applicable.
  • CLI and MCP/tool-catalog contract: no schema change. An open_binary or open_bundle_resources call that runs after process shutdown has begun returns AnalysisCancelledError for that operation. A gdb_console call whose turn starts after close has started returns the existing unavailable-session AnalysisInputError. A duplicate close_gdb_session returns the existing missing-session AnalysisInputError.
  • Provider, bridge, target-format, or platform compatibility: none.
  • Evidence, artifact, provenance, or reconstruction contract: none.
  • Target selection, authority, upstream behavior, and cleanup details (if applicable): shutdown no longer leaves a provider client created after its close. A queued GDB command is not sent to a debugger whose close has started, and a duplicate close does not repeat -gdb-exit or process stop. An ordinary binary close still allows a later open. A GDB command already ahead of close() in the session queue still runs before that close; closeAll() rejects queued commands because the manager is already closing.
  • Generated metadata (docs/public/product-catalog.json), package, or installation impact: none.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

The change refuses target acquisition and queued debugger commands during close. It does not add a read, launch, injection, mutation, network call, or protection recovery. Existing shutdown and GDB close still retire sessions they already own.

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration.
  • Contact network services.
  • Recover protected code or work around protections.
  • Documentation or usability only; no target interaction.
  • Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

  • Tests added or updated: tests/boundary/mcp/shutdown.test.ts and tests/boundary/providers/GdbSessionManagerCoverage.test.ts.
  • Base reproduction or other evidence: executed. On 47353e494, before the fix, the five new regression tests failed. A target open queued behind shutdown, and another open after shutdown settled, both resolved { ok: true } and left a provider client. A run queued before closeAll(), and a run queued behind close(), both resolved { ok: true }. A duplicate close() resolved { ok: true }. After the fix those calls return the typed errors above, the in-flight open's client is closed (created === closed), no further client is created, and -gdb-exit plus process stop run once. The ordinary close-then-open test passed on that base and still passes.
  • User-visible CLI/MCP output (if applicable): not captured from a live stdio client. The boundary result is the typed Result above (AnalysisCancelledError with operation open_binary or open_bundle_resources; AnalysisInputError with operation gdb_console and issue path: ["session_id"], reason: "invalid_value"; AnalysisInputError with operation close_gdb_session).
  • Remaining proof gaps: no real GDB, Hopper, Ghidra, or live stdio MCP client. The provider is a fixture client and the debugger is the fake GDB spawner in GdbSessionManagerCoverage.test.ts. HTTP prepareShutdown was not re-exercised in this run. An external mutation queued behind shutdown close is not sealed here and was not run as a leftover open session.

For evidence-bearing changes:

  • Observed, derived, and inferred claims remain distinguishable.
  • Artifact identity, source provenance, and failed attempts remain preserved.
  • Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

Review follow-up for retained GDB cleanup ownership:

  • Before the follow-up fix, both new failed-cleanup regression cases failed on 3b609a0: a queued run returned success after cleanup reported incomplete. The explicit-disposition remediation control passed.
  • After the fix, commands queued before close, arriving during held cleanup, or arriving after failed cleanup are rejected. Both close() and retryCleanupFailures() release the retained owner without repeating GDB commands and resolve the original cleanup marker. No directly related cleanup remains.
  • npm run check:fast — passed (typecheck and lint).
  • npm run test:focused -- tests/boundary/providers/GdbSessionManagerCoverage.test.ts tests/boundary/mcp/gdbCloseDisposition.test.ts tests/boundary/mcp/gdbCancellationMcp.test.ts tests/boundary/mcp/shutdown.test.ts src/server/shutdownRegistration.test.ts src/application/binary/BinarySessionLiveness.test.ts src/application/binary/BinarySessionBundleResources.test.ts — passed, 7 files, 222 tests. Node 24.18.0 and npm 11.16.0; macOS host with fixture providers and fake GDB, including MCP disposition/cancellation consumers. Real GDB and other real providers remain unverified.

Original implementation validation:

  • npm run check:fast — passed. Turbo typecheck and lint, 2 successful tasks.
  • npm run test:focused -- tests/boundary/mcp/shutdown.test.ts tests/boundary/providers/GdbSessionManagerCoverage.test.ts src/server/shutdownRegistration.test.ts src/application/binary/BinarySessionLiveness.test.ts src/application/binary/BinarySessionBundleResources.test.ts — passed, 5 files, 210 tests.
  • Before the fix, vitest run on tests/boundary/mcp/shutdown.test.ts and tests/boundary/providers/GdbSessionManagerCoverage.test.ts with the new test names — 5 failed, 1 passed (ordinary close-then-open), on 47353e494.

Compatibility, safety, and release

  • Breaking changes or migration steps: none. Callers that depended on an open succeeding after process shutdown, or on a queued GDB command running after close started, now receive the existing cancelled or input errors. Ordinary close() then open() is unchanged.
  • Real Hopper/Ghidra, browser, or OS coverage: not run. Linux fixture session and fake GDB spawner only.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: the fix stops a provider client and a queued debugger command from continuing after close has started. It does not grant new authority. The diff has no secrets.

Author readiness

Human owner: @morluto

  • I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
  • I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
  • I declare this PR ready for initial maintainer review.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.
Open in Web Open in Cursor 

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/6411 **Author:** [@morluto](https://github.com/morluto) **Created:** 10/22/2026 **Status:** ✅ Merged **Merged:** 10/23/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `cursor/session-close-races-94bf` --- ### 📝 Commits (2) - [`3b609a0`](https://github.com/morluto/rea/commit/3b609a012a07540d397b9653fdc07ba5dd7da6df) fix(lifecycle): reject session work once close has started - [`efd799c`](https://github.com/morluto/rea/commit/efd799c689102a11eeaf7bb4224b6dc083922008) fix(gdb): block console admission after cleanup starts ### 📊 Changes **5 files changed** (+436 additions, -44 deletions) <details> <summary>View changed files</summary> 📝 `src/application/binary/BinarySession.ts` (+22 -0) 📝 `src/gdb/GdbSessionManager.ts` (+58 -40) 📝 `src/main/shutdown.ts` (+5 -2) 📝 `tests/boundary/mcp/shutdown.test.ts` (+124 -0) 📝 `tests/boundary/providers/GdbSessionManagerCoverage.test.ts` (+227 -2) </details> ### 📄 Description <!-- CURSOR_AGENT_PR_BODY_BEGIN --> ## Summary Fixes #6399 and #6370. Once process shutdown or a GDB session close has started, later work must not reopen that session or repeat cleanup. A stdio tool call could still open the shared binary session after shutdown's close was queued, and a queued `gdb_console` command or second `close_gdb_session` could still run after close had started. ## Problem and expected behavior On `47353e494`, `createShutdown` does not seal the shared `BinarySession` before closing it. Stdio has no `prepareShutdown` drain. `session.close()` and a later `session.open()` share one transition queue, so an open that arrives after the shutdown close runs after that close and starts a provider nobody closes again. `GdbSessionManager.execute()` checks that the session is open when the call arrives, then waits in `#serialize`. When its turn starts it previously rechecked only `session.exited` and `session.protocolError`. `closeAll()` sets `#closing` immediately, and `close()` removes the session only inside its own turn. The child exit event can arrive after stop reports success, so `exited` is still false and the queued command is sent. A second `close()` queued behind the first sends `-gdb-exit` and stops the process again. Expected: a target open that loses the race with process shutdown fails as `AnalysisCancelledError` (`open_binary` or `open_bundle_resources`) and leaves no active target. A `gdb_console` command whose turn starts after close has started fails with the existing `AnalysisInputError` (`operation: "gdb_console"`, issue path `session_id`, reason `invalid_value`). A duplicate close fails with `AnalysisInputError` (`operation: "close_gdb_session"`) and does not repeat `-gdb-exit` or stop. An ordinary `close()` still allows a later `open()`. ## Change and scope - `BinarySession.beginShutdown()` rejects later `open` and `openBundleResources` turns. `createShutdown` calls it before the transport drain and owner closes. An acquisition already inside its transition still finishes; the shutdown close queued behind it retires that provider. - `GdbSessionManager` repeats console admission checks when a queued command gets its turn, including a per-session seal set before destructive cleanup. Failed cleanup retains the session for cleanup retries but blocks console commands and reports status as `closed`. A disposition refusal before destructive cleanup still permits explicit console remediation. Retried cleanup skips inferior inventory and `-gdb-exit`; concurrent closes of an exited session still share one stop attempt. A close whose turn starts after session removal returns the existing close input error. - No schema, catalog, or tool-contract change. Stdio still does not drain in-flight handlers the way HTTP `prepareShutdown` does; the session owner refuses acquisition instead. Contribution scope: not applicable. This is a bugfix inside the existing binary-session and GDB session lifecycles. It does not add target execution, injection, mutation, or protection recovery. The GDB admission outcome is the same fix still open in #6369. This PR carries it with the shutdown seal so both close races land together. ## Contract and boundary impact - Semantic owner and earliest changed stage: `BinarySession` target admission, invoked by `createShutdown` at the start of process shutdown; `GdbSessionManager` admission of queued `execute` and `close`. - Contribution scope fit and linked maintainer decision: not applicable. - CLI and MCP/tool-catalog contract: no schema change. An `open_binary` or `open_bundle_resources` call that runs after process shutdown has begun returns `AnalysisCancelledError` for that operation. A `gdb_console` call whose turn starts after close has started returns the existing unavailable-session `AnalysisInputError`. A duplicate `close_gdb_session` returns the existing missing-session `AnalysisInputError`. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details (if applicable): shutdown no longer leaves a provider client created after its close. A queued GDB command is not sent to a debugger whose close has started, and a duplicate close does not repeat `-gdb-exit` or process stop. An ordinary binary close still allows a later open. A GDB command already ahead of `close()` in the session queue still runs before that close; `closeAll()` rejects queued commands because the manager is already closing. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: none. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. The change refuses target acquisition and queued debugger commands during close. It does not add a read, launch, injection, mutation, network call, or protection recovery. Existing shutdown and GDB close still retire sessions they already own. - [ ] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: `tests/boundary/mcp/shutdown.test.ts` and `tests/boundary/providers/GdbSessionManagerCoverage.test.ts`. - Base reproduction or other evidence: executed. On `47353e494`, before the fix, the five new regression tests failed. A target open queued behind shutdown, and another open after shutdown settled, both resolved `{ ok: true }` and left a provider client. A `run` queued before `closeAll()`, and a `run` queued behind `close()`, both resolved `{ ok: true }`. A duplicate `close()` resolved `{ ok: true }`. After the fix those calls return the typed errors above, the in-flight open's client is closed (`created === closed`), no further client is created, and `-gdb-exit` plus process stop run once. The ordinary close-then-open test passed on that base and still passes. - User-visible CLI/MCP output (if applicable): not captured from a live stdio client. The boundary result is the typed `Result` above (`AnalysisCancelledError` with `operation` `open_binary` or `open_bundle_resources`; `AnalysisInputError` with `operation` `gdb_console` and issue `path: ["session_id"]`, `reason: "invalid_value"`; `AnalysisInputError` with `operation` `close_gdb_session`). - Remaining proof gaps: no real GDB, Hopper, Ghidra, or live stdio MCP client. The provider is a fixture client and the debugger is the fake GDB spawner in `GdbSessionManagerCoverage.test.ts`. HTTP `prepareShutdown` was not re-exercised in this run. An external mutation queued behind shutdown close is not sealed here and was not run as a leftover open session. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [ ] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed Review follow-up for retained GDB cleanup ownership: - Before the follow-up fix, both new failed-cleanup regression cases failed on `3b609a0`: a queued `run` returned success after cleanup reported `incomplete`. The explicit-disposition remediation control passed. - After the fix, commands queued before close, arriving during held cleanup, or arriving after failed cleanup are rejected. Both `close()` and `retryCleanupFailures()` release the retained owner without repeating GDB commands and resolve the original cleanup marker. No directly related cleanup remains. - `npm run check:fast` — passed (typecheck and lint). - `npm run test:focused -- tests/boundary/providers/GdbSessionManagerCoverage.test.ts tests/boundary/mcp/gdbCloseDisposition.test.ts tests/boundary/mcp/gdbCancellationMcp.test.ts tests/boundary/mcp/shutdown.test.ts src/server/shutdownRegistration.test.ts src/application/binary/BinarySessionLiveness.test.ts src/application/binary/BinarySessionBundleResources.test.ts` — passed, 7 files, 222 tests. Node 24.18.0 and npm 11.16.0; macOS host with fixture providers and fake GDB, including MCP disposition/cancellation consumers. Real GDB and other real providers remain unverified. Original implementation validation: - `npm run check:fast` — passed. Turbo typecheck and lint, 2 successful tasks. - `npm run test:focused -- tests/boundary/mcp/shutdown.test.ts tests/boundary/providers/GdbSessionManagerCoverage.test.ts src/server/shutdownRegistration.test.ts src/application/binary/BinarySessionLiveness.test.ts src/application/binary/BinarySessionBundleResources.test.ts` — passed, 5 files, 210 tests. - Before the fix, `vitest run` on `tests/boundary/mcp/shutdown.test.ts` and `tests/boundary/providers/GdbSessionManagerCoverage.test.ts` with the new test names — 5 failed, 1 passed (ordinary close-then-open), on `47353e494`. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Callers that depended on an open succeeding after process shutdown, or on a queued GDB command running after close started, now receive the existing cancelled or input errors. Ordinary `close()` then `open()` is unchanged. - Real Hopper/Ghidra, browser, or OS coverage: not run. Linux fixture session and fake GDB spawner only. - Package or release metadata impact: none. - Security, privacy, process, or containment review: the fix stops a provider client and a queued debugger command from continuing after close has started. It does not grant new authority. The diff has no secrets. ## Author readiness Human owner: @morluto - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [ ] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. <!-- CURSOR_AGENT_PR_BODY_END --> <div><a href="https://cursor.com/agents/bc-c9444efc-5525-53ef-a8e2-79b0bbed94bf?cursor_ref=pr_footer&cursor_cta=open_in_web"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-web-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-web-light.png"><img alt="Open in Web" width="114" height="28" src="https://cursor.com/assets/images/open-in-web-dark.png"></picture></a>&nbsp;<a href="https://cursor.com/background-agent?bcId=bc-c9444efc-5525-53ef-a8e2-79b0bbed94bf&cursor_ref=pr_footer&cursor_cta=open_in_cursor"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-cursor-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-cursor-light.png"><img alt="Open in Cursor" width="131" height="28" src="https://cursor.com/assets/images/open-in-cursor-dark.png"></picture></a>&nbsp;</div> --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-01 21:47:11 +00:00
Sign in to join this conversation.
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#7340
No description provided.