[PR #7319] fix(cli): report the byte-entropy provider id in cleanup diagnostics #7816

Open
opened 2026-11-05 01:47:52 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/7319
Author: @x-labb
Created: 10/26/2026
Status: 🔄 Open

Base: main ← Head: fix-7279-byte-entropy-cleanup-provider


📝 Commits (2)

  • dff5522 fix(cli): report the byte-entropy provider id in cleanup diagnostics
  • 2f43ac3 test(cli): assert retained entropy Evidence beside the cleanup diagnostic

📊 Changes

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

View changed files

📝 src/cli/binaryDiagnosticsCommands.ts (+2 -1)
➕ tests/process-global/byteEntropyCleanupCli.test.ts (+118 -0)

📄 Description

Summary

When rea inspect-byte-entropy finishes reading a file but the descriptor close fails, the cleanup diagnostic names provider rea-byte-entropy, while the retained Evidence names byte-entropy. This makes the cleanup diagnostic use the Evidence provider's identity.

Fixes #7279

Written with AI assistance.

Problem and expected behavior

src/cli/binaryDiagnosticsCommands.ts passed the literal "rea-byte-entropy" to finishAnalysisCommand, while the provider identity is BYTE_ENTROPY_PROVIDER.id ("byte-entropy"). It is the only command in that file whose literal differs from its provider constant. With a persistent descriptor-close failure, details.diagnostics.cleanup_error.details.provider_id was rea-byte-entropy. The failure status, resource list and completed Evidence were already correct.

Change and scope

Use BYTE_ENTROPY_PROVIDER.id at the call site, so the cleanup provider id and the Evidence provider id come from one owner. Nothing else changes.

Not included: the other "rea-…" literals in the same file match their provider constants today. Replacing them with the constants would be a separate cleanup, so I left them alone.

Contribution scope: ordinary fix within existing boundaries; no maintainer scope decision is needed.

Contract and boundary impact

  • Semantic owner and earliest changed stage: BYTE_ENTROPY_PROVIDER (src/domain/investigationProviders.ts); the CLI command registration passed a duplicated string. No superseded path was removed.
  • Contribution scope fit and linked maintainer decision: not applicable (bug fix within existing boundaries).
  • CLI and MCP/tool-catalog contract: no schema or catalog change. Only the nested cleanup diagnostic's provider_id value changes (rea-byte-entropy → byte-entropy) on the descriptor-close failure path.
  • Provider, bridge, target-format, or platform compatibility: none; no provider is involved.
  • Evidence, artifact, provenance, or reconstruction contract: Evidence is unchanged; the diagnostic now agrees with it.
  • Target selection, authority, upstream behavior, and cleanup details: not applicable.
  • 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 or updated: tests/process-global/byteEntropyCleanupCli.test.ts. It runs the real registered inspect-byte-entropy CLI on a 512-byte file with a persistent EIO injected on the file handle's close() (module mock of node:fs/promises). It asserts cleanup_incomplete, the retained cleanup: incomplete details, that the nested cleanup_error.details.provider_id equals BYTE_ENTROPY_PROVIDER.id, and that the retained partial observation (diagnostics.primary_error.details.partial_observation) still carries the Evidence provider id, the file's SHA-256 and its byte count. A normal-close control asserts successful Evidence with the same provider id.
  • Base reproduction or other evidence (executed): on the unfixed code the close-failure case failed with provider_id: "rea-byte-entropy" where "byte-entropy" was expected; the control passed. With the fix both pass (vitest run --project process-global).
  • User-visible CLI/MCP output: only the nested provider_id value above changes.
  • Remaining proof gaps: see "Validation performed".

Boundary/lifecycle regressions covered: public CLI delivery with a cleanup failure after a completed observation ("Cleanup fails after success"). Not covered: primary-failure-plus-cleanup-failure for this command, cancellation during final close, and a naturally occurring OS close failure (the failure is injected).

  • 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

Windows 11, Node 24.22.1:

  • npx vitest run --project process-global tests/process-global/byteEntropyCleanupCli.test.ts — fails on base with the provider-id mismatch; 2/2 pass with the fix. The neighboring binaryDiagnosticsCliCleanup.test.ts also passes (4/4).
  • npm run test:local -- src/application/binaryDiagnostics/ByteEntropyService.test.ts — 20 passed, 2 skipped, 1 failed ("reports a changed artifact when its parent directory moves during streaming", a Windows ENOENT on rename). The same failure occurs without this change.
  • npm run check:fast (typecheck and lint) — passed.
  • oxfmt --check on the two changed files — clean.
  • npm run verify:test-discovery — all 2591 test files discovered exactly once.

Not run:

  • npm run test:focused and the acceptance suite (e.g. tests/acceptance/localPathAdmissionCli.test.ts): they need the compiled runtime, and the build needs Windows native artifacts that this host lacks. Run directly, that acceptance test fails because no dist/ exists; that failure is environmental.
  • npm run check, npm run docs:check, and the real-provider suites.
  • Linux. The test relies only on a module mock, not on platform behavior, but it was run only on Windows.

Compatibility, safety, and release

  • Breaking changes or migration steps: none. The only observable change is the nested diagnostic's provider_id string.
  • Real Hopper/Ghidra, browser, or OS coverage: not applicable (no provider involved).
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: none; no authority, process or path logic changes.

Author readiness

Human owner: @x-labb

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

🔄 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/7319 **Author:** [@x-labb](https://github.com/x-labb) **Created:** 10/26/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix-7279-byte-entropy-cleanup-provider` --- ### 📝 Commits (2) - [`dff5522`](https://github.com/morluto/rea/commit/dff55223038ab3bfbb6703faea6df1db9390cf8f) fix(cli): report the byte-entropy provider id in cleanup diagnostics - [`2f43ac3`](https://github.com/morluto/rea/commit/2f43ac36d41dce71bff181e4ccca919444bc0b1d) test(cli): assert retained entropy Evidence beside the cleanup diagnostic ### 📊 Changes **2 files changed** (+120 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/cli/binaryDiagnosticsCommands.ts` (+2 -1) ➕ `tests/process-global/byteEntropyCleanupCli.test.ts` (+118 -0) </details> ### 📄 Description ## Summary When `rea inspect-byte-entropy` finishes reading a file but the descriptor close fails, the cleanup diagnostic names provider `rea-byte-entropy`, while the retained Evidence names `byte-entropy`. This makes the cleanup diagnostic use the Evidence provider's identity. Fixes #7279 Written with AI assistance. ## Problem and expected behavior `src/cli/binaryDiagnosticsCommands.ts` passed the literal `"rea-byte-entropy"` to `finishAnalysisCommand`, while the provider identity is `BYTE_ENTROPY_PROVIDER.id` (`"byte-entropy"`). It is the only command in that file whose literal differs from its provider constant. With a persistent descriptor-close failure, `details.diagnostics.cleanup_error.details.provider_id` was `rea-byte-entropy`. The failure status, resource list and completed Evidence were already correct. ## Change and scope Use `BYTE_ENTROPY_PROVIDER.id` at the call site, so the cleanup provider id and the Evidence provider id come from one owner. Nothing else changes. Not included: the other `"rea-…"` literals in the same file match their provider constants today. Replacing them with the constants would be a separate cleanup, so I left them alone. Contribution scope: ordinary fix within existing boundaries; no maintainer scope decision is needed. ## Contract and boundary impact - Semantic owner and earliest changed stage: `BYTE_ENTROPY_PROVIDER` (`src/domain/investigationProviders.ts`); the CLI command registration passed a duplicated string. No superseded path was removed. - Contribution scope fit and linked maintainer decision: not applicable (bug fix within existing boundaries). - CLI and MCP/tool-catalog contract: no schema or catalog change. Only the nested cleanup diagnostic's `provider_id` value changes (`rea-byte-entropy` → `byte-entropy`) on the descriptor-close failure path. - Provider, bridge, target-format, or platform compatibility: none; no provider is involved. - Evidence, artifact, provenance, or reconstruction contract: Evidence is unchanged; the diagnostic now agrees with it. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - 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 or updated: `tests/process-global/byteEntropyCleanupCli.test.ts`. It runs the real registered `inspect-byte-entropy` CLI on a 512-byte file with a persistent `EIO` injected on the file handle's `close()` (module mock of `node:fs/promises`). It asserts `cleanup_incomplete`, the retained `cleanup: incomplete` details, that the nested `cleanup_error.details.provider_id` equals `BYTE_ENTROPY_PROVIDER.id`, and that the retained partial observation (`diagnostics.primary_error.details.partial_observation`) still carries the Evidence provider id, the file's SHA-256 and its byte count. A normal-close control asserts successful Evidence with the same provider id. - Base reproduction or other evidence (executed): on the unfixed code the close-failure case failed with `provider_id: "rea-byte-entropy"` where `"byte-entropy"` was expected; the control passed. With the fix both pass (`vitest run --project process-global`). - User-visible CLI/MCP output: only the nested `provider_id` value above changes. - Remaining proof gaps: see "Validation performed". Boundary/lifecycle regressions covered: public CLI delivery with a cleanup failure after a completed observation ("Cleanup fails after success"). Not covered: primary-failure-plus-cleanup-failure for this command, cancellation during final close, and a naturally occurring OS close failure (the failure is injected). - [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 Windows 11, Node 24.22.1: - `npx vitest run --project process-global tests/process-global/byteEntropyCleanupCli.test.ts` — fails on base with the provider-id mismatch; 2/2 pass with the fix. The neighboring `binaryDiagnosticsCliCleanup.test.ts` also passes (4/4). - `npm run test:local -- src/application/binaryDiagnostics/ByteEntropyService.test.ts` — 20 passed, 2 skipped, 1 failed ("reports a changed artifact when its parent directory moves during streaming", a Windows `ENOENT` on rename). The same failure occurs without this change. - `npm run check:fast` (typecheck and lint) — passed. - `oxfmt --check` on the two changed files — clean. - `npm run verify:test-discovery` — all 2591 test files discovered exactly once. Not run: - `npm run test:focused` and the acceptance suite (e.g. `tests/acceptance/localPathAdmissionCli.test.ts`): they need the compiled runtime, and the build needs Windows native artifacts that this host lacks. Run directly, that acceptance test fails because no `dist/` exists; that failure is environmental. - `npm run check`, `npm run docs:check`, and the real-provider suites. - Linux. The test relies only on a module mock, not on platform behavior, but it was run only on Windows. ## Compatibility, safety, and release - Breaking changes or migration steps: none. The only observable change is the nested diagnostic's `provider_id` string. - Real Hopper/Ghidra, browser, or OS coverage: not applicable (no provider involved). - Package or release metadata impact: none. - Security, privacy, process, or containment review: none; no authority, process or path logic changes. ## Author readiness Human owner: @x-labb - [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 - [ ] 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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
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#7816
No description provided.