fix(agent-team): init_db drives migrate() so the version stamp actually advances
init_db's own schema_meta write was ON CONFLICT DO NOTHING, and the daemon (Coordinator.setup) calls init_db, never migrate() — so on an existing ledger the column was ensured but schema_version was never advanced (observed live: kind column present, schema_meta stuck at 3). migrate() already upserts the version correctly but was effectively dead code (no production caller). init_db now ends by calling migrate(conn), which steps the version and runs any version-gated steps. Idempotent — re-running the create/ensure statements is harmless. Regression test: an existing v3-stamped DB run through init_db now reports schema_version == SCHEMA_VERSION (4) and has the kind column. 1491 passed.
This commit is contained in:
parent
71edeb3f3f
commit
3ebeacdf31
2 changed files with 41 additions and 9 deletions
|
|
@ -274,15 +274,14 @@ def init_db(db_path: Path) -> None:
|
||||||
conn.execute(TASK_TRANSITIONS_DDL)
|
conn.execute(TASK_TRANSITIONS_DDL)
|
||||||
for stmt in _split_statements(TASK_TRANSITIONS_INDEXES_DDL):
|
for stmt in _split_statements(TASK_TRANSITIONS_INDEXES_DDL):
|
||||||
conn.execute(stmt)
|
conn.execute(stmt)
|
||||||
# Record the schema version (single-row table). DO NOTHING leaves an
|
# Step the version stamp forward AND apply any version-gated migrations.
|
||||||
# existing row's version untouched (an already-stamped DB just gains any
|
# init_db is the only schema entry point the daemon calls (Coordinator.
|
||||||
# IF-NOT-EXISTS tables above); migrate() is what steps the version stamp
|
# setup -> init_db), so it MUST drive migrate() — otherwise an existing
|
||||||
# forward on an existing DB.
|
# DB's schema_version is never advanced (migrate() upserts it; init_db's
|
||||||
conn.execute(
|
# own writes do not) and version-gated steps in migrate() never run in
|
||||||
"INSERT INTO schema_meta (id, schema_version) VALUES (1, ?) "
|
# production. migrate() is idempotent, so re-running the create/ensure
|
||||||
"ON CONFLICT(id) DO NOTHING",
|
# statements above is harmless.
|
||||||
(SCHEMA_VERSION,),
|
migrate(conn)
|
||||||
)
|
|
||||||
finally:
|
finally:
|
||||||
conn.close()
|
conn.close()
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -696,6 +696,39 @@ def test_migrate_helper_adds_kind_to_legacy_db(tmp_path: Path) -> None:
|
||||||
assert ver == SCHEMA_VERSION
|
assert ver == SCHEMA_VERSION
|
||||||
|
|
||||||
|
|
||||||
|
def test_init_db_advances_existing_version_stamp(tmp_path: Path) -> None:
|
||||||
|
"""init_db (the daemon's only schema entry point) bumps a stale version stamp.
|
||||||
|
|
||||||
|
Regression: init_db's own schema_meta write was ON CONFLICT DO NOTHING, so an
|
||||||
|
already-stamped DB (e.g. an old v3 ledger) kept its stale version forever —
|
||||||
|
the daemon calls init_db, never migrate(), so the stamp never advanced even
|
||||||
|
though the column was ensured. init_db now drives migrate(), which upserts.
|
||||||
|
"""
|
||||||
|
db = tmp_path / "stale.sqlite"
|
||||||
|
conn = connect(db)
|
||||||
|
try:
|
||||||
|
conn.execute(_LEGACY_PENDING_QUESTIONS_DDL)
|
||||||
|
conn.execute(
|
||||||
|
"CREATE TABLE IF NOT EXISTS schema_meta "
|
||||||
|
"(id INTEGER PRIMARY KEY CHECK (id = 1), schema_version INTEGER NOT NULL)"
|
||||||
|
)
|
||||||
|
conn.execute("INSERT INTO schema_meta (id, schema_version) VALUES (1, 3)")
|
||||||
|
finally:
|
||||||
|
conn.close()
|
||||||
|
|
||||||
|
init_db(db)
|
||||||
|
|
||||||
|
conn = connect(db)
|
||||||
|
try:
|
||||||
|
ver = conn.execute(
|
||||||
|
"SELECT schema_version FROM schema_meta WHERE id = 1"
|
||||||
|
).fetchone()[0]
|
||||||
|
assert "kind" in _pq_columns(conn)
|
||||||
|
finally:
|
||||||
|
conn.close()
|
||||||
|
assert ver == SCHEMA_VERSION
|
||||||
|
|
||||||
|
|
||||||
def test_kind_plan_decision_round_trips(tmp_path: Path) -> None:
|
def test_kind_plan_decision_round_trips(tmp_path: Path) -> None:
|
||||||
"""A row written with kind='plan_decision' round-trips; default is 'clarify'."""
|
"""A row written with kind='plan_decision' round-trips; default is 'clarify'."""
|
||||||
db = tmp_path / "db.sqlite"
|
db = tmp_path / "db.sqlite"
|
||||||
|
|
|
||||||
Reference in a new issue