Skip to content

fix(cli): add timeout to state.db health probe in hermes doctor (#72441) - #72527

Open
JonthanaHanh wants to merge 1 commit into
NousResearch:mainfrom
JonthanaHanh:fix/doctor-hang-state-db-72441
Open

fix(cli): add timeout to state.db health probe in hermes doctor (#72441)#72527
JonthanaHanh wants to merge 1 commit into
NousResearch:mainfrom
JonthanaHanh:fix/doctor-hang-state-db-72441

Conversation

@JonthanaHanh

Copy link
Copy Markdown
Contributor

Summary

Fixes #72441.

hermes doctor runs _db_opens_cleanly() which executes PRAGMA integrity_check on the state.db database. On profiles with 2000+ sessions, this can take minutes, making the doctor command appear to hang indefinitely.

Changes

  • hermes_cli/doctor.py: Wrap _db_opens_cleanly in a ThreadPoolExecutor with a 30-second timeout. On timeout, emit a warning with actionable suggestions.

Testing

  • py_compile: OK
  • ruff check: All checks passed
  • The timeout is generous (30s) to avoid false positives on slow but healthy databases.

…Research#72441)

hermes doctor runs _db_opens_cleanly() which executes PRAGMA
integrity_check on the state.db database.  On profiles with 2000+
sessions this can take minutes, making the doctor command appear to
hang indefinitely.

Fix: wrap _db_opens_cleanly in a ThreadPoolExecutor with a 30-second
timeout.  On timeout, emit a warning and suggest the user close other
Hermes instances or run 'hermes sessions repair' manually.

Fixes NousResearch#72441
@isak-ialogics

Copy link
Copy Markdown
Contributor

The 30s timeout currently does not bound hermes doctor: after _fut.result(timeout=30) raises, execution leaves the with ThreadPoolExecutor(...) block, whose implicit shutdown(wait=True) waits for _db_opens_cleanly() to finish before the warning can be observed. So the reported multi-minute hang is preserved (then a timeout warning appears afterward). A focused regression can use a blocking probe and assert elapsed time; the implementation needs a truly cancellable boundary (for example a subprocess that is terminated on timeout, or SQLite-level interruption/progress handling), since a running Python thread cannot be cancelled reliably.

@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard area/sessions Session lifecycle, resume, persistence, history P2 Medium — degraded but workaround exists labels Jul 27, 2026
@rdxhemadri

Copy link
Copy Markdown

@alt-glitch @teknium1 merge this pr,

@rdxhemadri

Copy link
Copy Markdown

hey @OutThisLife if you can pls merge this pr
@izumi0uu @helix4u @teknium1 @knoal @OutThisLife @benbarclay @alt-glitch

@rdxhemadri

Copy link
Copy Markdown

@rdxhemadri

Copy link
Copy Markdown

some one pls merge this pr @kshitijk4poor @OutThisLife
@izumi0uu @helix4u @teknium1 @knoal @benbarclay @alt-glitch
sry for spamming

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for targeting the state.db health probe; the current main path still invokes it immediately after the session-count row (hermes_cli/doctor.py:1501-1510), and _db_opens_cleanly() runs PRAGMA integrity_check (hermes_state.py:1091-1113).

Problems

  • hermes_cli/doctor.py:1505 in bf49560cc0eb uses a with ThreadPoolExecutor(...) block. Exiting that block waits for the running worker, so a Future.result(timeout=30) exception does not bound the command. The repository documents this exact behavior at tools/skills_hub.py:4078-4085.
  • bf49560cc0eb changes only hermes_cli/doctor.py; it does not add a regression test showing that doctor returns while the probe remains blocked.

Suggested changes

  • Use a genuinely cancellable SQLite-level deadline, such as a progress handler that interrupts the integrity scan, rather than a thread timeout alone.
  • Add an elapsed-time regression test for a blocking/over-deadline probe.

Automated hermes-sweeper review.

Comment thread hermes_cli/doctor.py
# See: #72441
import concurrent.futures
_probe_timeout = 30.0 # seconds
with concurrent.futures.ThreadPoolExecutor(max_workers=1) as _ex:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This context manager calls shutdown(wait=True) on exit. After result(timeout=...) raises, it still waits for _db_opens_cleanly() to finish, so the command remains blocked and the timeout warning is not observable at 30 seconds. Use a genuinely cancellable SQLite boundary and add an elapsed-time regression.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users area/install-update Installer, updater, packaging, wheels, doctor labels Jul 30, 2026
@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

Two PRs address #72441 by bounding the state.db health probe that follows the session-count check. #72527 adds a thread-pool timeout that still waits for the worker on context-manager shutdown, while #76003 replaces that ineffective boundary with SQLite-level cancellation, an abandonable fallback, and elapsed-time and repair-safety regressions.

Related pull requests

Duplicates

#72527 and #76003 target the same #72441 failure path; #76003 supersedes #72527 with a boundary that actually returns on timeout and with regression coverage.

Suggested consolidation

Keep #76003 open with the concrete salvage path already present in its diff: retain the cancellable SQLite deadline, abandonable fallback, non-corruption timeout semantics, and focused regression tests while completing normal review. Close #72527 as a duplicate of #76003; despite the keep_open review on #72527, its context-managed executor still waits for the blocked probe, whereas #76003 implements that review's requested SQLite-level cancellation and elapsed-time regression.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I72441(["issue #72441 (open)"])
    subgraph Dup72527 ["PRs duplicating each other"]
        P72527["PR #72527 (open)"]
        P76003["PR #76003 (open)"]
    end
    P72527 -->|best fix| I72441
    class I72441 open
    class P72527 open
    class P76003 open
    class P72527 best
    class P76003 best
    class P72527 target
    click I72441 "https://github.com/NousResearch/hermes-agent/issues/72441"
    click P72527 "https://github.com/NousResearch/hermes-agent/pull/72527"
    click P76003 "https://github.com/NousResearch/hermes-agent/pull/76003"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 2 pull requests and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 23 kB of PR diffs, 19 kB of issue/PR text, 8 kB of discussion (13 comments), 4 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

@alt-glitch alt-glitch added needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have duplicate This issue or pull request already exists and removed area/install-update Installer, updater, packaging, wheels, doctor P2 Medium — degraded but workaround exists needs-decision Awaiting maintainer decision before any implementation duplicate This issue or pull request already exists labels Aug 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/cli CLI entry point, hermes_cli/, setup wizard duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: hermes doctor hangs indefinitely after checking state.db (v0.19.0)

6 participants