[PR #7221] [MERGED] fix(artifacts): explain recovery for existing DMG attachments #7725

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

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/7221
Author: @hichipli
Created: 10/26/2026
Status: ✅ Merged
Merged: 10/26/2026
Merged by: @gensokyo-dev

Base: main ← Head: fix/dmg-attachment-recovery


📝 Commits (10+)

  • b533ab1 fix(artifacts): explain recovery for existing DMG attachments
  • 67f8585 test(artifacts): run DMG recovery in macOS CI
  • ab44487 fix(adb): preserve process names and pin fixture interpreters
  • d27e3f8 fix(native): preserve import names across dyld text formats
  • 13fba27 refactor(plist): centralize binary layout and offset ownership
  • 5508b2a fix(analysis): retain source evidence through directory exports (#7250)
  • f955e1a test(artifacts): check all DMG attachments after retry
  • 8423c5c refactor(macho): consolidate prefix interpretation across readers
  • 2cdc8cb refactor(managed): centralize TypeDef row facts across consumers
  • 1d32eb3 fix(providers): keep operation environments consistent through launch

📊 Changes

351 files changed (+18837 additions, -3876 deletions)

View changed files

📝 .agents/skills/reverse-engineer-anything/references/javascript-applications.md (+9 -0)
📝 .github/workflows/ci.yml (+3 -0)
📝 docs/client-configuration.md (+4 -2)
📝 docs/firmware-analysis.md (+1 -1)
📝 docs/ghidra-dos.md (+1 -1)
📝 docs/installation.md (+6 -4)
📝 docs/javascript-route-inventory.md (+36 -4)
📝 docs/native-investigation.md (+1 -1)
📝 docs/testing.md (+1 -1)
📝 docs/verification/native-decompile-directory-2026-10-23.md (+4 -0)
➕ docs/verification/web-bundle-paging-2026-10-26.md (+138 -0)
📝 docs/windows-ghidra-p0.md (+1 -1)
➕ experiments/analysis-efficiency/decompile-directory.md (+149 -0)
➕ experiments/analysis-efficiency/decompile-directory.mjs (+328 -0)
➕ experiments/analysis-efficiency/web-bundle-paging.mjs (+382 -0)
📝 scripts/ci/scopes.mjs (+4 -0)
📝 scripts/lib/browser-network-e2e.mjs (+11 -6)
📝 scripts/lib/browser-popup-e2e.mjs (+5 -4)
📝 scripts/lib/browser-screenshot-e2e.mjs (+8 -4)
📝 scripts/lib/mcp-verifier-results.mjs (+2 -1)

...and 80 more files

📄 Description

Summary

Fixes #5546’s remaining recovery-guidance gap. When a numeric unavailable hdiutil attach failure follows an inventory that lists the selected resolved image path and a device group, explain that the attachment’s owner can release it when safe before retrying.

The native adapter owns this positive path-match fact. It does not assert byte identity, attachment ownership, causality or current mount state. Other images, unresolved/missing paths and empty groups keep generic guidance. The existing attachment is neither borrowed nor detached. Original diagnostics, carrier digest/format, failed-command provenance and unknown child coverage remain available.

One optional parsed image-path and a producer-owned user message flow through existing reader cleanup, root-observation retention and shared CLI/MCP error projection. Tool schemas, failure categories and mount lifecycle stay unchanged. The directly related wrappers preserve that message rather than reinterpret stderr. Recovery guidance is owned by one producer constant; all new consumers compare against that owner or the generic error formatter, alongside independent fact assertions.

Base 0d1a454e2 is integrated; tested head 090a5d5c4f071c08e00228eed524930d323dbd21. The macOS lane now explicitly runs the real recovery acceptance test, the existing DMG provider suite and cancellation-reason checks; changes to those tests select the same lane.

Validation and scope

  • The new five-case boundary control fails only for the confirmed same-path guidance on main (one red, four controls pass); all pass with the fix. A separate projection regression preserves raw diagnostics and guidance through cleanup wrapping.
  • 392 unique local tests pass: 42 reader/error/resource tests, 348 selected boundary/scope tests and two additional CI-selection regressions. Six native-platform cases are skipped on Linux, including the real acceptance case; they are not counted as passes. check:fast, formatting, docs and diff checks pass.
  • Final head 090a5d5c4 passed real macOS CI in job 121835088759: 365 passed / five unrelated skips, including all 50 DMG provider tests, two cancellation-reason checks and the CLI recovery acceptance test (28.084s). The acceptance fixture creates a test-owned 20 MiB volume in UDZO format, verifies the existing attachment, observes the expected attach refusal and retained root facts, verifies existing devices stay untouched, releases the fixture through its owner, then verifies successful CLI inspection and no remaining attachment for the image. This uses actual hdiutil.
  • The two cancellation-reason tests use real owned subprocesses as the command stand-in; they do not establish hdiutil cancellation behavior. MCP shares the error projection; the native recovery scenario is exercised through CLI.
  • Prose mutation probes: changing both recovery and generic guidance wording leaves all nine selected tests passing; corrupting the failed attach exit-code fact makes all five root-observation cases fail. Both temporary mutations were restored. Final source regression rerun: 42 passed; focused provider/cleanup/acceptance rerun: 47 passed / four platform skips.
  • Final CI 40397501484 has the same unrelated failures: prewarm 100ms marker (SetupRegistrationPrewarm.test.ts) and two SAZ partial-observation assertions. All three were independently reproduced on exact main dd7a2da5b; the 18 relevant prewarm/SAZ/process/cleanup source and test files are unchanged at this PR base. Five shards were canceled by fail-fast and remain unverified. All jobs have finished: 30 pass, including Windows package/path checks, static/build/docs, real macOS, browser and the remaining real-provider/package lanes; four are policy-skipped. Four failures include the two test jobs above and their two aggregate gates; five canceled shards remain unverified.
  • Codex reviewed 090a5d5c4 with no findings; Greptile gives 5/5 on that same head, with no blocking issue. All review threads are resolved. Human declarations remain unchecked; the task owner explicitly authorized readiness and unrelated-baseline-failure bypass.

Resource evidence

Local checks use two-CPU affinity and process-family memory guards. Source tests: 4.780s wall / 7.338s CPU / 631.73 MiB sampled family peak. Focused boundary/build task: 69.656 / 87.151 / 1169.96. New scope cases: 2.075 / 2.209 / 379.05. Final static checks peak at 3947.30 MiB under a 7 GiB guard. RSS sampling can double-count shared pages or miss short peaks.

Path resolution runs only after the qualifying attach failure; normal attachment adds no extra realpath calls. No cache or resource limit is introduced. Native macOS CPU/peak memory, agent/schema tokens and before/after native latency have not been measured. The baseline failures and canceled coverage above are explicit verification limits.

Author readiness

Human owner: @hichipli

  • 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/7221 **Author:** [@hichipli](https://github.com/hichipli) **Created:** 10/26/2026 **Status:** ✅ Merged **Merged:** 10/26/2026 **Merged by:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Base:** `main` ← **Head:** `fix/dmg-attachment-recovery` --- ### 📝 Commits (10+) - [`b533ab1`](https://github.com/morluto/rea/commit/b533ab1045c17ed5bfc1eedd1a4bb82c6080bfd1) fix(artifacts): explain recovery for existing DMG attachments - [`67f8585`](https://github.com/morluto/rea/commit/67f85853c6105adc546c09c1c5782346481d8fe2) test(artifacts): run DMG recovery in macOS CI - [`ab44487`](https://github.com/morluto/rea/commit/ab44487ff86d064e924fd2c44b02f5a5430f57f3) fix(adb): preserve process names and pin fixture interpreters - [`d27e3f8`](https://github.com/morluto/rea/commit/d27e3f8e89c2a5b37bacb784e2275256441e6292) fix(native): preserve import names across dyld text formats - [`13fba27`](https://github.com/morluto/rea/commit/13fba271e6e14bf6235379564eb9d32aee7049d5) refactor(plist): centralize binary layout and offset ownership - [`5508b2a`](https://github.com/morluto/rea/commit/5508b2affd8bdd0788aed9d2246c373fd1b01bb7) fix(analysis): retain source evidence through directory exports (#7250) - [`f955e1a`](https://github.com/morluto/rea/commit/f955e1a294c34ef3dea816a00b90ddd3512e9829) test(artifacts): check all DMG attachments after retry - [`8423c5c`](https://github.com/morluto/rea/commit/8423c5c6d08512568c2a75753b6b7c3a9b6bbd4e) refactor(macho): consolidate prefix interpretation across readers - [`2cdc8cb`](https://github.com/morluto/rea/commit/2cdc8cb92ef3f800e32fe73ef80a334396461ceb) refactor(managed): centralize TypeDef row facts across consumers - [`1d32eb3`](https://github.com/morluto/rea/commit/1d32eb3aa4ac75ff8b240062a2036c9da5e5ccfa) fix(providers): keep operation environments consistent through launch ### 📊 Changes **351 files changed** (+18837 additions, -3876 deletions) <details> <summary>View changed files</summary> 📝 `.agents/skills/reverse-engineer-anything/references/javascript-applications.md` (+9 -0) 📝 `.github/workflows/ci.yml` (+3 -0) 📝 `docs/client-configuration.md` (+4 -2) 📝 `docs/firmware-analysis.md` (+1 -1) 📝 `docs/ghidra-dos.md` (+1 -1) 📝 `docs/installation.md` (+6 -4) 📝 `docs/javascript-route-inventory.md` (+36 -4) 📝 `docs/native-investigation.md` (+1 -1) 📝 `docs/testing.md` (+1 -1) 📝 `docs/verification/native-decompile-directory-2026-10-23.md` (+4 -0) ➕ `docs/verification/web-bundle-paging-2026-10-26.md` (+138 -0) 📝 `docs/windows-ghidra-p0.md` (+1 -1) ➕ `experiments/analysis-efficiency/decompile-directory.md` (+149 -0) ➕ `experiments/analysis-efficiency/decompile-directory.mjs` (+328 -0) ➕ `experiments/analysis-efficiency/web-bundle-paging.mjs` (+382 -0) 📝 `scripts/ci/scopes.mjs` (+4 -0) 📝 `scripts/lib/browser-network-e2e.mjs` (+11 -6) 📝 `scripts/lib/browser-popup-e2e.mjs` (+5 -4) 📝 `scripts/lib/browser-screenshot-e2e.mjs` (+8 -4) 📝 `scripts/lib/mcp-verifier-results.mjs` (+2 -1) _...and 80 more files_ </details> ### 📄 Description ## Summary Fixes #5546’s remaining recovery-guidance gap. When a numeric unavailable `hdiutil attach` failure follows an inventory that lists the selected resolved image path and a device group, explain that the attachment’s owner can release it when safe before retrying. The native adapter owns this positive path-match fact. It does not assert byte identity, attachment ownership, causality or current mount state. Other images, unresolved/missing paths and empty groups keep generic guidance. The existing attachment is neither borrowed nor detached. Original diagnostics, carrier digest/format, failed-command provenance and unknown child coverage remain available. One optional parsed `image-path` and a producer-owned user message flow through existing reader cleanup, root-observation retention and shared CLI/MCP error projection. Tool schemas, failure categories and mount lifecycle stay unchanged. The directly related wrappers preserve that message rather than reinterpret stderr. Recovery guidance is owned by one producer constant; all new consumers compare against that owner or the generic error formatter, alongside independent fact assertions. Base `0d1a454e2` is integrated; tested head `090a5d5c4f071c08e00228eed524930d323dbd21`. The macOS lane now explicitly runs the real recovery acceptance test, the existing DMG provider suite and cancellation-reason checks; changes to those tests select the same lane. ## Validation and scope - The new five-case boundary control fails only for the confirmed same-path guidance on main (one red, four controls pass); all pass with the fix. A separate projection regression preserves raw diagnostics and guidance through cleanup wrapping. - **392 unique local tests pass**: 42 reader/error/resource tests, 348 selected boundary/scope tests and two additional CI-selection regressions. Six native-platform cases are skipped on Linux, including the real acceptance case; they are not counted as passes. `check:fast`, formatting, docs and diff checks pass. - **Final head `090a5d5c4` passed real macOS CI** in [job 121835088759](https://github.com/morluto/rea/actions/runs/40397501484/job/121835088759): 365 passed / five unrelated skips, including all 50 DMG provider tests, two cancellation-reason checks and the CLI recovery acceptance test (28.084s). The acceptance fixture creates a test-owned 20 MiB volume in UDZO format, verifies the existing attachment, observes the expected attach refusal and retained root facts, verifies existing devices stay untouched, releases the fixture through its owner, then verifies successful CLI inspection and no remaining attachment for the image. This uses actual `hdiutil`. - The two cancellation-reason tests use real owned subprocesses as the command stand-in; they do not establish `hdiutil` cancellation behavior. MCP shares the error projection; the native recovery scenario is exercised through CLI. - Prose mutation probes: changing both recovery and generic guidance wording leaves all nine selected tests passing; corrupting the failed attach exit-code fact makes all five root-observation cases fail. Both temporary mutations were restored. Final source regression rerun: 42 passed; focused provider/cleanup/acceptance rerun: 47 passed / four platform skips. - Final CI [40397501484](https://github.com/morluto/rea/actions/runs/40397501484) has the same unrelated failures: prewarm 100ms marker (`SetupRegistrationPrewarm.test.ts`) and two SAZ partial-observation assertions. All three were independently reproduced on exact main `dd7a2da5b`; the 18 relevant prewarm/SAZ/process/cleanup source and test files are unchanged at this PR base. Five shards were canceled by fail-fast and remain unverified. All jobs have finished: 30 pass, including Windows package/path checks, static/build/docs, real macOS, browser and the remaining real-provider/package lanes; four are policy-skipped. Four failures include the two test jobs above and their two aggregate gates; five canceled shards remain unverified. - Codex reviewed `090a5d5c4` with no findings; Greptile gives 5/5 on that same head, with no blocking issue. All review threads are resolved. Human declarations remain unchecked; the task owner explicitly authorized readiness and unrelated-baseline-failure bypass. ## Resource evidence Local checks use two-CPU affinity and process-family memory guards. Source tests: 4.780s wall / 7.338s CPU / 631.73 MiB sampled family peak. Focused boundary/build task: 69.656 / 87.151 / 1169.96. New scope cases: 2.075 / 2.209 / 379.05. Final static checks peak at 3947.30 MiB under a 7 GiB guard. RSS sampling can double-count shared pages or miss short peaks. Path resolution runs only after the qualifying attach failure; normal attachment adds no extra realpath calls. No cache or resource limit is introduced. Native macOS CPU/peak memory, agent/schema tokens and before/after native latency have not been measured. The baseline failures and canceled coverage above are explicit verification limits. ## Author readiness Human owner: @hichipli - [ ] 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 - [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. - [ ] 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>
bsakaguchi 2026-11-03 13:39:55 +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#7725
No description provided.