morluto/rea

Fork 0

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

[PR #4051] [MERGED] fix(native): report an object file's unnamed segment by its empty name #6021

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/3875-macho-empty-segment-name

📝 Commits (1)

ff27cd4 fix(native): report an object file's unnamed segment by its empty name

📊 Changes

3 files changed (+76 additions, -10 deletions)

View changed files

📝 src/native/parsers/otool.ts (+15 -6)
➕ tests/boundary/cli/nativeMachoObjectSegmentCli.test.ts (+55 -0)
📝 tests/boundary/providers/native/nativeLoadKinds.golden.test.ts (+6 -4)

📄 Description

Summary

Fixes #3875. inspect-macho replaced an empty Mach-O segment or section name with the display label "unknown". For an ordinary relocatable object (MH_OBJECT), the exhaustive segment inventory therefore reported a segment literally named unknown, while the raw LC_SEGMENT_64 field in the same response showed segname: "". Names are now reported exactly as observed.

Problem and expected behavior

mach-o/loader.h places all sections of an object file in one unnamed segment, so "" is a known source value, not missing data. Reproduced on main with the issue's command and a real clang -c object:

{"file_type":"1","raw_segnames":[""],"segments":[{"name":"unknown","sections":["__TEXT/__text","__LD/__compact_unwind"]}],"exhaustive":true}

The "unknown" projection came from #1995. That PR admitted empty segname and sectname rows and kept the label "as main did"; it did not decide the semantics. That violates the rule in AGENTS.md to keep identity values separate from display formatting.

Change and scope

nonEmptyName (an empty or absent name became "unknown") is replaced by machoName, which returns the observed text.
segname and sectname are already required unique fields, so absence cannot reach the projection. machoName throws rather than inventing a label if that guarantee is ever broken.
Segment and section names for ordinary named segments are unchanged.

Contract and boundary impact

Semantic owner and earliest changed stage: src/native/parsers/otool.ts segment and section projection. Consumers (NativeMachoInspection image base and locations, Mach-O views) never key on the name.
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: schema unchanged (name: string). An empty name is now "" instead of "unknown".
Provider, bridge, target-format, or platform compatibility: macOS native provider only.
Evidence, artifact, provenance, or reconstruction contract: normalized names now agree with the raw load-command fields.
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:
- New tests/boundary/cli/nativeMachoObjectSegmentCli.test.ts (macOS only): a public CLI workflow on a real xcrun clang -c object. It checks that the raw segname is "", the normalized segment name is "", and the inventory is exhaustive.
- nativeLoadKinds.golden.test.ts: the unnamed-segment test asserted the "unknown" label. It now asserts "" for both the segment and an all-NUL section name, and that the raw field matches.
Base reproduction: with main's otool.ts restored, both files fail (2 failed / 47 passed). With the fix, they pass.
User-visible CLI output, inspect-macho --json, macOS arm64:

target	before	after
`clang -c` object `qa.o`	segment `"unknown"`, 6,725 bytes, 1,055 ms	segment `""`, 6,718 bytes, 953–972 ms (3 runs)
`/Applications/mpv.app/Contents/Frameworks/libMoltenVK.dylib` (named segments)	114,880 bytes	114,880 bytes, byte-identical size, 1,021 ms

Elapsed time is dominated by the external tools and unchanged within noise.

Remaining proof gaps: no Intel macOS run.

Validation performed

npm run test:focused -- tests/boundary/cli/nativeMachoObjectSegmentCli.test.ts tests/boundary/providers/native/nativeLoadKinds.golden.test.ts — 49 passed. The same files against main's parser: 2 failed, as expected.
npx vitest run tests/boundary/providers/native tests/boundary/native tests/boundary/cli/nativeMacho tests/boundary/cli/dylib tests/boundary/mcp/analysisView — 38 files, 369 passed.
npm run check:fast — passed.

Compatibility, safety, and release

Breaking changes or migration steps: none to the schema. Callers that matched the "unknown" label for object-file segments should match "", which is the actual Mach-O name.
Real Hopper/Ghidra, browser, or OS coverage: macOS arm64 Command Line Tools otool.
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/4051 **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/3875-macho-empty-segment-name` --- ### 📝 Commits (1) - [`ff27cd4`](https://github.com/morluto/rea/commit/ff27cd402bf7f7a526306821054c21098c966609) fix(native): report an object file's unnamed segment by its empty name ### 📊 Changes **3 files changed** (+76 additions, -10 deletions) <details> <summary>View changed files</summary> 📝 `src/native/parsers/otool.ts` (+15 -6) ➕ `tests/boundary/cli/nativeMachoObjectSegmentCli.test.ts` (+55 -0) 📝 `tests/boundary/providers/native/nativeLoadKinds.golden.test.ts` (+6 -4) </details> ### 📄 Description ## Summary Fixes #3875. `inspect-macho` replaced an empty Mach-O segment or section name with the display label `"unknown"`. For an ordinary relocatable object (`MH_OBJECT`), the exhaustive segment inventory therefore reported a segment literally named `unknown`, while the raw `LC_SEGMENT_64` field in the same response showed `segname: ""`. Names are now reported exactly as observed. ## Problem and expected behavior `mach-o/loader.h` places all sections of an object file in one unnamed segment, so `""` is a known source value, not missing data. Reproduced on main with the issue's command and a real `clang -c` object: ```json {"file_type":"1","raw_segnames":[""],"segments":[{"name":"unknown","sections":["__TEXT/__text","__LD/__compact_unwind"]}],"exhaustive":true} ``` The `"unknown"` projection came from #1995. That PR admitted empty `segname` and `sectname` rows and kept the label "as main did"; it did not decide the semantics. That violates the rule in AGENTS.md to keep identity values separate from display formatting. ## Change and scope - `nonEmptyName` (an empty or absent name became `"unknown"`) is replaced by `machoName`, which returns the observed text. - `segname` and `sectname` are already required unique fields, so absence cannot reach the projection. `machoName` throws rather than inventing a label if that guarantee is ever broken. - Segment and section names for ordinary named segments are unchanged. ## Contract and boundary impact - Semantic owner and earliest changed stage: `src/native/parsers/otool.ts` segment and section projection. Consumers (`NativeMachoInspection` image base and locations, Mach-O views) never key on the name. - 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: schema unchanged (`name: string`). An empty name is now `""` instead of `"unknown"`. - Provider, bridge, target-format, or platform compatibility: macOS native provider only. - Evidence, artifact, provenance, or reconstruction contract: normalized names now agree with the raw load-command fields. - 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: - New `tests/boundary/cli/nativeMachoObjectSegmentCli.test.ts` (macOS only): a public CLI workflow on a real `xcrun clang -c` object. It checks that the raw `segname` is `""`, the normalized segment name is `""`, and the inventory is exhaustive. - `nativeLoadKinds.golden.test.ts`: the unnamed-segment test asserted the `"unknown"` label. It now asserts `""` for both the segment and an all-NUL section name, and that the raw field matches. - Base reproduction: with main's `otool.ts` restored, both files fail (2 failed / 47 passed). With the fix, they pass. - User-visible CLI output, `inspect-macho --json`, macOS arm64: | target | before | after | |---|---|---| | `clang -c` object `qa.o` | segment `"unknown"`, 6,725 bytes, 1,055 ms | segment `""`, 6,718 bytes, 953–972 ms (3 runs) | | `/Applications/mpv.app/Contents/Frameworks/libMoltenVK.dylib` (named segments) | 114,880 bytes | 114,880 bytes, byte-identical size, 1,021 ms | Elapsed time is dominated by the external tools and unchanged within noise. - Remaining proof gaps: no Intel macOS run. ## Validation performed - `npm run test:focused -- tests/boundary/cli/nativeMachoObjectSegmentCli.test.ts tests/boundary/providers/native/nativeLoadKinds.golden.test.ts` — 49 passed. The same files against main's parser: 2 failed, as expected. - `npx vitest run tests/boundary/providers/native tests/boundary/native tests/boundary/cli/nativeMacho tests/boundary/cli/dylib tests/boundary/mcp/analysisView` — 38 files, 369 passed. - `npm run check:fast` — passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none to the schema. Callers that matched the `"unknown"` label for object-file segments should match `""`, which is the actual Mach-O name. - Real Hopper/Ghidra, browser, or OS coverage: macOS arm64 Command Line Tools `otool`. - 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:14 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

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

[PR #6021] [MERGED] fix(ci): validate artifact MCP replies against named schemas #7126

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

No description provided.

Rows
Columns