[PR #5867] [MERGED] fix(artifacts): report unknown inventory traversal in inspection coverage #7093

New issue

Closed

opened 2026-11-01 21:45:19 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:45:19 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/artifact-inventory-unknown-coverage

📝 Commits (2)

941fb57 fix(artifacts): keep unknown inventory traversal out of complete inspection coverage
a289813 Merge remote-tracking branch 'origin/main' into fix/artifact-inventory-unknown-coverage

📊 Changes

4 files changed (+145 additions, -14 deletions)

View changed files

📝 docs/mcp-contracts.md (+3 -1)
📝 src/domain/analysisView/artifactInventoryView.ts (+8 -12)
📝 src/domain/artifactInspection.ts (+4 -1)
➕ tests/boundary/mcp/artifactInspectionCoverageMcp.test.ts (+130 -0)

📄 Description

What broke: inspect_artifact reported coverage.status: "complete-within-substeps" (and its summary source_complete: true with no source_coverage) even when the nested inventory reported container-entries traversal coverage unknown, e.g. a directory with an omitted FIFO.
Fix: The inspection producer derives its status from the inventory substep's typed coverage (unknown traversal → partial), and artifact summaries always carry the producer's source_coverage plus the traversal-unknown caveat.
Risk: Low. Additive summary field (already declared optional in the output schema); inspections of roots with unknown traversal now report partial instead of complete-within-substeps.

Summary

Fixes #5837. Machine consumers reading an inspection or its summary were told coverage was complete while the inventory owner had already recorded unknown traversal. Only limitation prose or a separate inventory-view call revealed the gap.

Problem and expected behavior

Trigger: open_binary + inspect_artifact (or CLI rea inspect-artifact) on a directory that contains ordinary files and a FIFO. The inventory omits the FIFO and reports coverage: {scope: "container-entries", status: "unknown"}.

Violated invariant (docs/tool-design.md): completeness must come from examined coverage, and missing coverage is unknown. A completed inventory substep does not establish exhaustive traversal.

Change and scope

src/domain/artifactInspection.ts: inspectionCoverage now reads the inventory substep's coverage. It previously ignored that argument and checked only omission counts, which are always zero. Unknown traversal yields the existing partial status. truncated still takes precedence for omissions.
src/domain/analysisView/artifactInventoryView.ts: source_coverage and the "Producer traversal coverage is unknown" unknown no longer depend on the PCK/PyInstaller-only gate, so summaries of every format read the owner's fact instead of hiding it. That gate now controls only available_hash_status_counts and the hash-unknown caveat, which keep their existing behavior. This also fixes summaries of historical inline inspections that still say complete-within-substeps.
docs/mcp-contracts.md: documents the summary's source_coverage and the partial mapping.

Not included: no new status enum value. partial already existed and the precise unknown stays available as source_coverage.status. source_coverage stays exactOptional in the view schema so that older retained view Evidence still parses.

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

Contract and boundary impact

Semantic owner and earliest changed stage: inventory producer owns traversal coverage (unchanged); inspection coverage composition in src/domain/artifactInspection.ts is the earliest changed stage.
Contribution scope fit and linked maintainer decision: existing artifact inspection boundary; not applicable.
CLI and MCP/tool-catalog contract: no schema change. inspect_artifact can now emit the already-declared partial status. Artifact summaries now always include the already-declared optional source_coverage.
Provider, bridge, target-format, or platform compatibility: all artifact formats. Roots whose traversal is unknown (omitted entries, DMG/PKG/universal Mach-O without a native adapter, encrypted PCK, opaque PyInstaller) now report partial.
Evidence, artifact, provenance, or reconstruction contract: inspection IDs change for those roots because the status is part of the digested result. Nested inventory Evidence is unchanged.
Target selection, authority, upstream behavior, and cleanup details: not applicable.
Generated metadata, package, or installation impact: none (docs:check passes).
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/boundary/mcp/artifactInspectionCoverageMcp.test.ts uses the production createServer session over MCP. It covers a directory with a FIFO and one without, and checks inspect_artifact coverage plus both summary paths (inspect_analysis_view summary of the retained parent, and inspect_artifact {detail: "summary"}) against the advertised output schema.
Base reproduction or other evidence (executed): with src/ changes stashed, the new test fails on base in both cases: expected 'complete-within-substeps' to be 'partial' for the FIFO case, and expected undefined to deeply equal { scope: 'container-entries', … } for the summary's source_coverage. With the fix, both cases pass.

User-visible CLI/MCP output (executed, rea inspect-artifact <dir with Info.plist, main.js, FIFO> from the built CLI with isolated HOME/XDG/TMPDIR):

Before (base f801b2fbf):

{"inspection_status":"complete-within-substeps","inventory_coverage":{"scope":"container-entries","status":"unknown"}}
{"summary_inspection_status":"complete-within-substeps","source_complete":true,
 "unknowns":["Inventory does not establish application runtime behavior or complete format-specific analysis."]}

After:

{"inspection_status":"partial","inventory_coverage":{"scope":"container-entries","status":"unknown"}}
{"summary_inspection_status":"partial","source_complete":true,
 "source_coverage":{"scope":"container-entries","status":"unknown"},
 "unknowns":["Inventory does not establish application runtime behavior or complete format-specific analysis.",
             "Producer traversal coverage is unknown; missing matches do not prove source absence."]}

Cost: the CLI summary response for this fixture grows from 4531 to 4667 bytes (+136, measured). A complete-traversal summary adds only the source_coverage object (~67 bytes, computed, not measured). Token counts were not measured. There is no extra call, and the projection does no new work beyond reading the already-parsed coverage.
Remaining proof gaps: only the macOS directory+FIFO case was exercised end-to-end. Other unknown-traversal formats (DMG/PKG, encrypted PCK, opaque PyInstaller) go through the same composition but their new partial status was not asserted individually.
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

npm run test:focused -- tests/boundary/mcp/artifactInspectionCoverageMcp.test.ts on base (src stashed): 2 failed (expected). With fix: 2 passed.
npm run test:focused -- <39 related files> (artifact inspection/inventory/comparison domain tests, artifact/PCK/PyInstaller/desktop/prompt/session MCP boundary tests, artifact provider boundary tests, Apple/mobile/Harmony/ZIP/PCK/PyInstaller acceptance tests, CLI logging/environment): 39 files, 252 tests passed.
Real CLI repro (node scripts/rea.mjs inspect-artifact … --json and --detail summary --evidence_output …): before/after output above.
npm run check:fast: passed.
npm run format:check: passed.
npm run docs:check: passed.

Compatibility, safety, and release

Breaking changes or migration steps: none. The status value and the summary field were both already declared in the output schemas.
Real Hopper/Ghidra, browser, or OS coverage: no provider needed; macOS (Darwin arm64, Node 26) only.
Package or release metadata impact: none.
Security, privacy, process, or containment review: no new reads, processes or effects.

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/5867 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/21/2026 **Status:** ✅ Merged **Merged:** 10/21/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/artifact-inventory-unknown-coverage` --- ### 📝 Commits (2) - [`941fb57`](https://github.com/morluto/rea/commit/941fb57132245e5b9c8e7f6bbeef4a5fd6a9988a) fix(artifacts): keep unknown inventory traversal out of complete inspection coverage - [`a289813`](https://github.com/morluto/rea/commit/a289813ef4ebad88d5f72bff10ce3c31e0cbc6d0) Merge remote-tracking branch 'origin/main' into fix/artifact-inventory-unknown-coverage ### 📊 Changes **4 files changed** (+145 additions, -14 deletions) <details> <summary>View changed files</summary> 📝 `docs/mcp-contracts.md` (+3 -1) 📝 `src/domain/analysisView/artifactInventoryView.ts` (+8 -12) 📝 `src/domain/artifactInspection.ts` (+4 -1) ➕ `tests/boundary/mcp/artifactInspectionCoverageMcp.test.ts` (+130 -0) </details> ### 📄 Description **What broke:** `inspect_artifact` reported `coverage.status: "complete-within-substeps"` (and its summary `source_complete: true` with no `source_coverage`) even when the nested inventory reported `container-entries` traversal coverage `unknown`, e.g. a directory with an omitted FIFO. **Fix:** The inspection producer derives its status from the inventory substep's typed coverage (unknown traversal → `partial`), and artifact summaries always carry the producer's `source_coverage` plus the traversal-unknown caveat. **Risk:** Low. Additive summary field (already declared optional in the output schema); inspections of roots with unknown traversal now report `partial` instead of `complete-within-substeps`. ## Summary Fixes #5837. Machine consumers reading an inspection or its summary were told coverage was complete while the inventory owner had already recorded unknown traversal. Only limitation prose or a separate inventory-view call revealed the gap. ## Problem and expected behavior Trigger: `open_binary` + `inspect_artifact` (or CLI `rea inspect-artifact`) on a directory that contains ordinary files and a FIFO. The inventory omits the FIFO and reports `coverage: {scope: "container-entries", status: "unknown"}`. Violated invariant (docs/tool-design.md): completeness must come from examined coverage, and missing coverage is unknown. A completed inventory substep does not establish exhaustive traversal. ## Change and scope - `src/domain/artifactInspection.ts`: `inspectionCoverage` now reads the inventory substep's coverage. It previously ignored that argument and checked only omission counts, which are always zero. Unknown traversal yields the existing `partial` status. `truncated` still takes precedence for omissions. - `src/domain/analysisView/artifactInventoryView.ts`: `source_coverage` and the "Producer traversal coverage is unknown" unknown no longer depend on the PCK/PyInstaller-only gate, so summaries of every format read the owner's fact instead of hiding it. That gate now controls only `available_hash_status_counts` and the hash-unknown caveat, which keep their existing behavior. This also fixes summaries of historical inline inspections that still say `complete-within-substeps`. - `docs/mcp-contracts.md`: documents the summary's `source_coverage` and the `partial` mapping. Not included: no new status enum value. `partial` already existed and the precise `unknown` stays available as `source_coverage.status`. `source_coverage` stays `exactOptional` in the view schema so that older retained view Evidence still parses. Contribution scope: ordinary fix within existing boundaries; maintainer scope decision not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: inventory producer owns traversal coverage (unchanged); inspection coverage composition in `src/domain/artifactInspection.ts` is the earliest changed stage. - Contribution scope fit and linked maintainer decision: existing artifact inspection boundary; not applicable. - CLI and MCP/tool-catalog contract: no schema change. `inspect_artifact` can now emit the already-declared `partial` status. Artifact summaries now always include the already-declared optional `source_coverage`. - Provider, bridge, target-format, or platform compatibility: all artifact formats. Roots whose traversal is unknown (omitted entries, DMG/PKG/universal Mach-O without a native adapter, encrypted PCK, opaque PyInstaller) now report `partial`. - Evidence, artifact, provenance, or reconstruction contract: inspection IDs change for those roots because the status is part of the digested result. Nested inventory Evidence is unchanged. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: none (`docs:check` passes). - [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/boundary/mcp/artifactInspectionCoverageMcp.test.ts` uses the production `createServer` session over MCP. It covers a directory with a FIFO and one without, and checks `inspect_artifact` coverage plus both summary paths (`inspect_analysis_view` summary of the retained parent, and `inspect_artifact {detail: "summary"}`) against the advertised output schema. - Base reproduction or other evidence (executed): with `src/` changes stashed, the new test fails on base in both cases: `expected 'complete-within-substeps' to be 'partial'` for the FIFO case, and `expected undefined to deeply equal { scope: 'container-entries', … }` for the summary's `source_coverage`. With the fix, both cases pass. - User-visible CLI/MCP output (executed, `rea inspect-artifact <dir with Info.plist, main.js, FIFO>` from the built CLI with isolated HOME/XDG/TMPDIR): Before (base f801b2fbf): ```json {"inspection_status":"complete-within-substeps","inventory_coverage":{"scope":"container-entries","status":"unknown"}} {"summary_inspection_status":"complete-within-substeps","source_complete":true, "unknowns":["Inventory does not establish application runtime behavior or complete format-specific analysis."]} ``` After: ```json {"inspection_status":"partial","inventory_coverage":{"scope":"container-entries","status":"unknown"}} {"summary_inspection_status":"partial","source_complete":true, "source_coverage":{"scope":"container-entries","status":"unknown"}, "unknowns":["Inventory does not establish application runtime behavior or complete format-specific analysis.", "Producer traversal coverage is unknown; missing matches do not prove source absence."]} ``` - Cost: the CLI summary response for this fixture grows from 4531 to 4667 bytes (+136, measured). A complete-traversal summary adds only the `source_coverage` object (~67 bytes, computed, not measured). Token counts were not measured. There is no extra call, and the projection does no new work beyond reading the already-parsed coverage. - Remaining proof gaps: only the macOS directory+FIFO case was exercised end-to-end. Other unknown-traversal formats (DMG/PKG, encrypted PCK, opaque PyInstaller) go through the same composition but their new `partial` status was not asserted individually. - [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 - `npm run test:focused -- tests/boundary/mcp/artifactInspectionCoverageMcp.test.ts` on base (src stashed): 2 failed (expected). With fix: 2 passed. - `npm run test:focused -- <39 related files>` (artifact inspection/inventory/comparison domain tests, artifact/PCK/PyInstaller/desktop/prompt/session MCP boundary tests, artifact provider boundary tests, Apple/mobile/Harmony/ZIP/PCK/PyInstaller acceptance tests, CLI logging/environment): 39 files, 252 tests passed. - Real CLI repro (`node scripts/rea.mjs inspect-artifact … --json` and `--detail summary --evidence_output …`): before/after output above. - `npm run check:fast`: passed. - `npm run format:check`: passed. - `npm run docs:check`: passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none. The status value and the summary field were both already declared in the output schemas. - Real Hopper/Ghidra, browser, or OS coverage: no provider needed; macOS (Darwin arm64, Node 26) only. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new reads, processes or effects. ## 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 - [ ] 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](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:45:19 +00:00