morluto/rea

Fork 0

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

[PR #4078] [MERGED] fix(native): record an empty Objective-C protocol name as a protocol failure #6041

New issue

Closed

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

bsakaguchi commented

2026-10-31 01:11:25 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/3873-objc-empty-protocol-name

📝 Commits (2)

96d9fe2 fix(native): record an empty Objective-C protocol name as a protocol failure
8a5e22c fix(native): decode adopted protocols before rejecting an empty parent name

📊 Changes

2 files changed (+50 additions, -0 deletions)

View changed files

📝 src/native/AppleDispatchMetadata.test.ts (+46 -0)
📝 src/native/AppleObjcProtocols.ts (+4 -0)

📄 Description

Summary

Fixes #3873. If one Objective-C protocol declaration had an empty (immediately NUL-terminated) name, inspect-native-dispatch-metadata failed the whole inspection with execution_failure. It also threw away valid classes it had already decoded from the same image. That protocol is now recorded as a protocol-level partial-coverage failure, and the rest of the result is kept.

Problem and expected behavior

AppleObjcProtocols.record accepted "" as a decoded protocol name. The final objcSwiftMetadataSchema.parse then rejected objc_protocols[0].name, because protocol names must be nonempty. The class decoder already rejects an empty name inside its per-record boundary (AppleObjcDispatchFacets.ts), so a bad class becomes a recorded failure; protocols had no such check.

Reproduced on main with the issue's two generated 4,096-byte arm64 fixtures. They differ only in the protocol-name bytes:

fixture	before	after
valid protocol name	exit 0, class `RetainedClass`, protocol `RetainedProtocol`, coverage complete, 4,113 bytes	unchanged
empty protocol name	exit 1 `execution_failure`, no result, 940 bytes	exit 0, class `RetainedClass` retained, no protocol, `objc_class_method_ivar_metadata` partial: `Protocol 0x100000700: Objective-C protocol has an empty name`, 3,651 bytes

Change and scope

src/native/AppleObjcProtocols.ts: throw Objective-C protocol has an empty name inside the existing per-protocol try. The existing failure collection and AppleDispatchCoverage then turn it into partial coverage with the reason.
Adopted-protocol name lists (adopted_protocols, class protocols) accept any string in their schemas. They are unchanged: they report what the referencing record names, and the referenced declaration's own failure is recorded separately.

Contract and boundary impact

Semantic owner and earliest changed stage: the Objective-C protocol decoder, at the per-record decode boundary.
Contribution scope fit and linked maintainer decision: a bug fix within the existing static, read-only boundary; not applicable.
CLI and MCP/tool-catalog contract: schemas unchanged.
Provider, bridge, target-format, or platform compatibility: the Apple metadata reader only.
Evidence, artifact, provenance, or reconstruction contract: partial facts are now preserved, with a meaningful failure reason, instead of a whole-result failure.
Target selection, authority, upstream behavior, and cleanup details: not applicable.
Generated metadata, package, or installation impact: none.
Read existing files or captured data.

Evidence and regression coverage

Tests added or updated: src/native/AppleDispatchMetadata.test.ts gains a case that reuses the existing mapped class and protocol-list fixture with a protocol declaration. A nonempty name decodes, with complete coverage. An empty name keeps RetainedClass, records no protocol, and reports partial coverage with the exact reason.
Base reproduction: with main's AppleObjcProtocols.ts in place, the empty-name case fails (1 failed / 39 passed); with the fix it passes.
Real application: /System/Applications/Calculator.app/Contents/MacOS/Calculator (66 classes, 7 protocols), median of 5 CLI runs:

build	elapsed	output	normalized result SHA-256 (prefix)
main	481 ms	939,041 bytes	`b454adf667181131`
fix	484 ms (another run: 507 ms)	939,041 bytes	`b454adf667181131`

Results are identical for real images whose protocols have names. The one added check has no measurable cost.

Remaining proof gaps: no real compiler-produced image with an empty protocol name, so that case is covered only by the generated fixtures.

Validation performed

npx vitest run src/native/AppleDispatchMetadata.test.ts — 40 passed. Against main's decoder: 1 failed, as expected.
npx vitest run src/native tests/boundary/native tests/boundary/providers/native tests/boundary/filesystem/directAnalysisSnapshotReplay.test.ts tests/boundary/mcp/enhancedTools.01-lists-the-complete-target-open.test.ts tests/boundary/cli/nativeDispatchPropertyAttributes.test.ts — 53 files, 603 passed.
npm run check:fast — passed.
Real CLI: the issue's fixtures and Calculator, as tabulated.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: macOS arm64 Apple metadata reader.
Package or release metadata impact: none.
Security, privacy, process, or containment review: static parsing only.

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/4078 **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/3873-objc-empty-protocol-name` --- ### 📝 Commits (2) - [`96d9fe2`](https://github.com/morluto/rea/commit/96d9fe2e0cd768aef6a49f8086d0fbaa7614d99c) fix(native): record an empty Objective-C protocol name as a protocol failure - [`8a5e22c`](https://github.com/morluto/rea/commit/8a5e22cb5ddef3bc55fca805c133f63093231d99) fix(native): decode adopted protocols before rejecting an empty parent name ### 📊 Changes **2 files changed** (+50 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `src/native/AppleDispatchMetadata.test.ts` (+46 -0) 📝 `src/native/AppleObjcProtocols.ts` (+4 -0) </details> ### 📄 Description ## Summary Fixes #3873. If one Objective-C protocol declaration had an empty (immediately NUL-terminated) name, `inspect-native-dispatch-metadata` failed the whole inspection with `execution_failure`. It also threw away valid classes it had already decoded from the same image. That protocol is now recorded as a protocol-level partial-coverage failure, and the rest of the result is kept. ## Problem and expected behavior `AppleObjcProtocols.record` accepted `""` as a decoded protocol name. The final `objcSwiftMetadataSchema.parse` then rejected `objc_protocols[0].name`, because protocol names must be nonempty. The class decoder already rejects an empty name inside its per-record boundary (`AppleObjcDispatchFacets.ts`), so a bad class becomes a recorded failure; protocols had no such check. Reproduced on main with the issue's two generated 4,096-byte arm64 fixtures. They differ only in the protocol-name bytes: | fixture | before | after | |---|---|---| | valid protocol name | exit 0, class `RetainedClass`, protocol `RetainedProtocol`, coverage complete, 4,113 bytes | unchanged | | empty protocol name | exit 1 `execution_failure`, no result, 940 bytes | exit 0, class `RetainedClass` retained, no protocol, `objc_class_method_ivar_metadata` **partial**: `Protocol 0x100000700: Objective-C protocol has an empty name`, 3,651 bytes | ## Change and scope - `src/native/AppleObjcProtocols.ts`: throw `Objective-C protocol has an empty name` inside the existing per-protocol `try`. The existing failure collection and `AppleDispatchCoverage` then turn it into partial coverage with the reason. - Adopted-protocol name lists (`adopted_protocols`, class `protocols`) accept any string in their schemas. They are unchanged: they report what the referencing record names, and the referenced declaration's own failure is recorded separately. ## Contract and boundary impact - Semantic owner and earliest changed stage: the Objective-C protocol decoder, at the per-record decode boundary. - Contribution scope fit and linked maintainer decision: a bug fix within the existing static, read-only boundary; not applicable. - CLI and MCP/tool-catalog contract: schemas unchanged. - Provider, bridge, target-format, or platform compatibility: the Apple metadata reader only. - Evidence, artifact, provenance, or reconstruction contract: partial facts are now preserved, with a meaningful failure reason, instead of a whole-result failure. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: none. - [x] Read existing files or captured data. ## Evidence and regression coverage - Tests added or updated: `src/native/AppleDispatchMetadata.test.ts` gains a case that reuses the existing mapped class and protocol-list fixture with a protocol declaration. A nonempty name decodes, with complete coverage. An empty name keeps `RetainedClass`, records no protocol, and reports partial coverage with the exact reason. - Base reproduction: with main's `AppleObjcProtocols.ts` in place, the empty-name case fails (1 failed / 39 passed); with the fix it passes. - Real application: `/System/Applications/Calculator.app/Contents/MacOS/Calculator` (66 classes, 7 protocols), median of 5 CLI runs: | build | elapsed | output | normalized result SHA-256 (prefix) | |---|---|---|---| | main | 481 ms | 939,041 bytes | `b454adf667181131` | | fix | 484 ms (another run: 507 ms) | 939,041 bytes | `b454adf667181131` | Results are identical for real images whose protocols have names. The one added check has no measurable cost. - Remaining proof gaps: no real compiler-produced image with an empty protocol name, so that case is covered only by the generated fixtures. ## Validation performed - `npx vitest run src/native/AppleDispatchMetadata.test.ts` — 40 passed. Against main's decoder: 1 failed, as expected. - `npx vitest run src/native tests/boundary/native tests/boundary/providers/native tests/boundary/filesystem/directAnalysisSnapshotReplay.test.ts tests/boundary/mcp/enhancedTools.01-lists-the-complete-target-open.test.ts tests/boundary/cli/nativeDispatchPropertyAttributes.test.ts` — 53 files, 603 passed. - `npm run check:fast` — passed. - Real CLI: the issue's fixtures and Calculator, as tabulated. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: macOS arm64 Apple metadata reader. - Package or release metadata impact: none. - Security, privacy, process, or containment review: static parsing only. ## 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](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:11:25 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:46:32 +00:00

[PR #6285] [CLOSED] fix(javascript): attribute V8 role mismatch to runtime kind #7256

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

No description provided.

Rows
Columns