morluto/rea

Fork 0

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

[PR #3548] [MERGED] fix(hopper): skip non-file launcher candidates #5814

New issue

Closed

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

bsakaguchi commented

2026-10-31 01:09:38 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3548
Author: @N0zoM1z0
Created: 10/14/2026
Status: ✅ Merged
Merged: 10/14/2026
Merged by: @N0zoM1z0

Base: main ← Head: review/macos-hopper-discovery-3085-20261014

📝 Commits (2)

0f98b74 fix(hopper): skip non-file launcher candidates
2e362e4 test(hopper): respect installed system bundles in discovery fixtures

📊 Changes

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

View changed files

📝 docs/installation.md (+2 -0)
📝 src/config/parseConfig.ts (+2 -2)
➕ tests/boundary/filesystem/hopperLauncherSelection.test.ts (+34 -0)

📄 Description

Summary

Automatic Hopper discovery now requires an executable regular file. A directory at ~/Applications/Hopper Disassembler.app/Contents/MacOS/hopper (or in an earlier renamed bundle) previously satisfied POSIX X_OK, hiding a later valid launcher and causing startup to fail.

Follow-up to #3085 and #3054. Their shared macOS path discovery already landed independently in 7582f0dff2; this PR fixes a separate, reproduced file-kind omission in that implementation.

Change and scope

Keep candidate ordering and lazy discovery with the existing shared path owner.
Make configuration's filesystem predicate check statSync(...).isFile() after executable access, matching Doctor's existing file-kind requirement. File symlinks remain eligible; directories and failed probes are skipped.
Add two real-filesystem regressions using the production predicate, and clarify the installation guide.

Contract and boundary impact

Semantic owner: configuration's launcher eligibility predicate; no downstream path repair or duplicate bundle interpretation.
Scope: ordinary fix to an existing provider; additional capability/authority decision is not applicable.
CLI/MCP, Evidence and session contracts: unchanged. Explicit HOPPER_LAUNCHER_PATH remains caller-selected.
Discovery reads filesystem metadata; it does not launch Hopper or install software.
Generated metadata and package changes: none.

Evidence and validation

Executed on Linux with Node 24.18.0 / npm 11.16.0:

Base reproduction: both new regression cases failed on the original predicate, selecting a directory instead of the later regular file.
Related suites: 119 passed across seven files, covering configuration, shared paths, setup, Doctor and public CLI diagnostics. Final focused regression rerun on 2e362e435: 2 passed, none skipped. The fixtures now detect existing canonical or renamed system installations before creating owned user bundles.
npm run check: six tasks and architecture guards passed. npm run docs:check: passed, including five consistency tests. npm run verify:test-discovery: all 1,285 test files discovered exactly once. Final touched-file formatting and npm run check:fast (typecheck/lint) passed after the fixture-isolation correction.
Compiled production discovery against a genuine Hopper 6.4.2 Linux ELF, linked into an isolated macOS bundle layout: canonical and renamed file candidates selected; replacing the launcher with a directory now skips it. Original target SHA-256 stayed 0294ced141cc373468ee22d8343e7dac41980cb05a937994ca81c9f09afe7ded.
Public rea doctor --provider hopper --json against that actual ELF preserved the configured path and reported config_drift (exit 1). Host dependency inspection found required GLIBC 2.38 and Qt libraries unavailable. Successful real Hopper analysis and native macOS execution remain unverified. The simulated bundle layout establishes filesystem selection only.

Performance

No tool output or analysis-context expansion: discovery still returns one path and uses the existing lazy candidate iterator. The change adds one file-kind lookup per candidate that passes executable access; no new cache is warranted for this bounded startup check.

Single fresh-process filesystem probe: canonical 0.31 ms, renamed 0.74 ms, directory rejection 0.22 ms; total process 0.23 s, user/system CPU 0.145/0.060 s, sampled peak family RSS 28 MiB. These are observations, not a statistical speedup claim. Agent token usage was not measured.

Validation ran serially under two-CPU / 8 GiB guards: related suites peaked at 3.14 GiB, static checks at 4.27 GiB; no guard stop or OOM.

Compatibility and release

No breaking API, migration or release metadata change. No target-byte mutation, process execution authority, network traffic, or installation is added by discovery. Real-provider/platform gaps are recorded above.

Author readiness

Human owner: @N0zoM1z0

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.

_{🔄 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/3548 **Author:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Created:** 10/14/2026 **Status:** ✅ Merged **Merged:** 10/14/2026 **Merged by:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Base:** `main` ← **Head:** `review/macos-hopper-discovery-3085-20261014` --- ### 📝 Commits (2) - [`0f98b74`](https://github.com/morluto/rea/commit/0f98b7408eb1cc403f52c1d2a78a531df5c511a9) fix(hopper): skip non-file launcher candidates - [`2e362e4`](https://github.com/morluto/rea/commit/2e362e4358698e1ae04d1631e5b90854311ad8bd) test(hopper): respect installed system bundles in discovery fixtures ### 📊 Changes **3 files changed** (+38 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `docs/installation.md` (+2 -0) 📝 `src/config/parseConfig.ts` (+2 -2) ➕ `tests/boundary/filesystem/hopperLauncherSelection.test.ts` (+34 -0) </details> ### 📄 Description ## Summary Automatic Hopper discovery now requires an executable regular file. A directory at `~/Applications/Hopper Disassembler.app/Contents/MacOS/hopper` (or in an earlier renamed bundle) previously satisfied POSIX `X_OK`, hiding a later valid launcher and causing startup to fail. Follow-up to #3085 and #3054. Their shared macOS path discovery already landed independently in 7582f0dff29ae0e80c97b885ddd386ac61a92570; this PR fixes a separate, reproduced file-kind omission in that implementation. ## Change and scope - Keep candidate ordering and lazy discovery with the existing shared path owner. - Make configuration's filesystem predicate check `statSync(...).isFile()` after executable access, matching Doctor's existing file-kind requirement. File symlinks remain eligible; directories and failed probes are skipped. - Add two real-filesystem regressions using the production predicate, and clarify the installation guide. ## Contract and boundary impact - Semantic owner: configuration's launcher eligibility predicate; no downstream path repair or duplicate bundle interpretation. - Scope: ordinary fix to an existing provider; additional capability/authority decision is not applicable. - CLI/MCP, Evidence and session contracts: unchanged. Explicit `HOPPER_LAUNCHER_PATH` remains caller-selected. - Discovery reads filesystem metadata; it does not launch Hopper or install software. - Generated metadata and package changes: none. ## Evidence and validation Executed on Linux with Node 24.18.0 / npm 11.16.0: - **Base reproduction:** both new regression cases failed on the original predicate, selecting a directory instead of the later regular file. - Related suites: **119 passed across seven files**, covering configuration, shared paths, setup, Doctor and public CLI diagnostics. Final focused regression rerun on 2e362e435: **2 passed, none skipped**. The fixtures now detect existing canonical or renamed system installations before creating owned user bundles. - `npm run check`: six tasks and architecture guards passed. `npm run docs:check`: passed, including five consistency tests. `npm run verify:test-discovery`: all 1,285 test files discovered exactly once. Final touched-file formatting and `npm run check:fast` (typecheck/lint) passed after the fixture-isolation correction. - Compiled production discovery against a genuine Hopper 6.4.2 Linux ELF, linked into an isolated macOS bundle layout: canonical and renamed file candidates selected; replacing the launcher with a directory now skips it. Original target SHA-256 stayed `0294ced141cc373468ee22d8343e7dac41980cb05a937994ca81c9f09afe7ded`. - Public `rea doctor --provider hopper --json` against that actual ELF preserved the configured path and reported `config_drift` (exit 1). Host dependency inspection found required GLIBC 2.38 and Qt libraries unavailable. **Successful real Hopper analysis and native macOS execution remain unverified.** The simulated bundle layout establishes filesystem selection only. ## Performance No tool output or analysis-context expansion: discovery still returns one path and uses the existing lazy candidate iterator. The change adds one file-kind lookup per candidate that passes executable access; no new cache is warranted for this bounded startup check. Single fresh-process filesystem probe: canonical 0.31 ms, renamed 0.74 ms, directory rejection 0.22 ms; total process 0.23 s, user/system CPU 0.145/0.060 s, sampled peak family RSS 28 MiB. These are observations, not a statistical speedup claim. Agent token usage was not measured. Validation ran serially under two-CPU / 8 GiB guards: related suites peaked at 3.14 GiB, static checks at 4.27 GiB; no guard stop or OOM. ## Compatibility and release No breaking API, migration or release metadata change. No target-byte mutation, process execution authority, network traffic, or installation is added by discovery. Real-provider/platform gaps are recorded above. ## Author readiness Human owner: @N0zoM1z0 - [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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:09:38 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

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

[PR #5814] [MERGED] fix(cli): preserve Unicode and text across token windows #7078

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

No description provided.

Rows
Columns