Skip to content

fix(kanban): avoid fragile failure-column renames - #20848

Closed
helix4u wants to merge 1 commit into
NousResearch:mainfrom
helix4u:fix/kanban-copy-legacy-failure-columns
Closed

fix(kanban): avoid fragile failure-column renames#20848
helix4u wants to merge 1 commit into
NousResearch:mainfrom
helix4u:fix/kanban-copy-legacy-failure-columns

Conversation

@helix4u

@helix4u helix4u commented May 6, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes the Kanban DB migration regression where the embedded dispatcher can fail on startup with sqlite3.OperationalError: no such column: "spawn_failures".

This is a follow-up to #20410, not a duplicate of it. #20410 changed the Kanban failure counter schema from spawn_failures / last_spawn_error to consecutive_failures / last_failure_error and used ALTER TABLE ... RENAME COLUMN to migrate existing DBs. The reported failure happens when an existing Kanban DB is on an older or partial schema that does not actually have spawn_failures, so the rename path itself crashes before the dispatcher can start.

This PR makes that migration tolerant of those real-world DB shapes: add the new columns if missing, copy legacy values only when the legacy columns exist, and leave any old columns harmlessly in place. Installs with old counters keep their values; installs without the legacy columns no longer crash.

Related Issue

Fixes #20842

Follow-up to #20410

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • hermes_cli/kanban_db.py
    • Replaces fragile legacy column renames with add-and-copy migration logic.
    • Copies spawn_failures into consecutive_failures only when the legacy column exists.
    • Copies last_spawn_error into last_failure_error only when the legacy column exists.
    • Preserves old columns harmlessly instead of forcing a SQLite table/schema rewrite.
  • tests/hermes_cli/test_kanban_core_functionality.py
    • Adds a regression test proving legacy failure counters and last error text migrate without renaming columns.
    • Verifies the migration is idempotent.

How to Test

  1. Run ./.venv/bin/pytest tests/hermes_cli/test_kanban_core_functionality.py::test_legacy_db_without_skills_column_migrates tests/hermes_cli/test_kanban_core_functionality.py::test_legacy_spawn_failure_columns_are_copied_not_renamed -q.
  2. Run ./.venv/bin/pytest tests/hermes_cli/test_kanban_core_functionality.py -q.
  3. Update an install with an existing Kanban DB and confirm hermes kanban init / embedded gateway dispatcher startup no longer raises no such column: "spawn_failures".

Checklist

Code

  • I’ve read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn’t a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I’ve run pytest tests/ -q and all tests pass
  • I’ve added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I’ve tested on my platform: Ubuntu/WSL2

Documentation & Housekeeping

  • I’ve updated relevant documentation (README, docs/, docstrings) — or N/A
  • I’ve updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I’ve updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I’ve considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I’ve updated tool descriptions/schemas if I changed tool behavior — or N/A

Screenshots / Logs

Targeted migration checks pass:

$ .venv/bin/pytest tests/hermes_cli/test_kanban_core_functionality.py::test_legacy_db_without_skills_column_migrates tests/hermes_cli/test_kanban_core_functionality.py::test_legacy_spawn_failure_columns_are_copied_not_renamed -q
2 passed in 4.51s

Full Kanban core test file passes:

$ .venv/bin/pytest tests/hermes_cli/test_kanban_core_functionality.py -q
140 passed in 8.34s

Full suite was run and is not green on this checkout:

$ scripts/run_tests.sh
47 failed, 20129 passed, 51 skipped, 232 warnings in 471.41s (0:07:51)

The full-suite failures are outside the touched Kanban migration path. They are in cron script prompting, gateway approval/config/DingTalk/API/restart tests, update command tests, model provider persistence/validation, concurrent interrupt tests, browser Chromium detection tests, delegation credentials/heartbeat, Dockerfile TUI dependency checks, skill provenance, credential-pool dotenv fallback, Tirith marker handling, and Daytona/Vercel environment command wrappers.

@helix4u
helix4u marked this pull request as ready for review May 6, 2026 17:44
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard labels May 6, 2026
@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Merged via PR #20855. Your fix was cherry-picked onto current main with your authorship preserved in git log. A follow-up commit added expanded comments (root-cause clarity, stale-cols invariant note) and two additional tests for Scenario A (no legacy columns — the exact #20842 crash path) and Scenario D (both columns already present — idempotency). Thanks for the clean fix and thorough PR description! 🙏

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 P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Kanban migration fails after update: no such column spawn_failures

3 participants