[PR #2214] [MERGED] fix(artifacts): compare inventories using declared coverage #5088

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

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2214
Author: @thangldw
Created: 10/12/2026
Status: ✅ Merged
Merged: 10/13/2026
Merged by: @morluto

Base: main ← Head: codex/fix-plist-inventory-comparison


📝 Commits (10+)

  • 487d129 fix(artifacts): preserve complete root inventory comparisons
  • 51c3043 fix(artifacts): declare inventory coverage before comparison
  • b94e10a fix(artifacts): preserve traversal omissions and materialization limits
  • 17787e4 fix(artifacts): reconcile scoped inventory coverage with main
  • 4ff71fb fix(artifacts): preserve coverage declarations and projection scope
  • dd371ce Merge captured main into PR #2214
  • 4e36dc1 fix(artifacts): require entry coverage for child changes
  • 7b51ddf fix(artifacts): accept source-valid inventory limitations
  • f8bf31c Merge commit '8fef2aa64e' into HEAD
  • 8a35929 style(artifacts): format mobile graph projection

📊 Changes

24 files changed (+1023 additions, -162 deletions)

View changed files

📝 docs/mcp-contracts.md (+25 -0)
📝 src/artifacts/ArtifactReader.ts (+31 -2)
📝 src/artifacts/DirectoryArtifactReader.ts (+33 -6)
📝 src/artifacts/NativeDmgArtifactReader.ts (+14 -2)
📝 src/artifacts/inventory/reader.ts (+42 -35)
📝 src/artifacts/inventory/scanCanonical.ts (+28 -9)
📝 src/artifacts/inventory/scanReader.ts (+37 -2)
📝 src/domain/android/androidApplication.ts (+3 -2)
📝 src/domain/apple/appleApplication.ts (+18 -12)
📝 src/domain/artifactComparison.ts (+53 -12)
📝 src/domain/artifactGraph.ts (+13 -6)
📝 src/domain/artifactInventoryEvidence.ts (+89 -31)
📝 src/domain/artifactInventorySnapshot.ts (+4 -1)
📝 src/domain/harmony/harmonyApplication.ts (+3 -2)
📝 src/domain/mobileApplicationGraphProjection.ts (+2 -1)
➕ tests/acceptance/applications/artifactComparisonCoverage.test.ts (+432 -0)
📝 tests/boundary/filesystem/android/application.test.ts (+4 -4)
📝 tests/boundary/filesystem/appleApplication.test.ts (+5 -5)
📝 tests/boundary/filesystem/artifactComparison.test.ts (+113 -5)
📝 tests/boundary/filesystem/artifactInventoryRegularRoot.test.ts (+19 -0)

...and 4 more files

📄 Description

Summary

Fixes #2173.

Complete, identical plist inventories were classified as unknown and created false artifact-coverage findings and residual unknowns. Declare inventory scope and traversal coverage as typed producer facts, then compare those facts independently of diagnostic wording.

Problem and expected behavior

A fully observed plist root has one occurrence and no child container entries. Equal complete inventories within the same scope should return unchanged, one unchanged occurrence and zero unknowns. Rewording an informational notice must not change that conclusion. Missing observations, unavailable hashes and integrity contradictions must retain their uncertainty or contradictory status.

Change and scope

  • The inventory producer declares coverage: {scope: "root" | "container-entries", status: "complete" | "unknown"}. Missing container readers and deliberately omitted filesystem entries yield unknown traversal coverage; per-occurrence hash certainty stays separate. Case-distinct entries retained by an archive remain complete while preserving case-insensitive materialization diagnostics.
  • Comparison consumes parsed coverage facts, requires matching scopes and complete graph members, and preserves diagnostic text. The display-text predicate and shared notice constant are removed.
  • Historical inventory inputs may omit coverage without injecting defaults into authenticated payloads. The inventory parser owns a conservative compatibility adapter for exact historical notices; unfamiliar prose remains unknown. Historical integrity notices are recognized only when their exact count matches structured contradiction records.
  • Readers report omissions through a per-iterator callback, preserving omission paths and entry kinds without opening FIFO/socket/device nodes. Native DMG traversal forwards those facts into its existing path namespace. Directory readers retain the existing descent-predicate calling convention.
  • Current producer output requires coverage. Pages must agree on their coverage declaration, including field omission versus presence. Document the distinction between graph equality, traversal coverage and runtime behavior.

Contract and boundary impact

  • Semantic owner and earliest changed stage: inventory producer owns scope/coverage; the inventory Evidence parser validates and normalizes it; comparison consumes those facts.
  • CLI and MCP/tool-catalog contract: additive nested inventory coverage field; current output guarantees it, historical input remains compatible. No new commands/tools.
  • Provider, bridge, target-format, or platform compatibility: shared artifact workflow; no engine protocol or bridge changes.
  • Evidence, artifact, provenance, or reconstruction contract: original text, identities, citations and graph commitments preserved. New coverage participates in Evidence identity; older nested payloads remain unchanged on import. Incompatible page declarations are rejected.
  • Process execution, authorization, cleanup, or containment impact: no new authority or lifecycle mechanism; tests reuse existing ownership helpers.
  • Generated metadata, package, or installation impact: the generated catalog reflects the additive output schema. No dependencies, package versions or generated files are committed.

Evidence and regression coverage

  • Original defect reproduced through compiled CLI-left/real stdio MCP-right on main 10400fd and 7ae3c50; both initial regression tests failed before repair.

  • Review regression: the previous patch at 487d129 still returned unknown for an equivalent reworded notice with declared complete root coverage. The added public regression failed before the typed-coverage repair.

  • Public workflow after repair: equal plist returns unchanged, one unchanged occurrence, zero unknowns and no false artifact-coverage finding/residual unknown. Missing process Evidence still leaves runtime behavior unknown.

  • Controls: content and POSIX executable-bit changes; equal ZIP; missing graph members; typed unknown coverage with empty prose; mismatched scopes; legacy field omission and unfamiliar/contradiction-looking prose; conflicting page declarations; unavailable PKG traversal; retained historical ASAR contradictions; nested Evidence identity and retained inventory views.

  • Review regressions on 51c3043: real FIFO/socket entries in an admitted application directory previously compared equal to a directory without them; equal ZIPs containing both A and a previously compared unknown. Both public CLI/MCP regressions failed before repair and pass after repair. A mounted-directory fixture also verifies DMG omission forwarding; its attachment host is mocked.

  • Independent review of follow-up: no actionable findings. Synthetic controls are labeled; they do not establish native-adapter or real-device behavior.

  • 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

On Node 24.18.0 / npm 11.16.0, macOS arm64, based on main 86de439:

  • Focused artifact, comparison, schema, MCP, retained-view, prompt and mobile projection checks: 159 tests passed across 23 files, including contract/catalog schemas, directory ownership/open/FIFO-race controls and native DMG lifecycle/diagnostics. After updating the DMG mock signature, all gates and the 30-test DMG suite passed again.
  • npm run check: typecheck, lint, formatting, knip, metadata, module boundaries and architecture guards passed; 911 lint warnings and zero errors.
  • npm run docs:check: passed generated documentation/catalog checks.
  • npm run verify:test-discovery: all 934 test files discovered exactly once.
  • Original public CLI/MCP reproduction replay: passed with declared root/complete coverage and no false findings or residual unknowns.
  • git diff --check: passed.
  • CI on reviewed head 51c3043: all required checks passed, including all six test shards and coverage/timing. CI for this updated head is separate and is reported by GitHub.
  • npm run verify:macos-bundle: directory/ZIP/real-DMG inventory assertions, CLI/MCP projection, native provenance and detach checks completed, but the overall command failed in the later dyld-environment fixture assertion (dyld-environment.mjs:183, missing library search path). Running that unchanged fixture in isolation reproduced the same failure without traversing the modified readers. This lane is not claimed as passed.

Compatibility, safety, and release

  • Breaking changes or migration: no caller migration; coverage is additive in current output and optional for historical input. Legacy pages remain readable, but mixing page declarations is refused.
  • Real Hopper/Ghidra, browser, or OS coverage: compiled CLI and real stdio MCP on macOS arm64, real hdiutil diagnostics and the partial real macOS bundle lane described above. POSIX special-entry and executable-bit controls are skipped on Windows.
  • Package or release metadata impact: no version or dependency changes.
  • Security/privacy/containment: no new access or execution authority. Missing members, unknown coverage, unavailable hashes and integrity mismatches stay conservative.
  • Limits: local follow-up validation did not run the full CI suite/coverage or Linux/Windows execution.

Review checklist

  • Concrete issue, one focused outcome, Conventional Commit title and linked issue.
  • Tests cover observable behavior and meaningful failure paths; documentation updated.
  • Representative CLI/MCP output and unavailable coverage reported.
  • Final diff checked 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/2214 **Author:** [@thangldw](https://github.com/thangldw) **Created:** 10/12/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `codex/fix-plist-inventory-comparison` --- ### 📝 Commits (10+) - [`487d129`](https://github.com/morluto/rea/commit/487d129ee030e99c78599c1e1e32e7baa385722a) fix(artifacts): preserve complete root inventory comparisons - [`51c3043`](https://github.com/morluto/rea/commit/51c3043ec97ad09651014003fc8037e9e35ac08b) fix(artifacts): declare inventory coverage before comparison - [`b94e10a`](https://github.com/morluto/rea/commit/b94e10a706c9ae029674ce6cd9fdfdad7934161e) fix(artifacts): preserve traversal omissions and materialization limits - [`17787e4`](https://github.com/morluto/rea/commit/17787e42180f493b6c3f9e9d38635cc999f0dc3b) fix(artifacts): reconcile scoped inventory coverage with main - [`4ff71fb`](https://github.com/morluto/rea/commit/4ff71fb63751eba9da5b3cd44c2833aa9e16fdb1) fix(artifacts): preserve coverage declarations and projection scope - [`dd371ce`](https://github.com/morluto/rea/commit/dd371ce6f48bb0f473a56f5c8c450a27f549e01e) Merge captured main into PR #2214 - [`4e36dc1`](https://github.com/morluto/rea/commit/4e36dc14753bf794c9f2d40d16ab5be6cb4ab51b) fix(artifacts): require entry coverage for child changes - [`7b51ddf`](https://github.com/morluto/rea/commit/7b51ddf561ebbcdd402465001caafdc2a5e3e441) fix(artifacts): accept source-valid inventory limitations - [`f8bf31c`](https://github.com/morluto/rea/commit/f8bf31ce3cac8be40319ecc22525a134c75fa678) Merge commit '8fef2aa64ec1ee742e8b70b825938d30a0ea7319' into HEAD - [`8a35929`](https://github.com/morluto/rea/commit/8a35929914dc549f1298b7c1d19acb1833c92985) style(artifacts): format mobile graph projection ### 📊 Changes **24 files changed** (+1023 additions, -162 deletions) <details> <summary>View changed files</summary> 📝 `docs/mcp-contracts.md` (+25 -0) 📝 `src/artifacts/ArtifactReader.ts` (+31 -2) 📝 `src/artifacts/DirectoryArtifactReader.ts` (+33 -6) 📝 `src/artifacts/NativeDmgArtifactReader.ts` (+14 -2) 📝 `src/artifacts/inventory/reader.ts` (+42 -35) 📝 `src/artifacts/inventory/scanCanonical.ts` (+28 -9) 📝 `src/artifacts/inventory/scanReader.ts` (+37 -2) 📝 `src/domain/android/androidApplication.ts` (+3 -2) 📝 `src/domain/apple/appleApplication.ts` (+18 -12) 📝 `src/domain/artifactComparison.ts` (+53 -12) 📝 `src/domain/artifactGraph.ts` (+13 -6) 📝 `src/domain/artifactInventoryEvidence.ts` (+89 -31) 📝 `src/domain/artifactInventorySnapshot.ts` (+4 -1) 📝 `src/domain/harmony/harmonyApplication.ts` (+3 -2) 📝 `src/domain/mobileApplicationGraphProjection.ts` (+2 -1) ➕ `tests/acceptance/applications/artifactComparisonCoverage.test.ts` (+432 -0) 📝 `tests/boundary/filesystem/android/application.test.ts` (+4 -4) 📝 `tests/boundary/filesystem/appleApplication.test.ts` (+5 -5) 📝 `tests/boundary/filesystem/artifactComparison.test.ts` (+113 -5) 📝 `tests/boundary/filesystem/artifactInventoryRegularRoot.test.ts` (+19 -0) _...and 4 more files_ </details> ### 📄 Description ## Summary Fixes #2173. Complete, identical plist inventories were classified as unknown and created false artifact-coverage findings and residual unknowns. Declare inventory scope and traversal coverage as typed producer facts, then compare those facts independently of diagnostic wording. ## Problem and expected behavior A fully observed plist root has one occurrence and no child container entries. Equal complete inventories within the same scope should return unchanged, one unchanged occurrence and zero unknowns. Rewording an informational notice must not change that conclusion. Missing observations, unavailable hashes and integrity contradictions must retain their uncertainty or contradictory status. ## Change and scope - The inventory producer declares `coverage: {scope: "root" | "container-entries", status: "complete" | "unknown"}`. Missing container readers and deliberately omitted filesystem entries yield unknown traversal coverage; per-occurrence hash certainty stays separate. Case-distinct entries retained by an archive remain complete while preserving case-insensitive materialization diagnostics. - Comparison consumes parsed coverage facts, requires matching scopes and complete graph members, and preserves diagnostic text. The display-text predicate and shared notice constant are removed. - Historical inventory inputs may omit coverage without injecting defaults into authenticated payloads. The inventory parser owns a conservative compatibility adapter for exact historical notices; unfamiliar prose remains unknown. Historical integrity notices are recognized only when their exact count matches structured contradiction records. - Readers report omissions through a per-iterator callback, preserving omission paths and entry kinds without opening FIFO/socket/device nodes. Native DMG traversal forwards those facts into its existing path namespace. Directory readers retain the existing descent-predicate calling convention. - Current producer output requires coverage. Pages must agree on their coverage declaration, including field omission versus presence. Document the distinction between graph equality, traversal coverage and runtime behavior. ## Contract and boundary impact - Semantic owner and earliest changed stage: inventory producer owns scope/coverage; the inventory Evidence parser validates and normalizes it; comparison consumes those facts. - CLI and MCP/tool-catalog contract: additive nested inventory coverage field; current output guarantees it, historical input remains compatible. No new commands/tools. - Provider, bridge, target-format, or platform compatibility: shared artifact workflow; no engine protocol or bridge changes. - Evidence, artifact, provenance, or reconstruction contract: original text, identities, citations and graph commitments preserved. New coverage participates in Evidence identity; older nested payloads remain unchanged on import. Incompatible page declarations are rejected. - Process execution, authorization, cleanup, or containment impact: no new authority or lifecycle mechanism; tests reuse existing ownership helpers. - Generated metadata, package, or installation impact: the generated catalog reflects the additive output schema. No dependencies, package versions or generated files are committed. ## Evidence and regression coverage - Original defect reproduced through compiled CLI-left/real stdio MCP-right on main `10400fd` and `7ae3c50`; both initial regression tests failed before repair. - Review regression: the previous patch at `487d129` still returned unknown for an equivalent reworded notice with declared complete root coverage. The added public regression failed before the typed-coverage repair. - Public workflow after repair: equal plist returns unchanged, one unchanged occurrence, zero unknowns and no false artifact-coverage finding/residual unknown. Missing process Evidence still leaves runtime behavior unknown. - Controls: content and POSIX executable-bit changes; equal ZIP; missing graph members; typed unknown coverage with empty prose; mismatched scopes; legacy field omission and unfamiliar/contradiction-looking prose; conflicting page declarations; unavailable PKG traversal; retained historical ASAR contradictions; nested Evidence identity and retained inventory views. - Review regressions on `51c3043`: real FIFO/socket entries in an admitted application directory previously compared equal to a directory without them; equal ZIPs containing both `A` and `a` previously compared unknown. Both public CLI/MCP regressions failed before repair and pass after repair. A mounted-directory fixture also verifies DMG omission forwarding; its attachment host is mocked. - Independent review of follow-up: no actionable findings. Synthetic controls are labeled; they do not establish native-adapter or real-device behavior. - [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 On Node 24.18.0 / npm 11.16.0, macOS arm64, based on main `86de439`: - Focused artifact, comparison, schema, MCP, retained-view, prompt and mobile projection checks: 159 tests passed across 23 files, including contract/catalog schemas, directory ownership/open/FIFO-race controls and native DMG lifecycle/diagnostics. After updating the DMG mock signature, all gates and the 30-test DMG suite passed again. - `npm run check`: typecheck, lint, formatting, knip, metadata, module boundaries and architecture guards passed; 911 lint warnings and zero errors. - `npm run docs:check`: passed generated documentation/catalog checks. - `npm run verify:test-discovery`: all 934 test files discovered exactly once. - Original public CLI/MCP reproduction replay: passed with declared root/complete coverage and no false findings or residual unknowns. - `git diff --check`: passed. - CI on reviewed head `51c3043`: all required checks passed, including all six test shards and coverage/timing. CI for this updated head is separate and is reported by GitHub. - `npm run verify:macos-bundle`: directory/ZIP/real-DMG inventory assertions, CLI/MCP projection, native provenance and detach checks completed, but the overall command failed in the later dyld-environment fixture assertion (`dyld-environment.mjs:183`, missing library search path). Running that unchanged fixture in isolation reproduced the same failure without traversing the modified readers. This lane is not claimed as passed. ## Compatibility, safety, and release - Breaking changes or migration: no caller migration; coverage is additive in current output and optional for historical input. Legacy pages remain readable, but mixing page declarations is refused. - Real Hopper/Ghidra, browser, or OS coverage: compiled CLI and real stdio MCP on macOS arm64, real hdiutil diagnostics and the partial real macOS bundle lane described above. POSIX special-entry and executable-bit controls are skipped on Windows. - Package or release metadata impact: no version or dependency changes. - Security/privacy/containment: no new access or execution authority. Missing members, unknown coverage, unavailable hashes and integrity mismatches stay conservative. - Limits: local follow-up validation did not run the full CI suite/coverage or Linux/Windows execution. ## Review checklist - [x] Concrete issue, one focused outcome, Conventional Commit title and linked issue. - [x] Tests cover observable behavior and meaningful failure paths; documentation updated. - [x] Representative CLI/MCP output and unavailable coverage reported. - [x] Final diff checked 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:03:45 +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#5088
No description provided.