Skip to content

fix(state): log WAL checkpoint failures instead of silently swallowing - #44834

Open
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/wal-checkpoint-exception-logging
Open

fix(state): log WAL checkpoint failures instead of silently swallowing#44834
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/wal-checkpoint-exception-logging

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

Replaces the bare except Exception: pass in _try_wal_checkpoint() and related methods with proper exception logging and an integrity guard. The destructive PRAGMA wal_checkpoint(TRUNCATE) was silently swallowing all errors — when it fails mid-operation, the WAL is already truncated to zero bytes but data was not fully written back, causing state.db corruption that cascades to TUI session store, holographic memory provider, and SessionDB.

Related Issue

Fixes #44795

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • hermes_state.py (_try_wal_checkpoint): Replace except Exception: pass with except Exception as exc: logger.warning(...) plus PRAGMA quick_check(1) integrity guard that logs an error if the DB is corrupted after a failed checkpoint.
  • hermes_state.py (close): Replace except Exception: pass with except Exception as exc: logger.debug(...) for the final checkpoint.
  • hermes_state.py (vacuum): Replace except Exception: pass with except Exception as exc: logger.debug(...) for the pre-VACUUM checkpoint.
  • tests/test_wal_checkpoint_exception.py: New test file with 6 tests covering warning logging, integrity check on failure, error logging on integrity failure, silent success, close resilience, and vacuum resilience.

How to Test

  1. Run python -m pytest tests/test_wal_checkpoint_exception.py -v — all 6 tests should pass
  2. Verify the fix handles the scenario from [Bug]: _try_wal_checkpoint TRUNCATE silently swallows exceptions, corrupts state.db WAL to zero bytes #44795: when _try_wal_checkpoint encounters a checkpoint failure, it now logs a warning instead of silently continuing, and runs PRAGMA quick_check(1) to detect corruption
  3. Existing test_hermes_state.py tests remain unaffected — no behavioral change in the happy path

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Code Intelligence

The bare `except Exception: pass` in `_try_wal_checkpoint()` silently
discards errors from the destructive TRUNCATE checkpoint. If the checkpoint
fails mid-operation, the WAL is already truncated to zero bytes but data was
not fully written back to the main DB, causing state.db corruption.

Changes:
- `_try_wal_checkpoint()`: log warning on exception, run PRAGMA quick_check
  integrity guard after failure, log error if integrity check also fails
- `close()`: log debug on checkpoint failure (non-destructive context)
- `vacuum()`: log debug on pre-VACUUM checkpoint failure

The TRUNCATE checkpoint itself is unchanged — only the error handling
improves from silent to observable, enabling operators to detect and
respond to corruption before it cascades.

Fixes NousResearch#44795
@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists labels Jun 12, 2026

@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 making a real silent failure path observable. Current main still has the bare catch in hermes_state.py:1256-1267, so the logging change remains relevant.

Problems

  • The proposed guard at hermes_state.py:883 executes PRAGMA quick_check(1) but does not consume its returned row. Current integrity handling reads and validates PRAGMA results at hermes_state.py:532-535; therefore a returned corruption status is ignored by this implementation.
  • The new probe runs after _try_wal_checkpoint() leaves the self._lock scope. The checkpoint query itself is serialized at hermes_state.py:1256-1260, so the follow-up query should use the same mutex.

Suggested changes

  • Fetch and validate the quick-check result under self._lock, logging an error for a missing or non-ok value.
  • Add a test that returns a non-ok quick-check row; the current test only verifies invocation or an exception.
  • Consider keeping routine lock/busy checkpoint outcomes out of the warning-plus-probe path, consistent with hermes_state.py:1220-1231.

Automated hermes-sweeper review.

Comment thread hermes_state.py
except Exception as exc:
logger.warning("WAL checkpoint failed (non-fatal): %s", exc)
try:
self._conn.execute("PRAGMA quick_check(1)")

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.

quick_check(1) reports integrity failures as result rows, so this call must fetch and validate the returned value (and run under self._lock). Current main's probe does this at hermes_state.py:532-535; otherwise a readable corrupt database is treated as healthy.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform 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]: _try_wal_checkpoint TRUNCATE silently swallows exceptions, corrupts state.db WAL to zero bytes

3 participants