[PR #7177] feat(artifacts): add dry-run plan for artifact extraction #7694

Open
opened 2026-11-03 13:39:43 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/7177
Author: @hannesstrydom664-hash
Created: 10/25/2026
Status: 🔄 Open

Base: main ← Head: feat/artifact-extraction-dry-run-5489


📝 Commits (10+)

  • 777b04a feat(artifacts): add dry-run extraction plan without destination writes (#5489)
  • 47f478c fix(artifacts): address dry-run extraction plan review feedback (#5489)
  • 93c3ab3 fix(artifacts): preserve unknown mount case sensitivity and use destination case folding in dry-run (#5489)
  • 74029e0 fix(artifacts): handle hardlinked entries and leave non-ASCII case folds unresolved in dry-run (#5489)
  • 77f9884 fix(artifacts): bound case-sensitivity probes and observe Windows directories (#5489)
  • 545c4e3 fix(artifacts): verify dry-run directory freshness, over-limit paths, and case-probe close ownership
  • 20ae0c4 fix(artifacts): resolve canonical output path limits and full Unicode case folds in dry-run plan
  • a0bf542 fix(artifacts): revalidate unpacked ASAR companions and qualify unresolved collision limitation in dry-run
  • 8f417b4 fix(artifacts): reconcile non-file entries against the inventory snapshot in dry-run
  • 041f0ef fix(artifacts): scope non-file reconciliation to active reader and verify entry kinds

📊 Changes

17 files changed (+3916 additions, -134 deletions)

View changed files

📝 docs/mcp-contracts.md (+18 -0)
📝 docs/node-sea.md (+1 -0)
📝 src/application/environment/environmentBundleTool.ts (+18 -0)
📝 src/artifacts/ArtifactPaths.test.ts (+55 -0)
📝 src/artifacts/ArtifactPaths.ts (+99 -20)
📝 src/artifacts/ArtifactProvider.ts (+8 -0)
📝 src/artifacts/SafeOutputTree.ts (+180 -19)
➕ src/artifacts/extraction/ArtifactExtraction.sourceGuard.test.ts (+128 -0)
📝 src/artifacts/extraction/ArtifactExtraction.test.ts (+1521 -3)
📝 src/artifacts/extraction/ArtifactExtraction.ts (+1071 -58)
➕ src/artifacts/extraction/DryRunPlanCache.ts (+383 -0)
📝 src/cli/artifactCommands.ts (+65 -4)
📝 src/contracts/artifactToolContracts.ts (+124 -22)
📝 src/domain/artifactGraph.ts (+58 -0)
📝 src/firmware/FirmwareFiles.ts (+5 -5)
📝 src/firmware/FirmwareProvider.ts (+3 -3)
📝 tests/boundary/providers/native/artifactProvider.extraction.test.ts (+179 -0)

📄 Description

Summary

Fixes #5489

Adds a dry-run planning mode (dry_run: true / --dry-run) to extract_artifact so agents and operators can inspect what an extraction will materialize, verify destination admission and path-collision rules, and review labeled byte estimates before touching disk.

Problem and expected behavior

  • Smallest trigger: Call extract_artifact (or CLI rea extract-artifact) when evaluating an archive or application package before deciding whether to materialize files on disk.
  • Violated invariant: Previously, extract_artifact always staged and moved extracted entries into output_root, providing no dry-run mode to validate destination path safety (Windows/POSIX reserved characters, NFC collisions, prefix collisions) and inspect planned vs. unsupported/unresolved entries without writing to disk.
  • Expected behavior: When dry_run: true (or --dry-run) is passed, extract_artifact validates output_root admission via SafeOutputTree.checkDestination(input.outputRoot), traverses selected entries using the same normalizeDestinationPath and ArtifactPathRegistry rules as real extraction, creates zero destination directories/files or sidecar manifests, and returns a structured plan alongside dry_run: true.

Change and scope

Follow the contribution scope.
Scoped enhancement implementing issue #5489 (feat(artifacts): add dry-run plan for artifact extraction).

  • src/domain/artifactGraph.ts: Add artifactExtractionPlanEntrySchema, artifactExtractionPlanTotalsSchema, and artifactExtractionPlanSchema, plus dry_run and plan fields on artifactExtractionResultSchema. Each non-root inventory occurrence is classified as "planned", "unsupported" (directories, logical metadata declarations, or nested archive members not expanded during single-container extraction), or "unresolved" (symlinks), with byte totals explicitly labeled via byte_totals_basis: "declared-and-observed-content-estimates" and byte_totals_note.
  • src/artifacts/SafeOutputTree.ts & src/artifacts/extraction/ArtifactExtraction.ts: Export normalizeDestinationPath(relativePath, platform) so dry-run planning applies the exact same Windows/POSIX destination path rules (\, :, U+0001-U+001F, NFC normalization, and ArtifactPathRegistry prefix/collision checks) as SafeOutputTree.prototype.write(). When dryRun: true, extractArtifact validates the destination via SafeOutputTree.checkDestination(input.outputRoot) and walks reader.entries(signal, traversal) via planSelectedEntries() without creating outputRoot, staging directories, or sidecar files.
  • src/contracts/artifactToolContracts.ts, src/cli/artifactCommands.ts, src/artifacts/ArtifactProvider.ts: Add dry_run?: boolean to artifactExtractionInputSchema and --dry-run to the CLI extract-artifact command, forwarding dryRun and platform through ArtifactProvider.
  • src/artifacts/extraction/ArtifactExtraction.test.ts & tests/boundary/providers/native/artifactProvider.extraction.test.ts: Add unit and provider boundary tests covering dry-run plans, zero destination writes, and parity of path-collision/invalid-name refusals between dry-run and real extraction.

Contract and boundary impact

  • Semantic owner and earliest changed stage: src/domain/artifactGraph.ts and src/artifacts/extraction/ArtifactExtraction.ts.
  • Contribution scope fit and linked maintainer decision: Implements #5489 (feat(artifacts): add dry-run plan for artifact extraction).
  • CLI and MCP/tool-catalog contract: Adds optional dry_run?: boolean to extract_artifact input and --dry-run to rea extract-artifact, and optional dry_run?: boolean + plan?: ArtifactExtractionPlan to artifactExtractionResultSchema.
  • Provider, bridge, target-format, or platform compatibility: Preserves platform-specific destination validation (win32 vs POSIX) across both dry-run and real extraction.
  • Evidence, artifact, provenance, or reconstruction contract: Dry-run results report created: false and bytes_written: 0 on selected entries while preserving verified inventory artifact_id and sha256 values and explicitly labeling byte estimates.
  • Target selection, authority, upstream behavior, and cleanup details (if applicable): SafeOutputTree.checkDestination runs before planning, and no staging directory or destination root is created when dryRun: true.
  • Generated metadata (docs/public/product-catalog.json), package, or installation impact: Updated generated catalog metadata if applicable.

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.

Evidence and regression coverage

  • Tests added or updated:
    • src/artifacts/extraction/ArtifactExtraction.test.ts
    • tests/boundary/providers/native/artifactProvider.extraction.test.ts
  • Base reproduction or other evidence:
    • Executed: Verified on safe archive fixtures containing regular files, directories, and symlinks that dryRun: true produces plan.entries (planned, unsupported, unresolved) and plan.totals while leaving output_root nonexistent on disk, and rejects NFC path collisions and Windows : ADS names identically to dryRun: false.
  • User-visible CLI/MCP output (if applicable):
    • extract_artifact with dry_run: true returns dry_run: true and plan: { destination_root, entries, totals: { planned_files, unsupported_entries, unresolved_links, observed_bytes_estimate, declared_bytes_estimate, byte_totals_basis, byte_totals_note } }.
  • Remaining proof gaps: None.

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

  • npx vitest run src/artifacts/extraction/ArtifactExtraction.test.ts tests/boundary/providers/native/artifactProvider.extraction.test.ts — passed (39/39 tests).
  • npm run typecheck — passed (tsc --noEmit, 0 errors).
  • npm run format:check — passed (oxfmt --check .).
  • npm run lint — passed (oxlint 0 warnings/0 errors; verify:module-boundaries passed).
  • npm run verify:test-discovery — passed.
  • npm run verify:architecture-guards — passed.
  • npm run scan:todos — passed.
  • npm run jscpd — passed.

Compatibility, safety, and release

  • Breaking changes or migration steps: None (dry_run is optional and defaults to false).
  • Real Hopper/Ghidra, browser, or OS coverage: Tested with both POSIX and win32 platform normalization rules in ArtifactExtraction.test.ts.
  • Package or release metadata impact: None.
  • Security, privacy, process, or containment review: Ensures dry-run never writes to or creates output_root while enforcing identical path-traversal and collision checks.

Author readiness

Human owner: @hannesstrydom664-hash

  • 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/7177 **Author:** [@hannesstrydom664-hash](https://github.com/hannesstrydom664-hash) **Created:** 10/25/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `feat/artifact-extraction-dry-run-5489` --- ### 📝 Commits (10+) - [`777b04a`](https://github.com/morluto/rea/commit/777b04a6eb016b39ba0b9c310cac2c16668a8174) feat(artifacts): add dry-run extraction plan without destination writes (#5489) - [`47f478c`](https://github.com/morluto/rea/commit/47f478c625e52cd4f7814acb251c954ceef51717) fix(artifacts): address dry-run extraction plan review feedback (#5489) - [`93c3ab3`](https://github.com/morluto/rea/commit/93c3ab39534cdeb61e3359d0cb0963e42b80cb5a) fix(artifacts): preserve unknown mount case sensitivity and use destination case folding in dry-run (#5489) - [`74029e0`](https://github.com/morluto/rea/commit/74029e0f77de522f450b7078f1b902fd582f89f4) fix(artifacts): handle hardlinked entries and leave non-ASCII case folds unresolved in dry-run (#5489) - [`77f9884`](https://github.com/morluto/rea/commit/77f98841b2c931597e48b88bc3fd262604046c42) fix(artifacts): bound case-sensitivity probes and observe Windows directories (#5489) - [`545c4e3`](https://github.com/morluto/rea/commit/545c4e3e70149edb29e6d9f6a8d53d8e31005298) fix(artifacts): verify dry-run directory freshness, over-limit paths, and case-probe close ownership - [`20ae0c4`](https://github.com/morluto/rea/commit/20ae0c47c010736af97dd55f0694d537a8d0d620) fix(artifacts): resolve canonical output path limits and full Unicode case folds in dry-run plan - [`a0bf542`](https://github.com/morluto/rea/commit/a0bf542f77136c5b7d672990fbd84049e977c3eb) fix(artifacts): revalidate unpacked ASAR companions and qualify unresolved collision limitation in dry-run - [`8f417b4`](https://github.com/morluto/rea/commit/8f417b44a03871b057a33107428cfc1b25b73319) fix(artifacts): reconcile non-file entries against the inventory snapshot in dry-run - [`041f0ef`](https://github.com/morluto/rea/commit/041f0efa4d820c99f6297f5487b99e91c7034ed0) fix(artifacts): scope non-file reconciliation to active reader and verify entry kinds ### 📊 Changes **17 files changed** (+3916 additions, -134 deletions) <details> <summary>View changed files</summary> 📝 `docs/mcp-contracts.md` (+18 -0) 📝 `docs/node-sea.md` (+1 -0) 📝 `src/application/environment/environmentBundleTool.ts` (+18 -0) 📝 `src/artifacts/ArtifactPaths.test.ts` (+55 -0) 📝 `src/artifacts/ArtifactPaths.ts` (+99 -20) 📝 `src/artifacts/ArtifactProvider.ts` (+8 -0) 📝 `src/artifacts/SafeOutputTree.ts` (+180 -19) ➕ `src/artifacts/extraction/ArtifactExtraction.sourceGuard.test.ts` (+128 -0) 📝 `src/artifacts/extraction/ArtifactExtraction.test.ts` (+1521 -3) 📝 `src/artifacts/extraction/ArtifactExtraction.ts` (+1071 -58) ➕ `src/artifacts/extraction/DryRunPlanCache.ts` (+383 -0) 📝 `src/cli/artifactCommands.ts` (+65 -4) 📝 `src/contracts/artifactToolContracts.ts` (+124 -22) 📝 `src/domain/artifactGraph.ts` (+58 -0) 📝 `src/firmware/FirmwareFiles.ts` (+5 -5) 📝 `src/firmware/FirmwareProvider.ts` (+3 -3) 📝 `tests/boundary/providers/native/artifactProvider.extraction.test.ts` (+179 -0) </details> ### 📄 Description ## Summary Fixes #5489 Adds a dry-run planning mode (`dry_run: true` / `--dry-run`) to `extract_artifact` so agents and operators can inspect what an extraction will materialize, verify destination admission and path-collision rules, and review labeled byte estimates before touching disk. ## Problem and expected behavior - **Smallest trigger:** Call `extract_artifact` (or CLI `rea extract-artifact`) when evaluating an archive or application package before deciding whether to materialize files on disk. - **Violated invariant:** Previously, `extract_artifact` always staged and moved extracted entries into `output_root`, providing no dry-run mode to validate destination path safety (Windows/POSIX reserved characters, NFC collisions, prefix collisions) and inspect planned vs. unsupported/unresolved entries without writing to disk. - **Expected behavior:** When `dry_run: true` (or `--dry-run`) is passed, `extract_artifact` validates `output_root` admission via `SafeOutputTree.checkDestination(input.outputRoot)`, traverses selected entries using the same `normalizeDestinationPath` and `ArtifactPathRegistry` rules as real extraction, creates zero destination directories/files or sidecar manifests, and returns a structured `plan` alongside `dry_run: true`. ## Change and scope Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). Scoped enhancement implementing issue #5489 (`feat(artifacts): add dry-run plan for artifact extraction`). - `src/domain/artifactGraph.ts`: Add `artifactExtractionPlanEntrySchema`, `artifactExtractionPlanTotalsSchema`, and `artifactExtractionPlanSchema`, plus `dry_run` and `plan` fields on `artifactExtractionResultSchema`. Each non-root inventory occurrence is classified as `"planned"`, `"unsupported"` (directories, logical metadata declarations, or nested archive members not expanded during single-container extraction), or `"unresolved"` (symlinks), with byte totals explicitly labeled via `byte_totals_basis: "declared-and-observed-content-estimates"` and `byte_totals_note`. - `src/artifacts/SafeOutputTree.ts` & `src/artifacts/extraction/ArtifactExtraction.ts`: Export `normalizeDestinationPath(relativePath, platform)` so dry-run planning applies the exact same Windows/POSIX destination path rules (`\`, `:`, `U+0001-U+001F`, NFC normalization, and `ArtifactPathRegistry` prefix/collision checks) as `SafeOutputTree.prototype.write()`. When `dryRun: true`, `extractArtifact` validates the destination via `SafeOutputTree.checkDestination(input.outputRoot)` and walks `reader.entries(signal, traversal)` via `planSelectedEntries()` without creating `outputRoot`, staging directories, or sidecar files. - `src/contracts/artifactToolContracts.ts`, `src/cli/artifactCommands.ts`, `src/artifacts/ArtifactProvider.ts`: Add `dry_run?: boolean` to `artifactExtractionInputSchema` and `--dry-run` to the CLI `extract-artifact` command, forwarding `dryRun` and `platform` through `ArtifactProvider`. - `src/artifacts/extraction/ArtifactExtraction.test.ts` & `tests/boundary/providers/native/artifactProvider.extraction.test.ts`: Add unit and provider boundary tests covering dry-run plans, zero destination writes, and parity of path-collision/invalid-name refusals between dry-run and real extraction. ## Contract and boundary impact - Semantic owner and earliest changed stage: `src/domain/artifactGraph.ts` and `src/artifacts/extraction/ArtifactExtraction.ts`. - Contribution scope fit and linked maintainer decision: Implements #5489 (`feat(artifacts): add dry-run plan for artifact extraction`). - CLI and MCP/tool-catalog contract: Adds optional `dry_run?: boolean` to `extract_artifact` input and `--dry-run` to `rea extract-artifact`, and optional `dry_run?: boolean` + `plan?: ArtifactExtractionPlan` to `artifactExtractionResultSchema`. - Provider, bridge, target-format, or platform compatibility: Preserves platform-specific destination validation (`win32` vs POSIX) across both dry-run and real extraction. - Evidence, artifact, provenance, or reconstruction contract: Dry-run results report `created: false` and `bytes_written: 0` on selected entries while preserving verified inventory `artifact_id` and `sha256` values and explicitly labeling byte estimates. - Target selection, authority, upstream behavior, and cleanup details (if applicable): `SafeOutputTree.checkDestination` runs before planning, and no staging directory or destination root is created when `dryRun: true`. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: Updated generated catalog metadata if applicable. 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. - [ ] 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: - `src/artifacts/extraction/ArtifactExtraction.test.ts` - `tests/boundary/providers/native/artifactProvider.extraction.test.ts` - Base reproduction or other evidence: - Executed: Verified on safe archive fixtures containing regular files, directories, and symlinks that `dryRun: true` produces `plan.entries` (`planned`, `unsupported`, `unresolved`) and `plan.totals` while leaving `output_root` nonexistent on disk, and rejects NFC path collisions and Windows `:` ADS names identically to `dryRun: false`. - User-visible CLI/MCP output (if applicable): - `extract_artifact` with `dry_run: true` returns `dry_run: true` and `plan: { destination_root, entries, totals: { planned_files, unsupported_entries, unresolved_links, observed_bytes_estimate, declared_bytes_estimate, byte_totals_basis, byte_totals_note } }`. - Remaining proof gaps: None. 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 - `npx vitest run src/artifacts/extraction/ArtifactExtraction.test.ts tests/boundary/providers/native/artifactProvider.extraction.test.ts` — passed (`39/39` tests). - `npm run typecheck` — passed (`tsc --noEmit`, 0 errors). - `npm run format:check` — passed (`oxfmt --check .`). - `npm run lint` — passed (`oxlint` 0 warnings/0 errors; `verify:module-boundaries` passed). - `npm run verify:test-discovery` — passed. - `npm run verify:architecture-guards` — passed. - `npm run scan:todos` — passed. - `npm run jscpd` — passed. ## Compatibility, safety, and release - Breaking changes or migration steps: None (`dry_run` is optional and defaults to `false`). - Real Hopper/Ghidra, browser, or OS coverage: Tested with both POSIX and `win32` platform normalization rules in `ArtifactExtraction.test.ts`. - Package or release metadata impact: None. - Security, privacy, process, or containment review: Ensures dry-run never writes to or creates `output_root` while enforcing identical path-traversal and collision checks. ## Author readiness Human owner: @hannesstrydom664-hash - [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 - [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>
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#7694
No description provided.