Skip to content

fix(kanban): hoist all additive-column indexes out of SCHEMA_SQL (#28461) - #28781

Merged
kshitijk4poor merged 2 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/kanban-additive-indexes-28461
May 19, 2026
Merged

fix(kanban): hoist all additive-column indexes out of SCHEMA_SQL (#28461)#28781
kshitijk4poor merged 2 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/kanban-additive-indexes-28461

Conversation

@kshitijk4poor

@kshitijk4poor kshitijk4poor commented May 19, 2026

Copy link
Copy Markdown
Collaborator

Summary

Salvage of #28461 — fixes the Kanban DB legacy-migration crash reported in #28464 (and rediscovered in #28554, #28617, #28654, #28698) where connect() runs SCHEMA_SQL before _migrate_add_optional_columns(), so a legacy board missing any additive indexed column aborts schema init with OperationalError: no such column: <col> before the migration can add it.

The community filed six PRs for this bug (all on 2026-05-19). #28461 (itsreverence) was first and best-explained but only covered 2 of the 4 vulnerable indexes. #28754 (quocanh261997) extended coverage to 3 of the 4. This salvage combines the strengths of all the open PRs and closes the remaining gap.

What changed

Move every CREATE INDEX that depends on an additive column out of SCHEMA_SQL and create them after _migrate_add_optional_columns() adds the columns. All four additive-column indexes are now hoisted:

Index Additive column Introduced in
idx_tasks_session_id tasks.session_id #28447
idx_tasks_tenant tasks.tenant #16081
idx_tasks_idempotency tasks.idempotency_key #17805
idx_events_run task_events.run_id #17805

Three of these were ticking landmines for any user whose board predated the column. idx_tasks_session_id was the one that bit users today because it's the most recent (#28447 merged the day before the bug reports flooded in), but the same migration trap applied to all four.

Beyond #28754's coverage:

  • idx_events_run: also hoisted out of SCHEMA_SQL and made unconditional in the migration. A legacy task_events table predating feat(kanban): durable multi-profile collaboration board #17805 (no run_id) was still hitting the same SQLite trap that fix(kanban): migrate task session index after columns #28754 only addressed for tasks.
  • idx_tasks_idempotency: removed the redundant inner-conditional create (line ~1085) that was nested inside if "idempotency_key" not in cols:. Since the unconditional create lower in the function already runs CREATE INDEX IF NOT EXISTS, the inner one was dead code on fresh DBs once we removed the SCHEMA_SQL line.
  • Strengthened the regression test to cover all 4 indexes and to seed a pre-feat(kanban): durable multi-profile collaboration board #17805 task_events shape that exercises the run_id migration path.
  • Added explicit rationale comments so the next contributor adding tasks.<new_col> doesn't re-introduce the trap.
  • Picked up an adjacent test fix from fix(kanban): migrate task session index after columns #28754: test_max_runtime_uses_current_run_start_after_retry now monkeypatch-es kb._pid_alive so it stops tripping tests/conftest.py's live-system guard on os.kill(<fake_pid>, 0). Pre-existing failure on origin/main, unrelated to the index ordering bug; carried in via the cherry-pick rather than split out separately.

Validation

Confirmed against origin/main:

origin/main reproduces against pre-#16081 fixture: no such column: session_id

Confirmed on this branch:

Scenario A: legacy DB missing all 4 additive columns      ✅
Scenario B: fresh DB                                       ✅
Scenario C: idempotency on already-migrated DB             ✅
Scenario D: partially-migrated DB (tenant+idempotency)     ✅

Test suite:

$ bash scripts/run_tests.sh tests/hermes_cli/test_kanban_db.py tests/hermes_cli/test_kanban_db_init.py
============================= 159 passed in 5.06s ==============================

Full tests/hermes_cli/ suite — 4888 passed, 13 pre-existing failures unrelated to kanban (gateway service / model picker / plugins) that match origin/main 1:1.

Lint diff: zero new ruff/ty issues vs origin/main (123 ty issues on changed files, identical on both base and head).

Credit

Original bug report and PR by @itsreverence (#28464 + #28461 — first to file, best write-up of the underlying ordering trap).

Cherry-picked commit on this branch authored by @quocanh261997 from #28754 (broadest open coverage — included tasks.tenant and tasks.idempotency_key).

Other PRs absorbed and to be closed as duplicates: #28741 (@stormhierta), #28562 (@kungunier), #28620 (@vanhci), #28602 (@verybigdog, DRAFT). All of them correctly diagnosed the bug; this salvage combines the broadest coverage with the most thorough comments and tests.

Closes #28464.
Closes #28554.
Closes #28617.
Closes #28654.
Closes #28698.

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

3 participants