Judge the CLI teardown by what is left, and match flags as definitions

Two review follow-ups. The probe counted any mention of --tui/--query in
the help as support -- Hermes already writes "With --tui:" into --dev's
description, so a release that dropped the option while keeping the
prose would still read as ready. A flag now counts only when followed by
a shape argparse prints after a definition: the usage bracket, the gap
before same-line help text, an uppercase metavar, or the line end. Not
probed by parsing a real invocation on purpose -- a release that ignores
unknown arguments would turn the probe into a live session.

And the teardown trusted its commands: a stub rm that failed aborted
Remove Hermes under set -e before any ~/.hermes handling, while mise
failures vanished into || true. --remove now attempts every step, then
judges by what is left -- the marked stub still present, or mise still
resolving the tool -- and Remove Hermes tolerates the failure until the
runtime is handled, then carries it in its exit code.

Findings from the same codex review at xhigh, verified and proven by
mutation before landing.

Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
committed 2026-09-02 01:01:13 -05:00
1 parent c462aad9ee
commit 46cfc4ada5
4 files changed
+81 -10

No files matched your search

+24 -8
View File
@@ -88,21 +88,27 @@ hermes_runs() {
timeout 15 "$HOME/.local/bin/hermes" --version >/dev/null 2>&1
}
# 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 flag that merely
# shipped alongside them: the previous marker, --oneshot, was dropped in Hermes
# v0.20 and left working installs stranded as "not ready".
#
# Matched at a flag boundary, not as a substring: a bare grep for --tui also
# accepts a release that lists only --tui-theme while having dropped --tui
# itself, which would call an install ready that omarchy-agent cannot drive --
# the same false verdict this check exists to prevent, merely inverted.
hermes_prompt_ready() {
local help
hermes_runs &&
help=$(timeout 15 "$HOME/.local/bin/hermes" chat --help 2>/dev/null) &&
grep -qE -- '--tui([^[:alnum:]_-]|$)' <<<"$help" &&
grep -qE -- '--query([^[:alnum:]_-]|$)' <<<"$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
@@ -127,7 +133,17 @@ 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"
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.
if ours || mise where "$tool" >/dev/null 2>&1; then
echo "Could not remove the Hermes CLI Omarchy installed." >&2
echo "Remove ~/.local/bin/hermes and the mise tool '$tool', then run omarchy-install-hermes-cli --remove again." >&2
exit 1
fi
fi
exit 0
+10 -1
View File
@@ -13,7 +13,10 @@ omarchy-pkg-drop hermes-desktop
# 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.
omarchy-install-hermes-cli --remove
# 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
@@ -64,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
+34 -1
View File
@@ -430,12 +430,19 @@ 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"
OMARCHY_TEST_MISE_WHERE_OK=1 run_remove || fail "--remove succeeds tearing down an owned CLI"
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"
@@ -450,6 +457,16 @@ 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.
@@ -541,3 +558,19 @@ 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"
+13
View File
@@ -22,6 +22,7 @@ SH
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"/*
@@ -46,6 +47,7 @@ remove() {
: >"$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
}
@@ -131,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"