[PR #6074] [CLOSED] fix(apple): preserve Interface Builder read failures instead of invalid archives #7141

Closed
opened 2026-11-01 21:45:42 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6074
Author: @XiaoBinGan
Created: 10/21/2026
Status: ❌ Closed

Base: main ← Head: fix/interface-builder-read-failure


📝 Commits (4)

  • f4b267f fix(apple): preserve Interface Builder read failures instead of invalid archives
  • dc2eb7c chore: integrate Interface Builder read-failure fix with current main
  • 6119bc5 fix(apple): keep the typed read failure through final cleanup
  • 82be61f chore: integrate Interface Builder read-failure fix with current main

📊 Changes

2 files changed (+192 additions, -2 deletions)

View changed files

📝 src/artifacts/apple/InterfaceBuilderAnalysis.ts (+28 -2)
➕ src/artifacts/apple/InterfaceBuilderReadFailure.test.ts (+164 -0)

📄 Description

Summary

decode_interface_builder reports a selected document that was never read as a successful partial graph. When a readable-format nib cannot be opened (EACCES), or the selected name is replaced by byte-identical content on a new inode, the tool exits 0 with documents: [], graph.truncated: true and archive coverage reason one_or_more_archives_invalid. The typed read outcome survives only as a human-readable limitation, so an automated caller cannot branch on unavailable access versus malformed content.

Fixes #6066.

Problem and expected behavior

The per-document catch in analyzeInterfaceBuilderBundle only separates an aborted caller signal and InterfaceBuilderDecodeBudgetExceeded; every remaining exception is pushed onto invalid, which the coverage projection turns into one_or_more_archives_invalid. A read failure occurs before any decode, so calling those bytes "invalid archives" misstates what was established.

Expected: preserve the typed reader outcome (integrity for a changed entry, io for a refused open) for the artifact boundary, and keep describing only bytes that were actually read and rejected as invalid archives. Genuine decode failures, budget omissions and cancellation reporting stay as they are.

Change and scope

The catch rethrows an ArtifactReaderFailure before the decode-failure classification. The existing shared translation (ArtifactOperationFailure.ts) already maps a reader reason onto ArtifactOperationError with matching recovery, so no new schema or consumer is needed. This mirrors the existing convention for other artifact producers, which rethrow reader failures rather than reinterpreting them.

Intentionally not included: permission classification inside the shared directory reader, which is independently needed and cannot repair this catch by itself.

Contract and boundary impact

  • Semantic owner and earliest changed stage: analyzeInterfaceBuilderBundle per-document classification; earliest stage is the artifact read boundary.
  • Contribution scope fit and linked maintainer decision: ordinary bug fix within existing boundaries; not applicable.
  • CLI and MCP/tool-catalog contract: unchanged schemas; failures now surface as typed ArtifactOperationError instead of successful partial Evidence. No tool was added or renamed.
  • Provider, bridge, target-format, or platform compatibility: no provider change; the behavior is platform-independent, but the reported case was reproduced on macOS.
  • Evidence, artifact, provenance, or reconstruction contract: partial Evidence no longer claims malformed archive content for a document that was never read.
  • Target selection, authority, upstream behavior, and cleanup details: no target execution; reader cleanup already owns descriptors on the failure path.
  • Generated metadata: 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: src/artifacts/apple/InterfaceBuilderReadFailure.test.ts — four cases: a readable document control, a changed-inode document, a refused open, and a genuinely malformed document.
  • Base reproduction or other evidence: the two failure cases fail on the affected base revision and pass with the change; the malformed and readable controls pass on both. On base they returned success with documents: [] and reason: one_or_more_archives_invalid.
  • User-visible CLI/MCP output (if applicable): the refused-open case previously returned success plus a limitation naming EACCES; the changed-entry case previously returned success plus a limitation naming the changed entry. Both now surface the typed failure.
  • Remaining proof gaps: this was exercised through the production DirectoryArtifactReader and the analysis entry point, not through an end-to-end MCP session. Natural incidence and other platforms were not measured. No repaired software or known-good revision is established.

Evidence-bearing checks:

  • 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

  • node node_modules/vitest/vitest.mjs run src/artifacts/apple/InterfaceBuilderReadFailure.test.ts src/artifacts/apple/InterfaceBuilderDecodeBudget.test.ts --maxWorkers=1 — 19 passed (4 new, 15 existing).
  • npx tsc --noEmit -p . — no errors.
  • npx oxfmt --check <changed files> — formatted.
  • npm run verify:module-boundaries — 2481 source files checked, no violations.
  • node node_modules/vitest/vitest.mjs run src/artifacts/apple --maxWorkers=1 — 6 pre-existing failures on macOS-only/symlink fixtures reproduce identically on the unmodified base revision; the new file adds 4 passing tests and changes none of them.

Compatibility, safety, and release

  • Breaking changes or migration steps: none. Callers that previously received successful partial Evidence for an unread document now receive a typed failure; that is the reported defect, not a supported contract.
  • Real Hopper/Ghidra, browser, or OS coverage: none required; no analysis engine is involved.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: no target interaction, no new data exposure. A failed read now stops the operation instead of continuing with partial output.

Author readiness

Human owner: @XiaoBinGan

  • 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/6074 **Author:** [@XiaoBinGan](https://github.com/XiaoBinGan) **Created:** 10/21/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/interface-builder-read-failure` --- ### 📝 Commits (4) - [`f4b267f`](https://github.com/morluto/rea/commit/f4b267f4cf14e96104632ce1dc05542c55be254e) fix(apple): preserve Interface Builder read failures instead of invalid archives - [`dc2eb7c`](https://github.com/morluto/rea/commit/dc2eb7c08f754bb23c25db0745b997146d9dd031) chore: integrate Interface Builder read-failure fix with current main - [`6119bc5`](https://github.com/morluto/rea/commit/6119bc5e197522816da5fdb3e21f77fd03ffe72b) fix(apple): keep the typed read failure through final cleanup - [`82be61f`](https://github.com/morluto/rea/commit/82be61f938d21f60c91909ad29c8ed750f0737f9) chore: integrate Interface Builder read-failure fix with current main ### 📊 Changes **2 files changed** (+192 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `src/artifacts/apple/InterfaceBuilderAnalysis.ts` (+28 -2) ➕ `src/artifacts/apple/InterfaceBuilderReadFailure.test.ts` (+164 -0) </details> ### 📄 Description ## Summary `decode_interface_builder` reports a selected document that was never read as a successful partial graph. When a readable-format nib cannot be opened (`EACCES`), or the selected name is replaced by byte-identical content on a new inode, the tool exits 0 with `documents: []`, `graph.truncated: true` and archive coverage reason `one_or_more_archives_invalid`. The typed read outcome survives only as a human-readable limitation, so an automated caller cannot branch on unavailable access versus malformed content. Fixes #6066. ## Problem and expected behavior The per-document `catch` in `analyzeInterfaceBuilderBundle` only separates an aborted caller signal and `InterfaceBuilderDecodeBudgetExceeded`; every remaining exception is pushed onto `invalid`, which the coverage projection turns into `one_or_more_archives_invalid`. A read failure occurs before any decode, so calling those bytes "invalid archives" misstates what was established. Expected: preserve the typed reader outcome (`integrity` for a changed entry, `io` for a refused open) for the artifact boundary, and keep describing only bytes that were actually read and rejected as invalid archives. Genuine decode failures, budget omissions and cancellation reporting stay as they are. ## Change and scope The catch rethrows an `ArtifactReaderFailure` before the decode-failure classification. The existing shared translation (`ArtifactOperationFailure.ts`) already maps a reader reason onto `ArtifactOperationError` with matching recovery, so no new schema or consumer is needed. This mirrors the existing convention for other artifact producers, which rethrow reader failures rather than reinterpreting them. Intentionally not included: permission classification inside the shared directory reader, which is independently needed and cannot repair this catch by itself. ## Contract and boundary impact - Semantic owner and earliest changed stage: `analyzeInterfaceBuilderBundle` per-document classification; earliest stage is the artifact read boundary. - Contribution scope fit and linked maintainer decision: ordinary bug fix within existing boundaries; not applicable. - CLI and MCP/tool-catalog contract: unchanged schemas; failures now surface as typed `ArtifactOperationError` instead of successful partial Evidence. No tool was added or renamed. - Provider, bridge, target-format, or platform compatibility: no provider change; the behavior is platform-independent, but the reported case was reproduced on macOS. - Evidence, artifact, provenance, or reconstruction contract: partial Evidence no longer claims malformed archive content for a document that was never read. - Target selection, authority, upstream behavior, and cleanup details: no target execution; reader cleanup already owns descriptors on the failure path. - Generated metadata: 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: `src/artifacts/apple/InterfaceBuilderReadFailure.test.ts` — four cases: a readable document control, a changed-inode document, a refused open, and a genuinely malformed document. - Base reproduction or other evidence: the two failure cases fail on the affected base revision and pass with the change; the malformed and readable controls pass on both. On base they returned success with `documents: []` and `reason: one_or_more_archives_invalid`. - User-visible CLI/MCP output (if applicable): the refused-open case previously returned success plus a limitation naming `EACCES`; the changed-entry case previously returned success plus a limitation naming the changed entry. Both now surface the typed failure. - Remaining proof gaps: this was exercised through the production `DirectoryArtifactReader` and the analysis entry point, not through an end-to-end MCP session. Natural incidence and other platforms were not measured. No repaired software or known-good revision is established. Evidence-bearing checks: - [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 - `node node_modules/vitest/vitest.mjs run src/artifacts/apple/InterfaceBuilderReadFailure.test.ts src/artifacts/apple/InterfaceBuilderDecodeBudget.test.ts --maxWorkers=1` — 19 passed (4 new, 15 existing). - `npx tsc --noEmit -p .` — no errors. - `npx oxfmt --check <changed files>` — formatted. - `npm run verify:module-boundaries` — 2481 source files checked, no violations. - `node node_modules/vitest/vitest.mjs run src/artifacts/apple --maxWorkers=1` — 6 pre-existing failures on macOS-only/symlink fixtures reproduce identically on the unmodified base revision; the new file adds 4 passing tests and changes none of them. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Callers that previously received successful partial Evidence for an unread document now receive a typed failure; that is the reported defect, not a supported contract. - Real Hopper/Ghidra, browser, or OS coverage: none required; no analysis engine is involved. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no target interaction, no new data exposure. A failed read now stops the operation instead of continuing with partial output. ## Author readiness <!-- The human owner completes these declarations before requesting review. The PR author is the owner by default. A different owner must explicitly accept ownership on this PR. AI agents must leave these declarations to a human to dictate to them or there could be catastrophic consequences for many users of REA. Draft PRs may leave them unchecked. These declarations record initial readiness when opening the PR for review; later pushes do not require a new human sign-off. Keep the description and validation evidence current as the work changes. See CONTRIBUTING.md#pull-request-ownership-and-readiness. --> Human owner: @XiaoBinGan - [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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-01 21:45:42 +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#7141
No description provided.