From 02005cfe20055dfaa0e4eaf900dc69d35be76d7b Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Wed, 9 Sep 2026 04:58:24 -0700 Subject: [PATCH] fix(kanban): promote refuses undone parents instead of a false --force success `hermes kanban promote --force ` printed `Promoted -> ready` and then the very next claim (a human `claim`, or the dispatcher tick seconds later) demoted the task back to `todo` with `claim_rejected {parents_not_done}` and returned None (#106195). The non-force refusal even pointed operators at `--force` as the escape hatch. The claim gate is deliberate: `claim_task` is the single enforcement point ("never ready -> running with an undone parent, whichever writer set 'ready'", cda20eec0c0), and `complete_task`/`request_review` re-check the same predicate, so a child let through by a forced claim could still never finish. A promotion override therefore has no honest outcome; the dependency edge is the real knob. - drop `--force` from `promote` (parser, CLI handler, `promote_task` kwarg, the `forced` event field nothing read) - the refusal message now states why the gate cannot be bypassed and names the working remedies: complete the parents or `hermes kanban unlink` - two invariant tests: refusal on an undone parent leaves `todo` with no fake `ready`; the flag no longer parses Salvage direction from #75354 by @vyacheslavk (diagnosis of the promote -> claim gap); the consume-at-claim authorization there is not taken because the same parent gate also blocks completion of the forced child. --- hermes_cli/kanban.py | 6 ++-- hermes_cli/kanban_db.py | 38 +++++++++++++------------ hermes_cli/kanban_parser.py | 1 - tests/hermes_cli/test_kanban_promote.py | 16 +++++++++-- 4 files changed, 37 insertions(+), 24 deletions(-) diff --git a/hermes_cli/kanban.py b/hermes_cli/kanban.py index 052c2fff59..70eb8b4547 100644 --- a/hermes_cli/kanban.py +++ b/hermes_cli/kanban.py @@ -1013,13 +1013,13 @@ def _cmd_promote(args: argparse.Namespace) -> int: author = _profile_author() # Dedupe while preserving order; positional task_id always first. ids = list(dict.fromkeys(_bulk_ids(args))) - dry_run, force = bool(args.dry_run), bool(args.force) + dry_run = bool(args.dry_run) results: list[dict[str, object]] = [] with kbc.connect_closing() as conn: for tid in ids: - ok, err = kb.promote_task(conn, tid, actor=author, reason=reason, force=force, dry_run=dry_run) - results.append({"task_id": tid, "promoted": ok, "dry_run": dry_run, "forced": force, + ok, err = kb.promote_task(conn, tid, actor=author, reason=reason, dry_run=dry_run) + results.append({"task_id": tid, "promoted": ok, "dry_run": dry_run, "reason": reason, "error": err}) failed = [r for r in results if not r["promoted"]] diff --git a/hermes_cli/kanban_db.py b/hermes_cli/kanban_db.py index df1cec02b2..96a13d6ea3 100644 --- a/hermes_cli/kanban_db.py +++ b/hermes_cli/kanban_db.py @@ -3173,11 +3173,11 @@ def request_changes( def promote_task( conn: sqlite3.Connection, task_id: str, *, actor: str, reason: Optional[str] = None, - force: bool = False, dry_run: bool = False, + dry_run: bool = False, ) -> tuple[bool, Optional[str]]: """Operator promotion ``todo``/``blocked`` -> ``ready`` with an audit event. - Refused while a parent is unfinished unless ``force``; ``dry_run`` only - validates. Returns ``(ok, reason)``.""" + Refused while a parent is unfinished; ``dry_run`` only validates. + Returns ``(ok, reason)``.""" cur_status = _task_status(conn, task_id) if cur_status is None: return False, f"task {task_id} not found" @@ -3188,18 +3188,22 @@ def promote_task( f"'todo' or 'blocked'" ) - if not force: - parents = conn.execute( - "SELECT t.id, t.status FROM tasks t " - "JOIN task_links l ON l.parent_id = t.id " - "WHERE l.child_id = ?", (task_id,), - ).fetchall() - unsatisfied = [p["id"] for p in parents if p["status"] not in ("done", "archived")] - if unsatisfied: - return False, ( - f"unsatisfied parent dependencies: " - f"{', '.join(unsatisfied)} (use --force to override)" - ) + # No override: claim_task demotes ready -> todo on an undone parent whichever + # writer set 'ready', so a forced promotion would only report a success the + # first claim silently reverts (#106195). The dependency itself is the knob. + parents = conn.execute( + "SELECT t.id, t.status FROM tasks t " + "JOIN task_links l ON l.parent_id = t.id " + "WHERE l.child_id = ?", (task_id,), + ).fetchall() + unsatisfied = [p["id"] for p in parents if p["status"] not in ("done", "archived")] + if unsatisfied: + return False, ( + f"unsatisfied parent dependencies: {', '.join(unsatisfied)} " + f"(the ready -> running claim re-checks parents, so promotion cannot " + f"bypass them; complete the parents or drop the link with " + f"`hermes kanban unlink {task_id}`)" + ) if dry_run: return True, None @@ -3211,9 +3215,7 @@ def promote_task( ) if upd.rowcount != 1: return False, f"task {task_id} status changed during promotion" - _append_event( - conn, task_id, "promoted_manual", {"actor": actor, "reason": reason, "forced": force}, - ) + _append_event(conn, task_id, "promoted_manual", {"actor": actor, "reason": reason}) return True, None diff --git a/hermes_cli/kanban_parser.py b/hermes_cli/kanban_parser.py index 6d0b413558..85977d0251 100644 --- a/hermes_cli/kanban_parser.py +++ b/hermes_cli/kanban_parser.py @@ -328,7 +328,6 @@ _SPECS = [ _TASK_ID, _arg("reason", nargs="*", help="Audit-trail reason (recorded on the task_events row)"), _bulk_ids("promote"), - _arg("--force", action="store_true", help="Promote even if parent dependencies are not yet done/archived"), _arg("--dry-run", action="store_true", help="Validate the promotion without mutating state"), _arg("--json", dest="json", action="store_true", help="Emit machine-readable JSON result"), ], help="Manually move one or more todo/blocked tasks to ready (recovery path)"), diff --git a/tests/hermes_cli/test_kanban_promote.py b/tests/hermes_cli/test_kanban_promote.py index 7c32dfb5ff..55398b90f3 100644 --- a/tests/hermes_cli/test_kanban_promote.py +++ b/tests/hermes_cli/test_kanban_promote.py @@ -64,10 +64,22 @@ def test_promote_stuck_todo_succeeds(conn): assert kb.get_task(conn, child).status == "ready" +def test_promote_refuses_undone_parent_and_names_the_real_remedy(conn): + # #106195: promotion must never report a 'ready' that the first claim reverts. + child, (parent,) = _stuck_todo(conn, parents_done=False) + ok, err = kb.promote_task(conn, child, actor="tester", reason="recovery") + assert not ok + assert parent in err and "--force" not in err and f"unlink {child}" in err + assert kb.get_task(conn, child).status == "todo" + assert kb.claim_task(conn, child) is None # still gated; nothing pretended - - +def test_cli_promote_has_no_force_flag(kanban_home): + from hermes_cli import kanban_parser + parser = argparse.ArgumentParser(prog="hermes", add_help=False) + kanban_parser.build_parser(parser.add_subparsers(dest="command")) + with pytest.raises(SystemExit): + parser.parse_args(["kanban", "promote", "t_x", "--force"]) # ---------------------------------------------------------------------------