Address Hermes review feedback

bin/omarchy-install-hermes-cli

@@ -73,11 +73,14 @@ foreign_hermes() {
   [[ -e $HOME/.local/bin/hermes || -L $HOME/.local/bin/hermes ]] && ! ours
 }
 
-# A foreign path is usable when it is a command: a regular file that runs.
-# A directory passes -x on search permission alone, and is no more a command
-# than a dangling link is.
+# A foreign path is usable when it is a command that runs: a regular executable
+# whose --version answers. The executable bit alone proves little -- a directory
+# passes -x on search permission, and a wrapper whose interpreter or target is
+# gone passes it too. The desktop app applies the same probe with the same 15
+# second budget, so what passes here is what it will use.
 foreign_hermes_runs() {
-  [[ -f $HOME/.local/bin/hermes && -x $HOME/.local/bin/hermes ]]
+  [[ -f $HOME/.local/bin/hermes && -x $HOME/.local/bin/hermes ]] &&
+    timeout 15 "$HOME/.local/bin/hermes" --version >/dev/null 2>&1
 }
 
 # --check lets callers tell a cold stub from a working one before they commit

migrations/1787760281.sh

@@ -4,8 +4,14 @@ echo "Install the Hermes CLI wrapper for existing installs"
 # is one of them.
 [[ -f $HOME/.local/state/omarchy/preinstalls-removed ]] && exit 0
 
-# Hermes Desktop provides its own Hermes; the installer would only stand aside.
-omarchy-pkg-present hermes-desktop && exit 0
+# Hermes Desktop provides its own Hermes. The installer stands aside for it,
+# removing the mise copy and the Omarchy wrapper an earlier install may have
+# left beside the app. It also reports when the app has not finished setting
+# Hermes up, which is the app's to finish, not this migration's to fail on.
+if omarchy-pkg-present hermes-desktop; then
+  omarchy-install-hermes-cli || true
+  exit 0
+fi
 
 # Anything already answering to hermes that this installer did not write --
 # an official install, a hand-rolled wrapper, even a dangling link -- belongs to

test/shell.d/hermes-cli-migration-test.sh

@@ -24,8 +24,10 @@ cat >"$mock_bin/omarchy-cmd-missing" <<'SH'
 ! command -v "$1" >/dev/null 2>&1
 SH
 
+mise_log="$test_tmp/mise-log"
 cat >"$mock_bin/mise" <<'SH'
 #!/bin/bash
+printf '%s\0' "$@" >>"$OMARCHY_TEST_MISE_LOG"
 [[ $1 != "where" ]]
 SH
 
@@ -35,6 +37,7 @@ chmod +x "$mock_bin"/*
 # copy of it.
 run_migration() {
   OMARCHY_TEST_DESKTOP_INSTALLED="${1:-0}" \
+    OMARCHY_TEST_MISE_LOG="$mise_log" \
     HOME="$test_home" \
     PATH="$mock_bin:$ROOT/bin:$PATH" \
     bash -euo pipefail "$migration" >/dev/null 2>&1
@@ -66,6 +69,33 @@ run_migration 1 || fail "the migration succeeds when Hermes Desktop owns Hermes"
 [[ ! -e $hermes ]] || fail "the migration writes nothing when Hermes Desktop owns Hermes"
 pass "the migration stands aside for Hermes Desktop"
 
+# Standing aside is not the same as leaving a second Hermes behind: the wrapper
+# an earlier install wrote and the mise copy it points at both go when the
+# desktop app owns Hermes, even though the app has not finished setting up.
+printf '%s\n' "#!/bin/bash" "$marker" >"$hermes"
+chmod +x "$hermes"
+: >"$mise_log"
+run_migration 1 || fail "the migration succeeds when Hermes Desktop owns Hermes and the old wrapper is present"
+[[ ! -e $hermes ]] || fail "the migration removes the Omarchy wrapper when Hermes Desktop owns Hermes"
+mise_calls=$(tr '\0' ' ' <"$mise_log")
+[[ $mise_calls == *"rm -g "* ]] || fail "the migration removes the global mise Hermes for Hermes Desktop"
+[[ $mise_calls == *"uninstall --all "* ]] || fail "the migration uninstalls the mise Hermes for Hermes Desktop"
+pass "the migration clears the old Omarchy Hermes for Hermes Desktop"
+
+# ...while anyone else's hermes stays exactly where it is, and is not run.
+foreign_ran="$test_tmp/foreign-ran"
+foreign_body="#!/bin/bash
+touch $foreign_ran
+exec $test_home/.hermes/hermes-agent/venv/bin/hermes \"\$@\""
+printf '%s\n' "$foreign_body" >"$hermes"
+chmod +x "$hermes"
+run_migration 1 || fail "the migration succeeds over a foreign hermes when Hermes Desktop owns Hermes"
+[[ -x $hermes && $(cat "$hermes") == "$foreign_body" ]] ||
+  fail "the migration leaves a foreign hermes alone when Hermes Desktop owns Hermes"
+[[ ! -e $foreign_ran ]] || fail "the migration does not run a foreign hermes"
+pass "the migration preserves a foreign hermes for Hermes Desktop"
+rm -f "$hermes"
+
 official_body="#!/bin/bash
 unset PYTHONPATH
 unset PYTHONHOME

test/shell.d/hermes-cli-test.sh

@@ -87,8 +87,10 @@ pass "takeover removes an unhealthy mise copy"
 rm -rf "$test_home/.hermes"
 rm -f "$test_home/.local/bin/hermes"
 run_installer 1 --check && fail "--check reports Hermes missing before the app installs it"
+# The venv command answers --version, as the real one does: foreign wrappers
+# below exec it, and the installer probes them by running exactly that.
 mkdir -p "$test_home/.hermes/hermes-agent/venv/bin"
-printf '%s\n' "#!/bin/bash" >"$test_home/.hermes/hermes-agent/venv/bin/hermes"
+printf '%s\n' "#!/bin/bash" 'echo "hermes-agent 0.0.0-test"' >"$test_home/.hermes/hermes-agent/venv/bin/hermes"
 chmod +x "$test_home/.hermes/hermes-agent/venv/bin/hermes"
 run_installer 1 --check && fail "--check waits for the install to finish, not just the venv"
 touch "$test_home/.hermes/hermes-agent/.hermes-bootstrap-complete"
@@ -130,6 +132,32 @@ run_installer 0 && fail "the installer does not succeed over a non-executable fo
   fail "a non-executable foreign hermes is left untouched"
 pass "a non-executable foreign hermes is preserved"
 
+# The executable bit is not enough: a wrapper whose interpreter is gone passes
+# -x and still cannot run. The probe has to run it to find out, and finding
+# out never touches the file.
+broken_interp_body="#!$test_home/nowhere/python3
+print('hermes')"
+printf '%s\n' "$broken_interp_body" >"$test_home/.local/bin/hermes"
+chmod +x "$test_home/.local/bin/hermes"
+run_installer 0 --check && fail "--check rejects a foreign hermes whose interpreter is missing"
+run_installer 0 && fail "the installer does not succeed over a foreign hermes whose interpreter is missing"
+run_installer 0 --now && fail "--now does not succeed over a foreign hermes whose interpreter is missing"
+[[ -x $test_home/.local/bin/hermes && $(cat "$test_home/.local/bin/hermes") == "$broken_interp_body" ]] ||
+  fail "a foreign hermes whose interpreter is missing is left untouched"
+pass "a foreign hermes with a missing interpreter is preserved and rejected"
+
+# Likewise a wrapper that execs a target that is no longer there.
+broken_target_body="#!/bin/bash
+exec $test_home/nowhere/hermes \"\$@\""
+printf '%s\n' "$broken_target_body" >"$test_home/.local/bin/hermes"
+chmod +x "$test_home/.local/bin/hermes"
+run_installer 0 --check && fail "--check rejects a foreign hermes whose target is missing"
+run_installer 0 && fail "the installer does not succeed over a foreign hermes whose target is missing"
+run_installer 0 --now && fail "--now does not succeed over a foreign hermes whose target is missing"
+[[ -x $test_home/.local/bin/hermes && $(cat "$test_home/.local/bin/hermes") == "$broken_target_body" ]] ||
+  fail "a foreign hermes whose target is missing is left untouched"
+pass "a foreign hermes with a missing target is preserved and rejected"
+
 foreign_target="$test_home/foreign/hermes"
 mkdir -p "$(dirname "$foreign_target")"
 printf '%s\n' "$official_body" >"$foreign_target"
@@ -161,9 +189,9 @@ pass "a directory at the hermes path is preserved and rejected"
 
 # Mentioning the installer is not the same as being written by it.
 rmdir "$test_home/.local/bin/hermes"
-mentions_body='#!/bin/bash
+mentions_body="#!/bin/bash
 # Replaces the stub omarchy-install-hermes-cli used to write.
-exec /usr/local/bin/hermes "$@"'
+exec $test_home/.hermes/hermes-agent/venv/bin/hermes \"\$@\""
 printf '%s\n' "$mentions_body" >"$test_home/.local/bin/hermes"
 chmod +x "$test_home/.local/bin/hermes"
 run_installer 0 || fail "installing over a wrapper that mentions the installer returns success"