diff --git a/hermes_cli/kanban.py b/hermes_cli/kanban.py index a08cb8f9b4076..039771cb423d2 100644 --- a/hermes_cli/kanban.py +++ b/hermes_cli/kanban.py @@ -603,12 +603,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 (skills, failures, claim, result)", ) 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( @@ -621,6 +621,32 @@ 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( + "--skills", + nargs="*", + default=None, + help="Replace the task's force-loaded skills with these names " + "(recovery for unknown-skill dispatch failures). " + "`--skills []` or `--clear-skills` clears them. " + "Skill names only — paths are rejected.", + ) + p_edit.add_argument( + "--clear-skills", + action="store_true", + help="Clear the task's force-loaded skills (set to an empty list).", + ) + p_edit.add_argument( + "--reset-failures", + action="store_true", + help="Reset the consecutive-failure counter and last failure error " + "so the task becomes dispatchable again.", + ) + p_edit.add_argument( + "--clear-claim", + action="store_true", + help="Clear a stale claim lock so a stuck task can be re-dispatched. " + "Refuses to clear a live claim (use `reclaim` for a running worker).", + ) p_block = sub.add_parser("block", help="Mark one or more tasks blocked") p_block.add_argument("task_id") @@ -2238,20 +2264,72 @@ def _cmd_edit(args: argparse.Namespace) -> int: except (ValueError, json.JSONDecodeError) as exc: print(f"kanban: --metadata: {exc}", file=sys.stderr) return 2 + + skills_raw = getattr(args, "skills", None) + clear_skills = bool(getattr(args, "clear_skills", False)) + reset_failures = bool(getattr(args, "reset_failures", False)) + clear_claim = bool(getattr(args, "clear_claim", False)) + # The issue's `--skills []` convenience form behaves like --clear-skills. + if skills_raw == ["[]"]: + skills_raw = None + clear_skills = True + has_backfill = args.result is not None + has_recovery = ( + skills_raw is not None or clear_skills or reset_failures or clear_claim + ) + if not has_backfill and not has_recovery: + print( + "kanban: edit requires at least one operation " + "(--result, --skills, --clear-skills, --reset-failures, " + "or --clear-claim)", + file=sys.stderr, + ) + return 2 + + changed: list[str] = [] with kb.connect_closing() as conn: - if not kb.edit_completed_task_result( - conn, - args.task_id, - result=args.result, - summary=getattr(args, "summary", None), - metadata=metadata, - ): - print( - f"cannot edit {args.task_id} (unknown id or task is not done)", - file=sys.stderr, - ) - return 1 - print(f"Edited {args.task_id}") + if has_backfill: + if not kb.edit_completed_task_result( + conn, + args.task_id, + result=args.result, + summary=getattr(args, "summary", None), + metadata=metadata, + ): + print( + f"cannot edit {args.task_id} (unknown id or task is not done)", + file=sys.stderr, + ) + return 1 + changed.append("result") + if has_recovery: + try: + ok = kb.edit_task_recovery( + conn, + args.task_id, + skills=skills_raw, + clear_skills=clear_skills, + reset_failures=reset_failures, + clear_claim=clear_claim, + author=_profile_author(), + ) + except ValueError as exc: + # Bad input (invalid skill name, no-op) — usage error. + print(f"kanban: {exc}", file=sys.stderr) + return 2 + except RuntimeError as exc: + # Operational conflict (running task / live claim). + print(f"kanban: {exc}", file=sys.stderr) + return 1 + if not ok: + print( + f"cannot edit {args.task_id} (unknown or archived id)", + file=sys.stderr, + ) + return 1 + changed.append("recovery") + suffix = f" ({', '.join(changed)})" if changed else "" + print(f"Edited {args.task_id}{suffix}") return 0 @@ -3145,6 +3223,7 @@ def _cmd_repair(args: argparse.Namespace) -> int: `comment ` Append a comment `attach ` Attach a local file; `attachments ` to list `complete …` Mark task(s) done + `edit …` Recovery edit: `--skills`/`--clear-skills`, `--reset-failures`, `--clear-claim`, `--result` backfill `block [reason]` Mark blocked; `schedule [reason]` parks time-delay work; `unblock ` to revive `assign ` Reassign `boards list` Show all boards diff --git a/hermes_cli/kanban_db.py b/hermes_cli/kanban_db.py index c4bb7caf9482b..0c1dc961953bc 100644 --- a/hermes_cli/kanban_db.py +++ b/hermes_cli/kanban_db.py @@ -2878,6 +2878,88 @@ def _canonical_assignee(assignee: Optional[str]) -> Optional[str]: return normalize_profile_name(assignee) +def _looks_like_skill_path(name: str) -> bool: + """True when ``name`` looks like a filesystem path, not a skill name. + + ``hermes --skills`` (and the dispatcher's ``--skills`` passthrough) take + skill *names* — e.g. ``blogwatcher``, ``github-code-review``. Namespaced + hub ids like ``official/category/name`` are legitimate and preserved; + the forms below are almost always an agent dumping a filesystem path + into the skills field, which then fails worker startup with + "Unknown skill(s)". Path signals: + * leading ``./``, ``../``, ``/``, ``~/`` or a Windows backslash + * any backslash (Windows path separator) + * a ``/``-namespaced name whose leaf carries a file extension + (``sub/dir/skill.md``, ``skills/foo.yaml``) + """ + if name.startswith(("./", "../", "/", "~/", "\\")): + return True + if "\\" in name: + return True + if "/" in name: + leaf = name.rsplit("/", 1)[-1] + if leaf.casefold().endswith( + (".md", ".yaml", ".yml", ".skill", ".json", ".txt", ".toml") + ): + return True + return False + + +def _normalize_skill_names(skills: Iterable[str]) -> list[str]: + """Normalise + validate a skill-name iterable (shared by create/edit). + + Strips whitespace, drops empties, dedupes (preserving order). Refuses + 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. Refuses + toolset-name confusions (agents that confuse skills with toolsets + usually pass several at once and serial-correcting one per failure + round-trips wastes tokens, so all hits are collected and reported + together). Refuses path-like names via :func:`_looks_like_skill_path`. + + Raises :class:`ValueError` on invalid input; returns the cleaned list. + """ + cleaned: list[str] = [] + seen: set[str] = set() + # Collect all toolset-name confusions up front so the user sees the + # whole list at once. + toolset_typos: 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)" + ) + if name.casefold() in KNOWN_TOOLSET_NAMES: + toolset_typos.append(name) + continue + if _looks_like_skill_path(name): + raise ValueError( + f"skill name cannot be a path: {name!r} " + f"(pass skill names, e.g. `blogwatcher`, not filesystem paths)" + ) + if name in seen: + continue + seen.add(name) + cleaned.append(name) + if toolset_typos: + quoted = ", ".join(repr(n) for n in toolset_typos) + noun = "is a toolset name" if len(toolset_typos) == 1 else "are toolset names" + raise ValueError( + f"{quoted} {noun}, not skill name(s). " + "Put toolsets in the assignee profile's `toolsets:` config " + "instead of per-task skills. Skills are named skill bundles " + "(e.g. `blogwatcher`, `github-code-review`); toolsets are runtime " + "capabilities (e.g. `web`, `browser`, `terminal`)." + ) + return cleaned + + def create_task( conn: sqlite3.Connection, *, @@ -3071,50 +3153,15 @@ 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. + # Normalise + validate skills (shared with edit_task_recovery): 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), refuse toolset-name confusions, and + # refuse path-like names (``hermes --skills`` takes skill names, not + # filesystem paths). skills_list: Optional[list[str]] = None if skills is not None: - cleaned: list[str] = [] - seen: set[str] = set() - # Collect all toolset-name confusions up front so the user sees the - # whole list at once. Raising on the first hit is friendly when the - # input has one mistake, but agents that confuse skills with toolsets - # usually pass several at once (`skills=["web", "browser", "terminal"]`) - # and serial-correcting one per failure round-trips wastes tokens. - toolset_typos: 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)" - ) - if name.casefold() in KNOWN_TOOLSET_NAMES: - toolset_typos.append(name) - continue - if name in seen: - continue - seen.add(name) - cleaned.append(name) - if toolset_typos: - quoted = ", ".join(repr(n) for n in toolset_typos) - noun = "is a toolset name" if len(toolset_typos) == 1 else "are toolset names" - raise ValueError( - f"{quoted} {noun}, not skill name(s). " - "Put toolsets in the assignee profile's `toolsets:` config " - "instead of per-task skills. Skills are named skill bundles " - "(e.g. `blogwatcher`, `github-code-review`); toolsets are runtime " - "capabilities (e.g. `web`, `browser`, `terminal`)." - ) - skills_list = cleaned + skills_list = _normalize_skill_names(skills) # Idempotency check — return the existing task instead of creating a # duplicate. Done BEFORE entering write_txn to keep the fast path fast @@ -5615,6 +5662,153 @@ def edit_completed_task_result( return True +def edit_task_recovery( + conn: sqlite3.Connection, + task_id: str, + *, + skills: Optional[Iterable[str]] = None, + clear_skills: bool = False, + reset_failures: bool = False, + clear_claim: bool = False, + author: str = "operator", +) -> bool: + """Operator recovery edits on a task (upstream issue #22925). + + Covers the post-dispatch recovery slice that previously required raw + SQLite: replacing/clearing the task's force-loaded skills (the + "Unknown skill(s)" dispatch-failure class), resetting the + consecutive-failure counter + last failure error, and clearing a stale + claim lock so a stuck task becomes dispatchable again. Unlike + :func:`edit_completed_task_result` (which is restricted to ``done`` + tasks), recovery edits apply to any non-archived task. + + ``skills`` replaces the stored skills list (``[]`` or ``clear_skills`` + stores an explicit empty list = no extra skills, distinct from NULL = + defaults). Skill names are validated via + :func:`_normalize_skill_names` — commas, toolset names, and path-like + names are rejected with :class:`ValueError`. + + Safety invariants: + + * **Active-claim guard** — skills/failure edits are refused with + :class:`RuntimeError` while the task is actively claimed/running + (``status = 'running'`` and ``claim_lock`` set), mirroring + :func:`assign_task`. A mid-flight worker must not have its payload + mutated underneath it. + * **Live-claim guard** — ``clear_claim`` refuses to clear a *live* + claim (``claim_expires`` still in the future). A genuinely running + worker is aborted via :func:`reclaim_task` (``hermes kanban + reclaim``); ``clear_claim`` is only for stale/expired locks. + Clearing a stale claim returns the task to ``ready`` and closes the + dangling run as ``reclaimed`` so the runs invariant holds. + * **Audit trail** — every applied edit records a ``edited`` event + (with the changed fields) and a human-readable comment under + ``author``. + + Returns True when the task exists (and is not archived) and at least + one operation was applied; False for unknown/archived ids. Raises + ValueError when no operation is requested or a skill name is invalid. + """ + if skills is not None and clear_skills: + raise ValueError("pass either skills= or clear_skills=True, not both") + if not (skills is not None or clear_skills or reset_failures or clear_claim): + raise ValueError( + "no recovery operation requested " + "(pass skills=, clear_skills=True, reset_failures=True, " + "and/or clear_claim=True)" + ) + + skills_list: Optional[list[str]] = None + if clear_skills: + skills_list = [] + elif skills is not None: + skills_list = _normalize_skill_names(skills) + + now = int(time.time()) + with write_txn(conn): + row = conn.execute( + "SELECT status, claim_lock, claim_expires FROM tasks WHERE id = ?", + (task_id,), + ).fetchone() + if not row or row["status"] == "archived": + return False + + active_claim = row["claim_lock"] is not None and row["status"] == "running" + if active_claim and (skills_list is not None or reset_failures): + raise RuntimeError( + f"cannot edit {task_id}: currently running (claimed). " + "Wait for completion or reclaim the stale lock first " + "(`hermes kanban reclaim`)." + ) + live_claim = ( + active_claim + and row["claim_expires"] is not None + and int(row["claim_expires"]) >= now + ) + if clear_claim and live_claim: + raise RuntimeError( + f"cannot clear claim on {task_id}: claim is live (worker still " + "within its TTL). Use `hermes kanban reclaim` to abort a " + "running worker instead." + ) + + fields: list[str] = [] + if skills_list is not None: + conn.execute( + "UPDATE tasks SET skills = ? WHERE id = ?", + (json.dumps(skills_list, ensure_ascii=False), task_id), + ) + fields.append("skills") + if reset_failures: + conn.execute( + "UPDATE tasks SET consecutive_failures = 0, " + "last_failure_error = NULL WHERE id = ?", + (task_id,), + ) + fields.append("failures") + run_id = None + if clear_claim: + if row["claim_lock"] is not None: + if row["status"] == "running": + conn.execute( + "UPDATE tasks SET status = 'ready', claim_lock = NULL, " + "claim_expires = NULL, worker_pid = NULL WHERE id = ?", + (task_id,), + ) + run_id = _end_run( + conn, task_id, + outcome="reclaimed", status="reclaimed", + error=f"stale_claim_cleared lock={row['claim_lock']}", + ) + else: + conn.execute( + "UPDATE tasks SET claim_lock = NULL, claim_expires = NULL, " + "worker_pid = NULL WHERE id = ?", + (task_id,), + ) + fields.append("claim") + else: + # Idempotent no-op — record it so the audit trail shows the + # operator asked, and nothing was there to clear. + fields.append("claim(noop)") + + _append_event( + conn, task_id, "edited", + { + "fields": fields, + "skills": skills_list, + "author": author, + }, + run_id=run_id, + ) + if fields: + add_comment( + conn, task_id, author, + f"RECOVERY EDIT: {', '.join(fields)}", + ) + return True + + def block_task( conn: sqlite3.Connection, task_id: str, diff --git a/plugins/kanban/dashboard/dist/index.js b/plugins/kanban/dashboard/dist/index.js index 959e4e6d6da25..e8ff9ee68eebf 100644 --- a/plugins/kanban/dashboard/dist/index.js +++ b/plugins/kanban/dashboard/dist/index.js @@ -1531,6 +1531,159 @@ ); } + // ------------------------------------------------------------------------- + // Skill recovery control — operator fix for blocked/crashed tasks + // (issue #22925 dashboard slice). Lets the operator replace or clear the + // task's force-loaded skills, reset the consecutive-failure streak, or + // clear a stale claim, then retry — backed by POST /tasks/:id/recovery, + // which maps 1:1 to kanban_db.edit_task_recovery (never direct UI SQLite). + // + // The control is disabled while the task is claimed by a live worker + // (status === "running"): mutating a running task's payload is refused by + // the kernel (409), so the UI shows why instead of letting the operator + // click into a guaranteed rejection. + // ------------------------------------------------------------------------- + + function SkillRecoveryControl(props) { + const { t } = useI18n(); + const task = props.task; + const running = task.status === "running"; + const [skillsText, setSkillsText] = useState((task.skills || []).join(", ")); + const [busy, setBusy] = useState(false); + const [msg, setMsg] = useState(null); + + // Only render when there's actually something to recover: a stopped + // task (blocked/scheduled), a failure streak, force-loaded skills, or + // a claim to clear. Terminal/done cards and clean ready cards stay + // visually clean — matching the DiagnosticsSection's collapse rule. + const failures = task.consecutive_failures || 0; + const hasSkills = !!(task.skills && task.skills.length); + const hasClaim = !!task.claim_lock; + const recoverable = task.status !== "done" && task.status !== "archived" && ( + task.status === "blocked" || task.status === "scheduled" || + failures > 0 || hasSkills || hasClaim + ); + if (!recoverable) return null; + + const parseSkills = function () { + return skillsText + .split(",") + .map(function (s) { return s.trim(); }) + .filter(function (s) { return s.length > 0; }); + }; + + const runRecovery = function (body, retry) { + if (busy) return; + setBusy(true); setMsg(null); + const url = withBoard(`${API}/tasks/${encodeURIComponent(task.id)}/recovery`, props.boardSlug); + SDK.fetchJSON(url, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(body), + }).then(function (res) { + if (retry && task.status === "blocked") { + // Safe save-and-retry: recovery edit applied, then unblock to + // ready so the dispatcher picks the task up again on the next + // tick. Unblocking is the existing PATCH status=ready path. + return SDK.fetchJSON(withBoard(`${API}/tasks/${encodeURIComponent(task.id)}`, props.boardSlug), { + method: "PATCH", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ status: "ready" }), + }).then(function () { + setMsg({ ok: true, text: tx(t, "recoverySavedRetried", + "Skills saved and task unblocked — ready for the next dispatch tick.") }); + if (props.onRefresh) props.onRefresh(); + }); + } + setMsg({ ok: true, text: tx(t, "recoverySaved", + "Recovery edit saved.") }); + if (props.onRefresh) props.onRefresh(); + }).catch(function (err) { + setMsg({ ok: false, text: tx(t, "recoveryFailed", + "Recovery failed: ") + parseApiErrorMessage(err) }); + }).then(function () { setBusy(false); }); + }; + + const saveSkills = function (retry) { + const skills = parseSkills(); + if (!skills.length && !retry) { + setMsg({ ok: false, text: tx(t, "recoveryNoSkills", + "Enter at least one skill name, or use Clear skills.") }); + return; + } + const body = skills.length ? { skills: skills } : { clear_skills: true }; + runRecovery(body, retry); + }; + + const actionBtn = function (label, onClick, opts) { + return h("button", { + type: "button", + className: cn( + "hermes-kanban-recovery-btn", + opts && opts.suggested ? "hermes-kanban-recovery-btn--suggested" : "", + ), + disabled: busy || running, + onClick: onClick, + }, label); + }; + + return h("div", { className: "hermes-kanban-section" }, + h("div", { className: "hermes-kanban-section-head-row" }, + h("span", { className: "hermes-kanban-section-head" }, + tx(t, "recoveryTitle", "Recovery")), + ), + h("div", { className: "hermes-kanban-recovery" }, + h("div", { className: "hermes-kanban-recovery-hint" }, + tx(t, "recoveryHint", + "Fix the task's force-loaded skills, reset its failure streak, or clear a stale claim, then retry.")), + running + ? h("div", { className: "hermes-kanban-recovery-hint hermes-kanban-recovery-running" }, + tx(t, "recoveryRunningHint", + "Task is claimed by a live worker. Stop or reclaim the worker before editing its payload.")) + : h("div", { className: "hermes-kanban-recovery-section" }, + h("label", { className: "hermes-kanban-recovery-label" }, + tx(t, "skillsLabel", "Skills (comma-separated)")), + h("input", { + className: "hermes-kanban-recovery-input", + value: skillsText, + disabled: busy, + placeholder: tx(t, "skillsPlaceholder", "skill-a, skill-b"), + onChange: function (e) { setSkillsText(e.target.value); }, + }), + h("div", { className: "hermes-kanban-recovery-action-row" }, + actionBtn( + tx(t, "saveAndRetry", "Save & retry"), + function () { saveSkills(true); }, + { suggested: true }, + ), + actionBtn(tx(t, "saveSkills", "Save skills"), + function () { saveSkills(false); }), + hasSkills + ? actionBtn(tx(t, "clearSkills", "Clear skills"), + function () { runRecovery({ clear_skills: true }); }) + : null, + failures > 0 + ? actionBtn(tx(t, "resetFailures", "Reset failures"), + function () { runRecovery({ reset_failures: true }); }) + : null, + hasClaim && !running + ? actionBtn(tx(t, "clearClaim", "Clear stale claim"), + function () { runRecovery({ clear_claim: true }); }) + : null, + ), + ), + msg + ? h("div", { + className: cn( + "hermes-kanban-recovery-msg", + msg.ok ? "hermes-kanban-recovery-msg--ok" : "hermes-kanban-recovery-msg--err", + ), + }, msg.text) + : null, + ), + ); + } + // ------------------------------------------------------------------------- // Board switcher (multi-project) // ------------------------------------------------------------------------- @@ -3610,6 +3763,11 @@ diagnostics: t.diagnostics || [], onRefresh: props.onRefresh, }), + h(SkillRecoveryControl, { + task: t, + boardSlug: props.boardSlug, + onRefresh: props.onRefresh, + }), h(HomeSubsSection, { homeChannels: props.homeChannels || [], homeBusy: props.homeBusy || {}, diff --git a/plugins/kanban/dashboard/dist/style.css b/plugins/kanban/dashboard/dist/style.css index 352f55525b2a7..f3b20540369fb 100644 --- a/plugins/kanban/dashboard/dist/style.css +++ b/plugins/kanban/dashboard/dist/style.css @@ -1282,6 +1282,19 @@ .hermes-kanban-recovery-btn:hover:not(:disabled) { background: rgba(255, 158, 59, 0.25); } +.hermes-kanban-recovery-btn--suggested { + background: rgba(255, 158, 59, 0.18); + border-color: rgba(255, 158, 59, 0.5); + font-weight: 600; +} +.hermes-kanban-recovery-btn--suggested:hover:not(:disabled) { + background: rgba(255, 158, 59, 0.28); +} +.hermes-kanban-recovery-running { + color: #ff8b6b; + border-left: 2px solid rgba(255, 107, 61, 0.5); + padding-left: 0.5rem; +} .hermes-kanban-recovery-btn:disabled { opacity: 0.4; cursor: not-allowed; diff --git a/plugins/kanban/dashboard/plugin_api.py b/plugins/kanban/dashboard/plugin_api.py index fdc49da34f135..951d362c1563c 100644 --- a/plugins/kanban/dashboard/plugin_api.py +++ b/plugins/kanban/dashboard/plugin_api.py @@ -1777,6 +1777,82 @@ def reassign_task_endpoint( conn.close() +class RecoveryBody(BaseModel): + """Recovery edit on a stopped task (issue #22925). + + Maps 1:1 to ``kanban_db.edit_task_recovery`` — the same kernel path the + CLI uses for ``hermes kanban edit --skills/--clear-skills/ + --reset-failures/--clear-claim``, so the three surfaces cannot drift. + Never direct UI SQLite. + + ``skills`` replaces the stored force-loaded skill list; ``clear_skills`` + stores an explicit empty list (distinct from NULL = defaults). Both + together (or neither, with nothing else) is a 400. + """ + + skills: Optional[list[str]] = None + clear_skills: bool = False + reset_failures: bool = False + clear_claim: bool = False + + +@router.post("/tasks/{task_id}/recovery") +def recovery_task_endpoint( + task_id: str, + payload: RecoveryBody, + board: Optional[str] = Query(None), +): + """Operator recovery edit on a stopped/blocked/crashed task. + + Replaces or clears the task's force-loaded skills, resets the + consecutive-failure counter, and/or clears a stale claim — through + :func:`kanban_db.edit_task_recovery`, never direct SQL. + + Guards (mirroring the kernel's safety invariants): + + * **409** — the task is actively claimed/running (a live worker's + payload must not be mutated underneath it), or ``clear_claim`` is + pointed at a *live* claim (use ``POST /tasks/:id/reclaim`` to abort + a genuinely running worker). + * **400** — invalid skill names (path-like / toolset+comma), a no-op + request, or ``skills`` + ``clear_skills`` together. + * **404** — unknown or archived task id. + + Returns the updated task so the drawer can refresh without a second + round-trip. + """ + board = _resolve_board(board) + conn = _conn(board=board) + try: + try: + ok = kanban_db.edit_task_recovery( + conn, + task_id, + skills=payload.skills, + clear_skills=payload.clear_skills, + reset_failures=payload.reset_failures, + clear_claim=payload.clear_claim, + author="dashboard", + ) + except ValueError as exc: + raise HTTPException(status_code=400, detail=str(exc)) + except RuntimeError as exc: + raise HTTPException(status_code=409, detail=str(exc)) + if not ok: + raise HTTPException( + status_code=404, + detail=f"task {task_id} not found or archived", + ) + updated = kanban_db.get_task(conn, task_id) + return { + "ok": True, + "task_id": task_id, + "task": _task_dict(updated) if updated else None, + } + finally: + conn.close() + + # --------------------------------------------------------------------------- # Estimate — a rough token/complexity estimate for a task via the auxiliary # (auto-routed) model. NOT a dollar cost: providers don't report cost diff --git a/tests/hermes_cli/test_kanban_task_skill_recovery.py b/tests/hermes_cli/test_kanban_task_skill_recovery.py new file mode 100644 index 0000000000000..ec27ffbd60bad --- /dev/null +++ b/tests/hermes_cli/test_kanban_task_skill_recovery.py @@ -0,0 +1,489 @@ +"""Kanban task-skill recovery: kernel API + CLI surface (issue #22925). + +Covers the post-dispatch recovery slice that upstream issue #22925 asks +for — changing/clearing task skills after creation, resetting the +consecutive-failure state, clearing stale claims (with a live-claim +guard), and rejecting path-like skill input — while preserving the +existing completed-task result backfill behavior. + +Kernel tests exercise ``kanban_db.edit_task_recovery``; CLI tests drive +``hermes kanban edit`` through the same ``kanban_command`` / ``run_slash`` +entry points the CLI and gateway use. +""" + +from __future__ import annotations + +import argparse +import json +import re +import time +from pathlib import Path + +import pytest + +from hermes_cli import kanban as kc +from hermes_cli import kanban_db as kb + + +@pytest.fixture +def kanban_home(tmp_path, monkeypatch): + """Isolated HERMES_HOME with an empty kanban DB.""" + home = tmp_path / ".hermes" + home.mkdir() + monkeypatch.setenv("HERMES_HOME", str(home)) + monkeypatch.setattr(Path, "home", lambda: tmp_path) + kb.init_db() + return home + + +def _create_task(conn, *, title="recovery task", skills=None, assignee="alice"): + return kb.create_task( + conn, title=title, assignee=assignee, skills=skills, + ) + + +def _force_failures(conn, task_id, count: int = 3, error: str = "boom"): + conn.execute( + "UPDATE tasks SET consecutive_failures = ?, last_failure_error = ? " + "WHERE id = ?", + (count, error, task_id), + ) + conn.commit() + + +def _claim(conn, task_id): + assert kb.claim_task(conn, task_id) is not None + return conn.execute( + "SELECT status, claim_lock, claim_expires FROM tasks WHERE id = ?", + (task_id,), + ).fetchone() + + +def _build_kanban_parser(): + parser = argparse.ArgumentParser(prog="hermes", add_help=False) + sub = parser.add_subparsers(dest="command") + kc.build_parser(sub) + return parser + + +# --------------------------------------------------------------------------- +# Kernel: changing / clearing task skills after creation +# --------------------------------------------------------------------------- + + +def test_kernel_replace_skills_after_creation(kanban_home): + with kb.connect() as conn: + tid = _create_task(conn, skills=["alpha"]) + assert kb.get_task(conn, tid).skills == ["alpha"] + assert kb.edit_task_recovery(conn, tid, skills=["beta", "gamma"]) + task = kb.get_task(conn, tid) + assert task.skills == ["beta", "gamma"] + + +def test_kernel_clear_skills(kanban_home): + with kb.connect() as conn: + tid = _create_task(conn, skills=["alpha", "bogus"]) + assert kb.edit_task_recovery(conn, tid, clear_skills=True) + task = kb.get_task(conn, tid) + # Empty list (explicitly no extra skills), not None (defaults). + assert task.skills == [] + + +def test_kernel_skills_edit_records_audit_event_and_comment(kanban_home): + with kb.connect() as conn: + tid = _create_task(conn, skills=["alpha"]) + kb.edit_task_recovery(conn, tid, skills=["beta"], author="ops") + events = kb.list_events(conn, tid) + assert any(e.kind == "edited" for e in events) + edited = next(e for e in events if e.kind == "edited") + assert "skills" in edited.payload["fields"] + comments = kb.list_comments(conn, tid) + assert any(c.author == "ops" and "skill" in c.body.lower() for c in comments) + + +# --------------------------------------------------------------------------- +# Kernel: resetting failure state +# --------------------------------------------------------------------------- + + +def test_kernel_reset_failures(kanban_home): + with kb.connect() as conn: + tid = _create_task(conn) + _force_failures(conn, tid, count=3, error="spawn boom") + task = kb.get_task(conn, tid) + assert task.consecutive_failures == 3 + assert task.last_failure_error == "spawn boom" + assert kb.edit_task_recovery(conn, tid, reset_failures=True) + task = kb.get_task(conn, tid) + assert task.consecutive_failures == 0 + assert task.last_failure_error is None + + +# --------------------------------------------------------------------------- +# Kernel: guarding an actively claimed / running task +# --------------------------------------------------------------------------- + + +def test_kernel_skills_edit_refuses_actively_claimed_running_task(kanban_home): + with kb.connect() as conn: + tid = _create_task(conn, skills=["alpha"]) + row = _claim(conn, tid) + assert row["status"] == "running" + assert row["claim_lock"] is not None + with pytest.raises(RuntimeError, match="running"): + kb.edit_task_recovery(conn, tid, skills=["beta"]) + with pytest.raises(RuntimeError, match="running"): + kb.edit_task_recovery(conn, tid, clear_skills=True) + assert kb.get_task(conn, tid).skills == ["alpha"] + + +def test_kernel_reset_failures_refuses_actively_claimed_running_task(kanban_home): + with kb.connect() as conn: + tid = _create_task(conn) + _force_failures(conn, tid, count=3) + _claim(conn, tid) + with pytest.raises(RuntimeError, match="running"): + kb.edit_task_recovery(conn, tid, reset_failures=True) + assert kb.get_task(conn, tid).consecutive_failures == 3 + + +def test_kernel_clear_claim_refuses_live_claim(kanban_home): + with kb.connect() as conn: + tid = _create_task(conn) + row = _claim(conn, tid) + # Live claim: not expired. + assert row["claim_expires"] > int(time.time()) + with pytest.raises(RuntimeError, match="reclaim"): + kb.edit_task_recovery(conn, tid, clear_claim=True) + + +def test_kernel_clear_claim_clears_stale_claim(kanban_home): + with kb.connect() as conn: + tid = _create_task(conn) + _claim(conn, tid) + # Backdate the claim so it is stale (TTL passed). + conn.execute( + "UPDATE tasks SET claim_expires = ? WHERE id = ?", + (int(time.time()) - 60, tid), + ) + conn.commit() + assert kb.edit_task_recovery(conn, tid, clear_claim=True) + task = kb.get_task(conn, tid) + assert task.status == "ready" + assert task.claim_lock is None + assert task.claim_expires is None + assert task.worker_pid is None + + +# --------------------------------------------------------------------------- +# Kernel: rejecting / normalizing path-like skill input +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "bad", + [ + "./foo", + "../foo", + "/abs/path/foo", + "~/foo", + "sub/dir/skill.md", + r"C:\skills\foo", + "skills/foo.yaml", + ], +) +def test_kernel_rejects_path_like_skill_input(kanban_home, bad): + with kb.connect() as conn: + tid = _create_task(conn, skills=["alpha"]) + with pytest.raises(ValueError, match="path"): + kb.edit_task_recovery(conn, tid, skills=[bad]) + # Nothing changed. + assert kb.get_task(conn, tid).skills == ["alpha"] + + +def test_kernel_rejects_toolset_and_comma_skill_input(kanban_home): + with kb.connect() as conn: + tid = _create_task(conn, skills=["alpha"]) + with pytest.raises(ValueError, match="toolset"): + kb.edit_task_recovery(conn, tid, skills=["web"]) + with pytest.raises(ValueError, match="comma"): + kb.edit_task_recovery(conn, tid, skills=["blogwatcher,github"]) + assert kb.get_task(conn, tid).skills == ["alpha"] + + +# --------------------------------------------------------------------------- +# Kernel: edge cases and board resolution +# --------------------------------------------------------------------------- + + +def test_kernel_unknown_task_returns_false(kanban_home): + with kb.connect() as conn: + assert kb.edit_task_recovery(conn, "t_deadbeef", clear_skills=True) is False + + +def test_kernel_noop_raises_valueerror(kanban_home): + with kb.connect() as conn: + tid = _create_task(conn) + with pytest.raises(ValueError, match="operation"): + kb.edit_task_recovery(conn, tid) + + +def test_kernel_archived_task_returns_false(kanban_home): + with kb.connect() as conn: + tid = _create_task(conn) + assert kb.archive_task(conn, tid) + assert kb.edit_task_recovery(conn, tid, clear_skills=True) is False + + +def test_kernel_works_on_named_board(kanban_home): + kb.create_board("beta") + with kb.connect_closing(board="beta") as conn: + tid = _create_task(conn, skills=["alpha"]) + assert kb.edit_task_recovery(conn, tid, skills=["gamma"]) + assert kb.get_task(conn, tid).skills == ["gamma"] + # Default board untouched. + with kb.connect() as conn: + assert kb.list_tasks(conn, limit=100) == [] + + +def test_completed_result_backfill_preserved(kanban_home): + """edit_completed_task_result still backfills done tasks (regression).""" + with kb.connect() as conn: + tid = _create_task(conn) + assert kb.complete_task(conn, tid, result="old result") + assert kb.edit_completed_task_result( + conn, tid, result="new result", summary="new summary", + ) + assert kb.get_task(conn, tid).result == "new result" + + +# --------------------------------------------------------------------------- +# CLI: hermes kanban edit --skills / --clear-skills / --reset-failures +# --------------------------------------------------------------------------- + + +def _create_via_slash(conn, **kwargs): + out = kc.run_slash("create 'recovery task' --assignee alice") + m = re.search(r"(t_[a-f0-9]+)", out) + assert m, out + return m.group(1) + + +def test_cli_edit_skills_replace_after_creation(kanban_home): + with kb.connect() as conn: + tid = _create_task(conn, skills=["alpha"]) + out = kc.run_slash(f"edit {tid} --skills beta gamma") + assert "Edited" in out, out + with kb.connect() as conn: + assert kb.get_task(conn, tid).skills == ["beta", "gamma"] + + +def test_cli_edit_clear_skills(kanban_home): + with kb.connect() as conn: + tid = _create_task(conn, skills=["alpha", "bogus"]) + out = kc.run_slash(f"edit {tid} --clear-skills") + assert "Edited" in out, out + with kb.connect() as conn: + assert kb.get_task(conn, tid).skills == [] + + +def test_cli_edit_empty_array_syntax_clears_skills(kanban_home): + """The issue's `--skills []` form behaves like --clear-skills.""" + with kb.connect() as conn: + tid = _create_task(conn, skills=["alpha"]) + out = kc.run_slash(f"edit {tid} --skills []") + assert "Edited" in out, out + with kb.connect() as conn: + assert kb.get_task(conn, tid).skills == [] + + +def test_cli_edit_reset_failures(kanban_home): + with kb.connect() as conn: + tid = _create_task(conn) + _force_failures(conn, tid, count=3, error="spawn boom") + out = kc.run_slash(f"edit {tid} --reset-failures") + assert "Edited" in out, out + with kb.connect() as conn: + task = kb.get_task(conn, tid) + assert task.consecutive_failures == 0 + assert task.last_failure_error is None + + +def test_cli_edit_guards_actively_claimed_running_task(kanban_home): + with kb.connect() as conn: + tid = _create_task(conn, skills=["alpha"]) + _claim(conn, tid) + out = kc.run_slash(f"edit {tid} --skills beta") + assert "running" in out.lower(), out + with kb.connect() as conn: + assert kb.get_task(conn, tid).skills == ["alpha"] + + +def test_cli_edit_rejects_path_like_skill(kanban_home): + with kb.connect() as conn: + tid = _create_task(conn, skills=["alpha"]) + parser = _build_kanban_parser() + args = parser.parse_args(["kanban", "edit", tid, "--skills", "./foo"]) + rc = kc.kanban_command(args) + assert rc == 2 + with kb.connect() as conn: + assert kb.get_task(conn, tid).skills == ["alpha"] + + +def test_cli_edit_requires_at_least_one_operation(kanban_home): + with kb.connect() as conn: + tid = _create_task(conn) + parser = _build_kanban_parser() + args = parser.parse_args(["kanban", "edit", tid]) + rc = kc.kanban_command(args) + assert rc == 2 + + +def test_cli_edit_clear_stale_claim(kanban_home): + with kb.connect() as conn: + tid = _create_task(conn) + _claim(conn, tid) + conn.execute( + "UPDATE tasks SET claim_expires = ? WHERE id = ?", + (int(time.time()) - 60, tid), + ) + conn.commit() + out = kc.run_slash(f"edit {tid} --clear-claim") + assert "Edited" in out, out + with kb.connect() as conn: + task = kb.get_task(conn, tid) + assert task.status == "ready" + assert task.claim_lock is None + + +def test_cli_edit_works_on_named_board(kanban_home): + kb.create_board("beta") + with kb.connect_closing(board="beta") as conn: + tid = _create_task(conn, skills=["alpha"]) + parser = _build_kanban_parser() + args = parser.parse_args(["kanban", "--board", "beta", "edit", tid, "--skills", "gamma"]) + rc = kc.kanban_command(args) + assert rc == 0 + with kb.connect_closing(board="beta") as conn: + assert kb.get_task(conn, tid).skills == ["gamma"] + with kb.connect() as conn: + assert kb.list_tasks(conn, limit=100) == [] + + +def test_cli_edit_result_backfill_unchanged(kanban_home): + """`--result` backfill on done tasks keeps working alongside recovery.""" + with kb.connect() as conn: + tid = _create_task(conn, skills=["alpha"]) + assert kb.complete_task(conn, tid, result="old") + parser = _build_kanban_parser() + args = parser.parse_args( + ["kanban", "edit", tid, "--result", "new result", "--skills", "beta"] + ) + rc = kc.kanban_command(args) + assert rc == 0 + with kb.connect() as conn: + task = kb.get_task(conn, tid) + assert task.result == "new result" + assert task.skills == ["beta"] + + +# --------------------------------------------------------------------------- +# E2E: the full recovery loop — blocked dispatch failure → edit → retry +# (issue #22925: create with wrong skills → dispatch fails → task blocked +# with a failure streak → operator repairs skills + resets failures through +# the supported surface → unblock → dispatchable again, audit trail intact) +# --------------------------------------------------------------------------- + + +def _simulate_dispatch_failure(conn, task_id, *, error="Unknown skill(s)"): + """Leave the task in the state the dispatcher produces after a spawn + failure on wrong force-loaded skills: blocked with a failure streak.""" + assert kb.block_task(conn, task_id, reason=error) + _force_failures(conn, task_id, count=3, error=error) + return kb.get_task(conn, task_id) + + +def test_e2e_blocked_dispatch_failure_replace_skills_then_unblock(kanban_home): + """Wrong skills → blocked → `edit --skills X --reset-failures` → + `unblock` → ready with corrected skills, zeroed failures, audit trail.""" + with kb.connect() as conn: + tid = _create_task(conn, skills=["bogus-skill"], assignee="alice") + task = _simulate_dispatch_failure(conn, tid) + assert task.status == "blocked" + assert task.skills == ["bogus-skill"] + assert task.consecutive_failures == 3 + + # Repair through the supported CLI / /kanban surface (run_slash is the + # exact entry point the gateway and interactive CLI use). + out = kc.run_slash(f"edit {tid} --skills real-skill --reset-failures") + assert "Edited" in out, out + out = kc.run_slash(f"unblock {tid}") + assert "Unblocked" in out, out + + with kb.connect() as conn: + task = kb.get_task(conn, tid) + assert task.status == "ready" # dispatchable again + assert task.skills == ["real-skill"] + assert task.consecutive_failures == 0 # breaker reset + assert task.last_failure_error is None + # Audit trail: edited event + operator comment (no direct SQL). + events = kb.list_events(conn, tid) + assert any(e.kind == "edited" for e in events) + edited = next(e for e in events if e.kind == "edited") + assert "skills" in edited.payload["fields"] + assert "failures" in edited.payload["fields"] + comments = kb.list_comments(conn, tid) + assert any("RECOVERY EDIT" in c.body for c in comments) + + +def test_e2e_blocked_dispatch_failure_clear_skills_then_unblock(kanban_home): + """The --clear-skills variant: bogus skill removed entirely, breaker + reset, unblocked to ready.""" + with kb.connect() as conn: + tid = _create_task(conn, skills=["bogus-skill"], assignee="alice") + task = _simulate_dispatch_failure(conn, tid) + assert task.status == "blocked" + + out = kc.run_slash(f"edit {tid} --clear-skills --reset-failures") + assert "Edited" in out, out + assert "Unblocked" in kc.run_slash(f"unblock {tid}"), out + + with kb.connect() as conn: + task = kb.get_task(conn, tid) + assert task.status == "ready" + assert task.skills == [] # explicit empty list ≠ NULL + assert task.consecutive_failures == 0 + assert task.last_failure_error is None + assert any(e.kind == "edited" for e in kb.list_events(conn, tid)) + + +def test_e2e_full_recovery_loop_on_named_board(kanban_home): + """The complete loop (blocked → edit → unblock) resolves correctly on a + named board via `--board `, and leaves the default board alone.""" + kb.create_board("gamma") + with kb.connect_closing(board="gamma") as conn: + tid = _create_task(conn, skills=["bogus-skill"], assignee="alice") + _simulate_dispatch_failure(conn, tid) + + parser = _build_kanban_parser() + rc = kc.kanban_command( + parser.parse_args( + ["kanban", "--board", "gamma", "edit", tid, + "--skills", "real-skill", "--reset-failures"] + ) + ) + assert rc == 0 + rc = kc.kanban_command( + parser.parse_args(["kanban", "--board", "gamma", "unblock", tid]) + ) + assert rc == 0 + + with kb.connect_closing(board="gamma") as conn: + task = kb.get_task(conn, tid) + assert task.status == "ready" + assert task.skills == ["real-skill"] + assert task.consecutive_failures == 0 + assert any(e.kind == "edited" for e in kb.list_events(conn, tid)) + # Default board untouched. + with kb.connect() as conn: + assert kb.list_tasks(conn, limit=100) == [] diff --git a/tests/plugins/test_kanban_dashboard_plugin.py b/tests/plugins/test_kanban_dashboard_plugin.py index 5fdb750a385d5..f51bfb74816f1 100644 --- a/tests/plugins/test_kanban_dashboard_plugin.py +++ b/tests/plugins/test_kanban_dashboard_plugin.py @@ -704,7 +704,246 @@ def test_specify_happy_path(client, monkeypatch): # --------------------------------------------------------------------------- -# Final result visibility for Done cards +# POST /tasks/:id/recovery — operator skill/failure/claim recovery edits +# (issue #22925 dashboard slice). Maps 1:1 to kanban_db.edit_task_recovery — +# never direct UI SQLite. # --------------------------------------------------------------------------- +def _create_task_with_skills(client, skills=None, title="recovery task"): + body = {"title": title} + if skills is not None: + body["skills"] = skills + r = client.post("/api/plugins/kanban/tasks", json=body) + assert r.status_code == 200, r.text + return r.json()["task"] + + +def _force_claim(conn, task_id, *, live=True, status="running"): + """Set a claim on a task directly (bypassing claim_task) so tests can + control staleness. ``live=True`` → claim_expires in the future.""" + lock = f"lock-{task_id}" + expires = int(time.time()) + 3600 if live else int(time.time()) - 3600 + conn.execute( + "UPDATE tasks SET status=?, claim_lock=?, claim_expires=?, worker_pid=? WHERE id=?", + (status, lock, expires, 99999, task_id), + ) + conn.commit() + + +def test_recovery_endpoint_replaces_skills(client): + """POST /tasks/:id/recovery replaces the task's force-loaded skills via + the kernel function. UI contract: the response carries the updated task + so the drawer can refresh without a second round-trip.""" + t = _create_task_with_skills(client, skills=["alpha"]) + assert t["skills"] == ["alpha"] + r = client.post( + f"/api/plugins/kanban/tasks/{t['id']}/recovery", + json={"skills": ["beta", "gamma"]}, + ) + assert r.status_code == 200, r.text + body = r.json() + assert body["ok"] is True + assert body["task_id"] == t["id"] + assert body["task"]["skills"] == ["beta", "gamma"] + # Audit trail recorded through the kernel, not direct SQL. + detail = client.get(f"/api/plugins/kanban/tasks/{t['id']}").json() + assert any(e["kind"] == "edited" for e in detail["events"]) + + +def test_recovery_endpoint_clears_skills(client): + """clear_skills=True stores an explicit empty list (no extra skills), + distinct from NULL (defaults).""" + t = _create_task_with_skills(client, skills=["alpha", "bogus"]) + r = client.post( + f"/api/plugins/kanban/tasks/{t['id']}/recovery", + json={"clear_skills": True}, + ) + assert r.status_code == 200, r.text + assert r.json()["task"]["skills"] == [] + + +def test_recovery_endpoint_resets_failures(client): + """reset_failures=True zeroes consecutive_failures and clears the last + failure error so the dispatcher circuit breaker stops tripping.""" + t = _create_task_with_skills(client) + conn = kb.connect() + try: + conn.execute( + "UPDATE tasks SET consecutive_failures=3, last_failure_error='boom' WHERE id=?", + (t["id"],), + ) + conn.commit() + finally: + conn.close() + r = client.post( + f"/api/plugins/kanban/tasks/{t['id']}/recovery", + json={"reset_failures": True}, + ) + assert r.status_code == 200, r.text + task = r.json()["task"] + assert task["consecutive_failures"] == 0 + assert task["last_failure_error"] is None + + +def test_recovery_endpoint_clears_stale_claim(client): + """clear_claim=True on an expired claim returns the task to ready and + clears the lock (the kernel closes the dangling run as reclaimed).""" + t = _create_task_with_skills(client) + conn = kb.connect() + try: + _force_claim(conn, t["id"], live=False) # expired claim + finally: + conn.close() + r = client.post( + f"/api/plugins/kanban/tasks/{t['id']}/recovery", + json={"clear_claim": True}, + ) + assert r.status_code == 200, r.text + task = r.json()["task"] + assert task["status"] == "ready" + assert task["claim_lock"] is None + + +def test_recovery_endpoint_rejects_live_claimed_worker(client): + """Skills/failure edits must be refused while a worker is actively + claimed — the kernel raises RuntimeError, surfaced as 409 so the UI can + show an actionable message instead of silently mutating a live worker.""" + t = _create_task_with_skills(client) + conn = kb.connect() + try: + _force_claim(conn, t["id"], live=True) + finally: + conn.close() + r = client.post( + f"/api/plugins/kanban/tasks/{t['id']}/recovery", + json={"skills": ["beta"]}, + ) + assert r.status_code == 409, r.text + detail = r.json()["detail"].lower() + assert "running" in detail or "claim" in detail + + +def test_recovery_endpoint_rejects_clearing_live_claim(client): + """clear_claim must not release a live claim — a genuinely running + worker is aborted via /reclaim, not via the recovery edit.""" + t = _create_task_with_skills(client) + conn = kb.connect() + try: + _force_claim(conn, t["id"], live=True) + finally: + conn.close() + r = client.post( + f"/api/plugins/kanban/tasks/{t['id']}/recovery", + json={"clear_claim": True}, + ) + assert r.status_code == 409, r.text + + +def test_recovery_endpoint_validation_errors(client): + """Actionable 400s: path-like skill names, no-op requests, and the + skills+clear_skills conflict are all rejected with the kernel's + message as detail.""" + t = _create_task_with_skills(client) + # Path-like skill name rejected (kernel _normalize_skill_names). + r = client.post( + f"/api/plugins/kanban/tasks/{t['id']}/recovery", + json={"skills": ["../evil"]}, + ) + assert r.status_code == 400, r.text + # No operation requested. + r = client.post(f"/api/plugins/kanban/tasks/{t['id']}/recovery", json={}) + assert r.status_code == 400, r.text + # skills + clear_skills conflict. + r = client.post( + f"/api/plugins/kanban/tasks/{t['id']}/recovery", + json={"skills": ["beta"], "clear_skills": True}, + ) + assert r.status_code == 400, r.text + + +def test_recovery_endpoint_unknown_task(client): + r = client.post( + "/api/plugins/kanban/tasks/t_unknown/recovery", + json={"reset_failures": True}, + ) + assert r.status_code == 404, r.text + + +def test_recovery_endpoint_archived_task(client): + """Archived tasks are not recoverable — same 404 as unknown ids.""" + t = _create_task_with_skills(client) + r = client.patch(f"/api/plugins/kanban/tasks/{t['id']}", json={"status": "archived"}) + assert r.status_code == 200, r.text + r = client.post( + f"/api/plugins/kanban/tasks/{t['id']}/recovery", + json={"reset_failures": True}, + ) + assert r.status_code == 404, r.text + + +def test_recovery_save_then_retry_flow(client): + """UI contract: the drawer's 'Save & retry' chains a recovery edit then + unblocks to ready (PATCH status=ready). Both steps must succeed in + sequence on a blocked task, and the saved skills must survive.""" + t = _create_task_with_skills(client, skills=["bogus-skill"]) + # Put the task into blocked. + r = client.patch(f"/api/plugins/kanban/tasks/{t['id']}", json={"status": "blocked"}) + assert r.status_code == 200, r.text + # Recovery edit (replace bogus skill, clear the failure streak). + r = client.post( + f"/api/plugins/kanban/tasks/{t['id']}/recovery", + json={"skills": ["real-skill"], "reset_failures": True}, + ) + assert r.status_code == 200, r.text + assert r.json()["task"]["skills"] == ["real-skill"] + # …then unblock to ready so the dispatcher can retry. + r = client.patch(f"/api/plugins/kanban/tasks/{t['id']}", json={"status": "ready"}) + assert r.status_code == 200, r.text + task = r.json()["task"] + assert task["status"] == "ready" + assert task["skills"] == ["real-skill"] + + +# --------------------------------------------------------------------------- +# Recovery UI contract — the shipped dashboard bundle must expose the +# recovery control for blocked/crashed tasks (issue #22925 dashboard slice). +# Mirrors the markdown-sanitization bundle test above: read the committed +# dist bundle and pin the surface strings so a future rebuild that drops the +# control fails CI instead of silently shipping a dashboard without it. +# --------------------------------------------------------------------------- + + +def _dashboard_bundle() -> str: + repo_root = Path(__file__).resolve().parents[2] + bundle = repo_root / "plugins" / "kanban" / "dashboard" / "dist" / "index.js" + assert bundle.exists(), f"dashboard bundle missing: {bundle}" + return bundle.read_text() + + +def test_dashboard_bundle_ships_recovery_control(): + js = _dashboard_bundle() + # The control component itself is present. + assert "function SkillRecoveryControl(props)" in js + # It POSTs to the recovery endpoint backed by kanban_db.edit_task_recovery + # (never direct UI SQLite) and uses the board-scoped URL helper. + assert "/recovery" in js + assert "kanban_db.edit_task_recovery" in js or "edit_task_recovery" in js + assert "withBoard" in js + # Save-and-retry flow: recovery edit then PATCH status=ready. + assert "status: \"ready\"" in js + # Live-claim guard: the control is disabled/explained while running. + assert "hermes-kanban-recovery-running" in js + assert "claimed by a live worker" in js + + +def test_dashboard_bundle_recovery_does_not_touch_sqlite(): + js = _dashboard_bundle() + # The bundle must not reach for a SQLite client — recovery goes through + # the plugin API only. (The dashboard's other UI does the same; this pins + # the recovery slice against a future shortcut.) + assert "sqlite3" not in js + assert "better-sqlite3" not in js + assert "Database(" not in js + + diff --git a/website/docs/reference/cli-commands.md b/website/docs/reference/cli-commands.md index bd670e9fdd874..054d2669a1553 100644 --- a/website/docs/reference/cli-commands.md +++ b/website/docs/reference/cli-commands.md @@ -616,6 +616,7 @@ Multi-profile, multi-project collaboration board. Each install can host many boa | `claim ` | Atomically claim a ready task. Prints resolved workspace path. | | `comment ""` | Append a comment. The next worker that claims the task reads it as part of its `kanban_show()` response. | | `complete ` | Mark task done. Flags: `--result`, `--summary`, `--metadata`. | +| `edit ` | Recovery edit on a task. Flags: `--result`/`--summary`/`--metadata` (backfill a *done* task's result), `--skills NAME...` (replace the force-loaded skills), `--clear-skills` (store an explicit empty list; `--skills []` is an alias), `--reset-failures` (zero the consecutive-failure counter + error so the circuit breaker resets), `--clear-claim` (clear a *stale* claim and return the task to `ready`; refuses a live claim). Requires at least one flag. Refused while a task is actively claimed/running. Records an `edited` event + operator comment. | | `block ""` | Mark task blocked for human input. Also appends the reason as a comment. | | `schedule ""` | Park time-delay/follow-up work in `scheduled` so it is not shown as a human blocker. | | `unblock ` | Return a blocked or scheduled task to ready (or `todo` if dependencies are still open). | diff --git a/website/docs/user-guide/features/kanban.md b/website/docs/user-guide/features/kanban.md index 8e5859a3bed44..7b9975697c93e 100644 --- a/website/docs/user-guide/features/kanban.md +++ b/website/docs/user-guide/features/kanban.md @@ -457,6 +457,52 @@ hermes kanban create "audit auth flow" \ The dispatcher emits one `--skills ` flag per skill listed, so the worker spawns with all of them loaded on top of the auto-injected kanban guidance. The skill names must match skills that are actually installed on the assignee's profile (run `hermes skills list` to see what's available); there's no runtime install. +### Recovering a task after a dispatch failure (skills / failures / stale claim) + +Sometimes a task is created with the wrong force-loaded skills — a typo, a +renamed skill, a skill that only lives on another profile — and the worker +spawn fails. The dispatcher blocks the task and trips the per-task circuit +breaker (`consecutive_failures` + `last_failure_error`). Before the recovery +surface, the only way out was editing `~/.hermes/kanban.db` by hand. Now the +operator repairs it through the same surface that created it: + +```bash +# Replace the wrong skill and zero the failure streak, then release the task +hermes kanban edit t_abcd --skills translation --reset-failures +hermes kanban unblock t_abcd +``` + +`hermes kanban edit` (and `/kanban edit`) accepts any combination of: + +| Flag | Effect | +|------|--------| +| `--skills NAME...` | Replace the task's force-loaded skills (the "Unknown skill(s)" dispatch-failure class). Skill names only — path-like names (`./`, `../`, `/`, `~/`, backslashes), toolset names, and comma-joined strings are rejected with a usage error. Namespaced hub ids like `official/category/name` are preserved. | +| `--clear-skills` | Store an explicit empty skill list (no extra skills — distinct from unset = profile defaults). `--skills []` is an alias. | +| `--reset-failures` | Zero `consecutive_failures` and clear `last_failure_error`, so the dispatcher's circuit breaker stops tripping. | +| `--clear-claim` | Clear a **stale** claim lock (TTL expired) and return the task to `ready`, closing the dangling run as `reclaimed`. Refuses a **live** claim — a genuinely running worker is aborted with `hermes kanban reclaim`, never silently mutated underneath. | +| `--result` / `--summary` / `--metadata` | Existing backfill for a *done* task's result (unchanged). | + +Safety invariants (kernel `kanban_db.edit_task_recovery`, shared by the CLI, +`/kanban`, and the dashboard API): + +- **Live-worker mutation refused.** Skills/failure edits raise an error + while a task is actively claimed/running — a mid-flight worker's payload + is never mutated underneath it. +- **Stale-claim only.** `--clear-claim` refuses a claim whose TTL has not + expired. +- **Audit trail.** Every applied edit records an `edited` event with the + changed fields plus a `RECOVERY EDIT: …` comment under the operator's + name. No recovery path touches the SQLite DB directly. +- **Default and named boards.** `--board ` resolves exactly as it does + for every other verb; the recovery edit operates on whichever board the + flag (or current-board resolution) selects. + +The dashboard exposes the same flow in the task drawer: the **Skill +recovery** control replaces/clears skills, resets failures, and clears a +stale claim, with a **Save & retry** action (recovery edit, then unblock to +`ready`). It is disabled while a live worker is claimed. Backed by +`POST /api/plugins/kanban/tasks/:id/recovery`. + ### Per-task model override Pin a task's worker to a specific model (and optionally provider), independent of the assignee profile's default: @@ -630,6 +676,7 @@ All routes are mounted under `/api/plugins/kanban/` and protected by the dashboa | `PATCH` | `/tasks/:id` | Status / assignee / priority / title / body / result | | `POST` | `/tasks/bulk` | Apply the same patch (status / archive / assignee / priority) to every id in `ids`. Per-id failures reported without aborting siblings | | `POST` | `/tasks/:id/comments` | Append a comment | +| `POST` | `/tasks/:id/recovery` | Operator recovery edit on a stopped/blocked/crashed task — replace/clear force-loaded skills, reset `consecutive_failures`, clear a stale claim. 400 (bad input/no-op) / 404 (unknown/archived) / 409 (live worker or live claim). Maps 1:1 to `kanban_db.edit_task_recovery`, never direct SQL | | `POST` | `/tasks/:id/specify` | Run the triage specifier — auxiliary LLM fleshes out the task body and promotes it from `triage` to `todo`. Returns `{ok, task_id, reason, new_title}`; `ok=false` with a human-readable reason on "not in triage" / no aux client / LLM error is a 200, not a 4xx | | `POST` | `/tasks/:id/decompose` | Run the kanban decomposer — auxiliary LLM produces a task graph and the helper atomically creates the children + links the root + flips `triage → todo`. Returns `{ok, task_id, reason, fanout, child_ids, new_title}`. Same 200-on-LLM-error convention as `/specify`. | | `GET` | `/profiles` | List installed profiles with their descriptions (consumed by the dashboard's profile-description editor and the orchestrator picker). | @@ -707,8 +754,8 @@ hermes kanban list [--mine] [--assignee P] [--status S] [--tenant T] [--archived hermes kanban show [--json] hermes kanban assign # or 'none' to unassign hermes kanban reassign ... # bulk re-assign tasks to a profile -hermes kanban edit [--title ...] [--body ...] # edit task title / body / priority in place - [--priority N] +hermes kanban edit [--result ...] [--summary ...] [--metadata JSON] # recovery edit: skills / failures / claim / result + [--skills NAME ...] [--clear-skills] [--reset-failures] [--clear-claim] hermes kanban promote ... # move todo/blocked tasks to ready (recovery) hermes kanban schedule --at # set/clear a task's scheduled_at start time hermes kanban diagnostics [--json] # board health snapshot (alias: diag)