morluto/rea

Fork 0

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

[PR #3774] [MERGED] fix(native): resolve inspect_plist defaults from the selected target's bundle #5889

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/inspect-plist-target-root

📝 Commits (1)

9c6596d fix(native): resolve inspect_plist defaults from the selected target's bundle

📊 Changes

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

View changed files

📝 src/native/NativeMacOSProvider.ts (+17 -3)
📝 tests/boundary/cli/nativePlistCli.test.ts (+19 -6)
📝 tests/boundary/cli/nativePlistSelectionFailureCli.test.ts (+1 -1)

📄 Description

Summary

inspect-plist (MCP inspect_plist) failed on two inputs it accepts. The CLI documents its target as "App or Mach-O path" and --relative-path as relative to the app root:

Program file inside a bundle. rea inspect-plist /Applications/Ghostty.app/Contents/MacOS/ghostty looked for Contents/MacOS/Contents/Info.plist and returned capability_unavailable. --relative-path Contents/Info.plist resolved from Contents/MacOS/ instead of the app root.
Plist file target. rea inspect-plist /Applications/X.app/Contents/Info.plist looked for Contents/Contents/Info.plist instead of reading the file it was given.

Every app in a local /Applications sweep failed both ways. After this change, all of them resolve to the bundle's Contents/Info.plist.

Problem and expected behavior

resolvePlistPath used the bundle directory as the root when the target was a directory. For any file target it used dirname(target.path), and the default was always <root>/Contents/Info.plist. That rule holds for a bundle directory. It is wrong for a program file in <Bundle>/Contents/MacOS/ and for a plist artifact target.

Change and scope

The plist path owner, resolvePlistPath in src/native/NativeMacOSProvider.ts, now roots a file that sits directly under <Bundle>/Contents/MacOS/ at <Bundle>. This matches how bundle directories are already handled: the default is Contents/Info.plist, and --relative-path resolves from the app root.
A target the resolver classifies as a plist artifact is now its own default plist. A relative --relative-path still resolves from the plist's directory.
When the default plist is missing, the message now says "no readable default plist at " instead of "no readable Contents/Info.plist", because the default is no longer always that file.
Not included:
- Flat iOS-style bundles (X.app/<program>). From the path alone, a program file in such a bundle can't be told apart from any other file in a directory, so those targets keep the old behavior.
- BinaryTargetResolver stays unchanged. Recording bundleInfoPlist for file targets there would change trace_dylib_resolution, which treats that field as meaning sourcePath is the bundle directory.

Contract and boundary impact

Semantic owner and earliest changed stage: resolvePlistPath, the inspect_plist plist selection in the native macOS provider.
Contribution scope fit and linked maintainer decision: correctness fix within the existing read-only provider; not applicable.
CLI and MCP/tool-catalog contract: schemas unchanged. The default plist now matches the documented "app root" and "Info.plist from the active artifact" contracts. Wording of the missing-default message changed.
Provider, bridge, target-format, or platform compatibility: macOS native provider only.
Evidence, artifact, provenance, or reconstruction contract: source_path still reports the canonical plist read.
Target selection, authority, upstream behavior, and cleanup details: bundle directory targets and absolute --relative-path selections are unchanged.
Generated metadata, package, or installation impact: none.

Expected effects:

Read existing files or captured data.

Evidence and regression coverage

Tests added or updated: tests/boundary/cli/nativePlistCli.test.ts adds real CLI cases for a program-file default, a program file with --relative-path, and a plist-file target. nativePlistSelectionFailureCli.test.ts follows the message wording.
Base reproduction: against the unfixed src/native/NativeMacOSProvider.ts, the 3 new cases fail (Tests 3 failed | 7 passed). With the fix, all 10 pass.

User-visible CLI output, before:

$ rea inspect-plist /Applications/Ghostty.app/Contents/MacOS/ghostty --format json
{"code":"capability_unavailable",...,"message":"The target has no readable Contents/Info.plist at /Applications/Ghostty.app/Contents/MacOS/Contents/Info.plist: no file exists there. ..."}

After: normalized_result.source_path is /Applications/Ghostty.app/Contents/Info.plist.

Remaining proof gaps: flat iOS-style program targets (out of scope, see above).

Validation performed

npm run test:focused -- tests/boundary/cli/nativePlistCli.test.ts tests/boundary/cli/nativePlistSelectionFailureCli.test.ts: 15 passed
npm run test:focused -- tests/boundary/providers/native/nativeProvider.test.ts tests/boundary/cli/nativePlistPrototypeKeyCli.test.ts tests/acceptance/applications/appBundleEncodingCli.test.ts: 49 passed
npm run check:fast: passed
Real apps on macOS (Darwin 25.6, arm64): the built CLI was run against 35 bundles in /Applications, using both the CFBundleExecutable program file and Contents/Info.plist as targets. All 70 resolved to the bundle's canonical Contents/Info.plist (Safari's is reported at its Cryptex path). Before the fix, all 70 failed. Cost per call is unchanged, because the fix only adds path arithmetic (about 0.9 s per CLI invocation, mostly process startup).

Compatibility, safety, and release

Breaking changes or migration steps: none. Inputs that previously failed now succeed. A caller who passed --relative-path relative to Contents/MacOS/ with a program-file target now resolves from the app root, which is what the documentation says.
Real Hopper/Ghidra, browser, or OS coverage: macOS plutil only; no Hopper/Ghidra involvement.
Package or release metadata impact: none.
Security, privacy, process, or containment review: no new reads beyond the plist the caller selected or the bundle's own Info.plist.

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. (Found by sweeping real apps; no existing issue.)
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/3774 **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/inspect-plist-target-root` --- ### 📝 Commits (1) - [`9c6596d`](https://github.com/morluto/rea/commit/9c6596d80ea9e9d0e0727800e0a161308d76acf7) fix(native): resolve inspect_plist defaults from the selected target's bundle ### 📊 Changes **3 files changed** (+37 additions, -10 deletions) <details> <summary>View changed files</summary> 📝 `src/native/NativeMacOSProvider.ts` (+17 -3) 📝 `tests/boundary/cli/nativePlistCli.test.ts` (+19 -6) 📝 `tests/boundary/cli/nativePlistSelectionFailureCli.test.ts` (+1 -1) </details> ### 📄 Description ## Summary `inspect-plist` (MCP `inspect_plist`) failed on two inputs it accepts. The CLI documents its target as "App or Mach-O path" and `--relative-path` as relative to the app root: - **Program file inside a bundle.** `rea inspect-plist /Applications/Ghostty.app/Contents/MacOS/ghostty` looked for `Contents/MacOS/Contents/Info.plist` and returned `capability_unavailable`. `--relative-path Contents/Info.plist` resolved from `Contents/MacOS/` instead of the app root. - **Plist file target.** `rea inspect-plist /Applications/X.app/Contents/Info.plist` looked for `Contents/Contents/Info.plist` instead of reading the file it was given. Every app in a local `/Applications` sweep failed both ways. After this change, all of them resolve to the bundle's `Contents/Info.plist`. ## Problem and expected behavior `resolvePlistPath` used the bundle directory as the root when the target was a directory. For any file target it used `dirname(target.path)`, and the default was always `<root>/Contents/Info.plist`. That rule holds for a bundle directory. It is wrong for a program file in `<Bundle>/Contents/MacOS/` and for a plist artifact target. ## Change and scope - The plist path owner, `resolvePlistPath` in `src/native/NativeMacOSProvider.ts`, now roots a file that sits directly under `<Bundle>/Contents/MacOS/` at `<Bundle>`. This matches how bundle directories are already handled: the default is `Contents/Info.plist`, and `--relative-path` resolves from the app root. - A target the resolver classifies as a `plist` artifact is now its own default plist. A relative `--relative-path` still resolves from the plist's directory. - When the default plist is missing, the message now says "no readable default plist at <path>" instead of "no readable Contents/Info.plist", because the default is no longer always that file. - Not included: - Flat iOS-style bundles (`X.app/<program>`). From the path alone, a program file in such a bundle can't be told apart from any other file in a directory, so those targets keep the old behavior. - `BinaryTargetResolver` stays unchanged. Recording `bundleInfoPlist` for file targets there would change `trace_dylib_resolution`, which treats that field as meaning `sourcePath` is the bundle directory. ## Contract and boundary impact - Semantic owner and earliest changed stage: `resolvePlistPath`, the inspect_plist plist selection in the native macOS provider. - Contribution scope fit and linked maintainer decision: correctness fix within the existing read-only provider; not applicable. - CLI and MCP/tool-catalog contract: schemas unchanged. The default plist now matches the documented "app root" and "Info.plist from the active artifact" contracts. Wording of the missing-default message changed. - Provider, bridge, target-format, or platform compatibility: macOS native provider only. - Evidence, artifact, provenance, or reconstruction contract: `source_path` still reports the canonical plist read. - Target selection, authority, upstream behavior, and cleanup details: bundle directory targets and absolute `--relative-path` selections are unchanged. - Generated metadata, package, or installation impact: none. Expected effects: - [x] Read existing files or captured data. ## Evidence and regression coverage - Tests added or updated: `tests/boundary/cli/nativePlistCli.test.ts` adds real CLI cases for a program-file default, a program file with `--relative-path`, and a plist-file target. `nativePlistSelectionFailureCli.test.ts` follows the message wording. - Base reproduction: against the unfixed `src/native/NativeMacOSProvider.ts`, the 3 new cases fail (`Tests 3 failed | 7 passed`). With the fix, all 10 pass. - User-visible CLI output, before: ``` $ rea inspect-plist /Applications/Ghostty.app/Contents/MacOS/ghostty --format json {"code":"capability_unavailable",...,"message":"The target has no readable Contents/Info.plist at /Applications/Ghostty.app/Contents/MacOS/Contents/Info.plist: no file exists there. ..."} ``` After: `normalized_result.source_path` is `/Applications/Ghostty.app/Contents/Info.plist`. - Remaining proof gaps: flat iOS-style program targets (out of scope, see above). ## Validation performed - `npm run test:focused -- tests/boundary/cli/nativePlistCli.test.ts tests/boundary/cli/nativePlistSelectionFailureCli.test.ts`: 15 passed - `npm run test:focused -- tests/boundary/providers/native/nativeProvider.test.ts tests/boundary/cli/nativePlistPrototypeKeyCli.test.ts tests/acceptance/applications/appBundleEncodingCli.test.ts`: 49 passed - `npm run check:fast`: passed - Real apps on macOS (Darwin 25.6, arm64): the built CLI was run against 35 bundles in `/Applications`, using both the `CFBundleExecutable` program file and `Contents/Info.plist` as targets. All 70 resolved to the bundle's canonical `Contents/Info.plist` (Safari's is reported at its Cryptex path). Before the fix, all 70 failed. Cost per call is unchanged, because the fix only adds path arithmetic (about 0.9 s per CLI invocation, mostly process startup). ## Compatibility, safety, and release - Breaking changes or migration steps: none. Inputs that previously failed now succeed. A caller who passed `--relative-path` relative to `Contents/MacOS/` with a program-file target now resolves from the app root, which is what the documentation says. - Real Hopper/Ghidra, browser, or OS coverage: macOS plutil only; no Hopper/Ghidra involvement. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new reads beyond the plist the caller selected or the bundle's own Info.plist. ## 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. (Found by sweeping real apps; no existing issue.) - [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:12 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

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

[PR #6427] [MERGED] test(process): print the progress frames when the sparse-child bound fails #7353

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

No description provided.

Rows
Columns