[PR #1070] [MERGED] fix(artifacts): report keyed-archive selection mistakes as invalid input #2619

New issue

Closed

opened 2026-10-17 21:08:17 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-17 21:08:17 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/keyed-archive-selection-errors

📝 Commits (1)

25fb6d3 fix(artifacts): report keyed-archive selection mistakes as invalid input

📊 Changes

6 files changed (+195 additions, -17 deletions)

View changed files

📝 src/artifacts/ArtifactProvider.ts (+15 -5)
📝 src/artifacts/apple/KeyedArchiveReader.test.ts (+11 -1)
📝 src/artifacts/apple/KeyedArchiveReader.ts (+27 -10)
📝 src/domain/apple/keyedArchive.ts (+9 -1)
➕ tests/boundary/filesystem/keyedArchiveSelection.test.ts (+99 -0)
📝 tests/boundary/mcp/artifactMcp.inlineInventory.test.ts (+34 -0)

📄 Description

Problem

inspect_keyed_archive reported mistakes in the caller's selection as faults in the artifact. Each case gave recovery advice that did not apply:

Caller input	Before
`root: "missing"`	`artifact_operation_failed` / `format`: "Artifact is invalid or has changed. Get a fresh copy and try again." The detail was only "Cannot decode selected Foundation keyed archive", so the "root does not exist" reason was lost.
`path` absent from the bundle, non-canonical (`Contents/../x`), `.` on an app bundle, or a directory	`artifact_operation_failed` / `path`: "Artifact contains an unsafe or conflicting internal path … correct the artifact before retrying."
A `path` other than the active plist's basename	Same `path` failure
A plain plist or a compiled `NIBArchive`	The decoder's reason was replaced by the generic "Cannot decode" text

Reproduced on main with a real app (rea inspect-keyed-archive /Applications/Transmission.app Contents/Resources/Nope.plist) and with a synthetic XML keyed archive (--root nope).

Change

A missing named root is now an invalid_request issue on root, with expected listing the archive's $top roots. The check lives in the domain projection.
A missing, non-canonical, ., or non-regular-file archive path is now an invalid_request issue on path. A mismatched path for an active plist also lists its basename in expected. This matches how inspect_plist already reports a missing relative path.
ArtifactProvider.translateFailure passes AnalysisInputError through instead of turning it into a bare io failure.
Decode failures keep reason: "format" or "limit", and the detail now includes the decoder's message. A compiled NIBArchive gets a detail that points to decode_interface_builder.

Sanitized MCP result for inspect_keyed_archive {"root":"missing"}:

{"error":{"code":"invalid_request","category":"invalid_input","details":{"operation":"inspect_keyed_archive","issues":[{"path":["root"],"reason":"invalid_value","message":"Archive root does not exist: missing. Select one of the archive's $top roots or omit root.","expected":["root"]}]}}}

No input or output schemas change.

Verification

npm run test:focused -- src/artifacts/apple/KeyedArchiveReader.test.ts tests/boundary/filesystem/keyedArchiveSelection.test.ts tests/boundary/mcp/artifactMcp.inlineInventory.test.ts tests/boundary/providers/native/keyedArchiveEncoding.test.ts tests/boundary/filesystem/nativeArtifactGoldens.test.ts: 25 passed. With the source changes reverted, the new and updated assertions fail (10 failures).
npm run check:fast
Ran the CLI against /Applications/Transmission.app and a temporary keyed archive for each case in the table.

No real Hopper or Ghidra verification was run, because this change does not touch those providers.

🤖 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/1070 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/8/2026 **Status:** ✅ Merged **Merged:** 10/8/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/keyed-archive-selection-errors` --- ### 📝 Commits (1) - [`25fb6d3`](https://github.com/morluto/rea/commit/25fb6d32a325d88bb0f4e05e16f5f82e4f91c5ca) fix(artifacts): report keyed-archive selection mistakes as invalid input ### 📊 Changes **6 files changed** (+195 additions, -17 deletions) <details> <summary>View changed files</summary> 📝 `src/artifacts/ArtifactProvider.ts` (+15 -5) 📝 `src/artifacts/apple/KeyedArchiveReader.test.ts` (+11 -1) 📝 `src/artifacts/apple/KeyedArchiveReader.ts` (+27 -10) 📝 `src/domain/apple/keyedArchive.ts` (+9 -1) ➕ `tests/boundary/filesystem/keyedArchiveSelection.test.ts` (+99 -0) 📝 `tests/boundary/mcp/artifactMcp.inlineInventory.test.ts` (+34 -0) </details> ### 📄 Description ## Problem `inspect_keyed_archive` reported mistakes in the caller's selection as faults in the artifact. Each case gave recovery advice that did not apply: | Caller input | Before | | --- | --- | | `root: "missing"` | `artifact_operation_failed` / `format`: "Artifact is invalid or has changed. Get a fresh copy and try again." The detail was only "Cannot decode selected Foundation keyed archive", so the "root does not exist" reason was lost. | | `path` absent from the bundle, non-canonical (`Contents/../x`), `.` on an app bundle, or a directory | `artifact_operation_failed` / `path`: "Artifact contains an unsafe or conflicting internal path … correct the artifact before retrying." | | A `path` other than the active plist's basename | Same `path` failure | | A plain plist or a compiled `NIBArchive` | The decoder's reason was replaced by the generic "Cannot decode" text | Reproduced on `main` with a real app (`rea inspect-keyed-archive /Applications/Transmission.app Contents/Resources/Nope.plist`) and with a synthetic XML keyed archive (`--root nope`). ## Change - A missing named root is now an `invalid_request` issue on `root`, with `expected` listing the archive's `$top` roots. The check lives in the domain projection. - A missing, non-canonical, `.`, or non-regular-file archive path is now an `invalid_request` issue on `path`. A mismatched path for an active plist also lists its basename in `expected`. This matches how `inspect_plist` already reports a missing relative path. - `ArtifactProvider.translateFailure` passes `AnalysisInputError` through instead of turning it into a bare `io` failure. - Decode failures keep `reason: "format"` or `"limit"`, and the detail now includes the decoder's message. A compiled `NIBArchive` gets a detail that points to `decode_interface_builder`. Sanitized MCP result for `inspect_keyed_archive {"root":"missing"}`: ```json {"error":{"code":"invalid_request","category":"invalid_input","details":{"operation":"inspect_keyed_archive","issues":[{"path":["root"],"reason":"invalid_value","message":"Archive root does not exist: missing. Select one of the archive's $top roots or omit root.","expected":["root"]}]}}} ``` No input or output schemas change. ## Verification - `npm run test:focused -- src/artifacts/apple/KeyedArchiveReader.test.ts tests/boundary/filesystem/keyedArchiveSelection.test.ts tests/boundary/mcp/artifactMcp.inlineInventory.test.ts tests/boundary/providers/native/keyedArchiveEncoding.test.ts tests/boundary/filesystem/nativeArtifactGoldens.test.ts`: 25 passed. With the source changes reverted, the new and updated assertions fail (10 failures). - `npm run check:fast` - Ran the CLI against `/Applications/Transmission.app` and a temporary keyed archive for each case in the table. No real Hopper or Ghidra verification was run, because this change does not touch those providers. 🤖 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-10-17 21:08:17 +00:00