diff --git a/hermes_cli/active_sessions.py b/hermes_cli/active_sessions.py index 4f21e6cd9a..015b8a636f 100644 --- a/hermes_cli/active_sessions.py +++ b/hermes_cli/active_sessions.py @@ -107,11 +107,8 @@ def active_session_limit_message( max_sessions: int, entries: Optional[list[dict[str, Any]]] = None, ) -> str: - # Name the holders: the slots are shared across CLI, desktop/TUI and the - # messaging gateway, so the surface that gets rejected is usually NOT the - # one squatting on them (idle desktop chats starving a Discord bot, say). - # Without this the message is unactionable and the only way to find out is - # reading runtime/active_sessions.json by hand. + # Name the holders: slots are shared across CLI, desktop/TUI and gateway, + # so the rejected surface is usually NOT the one squatting on them. held = summarize_holders(entries or []) detail = f" Held by: {held}." if held else "" return ( @@ -124,42 +121,25 @@ def _registry_home(registry_home: str | Path | None = None) -> Path: return Path(registry_home) if registry_home is not None else Path(get_hermes_home()) -# WHY A REFUSAL IS REFUSED, in a form a caller can branch on. -# -# The two refusals mean opposite things to an automated client. Capacity is -# "the machine is busy, come back later". Ownership is "this specific session -# has a live owner, and writing to it would interleave with theirs". -# -# Callers used to have only the human-readable message, so anything that needed -# to DECIDE had to match prose -- which silently changes meaning whenever the -# wording is improved. The reason is the contract; the message is for people. +# Machine-readable refusal reasons: the reason is the contract, the message is +# for people. Capacity = "busy, come back later"; ownership = "this session +# has a live owner and writing would interleave with theirs". SESSION_NOT_OWNED = "SESSION_NOT_OWNED" MAX_CONCURRENT_SESSIONS = "MAX_CONCURRENT_SESSIONS" -# Ownership could not be PROVEN either way: the registry was unreadable or -# corrupt. Distinct from SESSION_NOT_OWNED on purpose -- "someone else owns -# this" and "I cannot tell who owns this" call for different operator action, -# and collapsing the second into a silent go-ahead is exactly the fail-open -# hole that let two writers share one session (#94595 review, blocker 2). +# Ownership could not be PROVEN (registry unreadable/corrupt). Deliberately +# distinct from SESSION_NOT_OWNED: collapsing "can't tell" into a silent +# go-ahead is the fail-open hole that let two writers share one session. SESSION_COORDINATION_UNAVAILABLE = "SESSION_COORDINATION_UNAVAILABLE" -# Advertised through the gateway so a client can tell a build that enforces -# per-session exclusivity from one that does not. -# -# A module constant rather than a config flag, deliberately: it is true because -# try_acquire_active_session below performs the check atomically, so it cannot -# be turned on by an operator who has not got the enforcement, and cannot drift -# out of step with it without this file changing. +# Advertised through the gateway. A module constant, not a config flag: it is +# true because try_acquire_active_session checks atomically, so it cannot drift +# out of step with the enforcement without this file changing. PER_SESSION_EXCLUSIVE_SUBMIT = True class ActiveSessionRefusal(str): - """A refusal message that also carries a machine-readable ``reason``. - - A ``str`` subclass so every existing caller keeps working untouched -- they - format it, hand it back as a JSON-RPC message, or just test it for None -- - while a caller that must act on WHICH refusal happened reads ``.reason`` - instead of matching the wording. - """ + """A refusal message (``str`` subclass, so existing callers are untouched) + that also carries a machine-readable ``reason``.""" reason: str @@ -170,13 +150,10 @@ class ActiveSessionRefusal(str): def _is_same_writer(entry: dict[str, Any], metadata: Optional[dict[str, Any]]) -> bool: - """True when an existing lease belongs to the very caller now re-acquiring it. - - Both halves are required. A pid alone would let two live sessions in one - process steal each other's lease -- which is a real hazard, since each holds - its own snapshot of the transcript. A live session id alone would let another - process with a coincidentally equal id do the same. - """ + """True when an existing lease belongs to the very caller re-acquiring it. + Identity is (pid, live_session_id): pid alone would let two live sessions in + one process steal each other's lease; the live id alone would let another + process with an equal id do the same.""" try: if int(entry.get("pid") or -1) != os.getpid(): return False @@ -223,6 +200,19 @@ def _lease_paths( return _state_path(home), _lock_path(home) +def _flock(fh, *, lock: bool) -> None: + """Exclusive whole-file lock/unlock on ``fh`` (fcntl on POSIX, msvcrt on Windows).""" + if os.name == "nt": + import msvcrt + + fh.seek(0) + msvcrt.locking(fh.fileno(), msvcrt.LK_LOCK if lock else msvcrt.LK_UNLCK, 1) + else: + import fcntl + + fcntl.flock(fh.fileno(), fcntl.LOCK_EX if lock else fcntl.LOCK_UN) + + class _FileLock: def __init__(self, path: Path): self.path = path @@ -231,45 +221,21 @@ class _FileLock: def __enter__(self): self.path.parent.mkdir(parents=True, exist_ok=True) self._fh = open(self.path, "a+b") - if os.name == "nt": - try: - import msvcrt - - self._fh.seek(0) - msvcrt.locking(self._fh.fileno(), msvcrt.LK_LOCK, 1) - except Exception as exc: - self._fh.close() - self._fh = None - raise RuntimeError("active session file lock unavailable") from exc - else: - try: - import fcntl - - fcntl.flock(self._fh.fileno(), fcntl.LOCK_EX) - except Exception as exc: - self._fh.close() - self._fh = None - raise RuntimeError("active session file lock unavailable") from exc + try: + _flock(self._fh, lock=True) + except Exception as exc: + self._fh.close() + self._fh = None + raise RuntimeError("active session file lock unavailable") from exc return self def __exit__(self, exc_type, exc, tb): if self._fh is None: return - if os.name == "nt": - try: - import msvcrt - - self._fh.seek(0) - msvcrt.locking(self._fh.fileno(), msvcrt.LK_UNLCK, 1) - except Exception: - pass - else: - try: - import fcntl - - fcntl.flock(self._fh.fileno(), fcntl.LOCK_UN) - except Exception: - pass + try: + _flock(self._fh, lock=False) + except Exception: + pass try: self._fh.close() finally: @@ -306,58 +272,51 @@ def _read_entries(path: Path, *, strict: bool = False) -> list[dict[str, Any]]: seen_leases: set[str] = set() for entry in valid: lease_id = entry.get("lease_id") - session_id = entry.get("session_id") - pid = entry.get("pid") - if not isinstance(lease_id, str) or not lease_id.strip(): - raise ActiveSessionRegistryError( - f"active session registry contains an invalid lease id: {path}" - ) - if lease_id in seen_leases: - raise ActiveSessionRegistryError( - f"active session registry contains a duplicate lease id: {path}" - ) - seen_leases.add(lease_id) - if not isinstance(session_id, str) or not session_id.strip(): - raise ActiveSessionRegistryError( - f"active session registry contains an invalid session id: {path}" - ) - if isinstance(pid, bool) or not isinstance(pid, (int, str)): - pid_int = 0 - else: - try: - pid_int = int(pid) - except (TypeError, ValueError): - pid_int = 0 - if pid_int <= 0: - raise ActiveSessionRegistryError( - f"active session registry contains an invalid pid: {path}" - ) - surface = entry.get("surface") - if surface is not None and not isinstance(surface, str): - raise ActiveSessionRegistryError( - f"active session registry contains an invalid surface: {path}" - ) - tracked = entry.get("track_liveness") - if tracked is not None and not isinstance(tracked, bool): - raise ActiveSessionRegistryError( - f"active session registry contains an invalid liveness marker: {path}" - ) - metadata = entry.get("metadata") - if metadata is not None and not isinstance(metadata, dict): - raise ActiveSessionRegistryError( - f"active session registry contains invalid metadata: {path}" - ) - process_start = entry.get("process_start_time") - parsed_process_start = _optional_float(process_start) - if process_start not in (None, "") and ( - parsed_process_start is None or not math.isfinite(parsed_process_start) + # (problem-if-True predicate, message fragment) — checked lazily, in + # this order, so an unhashable lease id is reported before the dup check. + for bad, what in ( + (lambda: not _nonblank_str(lease_id), "an invalid lease id"), + (lambda: lease_id in seen_leases, "a duplicate lease id"), + (lambda: not _nonblank_str(entry.get("session_id")), "an invalid session id"), + (lambda: _registry_pid(entry.get("pid")) <= 0, "an invalid pid"), + (lambda: not _optional_isinstance(entry.get("surface"), str), "an invalid surface"), + (lambda: not _optional_isinstance(entry.get("track_liveness"), bool), "an invalid liveness marker"), + (lambda: not _optional_isinstance(entry.get("metadata"), dict), "invalid metadata"), + (lambda: not _valid_process_start(entry.get("process_start_time")), "an invalid process start time"), ): - raise ActiveSessionRegistryError( - f"active session registry contains an invalid process start time: {path}" - ) + if bad(): + raise ActiveSessionRegistryError( + f"active session registry contains {what}: {path}" + ) + seen_leases.add(lease_id) return valid +def _nonblank_str(v: Any) -> bool: + return isinstance(v, str) and bool(v.strip()) + + +def _optional_isinstance(v: Any, typ) -> bool: + return v is None or isinstance(v, typ) + + +def _registry_pid(pid: Any) -> int: + """Registry pid as int; 0 for bools, non-int/str, or unparseable values.""" + if isinstance(pid, bool) or not isinstance(pid, (int, str)): + return 0 + try: + return int(pid) + except (TypeError, ValueError): + return 0 + + +def _valid_process_start(v: Any) -> bool: + if v in (None, ""): + return True + parsed = _optional_float(v) + return parsed is not None and math.isfinite(parsed) + + def _write_entries(path: Path, entries: list[dict[str, Any]]) -> None: path.parent.mkdir(parents=True, exist_ok=True) tmp = path.with_name(f"{path.name}.{os.getpid()}.{uuid.uuid4().hex}.tmp") @@ -392,20 +351,24 @@ def _optional_float(value: Any) -> Optional[float]: return None -def _pid_liveness(pid: Any, process_start_time: Any = None) -> Optional[bool]: - """Return True/False for live/dead, or None when liveness is unknowable.""" +def _pid_liveness(pid: Any, process_start_time: Any = None, *, lenient: bool = False) -> Optional[bool]: + """Return True/False for live/dead, or None when liveness is unknowable. + + ``lenient`` never returns None: an unparseable pid or a failed existence + probe counts as dead, an unreadable current start time as alive. + """ try: pid_int = int(pid) except (TypeError, ValueError): - return None + return False if lenient else None if pid_int <= 0: - return None + return False if lenient else None try: from gateway.status import _pid_exists exists = bool(_pid_exists(pid_int)) except Exception: - return None + return False if lenient else None if not exists: return False expected_start = _optional_float(process_start_time) @@ -413,32 +376,12 @@ def _pid_liveness(pid: Any, process_start_time: Any = None) -> Optional[bool]: return True current_start = _process_start_time(pid_int) if current_start is None: - return None + return True if lenient else None return abs(current_start - expected_start) < 0.001 def _pid_alive(pid: Any, process_start_time: Any = None) -> bool: - try: - pid_int = int(pid) - except (TypeError, ValueError): - return False - if pid_int <= 0: - return False - try: - from gateway.status import _pid_exists - - exists = bool(_pid_exists(pid_int)) - except Exception: - return False - if not exists: - return False - expected_start = _optional_float(process_start_time) - if expected_start is None: - return True - current_start = _process_start_time(pid_int) - if current_start is None: - return True - return abs(current_start - expected_start) < 0.001 + return bool(_pid_liveness(pid, process_start_time, lenient=True)) def _prune_dead( @@ -468,12 +411,9 @@ class ActiveSessionLease: surface: str enabled: bool = True released: bool = False - # Registry paths pinned at acquisition time. A lease acquired under the - # root ``HERMES_HOME`` must release against the same registry even when - # ``release()`` runs inside a profile home override (native multiplex - # routes turns under ``_profile_runtime_scope``), otherwise the root - # entry survives until process exit and the session cap fills with - # phantom leases (#85431). + # Pinned at acquisition: a lease acquired under the root HERMES_HOME must + # release against the same registry even when release() runs inside a + # profile-home override, or phantom leases fill the session cap. state_path: Optional[Path] = None lock_path: Optional[Path] = None track_liveness: bool = False @@ -484,6 +424,33 @@ class ActiveSessionLease: release_active_session(self) +def _clean_metadata(metadata: dict[str, Any]) -> dict[str, Any]: + return {str(k): v for k, v in metadata.items() if isinstance(k, str)} + + +def _without_lease(entries: list[dict[str, Any]], lease_id: str) -> list[dict[str, Any]]: + return [e for e in entries if str(e.get("lease_id") or "") != lease_id] + + +def _read_live_entries( + state_path: Path, *, track_liveness: bool, warn: str, +) -> Optional[tuple[list[dict[str, Any]], list[dict[str, Any]]]]: + """``(raw, pruned)`` from the registry, or None when it is unreadable. + + Liveness-tracked callers re-raise instead: they must not proceed on an + unprovable registry. Untracked callers get ``warn`` logged and decide + how to degrade themselves. + """ + try: + raw_entries = _read_entries(state_path, strict=True) + return raw_entries, _prune_dead(raw_entries, strict=track_liveness) + except ActiveSessionRegistryError: + if track_liveness: + raise + logger.warning(warn) + return None + + def _lease_entry( *, lease_id: str, @@ -505,9 +472,7 @@ def _lease_entry( if track_liveness: entry["track_liveness"] = True if metadata: - entry["metadata"] = { - str(k): v for k, v in metadata.items() if isinstance(k, str) - } + entry["metadata"] = _clean_metadata(metadata) return entry @@ -522,27 +487,20 @@ def try_acquire_active_session( ) -> tuple[Optional[ActiveSessionLease], Optional[str]]: """Acquire an active-session slot. - Per-session exclusivity is CORRECTNESS and is enforced unconditionally: - at most one live owner may run a given stored session, whether or not an - operator configured ``max_concurrent_sessions`` (#94595). The concurrency - cap remains a resource POLICY and applies only when configured. Liveness - tracking keeps richer desktop lifecycle semantics; ``registry_home`` lets - profile-scoped backends share the owning profile's registry even when - launched from another home. + Per-session exclusivity is CORRECTNESS, enforced unconditionally: at most + one live owner per stored session. ``max_concurrent_sessions`` is resource + POLICY and applies only when configured. ``registry_home`` lets + profile-scoped backends share the owning profile's registry. - Returns ``(lease, None)`` on success and ``(None, refusal)`` otherwise, - where ``refusal`` is an :class:`ActiveSessionRefusal` carrying a - machine-readable ``reason``. Ownership uncertainty fails CLOSED: when the - registry cannot be read, the caller gets ``SESSION_COORDINATION_UNAVAILABLE`` - rather than a silent go-ahead that could reopen the double-writer state. + Returns ``(lease, None)`` or ``(None, ActiveSessionRefusal)``. Ownership + uncertainty fails CLOSED with ``SESSION_COORDINATION_UNAVAILABLE``. """ max_sessions = resolve_max_concurrent_sessions(config) lease_id = uuid.uuid4().hex key = str(session_id or "") - # A session with no stored id yet cannot collide with another writer, and - # the strict registry schema (rightly) refuses entries with empty session - # ids. Nothing to fence, nothing to record: hand back a no-op lease. + # No stored id yet => nothing to fence or record (and the strict schema + # refuses empty session ids): hand back a no-op lease. if not key and not track_liveness: return ActiveSessionLease( lease_id=lease_id, @@ -560,22 +518,23 @@ def try_acquire_active_session( ) state_path, lock_path = _lease_paths(registry_home=registry_home) + lease = ActiveSessionLease( + lease_id=lease_id, + session_id=key, + surface=str(surface), + state_path=state_path, + lock_path=lock_path, + track_liveness=track_liveness, + ) with _FileLock(lock_path): - try: - raw_entries = _read_entries(state_path, strict=True) - entries = _prune_dead(raw_entries, strict=track_liveness) - except ActiveSessionRegistryError: - if track_liveness: - raise - # A capacity cap could afford to degrade open -- worst case, more - # sessions than the operator wanted. Exclusivity cannot: "could not - # prove ownership" must never be collapsed into "no owner exists", - # because that silently reopens the exact double-writer state this - # fence guarantees against. Refuse, and say which file to fix. - logger.warning( - "Active-session registry is unavailable; refusing the session " - "rather than risking a concurrent writer" - ) + # A capacity cap could degrade open; exclusivity cannot: "could not + # prove ownership" must never become "no owner exists". + loaded = _read_live_entries( + state_path, track_liveness=track_liveness, + warn="Active-session registry is unavailable; refusing the session " + "rather than risking a concurrent writer", + ) + if loaded is None: return None, ActiveSessionRefusal( ( "Hermes could not read the active-session registry at " @@ -584,47 +543,27 @@ def try_acquire_active_session( ), SESSION_COORDINATION_UNAVAILABLE, ) + raw_entries, entries = loaded pruned = len(raw_entries) - len(entries) if pruned: logger.info("Pruned %d stale active session lease(s)", pruned) - # Correctness first, and under the same lock that just pruned the dead - # owners -- so an owner that died is never mistaken for one that is - # running, and a live one is never overlooked. - # - # An empty key is exempt: a session with no stored id yet cannot collide - # with another, and treating "" as an identity would make every unsaved - # draft exclude every other one. + # Correctness first, under the same lock that just pruned dead owners. + # An empty key is exempt: treating "" as an identity would make every + # unsaved draft exclude every other one. if key: for index, existing in enumerate(entries): if str(existing.get("session_id") or "") != key: continue - # THE SAME WRITER IS NOT A SECOND WRITER. - # - # A live session that lost its lease reference -- its record was - # rebuilt in place, so the object holding the lease is unreachable - # while the session itself is still the one being driven -- would - # otherwise be fenced out of its own session by its own leak, and - # permanently: pruning only removes entries whose PROCESS is dead, - # and this process is very much alive. - # - # Identity here is (pid, live session id). Two processes never - # match, because their pids differ. Two live sessions inside one - # process never match, because their live ids differ. Only the - # exact same writer re-acquiring its own session matches, and that - # is re-entrancy rather than a concurrent writer. + # The same writer is not a second writer: a live session that + # leaked its lease reference would otherwise be fenced out of + # its own session permanently (pruning only removes entries + # whose PROCESS is dead). Re-entrancy, not concurrency. if _is_same_writer(existing, metadata): entries[index] = entry _write_entries(state_path, entries) - return ActiveSessionLease( - lease_id=lease_id, - session_id=key, - surface=str(surface), - state_path=state_path, - lock_path=lock_path, - track_liveness=track_liveness, - ), None + return lease, None _write_entries(state_path, entries) logger.info( @@ -656,14 +595,7 @@ def try_acquire_active_session( entries.append(entry) _write_entries(state_path, entries) - return ActiveSessionLease( - lease_id=lease_id, - session_id=key, - surface=str(surface), - state_path=state_path, - lock_path=lock_path, - track_liveness=track_liveness, - ), None + return lease, None def release_active_session(lease: ActiveSessionLease) -> None: @@ -673,25 +605,16 @@ def release_active_session(lease: ActiveSessionLease) -> None: with _FileLock(lock_path): if lease.released: return - try: - raw_entries = _read_entries(state_path, strict=True) - entries = _prune_dead(raw_entries, strict=lease.track_liveness) - except ActiveSessionRegistryError: - if lease.track_liveness: - raise - logger.warning( - "Active-session registry is unavailable; preserving it while " - "releasing an untracked lease" - ) - lease.released = True - return - kept = [ - entry - for entry in entries - if str(entry.get("lease_id") or "") != lease.lease_id - ] - if len(kept) != len(entries): - _write_entries(state_path, kept) + loaded = _read_live_entries( + state_path, track_liveness=lease.track_liveness, + warn="Active-session registry is unavailable; preserving it while " + "releasing an untracked lease", + ) + if loaded is not None: + entries = loaded[1] + kept = _without_lease(entries, lease.lease_id) + if len(kept) != len(entries): + _write_entries(state_path, kept) lease.released = True @@ -717,17 +640,14 @@ def transfer_active_session( # thread acquired the file lock. Never resurrect a durably removed lease. if lease.released: return False - try: - raw_entries = _read_entries(state_path, strict=True) - entries = _prune_dead(raw_entries, strict=lease.track_liveness) - except ActiveSessionRegistryError: - if lease.track_liveness: - raise - logger.warning( - "Active-session registry is unavailable; refusing to overwrite " - "it during lease transfer" - ) + loaded = _read_live_entries( + state_path, track_liveness=lease.track_liveness, + warn="Active-session registry is unavailable; refusing to overwrite " + "it during lease transfer", + ) + if loaded is None: return False + entries = loaded[1] updated = False for entry in entries: if str(entry.get("lease_id") or "") != lease.lease_id: @@ -735,9 +655,7 @@ def transfer_active_session( entry["session_id"] = new_session_id entry["updated_at"] = time.time() if metadata: - entry["metadata"] = { - str(k): v for k, v in metadata.items() if isinstance(k, str) - } + entry["metadata"] = _clean_metadata(metadata) updated = True break if not updated and lease.track_liveness: @@ -760,12 +678,10 @@ def transfer_active_session( def release_orphaned_leases(live_lease_ids: set[str]) -> int: """Drop this process's registry entries that no live session owns. - ``_prune_dead`` only reclaims leases whose owning process died. A server - that runs for days (``hermes dashboard`` / ``serve``) never trips that - check, so a lease whose session skipped teardown is held until restart. - The owning process is the only authority on which of its own leases are - real, so it drops the rest itself — exact, with no heartbeat write on the - turn path and no staleness threshold to tune. + ``_prune_dead`` only reclaims leases of dead processes; a days-long server + never trips it, so a lease whose session skipped teardown is held until + restart. The owning process is the only authority on its own leases — + exact, no heartbeat on the turn path, no staleness threshold. """ pid = os.getpid() state_path = _state_path() @@ -774,14 +690,13 @@ def release_orphaned_leases(live_lease_ids: set[str]) -> int: if not state_path.exists(): return 0 with _FileLock(_lock_path()): - try: - raw_entries = _read_entries(state_path, strict=True) - entries = _prune_dead(raw_entries) - except ActiveSessionRegistryError: - logger.warning( - "Active-session registry is unavailable; skipping orphaned-lease sweep" - ) + loaded = _read_live_entries( + state_path, track_liveness=False, + warn="Active-session registry is unavailable; skipping orphaned-lease sweep", + ) + if loaded is None: return 0 + entries = loaded[1] kept = [ entry for entry in entries @@ -813,12 +728,9 @@ def active_session_liveness_guard( *, registry_home: str | Path | None = None, ) -> Iterator[bool]: - """Hold the registry lock while reporting whether ``session_id`` is leased. - - Keeping the lock across the caller's lifecycle mutation prevents a new - backend from acquiring a lease and reopening the row between the liveness - check and the corresponding ``end_session`` write. - """ + """Hold the registry lock while reporting whether ``session_id`` is leased, + so no new backend can acquire a lease between the check and the caller's + ``end_session`` write.""" target = str(session_id or "") state_path, lock_path = _lease_paths(registry_home=registry_home) with _FileLock(lock_path): @@ -834,12 +746,9 @@ def release_active_session_liveness_guard( lease: ActiveSessionLease, session_id: str, ) -> Iterator[bool]: - """Remove ``lease`` and hold its registry lock through a lifecycle write. - - This makes automatic cleanup one atomic ownership decision: the local - runtime disappears, sibling liveness is checked, and the caller may end the - durable row before any new backend can acquire/reopen it. - """ + """Remove ``lease`` and hold its registry lock through a lifecycle write, + making cleanup one atomic ownership decision (release, check siblings, + end the durable row) before any new backend can reopen it.""" if not lease.enabled or lease.released: with active_session_liveness_guard( session_id, registry_home=_registry_home_for_lease(lease) @@ -850,13 +759,8 @@ def release_active_session_liveness_guard( target = str(session_id or "") state_path, lock_path = _lease_paths(lease) with _FileLock(lock_path): - raw_entries = _read_entries(state_path, strict=True) - entries = _prune_dead(raw_entries, strict=True) - kept = [ - entry - for entry in entries - if str(entry.get("lease_id") or "") != lease.lease_id - ] + entries = _prune_dead(_read_entries(state_path, strict=True), strict=True) + kept = _without_lease(entries, lease.lease_id) if len(kept) != len(entries): _write_entries(state_path, kept) lease.released = True diff --git a/hermes_cli/kanban.py b/hermes_cli/kanban.py index 7037941a46..806403224e 100644 --- a/hermes_cli/kanban.py +++ b/hermes_cli/kanban.py @@ -1,15 +1,8 @@ """CLI for the Hermes Kanban board — ``hermes kanban …`` subcommand. -Exposes the full Kanban command surface documented in the design spec -(``docs/hermes-kanban-v1-spec.pdf``). All DB work is delegated to -``kanban_db``. This module adds: - - * Argparse subcommand construction (``build_parser``). - * Argument dispatch (``kanban_command``). - * Output formatting (plain text + ``--json``). - * A short shared helper that parses a single slash-style string - (used by ``/kanban …`` in CLI and gateway) and forwards it to the - argparse surface. +All DB work is delegated to ``kanban_db``; this module adds argparse +construction (``build_parser``), dispatch (``kanban_command``), text/``--json`` +output, and ``run_slash`` for ``/kanban …`` from the CLI and gateway. """ from __future__ import annotations @@ -49,6 +42,57 @@ def _fmt_ts(ts: Optional[int]) -> str: return time.strftime("%Y-%m-%d %H:%M", time.localtime(ts)) +def _print_json(obj: Any, *, ascii: bool = False) -> None: + print(json.dumps(obj, indent=2, ensure_ascii=ascii)) + + +def _json_out(args: argparse.Namespace, obj: Any, *, ascii: bool = False) -> bool: + """Print ``obj`` as JSON and return True when ``--json`` was passed.""" + if not getattr(args, "json", False): + return False + _print_json(obj, ascii=ascii) + return True + + +def _fmt_counts(counts: dict, empty: str = "") -> str: + return ", ".join(f"{k}={v}" for k, v in sorted(counts.items())) or empty + + +def _err(msg: str, rc: int = 1) -> int: + print(msg, file=sys.stderr) + return rc + + +def _none_profile(value: str) -> Optional[str]: + """``none`` / ``-`` / ``null`` mean "unassign".""" + return None if value.lower() in {"none", "-", "null"} else value + + +def _parse_metadata_flag(raw: Optional[str]) -> tuple[Optional[dict], int]: + """Parse ``--metadata`` JSON; returns ``(dict|None, rc)`` with rc=2 on error.""" + if not raw: + return None, 0 + try: + metadata = json.loads(raw) + if not isinstance(metadata, dict): + raise ValueError("must be a JSON object") + except (ValueError, json.JSONDecodeError) as exc: + return None, _err(f"kanban: --metadata: {exc}", 2) + return metadata, 0 + + +def _bulk_apply(ids, op, ok_msg, fail_msg) -> int: + """Run ``op(tid) -> bool`` per id, print ok/fail lines, exit 1 if any failed.""" + failed = False + for tid in ids: + if not op(tid): + failed = True + print(fail_msg(tid), file=sys.stderr) + else: + print(ok_msg(tid)) + return 1 if failed else 0 + + def _fmt_task_line(t: kb.Task) -> str: icon = _STATUS_ICONS.get(t.status, "?") assignee = t.assignee or "(unassigned)" @@ -56,32 +100,34 @@ def _fmt_task_line(t: kb.Task) -> str: return f"{icon} {t.id} {t.status:8s} {assignee:20s}{tenant} {t.title}" +_TASK_DICT_FIELDS = ( + "id", "title", "body", "assignee", "status", "priority", "tenant", + "workspace_kind", "workspace_path", "branch_name", "project_id", + "created_by", "created_at", "started_at", "completed_at", "result", + "skills", "max_retries", "model_override", "provider_override", + "session_id", "workflow_template_id", "current_step_key", +) +_SHOW_RUN_FIELDS = ( + "id", "profile", "step_key", "status", "outcome", "summary", "error", + "metadata", "worker_pid", "started_at", "ended_at", +) +_RUNS_RUN_FIELDS = ( + "id", "profile", "status", "outcome", "started_at", "ended_at", + "summary", "error", "metadata", "worker_pid", "step_key", +) +_ATTACHMENT_FIELDS = ( + "id", "filename", "content_type", "size", "uploaded_by", "stored_path", "created_at", +) + + +def _obj_dict(obj: Any, fields: tuple[str, ...]) -> dict[str, Any]: + return {k: getattr(obj, k) for k in fields} + + def _task_to_dict(t: kb.Task) -> dict[str, Any]: - return { - "id": t.id, - "title": t.title, - "body": t.body, - "assignee": t.assignee, - "status": t.status, - "priority": t.priority, - "tenant": t.tenant, - "workspace_kind": t.workspace_kind, - "workspace_path": t.workspace_path, - "branch_name": t.branch_name, - "project_id": t.project_id, - "created_by": t.created_by, - "created_at": t.created_at, - "started_at": t.started_at, - "completed_at": t.completed_at, - "result": t.result, - "skills": list(t.skills) if t.skills else [], - "max_retries": t.max_retries, - "model_override": t.model_override, - "provider_override": t.provider_override, - "session_id": t.session_id, - "workflow_template_id": t.workflow_template_id, - "current_step_key": t.current_step_key, - } + d = _obj_dict(t, _TASK_DICT_FIELDS) + d["skills"] = list(t.skills) if t.skills else [] + return d def _run_state_kwargs(args: argparse.Namespace) -> Optional[dict[str, str]]: @@ -136,47 +182,33 @@ def _parse_branch_flag(value: Optional[str]) -> Optional[str]: def _check_dispatcher_presence( hermes_home: Optional[Path] = None, ) -> tuple[bool, str]: - """Return ``(running, message)``. + """Return ``(running, message)`` for the "will anything dispatch this?" warning. - - ``running=True``: a gateway is alive for this HERMES_HOME and its - config has ``kanban.dispatch_in_gateway`` on (default). Message - is a short status line. - - ``running=False``: either no gateway is running, or the gateway - is running but the config flag is off. Message is human guidance - explaining the next step. + ``running=True`` when a gateway is alive for this HERMES_HOME with + ``kanban.dispatch_in_gateway`` on (message is a status line); otherwise + ``False`` with human guidance. Fails OPEN — import/probe/config errors + return ``(True, "")`` — since a missed warning beats crying wolf. - Used by ``hermes kanban create`` (and callers) to warn when a task - will sit in ``ready`` because nothing is there to pick it up. - Defensive against import failures and config-read errors — if the - probe itself errors, we return ``(True, "")`` so we don't spam - false warnings (better to miss a warning than to cry wolf). - - ``hermes_home`` scopes the probe to a named profile's directory. The - dashboard plugin API passes it because the dashboard backend process can - be running under a different HERMES_HOME than the profile the request - targets, which otherwise produced a "no gateway is running" warning - against a perfectly healthy profile gateway (#71211). CLI callers leave - it ``None`` and keep the existing process-level behavior. + ``hermes_home`` scopes the probe to a profile's directory: the dashboard + backend may run under a different HERMES_HOME than the profile it serves, + which otherwise misreports a healthy gateway as absent. CLI callers pass + ``None``. """ try: from gateway.status import resolve_gateway_liveness # type: ignore except Exception: return (True, "") # can't probe — silent try: - # Same shared ladder the dashboard status endpoints use, so a - # PID-file-less (launch-service-managed) or cross-container gateway - # is not misreported as absent. use_cache=False: this is a one-shot - # CLI/create-time probe, not a polling loop, and it must observe the - # gateway's state right now rather than a cached snapshot. + # Same ladder the dashboard status endpoints use, so PID-file-less or + # cross-container gateways aren't misreported. use_cache=False: this + # one-shot probe must see the gateway's state right now. liveness = resolve_gateway_liveness( profile_dir=hermes_home, use_cache=False ) except Exception: return (True, "") # probe errored — silent if liveness.probe_error: - # The resolver swallows per-rung failures so status endpoints never - # 500. This caller must still fail OPEN: an unreadable probe means - # "can't tell", not "no gateway", and warning on it cries wolf. + # The resolver swallows per-rung failures; "can't tell" != "no gateway". return (True, "") pid = liveness.pid @@ -213,11 +245,38 @@ def _check_dispatcher_presence( # Argparse builder # --------------------------------------------------------------------------- -def build_parser(parent_subparsers: argparse._SubParsersAction) -> argparse.ArgumentParser: - """Attach the ``kanban`` subcommand tree under an existing subparsers. +def _add_run_state_filters(p: argparse.ArgumentParser, type_help: str) -> None: + p.add_argument( + "--state-type", + choices=("status", "outcome"), + default=None, + help=f"With --state-name: {type_help}", + ) + p.add_argument( + "--state-name", + default=None, + metavar="VALUE", + help="With --state-type: keep runs whose column equals this value", + ) - Returns the top-level ``kanban`` parser so caller can ``set_defaults``. - """ + +def _add_triage_sweep_args(p: argparse.ArgumentParser, verb: str, Verb: str, noun: str) -> None: + """Shared ``specify`` / ``decompose`` arguments.""" + p.add_argument("task_id", nargs="?", default=None, + help=f"Task id to {verb} (required unless --all is given)") + p.add_argument("--all", dest="all_triage", action="store_true", + help=f"{Verb} every task currently in the triage column") + p.add_argument("--tenant", default=None, + help="When used with --all, restrict the sweep to this tenant") + p.add_argument("--author", default=None, + help="Author name recorded on the audit comment " + f"(default: $HERMES_PROFILE or '{noun}')") + p.add_argument("--json", action="store_true", + help="Emit one JSON object per task on stdout") + + +def build_parser(parent_subparsers: argparse._SubParsersAction) -> argparse.ArgumentParser: + """Attach the ``kanban`` subcommand tree; returns the ``kanban`` parser.""" kanban_parser = parent_subparsers.add_parser( "kanban", help="Multi-profile collaboration board (tasks, links, comments)", @@ -229,28 +288,20 @@ def build_parser(parent_subparsers: argparse._SubParsersAction) -> argparse.Argu "or docs/hermes-kanban-v1-spec.pdf for the full design." ), ) - # --- global --board flag --- - # Applies to every subcommand below. When set, scopes all reads and - # writes to that board's DB. When omitted, resolves via the - # HERMES_KANBAN_BOARD env var, then the persisted current-board - # file, then "default". See kanban_db.get_current_board(). - kanban_parser.add_argument( - "--board", - default=None, - metavar="", - help=( - "Board slug to operate on. Defaults to the current board " - "(set via `hermes kanban boards switch ` or the " - "HERMES_KANBAN_BOARD env var). Use `hermes kanban boards list` " - "to see all boards." - ), - ) + # --board scopes every subcommand to one board's DB; when omitted the + # resolution is HERMES_KANBAN_BOARD, then the persisted current-board + # file, then "default" (kanban_db.get_current_board()). + kanban_parser.add_argument("--board", default=None, metavar="", + help="Board slug to operate on. Defaults to the current board (set " + "via `hermes kanban boards switch ` or the " + "HERMES_KANBAN_BOARD env var). Use `hermes kanban boards " + "list` to see all boards.") sub = kanban_parser.add_subparsers(dest="kanban_action") # --- init --- sub.add_parser("init", help="Create kanban.db if missing (idempotent)") - # --- boards (new in v2: multi-project support) --- + # --- boards --- p_boards = sub.add_parser( "boards", help="Manage kanban boards (one board per project / workstream)", @@ -264,24 +315,15 @@ def build_parser(parent_subparsers: argparse._SubParsersAction) -> argparse.Argu ) boards_sub = p_boards.add_subparsers(dest="boards_action") - b_list = boards_sub.add_parser( - "list", aliases=["ls"], - help="List all boards with task counts", - ) + b_list = boards_sub.add_parser("list", aliases=["ls"], help="List all boards with task counts") b_list.add_argument("--json", action="store_true") - b_list.add_argument("--all", action="store_true", - help="Include archived boards too") + b_list.add_argument("--all", action="store_true", help="Include archived boards too") - b_create = boards_sub.add_parser( - "create", aliases=["new"], - help="Create a new board", - ) - b_create.add_argument("slug", - help="Board slug (kebab-case, e.g. atm10-server)") + b_create = boards_sub.add_parser("create", aliases=["new"], help="Create a new board") + b_create.add_argument("slug", help="Board slug (kebab-case, e.g. atm10-server)") b_create.add_argument("--name", default=None, help="Human-readable display name (defaults to Title Case of slug)") - b_create.add_argument("--description", default=None, - help="Optional description") + b_create.add_argument("--description", default=None, help="Optional description") b_create.add_argument("--icon", default=None, help="Optional emoji or single-character icon for the dashboard") b_create.add_argument("--color", default=None, @@ -291,37 +333,27 @@ def build_parser(parent_subparsers: argparse._SubParsersAction) -> argparse.Argu b_create.add_argument("--default-workdir", default=None, help="Default workspace path for tasks created on this board") - b_rm = boards_sub.add_parser( - "rm", aliases=["remove", "delete"], - help="Archive (default) or delete a board", - ) + b_rm = boards_sub.add_parser("rm", aliases=["remove", "delete"], + help="Archive (default) or delete a board") b_rm.add_argument("slug") b_rm.add_argument("--delete", action="store_true", help="Hard-delete the board directory instead of archiving it. " "Default is to move it to boards/_archived/ so it's recoverable.") - b_switch = boards_sub.add_parser( - "switch", aliases=["use"], - help="Set the active board for subsequent CLI calls", - ) + b_switch = boards_sub.add_parser("switch", aliases=["use"], + help="Set the active board for subsequent CLI calls") b_switch.add_argument("slug") - boards_sub.add_parser( - "show", aliases=["current"], - help="Print the currently-active board slug", - ) + boards_sub.add_parser("show", aliases=["current"], help="Print the currently-active board slug") - b_rename = boards_sub.add_parser( - "rename", - help="Change a board's human-readable display name (slug is immutable)", - ) + b_rename = boards_sub.add_parser("rename", + help="Change a board's human-readable display name (slug is " + "immutable)") b_rename.add_argument("slug") b_rename.add_argument("name", help="New display name") - b_set_wd = boards_sub.add_parser( - "set-default-workdir", - help="Set the default workspace path for tasks on a board", - ) + b_set_wd = boards_sub.add_parser("set-default-workdir", + help="Set the default workspace path for tasks on a board") b_set_wd.add_argument("slug") b_set_wd.add_argument("path", nargs="?", default=None, help="Absolute path to use as default workdir. Omit to clear.") @@ -388,17 +420,15 @@ def build_parser(parent_subparsers: argparse._SubParsersAction) -> argparse.Argu help="Dedup key. If a non-archived task with this key exists, " "its id is returned instead of creating a duplicate.") p_create.add_argument("--max-runtime", default=None, - help="Per-task runtime cap. Accepts seconds (300) or " - "durations (90s, 30m, 2h, 1d). When exceeded, " - "the dispatcher SIGTERMs (then SIGKILLs) the worker " - "and re-queues the task.") + help="Per-task runtime cap. Accepts seconds (300) or durations (90s, " + "30m, 2h, 1d). When exceeded, the dispatcher SIGTERMs (then " + "SIGKILLs) the worker and re-queues the task.") p_create.add_argument("--created-by", default="user", help="Author name recorded on the task (default: user)") p_create.add_argument("--skill", action="append", default=[], dest="skills", - help="Skill to force-load into the worker " - "(repeatable). The kanban lifecycle is already " - "injected automatically. Example: " - "--skill translation --skill github-code-review") + help="Skill to force-load into the worker (repeatable). The kanban " + "lifecycle is already injected automatically. Example: --skill " + "translation --skill github-code-review") p_create.add_argument("--max-retries", type=int, default=None, metavar="N", help="Per-task override for the consecutive-failure " @@ -409,24 +439,19 @@ def build_parser(parent_subparsers: argparse._SubParsersAction) -> argparse.Argu "kanban.failure_limit config " f"(default {kb.DEFAULT_FAILURE_LIMIT}).") p_create.add_argument("--model", default=None, dest="model_override", - help="Pin the worker to this model (passed as " - "-m ) without changing the profile's " - "configured model. Combine with --provider " - "when the model belongs to a different " - "backend than the profile's default.") + help="Pin the worker to this model (passed as -m ) without " + "changing the profile's configured model. Combine with --provider " + "when the model belongs to a different backend than the profile's " + "default.") p_create.add_argument("--provider", default=None, dest="provider_override", - help="Provider the --model belongs to (passed as " - "--provider to the worker). Requires " - "--model.") + help="Provider the --model belongs to (passed as --provider to " + "the worker). Requires --model.") p_create.add_argument("--goal", action="store_true", dest="goal_mode", - help="Run the worker in a goal loop: after each " - "turn a judge checks the response against the " - "card title/body and, if not done, the worker " - "keeps going in the same session until the " - "judge agrees it's complete (or the turn " - "budget runs out, which blocks the card for " - "review). Best for open-ended cards one shot " - "rarely finishes.") + help="Run the worker in a goal loop: after each turn a judge checks the " + "response against the card title/body and, if not done, the worker " + "keeps going in the same session until the judge agrees it's " + "complete (or the turn budget runs out, which blocks the card for " + "review). Best for open-ended cards one shot rarely finishes.") p_create.add_argument("--goal-max-turns", type=int, default=None, metavar="N", dest="goal_max_turns", help="Turn budget for --goal workers (default 20). " @@ -440,10 +465,9 @@ def build_parser(parent_subparsers: argparse._SubParsersAction) -> argparse.Argu p_create.add_argument("--json", action="store_true", help="Emit JSON output") # --- swarm --- - p_swarm = sub.add_parser( - "swarm", - help="Create a Kanban Swarm v1 graph (parallel workers → verifier → synthesizer)", - ) + p_swarm = sub.add_parser("swarm", + help="Create a Kanban Swarm v1 graph (parallel workers → verifier → " + "synthesizer)") p_swarm.add_argument("goal", help="Swarm goal / final outcome") p_swarm.add_argument( "--worker", @@ -462,54 +486,27 @@ def build_parser(parent_subparsers: argparse._SubParsersAction) -> argparse.Argu # --- list --- p_list = sub.add_parser("list", aliases=["ls"], help="List tasks") - p_list.add_argument("--mine", action="store_true", - help="Filter by $HERMES_PROFILE as assignee") + p_list.add_argument("--mine", action="store_true", help="Filter by $HERMES_PROFILE as assignee") p_list.add_argument("--assignee", default=None) - p_list.add_argument("--status", default=None, - choices=sorted(kb.VALID_STATUSES)) + p_list.add_argument("--status", default=None, choices=sorted(kb.VALID_STATUSES)) p_list.add_argument("--tenant", default=None) p_list.add_argument("--session", default=None, help="Filter by originating chat/agent session id " "(set on tasks created from inside an ACP loop)") - p_list.add_argument("--archived", action="store_true", - help="Include archived tasks") + p_list.add_argument("--archived", action="store_true", help="Include archived tasks") p_list.add_argument("--json", action="store_true") - p_list.add_argument( - "--sort", - default=None, - choices=sorted(kb.VALID_SORT_ORDERS.keys()), - help="Sort order for listed tasks (default: priority)", - ) - p_list.add_argument( - "--workflow-template-id", - default=None, - metavar="ID", - help="Restrict to tasks with this workflow_template_id", - ) - p_list.add_argument( - "--step-key", - default=None, - dest="current_step_key", - metavar="KEY", - help="Restrict to tasks with this current_step_key", - ) + p_list.add_argument("--sort", default=None, choices=sorted(kb.VALID_SORT_ORDERS.keys()), + help="Sort order for listed tasks (default: priority)") + p_list.add_argument("--workflow-template-id", default=None, metavar="ID", + help="Restrict to tasks with this workflow_template_id") + p_list.add_argument("--step-key", default=None, dest="current_step_key", metavar="KEY", + help="Restrict to tasks with this current_step_key") # --- show --- p_show = sub.add_parser("show", help="Show a task with comments + events") p_show.add_argument("task_id") p_show.add_argument("--json", action="store_true") - p_show.add_argument( - "--state-type", - choices=("status", "outcome"), - default=None, - help="With --state-name: filter listed runs by task_runs column", - ) - p_show.add_argument( - "--state-name", - default=None, - metavar="VALUE", - help="With --state-type: keep runs whose column equals this value", - ) + _add_run_state_filters(p_show, "filter listed runs by task_runs column") # --- assign --- p_assign = sub.add_parser("assign", help="Assign or reassign a task") @@ -517,72 +514,41 @@ def build_parser(parent_subparsers: argparse._SubParsersAction) -> argparse.Argu p_assign.add_argument("profile", help="Profile name (or 'none' to unassign)") # --- set-model (per-task model/provider override) --- - p_set_model = sub.add_parser( - "set-model", - help="Set or clear a task's model/provider override " - "(takes effect on the next dispatch)", - ) + p_set_model = sub.add_parser("set-model", + help="Set or clear a task's model/provider override (takes " + "effect on the next dispatch)") p_set_model.add_argument("task_id") - p_set_model.add_argument( - "model", nargs="?", default=None, - help="Model to pin the worker to (or 'none' to clear the override)", - ) - p_set_model.add_argument( - "--provider", default=None, - help="Provider the model belongs to (worker is spawned with " - "--provider ). Cleared together with the model.", - ) + p_set_model.add_argument("model", nargs="?", default=None, + help="Model to pin the worker to (or 'none' to clear the override)") + p_set_model.add_argument("--provider", default=None, + help="Provider the model belongs to (worker is spawned with " + "--provider ). Cleared together with the model.") # --- reclaim / reassign (recovery) --- - p_reclaim = sub.add_parser( - "reclaim", - help="Release an active worker claim on a running task", - ) + p_reclaim = sub.add_parser("reclaim", help="Release an active worker claim on a running task") p_reclaim.add_argument("task_id") - p_reclaim.add_argument( - "--reason", default=None, - help="Human-readable reason (recorded on the reclaimed event)", - ) + p_reclaim.add_argument("--reason", default=None, + help="Human-readable reason (recorded on the reclaimed event)") - p_reassign = sub.add_parser( - "reassign", - help="Reassign a task to a different profile, optionally reclaiming first", - ) + p_reassign = sub.add_parser("reassign", + help="Reassign a task to a different profile, optionally " + "reclaiming first") p_reassign.add_argument("task_id") - p_reassign.add_argument( - "profile", - help="New profile name (or 'none' to unassign)", - ) - p_reassign.add_argument( - "--reclaim", action="store_true", - help="Release any active claim before reassigning (required if task is running)", - ) - p_reassign.add_argument( - "--reason", default=None, - help="Human-readable reason (recorded on the reclaimed event)", - ) + p_reassign.add_argument("profile", help="New profile name (or 'none' to unassign)") + p_reassign.add_argument("--reclaim", action="store_true", + help="Release any active claim before reassigning (required if task " + "is running)") + p_reassign.add_argument("--reason", default=None, + help="Human-readable reason (recorded on the reclaimed event)") # --- diagnostics (board-wide health) --- - p_diag = sub.add_parser( - "diagnostics", - aliases=["diag"], - help="List active diagnostics on the current board", - ) - p_diag.add_argument( - "--severity", - choices=["warning", "error", "critical"], - default=None, - help="Only show diagnostics at or above this severity", - ) - p_diag.add_argument( - "--task", - default=None, - help="Only show diagnostics for one task id", - ) - p_diag.add_argument( - "--json", action="store_true", - help="Emit JSON (structured) instead of the default human table", - ) + p_diag = sub.add_parser("diagnostics", aliases=["diag"], + help="List active diagnostics on the current board") + p_diag.add_argument("--severity", choices=["warning", "error", "critical"], default=None, + help="Only show diagnostics at or above this severity") + p_diag.add_argument("--task", default=None, help="Only show diagnostics for one task id") + p_diag.add_argument("--json", action="store_true", + help="Emit JSON (structured) instead of the default human table") # --- link / unlink --- p_link = sub.add_parser("link", help="Add a parent->child dependency") @@ -593,10 +559,8 @@ def build_parser(parent_subparsers: argparse._SubParsersAction) -> argparse.Argu p_unlink.add_argument("child_id") # --- claim --- - p_claim = sub.add_parser( - "claim", - help="Atomically claim a ready task (prints resolved workspace path)", - ) + p_claim = sub.add_parser("claim", + help="Atomically claim a ready task (prints resolved workspace path)") p_claim.add_argument("task_id") p_claim.add_argument("--ttl", type=int, default=kb.DEFAULT_CLAIM_TTL_SECONDS, help="Claim TTL in seconds (default: 900)") @@ -639,42 +603,26 @@ def build_parser(parent_subparsers: argparse._SubParsersAction) -> argparse.Argu help='JSON dict of structured facts (e.g. \'{"changed_files": [...], ' '"tests_run": 12}\'). Stored on the closing run.') - p_edit = sub.add_parser( - "edit", - help="Edit recovery fields on an already-completed task", - ) + p_edit = sub.add_parser("edit", help="Edit recovery fields on an already-completed task") p_edit.add_argument("task_id") - p_edit.add_argument( - "--result", - required=True, - help="Backfilled task result text for a done task", - ) - p_edit.add_argument( - "--summary", - default=None, - help="Structured handoff summary. Falls back to --result if omitted.", - ) - p_edit.add_argument( - "--metadata", - default=None, - help="JSON dict of structured facts to store on the latest completed run.", - ) + p_edit.add_argument("--result", required=True, + help="Backfilled task result text for a done task") + p_edit.add_argument("--summary", default=None, + help="Structured handoff summary. Falls back to --result if omitted.") + p_edit.add_argument("--metadata", default=None, + help="JSON dict of structured facts to store on the latest completed run.") p_block = sub.add_parser("block", help="Mark one or more tasks blocked") p_block.add_argument("task_id") p_block.add_argument("reason", nargs="*", help="Reason (also appended as a comment)") p_block.add_argument("--ids", nargs="+", default=None, help="Additional task ids to block with the same reason (bulk mode)") - p_block.add_argument( - "--kind", default=None, choices=sorted(kb.VALID_BLOCK_KINDS), - help=( - "Typed block reason. 'dependency' waits in todo (auto-promoted " - "when parents finish, no human); 'needs_input'/'capability' go to " - "blocked for a human; 'transient' marks a maybe-flaky failure. " - "Repeated same-kind re-blocks after unblock route the task to " - "triage to break unblock loops. Omit for a generic block." - ), - ) + p_block.add_argument("--kind", default=None, choices=sorted(kb.VALID_BLOCK_KINDS), + help="Typed block reason. 'dependency' waits in todo (auto-promoted when " + "parents finish, no human); 'needs_input'/'capability' go to " + "blocked for a human; 'transient' marks a maybe-flaky failure. " + "Repeated same-kind re-blocks after unblock route the task to " + "triage to break unblock loops. Omit for a generic block.") p_schedule = sub.add_parser("schedule", help="Park one or more tasks in Scheduled (waiting on time, not human input)") p_schedule.add_argument("task_id") @@ -682,104 +630,65 @@ def build_parser(parent_subparsers: argparse._SubParsersAction) -> argparse.Argu p_schedule.add_argument("--ids", nargs="+", default=None, help="Additional task ids to schedule with the same reason (bulk mode)") - p_unblock = sub.add_parser( - "unblock", - help="Return blocked/scheduled tasks to ready, or todo while parents remain open", - ) - p_unblock.add_argument( - "--reason", - default=None, - help="Optional reason/note — recorded as a comment before unblocking. Quote multi-word reasons.", - ) + p_unblock = sub.add_parser("unblock", + help="Return blocked/scheduled tasks to ready, or todo while " + "parents remain open") + p_unblock.add_argument("--reason", default=None, + help="Optional reason/note — recorded as a comment before unblocking. " + "Quote multi-word reasons.") p_unblock.add_argument("task_ids", nargs="+") - p_request_review = sub.add_parser( - "request-review", - help="Move a task to 'review' (implementation done, awaiting review) — NOT a block", - ) + p_request_review = sub.add_parser("request-review", + help="Move a task to 'review' (implementation done, " + "awaiting review) — NOT a block") p_request_review.add_argument("task_id") - p_request_review.add_argument( - "--summary", default=None, - help="What was implemented and how it was verified — shown to the reviewer.", - ) - p_request_review.add_argument( - "--reviewer", default=None, - help="Optional reviewer profile; reassigns the task before review dispatch.", - ) - p_request_review.add_argument( - "--metadata", default=None, - help="JSON object with structured reviewer handoff facts.", - ) - p_request_review.add_argument( - "--force", action="store_true", - help=( - "Override the live-claim guard: move a running, claimed task to " - "review even without owning its run (clears the worker's claim)." - ), - ) + p_request_review.add_argument("--summary", default=None, + help="What was implemented and how it was verified — shown to " + "the reviewer.") + p_request_review.add_argument("--reviewer", default=None, + help="Optional reviewer profile; reassigns the task before " + "review dispatch.") + p_request_review.add_argument("--metadata", default=None, + help="JSON object with structured reviewer handoff facts.") + p_request_review.add_argument("--force", action="store_true", + help="Override the live-claim guard: move a running, claimed " + "task to review even without owning its run (clears the " + "worker's claim).") - p_request_changes = sub.add_parser( - "request-changes", - help="Reviewer verdict: return the active review run to its implementer", - ) + p_request_changes = sub.add_parser("request-changes", + help="Reviewer verdict: return the active review run to " + "its implementer") p_request_changes.add_argument("task_id") - p_request_changes.add_argument( - "reason", nargs="+", help="Concrete changes required before re-review", - ) + p_request_changes.add_argument("reason", nargs="+", + help="Concrete changes required before re-review") - p_reopen_review = sub.add_parser( - "reopen-review", - help="Send one or more review tasks back for changes (review -> ready/todo)", - ) + p_reopen_review = sub.add_parser("reopen-review", + help="Send one or more review tasks back for changes (review " + "-> ready/todo)") p_reopen_review.add_argument("task_ids", nargs="+") - p_reopen_review.add_argument( - "--reason", default=None, - help="Optional reason/note — recorded as a comment before reopening. Quote multi-word reasons.", - ) + p_reopen_review.add_argument("--reason", default=None, + help="Optional reason/note — recorded as a comment before " + "reopening. Quote multi-word reasons.") - p_promote = sub.add_parser( - "promote", - help="Manually move one or more todo/blocked tasks to ready (recovery path)", - ) + p_promote = sub.add_parser("promote", + help="Manually move one or more todo/blocked tasks to ready " + "(recovery path)") p_promote.add_argument("task_id") - p_promote.add_argument( - "reason", - nargs="*", - help="Audit-trail reason (recorded on the task_events row)", - ) - p_promote.add_argument( - "--ids", - nargs="+", - default=None, - help="Additional task ids to promote with the same reason (bulk mode)", - ) - p_promote.add_argument( - "--force", - action="store_true", - help="Promote even if parent dependencies are not yet done/archived", - ) - p_promote.add_argument( - "--dry-run", - action="store_true", - help="Validate the promotion without mutating state", - ) - p_promote.add_argument( - "--json", - dest="json", - action="store_true", - help="Emit machine-readable JSON result", - ) + p_promote.add_argument("reason", nargs="*", + help="Audit-trail reason (recorded on the task_events row)") + p_promote.add_argument("--ids", nargs="+", default=None, + help="Additional task ids to promote with the same reason (bulk mode)") + p_promote.add_argument("--force", action="store_true", + help="Promote even if parent dependencies are not yet done/archived") + p_promote.add_argument("--dry-run", action="store_true", + help="Validate the promotion without mutating state") + p_promote.add_argument("--json", dest="json", action="store_true", + help="Emit machine-readable JSON result") p_archive = sub.add_parser("archive", help="Archive one or more tasks") - p_archive.add_argument("task_ids", nargs="*", - help="Task ids to archive (default mode)") - p_archive.add_argument( - "--rm", - dest="purge_ids", - nargs="+", - default=None, - help="Permanently delete already-archived task ids from the board", - ) + p_archive.add_argument("task_ids", nargs="*", help="Task ids to archive (default mode)") + p_archive.add_argument("--rm", dest="purge_ids", nargs="+", default=None, + help="Permanently delete already-archived task ids from the board") # --- tail --- p_tail = sub.add_parser("tail", help="Follow a task's event stream") @@ -787,14 +696,11 @@ def build_parser(parent_subparsers: argparse._SubParsersAction) -> argparse.Argu p_tail.add_argument("--interval", type=float, default=1.0) # --- dispatch --- - p_disp = sub.add_parser( - "dispatch", - help="One dispatcher pass: reclaim stale, promote ready, spawn workers", - ) + p_disp = sub.add_parser("dispatch", + help="One dispatcher pass: reclaim stale, promote ready, spawn workers") p_disp.add_argument("--dry-run", action="store_true", help="Don't actually spawn processes; just print what would happen") - p_disp.add_argument("--max", type=int, default=None, - help="Cap number of spawns this pass") + p_disp.add_argument("--max", type=int, default=None, help="Cap number of spawns this pass") p_disp.add_argument("--failure-limit", type=int, default=kb.DEFAULT_SPAWN_FAILURE_LIMIT, help=f"Auto-block a task after this many consecutive non-success attempts " @@ -802,31 +708,24 @@ def build_parser(parent_subparsers: argparse._SubParsersAction) -> argparse.Argu p_disp.add_argument("--json", action="store_true") # --- daemon (deprecated) --- - p_daemon = sub.add_parser( - "daemon", - help="DEPRECATED — dispatcher now runs in the gateway. Use `hermes gateway start`.", - ) + p_daemon = sub.add_parser("daemon", + help="DEPRECATED — dispatcher now runs in the gateway. Use `hermes " + "gateway start`.") p_daemon.add_argument("--interval", type=float, default=60.0, help="Seconds between dispatch ticks (default: 60)") - p_daemon.add_argument("--max", type=int, default=None, - help="Cap number of spawns per tick") - p_daemon.add_argument("--failure-limit", type=int, - default=kb.DEFAULT_SPAWN_FAILURE_LIMIT) + p_daemon.add_argument("--max", type=int, default=None, help="Cap number of spawns per tick") + p_daemon.add_argument("--failure-limit", type=int, default=kb.DEFAULT_SPAWN_FAILURE_LIMIT) p_daemon.add_argument("--pidfile", default=None, help="Write the daemon's PID to this file on start") p_daemon.add_argument("--verbose", "-v", action="store_true", help="Log each tick's outcome to stdout") - # Undocumented escape hatch for users who truly cannot run the gateway. - # Intentionally excluded from --help so nobody discovers it casually and - # keeps the old double-dispatcher pattern alive. - p_daemon.add_argument("--force", action="store_true", - help=argparse.SUPPRESS) + # Escape hatch for hosts that truly cannot run the gateway; hidden from + # --help so nobody casually keeps the double-dispatcher pattern alive. + p_daemon.add_argument("--force", action="store_true", help=argparse.SUPPRESS) # --- watch --- - p_watch = sub.add_parser( - "watch", - help="Live-stream task_events to the terminal (Ctrl+C to exit)", - ) + p_watch = sub.add_parser("watch", + help="Live-stream task_events to the terminal (Ctrl+C to exit)") p_watch.add_argument("--assignee", default=None, help="Only show events for tasks assigned to this profile") p_watch.add_argument("--tenant", default=None, @@ -838,35 +737,26 @@ def build_parser(parent_subparsers: argparse._SubParsersAction) -> argparse.Argu help="Poll interval in seconds (default: 0.5)") # --- stats --- - p_stats = sub.add_parser( - "stats", help="Per-status + per-assignee counts + oldest-ready age", - ) + p_stats = sub.add_parser("stats", help="Per-status + per-assignee counts + oldest-ready age") p_stats.add_argument("--json", action="store_true") # --- notify subscribe / list / remove --- - p_nsub = sub.add_parser( - "notify-subscribe", - help="Subscribe a gateway source to a task's terminal events " - "(used by /kanban subscribe in the gateway adapter)", - ) + p_nsub = sub.add_parser("notify-subscribe", + help="Subscribe a gateway source to a task's terminal events (used by " + "/kanban subscribe in the gateway adapter)") p_nsub.add_argument("task_id") p_nsub.add_argument("--platform", required=True) p_nsub.add_argument("--chat-id", required=True) p_nsub.add_argument("--thread-id", default=None) p_nsub.add_argument("--user-id", default=None) p_nsub.add_argument("--user-id-alt", default=None) - p_nsub.add_argument( - "--chat-type", - choices=("dm", "group", "channel", "thread"), - default=None, - help="Originating source chat_type, recorded so the active-wake " - "delivery modes resolve the operator's real session. Omit to " - "leave an existing sub unchanged (new subs default to 'dm').", - ) - p_nsub.add_argument( - "--notifier-profile", default=None, - help="Profile gateway that owns/delivers this subscription (default: active profile)", - ) + p_nsub.add_argument("--chat-type", choices=("dm", "group", "channel", "thread"), default=None, + help="Originating source chat_type, recorded so the active-wake delivery " + "modes resolve the operator's real session. Omit to leave an " + "existing sub unchanged (new subs default to 'dm').") + p_nsub.add_argument("--notifier-profile", default=None, + help="Profile gateway that owns/delivers this subscription (default: " + "active profile)") p_nsub.add_argument( "--delivery-mode", # Single source of truth shared with the DB/watcher enum. @@ -880,154 +770,69 @@ def build_parser(parent_subparsers: argparse._SubParsersAction) -> argparse.Argu "existing subscription's mode unchanged (new subs default to 'notify').", ) - p_nlist = sub.add_parser( - "notify-list", - help="List notification subscriptions (optionally for a single task)", - ) + p_nlist = sub.add_parser("notify-list", + help="List notification subscriptions (optionally for a single task)") p_nlist.add_argument("task_id", nargs="?", default=None) p_nlist.add_argument("--json", action="store_true") - p_nrm = sub.add_parser( - "notify-unsubscribe", - help="Remove a gateway subscription from a task", - ) + p_nrm = sub.add_parser("notify-unsubscribe", help="Remove a gateway subscription from a task") p_nrm.add_argument("task_id") p_nrm.add_argument("--platform", required=True) p_nrm.add_argument("--chat-id", required=True) p_nrm.add_argument("--thread-id", default=None) # --- log --- - p_log = sub.add_parser( - "log", - help="Print the worker log for a task (from /kanban/logs/)", - ) + p_log = sub.add_parser("log", + help="Print the worker log for a task (from /kanban/logs/)") p_log.add_argument("task_id") - p_log.add_argument("--tail", type=int, default=None, - help="Only print the last N bytes") + p_log.add_argument("--tail", type=int, default=None, help="Only print the last N bytes") # --- runs (per-attempt history for a task) --- - p_runs = sub.add_parser( - "runs", - help="Show attempt history for a task (one row per run: profile, " - "outcome, elapsed, summary)", - ) + p_runs = sub.add_parser("runs", + help="Show attempt history for a task (one row per run: profile, " + "outcome, elapsed, summary)") p_runs.add_argument("task_id") p_runs.add_argument("--json", action="store_true") - p_runs.add_argument( - "--state-type", - choices=("status", "outcome"), - default=None, - help="With --state-name: filter runs by task_runs column", - ) - p_runs.add_argument( - "--state-name", - default=None, - metavar="VALUE", - help="With --state-type: keep runs whose column equals this value", - ) + _add_run_state_filters(p_runs, "filter runs by task_runs column") # --- heartbeat (worker liveness signal) --- - p_hb = sub.add_parser( - "heartbeat", - help="Emit a heartbeat event for a running task (worker liveness signal)", - ) + p_hb = sub.add_parser("heartbeat", + help="Emit a heartbeat event for a running task (worker liveness signal)") p_hb.add_argument("task_id") p_hb.add_argument("--note", default=None, help="Optional short note attached to the heartbeat event") # --- assignees --- - p_asg = sub.add_parser( - "assignees", - help="List known profiles + per-profile task counts " - "(union of ~/.hermes/profiles/ and current assignees on the board)", - ) + p_asg = sub.add_parser("assignees", + help="List known profiles + per-profile task counts (union of " + "~/.hermes/profiles/ and current assignees on the board)") p_asg.add_argument("--json", action="store_true") # --- context --- (for spawned workers) - p_ctx = sub.add_parser( - "context", - help="Print the full context a worker sees for a task " - "(title + body + parent results + comments).", - ) + p_ctx = sub.add_parser("context", + help="Print the full context a worker sees for a task (title + body + " + "parent results + comments).") p_ctx.add_argument("task_id") # --- specify --- (triage → todo via auxiliary LLM) - p_specify = sub.add_parser( - "specify", - help="Flesh out a triage-column task into a concrete spec " - "(title + body) and promote it to todo. Uses the auxiliary " - "LLM configured under auxiliary.triage_specifier.", - ) - p_specify.add_argument( - "task_id", - nargs="?", - default=None, - help="Task id to specify (required unless --all is given)", - ) - p_specify.add_argument( - "--all", - dest="all_triage", - action="store_true", - help="Specify every task currently in the triage column", - ) - p_specify.add_argument( - "--tenant", - default=None, - help="When used with --all, restrict the sweep to this tenant", - ) - p_specify.add_argument( - "--author", - default=None, - help="Author name recorded on the audit comment " - "(default: $HERMES_PROFILE or 'specifier')", - ) - p_specify.add_argument( - "--json", - action="store_true", - help="Emit one JSON object per task on stdout", - ) + p_specify = sub.add_parser("specify", + help="Flesh out a triage-column task into a concrete spec (title + " + "body) and promote it to todo. Uses the auxiliary LLM " + "configured under auxiliary.triage_specifier.") + _add_triage_sweep_args(p_specify, "specify", "Specify", "specifier") # --- decompose --- (triage → fan-out via auxiliary LLM + orchestrator) - p_decompose = sub.add_parser( - "decompose", - help="Decompose a triage-column task into a graph of child tasks " - "routed to specialist profiles by description. Falls back to " - "specify-style single-task promotion when the task doesn't " - "benefit from fan-out. Uses auxiliary.kanban_decomposer.", - ) - p_decompose.add_argument( - "task_id", - nargs="?", - default=None, - help="Task id to decompose (required unless --all is given)", - ) - p_decompose.add_argument( - "--all", - dest="all_triage", - action="store_true", - help="Decompose every task currently in the triage column", - ) - p_decompose.add_argument( - "--tenant", - default=None, - help="When used with --all, restrict the sweep to this tenant", - ) - p_decompose.add_argument( - "--author", - default=None, - help="Author name recorded on the audit comment " - "(default: $HERMES_PROFILE or 'decomposer')", - ) - p_decompose.add_argument( - "--json", - action="store_true", - help="Emit one JSON object per task on stdout", - ) + p_decompose = sub.add_parser("decompose", + help="Decompose a triage-column task into a graph of child tasks " + "routed to specialist profiles by description. Falls back " + "to specify-style single-task promotion when the task " + "doesn't benefit from fan-out. Uses " + "auxiliary.kanban_decomposer.") + _add_triage_sweep_args(p_decompose, "decompose", "Decompose", "decomposer") # --- gc --- - p_gc = sub.add_parser( - "gc", help="Garbage-collect archived-task workspaces, old events, and old logs", - ) + p_gc = sub.add_parser("gc", + help="Garbage-collect archived-task workspaces, old events, and old logs") p_gc.add_argument("--event-retention-days", type=int, default=30, help="Delete task_events older than N days for terminal tasks (default: 30)") p_gc.add_argument("--log-retention-days", type=int, default=30, @@ -1049,8 +854,7 @@ def build_parser(parent_subparsers: argparse._SubParsersAction) -> argparse.Argu "is healthy or was repaired, non-zero when it is still corrupt." ), ) - p_repair.add_argument("--json", action="store_true", - help="Emit the repair report as JSON") + p_repair.add_argument("--json", action="store_true", help="Emit the repair report as JSON") kanban_parser.set_defaults(_kanban_parser=kanban_parser) return kanban_parser @@ -1061,10 +865,7 @@ def build_parser(parent_subparsers: argparse._SubParsersAction) -> argparse.Argu # --------------------------------------------------------------------------- def kanban_command(args: argparse.Namespace) -> int: - """Entry point from ``hermes kanban …`` argparse dispatch. - - Returns a shell-style exit code (0 on success, non-zero on error). - """ + """Entry point from ``hermes kanban …``; returns a shell-style exit code.""" action = getattr(args, "kanban_action", None) if not action: # No subaction given: print help via the stored parser reference. @@ -1082,123 +883,56 @@ def kanban_command(args: argparse.Namespace) -> int: # Fast-fail for clearer CLI UX only. The durable trust boundary is lower in # hermes_cli.kanban_db, because children can import DB mutators directly. if _is_delegated_child_cli_mutation(args): - print( - "kanban: delegate_task child contexts cannot mutate Kanban tasks via the CLI", - file=sys.stderr, - ) - return 1 + return _err("kanban: delegate_task child contexts cannot mutate Kanban tasks via the CLI") # Board-management commands operate on board metadata and the persisted - # current-board pointer itself. They must ignore the shared `--board` - # task-routing override; otherwise `/kanban --board beta boards show` - # reports beta as the current board even when the on-disk pointer is - # alpha. + # current-board pointer itself, so they must ignore the shared `--board` + # task-routing override (else `--board beta boards show` reports beta). if action == "boards": return _dispatch_boards(args) - # `--board ` applies to every subcommand below by way of an - # env-var pin for the duration of this call. Using HERMES_KANBAN_BOARD - # (rather than threading `board=` through 50+ kb.connect() sites) - # keeps the patch small and inherits the exact same resolution the - # dispatcher uses for workers — consistency is a feature here. + # `--board ` applies to every subcommand below via an env-var pin + # (HERMES_KANBAN_BOARD) for the duration of this call, so it inherits the + # exact resolution the dispatcher uses for workers. board_override = getattr(args, "board", None) board_scope = contextlib.nullcontext() if board_override: try: normed = kb._normalize_board_slug(board_override) except ValueError as exc: - print(f"kanban: {exc}", file=sys.stderr) - return 2 + return _err(f"kanban: {exc}", 2) if not normed: - print("kanban: --board requires a slug", file=sys.stderr) - return 2 + return _err("kanban: --board requires a slug", 2) # Boards other than 'default' must already exist — typoed slugs # would otherwise silently create an empty board. if normed != kb.DEFAULT_BOARD and not kb.board_exists(normed): - print( + return _err( f"kanban: board {normed!r} does not exist. " - f"Create it with `hermes kanban boards create {normed}`.", - file=sys.stderr, + f"Create it with `hermes kanban boards create {normed}`." ) - return 1 board_scope = kb.scoped_current_board(normed) - # Auto-initialize the DB before dispatching any subcommand. init_db - # is idempotent, so running it every invocation is cheap (one - # SELECT against sqlite_master when tables already exist) and - # prevents "no such table: tasks" on first use from a fresh - # HERMES_HOME. Previously only `init` and `daemon` triggered - # schema creation; `create` / `list` / every other command would - # error out on a fresh install. with board_scope: - # `repair` must dispatch BEFORE the auto-init below: on a corrupt DB - # init_db() itself raises KanbanDbCorruptError, which would turn - # every `hermes kanban repair` into "could not initialize database" - # without ever reaching the repair path. + # `repair` must dispatch BEFORE the auto-init: on a corrupt DB init_db() + # itself raises KanbanDbCorruptError, which would turn every + # `hermes kanban repair` into "could not initialize database". if action == "repair": return _cmd_repair(args) + # Auto-initialize the DB before any subcommand. init_db is idempotent + # (one SELECT against sqlite_master when tables exist) and prevents + # "no such table: tasks" on first use from a fresh HERMES_HOME. try: kb.init_db() except Exception as exc: - print(f"kanban: could not initialize database: {exc}", file=sys.stderr) - return 1 + return _err(f"kanban: could not initialize database: {exc}") - handlers = { - "init": _cmd_init, - "create": _cmd_create, - "swarm": _cmd_swarm, - "list": _cmd_list, - "ls": _cmd_list, - "show": _cmd_show, - "assign": _cmd_assign, - "set-model": _cmd_set_model, - "reclaim": _cmd_reclaim, - "reassign": _cmd_reassign, - "diagnostics": _cmd_diagnostics, - "diag": _cmd_diagnostics, - "link": _cmd_link, - "unlink": _cmd_unlink, - "claim": _cmd_claim, - "comment": _cmd_comment, - "attach": _cmd_attach, - "attachments": _cmd_attachments, - "attach-rm": _cmd_attach_rm, - "complete": _cmd_complete, - "edit": _cmd_edit, - "block": _cmd_block, - "schedule": _cmd_schedule, - "unblock": _cmd_unblock, - "request-review": _cmd_request_review, - "request-changes": _cmd_request_changes, - "reopen-review": _cmd_reopen_review, - "promote": _cmd_promote, - "archive": _cmd_archive, - "tail": _cmd_tail, - "dispatch": _cmd_dispatch, - "daemon": _cmd_daemon, - "watch": _cmd_watch, - "stats": _cmd_stats, - "log": _cmd_log, - "runs": _cmd_runs, - "heartbeat": _cmd_heartbeat, - "assignees": _cmd_assignees, - "notify-subscribe": _cmd_notify_subscribe, - "notify-list": _cmd_notify_list, - "notify-unsubscribe": _cmd_notify_unsubscribe, - "context": _cmd_context, - "specify": _cmd_specify, - "decompose": _cmd_decompose, - "gc": _cmd_gc, - } - handler = handlers.get(action) + handler = _HANDLERS.get(action) if not handler: - print(f"kanban: unknown action {action!r}", file=sys.stderr) - return 2 + return _err(f"kanban: unknown action {action!r}", 2) try: return int(handler(args) or 0) except (ValueError, RuntimeError) as exc: - print(f"kanban: {exc}", file=sys.stderr) - return 1 + return _err(f"kanban: {exc}") # --------------------------------------------------------------------------- @@ -1219,45 +953,15 @@ def _profile_author() -> str: _DELEGATED_CHILD_DENIED_ACTIONS: frozenset[str] = frozenset({ - "init", - "create", - "swarm", - "assign", - "reclaim", - "reassign", - "link", - "unlink", - "claim", - "comment", - "attach", - "attach-rm", - "complete", - "edit", - "block", - "schedule", - "unblock", - "promote", - "archive", - "dispatch", - "daemon", - "repair", - "heartbeat", - "notify-subscribe", - "notify-unsubscribe", - "specify", - "decompose", + "init", "create", "swarm", "assign", "reclaim", "reassign", "link", "unlink", + "claim", "comment", "attach", "attach-rm", "complete", "edit", "block", + "schedule", "unblock", "promote", "archive", "dispatch", "daemon", "repair", + "heartbeat", "notify-subscribe", "notify-unsubscribe", "specify", "decompose", "gc", }) _DELEGATED_CHILD_DENIED_BOARD_ACTIONS: frozenset[str] = frozenset({ - "create", - "new", - "rm", - "remove", - "delete", - "switch", - "use", - "rename", + "create", "new", "rm", "remove", "delete", "switch", "use", "rename", "set-default-workdir", }) @@ -1283,35 +987,13 @@ def _is_delegated_child_cli_mutation(args: argparse.Namespace) -> bool: # --------------------------------------------------------------------------- def _dispatch_boards(args: argparse.Namespace) -> int: - """Handle ``hermes kanban boards ``. - - Boards management is deliberately separate from the task-level - commands: it operates on the filesystem (board directories, - ``current`` pointer, ``board.json``), not on the per-board SQLite - DB, so a fresh HERMES_HOME that has never called ``kanban init`` - can still run ``boards create`` / ``boards list``. - """ + """``hermes kanban boards `` — filesystem-only (board dirs, the + ``current`` pointer, ``board.json``), so it works before ``kanban init``.""" sub = getattr(args, "boards_action", None) or "list" - if sub in {"list", "ls"}: - return _cmd_boards_list(args) - if sub in {"create", "new"}: - return _cmd_boards_create(args) - if sub in {"rm", "remove", "delete"}: - return _cmd_boards_rm(args) - if sub in {"switch", "use"}: - return _cmd_boards_switch(args) - if sub in {"show", "current"}: - return _cmd_boards_show(args) - if sub == "rename": - return _cmd_boards_rename(args) - if sub == "set-default-workdir": - return _cmd_boards_set_default_workdir(args) - if sub == "export": - return _cmd_boards_export(args) - if sub == "import": - return _cmd_boards_import(args) - print(f"kanban boards: unknown action {sub!r}", file=sys.stderr) - return 2 + handler = _BOARD_HANDLERS.get(sub) + if handler is None: + return _err(f"kanban boards: unknown action {sub!r}", 2) + return handler(args) def _board_task_counts(slug: str) -> dict[str, int]: @@ -1329,34 +1011,39 @@ def _board_task_counts(slug: str) -> dict[str, int]: return {} +def _board_slug_arg(args: argparse.Namespace, cmd: str, *, must_exist: bool) -> tuple[Optional[str], int]: + """Normalize ``args.slug`` for a ``boards`` subcommand; ``(slug, 0)`` or ``(None, rc)``.""" + try: + normed = kb._normalize_board_slug(args.slug) + except ValueError as exc: + return None, _err(f"kanban boards {cmd}: {exc}", 2) + if must_exist: + if not normed or not kb.board_exists(normed): + return None, _err(f"kanban boards {cmd}: board {args.slug!r} does not exist") + elif not normed: + return None, _err(f"kanban boards {cmd}: slug is required", 2) + return normed, 0 + + def _cmd_boards_list(args: argparse.Namespace) -> int: - include_archived = bool(getattr(args, "all", False)) - boards = kb.list_boards(include_archived=include_archived) - # Enrich each entry with task counts + whether it's the current board. + boards = kb.list_boards(include_archived=bool(getattr(args, "all", False))) current = kb.get_current_board() for b in boards: b["is_current"] = (b["slug"] == current) b["counts"] = _board_task_counts(b["slug"]) b["total"] = sum(b["counts"].values()) - if getattr(args, "json", False): - print(json.dumps(boards, indent=2, ensure_ascii=False)) + if _json_out(args, boards): return 0 - # Human table: marker (•) for current, slug, display name, counts. if not boards: print("(no boards — create one with `hermes kanban boards create `)") return 0 print(f"{'':2s} {'SLUG':24s} {'NAME':28s} COUNTS") for b in boards: marker = "●" if b["is_current"] else " " - counts = b["counts"] or {} - counts_str = ( - ", ".join(f"{k}={v}" for k, v in sorted(counts.items())) - or "(empty)" - ) name = b.get("name") or "" if b.get("archived"): name += " [archived]" - print(f"{marker:2s} {b['slug']:24s} {name:28s} {counts_str}") + print(f"{marker:2s} {b['slug']:24s} {name:28s} {_fmt_counts(b['counts'] or {}, '(empty)')}") print() print(f"Current board: {current}") if len(boards) > 1: @@ -1365,14 +1052,9 @@ def _cmd_boards_list(args: argparse.Namespace) -> int: def _cmd_boards_create(args: argparse.Namespace) -> int: - try: - normed = kb._normalize_board_slug(args.slug) - except ValueError as exc: - print(f"kanban boards create: {exc}", file=sys.stderr) - return 2 - if not normed: - print("kanban boards create: slug is required", file=sys.stderr) - return 2 + normed, rc = _board_slug_arg(args, "create", must_exist=False) + if rc: + return rc already = kb.board_exists(normed) and normed != kb.DEFAULT_BOARD meta = kb.create_board( normed, @@ -1395,16 +1077,13 @@ def _cmd_boards_create(args: argparse.Namespace) -> int: def _cmd_boards_rm(args: argparse.Namespace) -> int: - # When the user runs `hermes kanban boards delete ` (alias), the - # boards_action is 'delete' but args.delete is never set to True because - # the --delete flag belongs to the 'rm' subparser only. Detect the alias - # and treat it identically to `boards rm --delete` (fixes #23139). + # `boards delete ` (alias) never sets args.delete because --delete + # belongs to the 'rm' subparser only; treat the alias as `rm --delete`. force_delete = getattr(args, "delete", False) or getattr(args, "boards_action", "") == "delete" try: res = kb.remove_board(args.slug, archive=not force_delete) except ValueError as exc: - print(f"kanban boards rm: {exc}", file=sys.stderr) - return 1 + return _err(f"kanban boards rm: {exc}") if res["action"] == "archived": print(f"Board {res['slug']!r} archived → {res['new_path']}") print("Recover by moving the directory back to " @@ -1415,21 +1094,14 @@ def _cmd_boards_rm(args: argparse.Namespace) -> int: def _cmd_boards_switch(args: argparse.Namespace) -> int: - try: - normed = kb._normalize_board_slug(args.slug) - except ValueError as exc: - print(f"kanban boards switch: {exc}", file=sys.stderr) - return 2 - if not normed: - print("kanban boards switch: slug is required", file=sys.stderr) - return 2 + normed, rc = _board_slug_arg(args, "switch", must_exist=False) + if rc: + return rc if not kb.board_exists(normed): - print( + return _err( f"kanban boards switch: board {normed!r} does not exist. " - f"Create it with `hermes kanban boards create {normed}`.", - file=sys.stderr, + f"Create it with `hermes kanban boards create {normed}`." ) - return 1 kb.set_current_board(normed) print(f"Active board is now {normed!r}.") return 0 @@ -1439,43 +1111,29 @@ def _cmd_boards_show(args: argparse.Namespace) -> int: current = kb.get_current_board() meta = kb.read_board_metadata(current) counts = _board_task_counts(current) - total = sum(counts.values()) print(f"Current board: {current}") print(f" Display name: {meta.get('name', '')}") if meta.get("description"): print(f" Description: {meta['description']}") print(f" DB path: {meta['db_path']}") - print(f" Tasks: {total} total" - + (f" ({', '.join(f'{k}={v}' for k, v in sorted(counts.items()))})" - if counts else "")) + print(f" Tasks: {sum(counts.values())} total" + + (f" ({_fmt_counts(counts)})" if counts else "")) return 0 def _cmd_boards_rename(args: argparse.Namespace) -> int: - try: - normed = kb._normalize_board_slug(args.slug) - except ValueError as exc: - print(f"kanban boards rename: {exc}", file=sys.stderr) - return 2 - if not normed or not kb.board_exists(normed): - print(f"kanban boards rename: board {args.slug!r} does not exist", - file=sys.stderr) - return 1 + normed, rc = _board_slug_arg(args, "rename", must_exist=True) + if rc: + return rc meta = kb.write_board_metadata(normed, name=args.name) print(f"Board {normed!r} renamed to {meta['name']!r}.") return 0 def _cmd_boards_set_default_workdir(args: argparse.Namespace) -> int: - try: - normed = kb._normalize_board_slug(args.slug) - except ValueError as exc: - print(f"kanban boards set-default-workdir: {exc}", file=sys.stderr) - return 2 - if not normed or not kb.board_exists(normed): - print(f"kanban boards set-default-workdir: board {args.slug!r} does not exist", - file=sys.stderr) - return 1 + normed, rc = _board_slug_arg(args, "set-default-workdir", must_exist=True) + if rc: + return rc meta = kb.write_board_metadata(normed, default_workdir=args.path) new_val = meta.get("default_workdir") if new_val: @@ -1499,11 +1157,9 @@ def _cmd_boards_export(args: argparse.Namespace) -> int: include_logs=args.include_logs, ) except (OSError, ValueError) as exc: - print(f"kanban boards export: {exc}", file=sys.stderr) - return 1 + return _err(f"kanban boards export: {exc}") - if getattr(args, "json", False): - print(json.dumps(res, indent=2, ensure_ascii=False)) + if _json_out(args, res): return 0 counts = res["counts"] print(f"Exported board {res['board']!r} → {res['archive']}") @@ -1523,11 +1179,9 @@ def _cmd_boards_import(args: argparse.Namespace) -> int: args.archive, args.as_slug, activate=args.switch ) except (OSError, ValueError) as exc: - print(f"kanban boards import: {exc}", file=sys.stderr) - return 1 + return _err(f"kanban boards import: {exc}") - if getattr(args, "json", False): - print(json.dumps(res, indent=2, ensure_ascii=False)) + if _json_out(args, res): return 0 print(f"Imported board {res['board']!r} ({res['name']}).") if res["renamed"]: @@ -1543,15 +1197,25 @@ def _cmd_boards_import(args: argparse.Namespace) -> int: return 0 +_BOARD_HANDLERS = { + "list": _cmd_boards_list, "ls": _cmd_boards_list, + "create": _cmd_boards_create, "new": _cmd_boards_create, + "rm": _cmd_boards_rm, "remove": _cmd_boards_rm, "delete": _cmd_boards_rm, + "switch": _cmd_boards_switch, "use": _cmd_boards_switch, + "show": _cmd_boards_show, "current": _cmd_boards_show, + "rename": _cmd_boards_rename, + "set-default-workdir": _cmd_boards_set_default_workdir, + "export": _cmd_boards_export, + "import": _cmd_boards_import, +} + + # --------------------------------------------------------------------------- def _parse_duration(val) -> Optional[int]: - """Parse ``30s`` / ``5m`` / ``2h`` / ``1d`` or a raw integer → seconds. - - Returns None for empty input. Raises ValueError on malformed input so - the CLI can surface a usage error cleanly. - """ + """``30s`` / ``5m`` / ``2h`` / ``1d`` or a raw integer → seconds; None for + empty input; ValueError on malformed input.""" if val is None or val == "": return None s = str(val).strip().lower() @@ -1577,10 +1241,7 @@ def _cmd_init(args: argparse.Namespace) -> int: print() # Enumerate profiles on disk so the user knows what assignees are - # already addressable. Multica does this auto-detection on its - # daemon start; we do it here at init time instead because our - # dispatcher doesn't need to enumerate — we just pass the name - # through to `hermes -p `. + # already addressable. try: profiles = kb.list_profiles_on_disk() except Exception: @@ -1614,8 +1275,7 @@ def _cmd_heartbeat(args: argparse.Namespace) -> int: expected_run_id=_worker_run_id_for(args.task_id), ) if not ok: - print(f"cannot heartbeat {args.task_id} (not running?)", file=sys.stderr) - return 1 + return _err(f"cannot heartbeat {args.task_id} (not running?)") print(f"Heartbeat recorded for {args.task_id}") return 0 @@ -1623,19 +1283,15 @@ def _cmd_heartbeat(args: argparse.Namespace) -> int: def _cmd_assignees(args: argparse.Namespace) -> int: with kb.connect_closing() as conn: data = kb.known_assignees(conn) - if getattr(args, "json", False): - print(json.dumps(data, indent=2, ensure_ascii=False)) + if _json_out(args, data): return 0 if not data: print("(no assignees — create a profile with `hermes -p setup`)") return 0 - # Header print(f"{'NAME':20s} {'ON DISK':8s} COUNTS") for entry in data: on_disk = "yes" if entry["on_disk"] else "no" - counts = entry["counts"] or {} - count_str = ", ".join(f"{k}={v}" for k, v in sorted(counts.items())) or "(idle)" - print(f"{entry['name']:20s} {on_disk:8s} {count_str}") + print(f"{entry['name']:20s} {on_disk:8s} {_fmt_counts(entry['counts'] or {}, '(idle)')}") return 0 @@ -1644,24 +1300,20 @@ def _cmd_create(args: argparse.Namespace) -> int: ws_kind, ws_path = _parse_workspace_flag(args.workspace) branch_name = _parse_branch_flag(getattr(args, "branch", None)) except argparse.ArgumentTypeError as exc: - print(f"kanban: {exc}", file=sys.stderr) - return 2 + return _err(f"kanban: {exc}", 2) if branch_name and ws_kind != "worktree": - print("kanban: --branch is only valid with --workspace worktree", file=sys.stderr) - return 2 + return _err("kanban: --branch is only valid with --workspace worktree", 2) try: max_runtime = _parse_duration(getattr(args, "max_runtime", None)) except ValueError as exc: - print(f"kanban: --max-runtime: {exc}", file=sys.stderr) - return 2 + return _err(f"kanban: --max-runtime: {exc}", 2) max_retries = getattr(args, "max_retries", None) if max_retries is not None and max_retries < 1: - print( + return _err( f"kanban: --max-retries must be >= 1 (got {max_retries}); " "use 1 to trip on the first failure.", - file=sys.stderr, + 2, ) - return 2 with kb.connect_closing() as conn: task_id = kb.create_task( conn, @@ -1689,17 +1341,14 @@ def _cmd_create(args: argparse.Namespace) -> int: ) task = kb.get_task(conn, task_id) if getattr(args, "json", False): - print(json.dumps(_task_to_dict(task), indent=2, ensure_ascii=False)) + _print_json(_task_to_dict(task)) else: print(f"Created {task_id} ({task.status}, assignee={task.assignee or '-'})") # Warn when the task would sit in `ready` because no dispatcher is - # present. Only warn on ready+assigned tasks — triage/todo are - # expected to sit idle until promoted, and unassigned tasks - # can't be dispatched. Skipped in --json mode so the stdout - # stream stays strictly machine-parseable for callers (the JSON - # response itself carries enough info for them to decide if - # they want to check dispatcher presence separately). + # present. Only ready+assigned tasks — triage/todo idle by design, + # unassigned can't dispatch. Skipped in --json so stdout stays + # machine-parseable. if task.status == "ready" and task.assignee: running, message = _check_dispatcher_presence() if not running and message: @@ -1711,11 +1360,9 @@ def _cmd_swarm(args: argparse.Namespace) -> int: try: workers = [ks.parse_worker_arg(raw) for raw in (args.worker or [])] except ValueError as exc: - print(f"kanban swarm: {exc}", file=sys.stderr) - return 2 + return _err(f"kanban swarm: {exc}", 2) if not workers: - print("kanban swarm: at least one --worker is required", file=sys.stderr) - return 2 + return _err("kanban swarm: at least one --worker is required", 2) with kb.connect_closing() as conn: created = ks.create_swarm( conn, @@ -1729,7 +1376,7 @@ def _cmd_swarm(args: argparse.Namespace) -> int: idempotency_key=getattr(args, "idempotency_key", None), ) if getattr(args, "json", False): - print(json.dumps(created.as_dict(), indent=2, ensure_ascii=False)) + _print_json(created.as_dict()) else: print(f"Swarm root: {created.root_id}") print("Workers: " + ", ".join(created.worker_ids)) @@ -1757,12 +1404,9 @@ def _cmd_list(args: argparse.Namespace) -> int: workflow_template_id=args.workflow_template_id, current_step_key=args.current_step_key, ) - if getattr(args, "json", False): - print(json.dumps([_task_to_dict(t) for t in tasks], indent=2, ensure_ascii=False)) + if _json_out(args, [_task_to_dict(t) for t in tasks]): return 0 - # Passive discoverability: when the user has multiple boards, surface - # which one they're looking at in the list header. Single-board users - # never see this — the feature stays invisible until you opt in. + # Passive discoverability: only multi-board users see which board this is. try: all_boards = kb.list_boards(include_archived=False) except Exception: @@ -1783,69 +1427,54 @@ def _cmd_list(args: argparse.Namespace) -> int: return 0 +def _print_diagnostics(diags, indent: str, *, with_kind: bool) -> None: + """Shared human rendering for ``show`` and ``diagnostics`` (suggested actions only).""" + sev_marker = {"warning": "⚠", "error": "!!", "critical": "!!!"} + for d in diags: + head = f"{d.kind}: {d.title}" if with_kind else d.title + print(f"{indent}{sev_marker.get(d.severity, '?')} [{d.severity}] {head}") + if d.data: + bits = [ + f"{k}={','.join(str(x) for x in v)}" if isinstance(v, list) else f"{k}={v}" + for k, v in d.data.items() + ] + if bits: + print(f"{indent} data: {' | '.join(bits)}") + for a in d.actions: + if a.suggested: + print(f"{indent} → {a.label}") + + def _cmd_show(args: argparse.Namespace) -> int: rsk = _run_state_kwargs(args) if rsk is None: - print( - "kanban show: pass both --state-type and --state-name, or omit both", - file=sys.stderr, - ) - return 2 + return _err("kanban show: pass both --state-type and --state-name, or omit both", 2) graph = None with kb.connect_closing() as conn: task = kb.get_task(conn, args.task_id) if not task: - print(f"no such task: {args.task_id}", file=sys.stderr) - return 1 + return _err(f"no such task: {args.task_id}") comments = kb.list_comments(conn, args.task_id) events = kb.list_events(conn, args.task_id) parents = kb.parent_ids(conn, args.task_id) children = kb.child_ids(conn, args.task_id) runs = kb.list_runs(conn, args.task_id, **rsk) - # Workers hand off via ``task_runs.summary``; ``tasks.result`` is left NULL unless the caller explicitly passed - # ``result=``. Surfacing the latest summary here keeps ``show`` from - # looking like a no-op when the worker actually did real work. + # Workers hand off via task_runs.summary; tasks.result stays NULL unless + # explicitly set, so surface the latest summary here. latest_summary = kb.latest_summary(conn, args.task_id) if not getattr(args, "json", False): graph = kb.task_graph_context(conn, task.id) if getattr(args, "json", False): - payload = { + _print_json({ "task": _task_to_dict(task), "latest_summary": latest_summary, "parents": parents, "children": children, - "comments": [ - {"author": c.author, "body": c.body, "created_at": c.created_at} - for c in comments - ], - "events": [ - { - "kind": e.kind, - "payload": e.payload, - "created_at": e.created_at, - "run_id": e.run_id, - } - for e in events - ], - "runs": [ - { - "id": r.id, - "profile": r.profile, - "step_key": r.step_key, - "status": r.status, - "outcome": r.outcome, - "summary": r.summary, - "error": r.error, - "metadata": r.metadata, - "worker_pid": r.worker_pid, - "started_at": r.started_at, - "ended_at": r.ended_at, - } - for r in runs - ], - } - print(json.dumps(payload, indent=2, ensure_ascii=False)) + "comments": [_obj_dict(c, ("author", "body", "created_at")) for c in comments], + "events": [_obj_dict(e, ("kind", "payload", "created_at", "run_id")) for e in events], + "runs": [_obj_dict(r, _SHOW_RUN_FIELDS) for r in runs], + }) return 0 print(f"Task {task.id}: {task.title}") @@ -1862,10 +1491,8 @@ def _cmd_show(args: argparse.Namespace) -> int: if task.model_override: _prov = f" (provider: {task.provider_override})" if task.provider_override else "" print(f" model: {task.model_override}{_prov}") - # Effective retry threshold. Show the per-task override if set, - # otherwise the dispatcher's resolved value from config (or the - # default if config doesn't set it either). Helps operators see - # why a task auto-blocked earlier/later than they expected. + # Effective retry threshold: per-task override, else config, else default — + # so operators can see why a task auto-blocked when it did. if task.max_retries is not None: print(f" max-retries: {task.max_retries} (task)") else: @@ -1881,30 +1508,12 @@ def _cmd_show(args: argparse.Namespace) -> int: print(f" max-retries: {kb.DEFAULT_FAILURE_LIMIT} (default)") print(f" created: {_fmt_ts(task.created_at)} by {task.created_by or '-'}") - # Diagnostics section — surface active distress signals at the top - # of show output so CLI users see them before scrolling through - # comments / runs. + # Diagnostics up top so CLI users see distress signals before scrolling. from hermes_cli import kanban_diagnostics as kd diags = kd.compute_task_diagnostics(task, events, runs, graph=graph) if diags: - sev_marker = {"warning": "⚠", "error": "!!", "critical": "!!!"} print(f"\n Diagnostics ({len(diags)}):") - for d in diags: - print(f" {sev_marker.get(d.severity, '?')} [{d.severity}] {d.title}") - if d.data: - bits = [] - for k, v in d.data.items(): - if isinstance(v, list): - bits.append(f"{k}={','.join(str(x) for x in v)}") - else: - bits.append(f"{k}={v}") - if bits: - print(f" data: {' | '.join(bits)}") - # Only show suggested actions in show output to keep it tight; - # full list is available via `kanban diagnostics --task `. - for a in d.actions: - if a.suggested: - print(f" → {a.label}") + _print_diagnostics(diags, " ", with_kind=False) if task.started_at: print(f" started: {_fmt_ts(task.started_at)}") if task.completed_at: @@ -1922,9 +1531,6 @@ def _cmd_show(args: argparse.Namespace) -> int: print("Result:") print(task.result) elif latest_summary: - # Worker handoff lives on the latest run, not on tasks.result. - # Surface it at top-level so a glance at ``hermes kanban show `` - # tells you what the worker did even if tasks.result is empty. print() print("Latest summary:") print(latest_summary) @@ -1959,12 +1565,11 @@ def _cmd_show(args: argparse.Namespace) -> int: def _cmd_assign(args: argparse.Namespace) -> int: - profile = None if args.profile.lower() in {"none", "-", "null"} else args.profile + profile = _none_profile(args.profile) with kb.connect_closing() as conn: ok = kb.assign_task(conn, args.task_id, profile) if not ok: - print(f"no such task: {args.task_id}", file=sys.stderr) - return 1 + return _err(f"no such task: {args.task_id}") print(f"Assigned {args.task_id} to {profile or '(unassigned)'}") return 0 @@ -1978,11 +1583,9 @@ def _cmd_set_model(args: argparse.Namespace) -> int: with kb.connect_closing() as conn: ok = kb.set_model_override(conn, args.task_id, model, provider=provider) except (ValueError, RuntimeError) as exc: - print(f"kanban: {exc}", file=sys.stderr) - return 2 + return _err(f"kanban: {exc}", 2) if not ok: - print(f"no such task: {args.task_id}", file=sys.stderr) - return 1 + return _err(f"no such task: {args.task_id}") if model: label = f"{provider}:{model}" if provider else model print(f"Set model override on {args.task_id}: {label} " @@ -2000,17 +1603,13 @@ def _cmd_reclaim(args: argparse.Namespace) -> int: reason=getattr(args, "reason", None), ) if not ok: - print( - f"cannot reclaim {args.task_id} (not running or unknown id)", - file=sys.stderr, - ) - return 1 + return _err(f"cannot reclaim {args.task_id} (not running or unknown id)") print(f"Reclaimed {args.task_id}") return 0 def _cmd_reassign(args: argparse.Namespace) -> int: - profile = None if args.profile.lower() in {"none", "-", "null"} else args.profile + profile = _none_profile(args.profile) with kb.connect_closing() as conn: ok = kb.reassign_task( conn, args.task_id, profile, @@ -2018,12 +1617,10 @@ def _cmd_reassign(args: argparse.Namespace) -> int: reason=getattr(args, "reason", None), ) if not ok: - print( + return _err( f"cannot reassign {args.task_id} " - f"(unknown id, or still running — pass --reclaim to release first)", - file=sys.stderr, + f"(unknown id, or still running — pass --reclaim to release first)" ) - return 1 print( f"Reassigned {args.task_id} to " f"{profile or '(unassigned)'}" @@ -2032,10 +1629,19 @@ def _cmd_reassign(args: argparse.Namespace) -> int: return 0 +def _rows_by_task(conn, table: str, ids: list[str]) -> dict[str, list]: + """``{task_id: [rows ordered by id]}`` for every id (empty list when none).""" + by = {i: [] for i in ids} + placeholders = ",".join(["?"] * len(ids)) + for row in conn.execute( + f"SELECT * FROM {table} WHERE task_id IN ({placeholders}) ORDER BY id", tuple(ids), + ): + by.setdefault(row["task_id"], []).append(row) + return by + + def _cmd_diagnostics(args: argparse.Namespace) -> int: - """List active diagnostics on the board. Wraps the same rule engine - the dashboard uses, so CLI output matches what the UI shows. - """ + """List active diagnostics on the board via the same rule engine the dashboard uses.""" from hermes_cli import kanban_diagnostics as kd from hermes_cli.config import load_config @@ -2046,8 +1652,7 @@ def _cmd_diagnostics(args: argparse.Namespace) -> int: if getattr(args, "task", None): task = kb.get_task(conn, args.task) if task is None: - print(f"no such task: {args.task}", file=sys.stderr) - return 1 + return _err(f"no such task: {args.task}") diags_by_task = { args.task: kd.compute_task_diagnostics( task, @@ -2063,24 +1668,11 @@ def _cmd_diagnostics(args: argparse.Namespace) -> int: "SELECT * FROM tasks WHERE status != 'archived'" ).fetchall()) ids = [r["id"] for r in rows] - if not ids: - diags_by_task = {} - else: - placeholders = ",".join(["?"] * len(ids)) - ev_by = {i: [] for i in ids} - for row in conn.execute( - f"SELECT * FROM task_events WHERE task_id IN ({placeholders}) ORDER BY id", - tuple(ids), - ): - ev_by.setdefault(row["task_id"], []).append(row) - run_by = {i: [] for i in ids} - for row in conn.execute( - f"SELECT * FROM task_runs WHERE task_id IN ({placeholders}) ORDER BY id", - tuple(ids), - ): - run_by.setdefault(row["task_id"], []).append(row) + diags_by_task = {} + if ids: + ev_by = _rows_by_task(conn, "task_events", ids) + run_by = _rows_by_task(conn, "task_runs", ids) graph_by = kb.task_graph_contexts(conn, ids) - diags_by_task = {} for r in rows: tid = r["id"] dl = kd.compute_task_diagnostics( @@ -2096,12 +1688,12 @@ def _cmd_diagnostics(args: argparse.Namespace) -> int: # Severity filter. sev = getattr(args, "severity", None) if sev: - for tid in list(diags_by_task.keys()): - kept = [d for d in diags_by_task[tid] if kd.SEVERITY_ORDER.index(d.severity) >= kd.SEVERITY_ORDER.index(sev)] - if kept: - diags_by_task[tid] = kept - else: - del diags_by_task[tid] + floor = kd.SEVERITY_ORDER.index(sev) + diags_by_task = { + tid: kept + for tid, dl in diags_by_task.items() + if (kept := [d for d in dl if kd.SEVERITY_ORDER.index(d.severity) >= floor]) + } # Map task_id → title/status/assignee for the table output. meta: dict[str, dict] = {} @@ -2111,30 +1703,23 @@ def _cmd_diagnostics(args: argparse.Namespace) -> int: f"SELECT id, title, status, assignee FROM tasks WHERE id IN ({placeholders})", tuple(diags_by_task.keys()), ): - meta[r["id"]] = { - "title": r["title"], "status": r["status"], - "assignee": r["assignee"], - } + meta[r["id"]] = {k: r[k] for k in ("title", "status", "assignee")} if getattr(args, "json", False): - out_json = [ + _print_json([ { "task_id": tid, **meta.get(tid, {}), "diagnostics": [d.to_dict() for d in dl], } for tid, dl in diags_by_task.items() - ] - print(json.dumps(out_json, indent=2, ensure_ascii=False)) + ]) return 0 if not diags_by_task: print("No active diagnostics on this board.") return 0 - # Human-readable summary: grouped by task, severity-marked, with - # suggested actions inline. - sev_marker = {"warning": "⚠", "error": "!!", "critical": "!!!"} total = sum(len(dl) for dl in diags_by_task.values()) print( f"{total} active diagnostic(s) across " @@ -2146,22 +1731,7 @@ def _cmd_diagnostics(args: argparse.Namespace) -> int: status = m.get("status") or "?" assignee = m.get("assignee") or "(unassigned)" print(f" {tid} {status:8s} @{assignee:18s} {title}") - for d in dl: - print(f" {sev_marker.get(d.severity, '?')} [{d.severity}] {d.kind}: {d.title}") - if d.data: - # Compact key:value pairs on one line. - bits = [] - for k, v in d.data.items(): - if isinstance(v, list): - bits.append(f"{k}={','.join(str(x) for x in v)}") - else: - bits.append(f"{k}={v}") - if bits: - print(f" data: {' | '.join(bits)}") - # Suggested actions first. - for a in d.actions: - if a.suggested: - print(f" → {a.label}") + _print_diagnostics(dl, " ", with_kind=True) print() return 0 @@ -2177,8 +1747,7 @@ def _cmd_unlink(args: argparse.Namespace) -> int: with kb.connect_closing() as conn: ok = kb.unlink_tasks(conn, args.parent_id, args.child_id) if not ok: - print(f"No such link: {args.parent_id} -> {args.child_id}", file=sys.stderr) - return 1 + return _err(f"No such link: {args.parent_id} -> {args.child_id}") print(f"Unlinked {args.parent_id} -> {args.child_id}") return 0 @@ -2187,17 +1756,13 @@ def _cmd_claim(args: argparse.Namespace) -> int: with kb.connect_closing() as conn: task = kb.claim_task(conn, args.task_id, ttl_seconds=args.ttl) if task is None: - # Report why existing = kb.get_task(conn, args.task_id) if existing is None: - print(f"no such task: {args.task_id}", file=sys.stderr) - return 1 - print( + return _err(f"no such task: {args.task_id}") + return _err( f"cannot claim {args.task_id}: status={existing.status} " - f"lock={existing.claim_lock or '(none)'}", - file=sys.stderr, + f"lock={existing.claim_lock or '(none)'}" ) - return 1 workspace = kb.resolve_workspace(task) kb.set_workspace_path(conn, task.id, str(workspace)) print(f"Claimed {task.id}") @@ -2209,8 +1774,7 @@ def _cmd_comment(args: argparse.Namespace) -> int: body = " ".join(args.text).strip() if args.max_len is not None: if args.max_len < 1: - print("kanban: --max-len must be positive", file=sys.stderr) - return 2 + return _err("kanban: --max-len must be positive", 2) if len(body) > args.max_len: suffix = f"\n\n[trimmed to {args.max_len} chars by --max-len]" body = body[: max(0, args.max_len - len(suffix))].rstrip() + suffix @@ -2222,19 +1786,13 @@ def _cmd_comment(args: argparse.Namespace) -> int: def _cmd_attach(args: argparse.Namespace) -> int: - """Attach a local file to a task. - - Reads the file off disk, writes it under the task's attachments dir, - and records the metadata row via the shared ``store_attachment_bytes`` - path (same code the dashboard upload and the agent tool use), so the - 25 MB cap and name-sanitisation behave identically everywhere. - """ + """Attach a local file via the shared ``store_attachment_bytes`` path (same + 25 MB cap and name sanitisation as the dashboard upload and agent tool).""" import mimetypes src = Path(args.path).expanduser() if not src.is_file(): - print(f"kanban: no such file: {src}", file=sys.stderr) - return 1 + return _err(f"kanban: no such file: {src}") data = src.read_bytes() name = args.name or src.name content_type = args.content_type or mimetypes.guess_type(name)[0] @@ -2250,32 +1808,17 @@ def _cmd_attach(args: argparse.Namespace) -> int: uploaded_by=uploaded_by, ) except kb.AttachmentTooLarge as exc: - print(f"kanban: {exc}", file=sys.stderr) - return 1 + return _err(f"kanban: {exc}") print(f"Attached {name} to {args.task_id} (attachment {att_id}, {len(data)} bytes)") return 0 def _cmd_attachments(args: argparse.Namespace) -> int: - """List a task's attachments.""" with kb.connect_closing() as conn: if kb.get_task(conn, args.task_id) is None: - print(f"no such task: {args.task_id}", file=sys.stderr) - return 1 + return _err(f"no such task: {args.task_id}") atts = kb.list_attachments(conn, args.task_id) - if getattr(args, "json", False): - print(json.dumps([ - { - "id": a.id, - "filename": a.filename, - "content_type": a.content_type, - "size": a.size, - "uploaded_by": a.uploaded_by, - "stored_path": a.stored_path, - "created_at": a.created_at, - } - for a in atts - ], indent=2)) + if _json_out(args, [_obj_dict(a, _ATTACHMENT_FIELDS) for a in atts], ascii=True): return 0 if not atts: print(f"No attachments on {args.task_id}") @@ -2289,12 +1832,10 @@ def _cmd_attachments(args: argparse.Namespace) -> int: def _cmd_attach_rm(args: argparse.Namespace) -> int: - """Delete an attachment by id (removes the row and the on-disk blob).""" with kb.connect_closing() as conn: removed = kb.delete_attachment(conn, args.attachment_id) if removed is None: - print(f"no such attachment: {args.attachment_id}", file=sys.stderr) - return 1 + return _err(f"no such attachment: {args.attachment_id}") print(f"Deleted attachment {args.attachment_id} ({removed.filename}) from {removed.task_id}") return 0 @@ -2353,85 +1894,60 @@ def _cmd_complete(args: argparse.Namespace) -> int: """Mark one or more tasks done. Supports a single id or a list.""" ids = list(args.task_ids or []) if not ids: - print("at least one task_id is required", file=sys.stderr) - return 1 + return _err("at least one task_id is required") summary = getattr(args, "summary", None) raw_meta = getattr(args, "metadata", None) - # Guard: structured handoff fields are per-run, so they'd be - # copy-pasted identically across N runs — almost always a footgun. - # Refuse instead of silently doing the wrong thing. + # Structured handoff fields are per-run; copying them across N runs is + # almost always a footgun, so refuse rather than silently do it. if len(ids) > 1 and (summary or raw_meta): - print( + return _err( "kanban: --summary / --metadata are per-task and can't be used " "with multiple ids (would apply the same handoff to every task). " "Complete tasks one at a time, or drop the flags for the bulk close.", - file=sys.stderr, + 2, ) - return 2 - metadata = None - if raw_meta: - try: - metadata = json.loads(raw_meta) - if not isinstance(metadata, dict): - raise ValueError("must be a JSON object") - except (ValueError, json.JSONDecodeError) as exc: - print(f"kanban: --metadata: {exc}", file=sys.stderr) - return 2 - failed: list[str] = [] + metadata, rc = _parse_metadata_flag(raw_meta) + if rc: + return rc + fail_msg: dict[str, str] = {} with kb.connect_closing() as conn: - for tid in ids: - # Goal-mode judge gate (mirrors tools/kanban_tools.py). Apply it - # to every terminal handoff so request-review cannot bypass the - # acceptance contract that protects complete. - task = kb.get_task(conn, tid) + def op(tid): + # Goal-mode judge gate (mirrors tools/kanban_tools.py); applied to + # every terminal handoff so request-review can't bypass it. gate_verdict, rejection = _goal_mode_handoff_rejection( - task, + kb.get_task(conn, tid), (summary or args.result or "").strip(), ) if gate_verdict == "blocked": - print( + fail_msg[tid] = ( f"kanban: goal completion of {tid} rejected: judge ruled " f"the goal unachievable — {rejection}. Re-scope with " f"kanban edit, or record the block with kanban block " - f"instead of completing.", - file=sys.stderr, + f"instead of completing." ) - failed.append(tid) - continue + return False if rejection is not None: - print( + fail_msg[tid] = ( f"kanban: goal completion of {tid} rejected by judge: {rejection}. " - f"Provide evidence matching the task's acceptance criteria.", - file=sys.stderr, + f"Provide evidence matching the task's acceptance criteria." ) - failed.append(tid) - continue - - if not kb.complete_task( + return False + fail_msg[tid] = f"cannot complete {tid} (unknown id or terminal state)" + return kb.complete_task( conn, tid, result=args.result, summary=summary, metadata=metadata, expected_run_id=_worker_run_id_for(tid), - ): - failed.append(tid) - print(f"cannot complete {tid} (unknown id or terminal state)", file=sys.stderr) - else: - print(f"Completed {tid}") - return 0 if not failed else 1 + ) + + return _bulk_apply(ids, op, lambda tid: f"Completed {tid}", fail_msg.__getitem__) def _cmd_edit(args: argparse.Namespace) -> int: - raw_meta = getattr(args, "metadata", None) - metadata = None - if raw_meta: - try: - metadata = json.loads(raw_meta) - if not isinstance(metadata, dict): - raise ValueError("must be a JSON object") - except (ValueError, json.JSONDecodeError) as exc: - print(f"kanban: --metadata: {exc}", file=sys.stderr) - return 2 + metadata, rc = _parse_metadata_flag(getattr(args, "metadata", None)) + if rc: + return rc with kb.connect_closing() as conn: if not kb.edit_completed_task_result( conn, @@ -2440,11 +1956,7 @@ def _cmd_edit(args: argparse.Namespace) -> int: 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 + return _err(f"cannot edit {args.task_id} (unknown id or task is not done)") print(f"Edited {args.task_id}") return 0 @@ -2454,80 +1966,68 @@ def _cmd_block(args: argparse.Namespace) -> int: kind = getattr(args, "kind", None) author = _profile_author() ids = [args.task_id] + list(getattr(args, "ids", None) or []) - failed: list[str] = [] + suffix = f": {reason}" if reason else "" with kb.connect_closing() as conn: - for tid in ids: + def op(tid): if reason: kb.add_comment(conn, tid, author, f"BLOCKED: {reason}") - if not kb.block_task( - conn, - tid, - reason=reason, - kind=kind, + return kb.block_task( + conn, tid, reason=reason, kind=kind, expected_run_id=_worker_run_id_for(tid), - ): - failed.append(tid) - print(f"cannot block {tid}", file=sys.stderr) - else: - # Report where the task actually landed — dependency blocks go - # to todo, and a tripped unblock-loop breaker routes to triage. - landed = kb.get_task(conn, tid) - where = landed.status if landed else "blocked" - suffix = f": {reason}" if reason else "" - if where == "todo": - print(f"{tid} → todo (dependency wait){suffix}") - elif where == "triage": - print( - f"{tid} → triage (unblock loop detected — needs a " - f"human decision){suffix}" - ) - else: - print(f"Blocked {tid}{suffix}") - return 0 if not failed else 1 + ) + + def ok_msg(tid): + # Report where the task actually landed — dependency blocks go + # to todo, and a tripped unblock-loop breaker routes to triage. + landed = kb.get_task(conn, tid) + where = landed.status if landed else "blocked" + if where == "todo": + return f"{tid} → todo (dependency wait){suffix}" + if where == "triage": + return (f"{tid} → triage (unblock loop detected — needs a " + f"human decision){suffix}") + return f"Blocked {tid}{suffix}" + + return _bulk_apply(ids, op, ok_msg, lambda tid: f"cannot block {tid}") def _cmd_schedule(args: argparse.Namespace) -> int: reason = " ".join(args.reason).strip() if args.reason else None author = _profile_author() ids = [args.task_id] + list(getattr(args, "ids", None) or []) - failed: list[str] = [] + suffix = f": {reason}" if reason else "" with kb.connect_closing() as conn: - for tid in ids: + def op(tid): if reason: kb.add_comment(conn, tid, author, f"SCHEDULED: {reason}") - if not kb.schedule_task( - conn, - tid, - reason=reason, - expected_run_id=_worker_run_id_for(tid), - ): - failed.append(tid) - print(f"cannot schedule {tid}", file=sys.stderr) - else: - print(f"Scheduled {tid}" + (f": {reason}" if reason else "")) - return 0 if not failed else 1 + return kb.schedule_task( + conn, tid, reason=reason, expected_run_id=_worker_run_id_for(tid), + ) + + return _bulk_apply( + ids, op, lambda tid: f"Scheduled {tid}{suffix}", lambda tid: f"cannot schedule {tid}", + ) def _cmd_unblock(args: argparse.Namespace) -> int: ids = list(args.task_ids or []) if not ids: - print("at least one task_id is required", file=sys.stderr) - return 1 + return _err("at least one task_id is required") reason = getattr(args, "reason", None) if reason is not None: reason = reason.strip() or None author = _profile_author() if reason else None - failed: list[str] = [] + suffix = f": {reason}" if reason else "" with kb.connect_closing() as conn: - for tid in ids: + def op(tid): if reason: kb.add_comment(conn, tid, author, f"UNBLOCK: {reason}") - if not kb.unblock_task(conn, tid): - failed.append(tid) - print(f"cannot unblock {tid} (not blocked/scheduled?)", file=sys.stderr) - else: - print(f"Unblocked {tid}" + (f": {reason}" if reason else "")) - return 0 if not failed else 1 + return kb.unblock_task(conn, tid) + + return _bulk_apply( + ids, op, lambda tid: f"Unblocked {tid}{suffix}", + lambda tid: f"cannot unblock {tid} (not blocked/scheduled?)", + ) def _cmd_request_review(args: argparse.Namespace) -> int: @@ -2535,16 +2035,9 @@ def _cmd_request_review(args: argparse.Namespace) -> int: summary = getattr(args, "summary", None) if summary is not None: summary = summary.strip() or None - raw_metadata = getattr(args, "metadata", None) - metadata = None - if raw_metadata: - try: - metadata = json.loads(raw_metadata) - if not isinstance(metadata, dict): - raise ValueError("must be a JSON object") - except (ValueError, json.JSONDecodeError) as exc: - print(f"kanban: --metadata: {exc}", file=sys.stderr) - return 2 + metadata, rc = _parse_metadata_flag(getattr(args, "metadata", None)) + if rc: + return rc reviewer = getattr(args, "reviewer", None) with kb.connect_closing() as conn: gate_verdict, rejection = _goal_mode_handoff_rejection( @@ -2552,20 +2045,16 @@ def _cmd_request_review(args: argparse.Namespace) -> int: summary or "", ) if gate_verdict == "blocked": - print( + return _err( f"kanban: goal review handoff of {tid} rejected: judge ruled " f"the goal unachievable — {rejection}. Record the block with " - f"kanban block instead of requesting review.", - file=sys.stderr, + f"kanban block instead of requesting review." ) - return 1 if rejection is not None: - print( + return _err( f"kanban: goal review handoff of {tid} rejected by judge: " - f"{rejection}. Provide acceptance evidence matching the task.", - file=sys.stderr, + f"{rejection}. Provide acceptance evidence matching the task." ) - return 1 ok, reason = kb.request_review( conn, tid, @@ -2577,12 +2066,7 @@ def _cmd_request_review(args: argparse.Namespace) -> int: with_reason=True, ) if not ok: - detail = reason or "not running/ready?" - print( - f"cannot request review for {tid}: {detail}", - file=sys.stderr, - ) - return 1 + return _err(f"cannot request review for {tid}: {reason or 'not running/ready?'}") persisted_run = kb.latest_run(conn, tid) display_summary = persisted_run.summary if persisted_run else None print( @@ -2603,11 +2087,7 @@ def _cmd_request_changes(args: argparse.Namespace) -> int: expected_run_id=_worker_run_id_for(tid), ) if not ok: - print( - f"cannot request changes for {tid}: {detail or 'invalid review state'}", - file=sys.stderr, - ) - return 1 + return _err(f"cannot request changes for {tid}: {detail or 'invalid review state'}") print( f"Requested changes for {tid}" + (f"; routed to {detail}" if detail else "") @@ -2618,42 +2098,31 @@ def _cmd_request_changes(args: argparse.Namespace) -> int: def _cmd_reopen_review(args: argparse.Namespace) -> int: ids = list(args.task_ids or []) if not ids: - print("at least one task_id is required", file=sys.stderr) - return 1 + return _err("at least one task_id is required") reason = getattr(args, "reason", None) if reason is not None: reason = str(kb.redact_review_value(reason.strip())).strip() or None author = _profile_author() if reason else None - failed: list[str] = [] + suffix = f": {reason}" if reason else "" with kb.connect_closing() as conn: - for tid in ids: + def op(tid): if not kb.reopen_review_task(conn, tid): - failed.append(tid) - print(f"cannot reopen {tid} (not in review?)", file=sys.stderr) - else: - if reason: - kb.add_comment( - conn, - tid, - author or "operator", - f"CHANGES REQUESTED: {reason}", - ) - print(f"Reopened {tid}" + (f": {reason}" if reason else "")) - return 0 if not failed else 1 + return False + if reason: + kb.add_comment(conn, tid, author or "operator", f"CHANGES REQUESTED: {reason}") + return True + + return _bulk_apply( + ids, op, lambda tid: f"Reopened {tid}{suffix}", + lambda tid: f"cannot reopen {tid} (not in review?)", + ) def _cmd_promote(args: argparse.Namespace) -> int: reason = " ".join(args.reason).strip() if args.reason else None author = _profile_author() - as_json = getattr(args, "json", False) - extra_ids = list(getattr(args, "ids", None) or []) # Dedupe while preserving order; positional task_id always first. - ids: list[str] = [] - seen: set[str] = set() - for tid in [args.task_id, *extra_ids]: - if tid not in seen: - ids.append(tid) - seen.add(tid) + ids = list(dict.fromkeys([args.task_id, *(getattr(args, "ids", None) or [])])) results: list[dict[str, object]] = [] with kb.connect_closing() as conn: @@ -2676,10 +2145,9 @@ def _cmd_promote(args: argparse.Namespace) -> int: }) failed = [r for r in results if not r["promoted"]] - if as_json: + if getattr(args, "json", False): # Single-id stays a flat object for back-compat; bulk emits a list. - payload: object = results[0] if len(results) == 1 else results - print(json.dumps(payload, indent=2, ensure_ascii=False)) + _print_json(results[0] if len(results) == 1 else results) return 0 if not failed else 1 tag = " (dry)" if args.dry_run else "" @@ -2697,28 +2165,20 @@ def _cmd_archive(args: argparse.Namespace) -> int: ids = list(args.task_ids or []) purge_ids = list(getattr(args, "purge_ids", None) or []) if ids and purge_ids: - print("choose either task_ids to archive or --rm archived task_ids", file=sys.stderr) - return 1 + return _err("choose either task_ids to archive or --rm archived task_ids") if not ids and not purge_ids: - print("at least one task_id is required", file=sys.stderr) - return 1 - failed: list[str] = [] + return _err("at least one task_id is required") with kb.connect_closing() as conn: if purge_ids: - for tid in purge_ids: - if not kb.delete_archived_task(conn, tid): - failed.append(tid) - print(f"cannot delete {tid} (must already be archived)", file=sys.stderr) - else: - print(f"Deleted {tid}") - return 0 if not failed else 1 - for tid in ids: - if not kb.archive_task(conn, tid): - failed.append(tid) - print(f"cannot archive {tid}", file=sys.stderr) - else: - print(f"Archived {tid}") - return 0 if not failed else 1 + return _bulk_apply( + purge_ids, lambda tid: kb.delete_archived_task(conn, tid), + lambda tid: f"Deleted {tid}", + lambda tid: f"cannot delete {tid} (must already be archived)", + ) + return _bulk_apply( + ids, lambda tid: kb.archive_task(conn, tid), + lambda tid: f"Archived {tid}", lambda tid: f"cannot archive {tid}", + ) def _cmd_tail(args: argparse.Namespace) -> int: @@ -2739,39 +2199,33 @@ def _cmd_tail(args: argparse.Namespace) -> int: return 0 +def _coerce_positive_int(value): + if value is None: + return None + try: + ival = int(value) + except (TypeError, ValueError): + return None + return ival if ival >= 1 else None + + def _cmd_dispatch(args: argparse.Namespace) -> int: - # Honour kanban.default_assignee as the fallback for unassigned ready - # tasks (#27145), kanban.max_in_progress as the global concurrency cap - # (#33488), kanban.max_in_progress_per_profile as the per-profile - # cap (#21582), and kanban.max_spawn as the per-tick spawn limit - # (#28805). Same semantics as the gateway dispatch path so behavior - # matches whether the user runs the CLI directly or relies on the - # gateway-embedded dispatcher. + # Honour kanban.default_assignee, kanban.max_in_progress, + # kanban.max_in_progress_per_profile and kanban.max_spawn with the same + # semantics as the gateway dispatch path. try: from hermes_cli.config import load_config _cfg = load_config() _kanban_cfg = _cfg.get("kanban", {}) if isinstance(_cfg, dict) else {} default_assignee = (_kanban_cfg.get("default_assignee") or "").strip() or None - - def _coerce_positive_int(value): - if value is None: - return None - try: - ival = int(value) - except (TypeError, ValueError): - return None - return ival if ival >= 1 else None - max_in_progress_per_profile = _coerce_positive_int( _kanban_cfg.get("max_in_progress_per_profile") ) - max_in_progress = _coerce_positive_int(_kanban_cfg.get("max_in_progress")) - # Memory-derived default when unset (OOF-30/OOF-77) — same - # fallback the gateway-embedded dispatcher applies, so behaviour - # matches regardless of which path runs the tick. - max_in_progress = kb.resolve_max_in_progress(max_in_progress) - # CLI --max overrides config kanban.max_spawn when both are present; - # CLI is the more explicit signal so it wins. + # Memory-derived default when unset — same fallback the gateway applies. + max_in_progress = kb.resolve_max_in_progress( + _coerce_positive_int(_kanban_cfg.get("max_in_progress")) + ) + # CLI --max is the more explicit signal, so it wins over kanban.max_spawn. cli_max = getattr(args, "max", None) max_spawn = cli_max if cli_max is not None else _coerce_positive_int( _kanban_cfg.get("max_spawn") @@ -2792,7 +2246,7 @@ def _cmd_dispatch(args: argparse.Namespace) -> int: max_in_progress_per_profile=max_in_progress_per_profile, ) if getattr(args, "json", False): - print(json.dumps({ + _print_json({ "reclaimed": res.reclaimed, "crashed": res.crashed, "timed_out": res.timed_out, @@ -2810,25 +2264,22 @@ def _cmd_dispatch(args: argparse.Namespace) -> int: for (tid, who, current) in res.skipped_per_profile_capped ], "auto_assigned_default": res.auto_assigned_default, - }, indent=2)) + }, ascii=True) return 0 print(f"Reclaimed: {res.reclaimed}") - print(f"Crashed: {len(res.crashed)}") - if res.crashed: - print(f" {', '.join(res.crashed)}") - print(f"Timed out: {len(res.timed_out)}") - if res.timed_out: - print(f" {', '.join(res.timed_out)}") - print(f"Stale: {len(res.stale)}") - if res.stale: - print(f" {', '.join(res.stale)}") - print(f"Auto-blocked: {len(res.auto_blocked)}") - if res.auto_blocked: - print(f" {', '.join(res.auto_blocked)}") + for label, items in ( + ("Crashed: ", res.crashed), + ("Timed out: ", res.timed_out), + ("Stale: ", res.stale), + ("Auto-blocked:", res.auto_blocked), + ): + print(f"{label} {len(items)}") + if items: + print(f" {', '.join(items)}") print(f"Promoted: {res.promoted}") print(f"Spawned: {len(res.spawned)}") + tag = " (dry)" if args.dry_run else "" for tid, who, ws in res.spawned: - tag = " (dry)" if args.dry_run else "" print(f" - {tid} -> {who} @ {ws or '-'}{tag}") if res.auto_assigned_default: print( @@ -2837,11 +2288,8 @@ def _cmd_dispatch(args: argparse.Namespace) -> int: ) if res.skipped_unassigned: print(f"Skipped (unassigned): {', '.join(res.skipped_unassigned)}") - if res.skipped_per_profile_capped: - for tid, who, current in res.skipped_per_profile_capped: - print( - f"Deferred ({who} at per-profile cap, {current} running): {tid}" - ) + for tid, who, current in res.skipped_per_profile_capped: + print(f"Deferred ({who} at per-profile cap, {current} running): {tid}") if res.skipped_nonspawnable: print( f"Skipped (non-spawnable assignee — terminal lane, OK): " @@ -2853,20 +2301,13 @@ def _cmd_dispatch(args: argparse.Namespace) -> int: def _cmd_daemon(args: argparse.Namespace) -> int: """Deprecated — the dispatcher now runs inside the gateway. - Left in as a stub so users with the old command in scripts/systemd - units get a clear migration message instead of a cryptic - "no such command" error. A ``--force`` escape hatch keeps the old - standalone daemon alive for the rare edge case where someone truly - cannot run the gateway (e.g. running on a host that forbids - long-lived background services), but the default path exits 2 - with guidance so nobody accidentally keeps running two dispatchers - against the same kanban.db. + Kept as a stub so old scripts/systemd units get a clear migration message. + ``--force`` (hidden from --help) keeps the standalone loop for hosts that + truly cannot run the gateway; the default path exits 2 so nobody + accidentally runs two dispatchers against the same kanban.db. """ - # --force lets power users keep the standalone loop for one more - # release cycle. Undocumented in `--help` so nobody discovers it - # casually — intentional. if not getattr(args, "force", False): - print( + return _err( "hermes kanban daemon: DEPRECATED — the dispatcher now runs\n" "inside the gateway. To use kanban:\n" "\n" @@ -2883,13 +2324,11 @@ def _cmd_daemon(args: argparse.Namespace) -> int: "Running both the gateway AND this standalone daemon will\n" "race for claims. If you truly need the old standalone\n" "daemon (no gateway available), rerun with --force.", - file=sys.stderr, + 2, ) - return 2 - # Legacy path — same logic as before, kept behind --force. - # Make sure the DB exists before printing "started" so the user sees the - # correct DB path and any init error surfaces immediately. + # Init before printing "started" so the DB path is right and init errors + # surface immediately. kb.init_db() pidfile = getattr(args, "pidfile", None) @@ -2910,11 +2349,9 @@ def _cmd_daemon(args: argparse.Namespace) -> int: file=sys.stderr, ) - # Health telemetry: warn when every tick finds ready work but fails to - # spawn any worker. Catches broken profiles, PATH drift, missing venv, - # credential loss — cases where the per-task circuit breaker auto-blocks - # each task quietly but the operator has no signal that the dispatcher - # itself is dysfunctional. + # Health telemetry: warn when every tick finds ready work but spawns + # nothing (broken profile, PATH drift, missing venv, credential loss) — + # the per-task breaker auto-blocks quietly, so the operator needs a signal. HEALTH_WINDOW = 6 # ticks (default 30s at interval=5) health_state = {"bad_ticks": 0, "last_warn_at": 0} @@ -2925,11 +2362,9 @@ def _cmd_daemon(args: argparse.Namespace) -> int: health_state["bad_ticks"] += 1 else: health_state["bad_ticks"] = 0 - # Emit a warning once per HEALTH_WINDOW bad ticks (not every tick) - # so log volume stays bounded while the problem persists. + # Warn once per HEALTH_WINDOW bad ticks, at most every 5 minutes. if health_state["bad_ticks"] >= HEALTH_WINDOW: now = int(time.time()) - # Rate-limit repeats: at most one warning per 5 minutes. if now - health_state["last_warn_at"] >= 300: print( f"[{_fmt_ts(now)}] WARN dispatcher stuck: " @@ -2959,15 +2394,8 @@ def _cmd_daemon(args: argparse.Namespace) -> int: ) def _ready_queue_nonempty() -> bool: - """Cheap probe — is there at least one ready+assigned+unclaimed - task whose assignee maps to a real Hermes profile (i.e. one the - dispatcher would actually try to spawn for)? - - Filters out tasks assigned to control-plane lanes - (e.g. ``orion-cc``, ``orion-research``) that are pulled by - terminals via ``claim_task`` directly — those are correctly idle - from the dispatcher's perspective, not stuck. - """ + """Is there a ready+assigned+unclaimed task the dispatcher would spawn for? + Control-plane lanes pulled via ``claim_task`` are correctly idle, not stuck.""" try: with kb.connect_closing() as conn: return kb.has_spawnable_ready(conn) @@ -2997,7 +2425,6 @@ def _cmd_watch(args: argparse.Namespace) -> int: {k.strip() for k in args.kinds.split(",") if k.strip()} if args.kinds else None ) - cursor = 0 print("Watching kanban events. Ctrl-C to stop.", flush=True) # Seed cursor at the latest id so we don't replay history. with kb.connect_closing() as conn: @@ -3043,8 +2470,7 @@ def _cmd_watch(args: argparse.Namespace) -> int: def _cmd_stats(args: argparse.Namespace) -> int: with kb.connect_closing() as conn: stats = kb.board_stats(conn) - if getattr(args, "json", False): - print(json.dumps(stats, indent=2, ensure_ascii=False)) + if _json_out(args, stats): return 0 print("By status:") for k in ("triage", "todo", "scheduled", "ready", "running", "blocked", "done"): @@ -3052,8 +2478,7 @@ def _cmd_stats(args: argparse.Namespace) -> int: if stats["by_assignee"]: print("\nBy assignee:") for who, counts in sorted(stats["by_assignee"].items()): - parts = ", ".join(f"{k}={v}" for k, v in sorted(counts.items())) - print(f" {who:20s} {parts}") + print(f" {who:20s} {_fmt_counts(counts)}") age = stats["oldest_ready_age_seconds"] if age is not None: print(f"\nOldest ready task age: {int(age)}s") @@ -3063,8 +2488,7 @@ def _cmd_stats(args: argparse.Namespace) -> int: def _cmd_notify_subscribe(args: argparse.Namespace) -> int: with kb.connect_closing() as conn: if kb.get_task(conn, args.task_id) is None: - print(f"no such task: {args.task_id}", file=sys.stderr) - return 1 + return _err(f"no such task: {args.task_id}") kb.add_notify_sub( conn, task_id=args.task_id, platform=args.platform, chat_id=args.chat_id, @@ -3083,8 +2507,7 @@ def _cmd_notify_subscribe(args: argparse.Namespace) -> int: def _cmd_notify_list(args: argparse.Namespace) -> int: with kb.connect_closing() as conn: subs = kb.list_notify_subs(conn, args.task_id) - if getattr(args, "json", False): - print(json.dumps(subs, indent=2, ensure_ascii=False)) + if _json_out(args, subs): return 0 if not subs: print("(no subscriptions)") @@ -3110,8 +2533,7 @@ def _cmd_notify_unsubscribe(args: argparse.Namespace) -> int: thread_id=args.thread_id, ) if not ok: - print("(no such subscription)", file=sys.stderr) - return 1 + return _err("(no such subscription)") print(f"Unsubscribed from {args.task_id}") return 0 @@ -3119,9 +2541,7 @@ def _cmd_notify_unsubscribe(args: argparse.Namespace) -> int: def _cmd_log(args: argparse.Namespace) -> int: content = kb.read_worker_log(args.task_id, tail_bytes=args.tail) if content is None: - print(f"(no log for {args.task_id} — task may not have spawned yet)", - file=sys.stderr) - return 1 + return _err(f"(no log for {args.task_id} — task may not have spawned yet)") sys.stdout.write(content) if not content.endswith("\n"): sys.stdout.write("\n") @@ -3132,23 +2552,10 @@ def _cmd_runs(args: argparse.Namespace) -> int: """Show attempt history for a task.""" rsk = _run_state_kwargs(args) if rsk is None: - print( - "kanban runs: pass both --state-type and --state-name, or omit both", - file=sys.stderr, - ) - return 2 + return _err("kanban runs: pass both --state-type and --state-name, or omit both", 2) with kb.connect_closing() as conn: runs = kb.list_runs(conn, args.task_id, **rsk) - if getattr(args, "json", False): - print(json.dumps([ - { - "id": r.id, "profile": r.profile, "status": r.status, - "outcome": r.outcome, "started_at": r.started_at, - "ended_at": r.ended_at, "summary": r.summary, - "error": r.error, "metadata": r.metadata, - "worker_pid": r.worker_pid, "step_key": r.step_key, - } for r in runs - ], indent=2, ensure_ascii=False)) + if _json_out(args, [_obj_dict(r, _RUNS_RUN_FIELDS) for r in runs]): return 0 if not runs: print(f"(no runs yet for {args.task_id})") @@ -3167,9 +2574,7 @@ def _cmd_runs(args: argparse.Namespace) -> int: outcome = r.outcome or ("(running)" if not r.ended_at else r.status) print(f"{i:3d} {outcome:12s} {(r.profile or '-'):16s} {el:>8s} {_fmt_ts(r.started_at)}") if r.summary: - # Indent and truncate long summaries to keep the table readable. - summary = r.summary.splitlines()[0][:100] - print(f" → {summary}") + print(f" → {r.summary.splitlines()[0][:100]}") if r.error: print(f" ✖ {r.error[:100]}") return 0 @@ -3182,72 +2587,51 @@ def _cmd_context(args: argparse.Namespace) -> int: return 0 -def _cmd_specify(args: argparse.Namespace) -> int: - """Flesh out a triage task (or all of them) via auxiliary LLM, - then promote to todo. Thin wrapper over ``kanban_specify``.""" - from hermes_cli import kanban_specify as spec +def _triage_sweep_ids(args: argparse.Namespace, verb: str, list_triage_ids, json_key: str): + """Shared arg validation for ``specify`` / ``decompose``: ``(ids|None, rc)``. + ``ids is None`` with ``rc == 0`` means "nothing to do, already reported". + """ all_flag = bool(getattr(args, "all_triage", False)) tenant = getattr(args, "tenant", None) + if args.task_id and all_flag: + return None, _err("kanban: pass either a task id OR --all, not both", 2) + if all_flag: + ids = list_triage_ids(tenant=tenant) + if not ids: + if getattr(args, "json", False): + print(json.dumps({json_key: 0, "total": 0})) + else: + print("No triage tasks" + (f" for tenant {tenant!r}" if tenant else "") + ".") + return None, 0 + return ids, 0 + if args.task_id: + return [args.task_id], 0 + return None, _err(f"kanban: {verb} requires a task id or --all", 2) + + +def _run_triage_sweep(args: argparse.Namespace, verb: str, mod, run_one, json_key: str, + json_fields: tuple[str, ...], human_ok) -> int: + """Shared driver for ``specify`` / ``decompose``: validate ids, run + ``run_one(tid, author=...)`` per id, print JSON or human lines, exit code.""" + all_flag = bool(getattr(args, "all_triage", False)) author = getattr(args, "author", None) or _profile_author() want_json = bool(getattr(args, "json", False)) - - if args.task_id and all_flag: - print( - "kanban: pass either a task id OR --all, not both", - file=sys.stderr, - ) - return 2 - - if all_flag: - ids = spec.list_triage_ids(tenant=tenant) - if not ids: - msg = ( - "No triage tasks" - + (f" for tenant {tenant!r}" if tenant else "") - + "." - ) - if want_json: - print(json.dumps({"specified": 0, "total": 0})) - else: - print(msg) - return 0 - elif args.task_id: - ids = [args.task_id] - else: - print( - "kanban: specify requires a task id or --all", - file=sys.stderr, - ) - return 2 + ids, rc = _triage_sweep_ids(args, verb, mod.list_triage_ids, json_key) + if ids is None: + return rc ok_count = 0 - fail_count = 0 for tid in ids: - outcome = spec.specify_task(tid, author=author) + outcome = run_one(tid, author=author) if outcome.ok: ok_count += 1 - else: - fail_count += 1 if want_json: - print(json.dumps({ - "task_id": outcome.task_id, - "ok": outcome.ok, - "reason": outcome.reason, - "new_title": outcome.new_title, - })) + print(json.dumps(_obj_dict(outcome, json_fields))) elif outcome.ok: - title_suffix = ( - f" — retitled: {outcome.new_title!r}" - if outcome.new_title - else "" - ) - print(f"Specified {outcome.task_id} → todo{title_suffix}") + print(human_ok(outcome)) else: - print( - f"kanban: specify {outcome.task_id}: {outcome.reason}", - file=sys.stderr, - ) + print(f"kanban: {verb} {outcome.task_id}: {outcome.reason}", file=sys.stderr) if not all_flag: return 0 if ok_count == 1 else 1 # --all: succeed if at least one promotion landed; exit 1 only when @@ -3255,90 +2639,43 @@ def _cmd_specify(args: argparse.Namespace) -> int: return 0 if (ok_count > 0 or not ids) else 1 +def _retitled_suffix(outcome) -> str: + return f" — retitled: {outcome.new_title!r}" if outcome.new_title else "" + + +def _cmd_specify(args: argparse.Namespace) -> int: + """Flesh out a triage task (or all of them) via auxiliary LLM, then + promote to todo. Thin wrapper over ``kanban_specify``.""" + from hermes_cli import kanban_specify as spec + + return _run_triage_sweep( + args, "specify", spec, spec.specify_task, "specified", + ("task_id", "ok", "reason", "new_title"), + lambda o: f"Specified {o.task_id} → todo{_retitled_suffix(o)}", + ) + + +def _decompose_ok_line(o) -> str: + if o.fanout and o.child_ids: + return (f"Decomposed {o.task_id} → {len(o.child_ids)} " + f"children ({', '.join(o.child_ids)}); root promoted to todo") + return f"Specified {o.task_id} → todo (no fanout){_retitled_suffix(o)}" + + def _cmd_decompose(args: argparse.Namespace) -> int: - """Fan a triage task (or all of them) out into a graph of child - tasks via the auxiliary LLM, routed to specialist profiles by - description. Thin wrapper over ``kanban_decompose``.""" + """Fan a triage task (or all of them) out into a graph of child tasks via + the auxiliary LLM. Thin wrapper over ``kanban_decompose``.""" from hermes_cli import kanban_decompose as decomp - all_flag = bool(getattr(args, "all_triage", False)) - tenant = getattr(args, "tenant", None) - author = getattr(args, "author", None) or _profile_author() - want_json = bool(getattr(args, "json", False)) - - if args.task_id and all_flag: - print( - "kanban: pass either a task id OR --all, not both", - file=sys.stderr, - ) - return 2 - - if all_flag: - ids = decomp.list_triage_ids(tenant=tenant) - if not ids: - msg = ( - "No triage tasks" - + (f" for tenant {tenant!r}" if tenant else "") - + "." - ) - if want_json: - print(json.dumps({"decomposed": 0, "total": 0})) - else: - print(msg) - return 0 - elif args.task_id: - ids = [args.task_id] - else: - print( - "kanban: decompose requires a task id or --all", - file=sys.stderr, - ) - return 2 - - ok_count = 0 - for tid in ids: - outcome = decomp.decompose_task(tid, author=author) - if outcome.ok: - ok_count += 1 - if want_json: - print(json.dumps({ - "task_id": outcome.task_id, - "ok": outcome.ok, - "reason": outcome.reason, - "fanout": outcome.fanout, - "child_ids": outcome.child_ids, - "new_title": outcome.new_title, - })) - elif outcome.ok: - if outcome.fanout and outcome.child_ids: - child_summary = ", ".join(outcome.child_ids) - print( - f"Decomposed {outcome.task_id} → {len(outcome.child_ids)} " - f"children ({child_summary}); root promoted to todo" - ) - else: - title_suffix = ( - f" — retitled: {outcome.new_title!r}" - if outcome.new_title - else "" - ) - print( - f"Specified {outcome.task_id} → todo " - f"(no fanout){title_suffix}" - ) - else: - print( - f"kanban: decompose {outcome.task_id}: {outcome.reason}", - file=sys.stderr, - ) - if not all_flag: - return 0 if ok_count == 1 else 1 - return 0 if (ok_count > 0 or not ids) else 1 + return _run_triage_sweep( + args, "decompose", decomp, decomp.decompose_task, "decomposed", + ("task_id", "ok", "reason", "fanout", "child_ids", "new_title"), + _decompose_ok_line, + ) def _cmd_gc(args: argparse.Namespace) -> int: - """Remove scratch workspaces of archived tasks, prune old events, and - delete old worker logs.""" + """Remove archived tasks' scratch workspaces, old events, and old worker logs.""" import shutil scratch_root = kb.workspaces_root() removed_ws = 0 @@ -3349,9 +2686,8 @@ def _cmd_gc(args: argparse.Namespace) -> int: ).fetchall() for row in rows: if row["workspace_kind"] == "worktree": - # Backstop for worktrees that escaped the completion/archive hook - # (e.g. tasks archived before that hook existed). Same safety - # predicate: only clean, fully-pushed worktrees are removed. + # Backstop for worktrees that escaped the completion/archive hook. + # Same safety predicate: only clean, fully-pushed worktrees go. wt_path = row["workspace_path"] if wt_path and Path(wt_path).is_dir(): kb._cleanup_worktree_workspace(row["id"], wt_path, row["branch_name"]) @@ -3389,22 +2725,16 @@ def _cmd_gc(args: argparse.Namespace) -> int: def _cmd_repair(args: argparse.Namespace) -> int: - """Check DB integrity and apply the narrow index-REINDEX auto-repair. - - Dispatched BEFORE the auto ``kb.init_db()`` in :func:`kanban_command` - (init itself refuses corrupt DBs), so this is reachable on exactly the - boards that need it. Exit codes: 0 = healthy / repaired / no DB file, - 1 = still corrupt (non-index corruption, or REINDEX did not produce a - clean re-check). - """ + """Integrity check + narrow index-REINDEX auto-repair. Dispatched BEFORE + the auto ``kb.init_db()`` (init refuses corrupt DBs). Exit 0 = healthy / + repaired / no DB file, 1 = still corrupt.""" try: report = kb.repair_db() except Exception as exc: # locked/busy probe, unexpected I/O - print(f"kanban repair: {exc}", file=sys.stderr) - return 1 + return _err(f"kanban repair: {exc}") if getattr(args, "json", False): - print(json.dumps({ + _print_json({ "status": report.status, "db_path": str(report.db_path), "messages": report.messages, @@ -3413,7 +2743,7 @@ def _cmd_repair(args: argparse.Namespace) -> int: str(report.backup_path) if report.backup_path else None ), "reindexed": report.reindexed, - }, indent=2)) + }, ascii=True) return 0 if report.status in {"ok", "repaired", "missing"} else 1 if report.status == "missing": @@ -3458,6 +2788,29 @@ def _cmd_repair(args: argparse.Namespace) -> int: return 1 +_HANDLERS = { + "init": _cmd_init, "create": _cmd_create, "swarm": _cmd_swarm, + "list": _cmd_list, "ls": _cmd_list, "show": _cmd_show, + "assign": _cmd_assign, "set-model": _cmd_set_model, + "reclaim": _cmd_reclaim, "reassign": _cmd_reassign, + "diagnostics": _cmd_diagnostics, "diag": _cmd_diagnostics, + "link": _cmd_link, "unlink": _cmd_unlink, "claim": _cmd_claim, + "comment": _cmd_comment, "attach": _cmd_attach, + "attachments": _cmd_attachments, "attach-rm": _cmd_attach_rm, + "complete": _cmd_complete, "edit": _cmd_edit, "block": _cmd_block, + "schedule": _cmd_schedule, "unblock": _cmd_unblock, + "request-review": _cmd_request_review, "request-changes": _cmd_request_changes, + "reopen-review": _cmd_reopen_review, "promote": _cmd_promote, + "archive": _cmd_archive, "tail": _cmd_tail, "dispatch": _cmd_dispatch, + "daemon": _cmd_daemon, "watch": _cmd_watch, "stats": _cmd_stats, + "log": _cmd_log, "runs": _cmd_runs, "heartbeat": _cmd_heartbeat, + "assignees": _cmd_assignees, "notify-subscribe": _cmd_notify_subscribe, + "notify-list": _cmd_notify_list, "notify-unsubscribe": _cmd_notify_unsubscribe, + "context": _cmd_context, "specify": _cmd_specify, "decompose": _cmd_decompose, + "gc": _cmd_gc, +} + + # --------------------------------------------------------------------------- # Slash-command entry point (used by /kanban from CLI and gateway) # --------------------------------------------------------------------------- @@ -3490,26 +2843,22 @@ Read-only commands are safe while an agent is running.\ def run_slash(rest: str) -> str: """Execute a ``/kanban …`` string and return captured stdout/stderr. - ``rest`` is everything after ``/kanban`` (may be empty). Used from - both the interactive CLI (``self._handle_kanban_command``) and the - gateway (``_handle_kanban_command``) so formatting is identical. + ``rest`` is everything after ``/kanban``. Shared by the interactive CLI + and the gateway so formatting is identical. """ import io - import contextlib tokens = shlex.split(rest) if rest and rest.strip() else [] - # Bare ``/kanban`` or ``/kanban help`` / ``--help`` / ``-h`` / ``?``: - # show the curated short-help block instead of dumping argparse's full - # usage tree (which is enormous and reads as garbage in a chat - # bubble). Per-subcommand help still works via ``/kanban foo -h``. + # Bare ``/kanban`` / ``help`` / ``-h``: the curated short block, not + # argparse's full usage tree (garbage in a chat bubble). Per-subcommand + # help still works via ``/kanban foo -h``. if not tokens or tokens[0] in {"help", "--help", "-h", "?"}: return _SLASH_KANBAN_HELP - # Single argparse tree rooted at "/kanban". build_parser() expects a - # subparsers action to attach to, so build a throwaway one and pull - # the kanban_parser back out — then drive it directly so usage/error - # text reads as ``/kanban`` (not ``/kanban-wrap kanban``). + # build_parser() needs a subparsers action to attach to, so build a + # throwaway one and pull kanban_parser back out; drive it directly so + # usage/error text reads as ``/kanban`` (not ``/kanban-wrap kanban``). _wrap = argparse.ArgumentParser(prog="/kanban-wrap", add_help=False) _wrap.exit_on_error = False # type: ignore[attr-defined] _top_sub = _wrap.add_subparsers(dest="_top") @@ -3533,9 +2882,7 @@ def run_slash(rest: str) -> str: buf_out = io.StringIO() buf_err = io.StringIO() - # ``-h`` / ``--help`` makes argparse print to stdout and SystemExit(0). - # Capture both streams so neither the help text nor the error text - # bypasses our buffer. + # ``-h`` prints to stdout and SystemExit(0); capture both streams. try: with contextlib.redirect_stdout(buf_out), contextlib.redirect_stderr(buf_err): args = kanban_parser.parse_args(tokens) diff --git a/hermes_cli/kanban_decompose.py b/hermes_cli/kanban_decompose.py index d6c248c658..fffe52850f 100644 --- a/hermes_cli/kanban_decompose.py +++ b/hermes_cli/kanban_decompose.py @@ -11,40 +11,26 @@ so when the whole graph completes the root wakes back up — its assignee (the orchestrator profile) gets a chance to judge completion and add more tasks if the work isn't done yet. -Design notes ------------- - -* Mirrors the shape of ``hermes_cli/kanban_specify.py``: lazy aux - client import inside the function, lenient response parse, never - raises on expected failure modes. - -* The system prompt sees the *configured* profile roster — names plus - descriptions plus the default fallback. Profiles without a - description are still listed (with a note) so the decomposer can - match on name as a fallback, but the user has an obvious incentive - to describe them. - -* ``fanout=false`` collapses to the same effect as ``kanban specify``: - we tighten the body and flip ``triage -> todo`` as a single task, - no children created. This makes ``decompose`` a strict superset of - ``specify`` from the user's perspective. - -* If the LLM picks an assignee that doesn't exist as a profile, we - rewrite it to the configured ``default_assignee`` (or the default - profile if unset). A child task NEVER ends up with ``assignee=None``. +Design notes: mirrors ``kanban_specify`` (lazy aux import, lenient parse, +never raises on expected failures). The prompt sees the configured profile +roster; undescribed profiles are listed with a note so name-matching still +works. ``fanout=false`` collapses to the ``specify`` behaviour (tighten + +promote, no children), making ``decompose`` a strict superset. Unknown +assignees are rewritten to ``default_assignee`` — a child NEVER ends up with +``assignee=None``. """ from __future__ import annotations -import json import logging -import os import re from dataclasses import dataclass from typing import Optional from hermes_cli import kanban_db as kb from hermes_cli import profiles as profiles_mod +from hermes_cli.kanban_specify import _extract_json_blob, _title_body, _truncate +from hermes_cli.kanban_specify import _profile_author as _specify_author logger = logging.getLogger(__name__) @@ -136,37 +122,9 @@ class DecomposeOutcome: new_title: Optional[str] = None -def _truncate(text: str, limit: int) -> str: - if len(text) <= limit: - return text - return text[: limit - 1] + "…" - - -def _extract_json_blob(raw: str) -> Optional[dict]: - if not raw: - return None - stripped = _FENCE_RE.sub("", raw.strip()) - first = stripped.find("{") - last = stripped.rfind("}") - if first == -1 or last == -1 or last <= first: - return None - candidate = stripped[first : last + 1] - try: - val = json.loads(candidate) - except (ValueError, json.JSONDecodeError): - return None - if not isinstance(val, dict): - return None - return val - - def _profile_author() -> str: """Mirror of ``hermes_cli.kanban._profile_author``.""" - return ( - os.environ.get("HERMES_PROFILE") - or os.environ.get("USER") - or "decomposer" - ) + return _specify_author("decomposer") def _load_config() -> dict: @@ -177,31 +135,15 @@ def _load_config() -> dict: return {} -def _resolve_orchestrator_profile(cfg: dict) -> str: - """Resolve which profile owns the root/orchestration task after fan-out. +def _resolve_profile_from_cfg(cfg: dict, key: str) -> str: + """``kanban.`` if it names an existing profile, else the active + default profile — so a task is never stranded for lack of an owner. - Falls back to the active default profile when ``kanban.orchestrator_profile`` - is unset, so a task is never stranded for lack of an orchestrator. + ``orchestrator_profile`` owns the root after fan-out; ``default_assignee`` + catches children the decomposer can't route. """ kanban_cfg = cfg.get("kanban", {}) if isinstance(cfg, dict) else {} - explicit = (kanban_cfg.get("orchestrator_profile") or "").strip() - if explicit: - try: - if profiles_mod.profile_exists(explicit): - return explicit - except Exception: - pass - # Fall back to the active default profile. - try: - return profiles_mod.get_active_profile_name() or "default" - except Exception: - return "default" - - -def _resolve_default_assignee(cfg: dict) -> str: - """Resolve which profile catches child tasks the orchestrator can't route.""" - kanban_cfg = cfg.get("kanban", {}) if isinstance(cfg, dict) else {} - explicit = (kanban_cfg.get("default_assignee") or "").strip() + explicit = (kanban_cfg.get(key) or "").strip() if explicit: try: if profiles_mod.profile_exists(explicit): @@ -215,12 +157,8 @@ def _resolve_default_assignee(cfg: dict) -> str: def _build_roster() -> tuple[list[dict], set[str]]: - """Return (roster_for_prompt, valid_assignee_names). - - Each roster entry is ``{name, description, has_description}``. The - valid-set is used after the LLM responds to rewrite invalid - assignees to the default fallback. - """ + """``(roster_for_prompt, valid_assignee_names)``; entries are + ``{name, description, has_description}``.""" roster: list[dict] = [] valid: set[str] = set() try: @@ -255,11 +193,8 @@ def _normalize_assignee_choice( default_assignee: str, valid_names: set[str], ) -> str: - """Return a valid assignee, falling back to ``default_assignee``. - - Fan-out children and the single-task fallback should share the same - routing guarantee: promoted work must not be left unassigned. - """ + """A valid assignee, else ``default_assignee`` — promoted work is never + left unassigned.""" if not isinstance(assignee, str) or not assignee.strip(): return default_assignee chosen = assignee.strip() @@ -274,13 +209,9 @@ def decompose_task( author: Optional[str] = None, timeout: Optional[int] = None, ) -> DecomposeOutcome: - """Decompose a triage task into a graph of child tasks. - - Returns an outcome describing what happened. Never raises for - expected failure modes (task not in triage, no aux client - configured, API error, malformed response, decomposer returned - fanout=true with empty task list) — those surface via ``ok=False``. - """ + """Decompose a triage task into a graph of child tasks. Expected failures + (not in triage, no aux client, API error, malformed/empty reply) surface + as ``ok=False``.""" with kb.connect_closing() as conn: task = kb.get_task(conn, task_id) if task is None: @@ -291,8 +222,8 @@ def decompose_task( ) cfg = _load_config() - orchestrator = _resolve_orchestrator_profile(cfg) - default_assignee = _resolve_default_assignee(cfg) + orchestrator = _resolve_profile_from_cfg(cfg, "orchestrator_profile") + default_assignee = _resolve_profile_from_cfg(cfg, "default_assignee") kanban_cfg = cfg.get("kanban", {}) if isinstance(cfg, dict) else {} auto_promote = bool(kanban_cfg.get("auto_promote_children", True)) roster, valid_names = _build_roster() @@ -312,10 +243,8 @@ def decompose_task( ) try: - # Route through call_llm so auxiliary.kanban_decomposer.* config - # (provider/model/base_url, extra_body, reasoning_effort, retries) - # all apply — the previous direct client.chat.completions.create() - # path dropped auxiliary..extra_body entirely (#35566). + # call_llm applies all auxiliary.kanban_decomposer.* config + # (provider/model/base_url, extra_body, reasoning_effort, retries). resp = call_llm( task="kanban_decomposer", messages=[ @@ -337,7 +266,7 @@ def decompose_task( except Exception: raw = "" - parsed = _extract_json_blob(raw) + parsed = _extract_json_blob(raw, _FENCE_RE) if parsed is None: return DecomposeOutcome(task_id, False, "LLM returned malformed JSON") @@ -346,10 +275,7 @@ def decompose_task( if not fanout: # Fall back to single-task spec promotion (same effect as specify). - new_title = parsed.get("title") - new_body = parsed.get("body") - title_val = new_title.strip() if isinstance(new_title, str) and new_title.strip() else None - body_val = new_body if isinstance(new_body, str) and new_body.strip() else None + title_val, body_val = _title_body(parsed) assignee_val = None if not task.assignee: assignee_val = _normalize_assignee_choice( @@ -385,8 +311,7 @@ def decompose_task( task_id, False, "decomposer returned fanout=true with empty tasks list", ) - # Rewrite invalid assignees to the default fallback. Never leave a - # task with assignee=None — the user explicitly does not want that. + # Unknown assignees route to the default; never assignee=None. children: list[dict] = [] for idx, entry in enumerate(raw_tasks): if not isinstance(entry, dict): diff --git a/hermes_cli/kanban_diagnostics.py b/hermes_cli/kanban_diagnostics.py index 1a0309aa01..61d1e201ce 100644 --- a/hermes_cli/kanban_diagnostics.py +++ b/hermes_cli/kanban_diagnostics.py @@ -1,30 +1,13 @@ """Kanban diagnostics — structured, actionable distress signals for tasks. -A ``Diagnostic`` is a machine-readable description of something that's wrong -with a kanban task: a hallucinated card id, a spawn crash-loop, a task -stuck blocked for too long, etc. Each one carries: +A ``Diagnostic`` carries a **kind** (canonical code the UI/tests match on), a +**severity**, title/detail text, and **actions** the dashboard renders as +buttons and the CLI as hints. Rules are stateless and read-only over +(task, events, runs, optional graph); callers compute on demand. -* A **kind** (canonical code; UI/tests match on this). -* A **severity** (``warning`` / ``error`` / ``critical``). -* A **title** (one-line human description) and **detail** (longer text). -* A list of **suggested actions** — structured entries the dashboard - turns into buttons and the CLI turns into hints. - -Rules run over (task, recent events, recent runs, optional graph context) and -emit diagnostics. They are stateless and read-only — no DB writes. Callers compute -diagnostics on demand (on ``/board`` load, ``/tasks/:id`` fetch, or -``hermes kanban diagnostics``). - -Design goals: - -* Fixable-on-the-operator's-side signals only (missing config, phantom - ids, crash loop). Not "the provider returned 502 once" — that's a - transient runtime blip, not a diagnostic. -* Recoverable: every diagnostic comes with at least one suggested - recovery action the operator can actually take from the UI. -* Auto-clearing: when the underlying failure mode resolves (a clean - ``completed`` event arrives, a spawn succeeds, the task gets - unblocked), the diagnostic stops firing. The audit event trail stays. +Design goals: operator-fixable signals only (not a one-off provider 502); +every diagnostic has at least one recovery action; diagnostics auto-clear +when the failure mode resolves (the audit event trail stays). """ from __future__ import annotations @@ -35,9 +18,7 @@ import json import time -# Severity rungs, ordered least → most urgent. The UI colors them -# amber (warning), orange (error), red (critical). Sorted outputs put -# critical first so operators see the worst fires at the top. +# Least → most urgent; sorted outputs put critical first. SEVERITY_ORDER = ("warning", "error", "critical") @@ -52,24 +33,10 @@ def severity_at_or_above(severity: Optional[str], threshold: Optional[str]) -> b @dataclass class DiagnosticAction: - """A single recovery action attached to a diagnostic. - - The ``kind`` determines how both the UI and CLI render it: - - * ``reclaim`` / ``reassign`` — POST to the matching /tasks/:id/* - endpoint; dashboard wires into the existing recovery popover. - * ``unblock`` — PATCH status back to ``ready`` (for stuck-blocked - diagnostics). - * ``cli_hint`` — print/copy a shell command (e.g. - ``hermes -p auth``). No HTTP side effect. - * ``open_docs`` — deep-link to the docs URL named in ``payload.url``. - * ``comment`` — nudge the operator to add a comment (for - stuck-blocked tasks that need human input). - - ``suggested=True`` marks the action as the recommended first step; - the UI highlights it. Multiple actions can be suggested if they're - equally valid. - """ + """A recovery action. ``kind`` drives rendering: ``reclaim``/``reassign`` + POST to /tasks/:id/*; ``unblock`` PATCHes status to ready; ``cli_hint`` + shows ``payload.command``; ``open_docs`` links ``payload.url``; ``comment`` + nudges the operator. ``suggested=True`` = recommended first step.""" kind: str label: str @@ -122,20 +89,10 @@ class Diagnostic: # --------------------------------------------------------------------------- def _task_field(task, name, default=None): - """Read a field from a task regardless of representation. - - Callers pass sqlite3.Row (dict-like with [] but no attribute - access), kanban_db.Task dataclasses (attribute access), or plain - dicts (both). This normalises them so rule functions don't have - to branch on type each time. - """ + """Read a field from a sqlite3.Row, a kanban_db.Task dataclass, or a dict.""" if task is None: return default - # sqlite Row + plain dicts both support mapping access; Row also - # supports .keys(). try: - # Row raises IndexError if the key isn't a column in the query; - # dicts return default via .get. Handle both. if hasattr(task, "keys") and name in task.keys(): return task[name] except Exception: @@ -169,20 +126,42 @@ def _event_ts(ev) -> int: return int(t or 0) +def _first_field(task, primary: str, legacy: str, default=None): + """``task[primary]`` unless it is None, else ``task[legacy]`` (old DB rows).""" + v = _task_field(task, primary, None) + return v if v is not None else _task_field(task, legacy, default) + + +def _latest_event_ts(events: Iterable[Any], kinds: set[str]) -> int: + """Max ``created_at`` over events whose kind is in ``kinds`` (0 if none).""" + latest = 0 + for ev in events: + if _event_kind(ev) in kinds: + latest = max(latest, _event_ts(ev)) + return latest + + +def _log_hint_action(task_id: str) -> DiagnosticAction: + return DiagnosticAction( + kind="cli_hint", + label=f"Check logs: hermes kanban log {task_id}", + payload={"command": f"hermes kanban log {task_id}"}, + suggested=True, + ) + + +def _error_snippet(last_err) -> str: + """First 500 chars of the error (with ellipsis), or "" when absent.""" + err_text = (last_err or "").strip() if last_err else "" + return err_text[:500] + ("…" if len(err_text) > 500 else "") if err_text else "" + + def _active_hallucination_events( events: Iterable[Any], kind: str, ) -> list[Any]: - """Return events of ``kind`` that have no ``completed``/``edited`` - event *strictly after* them. Walks chronologically: each clean - event resets the accumulator; each matching event gets appended. - - Events must be sorted by id (i.e. arrival order); callers pass the - task's full event list which the DB already returns in that order. - """ - # Events arrive sorted by id asc (chronological). Walk once, track - # which hallucination events are still "active" (no clean event - # supersedes them). + """Events of ``kind`` with no ``completed``/``edited`` event strictly after + them. Requires id-sorted (arrival-order) input, which the DB provides.""" active: list[Any] = [] for ev in events: k = _event_kind(ev) @@ -191,9 +170,9 @@ def _active_hallucination_events( elif k == kind: active.append(ev) return active -# Standard always-available actions. Every diagnostic can offer these as -# fallbacks regardless of kind — they're the two baseline recovery -# primitives the kernel supports. + + +# Baseline recovery primitives every diagnostic can fall back on. def _generic_recovery_actions(task: Any, *, running: bool) -> list[DiagnosticAction]: out: list[DiagnosticAction] = [] if running: @@ -214,22 +193,16 @@ def _generic_recovery_actions(task: Any, *, running: bool) -> list[DiagnosticAct # Rule implementations # --------------------------------------------------------------------------- -# Each rule takes (task, events, runs, now_ts, config) and returns -# zero or more Diagnostic instances. ``events`` / ``runs`` are lists of -# kanban_db.Event / kanban_db.Run (or plain dicts matching the same -# shape — for test convenience). +# Each rule: (task, events, runs, now_ts, config) -> list[Diagnostic]. +# ``events``/``runs`` are kanban_db rows/dataclasses or same-shaped dicts. RuleFn = Callable[[Any, list[Any], list[Any], int, dict], list[Diagnostic]] def _aux_slot_explicit(slot: Any) -> bool: - """Return True if the auxiliary slot has user-supplied non-default fields. - - Defaults from ``DEFAULT_CONFIG`` use ``provider: "auto"`` with empty - model/base_url/api_key — that path falls through to the main model. An - "explicit" config is one where the user actively set a provider (not - "auto"), or supplied a model / base_url / api_key. - """ + """True if the aux slot was user-configured: provider other than "auto", + or any of model/base_url/api_key set (the default falls through to the + main model).""" if not isinstance(slot, dict): return False provider = str(slot.get("provider") or "").strip().lower() @@ -242,12 +215,9 @@ def _aux_slot_explicit(slot: Any) -> bool: def _main_model_visible(raw_config: Any) -> bool: - """Best-effort check that a main model is configured. - - Diagnostics runs in the dashboard process which may not share the CLI's - runtime state, so we read the raw config dict. If we cannot prove the - main model is set, we err on the side of NOT firing the diagnostic. - """ + """Best-effort "a main model is configured" from the raw config dict (the + dashboard process may not share CLI runtime state). Unprovable => False, + which errs toward NOT firing the diagnostic.""" if not isinstance(raw_config, dict): return False model_cfg = raw_config.get("model") @@ -264,17 +234,9 @@ def _main_model_visible(raw_config: Any) -> bool: def triage_aux_status(config: Optional[dict]) -> Optional[dict]: - """Inspect raw config and report whether triage paths look configured. - - Returns ``None`` when config context is unavailable (suppress diagnostic - to avoid noisy false positives in tests / low-level callers). Otherwise - returns a dict with: - - - ``auto_decompose``: bool — whether the dispatcher auto-runs decompose - - ``decomposer_explicit``: bool — user-supplied decomposer slot - - ``specifier_explicit``: bool — user-supplied specifier slot - - ``main_model_visible``: bool — main model can serve as auto fallback - """ + """Report whether the triage aux paths look configured: ``{auto_decompose, + decomposer_explicit, specifier_explicit, main_model_visible}``. ``None`` + when no config context is present (keeps low-level callers/tests silent).""" if not isinstance(config, dict): return None @@ -285,9 +247,7 @@ def triage_aux_status(config: Optional[dict]) -> Optional[dict]: aux = config.get("auxiliary") kanban_cfg = config.get("kanban") if isinstance(config.get("kanban"), dict) else {} - # Have we been handed any config context at all? When neither auxiliary - # nor kanban nor model keys are present, the caller is a low-level test - # passing {} — stay silent. + # No auxiliary/kanban/model keys at all => a low-level caller passing {}. if ( not isinstance(aux, dict) and not kanban_cfg @@ -323,14 +283,8 @@ def _positive_int(value: Any, default: int) -> int: def _rule_hallucinated_cards(task, events, runs, now, cfg) -> list[Diagnostic]: - """Blocked-hallucination gate fires: a worker called kanban_complete - with created_cards that didn't exist or weren't created by the - completing profile. Task stayed in its prior state; the operator - needs to decide how to proceed. - - Auto-clears when a successful completion (or edit) follows the - blocked event. - """ + """A worker's kanban_complete named created_cards that don't exist / weren't + its own; the completion was blocked. Clears on a later completion/edit.""" hits = _active_hallucination_events(events, "completion_blocked_hallucination") if not hits: return [] @@ -343,13 +297,11 @@ def _rule_hallucinated_cards(task, events, runs, now, cfg) -> list[Diagnostic]: if pid not in phantom_ids: phantom_ids.append(pid) running = _task_field(task, "status") == "running" - actions: list[DiagnosticAction] = [] - actions.append(DiagnosticAction( + actions = [DiagnosticAction( kind="comment", label="Add a comment explaining what to do", suggested=False, - )) - actions.extend(_generic_recovery_actions(task, running=running)) + )] + _generic_recovery_actions(task, running=running) return [Diagnostic( kind="hallucinated_cards", severity="error", @@ -370,22 +322,12 @@ def _rule_hallucinated_cards(task, events, runs, now, cfg) -> list[Diagnostic]: def _rule_triage_aux_unavailable(task, events, runs, now, cfg) -> list[Diagnostic]: - """A triage task cannot leave triage without an auxiliary helper. - - With the auto-decompose dispatcher (kanban.auto_decompose, default True), - triage tasks fan out via ``auxiliary.kanban_decomposer`` and fall back to - ``auxiliary.triage_specifier`` when the decomposer returns ``fanout=false``. - With auto-decompose off, the user must run ``hermes kanban specify``, - which only needs ``auxiliary.triage_specifier``. - - The default slot is ``provider: auto`` → auto-falls back to the main model, - so this rule only fires when: - - - the relevant slot is explicitly set to something broken, OR - - the auto fallback has no main model to fall back to. - - Config context is required; pass {} from tests to keep the rule silent. - """ + """A triage task can't leave triage without a usable aux model. With + auto-decompose on the primary slot is ``auxiliary.kanban_decomposer`` + (specifier as fallback); off, it is ``auxiliary.triage_specifier``. The + default ``provider: auto`` falls back to the main model, so this fires only + when the slot isn't explicit AND no main model is visible. Requires config + context ({} keeps it silent).""" if _task_field(task, "status") != "triage": return [] @@ -421,9 +363,6 @@ def _rule_triage_aux_unavailable(task, events, runs, now, cfg) -> list[Diagnosti "`hermes kanban specify`, which uses auxiliary.triage_specifier." ) - # The primary slot is usable when either: it was explicitly configured by - # the user, OR the default `provider: auto` can fall back to the main - # model. If both fail, we have a real configuration gap. if primary_explicit or main_visible: return [] @@ -482,12 +421,8 @@ def _rule_triage_aux_unavailable(task, events, runs, now, cfg) -> list[Diagnosti def _rule_prose_phantom_refs(task, events, runs, now, cfg) -> list[Diagnostic]: - """Advisory prose-scan: the completion summary mentions ``t_`` - ids that don't resolve. Non-blocking; surfaced as a warning only. - - Auto-clears when a fresh clean completion arrives AFTER the - suspected event. - """ + """Advisory: the completion summary mentions ``t_`` ids that don't + resolve. Warning only; clears on a later clean completion.""" hits = _active_hallucination_events(events, "suspected_hallucinated_references") if not hits: return [] @@ -516,32 +451,14 @@ def _rule_prose_phantom_refs(task, events, runs, now, cfg) -> list[Diagnostic]: def _rule_repeated_failures(task, events, runs, now, cfg) -> list[Diagnostic]: - """Task's unified ``consecutive_failures`` counter is climbing — - something about this task+profile combo is broken and each retry - fails the same way. Triggers regardless of the specific failure - mode (spawn error, timeout, crash) because operationally they - all look the same: the kernel keeps retrying and the operator - needs to intervene. + """``consecutive_failures`` >= cfg["failure_threshold"] (legacy key + ``spawn_failure_threshold``), regardless of failure mode — the kernel keeps + retrying and the operator must intervene. Runtime callers derive the + threshold from ``kanban.failure_limit`` so it doesn't lag the breaker. - Threshold: cfg["failure_threshold"]. Runtime callers should derive - this from ``kanban.failure_limit`` unless the user explicitly set a - diagnostics threshold, so the signal does not lag behind the - dispatcher's circuit breaker. - - Accepts the legacy ``spawn_failure_threshold`` config key for - back-compat. - - Terminal statuses are exempt: a done/archived card has nothing left - to retry, so a lingering failure streak is history, not a signal. - (``complete_task`` resets the counter, but a manual done — e.g. a - dashboard drag — ends no run and used to leave the flag stuck.) - - A fresh attempt in flight (``running``) is also exempt: retrying a - task should clear the stale failure banner until this attempt also - resolves. Otherwise a card that's actively trying again still shows - "failed Nx", which reads as a current failure. It re-fires if the new - run fails too (status leaves ``running`` with a recorded outcome). - """ + Exempt: done/archived (a manual done ends no run, so the streak is history) + and running (a retry in flight must not read as a current failure; re-fires + if it fails too).""" if _task_field(task, "status") in ("done", "archived", "running"): return [] threshold = _positive_int(cfg.get( @@ -549,26 +466,13 @@ def _rule_repeated_failures(task, events, runs, now, cfg) -> list[Diagnostic]: cfg.get("spawn_failure_threshold", 3), ), 3) failure_limit = _positive_int(cfg.get("failure_limit"), threshold) - # Read the new unified counter name, with a fallback to the legacy - # column name so this rule keeps working against old DB rows the - # caller somehow materialised without running the migration. - failures = ( - _task_field(task, "consecutive_failures", None) - if _task_field(task, "consecutive_failures", None) is not None - else _task_field(task, "spawn_failures", 0) - ) + failures = _first_field(task, "consecutive_failures", "spawn_failures", 0) if failures is None or failures < threshold: return [] - last_err = ( - _task_field(task, "last_failure_error", None) - if _task_field(task, "last_failure_error", None) is not None - else _task_field(task, "last_spawn_error", None) - ) + last_err = _first_field(task, "last_failure_error", "last_spawn_error") assignee = _task_field(task, "assignee") - # Classify the most recent failure by peeking at run outcomes so - # the title + suggested action can be specific without a separate - # per-outcome rule. + # Most recent failure outcome makes the title/action specific. ordered_runs = sorted(runs, key=lambda r: _task_field(r, "id", 0)) most_recent_outcome = None for r in reversed(ordered_runs): @@ -596,19 +500,13 @@ def _rule_repeated_failures(task, events, runs, now, cfg) -> list[Diagnostic]: # to diagnose; reclaim/reassign are the recovery levers. task_id = _task_field(task, "id") if task_id: - actions.append(DiagnosticAction( - kind="cli_hint", - label=f"Check logs: hermes kanban log {task_id}", - payload={"command": f"hermes kanban log {task_id}"}, - suggested=True, - )) + actions.append(_log_hint_action(task_id)) actions.extend(_generic_recovery_actions( task, running=_task_field(task, "status") == "running", )) severity = "critical" if failures >= threshold * 2 else "error" - err_text = (last_err or "").strip() if last_err else "" - err_snippet = err_text[:500] + ("…" if len(err_text) > 500 else "") if err_text else "" + err_snippet = _error_snippet(last_err) outcome_label = { "spawn_failed": "spawn", "timed_out": "timeout", @@ -651,29 +549,14 @@ def _rule_repeated_failures(task, events, runs, now, cfg) -> list[Diagnostic]: def _rule_repeated_crashes(task, events, runs, now, cfg) -> list[Diagnostic]: - """The worker spawns fine but keeps crashing mid-run. Check the last - N runs' outcomes; N consecutive ``crashed`` without a successful - ``completed`` means something about the task + profile combo is - broken (OOM, missing dependency, tool it needs is down). + """Trailing run outcomes show >= cfg["crash_threshold"] (default 2) + consecutive ``crashed`` with no ``completed``/``reclaimed`` between. Fires + earlier than ``repeated_failures`` for a crash-specific heads-up and + suppresses itself when the unified rule is about to fire. - Threshold: cfg["crash_threshold"] (default 2). - - Narrower than ``repeated_failures`` — fires earlier (2 crashes vs 3 - total failures) so the operator gets a crash-specific heads-up - before the unified rule kicks in. Suppresses itself when the - unified rule is also about to fire, to avoid double-flagging. - - Terminal statuses are exempt for the same reason as - ``repeated_failures`` — with one extra wrinkle: this rule reads run - history, and a manual done (dashboard drag) appends no ``completed`` - run to break the crash streak, so the flag was permanent (#kanban - desktop dogfood). Done means done. - - ``running`` is exempt too: a fresh attempt is in flight, and its - in-flight run (no outcome yet) doesn't break the trailing crash scan, - so a retried card kept showing "crashed Nx" over an active run. The - banner re-fires if the new attempt also crashes. - """ + Exempt: done/archived (a manual done appends no completed run, so the + streak would be permanent) and running (an in-flight run has no outcome + and wouldn't break the scan).""" if _task_field(task, "status") in ("done", "archived", "running"): return [] failure_threshold = int(cfg.get( @@ -702,29 +585,19 @@ def _rule_repeated_crashes(task, events, runs, now, cfg) -> list[Diagnostic]: # A success (or manual reclaim) breaks the streak. break else: - # Other outcomes (timed_out, blocked, spawn_failed, gave_up) - # aren't crash signals — don't count them, but they also - # don't break the crash streak. + # Other outcomes neither count as crashes nor break the streak. continue if consecutive < threshold: return [] task_id = _task_field(task, "id") actions: list[DiagnosticAction] = [] if task_id: - actions.append(DiagnosticAction( - kind="cli_hint", - label=f"Check logs: hermes kanban log {task_id}", - payload={"command": f"hermes kanban log {task_id}"}, - suggested=True, - )) + actions.append(_log_hint_action(task_id)) running = _task_field(task, "status") == "running" actions.extend(_generic_recovery_actions(task, running=running)) severity = "critical" if consecutive >= threshold * 2 else "error" - # Put the actual error up-front so operators see WHAT broke without - # having to open the logs. Truncate defensively — these can be huge - # (full tracebacks). - err_text = (last_err or "").strip() if last_err else "" - err_snippet = err_text[:500] + ("…" if len(err_text) > 500 else "") if err_text else "" + # Error up-front so operators see WHAT broke without opening the logs. + err_snippet = _error_snippet(last_err) if err_snippet: title = f"Agent crashed {consecutive}x: {err_snippet.splitlines()[0][:160]}" detail = ( @@ -751,14 +624,9 @@ def _rule_repeated_crashes(task, events, runs, now, cfg) -> list[Diagnostic]: def _rule_review_dependency_deadlock(task, events, runs, now, cfg) -> list[Diagnostic]: - """Detect a legacy review handoff that starves downstream children. - - Older workers were instructed to sticky-block an implementation with a - ``review-required:`` reason. A separately modelled reviewer child cannot - promote until that parent is terminal, so the lane has no autonomous next - step. This compatibility diagnostic is graph-aware but deliberately leaves - both the dependency graph and the user's sticky block unchanged. - """ + """Legacy review handoff starving children: the implementation is + sticky-blocked with a ``review-required:`` reason while todo children wait + for it to be terminal. Graph-aware; deliberately mutates nothing.""" if _task_field(task, "status") != "blocked": return [] @@ -829,21 +697,13 @@ def _rule_review_dependency_deadlock(task, events, runs, now, cfg) -> list[Diagn def _rule_stuck_in_blocked(task, events, runs, now, cfg) -> list[Diagnostic]: - """Task has been in ``blocked`` status for too long without a comment. - - Threshold: cfg["blocked_stale_hours"] (default 24). - Surfaced as a warning so humans know there's a pending unblock. - """ + """Blocked for >= cfg["blocked_stale_hours"] (default 24) with no comment + or unblock since the last ``blocked`` event.""" hours = float(cfg.get("blocked_stale_hours", 24)) status = _task_field(task, "status") if status != "blocked": return [] - # Find the most recent ``blocked`` event. - last_blocked_ts = 0 - for ev in events: - if _event_kind(ev) == "blocked": - t = _event_ts(ev) - last_blocked_ts = max(last_blocked_ts, t) + last_blocked_ts = _latest_event_ts(events, {"blocked"}) if last_blocked_ts == 0: return [] age_hours = (now - last_blocked_ts) / 3600.0 @@ -879,29 +739,17 @@ def _rule_stuck_in_blocked(task, events, runs, now, cfg) -> list[Diagnostic]: def _rule_block_unblock_cycling(task, events, runs, now, cfg) -> list[Diagnostic]: - """Task has cycled through blocked → unblocked many times — the - ``unblock`` is not fixing the underlying problem and the worker - keeps re-blocking for substantially the same reason. - - ``_rule_stuck_in_blocked`` resets its timer on any ``commented`` / - ``unblocked`` event, so a task that cycles every few minutes is - invisible to it regardless of how many times it cycles (#29747 - gap 1). This rule complements that one by counting block→unblock - cycles in a sliding window. - - Threshold: cfg["block_cycle_threshold"] (default 3) cycles within - cfg["block_cycle_window_seconds"] (default 24h). - """ + """>= cfg["block_cycle_threshold"] (default 3) blocked-after-unblocked + cycles within cfg["block_cycle_window_seconds"] (default 24h). Complements + ``_rule_stuck_in_blocked``, whose timer any unblock resets, so fast cyclers + are invisible to it.""" threshold = _positive_int(cfg.get("block_cycle_threshold"), 3) window_seconds = float(cfg.get("block_cycle_window_seconds", 24 * 3600)) cycle_cutoff = now - window_seconds - # Walk events chronologically (arrival order — callers pre-sort by - # id, which is the canonical chronological order; ``created_at`` - # alone is insufficient because multiple events can share the same - # second). Count "blocked after unblocked" transitions: every time - # a blocked event follows at least one unblocked event since the - # last cycle was counted, that's a new cycle. + # Walk in id (arrival) order — created_at alone can't order events that + # share a second. A blocked event after >= 1 unblocked since the last + # counted cycle is a new cycle. cycles = 0 seen_unblock_since_last_cycle = False initial_blocked_ts = 0 @@ -956,66 +804,31 @@ def _rule_block_unblock_cycling(task, events, runs, now, cfg) -> list[Diagnostic def _rule_stranded_in_ready(task, events, runs, now, cfg) -> list[Diagnostic]: - """Task has been in ``ready`` status for too long without any worker - claiming it. - - Threshold: cfg["stranded_threshold_seconds"] (default 1800 = 30 min). - - Catches every "task waiting for a worker that never comes" case - without caring WHY: - - * Operator typo'd the assignee — no profile or external worker matches. - * Profile was deleted, leaving its tasks stranded. - * External worker pool (Codex CLI, Claude Code lane, custom daemon) - is down, hung, or wasn't started. - * Dispatcher is misconfigured (wrong board, wrong HERMES_HOME). - - Pre-rule, all of these silently rotted in ``skipped_nonspawnable`` — - the dispatcher correctly skipped them (good — no respawn loop) but - nobody surfaced the fact that operator-actionable work was - accumulating. The rule fires when a ready task's promoted-to-ready - timestamp is older than the threshold AND the assignee is non-empty - (truly unassigned tasks have their own ``skipped_unassigned`` signal - on the dispatcher and a different operator response). - - The signal is age-based on purpose: it's identity-agnostic, so it - works for Hermes profiles, registered lanes, external workers, and - typos uniformly. No registry to curate, no per-board allowlist. - """ + """Assigned, unclaimed, ``ready`` for >= cfg["stranded_threshold_seconds"] + (default 30 min). Deliberately age-based and identity-agnostic so it + catches typo'd assignees, deleted profiles, and down external worker + pools alike without a registry to curate. Unassigned tasks are excluded — + the dispatcher's ``skipped_unassigned`` already covers them.""" threshold_seconds = float( cfg.get("stranded_threshold_seconds", 30 * 60) ) status = _task_field(task, "status") if status != "ready": return [] - # Skip tasks with a live claim — they're being worked on, even if - # the worker hasn't reported progress yet (run-level liveness - # extends the claim TTL; we don't want to second-guess that here). + # A live claim means it's being worked on even without progress yet. if _task_field(task, "claim_lock"): return [] assignee = _task_field(task, "assignee") or "" if not assignee.strip(): - # Unassigned tasks: the dispatcher's ``skipped_unassigned`` is - # already the right signal. A separate diagnostic here would - # double-flag the same condition. return [] - # Find the most recent event that put this task into ready. - # ``created`` covers tasks born ready; ``promoted`` covers parent- - # done auto-promotion; ``reclaimed`` covers TTL/crash recovery; - # ``unblocked`` covers human-driven resumes. - READY_TRANSITION_KINDS = { - "created", "promoted", "reclaimed", "unblocked", - } - last_ready_ts = 0 - for ev in events: - if _event_kind(ev) in READY_TRANSITION_KINDS: - t = _event_ts(ev) - last_ready_ts = max(last_ready_ts, t) + # Most recent event that put the task into ready. + last_ready_ts = _latest_event_ts( + events, {"created", "promoted", "reclaimed", "unblocked"}, + ) - # Fallback: if no qualifying event exists (very old task or events - # truncated), fall back to ``created_at`` on the task row. Better - # to occasionally over-flag an ancient task than miss a stranded one. + # No qualifying event (old task / truncated events): fall back to + # created_at — over-flagging an ancient task beats missing a stranded one. if last_ready_ts == 0: last_ready_ts = int(_task_field(task, "created_at", default=0) or 0) if last_ready_ts == 0: @@ -1031,9 +844,7 @@ def _rule_stranded_in_ready(task, events, runs, now, cfg) -> list[Diagnostic]: else: age_str = f"{int(age_seconds / 60)}m" - # Severity escalates with age. Below 2x threshold = warning; - # 2x – 6x = error; beyond 6x = critical (something is clearly - # broken, not just slow). + # Escalate with age: <2x threshold warning, 2x-6x error, >6x critical. if age_seconds >= threshold_seconds * 6: severity = "critical" elif age_seconds >= threshold_seconds * 2: @@ -1078,8 +889,7 @@ def _rule_stranded_in_ready(task, events, runs, now, cfg) -> list[Diagnostic]: )] -# Registry — order matters: rules higher on the list render first when -# severity ties. Add new rules here. +# Order matters: earlier rules render first on severity ties. _RULES: list[RuleFn] = [ _rule_hallucinated_cards, _rule_triage_aux_unavailable, @@ -1093,21 +903,6 @@ _RULES: list[RuleFn] = [ ] -# Known kinds (for the UI's filter / legend / i18n keys). Update when -# rules are added. -DIAGNOSTIC_KINDS = ( - "hallucinated_cards", - "triage_aux_unavailable", - "prose_phantom_refs", - "repeated_failures", - "repeated_crashes", - "review_dependency_deadlock", - "stuck_in_blocked", - "block_unblock_cycling", - "stranded_in_ready", -) - - DEFAULT_CONFIG = { # Match the dispatcher default (kanban.failure_limit) so repeated-failure # diagnostics do not lag behind the default auto-block threshold. @@ -1116,21 +911,16 @@ DEFAULT_CONFIG = { "spawn_failure_threshold": 2, "crash_threshold": 2, "blocked_stale_hours": 24, - # Stranded-task threshold. 30 min by default — below that, the - # signal is dominated by tasks that are about to be claimed on the - # next dispatcher tick (default 60s) and would just be noise. + # Below 30 min the signal is dominated by tasks about to be claimed on + # the next dispatcher tick. "stranded_threshold_seconds": 30 * 60, } def config_from_kanban_config(kanban_cfg: Optional[dict]) -> dict: - """Build diagnostics config from the runtime ``kanban`` config section. - - ``kanban.diagnostics.failure_threshold`` remains an explicit override. - Otherwise, derive the repeated-failure threshold from - ``kanban.failure_limit`` so CLI/dashboard diagnostics match the - dispatcher's actual circuit-breaker threshold. - """ + """Diagnostics config from the ``kanban`` section. ``kanban.diagnostics. + failure_threshold`` is an explicit override; otherwise the threshold is + ``kanban.failure_limit`` so diagnostics match the dispatcher's breaker.""" kanban_cfg = kanban_cfg or {} diag_cfg = dict(kanban_cfg.get("diagnostics") or {}) diag_cfg.setdefault( @@ -1146,13 +936,9 @@ def config_from_kanban_config(kanban_cfg: Optional[dict]) -> dict: def config_from_runtime_config(raw_config: Optional[dict]) -> dict: - """Build diagnostics config from the full Hermes runtime config. - - Carries through ``kanban``, ``auxiliary``, and ``model`` keys so triage- - aware rules can inspect the active aux-helper and main-model state. - Folds the ``kanban`` block through ``config_from_kanban_config`` so the - repeated-failure threshold derivation still applies. - """ + """Diagnostics config from the full runtime config: folds ``kanban`` through + ``config_from_kanban_config`` and carries ``kanban``/``auxiliary``/``model`` + through for the triage-aware rules.""" raw_config = raw_config or {} if not isinstance(raw_config, dict): return {} @@ -1177,12 +963,8 @@ def compute_task_diagnostics( config: Optional[dict] = None, graph: Optional[dict] = None, ) -> list[Diagnostic]: - """Run every rule against a single task's state and return a - severity-sorted list of active diagnostics. - - Sorting: critical first, then error, then warning; ties broken by - most-recent ``last_seen_at``. - """ + """Run every rule for one task; critical first, then error, warning; ties + broken by most-recent ``last_seen_at``.""" now_ts = int(now if now is not None else time.time()) config = config or {} cfg = {**DEFAULT_CONFIG, **config} @@ -1202,9 +984,7 @@ def compute_task_diagnostics( try: out.extend(rule(task, events, runs, now_ts, cfg)) except Exception: - # A broken rule must never crash the dashboard. Rule bugs - # get caught in tests; in production we'd rather drop the - # diagnostic than 500 a whole /board request. + # A broken rule must never 500 a whole /board request. continue severity_idx = {s: i for i, s in enumerate(SEVERITY_ORDER)} out.sort( diff --git a/hermes_cli/kanban_specify.py b/hermes_cli/kanban_specify.py index e7aba34a37..24605d3fb5 100644 --- a/hermes_cli/kanban_specify.py +++ b/hermes_cli/kanban_specify.py @@ -1,32 +1,13 @@ """Kanban triage specifier — flesh out a one-liner into a real spec. -Used by ``hermes kanban specify [task_id | --all]``. Takes a task that -lives in the Triage column (a rough idea, typically only a title), calls -the auxiliary LLM to produce: +``hermes kanban specify [task_id | --all]`` asks the auxiliary LLM for a +tightened title + concrete body for a Triage task, then flips it +``triage -> todo`` via ``kanban_db.specify_triage_task``. - * A tightened title (optional — only replaces if the model proposes a - materially different one) - * A concrete body: goal, proposed approach, acceptance criteria - -and then flips the task ``triage -> todo`` via -``kanban_db.specify_triage_task``. The dispatcher promotes it to -``ready`` on its next tick (or immediately if there are no open parents). - -Design notes ------------- - -* This module intentionally mirrors ``hermes_cli/goals.py`` — same aux - client pattern, same "empty config => skip, don't crash" tolerance. - Keeps the surface area tiny and the failure modes predictable. - -* The prompt is a short system + user pair. We ask for JSON with - ``{title, body}``; if parsing fails, we fall back to treating the - whole response as the body and leave the title untouched. No - retry loop — one shot, keep cost bounded. - -* Structured output / JSON mode is not requested explicitly so the - specifier works on providers that don't implement it. The parse - is lenient (tolerates markdown code fences around the JSON). +Mirrors ``hermes_cli/goals.py``: same aux-client pattern, same "empty config +=> skip, don't crash" tolerance. One shot, no retry loop. JSON mode is not +requested (works on providers without it); the parse is lenient and falls +back to "whole reply is the body" so a malformed reply never strands a task. """ from __future__ import annotations @@ -108,13 +89,12 @@ def _truncate(text: str, limit: int) -> str: _FENCE_RE = re.compile(r"^\s*```(?:json)?\s*|\s*```\s*$", re.IGNORECASE) -def _extract_json_blob(raw: str) -> Optional[dict]: - """Lenient JSON extraction — tolerates fenced code blocks and - leading/trailing whitespace. Returns None if nothing parses.""" +def _extract_json_blob(raw: str, fence_re: re.Pattern = _FENCE_RE) -> Optional[dict]: + """Lenient JSON object extraction: strip code fences, take the first ``{`` + to the last ``}``. None if nothing parses to a dict.""" if not raw: return None - stripped = _FENCE_RE.sub("", raw.strip()) - # Greedy: find the first `{` and last `}` and try that slice. + stripped = fence_re.sub("", raw.strip()) first = stripped.find("{") last = stripped.rfind("}") if first == -1 or last == -1 or last <= first: @@ -124,19 +104,24 @@ def _extract_json_blob(raw: str) -> Optional[dict]: val = json.loads(candidate) except (ValueError, json.JSONDecodeError): return None - if not isinstance(val, dict): - return None - return val + return val if isinstance(val, dict) else None -def _profile_author() -> str: +def _nonblank(v) -> Optional[str]: + return v if isinstance(v, str) and v.strip() else None + + +def _title_body(parsed: dict) -> tuple[Optional[str], Optional[str]]: + """``(title, body)`` from an LLM reply: title stripped, body verbatim, + either None when missing/blank.""" + title = _nonblank(parsed.get("title")) + return (title.strip() if title else None), _nonblank(parsed.get("body")) + + +def _profile_author(default: str = "specifier") -> str: """Mirror of ``hermes_cli.kanban._profile_author``. Kept local to avoid a circular import when kanban.py imports this module.""" - return ( - os.environ.get("HERMES_PROFILE") - or os.environ.get("USER") - or "specifier" - ) + return os.environ.get("HERMES_PROFILE") or os.environ.get("USER") or default def specify_task( @@ -145,13 +130,9 @@ def specify_task( author: Optional[str] = None, timeout: Optional[int] = None, ) -> SpecifyOutcome: - """Specify a single triage task and promote it to ``todo``. - - Returns an outcome describing what happened. Never raises for expected - failure modes (task not in triage, no aux client configured, API - error, malformed response) — those surface via ``ok=False`` so the - ``--all`` sweep can continue past individual failures. - """ + """Specify one triage task and promote it to ``todo``. Expected failures + (not in triage, no aux client, API error, malformed reply) surface as + ``ok=False`` so an ``--all`` sweep continues.""" with kb.connect_closing() as conn: task = kb.get_task(conn, task_id) if task is None: @@ -174,9 +155,8 @@ def specify_task( ) try: - # Route through call_llm so auxiliary.triage_specifier.* config - # (provider/model/base_url, extra_body, reasoning_effort, retries) - # all apply — the direct-create path dropped extra_body (#35566). + # call_llm applies all auxiliary.triage_specifier.* config + # (provider/model/base_url, extra_body, reasoning_effort, retries). resp = call_llm( task="triage_specifier", messages=[ @@ -206,9 +186,7 @@ def specify_task( new_title: Optional[str] new_body: Optional[str] if parsed is None: - # Fall back: treat the whole reply as the body, leave title as-is. - # Worst case the user edits afterward — still better than stranding - # the task in triage on a malformed LLM reply. + # Whole reply becomes the body; the user can edit afterward. stripped_raw = raw.strip() if not stripped_raw: return SpecifyOutcome( @@ -217,16 +195,7 @@ def specify_task( new_title = None new_body = stripped_raw else: - title_val = parsed.get("title") - body_val = parsed.get("body") - new_title = ( - title_val.strip() - if isinstance(title_val, str) and title_val.strip() - else None - ) - new_body = ( - body_val if isinstance(body_val, str) and body_val.strip() else None - ) + new_title, new_body = _title_body(parsed) if new_body is None and new_title is None: return SpecifyOutcome( task_id, False, "LLM response missing title and body" @@ -241,8 +210,7 @@ def specify_task( author=author or _profile_author(), ) if not ok: - # Race: someone else promoted / archived the task between our - # read above and the write. Report, don't crash. + # Race: promoted/archived between our read and the write. return SpecifyOutcome( task_id, False, "task moved out of triage before promotion" ) @@ -250,10 +218,7 @@ def specify_task( def list_triage_ids(*, tenant: Optional[str] = None) -> list[str]: - """Return task ids currently in the triage column. - - ``tenant`` narrows the sweep; ``None`` returns every triage task. - """ + """Task ids in the triage column; ``tenant`` narrows the sweep.""" with kb.connect_closing() as conn: tasks = kb.list_tasks( conn, diff --git a/hermes_cli/kanban_swarm.py b/hermes_cli/kanban_swarm.py index ef1477f675..5a047eb67d 100644 --- a/hermes_cli/kanban_swarm.py +++ b/hermes_cli/kanban_swarm.py @@ -19,6 +19,7 @@ from __future__ import annotations from dataclasses import dataclass, field import json import sqlite3 +import time from typing import Any, Iterable, Optional from hermes_cli import kanban_db as kb @@ -83,16 +84,12 @@ def _activate_root_inline( ) -> bool: """Inline blocked→done CAS flip + event insert for the swarm root. - Runs INSIDE create_swarm's outer write_txn, so it must not call - ``kb.complete_task`` — that helper opens its own transaction and fires - post-commit side effects (workspace cleanup, failure-counter clear, - ``recompute_ready``) that would execute while the outer transaction can - still roll back. Instead we do the minimal durable writes here and let - the caller run ``recompute_ready`` after the outer commit. + Runs INSIDE create_swarm's write_txn, so it must not call + ``kb.complete_task`` (own transaction + post-commit side effects that + would run while the outer txn can still roll back). The caller runs + ``recompute_ready`` after the outer commit. """ - import time as _time - - now = int(_time.time()) + now = int(time.time()) cur = conn.execute( """ UPDATE tasks @@ -179,9 +176,8 @@ def create_swarm( raise RuntimeError("could not activate the completed swarm topology") activated = True if activated: - # Outside the outer transaction: promote the root's children now - # that its 'done' flip is durable (recompute_ready opens its own - # txn and must never run under an open write_txn). + # After commit: recompute_ready opens its own txn and must never run + # under an open write_txn. kb.recompute_ready(conn) root = kb.get_task(conn, created.root_id) run = kb.latest_run(conn, created.root_id) @@ -249,9 +245,8 @@ def _create_swarm_uncommitted( workspace_path=workspace_path, ) - # If idempotency returned an existing non-archived root, do not duplicate the - # swarm graph. Recover the topology from the root's latest blackboard, if it - # was created by this helper previously. + # Idempotency may return an existing root: recover its topology from the + # blackboard instead of duplicating the graph. existing = latest_blackboard(conn, root).get("topology") if isinstance(existing, dict): worker_ids = [str(x) for x in existing.get("worker_ids", []) if x] @@ -266,61 +261,54 @@ def _create_swarm_uncommitted( ) context_suffix = _swarm_context(root, goal) - worker_ids: list[str] = [] - for spec in worker_specs: - worker_id = kb.create_task( + common = dict( + created_by=created_by, tenant=tenant, + workspace_kind=workspace_kind, workspace_path=workspace_path, + ) + worker_ids = [ + kb.create_task( conn, title=spec.title, body=(spec.body or "") + context_suffix, assignee=spec.profile, - created_by=created_by, parents=[root], - tenant=tenant, priority=spec.priority or priority, - workspace_kind=workspace_kind, - workspace_path=workspace_path, skills=spec.skills or None, max_runtime_seconds=spec.max_runtime_seconds, + **common, ) - worker_ids.append(worker_id) + for spec in worker_specs + ] - verifier_body = ( - "Review every worker handoff and blackboard update. Gate the swarm: " - "complete only with metadata {\"gate\": \"pass\"} when evidence is " - "sufficient; otherwise block with exact missing work." - + context_suffix - ) verifier = kb.create_task( conn, title=verifier_title, - body=verifier_body, + body=( + "Review every worker handoff and blackboard update. Gate the swarm: " + "complete only with metadata {\"gate\": \"pass\"} when evidence is " + "sufficient; otherwise block with exact missing work." + + context_suffix + ), assignee=verifier_assignee, - created_by=created_by, parents=worker_ids, - tenant=tenant, priority=priority, - workspace_kind=workspace_kind, - workspace_path=workspace_path, skills=["requesting-code-review"], + **common, ) - synthesizer_body = ( - "Synthesize the verified worker outputs into the final deliverable. " - "Do not start until the verifier has passed the gate." - + context_suffix - ) synthesizer = kb.create_task( conn, title=synthesizer_title, - body=synthesizer_body, + body=( + "Synthesize the verified worker outputs into the final deliverable. " + "Do not start until the verifier has passed the gate." + + context_suffix + ), assignee=synthesizer_assignee, - created_by=created_by, parents=[verifier], - tenant=tenant, priority=priority, - workspace_kind=workspace_kind, - workspace_path=workspace_path, skills=["humanizer"], + **common, ) created = SwarmCreated(root, worker_ids, verifier, synthesizer) diff --git a/hermes_cli/kanban_transfer.py b/hermes_cli/kanban_transfer.py index a5bf3aaab4..41b1a0daeb 100644 --- a/hermes_cli/kanban_transfer.py +++ b/hermes_cli/kanban_transfer.py @@ -14,30 +14,21 @@ source board's slug):: attachments//… attachment blobs (unless --no-attachments) logs/.log worker logs (only with --include-logs) -Two things make this more than a ``tar czf`` of the board directory. +Two things make this more than ``tar czf`` of the board directory: -**The database is live.** Kanban runs in WAL mode and a dispatcher may be -mid-write, so copying ``kanban.db`` off the filesystem yields a torn -snapshot that is missing whatever still sits in the ``-wal`` file. Export -goes through SQLite's online-backup API instead, which produces a -consistent single-file image of a database that is being written to. +* **The database is live** (WAL mode, dispatcher may be mid-write), so the + export uses SQLite's online-backup API for a consistent image instead of a + file copy that would miss the ``-wal`` sidecar. +* **Rows carry machine-local state** — claims, PIDs, heartbeats, absolute + paths, gateway chat subscriptions, session ids. Shipping them verbatim + would import claims owned by a stranger's process or push events into a + stranger's Telegram thread. Scrubbed on export and re-scrubbed on import + (an archive is untrusted); see :func:`_scrub_local_state` and + :func:`_relocate_imported_rows`. -**Rows carry machine-local state.** Claims, PIDs, heartbeats, absolute -workspace and attachment paths, gateway chat subscriptions, and session -ids are all meaningful only on the machine that wrote them. Shipping them -verbatim is how an imported board arrives holding claims owned by a -process on somebody else's laptop, or starts pushing task events into a -stranger's Telegram thread. Everything machine-local is scrubbed on the -export side (so the archive itself never carries it) and defensively -re-scrubbed on import; see :func:`_scrub_local_state` and -:func:`_relocate_imported_rows`. - -Imports always land as a **new** board — the slug auto-suffixes on -collision — so an import can never mutate a board that is already there. -That also means an imported board is never ``default``, which is what -lets the import side ignore the default board's split on-disk layout -(``/kanban.db`` beside ``/kanban/attachments/``) and put -everything inside one ``boards//`` directory. +Imports always land as a **new** board (slug auto-suffixes on collision), so +an import never mutates an existing board and is never ``default`` — which +lets the importer ignore the default board's split on-disk layout. """ from __future__ import annotations @@ -74,13 +65,8 @@ _DISPATCHABLE_STATUSES = ("ready", "running", "todo", "scheduled") # --------------------------------------------------------------------------- def _snapshot_db(source: Path, target: Path) -> None: - """Write a consistent copy of ``source`` to ``target``. - - Uses SQLite's online-backup API rather than a file copy: in WAL mode - a just-committed page can still live in the ``-wal`` sidecar, so - copying only ``kanban.db`` loses recent writes and can produce a - torn image if the dispatcher commits mid-copy. - """ + """Consistent copy of ``source`` via the online-backup API (a file copy + would miss pages still in the ``-wal`` sidecar and could tear).""" src = sqlite3.connect(str(source)) try: dst = sqlite3.connect(str(target)) @@ -93,14 +79,9 @@ def _snapshot_db(source: Path, target: Path) -> None: def _scrub_local_state(conn: sqlite3.Connection) -> None: - """Strip machine-local runtime state. Caller owns the transaction. - - Runs on the export side so the archive itself never carries another - machine's claims, PIDs, or — the one that actually matters for a - board shared with someone else — the gateway chat ids subscribed to - its task events. Repeated on import because an archive is untrusted - input. - """ + """Strip machine-local runtime state (claims, PIDs, and above all the + gateway chat ids subscribed to task events). Caller owns the transaction. + Run on export and again on import (an archive is untrusted input).""" conn.execute("DELETE FROM kanban_notify_subs") conn.execute( """ @@ -157,12 +138,9 @@ def export_board( include_attachments: bool = True, include_logs: bool = False, ) -> dict[str, Any]: - """Export ``board`` to a ``tar.gz`` archive. Returns a summary dict. - - ``output_path`` may be given with or without the ``.tar.gz`` suffix. - Workspaces are never included: they are git worktrees and scratch - trees that are large, machine-local, and rebuilt on demand. - """ + """Export ``board`` to a ``tar.gz`` (suffix optional on ``output_path``); + returns a summary dict. Workspaces are never included — large, + machine-local, rebuilt on demand.""" slug = kb._normalize_board_slug(board) or kb.get_current_board() if not kb.board_exists(slug): raise ValueError(f"board {slug!r} does not exist") @@ -241,12 +219,8 @@ def export_board( # --------------------------------------------------------------------------- def _available_slug(preferred: str) -> str: - """Return ``preferred``, or the first free ``-N`` variant. - - ``default`` always reports as existing, so an archive exported from a - default board naturally lands as ``default-2`` instead of colliding - with the importer's own default board. - """ + """``preferred`` or the first free ``-N``. ``default`` always + exists, so a default-board export lands as ``default-2``.""" if not kb.board_exists(preferred): return preferred # Leave headroom for the suffix inside the 64-char slug limit. @@ -295,22 +269,17 @@ def _read_board_metadata(path: Path) -> dict[str, Any]: def _relocate_imported_rows( conn: sqlite3.Connection, slug: str ) -> tuple[dict[str, int], list[str]]: - """Re-anchor an imported board's rows to this machine. + """Re-anchor an imported board's rows to this machine; returns + ``(stats, warnings)``. - Returns ``(stats, warnings)``. Three things move: - - * Attachment rows are repointed at this board's attachments tree. - Rows whose blob did not travel (an export made with - ``--no-attachments``) are dropped, because a row pointing at a file - that does not exist breaks download in every UI that lists it. - * Workspace paths are cleared. ``scratch`` tasks regenerate one under - this board on the next claim, so they are simply reset. ``dir`` and - ``worktree`` tasks cannot be resolved without a path that means - something here, so any that are still dispatchable are parked in - ``triage`` — otherwise the dispatcher claims them, fails to build a - workspace, and burns them straight into the failure breaker. - * Runtime state is scrubbed again. Export already did this, but an - archive is an untrusted input and the cost is one UPDATE. + * Attachment rows are repointed at this board's tree; rows whose blob + did not travel (``--no-attachments``) are dropped, since a dangling row + breaks download in every UI. + * Workspace paths are cleared. ``scratch`` regenerates on next claim; + dispatchable ``dir``/``worktree`` tasks are parked in ``triage``, + otherwise the dispatcher claims them, fails to build a workspace, and + burns them into the failure breaker. + * Runtime state is scrubbed again (untrusted input, one UPDATE). """ warnings: list[str] = [] now = int(time.time()) @@ -389,12 +358,8 @@ def import_board( *, activate: bool = False, ) -> dict[str, Any]: - """Import a board archive as a new board. Returns a summary dict. - - ``slug`` overrides the name from the archive. Either way the final - slug auto-suffixes if it is taken, so an import never merges into or - overwrites an existing board. - """ + """Import an archive as a NEW board (``slug`` overrides the archive's; + either way it auto-suffixes if taken). Returns a summary dict.""" archive = Path(archive_path).expanduser() if not archive.exists(): raise FileNotFoundError(f"archive not found: {archive}")