From 6049ef0fb73355502e129ab47b46e91afd377136 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Wed, 16 Sep 2026 10:07:08 -0700 Subject: [PATCH] fix(update): cover every flat-install .db.* artifact, cron stores too, one test module Fold the dot-suffixed database-artifact class into the existing flat-install ignore block and test module instead of a second fixture: - `/*.db.*` now replaces `/*.db.retired-wal-*/` (which it subsumes) and the same class rule is added for the cron SQLite stores (`/cron/*.db.*`), so a quarantine/repair/rebuild/maintenance/init/dispatch lock, the repair-attempts ledger or a malformed-backup copy next to ANY root or cron store survives `hermes update`'s `git stash push --include-untracked`. - The runtime-state list in test_update_flat_install_state_gitignore.py gains one representative per artifact producer (hermes_state_dbfile, hermes_state_repair, hermes_state_common, hermes_cli/kanban_db_*), and the inode/flock invariant from #111667 moves into that module, driving the real updater seam `_stash_local_changes_if_needed` rather than a bare `git stash`. Why: a swept lock pathname is re-created on a new inode and a second exclusive flock succeeds while the first holder is still live (#112974); naming one file would leave the sibling locks exposed. --- .gitignore | 11 ++- ...t_update_flat_install_db_artifact_locks.py | 92 ------------------- ...est_update_flat_install_state_gitignore.py | 39 ++++++++ 3 files changed, 46 insertions(+), 96 deletions(-) delete mode 100644 tests/hermes_cli/test_update_flat_install_db_artifact_locks.py diff --git a/.gitignore b/.gitignore index f384ebb119..63c2583b76 100644 --- a/.gitignore +++ b/.gitignore @@ -171,7 +171,11 @@ docs/superpowers/* # Flat-install runtime state (checkout root == $HERMES_HOME, e.g. installs made # with HERMES_INSTALL_DIR=$HERMES_HOME or by older installers): every root-level # SQLite store (state.db, kanban.db, response_store.db, ...) with its -# WAL/SHM/journal sidecars and retired-WAL capture dirs, the legacy transcripts, +# WAL/SHM/journal sidecars and every dot-suffixed `.db.*` runtime artifact +# (retired-WAL capture dirs, quarantine/repair/rebuild/maintenance/init/dispatch +# lock files, the repair-attempts ledger, malformed-backup copies — a swept lock +# path is re-created on a new inode and a second exclusive flock succeeds while +# the first holder is still live, #112974), the legacy transcripts, # the cron job store (jobs.json), its lock/heartbeat/output files and all three # cron SQLite stores (executions/deliveries/notepad, WAL-mode like the root # ones), gateway lock/pid/state files and per-launch markers, cache/spill @@ -184,11 +188,10 @@ docs/superpowers/* # gateway (#110648). Nested installs keep all of this under $HERMES_HOME outside # the checkout, where the `.hermes/` rule above already applies. /*.db -/*.db.* /*.db-wal /*.db-shm /*.db-journal -/*.db.retired-wal-*/ +/*.db.* /gateway/discord_message_recovery.db* /sessions/ /browser-profile/ @@ -196,7 +199,7 @@ docs/superpowers/* /cron/*.db-wal /cron/*.db-shm /cron/*.db-journal -/cron/*.db.retired-wal-*/ +/cron/*.db.* /cron/jobs.json /cron/.jobs.lock /cron/ticker_heartbeat diff --git a/tests/hermes_cli/test_update_flat_install_db_artifact_locks.py b/tests/hermes_cli/test_update_flat_install_db_artifact_locks.py deleted file mode 100644 index 84efe64e07..0000000000 --- a/tests/hermes_cli/test_update_flat_install_db_artifact_locks.py +++ /dev/null @@ -1,92 +0,0 @@ -"""Flat-install database-adjacent runtime artifacts must survive update autostash.""" - -import os -import shutil -import subprocess -from pathlib import Path - -import pytest - -REPO_ROOT = Path(__file__).resolve().parents[2] - - -def _git(repo: Path, *args: str) -> subprocess.CompletedProcess: - return subprocess.run( - ["git", "-C", str(repo), *args], - check=True, - capture_output=True, - text=True, - ) - - -@pytest.fixture -def flat_install_repo(tmp_path: Path) -> Path: - repo = tmp_path / "flat-install-checkout" - repo.mkdir() - subprocess.run(["git", "init", "-q", str(repo)], check=True) - shutil.copyfile(REPO_ROOT / ".gitignore", repo / ".gitignore") - (repo / "app.py").write_text("print('hermes')\n") - _git(repo, "add", ".gitignore", "app.py") - _git( - repo, - "-c", - "user.email=t@t", - "-c", - "user.name=t", - "commit", - "-qm", - "init", - ) - return repo - - -def test_database_adjacent_runtime_artifacts_are_ignored(flat_install_repo: Path): - artifacts = ( - "state.db.quarantine.lock", - "state.db.repair.lock", - "state.db.fts_rebuild.lock", - "state.db.auto-maintenance.lock", - "state.db.repair-attempts.json", - "state.db.malformed-backup-20260915_060000", - "state.db.pre-update-emergency-2026-09-15T06-00-00-000Z.bak", - "kanban.db.init.lock", - "kanban.db.dispatch.lock", - "kanban.db.corrupt.20260915.bak", - ) - for name in artifacts: - (flat_install_repo / name).write_bytes(b"runtime state") - - status = _git( - flat_install_repo, - "status", - "--porcelain", - "--untracked-files=all", - ) - assert status.stdout == "", status.stdout - - -@pytest.mark.skipif(os.name == "nt", reason="fcntl is POSIX-only") -def test_autostash_cannot_split_live_database_lock_inode(flat_install_repo: Path): - import fcntl - - lock_path = flat_install_repo / "state.db.quarantine.lock" - with lock_path.open("a+b") as first: - fcntl.flock(first.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB) - first_stat = os.fstat(first.fileno()) - first_identity = first_stat.st_dev, first_stat.st_ino - - (flat_install_repo / "app.py").write_text("print('changed')\n") - _git( - flat_install_repo, - "stash", - "push", - "--include-untracked", - "-m", - "hermes-update-autostash", - ) - - assert lock_path.exists() - assert (lock_path.stat().st_dev, lock_path.stat().st_ino) == first_identity - with lock_path.open("a+b") as second: - with pytest.raises(BlockingIOError): - fcntl.flock(second.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB) diff --git a/tests/hermes_cli/test_update_flat_install_state_gitignore.py b/tests/hermes_cli/test_update_flat_install_state_gitignore.py index 188626545d..ce915b9938 100644 --- a/tests/hermes_cli/test_update_flat_install_state_gitignore.py +++ b/tests/hermes_cli/test_update_flat_install_state_gitignore.py @@ -9,6 +9,7 @@ every transcript when the restore is declined or fails its health check. The tracked .gitignore must cover the runtime state set, mirroring the .hermes-bootstrap-complete / .install_method precedent (#38529 / #66189). """ +import os import shutil import sqlite3 import subprocess @@ -28,7 +29,18 @@ FLAT_INSTALL_RUNTIME_STATE = ( "state.db-shm", "state.db-journal", "state.db.retired-wal-20260914T000000Z-1234/manifest.json", + # Dot-suffixed `.db.*` runtime artifacts (#112974): cross-process lock + # files from hermes_state_dbfile / hermes_state_repair / hermes_state_common / + # hermes_cli/kanban_db_*, the repair-attempts ledger and malformed backups. + "state.db.quarantine.lock", + "state.db.repair.lock", + "state.db.fts_rebuild.lock", + "state.db.auto-maintenance.lock", + "state.db.repair-attempts.json", + "state.db.malformed-backup-20260914_060000", "kanban.db", + "kanban.db.init.lock", + "kanban.db.dispatch.lock", "response_store.db", "response_store.db-wal", "gateway/discord_message_recovery.db", @@ -174,3 +186,30 @@ def test_untracked_autostash_leaves_open_wal_database_readable(flat_install_repo assert reader.execute("SELECT status FROM executions").fetchall() == [("ok",)] finally: conn.close() + + +@pytest.mark.skipif(os.name == "nt", reason="fcntl is POSIX-only") +def test_untracked_autostash_cannot_split_live_database_lock_inode(flat_install_repo): + """The updater's real stash step must leave a held ``state.db.quarantine.lock`` + on its original inode (#112974). Unlinking a flocked path does not release the + lock, so a swept pathname would be re-created on a new inode and a second + exclusive lock would succeed while the first holder is still live.""" + import fcntl + + from hermes_cli.update_cmd_stash import _stash_local_changes_if_needed + + lock_path = flat_install_repo / "state.db.quarantine.lock" + with lock_path.open("a+b") as first: + fcntl.flock(first.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB) + first_stat = os.fstat(first.fileno()) + # A tracked local change makes the updater actually enter its stash step. + (flat_install_repo / "app.py").write_text("print('changed')\n") + + assert _stash_local_changes_if_needed(["git"], flat_install_repo) + + assert lock_path.exists() + after = lock_path.stat() + assert (after.st_dev, after.st_ino) == (first_stat.st_dev, first_stat.st_ino) + with lock_path.open("a+b") as second: + with pytest.raises(BlockingIOError): + fcntl.flock(second.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB)