perf(state): port external-content FTS storage and CJK tokenizer - #1
Merged
Conversation
…ma v23) (NousResearch#65798) * fix(desktop): refresh repo status on session switch with unchanged cwd (#68208) fix(desktop): refresh repo status on session switch with unchanged cwd * fix(checkpoints): honor gateway config and task cwd (#68195) * fix(gateway): wire checkpoint config into agents * fix(checkpoints): resolve gateway file paths by task cwd * ci: live-updating PR review comment with structured job statuses Replace the static comment-pending + comment-results two-job pattern with a live-updating comment system that polls the GitHub Actions API every 15s, re-assembles the review comment from whatever results are available, and upserts it via the <!-- hermes-ci-review-bot --> marker. The comment updates in real time as each job finishes — no waiting for the full pipeline. Every CI job that wants to appear in the review comment emits a review_status output — a JSON array of objects, each with a source and a results array: [ { "source": "review-label-gate", "results": [ {"kind": "action_required", "title": "...", "summary": "...", "how_to_fix": "..."}, {"kind": "info", "title": "...", "summary": "..."} ] }, { "source": "ci timing", "results": [ {"kind": "warning", "title": "CI timings", "summary": "...", "detail": "...", "link": "..."} ] } ] One job can emit multiple results of different kinds. The source field is used to exclude the corresponding job from the synthesized error list (case-insensitive, hyphen-normalized matching against GitHub Actions job display names). | job | source | kind (on failure) | section | |----------------------------|--------------------------|---------------------------|----------------------| | review-labels | review label gate | action_required / info | Action required | | lockfile-diff | lockfile-diff | action_required | Action required | | ci-timings | ci timing | warning / info | Warnings | | supply-chain scan | supply chain | error / (none) | Job failures | | supply-chain dep-bounds | supply chain | action_required / (none) | Action required | | osv-scanner | osv scan | warning / (none) | Warnings | | uv-lockfile-check | uv.lock check | action_required / (none) | Action required | | history-check | unrelated histories | action_required | Action required | | contributor-check | contributor attribution | action_required | Action required | Jobs that find nothing emit [] (empty array) — no noise info items. A single comment-live job polls the GitHub Actions API every 15s, classifies jobs into (completed, pending), assembles the comment, and upserts it. Merges review_status outputs from all needs jobs via toJSON(needs.*.outputs.review_status), and downloads the ci-timings artifact when it becomes available. Shows commit SHA + message below the header. The assembler has ZERO job-specific knowledge. It just: 1. collect_from_statuses() — flattens all nested status objects into ReviewItems 2. collect_failed_jobs() — synthesizes errors for failed jobs with no declared status 3. _attach_job_urls() — fills in per-job log links for ALL items 4. render_comment() — groups by severity, renders with group headers Each item shows links inline next to the title: View report (job-emitted URL) and View job (auto-attached logs link). Each info item is its own collapsible <details> block. # ૮ >ﻌ< ა ci review running on abc1234 — commit message first line ## ❌ Job failures ### {title} · [View job](url) {summary} ## ⚠️ Action required ### {title} · [View job](url) {summary} **How to fix:** {how_to_fix} ## ⚠️ Warnings ### {title} · [View report](url) · [View job](url) {summary} {detail} <details><summary>{title}</summary> {content} </details> Still running 3 jobs: ci-timings, docker - test_assemble_review_comment.py (48 tests): collect_from_statuses, collect_failed_jobs with exclude_sources, _attach_job_urls, render_comment (group headers, inline links, commit info, per-item details, pending footer), assemble integration - test_live_comment.py (16 tests): classify_jobs pure function - test_timings_report.py (10 tests): generate_review_status nested format - test_lockfile_diff.py (6 tests) - test_classify_changes.py (32 tests, pre-existing) * ci: migrate AUTOFIX_BOT_PAT to GitHub App token Replace the long-lived fine-grained PAT (AUTOFIX_BOT_PAT) with short-lived (1-hour) installation access tokens minted via a new get-app-token composite action wrapping actions/create-github-app-token@v3.2.0. The PAT was used in 13 spots across 8 workflow files for gh CLI / GitHub API calls. The per-repo GITHUB_TOKEN (1,000 req/hr) was getting rate-limited when multiple workflows fire concurrently (deploy-site, skills-index, ci-timings, supply-chain-audit, js-autofix). App installation tokens get 5,000 req/hr per installation and are scoped to the App's permissions, not a user account. New composite action: .github/actions/get-app-token/ - Wraps actions/create-github-app-token@bcd2ba49 (v3.2.0, SHA-pinned) - Reads APP_ID + APP_PRIVATE_KEY repo secrets - Outputs a 1hr installation token via steps.app-token.outputs.token Requires two new repo secrets (set after creating the GitHub App): - APP_ID: the App's numeric ID - APP_PRIVATE_KEY: the PEM private key App installation permissions needed: contents: write (js-autofix push, pypi release upload) pull-requests: write (js-autofix PR create/merge, supply-chain comment) issues: write (skills-index-freshness issue creation) actions: write (skills-index workflow trigger) workflows: write (skills-index triggers deploy-site.yml) The AUTOFIX_BOT_PAT secret can be deleted once CI passes on this PR. The comment in js-autofix.yml noting that PAT pushes trigger downstream workflows is updated — App tokens have the same property (they are not GITHUB_TOKEN), so the concurrency-cancel loop logic is unchanged. * style(desktop): satisfy merged eslint/prettier config The SSH modules predate the stricter lint config that landed on main (curly, no-empty, perfectionist sorting, prettier). Mechanical lint:fix + fmt pass, empty catch blocks filled with the codebase's void-0 convention, and inline no-control-regex disables on the three deliberate control-char patterns (same pattern as lib/ansi.ts). * fix(ci): pass App secrets as inputs to composite action Composite actions cannot access the secrets context — the runner's template engine rejects secrets.* references at load time with 'Unrecognized named-value: secrets'. Move APP_ID and APP_PRIVATE_KEY from direct secrets.* references inside the composite action to inputs passed by each calling workflow. The fallback logic (GITHUB_TOKEN when APP_ID is empty, for fork PRs) stays in the composite action's check step. * fix(ci): add detect to all-checks-pass needs so its failure blocks merge If detect fails, all downstream sub-workflows get SKIPPED (they have needs: detect). all-checks-pass used if: always() and only checked the sub-workflows — which all showed as 'skipped' (= success) — so it passed even though the root cause (detect) failed. This made the PR mergeable despite a broken CI pipeline. Add detect to all-checks-pass needs so its failure propagates to the gate job and blocks the merge. * fix(desktop): bump skills test timeout to fix cold-start flake (#68235) Test 1 in skills/index.test.tsx pays the full cold-start cost (jsdom env init + module transform + the @/hermes/@/store/profile import graph), which pushed past vitest's 5000ms default under load — caught at 8871ms on one run, 6.6s pure test time on another. Tests 2-4 are ~30-130ms each because all that setup is already cached, so only test 1 was at risk of timing out. Bump the describe-level timeout to 15s. Verified with 10 consecutive runs, 4 of which took 5.5-6.6s of test time and would have hard-failed under the old 5s default. * feat(desktop): open multiple full app windows (electron) Add createInstanceWindow() — a full-chrome peer of the primary that renders the complete app (sidebar, routing, its own draft) against the shared backend, so several GUI windows can run at once. Mirrors the primary's window options + chatWindowWebPreferences (backgroundThrottling stays off so a streamed answer never stalls when blurred) but never overwrites the mainWindow global and doesn't respawn the backend — the renderer's getConnection() joins the running one. New windows cascade off their source via the pure, tested instanceWindowBounds(). Exposed via the hermes:window:openInstance IPC and a "New Window" File menu item. Per-window fullscreen state now targets the window itself, and titlebar/native-theme repaints reach every open chat window instead of only the primary. Retires the now-orphaned compact new-session pop-out (its only caller was ⌘⇧N, repointed in the follow-up commit): drops createNewSessionWindow, the hermes:window:openNewSession handler, and the newSession/new=1 URL flag. * feat(desktop): wire New Window to ⌘⇧N + command palette Repoint session.newWindow (⌘⇧N) from the compact new-session pop-out to openNewWindow(), which opens a full peer instance via the new openWindow bridge, and add a "New Window" entry to the ⌘K palette (shown with its hotkey hint, gated on canOpenNewWindow()). Relabel the action "New window". Drops the retired openNewSessionWindow bridge and the vestigial isNewSessionWindow()/new=1 flag; renames the shared opener helper. * fix(desktop): de-dupe cross-window cues so peers don't spam With multiple full windows, each renderer independently reacts to the same backend event, so one-shot cues fired N times: OS notifications (the per-renderer throttle can't see other windows), the turn-end sound (playCompletionSound runs on every message.complete, ungated by focus), and auto-spoken replies (double voice when a chat is open in two windows). Add a single race-free owner in the main process (electron/event-dedupe.ts): main handles IPC serially, so the first window to claim a key within a short window wins and peers stay quiet. Notifications collapse at the hermes:notify choke point; the sound and spoken replies claim via a new hermes:ambient:claim IPC (keyed by session / reply id). Off Electron the claim falls back to "emit", preserving single-window behavior. The sound's mute check runs before the claim so a muted window can't win the cue and silence an audible peer. * refactor(desktop): tidy the cross-window deduper Drop the unused DEDUPE_WINDOW_MS export and rename its interval so "window" isn't overloaded against BrowserWindow in a multi-window feature (windowMs → intervalMs). DRY the completion-sound play path. No behavior change. * nix: add cage to devDeps * fix(desktop): avoid false remote gateway reauthentication (#68250) * fix(desktop): avoid false remote gateway reauthentication Co-authored-by: Rod-fernandez <rodrigo@nxtlevelsaas.com> Co-authored-by: David Andrews (LexGenius.ai) <david@lexgenius.ai> * fix(desktop): harden remote revalidation state --------- Co-authored-by: Rod-fernandez <rodrigo@nxtlevelsaas.com> Co-authored-by: David Andrews (LexGenius.ai) <david@lexgenius.ai> * fix(desktop): keep composer draft across compression tip rotation (#68079) * fix(desktop): keep composer draft across compression tip rotation Auto-compression swaps the live stored session id while the user may still be typing. Scope the composer/queue key on the lineage root and migrate any tip-keyed draft/queue entries onto that durable key when the tip rotates so the in-progress prompt does not vanish when the response lands. * test(desktop): cover draft survival across compression tip rotation Add regression coverage for migrateSessionDraft, lineage-scoped composer keys, and the rotation path that previously wiped an in-progress draft. * fmt(js): `npm run fix` on merge (#68305) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * fix(desktop): Stop parks the queue instead of firing the next queued prompt Interrupting a busy turn with the Stop button (or Esc) settles the session to idle, and the edge-independent auto-drain immediately submits the head of the composer queue. The user pressed Stop to halt the agent, but it looks like Stop skipped the current turn and kept going — and the queued text is hard to find, since its only surface is the collapsed 'N queued' pill above the composer. The old userInterruptedRef latch (a23728dcc) fixed this but was removed in #40221 because it also suppressed the drain that send-now-while-busy depends on. This reintroduces the halt with source awareness instead of a blanket latch: - Explicit halts (Stop button, composer Esc, chat-focus Esc, the streaming message's hover Stop, runtime cancel) park the session's queue before interrupting. Parked queues are skipped by both auto-drain paths (mounted ChatBar + background drainer). - Interrupts that exist to advance the queue (send-now-while-busy) unpark first, so the settle drain they rely on still flows. - The park lifts on any renewed intent: resume, a manual drain (Enter on empty composer or the per-row send arrow), queueing a new prompt, or emptying the queue. It migrates with entries on a runtime re-key and is deliberately not persisted (a fresh process starts unparked). - The queue panel expands on park, switches to 'N Queued — paused' with a pause icon, and grows a Resume action, so the held prompts are visible instead of reading as vanished. Store contract, hook wiring, and background-drain coverage included; docs updated. * fix(cli,tui): recall real paste content on up-arrow Large pastes collapse to a placeholder in the composer, but input history stored the placeholder — so up-arrow recall showed a truncated reference (CLI) or lost the content entirely (TUI, where the `[[…]]` label has no backing snip after submit). Store the expanded content in history instead: - CLI: `_inline_pastes()` expands `[Pasted text #N -> file]` into the buffer before `reset(append_to_history=True)`; also reused by the external editor (dedup). History nav suppresses re-collapse of recalled content. - TUI: `dispatchSubmission` pushes `expandSnips(pasteSnips)(full)`; idempotent on label-free text so re-submitting a recalled entry stays stable. * fix(cli): suppress CPR on POSIX local TTYs under load Delayed ESC[6n replies leak as ^[[row;colR into the classic CLI on SSH/slow PTYs (#13870) and on local POSIX TTYs under heavy subagent load. Suppress CPR on non-Windows platforms (layout hint only); keep native Windows on prompt_toolkit's default pending native coverage. Wire selection through _select_classic_cli_pt_output. * test(cli): prove local CPR leak and Application CPR-disabled wiring Add a delayed-CPR PTY harness (no SSH) plus selection/Application assertions for POSIX local and Windows preserve-default. Update the gating unit test to the new contract. * refactor: drop platform kwarg, fix PTY test cleanup - Remove redundant platform= test seam from _terminal_may_leak_cpr(); use monkeypatch.setattr(sys, 'platform', ...) consistently in both test files. - Wrap PTY tests in try/finally for fd cleanup on assertion failure. - Guard select.select() in terminal thread against OSError after fd close (fixes PytestUnhandledThreadExceptionWarning). - Trim PR-number reference from test module docstring. * docs(portal): remove retired Nous Chat references * fix(web/ddgs): isolate DuckDuckGo search in a disposable process ThreadPoolExecutor timeouts cannot fire when primp holds the GIL in native code (#68096). Run each search in a child process the parent can terminate/kill, and honor tools.interrupt between polls. * test(web/ddgs): cover GIL-hold timeout, interrupt, and worker reap Regression tests for #68096: native GIL-hold and sleep hooks must time out or interrupt promptly with no orphaned search workers. * fix: sanitize subprocess env for DDGS worker os.environ.copy() passes all Hermes secrets (gateway tokens, API keys, dashboard session tokens) into the DDGS child process. Use _sanitize_subprocess_env() to strip Hermes-managed secrets before spawning the worker. * fix(agent): pass persisted-prefix boundary when rotation flushes on cold resume (#68196) The legacy rotation branch in agent/conversation_compression.py flushes the current turn to the OLD session before ending it (#47202) via _flush_messages_to_session_db(messages) with no conversation_history boundary. On the first turn after a cold Desktop resume, the restored transcript rows live in the message list as plain dicts that have not yet been stamped with _DB_PERSISTED_MARKER — the normal turn flush that stamps them runs after preflight compression. With no boundary, _flush_messages_to_session_db builds an empty history_ids set and treats every restored row as new, durably re-appending the whole transcript to the parent session. Repeated restart/resume + threshold compression keeps growing the parent transcript. Pass messages[:_persist_user_message_idx] (the already-durable prefix that turn_context anchors before preflight runs, guarded for int/bounds) as conversation_history so the flush skips the persisted rows by identity and writes only the current turn's new messages. Adds a regression test that pre-populates SQLite, cold-loads the transcript, appends one current user row, and forces rotating compression: it fails before this change (parent grows to 5 rows) and passes after (parent holds the two originals plus the single new turn). * fix(desktop): prevent contentEditable composer input from visually collapsing to near-zero height Fix #68095 The composer input box (contentEditable div) randomly shrank to a tiny/pixelated size when typing character-by-character (paste worked fine). Root cause: during per-keystroke input, the normalizeComposerEditorDom cleanup could briefly leave the contentEditable with zero child nodes, and without intrinsic content the browser collapsed it visually despite the CSS min-height. Two-pronged fix: 1. Add min-h-[1.625rem] bracket syntax alongside the CSS variable min-height to ensure the minimum height is enforced even if the CSS variable resolution is delayed or overridden by browser defaults. 2. In normalizeComposerEditorDom, ensure the contentEditable always has at least one <br> child when empty, giving it intrinsic height that the browser cannot collapse. This is a belt-and-suspenders approach with the CSS min-height. Closes #68095 * fix(agent): circuit-break AttributeError from commit-splice and detect code skew Fix #68178 The git-install auto-updater rewrites source while the desktop backend is live. Because agent/conversation_loop.py is imported lazily on the first API call, a process can end up running two different commits spliced together — one commit's AIAgent against another commit's conversation_loop. When the interface differs, every turn fails permanently with an AttributeError, and the loop retries indefinitely, burning provider API calls (576 failures, 149 wasted API calls observed). Three-prong fix: 1. Circuit-break AttributeError on agent objects: the outer-loop error classifier now detects AttributeError targeting agent/run_agent modules and breaks immediately instead of continuing the retry loop. 2. Code skew detection for desktop/serve backend: run_agent.py now snapshots the checkout revision at import time and exposes a cheap per-iteration check that the conversation loop uses to refuse new work with a clear 'restart required' message before the lazy import can crash. 3. Informative error message: when code skew is detected, the user gets a clear explanation of the mismatch (boot revision vs current revision) and actionable guidance to restart the application. * fix(telegram): preserve fatal recovery handoff Release the current polling-recovery task's ownership before invoking the fatal-error handler. The runner bounds adapter cleanup in a child task; disconnect() cancels the tracked polling-recovery task, so retaining the current notifier in _polling_error_task would cancel the fatal callback before the runner can finish its reconnect-queue or shutdown decision. The new _handoff_polling_fatal_error() helper clears _polling_error_task only when it is the current notifier. Other recovery tasks remain tracked and are still cancelled and awaited during teardown. Covers both network retry exhaustion and polling-conflict exhaustion. Replaces the misleading "Restarting gateway" message with "Escalating to gateway recovery". Fixes #68406. * fix(telegram): widen fatal handoff to heartbeat watchdog path The wedged-recovery heartbeat watchdog (line 2526) calls _notify_fatal_error() directly from the heartbeat task. disconnect() cancels _polling_heartbeat_task unconditionally (no current_task guard, unlike _polling_error_task). Same bug class as #68406: the child disconnect cancels the heartbeat parent before the runner can queue reconnect. Widen _handoff_polling_fatal_error() to also clear _polling_heartbeat_task when it is the current task, and route the heartbeat watchdog call site through the handoff helper. Co-authored-by: Imgaojp <6065749+Imgaojp@users.noreply.github.com> * fix(tests): make the live-system-guard canary fail closed tests/test_live_system_guard_self_test.py executes real kill primitives (os.kill(-1, SIGTERM), os.killpg, pkill -f python) and depends entirely on the autouse _live_system_guard fixture in tests/conftest.py to intercept them. That makes the canary fail-OPEN: in any collection context where the file is present but its home conftest is not — a published sdist that ships tests/ but not tests/conftest.py, a tree assembled by copying test*.py (that glob does not match conftest.py), pytest --noconftest, or a foreign rootdir — the primitives fire for real, and os.kill(-1, SIGTERM) SIGTERMs every process the invoking user owns (a full desktop-session kill was reported in the field). Add an autouse fixture that refuses to run any canary test unless the guard is provably active. The one thing the canary can detect about its own safety is that the guard monkeypatches os.kill with a plain Python function, whereas the unguarded primitive is a C builtin — so the probe keys off that. Tests marked @pytest.mark.live_system_guard_bypass still opt out, matching the guard's own bypass contract (e.g. test_bypass_marker_disables_guard). With the guard loaded every canary test behaves exactly as before; without it each test refuses at setup with zero side effects. Fixes #68311 * fix(billing): rename user-facing "terminal billing" copy to Remote Spending (#68355) * fix(billing): rename user-facing "terminal billing" copy to Remote Spending The capability was renamed Remote Spending on the portal (consent CTA: "Allow Remote Spending"; per-terminal states Granted/Stopped), but the terminal, desktop, and docs still said "terminal billing" everywhere. - Feature name: Remote Spending in titles/labels, lowercase mid-sentence. - Step-up action verb is now "allow", matching the portal consent CTA. - Kill-switch-off recovery copy points at the actual control ("a billing admin can turn it on from the portal's Hermes Agent page") instead of the dead-end "manage it on the portal". - Per-terminal revoke copy uses the portal vocabulary ("stopped"). - Wire identifiers (cli_billing_enabled, cli_billing_disabled, ...) are unchanged; copy, comments, docs, and test expectations only. * fix(billing): correct the post-step-up denial diagnosis + finish the desktop rename Adversarial review findings: (1) a repeated insufficient_scope after a successful step-up is a per-terminal authorization failure, but the copy blamed the org kill-switch and pointed at the wrong recovery control — now: "Remote Spending still isn't active for this terminal — the authorization didn't take. Retry, or make this change on the portal." (2) the desktop step-up flow started in Remote Spending vocabulary but finished in "billing management access" — renamed both end states. (3) prettier formatting on the touched files (matches the post-merge fmt bot). * feat(tui): show the plan catalog in /subscription on Free (#68357) * feat(tui): show the plan catalog in /subscription on Free The server returns the tier list even with no subscription, but the overlay hid the picker behind can_change_plan && !isFree, so a Free account got only "Start a subscription" with no idea what the plans cost. Now: - Overview on Free offers "Choose a plan" whenever the catalog has enabled paid tiers. - The picker on Free lists each plan as name · price · monthly credits (no upgrade/downgrade hints — there is nothing to move from), and picking one opens the portal, where starting a subscription actually happens (card capture + checkout live there; the upgrade RPC requires an existing subscription). - Paid-plan behavior (preview → confirm → apply) is unchanged. * refactor(tui): compute the picker row suffix once Review feedback: the isFree fork duplicated the label template and run handler; only the suffix differs. * fix(tui): arm the busy guard before the Free portal handoff Adversarial review: the Free branch returned before setting busyRef, so a double-Enter could open the portal twice; and the picker narrated a handoff that openManageLink already narrates (duplicate on success, contradictory on failure). Guard first, let the helper do the talking. * fix(tui): monthly credits are dollars — label them as such The Free picker showed "1000 credits/mo" for what is $1,000 of monthly credit — render "$1,000 credits/mo" (grouped, dollar-signed). * feat(tui): render the Free-plan catalog inline in the /subscription overview Sid ruling: the upsell belongs where the user already is — no intermediate "Choose a plan" hop. On Free the overview lists each paid plan (name · $/mo · $credits/mo) as a pickable row; picking opens the portal (openManageLink narrates). The generic "Start a subscription" row survives only when the catalog is empty. The picker reverts to its original change-only form (Free never reaches it). * feat(desktop): tier catalog chips on the Subscription row Desktop parity with the TUI inline catalog (Sid ruling): accounts that can act see the plans where they already are — Free gets the upsell list (every chip opens the portal), a subscriber sees all tiers with the current one marked inert. Members and team contexts see no chips. Chips learn an optional url (portal handoff) in the shared row model. * chore(tui): fixture harness mirrors the live tier catalog The dev screenshot fixtures showed invented plans ($50 Super / $99 Ultra, "1,000 credits"); align with the real catalog ($20/$100/$200 with $22/$110/$220 monthly credits) so fixture renders cannot be mistaken for product truth. The overlay itself always reads tiers from the subscription API. * chore: trim narration comments * fmt(js): `npm run fix` on merge (#68462) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * fix(relay): attach metadata.user_id on guild replies for egress fallback (#68320) The relay adapter re-attaches an egress discriminator on outbound replies so the connector can resolve the owning tenant. It captured scope_id for scoped (guild) messages and user_id for DMs, but as MUTUALLY EXCLUSIVE: a scoped inbound hit an early return, so the author's user_id was never recorded, and _with_scope only attached user_id when there was no scope_id. Guild replies therefore went out with scope_id only. That's fine while the guild has a provision-time route row. But a MANAGED Discord agent joins guilds dynamically (the shared bot is added to / removed from servers at runtime), and GATEWAY_RELAY_ROUTE_KEYS — the only thing that writes guild route rows — is a self-hosted, static field never stamped for managed agents. So their guild has no route row, the connector's guild-route lookup misses, and with no user_id on the frame there's nothing to fall back to → every guild reply is declined "discord egress declined: target not routed to an onboarded tenant" even though INBOUND resolved the same guild fine (via the author-first SharedSocketRouter.targets() fallback). Fix: capture the authentic author user_id for EVERY inbound (DM and scoped alike) and re-attach it on the outbound reply alongside scope_id. The connector consults it only on a route/scope miss, so carrying both never overrides routing-table resolution. This is the gateway half of the paired gateway-gateway change (makeDiscordTenantOf guild-route-miss author-binding fallback); together they make guild replies resolve the same observed-author way inbound already does. Tests (tests/gateway/relay/test_relay_adapter.py): a guild reply now carries both scope_id AND user_id; a scoped inbound with no author still yields scope_id only (never invents one). Verified fail-without / pass-with. * build: declare pywin32 as a direct win32 dependency hermes_cli/windows_ssh_runtime.py imports win32security/win32file/etc. directly but pywin32 only arrived transitively via concurrent-log-handler -> portalocker. Declare it with a sys_platform gate so the Windows SSH runtime doesn't depend on the logging dep chain. Review follow-up on PR #68130. * fix(desktop): preserve dragging with empty titlebar slots * Revert "fix(agent): circuit-break AttributeError from commit-splice and detect code skew" This reverts commit 3a9b9d65d505646212c4c875bab19b96ae14b2e6. * fix(context): revalidate Codex OAuth context windows * test(context): document Codex cache persistence coverage * fix(context): scope Codex catalogue cache by credential * test(context): cover Codex context rollback * fix(compression): report live-resolved Codex window in the autoraise notice The autoraise banner hardcoded '272K' for the gpt-5.4/5.5/5.6 family, but the Codex /models catalog is authoritative and shifts server-side (gpt-5.6 served 372K during July 9-18, 2026 before OpenAI rolled it back). Pass the compressor's live-resolved context_length through so the notice reports the window the session actually got; the static 272K/128K text remains as the fallback when no resolved value is available. * fix(codex): send ChatGPT-Account-Id on /models probes The Codex backend returns the per-account model catalog only when the ChatGPT-Account-Id header is present. Without it, GET /backend-api/codex/models responds 200 OK with {"models":[]} and the picker silently degrades to the hardcoded fallback list — which is stale or wrong for the active plan (no GPT-5.6 family, wrong context windows). This was the upstream bug behind slow first responses and HTTP 520/120s SSE hangs: Hermes was sending invalid slugs because the probe never saw them in the catalog, and Codex's request builder also depends on the same JWT claim that's now being threaded through both probe paths. Fixes the probe-side paths in hermes_cli/codex_models.py and agent/model_metadata.py by extracting chatgpt_account_id from the OAuth JWT (mirroring the request-side logic already in auxiliary_client.py) and sending it as a header. Verified live: - _fetch_models_from_api now returns the 10-model catalog (gpt-5.6-sol, gpt-5.6-terra, gpt-5.6-luna, gpt-5.5, gpt-5.4, gpt-5.4-mini, gpt-5.3-codex-spark, 3x -pro variants) instead of []. - _fetch_codex_oauth_context_lengths resolves all 8 account models to 272K context (matches direct API probes of the same account). - end-to-end: hermes chat -m gpt-5.6-sol -q 'Reply with one word: pong' returns 'pong' cleanly via the openai-codex route. Same class of bug as PR #64760. * test(codex): cover ChatGPT-Account-Id header on /models probe Add regression tests locking in the new behavior: a JWT carrying a chatgpt_account_id claim causes the probe to send ChatGPT-Account-Id, while a malformed token omits the header instead of crashing. * fix(tools): make the tool-search context gate provider-aware (#68589) _resolve_active_context_length() called get_model_context_length() with the model id alone, so provider-enforced windows (e.g. Codex OAuth's 272K for gpt-5.x vs the direct API's 1.05M) never reached the tool-search activation gate — it sized against generic metadata for the same slug. Resolve the runtime provider for the configured model and pass provider, base_url, and api_key through. If credential resolution fails (offline, no keys), degrade to a provider+base_url-only lookup so the static provider-aware fallbacks still apply; explicit model.context_length keeps short-circuiting as before (#46620). Gap flagged during review of #16735. * feat(skills): bundle docx, xlsx, and pdf office skills; refresh powerpoint (#68595) Non-technical users asking for Word docs, spreadsheets, or PDF work had no bundled skill coverage — docx/xlsx creation required discovering and installing hub skills, and PDF manipulation had no skill at all beyond OCR extraction and nano-pdf edits. - skills/productivity/docx: create (docx-js), edit (unzip -> XML -> zip), tracked changes, comments, validation. Adapted from anthropics/skills. - skills/productivity/xlsx: openpyxl creation/editing, mandatory LibreOffice recalc gate, formula-compatibility rules, financial-model conventions. Points at optional excel-author for finance-grade work. - skills/productivity/pdf: merge/split/rotate/watermark/encrypt, form filling (AcroForm + flat overlay scripts), text/table extraction, reportlab creation, forms.md + reference.md companions. - skills/productivity/powerpoint: synced to current upstream pptx skill — richer pptxgenjs corruption footguns, template workflow, validate.py + validators + thumbnail.py, font-substitution QA guidance; drops the stale pack.py/editing.md/pptxgenjs.md workflow files. - Cross-linked ocr-and-documents, nano-pdf, excel-author via related_skills so each office skill routes to its siblings. - deliverable-mode docs mention the new skills; regenerated per-skill docs pages, catalogs, and sidebar. - tests/skills/test_office_document_skills.py: frontmatter contracts, referenced-script existence, schema-map integrity, cross-link resolution, script compilation. E2E validated: docx create->render->edit->validate, xlsx recalc (SUM + _xlfn.TEXTJOIN evaluate correctly), pdf create->merge->extract, pptx generate->validate->thumbnail. * fix(approval): raise gateway approval timeout to 300s, honest stale-tap UX, offer Always on mixed prompts (#68597) Three related messaging-approval fixes: 1. approvals.timeout default 60 -> 300. PR #63501 collapsed the gateway wait onto the canonical approvals.timeout (previously gateway_timeout=300), silently shrinking messaging approval windows to 60s. Push-notification approvals routinely arrive later than a minute; taps landed after the wait had already failed closed. 2. Stale-tap honesty: adapters resolved the approval AFTER rendering '<checkmark> Approved by <user>' (Telegram/Discord/Slack), or ignored a zero resolve count (WhatsApp Cloud/Feishu). A tap on an expired prompt claimed approval while the command had already been denied. All button paths now resolve first and render 'Approval expired - command was not run' when nothing was waiting. 3. Mixed-warning prompts (dangerous pattern + tirith finding) now offer Always: the persistence layer already permanently allowlists the pattern key and downgrades the tirith key to session scope, but the UI hid Always whenever ANY tirith warning was present. Pure-tirith prompts still withhold Always (content findings are session-max by design), and Smart-DENY overrides remain once-only. * feat(secrets): one-command token rotation + actionable startup errors for all secret sources (#68605) * feat(secrets): one-command token rotation + actionable startup errors for all secret sources When a Bitwarden machine-account token expired, users saw a raw Rust error dump (invalid_client + Location: + backtrace hints) and the only fix was manually editing .env or re-running the whole setup wizard. - New `hermes secrets bitwarden token` / `hermes secrets onepassword token`: paste a new token (masked prompt or flag), the command probes the backend BEFORE persisting — a rejected token changes nothing; a good one is written to .env and the fetch caches are cleared. - New optional SecretSource.remediation(kind, cfg) hook: startup warnings now print a '→ Run `hermes secrets <name> token`…' fix-it line after any fetch error, for bundled AND plugin sources (generic per-ErrorKind defaults in the ABC). - bws stderr is summarized to its cause line (Location:/backtrace noise dropped) and invalid_client/invalid_grant/400 identity rejects are now classified AUTH_FAILED (was INTERNAL) with a plain-English explanation naming the token env var. - op whoami probe accepts a candidate token so rotation validates the NEW credential, not the ambient one. Additive hook with defaults — no SECRET_SOURCE_API_VERSION bump. * docs: fix MDX parse error in secret-source-plugin hook table Escaped backticks around a <name> placeholder made MDX parse it as an unclosed JSX tag, breaking the docs-site build. Use a plain code span instead. * feat(desktop): configure repository discovery (supersedes #67630) (#68642) * feat(desktop): configure repository discovery * fix(config): preserve additive default migration * fix(desktop): stabilize session-actions-menu gateway mock for repo-scan subscribe projects.ts now runs $gateway.subscribe(syncReposScanning) at module load, and nanostores fires the subscriber synchronously. session-actions-menu.test.ts reaches projects.ts transitively via the session store but mocked @/store/gateway without $gateway, crashing the whole desktop vitest suite ("No \ export is defined"). Simply adding $gateway: atom(null) exposed a second issue: the synchronous subscriber calls the mock's activeGateway() during the transitive import, before the module-level const initializes (TDZ). Hoist the mock fns via vi.hoisted() so activeGateway is defined before the hoisted vi.mock factory runs, and add $gateway: atom(null) to the mock. Mirrors the self-contained mock pattern already used in projects.test.ts. Also maps the PR author's commit email for attribution. Supersedes #67630; incorporates review feedback from that PR. Co-authored-by: Rudimar Ronsoni <rudimar@outlook.com> --------- Co-authored-by: Rudimar Ronsoni <rudimar@outlook.com> Co-authored-by: Austin Pickett <austinpickett@users.noreply.github.com> * fix(desktop): ⌘W closes visible file tab when preview selection is stale (#68639) * fix(desktop): make ⌘W close visible file tab on stale preview selection When the live preview target is gone but $rightRailActiveTabId still points at preview, file tabs remain on screen while ⌘W fell through to a workspace no-op. Close the visible file tab instead. * test(desktop): cover ⌘W close for file tabs and ghost preview selection Lock the happy path and the stale-preview regression so ⌘W keeps closing the file tab the rail is actually showing. * fmt(js): `npm run fix` on merge (#68681) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * feat(billing): plan chips and rows deep-link their tier (#68666) * fix(desktop): drop the decorative top-up credits bar (#68649) The bar rendered full-or-empty (value 1|0) because top-ups have no denominator — the wire carries only the current balance and the pool is open-ended, so a fill fraction is fiction. Show the amount alone; subscription credits and the monthly cap keep their bars (real denominators). * fix(ci): route critical supply-chain findings through review gate (#68833) Let the scanner report critical findings without failing. The review-label gate owns the action-required status and blocking result, allowing the ci-reviewed label rerun to clear both CI and the PR comment. * fix: `tool_calls` double-encoding on import (#68856) * nix: add `cage` to devShell * test(desktop): add pre-filled sessions support Exports createSandbox, writeMockProviderConfig, writeEnvFile, buildAppEnv, findElectron, and launchDesktop from fixtures.ts so specs can compose their own seeded-backend fixtures without duplicating the sandbox/config/launch logic. * test(desktop): auto-fail e2e tests on error banner Adds a shared test fixture (e2e/test.ts) that wraps @playwright/test's page with an error-banner guard. When any [role="alert"] element (error notification toast) appears in the DOM during a test, the test fails with the error message text. The guard uses: - A MutationObserver (injected via addInitScript) that watches for [role="alert"] elements appearing at any point during the test - A final DOM scan in afterEach for alerts still visible at teardown - Deduplication so the same error text only fires once All existing e2e specs updated to import { test, expect } from './test' instead of '@playwright/test'. No per-spec setup needed — the guard is auto-installed on every page via the extended fixture. This catches issues like the "resume failed" error banner that can appear during session loading — previously the test would pass while an error toast was silently visible on screen. * fix(state): parse tool_calls JSON string before re-serializing _insert_message_rows and append_message both do json.dumps(tool_calls) to serialize the field for SQLite storage. But when tool_calls arrives as a JSON string (from import_sessions / export_session, which store it as TEXT), json.dumps double-encodes it — wrapping the already-serialized string in quotes and escaping the inner quotes. When _rows_to_conversation later does json.loads(row['tool_calls']), the double-encoded string parses back to a plain string (not a list). _history_to_messages then iterates this string character-by-character, calling tc.get('function', {}) on each char — 'str' object has no attribute 'get'. This was a pre-existing bug (on main), but only triggered by the import_sessions path (the live agent always passes tool_calls as a Python list). The e2e error-banner guard caught it via the 'Resume failed' notification toast. Fix: in both append_message and _insert_message_rows, parse tool_calls with json.loads first if it's a string, then re-serialize. * fix(desktop): exempt boot-failure from error guard - boot-failure: add allowErrorBanners() beforeEach — these tests deliberately trigger boot errors, so error toasts are expected - test.ts: export allowErrorBanners() opt-out + reset flag in afterEach * feat(status-bar): add /battery toggle for a color-coded battery read-out Add an opt-in battery indicator to the CLI and TUI status bars, shown as the first element and colour-coded by charge (green/yellow/orange/red, or green while charging). Off by default and a no-op on machines without a battery. - agent/battery.py: shared psutil-backed reader with a short TTL cache, category bucketing, and a compact 🔋/⚡ label. Fails open to "unavailable" everywhere. - CLI: /battery [on|off|status] toggle persisted to display.battery, rendered first in every status-bar width tier. - TUI: /battery slash command, config sync, a system.battery RPC polled while enabled, and a pinned first segment in StatusRule. * fix(approval): restore session approval for Tirith-flagged commands Adds an allow_session flag to the gateway approval payload so adapters can render the session tier independently of the permanent tier. Matrix gains a session reaction (🌀) and a reaction legend; pure-tirith prompts now offer once/session/deny instead of collapsing to once/deny. Salvaged from PR #67312, adapted to the allow_permanent semantics that landed in #68597 (Always offered when any dangerous-pattern warning is persistable; pure-tirith prompts stay session-max). * fix(approval): honor allow_session across all button adapters Widen the allow_session tier from Matrix to every adapter the gateway notifies: Telegram, Discord, Slack, Feishu, and Teams gate their Session button on it; WhatsApp Cloud and qqbot accept the kwarg (no session tier in their button sets). Also thread allow_session through the plugin- escalation gate, the execute_code guard payload, and the plain-text fallback so every notify path carries the same capability flags. * test(approval): cover allow_session tiers in Matrix reaction seeding and gateway payload Update the Matrix reaction-seeding contract to the four-reaction default (once/session/always/deny), add tirith-tier (session without always) and no-session-tier cases, and assert allow_session=True in the tirith gateway payload. * fix(desktop): wrap missing sidebar icon-button tooltips (#67500) * fix(desktop): wrap sidebar icon buttons in Tip tooltips Several icon-only buttons in the sidebar (header actions, workspace menu, project menu, session actions, load-more) had aria-label but no visual tooltip on hover. Wrap them in the existing <Tip> component, matching the pattern already used elsewhere (e.g. ProfilePill). No behavioral changes -- purely wraps existing buttons. Adds vitest coverage asserting the Tip wrapper (data-slot=tooltip-trigger) for 6 of 7 files; index.tsx is a 1500+ line top-level page component and was verified manually via screenshots instead. * fix(desktop): satisfy consistent-type-imports lint rule in project-dialog test * test(desktop): update session-row mocks for restored sessionColorById * fix(desktop): compose Tip around the real trigger instead of inside it Tip was being placed as SessionActionsMenu's/PlatformAvatar's DIRECT child, which asChild then cloned instead of the actual button/span. Neither Tip nor PlatformAvatar forwarded the injected onClick/ref, so both silently dropped the wiring: - session-actions-menu.tsx: Tip now wraps DropdownMenuTrigger internally (new ooltip prop) instead of the caller wrapping its children in Tip. - platform-icon.tsx: PlatformAvatar now forwards ref and spreads rest props onto its span so a wrapping Tip's trigger actually attaches. - session-row.tsx: updated call site to use the new tooltip prop. - Added session-actions-menu.test.tsx exercising the real DropdownMenu open behavior end-to-end (no Tip/Dropdown mocks). - session-row.test.tsx no longer mocks PlatformAvatar's behavior; it now exercises the real (fixed) component for the handoff-avatar tooltip. * fix(desktop): compose Tip outside PopoverAnchor in ProjectMenu (#67500) * test(desktop): update session-row test for the tooltip-prop composition (cbbbeb2fd) * fix(desktop): satisfy consistent-type-imports in session-row.test.tsx mocks * chore: retrigger CI * test(desktop): stop mocking PlatformAvatar's behavior (#67500, third pass) The mock was re-introduced by a prior edit that fixed an unrelated lint error, silently undoing the earlier fix where this test started exercising the real (forwardRef) PlatformAvatar. Removed the mock; updated the two handoff-avatar tests to query the real component's rendered span instead of text content, since it renders a brand SVG icon for known platforms rather than the platform name as text. * fmt(js): `npm run fix` on merge (#68867) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * fix(gateway): detect stale lock when macOS psutil returns valid start_time for recycled PID On macOS, the lock record's start_time is None (no /proc at creation), but psutil.Process(recycled_pid).create_time() returns a valid float for the unrelated process that now owns the PID. The old condition required both sides to be None before falling back to cmdline checking, so the recycled PID was never detected as stale. Change the fallback condition from AND to OR: when either side's start_time is missing, fall back to cmdline-based gateway detection. Fixes #53763 * fix(gateway): handle PermissionError on stale root-owned lock file When the macOS launchd service runs in a Background session, the gateway process spawns as root and creates a root-owned gateway.lock. On restart as the normal user, open() on that file raises PermissionError, crashing the gateway immediately and entering a launchd crash loop. Catch PermissionError in is_gateway_runtime_lock_active(), remove the stale lock file, and return False so the new process can start cleanly. Fixes #42685 * fix(gateway): guard acquire_gateway_runtime_lock against root-owned lock PermissionError Widen the PermissionError handling from is_gateway_runtime_lock_active (#42689) to the sibling open() in acquire_gateway_runtime_lock: a stale root-owned gateway.lock left by a launchd Background session previously crashed the acquiring process. Unlink the stale file and retry once; if the unlink or retry fails, return False cleanly instead of raising. * fix(gateway): make stale scoped-lock removal atomic via tombstone rename Replace the unlink()+O_EXCL sequence in acquire_scoped_lock with an atomic os.replace() of the stale lock to a <lock>.stale tombstone followed by the existing O_EXCL create. With plain unlink(), two racing starters could both judge the lock stale and the second unlink() would silently delete the first racer's freshly-created lock — both would then 'win'. os.replace() guarantees exactly one racer claims the stale file; the loser gets FileNotFoundError and falls through to O_EXCL, which admits at most one winner. Tombstones are cleaned up immediately; behavior is otherwise identical. * fix(gateway): detect stale gateway_state.json in `gateway status` (TTL + PID liveness) Verified: applies cleanly and the patched module compiles. Tests are described in the PR body (not bundled in this commit). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(gateway): cover stale gateway_state.json detection (TTL + PID liveness) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(gateway): take over live platform-lock token holders once When --replace misses a cross-HERMES_HOME Telegram token holder, platform connect used to retry forever. Terminate a verified gateway holder once (with the takeover marker) and re-acquire the scoped lock (#65176). Co-authored-by: Cursor <cursoragent@cursor.com> * chore(contributors): map jaretbottoms@gmail.com -> jbbottoms (PR #65178 salvage) * fix(gateway): reap the replaced gateway's orphaned children on POSIX Builds on jbbottoms's #65178 takeover fix (cherry-picked as the previous commit). Windows --replace already tree-kills via taskkill /T, but the POSIX paths signalled only the recorded gateway PID — adapter subprocesses that outlived their parent kept holding scoped token locks and blocked the replacement gateway. - gateway/status.py: _snapshot_gateway_children() captures the old gateway's descendants (psutil, recursive) while it is still alive; reap_gateway_children() SIGTERMs verified orphans after the main PID is confirmed dead, waits bounded, SIGKILLs survivors. Identity-aware (psutil is_running is PID+create-time), skips zombies and children whose ppid still equals the old gateway (parent actually alive), and never raises — best-effort with debug/info logging only. - take_over_scoped_lock_holder() snapshots before terminating and reaps only on a confirmed successful handoff. - gateway/run.py: start_gateway --replace snapshots before SIGTERM and reaps after the old PID is confirmed gone, mirroring taskkill /T. - tests/gateway/test_replace_child_reap.py: reap/skip/never-raise unit coverage plus end-to-end --replace ordering (snapshot → terminate → reap) and the no---replace path never touching the old process. * chore(contributors): map emails for PRs #66906, #66420, #63398 salvage * fix(state): probe FTS5 read path in _db_opens_cleanly so partial index corruption is detected (#66724) `hermes sessions repair --check-only` opens cleanly on state.db files with partial FTS5 index corruption — base tables read fine, the rolled-back write probe from #50502 succeeds, and `PRAGMA integrity_check` returns "ok". But every session_search / /resume title resolution / feature backed by MATCH / snippet / rank queries errors out with `database disk image is malformed` because internal shadow-table segments are bad. The official repair tool then gives false confidence. Add a representative FTS5 read probe against both `messages_fts` and `messages_fts_trigram` (the latter backs title resolution). Empty MATCH strings are accepted by every FTS5 index without requiring populated content, so the probe is safe on a freshly-init'd DB; missing-table / missing-column errors fall through to the existing "not yet a populated DB" branch, matching the write-probe's behaviour. Any other OperationalError is surfaced as the check reason, which sends `hermes sessions repair` to its existing FTS 'rebuild' path (repair_state_db_schema, line 616). Single-file change in hermes_state.py::_db_opens_cleanly. No public API change. No new imports. Fixes #66724. * fix(state): also catch sqlite3.DatabaseError in FTS5 read probe (#66724) The FTS5 read probe in _db_opens_cleanly() only caught sqlite3.OperationalError. But the corruption class #66724 actually wants caught — partial shadow-table damage where MATCH / snippet / rank queries raise DatabaseError("database disk image is malformed") — is a DatabaseError, not OperationalError. Without this catch the probe crashes the caller instead of returning a reason, which is exactly the silent-fail mode the issue describes. Move the try/except inside the for-loop so each FTS table is probed independently (one table corrupted should still surface as a reason), add a separate except clause for DatabaseError that surfaces the same reason format, and use continue instead of pass so the loop still walks both tables when only one is missing on a brand-new DB. Tested by hand: with a corrupted messages_fts_trigram shadow table the function now returns 'fts5 read probe failed on messages_fts_trigram: database disk image is malformed' instead of crashing out. Without this fix it would still crash. * fix(state): preserve degraded-runtime read probe + use canonical FTS5 classifier Two follow-ups on top of f842733 (the FTS5 read probe added in #66906): 1. The original probe query used MATCH '', which FTS5 rejects with 'fts5: syntax error near '. Empty MATCH syntax is not valid FTS5. Switch to MATCH '""' — a quoted empty phrase that parses, scans zero rows, and exercises the same shadow-table read path the search tools use. The probe previously never reached the shadow segments at all on a healthy DB; the read-corruption class was only being detected because the existing write probe happens to fail first on a DatabaseError. 2. The probe's degraded-runtime branch only checked the substrings 'no such table' / 'no such column'. On a SQLite build without the fts5 module, MATCH against a legacy messages_fts table raises 'no such module: fts5' (a different OperationalError class). The substring check would misclassify that as corruption and trigger repair, whose final fallback deletes the messages_fts% schema (#66906 review). Use SessionDB._is_fts5_unavailable_error() — the canonical classifier already used by the degraded-runtime init path — to recognize both 'no such module: fts5' and 'no such tokenizer: trigram' as capability errors. Add tests covering: - Partial shadow-table damage (read-corruption class) - Repair brings reads back online - Healthy degraded DB without fts5 module stays healthy (regression for the misclassification risk) - Healthy degraded DB without trigram tokenizer stays healthy Closes #66906 review feedback Refs #66724 * fix(state): self-heal FTS corruption on the SessionDB search path too Complements #66296 (self-heal on the write path): search_messages()'s main FTS5 MATCH query caught only sqlite3.OperationalError (a query-syntax error → return empty). A corrupt FTS index raises the malformed / "fts5: corrupt structure record" class, which is a sqlite3.DatabaseError — the parent of OperationalError, so it was NOT caught and propagated straight out of search_messages, crashing session/history search. The write path now rebuilds and retries on that class, but a read-only session (cron/CLI history search, or a search issued before any write) never triggers a write, so its search stayed broken until the next process restart ran the offline repair. Catch the DatabaseError corruption class on the search MATCH read too and route it through the existing one-shot _try_runtime_fts_rebuild(), then retry the query. The catch is moved outside `with self._lock` so rebuild_fts() can re-acquire the lock (mirrors _execute_write). The one-shot guard is shared with the write path, so a single instance never loops on a genuinely unrecoverable index. OperationalError syntax handling is unchanged (caught first). Adds a regression test: with a corrupted messages_fts and no post-corruption write, search_messages() rebuilds in place and returns the match; without the fix it raises DatabaseError. * fix(state): extend search-path FTS self-heal to the CJK/trigram branch The trigram MATCH branch in search_messages() had the same OperationalError-only catch that #66420 fixed on the main FTS5 branch: a corrupt messages_fts_trigram shadow table raises the malformed / 'fts5: corrupt structure record' class (sqlite3.DatabaseError, parent of OperationalError), which propagated straight out of search_messages and crashed CJK session/history search for read-only sessions. Route that class through the shared one-shot _try_runtime_fts_rebuild() and retry the trigram query (catch moved outside self._lock so rebuild_fts() can re-acquire it, mirroring the main branch). If the rebuild is refused (guard consumed / FTS disabled / different error) or the retry fails, fall through to the existing LIKE substring fallback — which reads only the canonical messages table — instead of raising, so CJK search degrades gracefully rather than crashing. Adds two regression tests: trigram search self-heals in place after shadow-table corruption (answers from the rebuilt trigram index, not the LIKE fallback), and degrades to LIKE without raising when the one-shot rebuild was already consumed. Follow-up to #66420; refs #66296 #66724 * fix(state): add REINDEX strategy to repair stale B-tree indexes (#63386) When PRAGMA integrity_check reports 'wrong # of entries in index' for B-tree indexes (e.g. idx_sessions_handoff_state), the existing repair strategies (FTS rebuild, sqlite_master dedup, drop-FTS+VACUUM) don't address the mismatch. Add Strategy 0.5: run REINDEX to rewrite the index b-tree from canonical table rows before escalating to more destructive strategies. * test(state): exercise REINDEX repair against a REAL stale B-tree index Replace the mocked test for #63398's REINDEX strategy: the original monkeypatched _db_opens_cleanly to return the corruption string, so the REINDEX pass itself was never exercised against actual index corruption — the test would pass even if REINDEX didn't fix anything. New fixture _corrupt_btree_index() builds genuine on-disk staleness with a writable_schema hack: rewrite the index definition to a partial index (WHERE 0), REINDEX so the b-tree is rebuilt empty, then restore the full definition. integrity_check then reports the real 'wrong # of entries in index idx_messages_session' / 'row N missing from index' class from #63386 — no mocks anywhere. The rewritten test asserts end-to-end with real function calls: - the real _db_opens_cleanly detects the stale index, - repair_state_db_schema repairs it with strategy 'reindex_btree', - post-repair the detector and raw PRAGMA integrity_check both report healthy, and a query forced through the rebuilt index (INDEXED BY) sees every row. Adds a second test asserting the REINDEX strategy is non-destructive (all sessions/messages survive, readable via SessionDB). Follow-up to #63398; refs #63386 * fix(kanban): auto-repair index-only kanban.db corruption via REINDEX _guard_existing_db_is_healthy previously failed closed on ANY integrity_check failure, including the index-scoped class ('wrong # of entries in index <name>' / 'row N missing from index <name>') where the table b-trees are intact and REINDEX rebuilds the damaged indexes losslessly. Boards hit by that class were bricked until manual surgery even though SQLite can fix them in-place. Now, when integrity_check output consists ONLY of index-scoped errors (index name parsed generically from the message — no hardcoded list): 1. quarantine the corrupt bytes FIRST via the existing content- addressed _backup_corrupt_db, 2. under the caller-held cross-process init flock, REINDEX each named index (falling back to bare REINDEX if a parsed name doesn't resolve), 3. re-run integrity_check and proceed only if it comes back clean. Any non-index error class (page corruption, malformed image, freelist damage) — or a REINDEX whose re-check is still dirty — fails closed exactly as before: backup + KanbanDbCorruptError, no silent recreation. Transient OperationalError (locked/busy) still propagates raw with no quarantine. Tests build a real board DB and corrupt a live index via the writable_schema/partial-index REINDEX trick to produce the genuine 'wrong # of entries in index' shape, then assert auto-repair recovers with data intact, page corruption still raises, and a dirty re-check fails closed. * fix(kanban): cap corrupt-backup retention at 10 files per board DB Content-addressed quarantine backups dedupe identical corrupt bytes, but corruption that keeps mutating between failures (partial repairs, further damage across dispatcher retries, multi-profile fleets) mints a new sha-named backup every round — a user accumulated 124 .corrupt.*.bak files with no bound. After each NEW backup is created, prune oldest-by-mtime backups beyond _CORRUPT_BACKUP_RETENTION (module constant, default 10), including the copied -wal/-shm sidecars. The just-created backup is always exempt (copy2 preserves the source mtime, which can be older than existing backups). Pruning is best-effort and never masks the corruption error about to be raised; dedupe of identical corrupt bytes is unchanged. * feat(kanban): periodic WAL checkpoint (TRUNCATE) on the dispatcher tick Kanban connections set wal_autocheckpoint=100, but SQLite's passive autocheckpoint backs off whenever any reader holds an open snapshot — on a busy multi-process board the -wal file can grow without bound between gateway restarts. After each successful dispatch tick, while still holding the board's single-writer dispatch flock, run PRAGMA wal_checkpoint(TRUNCATE) best-effort at a coarse interval (>=5 min since this process last checkpointed that board; module-level per-path monotonic timestamp, so multi-board dispatchers checkpoint each board on its own clock). Success and busy/locked skips are both logged at DEBUG; a failing checkpoint can never fail the tick. * feat(kanban): add `hermes kanban repair` CLI verb Adds kanban_db.repair_db() — a structured, non-raising wrapper around the same narrow repair policy as the connect-time guard: probe with PRAGMA integrity_check under the board's cross-process init flock; quarantine the corrupt bytes FIRST via the content-addressed backup; REINDEX only when every integrity message is index-scoped; re-check; report ok / repaired / corrupt / missing. Locked/busy OperationalError still propagates raw (a locked healthy DB is not corruption and gets no quarantine), and a repair invalidates the per-process healthy-path cache so the next connect() re-probes. The CLI verb reports status human-readably (or --json), exits 0 for ok/repaired/missing and 1 when the DB is still corrupt (non-index corruption stays fail-closed with manual-recovery guidance). It dispatches BEFORE kanban_command's auto-init: init_db() raises KanbanDbCorruptError on a corrupt board, which previously would have made a repair verb unreachable on exactly the boards that need it. CLI tests drive the real argparse surface (build_parser + kanban_command) against real corrupted SQLite fixtures. * fix(packaging): graft web_dist in MANIFEST.in and add sdist regression test Wheels ship hermes_cli/web_dist via pyproject package-data, but the sdist did not: MANIFEST.in had no graft and .gitignore excludes web_dist, so source tarballs installed a dashboard-less package. Graft the directory and add an sdist regression test that builds the tarball and asserts index.html is inside. Salvaged from #29661; the PR's [web]-extra 404-message change was dropped per maintainer review (misleading guidance for source installs). * fix(dashboard): attempt one recovery build when --skip-build finds no dist --skip-build with a missing web_dist/index.html p…
…native extension) unicode61 indexes a CJK run as ONE token, so 2-char Korean terms (일본, 구글, 우리, ...) can never match it and the trigram tokenizer needs >=3 chars per term — any query containing a 1-2 char CJK token falls through to a LIKE full-table scan (measured 3-6.4s CPU per query on a 6.8GB production state.db; the #1 base cost behind a 12.4s session_search average on CJK workloads). This ships a ~250-line loadable FTS5 tokenizer (no deps) that wraps unicode61: maximal CJK runs inside its tokens are re-emitted as overlapping character bigrams (Lucene CJKAnalyzer semantics), everything else passes through unchanged. FTS5 phrase semantics turn consecutive sub-tokens into exact substring matching down to 2-char terms at index speed. Build: native/fts5_cjk/build.sh -> ~/.hermes/lib/libfts5_cjk.so (override: HERMES_FTS5_CJK_SO). Salvaged from PR NousResearch#65544; the schema integration lands separately on the v23 external-content layout.
…content layout Integration layer for the cjk_unicode61 tokenizer, rebuilt on the v23 schema (the contributed integration in PR NousResearch#65544 predated it): - messages_fts_cjk: external-content FTS5 over a tool-row-excluding view (same v23 storage discipline as the trigram index it supersedes — zero inline text copies). Serves EVERY CJK query shape the legacy routing split between trigram (>=3 chars/token) and LIKE full scans (1-2 char tokens). Lone 1-char CJK runs and role_filter=['tool'] queries keep their legacy routes. - Dedicated marker pair (fts_cjk_rebuild_high_water/progress) gates the id-scoped triggers, so a cjk-only backfill never gates the complete messages_fts/trigram triggers. - Transitions ride (the existing throttled/resumable chunk engine): fresh DBs are born with the index; legacy v22 DBs land on v23+cjk in one run; already-optimized v23 DBs gaining the tokenizer get a marker-gated backfill; live writes are indexed immediately in every case. - Tokenizer-loss self-heal: a process that can't load the extension drops the cjk triggers (writes keep working), leaves a stale breadcrumb, and the index is rebuilt from scratch on the next optimize run — triggers are never reinstalled over a gap (external-content 'delete' on an unindexed rowid is the FTS5 corruption hazard the marker gating exists to prevent). - Capability classification: 'no such tokenizer: cjk_unicode61' joins the degraded-runtime error class everywhere (read probe, write probe, repair) so tokenizer absence is never misclassified as corruption. - Config: sessions.cjk_fts (default on, inert without the .so) and sessions.search_slow_ms in config.yaml, bridged to env by CLI + gateway (startup + per-turn reload). build.sh falls back to vendored SQLite headers so no libsqlite3-dev is needed. Slow-query log path attribution updated: fts_cjk / fts5 / trigram / like_scan. Tests: 14 lifecycle tests (fresh/legacy/stale/backfill paths, tokenizer-loss round-trip) + 5 config-bridge tests + slow-log suite.
…ousResearch#54242) A pure-Latin query (no CJK characters) routes to the unicode61 `messages_fts` table, whose tokenizer does not insert a boundary between Latin letters and adjacent CJK characters. Content like "修改youer服务端" is indexed as a single token, so `search_messages("youer")` returned zero results even though the substring is present, and the Latin path had no fallback. Add a zero-result trigram fallback to the pure-Latin path: when the unicode61 search misses, retry against the existing `messages_fts_trigram` table, which matches substrings regardless of word boundaries. The fallback is gated on `_trigram_available` and on every token being >=3 chars (the trigram minimum), and only fires on a zero-result miss, so successful Latin searches keep their unicode61 ranking unchanged. The trigram query construction shared with the CJK path is extracted into a `_run_trigram_search()` helper; the CJK branch is refactored to use it with no behavior change. Adds regression tests in tests/test_hermes_state.py::TestCJKSearchFallback.
The zero-result fallback prefers messages_fts_cjk when built: exact ranked token match for Latin runs unicode61 fused onto CJK, including <3-char tokens the trigram leg can't recover.
Closes NousResearch#70146 When hermes sessions optimize-storage runs and the database grows (rather than shrinks), the old code always printed '(reclaimed X MB)' with a negative value. Fix by using a conditional label: - positive delta → 'reclaimed X MB' - negative delta → 'grew by X MB'
Builds on @ms-alan's label fix: the negative figure had a second cause that relabelling alone leaves in place. `hermes sessions optimize-storage` reported "reclaimed -3820.1 MB" on a database that had in fact shrunk 60% (25069 MB -> 9975 MB). Both figures came from os.path.getsize(). In WAL mode a VACUUM's rewrite lands in the -wal file, and the checkpoint that folds it back is REFUSED (SQLITE_BUSY) while any other connection holds a read-mark — e.g. the live gateway. So the main file still carried its pre-VACUUM size AND kept growing while the command ran, making the after-figure larger than the before-figure. The TRUNCATE checkpoint in close() lands after the caller has already measured and printed. - hermes_state.py: add SessionDB.logical_size_bytes() — page_count * page_size, the size the file settles at once the WAL is folded back. Correct immediately, readers or not; returns None when the connection is gone so callers fall back to stat(). - hermes_state.py: best-effort TRUNCATE checkpoint after the optimize VACUUM so the file settles promptly when nothing else holds the DB. Documented as insufficient alone (busy under a live gateway) so the stat() approach is not reintroduced. - hermes_cli/main.py: both `optimize-storage` and `optimize` report via logical_size_bytes(); fixing only the reported command would have left the same bug class next door. - hermes_cli/main.py: lift the contributor's inline label to a module-level _size_delta_label() shared by both sites, so the "grew by" wording is consistent and unit-testable without reading source. The two halves are complementary: page accounting removes the phantom negative, and "grew by" still covers a genuine increase when concurrent session writes outweigh what the rebuild freed. Verified: sabotaging logical_size_bytes() back to stat() fails the new test with a 22 MB overstatement, reproducing the report. The test asserts its own precondition (stat() must actually be lagging) so it cannot silently stop exercising the bug. Tests: 464 passed (test_hermes_state.py + the new label tests). Co-authored-by: chenbin <h-chenbin@voyah.com.cn>
…ogress-bounded continuation, tolerate mid-rebuild missing index Follow-up on top of NousResearch#65554 (@the3asic): - Lower FTS5 'usermerge' to its minimum of 2 (persisted in the config shadow table, applied once per SessionDB instance). Without this a positive-rank 'merge' skips any level holding fewer than 4 segments (SQLite FTS5 §6.8), so the fragmented-index case the cadence targets never converges. - Run up to _FTS_MERGE_COMMANDS_PER_PASS (4) bounded merge commands per index per cadence, stopping early on the documented no-progress signal (total_changes delta < 2). Each command is its own implicit transaction, so the write lock is released between commands. - Skip a missing messages_fts instead of raising: the chunked optimize-storage rebuild legitimately drops + backfills FTS tables while writers keep running; warning every 1000 writes for the whole backfill window would be noise, and optimize_fts() has always treated missing tables as skippable. - Replace traced-SQL shape assertions with behavioral tests: real fragmented-index convergence (automerge suppressed, 60 segments) and a 3-segment below-default-usermerge compaction test that fails against a bare positive-rank merge (sabotage-verified). Validation on a sqlite3.backup() copy of a real 10.7 GB production state.db (1.49M messages, 1.3 GB + 2.9 GB FTS shadow tables): worst per-command write-lock hold 41.8 ms (was 9.2 s / 18.1 s per index with 'optimize'), search results byte-identical, fts5 integrity-check and PRAGMA integrity_check clean, steady-state pass 0.0 ms.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
choi138
pushed a commit
that referenced
this pull request
Aug 3, 2026
…own (NousResearch#74136) Fix-up for the cherry-picked cooldown persistence: the PR's tests mocked the DB (SimpleNamespace(_db=MagicMock())), which cannot prove the cooldown survives a restart. Replace with the production shape — a real SessionDB on disk behind the real AsyncSessionDB facade — and add a restart regression: fail a hygiene compression on runner #1, tear it down, build a fresh GatewayRunner on the SAME database, and assert the cooldown is still honored (no compression agent instantiated). Also updates the timeout test to assert the DB-backed record_compression_failure_cooldown write instead of the removed in-memory dict. Sabotage-verified: reverting gateway/run.py to the in-memory dict makes the restart test fail.
choi138
pushed a commit
that referenced
this pull request
Aug 3, 2026
Users following abbreviated links guess /docs/quickstart and /docs/installation and hit raw GitHub-Pages 404s — the real pages live under /docs/getting-started/. Add client redirects for both. Consumer-onboarding audit finding #1, Aug 2026.
choi138
pushed a commit
that referenced
this pull request
Aug 3, 2026
The #1 patch failure class in production (state.db mining, 250k-window) is a re-send of an edit that already landed: 'old_string and new_string are identical' (299 occurrences) plus a share of hunk-not-found errors where the new text is already in the file. These errored, sending models into re-read/re-patch loops. New tools/fuzzy_match.is_already_applied(content, old, new) — a conservative check requiring (1) non-trivial new_string (>=8 chars), (2) EXACT presence of new_string, (3) old_string gone (unless identical). Wired into three sites: - patch_replace (replace mode): returns success + no_change: true + an explicit note instead of the identical-strings / no-match error. - V4A validation phase: an already-applied hunk validates as a no-op so multi-hunk patches no longer fail wholesale when one hunk landed in a prior call. - V4A apply phase: mirrors the same skip so the two phases agree. Genuine no-matches (new text absent) and half-applied renames (old text still present) keep their error behavior — covered by tests.
choi138
pushed a commit
that referenced
this pull request
Aug 3, 2026
process(action='wait') hitting its window returned status='timeout' with a terse note — models read it as an error and re-issued identical waits (process is the #1 exact-duplicate tool call in production: 511 dupes in a 400k-msg window; wait is 57% of all process actions). The timeout result now carries: - process_running: true — machine-readable 'this is a status, not a failure' - an explicit note: 'Wait window of Ns elapsed — the process is still running. This is not an error. Uptime: Ms.' plus the right next step: when notify_on_complete is set, 'you will be notified on exit — do more work instead of waiting again'; otherwise a pointer to notify_on_complete for next time. - the clamp note (requested > max) now composes with the status note instead of replacing it. Exited/interrupted results are unchanged.
choi138
pushed a commit
that referenced
this pull request
Aug 3, 2026
…e-review #1) revoke_commit_admission() used to invoke the holder-qualified lease release unconditionally — including while an admitted commit was still mutating SessionDB — letting a second compressor acquire the durable lock mid-commit and interleave with the first commit's writes. The admission_revoked flag store stays lock-free, but the lease-release decision now coordinates with the fence lock: - revoke acquires the fence lock non-blocking; on success no commit can be in flight (an admitted commit retains the lock until finish_commit) and the release runs immediately, still under the lock so a racing begin_commit cannot slip between the check and the release. - on failure the release is deferred: finish_commit() re-checks _admission_revoked and performs it AFTER the commit completes (prompt even if the worker thread is later parked), and the begin_commit refusal path does the same for a revoke that lost the race to a transient lock-setup/cancel boundary. All paths are idempotent with the worker's own outer cleanup (DB release is holder-qualified). Invariant encoded + tested: no second compressor can acquire the durable lock while an admitted commit is still mutating; after a post-revoke commit finishes the lease is released promptly. Both regressions (revoke-during-commit deferral, revoke-before-commit immediate release + refused begin_commit) are sabotage-verified.
choi138
added a commit
that referenced
this pull request
Aug 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
hermes sessions optimize-storageNousResearch/hermes-agent#65798.cjk_unicode61tokenizer andmessages_fts_cjkindex from upstream work related to perf(state): CJK-bigram FTS index — 2-char CJK terms at index speed (salvage of #65544) NousResearch/hermes-agent#69423.Validation
Migration rehearsal
fts_storage_version=1, and all three FTS integrity checks passed.