[PR #3833] [MERGED] fix(native): retain valid Objective-C properties beside a malformed record #5934

New issue

Closed

opened 2026-10-31 01:10:33 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:10:33 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/objc-property-record-isolation

📝 Commits (2)

3ccf11a fix(native): retain valid Objective-C properties beside a malformed record
2112dee test(native): cover property record isolation on every platform

📊 Changes

5 files changed (+181 additions, -64 deletions)

View changed files

📝 docs/native-investigation.md (+4 -1)
📝 src/native/AppleDispatchMetadata.test.ts (+33 -0)
📝 src/native/AppleObjcProperties.ts (+44 -18)
📝 tests/boundary/cli/nativeDispatchPropertyAttributes.test.ts (+49 -45)
➕ tests/fixtures/objcPropertyMacho.ts (+51 -0)

📄 Description

Summary

Fixes #3336. inspect-native-dispatch-metadata (MCP inspect_native_dispatch_metadata) lost valid Objective-C metadata when a single property record was malformed:

An unmapped name pointer in the second property removed the already decoded first property (Fixture.properties: []).
An empty name in the first property failed the whole inspection: exit 1, execution_failure with a schema violation at ["objc_classes",0,"properties",0,"name"], and no Sibling class or valid second property.

Now each property record is decoded on its own. The failing record becomes a property_decode_failed diagnostic at its entry address, the facet is marked partial, and every valid sibling property and class is retained.

Problem and expected behavior

readObjcPropertyList accumulated a whole property_list_t in one operation, and readObjcPropertiesOf returned [] from its list-level catch. A failure on entry n therefore discarded entries 0..n-1. An empty name raised nothing during decoding, so it reached appleDispatchMetadataSchema validation of the whole result. That rejected every class, matching the class-level case #2611 already fixed for empty class names.

Change and scope

The property list owner, src/native/AppleObjcProperties.ts, decodes each property_t entry inside its own try. An empty name is rejected there. The function returns { properties, issues }, where each issue has a code, message and entry address.
readObjcPropertiesOf forwards those issues through the existing class and category issue paths. objc_properties_categories is therefore partial with property_decode_failed at <entry>: <owner>: property <index> could not be decoded: <cause>.
The property_attributes_incomplete issue from #3825 now uses the same path and reports the entry address instead of the list address.
List-header failures (an unreadable count or an unsupported entry size) still fail only that list, as before.
Docs describe the per-record diagnostic.
Not included: other metadata tables such as methods and ivars. They have their own decoders, and the issue concerns property records.

Contract and boundary impact

Semantic owner and earliest changed stage: Objective-C property list decoding (AppleObjcProperties.ts).
Contribution scope fit and linked maintainer decision: correctness fix to an existing read-only decoder; not applicable.
CLI and MCP/tool-catalog contract: schemas unchanged. Inputs that failed now return partial results with diagnostics.
Provider, bridge, target-format, or platform compatibility: Apple Mach-O dispatch metadata.
Evidence, artifact, provenance, or reconstruction contract: partial facts are retained with explicit partial coverage, per AGENTS.md and docs/tool-design.md.
Target selection, authority, upstream behavior, and cleanup details: unchanged.
Generated metadata, package, or installation impact: none.

Expected effects:

Read existing files or captured data.

Evidence and regression coverage

Tests added: tests/boundary/cli/nativeDispatchPropertyAttributes.test.ts runs the built CLI on the issue's synthetic arm64 Mach-O (darwin).
- An unmapped second name pointer keeps first, with property_decode_failed at 0x100000518.
- An empty first name keeps second, with the diagnostic at 0x100000508.
Existing attribute cases in the same file still pass.
Base reproduction: against the unfixed module, both new cases fail (Tests 2 failed | 4 passed).

User-visible output on the issue's fixtures, after:

valid.bin     exit=0 classes=Fixture[first,second] Sibling[]  facet=complete
unmapped.bin  exit=0 classes=Fixture[first] Sibling[]         facet=partial reason=property_decode_failed at 0x100000518: Fixture: property 1 could not be decoded: Unmapped or ambiguous metadata pointer 0x100002000
empty.bin     exit=0 classes=Fixture[second] Sibling[]        facet=partial reason=property_decode_failed at 0x100000508: Fixture: property 0 could not be decoded: the property name is empty

Before: unmapped.bin returned Fixture[], and empty.bin exited 1 with no metadata.

Remaining proof gaps: none for this issue.

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

npm run test:focused -- tests/boundary/cli/nativeDispatchPropertyAttributes.test.ts src/native/AppleMachoFixups.test.ts src/native/AppleDispatchMetadata.test.ts tests/boundary/providers/native/nativeProvider.test.ts: 126 passed
npm run check:fast, npm run knip, npm run docs:check: passed
Real apps on macOS (Darwin 25.6, arm64). Transmission, KeepingYouAwake, BetterDisplay, Raycast and XLD produced the same 768 properties with no new diagnostics, at 0.49–0.63 s per app against 0.50–0.66 s before. The change only adds one try per property entry.

Compatibility, safety, and release

Breaking changes or migration steps: none. A failing whole-inspection case now succeeds with partial coverage.
Real Hopper/Ghidra, browser, or OS coverage: macOS only; the metadata reader runs in-process.
Package or release metadata impact: none.
Security, privacy, process, or containment review: none.

Author readiness

Human owner:

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/3833 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/15/2026 **Status:** ✅ Merged **Merged:** 10/15/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/objc-property-record-isolation` --- ### 📝 Commits (2) - [`3ccf11a`](https://github.com/morluto/rea/commit/3ccf11ae4e22bb1503e54492bd7dfa4793c85b15) fix(native): retain valid Objective-C properties beside a malformed record - [`2112dee`](https://github.com/morluto/rea/commit/2112deed0451f9c28032c97e84ee37b309110e46) test(native): cover property record isolation on every platform ### 📊 Changes **5 files changed** (+181 additions, -64 deletions) <details> <summary>View changed files</summary> 📝 `docs/native-investigation.md` (+4 -1) 📝 `src/native/AppleDispatchMetadata.test.ts` (+33 -0) 📝 `src/native/AppleObjcProperties.ts` (+44 -18) 📝 `tests/boundary/cli/nativeDispatchPropertyAttributes.test.ts` (+49 -45) ➕ `tests/fixtures/objcPropertyMacho.ts` (+51 -0) </details> ### 📄 Description ## Summary Fixes #3336. `inspect-native-dispatch-metadata` (MCP `inspect_native_dispatch_metadata`) lost valid Objective-C metadata when a single property record was malformed: - An unmapped name pointer in the second property removed the already decoded first property (`Fixture.properties: []`). - An empty name in the first property failed the whole inspection: exit `1`, `execution_failure` with a schema violation at `["objc_classes",0,"properties",0,"name"]`, and no `Sibling` class or valid `second` property. Now each property record is decoded on its own. The failing record becomes a `property_decode_failed` diagnostic at its entry address, the facet is marked partial, and every valid sibling property and class is retained. ## Problem and expected behavior `readObjcPropertyList` accumulated a whole `property_list_t` in one operation, and `readObjcPropertiesOf` returned `[]` from its list-level `catch`. A failure on entry *n* therefore discarded entries *0..n-1*. An empty name raised nothing during decoding, so it reached `appleDispatchMetadataSchema` validation of the whole result. That rejected every class, matching the class-level case #2611 already fixed for empty class names. ## Change and scope - The property list owner, `src/native/AppleObjcProperties.ts`, decodes each `property_t` entry inside its own `try`. An empty name is rejected there. The function returns `{ properties, issues }`, where each issue has a code, message and entry address. - `readObjcPropertiesOf` forwards those issues through the existing class and category issue paths. `objc_properties_categories` is therefore `partial` with `property_decode_failed at <entry>: <owner>: property <index> could not be decoded: <cause>`. - The `property_attributes_incomplete` issue from #3825 now uses the same path and reports the entry address instead of the list address. - List-header failures (an unreadable count or an unsupported entry size) still fail only that list, as before. - Docs describe the per-record diagnostic. - Not included: other metadata tables such as methods and ivars. They have their own decoders, and the issue concerns property records. ## Contract and boundary impact - Semantic owner and earliest changed stage: Objective-C property list decoding (`AppleObjcProperties.ts`). - Contribution scope fit and linked maintainer decision: correctness fix to an existing read-only decoder; not applicable. - CLI and MCP/tool-catalog contract: schemas unchanged. Inputs that failed now return partial results with diagnostics. - Provider, bridge, target-format, or platform compatibility: Apple Mach-O dispatch metadata. - Evidence, artifact, provenance, or reconstruction contract: partial facts are retained with explicit partial coverage, per `AGENTS.md` and `docs/tool-design.md`. - Target selection, authority, upstream behavior, and cleanup details: unchanged. - Generated metadata, package, or installation impact: none. Expected effects: - [x] Read existing files or captured data. ## Evidence and regression coverage - Tests added: `tests/boundary/cli/nativeDispatchPropertyAttributes.test.ts` runs the built CLI on the issue's synthetic arm64 Mach-O (darwin). - An unmapped second name pointer keeps `first`, with `property_decode_failed at 0x100000518`. - An empty first name keeps `second`, with the diagnostic at `0x100000508`. - Existing attribute cases in the same file still pass. - Base reproduction: against the unfixed module, both new cases fail (`Tests 2 failed | 4 passed`). - User-visible output on the issue's fixtures, after: ``` valid.bin exit=0 classes=Fixture[first,second] Sibling[] facet=complete unmapped.bin exit=0 classes=Fixture[first] Sibling[] facet=partial reason=property_decode_failed at 0x100000518: Fixture: property 1 could not be decoded: Unmapped or ambiguous metadata pointer 0x100002000 empty.bin exit=0 classes=Fixture[second] Sibling[] facet=partial reason=property_decode_failed at 0x100000508: Fixture: property 0 could not be decoded: the property name is empty ``` Before: `unmapped.bin` returned `Fixture[]`, and `empty.bin` exited 1 with no metadata. - Remaining proof gaps: none for this issue. 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 - `npm run test:focused -- tests/boundary/cli/nativeDispatchPropertyAttributes.test.ts src/native/AppleMachoFixups.test.ts src/native/AppleDispatchMetadata.test.ts tests/boundary/providers/native/nativeProvider.test.ts`: 126 passed - `npm run check:fast`, `npm run knip`, `npm run docs:check`: passed - Real apps on macOS (Darwin 25.6, arm64). Transmission, KeepingYouAwake, BetterDisplay, Raycast and XLD produced the same 768 properties with no new diagnostics, at 0.49–0.63 s per app against 0.50–0.66 s before. The change only adds one `try` per property entry. ## Compatibility, safety, and release - Breaking changes or migration steps: none. A failing whole-inspection case now succeeds with partial coverage. - Real Hopper/Ghidra, browser, or OS coverage: macOS only; the metadata reader runs in-process. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none. ## Author readiness Human owner:  - [ ] 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. - [x] 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-10-31 01:10:33 +00:00