From 0cb7a2fd60dd33b46dfcf433fa27569fc593ca43 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Wed, 16 Sep 2026 12:44:11 -0700 Subject: [PATCH] fix(kanban): heal every nullable v1 tasks column and index assignee after the ALTER pass Review of the foreign-schema heal found two gaps in what it claimed to cover: - `_BASE_TASK_COLUMNS` said it listed the nullable/defaulted v1 columns but omitted `priority`, `created_by` and `completed_at`. A harness that seeded `tasks(id,title,assignee,status,created_at,started_at)` connected fine and then failed `list_tasks` with "no such column: priority" (`SELECT *` feeds the Task row shape). Add the three entries with DDL copied from SCHEMA_SQL. - The `assignee` heal was unreachable: `connect()` runs `executescript(SCHEMA_SQL)` before `_migrate_add_optional_columns`, and SCHEMA_SQL's `CREATE INDEX idx_tasks_assignee_status ON tasks(assignee, status)` aborts init on a board without `assignee` before any ALTER runs. Move that index into the post-ALTER `CREATE INDEX IF NOT EXISTS` block next to the other additive-column indexes; fresh DBs end up with the identical index. The existing foreign-schema test now seeds the narrowest schema (`tasks(id,title,status,created_at)`), asserts all ten healed columns match the fresh DDL, and reads the board back through `list_tasks`. The "already fully migrated" fixture in `test_migrate_add_optional_columns_tolerates_concurrent_migration` gains the v1 `status` column the assignee/status index now needs (a real migrated board always has it). --- hermes_cli/kanban_db.py | 1 - hermes_cli/kanban_db_connect.py | 4 ++++ tests/hermes_cli/test_kanban_db.py | 22 +++++++++++++++------- 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/hermes_cli/kanban_db.py b/hermes_cli/kanban_db.py index 95e77e44de..6bb73d7c04 100644 --- a/hermes_cli/kanban_db.py +++ b/hermes_cli/kanban_db.py @@ -1057,7 +1057,6 @@ CREATE TABLE IF NOT EXISTS kanban_notify_subs ( PRIMARY KEY (task_id, platform, chat_id, thread_id) ); -CREATE INDEX IF NOT EXISTS idx_tasks_assignee_status ON tasks(assignee, status); CREATE INDEX IF NOT EXISTS idx_tasks_status ON tasks(status); 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); diff --git a/hermes_cli/kanban_db_connect.py b/hermes_cli/kanban_db_connect.py index 8654bb0280..0886ef4725 100644 --- a/hermes_cli/kanban_db_connect.py +++ b/hermes_cli/kanban_db_connect.py @@ -777,7 +777,10 @@ def init_db(db_path: Optional[Path] = None, *, board: Optional[str] = None) -> P _BASE_TASK_COLUMNS = ( ("body", "body TEXT"), ("assignee", "assignee TEXT"), + ("priority", "priority INTEGER DEFAULT 0"), + ("created_by", "created_by TEXT"), ("started_at", "started_at INTEGER"), + ("completed_at", "completed_at INTEGER"), ("workspace_kind", "workspace_kind TEXT NOT NULL DEFAULT 'scratch'"), ("workspace_path", "workspace_path TEXT"), ("claim_lock", "claim_lock TEXT"), @@ -895,6 +898,7 @@ def _migrate_add_optional_columns(conn: sqlite3.Connection) -> None: # so a ``CREATE INDEX`` over a missing column in SCHEMA_SQL would abort # init on legacy boards before the ALTER TABLE pass runs. ``IF NOT EXISTS`` # keeps re-running here cheap and correct on fresh DBs. + conn.execute("CREATE INDEX IF NOT EXISTS idx_tasks_assignee_status ON tasks(assignee, status)") 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)") conn.execute("CREATE INDEX IF NOT EXISTS idx_tasks_session_id ON tasks(session_id)") diff --git a/tests/hermes_cli/test_kanban_db.py b/tests/hermes_cli/test_kanban_db.py index be31d8c0ed..78064e878c 100644 --- a/tests/hermes_cli/test_kanban_db.py +++ b/tests/hermes_cli/test_kanban_db.py @@ -1316,6 +1316,7 @@ def test_migrate_add_optional_columns_tolerates_concurrent_migration(kanban_home CREATE TABLE tasks ( id INTEGER PRIMARY KEY, title TEXT NOT NULL, + status TEXT NOT NULL DEFAULT '', tenant TEXT, result TEXT, idempotency_key TEXT, @@ -1354,7 +1355,7 @@ def test_migrate_add_optional_columns_tolerates_concurrent_migration(kanban_home def test_connect_heals_reduced_tasks_schema_seeded_by_external_harness(kanban_home): """A board whose ``tasks`` table was created by an external harness without - the base v1 columns (body, workspace_kind, workspace_path, claim_lock, + the nullable/defaulted v1 columns (body, assignee, priority, ..., claim_lock, claim_expires) but which already has ``task_runs`` must connect: the connect-time in-flight backfill SELECTs ``claim_lock`` from ``tasks`` and used to raise ``no such column`` on every call (#112953), before @@ -1363,15 +1364,17 @@ def test_connect_heals_reduced_tasks_schema_seeded_by_external_harness(kanban_ho db_path = kanban_home / "foreign.db" seed = sqlite3.connect(db_path) seed.execute( - "CREATE TABLE tasks (id TEXT PRIMARY KEY, title TEXT NOT NULL, assignee TEXT," - " status TEXT NOT NULL, priority INTEGER DEFAULT 0, created_by TEXT," - " created_at INTEGER NOT NULL, started_at INTEGER, completed_at INTEGER)" + "CREATE TABLE tasks (id TEXT PRIMARY KEY, title TEXT NOT NULL," + " status TEXT NOT NULL, created_at INTEGER NOT NULL)" ) seed.execute(kbc._REBUILD_SPECS["task_runs"][0]) seed.commit() seed.close() - healed = {"body", "workspace_kind", "workspace_path", "claim_lock", "claim_expires"} + healed = { + "body", "assignee", "priority", "created_by", "started_at", "completed_at", + "workspace_kind", "workspace_path", "claim_lock", "claim_expires", + } conn = kbc.connect(db_path) try: cols = {r["name"] for r in conn.execute("PRAGMA table_info(tasks)")} @@ -1384,8 +1387,13 @@ def test_connect_heals_reduced_tasks_schema_seeded_by_external_harness(kanban_ho assert {c: healed_info[c] for c in healed} == {c: fresh_info[c] for c in healed} finally: conn.close() - # Second connect (the next dispatcher tick) is a no-op, not a re-raise. - kbc.connect(db_path).close() + # Second connect (the next dispatcher tick) is a no-op, not a re-raise, and + # the healed board is queryable (SELECT * reads every v1 column). + conn = kbc.connect(db_path) + try: + assert kb.list_tasks(conn) == [] + finally: + conn.close() # ---------------------------------------------------------------------------