diff --git a/hermes_cli/kanban.py b/hermes_cli/kanban.py index 00a61b41d4d0..4eeb371030c4 100644 --- a/hermes_cli/kanban.py +++ b/hermes_cli/kanban.py @@ -406,12 +406,12 @@ def build_parser(parent_subparsers: argparse._SubParsersAction) -> argparse.Argu p_edit = sub.add_parser( "edit", - help="Edit recovery fields on an already-completed task", + help="Edit recovery fields on a task", ) p_edit.add_argument("task_id") p_edit.add_argument( "--result", - required=True, + default=None, help="Backfilled task result text for a done task", ) p_edit.add_argument( @@ -424,6 +424,21 @@ def build_parser(parent_subparsers: argparse._SubParsersAction) -> argparse.Argu default=None, help="JSON dict of structured facts to store on the latest completed run.", ) + p_edit.add_argument( + "--clear-skills", + action="store_true", + help="Clear persisted task skills on a non-running task.", + ) + p_edit.add_argument( + "--reset-failures", + action="store_true", + help="Reset consecutive failure counter and last failure error.", + ) + p_edit.add_argument( + "--clear-claim", + action="store_true", + help="Clear persisted claim fields on a non-running task.", + ) p_block = sub.add_parser("block", help="Mark one or more tasks blocked") p_block.add_argument("task_id") @@ -1037,25 +1052,29 @@ def _cmd_create(args: argparse.Namespace) -> int: file=sys.stderr, ) return 2 - with kb.connect() as conn: - task_id = kb.create_task( - conn, - title=args.title, - body=args.body, - assignee=args.assignee, - created_by=args.created_by or _profile_author(), - workspace_kind=ws_kind, - workspace_path=ws_path, - tenant=args.tenant, - priority=args.priority, - parents=tuple(args.parent or ()), - triage=bool(getattr(args, "triage", False)), - idempotency_key=getattr(args, "idempotency_key", None), - max_runtime_seconds=max_runtime, - skills=getattr(args, "skills", None) or None, - max_retries=max_retries, - ) - task = kb.get_task(conn, task_id) + try: + with kb.connect() as conn: + task_id = kb.create_task( + conn, + title=args.title, + body=args.body, + assignee=args.assignee, + created_by=args.created_by or _profile_author(), + workspace_kind=ws_kind, + workspace_path=ws_path, + tenant=args.tenant, + priority=args.priority, + parents=tuple(args.parent or ()), + triage=bool(getattr(args, "triage", False)), + idempotency_key=getattr(args, "idempotency_key", None), + max_runtime_seconds=max_runtime, + skills=getattr(args, "skills", None) or None, + max_retries=max_retries, + ) + task = kb.get_task(conn, task_id) + except ValueError as exc: + print(f"kanban: create: {exc}", file=sys.stderr) + return 2 if getattr(args, "json", False): print(json.dumps(_task_to_dict(task), indent=2, ensure_ascii=False)) else: @@ -1555,6 +1574,85 @@ def _cmd_complete(args: argparse.Namespace) -> int: def _cmd_edit(args: argparse.Namespace) -> int: + recovery_flags = sum(bool(x) for x in [ + getattr(args, "clear_skills", False), + getattr(args, "reset_failures", False), + getattr(args, "clear_claim", False), + ]) + if recovery_flags > 1: + print( + "kanban: edit accepts at most one recovery action flag " + "(--clear-skills, --reset-failures, or --clear-claim)", + file=sys.stderr, + ) + return 2 + if getattr(args, "clear_skills", False) and any([ + args.result is not None, + getattr(args, "summary", None) is not None, + getattr(args, "metadata", None) is not None, + ]): + print( + "kanban: edit --clear-skills cannot be combined with " + "--result/--summary/--metadata", + file=sys.stderr, + ) + return 2 + if getattr(args, "reset_failures", False) and any([ + args.result is not None, + getattr(args, "summary", None) is not None, + getattr(args, "metadata", None) is not None, + ]): + print( + "kanban: edit --reset-failures cannot be combined with " + "--result/--summary/--metadata", + file=sys.stderr, + ) + return 2 + if getattr(args, "clear_claim", False) and any([ + args.result is not None, + getattr(args, "summary", None) is not None, + getattr(args, "metadata", None) is not None, + ]): + print( + "kanban: edit --clear-claim cannot be combined with " + "--result/--summary/--metadata", + file=sys.stderr, + ) + return 2 + if ( + not getattr(args, "clear_skills", False) + and not getattr(args, "reset_failures", False) + and not getattr(args, "clear_claim", False) + and args.result is None + ): + print( + "kanban: edit requires --result unless a recovery flag is used", + file=sys.stderr, + ) + return 2 + if getattr(args, "reset_failures", False): + with kb.connect() as conn: + if not kb.reset_task_failures(conn, args.task_id): + print(f"cannot edit {args.task_id} (unknown id)", file=sys.stderr) + return 1 + print(f"Edited {args.task_id}") + return 0 + if getattr(args, "clear_claim", False): + with kb.connect() as conn: + ok = kb.edit_task_recovery_fields( + conn, + args.task_id, + clear_claim=True, + ) + if not ok: + print( + f"cannot clear claim on {args.task_id} " + f"(unknown id or task is running)", + file=sys.stderr, + ) + return 1 + print(f"Edited {args.task_id}") + return 0 raw_meta = getattr(args, "metadata", None) metadata = None if raw_meta: @@ -1566,13 +1664,29 @@ def _cmd_edit(args: argparse.Namespace) -> int: print(f"kanban: --metadata: {exc}", file=sys.stderr) return 2 with kb.connect() as conn: - if not kb.edit_completed_task_result( + if not kb.edit_task_recovery_fields( conn, args.task_id, result=args.result, summary=getattr(args, "summary", None), metadata=metadata, + clear_skills=bool(getattr(args, "clear_skills", False)), + clear_claim=bool(getattr(args, "clear_claim", False)), ): + if getattr(args, "clear_skills", False): + print( + f"cannot clear skills on {args.task_id} " + f"(unknown id or task is running)", + file=sys.stderr, + ) + return 1 + if getattr(args, "clear_claim", False): + print( + f"cannot clear claim on {args.task_id} " + f"(unknown id or task is running)", + file=sys.stderr, + ) + return 1 print( f"cannot edit {args.task_id} (unknown id or task is not done)", file=sys.stderr, @@ -1675,6 +1789,7 @@ def _cmd_dispatch(args: argparse.Namespace) -> int: ], "skipped_unassigned": res.skipped_unassigned, "skipped_nonspawnable": res.skipped_nonspawnable, + "skipped_invalid_skills": res.skipped_invalid_skills, }, indent=2)) return 0 print(f"Reclaimed: {res.reclaimed}") @@ -1699,6 +1814,11 @@ def _cmd_dispatch(args: argparse.Namespace) -> int: f"Skipped (non-spawnable assignee — terminal lane, OK): " f"{', '.join(res.skipped_nonspawnable)}" ) + if res.skipped_invalid_skills: + print( + f"Skipped (invalid task skills): " + f"{', '.join(res.skipped_invalid_skills)}" + ) return 0 diff --git a/hermes_cli/kanban_db.py b/hermes_cli/kanban_db.py index aa3655b17629..abab26a5a25e 100644 --- a/hermes_cli/kanban_db.py +++ b/hermes_cli/kanban_db.py @@ -83,6 +83,8 @@ from pathlib import Path from typing import Any, Iterable, Optional +from toolsets import get_toolset_names + # --------------------------------------------------------------------------- # Constants @@ -91,6 +93,15 @@ VALID_STATUSES = {"triage", "todo", "ready", "running", "blocked", "done", "archived"} VALID_WORKSPACE_KINDS = {"scratch", "worktree", "dir"} +# ``tasks.skills`` stores worker SKILL bundles that are forwarded to +# ``hermes --skills ...``. Toolset names are a different config surface; when +# they land here by mistake, worker startup eventually fails with +# "Unknown skill(s): ...". Resolve against the toolset registry so the +# validation stays aligned with the actual shipped toolset surface. +INVALID_TASK_SKILL_NAMES = frozenset( + str(name).strip().casefold() for name in get_toolset_names() +) + # A running task's claim is valid for 15 minutes; after that the next # dispatcher tick reclaims it. Workers that outlive this window should call # ``heartbeat_claim(task_id)`` periodically. In practice most kanban @@ -1211,6 +1222,42 @@ def _canonical_assignee(assignee: Optional[str]) -> Optional[str]: return normalize_profile_name(assignee) +def _normalize_task_skills(skills: Optional[Iterable[str]]) -> Optional[list[str]]: + """Normalize task skills and reject obvious toolset-name confusion.""" + if skills is None: + return None + cleaned: list[str] = [] + seen: set[str] = set() + invalid: list[str] = [] + for s in skills: + if not s: + continue + name = str(s).strip() + if not name: + continue + if "," in name: + raise ValueError( + f"skill name cannot contain comma: {name!r} " + f"(pass a list of separate names instead of a comma-joined string)" + ) + lowered = name.casefold() + if lowered in INVALID_TASK_SKILL_NAMES: + if lowered not in invalid: + invalid.append(lowered) + continue + if name in seen: + continue + seen.add(name) + cleaned.append(name) + if invalid: + raise ValueError( + "task skills must be SKILL bundle names, not toolset names. " + f"Invalid values: {', '.join(invalid)}. Configure toolsets on " + "the assignee profile instead." + ) + return cleaned + + def create_task( conn: sqlite3.Connection, *, @@ -1263,31 +1310,7 @@ def create_task( ) parents = tuple(p for p in parents if p) - # Normalise + validate skills: strip whitespace, drop empties, dedupe - # (preserving order). Refuse commas inside a single name so we don't - # invisibly splatter a comma-joined string into one argv slot — the - # `hermes --skills X,Y` comma syntax is handled in the dispatcher, - # not here. - skills_list: Optional[list[str]] = None - if skills is not None: - cleaned: list[str] = [] - seen: set[str] = set() - for s in skills: - if not s: - continue - name = str(s).strip() - if not name: - continue - if "," in name: - raise ValueError( - f"skill name cannot contain comma: {name!r} " - f"(pass a list of separate names instead of a comma-joined string)" - ) - if name in seen: - continue - seen.add(name) - cleaned.append(name) - skills_list = cleaned + skills_list = _normalize_task_skills(skills) # Idempotency check — return the existing task instead of creating a # duplicate. Done BEFORE entering write_txn to keep the fast path fast @@ -2413,21 +2436,97 @@ def complete_task( return True -def edit_completed_task_result( +def edit_task_recovery_fields( conn: sqlite3.Connection, task_id: str, *, - result: str, + result: Optional[str] = None, summary: Optional[str] = None, metadata: Optional[dict] = None, + clear_skills: bool = False, + clear_claim: bool = False, ) -> bool: - """Backfill the user-visible result for an already completed task.""" + """Edit narrow recovery fields on a task. + + Supported today: + * backfill result/summary/metadata on an already-completed task + * clear persisted ``skills`` on a non-running task + * clear a stale claim on a non-running task that still has claim state + """ + if not clear_skills and not clear_claim and result is None: + raise ValueError( + "result is required unless clear_skills=True or clear_claim=True" + ) handoff_summary = summary if summary is not None else result with write_txn(conn): row = conn.execute( - "SELECT status FROM tasks WHERE id = ?", (task_id,), + "SELECT status, claim_lock, claim_expires, worker_pid, current_run_id " + "FROM tasks WHERE id = ?", (task_id,), ).fetchone() - if not row or row["status"] != "done": + if not row: + return False + if clear_claim: + if row["status"] == "running": + return False + had_claim_state = not ( + row["claim_lock"] is None + and row["claim_expires"] is None + and row["worker_pid"] is None + and row["current_run_id"] is None + ) + if not had_claim_state: + return True + run_id = row["current_run_id"] + if run_id is not None: + _end_run( + conn, task_id, + outcome="reclaimed", status="reclaimed", + error="operator_clear_claim", + ) + conn.execute( + "UPDATE tasks SET claim_lock = NULL, claim_expires = NULL, " + "worker_pid = NULL, last_heartbeat_at = NULL, " + "current_run_id = NULL WHERE id = ?", + (task_id,), + ) + _append_event( + conn, task_id, "edited", + { + "fields": [ + "claim_lock", + "claim_expires", + "worker_pid", + "last_heartbeat_at", + "current_run_id", + ], + "claim_cleared": True}, + run_id=run_id, + ) + return True + if clear_skills: + if row["status"] == "running": + return False + run_id = None + if row["status"] == "done": + run = conn.execute( + """ + SELECT id FROM task_runs + WHERE task_id = ? + AND outcome = 'completed' + ORDER BY COALESCE(ended_at, started_at, 0) DESC, id DESC + LIMIT 1 + """, + (task_id,), + ).fetchone() + run_id = int(run["id"]) if run else None + conn.execute("UPDATE tasks SET skills = NULL WHERE id = ?", (task_id,)) + _append_event( + conn, task_id, "edited", + {"fields": ["skills"], "skills_cleared": True}, + run_id=run_id, + ) + return True + if row["status"] != "done": return False conn.execute( "UPDATE tasks SET result = ? WHERE id = ?", @@ -2480,6 +2579,26 @@ def edit_completed_task_result( return True +def edit_completed_task_result( + conn: sqlite3.Connection, + task_id: str, + *, + result: Optional[str] = None, + summary: Optional[str] = None, + metadata: Optional[dict] = None, + clear_skills: bool = False, +) -> bool: + """Back-compat wrapper for the broader recovery-field editor.""" + return edit_task_recovery_fields( + conn, + task_id, + result=result, + summary=summary, + metadata=metadata, + clear_skills=clear_skills, + ) + + def block_task( conn: sqlite3.Connection, task_id: str, @@ -2810,6 +2929,11 @@ class DispatchResult: operator-actionable failure. Tracked separately so health telemetry can distinguish "real stuck" (nothing spawned but spawnable work available) from "correctly idle" (nothing spawnable in the queue).""" + skipped_invalid_skills: list[str] = field(default_factory=list) + """Ready task ids skipped because persisted ``task.skills`` contains + invalid toolset names. Operator-actionable configuration error: the + task stays ready but the dispatcher refuses to spawn it until the + bad skills are cleared or the task is recreated.""" crashed: list[str] = field(default_factory=list) """Task ids reclaimed because their worker PID disappeared.""" auto_blocked: list[str] = field(default_factory=list) @@ -3526,6 +3650,30 @@ def _clear_failure_counter(conn: sqlite3.Connection, task_id: str) -> None: ) +def reset_task_failures(conn: sqlite3.Connection, task_id: str) -> bool: + """Clear a task's consecutive-failure counter as an operator recovery + action. Returns ``True`` if the task exists.""" + with write_txn(conn): + row = conn.execute( + "SELECT status, current_run_id FROM tasks WHERE id = ?", + (task_id,), + ).fetchone() + if row is None: + return False + conn.execute( + "UPDATE tasks SET consecutive_failures = 0, " + "last_failure_error = NULL WHERE id = ?", + (task_id,), + ) + _append_event( + conn, task_id, "edited", + {"fields": ["consecutive_failures", "last_failure_error"], + "failures_reset": True}, + run_id=row["current_run_id"], + ) + return True + + # Legacy alias for test-code and anything else that still imports it. _clear_spawn_failures = _clear_failure_counter @@ -3650,6 +3798,16 @@ def dispatch_once( if not row["assignee"]: result.skipped_unassigned.append(row["id"]) continue + task = get_task(conn, row["id"]) + if task is not None: + invalid_skills = [ + str(s).casefold() + for s in (task.skills or []) + if str(s).strip().casefold() in INVALID_TASK_SKILL_NAMES + ] + if invalid_skills: + result.skipped_invalid_skills.append(row["id"]) + continue # Skip ready tasks whose assignee is not a real Hermes profile. # `_default_spawn` invokes ``hermes -p `` which fails # with "Profile 'X' does not exist" when the assignee names a diff --git a/hermes_cli/kanban_diagnostics.py b/hermes_cli/kanban_diagnostics.py index d2ba26cb835b..b594e0047e55 100644 --- a/hermes_cli/kanban_diagnostics.py +++ b/hermes_cli/kanban_diagnostics.py @@ -34,6 +34,8 @@ import json import time +from hermes_cli.kanban_db import INVALID_TASK_SKILL_NAMES + # Severity rungs, ordered least → most urgent. The UI colors them # amber (warning), orange (error), red (critical). Sorted outputs put @@ -219,6 +221,37 @@ def _generic_recovery_actions(task: Any, *, running: bool) -> list[DiagnosticAct return out +def _profile_exists_safe(name: Optional[str]) -> Optional[bool]: + if not name: + return None + try: + from hermes_cli import profiles + return bool(profiles.profile_exists(name)) + except Exception: + return None + + +def _profile_toolsets_safe(name: Optional[str]) -> Optional[list[str]]: + if not name: + return None + try: + from hermes_cli import profiles + from hermes_cli import config as config_mod + from hermes_constants import _profile_override_context + + profile_dir = profiles.get_profile_dir(name) + if not profile_dir.is_dir(): + return None + with _profile_override_context(str(profile_dir)): + cfg = config_mod.read_raw_config() + toolsets = cfg.get("toolsets") + if not isinstance(toolsets, list): + return None + return [str(t).strip() for t in toolsets if str(t).strip()] + except Exception: + return None + + # --------------------------------------------------------------------------- # Rule implementations # --------------------------------------------------------------------------- @@ -570,9 +603,138 @@ def _rule_stuck_in_blocked(task, events, runs, now, cfg) -> list[Diagnostic]: )] +def _rule_invalid_task_skills(task, events, runs, now, cfg) -> list[Diagnostic]: + skills = _task_field(task, "skills") or [] + if not skills: + return [] + invalid = [s for s in skills if str(s).casefold() in INVALID_TASK_SKILL_NAMES] + if not invalid: + return [] + task_id = _task_field(task, "id") or "TASK_ID" + running = _task_field(task, "status") == "running" + actions: list[DiagnosticAction] = [] + if not running: + actions.append(DiagnosticAction( + kind="cli_hint", + label=f"Clear invalid skills: hermes kanban edit {task_id} --clear-skills", + payload={"command": f"hermes kanban edit {task_id} --clear-skills"}, + suggested=True, + )) + actions.append(DiagnosticAction( + kind="cli_hint", + label=f"Inspect task: hermes kanban show {task_id}", + payload={"command": f"hermes kanban show {task_id}"}, + )) + actions.extend(_generic_recovery_actions(task, running=running)) + return [Diagnostic( + kind="invalid_task_skills", + severity="error", + title="Task skills contain toolset names", + detail=( + "This task's skills list contains Hermes toolset names rather than " + "SKILL bundle names. Dispatcher-spawned workers forward task.skills " + "through `--skills ...`, so values like web/browser/terminal/file " + "eventually fail worker startup with unknown-skill errors." + ), + actions=actions, + first_seen_at=now, + last_seen_at=now, + count=len(invalid), + data={"invalid_skills": invalid}, + )] + + +def _rule_assignee_profile_not_found(task, events, runs, now, cfg) -> list[Diagnostic]: + assignee = _task_field(task, "assignee") + status = _task_field(task, "status") + if not assignee or status not in ("triage", "todo", "ready", "running", "blocked"): + return [] + exists = _profile_exists_safe(assignee) + if exists is not False: + return [] + running = status == "running" + actions = [ + DiagnosticAction( + kind="cli_hint", + label=f"Create profile: hermes profile create {assignee}", + payload={"command": f"hermes profile create {assignee}"}, + ), + ] + actions.extend(_generic_recovery_actions(task, running=running)) + if actions: + actions[-1].suggested = True + return [Diagnostic( + kind="assignee_profile_not_found", + severity="error", + title="Assigned profile does not exist", + detail=( + f"This task is assigned to profile '{assignee}', but Hermes " + "cannot resolve that profile on disk. Dispatcher-driven spawn " + "will skip or fail until the profile is created or the task is " + "reassigned." + ), + actions=actions, + first_seen_at=now, + last_seen_at=now, + count=1, + data={"assignee": assignee}, + )] + + +def _rule_stale_running_claim(task, events, runs, now, cfg) -> list[Diagnostic]: + if _task_field(task, "status") != "running": + return [] + claim_expires = _task_field(task, "claim_expires") + if claim_expires is None or int(claim_expires) >= now: + return [] + task_id = _task_field(task, "id") or "TASK_ID" + age_seconds = max(0, now - int(claim_expires)) + return [Diagnostic( + kind="stale_running_claim", + severity="critical", + title="Running task has an expired claim", + detail=( + "This task is still marked running, but its claim TTL has already " + "expired. The dispatcher normally reclaims expired claims on the " + "next tick; if it remains stuck, reclaim it manually and inspect " + "the worker log before retrying." + ), + actions=[ + DiagnosticAction( + kind="reclaim", + label="Reclaim task", + payload={}, + suggested=True, + ), + DiagnosticAction( + kind="cli_hint", + label=f"Check worker log: hermes kanban log {task_id}", + payload={"command": f"hermes kanban log {task_id}"}, + ), + DiagnosticAction( + kind="reassign", + label="Reassign to different profile", + payload={"reclaim_first": True}, + ), + ], + first_seen_at=int(claim_expires), + last_seen_at=now, + count=1, + data={ + "claim_expires": int(claim_expires), + "age_seconds": age_seconds, + "worker_pid": _task_field(task, "worker_pid"), + "current_run_id": _task_field(task, "current_run_id"), + }, + )] + + # Registry — order matters: rules higher on the list render first when # severity ties. Add new rules here. _RULES: list[RuleFn] = [ + _rule_stale_running_claim, + _rule_invalid_task_skills, + _rule_assignee_profile_not_found, _rule_hallucinated_cards, _rule_prose_phantom_refs, _rule_repeated_failures, @@ -584,6 +746,9 @@ def _rule_stuck_in_blocked(task, events, runs, now, cfg) -> list[Diagnostic]: # Known kinds (for the UI's filter / legend / i18n keys). Update when # rules are added. DIAGNOSTIC_KINDS = ( + "stale_running_claim", + "invalid_task_skills", + "assignee_profile_not_found", "hallucinated_cards", "prose_phantom_refs", "repeated_failures", diff --git a/tests/hermes_cli/test_kanban_core_functionality.py b/tests/hermes_cli/test_kanban_core_functionality.py index e660764c6d06..4132de50d249 100644 --- a/tests/hermes_cli/test_kanban_core_functionality.py +++ b/tests/hermes_cli/test_kanban_core_functionality.py @@ -1739,6 +1739,132 @@ def test_cli_edit_rejects_non_done_task(kanban_home): assert "not done" in out +def test_cli_edit_clear_skills_on_non_running_task(kanban_home): + conn = kb.connect() + try: + tid = kb.create_task(conn, title="x", assignee="worker", skills=["translation"]) + finally: + conn.close() + + out = run_slash(f"edit {tid} --clear-skills") + + assert "Edited" in out + conn = kb.connect() + try: + task = kb.get_task(conn, tid) + events = kb.list_events(conn, tid) + finally: + conn.close() + assert task.skills is None + assert events[-1].kind == "edited" + assert events[-1].payload["skills_cleared"] is True + + +def test_cli_edit_clear_skills_rejects_running_task(kanban_home): + conn = kb.connect() + try: + tid = kb.create_task(conn, title="x", assignee="worker", skills=["translation"]) + kb.claim_task(conn, tid) + finally: + conn.close() + + out = run_slash(f"edit {tid} --clear-skills") + + assert "cannot clear skills" in out + + +def test_cli_edit_clear_skills_rejects_result_fields(kanban_home): + conn = kb.connect() + try: + tid = kb.create_task(conn, title="x", assignee="worker", skills=["translation"]) + finally: + conn.close() + + out = run_slash(f"edit {tid} --clear-skills --result nope") + + assert "--clear-skills cannot be combined" in out + + +def test_cli_edit_reset_failures(kanban_home): + conn = kb.connect() + try: + tid = kb.create_task(conn, title="x", assignee="worker") + with kb.write_txn(conn): + conn.execute( + "UPDATE tasks SET consecutive_failures = 3, " + "last_failure_error = 'bad run' WHERE id = ?", + (tid,), + ) + finally: + conn.close() + + out = run_slash(f"edit {tid} --reset-failures") + + assert "Edited" in out + conn = kb.connect() + try: + task = kb.get_task(conn, tid) + events = kb.list_events(conn, tid) + finally: + conn.close() + assert task.consecutive_failures == 0 + assert task.last_failure_error is None + assert events[-1].kind == "edited" + assert events[-1].payload["failures_reset"] is True + + +def test_cli_edit_reset_failures_rejects_result_fields(kanban_home): + conn = kb.connect() + try: + tid = kb.create_task(conn, title="x", assignee="worker") + finally: + conn.close() + + out = run_slash(f"edit {tid} --reset-failures --result nope") + + assert "--reset-failures cannot be combined" in out + + +def test_cli_edit_clear_claim(kanban_home): + conn = kb.connect() + try: + tid = kb.create_task(conn, title="x", assignee="worker") + with kb.write_txn(conn): + conn.execute( + "UPDATE tasks SET status='ready', claim_lock=?, " + "claim_expires=?, worker_pid=? WHERE id=?", + ("lock-1", 1234567890, 9999, tid), + ) + finally: + conn.close() + + out = run_slash(f"edit {tid} --clear-claim") + + assert "Edited" in out + conn = kb.connect() + try: + task = kb.get_task(conn, tid) + events = kb.list_events(conn, tid) + finally: + conn.close() + assert task.claim_lock is None + assert task.claim_expires is None + assert task.worker_pid is None + assert events[-1].payload["claim_cleared"] is True + + +def test_cli_edit_clear_claim_rejects_result_fields(kanban_home): + conn = kb.connect() + try: + tid = kb.create_task(conn, title="x", assignee="worker") + finally: + conn.close() + + out = run_slash(f"edit {tid} --clear-claim --result nope") + + assert "--clear-claim cannot be combined" in out + + def test_cli_complete_bad_metadata_exits_nonzero(kanban_home): conn = kb.connect() try: @@ -2798,6 +2924,11 @@ def test_cli_create_without_skill_flag_leaves_none(kanban_home): assert task.skills is None +def test_cli_create_rejects_toolset_names_in_skills(kanban_home): + out = run_slash("create 'bad-skill' --assignee x --skill web --json") + assert "toolset names" in out + + def test_cli_show_renders_skills(kanban_home): """`hermes kanban show ` prints a skills row when present.""" out = run_slash( diff --git a/tests/hermes_cli/test_kanban_db.py b/tests/hermes_cli/test_kanban_db.py index b750139f4544..71feb8b6712f 100644 --- a/tests/hermes_cli/test_kanban_db.py +++ b/tests/hermes_cli/test_kanban_db.py @@ -80,6 +80,16 @@ def test_workspace_kind_validation(kanban_home): kb.create_task(conn, title="bad ws", workspace_kind="cloud") +def test_create_task_rejects_toolset_names_in_skills(kanban_home): + with kb.connect() as conn, pytest.raises(ValueError, match="toolset names"): + kb.create_task( + conn, + title="bad skills", + assignee="alice", + skills=["web", "browser"], + ) + + # --------------------------------------------------------------------------- # Links + dependency resolution # --------------------------------------------------------------------------- @@ -540,6 +550,92 @@ def test_dispatch_skips_nonspawnable_into_separate_bucket(kanban_home, monkeypat assert not res.spawned +def test_dispatch_skips_invalid_task_skills_and_keeps_ready( + kanban_home, all_assignees_spawnable +): + with kb.connect() as conn: + tid = kb.create_task(conn, title="bad-skills", assignee="worker") + with kb.write_txn(conn): + conn.execute( + "UPDATE tasks SET skills = ? WHERE id = ?", + ('["web", "translation"]', tid), + ) + res = kb.dispatch_once(conn, dry_run=True) + task = kb.get_task(conn, tid) + events = kb.list_events(conn, tid) + assert tid in res.skipped_invalid_skills + assert tid not in res.spawned + assert task.status == "ready" + assert [e.kind for e in events] == ["created"] + + +def test_dispatch_skips_invalid_task_skills_without_event_spam( + kanban_home, all_assignees_spawnable +): + with kb.connect() as conn: + tid = kb.create_task(conn, title="bad-skills", assignee="worker") + with kb.write_txn(conn): + conn.execute( + "UPDATE tasks SET skills = ? WHERE id = ?", + ('["web", "translation"]', tid), + ) + res = kb.dispatch_once(conn, dry_run=False) + events = kb.list_events(conn, tid) + assert tid in res.skipped_invalid_skills + assert [e.kind for e in events] == ["created"] + + +def test_reset_task_failures_clears_counter_and_emits_event(kanban_home): + with kb.connect() as conn: + tid = kb.create_task(conn, title="retrying", assignee="worker") + with kb.write_txn(conn): + conn.execute( + "UPDATE tasks SET consecutive_failures = 4, " + "last_failure_error = 'boom' WHERE id = ?", + (tid,), + ) + assert kb.reset_task_failures(conn, tid) is True + task = kb.get_task(conn, tid) + events = kb.list_events(conn, tid) + assert task.consecutive_failures == 0 + assert task.last_failure_error is None + assert events[-1].kind == "edited" + assert events[-1].payload["failures_reset"] is True + + +def test_edit_task_recovery_fields_clear_claim_on_non_running_task(kanban_home): + with kb.connect() as conn: + tid = kb.create_task(conn, title="stale", assignee="worker") + claimed = kb.claim_task(conn, tid) + assert claimed is not None + run_id = claimed.current_run_id + assert run_id is not None + with kb.write_txn(conn): + conn.execute( + "UPDATE tasks SET status = 'ready', claim_lock = ?, " + "claim_expires = ?, worker_pid = ?, last_heartbeat_at = ?, " + "current_run_id = ? WHERE id = ?", + ("lock-1", 1234567890, 9999, 1234567000, run_id, tid), + ) + assert kb.edit_task_recovery_fields(conn, tid, clear_claim=True) is True + task = kb.get_task(conn, tid) + events = kb.list_events(conn, tid) + run_row = conn.execute( + "SELECT status, outcome, ended_at FROM task_runs WHERE id = ?", + (run_id,), + ).fetchone() + assert task.claim_lock is None + assert task.claim_expires is None + assert task.worker_pid is None + assert task.last_heartbeat_at is None + assert task.current_run_id is None + assert run_row["status"] == "reclaimed" + assert run_row["outcome"] == "reclaimed" + assert run_row["ended_at"] is not None + assert events[-1].kind == "edited" + assert events[-1].payload["claim_cleared"] is True + + def test_has_spawnable_ready_false_when_only_terminal_lanes(kanban_home, monkeypatch): """``has_spawnable_ready`` returns False when every ready task is assigned to a control-plane lane — used by gateway/CLI dispatchers diff --git a/tests/hermes_cli/test_kanban_diagnostics.py b/tests/hermes_cli/test_kanban_diagnostics.py index d39695ca94d3..6d03d6f9190e 100644 --- a/tests/hermes_cli/test_kanban_diagnostics.py +++ b/tests/hermes_cli/test_kanban_diagnostics.py @@ -33,6 +33,16 @@ def kanban_home(tmp_path, monkeypatch): return home +@pytest.fixture(autouse=True) +def profiles_resolve_by_default(monkeypatch): + """Most rule tests isolate one failure mode at a time. + + Keep profile-existence diagnostics silent unless a test explicitly + overrides the helper to exercise that rule. + """ + monkeypatch.setattr(kd, "_profile_exists_safe", lambda _name: True) + + def _task(**overrides): base = { "id": "t_demo00", @@ -243,6 +253,60 @@ def test_stuck_in_blocked_silent_when_not_blocked(): assert kd.compute_task_diagnostics(task, events, [], now=9999999) == [] +def test_invalid_task_skills_fires_on_toolset_names(): + task = _task(status="ready", skills=["web", "browser", "translation"]) + diags = kd.compute_task_diagnostics(task, [], []) + assert len(diags) == 1 + d = diags[0] + assert d.kind == "invalid_task_skills" + assert d.severity == "error" + assert d.data["invalid_skills"] == ["web", "browser"] + + +def test_invalid_task_skills_silent_for_real_skill_names(): + task = _task(status="ready", skills=["translation", "github-code-review"]) + assert kd.compute_task_diagnostics(task, [], []) == [] + + +def test_missing_assignee_profile_fires(monkeypatch): + monkeypatch.setattr(kd, "_profile_exists_safe", lambda _name: False) + task = _task(status="ready", assignee="ghost") + diags = kd.compute_task_diagnostics(task, [], []) + assert len(diags) == 1 + d = diags[0] + assert d.kind == "assignee_profile_not_found" + assert d.severity == "error" + assert d.data["assignee"] == "ghost" + + +def test_missing_assignee_profile_silent_when_profile_exists(monkeypatch): + monkeypatch.setattr(kd, "_profile_exists_safe", lambda _name: True) + task = _task(status="ready", assignee="worker") + assert kd.compute_task_diagnostics(task, [], []) == [] + + +def test_stale_running_claim_fires(): + now = int(time.time()) + task = _task( + status="running", + claim_expires=now - 3600, + worker_pid=1234, + current_run_id=99, + ) + diags = kd.compute_task_diagnostics(task, [], [], now=now) + assert len(diags) == 1 + d = diags[0] + assert d.kind == "stale_running_claim" + assert d.severity == "critical" + assert d.data["age_seconds"] >= 3600 + + +def test_stale_running_claim_silent_when_claim_not_expired(): + now = int(time.time()) + task = _task(status="running", claim_expires=now + 3600) + assert kd.compute_task_diagnostics(task, [], [], now=now) == [] + + def test_repeated_crashes_surfaces_actual_error_in_title(): """The title should lead with the actual error text so operators see WHAT broke (e.g. rate-limit, auth, OOM) without opening logs. @@ -321,6 +385,20 @@ def test_diagnostics_sorted_critical_first(): assert "prose_phantom_refs" in kinds +def test_diagnostics_sorts_stale_running_claim_before_error(monkeypatch): + monkeypatch.setattr(kd, "_profile_exists_safe", lambda _name: False) + now = int(time.time()) + task = _task( + status="running", + assignee="ghost", + claim_expires=now - 10, + ) + diags = kd.compute_task_diagnostics(task, [], [], now=now) + kinds = [d.kind for d in diags] + assert kinds[0] == "stale_running_claim" + assert "assignee_profile_not_found" in kinds + + # --------------------------------------------------------------------------- # Integration — runs through real kanban_db so sqlite.Row fields work # --------------------------------------------------------------------------- diff --git a/tests/plugins/test_kanban_dashboard_plugin.py b/tests/plugins/test_kanban_dashboard_plugin.py index cb3793db02e0..bd9b81a317dd 100644 --- a/tests/plugins/test_kanban_dashboard_plugin.py +++ b/tests/plugins/test_kanban_dashboard_plugin.py @@ -18,6 +18,7 @@ from fastapi.testclient import TestClient from hermes_cli import kanban_db as kb +from hermes_cli import kanban_diagnostics as kd # --------------------------------------------------------------------------- @@ -52,6 +53,16 @@ def kanban_home(tmp_path, monkeypatch): return home +@pytest.fixture(autouse=True) +def profiles_resolve_by_default(monkeypatch): + """Keep dashboard diagnostics tests focused on their target signal. + + New profile-existence diagnostics should not implicitly pollute older + dashboard expectations unless a test explicitly opts into that path. + """ + monkeypatch.setattr(kd, "_profile_exists_safe", lambda _name: True) + + @pytest.fixture def client(kanban_home): app = FastAPI() diff --git a/tests/tools/test_kanban_tools.py b/tests/tools/test_kanban_tools.py index d0da47d0bcc2..951690c2d636 100644 --- a/tests/tools/test_kanban_tools.py +++ b/tests/tools/test_kanban_tools.py @@ -169,6 +169,18 @@ def test_show_explicit_task_id(worker_env): assert d["task"]["id"] == other +def test_create_rejects_toolset_names_in_skills(worker_env): + from tools import kanban_tools as kt + out = kt._handle_create({ + "title": "bad child", + "assignee": "test-worker", + "skills": ["web", "browser"], + }) + err = json.loads(out) + assert err.get("error") + assert "toolset names" in err["error"] + + def test_list_filters_tasks(monkeypatch, worker_env): """kanban_list gives orchestrators filtered board discovery.""" monkeypatch.delenv("HERMES_KANBAN_TASK", raising=False)