fix(update): cover every flat-install <db>.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.
This commit is contained in:
11
.gitignore
vendored
11
.gitignore
vendored
@@ -171,7 +171,11 @@ docs/superpowers/*
|
|||||||
# Flat-install runtime state (checkout root == $HERMES_HOME, e.g. installs made
|
# 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
|
# with HERMES_INSTALL_DIR=$HERMES_HOME or by older installers): every root-level
|
||||||
# SQLite store (state.db, kanban.db, response_store.db, ...) with its
|
# 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>.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
|
# 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
|
# cron SQLite stores (executions/deliveries/notepad, WAL-mode like the root
|
||||||
# ones), gateway lock/pid/state files and per-launch markers, cache/spill
|
# 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
|
# gateway (#110648). Nested installs keep all of this under $HERMES_HOME outside
|
||||||
# the checkout, where the `.hermes/` rule above already applies.
|
# the checkout, where the `.hermes/` rule above already applies.
|
||||||
/*.db
|
/*.db
|
||||||
/*.db.*
|
|
||||||
/*.db-wal
|
/*.db-wal
|
||||||
/*.db-shm
|
/*.db-shm
|
||||||
/*.db-journal
|
/*.db-journal
|
||||||
/*.db.retired-wal-*/
|
/*.db.*
|
||||||
/gateway/discord_message_recovery.db*
|
/gateway/discord_message_recovery.db*
|
||||||
/sessions/
|
/sessions/
|
||||||
/browser-profile/
|
/browser-profile/
|
||||||
@@ -196,7 +199,7 @@ docs/superpowers/*
|
|||||||
/cron/*.db-wal
|
/cron/*.db-wal
|
||||||
/cron/*.db-shm
|
/cron/*.db-shm
|
||||||
/cron/*.db-journal
|
/cron/*.db-journal
|
||||||
/cron/*.db.retired-wal-*/
|
/cron/*.db.*
|
||||||
/cron/jobs.json
|
/cron/jobs.json
|
||||||
/cron/.jobs.lock
|
/cron/.jobs.lock
|
||||||
/cron/ticker_heartbeat
|
/cron/ticker_heartbeat
|
||||||
|
|||||||
@@ -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)
|
|
||||||
@@ -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
|
tracked .gitignore must cover the runtime state set, mirroring the
|
||||||
.hermes-bootstrap-complete / .install_method precedent (#38529 / #66189).
|
.hermes-bootstrap-complete / .install_method precedent (#38529 / #66189).
|
||||||
"""
|
"""
|
||||||
|
import os
|
||||||
import shutil
|
import shutil
|
||||||
import sqlite3
|
import sqlite3
|
||||||
import subprocess
|
import subprocess
|
||||||
@@ -28,7 +29,18 @@ FLAT_INSTALL_RUNTIME_STATE = (
|
|||||||
"state.db-shm",
|
"state.db-shm",
|
||||||
"state.db-journal",
|
"state.db-journal",
|
||||||
"state.db.retired-wal-20260914T000000Z-1234/manifest.json",
|
"state.db.retired-wal-20260914T000000Z-1234/manifest.json",
|
||||||
|
# Dot-suffixed `<db>.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",
|
||||||
|
"kanban.db.init.lock",
|
||||||
|
"kanban.db.dispatch.lock",
|
||||||
"response_store.db",
|
"response_store.db",
|
||||||
"response_store.db-wal",
|
"response_store.db-wal",
|
||||||
"gateway/discord_message_recovery.db",
|
"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",)]
|
assert reader.execute("SELECT status FROM executions").fetchall() == [("ok",)]
|
||||||
finally:
|
finally:
|
||||||
conn.close()
|
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)
|
||||||
|
|||||||
Reference in New Issue
Block a user