[PR #5602] [CLOSED] fix(cli): route analyze based on bundle layout, not directory name suffix #6992

Closed
opened 2026-11-01 21:44:33 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5602
Author: @pardeep1916P
Created: 10/20/2026
Status: ❌ Closed

Base: main ← Head: fix/analyze-workflow-selection-app-bundles


📝 Commits (7)

  • e852b47 fix(cli): route analyze based on bundle layout, not directory name suffix
  • e6625be test(cli): use cliTest fixture in app bundle routing regression tests
  • 7942562 test(cli): match statistics rather than summary in js application analysis assertion
  • dd1bba2 Merge remote-tracking branch 'upstream/main' into fix/analyze-workflow-selection-app-bundles
  • 087c9e1 Merge remote-tracking branch 'upstream/main' into fix/analyze-workflow-selection-app-bundles
  • fc7cfe7 Merge remote-tracking branch 'upstream/main' into fix/analyze-workflow-selection-app-bundles
  • 653735f test(applications): reconcile unit test suite with upstream bundle root helpers

📄 Description

Summary

Fixes #5595. rea analyze decided between JS application analysis and native analysis by checking path.endsWith(".app"). That string check broke in two common cases: symlinks pointing to .app bundles were misrouted to empty JS analysis, while regular JS directories named .app failed in native binary analysis.

Analysis routing now checks whether the target path actually has an application bundle layout (macOS Contents/Info.plist, root Info.plist, or wrapper layouts) before falling back to directory-based JavaScript analysis.

Problem and expected behavior

When path.endsWith(".app") was the only discriminator:

  1. rea analyze bundle-alias (symlink to Sample.app) didn't match .app, so it treated the symlink as an ordinary directory and ran analyze-javascript-application, returning zero relevant files and empty JS evidence.
  2. rea analyze web.app (a normal JS project with package.json) matched .app, so it was forwarded to native binary analysis and failed with target_unavailable because no Info.plist existed.

Expected: determine whether a path is an application bundle by inspecting its actual bundle structure (Contents/Info.plist, root Info.plist, or wrapper layouts) rather than relying on the folder name suffix.

Change and scope

  • Export isAppBundlePath(path) from src/application/AppBundleExecutable.ts using the existing resolveAppBundleResourceRoot layout resolver.
  • Use isAppBundlePath(path) in src/cli/coreBinaryCommands.ts (isJavaScriptApplicationPath) instead of checking lower.endsWith(".app").
  • Add unit tests in src/application/AppBundleExecutable.test.ts for bundle detection across aliases and non-bundle folders.
  • Add CLI boundary tests in tests/boundary/cli/analyzeAppBundlePathCli.test.ts for both symlink bundle routing and JS .app directory routing.

Contract and boundary impact

  • Semantic owner: CLI binary analysis routing (src/cli/coreBinaryCommands.ts) and bundle resolution (src/application/AppBundleExecutable.ts).
  • Contribution scope: bug fix within existing CLI routing boundaries.
  • CLI and MCP contracts: unchanged.
  • Platform compatibility: platform-neutral; follows standard macOS and iOS bundle conventions.
  • Evidence and provenance: unchanged.
  • Target interaction: read-only filesystem inspection; no processes launched or modified.
  • Generated metadata: none.

Expected effects:

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration.
  • Contact network services.
  • Recover protected code or work around protections.
  • Documentation or usability only; no target interaction.
  • Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

  • Tests added:
    • src/application/AppBundleExecutable.test.ts: verifies isAppBundlePath for macOS bundles, symlink aliases, non-bundle .app folders, and missing/regular files.
    • tests/boundary/cli/analyzeAppBundlePathCli.test.ts: CLI boundary tests ensuring bundle-alias routes to native provider selection, and web.app routes to JS analysis with exit 0.
  • Base reproduction: both routing bugs reproduced on base revision 90e433df0.
  • Peer verification: @x1mm4nu3l independently verified all 13 tests on Linux / Node 24.18.0, and confirmed flat iOS and wrapper alias layouts route properly.
  • Output: rea analyze web.app --json exits 0 with format: "directory"; rea analyze bundle-alias --json stops at native provider selection.
  • Remaining gaps: native providers (Hopper/Ghidra) were not invoked; tests assert routing up to provider selection.

For evidence-bearing changes:

  • Observed, derived, and inferred claims remain distinguishable.
  • Artifact identity, source provenance, and failed attempts remain preserved.
  • Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

  • npm run test:local -- src/application/AppBundleExecutable.test.ts — 7 passed
  • npm run test:focused -- tests/boundary/cli/analyzeAppBundlePathCli.test.ts — 6 passed
  • npm run check:fast — 2 tasks successful (typecheck and lint clean)
  • Peer verification on Linux / Node 24.18.0 by @x1mm4nu3l — all 13 tests passed, plus flat iOS and wrapper alias checks

Compatibility, safety, and release

  • Breaking changes: none.
  • Security and safety: read-only layout check; no code execution or target mutation.

Author readiness

Human owner: @pardeep1916P

  • 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.

🔄 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/5602 **Author:** [@pardeep1916P](https://github.com/pardeep1916P) **Created:** 10/20/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/analyze-workflow-selection-app-bundles` --- ### 📝 Commits (7) - [`e852b47`](https://github.com/morluto/rea/commit/e852b47972ac5c3e6934c3a35cbae39c6becfdba) fix(cli): route analyze based on bundle layout, not directory name suffix - [`e6625be`](https://github.com/morluto/rea/commit/e6625bef9df79e07f939ed1a95b28f0ed0c3d2f0) test(cli): use cliTest fixture in app bundle routing regression tests - [`7942562`](https://github.com/morluto/rea/commit/7942562e7514061b4cab31851677307b15b705e6) test(cli): match statistics rather than summary in js application analysis assertion - [`dd1bba2`](https://github.com/morluto/rea/commit/dd1bba2d0f5a62e6ee2d48c93f408913ac364a02) Merge remote-tracking branch 'upstream/main' into fix/analyze-workflow-selection-app-bundles - [`087c9e1`](https://github.com/morluto/rea/commit/087c9e169df6e3a99beb1e35d524a2c171132c94) Merge remote-tracking branch 'upstream/main' into fix/analyze-workflow-selection-app-bundles - [`fc7cfe7`](https://github.com/morluto/rea/commit/fc7cfe76c165d8ebd7589d68f2efb8f7bcef69b2) Merge remote-tracking branch 'upstream/main' into fix/analyze-workflow-selection-app-bundles - [`653735f`](https://github.com/morluto/rea/commit/653735f531f1a85690351e699e59081ad3fb7869) test(applications): reconcile unit test suite with upstream bundle root helpers ### 📄 Description ## Summary Fixes #5595. `rea analyze` decided between JS application analysis and native analysis by checking `path.endsWith(".app")`. That string check broke in two common cases: symlinks pointing to `.app` bundles were misrouted to empty JS analysis, while regular JS directories named `.app` failed in native binary analysis. Analysis routing now checks whether the target path actually has an application bundle layout (macOS `Contents/Info.plist`, root `Info.plist`, or wrapper layouts) before falling back to directory-based JavaScript analysis. ## Problem and expected behavior When `path.endsWith(".app")` was the only discriminator: 1. `rea analyze bundle-alias` (symlink to `Sample.app`) didn't match `.app`, so it treated the symlink as an ordinary directory and ran `analyze-javascript-application`, returning zero relevant files and empty JS evidence. 2. `rea analyze web.app` (a normal JS project with `package.json`) matched `.app`, so it was forwarded to native binary analysis and failed with `target_unavailable` because no `Info.plist` existed. Expected: determine whether a path is an application bundle by inspecting its actual bundle structure (`Contents/Info.plist`, root `Info.plist`, or wrapper layouts) rather than relying on the folder name suffix. ## Change and scope - Export `isAppBundlePath(path)` from `src/application/AppBundleExecutable.ts` using the existing `resolveAppBundleResourceRoot` layout resolver. - Use `isAppBundlePath(path)` in `src/cli/coreBinaryCommands.ts` (`isJavaScriptApplicationPath`) instead of checking `lower.endsWith(".app")`. - Add unit tests in `src/application/AppBundleExecutable.test.ts` for bundle detection across aliases and non-bundle folders. - Add CLI boundary tests in `tests/boundary/cli/analyzeAppBundlePathCli.test.ts` for both symlink bundle routing and JS `.app` directory routing. ## Contract and boundary impact - Semantic owner: CLI binary analysis routing (`src/cli/coreBinaryCommands.ts`) and bundle resolution (`src/application/AppBundleExecutable.ts`). - Contribution scope: bug fix within existing CLI routing boundaries. - CLI and MCP contracts: unchanged. - Platform compatibility: platform-neutral; follows standard macOS and iOS bundle conventions. - Evidence and provenance: unchanged. - Target interaction: read-only filesystem inspection; no processes launched or modified. - Generated metadata: none. Expected effects: - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added: - `src/application/AppBundleExecutable.test.ts`: verifies `isAppBundlePath` for macOS bundles, symlink aliases, non-bundle `.app` folders, and missing/regular files. - `tests/boundary/cli/analyzeAppBundlePathCli.test.ts`: CLI boundary tests ensuring `bundle-alias` routes to native provider selection, and `web.app` routes to JS analysis with exit 0. - Base reproduction: both routing bugs reproduced on base revision `90e433df0`. - Peer verification: @x1mm4nu3l independently verified all 13 tests on Linux / Node 24.18.0, and confirmed flat iOS and wrapper alias layouts route properly. - Output: `rea analyze web.app --json` exits 0 with `format: "directory"`; `rea analyze bundle-alias --json` stops at native provider selection. - Remaining gaps: native providers (Hopper/Ghidra) were not invoked; tests assert routing up to provider selection. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npm run test:local -- src/application/AppBundleExecutable.test.ts` — 7 passed - `npm run test:focused -- tests/boundary/cli/analyzeAppBundlePathCli.test.ts` — 6 passed - `npm run check:fast` — 2 tasks successful (typecheck and lint clean) - Peer verification on Linux / Node 24.18.0 by @x1mm4nu3l — all 13 tests passed, plus flat iOS and wrapper alias checks ## Compatibility, safety, and release - Breaking changes: none. - Security and safety: read-only layout check; no code execution or target mutation. ## Author readiness Human owner: @pardeep1916P - [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. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-01 21:44:33 +00:00
Sign in to join this conversation.
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#6992
No description provided.