Skip to content

Ask, default no, before Remove Hermes deletes the user's data - #10271

Merged
spencerbull merged 2 commits into
omacom:quattrofrom
spencerbull:hermes-remove-ask-user-data
Sep 6, 2026
Merged

Ask, default no, before Remove Hermes deletes the user's data#10271
spencerbull merged 2 commits into
omacom:quattrofrom
spencerbull:hermes-remove-ask-user-data

Conversation

@spencerbull

@spencerbull spencerbull commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Remove > AI > Hermes keeps the user's data silently today: the chats, memories and skills in ~/.hermes, the connections and their encrypted tokens in ~/.config/Hermes. Keeping them stays the default, but a removal meant to be complete currently ends with two paths to delete by hand — the tokens among them. This brings over the flow #10246 introduced for OpenClaw: after the teardown, when there is a terminal to ask in and gum to ask with, offer — gum confirm --default=false, size shown — to delete that data too.

Where the two differ, and why this version stays smaller than OpenClaw's:

  • OpenClaw has to ask because ~/.openclaw cannot be split: chats and credentials share one tree with hundreds of megabytes of plugin runtimes. Hermes' tree splits, and the remover already deletes the heavy disposable runtime unasked (gated on the bootstrap marker), so the question here covers only what is left after that.
  • The question itself follows OpenClaw exactly: asked whenever the directories exist, marker or no marker. A first cut gated it on the bootstrap marker too, which made it unreachable on precisely the machines with data worth asking about — a runtime already torn down on an earlier run, or data from the terminal CLI the app never superseded. The prompt names the paths, so on a machine where ~/.hermes still carries a runtime the app never owned, a yes knowingly takes that with it; a decline — and the silent path — still touches none of it.
  • Non-interactive removal keeps everything, exactly as before: without a terminal there is nobody to ask, and gum is never reached.

One incidental hardening: the size probe tolerates a missing directory, where du's non-zero exit would otherwise abort the removal through pipefail.

Verified

test/shell.d/hermes-remove-test.sh: 15 ok, six of them new — no prompt without a terminal, ask-and-decline keeps everything (with and without the bootstrap marker), an explicit yes takes ~/.hermes and ~/.config/Hermes (again both sides of the marker), and a runtime the app never installed is never touched by anything short of that yes. The interactive cases put the remover on a pty with script(1) and answer through a stubbed gum, so no test ever depends on where the suite itself runs. The manual's Hermes paragraph now says the removal asks.

@spencerbull

Copy link
Copy Markdown
Contributor Author

Reviewed against origin/quattro (merge-base 4930677), diff at 8f15549, with an independent second opinion from Codex at xhigh reasoning. Ran test/shell.d/hermes-remove-test.sh directly: all 14 tests pass, including the 4 new ones, and shellcheck -x is clean on bin/omarchy-remove-ai-hermes.

Codex found no production defect and flagged one low-severity test-rigor gap, which I verified by mutation rather than taking on trust: the assertion at test/shell.d/hermes-remove-test.sh:174 only checks that gum was invoked with an argument literally equal to confirm, never that --default=false was passed. Stripping --default=false from the script entirely and rerunning the suite leaves all 14 tests passing, so the "default no" behavior isn't actually pinned down by a test — only the stub's exit-status env var controls accept/decline, independent of the real flag. The bootstrap-marker gating, the pipefail/du interaction, the rm -rf targets, and the omitted -d existence check relative to OpenClaw's version were each checked separately and are correct or harmless.

Nothing here blocks the PR; the test gap is a one-line fix if you want it — assert the gum-log contains an arg exactly --default=false, not just confirm.

🤖 Generated by Sonnet 5 in Claude Code. Reviewed by Codex XHigh.

@Chessing234 Chessing234 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

default-no confirm before wiping hermes data is the right fail-safe.

@spencerbull

Copy link
Copy Markdown
Contributor Author

Re-reviewed at the new head a1095af0, with a fresh independent second opinion from Codex at xhigh reasoning, since the new commit is a real design pivot rather than a fix to the earlier finding: it removes the bootstrap-marker gate on the offer to delete data, while keeping that gate on the unasked runtime deletion. Ran test/shell.d/hermes-remove-test.sh directly: all 15 tests pass, and shellcheck -x is clean.

Codex and I independently agree the design pivot itself is sound: no terminal or no gum still preserves everything, the default is still no, only an explicit yes deletes anything, and the prompt now names both paths explicitly so consent stays informed even without the marker.

One new low-severity defect, which I reproduced directly rather than took on trust: bin/omarchy-remove-ai-hermes:87-89 uses elif [[ -d $HOME/.hermes || -d $HOME/.config/Hermes ]] to decide whether to print the "your data is still there" message, but the message text unconditionally claims data survives in both paths. I seeded only ~/.config/Hermes (no ~/.hermes at all, no bootstrap marker) and ran the real script: it printed "Your chats, memories, and skills are still in ~/.hermes" for a directory that was never there. Cosmetic, not data loss.

Two test-rigor gaps, both confirmed by mutating the real script and rerunning the suite rather than by inspection alone: the --default=false gap noted in the previous review is still unaddressed — stripping the flag from the gum confirm call still passes all 15 tests. New gap: the -d $HOME/.hermes || -d $HOME/.config/Hermes existence guard on the prompt is live code (confirmed with a real empty-$HOME run that gum is correctly skipped), but removing that guard also still passes all 15 tests, because every seeded test case creates both directories — nothing exercises "Hermes was never installed at all, don't show an empty-looking prompt."

Nothing here blocks the PR.

🤖 Generated by Sonnet 5 in Claude Code. Reviewed by Codex XHigh.

@spencerbull
spencerbull merged commit 959e49d into omacom:quattro Sep 6, 2026
@spencerbull
spencerbull deleted the hermes-remove-ask-user-data branch September 6, 2026 01:00
ryanrhughes pushed a commit that referenced this pull request Sep 8, 2026
Ask, default no, before Remove Hermes deletes the user's data

(cherry picked from commit 959e49d)
(cherry picked from commit 4e77199945a15ac194511b1af6317761fb813ba0)
ryanrhughes pushed a commit that referenced this pull request Sep 8, 2026
Ask, default no, before Remove Hermes deletes the user's data

(cherry picked from commit 959e49d)
(cherry picked from commit 4e77199945a15ac194511b1af6317761fb813ba0)
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.

2 participants