morluto/rea

Fork 0

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

[PR #4979] [MERGED] fix(native): preserve Mach-O install-name whitespace #6705

New issue

Closed

opened 2026-11-01 21:42:26 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:42:26 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4979
Author: @flyl1u
Created: 10/18/2026
Status: ✅ Merged
Merged: 10/22/2026
Merged by: @morluto

Base: main ← Head: codex/fix-macho-install-name-whitespace

📝 Commits (3)

dca461a fix(native): preserve whitespace in otool load-command strings
7f66bcc test(native): verify exact Mach-O install names through CLI and MCP
c416243 chore: merge main into Mach-O install-name preservation

📊 Changes

3 files changed (+174 additions, -2 deletions)

View changed files

📝 scripts/verify/apple/native-provenance.mjs (+120 -1)
📝 src/native/parsers/otool.ts (+11 -1)
📝 tests/boundary/providers/native/nativeLoadKinds.golden.test.ts (+43 -0)

📄 Description

Summary

Fixes #3384. Preserve whitespace in Mach-O install names and rpaths so inspect-macho reports the recorded dependency identity, including " leading.dylib ", consistently with import sources.

Continues the closed, unmerged #3430 by @bharadwajdaya. The original fix is cherry-picked with its author metadata preserved; this PR adapts it to current main and adds broader regressions and real CLI/stdio MCP verification.

Problem and expected behavior

The generic field parser trims the install name and can misread name version 2.dylib as a name version key. The offset-removal expression also consumes trailing whitespace belonging to the name. Parse the exact name/path <value> (offset N) format before generic fields and remove only the final display annotation.

Change and scope

Keep the correction at the owning otool parser. Preserve upstream linker-option parsing and the exclusion of LC_ID_DYLIB from dependencies. Extend the existing Apple verifier with independent compiled inputs; fixture executables are never run. CR/LF names remain outside this fix.

Contract and boundary impact

Semantic owner: parseOtoolLoadCommands; consumers use its parsed fields and dependency names directly.
Scope decision: not applicable; routine correctness fix within existing static-inspection boundaries.
CLI/MCP/catalog: existing arguments and schemas; corrected string values.
Compatibility: Apple otool load-command output; no new provider or dependency.
Evidence: preserve source names, raw load-command fields, provenance and adapter parity.
Authority/cleanup: existing inspection authority; verifier owns and removes its temporary inputs and closes MCP transports.
Generated metadata/package/install: no changes.

Expected product effects:

Read existing files or captured data.

Evidence and regression coverage

On base b83b9caf4, the expanded native-load test fails 3 cases (spaces, tabs, version prefix), with 57 passing. With the fix, all 60 pass.
Additional controls preserve an embedded (offset 99), a numeric name and an ordinary name; each checks dependency values and raw name/path fields.
Six relevant parser/provider files pass 132 tests, including malformed framing, missing/repeated fields, dylib-use options, linker options and self-identity coverage.
The Apple verifier compiles ordinary/spaced install names, checks real otool output and import sources, compares public CLI/MCP results, and verifies input bytes remain unchanged.
The ordinary control expects dyld_info's native leading abbreviation. The padded name retains its full import source; the verifier checks both cases explicitly.
Real macOS CLI/MCP E2E passed on this PR's exact head, 7f66bccdadfe16abf72d0b3802b58470f3490b6a.
Remaining gate: upstream PR workflows await maintainer approval for the external contribution. Historical #3430 results are not claimed as this PR's validation.

Verified field projection from both adapters (other dependency/import items omitted):

{
  "dependencies": { "items": [{ "path": "  leading.dylib  " }] },
  "imports": { "items": [{ "name": "_exported_value", "source": "  leading.dylib  " }] }
}

Validation performed

Windows x64, Node 24.18.0 / npm 11.16.0.
Focused tests below: 132 passed.
npm run check: passed (typecheck, lint, formatting, knip, metadata and architecture guards).
npm run check:fast: passed in pre-push.
git diff --check: passed.
npm run verify:macos-bundle: passed with real Apple utilities and public CLI/stdio MCP on the macos-15 runner; both install-name cases retained their bytes and matched across adapters.
The Apple lane's existing focused regressions: 19 files, 261 passed, 4 skipped.
Full fork CI: passed, 39 jobs concluded with no failures, on exact head 7f66bccdadfe16abf72d0b3802b58470f3490b6a. This is fork verification; upstream PR workflows still require maintainer approval.

Focused test command and local prerequisite

npm run test:focused -- tests/boundary/providers/native/nativeLoadKinds.golden.test.ts tests/boundary/providers/native/nativeLoadText.golden.test.ts tests/boundary/providers/native/nativeInventory.golden.test.ts tests/boundary/providers/native/nativeMachoHeader.golden.test.ts tests/boundary/providers/native/nativeTextRobustness.test.ts src/native/NativeMachoInspection.test.ts

The first attempt stopped before tests because Windows native controls were absent. The matching 6.6.0 controls were restored from upstream CI run 39100876382 at the same base SHA; subsequent baseline and fixed runs completed. These ignored artifacts are not included in the PR.

Compatibility, safety, and release

No schema migration or package metadata changes.
Real platform coverage: macos-15 CLI/MCP lane passed; no Hopper/Ghidra claim.
Static parsing only. Verification compiles disposable self-owned inputs and launches CLT/REA subprocesses without executing the fixture programs.

Merge follow-up: integrated main 15ec8e98d, adapting the install-name verifier to the current advertised MCP-schema helper and named output contract. On macOS with Node 24.18.0, all six focused parser/provider files passed 142 tests and check:ci:fast passed all five tasks. npm run verify:macos-bundle passed locally with real Apple tools, public CLI and stdio MCP: ordinary and whitespace-padded install names and padded rpaths matched across adapters, original bytes remained unchanged, and temporary DMG cleanup completed. Hosted checks on the merged head are pending.

Author readiness

Human owner: @flyl1u

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.

Co-authored-by: Bhaaraadwaj bharadwajdaya@gmail.com

_{🔄 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/4979 **Author:** [@flyl1u](https://github.com/flyl1u) **Created:** 10/18/2026 **Status:** ✅ Merged **Merged:** 10/22/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `codex/fix-macho-install-name-whitespace` --- ### 📝 Commits (3) - [`dca461a`](https://github.com/morluto/rea/commit/dca461afc60b178939859ec67a901b98203de34f) fix(native): preserve whitespace in otool load-command strings - [`7f66bcc`](https://github.com/morluto/rea/commit/7f66bccdadfe16abf72d0b3802b58470f3490b6a) test(native): verify exact Mach-O install names through CLI and MCP - [`c416243`](https://github.com/morluto/rea/commit/c416243344a69b8785b668224fb8de6e50bacb18) chore: merge main into Mach-O install-name preservation ### 📊 Changes **3 files changed** (+174 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `scripts/verify/apple/native-provenance.mjs` (+120 -1) 📝 `src/native/parsers/otool.ts` (+11 -1) 📝 `tests/boundary/providers/native/nativeLoadKinds.golden.test.ts` (+43 -0) </details> ### 📄 Description ## Summary Fixes #3384. Preserve whitespace in Mach-O install names and rpaths so `inspect-macho` reports the recorded dependency identity, including `" leading.dylib "`, consistently with import sources. Continues the closed, unmerged #3430 by @bharadwajdaya. The original fix is cherry-picked with its author metadata preserved; this PR adapts it to current main and adds broader regressions and real CLI/stdio MCP verification. ## Problem and expected behavior The generic field parser trims the install name and can misread `name version 2.dylib` as a `name version` key. The offset-removal expression also consumes trailing whitespace belonging to the name. Parse the exact `name/path <value> (offset N)` format before generic fields and remove only the final display annotation. ## Change and scope Keep the correction at the owning `otool` parser. Preserve upstream linker-option parsing and the exclusion of `LC_ID_DYLIB` from dependencies. Extend the existing Apple verifier with independent compiled inputs; fixture executables are never run. CR/LF names remain outside this fix. ## Contract and boundary impact - Semantic owner: `parseOtoolLoadCommands`; consumers use its parsed fields and dependency names directly. - Scope decision: not applicable; routine correctness fix within existing static-inspection boundaries. - CLI/MCP/catalog: existing arguments and schemas; corrected string values. - Compatibility: Apple `otool` load-command output; no new provider or dependency. - Evidence: preserve source names, raw load-command fields, provenance and adapter parity. - Authority/cleanup: existing inspection authority; verifier owns and removes its temporary inputs and closes MCP transports. - Generated metadata/package/install: no changes. Expected product effects: - [x] Read existing files or captured data. ## Evidence and regression coverage - On base `b83b9caf4`, the expanded native-load test fails 3 cases (spaces, tabs, `version` prefix), with 57 passing. With the fix, all 60 pass. - Additional controls preserve an embedded `(offset 99)`, a numeric name and an ordinary name; each checks dependency values and raw `name`/`path` fields. - Six relevant parser/provider files pass 132 tests, including malformed framing, missing/repeated fields, dylib-use options, linker options and self-identity coverage. - The Apple verifier compiles ordinary/spaced install names, checks real `otool` output and import sources, compares public CLI/MCP results, and verifies input bytes remain unchanged. - The ordinary control expects `dyld_info`'s native `leading` abbreviation. The padded name retains its full import source; the verifier checks both cases explicitly. - [Real macOS CLI/MCP E2E passed](https://github.com/flyl1u/rea/actions/runs/39104762556/job/117608578949) on this PR's exact head, `7f66bccdadfe16abf72d0b3802b58470f3490b6a`. - Remaining gate: upstream PR workflows await maintainer approval for the external contribution. Historical #3430 results are not claimed as this PR's validation. Verified field projection from both adapters (other dependency/import items omitted): ```json { "dependencies": { "items": [{ "path": " leading.dylib " }] }, "imports": { "items": [{ "name": "_exported_value", "source": " leading.dylib " }] } } ``` ## Validation performed - Windows x64, Node 24.18.0 / npm 11.16.0. - Focused tests below: **132 passed**. - `npm run check`: **passed** (typecheck, lint, formatting, knip, metadata and architecture guards). - `npm run check:fast`: **passed** in pre-push. - `git diff --check`: **passed**. - `npm run verify:macos-bundle`: **passed** with real Apple utilities and public CLI/stdio MCP on the `macos-15` runner; both install-name cases retained their bytes and matched across adapters. - The Apple lane's existing focused regressions: **19 files, 261 passed, 4 skipped**. - [Full fork CI](https://github.com/flyl1u/rea/actions/runs/39104762556): **passed**, 39 jobs concluded with no failures, on exact head `7f66bccdadfe16abf72d0b3802b58470f3490b6a`. This is fork verification; upstream PR workflows still require maintainer approval. <details> <summary>Focused test command and local prerequisite</summary> ```sh npm run test:focused -- tests/boundary/providers/native/nativeLoadKinds.golden.test.ts tests/boundary/providers/native/nativeLoadText.golden.test.ts tests/boundary/providers/native/nativeInventory.golden.test.ts tests/boundary/providers/native/nativeMachoHeader.golden.test.ts tests/boundary/providers/native/nativeTextRobustness.test.ts src/native/NativeMachoInspection.test.ts ``` The first attempt stopped before tests because Windows native controls were absent. The matching 6.6.0 controls were restored from upstream CI run 39100876382 at the same base SHA; subsequent baseline and fixed runs completed. These ignored artifacts are not included in the PR. </details> ## Compatibility, safety, and release - No schema migration or package metadata changes. - Real platform coverage: `macos-15` CLI/MCP lane passed; no Hopper/Ghidra claim. - Static parsing only. Verification compiles disposable self-owned inputs and launches CLT/REA subprocesses without executing the fixture programs. Merge follow-up: integrated main `15ec8e98d`, adapting the install-name verifier to the current advertised MCP-schema helper and named output contract. On macOS with Node 24.18.0, all six focused parser/provider files passed **142 tests** and `check:ci:fast` passed all five tasks. `npm run verify:macos-bundle` passed locally with real Apple tools, public CLI and stdio MCP: ordinary and whitespace-padded install names and padded rpaths matched across adapters, original bytes remained unchanged, and temporary DMG cleanup completed. Hosted checks on the merged head are pending. ## Author readiness Human owner: @flyl1u - [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 - [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. - [ ] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. Co-authored-by: Bhaaraadwaj <bharadwajdaya@gmail.com> --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:42:26 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

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

[PR #6705] [CLOSED] fix(flutter): accept GNU build IDs shorter than four bytes #7394

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

No description provided.

Rows
Columns