morluto/rea

Fork 0

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

[PR #5374] [CLOSED] fix(gdb): preserve the caller's abort reason when startup is cancelled #6905

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/5374
Author: @Lew1sWong
Created: 10/20/2026
Status: ❌ Closed

Base: main ← Head: fix/gdb-startup-cancellation-reason

📝 Commits (1)

cef1282 fix(gdb): preserve the caller's abort reason when startup is cancelled

📊 Changes

2 files changed (+213 additions, -15 deletions)

View changed files

📝 src/gdb/GdbSessionManager.ts (+55 -15)
📝 tests/boundary/providers/GdbSessionManagerCoverage.test.ts (+158 -0)

📄 Description

Summary

Fixes #5373. A cancelled start_gdb_session now carries the caller's exact abort reason as the typed cancellation error's cause, as the cancellation contract requires.

Problem and expected behavior

Pre-aborted start: cause was undefined.
Cancelled after spawn, after the prompt, during a startup command, or in the final readiness drain: cause was an internal AnalysisCancelledError with no reason.
Cancelled during async spawn preparation: cause was the spawner's own error.
Late abort after a real failure: the startup catch classified any failure as cancellation once the signal was aborted. A rejected -gdb-set auto-load off became AnalysisCancelledError when the caller aborted during the following cleanup.

Expected: error.cause === reason with the cancellation tag and no-spawn behaviour kept. A failure observed before a late abort stays primary.

Change and scope

start() relays the caller's reason unchanged into the internal startup controller, and closeAll() aborts that controller without one. So the internal signal's reason is the caller's exact reason for caller cancellation, and the default abort reason for provider shutdown. Every startup cancellation site now uses { cause: signal.reason }. The pre-aborted check is split so a caller abort keeps its reason while a closing provider still returns a cause-less cancellation.

The startup catch classifies a failure as cancellation only when the failure itself is an AnalysisCancelledError. Cleanup ownership, captured output, and partial startup Evidence are unchanged.

Not included: the queue wait (waitForGdbCommandTurn) and the active MI abort branches in gdb_console. The issue notes them as source-only observations without runtime proof. They can follow separately.

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

Contract and boundary impact

Semantic owner and earliest changed stage: GdbSessionManager startup cancellation.
Contribution scope fit and linked maintainer decision: not applicable (bug fix).
CLI and MCP/tool-catalog contract: none. cause is in-process data; projected payloads are unchanged.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: none.
Target selection, authority, upstream behavior, and cleanup details: cleanup classification and retained owners 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:
- pre-aborted start keeps the exact reason and never spawns;
- cancellation after spawn, after the prompt, during a pending startup command, and during the final readiness drain;
- cancellation during async spawn preparation;
- the exact reason sits beside retained cleanup when the startup stop is uncertain;
- a rejected startup configuration stays primary when the caller aborts during its cleanup.
Base reproduction: all 8 new tests fail on 0134732fc. The pre-aborted case receives undefined; the staged cases receive an internal AnalysisCancelledError; the configuration case receives AnalysisCancelledError instead of AnalysisCapabilityUnavailableError.
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 — 195 passed
npm run test:focused -- tests/boundary/filesystem/productCatalog.test.ts tests/boundary/mcp/toolSchemaValidity.test.ts — 25 passed
npm run typecheck — passed
npm run check:fast — passed
oxlint on changed files — no new findings; startProtocol complexity drops from 55 to 52
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: the reason is kept as opaque data and is never interpreted as cleanup authority.

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/5374 **Author:** [@Lew1sWong](https://github.com/Lew1sWong) **Created:** 10/20/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/gdb-startup-cancellation-reason` --- ### 📝 Commits (1) - [`cef1282`](https://github.com/morluto/rea/commit/cef1282c9bc891d3a1772236d75a1616826c47ca) fix(gdb): preserve the caller's abort reason when startup is cancelled ### 📊 Changes **2 files changed** (+213 additions, -15 deletions) <details> <summary>View changed files</summary> 📝 `src/gdb/GdbSessionManager.ts` (+55 -15) 📝 `tests/boundary/providers/GdbSessionManagerCoverage.test.ts` (+158 -0) </details> ### 📄 Description ## Summary Fixes #5373. A cancelled `start_gdb_session` now carries the caller's exact abort reason as the typed cancellation error's `cause`, as the [cancellation contract](https://github.com/morluto/rea/blob/main/docs/tool-design.md) requires. ## Problem and expected behavior - **Pre-aborted start:** `cause` was `undefined`. - **Cancelled after spawn, after the prompt, during a startup command, or in the final readiness drain:** `cause` was an internal `AnalysisCancelledError` with no reason. - **Cancelled during async spawn preparation:** `cause` was the spawner's own error. - **Late abort after a real failure:** the startup catch classified any failure as cancellation once the signal was aborted. A rejected `-gdb-set auto-load off` became `AnalysisCancelledError` when the caller aborted during the following cleanup. Expected: `error.cause === reason` with the cancellation tag and no-spawn behaviour kept. A failure observed before a late abort stays primary. ## Change and scope `start()` relays the caller's reason unchanged into the internal startup controller, and `closeAll()` aborts that controller without one. So the internal signal's `reason` is the caller's exact reason for caller cancellation, and the default abort reason for provider shutdown. Every startup cancellation site now uses `{ cause: signal.reason }`. The pre-aborted check is split so a caller abort keeps its reason while a closing provider still returns a cause-less cancellation. The startup catch classifies a failure as cancellation only when the failure itself is an `AnalysisCancelledError`. Cleanup ownership, captured output, and partial startup Evidence are unchanged. Not included: the queue wait (`waitForGdbCommandTurn`) and the active MI abort branches in `gdb_console`. The issue notes them as source-only observations without runtime proof. They can follow separately. Contribution scope: ordinary fix within existing boundaries; maintainer scope decision not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: `GdbSessionManager` startup cancellation. - Contribution scope fit and linked maintainer decision: not applicable (bug fix). - CLI and MCP/tool-catalog contract: none. `cause` is in-process data; projected payloads are unchanged. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details: cleanup classification and retained owners 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: - pre-aborted start keeps the exact reason and never spawns; - cancellation after spawn, after the prompt, during a pending startup command, and during the final readiness drain; - cancellation during async spawn preparation; - the exact reason sits beside retained cleanup when the startup stop is uncertain; - a rejected startup configuration stays primary when the caller aborts during its cleanup. - Base reproduction: all 8 new tests fail on `0134732fc`. The pre-aborted case receives `undefined`; the staged cases receive an internal `AnalysisCancelledError`; the configuration case receives `AnalysisCancelledError` instead of `AnalysisCapabilityUnavailableError`. - 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` — 195 passed - `npm run test:focused -- tests/boundary/filesystem/productCatalog.test.ts tests/boundary/mcp/toolSchemaValidity.test.ts` — 25 passed - `npm run typecheck` — passed - `npm run check:fast` — passed - `oxlint` on changed files — no new findings; `startProtocol` complexity drops from 55 to 52 - 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: the reason is kept as opaque data and is never interpreted as cleanup authority. ## 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:51 +00:00

[PR #6905] [CLOSED] fix(environment)!: keep unsupported HAR body encodings opaque #7437

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

No description provided.

Rows
Columns