[PR #7023] [MERGED] fix(lldb): retain completed trace facts through cleanup failures #7582

Closed
opened 2026-11-03 13:38:52 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/7023
Author: @WenyuChiou
Created: 10/24/2026
Status: ✅ Merged
Merged: 10/24/2026
Merged by: @morluto

Base: main ← Head: fix/5913-completed-trace-retention-20261024


📝 Commits (1)

  • d7a0a98 fix(lldb): retain completed trace facts through cleanup failures

📊 Changes

4 files changed (+314 additions, -25 deletions)

View changed files

📝 src/native/LldbCallTracer.ts (+52 -19)
📝 src/native/NativeCallObservation.ts (+9 -5)
📝 tests/boundary/providers/native/lldbCallTracerFailures.test.ts (+251 -0)
📝 tests/boundary/providers/native/nativeCallObservation.test.ts (+2 -1)

📄 Description

Summary

Fixes #5913's remaining completed-trace retention failures during cleanup. The original output-close cancellation case is already covered upstream; this change addresses the two residual cleanup paths described in the issue follow-up.

Problem and expected behavior

After reading a valid completed LLDB result, process-group cleanup failure or uncertain target termination falls back to journal-only observations. This discards outcome, exit status, elapsed time, launch identity and breakpoints, and replaces final stdout bytes 4096 with the retained abc prefix length 3. Target-only uncertainty also reports tracer-failure instead of cleanup-failure.

Completed observations should survive without treating the producer's termination report as proof of host cleanup.

Change and scope

  • Reuse the existing completed-observation projector and share capture readback with the success path.
  • Keep cleanup failure primary when a capture read also fails; retain sibling captures and other completed facts.
  • Preserve journal-only fallback for missing or invalid completed results.
  • Derive limit-outcome wording from the same conservative termination decision as process.terminated, avoiding “was killed” when termination is unconfirmed.
  • Retain cleanup ownership, replacement blocking and retry behavior.

This is a routine correction within existing LLDB boundaries, not a new execution or mutation capability; a separate advance maintainer scope decision is not applicable. It does not duplicate #4751's descriptor-close ownership work.

Contract and boundary impact

  • Semantic owner: LLDB result readback and native-call observation projection.
  • CLI/MCP schemas, provider selection, target formats and platform support: unchanged.
  • Evidence: retain validated completed facts and final counters; keep capture completeness and host cleanup uncertainty explicit.
  • Target authority and cleanup: unchanged. Producer termination and host confirmation must both hold before reporting termination.
  • Generated metadata, dependencies, package and installation behavior: unchanged.

Expected workflow effects inherited from the existing provider:

  • 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.

The checked effects describe the existing debugger workflow and temporary capture/cleanup files, not new authority. Added regressions use synthetic launcher/cleanup fixtures; no real LLDB or target binary was executed.

Evidence and regression coverage

Executed locally on base 397aa62f2e203bc0f8350634752af0339048e81e and the reviewed four-file patch:

  • Before the fix, the clean control passed and both new cleanup regressions failed: completed fields/counters were missing, and target uncertainty had the wrong reason.
  • An added duration-limit/uncertain-termination regression failed on the false “was killed” note, then passed after the projection correction.
  • Seven added cases cover clean/group/target-uncertain/duration-limit-uncertain/group-plus-capture-failure paths and missing/invalid-result fallback. Group cleanup also checks replacement blocking, failed close and successful retry.
  • The existing wording oracle was corrected to exact uncertainty wording while retaining its structured assertions and adding a no-“was killed” assertion; coverage was not weakened.
  • Full final stdout count remains 4096 while retained text remains abc; sibling stderr retains its complete two-byte capture. Capture completeness semantics are unchanged.

The original failing wording-oracle log was overwritten by its passing rerun; only a transcript-derived failure excerpt remains. That excerpt is not represented as a preserved raw log. Automated static review found the wording and combined capture-failure gaps above; both were addressed and re-reviewed. This is not human review.

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

Validation performed

Linux cloud executor, Node 24.22.1 (repository pin), npm 11.9.0 (repository pin is 11.16.0), lockfile-pinned Vitest 5.0.3. Checks ran serially with bounded timeouts.

  • node node_modules/vitest/vitest.mjs run src/native/NativeCallObservation.test.ts tests/boundary/providers/native/nativeCallObservation.test.ts tests/boundary/providers/native/lldbRetainedObservations.test.ts tests/boundary/providers/native/nativeCallTargetCleanup.test.ts tests/boundary/providers/native/lldbCallTracerFailures.test.ts --maxWorkers 1 --no-file-parallelism — 68 passed, 5 files.
  • node scripts/run-typescript.mjs --noEmit --checkers 1 --maxMemory 2G — passed.
  • node scripts/verify-module-boundaries.mjs src/native/LldbCallTracer.ts src/native/NativeCallObservation.ts — passed.
  • node scripts/verify-test-discovery.mjs — passed; 2,422 files discovered exactly once.
  • node scripts/verify/architecture-guards.mjs — passed; 2,031 sources.
  • Focused oxlint on all four changed files — no errors, 13 warnings.
  • Focused oxfmt --check and git diff --check — passed.

Initial fixture type errors and an oversized test callback lint error were corrected before these final checks. No full npm run check, full suite/coverage, package verification, real-provider lane or public CLI/MCP end-to-end run was performed. Real debugger behavior and race frequency remain unverified; CI is separate from this local evidence.

Compatibility, safety, and release

No schema migration, package/release metadata change or expanded target authority. Synthetic fixtures exercise production readback/projection/ownership boundaries, not real LLDB. Existing focused tests include inert Node peers. No performance improvement or real-provider verification is claimed.

The API-published commit has different metadata from local reviewed commit d04294d6697ca1a05e120cd007df00a05193ab9d; its complete tree is identical: b35e7fb30b176d8bfbc295664383b02c1fe88a0b. All four public file blob hashes were verified against the reviewed files before opening this draft.

Author readiness

Human owner: @WenyuChiou

  • 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.

The human owner confirmed review completion and authorized recording these declarations.

Before merge

  • Required current-head checks and real-provider verification are satisfied or their gaps are explicitly resolved by maintainers.
  • The human owner has completed the readiness declarations.
  • Material scope and authority questions are resolved.

🔄 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/7023 **Author:** [@WenyuChiou](https://github.com/WenyuChiou) **Created:** 10/24/2026 **Status:** ✅ Merged **Merged:** 10/24/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/5913-completed-trace-retention-20261024` --- ### 📝 Commits (1) - [`d7a0a98`](https://github.com/morluto/rea/commit/d7a0a98a870e1da72b445fabec1fb19de09b173e) fix(lldb): retain completed trace facts through cleanup failures ### 📊 Changes **4 files changed** (+314 additions, -25 deletions) <details> <summary>View changed files</summary> 📝 `src/native/LldbCallTracer.ts` (+52 -19) 📝 `src/native/NativeCallObservation.ts` (+9 -5) 📝 `tests/boundary/providers/native/lldbCallTracerFailures.test.ts` (+251 -0) 📝 `tests/boundary/providers/native/nativeCallObservation.test.ts` (+2 -1) </details> ### 📄 Description ## Summary Fixes #5913's remaining completed-trace retention failures during cleanup. The original output-close cancellation case is already covered upstream; this change addresses the two residual cleanup paths described in [the issue follow-up](https://github.com/morluto/rea/issues/5913#issuecomment-6380896456). ## Problem and expected behavior After reading a valid completed LLDB result, process-group cleanup failure or uncertain target termination falls back to journal-only observations. This discards outcome, exit status, elapsed time, launch identity and breakpoints, and replaces final stdout bytes `4096` with the retained `abc` prefix length `3`. Target-only uncertainty also reports `tracer-failure` instead of `cleanup-failure`. Completed observations should survive without treating the producer's termination report as proof of host cleanup. ## Change and scope - Reuse the existing completed-observation projector and share capture readback with the success path. - Keep cleanup failure primary when a capture read also fails; retain sibling captures and other completed facts. - Preserve journal-only fallback for missing or invalid completed results. - Derive limit-outcome wording from the same conservative termination decision as `process.terminated`, avoiding “was killed” when termination is unconfirmed. - Retain cleanup ownership, replacement blocking and retry behavior. This is a routine correction within existing LLDB boundaries, not a new execution or mutation capability; a separate advance maintainer scope decision is not applicable. It does not duplicate #4751's descriptor-close ownership work. ## Contract and boundary impact - Semantic owner: LLDB result readback and native-call observation projection. - CLI/MCP schemas, provider selection, target formats and platform support: unchanged. - Evidence: retain validated completed facts and final counters; keep capture completeness and host cleanup uncertainty explicit. - Target authority and cleanup: unchanged. Producer termination and host confirmation must both hold before reporting termination. - Generated metadata, dependencies, package and installation behavior: unchanged. Expected workflow effects inherited from the existing provider: - [x] Read existing files or captured data. - [x] Launch or attach to a process. - [x] Inject instrumentation or run scripts/code. - [x] 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. The checked effects describe the existing debugger workflow and temporary capture/cleanup files, not new authority. Added regressions use synthetic launcher/cleanup fixtures; no real LLDB or target binary was executed. ## Evidence and regression coverage Executed locally on base `397aa62f2e203bc0f8350634752af0339048e81e` and the reviewed four-file patch: - Before the fix, the clean control passed and both new cleanup regressions failed: completed fields/counters were missing, and target uncertainty had the wrong reason. - An added duration-limit/uncertain-termination regression failed on the false “was killed” note, then passed after the projection correction. - Seven added cases cover clean/group/target-uncertain/duration-limit-uncertain/group-plus-capture-failure paths and missing/invalid-result fallback. Group cleanup also checks replacement blocking, failed close and successful retry. - The existing wording oracle was corrected to exact uncertainty wording while retaining its structured assertions and adding a no-“was killed” assertion; coverage was not weakened. - Full final stdout count remains `4096` while retained text remains `abc`; sibling stderr retains its complete two-byte capture. Capture completeness semantics are unchanged. The original failing wording-oracle log was overwritten by its passing rerun; only a transcript-derived failure excerpt remains. That excerpt is not represented as a preserved raw log. Automated static review found the wording and combined capture-failure gaps above; both were addressed and re-reviewed. This is not human review. - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved, with the log limitation disclosed above. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed Linux cloud executor, Node 24.22.1 (repository pin), npm 11.9.0 (repository pin is 11.16.0), lockfile-pinned Vitest 5.0.3. Checks ran serially with bounded timeouts. - `node node_modules/vitest/vitest.mjs run src/native/NativeCallObservation.test.ts tests/boundary/providers/native/nativeCallObservation.test.ts tests/boundary/providers/native/lldbRetainedObservations.test.ts tests/boundary/providers/native/nativeCallTargetCleanup.test.ts tests/boundary/providers/native/lldbCallTracerFailures.test.ts --maxWorkers 1 --no-file-parallelism` — **68 passed, 5 files**. - `node scripts/run-typescript.mjs --noEmit --checkers 1 --maxMemory 2G` — passed. - `node scripts/verify-module-boundaries.mjs src/native/LldbCallTracer.ts src/native/NativeCallObservation.ts` — passed. - `node scripts/verify-test-discovery.mjs` — passed; 2,422 files discovered exactly once. - `node scripts/verify/architecture-guards.mjs` — passed; 2,031 sources. - Focused `oxlint` on all four changed files — no errors, 13 warnings. - Focused `oxfmt --check` and `git diff --check` — passed. Initial fixture type errors and an oversized test callback lint error were corrected before these final checks. No full `npm run check`, full suite/coverage, package verification, real-provider lane or public CLI/MCP end-to-end run was performed. Real debugger behavior and race frequency remain unverified; CI is separate from this local evidence. ## Compatibility, safety, and release No schema migration, package/release metadata change or expanded target authority. Synthetic fixtures exercise production readback/projection/ownership boundaries, not real LLDB. Existing focused tests include inert Node peers. No performance improvement or real-provider verification is claimed. The API-published commit has different metadata from local reviewed commit `d04294d6697ca1a05e120cd007df00a05193ab9d`; its complete tree is identical: `b35e7fb30b176d8bfbc295664383b02c1fe88a0b`. All four public file blob hashes were verified against the reviewed files before opening this draft. ## Author readiness Human owner: @WenyuChiou - [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. The human owner confirmed review completion and authorized recording these declarations. ## Before merge - [ ] Required current-head checks and real-provider verification are satisfied or their gaps are explicitly resolved by maintainers. - [ ] The human owner has completed the readiness declarations. - [ ] Material scope and authority questions are resolved. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-03 13:38:52 +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#7582
No description provided.