refactor(config): config/state mutators flush, callers commit (E3 S3, B-core-10)

Restores CLAUDE.md's canonical transaction rule ("the handler commits; helpers
flush; only quota owns its session") on the config path: sysconfig.set_value /
sysconfig.reset and state.set_sync_paused now flush instead of committing, so a
change lands in ONE transaction with the audit row the route records for it —
previously the helper's internal commit split them, and a crash between left the
mutation persisted with no audit trail.

All callers already commit (the config PATCH/DELETE + audit, the sync pause/resume
+ audit, the scheduler maintenance PATCH); the setup wizard, which relied on the
internal commit, gets an explicit db.commit() after its loop. apply_interval is
deliberately left committing — it performs a live APScheduler reschedule that must
follow the DB persist, so it owns its own unit of work (a legitimate exception,
like the download service transitions). New test locks the flush-not-commit
contract (a rollback before the caller commits discards the change). 368 pass.
This commit is contained in:
2026-08-18 23:46:48 +02:00
parent 8a97c79d37
commit 3f047bbad1
4 changed files with 49 additions and 5 deletions
+1
View File
@@ -96,6 +96,7 @@ def setup_config(settings_map: dict[str, Any], db: Session = Depends(require_set
except (ValueError, RuntimeError) as exc:
raise HTTPException(status_code=400, detail=str(exc))
saved.append(key)
db.commit() # sysconfig.set_value only flushes now — persist the wizard's config in one commit
return {"saved": saved}
+3 -1
View File
@@ -28,10 +28,12 @@ def is_sync_paused(db: Session) -> bool:
def set_sync_paused(db: Session, paused: bool) -> None:
# Flushes, doesn't commit — the caller owns the transaction so the change lands with its audit
# row in one transaction (B-core-10).
row = _row(db)
row.sync_paused = paused
db.add(row)
db.commit()
db.flush()
log.info("Background sync %s", "paused" if paused else "resumed")
+7 -4
View File
@@ -186,7 +186,9 @@ def get_bool(db: Session, key: str) -> bool:
def set_value(db: Session, key: str, value) -> None:
"""Validate, coerce, and persist a DB override for `key`. Secrets are encrypted."""
"""Validate, coerce, and stage a DB override for `key` (secrets are encrypted). Flushes but does
NOT commit — the caller owns the transaction, so the change lands together with its audit row
(B-core-10). The staged value is visible to a same-session `get()` before the commit."""
s = _BY_KEY[key]
if s.type == "int":
v = int(value)
@@ -212,15 +214,16 @@ def set_value(db: Session, key: str, value) -> None:
db.add(row)
row.value = stored
row.encrypted = encrypted
db.commit()
db.flush()
def reset(db: Session, key: str) -> None:
"""Remove the DB override for `key` (fall back to the env/config default)."""
"""Remove the DB override for `key` (fall back to the env/config default). Flushes but does NOT
commit — the caller owns the transaction (B-core-10)."""
row = db.get(SystemConfig, key)
if row is not None:
db.delete(row)
db.commit()
db.flush()
def describe(db: Session) -> dict:
@@ -0,0 +1,38 @@
"""E3 S3 (B-core-10): the config/state mutators flush but do NOT self-commit, so the caller owns the
transaction and a change lands together with its audit row. These lock that contract — a rollback
before the caller commits discards the change (proving no hidden internal commit), while a same-session
read still sees it (flush visibility). DB lane (`db` fixture) — skipped without Postgres.
"""
from app import state, sysconfig
def test_set_value_flushes_but_leaves_the_commit_to_the_caller(db):
default = sysconfig.get_int(db, "quota_daily_budget")
new = default + 4321
sysconfig.set_value(db, "quota_daily_budget", new)
assert sysconfig.get_int(db, "quota_daily_budget") == new # visible in-session (flushed)
db.rollback() # the caller never committed
assert sysconfig.get_int(db, "quota_daily_budget") == default # discarded — no hidden self-commit
def test_reset_flushes_but_leaves_the_commit_to_the_caller(db):
default = sysconfig.get_int(db, "quota_daily_budget")
sysconfig.set_value(db, "quota_daily_budget", default + 10)
db.commit() # a persisted override to reset
sysconfig.reset(db, "quota_daily_budget")
assert sysconfig.get_int(db, "quota_daily_budget") == default # reset visible in-session
db.rollback()
assert sysconfig.get_int(db, "quota_daily_budget") == default + 10 # reset discarded until committed
def test_set_sync_paused_flushes_but_leaves_the_commit_to_the_caller(db):
assert state.is_sync_paused(db) is False
state.set_sync_paused(db, True)
assert state.is_sync_paused(db) is True # visible in-session
db.rollback()
assert state.is_sync_paused(db) is False # discarded — the caller owns the commit