Skip to content

Commit 32eb272

Browse files
fix: auto-create sessions DB on first use + atomic state file writes
Two fixes for CI workflow failures: 1. history/_connection.py: _connect() now creates sessions.db and runs migrations when the file doesn't exist, instead of returning None. This fixes all 60+ UAT test timeouts on fresh CI runners where no DB existed. Follows the same self-healing pattern as review_db.py. 2. session_state.py: write_session_state() now uses a threading.Lock around the read-merge-write cycle and writes via temp file + os.replace() for atomic file replacement. Fixes the sidebar test_p_toggles_pause race condition where the poll thread and pause action could produce concatenated JSON objects. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 0dfeb7d commit 32eb272

5 files changed

Lines changed: 67 additions & 25 deletions

File tree

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,53 @@
1-
"""Shared database connection helpers for the history package."""
1+
"""Shared database connection helpers for the history package.
2+
3+
Auto-creates the sessions DB and applies migrations on first use,
4+
so ``studyctl study`` works on a fresh machine without ``studyctl doctor``
5+
or any other bootstrap step.
6+
"""
27

38
from __future__ import annotations
49

10+
import logging
511
import sqlite3
6-
from typing import TYPE_CHECKING
712

813
from ..settings import load_settings
914

10-
if TYPE_CHECKING:
11-
from pathlib import Path
15+
logger = logging.getLogger(__name__)
1216

1317

14-
def _find_db() -> Path | None:
15-
db = load_settings().session_db
16-
return db if db.exists() else None
18+
def _get_db_path():
19+
"""Return the configured sessions DB path (always a Path, never None)."""
20+
return load_settings().session_db
1721

1822

1923
def _connect() -> sqlite3.Connection | None:
20-
db = _find_db()
21-
if not db:
22-
return None
24+
"""Open a connection to sessions.db, creating it if necessary.
25+
26+
On first use the file and all tables are created via the
27+
agent-session-tools migration chain. Returns ``None`` only if
28+
the migration import is unavailable (agent-session-tools not
29+
installed).
30+
"""
31+
db = _get_db_path()
32+
db.parent.mkdir(parents=True, exist_ok=True)
33+
34+
is_new = not db.exists()
2335
conn = sqlite3.connect(db, timeout=5)
2436
conn.row_factory = sqlite3.Row
37+
conn.execute("PRAGMA journal_mode=WAL")
38+
conn.execute("PRAGMA busy_timeout=5000")
39+
40+
if is_new:
41+
try:
42+
from agent_session_tools.migrations import migrate
43+
44+
migrate(conn)
45+
logger.info("Created sessions DB at %s", db)
46+
except ImportError:
47+
logger.debug("agent-session-tools not installed — skipping migrations")
48+
except Exception:
49+
logger.exception("Failed to initialise sessions DB")
50+
conn.close()
51+
return None
52+
2553
return conn

packages/studyctl/src/studyctl/session_state.py

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
import json
1010
import os
11+
import threading
1112
from dataclasses import dataclass
1213
from pathlib import Path
1314

@@ -51,18 +52,33 @@ def _ensure_session_dir() -> None:
5152

5253

5354
def _write_file_secure(path: Path, content: str) -> None:
54-
"""Write content to a file with 0600 permissions (owner-only read/write)."""
55-
fd = os.open(str(path), os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600)
55+
"""Write content atomically with 0600 permissions.
56+
57+
Writes to a temp file then replaces the target, preventing partial
58+
reads and the truncation race where two concurrent O_TRUNC opens
59+
leave trailing bytes from the longer write.
60+
"""
61+
tmp = str(path) + ".tmp"
62+
fd = os.open(tmp, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600)
5663
with os.fdopen(fd, "w") as f:
5764
f.write(content)
65+
os.replace(tmp, str(path))
66+
67+
68+
_state_lock = threading.Lock()
5869

5970

6071
def write_session_state(updates: dict) -> None:
61-
"""Atomic read-merge-write of session state. Creates file if missing."""
72+
"""Atomic read-merge-write of session state. Creates file if missing.
73+
74+
Thread-safe: a lock serialises concurrent updates from the poll
75+
thread and action handlers to prevent read-merge-write clobbering.
76+
"""
6277
_ensure_session_dir()
63-
current = read_session_state()
64-
current.update(updates)
65-
_write_file_secure(STATE_FILE, json.dumps(current, indent=2, default=str))
78+
with _state_lock:
79+
current = read_session_state()
80+
current.update(updates)
81+
_write_file_secure(STATE_FILE, json.dumps(current, indent=2, default=str))
6682

6783

6884
def parse_topics_file() -> list[TopicEntry]:

packages/studyctl/tests/test_cli.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
"""Tests for studyctl CLI commands using Click's CliRunner.
22
33
Strategy: Most commands depend on history functions that need a live sessions.db.
4-
We monkeypatch `studyctl.history._find_db` to return None, which makes `_connect()`
5-
return None, and each history function returns its empty/default sentinel.
6-
Commands are designed to handle this gracefully with user-friendly messages.
4+
We monkeypatch `_connect` to return None, and each history function returns its
5+
empty/default sentinel. Commands are designed to handle this gracefully with
6+
user-friendly messages.
77
88
Note on `review`: With no DB, `spaced_repetition_due` still returns entries for
99
configured topics (marked "New topic") because `last_studied()` returns None when
@@ -35,7 +35,7 @@ def _no_db(monkeypatch: pytest.MonkeyPatch) -> None:
3535
"""Ensure no real database is ever touched during CLI tests."""
3636
import studyctl.history._connection as _conn
3737

38-
monkeypatch.setattr(_conn, "_find_db", lambda: None)
38+
monkeypatch.setattr(_conn, "_connect", lambda: None)
3939

4040

4141
# ---------------------------------------------------------------------------

packages/studyctl/tests/test_cli_session.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,9 @@ def session_env(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path:
4949
conn.close()
5050

5151
# Patch DB path for history, parking, and settings modules.
52-
# history.py uses _find_db() → load_settings().session_db → checks .exists()
53-
# so we must patch _find_db directly to return the test DB path.
5452
monkeypatch.setattr("studyctl.settings.get_db_path", lambda: db_path)
5553
monkeypatch.setattr("studyctl.parking.get_db_path", lambda: db_path)
56-
monkeypatch.setattr("studyctl.history._connection._find_db", lambda: db_path)
54+
monkeypatch.setattr("studyctl.history._connection._get_db_path", lambda: db_path)
5755

5856
# Patch session state paths to use temp dir
5957
session_dir = tmp_path / "session"

packages/studyctl/tests/test_history.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ def test_no_db_candidates_attribute(self):
146146
"_DB_CANDIDATES should not exist — load_settings() must not be called at import time"
147147
)
148148

149-
def test_find_db_uses_settings(self, tmp_path, monkeypatch):
149+
def test_get_db_path_uses_settings(self, tmp_path, monkeypatch):
150150
db_path = tmp_path / "sessions.db"
151151
db_path.touch()
152152

@@ -158,7 +158,7 @@ class FakeSettings:
158158

159159
monkeypatch.setattr(_conn, "load_settings", lambda: FakeSettings())
160160

161-
result = _conn._find_db()
161+
result = _conn._get_db_path()
162162
assert result == db_path
163163

164164

0 commit comments

Comments
 (0)