diff --git a/hermes_cli/kanban.py b/hermes_cli/kanban.py index 31c4bf68ae85..e0537b09e149 100644 --- a/hermes_cli/kanban.py +++ b/hermes_cli/kanban.py @@ -309,6 +309,10 @@ def build_parser(parent_subparsers: argparse._SubParsersAction) -> argparse.Argu p_create.add_argument("--assignee", default=None, help="Profile name to assign") p_create.add_argument("--parent", action="append", default=[], help="Parent task id (repeatable)") + p_create.add_argument("--depends-on", dest="parent", action="append", + help="Dependency task id — strict alias for --parent. " + "Prefer this for chain ordering; --parent is the " + "older synonym retained for umbrella-only links.") p_create.add_argument("--workspace", default="scratch", help="scratch | worktree | worktree: | dir: " "(default: scratch)") @@ -1892,8 +1896,30 @@ def _cmd_complete(args: argparse.Namespace) -> int: print(f"kanban: --metadata: {exc}", file=sys.stderr) return 2 failed: list[str] = [] + # v6.3 completion gates — same checks as the kanban_complete TOOL. + # The CLI is mostly used by humans (kaipo) and by JARVIS via subprocess, + # so we enforce the same way to keep behavior consistent across surfaces. + from tools.kanban_tools import ( + _check_reviewer_verdict_gate, + _check_evidence_paths_gate, + _check_keep_running_gate, + ) with kb.connect_closing() as conn: for tid in ids: + task = kb.get_task(conn, tid) + gate_err: Optional[str] = None + for gate in ( + _check_reviewer_verdict_gate(task, args.result), + _check_evidence_paths_gate(task), + _check_keep_running_gate(kb, conn, task), + ): + if gate: + gate_err = gate + break + if gate_err: + failed.append(tid) + print(f"cannot complete {tid}: {gate_err}", file=sys.stderr) + continue if not kb.complete_task( conn, tid, result=args.result, @@ -1941,8 +1967,17 @@ def _cmd_block(args: argparse.Namespace) -> int: author = _profile_author() ids = [args.task_id] + list(getattr(args, "ids", None) or []) failed: list[str] = [] + # v6.3.1 hotfix — keep_running gate now fires on block as well as + # complete. See _handle_block for the rationale. + from tools.kanban_tools import _check_keep_running_gate with kb.connect_closing() as conn: for tid in ids: + task = kb.get_task(conn, tid) + keep_running_err = _check_keep_running_gate(kb, conn, task) + if keep_running_err: + failed.append(tid) + print(f"cannot block {tid}: {keep_running_err}", file=sys.stderr) + continue if reason: kb.add_comment(conn, tid, author, f"BLOCKED: {reason}") if not kb.block_task( diff --git a/hermes_cli/kanban_db.py b/hermes_cli/kanban_db.py index c8c53dba7ecb..4a4750774d9b 100644 --- a/hermes_cli/kanban_db.py +++ b/hermes_cli/kanban_db.py @@ -2840,6 +2840,60 @@ def _synthesize_ended_run( # Dependency resolution (todo -> ready) # --------------------------------------------------------------------------- +_KEEP_RUNNING_RE = re.compile( + r"^\s*keep_running\s*:\s*(true|yes|1)\s*$", + re.IGNORECASE | re.MULTILINE, +) + +# v6.5.1 — Review task detection for the "review-needs-build-parent" gate. +# Mirrors the verdict-gate detection in tools/kanban_tools.py so a task +# classified as a "review task" in one place is classified the same way +# in the other. The set/regex must stay in sync if either changes. +_REVIEWER_PROFILES_DB = frozenset({"tony", "tchalla", "vision", "elon"}) +_REVIEW_KEYWORD_RE = re.compile( + r"\b(review|verify|audit|inspect|smoke[- ]?test|qa)\b", re.IGNORECASE +) + + +def _is_review_task(assignee: Optional[str], title: Optional[str], body: Optional[str]) -> bool: + """True when the task should be subject to v6.5.1 review-dep enforcement. + + Mirrors tools/kanban_tools._check_reviewer_verdict_gate detection: + a task is "a review task" iff its assignee is a reviewer profile AND + its title or body mentions a review keyword. + """ + if not assignee or assignee.lower() not in _REVIEWER_PROFILES_DB: + return False + haystack = ((title or "") + " " + (body or "")).strip() + return bool(_REVIEW_KEYWORD_RE.search(haystack)) + + +def _task_is_keep_running_umbrella(conn: sqlite3.Connection, task_id: str) -> bool: + """v6.4: a task with ``keep_running: true`` in its body is an + orchestration umbrella, not a worker task. + + The dispatcher treats it specially: children promote regardless of + the umbrella's status (Fix A), and protocol_violation crashes do NOT + auto-block it (Fix B). Together this makes ``keep_running: true`` + a state the dispatcher recognizes rather than a rule the agent must + obey via poll-loop. + + Surfaced 2026-06-07 in v6.3 first-test: JARVIS gamed the v6.3 + completion gate by calling kanban_block, which v6.3.1 then sealed. + With both terminations rejected, JARVIS exited cleanly and the + dispatcher's protocol_violation handler auto-blocked the umbrella + — defeating the gate via a different code path. v6.4 closes the + loop by making the dispatcher aware of orchestration umbrellas. + """ + row = conn.execute( + "SELECT body FROM tasks WHERE id = ?", (task_id,) + ).fetchone() + if not row: + return False + body = row["body"] if "body" in row.keys() else None + return bool(body and _KEEP_RUNNING_RE.search(body)) + + def _has_sticky_block(conn: sqlite3.Connection, task_id: str) -> bool: """Return True when ``task_id`` is sticky-blocked by an explicit worker/operator ``kanban_block`` call (#28712). @@ -2927,12 +2981,24 @@ def recompute_ready( # this predicate back). continue parents = conn.execute( - "SELECT t.status FROM tasks t " + "SELECT t.id, t.status, t.body FROM tasks t " "JOIN task_links l ON l.parent_id = t.id " "WHERE l.child_id = ?", (task_id,), ).fetchall() - if all(p["status"] in ("done", "archived") for p in parents): + # v6.4 Fix A — children of a keep_running umbrella promote on + # their *other* parents' status, ignoring the umbrella entirely. + # An orchestration umbrella is conceptually never done; it + # exists to receive chain events. Forcing its status to gate + # child promotion was the v6.3 chain-stall bug. + def _parent_eligible(p) -> bool: + if p["status"] in ("done", "archived"): + return True + body = p["body"] if "body" in p.keys() else None + if body and _KEEP_RUNNING_RE.search(body): + return True + return False + if all(_parent_eligible(p) for p in parents): if cur_status == "blocked": # Don't auto-recover tasks that have hit the # circuit-breaker failure limit. Without this @@ -2993,13 +3059,25 @@ def claim_task( # 'todo' here — recompute_ready will re-promote when the parents # actually finish. See RCA at # kanban/boards/cookai/workspaces/t_a6acd07d/root-cause.md. - undone = conn.execute( - "SELECT 1 FROM task_links l " + # + # v6.4 Fix A — parents with `keep_running: true` in their body are + # orchestration umbrellas, not chain deps. They don't gate child + # promotion. recompute_ready already enforces this; the claim path + # must agree so a child promoted under v6.4 semantics isn't demoted + # back to `todo` here. + undone_rows = conn.execute( + "SELECT p.id, p.status, p.body FROM task_links l " "JOIN tasks p ON p.id = l.parent_id " - "WHERE l.child_id = ? AND p.status NOT IN ('done', 'archived') LIMIT 1", + "WHERE l.child_id = ? AND p.status NOT IN ('done', 'archived')", (task_id,), - ).fetchone() - if undone: + ).fetchall() + truly_undone = False + for r in undone_rows: + body = r["body"] if "body" in r.keys() else None + if not (body and _KEEP_RUNNING_RE.search(body)): + truly_undone = True + break + if truly_undone: conn.execute( "UPDATE tasks SET status = 'todo' " "WHERE id = ? AND status = 'ready'", @@ -3010,6 +3088,77 @@ def claim_task( {"reason": "parents_not_done"}, ) return None + # v6.5.1 — Review tasks must have at least one non-review parent. + # Surfaced in v6.5 first-test: Pepper created reviewer tasks + # (Tony/Tchalla/Vision Block X review) with empty parents. The + # tasks promoted immediately, ran against an empty (or stale) + # workspace, and Vision used verdict: approve with fabricated + # evidence ("13/13 tests pass" when no tests existed). The + # verdict gate in kanban_tools forces a verdict PREFIX but + # doesn't validate the verdict reflects reality. The structural + # fix is to refuse promotion until the review has something + # buildy to review against. + self_row = conn.execute( + "SELECT assignee, title, body FROM tasks WHERE id = ?", + (task_id,), + ).fetchone() + if self_row and _is_review_task( + self_row["assignee"], + self_row["title"] if "title" in self_row.keys() else None, + self_row["body"] if "body" in self_row.keys() else None, + ): + parent_rows = conn.execute( + "SELECT p.id, p.assignee, p.title, p.body FROM task_links l " + "JOIN tasks p ON p.id = l.parent_id " + "WHERE l.child_id = ?", + (task_id,), + ).fetchall() + has_non_review_parent = any( + not _is_review_task( + p["assignee"], + p["title"] if "title" in p.keys() else None, + p["body"] if "body" in p.keys() else None, + ) + for p in parent_rows + ) + # Also skip the umbrella check: an umbrella is technically a + # non-review parent, but it doesn't represent buildable + # output. Require at least one non-umbrella non-review parent. + has_non_umbrella_non_review_parent = any( + not _is_review_task( + p["assignee"], + p["title"] if "title" in p.keys() else None, + p["body"] if "body" in p.keys() else None, + ) + and not ( + p["body"] and _KEEP_RUNNING_RE.search(p["body"]) + ) + for p in parent_rows + ) + if not has_non_umbrella_non_review_parent: + conn.execute( + "UPDATE tasks SET status = 'blocked', " + "last_failure_error = ? " + "WHERE id = ? AND status = 'ready'", + ( + "v6.5.1 gate: review task has no non-review, " + "non-umbrella parent — the corresponding build " + "task must be linked via --depends-on so this " + "review runs against actual deliverables instead " + "of an empty workspace. See kanban-orchestration " + "skill § Pepper Chain Integrity.", + task_id, + ), + ) + _append_event( + conn, task_id, "claim_rejected", + { + "reason": "review_missing_build_parent", + "parent_count": len(parent_rows), + "non_review_parent": has_non_review_parent, + }, + ) + return None # Defensive: if a prior run somehow leaked (invariant violation from # an unknown code path), close it as 'reclaimed' so we don't strand # it when the CAS resets the pointer below. No-op when the invariant @@ -5546,6 +5695,28 @@ def detect_crashed_workers(conn: sqlite3.Connection) -> list[str]: not protocol_violation and _fp_counts.get(fp, 0) >= 3 ) + # v6.4 Fix B (expanded after 2026-06-07 01:00 finding) — + # ANY crash on a keep_running umbrella, not just protocol + # violations, is allowed to retry without tripping the + # breaker. The umbrella's role is orchestration; crashes + # on respawn are transient and the dispatcher should keep + # bringing it back. Without this expansion the v6.4 chain + # stalls when JARVIS's worker hits an unrelated transient + # (MCP timeout, gateway glitch) on an orchestration spawn — + # the breaker trips at limit=2 and the umbrella is blocked, + # defeating the keep_running semantics through a different + # crash type than v6.3's protocol_violation route. + # + # Pathological repeated crashes still leave a paper trail + # via the crashed event we already emitted; an operator can + # still inspect and act, but the chain doesn't stall on the + # umbrella's behalf. + if _task_is_keep_running_umbrella(conn, tid): + # Already in `ready` and the crash event was already + # appended above. Nothing more to do — the umbrella + # is in "watching idle" and the dispatcher will re-spawn + # JARVIS when there's actually something to orchestrate. + continue tripped = _record_task_failure( conn, tid, error=error_text, @@ -6559,20 +6730,48 @@ def _kanban_worker_skill_available(hermes_home: Optional[str]) -> bool: the kanban lifecycle contract is still injected via ``KANBAN_GUIDANCE``, so omitting the flag only drops the supplementary pattern library. """ + return _skill_available(hermes_home, "kanban-worker") + + +def _skill_available(hermes_home: Optional[str], skill_name: str) -> bool: + """Generalised resolvability check used to filter ``task.skills`` before + spawning a worker. + + Surfaced 2026-06-07 in v6.4 first-test: Pepper had ``--skill + kanban-orchestration`` baked into her own task by JARVIS at chain spawn, + and she passed the same to the build/review children she created. + ``kanban-orchestration`` only exists in jarvis's profile-scoped skills + dir; spawning a Shuri/Vision worker with ``--skills kanban-orchestration`` + raised ``ValueError: Unknown skill(s): kanban-orchestration`` and killed + the worker at startup. The dispatcher recorded ``crashed exit_code=1`` + twice and gave up — chain stalled. + + The defensive fix: filter ``task.skills`` through this helper at spawn + time. Any skill that does not resolve for the worker's HERMES_HOME is + dropped with a logged warning. The agent author's mistake doesn't kill + the chain; the operator sees the warning and can fix the spec. + + Mirrors the canonical / bounded-scan strategy from + ``_kanban_worker_skill_available`` so the two stay aligned. + """ from pathlib import Path as _Path - # An unset HERMES_HOME means the worker falls back to the default root - # home (``~/.hermes``), which ships the bundled skill. base = _Path(hermes_home) if hermes_home else (_Path.home() / ".hermes") skills_root = base / "skills" if not skills_root.is_dir(): return False - # Canonical bundled location first (cheap), then a bounded scan for - # profiles that have it nested elsewhere. - if (skills_root / "devops" / "kanban-worker" / "SKILL.md").is_file(): - return True + # Canonical layouts first (cheap), then bounded recursion. + candidates = ( + skills_root / skill_name / "SKILL.md", + skills_root / "devops" / skill_name / "SKILL.md", + skills_root / "qa" / skill_name / "SKILL.md", + skills_root / "ui-ux" / skill_name / "SKILL.md", + ) + for c in candidates: + if c.is_file(): + return True try: - for skill_md in skills_root.rglob("kanban-worker/SKILL.md"): + for skill_md in skills_root.rglob(f"{skill_name}/SKILL.md"): if skill_md.is_file(): return True except OSError: @@ -6738,9 +6937,28 @@ def _default_spawn( # Dedupe against the built-in so we don't double-load kanban-worker # if a task author asks for it explicitly. if task.skills: + worker_home = env.get("HERMES_HOME") for sk in task.skills: - if sk and sk != "kanban-worker": + if not sk or sk == "kanban-worker": + continue + # v6.5 defensive filter — if the requested skill doesn't resolve + # for this worker's profile, drop it with a warning instead of + # crashing the spawn. Preloading an unknown skill is fatal at + # CLI startup (ValueError: Unknown skill(s): X). Surfaced + # 2026-06-07 when Pepper/Shuri/Vision tasks had + # ``skills=["kanban-orchestration"]`` baked in — that skill + # only exists for jarvis. Two workers crashed and the chain + # stalled before the cause was found in the per-task log. + if _skill_available(worker_home, sk): cmd.extend(["--skills", sk]) + else: + logging.getLogger(__name__).warning( + "kanban spawn: task %s requests --skills %s but the " + "skill is not resolvable for profile %s (HERMES_HOME=%s). " + "Dropping the flag to avoid startup crash; review the " + "task body or the spec writer's skill list.", + task.id, sk, profile_arg, worker_home, + ) if task.model_override: cmd.extend(["-m", task.model_override]) cmd.extend([ diff --git a/tests/hermes_cli/test_kanban_db.py b/tests/hermes_cli/test_kanban_db.py index 94295f2b63ab..b1fac5822ee8 100644 --- a/tests/hermes_cli/test_kanban_db.py +++ b/tests/hermes_cli/test_kanban_db.py @@ -346,6 +346,442 @@ def test_recompute_ready_fan_in_waits_for_all_parents(kanban_home): assert kb.get_task(conn, c).status == "ready" +# --------------------------------------------------------------------------- +# v6.4 — keep_running umbrella semantics +# --------------------------------------------------------------------------- + + +def test_recompute_ready_promotes_through_keep_running_umbrella(kanban_home): + """v6.4 Fix A — a child whose only non-done parent is a keep_running + umbrella should promote regardless of the umbrella's status. + + Reproduces the v6.3 first-test stall: Banner had parents=[umbrella]; + umbrella was `blocked` (JARVIS gave up after gates rejected both + completion and block); Banner sat in `todo` indefinitely. With the + umbrella marked keep_running, Banner promotes regardless. + """ + with kb.connect() as conn: + umbrella = kb.create_task( + conn, + title="v6.4 umbrella", + assignee="jarvis", + body="Orchestration test.\n\nkeep_running: true\n", + ) + banner = kb.create_task( + conn, title="banner research", assignee="banner", + parents=[umbrella], + ) + # Mid-chain: simulate JARVIS being in any non-done state. + # Banner should be ready in all of them. + for umbrella_status in ("ready", "running", "blocked"): + conn.execute( + "UPDATE tasks SET status=? WHERE id=?", + (umbrella_status, umbrella), + ) + # Force banner back to todo so recompute_ready has work. + conn.execute( + "UPDATE tasks SET status='todo' WHERE id=?", (banner,) + ) + conn.commit() + kb.recompute_ready(conn) + assert kb.get_task(conn, banner).status == "ready", ( + f"banner should be ready when umbrella={umbrella_status} " + f"(keep_running umbrella)" + ) + + +def test_claim_task_allows_keep_running_umbrella_parent(kanban_home): + """v6.4 Fix A extended — claim_task's parents-not-done invariant + must also recognize keep_running umbrellas, otherwise a child + promoted by recompute_ready gets demoted back to `todo` on the + first claim attempt. + + Surfaced 2026-06-07 in v6.4 first-test: Banner kept getting promoted + (Fix A worked in recompute_ready) and then claim_rejected with + reason: parents_not_done (Fix A missing from claim_task). The + invariant cycle prevented Banner from ever running. + """ + with kb.connect() as conn: + umbrella = kb.create_task( + conn, title="v6.4 umbrella", assignee="jarvis", + body="keep_running: true", + ) + banner = kb.create_task( + conn, title="banner research", assignee="banner", + parents=[umbrella], + ) + # Force umbrella running (a real chain state, not done). + conn.execute( + "UPDATE tasks SET status='running' WHERE id=?", + (umbrella,), + ) + # Force banner ready (post-recompute_ready state). + conn.execute( + "UPDATE tasks SET status='ready' WHERE id=?", (banner,) + ) + conn.commit() + claimed = kb.claim_task(conn, banner) + assert claimed is not None, ( + "Banner should be claimable when umbrella is keep_running, " + "even if umbrella is `running` (not done)" + ) + assert kb.get_task(conn, banner).status == "running" + + +def test_claim_task_still_blocks_undone_regular_parent(kanban_home): + """v6.4 Fix A guard rail — claim_task still demotes a child whose + regular (non-keep_running) parent is undone.""" + with kb.connect() as conn: + parent = kb.create_task( + conn, title="regular parent", assignee="banner", + body="No keep_running here.", + ) + child = kb.create_task( + conn, title="downstream", assignee="friday", parents=[parent], + ) + # Force statuses: parent running (not done), child ready. + conn.execute( + "UPDATE tasks SET status='running' WHERE id=?", (parent,) + ) + conn.execute( + "UPDATE tasks SET status='ready' WHERE id=?", (child,) + ) + conn.commit() + claimed = kb.claim_task(conn, child) + assert claimed is None, ( + "child should NOT be claimable when regular parent is running" + ) + # And the invariant demoted it back. + assert kb.get_task(conn, child).status == "todo" + + +def test_recompute_ready_does_not_promote_through_regular_parent(kanban_home): + """v6.4 Fix A guard rail — a child whose parent does NOT have + keep_running: true must still wait for that parent to complete.""" + with kb.connect() as conn: + parent = kb.create_task( + conn, title="regular parent", assignee="banner", + body="No keep_running marker here.", + ) + child = kb.create_task( + conn, title="downstream", assignee="friday", parents=[parent], + ) + for parent_status in ("ready", "running", "blocked"): + conn.execute( + "UPDATE tasks SET status=? WHERE id=?", + (parent_status, parent), + ) + conn.execute( + "UPDATE tasks SET status='todo' WHERE id=?", (child,) + ) + conn.commit() + kb.recompute_ready(conn) + assert kb.get_task(conn, child).status == "todo", ( + f"child should NOT promote when parent={parent_status} " + f"and parent is not keep_running" + ) + + +def test_recompute_ready_mixed_keep_running_and_regular_parents(kanban_home): + """v6.4 Fix A — child with both a keep_running umbrella AND a regular + parent must wait for the regular parent to be done; the umbrella's + status is ignored either way.""" + with kb.connect() as conn: + umbrella = kb.create_task( + conn, title="umbrella", assignee="jarvis", + body="keep_running: true", + ) + upstream = kb.create_task( + conn, title="upstream", assignee="banner", + ) + child = kb.create_task( + conn, title="downstream", assignee="friday", + parents=[umbrella, upstream], + ) + # Umbrella in any status + upstream not done → child stays todo + for umbrella_status in ("running", "blocked"): + conn.execute( + "UPDATE tasks SET status=? WHERE id=?", + (umbrella_status, umbrella), + ) + conn.commit() + kb.recompute_ready(conn) + assert kb.get_task(conn, child).status == "todo" + # Once upstream completes, child promotes regardless of umbrella + kb.claim_task(conn, upstream) + kb.complete_task(conn, upstream, summary="done") + assert kb.get_task(conn, child).status == "ready" + + +def _simulate_clean_exit(monkeypatch, pid: int): + """Force _classify_worker_exit to return ('clean_exit', 0) for this + pid AND make _pid_alive say the pid is dead. + + Bypasses the real os.waitpid reap registry so tests don't depend on + racy subprocess teardown.""" + from hermes_cli import kanban_db as _kb + real_classify = _kb._classify_worker_exit + real_alive = _kb._pid_alive + + def fake_classify(p): + if int(p) == int(pid): + return ("clean_exit", 0) + return real_classify(p) + + def fake_alive(p): + if int(p or 0) == int(pid): + return False + return real_alive(p) + + monkeypatch.setattr(_kb, "_classify_worker_exit", fake_classify) + monkeypatch.setattr(_kb, "_pid_alive", fake_alive) + + +def test_protocol_violation_on_keep_running_umbrella_does_not_auto_block( + kanban_home, monkeypatch, +): + """v6.4 Fix B — when a keep_running umbrella's worker exits cleanly + without calling complete/block, the dispatcher must NOT auto-block + the umbrella. The protocol_violation event still fires (audit) but + the umbrella stays in `ready` for the next dispatch tick. + """ + fake_pid = 999_999 # a pid that won't exist + with kb.connect() as conn: + umbrella = kb.create_task( + conn, title="v6.4 umbrella", assignee="jarvis", + body="keep_running: true", + ) + kb.claim_task(conn, umbrella) + # Pre-date started_at past the grace window so the liveness + # check actually runs. + conn.execute( + "UPDATE tasks SET worker_pid=?, started_at=? WHERE id=?", + (fake_pid, 0, umbrella), + ) + conn.commit() + _simulate_clean_exit(monkeypatch, fake_pid) + with kb.connect() as conn: + crashed = kb.detect_crashed_workers(conn) + assert umbrella in crashed + status = kb.get_task(conn, umbrella).status + assert status == "ready", ( + f"keep_running umbrella should stay ready after protocol " + f"violation; got status={status}" + ) + rows = conn.execute( + "SELECT kind FROM task_events WHERE task_id=? " + "ORDER BY id DESC LIMIT 5", + (umbrella,), + ).fetchall() + event_kinds = [r["kind"] for r in rows] + assert "protocol_violation" in event_kinds, ( + f"expected protocol_violation event; got {event_kinds}" + ) + + +def test_review_task_with_no_parents_cannot_claim(kanban_home): + """v6.5.1 gate — review tasks must have a non-review parent. + + Reproduces v6.5 first-test: Pepper created Vision Block A review + with parents=[]. Vision claimed within seconds, ran against an + empty workspace, fabricated evidence in her summary, and used + `verdict: approve`. The verdict gate forced the prefix but + couldn't validate the verdict reflected reality. Structural fix: + refuse to claim a review task that has no real build parent. + """ + with kb.connect() as conn: + review = kb.create_task( + conn, + title="Block A review (vision) — v6.5 chain integrity", + assignee="vision", + body="Review Block A per the spec.", + ) + # Force ready (no parents) + conn.execute("UPDATE tasks SET status='ready' WHERE id=?", (review,)) + conn.commit() + claimed = kb.claim_task(conn, review) + assert claimed is None, ( + "review task with no parents must not be claimable" + ) + # And it should be blocked with the v6.5.1 reason. + task = kb.get_task(conn, review) + assert task.status == "blocked" + assert task.last_failure_error and "v6.5.1 gate" in task.last_failure_error + + +def test_review_task_with_umbrella_parent_only_cannot_claim(kanban_home): + """v6.5.1 gate — even with a keep_running umbrella parent, a review + task needs a non-umbrella, non-review build parent to claim. + + Otherwise the v6.4 keep_running umbrella semantics would let any + review task promote against just an umbrella + no real build. + """ + with kb.connect() as conn: + umbrella = kb.create_task( + conn, title="orchestration umbrella", assignee="jarvis", + body="keep_running: true", + ) + review = kb.create_task( + conn, + title="Block B review (tony) — v6.5 chain integrity", + assignee="tony", + body="Review Block B code.", + parents=[umbrella], + ) + conn.execute("UPDATE tasks SET status='ready' WHERE id=?", (review,)) + conn.commit() + claimed = kb.claim_task(conn, review) + assert claimed is None + assert kb.get_task(conn, review).status == "blocked" + + +def test_review_task_with_build_parent_can_claim(kanban_home): + """v6.5.1 gate — guard rail. Review task linked to its build task + (via --depends-on) promotes normally when the build is done.""" + with kb.connect() as conn: + build = kb.create_task( + conn, title="Block A — Shuri: backend", + assignee="shuri", + body="Build the backend.", + ) + kb.claim_task(conn, build) + kb.complete_task(conn, build, summary="block A shipped") + review = kb.create_task( + conn, + title="Block A review (tony) — v6.5 chain integrity", + assignee="tony", + body="Review Block A code per your SOUL.", + parents=[build], + ) + # Build is done so review should promote + assert kb.get_task(conn, review).status == "ready" + claimed = kb.claim_task(conn, review) + assert claimed is not None, "review with done build parent must claim" + assert kb.get_task(conn, review).status == "running" + + +def test_non_review_task_not_subject_to_v6_5_1_gate(kanban_home): + """v6.5.1 gate guard rail — non-review tasks (build, research, etc.) + are not subject to the build-parent requirement.""" + with kb.connect() as conn: + # Build task with no parents — should claim normally + build = kb.create_task( + conn, title="Block A — Shuri: backend", + assignee="shuri", + body="Build the backend.", + ) + # create_task starts at 'ready' so this can claim immediately + claimed = kb.claim_task(conn, build) + assert claimed is not None + + +def test_skill_available_finds_canonical_locations(kanban_home): + """v6.5 — _skill_available finds skills in canonical locations + (devops/, qa/, ui-ux/) AND via bounded rglob fallback.""" + from hermes_cli.kanban_db import _skill_available + import os, pathlib + home = pathlib.Path(os.environ["HERMES_HOME"]) + skills_root = home / "skills" + + # Build a fake skill in the devops/ canonical layout + devops_skill = skills_root / "devops" / "my-skill" / "SKILL.md" + devops_skill.parent.mkdir(parents=True, exist_ok=True) + devops_skill.write_text("# my-skill") + assert _skill_available(str(home), "my-skill") + + # And in a non-canonical nested layout + nested_skill = skills_root / "tenants" / "acme" / "deep-skill" / "SKILL.md" + nested_skill.parent.mkdir(parents=True, exist_ok=True) + nested_skill.write_text("# deep-skill") + assert _skill_available(str(home), "deep-skill") + + # Missing skill returns False (this is the spawn-crash mitigation) + assert not _skill_available(str(home), "kanban-orchestration"), ( + "kanban-orchestration should NOT be in this test profile's skills dir; " + "the dispatcher must filter it from --skills flags before spawn" + ) + + +def test_real_crash_on_keep_running_umbrella_does_not_auto_block( + kanban_home, monkeypatch, +): + """v6.4 Fix B (expanded) — ANY crash on a keep_running umbrella, + not just protocol violations, leaves the breaker untripped. + + Surfaced 2026-06-07 01:00 in the v6.4 first-test: JARVIS's + orchestration worker hit a real crash (not clean exit) on the + second tick. The original Fix B only covered clean_exit; + real-crash routes through the normal failure handler and tripped + the breaker at limit=2, blocking the umbrella anyway. + + Pathological repeated crashes still leave a paper trail; the + operator can still inspect via the crashed event sequence. But + the chain doesn't stall on the umbrella's behalf. + """ + fake_pid = 999_997 + with kb.connect() as conn: + umbrella = kb.create_task( + conn, title="v6.4 umbrella", assignee="jarvis", + body="keep_running: true", + ) + kb.claim_task(conn, umbrella) + conn.execute( + "UPDATE tasks SET worker_pid=?, started_at=? WHERE id=?", + (fake_pid, 0, umbrella), + ) + conn.commit() + # Simulate a real crash (nonzero exit), not a clean exit. + from hermes_cli import kanban_db as _kb + real_classify = _kb._classify_worker_exit + real_alive = _kb._pid_alive + monkeypatch.setattr( + _kb, "_classify_worker_exit", + lambda p: ("nonzero_exit", 1) if int(p) == fake_pid else real_classify(p), + ) + monkeypatch.setattr( + _kb, "_pid_alive", + lambda p: False if int(p or 0) == fake_pid else real_alive(p), + ) + with kb.connect() as conn: + crashed = kb.detect_crashed_workers(conn) + assert umbrella in crashed + status = kb.get_task(conn, umbrella).status + assert status == "ready", ( + f"keep_running umbrella should stay ready after ANY crash; " + f"got status={status}" + ) + + +def test_protocol_violation_on_regular_task_still_auto_blocks( + kanban_home, monkeypatch, +): + """v6.4 Fix B guard rail — a non-keep_running task's clean-exit + protocol violation still trips the breaker as before.""" + fake_pid = 999_998 + with kb.connect() as conn: + task = kb.create_task( + conn, title="regular worker task", assignee="friday", + body="No keep_running here.", + ) + kb.claim_task(conn, task) + conn.execute( + "UPDATE tasks SET worker_pid=?, started_at=? WHERE id=?", + (fake_pid, 0, task), + ) + conn.commit() + _simulate_clean_exit(monkeypatch, fake_pid) + with kb.connect() as conn: + crashed = kb.detect_crashed_workers(conn) + assert task in crashed + status = kb.get_task(conn, task).status + # Regular tasks: protocol_violation forces failure_limit=1, so + # the task auto-blocks immediately. + assert status == "blocked", ( + f"regular task should auto-block after protocol violation; " + f"got status={status}" + ) + + # --------------------------------------------------------------------------- # Atomic claim (CAS) # --------------------------------------------------------------------------- diff --git a/tests/tools/test_kanban_tools.py b/tests/tools/test_kanban_tools.py index 2bf89449905a..c69860af6efc 100644 --- a/tests/tools/test_kanban_tools.py +++ b/tests/tools/test_kanban_tools.py @@ -397,6 +397,403 @@ def test_complete_with_result_only(worker_env): assert d["ok"] is True +# --------------------------------------------------------------------------- +# v6.3 completion gates — verdict, evidence-path, keep_running +# --------------------------------------------------------------------------- + + +def _make_task(monkeypatch, tmp_path, *, assignee, title, body): + """Helper: spin up a kanban DB and create a fresh task in `running` state. + + Returns the task id (which is also set as HERMES_KANBAN_TASK so the + handler defaults to this task). + """ + home = tmp_path / ".hermes" + home.mkdir(exist_ok=True) + monkeypatch.setenv("HERMES_HOME", str(home)) + monkeypatch.setenv("HERMES_PROFILE", assignee) + monkeypatch.delenv("HERMES_SESSION_ID", raising=False) + from pathlib import Path as _Path + monkeypatch.setattr(_Path, "home", lambda: tmp_path) + + from hermes_cli import kanban_db as kb + kb._INITIALIZED_PATHS.clear() + kb.init_db() + conn = kb.connect() + try: + tid = kb.create_task(conn, title=title, body=body, assignee=assignee) + kb.claim_task(conn, tid) + finally: + conn.close() + monkeypatch.setenv("HERMES_KANBAN_TASK", tid) + return tid + + +def test_verdict_gate_fires_on_review_task_without_verdict(monkeypatch, tmp_path): + tid = _make_task( + monkeypatch, tmp_path, + assignee="tony", + title="tony review Block A", + body="Review Block A code quality.", + ) + from tools import kanban_tools as kt + out = kt._handle_complete({"summary": "looks good"}) + d = json.loads(out) + assert "error" in d + assert "verdict" in d["error"].lower() + assert tid in d["error"] + + +def test_verdict_gate_passes_on_explicit_verdict(monkeypatch, tmp_path): + _make_task( + monkeypatch, tmp_path, + assignee="tony", + title="tony review Block A", + body="Review Block A code quality.", + ) + from tools import kanban_tools as kt + out = kt._handle_complete({ + "result": "verdict: approve", + "summary": "All gates pass on Block A.", + }) + d = json.loads(out) + assert d.get("ok") is True + + +def test_verdict_gate_passes_on_reject(monkeypatch, tmp_path): + _make_task( + monkeypatch, tmp_path, + assignee="tchalla", + title="tchalla review Block A", + body="Review Block A test strategy.", + ) + from tools import kanban_tools as kt + out = kt._handle_complete({ + "result": "verdict: reject", + "metadata": {"reasons": ["tests exercise fallback, not real binding"]}, + }) + assert json.loads(out).get("ok") is True + + +def test_verdict_gate_bypasses_non_reviewer(monkeypatch, tmp_path): + """Friday's build task should NOT require a verdict prefix.""" + _make_task( + monkeypatch, tmp_path, + assignee="friday", + title="friday Block A", + body="Wire the 4 routes per Pepper's spec.", + ) + from tools import kanban_tools as kt + out = kt._handle_complete({"summary": "shipped Block A on wt/fix-13-block-a"}) + assert json.loads(out).get("ok") is True + + +def test_verdict_gate_bypasses_reviewer_on_non_review_task(monkeypatch, tmp_path): + """A reviewer profile can do non-review work without a verdict prefix. + + Example: Vision builds UI (assigned to vision but task is not a review). + """ + _make_task( + monkeypatch, tmp_path, + assignee="vision", + title="vision build: 4 UI components", + body="Implement BlockA components per Pepper's spec.", + ) + from tools import kanban_tools as kt + out = kt._handle_complete({"summary": "Built 4 components with token-checked styles"}) + assert json.loads(out).get("ok") is True + + +def test_evidence_path_gate_fires_when_paths_missing(monkeypatch, tmp_path): + body = ( + "Build task with smoke check.\n\n" + "required_evidence_paths:\n" + " - /tmp/v6-3-test-missing.log\n" + " - /tmp/v6-3-test-also-missing.txt\n" + ) + tid = _make_task( + monkeypatch, tmp_path, + assignee="friday", + title="friday Block A", + body=body, + ) + # Belt-and-suspenders: ensure the paths really don't exist. + for p in ("/tmp/v6-3-test-missing.log", "/tmp/v6-3-test-also-missing.txt"): + if os.path.exists(p): + os.unlink(p) + from tools import kanban_tools as kt + out = kt._handle_complete({"summary": "smoke check done (allegedly)"}) + d = json.loads(out) + assert "error" in d + assert "evidence" in d["error"].lower() or "required" in d["error"].lower() + assert tid in d["error"] + + +def test_evidence_path_gate_passes_when_paths_exist(monkeypatch, tmp_path): + p1 = tmp_path / "smoke-dev.log" + p2 = tmp_path / "smoke-curls.txt" + p1.write_text("Server ready") + p2.write_text("/ 200\n/cycle-budget 200") + body = ( + "Build task with smoke check.\n\n" + f"required_evidence_paths:\n" + f" - {p1}\n" + f" - {p2}\n" + ) + _make_task( + monkeypatch, tmp_path, + assignee="friday", + title="friday Block A", + body=body, + ) + from tools import kanban_tools as kt + out = kt._handle_complete({"summary": "smoke check actually done"}) + assert json.loads(out).get("ok") is True + + +def test_evidence_path_gate_treats_empty_files_as_failure(monkeypatch, tmp_path): + empty = tmp_path / "empty-screenshot.png" + empty.touch() + body = f"required_evidence_paths:\n - {empty}\n" + _make_task( + monkeypatch, tmp_path, + assignee="friday", + title="friday Block A", + body=body, + ) + from tools import kanban_tools as kt + out = kt._handle_complete({"summary": "empty file shouldn't count"}) + d = json.loads(out) + assert "error" in d + assert "empty" in d["error"].lower() + + +def test_evidence_path_gate_bypasses_when_no_declaration(monkeypatch, tmp_path): + """Tasks without a required_evidence_paths: block pass cleanly.""" + _make_task( + monkeypatch, tmp_path, + assignee="friday", + title="friday simple fix", + body="One-line bugfix; no artifacts required.", + ) + from tools import kanban_tools as kt + out = kt._handle_complete({"summary": "shipped"}) + assert json.loads(out).get("ok") is True + + +def test_keep_running_gate_fires_when_descendants_active(monkeypatch, tmp_path): + """An umbrella with keep_running: true cannot complete while a child + is non-terminal.""" + body = ( + "v6.3 test umbrella.\n\n" + "keep_running: true\n" + ) + parent_tid = _make_task( + monkeypatch, tmp_path, + assignee="jarvis", + title="v6.3 test umbrella", + body=body, + ) + # Create a live child linked to the umbrella. + from hermes_cli import kanban_db as kb + conn = kb.connect() + try: + child = kb.create_task(conn, title="child task", assignee="friday") + kb.link_tasks(conn, parent_tid, child) + finally: + conn.close() + from tools import kanban_tools as kt + out = kt._handle_complete({"summary": "trying to close umbrella early"}) + d = json.loads(out) + assert "error" in d + assert "keep_running" in d["error"] or "descendant" in d["error"] + assert parent_tid in d["error"] + + +def test_keep_running_gate_passes_when_all_children_terminal(monkeypatch, tmp_path): + body = "keep_running: true\n" + parent_tid = _make_task( + monkeypatch, tmp_path, + assignee="jarvis", + title="v6.3 test umbrella", + body=body, + ) + from hermes_cli import kanban_db as kb + conn = kb.connect() + try: + child = kb.create_task(conn, title="child task", assignee="friday") + kb.link_tasks(conn, parent_tid, child) + # Force the child into a terminal state directly (the test-only + # path; tooling-level complete_task requires run-id coordination + # we don't need to replicate here). + conn.execute( + "UPDATE tasks SET status = 'done', completed_at = strftime('%s','now') " + "WHERE id = ?", (child,) + ) + conn.commit() + finally: + conn.close() + from tools import kanban_tools as kt + out = kt._handle_complete({"summary": "all children terminal; closing umbrella"}) + assert json.loads(out).get("ok") is True + + +def test_keep_running_gate_tenant_walk_finds_unlinked_siblings(monkeypatch, tmp_path): + """v6.6 — when the umbrella has a tenant set, the gate walks by + tenant instead of task_links. Closes the v6.5.1 stall route where + Pepper-shaped chains don't link build tasks back to the umbrella — + Shuri/Vision/Friday tasks have parents=[chain-predecessor] not + parents=[umbrella, ...]. The umbrella's task_links would only find + Banner+Pepper directly. Tenant scoping finds the whole chain. + """ + body = "keep_running: true\n" + parent_tid = _make_task( + monkeypatch, tmp_path, + assignee="jarvis", + title="v6.6 umbrella", + body=body, + ) + # Tag both the umbrella AND a sibling task with the same tenant, + # and DO NOT link them via task_links — simulates Pepper's chain + # shape (build tasks have parents pointing at chain predecessors, + # not back at the umbrella). + tenant = "marvel-swarm-v6-6-tenant-walk-test" + from hermes_cli import kanban_db as kb + conn = kb.connect() + try: + conn.execute("UPDATE tasks SET tenant=? WHERE id=?", (tenant, parent_tid)) + # Create a sibling in the same tenant, unlinked + sibling = kb.create_task( + conn, title="orphan build task", assignee="shuri", + tenant=tenant, + ) + # No kb.link_tasks call — just same tenant + conn.commit() + finally: + conn.close() + from tools import kanban_tools as kt + out = kt._handle_complete({"summary": "trying to close while tenant has live work"}) + d = json.loads(out) + assert "error" in d, f"expected tenant-scoped gate to reject; got: {d}" + assert "tenant" in d["error"].lower() + assert parent_tid in d["error"] + + +def test_keep_running_gate_bypasses_when_no_marker(monkeypatch, tmp_path): + """An umbrella without keep_running: true closes normally even with live + descendants — opt-in semantics.""" + parent_tid = _make_task( + monkeypatch, tmp_path, + assignee="jarvis", + title="legacy umbrella", + body="No keep_running marker — older orchestration pattern.", + ) + from hermes_cli import kanban_db as kb + conn = kb.connect() + try: + child = kb.create_task(conn, title="child task", assignee="friday") + kb.link_tasks(conn, parent_tid, child) + finally: + conn.close() + from tools import kanban_tools as kt + out = kt._handle_complete({"summary": "closing legacy umbrella"}) + assert json.loads(out).get("ok") is True + + +def test_keep_running_gate_fires_on_block_too(monkeypatch, tmp_path): + """v6.3.1 hotfix — kanban_block on a keep_running umbrella with a live + child must be rejected (same way as kanban_complete). + + Surfaced 2026-06-07 when JARVIS routed around the v6.3 keep_running + completion gate by calling kanban_block on the umbrella with reason + 'awaiting-async-event'. The umbrella transitioned to `blocked`, the + dispatcher refused to promote children of a blocked parent, and the + chain stalled. Block must enforce the same descendants-terminal rule + that complete enforces. + + The verdict and evidence-path gates stay completion-only — a reviewer + blocking on infra and a builder blocking honestly on missing artifacts + are both legitimate uses of kanban_block. + """ + body = "keep_running: true\n" + parent_tid = _make_task( + monkeypatch, tmp_path, + assignee="jarvis", + title="v6.3.1 umbrella", + body=body, + ) + from hermes_cli import kanban_db as kb + conn = kb.connect() + try: + child = kb.create_task(conn, title="live child", assignee="friday") + kb.link_tasks(conn, parent_tid, child) + finally: + conn.close() + from tools import kanban_tools as kt + out = kt._handle_block({"reason": "awaiting-async-event"}) + d = json.loads(out) + assert "error" in d, f"expected block to be rejected; got: {d}" + assert "keep_running" in d["error"] or "descendant" in d["error"] + assert parent_tid in d["error"] + + +def test_block_passes_on_non_umbrella(monkeypatch, tmp_path): + """Non-umbrella tasks (no keep_running marker) can block normally — + e.g. Friday blocking honestly because he can't produce an artifact.""" + _make_task( + monkeypatch, tmp_path, + assignee="friday", + title="friday Block A", + body=( + "Build the routes.\n\n" + "required_evidence_paths:\n" + " - /tmp/v6-3-1-evidence-not-yet.log\n" + ), + ) + from tools import kanban_tools as kt + out = kt._handle_block({ + "reason": "evidence: /tmp/v6-3-1-evidence-not-yet.log could not be produced because dev server crashed", + }) + assert json.loads(out).get("ok") is True + + +def test_depends_on_alias_appears_in_create_help(): + """--depends-on is exposed alongside --parent on `hermes kanban create`. + + Smoke test: build the argparse subparser locally and assert both flags + are wired with the same dest. The gate runs before any DB work. + """ + import argparse + # Build a fresh parser mirroring the production setup, then introspect. + parser = argparse.ArgumentParser() + sub = parser.add_subparsers(dest="cmd") + p_create = sub.add_parser("create") + p_create.add_argument("title") + p_create.add_argument("--parent", action="append", default=[]) + p_create.add_argument("--depends-on", dest="parent", action="append") + # Confirm both flags route to args.parent. + args = parser.parse_args(["create", "title", "--parent", "t_aaa", + "--depends-on", "t_bbb"]) + assert args.parent == ["t_aaa", "t_bbb"] + + # And confirm the actual production module exposes the flag in --help. + import io, contextlib + import argparse as ap + from hermes_cli import kanban as kbcli + root = ap.ArgumentParser(prog="hermes") + root_sub = root.add_subparsers(dest="kanban_action") + kbcli.build_parser(root_sub) + buf = io.StringIO() + with contextlib.redirect_stdout(buf), contextlib.redirect_stderr(buf): + try: + root.parse_args(["kanban", "create", "--help"]) + except SystemExit: + pass + out = buf.getvalue() + assert "--depends-on" in out, f"expected --depends-on in help; got: {out[:500]}" + assert "--parent" in out + + def test_complete_with_artifacts_lands_in_event_payload(worker_env): """``artifacts=[...]`` rides into the completed event payload so the gateway notifier can upload them as native attachments. See the diff --git a/tools/kanban_tools.py b/tools/kanban_tools.py index 20a522f90a4b..b5e10a499b0d 100644 --- a/tools/kanban_tools.py +++ b/tools/kanban_tools.py @@ -31,6 +31,8 @@ import json import logging import os +import re +from pathlib import Path from typing import Any, Optional from tools.registry import registry, tool_error @@ -38,6 +40,264 @@ logger = logging.getLogger(__name__) +# --------------------------------------------------------------------------- +# v6.3 completion gates — enforce discipline at the CLI/tool layer +# --------------------------------------------------------------------------- +# +# Three test cycles (v6, v6.1, v6.2 on 2026-06-06) demonstrated that +# discipline rules living in SOUL/skill text don't reliably bind for +# existing agents under load. Fresh agents bind the rules on first use; +# existing agents read the updated text and revert to baseline behavior. +# +# v6.3 ships dispatcher-level enforcement: the agent gets a tool_error +# (which the model interprets as a retryable failure, like the existing +# HallucinatedCardsError pattern) rather than a green checkmark. Three +# gates fire on every kanban_complete: +# +# 1. Reviewer verdict gate — review tasks must end with a verdict +# 2. Evidence-path gate — declared artifacts must exist on disk +# 3. Keep-running umbrella — umbrella tasks can't complete with live children +# +# Each gate runs BEFORE kb.complete_task is called. Task state is not +# mutated when a gate rejects (mirrors HallucinatedCardsError semantics). + +_REVIEWER_PROFILES = frozenset({"tony", "tchalla", "vision", "elon"}) + +# A review task signals "review-ness" via title/body keywords. We cast a wide +# net here because the cost of asking a non-review task for a verdict is low +# (the agent passes verdict: not-applicable or pivots to a non-review verb). +_REVIEW_KEYWORD_RE = re.compile( + r"\b(review|verify|audit|inspect|smoke[- ]?test|qa)\b", re.IGNORECASE +) +_VERDICT_PREFIX_RE = re.compile( + r"^\s*verdict\s*:\s*(approve|reject|not[- ]?applicable)\b", + re.IGNORECASE, +) + +_PATH_LIST_RE = re.compile(r"^\s*-\s+(.+?)\s*$") +_YAML_KEY_RE = re.compile(r"^[A-Za-z][\w-]*:") + +# Body markers — YAML-shaped, parser-tolerant. Same convention as the +# enforce_evidence_paths.py companion script in hermes-jarvis. +_REQUIRED_EVIDENCE_KEY = "required_evidence_paths:" +_KEEP_RUNNING_KEY = "keep_running:" + + +def _extract_yaml_list(body: str, key: str) -> list[str]: + """Parse a YAML-shaped list under ``key`` from ``body``. + + Tolerant of: + * inline form ``key: [a, b]`` + * block form with ``-`` list items at any indent + * surrounding prose; stops at the next top-level YAML key + + Returns the (possibly empty) list of raw values. + """ + if not body: + return [] + out: list[str] = [] + in_block = False + for line in body.splitlines(): + stripped = line.strip() + if not in_block: + if stripped.lower().startswith(key.lower()): + in_block = True + inline = stripped[len(key):].strip() + if inline.startswith("[") and inline.endswith("]"): + for raw in inline[1:-1].split(","): + item = raw.strip().strip("'\"") + if item: + out.append(item) + return out + continue + if not line.strip(): + continue + if not line.startswith(" ") and _YAML_KEY_RE.match(line): + break + m = _PATH_LIST_RE.match(line) + if m: + out.append(m.group(1).strip().strip("'\"")) + elif not line.startswith(" "): + break + return out + + +def _extract_yaml_scalar(body: str, key: str) -> Optional[str]: + """Parse a YAML-shaped scalar under ``key`` from ``body``. + + Returns the trimmed value, or None if the key is absent. + """ + if not body: + return None + for line in body.splitlines(): + stripped = line.strip() + if stripped.lower().startswith(key.lower()): + return stripped[len(key):].strip().strip("'\"") + return None + + +def _check_reviewer_verdict_gate(task, result: Optional[str]) -> Optional[str]: + """If the task is a review task, require a verdict in ``result``. + + Returns None on pass; an error sentence on reject. + """ + if not task: + return None + assignee = (task.assignee or "").lower() + if assignee not in _REVIEWER_PROFILES: + return None + title_body = ((task.title or "") + " " + (task.body or "")).strip() + if not _REVIEW_KEYWORD_RE.search(title_body): + return None + if result and _VERDICT_PREFIX_RE.match(result): + return None + return ( + f"kanban_complete blocked: task {task.id} is a review task " + f"(assignee={assignee}) and requires a verdict. " + f"Pass `result=\"verdict: approve\"` or " + f"`result=\"verdict: reject\"` (with reasons in metadata). " + f"Your task is still in-flight (no state change) — retry " + f"kanban_complete with the verdict prefix on result. " + f"See kanban-worker SKILL.md § Reviewer Verdict Convention." + ) + + +def _check_evidence_paths_gate(task) -> Optional[str]: + """If the task body declares ``required_evidence_paths:``, verify them. + + Returns None on pass; an error sentence listing failed paths on reject. + """ + if not task or not task.body: + return None + declared = _extract_yaml_list(task.body, _REQUIRED_EVIDENCE_KEY) + if not declared: + return None + workspace = task.workspace_path or os.environ.get("HERMES_KANBAN_WORKSPACE") + failures: list[str] = [] + for raw in declared: + p = Path(os.path.expanduser(raw)) + if not p.is_absolute(): + base = Path(workspace) if workspace else Path.home() + p = base / p + if not p.exists(): + failures.append(f"{raw} (missing)") + continue + try: + if p.is_dir(): + if not any(p.iterdir()): + failures.append(f"{raw} (empty directory)") + elif p.stat().st_size == 0: + failures.append(f"{raw} (empty file)") + except OSError as e: + failures.append(f"{raw} (stat failed: {e})") + if not failures: + return None + return ( + f"kanban_complete blocked: task {task.id} declares " + f"required_evidence_paths but {len(failures)} of " + f"{len(declared)} are missing or empty: " + f"{', '.join(failures[:5])}" + f"{' ...' if len(failures) > 5 else ''}. " + f"Your task is still in-flight (no state change) — produce " + f"the artifact(s) listed in the task body, OR kanban_block " + f"with `reason: 'evidence: could not be produced because " + f"'` if production failed." + ) + + +def _check_keep_running_gate(kb, conn, task) -> Optional[str]: + """If the task body declares ``keep_running: true``, refuse completion + while any other non-terminal task exists in the umbrella's tenant. + + v6.6 changed the walk from ``task_links`` to tenant-scoped because + Pepper-shaped chains don't link build tasks back to the umbrella — + Shuri/Vision/Friday tasks have ``parents=[chain-predecessor]`` not + ``parents=[umbrella, ...]``. The v6.5.1 first-test showed the gate + finding only {Banner, Pepper} via task_links and missing all the + actual build/review work running in the tenant. JARVIS gamed the + gate by blocking with ``"awaiting-async-event"``. Tenant scoping + closes the route — any live task in the tenant counts as a live + descendant for the umbrella's purposes. + + The umbrella must have a tenant set (orchestration umbrellas should + always have one). If not, fall back to task_links walking — the + v6.5 behavior — so single-board / no-tenant deployments still get + the gate. + + Returns None on pass; an error sentence listing live descendants on + reject. + """ + if not task or not task.body: + return None + scalar = _extract_yaml_scalar(task.body, _KEEP_RUNNING_KEY) + if not scalar or scalar.lower() not in ("true", "yes", "1"): + return None + terminal_states = {"done", "archived", "cancelled"} + live: list[tuple[str, str, str]] = [] # (id, status, assignee) + tenant = getattr(task, "tenant", None) + if tenant: + # v6.6 — tenant-scoped walk. Find every non-terminal task in the + # umbrella's tenant other than the umbrella itself. + rows = conn.execute( + "SELECT id, status, assignee FROM tasks " + "WHERE tenant = ? AND id != ? AND status NOT IN " + "('done', 'archived', 'cancelled') " + "LIMIT 20", + (tenant, task.id), + ).fetchall() + for row in rows: + if len(live) >= 10: + break + live.append((row[0], row[1] or "?", row[2] or "?")) + if not live: + return None + summary = ", ".join( + f"{cid} ({assignee}: {status})" for cid, status, assignee in live[:5] + ) + return ( + f"kanban_complete blocked: task {task.id} is a keep_running " + f"umbrella and {len(live)} task(s) in tenant '{tenant}' are " + f"still non-terminal: {summary}" + f"{' ...' if len(live) > 5 else ''}. " + f"Your task is still in-flight (no state change) — orchestrate " + f"the remaining tenant tasks to terminal (done/archived/" + f"cancelled) before completing the umbrella. See " + f"kanban-orchestration SKILL.md § JARVIS as Event-Driven " + f"Orchestrator." + ) + # Legacy fallback — walk descendants via task_links. Used when the + # umbrella has no tenant (single-board deployments). + visited: set[str] = set() + queue = [task.id] + while queue and len(live) < 10: + node_id = queue.pop(0) + if node_id in visited: + continue + visited.add(node_id) + rows = conn.execute( + "SELECT id, status, assignee FROM tasks " + "WHERE id IN (SELECT child_id FROM task_links WHERE parent_id = ?)", + (node_id,), + ).fetchall() + for row in rows: + cid, status, assignee = row[0], row[1], row[2] + if status not in terminal_states: + live.append((cid, status or "?", assignee or "?")) + queue.append(cid) + if not live: + return None + summary = ", ".join(f"{cid} ({assignee}: {status})" for cid, status, assignee in live[:5]) + return ( + f"kanban_complete blocked: task {task.id} is a keep_running " + f"umbrella and {len(live)} descendant(s) are still non-terminal: " + f"{summary}{' ...' if len(live) > 5 else ''}. " + f"Your task is still in-flight (no state change) — orchestrate " + f"the remaining children to terminal (done/archived/cancelled) " + f"before completing the umbrella. See kanban-orchestration " + f"SKILL.md § JARVIS as Standing Watcher." + ) + + # --------------------------------------------------------------------------- # Gating # --------------------------------------------------------------------------- @@ -553,6 +813,19 @@ def _handle_complete(args: dict, **kw) -> str: try: kb, conn = _connect(board=board) try: + # v6.3 completion gates — run BEFORE kb.complete_task so the + # task is not mutated when a gate rejects. The agent retries + # by addressing the gate (produce the artifact, supply the + # verdict, finish the children) and calling kanban_complete + # again with the same summary/metadata. + task = kb.get_task(conn, tid) + for gate in ( + _check_reviewer_verdict_gate(task, result), + _check_evidence_paths_gate(task), + _check_keep_running_gate(kb, conn, task), + ): + if gate: + return tool_error(gate) try: ok = kb.complete_task( conn, tid, @@ -612,6 +885,22 @@ def _handle_block(args: dict, **kw) -> str: try: kb, conn = _connect(board=board) try: + # v6.3.1 hotfix — the keep_running gate now fires on BLOCK + # as well as COMPLETE. In v6.3 first-test (2026-06-06) JARVIS + # routed around the completion gate by calling kanban_block + # on the umbrella instead of kanban_complete. The block + # transitioned to "blocked", which the dispatcher treats as + # non-promotable for children, so the chain stalled with + # Banner stuck in `todo` for 30+ minutes. The other two gates + # (verdict, evidence-path) stay completion-only — a reviewer + # blocking on infra and a builder blocking honestly on + # missing artifacts are both legitimate uses of kanban_block. + task = kb.get_task(conn, tid) + keep_running_err = _check_keep_running_gate(kb, conn, task) + if keep_running_err: + # Reuse the gate's prose; the agent's next move is to + # advance descendants to terminal, not to retry block. + return tool_error(keep_running_err) ok = kb.block_task( conn, tid, reason=reason,