diff --git a/hermes_cli/kanban.py b/hermes_cli/kanban.py index 0937fc40f2e76..5ce2e5319bb31 100644 --- a/hermes_cli/kanban.py +++ b/hermes_cli/kanban.py @@ -626,6 +626,14 @@ def build_parser(parent_subparsers: argparse._SubParsersAction) -> argparse.Argu action="store_true", help="Promote even if parent dependencies are not yet done/archived", ) + p_promote.add_argument( + "--from-triage", + action="store_true", + help=( + "Recover an already specified/re-triaged task currently in triage " + "(requires an audit reason and clear claim/runtime ownership)" + ), + ) p_promote.add_argument( "--dry-run", action="store_true", @@ -2192,6 +2200,7 @@ def _cmd_promote(args: argparse.Namespace) -> int: reason = " ".join(args.reason).strip() if args.reason else None author = _profile_author() as_json = getattr(args, "json", False) + from_triage = bool(getattr(args, "from_triage", False)) extra_ids = list(getattr(args, "ids", None) or []) # Dedupe while preserving order; positional task_id always first. ids: list[str] = [] @@ -2211,12 +2220,14 @@ def _cmd_promote(args: argparse.Namespace) -> int: reason=reason, force=bool(args.force), dry_run=bool(args.dry_run), + from_triage=from_triage, ) results.append({ "task_id": tid, "promoted": ok, "dry_run": bool(args.dry_run), "forced": bool(args.force), + "from_triage": from_triage, "reason": reason, "error": err, }) diff --git a/hermes_cli/kanban_db.py b/hermes_cli/kanban_db.py index a99cda572862e..96b6d0d41e3cb 100644 --- a/hermes_cli/kanban_db.py +++ b/hermes_cli/kanban_db.py @@ -5431,31 +5431,91 @@ def promote_task( reason: Optional[str] = None, force: bool = False, dry_run: bool = False, + from_triage: bool = False, ) -> tuple[bool, Optional[str]]: - """Manually promote a `todo` or `blocked` task to `ready`. + """Manually promote a `todo`, `blocked`, or explicitly recovered triage task. Mirrors the automatic promotion done by ``recompute_ready`` but drives it from a deliberate operator action with an audit-trail entry. Refuses to promote if any parent dep is not in a terminal state (`done`/`archived`) unless ``force=True``. Does NOT change - assignee or claim state. Returns ``(True, None)`` on success and - ``(False, reason)`` if refused. ``dry_run=True`` validates the - promotion would succeed without mutating state. + task content, assignment, or workspace. Triage recovery requires a + nonempty reason, durable specification/re-triage evidence, and no active + claim/runtime ownership, and never permits ``force=True``. Returns + ``(True, None)`` on success and ``(False, reason)`` if refused. + ``dry_run=True`` validates the promotion would succeed without mutating + state. """ - row = conn.execute( - "SELECT status FROM tasks WHERE id = ?", (task_id,) - ).fetchone() - if row is None: - return False, f"task {task_id} not found" + if from_triage and force: + return False, "--from-triage cannot be combined with --force" + audit_reason = reason.strip() if reason is not None else None + if from_triage and not audit_reason: + return False, "--from-triage requires a nonempty audit reason" - cur_status = row["status"] - if cur_status not in ("todo", "blocked"): - return False, ( - f"task {task_id} is {cur_status!r}; promote only applies to " - f"'todo' or 'blocked'" - ) + with write_txn(conn): + row = conn.execute( + "SELECT status, block_kind, block_recurrences, " + "consecutive_failures, claim_lock, claim_expires, worker_pid, " + "current_run_id FROM tasks WHERE id = ?", + (task_id,), + ).fetchone() + if row is None: + return False, f"task {task_id} not found" + + cur_status = row["status"] + if from_triage: + if cur_status != "triage": + return False, ( + f"task {task_id} is {cur_status!r}; --from-triage " + "requires current status 'triage'" + ) + else: + if cur_status == "triage": + return False, ( + f"task {task_id} is 'triage'; use --from-triage with an " + "audit reason for manual recovery" + ) + if cur_status not in ("todo", "blocked"): + return False, ( + f"task {task_id} is {cur_status!r}; promote only applies " + "to todo or blocked" + ) + + if from_triage: + task_owned = any( + row[field] is not None + for field in ( + "claim_lock", + "claim_expires", + "worker_pid", + "current_run_id", + ) + ) + run_owned = conn.execute( + "SELECT 1 FROM task_runs WHERE task_id = ? AND (" + "ended_at IS NULL OR status = 'running' OR claim_lock IS NOT NULL " + "OR claim_expires IS NOT NULL OR worker_pid IS NOT NULL" + ") LIMIT 1", + (task_id,), + ).fetchone() + if task_owned or run_owned is not None: + return False, ( + f"task {task_id} has an active claim/runtime ownership " + "invariant; resolve it with a separate audited reclaim/" + "repair operation before --from-triage recovery" + ) + + proof = conn.execute( + "SELECT 1 FROM task_events WHERE task_id = ? " + "AND kind IN ('specified', 'block_loop_detected') LIMIT 1", + (task_id,), + ).fetchone() + if proof is None: + return False, ( + "--from-triage requires durable 'specified' or " + "'block_loop_detected' event proof" + ) - if not force: parents = conn.execute( "SELECT t.id, t.status FROM tasks t " "JOIN task_links l ON l.parent_id = t.id " @@ -5466,29 +5526,46 @@ def promote_task( p["id"] for p in parents if p["status"] not in ("done", "archived") ] - if unsatisfied: + if unsatisfied and (from_triage or not force): return False, ( f"unsatisfied parent dependencies: " - f"{', '.join(unsatisfied)} (use --force to override)" + f"{', '.join(unsatisfied)}" + + ("" if from_triage else " (use --force to override)") ) + if dry_run: + return True, None - if dry_run: - return True, None - - with write_txn(conn): + statuses = ("triage",) if from_triage else ("todo", "blocked") + placeholders = ", ".join("?" for _ in statuses) upd = conn.execute( "UPDATE tasks SET status = 'ready' " - "WHERE id = ? AND status IN ('todo', 'blocked')", - (task_id,), + f"WHERE id = ? AND status IN ({placeholders})", + (task_id, *statuses), ) 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}, - ) + if cur_status == "triage": + _append_event( + conn, + task_id, + "triage_recovered_manual", + { + "actor": actor, + "reason": audit_reason, + "prior_status": cur_status, + "parent_gate": "satisfied", + "block_kind": row["block_kind"], + "block_recurrences": row["block_recurrences"], + "consecutive_failures": row["consecutive_failures"], + }, + ) + else: + _append_event( + conn, + task_id, + "promoted_manual", + {"actor": actor, "reason": reason, "forced": force}, + ) return True, None diff --git a/tests/hermes_cli/test_kanban_promote.py b/tests/hermes_cli/test_kanban_promote.py index 6cbf3b77071b8..92bc98968252d 100644 --- a/tests/hermes_cli/test_kanban_promote.py +++ b/tests/hermes_cli/test_kanban_promote.py @@ -166,17 +166,317 @@ def test_promote_blocked_task_works(conn): def _promote_ns(task_id, *, ids=None, reason=None, force=False, - dry_run=False, as_json=False): + dry_run=False, as_json=False, from_triage=False): return argparse.Namespace( task_id=task_id, reason=list(reason or []), ids=list(ids or []) or None, force=force, + from_triage=from_triage, dry_run=dry_run, json=as_json, ) +def _triage_task( + conn, + *, + parent_status="done", + body="already specified", + proof_kind="block_loop_detected", +): + parent = kb.create_task(conn, title="parent", assignee="setup") + task = kb.create_task( + conn, + title="triaged", + body=body, + parents=[parent], + assignee="owner", + workspace_kind="dir", + workspace_path="/tmp/hermes-triage-recovery", + triage=True, + ) + if parent_status is not None: + conn.execute( + "UPDATE tasks SET status=? WHERE id=?", (parent_status, parent) + ) + conn.execute( + "UPDATE tasks SET block_kind='transient', block_recurrences=3, " + "consecutive_failures=4, last_failure_error='kept' WHERE id=?", + (task,), + ) + if proof_kind: + kb._append_event(conn, task, proof_kind, None) + return task, parent + + +def test_triage_promote_requires_explicit_flag(conn): + task, _ = _triage_task(conn) + ok, err = kb.promote_task(conn, task, actor="tester", reason="audited") + assert ok is False and "--from-triage" in err + assert kb.get_task(conn, task).status == "triage" + + +@pytest.mark.parametrize("reason", [None, "", " "]) +def test_triage_promote_requires_nonempty_reason(conn, reason): + task, _ = _triage_task(conn) + ok, err = kb.promote_task( + conn, task, actor="tester", reason=reason, from_triage=True + ) + assert ok is False and "audit reason" in err + assert kb.get_task(conn, task).status == "triage" + + +def test_triage_promote_rejects_force(conn): + task, _ = _triage_task(conn) + ok, err = kb.promote_task( + conn, + task, + actor="tester", + reason="audited", + force=True, + from_triage=True, + ) + assert ok is False and "cannot be combined" in err + assert kb.get_task(conn, task).status == "triage" + + +def test_triage_promote_enforces_parent_gate(conn): + task, parent = _triage_task(conn, parent_status=None) + ok, err = kb.promote_task( + conn, task, actor="tester", reason="audited", from_triage=True + ) + assert ok is False and parent in err + assert kb.get_task(conn, task).status == "triage" + + +@pytest.mark.parametrize("status", ["todo", "blocked"]) +def test_from_triage_rejects_non_triage_status(conn, status): + task, _ = _triage_task(conn) + conn.execute("UPDATE tasks SET status=? WHERE id=?", (status, task)) + ok, err = kb.promote_task( + conn, task, actor="tester", reason="audited", from_triage=True + ) + assert ok is False and "status 'triage'" in err + assert kb.get_task(conn, task).status == status + + +def test_triage_promote_rejects_fresh_bodyless_card(conn): + task, _ = _triage_task(conn, body=None, proof_kind=None) + ok, err = kb.promote_task( + conn, task, actor="tester", reason="audited", from_triage=True + ) + assert ok is False and "durable" in err + assert kb.get_task(conn, task).status == "triage" + + +def test_triage_promote_requires_event_proof_not_just_body(conn): + task, _ = _triage_task(conn, proof_kind=None) + ok, err = kb.promote_task( + conn, task, actor="tester", reason="audited", from_triage=True + ) + assert ok is False and "durable" in err + assert kb.get_task(conn, task).status == "triage" + + +@pytest.mark.parametrize( + "ownership", + [ + "task_claim_lock", + "task_claim_expires", + "task_worker_pid", + "task_current_run_id", + "run_open", + "run_running", + "run_claim_lock", + "run_claim_expires", + "run_worker_pid", + ], +) +def test_triage_promote_rejects_any_runtime_ownership(conn, ownership): + task, _ = _triage_task(conn) + if ownership.startswith("task_") and ownership != "task_current_run_id": + field = ownership.removeprefix("task_") + value = { + "claim_lock": "task-lease", + "claim_expires": 2000000000, + "worker_pid": 12345, + }[field] + conn.execute(f"UPDATE tasks SET {field}=? WHERE id=?", (value, task)) + else: + run = { + "status": "reclaimed", + "claim_lock": None, + "claim_expires": None, + "worker_pid": None, + "ended_at": 2, + } + if ownership != "task_current_run_id": + field = ownership.removeprefix("run_") + if field == "open": + run["ended_at"] = None + elif field == "running": + run["status"] = "running" + else: + run[field] = { + "claim_lock": "run-lease", + "claim_expires": 2000000000, + "worker_pid": 12345, + }[field] + run_id = conn.execute( + "INSERT INTO task_runs " + "(task_id, profile, status, claim_lock, claim_expires, worker_pid, " + "started_at, ended_at) VALUES (?, 'owner', ?, ?, ?, ?, 1, ?)", + ( + task, + run["status"], + run["claim_lock"], + run["claim_expires"], + run["worker_pid"], + run["ended_at"], + ), + ).lastrowid + if ownership == "task_current_run_id": + conn.execute( + "UPDATE tasks SET current_run_id=? WHERE id=?", (run_id, task) + ) + task_before = dict(conn.execute( + "SELECT * FROM tasks WHERE id=?", (task,) + ).fetchone()) + runs_before = [dict(row) for row in conn.execute( + "SELECT * FROM task_runs WHERE task_id=? ORDER BY id", (task,) + ).fetchall()] + + ok, err = kb.promote_task( + conn, + task, + actor="tester", + reason="operator reviewed recurrence", + from_triage=True, + ) + + assert ok is False and "reclaim" in err + assert dict(conn.execute( + "SELECT * FROM tasks WHERE id=?", (task,) + ).fetchone()) == task_before + assert [dict(row) for row in conn.execute( + "SELECT * FROM task_runs WHERE task_id=? ORDER BY id", (task,) + ).fetchall()] == runs_before + assert conn.execute( + "SELECT COUNT(*) AS n FROM task_events WHERE task_id=? " + "AND kind IN ('triage_recovered_manual', 'reclaimed')", + (task,), + ).fetchone()["n"] == 0 + + +@pytest.mark.parametrize("parent_status", ["done", "archived"]) +@pytest.mark.parametrize("proof_kind", ["specified", "block_loop_detected"]) +def test_triage_promote_is_immediately_claimable( + conn, parent_status, proof_kind +): + task, _ = _triage_task( + conn, parent_status=parent_status, proof_kind=proof_kind + ) + conn.execute( + "INSERT INTO task_runs " + "(task_id, profile, status, last_heartbeat_at, started_at, ended_at, " + "outcome) VALUES (?, 'owner', 'reclaimed', 2, 1, 3, 'reclaimed')", + (task,), + ) + before = kb.get_task(conn, task) + + ok, err = kb.promote_task( + conn, + task, + actor="tester", + reason="operator reviewed recurrence", + from_triage=True, + ) + + assert ok and err is None + after = kb.get_task(conn, task) + assert after.status == "ready" + preserved = ( + "title", "body", "assignee", "workspace_kind", "workspace_path", + "block_kind", "block_recurrences", "consecutive_failures", + "last_failure_error", + ) + assert {name: getattr(after, name) for name in preserved} == { + name: getattr(before, name) for name in preserved + } + assert (after.claim_lock, after.claim_expires, after.worker_pid) == ( + None, None, None + ) + assert after.current_run_id is None + + event = conn.execute( + "SELECT payload FROM task_events WHERE task_id=? " + "AND kind='triage_recovered_manual'", + (task,), + ).fetchone() + assert event is not None + assert json.loads(event["payload"]) == { + "actor": "tester", + "reason": "operator reviewed recurrence", + "prior_status": "triage", + "parent_gate": "satisfied", + "block_kind": "transient", + "block_recurrences": 3, + "consecutive_failures": 4, + } + claimed = kb.claim_task(conn, task, claimer="recovery-test") + assert claimed is not None + assert claimed.status == "running" + assert claimed.claim_lock == "recovery-test" + assert claimed.current_run_id is not None + + +def test_triage_promote_dry_run_has_no_mutation_or_event(conn): + task, _ = _triage_task(conn) + ok, err = kb.promote_task( + conn, + task, + actor="tester", + reason="audited", + from_triage=True, + dry_run=True, + ) + assert ok and err is None + assert kb.get_task(conn, task).status == "triage" + assert conn.execute( + "SELECT COUNT(*) AS n FROM task_events WHERE task_id=? " + "AND kind='triage_recovered_manual'", + (task,), + ).fetchone()["n"] == 0 + + +def test_cli_parser_accepts_from_triage_before_task(): + parser = argparse.ArgumentParser() + kb_cli.build_parser(parser.add_subparsers(dest="command")) + args = parser.parse_args([ + "kanban", "promote", "--from-triage", "t_example", "operator", "reviewed", + ]) + assert args.from_triage is True + assert args.task_id == "t_example" + assert args.reason == ["operator", "reviewed"] + + +def test_cli_triage_promote_json(kanban_home, capsys): + with kb.connect() as conn: + task, _ = _triage_task(conn) + rc = kb_cli._cmd_promote(_promote_ns( + task, + reason=["operator", "reviewed"], + from_triage=True, + as_json=True, + )) + assert rc == 0 + payload = json.loads(capsys.readouterr().out) + assert payload["task_id"] == task + assert payload["from_triage"] is True + assert payload["promoted"] is True + + def test_cli_promote_bulk_ids_promotes_all(kanban_home, capsys): with kb.connect() as conn: parent = kb.create_task(conn, title="parent") diff --git a/website/docs/user-guide/features/kanban.md b/website/docs/user-guide/features/kanban.md index 2b68409249c06..e293e7dc0a433 100644 --- a/website/docs/user-guide/features/kanban.md +++ b/website/docs/user-guide/features/kanban.md @@ -676,6 +676,7 @@ hermes kanban reassign ... # bulk re-assign tasks to hermes kanban edit [--title ...] [--body ...] # edit task title / body / priority in place [--priority N] hermes kanban promote ... # move todo/blocked tasks to ready (recovery) + [--from-triage] [...] # audited specified/re-triaged recovery; no active ownership hermes kanban schedule --at # set/clear a task's scheduled_at start time hermes kanban diagnostics [--json] # board health snapshot (alias: diag) hermes kanban link @@ -944,6 +945,7 @@ Every transition appends a row to `task_events`. Each row carries an optional `r | `dependency_wait` | `{reason, kind}` | Worker blocked with `kind=dependency` — the task is only waiting on another task, so it routes to `todo` (parent-gated, auto-promoted) instead of `blocked`. No human needed. | | `block_loop_detected` | `{reason, kind, recurrences, limit}` | A task was unblocked and re-blocked for the same reason `BLOCK_RECURRENCE_LIMIT` times (default 2). Instead of landing in `blocked` again — where a cron would keep unblocking it — it routes to `triage` for a human decision, breaking the unblock↔re-block loop. | | `unblocked` | — | `blocked → ready` (or `todo` if parents are still open), either manually or via `/unblock`. Resets the dispatcher's `consecutive_failures` but deliberately preserves `block_recurrences` so the loop breaker keeps its memory. `run_id` is `NULL`. | +| `triage_recovered_manual` | `{actor, reason, prior_status, parent_gate, block_kind, block_recurrences, consecutive_failures}` | An operator ran `kanban promote --from-triage ` after reviewing a task whose current status is exactly `triage`. The reason must be nonempty, `--force` is rejected, every parent must already be `done` or `archived`, and a durable prior `specified` or `block_loop_detected` event must prove the task was already specified/re-triaged. Any task/run claim or runtime ownership blocks recovery and must be resolved by a separate audited reclaim/repair operation first. The transition changes only `triage → ready`: it does not call an LLM, rewrite the task, change its assignee/workspace, alter ownership, or clear failure/recurrence evidence. Dry runs emit no event. | | `archived` | — | Hidden from the default board. If the task was still running, carries the `run_id` of the run that was reclaimed as a side effect. | **Edits** (human-driven changes that aren't transitions):