Skip to content

fix(state): retire orphaned FTS v2 during storage optimize - #2

Open
Soju06 wants to merge 1 commit into
base/v2026.8.3from
soju/patches/fts-v2-orphan-hardening
Open

fix(state): retire orphaned FTS v2 during storage optimize#2
Soju06 wants to merge 1 commit into
base/v2026.8.3from
soju/patches/fts-v2-orphan-hardening

Conversation

@Soju06

@Soju06 Soju06 commented Aug 13, 2026

Copy link
Copy Markdown
Owner

Problem

hermes sessions optimize-storage aborts on this host:

Error: optimization failed: error in trigger messages_fts_v2_delete after rename:
       vtable constructor failed: messages_fts_v2
No data was lost. Re-run to resume.

messages_fts_v2 (tokenize='cjk_unicode61') was superseded by messages_fts_cjk in f13f845116. A DB that stopped mid-transition keeps the v2 vtable, its three AFTER INSERT/UPDATE/DELETE ON messages triggers, and its fts_v2_* state_meta markers — while no code references it any more (grep -c messages_fts_v2 *.py = 0 on HEAD).

Two defects turn that residue into a hard stop:

  1. Shadow collection was name-based. _demote_legacy_fts_to_trash gathered shadow tables with a messages_fts_% LIKE glob, which also matches every sibling generation's shadows (messages_fts_v2_data, messages_fts_v2_idx, ...). Renaming them re-parses the v2 triggers; without the loadable cjk_unicode61 tokenizer on that connection the vtable cannot be constructed, and the whole migration rolls back.
  2. Nothing retired the orphan. So the failure recurred on every re-run and the DB stayed on the pre-v23 layout — 20.0 GB for 4 GB of messages here, with ~14 GB of duplicate index copies.

Worth calling out separately: while the orphan exists, plain PRAGMA integrity_check fails with no such tokenizer: cjk_unicode61, so the database cannot be verified with stock sqlite3 at all. The size is a symptom; the stuck generation is the disease.

Fix

Both changes live in the single shared path, so sync and async callers inherit them.

  • Exact shadow matching. The shadow set is derived from the vtable names actually being demoted, combined with FTS5's fixed shadow suffixes (_data, _idx, _content, _docsize, _config), and matched with IN (...). A sibling generation can no longer be swept in by name resemblance.
  • Explicit orphan retirement before the demote. messages_fts_v2 is retired via a named allowlist, not a prefix scan — deliberately, so that a future generation cannot be dropped for merely looking similar. Trigger and vtable definitions are removed through writable_schema, which is why this works without the tokenizer present; the shadow tables are handed to the existing chunked teardown rather than deleted inline.
  • Fail-closed preflight. Retirement proceeds only when: messages is non-empty, a live base index exists and covers every row, the integrity check is clean, and rowcount parity holds. Otherwise the orphan is left untouched and the run behaves as before.

Verification

Tests fail on unpatched code and pass with the patch — verified by reverting hermes_state_search.py alone while keeping the new tests:

result
RED (fix reverted, tests kept) 3 failed — test_demote_only_stages_exact_legacy_generation, test_optimize_retires_poisoned_v2_but_preserves_live_cjk, test_orphaned_v2_alone_is_offered_and_retired
GREEN (on base 3c27eb623) 4 files, 27 tests passed, 0 failed incl. test_state_db_malformed_repair.py, test_fts_runtime_rebuild.py, test_fts_update_of_narrowing.py

Live run on the affected 20 GB database, gateway serving traffic throughout:

  • 20041.3 MB → 8164.3 MB (11.9 GB reclaimed)
  • PRAGMA integrity_check (tokenizer loaded): ok
  • No messages_fts_v2* or fts_v22_trash* residue; state_meta reduced to fts_storage_version=1
  • No data loss — messages 1,268,987 = messages_fts 1,268,987, parity holding while rows kept arriving mid-run
  • messages_fts_cjk present and serving: EN 87,848 hits / CJK 2-char 811 / trigram 3,100, all sub-10 ms

Notes

  • hermes_state_search.py is untouched by every other patch in the stack, so this applies directly on base and does not interact with the assembly fixes.
  • Upstream candidate: the defect is generic to any install that carries the cjk_unicode61 generation, not host-specific. Filed as local-only for now since it needs the v2 generation to exist to be observable.

`hermes sessions optimize-storage` aborted on this host with:

    error in trigger messages_fts_v2_delete after rename:
    vtable constructor failed: messages_fts_v2

`messages_fts_v2` (`tokenize='cjk_unicode61'`) was superseded by
`messages_fts_cjk` in f13f845, but a DB that stopped mid-transition keeps
the v2 vtable, its three `AFTER INSERT/UPDATE/DELETE ON messages` triggers,
and its `fts_v2_*` state_meta markers, while no code references it. Two
defects turn that residue into a hard stop:

1. `_demote_legacy_fts_to_trash` collected shadow tables with a
   `messages_fts_%` LIKE glob, which also matches every sibling generation's
   shadows (`messages_fts_v2_data`, ...). Renaming them re-parses the v2
   triggers; without the loadable `cjk_unicode61` tokenizer on the connection
   the vtable cannot be constructed and the whole migration rolls back.
2. Nothing retired the orphan, so the failure recurred on every re-run and the
   DB stayed on the pre-v23 layout — 20.0 GB for 4 GB of messages here, with
   ~14 GB of duplicate index copies.

The orphan also breaks plain `PRAGMA integrity_check` (`no such tokenizer`),
so the DB cannot be verified with stock sqlite3 while it exists.

Fix, both in the single shared path so sync/async callers inherit it:

- Derive the shadow set from the vtable names actually being demoted, using
  FTS5's fixed shadow suffixes, and match exactly. A sibling generation can no
  longer be swept in by name resemblance.
- Retire `messages_fts_v2` explicitly (named allowlist, not a prefix scan, so
  a future generation cannot be dropped for merely looking similar) before the
  demote. Trigger and vtable definitions are removed via `writable_schema`, so
  the tokenizer is not needed; shadows are handed to the existing chunked
  teardown. Preflight is fail-closed: messages present, live base index
  covering every row, integrity check clean, rowcount parity — otherwise the
  orphan is left untouched.

Verified on the reporting host: 20041.3 MB -> 8164.3 MB (11.9 GB reclaimed),
`integrity_check` ok afterwards, no `messages_fts_v2*` or trash residue, and
EN/CJK/trigram searches all serve correctly with the gateway running
throughout. Tests fail on the unpatched code and pass with it.

Origin: local-author
Upstream-PR: none
Patch-State: local-only
@Soju06
Soju06 changed the base branch from main to base/v2026.8.3 August 13, 2026 09:14
@chatgpt-codex-connector

Copy link
Copy Markdown

💡 Codex Review

https://github.com/Soju06/hermes-agent/blob/32f562255969e62f4001be25145814a9883f4806/.github/workflows/upload_to_pypi.yml#L1
P1 Badge Restore the PyPI publishing workflow

This deletion removes the only repository workflow that builds and uploads distributions on CalVer tag pushes; a repo-wide search of .github and scripts finds no replacement publisher. Consequently, the next tagged release will create no new PyPI package or signed distribution artifacts. This FTS maintenance fix also changes roughly 5,000 unrelated files relative to its parent, so these inherited changes should be removed by rebasing the focused three-file patch onto the intended base.

AGENTS.md reference: AGENTS.md:L170-L174


if indexed_rows != source_rows:
raise sqlite3.OperationalError(
"refusing to retire messages_fts_v2: live base FTS "
f"row-count mismatch ({indexed_rows} != {source_rows})"
)

P2 Badge Verify indexed row IDs before retiring v2

When the external base index has one missing message row and one stale row, its internal FTS integrity check succeeds and messages_fts_docsize still has the same count as messages, so this condition retires v2 and reports a successful optimization even though the live index does not cover every source message. This can occur after an interrupted or faulty repair and leaves historical terms absent from search; validate row-ID coverage in both directions rather than relying only on equal counts.

AGENTS.md reference: AGENTS.md:L80-L83

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Soju06 pushed a commit that referenced this pull request Aug 19, 2026
Addresses both review findings on the remote-gateway download PR:

1. Unbounded buffering (finding #1). fetchBuffer / fetchBufferViaOauthSession
   accumulated the entire response (then copied it again via Buffer.concat)
   before saveGatewayFile even opened the save dialog, so a large gateway file
   could exhaust the native process. Both auth paths now stream: once response
   headers arrive the connect timeout is cleared, the filename is derived, the
   save dialog is shown, and the body is piped to the chosen destination with
   backpressure. A read/write error tears down the stream and unlinks the
   partial file. The byte-moving, data-URL decoding, and filename/path helpers
   are extracted into gateway-file-download.ts so they're unit-testable without
   Electron.

2. No fallback for older gateways (finding #2). saveGatewayFile required the new
   /api/fs/download route. Desktop and the remote gateway update independently,
   so a gateway predating this PR 404s. Added a 404-only compatibility fallback
   to the existing capped /api/fs/read-data-url route (bounded, so it only
   serves smaller files — enough to keep older backends working).

Tests: gateway-file-download.test.ts covers streaming, backpressure,
error-cleanup (unlink on write/response error), data-URL decoding, filename
derivation (incl. traversal reduction), and 404 detection;
gateway-file-download-transport.test.ts asserts both transports stream (no
whole-body Buffer.concat) and that the 404 fallback is wired. Both registered
in the desktop platform test list. Server-side /api/fs/download tests
(streaming + sensitive-file reject) already pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Soju06 pushed a commit that referenced this pull request Aug 19, 2026
Two independent bugs let a deleted profile reappear / leave orphaned
resources on next launch:

1. hermes_cli/profiles.py's backend-process scanner required argv[0] to
   resolve to an executable literally named "hermes". Electron's
   pool-backend spawn resolves the hermes console-script shim's path and
   execs it via the interpreter directly (python3 /path/to/hermes ...), so
   argv[0] reports as "python3" and the scanner never matched the running
   backend -- delete removed the profile's files but left its live backend
   process running (still bound to a port via uvicorn), which
   accumulates across repeated delete/recreate cycles.
2. The desktop sidebar's ProfileRail only refreshed its cached profile
   list once, on mount, so a delete/create/rename from another surface
   (another window, or the CLI) left a stale ghost entry until something
   unrelated triggered a refetch. Note: a delete via this window's own
   Manage-Profiles view already refreshes the shared $profiles atom
   ProfileRail subscribes to (confirmed by reading refreshProfiles() and
   handleConfirmDelete()) -- this fix only covers the cross-window/cross-
   process staleness gap, not a duplicate of the already-merged
   NousResearch#57329's Manage-Profiles rail-refresh work.

Fix 1: recognize a python-interpreter argv[0] exec'ing a hermes-named
console-script shim via argv[1]. Fix 2: refresh the profile list on window
focus/visibilitychange, matching the existing pattern used elsewhere in
the sidebar (sidebar/index.tsx, use-background-sync.ts, star-map.tsx,
use-gateway-boot.ts all use the same focus+visibilitychange pattern).

## Related work already on main

PR NousResearch#57329 (merged) fixed the *headline* symptom from issue NousResearch#52279
(deleted profile respawns) via a different, non-overlapping mechanism:
routing profile-delete through the primary backend instead of spawning a
fresh pool backend, plus a separate recreation guard in
ensure_hermes_home() (NousResearch#49435, merged) that makes a backend spawned into a
deleted profile's directory raise FileNotFoundError instead of silently
recreating it.

This PR is NOT a duplicate of that fix. Verified: even with both of those
merged, a backend process that survives because of gap #1 above still
holds a bound port via uvicorn -- it just can no longer resurrect the
profile directory. That's real resource-hygiene, not a symptom already
covered. Gap #2 touches a different file/component (ProfileRail /
profile-switcher.tsx) than NousResearch#57329's rail-refresh half (which touched the
Manage-Profiles view's own $profiles.ts / index.tsx) and covers a
distinct staleness path (cross-window/cross-process, not same-window
delete-then-refresh).

Tests: tests/hermes_cli/test_profiles.py -- 156 passed (existing +
regression coverage for the argv[0] python-interpreter detection case).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Soju06 added a commit that referenced this pull request Aug 19, 2026
Optional owner-agnostic persona layer: when HERMES_HOME/personas/modes.yaml
exists, an audience mode is selected per session as a pure deterministic
function of session-constant inputs (platform, chat_type, chat_id, chat_name,
user_id) — first-match rules with string-or-list exact matching (chat_name
case-insensitive, missing key = wildcard, AND semantics), then a non-owner
guard (non-empty user_id not listed in owners[platform] forces
guards.non_owner_mode; missing owners entry fails safe; empty user_id skips),
then default_mode with unknown-mode fallback. The selected mode's persona
markdown is injected as stable-tier slot #2, immediately after the
SOUL.md/DEFAULT_AGENT_IDENTITY identity block, after passing the same threat
scan and truncation cap as SOUL.md. With no modes.yaml, prompt assembly is
byte-identical to before (strict no-op); every failure path degrades to the
same no-op at DEBUG.

Cache correctness: while active, the volatile tail gains an
"AudienceMode: <mode>" line next to Model:/Provider:, and
_stored_prompt_matches_runtime recomputes the expected mode via a cheap
mode-only resolver (no persona scan) — both-absent passes so pre-deploy
stored prompts stay valid, mismatch or one-sided presence rebuilds exactly
once per session. The label is distinct from Model:/Provider: so the
fallback model-swap regex rewrite cannot touch it. Loader/resolver are
re-exported through run_agent (load_soul_md pattern) for test patchability.

Plumbing parity with SOUL.md: personas/ added to the profile clone copy set
and the default-export include root, and the hermes_config_mod threat
pattern now also covers .hermes/personas/ paths. The artifact_register
section of modes.yaml is intentionally not consumed by core (tone-gate
plugin contract).

Origin: local-author
Upstream-PR: none
Patch-State: local-only

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.

1 participant