[PR #5572] [CLOSED] fix(artifacts): keep DMG root observations when the image is already attached #6972

New issue

Closed

opened 2026-11-01 21:44:24 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:44:24 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5572
Author: @kaoru0822-kitauji
Created: 10/20/2026
Status: ❌ Closed

Base: main ← Head: fix/dmg-mounted-root-observations

📝 Commits (1)

790ecf8 fix(artifacts): keep DMG root observations when the image is already attached

📊 Changes

4 files changed (+274 additions, -29 deletions)

View changed files

📝 src/artifacts/NativeDmgArtifactReader.ts (+69 -29)
📝 src/artifacts/NativeDmgOwnership.ts (+1 -0)
➕ tests/acceptance/artifactDmgPreexistingAttachment.test.ts (+128 -0)
📝 tests/boundary/providers/native/nativeDmgArtifactReader.test.ts (+76 -0)

📄 Description

Summary

Fixes #5546. On macOS, inspect-artifact on a valid DMG that something else had already attached exited 1 with artifact_operation_failed / unavailable and advice to "check artifact support and required tools". The root identity and digest it had already computed were thrown away. Now it exits 0 with root inventory Evidence, container-entries coverage set to unknown, and a limitation that says another attachment blocked the read and the caller should release it and retry.

Problem and expected behavior

How to trigger it: attach a DMG read-only (hdiutil attach -readonly -nobrowse -mountroot <dir> image.dmg), then run rea inspect-artifact image.dmg --json. REA's own private hdiutil attach fails with Resource busy (exit 1). NativeDmgArtifactReader maps any numeric attach failure to unavailable, and scanCanonicalArtifactInventory passes traversal failures straight up. So the root observations made before traversal never reach the result.

The contract says inspect_artifact keeps root observations and states its limitations. Unknown child coverage should stay visible as unknown. A failed attach should not wipe out the whole result.

Change and scope

The fix is in the DMG reader, which already decides attach failures and mount ownership:

parseNativeDmgInfo now keeps the optional image-path that hdiutil info -plist reports. The reader already reads this baseline before attaching.
A numeric attach failure becomes "unknown coverage" only when all of these hold:
1. the baseline lists the same image path (compared with realpath on both sides);
2. the failure is an hdiutil exit, not a spawn failure or a cancellation;
3. ownership reconciliation finds nothing new mounted under the reader's own mount root ("none").
In that case entries() reports { status: "unknown", limitations: [...] } through the existing onCoverage hook (the Godot PCK reader uses the same hook) and yields no entries. The failed attach command and its exit code are kept in provenance.
Every other attach failure, including unknown ownership, is still thrown with the same typed error as before.
REA never traverses, borrows or detaches the existing attachment. Its own empty mount root is still removed on close.

Not included: the related DMG guidance issues #5547 and #5549, and any change to how the outer inspection coverage (complete-within-substeps) is derived. Contribution scope: this is an ordinary fix inside the existing read-only DMG boundary, so no maintainer scope decision is needed.

Contract and boundary impact

Semantic owner and earliest changed stage: NativeDmgArtifactReader (attach classification) and NativeDmgOwnership (parsing hdiutil info).
Contribution scope fit and linked maintainer decision: fits existing scope; no decision needed.
CLI and MCP/tool-catalog contract: no schema change. The shared inventory workflow now returns a result instead of an error in this one case.
Provider, bridge, target-format, or platform compatibility: macOS DMG only. Non-macOS behavior is unchanged.
Evidence, artifact, provenance, or reconstruction contract: root manifest and digest are kept. Coverage is unknown with an explicit limitation. Provenance includes the failed attach (exit code, effects: ["read"]).
Target selection, authority, upstream behavior, and cleanup details: the other owner's mount is left alone. REA's own mount root is cleaned up as before.
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/acceptance/artifactDmgPreexistingAttachment.test.ts (macOS only, real hdiutil, public CLI). It creates and attaches a UDZO DMG, runs inspect-artifact --json and checks: exit 0; root_sha256 equals the file's SHA-256; coverage is unknown; the limitation and the failed-attach provenance are present; the test's own attachment is still attached afterwards. It then detaches only the attachment the test created.
- tests/boundary/providers/native/nativeDmgArtifactReader.test.ts, "native DMG preexisting attachment" (simulated host, runs on every platform). It checks that unknown coverage is reported when the baseline lists the target path, and that the original unavailable failure is kept when the baseline lists a different image.
Base reproduction: before the fix I ran the issue's CLI script on this host. The busy run exited 1 with artifact_operation_failed and stderr hdiutil: attach failed - Resource busy. The detached control run exited 0. With the source fix reverted, both new tests fail (2 failed, 49 passed). With the fix, both pass.

User-visible CLI output after the fix (busy case, exit 0), inventory substep:

"coverage": { "scope": "container-entries", "status": "unknown" },
"limitations": ["DMG child inventory is unknown: hdiutil refused a private read-only attachment (exit 1) while it already listed an attachment of this image path. REA did not traverse, borrow or detach that attachment; release it and retry to inventory children."]

The manifest has root_format: "dmg", occurrence_count: 1. The same limitation appears in the top-level inspection limitations.

Remaining proof gaps:
- Not exercised: MCP adapter, a foreign attachment held by another app or another user, and images attached through a different path spelling that realpath does not normalize. The last case keeps the old typed failure.
- Path identity is a path match, not a byte-identity proof. The limitation wording only claims that hdiutil listed an attachment of this image path.

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

Issue reproduction script via node scripts/rea.mjs inspect-artifact (macOS, arm64) — before fix: busy exit 1, control exit 0. After fix: busy exit 0 with unknown coverage and limitation, control exit 0. hdiutil info shows no leftover fixture attachment.
npm run test:focused -- tests/boundary/providers/native/nativeDmgArtifactReader.test.ts tests/acceptance/artifactDmgPreexistingAttachment.test.ts — 2 files, 51 tests passed. With the source fix reverted: 2 failed, 49 passed.
npm run check:fast — passed (typecheck, lint).

Compatibility, safety, and release

Breaking changes or migration steps: none. One case that used to return an error now returns a successful result with unknown coverage.
Real Hopper/Ghidra, browser, or OS coverage: real macOS hdiutil only. No Hopper or Ghidra involved.
Package or release metadata impact: none.
Security, privacy, process, or containment review: no new mutation. REA never detaches or reads through a mount it does not own. Ownership reconciliation still runs before the downgrade.

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/5572 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/20/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/dmg-mounted-root-observations` --- ### 📝 Commits (1) - [`790ecf8`](https://github.com/morluto/rea/commit/790ecf825ade7606f7fe5f694715e550f37b2c23) fix(artifacts): keep DMG root observations when the image is already attached ### 📊 Changes **4 files changed** (+274 additions, -29 deletions) <details> <summary>View changed files</summary> 📝 `src/artifacts/NativeDmgArtifactReader.ts` (+69 -29) 📝 `src/artifacts/NativeDmgOwnership.ts` (+1 -0) ➕ `tests/acceptance/artifactDmgPreexistingAttachment.test.ts` (+128 -0) 📝 `tests/boundary/providers/native/nativeDmgArtifactReader.test.ts` (+76 -0) </details> ### 📄 Description ## Summary Fixes #5546. On macOS, `inspect-artifact` on a valid DMG that something else had already attached exited 1 with `artifact_operation_failed` / `unavailable` and advice to "check artifact support and required tools". The root identity and digest it had already computed were thrown away. Now it exits 0 with root inventory Evidence, `container-entries` coverage set to `unknown`, and a limitation that says another attachment blocked the read and the caller should release it and retry. ## Problem and expected behavior How to trigger it: attach a DMG read-only (`hdiutil attach -readonly -nobrowse -mountroot <dir> image.dmg`), then run `rea inspect-artifact image.dmg --json`. REA's own private `hdiutil attach` fails with `Resource busy` (exit 1). `NativeDmgArtifactReader` maps any numeric attach failure to `unavailable`, and `scanCanonicalArtifactInventory` passes traversal failures straight up. So the root observations made before traversal never reach the result. The contract says `inspect_artifact` keeps root observations and states its limitations. Unknown child coverage should stay visible as unknown. A failed attach should not wipe out the whole result. ## Change and scope The fix is in the DMG reader, which already decides attach failures and mount ownership: - `parseNativeDmgInfo` now keeps the optional `image-path` that `hdiutil info -plist` reports. The reader already reads this baseline before attaching. - A numeric attach failure becomes "unknown coverage" only when all of these hold: 1. the baseline lists the same image path (compared with `realpath` on both sides); 2. the failure is an hdiutil exit, not a spawn failure or a cancellation; 3. ownership reconciliation finds nothing new mounted under the reader's own mount root (`"none"`). - In that case `entries()` reports `{ status: "unknown", limitations: [...] }` through the existing `onCoverage` hook (the Godot PCK reader uses the same hook) and yields no entries. The failed attach command and its exit code are kept in provenance. - Every other attach failure, including unknown ownership, is still thrown with the same typed error as before. - REA never traverses, borrows or detaches the existing attachment. Its own empty mount root is still removed on close. Not included: the related DMG guidance issues #5547 and #5549, and any change to how the outer inspection coverage (`complete-within-substeps`) is derived. Contribution scope: this is an ordinary fix inside the existing read-only DMG boundary, so no maintainer scope decision is needed. ## Contract and boundary impact - Semantic owner and earliest changed stage: `NativeDmgArtifactReader` (attach classification) and `NativeDmgOwnership` (parsing `hdiutil info`). - Contribution scope fit and linked maintainer decision: fits existing scope; no decision needed. - CLI and MCP/tool-catalog contract: no schema change. The shared inventory workflow now returns a result instead of an error in this one case. - Provider, bridge, target-format, or platform compatibility: macOS DMG only. Non-macOS behavior is unchanged. - Evidence, artifact, provenance, or reconstruction contract: root manifest and digest are kept. Coverage is `unknown` with an explicit limitation. Provenance includes the failed attach (exit code, `effects: ["read"]`). - Target selection, authority, upstream behavior, and cleanup details: the other owner's mount is left alone. REA's own mount root is cleaned up as before. - 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/acceptance/artifactDmgPreexistingAttachment.test.ts` (macOS only, real `hdiutil`, public CLI). It creates and attaches a UDZO DMG, runs `inspect-artifact --json` and checks: exit 0; `root_sha256` equals the file's SHA-256; coverage is `unknown`; the limitation and the failed-attach provenance are present; the test's own attachment is still attached afterwards. It then detaches only the attachment the test created. - `tests/boundary/providers/native/nativeDmgArtifactReader.test.ts`, "native DMG preexisting attachment" (simulated host, runs on every platform). It checks that unknown coverage is reported when the baseline lists the target path, and that the original `unavailable` failure is kept when the baseline lists a different image. - Base reproduction: before the fix I ran the issue's CLI script on this host. The busy run exited 1 with `artifact_operation_failed` and stderr `hdiutil: attach failed - Resource busy`. The detached control run exited 0. With the source fix reverted, both new tests fail (2 failed, 49 passed). With the fix, both pass. - User-visible CLI output after the fix (busy case, exit 0), inventory substep: ```json "coverage": { "scope": "container-entries", "status": "unknown" }, "limitations": ["DMG child inventory is unknown: hdiutil refused a private read-only attachment (exit 1) while it already listed an attachment of this image path. REA did not traverse, borrow or detach that attachment; release it and retry to inventory children."] ``` The manifest has `root_format: "dmg"`, `occurrence_count: 1`. The same limitation appears in the top-level inspection `limitations`. - Remaining proof gaps: - Not exercised: MCP adapter, a foreign attachment held by another app or another user, and images attached through a different path spelling that `realpath` does not normalize. The last case keeps the old typed failure. - Path identity is a path match, not a byte-identity proof. The limitation wording only claims that hdiutil listed an attachment of this image path. 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 - Issue reproduction script via `node scripts/rea.mjs inspect-artifact` (macOS, arm64) — before fix: busy exit 1, control exit 0. After fix: busy exit 0 with unknown coverage and limitation, control exit 0. `hdiutil info` shows no leftover fixture attachment. - `npm run test:focused -- tests/boundary/providers/native/nativeDmgArtifactReader.test.ts tests/acceptance/artifactDmgPreexistingAttachment.test.ts` — 2 files, 51 tests passed. With the source fix reverted: 2 failed, 49 passed. - `npm run check:fast` — passed (typecheck, lint). ## Compatibility, safety, and release - Breaking changes or migration steps: none. One case that used to return an error now returns a successful result with unknown coverage. - Real Hopper/Ghidra, browser, or OS coverage: real macOS `hdiutil` only. No Hopper or Ghidra involved. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new mutation. REA never detaches or reads through a mount it does not own. Ownership reconciliation still runs before the downgrade. ## 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:44:24 +00:00