fix(kanban): promote refuses undone parents instead of a false --force success
`hermes kanban promote --force <id>` printed `Promoted <id> -> 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'", cda20eec0c), 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.
This commit is contained in:
@@ -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"]]
|
||||
|
||||
@@ -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 <parent_id> {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
|
||||
|
||||
|
||||
@@ -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)"),
|
||||
|
||||
@@ -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 <parent_id> {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"])
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user