From e87c495dc2d827dbc9bc1df86f4e75673e11ed24 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Thu, 9 Jul 2026 09:02:04 -0500 Subject: [PATCH] =?UTF-8?q?fix(kanban):=20spawn=20workers=20headless=20?= =?UTF-8?q?=E2=80=94=20TUI=20can=20never=20eat=20a=20worker=20run?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An inherited HERMES_TUI=1 or a `display.interface: tui` config default sent kanban workers into the Ink TUI, whose no-TTY bail-out exits 0 without doing the task — every attempt ended in "protocol violation". Two layers: - _default_spawn pins `--cli` (highest-precedence interface flag) and strips HERMES_TUI from the child env (covers older builds on PATH). - _resolve_use_tui gates ambient TUI prefs (env/config) behind a real TTY; an explicit --tui still wins so the informative bail-out stays reachable. --- hermes_cli/kanban_db.py | 9 ++++ hermes_cli/main.py | 26 ++++++++++-- .../test_default_interface_resolution.py | 42 ++++++++++++++++++- .../test_kanban_worker_spawn_toolsets.py | 36 ++++++++++++++++ 4 files changed, 108 insertions(+), 5 deletions(-) diff --git a/hermes_cli/kanban_db.py b/hermes_cli/kanban_db.py index 6150b141537..4a958d6ae7e 100644 --- a/hermes_cli/kanban_db.py +++ b/hermes_cli/kanban_db.py @@ -7768,9 +7768,18 @@ def _default_spawn( # attributed correctly regardless of how the child loads config. env["HERMES_PROFILE"] = profile_arg + # A worker must NEVER boot the interactive TUI: an inherited HERMES_TUI=1 + # or a `display.interface: tui` in the profile's config would send the + # quiet chat run into the Ink TUI, whose no-TTY bail-out exits 0 without + # doing the task → "protocol violation" on every attempt. `--cli` is the + # highest-precedence interface override; dropping the env var covers + # older hermes builds on PATH that predate the flag's precedence. + env.pop("HERMES_TUI", None) + cmd = [ *_resolve_hermes_argv(), "-p", profile_arg, + "--cli", # Worker subprocesses switch to a profile-scoped HERMES_HOME above, # so they see that profile's shell-hook allowlist instead of the # dispatcher's root allowlist. Pass --accept-hooks explicitly so diff --git a/hermes_cli/main.py b/hermes_cli/main.py index f450a76666f..2c5b0361662 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -2193,16 +2193,34 @@ def _resolve_use_tui(args) -> bool: Precedence (highest first): 1. ``--cli`` flag → always classic REPL - 2. ``--tui`` flag / ``HERMES_TUI=1`` → always TUI - 3. ``display.interface`` config value ("cli" | "tui") - 4. default → classic REPL + 2. ``--tui`` flag → always TUI (explicit ask) + 3. no TTY → always classic (ambient prefs don't apply) + 4. ``HERMES_TUI=1`` env → TUI + 5. ``display.interface`` config value ("cli" | "tui") + 6. default → classic REPL Explicit flags always win over config so muscle memory and scripts keep working regardless of the configured default. + + The TTY gate (3) is load-bearing: ambient TUI preferences (env var or + config default) must never hijack a NON-interactive invocation. Kanban + workers, cron jobs, and pipelines run ``hermes … chat -q`` with stdout + on a pipe; booting the Ink TUI there hits its no-TTY bail-out, which + prints a resume hint and exits 0 — a kanban worker then dies with + "exited cleanly without calling kanban_complete — protocol violation" + on every attempt (found dogfooding the desktop kanban board). A user + who *explicitly* passes ``--tui`` still gets the informative bail-out. """ if getattr(args, "cli", False): return False - if getattr(args, "tui", False) or os.environ.get("HERMES_TUI") == "1": + if getattr(args, "tui", False): + return True + try: + if not (sys.stdin.isatty() and sys.stdout.isatty()): + return False + except Exception: + return False + if os.environ.get("HERMES_TUI") == "1": return True try: from hermes_cli.config import load_config diff --git a/tests/hermes_cli/test_default_interface_resolution.py b/tests/hermes_cli/test_default_interface_resolution.py index c04f8093b29..ce7c5d8b55c 100644 --- a/tests/hermes_cli/test_default_interface_resolution.py +++ b/tests/hermes_cli/test_default_interface_resolution.py @@ -5,10 +5,18 @@ flip ``display.interface: tui`` in config.yaml to make the modern Ink TUI the default for bare ``hermes`` / ``hermes chat``. Explicit flags always win: --cli forces the classic REPL (highest precedence) - --tui / HERMES_TUI=1 forces the TUI + --tui forces the TUI + (no TTY) forces the classic REPL — ambient prefs don't apply + HERMES_TUI=1 the env default display.interface the configured default (unset) classic REPL +The no-TTY gate exists because ambient TUI preferences must never hijack +non-interactive invocations: kanban workers / cron / pipelines run +``hermes … chat -q`` on a pipe, and the TUI's no-TTY bail-out exits 0 +without doing the work (a kanban worker then dies with "protocol +violation" on every attempt). + These tests pin that precedence at every layer that makes the decision: * ``_resolve_use_tui(args)`` — the canonical args-aware resolver used by @@ -46,6 +54,14 @@ def _args(**kw): return SimpleNamespace(**kw) +def _fake_tty(monkeypatch, interactive: bool): + """Pin stdin/stdout TTY-ness — pytest's capture is never a real TTY.""" + import sys as _sys + + monkeypatch.setattr(_sys.stdin, "isatty", lambda: interactive, raising=False) + monkeypatch.setattr(_sys.stdout, "isatty", lambda: interactive, raising=False) + + def _patch_config(monkeypatch, interface): import hermes_cli.config as cfg @@ -73,19 +89,23 @@ class TestResolveUseTui: def test_env_beats_config_cli(self, monkeypatch): _patch_config(monkeypatch, "cli") + _fake_tty(monkeypatch, True) monkeypatch.setenv("HERMES_TUI", "1") assert m._resolve_use_tui(_args()) is True def test_config_tui_with_no_flags(self, monkeypatch): _patch_config(monkeypatch, "tui") + _fake_tty(monkeypatch, True) assert m._resolve_use_tui(_args()) is True def test_config_cli_is_default(self, monkeypatch): _patch_config(monkeypatch, "cli") + _fake_tty(monkeypatch, True) assert m._resolve_use_tui(_args()) is False def test_interface_value_is_case_insensitive(self, monkeypatch): _patch_config(monkeypatch, "TUI") + _fake_tty(monkeypatch, True) assert m._resolve_use_tui(_args()) is True def test_load_config_failure_falls_back_to_cli(self, monkeypatch): @@ -95,8 +115,28 @@ class TestResolveUseTui: raise RuntimeError("config unreadable") monkeypatch.setattr(cfg, "load_config", boom) + _fake_tty(monkeypatch, True) assert m._resolve_use_tui(_args()) is False + # ── the no-TTY gate: ambient prefs never hijack non-interactive runs ──── + def test_no_tty_blocks_env_tui(self, monkeypatch): + _patch_config(monkeypatch, "cli") + _fake_tty(monkeypatch, False) + monkeypatch.setenv("HERMES_TUI", "1") + assert m._resolve_use_tui(_args()) is False + + def test_no_tty_blocks_config_tui(self, monkeypatch): + _patch_config(monkeypatch, "tui") + _fake_tty(monkeypatch, False) + assert m._resolve_use_tui(_args()) is False + + def test_explicit_tui_flag_survives_no_tty(self, monkeypatch): + # An explicit --tui is the user's own ask — keep the informative + # no-TTY bail-out instead of silently swapping interfaces. + _patch_config(monkeypatch, "cli") + _fake_tty(monkeypatch, False) + assert m._resolve_use_tui(_args(tui=True)) is True + # --------------------------------------------------------------------------- # _wants_tui_early — dependency-free early resolver diff --git a/tests/hermes_cli/test_kanban_worker_spawn_toolsets.py b/tests/hermes_cli/test_kanban_worker_spawn_toolsets.py index 7469a7bf057..da9b6b957b2 100644 --- a/tests/hermes_cli/test_kanban_worker_spawn_toolsets.py +++ b/tests/hermes_cli/test_kanban_worker_spawn_toolsets.py @@ -89,6 +89,42 @@ agent: assert required in pinned +def test_default_spawn_never_boots_the_tui(monkeypatch, tmp_path): + """Workers are headless: an inherited HERMES_TUI=1 (or a TUI-default + config) must not send the quiet chat run into the Ink TUI, whose no-TTY + bail-out exits 0 without doing the task — every attempt then ends in + "protocol violation". The spawn pins --cli (highest-precedence interface + flag) and strips HERMES_TUI from the child env.""" + root = tmp_path / ".hermes" + (root / "profiles" / "elias").mkdir(parents=True) + root.joinpath("config.yaml").write_text("display:\n interface: tui\n", encoding="utf-8") + monkeypatch.setenv("HERMES_HOME", str(root)) + monkeypatch.setenv("HERMES_TUI", "1") + + from hermes_cli import kanban_db as kb + + monkeypatch.setattr(kb, "_resolve_hermes_argv", lambda: ["hermes"]) + + captured = {} + + class FakeProc: + pid = 4243 + + def fake_popen(cmd, *args, **kwargs): + captured["cmd"] = list(cmd) + captured["env"] = dict(kwargs.get("env") or {}) + return FakeProc() + + monkeypatch.setattr(subprocess, "Popen", fake_popen) + + workspace = tmp_path / "workspace" + workspace.mkdir() + kb._default_spawn(_make_task(kb, assignee="elias"), str(workspace)) + + assert "--cli" in captured["cmd"] + assert "HERMES_TUI" not in captured["env"] + + def test_resolve_worker_cli_toolsets_uses_profile_home_not_parent_config(monkeypatch, tmp_path): root = tmp_path / ".hermes" profile = root / "profiles" / "elias"