Skip to content

fix(desktop): prefer the bundle's harness binaries over workspace target dirs - #1

Merged
wiggdevin merged 4 commits into
zs/mainfrom
fix/harness-discovery-bundle
Sep 4, 2026
Merged

wiggdevin merged 4 commits into
zs/mainfrom
fix/harness-discovery-bundle

Conversation

@wiggdevin

@wiggdevin wiggdevin commented Sep 4, 2026

Copy link
Copy Markdown

fix(desktop): prefer the bundle's harness binaries over workspace target dirs

Summary

When the desktop app's exe path sits inside a .app bundle, harness/agent
discovery now searches the bundle's own binary directory before any
workspace target/ directory. This matches upstream's command_search_dirs
intent for bundled (packaged) builds — a bundled sidecar binary should win
over a stale binary left in a developer's workspace target/release — while
leaving dev-build search order unchanged (no .app ancestor in the exe
path).

The fix adds a bundle_exe_prefers_bundle_over_workspace_target regression
test in managed_agents::discovery::tests, plus a #[cfg(test)]-only seam
so the regression tests can drive the real command_search_dirs() /
resolve_command() entry points against a controlled exe-parent path,
instead of only the pre-existing extraction functions.

Fork deviations

  • Added a #[cfg(test)]-only exe-parent override seam
    (exe_parent_for_search, exe_parent_override_cell,
    set_exe_parent_override_for_test, exe_parent_override_test_lock) to
    discovery.rs, beyond what the ticket's own eval commands required, so
    findings 2 and 3 could bind the real command_search_dirs() /
    resolve_command() entry points instead of only the already-tested
    order_search_dirs / resolve_workspace_command_from extraction —
    required by AGENTS.md Review-Proven Rule 3 (regression tests must bind
    the production seam) and matches the existing
    custom_harnesses::registry_test_lock process-global-test-state pattern
    already used in this codebase.
  • The existing
    command_search_dirs_is_exercised_directly_and_deduplicated test was also
    given the new exe_parent_override_test_lock guard (it sets no override
    itself, but its assertion depends on the real, non-overridden exe parent,
    so it must not interleave with a test that has the override active).

Gates

Command Result
cargo test managed_agents::discovery::tests::bundle_exe_prefers_bundle_over_workspace_target -- --list | grep -c ': test' 1 (== 1, gate met)
cargo test managed_agents::discovery::tests::bundle_exe_prefers_bundle_over_workspace_target -- --exact 1 passed
cargo test managed_agents::discovery 137 passed / 0 failed (130 pre-existing + 7 new)
just desktop-tauri-clippy exit 0
just fmt-check exit 0
just desktop-tauri-fmt-check exit 0
just file-size-check exit 0
cargo test --lib managed_agents:: (3x, flake check) 1259 passed each run, no flake
DCO (git log --format=%B origin/zs/main..HEAD | grep -c Signed-off-by vs commit count) 3 / 3
Working tree clean

just ci on the rebased branch: exit 0 (full pipeline — check, test-unit,
desktop-test, desktop-build, desktop-tauri-check, desktop-tauri-test,
web-build, mobile-test — ran to completion, ending All tests passed!
with no failures).

Tested base OID: 35d4e2c9aa311837365b1708e3e28f870c483c46 (origin/zs/main
at rebase time).

Gemini 3.8 Flash tester

  • Verdict: PASS
  • Missing tests: none
  • Dirty files after run: none
  • Full report: /private/tmp/claude-501/-Users-zero-suminc-/a0fab389-7ac7-4f57-88e6-599d452e8e3a/scratchpad/wave1/harness-discovery-bundle/gemini-report.md

Critic

Winner: ours.

Checklist: parity ✅ · deviations named ✅ · acceptance met ✅ · nothing above
NIT ✅

Biggest gap (NIT, not blocking): the module doc in
desktop/src-tauri/src/managed_agents/discovery/bundle_search_tests.rs
claims "any test whose assertion depends on the real (non-overridden) exe
parent also takes that lock," but
desktop/src-tauri/src/managed_agents/discovery/tests/managed_path_resolution.rs
calls resolve_command/resolve_command_cached in the same test binary
without taking exe_parent_override_test_lock(). The new #[cfg(test)]
process-global exe-parent override can therefore be live during those
assertions. It cannot change their outcome today (the injected bundle dir
contains only uuid-named files) and 3x full managed_agents runs were
clean, so this is a documented-but-unenforced invariant, not a live flake.

Notes:

  • Bar fetched from origin/zs/main:discovery.rs (upstream
    command_search_dirs, lines 355-405); order_search_dirs provably
    reduces to that exact sequence when the exe parent has no .app ancestor,
    so dev-build semantics are unchanged.
  • All four ticket evals run from the worktree after
    . ./bin/activate-hermit: named-filter --list == 1 exactly; --exact 1
    passed; cargo test managed_agents::discovery 137 passed / 0 failed (130
    pre-existing + 7 new); just desktop-tauri-clippy exit 0. Also green:
    fmt-check, desktop-tauri-fmt-check, file-size-check; 3x
    cargo test --lib managed_agents:: = 1259 passed, no flake. DCO 3/3,
    clean tree.
  • Fix targets the right directory — Tauri externalBin sidecars land in
    Contents/MacOS/ (tauri.conf.json:55-61,
    scripts/bundle-sidecars.sh), which is current_exe().parent(), and
    resolve_command_uncached hits resolve_workspace_command first.
  • Deviation from "existing discovery tests unchanged" is real and
    justified: discovery/tests.rs import list collapsed to use super::*;
    to free ratchet headroom (base 1778 lines vs 1500 ceiling,
    allowedLineCount in check-file-sizes-core.mjs:31) for a 4-line
    include!, which is itself forced by the ticket naming the exact
    discovery::tests::… path. Visible in the diff, commented, and stated in
    commit bf456a5f6.
  • Warning for the landing/gate scripts: rtk filters git log and
    cargo test -- --list; the plan's own DCO and count pipes reported 2/3
    signoffs and 0 tests on a green branch. Use rtk proxy.
  • Other NITs: eager command_search_dirs() before the path-form early
    return; is_inside_app_bundle silently false on non-UTF-8 components; 116
    added lines and 3 overlapping preference tests for an S ticket.

GPT-5.6 Sol audit

Sol ran; verdict: "Found 2 BLOCK and 2 WARN findings." After verification
against the code, both BLOCKs were discarded (not regressions — see below)
and both WARNs were downgraded to NIT.

Verified findings (both NIT, not blocking):

  1. desktop/src-tauri/src/managed_agents/discovery/tests/bundle_search.rs:3
    — the comment spells the test's module path as
    discovery::tests::bundle_exe_prefers_bundle_over_workspace_target,
    omitting the crate-root segment; the compiled name is
    managed_agents::discovery::tests::bundle_exe_prefers_bundle_over_workspace_target.
    Reproduced: running the comment's path with -- --exact exits 0 with
    0 passed; 3191 filtered out — a green-and-empty run. Downgraded from
    WARN because the plan's own required eval already uses the full name and
    returns count 1; only the comment is wrong, not the gate. Fix: rewrite
    the comment to the full compiled path (one line, no behavior change).
  2. desktop/src-tauri/src/managed_agents/discovery.rs:368
    is_inside_app_bundle matches any ancestor component ending in .app,
    with no macOS gate and no Contents/MacOS structural requirement.
    Downgraded from WARN: no realistic trigger exists here — the desktop exe
    lives at target/{debug,release}/<exe> for dev runs and
    <name>.app/Contents/MacOS/<exe> when bundled, and Tauri emits
    AppImage/deb/msi off macOS. The ticket bar (dev-build ordering unchanged)
    holds: existing_discovery_tests_are_unaffected_by_ordering_change
    passes and all 137 managed_agents::discovery tests are green. Optional
    hardening noted for a follow-up, not this ticket.

Discarded findings:

  • Sol BLOCK fix(desktop): prefer the bundle's harness binaries over workspace target dirs #1 — claimed a corrupted-but-present bundled sidecar falls
    through to an untrusted workspace binary that then receives
    BUZZ_PRIVATE_KEY. Not a regression: origin/zs/main's
    discovery.rs:363-380 shows the pre-branch order already put the
    workspace binary first unconditionally (executable or not); after this
    change the workspace binary wins only in the narrower non-executable-
    bundle case, since is_executable_file (untouched by this diff) already
    skips non-executable candidates. workspace_root_dir() is also a
    compile-time CARGO_MANIFEST_DIR path on the build machine's own
    checkout, not attacker-supplied. Sol's fix would make discovery terminal
    on a corrupt bundle — a behavior change outside the ticket, and it breaks
    the ticket's own bar (upstream command_search_dirs dev-build semantics
    unchanged).
  • Sol BLOCK docs: calendar authorization contract (T11) #2 — claimed std::env::current_exe() failure is silently
    swallowed via .ok(), disabling bundle precedence, and that no test
    covers the real call. The .ok() is verbatim pre-existing upstream code
    (origin/zs/main discovery.rs:373-377); this branch only moved it into
    exe_parent_for_search() without changing its error handling, so there
    is no regression to fix. Review-Proven Rule 1 governs a caught failure
    that abandons durable work or converts a terminal failure into an
    authoritative success; best-effort path discovery that falls through to
    workspace dirs and PATH is neither. The coverage point is inherent (a
    cargo test binary is never itself inside a .app) and already
    mitigated: the override is #[cfg(test)]-only, and
    command_search_dirs_is_exercised_directly_and_deduplicated calls the
    real, non-overridden command_search_dirs() and asserts the real exe
    parent is searched last.

Full report:
/private/tmp/claude-501/-Users-zero-suminc-/a0fab389-7ac7-4f57-88e6-599d452e8e3a/scratchpad/wave1/harness-discovery-bundle/audit.md

Test plan

  • cargo test managed_agents::discovery::tests::bundle_exe_prefers_bundle_over_workspace_target -- --list | grep -c ': test' == 1
  • cargo test managed_agents::discovery::tests::bundle_exe_prefers_bundle_over_workspace_target -- --exact passes
  • cargo test managed_agents::discovery — 137 passed / 0 failed
  • just desktop-tauri-clippy — exit 0
  • just fmt-check, just desktop-tauri-fmt-check, just file-size-check — all green
  • cargo test --lib managed_agents:: x3 for flake check — 1259 passed each run
  • just ci on the rebased branch — full pipeline green
  • Gemini 3.8 Flash tester — PASS, no missing tests, no dirty files
  • Critic gauntlet — winner ours, nothing above NIT
  • GPT-5.6 Sol audit — 2 BLOCK discarded with written reasons (not regressions), 2 WARN downgraded to NIT and verified

🤖 Generated with Claude Code

https://claude.ai/code/session_01E51uwemNnQ6wdrBWU9EhPE


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

wiggdevin and others added 3 commits September 4, 2026 05:10
When the running exe lives inside a macOS .app bundle, discovery now
searches the bundle's own directory before any workspace target/
dir, so a sibling checkout's build output on the same machine can no
longer shadow the shipped binary.

Extracts the search-dir ordering into a pure, testable
order_search_dirs() and adds a bundle_exe_prefers_bundle_over_workspace_target
regression test. The test lives in its own file
(discovery/bundle_search_tests.rs) rather than discovery/tests.rs
because that file is already at the repo's file-size ratchet ceiling
and may not grow.

Signed-off-by: wiggdevin <202901685+wiggdevin@users.noreply.github.com>
Address Gemini tester findings on the bundle-search fix:
- Move the ticket-named test to the exact `discovery::tests::…` path
  the eval commands require, via `include!` (not `mod`, which would
  add a module segment) since `discovery/tests.rs` sits at the
  desktop file-size ratchet ceiling with zero headroom. Freed room by
  replacing that file's named `use super::{...}` import list with
  `use super::*;`.
- Add regression coverage that binds the real production entry points
  (`command_search_dirs`, `resolve_workspace_command`,
  `resolve_command_cached`) instead of only the pure `order_search_dirs`/
  `resolve_workspace_command_from` helpers.
- Fix `is_inside_app_bundle` to detect `.App`/`.APP` bundle extensions
  case-insensitively (APFS is case-insensitive-but-preserving), with a
  test.
- Add a test for a non-executable bundle candidate falling through to
  the workspace binary instead of stopping the search.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E51uwemNnQ6wdrBWU9EhPE
Signed-off-by: wiggdevin <202901685+wiggdevin@users.noreply.github.com>
…lve_command

Adds a #[cfg(test)] exe-parent override seam so the production entry
points themselves (not just the extracted order_search_dirs /
resolve_workspace_command_from helpers) can be exercised with a
bundle-shaped exe path -- std::env::current_exe() inside cargo test is
always the test runner's own binary and can never be inside a .app
bundle.

- command_search_dirs_prefers_bundle_exe_parent_when_overridden: binds
  command_search_dirs() itself to the bundle-preference behavior.
- resolve_command_prefers_bundle_binary_over_workspace_target_binary:
  binds the top-level resolve_command() forced-discovery entry point,
  proving it resolves a bundle-located binary before a workspace
  target binary of the same name.

The override is process-global (same pattern as
custom_harnesses::registry_test_lock); tests that set it, or whose
assertion depends on the real exe parent, serialize via
exe_parent_override_test_lock.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E51uwemNnQ6wdrBWU9EhPE
Signed-off-by: wiggdevin <202901685+wiggdevin@users.noreply.github.com>
@wiggdevin wiggdevin closed this Sep 4, 2026
@wiggdevin wiggdevin reopened this Sep 4, 2026
…ery-bundle

Signed-off-by: wiggdevin <202901685+wiggdevin@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant