morluto/rea

Fork 0

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

[PR #5375] [CLOSED] fix(gdb): keep a failed startup retained for cleanup out of ready state #6907

New issue

Closed

opened 2026-11-01 21:43:58 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:43:58 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5375
Author: @Lew1sWong
Created: 10/20/2026
Status: ❌ Closed

Base: main ← Head: fix/gdb-failed-startup-not-ready

📝 Commits (3)

25d0ec0 fix(gdb): keep a failed startup retained for cleanup out of ready state
606378d test(gdb): group failed-startup readiness cases with startup TTY checks
651aa7d test(gdb): name each failed-startup cancellation by its observed boundary

📊 Changes

2 files changed (+103 additions, -1 deletions)

View changed files

📝 src/gdb/GdbSessionManager.ts (+3 -1)
📝 tests/boundary/providers/GdbSessionManagerCoverage.test.ts (+100 -0)

📄 Description

Summary

Fixes #5331. A failed start_gdb_session whose owned GDB process cannot be confirmed stopped is retained for cleanup. That retained owner no longer reports ready, and gdb_console no longer dispatches commands to it.

Problem and expected behavior

session.ready = true was set when the MI prompt arrived, before -gdb-set auto-load off and the optional inferior-TTY set and readback. If one of those checks failed, or the caller's abort was observed at a startup checkpoint after the prompt, startup threw without setting a protocol error. When the stop was then incomplete, the startup catch kept the same session in the sessions map:

gdb_session_status reported state: "ready";
gdb_console accepted the ID and sent -interpreter-exec console ... to a GDB whose required configuration had failed.

Expected, per the GDB guide and the start_gdb_session contract ("before the session becomes ready"): the retained ID stays available for ownership cleanup but reports closed, gdb_console rejects it with the existing unavailable-or-closed input error and dispatches nothing, and close_gdb_session still releases it.

Change and scope

Readiness is derived from startup completion, as the lifecycle rules in docs/tool-design.md require. The only assignment of session.ready moves from prompt arrival to the publication point, after every startup check has passed. ready is read only by the console admission check and the status projection; startup MI requests do not depend on it. Close does not check ready, so the retained owner can still be closed.

closed is the existing status enum value. It does not claim the process exited.

Not included: the startup catch still does not add its retained failure to session.cleanupFailures, so retryCleanupFailures() skips failed-startup owners. That gap predates this change and also applies to retained startup timeouts. close_gdb_session on the retained ID remains the release path.

Contribution scope: ordinary fix within existing boundaries; maintainer scope decision not applicable.

Contract and boundary impact

Semantic owner and earliest changed stage: GdbSessionManager startup readiness.
Contribution scope fit and linked maintainer decision: not applicable (bug fix).
CLI and MCP/tool-catalog contract: no schema change. A retained failed startup now projects state: "closed", and gdb_console returns the existing input error for it.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: none.
Target selection, authority, upstream behavior, and cleanup details: retained ownership, cleanup reason, and close_gdb_session release are unchanged.
Generated metadata, package, or installation impact: none.
Launch or attach to a process.

Evidence and regression coverage

Tests added in tests/boundary/providers/GdbSessionManagerCoverage.test.ts, using the existing in-memory fake GDB spawner and a stop that is incomplete on the first attempt. Four cases: auto-load rejected, inferior-TTY readback mismatch, caller cancellation after the startup prompt (no startup command sent), and caller cancellation during the final readiness drain (after automatic-loading disable). Each asserts its error tag, the exact startup commands dispatched, a closed status, a rejected gdb_console with no command dispatched, a successful same-ID close, and a single spawn.
Base reproduction: all 4 tests fail on 0134732fc at the status assertion with state: "ready". The cancellation cases confirm the issue's source-only hypothesis.
Remaining proof gaps: no real GDB or Linux run; the fake spawner never starts an OS process.

Validation performed

Node 24.18.0, macOS arm64.

npx vitest run on all 7 GDB test files (provider, MCP, CLI), after npm run build — 191 passed
npm run test:focused -- on the product catalog, tool schema, change-scope, and local tool contract tests — 266 passed
npm run typecheck and npm run check:fast — passed
oxlint on changed files — no new findings
Full npm run check:pr — not run

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: none; real GDB not run.
Package or release metadata impact: none.
Security, privacy, process, or containment review: this stops new debugger work from reaching a GDB process whose automatic-loading disable or TTY check failed.

Related: #5374 changes cancellation causes in the same startup path. The fixes are independent, but they edit adjacent lines after the startup prompt wait, so whichever PR merges second needs a small rebase.

Author readiness

Human owner: @Lew1sWong

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.

🤖 Generated with Claude Code

_{🔄 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/5375 **Author:** [@Lew1sWong](https://github.com/Lew1sWong) **Created:** 10/20/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/gdb-failed-startup-not-ready` --- ### 📝 Commits (3) - [`25d0ec0`](https://github.com/morluto/rea/commit/25d0ec0c4445b04e4a3652e9955d43d10677442a) fix(gdb): keep a failed startup retained for cleanup out of ready state - [`606378d`](https://github.com/morluto/rea/commit/606378d3d9052854588201ad4c072f3b49939b78) test(gdb): group failed-startup readiness cases with startup TTY checks - [`651aa7d`](https://github.com/morluto/rea/commit/651aa7d0a44b1ebe0292ce45a8284d5168eb4eb4) test(gdb): name each failed-startup cancellation by its observed boundary ### 📊 Changes **2 files changed** (+103 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/gdb/GdbSessionManager.ts` (+3 -1) 📝 `tests/boundary/providers/GdbSessionManagerCoverage.test.ts` (+100 -0) </details> ### 📄 Description ## Summary Fixes #5331. A failed `start_gdb_session` whose owned GDB process cannot be confirmed stopped is retained for cleanup. That retained owner no longer reports `ready`, and `gdb_console` no longer dispatches commands to it. ## Problem and expected behavior `session.ready = true` was set when the MI prompt arrived, before `-gdb-set auto-load off` and the optional inferior-TTY set and readback. If one of those checks failed, or the caller's abort was observed at a startup checkpoint after the prompt, startup threw without setting a protocol error. When the stop was then incomplete, the startup catch kept the same session in the sessions map: - `gdb_session_status` reported `state: "ready"`; - `gdb_console` accepted the ID and sent `-interpreter-exec console ...` to a GDB whose required configuration had failed. Expected, per the GDB guide and the `start_gdb_session` contract ("before the session becomes ready"): the retained ID stays available for ownership cleanup but reports `closed`, `gdb_console` rejects it with the existing unavailable-or-closed input error and dispatches nothing, and `close_gdb_session` still releases it. ## Change and scope Readiness is derived from startup completion, as the lifecycle rules in `docs/tool-design.md` require. The only assignment of `session.ready` moves from prompt arrival to the publication point, after every startup check has passed. `ready` is read only by the console admission check and the status projection; startup MI requests do not depend on it. Close does not check `ready`, so the retained owner can still be closed. `closed` is the existing status enum value. It does not claim the process exited. Not included: the startup catch still does not add its retained failure to `session.cleanupFailures`, so `retryCleanupFailures()` skips failed-startup owners. That gap predates this change and also applies to retained startup timeouts. `close_gdb_session` on the retained ID remains the release path. Contribution scope: ordinary fix within existing boundaries; maintainer scope decision not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: `GdbSessionManager` startup readiness. - Contribution scope fit and linked maintainer decision: not applicable (bug fix). - CLI and MCP/tool-catalog contract: no schema change. A retained failed startup now projects `state: "closed"`, and `gdb_console` returns the existing input error for it. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details: retained ownership, cleanup reason, and `close_gdb_session` release are unchanged. - Generated metadata, package, or installation impact: none. - [x] Launch or attach to a process. ## Evidence and regression coverage - Tests added in `tests/boundary/providers/GdbSessionManagerCoverage.test.ts`, using the existing in-memory fake GDB spawner and a stop that is incomplete on the first attempt. Four cases: auto-load rejected, inferior-TTY readback mismatch, caller cancellation after the startup prompt (no startup command sent), and caller cancellation during the final readiness drain (after automatic-loading disable). Each asserts its error tag, the exact startup commands dispatched, a `closed` status, a rejected `gdb_console` with no command dispatched, a successful same-ID close, and a single spawn. - Base reproduction: all 4 tests fail on `0134732fc` at the status assertion with `state: "ready"`. The cancellation cases confirm the issue's source-only hypothesis. - Remaining proof gaps: no real GDB or Linux run; the fake spawner never starts an OS process. ## Validation performed Node 24.18.0, macOS arm64. - `npx vitest run` on all 7 GDB test files (provider, MCP, CLI), after `npm run build` — 191 passed - `npm run test:focused --` on the product catalog, tool schema, change-scope, and local tool contract tests — 266 passed - `npm run typecheck` and `npm run check:fast` — passed - `oxlint` on changed files — no new findings - Full `npm run check:pr` — not run ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: none; real GDB not run. - Package or release metadata impact: none. - Security, privacy, process, or containment review: this stops new debugger work from reaching a GDB process whose automatic-loading disable or TTY check failed. Related: #5374 changes cancellation causes in the same startup path. The fixes are independent, but they edit adjacent lines after the startup prompt wait, so whichever PR merges second needs a small rebase. ## Author readiness Human owner: @Lew1sWong - [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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:43:58 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:47:52 +00:00

[PR #6907] [MERGED] perf(native): query retained export table facts #7439

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

No description provided.

Rows
Columns