[PR #2997] [CLOSED] fix(providers): keep failed launches from reporting the version probe output #5547

Closed
opened 2026-10-31 01:07:29 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2997
Author: @Dragolone
Created: 10/13/2026
Status: ❌ Closed

Base: main ← Head: fix/sigrok-launch-failure-captured-output


📝 Commits (4)

  • f4c04f5 fix(signals): keep a failed launch from reporting the version probe output
  • a23654d fix(captures): keep a failed launch from reporting the version probe output
  • e0db1c5 Merge upstream main into fix/sigrok-launch-failure-captured-output
  • cebd72a fix(wasm): keep a failed wasm2wat launch from reporting wasm-objdump output

📊 Changes

6 files changed (+86 additions, -0 deletions)

View changed files

📝 src/captures/TsharkCaptureProvider.ts (+3 -0)
📝 src/signals/SigrokRecordingProvider.ts (+3 -0)
📝 src/wasm/WabtArtifactProvider.test.ts (+43 -0)
📝 src/wasm/WabtArtifactProvider.ts (+3 -0)
📝 tests/boundary/captures/captureProvider.test.ts (+17 -0)
📝 tests/boundary/signals/recordingProvider.test.ts (+17 -0)

📄 Description

Summary

The sigrok, TShark and WABT providers share one bug. When the main command (sigrok decode or decoder discovery, TShark capture inspection or field discovery, WABT wasm2wat after wasm-objdump) fails to launch after the --version probe succeeded, the resulting error carries the version probe's output as its captured_output. That output is the sigrok-cli 0.7.2 … banner, the version warning and exit_code: 0, so it describes a command that never started.

SigrokRecordingProvider, TsharkCaptureProvider and WabtArtifactProvider each keep one capturedOutput variable for both commands and assigns it only after runOwnedCommand returns. A launcher error thrown before any output (for example a synchronous spawn E2BIG) leaves the probe's value in place, and #failure then attaches it to the final AnalysisOutputError.

No issue is linked: I searched issues and PRs for "E2BIG", "captured output version probe", "stale captured output" and "spawn failure" and found no report. No open PR touches this logic.

Problem and expected behavior

Smallest trigger: a launcher that succeeds for --version and throws spawn E2BIG for the next command. The same thing happens with a real kernel E2BIG: a large inherited environment lets the small version argv fit while a long, budget-valid decoder selector does not. The docs note that "a large inherited environment may still exhaust it".

  • Before: the error's capturedOutput is {"stdout":"sigrok-cli 0.7.2\n\n- libsigrokdecode 0.5.3\n","stderr":"version warning\n","truncated":false,"exit_code":0,"signal":null}, for both decode and discovery.
    TShark capture inspection and field discovery behave the same way, carrying "TShark (Wireshark) …" with exit_code: 0.
  • After: no capturedOutput, because the failed command produced none. The error message still names the launch failure (spawn E2BIG).

Change and scope

In all three providers, execute clears capturedOutput before each command, so captured output always belongs to the most recent command or is absent. Each provider is fixed in its own commit. These are the only providers with a shared captured-output slot (the WABT case came from the Codex review).

Not included: the error's classification. A non-OwnedCommandFailure launch error still becomes AnalysisOutputError. Mapping launch failures to a separate category is a broader contract question I left for maintainers. Bounding the size of captured output, a separate issue, is also not changed here.

Contribution scope: an ordinary evidence-provenance fix within existing boundaries; a maintainer scope decision is not applicable.

Contract and boundary impact

  • Semantic owner and earliest changed stage: execute in src/signals/SigrokRecordingProvider.ts and src/captures/TsharkCaptureProvider.ts and src/wasm/WabtArtifactProvider.ts.
  • Contribution scope fit and linked maintainer decision: in scope; not applicable.
  • CLI and MCP/tool-catalog contract: no schema change; launch failures no longer include another command's captured output.
  • Provider, bridge, target-format, or platform compatibility: none.
  • Evidence, artifact, provenance, or reconstruction contract: captured output is attributed only to the command that produced it.
  • Target selection, authority, upstream behavior, and cleanup details (if applicable): unchanged; the same commands run with the same limits.
  • 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.

  • 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 launch effect is the provider's existing sigrok invocation; this PR does not add or change any launch.)

Evidence and regression coverage

  • Tests added or updated: tests/boundary/signals/recordingProvider.test.ts and tests/boundary/captures/captureProvider.test.ts each add a launch-failure fixture mode, in which the launcher throws spawn E2BIG for every command except --version. Each adds "does not attribute the version probe's output to a %s command that fails to launch" for decode/discovery and capture/fields respectively. src/wasm/WabtArtifactProvider.test.ts adds "does not attribute wasm-objdump output to a wasm2wat launch failure".
  • Base reproduction: with each provider's change reverted, its two cases fail with expected { …(5) } to be undefined (the version probe's captured output).
  • User-visible CLI/MCP output: not captured separately; this affects only the captured_output of a launch failure.
  • Remaining proof gaps: the committed test injects the launch error; a real-kernel E2BIG reproduction was run separately during investigation but is too host-dependent to commit.

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

macOS (Apple silicon), Node 24.19.0.

  • npm run build:cached && npx vitest run tests/boundary/signals/recordingProvider.test.ts -t "fails to launch" on the base — both cases fail as quoted.
  • With both fixes, rebased on current main: the signal test files plus tests/boundary/captures and src/captures — 100 passed, 19 skipped (Linux-only cases).
  • npx tsc --noEmit — passed (0 errors).
  • npx oxfmt --check and npx oxlint on both changed files — passed, no errors.
  • npm run verify:module-boundaries — passed.

Compatibility, safety, and release

  • Breaking changes or migration steps: none.
  • Real Hopper/Ghidra, browser, or OS coverage: macOS with the repository's command-boundary simulator; sigrok-cli itself not installed.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: no change to limits, process ownership or cleanup.

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.

🔄 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/2997 **Author:** [@Dragolone](https://github.com/Dragolone) **Created:** 10/13/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/sigrok-launch-failure-captured-output` --- ### 📝 Commits (4) - [`f4c04f5`](https://github.com/morluto/rea/commit/f4c04f518b097822280beff9be5bf41f4173d625) fix(signals): keep a failed launch from reporting the version probe output - [`a23654d`](https://github.com/morluto/rea/commit/a23654d193de4d29b5555e64985be36041943b98) fix(captures): keep a failed launch from reporting the version probe output - [`e0db1c5`](https://github.com/morluto/rea/commit/e0db1c576110fd1e29e63411e29ab4f54566d15b) Merge upstream main into fix/sigrok-launch-failure-captured-output - [`cebd72a`](https://github.com/morluto/rea/commit/cebd72a2e7deea89906821f1d6d7df2dae31e744) fix(wasm): keep a failed wasm2wat launch from reporting wasm-objdump output ### 📊 Changes **6 files changed** (+86 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `src/captures/TsharkCaptureProvider.ts` (+3 -0) 📝 `src/signals/SigrokRecordingProvider.ts` (+3 -0) 📝 `src/wasm/WabtArtifactProvider.test.ts` (+43 -0) 📝 `src/wasm/WabtArtifactProvider.ts` (+3 -0) 📝 `tests/boundary/captures/captureProvider.test.ts` (+17 -0) 📝 `tests/boundary/signals/recordingProvider.test.ts` (+17 -0) </details> ### 📄 Description ## Summary The sigrok, TShark and WABT providers share one bug. When the main command (sigrok decode or decoder discovery, TShark capture inspection or field discovery, WABT `wasm2wat` after `wasm-objdump`) fails to launch after the `--version` probe succeeded, the resulting error carries the version probe's output as its `captured_output`. That output is the `sigrok-cli 0.7.2 …` banner, the version warning and `exit_code: 0`, so it describes a command that never started. `SigrokRecordingProvider`, `TsharkCaptureProvider` and `WabtArtifactProvider` each keep one `capturedOutput` variable for both commands and assigns it only after `runOwnedCommand` returns. A launcher error thrown before any output (for example a synchronous `spawn E2BIG`) leaves the probe's value in place, and `#failure` then attaches it to the final `AnalysisOutputError`. No issue is linked: I searched issues and PRs for "E2BIG", "captured output version probe", "stale captured output" and "spawn failure" and found no report. No open PR touches this logic. ## Problem and expected behavior Smallest trigger: a launcher that succeeds for `--version` and throws `spawn E2BIG` for the next command. The same thing happens with a real kernel `E2BIG`: a large inherited environment lets the small version argv fit while a long, budget-valid decoder selector does not. The docs note that "a large inherited environment may still exhaust it". - Before: the error's `capturedOutput` is `{"stdout":"sigrok-cli 0.7.2\n\n- libsigrokdecode 0.5.3\n","stderr":"version warning\n","truncated":false,"exit_code":0,"signal":null}`, for both decode and discovery. TShark capture inspection and field discovery behave the same way, carrying `"TShark (Wireshark) …"` with `exit_code: 0`. - After: no `capturedOutput`, because the failed command produced none. The error message still names the launch failure (`spawn E2BIG`). ## Change and scope In all three providers, `execute` clears `capturedOutput` before each command, so captured output always belongs to the most recent command or is absent. Each provider is fixed in its own commit. These are the only providers with a shared captured-output slot (the WABT case came from the Codex review). Not included: the error's classification. A non-`OwnedCommandFailure` launch error still becomes `AnalysisOutputError`. Mapping launch failures to a separate category is a broader contract question I left for maintainers. Bounding the size of captured output, a separate issue, is also not changed here. Contribution scope: an ordinary evidence-provenance fix within existing boundaries; a maintainer scope decision is not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: `execute` in `src/signals/SigrokRecordingProvider.ts` and `src/captures/TsharkCaptureProvider.ts` and `src/wasm/WabtArtifactProvider.ts`. - Contribution scope fit and linked maintainer decision: in scope; not applicable. - CLI and MCP/tool-catalog contract: no schema change; launch failures no longer include another command's captured output. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: captured output is attributed only to the command that produced it. - Target selection, authority, upstream behavior, and cleanup details (if applicable): unchanged; the same commands run with the same limits. - 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. - [x] Read existing files or captured data. - [x] 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 launch effect is the provider's existing sigrok invocation; this PR does not add or change any launch.) ## Evidence and regression coverage - Tests added or updated: `tests/boundary/signals/recordingProvider.test.ts` and `tests/boundary/captures/captureProvider.test.ts` each add a `launch-failure` fixture mode, in which the launcher throws `spawn E2BIG` for every command except `--version`. Each adds "does not attribute the version probe's output to a %s command that fails to launch" for `decode`/`discovery` and `capture`/`fields` respectively. `src/wasm/WabtArtifactProvider.test.ts` adds "does not attribute wasm-objdump output to a wasm2wat launch failure". - Base reproduction: with each provider's change reverted, its two cases fail with `expected { …(5) } to be undefined` (the version probe's captured output). - User-visible CLI/MCP output: not captured separately; this affects only the `captured_output` of a launch failure. - Remaining proof gaps: the committed test injects the launch error; a real-kernel `E2BIG` reproduction was run separately during investigation but is too host-dependent to commit. For evidence-bearing changes: - [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 macOS (Apple silicon), Node 24.19.0. - `npm run build:cached && npx vitest run tests/boundary/signals/recordingProvider.test.ts -t "fails to launch"` on the base — both cases fail as quoted. - With both fixes, rebased on current `main`: the signal test files plus `tests/boundary/captures` and `src/captures` — 100 passed, 19 skipped (Linux-only cases). - `npx tsc --noEmit` — passed (0 errors). - `npx oxfmt --check` and `npx oxlint` on both changed files — passed, no errors. - `npm run verify:module-boundaries` — passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: macOS with the repository's command-boundary simulator; sigrok-cli itself not installed. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no change to limits, process ownership or cleanup. ## 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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-10-31 01:07:29 +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#5547
No description provided.