Merge pull request #9663 from spencerbull/fix/hermes-prompt-probe

Fix the Hermes CLI readiness probe, and tear the CLI down on uninstall

bin/omarchy-install-hermes-cli

@@ -1,7 +1,7 @@
 #!/bin/bash
 
 # omarchy:summary=Install the Hermes CLI as a mise-backed wrapper in ~/.local/bin
-# omarchy:args=[--check|--now|--owns]
+# omarchy:args=[--check|--now|--owns|--remove]
 # omarchy:examples=omarchy install hermes cli | omarchy install hermes cli --now
 
 # Hermes pins every one of its dependencies exactly and declares
@@ -88,13 +88,28 @@ hermes_runs() {
     timeout 15 "$HOME/.local/bin/hermes" --version >/dev/null 2>&1
 }
 
-# The chat subcommand's --oneshot opt-out arrived with native interactive -q,
-# so its presence is a stable capability check without relying on a version.
+# A flag counts only when the help defines it, not whenever it is mentioned:
+# what follows must be a shape argparse prints after a definition -- the usage
+# line's closing bracket, the gap before same-line help text, an uppercase
+# metavar, or the end of the line. Prose like "With --tui: run ..." stays
+# prose, and --tui-theme or --tui_mode never answers for --tui. Not probed by
+# parsing an actual invocation on purpose: a release that ignores unknown
+# arguments would turn the probe into a live session.
+help_defines_flag() {
+  grep -qE -- "$1(]|[[:space:]][[:upper:]]|[[:space:]]{2}|$)" <<<"$2"
+}
+
+# Probed for the flags omarchy-agent actually passes -- --query to seed the
+# session and --tui to keep it interactive -- rather than a marker standing in
+# for them: the old probe keyed on chat carrying --oneshot, which no released
+# Hermes did (it lived at the top level until v0.21 added chat's own), so
+# every release read as "not ready".
 hermes_prompt_ready() {
   local help
   hermes_runs &&
     help=$(timeout 15 "$HOME/.local/bin/hermes" chat --help 2>/dev/null) &&
-    grep -qF -- '--oneshot' <<<"$help"
+    help_defines_flag '--tui' "$help" &&
+    help_defines_flag '--query' "$help"
 }
 
 # --owns answers whether the wrapper on PATH is the one this command wrote, so
@@ -104,6 +119,42 @@ if [[ $mode == "--owns" ]]; then
   if ours; then exit 0; else exit 1; fi
 fi
 
+# --remove tears down a Hermes CLI this installer put in place -- the mise tool
+# it installed and the stub it marked -- so Remove Hermes clears a CLI the app
+# never superseded (an interrupted install, or the terminal CLI from before the
+# app existed) rather than leaving it stranded on PATH. The whole teardown
+# turns on the marked stub, exactly as replacement does further down: without
+# it nothing proves the mise environment is Omarchy's rather than one the user
+# built against the same spec, and a user's stays theirs. The desktop takeover
+# removes the environment without asking, but that is its own bargain -- a
+# second Hermes has to go whoever built it, and the app still provides the
+# command afterwards; here nothing would. Idempotent: nothing owned, nothing
+# to do.
+if [[ $mode == "--remove" ]]; then
+  if ours; then
+    mise rm -g "$tool" >/dev/null 2>&1 || true
+    mise uninstall --all "$tool" >/dev/null 2>&1 || true
+    rm -f "$HOME/.local/bin/hermes" 2>/dev/null || true
+
+    # Every step is attempted before any is judged, and judged by what is left
+    # rather than by what the commands claimed: the marked stub still answering
+    # hermes, or mise still resolving the tool, is a CLI still installed no
+    # matter how the removal exited.
+    # Not "run --remove again": once the stub is gone nothing marks the mise
+    # environment as ours, so a rerun would find nothing it owns and succeed
+    # without touching what was left. Only the full commands finish the job.
+    if ours || mise where "$tool" >/dev/null 2>&1; then
+      echo "Could not remove the Hermes CLI Omarchy installed. Finish by hand:" >&2
+      echo "  rm -f ~/.local/bin/hermes" >&2
+      echo "  mise rm -g '$tool'" >&2
+      echo "  mise uninstall --all '$tool'" >&2
+      exit 1
+    fi
+  fi
+
+  exit 0
+fi
+
 # --check lets callers tell a cold stub from a working one before they commit
 # to a path that assumes Hermes is ready.
 if [[ $mode == "--check" ]]; then

bin/omarchy-remove-ai-hermes

@@ -8,6 +8,16 @@ set -euo pipefail
 
 omarchy-pkg-drop hermes-desktop
 
+# The mise CLI is the app's predecessor, not the app itself: Hermes Desktop takes
+# it over on install and runs its own runtime instead, so a copy still here is one
+# the app never superseded -- an interrupted install, or the terminal CLI from
+# before the app existed. Remove Hermes clears that too, scoped by the installer
+# to what Omarchy owns so a hermes the user set up themselves is left alone.
+# Tolerated here rather than fatal, so the ~/.hermes handling below still runs;
+# the failure is answered for at the end instead of being swallowed.
+cli_removed=true
+omarchy-install-hermes-cli --remove || cli_removed=false
+
 # The app writes this when the runtime it provisions under ~/.hermes has landed,
 # and it is the only thing that tells that runtime apart from one the user
 # installed themselves -- the paths are the same either way. Without it the app
@@ -57,3 +67,9 @@ else
   echo "Hermes Desktop has been removed."
   echo "It never finished installing its own Hermes, so nothing in ~/.hermes was touched."
 fi
+
+# The messages above still hold -- the app and its runtime are gone -- but a CLI
+# teardown that failed already said so on stderr, and that stands.
+if [[ $cli_removed == "false" ]]; then
+  exit 1
+fi

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

@@ -23,6 +23,11 @@ cat >"$mock_bin/omarchy-cmd-missing" <<'SH'
 SH
 
 # `mise where` must fail so the installer sees no Hermes behind the stub.
+#
+# With OMARCHY_TEST_MISE_X_HERMES=1, `mise x -- hermes ...` emulates the Hermes
+# the Omarchy stub runs, so the readiness probe can be exercised through a
+# mise-installed hermes and not only the foreign and desktop wrappers. Off by
+# default, so `mise x` stays silent for every test that does not opt in.
 cat >"$mock_bin/mise" <<'SH'
 #!/bin/bash
 printf '%s\0' "$@" >>"$OMARCHY_TEST_MISE_LOG"
@@ -30,6 +35,18 @@ if [[ $1 == "where" && ${OMARCHY_TEST_MISE_WHERE_OK:-0} == 1 ]]; then
   printf '%s\n' "$OMARCHY_TEST_MISE_ROOT"
   exit 0
 fi
+if [[ $1 == "x" && ${OMARCHY_TEST_MISE_X_HERMES:-0} == 1 ]]; then
+  # Args are `x <tool> -- hermes <hermes-args...>`; skip to what follows hermes.
+  shift
+  while (( $# )) && [[ $1 != "--" ]]; do shift; done
+  shift 2
+  if [[ ${1:-} == "chat" && ${2:-} == "--help" ]]; then
+    [[ ${OMARCHY_TEST_HERMES_CAPABLE:-1} == 1 ]] && echo "[-q QUERY, --query QUERY] [--tui]"
+  else
+    echo "hermes-agent 0.0.0-test"
+  fi
+  exit 0
+fi
 [[ $1 != "where" ]]
 SH
 
@@ -98,7 +115,7 @@ mkdir -p "$test_home/.hermes/hermes-agent/venv/bin"
 cat >"$test_home/.hermes/hermes-agent/venv/bin/hermes" <<'SH'
 #!/bin/bash
 if [[ ${1:-} == "chat" && ${2:-} == "--help" ]]; then
-  [[ ${OMARCHY_TEST_HERMES_CAPABLE:-1} == 1 ]] && echo "--oneshot"
+  [[ ${OMARCHY_TEST_HERMES_CAPABLE:-1} == 1 ]] && echo "[-q QUERY, --query QUERY] [--tui]"
 else
   echo "hermes-agent 0.0.0-test"
 fi
@@ -237,6 +254,26 @@ tr '\0' '\n' <"$mise_log" | grep -q '^rm$' || fail "an older owned Hermes enviro
 tr '\0' '\n' <"$mise_log" | grep -q '^uninstall$' || fail "an older owned Hermes environment is uninstalled"
 pass "reinstalling replaces an older owned Hermes environment"
 
+# The mise-installed path is what a machine without the desktop app runs, and
+# --check gates the default agent there too. The stub is present and its mise
+# environment resolves, so readiness turns on the hermes mise runs -- exercised
+# here in both directions, since the desktop and foreign cases cover only their
+# own wrappers.
+run_mise_check() {
+  OMARCHY_TEST_DESKTOP_INSTALLED=0 \
+    OMARCHY_TEST_MISE_WHERE_OK=1 \
+    OMARCHY_TEST_MISE_ROOT="$test_tmp/mise" \
+    OMARCHY_TEST_MISE_LOG="$mise_log" \
+    OMARCHY_TEST_MISE_X_HERMES=1 \
+    OMARCHY_TEST_HERMES_CAPABLE="$1" \
+    HOME="$test_home" \
+    PATH="$mock_bin:$PATH" \
+    bash "$ROOT/bin/omarchy-install-hermes-cli" --check >/dev/null 2>&1
+}
+run_mise_check 1 || fail "--check accepts a mise-installed hermes that runs the seeded session"
+run_mise_check 0 && fail "--check rejects a mise-installed hermes without the flags omarchy-agent passes"
+pass "--check follows the mise-installed hermes it would actually run"
+
 rm -f "$test_home/.local/bin/hermes"
 : >"$mise_log"
 OMARCHY_TEST_MISE_WHERE_OK=1 run_installer 0 &&
@@ -365,6 +402,71 @@ marker_copies=$(grep -rl "Written by omarchy-install-hermes-cli" \
   fail "only omarchy-install-hermes-cli spells out the ownership marker"
 pass "the ownership marker is written down once"
 
+# --remove tears down a Hermes CLI this installer owns, so Remove Hermes can
+# clear one the desktop app never superseded. It turns on the same ownership as
+# the rest of the file, so its cases mirror that split.
+remove_home="$test_tmp/remove-home"
+mkdir -p "$remove_home/.local/bin"
+
+run_remove() {
+  OMARCHY_TEST_DESKTOP_INSTALLED=0 \
+    OMARCHY_TEST_MISE_WHERE_OK="${OMARCHY_TEST_MISE_WHERE_OK:-0}" \
+    OMARCHY_TEST_MISE_ROOT="$test_tmp/mise" \
+    OMARCHY_TEST_MISE_LOG="$mise_log" \
+    HOME="$remove_home" \
+    PATH="$mock_bin:$PATH" \
+    bash "$ROOT/bin/omarchy-install-hermes-cli" --remove
+}
+
+rm -f "$remove_home/.local/bin/hermes"
+: >"$mise_log"
+run_remove || fail "--remove succeeds when there is nothing to remove"
+# No stub means no proof the mise environment -- if one even exists -- is
+# Omarchy's, so nothing may reach mise at all.
+tr '\0' '\n' <"$mise_log" | grep -Eq '^(rm|uninstall)$' &&
+  fail "--remove leaves mise alone when nothing proves ownership"
+pass "--remove is idempotent when no Hermes CLI is present"
+
+printf '%s\n' "#!/bin/bash" "$stub_marker" >"$remove_home/.local/bin/hermes"
+chmod +x "$remove_home/.local/bin/hermes"
+: >"$mise_log"
+run_remove || fail "--remove succeeds tearing down an owned CLI"
+tr '\0' '\n' <"$mise_log" | grep -q '^rm$' || fail "--remove drops the mise tool from config"
+tr '\0' '\n' <"$mise_log" | grep -q '^uninstall$' || fail "--remove uninstalls the mise tool"
+[[ ! -e $remove_home/.local/bin/hermes ]] || fail "--remove takes the stub it owns"
+pass "--remove tears down the mise CLI and the stub this installer owns"
+
+# When mise still resolves the tool after the teardown, the environment
+# survived whatever uninstall claimed, and --remove has to say so.
+printf '%s\n' "#!/bin/bash" "$stub_marker" >"$remove_home/.local/bin/hermes"
+chmod +x "$remove_home/.local/bin/hermes"
+OMARCHY_TEST_MISE_WHERE_OK=1 run_remove && fail "--remove claims success while mise still resolves the tool"
+pass "--remove fails when the mise environment survives the teardown"
+
+foreign_remove_body="#!/bin/bash
+exec /usr/local/bin/my-own-hermes \"\$@\""
+printf '%s\n' "$foreign_remove_body" >"$remove_home/.local/bin/hermes"
+chmod +x "$remove_home/.local/bin/hermes"
+: >"$mise_log"
+OMARCHY_TEST_MISE_WHERE_OK=1 run_remove || fail "--remove succeeds with a foreign hermes present"
+[[ -f $remove_home/.local/bin/hermes && $(cat "$remove_home/.local/bin/hermes") == "$foreign_remove_body" ]] ||
+  fail "--remove leaves a hermes it does not own untouched"
+# The wrapper may front a mise environment the user built against the very same
+# spec; without the marker there is no telling, so the environment stays too.
+tr '\0' '\n' <"$mise_log" | grep -Eq '^(rm|uninstall)$' &&
+  fail "--remove never removes a mise environment it cannot prove is Omarchy's"
+pass "--remove leaves a Hermes the user installed themselves"
+
+# Judged by what is left, not by what rm claimed: a stub that survives the
+# teardown is a CLI still installed, and --remove has to say so.
+printf '%s\n' "#!/bin/bash" "$stub_marker" >"$remove_home/.local/bin/hermes"
+chmod +x "$remove_home/.local/bin/hermes"
+chmod 555 "$remove_home/.local/bin"
+run_remove && fail "--remove claims success while the stub survives"
+chmod 755 "$remove_home/.local/bin"
+rm -f "$remove_home/.local/bin/hermes"
+pass "--remove fails when the stub cannot be removed"
+
 # The app's marker says its install once landed, not that it is still there. A
 # wrapper whose runtime has since gone answers for nothing, so readiness runs
 # the command, exactly as it does for a hermes the user installed themselves.
@@ -388,7 +490,7 @@ run_ready_check && fail "--check rejects the app's wrapper when its runtime is g
 cat >"$ready_home/.hermes/hermes-agent/venv/bin/hermes" <<'SH'
 #!/bin/bash
 if [[ ${1:-} == "chat" && ${2:-} == "--help" ]]; then
-  echo "--oneshot"
+  echo "[-q QUERY, --query QUERY] [--tui]"
 else
   echo "hermes-agent 0.0.0-test"
 fi
@@ -396,3 +498,79 @@ SH
 chmod +x "$ready_home/.hermes/hermes-agent/venv/bin/hermes"
 run_ready_check || fail "--check accepts the app's wrapper once it runs"
 pass "readiness runs the app's command rather than trusting its marker"
+
+# A release whose help lists only the old probe's --oneshot marker cannot run
+# the seeded --tui --query session omarchy-agent starts, so it is not ready.
+cat >"$ready_home/.hermes/hermes-agent/venv/bin/hermes" <<'SH'
+#!/bin/bash
+if [[ ${1:-} == "chat" && ${2:-} == "--help" ]]; then
+  echo "--oneshot"
+else
+  echo "hermes-agent 0.0.0-test"
+fi
+SH
+chmod +x "$ready_home/.hermes/hermes-agent/venv/bin/hermes"
+run_ready_check && fail "--check accepts a release without the flags omarchy-agent passes"
+pass "a release listing only --oneshot is not prompt-ready"
+
+# A release that lists --tui-theme and --query-log but has dropped the bare
+# --tui/--query omarchy-agent passes must not read as ready on the substring
+# alone. The probe matches at a flag boundary for exactly this case.
+cat >"$ready_home/.hermes/hermes-agent/venv/bin/hermes" <<'SH'
+#!/bin/bash
+if [[ ${1:-} == "chat" && ${2:-} == "--help" ]]; then
+  echo "[--tui-theme THEME] [--query-log FILE]"
+else
+  echo "hermes-agent 0.0.0-test"
+fi
+SH
+chmod +x "$ready_home/.hermes/hermes-agent/venv/bin/hermes"
+run_ready_check && fail "--check accepts a release whose flags only contain --tui/--query as a substring"
+pass "a flag that merely contains --tui or --query is not prompt-ready"
+
+# Each flag answers for itself: a release that kept --tui but dropped --query,
+# or the reverse, cannot run the seeded session either, so neither grep may
+# ride on the other's match.
+for kept in '--tui' '-q QUERY, --query QUERY'; do
+  cat >"$ready_home/.hermes/hermes-agent/venv/bin/hermes" <<SH
+#!/bin/bash
+if [[ \${1:-} == "chat" && \${2:-} == "--help" ]]; then
+  echo "[$kept]"
+else
+  echo "hermes-agent 0.0.0-test"
+fi
+SH
+  chmod +x "$ready_home/.hermes/hermes-agent/venv/bin/hermes"
+  run_ready_check && fail "--check accepts a release listing only $kept"
+done
+pass "either flag alone is not prompt-ready"
+
+# An underscore continues a flag name just as a dash does: --tui_mode is not
+# --tui.
+cat >"$ready_home/.hermes/hermes-agent/venv/bin/hermes" <<'SH'
+#!/bin/bash
+if [[ ${1:-} == "chat" && ${2:-} == "--help" ]]; then
+  echo "[--tui_mode MODE] [--query_log FILE]"
+else
+  echo "hermes-agent 0.0.0-test"
+fi
+SH
+chmod +x "$ready_home/.hermes/hermes-agent/venv/bin/hermes"
+run_ready_check && fail "--check accepts flags that extend --tui/--query with an underscore"
+pass "an underscore continuation is not the bare flag"
+
+# A flag mentioned in another option's help text is not that option. Hermes
+# already writes "With --tui:" into --dev's description, so prose has to stay
+# prose even when both names appear in it.
+cat >"$ready_home/.hermes/hermes-agent/venv/bin/hermes" <<'SH'
+#!/bin/bash
+if [[ ${1:-} == "chat" && ${2:-} == "--help" ]]; then
+  echo "  --dev                 With --tui: run sources via tsx"
+  echo "  --log FILE            Where --query output lands"
+else
+  echo "hermes-agent 0.0.0-test"
+fi
+SH
+chmod +x "$ready_home/.hermes/hermes-agent/venv/bin/hermes"
+run_ready_check && fail "--check accepts flags that appear only in option descriptions"
+pass "a flag mentioned in prose is not a defined option"

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

@@ -15,6 +15,15 @@ cat >"$mock_bin/omarchy-pkg-drop" <<'SH'
 #!/bin/bash
 printf '%s\0' "$@" >>"$OMARCHY_TEST_DROP_LOG"
 SH
+
+# The CLI teardown is the installer's own, exercised in hermes-cli-test.sh; here
+# it is mocked to a logger so this test stays about what Remove Hermes does with
+# ~/.hermes, and to keep real mise out of a run with HOME pointed at a fixture.
+cat >"$mock_bin/omarchy-install-hermes-cli" <<'SH'
+#!/bin/bash
+printf '%s\0' "$@" >>"$OMARCHY_TEST_INSTALLER_LOG"
+exit "${OMARCHY_TEST_INSTALLER_STATUS:-0}"
+SH
 chmod +x "$mock_bin"/*
 
 seed_install() {
@@ -35,7 +44,11 @@ seed_install() {
 }
 
 remove() {
-  OMARCHY_TEST_DROP_LOG="$test_tmp/drop-log" HOME="$test_home" PATH="$mock_bin:$PATH" \
+  : >"$test_tmp/installer-log"
+  OMARCHY_TEST_DROP_LOG="$test_tmp/drop-log" \
+    OMARCHY_TEST_INSTALLER_LOG="$test_tmp/installer-log" \
+    OMARCHY_TEST_INSTALLER_STATUS="${OMARCHY_TEST_INSTALLER_STATUS:-0}" \
+    HOME="$test_home" PATH="$mock_bin:$PATH" \
     bash "$ROOT/bin/omarchy-remove-ai-hermes" >/dev/null 2>&1
 }
 
@@ -67,6 +80,12 @@ pass "removal keeps what belongs to the user"
 [[ ! -e $test_home/.local/bin/hermes ]] || fail "the app's own hermes command is removed"
 pass "removal takes the command the app installed"
 
+# Removal also asks the installer to tear down a mise CLI the app superseded, so
+# a copy left from before the app took over does not linger once Hermes is gone.
+tr '\0' '\n' <"$test_tmp/installer-log" | grep -qx -- '--remove' ||
+  fail "removal asks the installer to tear down its own CLI"
+pass "removal tears down the mise CLI through the installer"
+
 # A hermes command the app did not write survives even when the app did install
 # a runtime of its own.
 seed_install
@@ -87,6 +106,10 @@ printf 'my local edit\n' >"$test_home/.hermes/hermes-agent/PATCH"
 printf '%s\n' "#!/bin/bash" "exec $test_home/.hermes/hermes-agent/venv/bin/hermes \"\$@\"" \
   >"$test_home/.local/bin/hermes"
 remove || fail "remove succeeds when the app never finished installing Hermes"
+# The stranded pre-desktop CLI is exactly the interrupted-install case, so the
+# teardown must be asked for here too, not only when the app's runtime landed.
+tr '\0' '\n' <"$test_tmp/installer-log" | grep -qx -- '--remove' ||
+  fail "removal tears down the CLI even when the app never finished installing"
 [[ -d $test_home/.hermes/hermes-agent ]] ||
   fail "a Hermes runtime the app never installed survives removal"
 [[ -f $test_home/.hermes/hermes-agent/PATCH ]] ||
@@ -110,3 +133,14 @@ remove || fail "remove succeeds with a wrapper pointing at a sibling directory"
 [[ -f $test_home/.local/bin/hermes && $(cat "$test_home/.local/bin/hermes") == "$sibling_body" ]] ||
   fail "a wrapper pointing at ~/xhermes is not mistaken for one pointing into ~/.hermes"
 pass "removal matches the runtime path as a plain string"
+
+# A CLI teardown that fails must not stop the runtime handling, and must not be
+# papered over either: the data work still happens, and the failure reaches the
+# caller's exit code.
+seed_install
+printf '%s\n' "#!/bin/bash" "exec $test_home/.hermes/hermes-agent/venv/bin/hermes \"\$@\"" \
+  >"$test_home/.local/bin/hermes"
+OMARCHY_TEST_INSTALLER_STATUS=1 remove && fail "a failed CLI teardown surfaces in the exit code"
+[[ ! -d $test_home/.hermes/hermes-agent ]] ||
+  fail "a failed CLI teardown does not stop the runtime removal"
+pass "a failed CLI teardown is reported after the runtime is handled"