✨ feat(kanban): add move_task helper + hermes kanban move CLI - #12
Conversation
The one-card review model moves a single card through lanes (todo -> doing -> review -> merge -> done) instead of spawning a card per stage. The homestead webhook skills (slices 1-2) perform that status transition inline with an UPDATE inside write_txn plus hand-emitted status_changed/assigned events, but kanban_db had no sanctioned home for it: assign_task changes assignee only, complete_task/archive_task hit one terminal status each. Add move_task(conn, task_id, *, status, assignee=None, reason=None): it validates status against VALID_STATUSES, refuses to move a running (claimed) card, optionally reassigns (resetting the failure streak like assign_task does), and emits a status_changed event — idempotent, so a re-fired webhook or repeated CLI converges. Wire a `hermes kanban move <id> --lane <status> [--assignee <p>] [--reason <r>]` subcommand on top of it. This is Casey's outer-loop affordance: one command returns a card from the merge lane to doing+eckert. Per AGENTS.md narrow-waist rule this is a library helper + CLI command, NOT a new model tool — the core tool schema is unchanged. The 3 webhook skills' inline UPDATEs can later be refactored to call move_task (cross-repo follow-up).
cwest
left a comment
cwest
left a comment
There was a problem hiding this comment.
Reviewed the diff and ran it. The move_task helper does what the description says: it validates status against VALID_STATUSES, refuses to move a claimed running card, resets the failure streak on an owner change, and stays idempotent when the target lane and owner already match. SQL is parameterized and there's nothing security-relevant in the added lines.
I ran the suites the description cites and exercised the real CLI, not just the test harness:
- tests/hermes_cli/test_kanban_db.py + test_kanban_cli.py: 283 passed, 0 failed. The 14 new tests (10 for move_task, 4 for the CLI) all pass.
- hermes kanban move is wired into the real parser and dispatch (shows in --help with proper subcommand help). End-to-end against an isolated DB: a lane+owner move emits both status_changed and assigned, a lane-only move leaves ownership intact, --assignee none unassigns, and a repeated identical move adds no new events. Confirmed each against the actual task and task_events rows, not just stdout.
Two things worth a look, neither blocking. Holding this at a comment since it's still a draft.
| print(f"no such task: {args.task_id}", file=sys.stderr) | ||
| return 1 | ||
| if unassign: | ||
| kb.assign_task(conn, args.task_id, None) |
There was a problem hiding this comment.
The unassign case runs as two separate transactions: move_task commits the lane change, then assign_task commits the unassign. If the process dies between them the card is moved but still owned. The lane move and the unassign aren't atomic the way a lane+reassign in a single move_task call is. Probably fine for the CLI's interactive use, but if a webhook ever drives an unassigning move it's a torn-write window. One option is to let move_task accept an explicit unassign sentinel so the whole thing lands in one write_txn.
| ) | ||
| if not ok: | ||
| print(f"no such task: {args.task_id}", file=sys.stderr) | ||
| return 1 |
There was a problem hiding this comment.
This returns 1, and kanban_command propagates it, but the process still exits 0 — I confirmed hermes kanban move t_nope --lane review and a bad --lane both print the error and exit 0. It's not introduced here: assign, complete, and show on an unknown id all exit 0 the same way, so the exit code is being dropped upstream in main(), outside this PR's surface. Flagging because the plan lists exit 1 / non-zero as acceptance for the unknown-id and bad-lane cases, and the two CLI tests assert on the printed string rather than the exit code, so they pass while the actual exit status doesn't meet the stated bar. A follow-up to fix the propagation in main() (and tighten those two tests to check the code) would close the gap.
|
Closing as superseded-by-design. The one-card lifecycle moves cards between lanes via the sanctioned move_card helper (homestead skill machinery running inside write_txn with status_changed/assigned events), which covers the lane-transition need this CLI verb was for. A first-class 'move' CLI verb overlaps that existing path; not adding a second mechanism. The lane-motion verbs that remain (block/unblock/promote/complete/archive) plus move_card are sufficient. |
1 participant
What
Slice 3 Build B of eliminating the multi-card review chain (parent card t_8fb5741d; slices 1-2 shipped as homestead PRs #56/#58). The one-card review model moves a single card through lanes (
todo -> doing -> review -> merge -> done, plus the inner/outer review loops) instead of spawning a separate card per stage.This PR gives that status transition a sanctioned home in
kanban_db.pyand an outer-loop CLI affordance:move_task(conn, task_id, *, status, assignee=None, reason=None)— validatesstatusagainstVALID_STATUSES, refuses to move a running (claimed) card (RuntimeError, same safety asassign_task), optionally reassigns (resetting the per-profile failure streak on an owner change, likeassign_task), and emits astatus_changedevent. Idempotent: moving a card to the lane+owner it already occupies returnsTruewith no spurious events, so a re-fired webhook or a repeated CLI invocation converges.hermes kanban move <id> --lane <status> [--assignee <p>] [--reason <r>]— Casey's outer loop in one command:hermes kanban move t_abc --lane ready --assignee eckertreturns a card from the merge lane to doing+eckert.--assignee noneunassigns; omitting it leaves ownership untouched.Why this shape
Before this, the three homestead webhook skills did the lane move inline with an
UPDATE tasks SET status=?insidewrite_txnplus hand-emittedstatus_changed/assignedevents, becausekanban_dbhad no general status-move helper (assign_task= assignee only;complete_task/archive_task= one terminal status each).move_taskformalises that exact pattern so webhook moves and CLI moves share one audited transition path.Per
AGENTS.mdnarrow-waist rule, this is a library helper + CLI command, NOT a new model tool — the core tool schema is unchanged.Tests (TDD, RED -> GREEN)
move_task: status-only move +status_changed; status+reassign emits both events; reassign-without-status-change emits onlyassigned; reason recorded; status validation (ValueError); unknown task ->False; refuses running card; idempotent (no duplicate events); resets failure streak on reassign;assignee=Noneleaves ownership untouched.move: changes lane+assignee; lane-only; unknown id -> error+exit 1; bad lane ->ValueErrorsurfaced, card unmoved.Verification
test_kanban_db.py+test_kanban_cli.py+test_kanban_core_functionality.py+tools/test_kanban_tools.py+test_kanban_decompose.py): 561 passed, 1 skipped, 0 failed.test_signal_handler_kanban_worker.py::test_sigterm_with_kanban_task_env_terminates_quicklyfails identically on pristineorigin/cwest/integration(verified against a clean baseline worktree). Thetest_kanban_decompose.pyfailures seen only in the full 700-test parallel run are cross-test ordering pollution — those tests pass in isolation and in the combined relevant suite above; not caused by this change.Base branch
Targets
cwest/integration(notmain): the active kanban work — including the review-card dedup fixes slices 1-2 depend on — lives there (211+ lines ahead ofmaininkanban_db.py). Building onmainwould landmove_taskon a stale kanban_db.Follow-up (out of scope)
UPDATEs to callmove_task(cross-repo; homestead).Plan:
docs/plans/2026-06-27-kanban-move-cli.md