Skip to content

fix(state): make the corruption-recovery remedy pasteable for any db path - #119604

Open
Kyzcreig wants to merge 2 commits into
NousResearch:mainfrom
ANG-Ventures:up/state-repair-hint-pasteable
Open

Kyzcreig wants to merge 2 commits into
NousResearch:mainfrom
ANG-Ventures:up/state-repair-hint-pasteable

Conversation

@Kyzcreig

Copy link
Copy Markdown

The printed recovery command does not parse when pasted

hermes_state_repair prints a copy-pasteable salvage command at three sites on the
DB-corruption path, and interpolates the database path bare into the backticked span:

  • _persistent_repair_exhausted_error — two spans (--inspect-only, --output)
  • _backup_free_space_error — _MANUAL_RECOVER_HINT, returned to the operator by
    _backup_db_file on both refusal branches (no headroom / unstattable volume)

With a HERMES_HOME containing whitespace, the shell re-lexes the printed text before
argparse ever sees it. Measured on main @ ade48144 with a real /bin/bash and the real
sessions recover subparser:

printed  : hermes sessions recover --source /Users/x/My Drive/hermes/state.db --inspect-only
bash argv: ['hermes','sessions','recover','--source','/Users/x/My',
            'Drive/hermes/state.db','--inspect-only']
argparse : exit 2

All 4 printed spans split; the parser refuses every one.

This is reachable on ordinary installs, not a contrived path:

  • HERMES_HOME is an arbitrary user-set environment variable
  • Google Drive mounts as My Drive on macOS
  • the Windows default lives under C:/Users/<First Last>, where a space is the norm

And it lands on the one path where the operator has least slack: automatic repair has
already exhausted its budget, so the printed remedy is the remaining instruction — and it
is unreachable as printed.

The fix

hermes_cli/cli_hint.py (new, ~30 lines, stdlib shlex only) is the single place that
knows the two escaping rules:

  • the shell re-lexes the printed span, so whitespace splits, ;&|() are control
    operators, {a,b} brace-expands, *?[] glob against the operator's CWD, and
    $VAR / `cmd` / $(cmd) / ~ expand or execute;
  • argparse then binds a value beginning with - as an option and refuses with
    expected one argument.

hint_arg(flag, value) returns the plain --flag value form when shlex.quote leaves the
token unchanged — so ordinary paths keep the readable spelling and every existing message
is byte-identical
. Only a path that actually needs escaping gets the quoted attached
'--source=<path>' form, which is the only spelling argparse accepts for a value beginning
with -.

hermes_state_repair._source_arg routes the three sites through it, with an
always-quoted-attached fallback for scaffold/embed installs that have no hermes_cli
(the module already guards its other hermes_cli imports the same way).

I deliberately kept this to the three hermes_state_repair sites so the diff is reviewable.
The same bare interpolation exists at other guidance sites (hermes_cli/doctor_state.py,
agent/turn_explainers.py, gateway/run_notifications.py, hermes_cli/sessions_cmd.py,
hermes_state.py) — happy to follow up with those in a second PR routed through the same
helper, or to inline shlex.quote at the sites instead if you'd rather not take the helper.
Your call on the shape.

Verification — a real shell and the real parser, not a shlex simulation

tests/hermes_cli/test_state_repair_hint_pasteable.py refuses the tautology of simulating
the paste with shlex while the implementation decides safety with shlex:

  1. the real message builders are called (_disk_budget is patched to drive each real
    refusal branch — the message code itself is untouched);
  2. the backticked span is extracted from what they actually returned;
  3. the words come from a real /bin/bash (printf "%s\0", NUL-delimited so a word
    containing whitespace survives the round trip);
  4. those words are handed to the real sessions recover subparser built by
    build_sessions_parser, and it must bind the exact path to --source.

3 sites × 3 hostile directory shapes (My Drive, Program Files, First Last), plus
over-fix controls pinning that ordinary paths gain no quotes, plus the leading-- attached
form, plus the no-hermes_cli fallback, plus a choke-point test.

# clean main, ade48144 — the hostile cases only
9 failed
E   Failed: the real parser REFUSED the pasted remedy (exit 2)
E     printed: hermes sessions recover --source .../My Drive/hermes/state.db --inspect-only
E     argv: ['hermes','sessions','recover','--source','.../My','Drive/hermes/state.db','--inspect-only']

# this branch
14 passed in 1.27s

Regression check

Ran the touched subsystem on both trees, back to back, same conditions:

tests/hermes_state/ + tests/hermes_cli/test_sqlite3_cli_salvage_gate.py
                    + tests/agent/test_corruption_recovery_guidance.py

clean main  ade48144 : 4 failed, 1447 passed, 45 skipped, 1 error
this branch          : 4 failed, 1447 passed, 45 skipped, 1 error

Identical node IDs on both (test_cross_vm_fs_wal_refusal ×3,
test_guest_durability_barriers, and the test_state_db_repair_non_destructive error) —
inherited from the base, not added here. In particular the #100368 salvage gate
(test_sqlite3_cli_salvage_gate.py, 19 tests) and test_corruption_recovery_guidance.py
are green: the guidance still names the safe sessions recover lane and still warns against
pointing a raw sqlite3 shell at the live database. ruff check clean,
git diff --check clean.

Known limit

A literal backtick inside HERMES_HOME cannot be made safe inside a backticked span —
that is a message-format limit, not an escaping one, and this PR does not attempt it.

…path

`_persistent_repair_exhausted_error` and both `_backup_free_space_error`
refusals print a backticked `hermes sessions recover --source <db> ...`
span and tell the operator to paste it. The path was interpolated bare,
so a HERMES_HOME containing whitespace printed a remedy that does not
parse:

    printed  : hermes sessions recover --source /Users/x/My Drive/hermes/state.db --inspect-only
    bash argv: ['hermes','sessions','recover','--source','/Users/x/My',
                'Drive/hermes/state.db','--inspect-only']
    argparse : exit 2

Reachable on ordinary installs: HERMES_HOME is user-set, Google Drive
mounts as "My Drive" on macOS, and the Windows default sits under
C:/Users/<First Last>. The failure lands on the one path where the
operator has least slack -- automatic repair has already given up.

Adds `hermes_cli/cli_hint.hint_arg`, the single place that knows the two
escaping rules (shell re-lexing; argparse binding a `-`-leading value as
an option), and routes the three sites through `_source_arg`. Ordinary
paths keep the plain readable `--source <db>` spelling; only a path that
`shlex.quote` would change gets the quoted attached `'--source=<db>'`
form, which is the only spelling argparse accepts for a value beginning
with `-`. Installs without hermes_cli fall back to the same attached form.

Verified with a real /bin/bash and the real `sessions recover` argparse
subparser, not a shlex simulation: the printed span is extracted from
what the real builders returned, bash produces the words, and the real
parser must bind the exact path to --source. On clean main 9/9 hostile
cases fail (argparse exit 2); with this change 14/14 pass, including
over-fix controls pinning that ordinary paths gain no quotes.
…the predicate

The PR's `test_hint_arg_is_the_choke_point` asserts
`_source_arg(p) == hint_arg("--source", p)`. Both sides call the same
`hint_arg`, so the assertion holds BY CONSTRUCTION when the predicate itself
changes: it pins DELEGATION, not correctness. Measured -- narrowing
`_is_shell_literal` to the `shlex.split` under-approximation its own docstring
warns against leaves all 14 of the PR's tests green while a real /bin/bash
expands `$HOME`, EXECUTES `` `id` `` and `$(id)`, brace-expands `{a,b}` and
refuses `a;b` outright.

Adds `tests/hermes_cli/test_cli_hint.py` -- the owner suite for the helper,
driven through a REAL bash into a real argparse parser (a `shlex`-based paste
simulator would be the same function on both sides of the assertion).

Verified on this tree (repo venv, python 3.11.15):
  unmutated                     : 41 passed (guard) / 55 passed (guard + PR suite)
  _is_shell_literal -> shlex.split mutant:
      new owner suite           : 20 failed, 21 passed   <- kills it
      PR's own suite, same tree : 14 passed              <- the gap
  always-quote (over-fix) mutant:
      new owner suite           : 7 failed               <- readable form guarded
  _source_arg re-derives (bypass chokepoint) mutant:
      PR's own suite            : 11 failed              <- still owns delegation
      new owner suite           : 41 passed              <- complementary, not redundant
  guard kills the predicate mutant with hermes_state_repair.py REMOVED
  (20 failed) -- it fires from hint_arg's own surface, not via the call site.
  Adjacency: 68 passed across the hint-adjacent modules.
  ruff 0.15.20 and 0.16.2 clean; git diff --check clean.
  Both implementation files restored byte-identical after every mutant
  (cli_hint.py 71590f6c..., hermes_state_repair.py 05c05afe...).

No implementation file changed; this commit is test-only.
@Kyzcreig

Copy link
Copy Markdown
Author

Follow-up from our own round-2 review of this PR: the port brought hermes_cli/cli_hint.py across but not an owner suite for it, and the predicate turned out to be unguarded. Pushed f6f1f7ac4f (test-only, +126/-0) to address it.

The gap, measured. test_hint_arg_is_the_choke_point asserts _source_arg(p) == hint_arg("--source", p). Both sides call the same hint_arg, so the assertion holds by construction when the predicate itself moves — it pins delegation, not correctness. Narrowing _is_shell_literal to the shlex.split under-approximation its own docstring warns against:

-    return shlex.quote(value) == value
+    return len(shlex.split(value)) == 1

leaves all 14 tests on this branch green, while a real /bin/bash expands $HOME, executes `id` and $(id), brace-expands {a,b}, and refuses a;b / a(b)c outright — 7 of 14 path shapes broken, suite still green.

What the commit adds. tests/hermes_cli/test_cli_hint.py — the owner suite for the helper. Same instrument as the existing test module (printed string → real /bin/bash → real argparse); a shlex-based paste simulator would put the same function on both sides of the assertion, which is the failure mode above.

Numbers on this tree (repo venv, python 3.11.15, -p no:randomly):

run new owner suite this PR's existing suite
unmutated 41 passed 14 passed
_is_shell_literal → shlex.split 20 failed, 21 passed 14 passed
always-quote (over-fix) 7 failed, 34 passed 2 failed, 12 passed
_source_arg re-derives (bypasses hint_arg) 41 passed 11 failed

The last row is the point: the two suites are complementary, not redundant. The existing chokepoint test owns delegation and kills the bypass mutant; the new suite owns the predicate. Neither covers the other.

The new suite also kills the predicate mutant (20 failed) with hermes_state_repair.py removed from the tree entirely — it fires from hint_arg's own surface, not through the call site.

Adjacency: 68 passed across the hint-adjacent modules. ruff 0.15.20 and 0.16.2 both clean, git diff --check clean. No implementation file changed; both were restored byte-identical after every mutant (cli_hint.py sha256 71590f6c…, hermes_state_repair.py 05c05afe…).

Deliberately not included: hint_value and the fork-side tests that depend on it, the kanban parser, or the update-rollback / live-checkout message sites. Those are out of this PR's scope and that scoping was correct.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard area/sessions Session lifecycle, resume, persistence, history sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Sep 23, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants