fix(kanban): also hoist idx_events_run + drop redundant inner create

Extends the previous commit to cover the remaining additive-column index
that sits on the same migration trap:

- ``task_events.run_id`` -> ``idx_events_run`` was still in SCHEMA_SQL.
  A legacy ``task_events`` table predating #17805 (no ``run_id``) would
  still abort ``executescript`` before ``_migrate_add_optional_columns``
  could add the column. Hoisted out of SCHEMA_SQL and made unconditional
  in the migration alongside the other three indexes.

- Removed the now-redundant ``CREATE INDEX idx_tasks_idempotency`` that
  was nested inside the ``if "idempotency_key" not in cols`` branch.
  The unconditional create lower in the function makes it idempotent
  on both fresh and legacy DBs.

- Strengthened the regression test to cover all four indexes
  (``idx_tasks_session_id``, ``idx_tasks_tenant``, ``idx_tasks_idempotency``,
  ``idx_events_run``) and to seed a pre-#17805 ``task_events`` shape that
  exercises the ``run_id`` migration path.

The result: every ``CREATE INDEX`` that depends on an additive column now
runs after the migration ensures the column exists. Verified against a
realistic pre-#16081 board fixture (tasks + task_events both legacy
shape) — origin/main reproduces ``no such column: session_id``; this
branch migrates cleanly and creates all four indexes.
This commit is contained in:
kshitijk4poor 2026-05-19 20:01:59 +05:30 committed by kshitij
parent 7c622b6c74
commit 7552e0f3c0
2 changed files with 54 additions and 12 deletions

View file

@ -939,7 +939,6 @@ CREATE INDEX IF NOT EXISTS idx_links_child ON task_links(child_id);
CREATE INDEX IF NOT EXISTS idx_links_parent ON task_links(parent_id);
CREATE INDEX IF NOT EXISTS idx_comments_task ON task_comments(task_id, created_at);
CREATE INDEX IF NOT EXISTS idx_events_task ON task_events(task_id, created_at);
CREATE INDEX IF NOT EXISTS idx_events_run ON task_events(run_id, id);
CREATE INDEX IF NOT EXISTS idx_runs_task ON task_runs(task_id, started_at);
CREATE INDEX IF NOT EXISTS idx_runs_status ON task_runs(status);
CREATE INDEX IF NOT EXISTS idx_notify_task ON kanban_notify_subs(task_id);
@ -1079,10 +1078,9 @@ def _migrate_add_optional_columns(conn: sqlite3.Connection) -> None:
_add_column_if_missing(
conn, "tasks", "idempotency_key", "idempotency_key TEXT"
)
conn.execute(
"CREATE INDEX IF NOT EXISTS idx_tasks_idempotency "
"ON tasks(idempotency_key)"
)
# ``idx_tasks_idempotency`` is created unconditionally below alongside
# the other additive-column indexes — see the block after the
# legacy-column migration. Creating it here too would be redundant.
# Refresh after early additive migrations above. Some existing DBs were
# partially migrated in older releases and can already contain the later
@ -1170,9 +1168,13 @@ def _migrate_add_optional_columns(conn: sqlite3.Connection) -> None:
conn, "tasks", "session_id", "session_id TEXT"
)
# Indexes over additive task columns must be created after the columns
# exist. Keeping them in SCHEMA_SQL breaks legacy boards because
# CREATE TABLE IF NOT EXISTS does not add new columns to existing tables.
# Indexes over additive ``tasks`` columns must be created after the
# columns exist. Keeping them in SCHEMA_SQL breaks legacy boards: SQLite
# parses each statement in ``executescript`` against the live schema, so a
# ``CREATE INDEX`` over a missing column aborts initialization before the
# additive ``ALTER TABLE`` migrations below can run. Re-running them here
# is cheap thanks to ``IF NOT EXISTS`` and stays correct on fresh DBs
# (where the columns already exist from SCHEMA_SQL).
conn.execute("CREATE INDEX IF NOT EXISTS idx_tasks_tenant ON tasks(tenant)")
conn.execute(
"CREATE INDEX IF NOT EXISTS idx_tasks_idempotency ON tasks(idempotency_key)"
@ -1186,10 +1188,14 @@ def _migrate_add_optional_columns(conn: sqlite3.Connection) -> None:
ev_cols = {row["name"] for row in conn.execute("PRAGMA table_info(task_events)")}
if "run_id" not in ev_cols:
_add_column_if_missing(conn, "task_events", "run_id", "run_id INTEGER")
conn.execute(
"CREATE INDEX IF NOT EXISTS idx_events_run "
"ON task_events(run_id, id)"
)
# Same ordering rule as the additive ``tasks`` indexes above: create the
# index after the additive column migration so legacy ``task_events``
# tables don't fail during SCHEMA_SQL execution before ``run_id`` exists.
conn.execute(
"CREATE INDEX IF NOT EXISTS idx_events_run "
"ON task_events(run_id, id)"
)
notify_table_exists = conn.execute(
"SELECT name FROM sqlite_master WHERE type='table' AND name='kanban_notify_subs'"

View file

@ -49,8 +49,22 @@ def test_init_creates_expected_tables(kanban_home):
def test_connect_migrates_legacy_db_before_optional_column_indexes(tmp_path):
"""Legacy DBs missing additive indexed columns must migrate cleanly.
SCHEMA_SQL runs in ``connect()`` before ``_migrate_add_optional_columns``.
Indexes over additive columns therefore must be created after the
migration adds those columns, or boards predating the column fail to
open before migration can run.
Covers all four indexes that sit on additive columns:
- ``tasks.session_id`` -> ``idx_tasks_session_id`` (#28447)
- ``tasks.tenant`` -> ``idx_tasks_tenant`` (#16081)
- ``tasks.idempotency_key`` -> ``idx_tasks_idempotency`` (#17805)
- ``task_events.run_id`` -> ``idx_events_run`` (#17805)
"""
db_path = tmp_path / "legacy-kanban.db"
conn = sqlite3.connect(str(db_path))
# Pre-#16081 ``tasks`` shape: missing tenant, idempotency_key, session_id.
conn.execute("""
CREATE TABLE tasks (
id TEXT PRIMARY KEY,
@ -69,6 +83,18 @@ def test_connect_migrates_legacy_db_before_optional_column_indexes(tmp_path):
claim_expires INTEGER
)
""")
# Pre-#17805 ``task_events`` shape: missing run_id. Required because
# ``_migrate_add_optional_columns`` unconditionally runs PRAGMA on
# ``task_events`` for run_id back-fill.
conn.execute("""
CREATE TABLE task_events (
id INTEGER PRIMARY KEY AUTOINCREMENT,
task_id TEXT NOT NULL,
kind TEXT NOT NULL,
payload TEXT,
created_at INTEGER NOT NULL
)
""")
conn.execute(
"INSERT INTO tasks (id, title, status, created_at) "
"VALUES ('legacy', 'old board task', 'ready', 1)"
@ -80,6 +106,10 @@ def test_connect_migrates_legacy_db_before_optional_column_indexes(tmp_path):
task_columns = {
row["name"] for row in migrated.execute("PRAGMA table_info(tasks)")
}
event_columns = {
row["name"]
for row in migrated.execute("PRAGMA table_info(task_events)")
}
indexes = {
row["name"]
for row in migrated.execute(
@ -87,10 +117,16 @@ def test_connect_migrates_legacy_db_before_optional_column_indexes(tmp_path):
)
}
# Additive columns added by migration:
assert "session_id" in task_columns
assert "tenant" in task_columns
assert "idempotency_key" in task_columns
assert "run_id" in event_columns
# And their indexes — the regression scope of this test:
assert "idx_tasks_session_id" in indexes
assert "idx_tasks_tenant" in indexes
assert "idx_tasks_idempotency" in indexes
assert "idx_events_run" in indexes
# ---------------------------------------------------------------------------