morluto/rea

Fork 0

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

[PR #7111] [MERGED] fix(lldb): keep the bridge's no-target proof through interruption #7649

New issue

Closed

opened 2026-11-03 13:39:23 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-03 13:39:23 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/7111
Author: @kaoru0822-kitauji
Created: 10/25/2026
Status: ✅ Merged
Merged: 10/25/2026
Merged by: @kaoru0822-kitauji

Base: main ← Head: fix/5385-lldb-no-target-proof

📝 Commits (1)

7f6f7eb fix(lldb): keep the bridge's no-target proof through interruption

📊 Changes

2 files changed (+274 additions, -26 deletions)

View changed files

📝 src/native/LldbCallTracer.ts (+48 -26)
📝 tests/boundary/providers/native/lldbCallTracerFailures.test.ts (+226 -0)

📄 Description

What broke: When cancellation or the deadline interrupted LLDB after the bridge had already published target_process_created: false, the tracer never read the result. It kept a native-target:unknown marker that no close() retry could release, so every later trace failed on retained cleanup.
Fix: Once LLDB's process group is stopped, read the published result for an interrupted run too, and apply one shared target-gone proof (bridgeProvesTargetGone) in both #run and #retryPendingCleanup. Cancellation or timeout stays the primary outcome.
Risk: Low. One extra small file read on interrupted runs. The proof rule is the existing one, now shared. A created target, a truncated result, or a missing PID alone still stays unresolved.

Summary

Fixes #5385

The exact timing in the issue is already fixed on main by cc73587ec (fix(lldb): preserve observed failures through interrupted cleanup). That timing is: a peer exits 17, then an abort arrives while the first ProviderProcessSupervisor.stop() is held. Its regression test (preserves a physical bridge failure and no-target proof when %s arrives during successful process cleanup) passes on bc143125e, so both halves hold there.

The no-target half still reproduced on bc143125e at the neighbouring timing: the abort arrives after LLDB published its result but before #run reads it. lldb --batch exits 0 after rea_trace, because run() never re-raises. So the nonzero-exit latch does not apply, and the outcome stays cancelled/timeout. This PR fixes that path.

Problem and expected behavior

Smallest trigger (executed, controlled): the default runLldb launch with an authored Node peer selected as LLDB. The peer writes {status:"target-error", target_process_created:false} to config.result_path, then exits 0 while an inherited child holds its output open. The caller aborts after the leader exit is observed and before output closes.

Observed on bc143125e:

{"error":"AnalysisCancelledError","cleanupIncomplete":true,
 "resources":["native-target:unknown","<owned runtime root>"],
 "closeOk":false,"closeResources":["native-target:unknown","<owned runtime root>"]}

Violated invariant: an already-published producer observation (no target created, no PID published) was dropped because the run was interrupted. The tracer then could never release its runtime.

Expected: cancellation stays primary with partial reason cancelled, no target marker is pending, and close() succeeds. With this PR the same run returns {"error":"AnalysisCancelledError","cleanupIncomplete":false,"resources":[],"closeOk":true}.

Change and scope

#run reads the result file when LLDB exited, or when an interrupted LLDB was launched and its process group was verified stopped. The result is read once. Only an exited outcome selects it as the primary result (output). The target-cleanup proof uses it either way (published).
bridgeProvesTargetGone(output, targetPid) replaces the two inline checks: target_process_exit_observed === true, or target_process_created === false with no published PID. Behaviour on the exited path is unchanged.
#retryPendingCleanup applies the same proof after a retry stops a previously unstopped LLDB. Before this, it could only call ensureTerminated(undefined), which never succeeds.
Not included: an interrupted run whose bridge reported a failure still returns cancellation or timeout, not the bridge error. The caller's interruption stays primary, and the bridge result's error text is not attached. I did not add a cache for the retry readback. The retry reads the small result file only while the target marker is still unresolved.

Contribution scope: an ordinary fix inside the existing LLDB tracer boundary. No maintainer scope decision is needed.

Contract and boundary impact

Semantic owner and earliest changed stage: LldbCallTracer#run readback of the bridge result. Its consumers are the target-termination decision and #retryPendingCleanup. The two duplicated inline proof checks were replaced by one helper.
Contribution scope fit and linked maintainer decision: not applicable (bug fix within an existing boundary).
CLI and MCP/tool-catalog contract: none. Error types and fields are unchanged; only cleanup state for these runs is now accurate.
Provider, bridge, target-format, or platform compatibility: the bridge and its result schema are unchanged.
Evidence, artifact, provenance, or reconstruction contract: partial observations for interrupted runs are unchanged (journal/capture projection with reason cancelled/timeout).
Target selection, authority, upstream behavior, and cleanup details: the target marker is released only on an explicit producer proof. Without one it stays owned and unresolved, as before.
Generated metadata, package, or installation impact: none.

Expected effects:

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, in tests/boundary/providers/native/lldbCallTracerFailures.test.ts, through the production default launch with an authored Node peer:
- LLDB target proof published before interruption: {caller cancellation, deadline timeout} × {no target created, target exit observed, target created, truncated result}. The interruption arrives after the leader exits and before result readback. Proofs release every marker and close() succeeds. target created and a truncated result keep native-target:unknown owned, and close() keeps failing.
- LLDB target proof read by cleanup retry: the first stop() returns incomplete, so #run cannot read the result. close() then stops LLDB and reads the result. no target created releases the root. target created keeps only native-target:unknown.
Base reproduction: with the source change reverted, 5 of the 10 new tests fail: every proof case, under both cancellation and timeout, plus the retry proof. The 5 controls pass on both revisions.
The existing exit-17 tests stay green: abort during the held final stop(), under caller cancellation and deadline, plus the inherited-output and loop-exit variants.
Lifecycle regressions covered: interruption before result readback, interruption during final cleanup (existing tests), and cleanup retry after an incomplete first stop.
Remaining proof gaps: no real LLDB, debugserver or debuggee was run. A Node peer whose leader is still alive cannot be stopped verifiably by the owned-process cleanup in this harness. So the "abort during LLDB teardown after publishing" window is covered by source reasoning (the same processStopped gate), not by its own test. Windows is skipped, as with the sibling tests.
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

npx vitest run --project boundary --maxWorkers=2 on lldbCallTracerFailures, LldbCallTracer.cleanup, lldbPrelaunchCleanup, nativeCallObservation, lldbDiscovery (boundary) — 5 files, 71 tests passed
The new tests plus the existing primary-outcome and inherited-output tests, run 3 times — 13/13 passed each time
The same new tests against the unfixed source — 5 failed (the proof cases), 5 passed (the controls)
npm run test:local -- src/native/NativeCallObservation.test.ts — 5 passed
npm run check:fast — passed
npm run format:check — passed

Local Node was v26.5.0 (not the pinned 24.22.1); macOS arm64.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: macOS only, with an authored Node peer in place of LLDB. No real debugger.
Package or release metadata impact: none.
Security, privacy, process, or containment review: a target marker is released only on the bridge's explicit published proof. A missing PID or an unreadable or partial result keeps the target owned, so the change stays fail-closed.

Author readiness

Human owner: @kaoru0822-kitauji

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.

🤖 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/7111 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/25/2026 **Status:** ✅ Merged **Merged:** 10/25/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/5385-lldb-no-target-proof` --- ### 📝 Commits (1) - [`7f6f7eb`](https://github.com/morluto/rea/commit/7f6f7eb70841d46edaf2c8b89904d7431935f4e1) fix(lldb): keep the bridge's no-target proof through interruption ### 📊 Changes **2 files changed** (+274 additions, -26 deletions) <details> <summary>View changed files</summary> 📝 `src/native/LldbCallTracer.ts` (+48 -26) 📝 `tests/boundary/providers/native/lldbCallTracerFailures.test.ts` (+226 -0) </details> ### 📄 Description **What broke:** When cancellation or the deadline interrupted LLDB after the bridge had already published `target_process_created: false`, the tracer never read the result. It kept a `native-target:unknown` marker that no `close()` retry could release, so every later trace failed on retained cleanup. **Fix:** Once LLDB's process group is stopped, read the published result for an interrupted run too, and apply one shared target-gone proof (`bridgeProvesTargetGone`) in both `#run` and `#retryPendingCleanup`. Cancellation or timeout stays the primary outcome. **Risk:** Low. One extra small file read on interrupted runs. The proof rule is the existing one, now shared. A created target, a truncated result, or a missing PID alone still stays unresolved. ## Summary Fixes #5385 The exact timing in the issue is already fixed on main by cc73587ec (`fix(lldb): preserve observed failures through interrupted cleanup`). That timing is: a peer exits 17, then an abort arrives while the first `ProviderProcessSupervisor.stop()` is held. Its regression test (`preserves a physical bridge failure and no-target proof when %s arrives during successful process cleanup`) passes on bc143125e, so both halves hold there. The no-target half still reproduced on bc143125e at the neighbouring timing: the abort arrives after LLDB published its result but before `#run` reads it. `lldb --batch` exits 0 after `rea_trace`, because `run()` never re-raises. So the nonzero-exit latch does not apply, and the outcome stays `cancelled`/`timeout`. This PR fixes that path. ## Problem and expected behavior Smallest trigger (executed, controlled): the default `runLldb` launch with an authored Node peer selected as LLDB. The peer writes `{status:"target-error", target_process_created:false}` to `config.result_path`, then exits 0 while an inherited child holds its output open. The caller aborts after the leader exit is observed and before output closes. Observed on bc143125e: ```json {"error":"AnalysisCancelledError","cleanupIncomplete":true, "resources":["native-target:unknown","<owned runtime root>"], "closeOk":false,"closeResources":["native-target:unknown","<owned runtime root>"]} ``` Violated invariant: an already-published producer observation (no target created, no PID published) was dropped because the run was interrupted. The tracer then could never release its runtime. Expected: cancellation stays primary with partial reason `cancelled`, no target marker is pending, and `close()` succeeds. With this PR the same run returns `{"error":"AnalysisCancelledError","cleanupIncomplete":false,"resources":[],"closeOk":true}`. ## Change and scope - `#run` reads the result file when LLDB exited, or when an interrupted LLDB was launched and its process group was verified stopped. The result is read once. Only an `exited` outcome selects it as the primary result (`output`). The target-cleanup proof uses it either way (`published`). - `bridgeProvesTargetGone(output, targetPid)` replaces the two inline checks: `target_process_exit_observed === true`, or `target_process_created === false` with no published PID. Behaviour on the `exited` path is unchanged. - `#retryPendingCleanup` applies the same proof after a retry stops a previously unstopped LLDB. Before this, it could only call `ensureTerminated(undefined)`, which never succeeds. - Not included: an interrupted run whose bridge reported a failure still returns cancellation or timeout, not the bridge error. The caller's interruption stays primary, and the bridge result's `error` text is not attached. I did not add a cache for the retry readback. The retry reads the small result file only while the target marker is still unresolved. Contribution scope: an ordinary fix inside the existing LLDB tracer boundary. No maintainer scope decision is needed. ## Contract and boundary impact - Semantic owner and earliest changed stage: `LldbCallTracer#run` readback of the bridge result. Its consumers are the target-termination decision and `#retryPendingCleanup`. The two duplicated inline proof checks were replaced by one helper. - Contribution scope fit and linked maintainer decision: not applicable (bug fix within an existing boundary). - CLI and MCP/tool-catalog contract: none. Error types and fields are unchanged; only cleanup state for these runs is now accurate. - Provider, bridge, target-format, or platform compatibility: the bridge and its result schema are unchanged. - Evidence, artifact, provenance, or reconstruction contract: partial observations for interrupted runs are unchanged (journal/capture projection with reason `cancelled`/`timeout`). - Target selection, authority, upstream behavior, and cleanup details: the target marker is released only on an explicit producer proof. Without one it stays owned and unresolved, as before. - Generated metadata, package, or installation impact: none. Expected effects: - [x] 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, in `tests/boundary/providers/native/lldbCallTracerFailures.test.ts`, through the production default launch with an authored Node peer: - `LLDB target proof published before interruption`: {caller cancellation, deadline timeout} × {no target created, target exit observed, target created, truncated result}. The interruption arrives after the leader exits and before result readback. Proofs release every marker and `close()` succeeds. `target created` and a truncated result keep `native-target:unknown` owned, and `close()` keeps failing. - `LLDB target proof read by cleanup retry`: the first `stop()` returns `incomplete`, so `#run` cannot read the result. `close()` then stops LLDB and reads the result. `no target created` releases the root. `target created` keeps only `native-target:unknown`. - Base reproduction: with the source change reverted, 5 of the 10 new tests fail: every proof case, under both cancellation and timeout, plus the retry proof. The 5 controls pass on both revisions. - The existing exit-17 tests stay green: abort during the held final `stop()`, under caller cancellation and deadline, plus the inherited-output and loop-exit variants. - Lifecycle regressions covered: interruption before result readback, interruption during final cleanup (existing tests), and cleanup retry after an incomplete first stop. - Remaining proof gaps: no real LLDB, debugserver or debuggee was run. A Node peer whose leader is still alive cannot be stopped verifiably by the owned-process cleanup in this harness. So the "abort during LLDB teardown after publishing" window is covered by source reasoning (the same `processStopped` gate), not by its own test. Windows is skipped, as with the sibling tests. - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npx vitest run --project boundary --maxWorkers=2` on `lldbCallTracerFailures`, `LldbCallTracer.cleanup`, `lldbPrelaunchCleanup`, `nativeCallObservation`, `lldbDiscovery` (boundary) — 5 files, 71 tests passed - The new tests plus the existing primary-outcome and inherited-output tests, run 3 times — 13/13 passed each time - The same new tests against the unfixed source — 5 failed (the proof cases), 5 passed (the controls) - `npm run test:local -- src/native/NativeCallObservation.test.ts` — 5 passed - `npm run check:fast` — passed - `npm run format:check` — passed Local Node was v26.5.0 (not the pinned 24.22.1); macOS arm64. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: macOS only, with an authored Node peer in place of LLDB. No real debugger. - Package or release metadata impact: none. - Security, privacy, process, or containment review: a target marker is released only on the bridge's explicit published proof. A missing PID or an unreadable or partial result keeps the target owned, so the change stays fail-closed. ## Author readiness Human owner: @kaoru0822-kitauji - [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. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. 🤖 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-03 13:39:23 +00:00

closed this issue
added the
pull-request
label

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

No description provided.

Rows
Columns