[BetterAndBetterII]

The Homebrew cask PATH-links only abctl, not arcbox-daemon. abctl daemon start then resolves current_exe() to that brew symlink, looks for a sibling in /opt/homebrew/bin, and errors even though the daemon sits next to the real binary inside ArcBox.app/Contents/MacOS/bin.

resolve_daemon_binary now canonicalizes the executable and looks next to the real path before falling back to PATH. Direct sibling installs are unchanged.

Fixes #644

[claude]

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[greptile-apps]

Greptile Summary

The PR updates daemon executable discovery so abctl can locate arcbox-daemon through a Homebrew cask symlink while preserving direct-sibling and PATH fallbacks.

• Extracts daemon resolution into a focused helper module.
• Checks beside the invoked executable first, then beside its canonicalized target, and finally searches PATH.
• Adds coverage for Homebrew symlinks, direct installs, PATH fallback, and missing binaries.

Confidence Score: 5/5

The PR appears safe to merge, with no concrete changed-code failure identified.

The resolver preserves existing direct-sibling and PATH behavior while adding the canonicalized-sibling lookup needed for the Homebrew cask layout, and the added tests cover the relevant resolution paths.

Important Files Changed

Filename	Overview
app/arcbox-cli/src/commands/daemon.rs	Delegates daemon executable discovery to the new resolver without changing the subsequent launch paths.
app/arcbox-cli/src/commands/resolve_daemon.rs	Implements direct, canonicalized-sibling, and PATH lookup order with focused tests for each outcome.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Resolve current abctl executable] --> B{Daemon is direct sibling?}
    B -- Yes --> C[Use direct sibling]
    B -- No --> D{Canonicalize abctl path}
    D -- Success --> E{Daemon is beside canonical target?}
    E -- Yes --> F[Use canonical sibling]
    E -- No --> G[Search PATH]
    D -- Failure --> G
    G --> H{Daemon found?}
    H -- Yes --> I[Launch daemon]
    H -- No --> J[Return location error]

Loading

_{Reviews (1): Last reviewed commit: "fix(cli): find arcbox-daemon across Home..." | Re-trigger Greptile}

[pullfrog]

Important

The functional fix is correct and well-tested, but the PR's cosmetic import reorderings fail cargo fmt --check — a CI gate (.github/workflows/ci.yml:85) — so this cannot merge as-is. Reverting the three reorderings (keeping the bail import, which is still used) resolves it.

Reviewed changes

Reviewed at head e0e6310 (1 commit, 2 files, +133/−17): abctl daemon start/status now locate arcbox-daemon next to the canonicalized executable before falling back to PATH, fixing the Homebrew cask flow (#644) where only abctl is PATH-linked.

• Symlink-aware daemon resolution — new resolve_daemon::resolve_daemon_binary_from tries the raw sibling, then the sibling of canonicalize()d current_exe() (the cask fix), then PATH. Lookup precedence for existing installs is unchanged, and the failure message is byte-identical to the old bail!.
• Unit tests — four tests build real tempdir symlink layouts and pin each behavior with exact assert_eq! assertions; each would fail if its behavior were removed. All four pass (verification note below).

Verification: the unit tests pass under edition 2024, run standalone (rustc --test) because the workspace does not currently compile on this Linux runner — a pre-existing arcbox-core E0308 at app/arcbox-core/src/runtime.rs:1268, untouched by this PR and invisible to the macOS-only CI build. cargo fmt --check on rustfmt 1.9.0-stable reproduces the failure above.

ℹ️ Same unresolved-symlink pattern in nearby call sites (pre-existing)

abctl _install resolves arcbox-helper and the launchd daemon path from raw current_exe() (install.rs:117-119, install.rs:215-217), and detect_bundle_xbin (symlink.rs:15) walks current_exe().parent() without canonicalizing. Whether these bite under the brew cask depends on how the tap invokes abctl (linked /opt/homebrew/bin/abctl vs the staged Caskroom path), which is not verifiable from this repo — a heads-up for a possible follow-up, not a request to expand this PR.

Technical details

# Unresolved current_exe sibling lookups outside this PR

## Affected sites
- app/arcbox-cli/src/commands/install.rs:117-119 — arcbox-helper source lookup uses the raw `current_exe()` parent
- app/arcbox-cli/src/commands/install.rs:215-217 — launchd plist daemon path: raw sibling, then `data_dir/bin/arcbox-daemon`, then bare `arcbox-daemon`
- app/arcbox-cli/src/commands/symlink.rs:15-17 — `detect_bundle_xbin` walks `current_exe().parent()?.parent()?`

## Required outcome
- With access to the homebrew tap, determine whether any installer path invokes `abctl` through the brew symlink; if so, canonicalize there or reuse the new `resolve_daemon_binary_from`.

## Open questions for the human (optional)
- Does the cask postflight run `abctl _install` via the linked path or the Caskroom-staged path?

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using openai-compatible/@cf/zai-org/glm-5.3-flash ｜ 𝕏}

[redwhisk]

Superseded by 70620c8844c84d64001648eaf528c129265b5900, already on master. The shared commands::bundle::locate_daemon canonicalizes the invoked abctl and checks the sibling, current Frameworks layout, legacy Helpers layout, data directory, and PATH. Both foreground and background daemon starts use that locator.

The existing symlink/bundle and sibling/fallback regression tests passed in the current integration workspace, as did workspace formatting. Both outdated review threads now have implementation evidence and are resolved. Closing this duplicate PR; its source branch remains intact.

[pullfrog]

✅ No new issues found.

Reviewed changes

Re-reviewed the PR at head e0e6310 — no commits have landed since the prior pullfrog review, so this pass verified the resolutions of the two prior findings and the superseding fix on master.

• Settled the cargo fmt --check finding — the thread was resolved: this branch is superseded by master commit 70620c88 and will be closed without merging, so the import reorderings never land.
• Settled the #[path] module nitpick — the superseding commit declares the new locator as a normal module (pub mod bundle; in app/arcbox-cli/src/commands/mod.rs) instead of the one-off #[path] wiring proposed here.
• Verified the superseding fix on master — 70620c88 is an ancestor of master and covers this PR's goal more thoroughly: bundle::locate_daemon starts every lookup from the canonicalized executable and searches the sibling directory, the Desktop bundle (Contents/Frameworks/<label>.app, plus the legacy Contents/Helpers layout), <data_dir>/bin, then PATH, with both exec_foreground and spawn_background calling it. The same-class sites the prior review flagged informationally are fixed too: install.rs resolves arcbox-helper via bundle::current_executable(), the launchd plist path uses locate_daemon, and symlink.rs is deleted with detect_bundle_xbin moved into bundle.rs on the resolved executable.

Approving since no new commits need review, the prior findings are settled, and no new issues remain.

  ^{｜ View workflow run ｜ Using glm-5.3-flash ｜ 𝕏}