Skip to content

fix: bound pre-update SQLite backups - #72966

Closed
roycepersonalassistant wants to merge 1 commit into
NousResearch:mainfrom
roycepersonalassistant:fix/bounded-pre-update-db-backups
Closed

roycepersonalassistant wants to merge 1 commit into
NousResearch:mainfrom
roycepersonalassistant:fix/bounded-pre-update-db-backups

Conversation

@roycepersonalassistant

@roycepersonalassistant roycepersonalassistant commented Jul 27, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • bound sqlite3.Connection.backup() when it makes no forward progress, preventing SQLITE_BUSY / SQLITE_LOCKED and non-advancing SQLITE_OK callbacks from hanging update backups indefinitely
  • prune only regenerable Chrome cache subtrees from both default and named-profile chrome-debug data while preserving durable browser state such as cookies and logins
  • keep durable SQLite state backups fail-closed: snapshot failure still aborts and removes an incomplete full/pre-update archive

Root cause

CPython's SQLite backup loop retries a busy source internally. The source connection's default busy handler can delay the first progress callback, and without an exception from that callback the loop can retry forever. A Chrome GPU/cache database held by a live helper process therefore stalled the pre-update full backup.

This change uses a short connection busy timeout plus a five-second no-progress deadline in the backup progress callback. The deadline resets only when the remaining page count decreases, so healthy large databases are not subject to a total wall-clock limit while busy, locked, or non-advancing sources remain bounded.

Current-main refresh

  • rebased the existing PR branch onto current upstream main at 66ea4e686de8f0147058fd97860f9cba154d821a
  • relocated the issue-specific regressions from the pruned monolithic backup test into tests/hermes_cli/test_backup_stability.py
  • preserved the current atomic full-backup publisher and its fail-closed durable-database behavior
  • independent review caught and the refresh fixed a repeated non-advancing SQLITE_OK path that the original implementation did not bound

Test plan

  • /Users/royce/.local/bin/uv run --extra dev pytest -o 'addopts=' tests/hermes_cli/test_backup.py tests/hermes_cli/test_backup_stability.py -q — 50 passed
  • /Users/royce/.local/bin/uv run --extra dev ruff check hermes_cli/backup.py tests/hermes_cli/test_backup_stability.py — passed
  • /Users/royce/.local/bin/uv run python -m compileall -q hermes_cli/backup.py tests/hermes_cli/test_backup_stability.py — passed
  • git diff --check origin/main...HEAD — passed
  • exact tested head: ba437934a0c3044da08b4d10191a6ff903df5f5b

Focused regressions cover a locked SQLite source, repeated non-advancing SQLITE_OK callbacks, locked Chrome cache data in a named profile, preservation of persistent browser state, and the existing fail-closed durable DB archive behavior.

Review

  • independent pre-commit reviewer: approved after the SQLITE_OK no-progress remediation
  • no schema, dependency, config-format, auth, deployment, or runtime migration is involved

Risk / rollback

Low and isolated to backup traversal/snapshot behavior. Revert the refreshed commit to restore prior behavior. No live databases, backups, gateways, or runtime configuration were touched while preparing this refresh.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard tool/browser Browser automation (CDP, Playwright) area/install-update Installer, updater, packaging, wheels, doctor labels Jul 27, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the focused backup hardening. The premise remains live on current main: hermes_cli/backup.py:256-268 calls conn.backup(backup_conn) with no bounded progress callback, and _write_full_zip_backup() reaches that helper for every .db at hermes_cli/backup.py:1521-1537.

The Chrome profile targeting also matches the managed browser path: chrome_debug_data_dir() resolves to $HERMES_HOME/chrome-debug in hermes_cli/browser_connect.py:132-140. The PR preserves the fail-closed full-zip behavior while avoiding the regenerable cache trees.

Current-main production code has not changed since the PR base; only tests/hermes_cli/test_backup.py was heavily pruned and reorganized, so salvaging should require relocating the new regression tests rather than reworking the implementation.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 30, 2026
@roycepersonalassistant

Copy link
Copy Markdown
Contributor Author

Superseded by #88853. The replacement is attached to the rebased and locally verified tree at 24440be, reconciles the contribution with current main’s independently landed busy/locked timeout, and preserves the remaining non-advancing-progress and Chrome-cache protections. Closing this stale record because GitHub no longer associated it with updates to its original fork branch.

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

Labels

area/install-update Installer, updater, packaging, wheels, doctor comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state tool/browser Browser automation (CDP, Playwright) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants