morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #3825] [MERGED] fix(native): keep property flags unknown for unfinished quoted types #5926

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/objc-property-unfinished-quote

📝 Commits (1)

d0a14c9 fix(native): keep property flags unknown for unfinished quoted types

📊 Changes

4 files changed (+189 additions, -22 deletions)

View changed files

📝 docs/native-investigation.md (+3 -1)
📝 src/native/AppleMachoFixups.test.ts (+13 -0)
📝 src/native/AppleObjcProperties.ts (+63 -21)
➕ tests/boundary/cli/nativeDispatchPropertyAttributes.test.ts (+110 -0)

📄 Description

Summary

Fixes #3345. inspect-native-dispatch-metadata (MCP inspect_native_dispatch_metadata) reported atomicity: "atomic" and is_readonly: false for an Objective-C property whose type attribute has an unfinished quoted class name (T@"NSString). objc_properties_categories coverage stayed complete. The documented semantics (docs/native-investigation.md) say incomplete attribute metadata leaves atomicity and any unestablished read-only value null.

Now such a property reports atomicity: null and is_readonly: null, unless an explicit R token establishes read-only. The raw type and attribute pairs are kept, and the facet becomes partial with a property_attributes_incomplete reason.

Problem and expected behavior

splitAttributes toggles a quote state while splitting on commas but never reported an unbalanced quote. parsePropertyAttributes then treated any nonempty T token as a complete type and inferred the atomic/read-write defaults from the absence of N/R. With an unfinished quote, any later N or R would sit inside the unfinished type (T@"NSString,N,R yields one token), so their absence establishes nothing.

Change and scope

The attribute owner, src/native/AppleObjcProperties.ts, changes as follows. splitAttributes returns { tokens, complete }, and decodePropertyAttributes establishes the defaults only for a complete typed string. Explicit N/R tokens remain established, as do G/S accessors and the raw type_encoding.
readObjcPropertyList returns the names of incomplete properties. readObjcPropertiesOf records one property_attributes_incomplete issue per property through the existing class and category issue paths, which already mark objc_properties_categories partial with the reason.
parsePropertyAttributes keeps its signature.
Docs name the new coverage reason.
Not included: other malformed type encodings, such as unbalanced braces in struct types. The issue concerns the quoted class-name splitter that already tracks quote state.

Contract and boundary impact

Semantic owner and earliest changed stage: Objective-C property attribute 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. Nullable fields that were wrongly established are now null, and there is a new coverage reason code.
Provider, bridge, target-format, or platform compatibility: Apple Mach-O dispatch metadata.
Evidence, artifact, provenance, or reconstruction contract: unknowns are no longer reported as established facts; the raw attribute is preserved.
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) for four cases. The complete Tq,N control and the complete default T@"NSString",C keep their flags. An unfinished T@"NSString gives null flags, and R,T@"NSString keeps is_readonly: true. The facet is partial with the reason only for the unfinished cases.
- src/native/AppleMachoFixups.test.ts checks that N,R swallowed by an unfinished quote are not flags, while N/R before the type still are.
Base reproduction: against the unfixed AppleObjcProperties.ts, all three unfinished-quote cases fail (Tests 3 failed | 39 passed).

User-visible output for the issue's incomplete-attributes.bin, after:

Fixture.first: type="@\"NSString" atomicity=null readonly=null
facet=partial reason=property_attributes_incomplete at 0x100000500: Fixture.first: the type attribute has an unterminated quoted class name, so atomicity and unestablished read-only state are unknown; ...

Before: atomicity=atomic readonly=false, facet=complete reason=null.

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 -- src/native/AppleMachoFixups.test.ts tests/boundary/cli/nativeDispatchPropertyAttributes.test.ts tests/boundary/providers/native/nativeProvider.test.ts: 88 passed
npm run check:fast, npm run knip, npm run docs:check: passed
Issue reproduction on macOS (Darwin 25.6, arm64) with the issue's fixture generator: valid.bin stays nonatomic and read-write with the facet complete; incomplete-attributes.bin matches the expected behavior above.
No false positives on real apps. inspect-native-dispatch-metadata on Transmission, KeepingYouAwake, BetterDisplay, Raycast and XLD decoded 768 real properties: 0 got null atomicity and 0 property_attributes_incomplete issues. Where a facet was partial, it was already partial for other reasons. Run times were 0.5–0.7 s per app; the fix only keeps a boolean the splitter already computed.

Compatibility, safety, and release

Breaking changes or migration steps: none. Callers already handle nullable atomicity/is_readonly.
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/3825 **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-unfinished-quote` --- ### 📝 Commits (1) - [`d0a14c9`](https://github.com/morluto/rea/commit/d0a14c9c032041a316169ab862d7a8f31cef3ff2) fix(native): keep property flags unknown for unfinished quoted types ### 📊 Changes **4 files changed** (+189 additions, -22 deletions) <details> <summary>View changed files</summary> 📝 `docs/native-investigation.md` (+3 -1) 📝 `src/native/AppleMachoFixups.test.ts` (+13 -0) 📝 `src/native/AppleObjcProperties.ts` (+63 -21) ➕ `tests/boundary/cli/nativeDispatchPropertyAttributes.test.ts` (+110 -0) </details> ### 📄 Description ## Summary Fixes #3345. `inspect-native-dispatch-metadata` (MCP `inspect_native_dispatch_metadata`) reported `atomicity: "atomic"` and `is_readonly: false` for an Objective-C property whose type attribute has an unfinished quoted class name (`T@"NSString`). `objc_properties_categories` coverage stayed `complete`. The documented semantics (`docs/native-investigation.md`) say incomplete attribute metadata leaves `atomicity` and any unestablished read-only value null. Now such a property reports `atomicity: null` and `is_readonly: null`, unless an explicit `R` token establishes read-only. The raw type and attribute pairs are kept, and the facet becomes `partial` with a `property_attributes_incomplete` reason. ## Problem and expected behavior `splitAttributes` toggles a quote state while splitting on commas but never reported an unbalanced quote. `parsePropertyAttributes` then treated any nonempty `T` token as a complete type and inferred the `atomic`/read-write defaults from the absence of `N`/`R`. With an unfinished quote, any later `N` or `R` would sit inside the unfinished type (`T@"NSString,N,R` yields one token), so their absence establishes nothing. ## Change and scope - The attribute owner, `src/native/AppleObjcProperties.ts`, changes as follows. `splitAttributes` returns `{ tokens, complete }`, and `decodePropertyAttributes` establishes the defaults only for a complete typed string. Explicit `N`/`R` tokens remain established, as do `G`/`S` accessors and the raw `type_encoding`. - `readObjcPropertyList` returns the names of incomplete properties. `readObjcPropertiesOf` records one `property_attributes_incomplete` issue per property through the existing class and category issue paths, which already mark `objc_properties_categories` partial with the reason. - `parsePropertyAttributes` keeps its signature. - Docs name the new coverage reason. - Not included: other malformed type encodings, such as unbalanced braces in struct types. The issue concerns the quoted class-name splitter that already tracks quote state. ## Contract and boundary impact - Semantic owner and earliest changed stage: Objective-C property attribute 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. Nullable fields that were wrongly established are now null, and there is a new coverage reason code. - Provider, bridge, target-format, or platform compatibility: Apple Mach-O dispatch metadata. - Evidence, artifact, provenance, or reconstruction contract: unknowns are no longer reported as established facts; the raw attribute is preserved. - 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) for four cases. The complete `Tq,N` control and the complete default `T@"NSString",C` keep their flags. An unfinished `T@"NSString` gives null flags, and `R,T@"NSString` keeps `is_readonly: true`. The facet is partial with the reason only for the unfinished cases. - `src/native/AppleMachoFixups.test.ts` checks that `N,R` swallowed by an unfinished quote are not flags, while `N`/`R` before the type still are. - Base reproduction: against the unfixed `AppleObjcProperties.ts`, all three unfinished-quote cases fail (`Tests 3 failed | 39 passed`). - User-visible output for the issue's `incomplete-attributes.bin`, after: ``` Fixture.first: type="@\"NSString" atomicity=null readonly=null facet=partial reason=property_attributes_incomplete at 0x100000500: Fixture.first: the type attribute has an unterminated quoted class name, so atomicity and unestablished read-only state are unknown; ... ``` Before: `atomicity=atomic readonly=false`, `facet=complete reason=null`. - 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 -- src/native/AppleMachoFixups.test.ts tests/boundary/cli/nativeDispatchPropertyAttributes.test.ts tests/boundary/providers/native/nativeProvider.test.ts`: 88 passed - `npm run check:fast`, `npm run knip`, `npm run docs:check`: passed - Issue reproduction on macOS (Darwin 25.6, arm64) with the issue's fixture generator: `valid.bin` stays nonatomic and read-write with the facet complete; `incomplete-attributes.bin` matches the expected behavior above. - No false positives on real apps. `inspect-native-dispatch-metadata` on Transmission, KeepingYouAwake, BetterDisplay, Raycast and XLD decoded 768 real properties: 0 got null atomicity and 0 `property_attributes_incomplete` issues. Where a facet was partial, it was already partial for other reasons. Run times were 0.5–0.7 s per app; the fix only keeps a boolean the splitter already computed. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Callers already handle nullable `atomicity`/`is_readonly`. - 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:30 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:45:33 +00:00

[PR #6012] [MERGED] fix(capture): keep excluded MIME protobuf applicability partial #7120

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#5926

No description provided.

Rows
Columns