From 7ea400249590df006e410d0bd718c084bf4dfd9d Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 22:40:25 -0700 Subject: [PATCH 1/8] =?UTF-8?q?refactor(hermes=5Fcli):=20cron.py=20?= =?UTF-8?q?=E2=80=94=20lift=20manual-run=20verdict=20into=20=5Frun=5Foutco?= =?UTF-8?q?me,=20flatten=20cron=5Fresume=20mode=20check?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- hermes_cli/cron.py | 48 +++++++++++++++++++++++----------------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/hermes_cli/cron.py b/hermes_cli/cron.py index 03d37ed8ca..293b58c879 100644 --- a/hermes_cli/cron.py +++ b/hermes_cli/cron.py @@ -714,46 +714,46 @@ def _job_action(action: str, job_id: str, success_verb: str) -> int: if action in {"resume", "run"} and result.get("job", {}).get("next_run_at"): print(f" Next run: {result['job']['next_run_at']}") if action == "run": - job = result.get("job", {}) - # A manual run may be dispatched to the gateway daemon's background delegation worker - # (execution_mode="background" and/or a delegation_id) and keeps running AFTER this - # CLI exits — a terminal success/failure verdict would be a lie, so report the dispatch. - delegation_id = job.get("delegation_id") - if delegation_id: - print(f" Running in background (delegation {delegation_id}).") - elif job.get("execution_mode") == "background": - print(" Running in background.") - elif job.get("executed"): - print(f" Ran now: {'succeeded' if job.get('execution_success') else 'failed'}.") - elif job.get("execution_skipped"): - print(f" {job['execution_skipped']}") - else: - print(" It will run on the next scheduler tick.") + print(f" {_run_outcome(result.get('job', {}))}") return 0 +def _run_outcome(job: Dict[str, Any]) -> str: + """One-line verdict for a manual run. + + A run may be dispatched to the gateway daemon's background delegation worker + (execution_mode="background" and/or a delegation_id) and keeps running AFTER this CLI + exits — a terminal success/failure verdict would be a lie, so report the dispatch. + """ + if job.get("delegation_id"): + return f"Running in background (delegation {job['delegation_id']})." + if job.get("execution_mode") == "background": + return "Running in background." + if job.get("executed"): + return f"Ran now: {'succeeded' if job.get('execution_success') else 'failed'}." + return job.get("execution_skipped") or "It will run on the next scheduler tick." + + def cron_resume(args) -> int: """Resume a paused job or explicitly re-arm a completed one-shot.""" run_at = getattr(args, "run_at", None) run_now = getattr(args, "run_now", False) - if bool(run_at) == bool(run_now): - if run_at or run_now: - print(color("Use exactly one of --at or --run-now.", Colors.RED)) - return 1 + if run_at and run_now: + print(color("Use exactly one of --at or --run-now.", Colors.RED)) + return 1 + if not run_at and not run_now: return _job_action("resume", args.job_id, "Resumed") from cron.jobs import AmbiguousJobReference, _hermes_now, rearm_oneshot - if run_now: - run_at = _hermes_now().isoformat() try: - job = rearm_oneshot(args.job_id, run_at) + job = rearm_oneshot(args.job_id, _hermes_now().isoformat() if run_now else run_at) except (AmbiguousJobReference, ValueError) as exc: print(color(f"Failed to re-arm job: {exc}", Colors.RED)) return 1 if not job: print(color(f"Job not found: {args.job_id}", Colors.RED)) return 1 - print(color(f"Re-armed job: {job.get('name', args.job_id)} ({args.job_id})", Colors.GREEN)) - print(f" Next run: {job.get('next_run_at')}") + print(color(f"Re-armed job: {job.get('name', args.job_id)} ({args.job_id})", Colors.GREEN) + + f"\n Next run: {job.get('next_run_at')}") return 0 From 7ff29f4837f14b7811df978a1bcc955e94bca364 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 22:42:26 -0700 Subject: [PATCH 2/8] =?UTF-8?q?refactor(hermes=5Fcli):=20cron.py=20?= =?UTF-8?q?=E2=80=94=20split=20cron=5Flist=20into=20=5Fjob=5Frows/=5Fjob?= =?UTF-8?q?=5Fwarnings/=5Flast=5Frun=5Fdisplay?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- hermes_cli/cron.py | 155 +++++++++++++++++++++------------------------ 1 file changed, 72 insertions(+), 83 deletions(-) diff --git a/hermes_cli/cron.py b/hermes_cli/cron.py index 293b58c879..60d7ddf3df 100644 --- a/hermes_cli/cron.py +++ b/hermes_cli/cron.py @@ -168,98 +168,87 @@ def cron_list(show_all: bool = False): _print_banner("Scheduled Jobs") for job in jobs: - job_id = job.get("id", "?") # Scheduler-honoured flag — never show [paused] when enabled=true. - state = effective_job_state(job) - badge = _STATE_BADGES.get(state) or ( + badge = _STATE_BADGES.get(effective_job_state(job)) or ( ("[active]", Colors.GREEN) if job.get("enabled", True) else ("[disabled]", Colors.RED) ) - - # `repeat` / `deliver` may be present-but-null in the record (a one-shot persisted - # with "repeat": null), so coalesce rather than rely on the dict-default, which only - # applies to a missing key — a null deliver would crash `", ".join(None)`. - repeat_info = job.get("repeat") or {} - repeat_times = repeat_info.get("times") - repeat_str = f"{repeat_info.get('completed', 0)}/{repeat_times}" if repeat_times else "∞" - deliver = job.get("deliver") or ["local"] - if isinstance(deliver, str): - deliver = [deliver] - - rows = [ - ("Name", job.get("name", "(unnamed)")), - ("Schedule", job.get("schedule_display", job.get("schedule", {}).get("value", "?"))), - ("Repeat", repeat_str), - ("Next run", job.get("next_run_at", "?")), - ("Deliver", ", ".join(deliver)), - ] - skills = job.get("skills") or ([job["skill"]] if job.get("skill") else []) - if skills: - rows.append(("Skills", ", ".join(skills))) - if job.get("script"): - rows.append(("Script", job["script"])) - monitor_source = job.get("monitor_script") or job.get("monitor_url") - if monitor_source: - rows.append(("Monitor", f"{monitor_source} (agent runs only on output change)")) - mon_state = job.get("monitor_state") or {} - if mon_state.get("last_changed_at"): - rows.append(("Changed", mon_state["last_changed_at"])) - if job.get("no_agent"): - mode = color("no-agent", Colors.DIM) + " (script stdout delivered directly)" - rows.append(("Mode", mode)) - if job.get("workdir"): - rows.append(("Workdir", job["workdir"])) - - last_status = job.get("last_status") - if last_status: - if last_status == "ok": - status_display = color("ok", Colors.GREEN) - elif last_status == "delivery_failed": - # Agent succeeded but the result never reached the user — not green; the - # detail lives in last_delivery_error (last_error is None for these runs). - detail = job.get("last_delivery_error") or "?" - status_display = color(f"delivery_failed: {detail}", Colors.YELLOW) - else: - status_display = color(f"{last_status}: {job.get('last_error', '?')}", Colors.RED) - streak = int(job.get("failure_streak") or 0) - if streak >= 2: - status_display += color(f" ({streak} failures in a row)", Colors.RED) - rows.append(("Last run", f"{job.get('last_run_at', '?')} {status_display}")) - - dispatch_line = _dispatch_display(job.get("last_dispatch")) - if dispatch_line: - rows.append(("Dispatch", dispatch_line)) - latest_execution = job.get("latest_execution") - if latest_execution: - rows.append(( - "Execution", - f"{latest_execution.get('status', '?')} {latest_execution.get('id', '?')}", - )) - - print(f" {color(job_id, Colors.YELLOW)} {color(*badge)}") - for label, value in rows: + print(f" {color(job.get('id', '?'), Colors.YELLOW)} {color(*badge)}") + for label, value in _job_rows(job): print(f" {label + ':':<11}{value}") - - delivery_err = job.get("last_delivery_error") - if delivery_err: - print(f" {color('⚠ Delivery failed:', Colors.YELLOW)} {delivery_err}") - - # A live adapter acked the last send but returned no message_id / raw_response - # (Slack/Matrix/Mattermost shape): accepted as delivered, but say so here. - unverified = job.get("last_delivery_unverified") - if unverified: - print(f" {color('⚠ Delivery UNVERIFIED:', Colors.YELLOW)} adapter acked " - f"{_unverified_targets(unverified)} without message_id/raw_response") - - fire_err = job.get("last_fire_error") - if isinstance(fire_err, dict) and fire_err.get("detail"): - print(f" {color('⚠ Missed scheduled fire:', Colors.RED)} " - f"{fire_err.get('at', '?')} {fire_err['detail']}") - + for line in _job_warnings(job): + print(f" {line}") print() _warn_if_gateway_not_running() +def _last_run_display(job: Dict[str, Any]) -> str: + last_status = job["last_status"] + if last_status == "ok": + return color("ok", Colors.GREEN) + if last_status == "delivery_failed": + # Agent succeeded but the result never reached the user — not green; the detail + # lives in last_delivery_error (last_error is None for these runs). + return color(f"delivery_failed: {job.get('last_delivery_error') or '?'}", Colors.YELLOW) + display = color(f"{last_status}: {job.get('last_error', '?')}", Colors.RED) + streak = int(job.get("failure_streak") or 0) + if streak >= 2: + display += color(f" ({streak} failures in a row)", Colors.RED) + return display + + +def _job_rows(job: Dict[str, Any]) -> List[tuple[str, str]]: + """``(label, value)`` detail rows for one job in ``cron list``.""" + # `repeat` / `deliver` may be present-but-null in the record (a one-shot persisted with + # "repeat": null), so coalesce rather than rely on the dict-default, which only applies + # to a missing key — a null deliver would crash `", ".join(None)`. + repeat_info = job.get("repeat") or {} + repeat_times = repeat_info.get("times") + deliver = job.get("deliver") or ["local"] + skills = job.get("skills") or ([job["skill"]] if job.get("skill") else []) + monitor_source = job.get("monitor_script") or job.get("monitor_url") + mon_state = job.get("monitor_state") or {} + latest_execution = job.get("latest_execution") or {} + optional = [ + ("Skills", ", ".join(skills) if skills else ""), + ("Script", job.get("script")), + ("Monitor", f"{monitor_source} (agent runs only on output change)" if monitor_source else ""), + ("Changed", mon_state.get("last_changed_at") if monitor_source else ""), + ("Mode", color("no-agent", Colors.DIM) + " (script stdout delivered directly)" + if job.get("no_agent") else ""), + ("Workdir", job.get("workdir")), + ("Last run", f"{job.get('last_run_at', '?')} {_last_run_display(job)}" + if job.get("last_status") else ""), + ("Dispatch", _dispatch_display(job.get("last_dispatch"))), + ("Execution", f"{latest_execution.get('status', '?')} {latest_execution.get('id', '?')}" + if latest_execution else ""), + ] + return [ + ("Name", job.get("name", "(unnamed)")), + ("Schedule", job.get("schedule_display", job.get("schedule", {}).get("value", "?"))), + ("Repeat", f"{repeat_info.get('completed', 0)}/{repeat_times}" if repeat_times else "∞"), + ("Next run", job.get("next_run_at", "?")), + ("Deliver", deliver if isinstance(deliver, str) else ", ".join(deliver)), + ] + [(label, value) for label, value in optional if value] + + +def _job_warnings(job: Dict[str, Any]) -> List[str]: + """Delivery / fire warning lines for one job in ``cron list``.""" + lines = [] + if job.get("last_delivery_error"): + lines.append(f"{color('⚠ Delivery failed:', Colors.YELLOW)} {job['last_delivery_error']}") + # A live adapter acked the last send but returned no message_id / raw_response + # (Slack/Matrix/Mattermost shape): accepted as delivered, but say so here. + if unverified := job.get("last_delivery_unverified"): + lines.append(f"{color('⚠ Delivery UNVERIFIED:', Colors.YELLOW)} adapter acked " + f"{_unverified_targets(unverified)} without message_id/raw_response") + fire_err = job.get("last_fire_error") + if isinstance(fire_err, dict) and fire_err.get("detail"): + lines.append(f"{color('⚠ Missed scheduled fire:', Colors.RED)} " + f"{fire_err.get('at', '?')} {fire_err['detail']}") + return lines + + def cron_tick(): """Run due jobs once and exit.""" from cron.scheduler import CronTickYielded, tick From 97293216ee761fc3bda886c79fcac14b647ac346 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 22:46:32 -0700 Subject: [PATCH 3/8] =?UTF-8?q?refactor(hermes=5Fcli):=20container=5Fboot.?= =?UTF-8?q?py=20=E2=80=94=20second=20pass=20compacting=20rationale=20comme?= =?UTF-8?q?nts=20(every=20WHY=20kept)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- hermes_cli/container_boot.py | 146 ++++++++++++++--------------------- 1 file changed, 60 insertions(+), 86 deletions(-) diff --git a/hermes_cli/container_boot.py b/hermes_cli/container_boot.py index 038ff5bf88..d6f944799b 100644 --- a/hermes_cli/container_boot.py +++ b/hermes_cli/container_boot.py @@ -15,32 +15,25 @@ from typing import Literal, Sequence log = logging.getLogger(__name__) -# Only this desired state triggers automatic restart. Everything else (startup_failed, -# starting, stopped, missing) registers the slot down and waits for explicit user action — -# avoiding the crash-loop where a broken gateway keeps restarting across `docker restart`. -# Older installs only have gateway_state; newer lifecycle commands persist desired_state -# separately so a transient runtime state does not erase the operator's durable intent. +# Only this desired state auto-restarts; everything else (startup_failed, starting, stopped, +# missing) registers the slot down and waits for the user — avoiding the crash-loop where a +# broken gateway keeps restarting across `docker restart`. Older installs only have +# gateway_state; newer lifecycle commands persist desired_state separately so a transient +# runtime state does not erase the operator's durable intent. _AUTOSTART_STATES = frozenset({"running"}) -# Transient runtime sub-states of a RUNNING gateway — reached only while up and serving, so -# NOT an operator stop and NOT a failed boot: -# - `draining` — drain watcher / scale-to-zero go-dormant path, in-flight quiesce begun. -# - `degraded` — came up with some platforms queued for retry, then fell through to the -# normal running state; the reconnect watcher takes it from there. -# When a gateway is hard-killed *in one of these states* (container/VM recreate SIGTERMs it -# before `_stop_impl` persists a terminal state), the marker left in gateway_state.json is -# the transient sub-state. With no `desired_state` to fall back to, treating it literally -# would leave the gateway DOWN on every subsequent boot (observed: a relay-opted-in staging -# instance stranded at `draining`; `degraded` is the same wedge class). Map them to `running` -# — mirrors gateway/run.py persisting `running` (not mid-shutdown `draining`) on unexpected -# signal, extended to the case where the gateway died before persisting anything. -# `starting` / `startup_failed` are deliberately NOT included: those mean the gateway died -# mid-boot or failed to come up, and auto-restarting would reintroduce the crash-loop. +# Transient sub-states of a RUNNING gateway (`draining`: in-flight quiesce; `degraded`: up +# with some platforms queued for reconnect) — NOT an operator stop, NOT a failed boot. When a +# gateway is hard-killed in one of them (container recreate SIGTERMs it before `_stop_impl` +# persists a terminal state) and there is no `desired_state`, reading the marker literally +# would leave the gateway DOWN on every later boot (observed: staging instance stranded at +# `draining`). Map them to `running`, mirroring gateway/run.py's unexpected-signal handling. +# `starting` / `startup_failed` are deliberately excluded: auto-restarting a gateway that died +# mid-boot reintroduces the crash-loop. _TRANSIENT_RUNNING_STATES = frozenset({"draining", "degraded"}) -# Stale runtime files swept before recreating service slots: they hold container-namespaced -# state (PIDs, process tables) that's garbage post-restart — a numerically-equal PID in the -# new container is a different process. +# Swept before recreating slots: container-namespaced state (PIDs, process tables) is garbage +# post-restart — a numerically-equal PID in the new container is a different process. _STALE_RUNTIME_FILES = ("gateway.pid", "processes.json") ReconcileActionLabel = Literal["started", "registered", "skipped"] @@ -52,9 +45,8 @@ class ReconcileAction: profile: str prior_state: str | None action: ReconcileActionLabel - # How the previous gateway life ended: "clean" (exit path ran), "unclean" (sentinel still - # says running — SIGKILL/OOM/VM death), or "unknown" (no sentinel / never ran). Container - # boot is the one place that can stamp "the previous life ended violently" into a + # "clean" (exit path ran) / "unclean" (sentinel still says running — SIGKILL/OOM/VM death) + # / "unknown". Boot is the one place that can stamp a violent previous death into a # durable, volume-persisted log line (gateway.lifecycle_ledger). prior_exit: str = "unknown" @@ -74,17 +66,14 @@ def reconcile_profile_gateways( """Recreate s6 service registrations for every persistent profile. Always registers a ``gateway-default`` slot for the root profile (the implicit profile at - the top of ``$HERMES_HOME``, not under ``profiles/``): the dispatcher in - ``hermes_cli.gateway`` maps an empty profile suffix to it, so it is what - ``hermes gateway start`` (no ``-p``) targets. + the top of ``$HERMES_HOME``): ``hermes_cli.gateway`` maps an empty profile suffix to it, + so it is what ``hermes gateway start`` (no ``-p``) targets. """ actions: list[ReconcileAction] = [] - # A multiplexing root/default gateway owns inbound platform connections for every - # profile. Named slots must still be registered (explicit lifecycle management stays - # available), but booting them from their persisted run intent would create additional - # multiplex owners. The runtime resolver gives a recognized environment override - # precedence over config.yaml and otherwise preserves the configured value. + # A multiplexing root gateway owns inbound platform connections for every profile: named + # slots are still registered (lifecycle management stays available) but must not boot + # from their persisted run intent, or they would become additional multiplex owners. from gateway.config import load_gateway_config from utils import is_truthy_value try: @@ -94,10 +83,9 @@ def reconcile_profile_gateways( "GATEWAY_MULTIPLEX_PROFILES override if set.", exc_info=True) multiplex_profiles = is_truthy_value(os.environ.get("GATEWAY_MULTIPLEX_PROFILES")) - # Default profile — always register, even if nothing has ever populated the root profile - # dir; auto-up only when the prior state was "running" (same rule as named profiles). A - # legacy `gateway run` container with no state yet seeds that intent as `running` so the - # s6 reconciler preserves the pre-s6 behavior. + # Default profile — always registered; auto-up only when the prior state was "running" + # (same rule as named profiles). A legacy `gateway run` container with no state yet seeds + # that intent as `running` so the pre-s6 behavior is preserved. legacy_default_state = _maybe_migrate_legacy_gateway_run_state( hermes_home, container_argv=container_argv, dry_run=dry_run) default_prior_state = legacy_default_state or _read_desired_state(hermes_home) @@ -110,9 +98,8 @@ def reconcile_profile_gateways( profiles_root = hermes_home / "profiles" if profiles_root.is_dir(): for entry in sorted(profiles_root.iterdir()): - # SOUL.md is always seeded by `hermes profile create` (config.yaml is not — that - # comes later via `hermes setup`): the "real profile" marker so stray dirs - # (backups, manual mkdir) aren't picked up. + # SOUL.md is always seeded by `hermes profile create` (config.yaml comes later via + # `hermes setup`): the "real profile" marker that skips stray dirs (backups, mkdir). if not entry.is_dir() or not (entry / "SOUL.md").exists(): continue # "default" is reserved for the root profile (above); skip a stray @@ -141,9 +128,8 @@ def _maybe_migrate_legacy_gateway_run_state( ) -> str | None: """Seed root gateway_state for pre-s6 `gateway run` containers. - The tini image let Docker users run the gateway as the container command. After the s6 - migration profile gateways are restored from persisted gateway_state.json, so a legacy - container with no state file would register the default service down and never start. + The tini image let users run the gateway as the container command; post-s6, gateways are + restored from gateway_state.json, so such a container would register down and never start. """ state_file = hermes_home / "gateway_state.json" if state_file.exists(): @@ -173,11 +159,10 @@ def _cmdline_argv(cmdline: Path) -> tuple[str, ...]: def _read_container_argv() -> tuple[str, ...]: - """Best-effort read of the container's main program argv. + """Best-effort read of the container's main program argv (the one holding ``main-wrapper.sh``). - s6-overlay v2: PID 1 is ``/init`` and its argv holds ``main-wrapper.sh``. v3: PID 1 is - ``s6-svscan`` and the real command lives on another PID, so after the PID 1 fast path we - scan ``/proc/*/cmdline`` for a process whose argv contains ``main-wrapper.sh``. + s6-overlay v2: PID 1 is ``/init``. v3: PID 1 is ``s6-svscan`` and the real command lives on + another PID, so after the PID 1 fast path we scan ``/proc/*/cmdline``. """ def _cmdlines(): yield Path("/proc/1/cmdline") @@ -201,9 +186,8 @@ def _read_container_argv() -> tuple[str, ...]: def _strip_container_argv_prefix(argv: Sequence[str]) -> list[str]: """Strip the s6/wrapper prefix off the container argv, leaving the hermes args. - Drop everything through the ``main-wrapper.sh`` token rather than peel leading tokens - positionally (which broke on the s6 v2→v3 launcher-shape change): the wrapper path is the - stable boundary the image owns, and the subcommand always follows it. + Drops everything through the ``main-wrapper.sh`` token — the stable boundary the image + owns — rather than peeling tokens positionally (which broke on the s6 v2→v3 bump). """ args = list(argv) @@ -239,12 +223,10 @@ def _is_dashboard_container(argv: Sequence[str]) -> bool: def _read_desired_state(profile_dir: Path) -> str | None: - """Persisted gateway desired state for reconciliation. + """Persisted gateway desired state: ``desired_state`` (operator intent), else ``gateway_state``. - Newer state files carry ``desired_state`` (operator intent from s6 lifecycle commands); - older ones only ``gateway_state``, kept as a fallback so existing profiles preserve their - behavior until the next explicit start/stop. Missing/unparseable files count as "no - desired state" so a corrupt file can't bork the whole reconciliation. + The older key is a fallback so existing profiles keep their behavior until the next explicit + start/stop. Missing/unparseable files count as "no state" so a corrupt file can't bork boot. """ state_file = profile_dir / "gateway_state.json" if not state_file.exists(): @@ -286,11 +268,10 @@ def _write_exec(path: Path, content: str) -> None: def _register_service(scandir: Path, profile: str, *, start: bool) -> None: """Recreate the s6 service slot for one profile. - Mirrors ``S6ServiceManager.register_profile_gateway`` but sets start state via the - ``down`` marker directly: cont-init.d runs as root before s6-svscan scans the dynamic - scandir, so the manager's ``s6-svscanctl -a`` would fail with no control socket. Built in - a sibling temp dir and ``Path.replace``d into place so an interrupted write never leaves - a half-populated dir. + Mirrors ``S6ServiceManager.register_profile_gateway`` but sets start state via the ``down`` + marker: cont-init.d runs before s6-svscan scans the scandir, so ``s6-svscanctl -a`` has no + control socket yet. Built in a sibling temp dir and ``Path.replace``d into place so an + interrupted write never leaves a half-populated dir. """ import shutil @@ -300,10 +281,9 @@ def _register_service(scandir: Path, profile: str, *, start: bool) -> None: validate_profile_name(profile) service_dir = scandir / f"gateway-{profile}" - # Dot-prefix the staging dir so s6-svscan skips it while half-built. A non-dotted staging - # name is supervised AS ROOT by any concurrent ``s6-svscanctl -a`` rescan the moment it - # has a valid ``type``/``run``, creating a root-owned ``supervise/`` that makes - # ``_seed_supervise_skeleton`` EACCES (see ``S6ServiceManager.register_profile_gateway``). + # Dot-prefix the staging dir so s6-svscan skips it while half-built: a non-dotted name is + # supervised AS ROOT by any concurrent rescan the moment it has ``type``/``run``, creating + # a root-owned ``supervise/`` that makes ``_seed_supervise_skeleton`` EACCES. tmp_dir = service_dir.with_name("." + service_dir.name + ".tmp") if tmp_dir.exists(): @@ -313,9 +293,8 @@ def _register_service(scandir: Path, profile: str, *, start: bool) -> None: try: (tmp_dir / "type").write_text("longrun\n", encoding="utf-8") - # Reuse the manager's script rendering — single source of truth so both registration - # paths stay consistent. extra_env is empty here; per-profile env comes from the - # profile's config.yaml (which the gateway itself loads). + # Reuse the manager's script rendering so both registration paths stay consistent. + # extra_env is empty: per-profile env comes from the profile's config.yaml. _write_exec(tmp_dir / "run", S6ServiceManager._render_run_script(profile, extra_env={})) _write_exec(tmp_dir / "finish", S6ServiceManager._render_finish_script()) @@ -323,18 +302,16 @@ def _register_service(scandir: Path, profile: str, *, start: bool) -> None: (tmp_dir / "log").mkdir() _write_exec(tmp_dir / "log" / "run", S6ServiceManager._render_log_run(profile)) - # A `down` file tells s6-supervise NOT to start the service when s6-svscan picks it - # up; the user brings it up with `hermes -p gateway start` (→ `s6-svc -u`). + # `down` tells s6-supervise NOT to start on pickup; `hermes -p gateway start` + # brings it up (→ `s6-svc -u`). if not start: (tmp_dir / "down").touch() - # Pre-create supervise/ with hermes ownership BEFORE publishing: the s6-supervise - # spawned on pickup will EEXIST our dirs/FIFOs and inherit that ownership, so runtime - # s6-svc / s6-svstat / s6-svwait calls (dispatched as the hermes user) won't EACCES. + # Pre-create supervise/ with hermes ownership BEFORE publishing: s6-supervise will EEXIST + # our dirs/FIFOs and inherit it, so runtime s6-svc calls as the hermes user won't EACCES. _seed_supervise_skeleton(tmp_dir) - # Publish atomically: Path.replace silently replaces an existing target on POSIX, so a - # previous reconcile pass's slot is overwritten in one operation. + # Publish atomically (Path.replace overwrites a previous pass's slot in one operation). if service_dir.exists(): shutil.rmtree(service_dir) tmp_dir.replace(service_dir) @@ -343,18 +320,16 @@ def _register_service(scandir: Path, profile: str, *, start: bool) -> None: raise -# 256 KiB soft cap on container-boot.log, rotated to .1 when crossed (~3000 lines at ~80 B -# each — about a year of daily reboots on a 5-profile container). Tuned for grep-ability -# more than space (the persistent volume has GB). +# 256 KiB soft cap on container-boot.log (~3000 lines ≈ a year of daily reboots on a +# 5-profile container), rotated to .1 when crossed. Tuned for grep-ability, not space. _LOG_ROTATE_BYTES = 256 * 1024 def _write_reconcile_log(hermes_home: Path, actions: list[ReconcileAction]) -> None: - """Append one line per profile to $HERMES_HOME/logs/container-boot.log. + """Append one line per profile to $HERMES_HOME/logs/container-boot.log (rotated to ``.1``). - A separate file (vs. agent.log) lets operators debugging "why didn't my profile come back - up" grep for "profile=foo" without unrelated activity. Rotated to ``.1`` (replacing any - previous rotation) before appending once it exceeds ``_LOG_ROTATE_BYTES``. + A separate file (vs. agent.log) lets operators grep "profile=foo" when debugging "why + didn't my profile come back up". """ import time log_dir = hermes_home / "logs" @@ -379,11 +354,10 @@ def _write_reconcile_log(hermes_home: Path, actions: list[ReconcileAction]) -> N def main() -> int: """Entry point invoked from /etc/cont-init.d/02-reconcile-profiles.""" - # A dashboard-only container never supervises per-profile gateways, and reconciling here - # is actively harmful: when gateway and dashboard containers share a bind-mounted - # HERMES_HOME, both race to flock() the same s6-log lock files under - # logs/gateways//lock → "Resource busy" restart storm. The role is detected from - # PID 1 argv, not an operator flag — a flag can be forgotten in a hand-written manifest. + # A dashboard-only container never supervises gateways, and reconciling here is harmful: + # with a shared bind-mounted HERMES_HOME both containers race to flock() the same s6-log + # lock files → "Resource busy" restart storm. Detected from PID 1 argv, not an operator + # flag — a flag can be forgotten in a hand-written manifest. if _is_dashboard_container(_read_container_argv()): print("reconcile: skipping (dashboard container — does not need per-profile gateways)") return 0 From 5f392d511cbf6bcd04932d15b597c2a6d29bd237 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 22:51:03 -0700 Subject: [PATCH 4/8] =?UTF-8?q?refactor(hermes=5Fcli):=20debug.py=20?= =?UTF-8?q?=E2=80=94=20iterate=20bundle=20items=20for=20upload/local=20pri?= =?UTF-8?q?nt=20instead=20of=20re-deriving=20labels?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- hermes_cli/debug.py | 33 +++++++++------------------------ 1 file changed, 9 insertions(+), 24 deletions(-) diff --git a/hermes_cli/debug.py b/hermes_cli/debug.py index 420eb225dc..fa28e5cbd0 100644 --- a/hermes_cli/debug.py +++ b/hermes_cli/debug.py @@ -460,14 +460,12 @@ def collect_share_bundle(log_lines: int = 200, redact: bool = True) -> dict[str, """ dump_text = _capture_dump() log_snapshots = _capture_default_log_snapshots(log_lines, redact=redact) - report = collect_debug_report(log_lines=log_lines, dump_text=dump_text, log_snapshots=log_snapshots) banner = _REDACTION_BANNER if redact else "" bundle: dict[str, str] = {"report": banner + report} for name in _FULL_LOGS: - full = log_snapshots[name].full_text - if full: + if full := log_snapshots[name].full_text: bundle[f"{name}.log"] = banner + dump_text + f"\n\n--- full {name}.log ---\n" + full return bundle @@ -506,35 +504,24 @@ def build_debug_share( Blocking network I/O — callers inside an event loop must run it in a worker thread. """ _best_effort_sweep_expired_pastes() - bundle = collect_share_bundle(log_lines=log_lines, redact=redact) - if redact: logger.info( "hermes debug share: applied force-mode redaction to log snapshots before upload" ) - report = bundle["report"] - - urls: dict[str, str] = {} failures: list[str] = [] - - # Summary report is required — raises on failure so callers can fall back. - urls["Report"] = upload_to_pastebin(report, expiry_days=expiry) - - # Full logs are optional — failures are collected, not raised. - for name in _FULL_LOGS: - label = f"{name}.log" - content = bundle.get(label) - if not content: + # Summary report is required — raises on failure so callers can fall back; full logs are + # optional — failures are collected, not raised. + urls = {"Report": upload_to_pastebin(report, expiry_days=expiry)} + for label, content in bundle.items(): + if label == "report": continue try: urls[label] = upload_to_pastebin(content, expiry_days=expiry) except Exception as exc: failures.append(f"{label}: {exc}") - _schedule_auto_delete(list(urls.values())) - return DebugShareResult(urls=urls, failures=failures, redacted=redact, auto_delete_seconds=_AUTO_DELETE_SECONDS, report=report) @@ -574,11 +561,9 @@ def run_debug_share(args): print("Collecting debug report...") bundle = collect_share_bundle(log_lines=log_lines, redact=redact) print(bundle["report"]) - for name in _FULL_LOGS: - body = bundle.get(f"{name}.log") - if body: - print(f"\n\n{'=' * 60}\nFULL {name}.log\n{'=' * 60}\n") - print(body) + for label, body in bundle.items(): + if label != "report": + print(f"\n\n{'=' * 60}\nFULL {label}\n{'=' * 60}\n\n{body}") return if getattr(args, "nous", False): From 02586dbed0e30c5ed43fe4b2ae4a2503c5b58ad8 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 22:58:35 -0700 Subject: [PATCH 5/8] refactor(hermes_cli): cron incidents rows table, debug share flow tightening, copilot env-var loop invert --- hermes_cli/copilot_auth.py | 20 +++++++++++--------- hermes_cli/cron.py | 26 ++++++++++++++------------ hermes_cli/debug.py | 23 ++++++++--------------- 3 files changed, 33 insertions(+), 36 deletions(-) diff --git a/hermes_cli/copilot_auth.py b/hermes_cli/copilot_auth.py index cecc6451c5..18d8f0ba37 100644 --- a/hermes_cli/copilot_auth.py +++ b/hermes_cli/copilot_auth.py @@ -66,13 +66,13 @@ def resolve_copilot_token() -> tuple[str, str]: any_env_var_set = False for env_var in COPILOT_ENV_VARS: val = os.getenv(env_var, "").strip() - if val: - any_env_var_set = True - valid, msg = validate_copilot_token(val) - if not valid: - logger.warning("Token from %s is not supported: %s", env_var, msg) - continue + if not val: + continue + any_env_var_set = True + valid, msg = validate_copilot_token(val) + if valid: return val, env_var + logger.warning("Token from %s is not supported: %s", env_var, msg) # Fall back to gh auth token ONLY when no Copilot env var was explicitly set: an exported # GITHUB_TOKEN (even an unsupported classic PAT) means the user intends *that* token, not @@ -207,7 +207,8 @@ def copilot_device_code_login( print(" ✗ GitHub did not return a device code.") return None - print(f"\n Open this URL in your browser: {verification_uri}\n Enter this code: {user_code}\n") + print(f"\n Open this URL in your browser: {verification_uri}\n" + f" Enter this code: {user_code}\n") print(" Waiting for authorization...", end="", flush=True) deadline = time.monotonic() + timeout_seconds @@ -243,7 +244,8 @@ def copilot_device_code_login( print(".", end="", flush=True) continue if error: - print("\n" + _DEVICE_CODE_TERMINAL_ERRORS.get(error, f" ✗ Authorization failed: {error}")) + print("\n" + _DEVICE_CODE_TERMINAL_ERRORS.get(error, + f" ✗ Authorization failed: {error}")) return None print("\n ✗ Timed out waiting for authorization.") @@ -349,7 +351,7 @@ def _jwt_disk_path() -> Optional[Path]: def _with_jwt_store(verb: str, op): - """Run ``op(path, store_or_None)`` against the disk store; any failure is logged, never raised.""" + """Run ``op(path, store_or_None)`` against the disk store; failures are logged, never raised.""" path = _jwt_disk_path() if not path: return None diff --git a/hermes_cli/cron.py b/hermes_cli/cron.py index 60d7ddf3df..760cabbce6 100644 --- a/hermes_cli/cron.py +++ b/hermes_cli/cron.py @@ -67,7 +67,8 @@ def _builtin_gateway_liveness() -> Optional[bool]: # gateway process it short-circuits to True so the in-gateway cron tool never # emits a false "gateway not running" (find_gateway_pids can transiently miss the # gateway just after a restart). - with contextlib.suppress(Exception): # a crashing probe is "unknown" — let the pid scan decide + # A crashing lock probe is "unknown", not "dead" — let the pid scan decide. + with contextlib.suppress(Exception): from gateway.status import is_gateway_runtime_lock_active if is_gateway_runtime_lock_active(): return True @@ -212,7 +213,8 @@ def _job_rows(job: Dict[str, Any]) -> List[tuple[str, str]]: optional = [ ("Skills", ", ".join(skills) if skills else ""), ("Script", job.get("script")), - ("Monitor", f"{monitor_source} (agent runs only on output change)" if monitor_source else ""), + ("Monitor", f"{monitor_source} (agent runs only on output change)" if monitor_source + else ""), ("Changed", mon_state.get("last_changed_at") if monitor_source else ""), ("Mode", color("no-agent", Colors.DIM) + " (script stdout delivered directly)" if job.get("no_agent") else ""), @@ -318,17 +320,17 @@ def cron_incidents(args) -> int: _print_banner("Cron Failure Incidents") for inc in incidents: state_display = color(inc["state"], _INCIDENT_STATE_COLORS.get(inc["state"], Colors.DIM)) - print(f" {color(inc['id'], Colors.YELLOW)} {state_display}") - print(f" Job: {inc['job_id']}") - print(f" Type: {inc.get('failure_type', 'unknown')}") - print(f" First seen: {inc.get('first_seen_at', '?')}") - print(f" Last seen: {inc.get('last_seen_at', '?')}") error_text = re.sub(r"\s+", " ", inc.get("error") or "").strip() if len(error_text) > 160: error_text = error_text[:157].rstrip() + "..." - print(f" Error: {error_text}") - if inc.get("output_file"): - print(f" Output: {inc['output_file']}") + rows = [("Job", inc["job_id"]), ("Type", inc.get("failure_type", "unknown")), + ("First seen", inc.get("first_seen_at", "?")), + ("Last seen", inc.get("last_seen_at", "?")), ("Error", error_text), + ("Output", inc.get("output_file"))] + print(f" {color(inc['id'], Colors.YELLOW)} {state_display}") + for label, value in rows: + if label != "Output" or value: + print(f" {label + ':':<12}{value}") print() print(color(f" {len(incidents)} incident(s) | ack one with: hermes cron incidents ack ", Colors.DIM)) @@ -566,8 +568,8 @@ def cron_doctor() -> int: if not findings: print(color("✓ Cron doctor found no issues", Colors.GREEN)) - print(color(f" Checked {len(jobs)} active job(s)." if jobs else " No active jobs configured.", - Colors.DIM)) + summary = f" Checked {len(jobs)} active job(s)." if jobs else " No active jobs configured." + print(color(summary, Colors.DIM)) return 0 issue_count = sum(len(issues) for _, issues in findings) diff --git a/hermes_cli/debug.py b/hermes_cli/debug.py index fa28e5cbd0..436e10614f 100644 --- a/hermes_cli/debug.py +++ b/hermes_cli/debug.py @@ -531,21 +531,18 @@ def _confirm_upload(args) -> bool: if bool(getattr(args, "yes", False)): return True if not sys.stdin.isatty(): - print( - "ERROR: Non-interactive mode requires --yes to confirm upload.\n" - " This prevents accidental exposure of personal data.\n" - " Use --local to view the report without uploading.", - file=sys.stderr, - ) + print("ERROR: Non-interactive mode requires --yes to confirm upload.\n" + " This prevents accidental exposure of personal data.\n" + " Use --local to view the report without uploading.", file=sys.stderr) sys.exit(1) try: answer = input("Upload debug report? [y/N] ").strip().lower() except (EOFError, KeyboardInterrupt): answer = "" - if answer not in ("y", "yes"): - print("Aborted.") - return False - return True + if answer in ("y", "yes"): + return True + print("Aborted.") + return False def run_debug_share(args): @@ -574,7 +571,6 @@ def run_debug_share(args): if not _confirm_upload(args): return print("Collecting debug report...\nUploading...") - try: result = build_debug_share(log_lines=log_lines, expiry=expiry, redact=redact) except RuntimeError as exc: @@ -618,15 +614,12 @@ def _run_debug_share_nous(args, *, log_lines: int, redact: bool) -> None: print("⚠️ --no-redact is set: secrets in your logs will NOT be redacted before upload.\n") print("Collecting debug report...") _best_effort_sweep_expired_pastes() - bundle = collect_share_bundle(log_lines=log_lines, redact=redact) if redact: logger.info("hermes debug share --nous: applied force-mode redaction before upload") - blob = build_nous_bundle(bundle, redact=redact) - print("Uploading to Nous diagnostics storage...") try: - res = share_to_nous(blob) + res = share_to_nous(build_nous_bundle(bundle, redact=redact)) except Exception as exc: print( f"\nNous upload failed: {exc}\n" From 9f854665e8e18a1e1f6dc86d632b51522841624e Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 23:02:34 -0700 Subject: [PATCH 6/8] =?UTF-8?q?refactor(hermes=5Fcli):=20dashboard=5Fprocs?= =?UTF-8?q?/copilot=5Fauth=20=E2=80=94=20compact=20rationale=20docstrings?= =?UTF-8?q?=20and=20comments=20(every=20WHY=20kept)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- hermes_cli/copilot_auth.py | 52 ++++++++-------- hermes_cli/dashboard_procs.py | 108 ++++++++++++++-------------------- 2 files changed, 66 insertions(+), 94 deletions(-) diff --git a/hermes_cli/copilot_auth.py b/hermes_cli/copilot_auth.py index 18d8f0ba37..fca2f96b70 100644 --- a/hermes_cli/copilot_auth.py +++ b/hermes_cli/copilot_auth.py @@ -24,10 +24,9 @@ from hermes_cli._subprocess_compat import IS_WINDOWS, windows_hide_flags logger = logging.getLogger(__name__) -# OAuth device code flow — VS Code's GitHub App client ID. The opencode OAuth App ID -# (Ov23li8tweQw6odWQebz) produces gho_* tokens that cannot be exchanged for Copilot API JWTs -# (404 on /copilot_internal/v2/token); VS Code's produces ghu_* tokens that support exchange, -# required for internal-only models and enterprise endpoints. Tested on Individual + Enterprise. +# OAuth device code flow — VS Code's GitHub App client ID: it mints ghu_* tokens that can be +# exchanged for Copilot API JWTs (required for internal-only models and enterprise endpoints). +# The opencode App ID (Ov23li8tweQw6odWQebz) mints gho_* tokens that 404 on exchange. COPILOT_OAUTH_CLIENT_ID = "Iv1.b507a08c87ecfe98" # ghp_ classic PATs are rejected by the Copilot API (gho_ / github_pat_ / ghu_ work). _CLASSIC_PAT_PREFIX = "ghp_" @@ -105,12 +104,11 @@ def _gh_cli_candidates() -> list[str]: return candidates -# ``gh auth token`` result cache. When gh has no credential store for this HOME (fresh -# profile, desktop-spawned backend, CI) the probe can block for its full 5s timeout on -# keyring / D-Bus prompts. Provider inventory builds (``/api/model/options``, ``hermes tools``) -# probe Copilot auth several times per request, so an uncached miss turned one settings-page -# load into a 4×5s stall exceeding the Desktop renderer's 15s IPC budget. Successes and -# failures are both cached; a short TTL keeps a fresh ``gh auth login`` discoverable. +# ``gh auth token`` result cache. With no credential store for this HOME (fresh profile, CI) +# the probe blocks for its full 5s timeout on keyring / D-Bus prompts, and provider inventory +# builds probe Copilot auth several times per request — an uncached miss made one settings +# page a 4×5s stall past the Desktop renderer's 15s IPC budget. Misses are cached too; a short +# TTL keeps a fresh ``gh auth login`` discoverable. _GH_CLI_TOKEN_CACHE_TTL_SECONDS = 300.0 _gh_cli_token_cache: tuple[float, Optional[str]] | None = None @@ -264,27 +262,24 @@ _TOKEN_EXCHANGE_URL = "https://api.github.com/copilot_internal/v2/token" _EDITOR_VERSION = "vscode/1.104.1" _EXCHANGE_USER_AGENT = "GitHubCopilotChat/0.26.7" -# Transient-failure hardening. Gateway startup often races network readiness (launchd -# relaunch, DHCP/VPN settling); a single-shot exchange that fails there silently degrades to -# the RAW GitHub token, which the Copilot server routes to the "copilot-language-server" -# integrator whose model allowlist omits enterprise-only models → HTTP 400 on every turn -# until the next restart. Retry a few times, and persist the last good exchanged JWT so a -# restart during a blip reuses the still-valid ~30-min token instead of degrading. +# Transient-failure hardening. Gateway startup races network readiness (launchd relaunch, +# DHCP/VPN settling); a single-shot exchange failing there silently degrades to the RAW GitHub +# token, which Copilot routes to the "copilot-language-server" integrator whose allowlist omits +# enterprise-only models → HTTP 400 every turn until restart. Retry, and persist the last good +# JWT so a restart during a blip reuses the still-valid ~30-min token. _EXCHANGE_MAX_ATTEMPTS = 3 _EXCHANGE_BACKOFF_BASE_SECONDS = 1.5 # sleeps ~1.5s, ~3.0s between attempts _JWT_DISK_FILENAME = ".copilot_jwt.json" _JWT_DISK_MAX_BYTES = 1_048_576 # 1 MiB cap on the persisted JWT store read -# Negative cache for failed exchanges: raw-token fingerprint -> epoch until which attempts -# raise immediately (success clears the entry). Without it every load_pool("copilot") re-ran -# the full exchange, and on a permanently-rejected token (403: not Copilot-entitled, expired -# grant, org policy) the retry backoff burned ~4.5s of sleep on EVERY provider-discovery pass -# (/model picker, delegation child spawns, web dashboard). +# Negative cache: fingerprint -> epoch until which attempts raise immediately (success clears +# it). Without it a permanently-rejected token (403: not entitled, expired grant, org policy) +# burned ~4.5s of retry backoff on EVERY provider-discovery pass (/model picker, delegation +# spawns, dashboard). _exchange_failure_cache: dict[str, float] = {} -# Single-flight guard per token fingerprint: concurrent callers (the dashboard polls -# /api/credentials/pool every few seconds, each poll off-loop) wait on the ONE in-flight -# exchange and then hit the positive/negative cache, instead of each spawning their own hung -# resolver thread during a DNS outage. +# Single-flight guard per fingerprint: concurrent callers (dashboard polls /api/credentials/ +# pool every few seconds) wait on the ONE in-flight exchange and then hit the cache, instead +# of each spawning its own hung resolver thread during a DNS outage. _exchange_locks: dict[str, threading.Lock] = {} _exchange_locks_guard = threading.Lock() @@ -411,10 +406,9 @@ def _save_jwt_to_disk(fp: str, api_token: str, expires_at: float, base_url: Opti _with_jwt_store("persist", _save) -# Hard wall-clock cap for the token-exchange HTTP call. urllib's ``timeout`` only bounds -# socket operations AFTER DNS resolution succeeds; getaddrinfo blocks in C and ignores it, -# so on a networkless Windows host the resolver can hang for many minutes (observed: a -# 17-minute event-loop stall that took the whole backend down). +# Hard wall-clock cap for the exchange call: urllib's ``timeout`` only bounds socket ops AFTER +# DNS succeeds; getaddrinfo blocks in C and ignores it, so a networkless Windows host can hang +# for many minutes (observed: a 17-minute event-loop stall that took the backend down). _DNS_GRACE_SECONDS = 5.0 diff --git a/hermes_cli/dashboard_procs.py b/hermes_cli/dashboard_procs.py index 5ec2e96868..afde2e30dc 100644 --- a/hermes_cli/dashboard_procs.py +++ b/hermes_cli/dashboard_procs.py @@ -10,9 +10,8 @@ import subprocess import sys from pathlib import Path -# Cmdline substrings identifying the long-lived server. ``hermes serve`` is the same server -# under the headless name the desktop app spawns; it is reaped on update for the same -# frontend/backend-mismatch reason as ``dashboard``. +# Cmdline substrings identifying the long-lived server; ``hermes serve`` is the same server +# under the headless name the desktop app spawns, reaped on update for the same reason. _DASHBOARD_PATTERNS = tuple( f"{launcher} {cmd}" for cmd in ("dashboard", "serve") @@ -42,10 +41,9 @@ def _iter_process_table() -> list[tuple[int, str]]: """``(pid, cmdline)`` for every process, via wmic (Windows) or ps. Raises on scan failure.""" rows: list[tuple[int, str]] = [] if sys.platform == "win32": - # errors="ignore": wmic may emit the system code page; a decode error would leave - # stdout=None. bounded_probe_run (not run()): run()'s post-timeout cleanup joins pipe - # readers unbounded and a conhost descendant holding duplicated handles wedges it - # forever. It also passes CREATE_NO_WINDOW for the pythonw.exe backend. + # errors="ignore": wmic may emit the system code page (a decode error leaves stdout + # None). bounded_probe_run, not run(): run()'s post-timeout cleanup joins pipe readers + # unbounded and a conhost descendant holding duplicated handles wedges it forever. from hermes_cli._subprocess_compat import bounded_probe_run result = bounded_probe_run( ["wmic", "process", "get", "ProcessId,CommandLine", "/FORMAT:LIST"], @@ -73,13 +71,11 @@ def _iter_process_table() -> list[tuple[int, str]]: def _scan_dashboard_processes(*, exclude_pids: set[int] | None = None) -> list[tuple[int, str]]: - """Return matching ``dashboard``/``serve`` processes with their cmdlines. + """``(pid, cmdline)`` of running ``dashboard``/``serve`` processes; empty on any scan error. - ``hermes update`` swaps files on disk while a forgotten dashboard keeps the old Python - backend in memory against the new JS bundle — a silent mismatch (new auth headers → every - API call 401s). *exclude_pids* must never be returned: Hermes Desktop sets - ``HERMES_DESKTOP_CHILD_PID`` on the backend it spawns so an auto-update never kills the - backend it manages itself. Empty list on any scan error (missing ps/wmic, timeout, ...). + A forgotten dashboard keeps the old Python backend in memory against the new JS bundle + after ``hermes update`` (new auth headers → every API call 401s). *exclude_pids* must never + be returned: Desktop marks the backend it manages via ``HERMES_DESKTOP_CHILD_PID``. """ self_pid = os.getpid() skip = {self_pid, *(exclude_pids or ())} @@ -91,10 +87,8 @@ def _scan_dashboard_processes(*, exclude_pids: set[int] | None = None) -> list[t except (FileNotFoundError, subprocess.TimeoutExpired, OSError): return [] - # Spawn-ledger augmentation: substring patterns miss profiled launches - # (`hermes --profile p serve ...`). Every serve/dashboard registers itself in the spawn - # ledger with live-verified (pid, create_time) — positive identity. Add entries the scan - # missed, preferring the ledger's full argv. + # Spawn-ledger augmentation: substring patterns miss profiled launches (`hermes --profile + # p serve`); the ledger holds live-verified (pid, create_time) — positive identity. try: from hermes_cli.process_identity import ledger_entries seen = {pid for pid, _ in found} | skip @@ -146,10 +140,8 @@ def _profile_flag_value(argv: list[str]) -> str | None: def _is_ephemeral_port_zero_backend(argv: list[str]) -> bool: """True for Desktop-style ``serve|dashboard --port 0`` backends. - Ephemeral-port backends are owned by Hermes Desktop (or are PPID-1 orphans of a prior - update respawn). Replaying them after ``hermes update`` multiplies listening backends - because ``--port 0`` always binds a fresh port. Covers ``serve`` and the legacy - ``dashboard --no-open`` fallback older Desktop runtimes use. + Owned by Hermes Desktop (or PPID-1 orphans of a prior respawn); replaying them after + ``hermes update`` multiplies listening backends because ``--port 0`` binds a fresh port. """ if _dashboard_subcommand_index(argv) is None: return False @@ -210,20 +202,14 @@ def _profile_key_for_respawn(argv: list[str], hermes_home: str | None = None) -> def _filter_dashboard_respawn_candidates( candidates: list[tuple[int, list[str], str | None]], *, own_home: str | None = None ) -> list[list[str]]: - """Select which killed manual backends to respawn after ``hermes update``. + """Select which killed manual backends ``(pid, argv, hermes_home)`` to respawn after update. - Candidates are ``(pid, argv, hermes_home)``; *own_home* (default ``get_hermes_home()``) - is a parameter so tests can pin it. Rules: - 1. Never resurrect Desktop ephemeral ``--port 0`` backends — Desktop owns their lifecycle. - 2. Never replay a backend from a **foreign** ``HERMES_HOME``: the respawn is argv-only - (no ``env=``), so it would come back on the *updating* install's home and steal the - foreign install's fixed port, leaving its supervisor to crash-loop on ``EADDRINUSE``. - Unreadable (``None``) stays eligible. - 3. Dedupe by normalized cmdline. 4. At most one backend per profile / home. - - Does **not** blanket-skip PPID-1: a prior update respawn detaches with - ``start_new_session=True``, so fixed-port manual backends sit under init and must stay - eligible next update. + Rules: never resurrect Desktop ``--port 0`` backends (Desktop owns them); never replay a + backend from a **foreign** ``HERMES_HOME`` (the respawn is argv-only, so it would come back + on the updating install's home and steal the foreign install's fixed port → its supervisor + crash-loops on ``EADDRINUSE``; unreadable ``None`` stays eligible); dedupe by normalized + cmdline; at most one backend per profile / home. PPID-1 is NOT skipped: a prior respawn + detaches with ``start_new_session=True``, so fixed-port manual backends sit under init. """ if own_home is None: try: @@ -334,12 +320,10 @@ def _kill_stale_dashboard_processes( ) -> dict[str, list]: """Kill running ``hermes dashboard`` / ``hermes serve`` processes (update end, ``--stop``). - With ``restart_managed`` (update path only) a detected ``hermes-dashboard.service`` is - restarted through systemd, any other killed PID owned by a systemd unit has that unit - restarted after the kill (systemd treats our SIGTERM as a clean stop, so - ``Restart=on-failure`` never fires), and manual PIDs are respawned from their captured - argv. *already_restarted_units* (no ``.service`` suffix) were restarted by the caller - already; PIDs they own are left untouched, not killed twice. + With ``restart_managed`` (update only) systemd-owned PIDs get their unit restarted after the + kill (systemd treats our SIGTERM as a clean stop, so ``Restart=on-failure`` never fires) and + manual PIDs are respawned from captured argv. PIDs owned by *already_restarted_units* (no + ``.service`` suffix) are left untouched, not killed twice. """ if restart_managed and _m()._restart_managed_dashboard_service(reason): # The dashboard unit is handled but every OTHER backend is not (a host may also run @@ -460,13 +444,11 @@ def _detect_concurrent_hermes_instances( ) -> list[tuple[int, str]]: """``(pid, name)`` of other live processes whose .exe is one of our entry-point shims. - Windows blocks DELETE/REPLACE on a running .exe; Desktop spawns ``hermes.EXE`` as a backend - child, so the update's quarantine rename fails with ``[WinError 32]``. Excludes our own - PID and every *shim* ancestor (the setuptools launcher is a separate native process from - the ``python.exe`` it loads — otherwise every update reports its own launcher); a second - hermes.exe under a non-Hermes parent (Desktop child) is still flagged. ``proc.parents()`` - (whole chain at once) because a per-hop loop bailed on the first AccessDenied. - Empty off-Windows, without psutil, or with no other instances. Never raises. + Windows blocks DELETE/REPLACE on a running .exe, so a Desktop-spawned ``hermes.EXE`` makes + the update's quarantine rename fail with ``[WinError 32]``. Excludes our PID and every + *shim* ancestor (the setuptools launcher is a separate native process from the + ``python.exe`` it loads); ``proc.parents()`` at once because a per-hop loop bailed on the + first AccessDenied. Empty off-Windows / without psutil. Never raises. """ if not _m()._is_windows(): return [] @@ -539,10 +521,10 @@ def _process_ppid(pid: int) -> int | None: # --- SSH remote-backend lock ownership ------------------------------------- # ``backend.lock.json`` is written by the Desktop SSH runtime on the *remote* host for every -# ``hermes serve`` it spawns (apps/desktop/electron/remote-lifecycle.ts). A backend another -# client started is legitimate and lock-owned even with no parent here (sshd exited → ppid 1); -# the reap must NEVER kill a PID a valid lock claims — that once killed a production backend. -# Schema constants mirror the writer; a mismatched record is ignored (the reap only *spares*). +# ``hermes serve`` it spawns (apps/desktop/electron/remote-lifecycle.ts). Such a backend is +# legitimate even with no parent here (sshd exited → ppid 1); the reap must NEVER kill a PID a +# valid lock claims — that once killed a production backend. Schema mirrors the writer; a +# mismatched record is ignored (the reap only *spares*). _LOCKFILE_SCHEMA_VERSION = 2 _PROTOCOL_VERSION = 1 _REMOTE_LOCK_SUBDIR = "desktop-ssh" @@ -581,9 +563,8 @@ def _valid_lockfile_payload(parsed: object, ownership_id: str) -> bool: value = parsed.get(field) if not isinstance(value, str) or len(value) > 1024: return False - # logPath is ``{lock_root}/{ownershipId}/{spawnNonce}.log``. Only the suffix is checked so - # a relocated HERMES_HOME doesn't falsely reject a legitimate remote-owned backend (a false - # reject re-introduces the kill). + # logPath is ``{lock_root}/{ownershipId}/{spawnNonce}.log``; only the suffix is checked so a + # relocated HERMES_HOME can't falsely reject a legitimate backend (= re-introduce the kill). return parsed["logPath"].endswith(f"/{ownership_id}/{parsed['spawnNonce']}.log") @@ -636,18 +617,15 @@ def _reap_orphaned_desktop_local_serves( *, reason: str = "orphaned desktop-local hermes serve", signal_term=None, signal_kill=None, sleep_fn=None, lock_owned_pids_fn=None, process_age_seconds_fn=None, ) -> dict[str, list]: - """Kill leftover Desktop-local ``hermes serve`` backends with no parent. + """Kill leftover Desktop-local ``hermes serve`` backends with no parent. Never raises. - When Electron dies uncleanly, ``serve --host 127.0.0.1 --port 0`` children get reparented - to pid 1 with their MCP trees alive; each Desktop boot then stacks a fresh backend on the - corpses until EMFILE. The parent-death watchdog prevents *future* orphans; this clears - *already* orphaned ones. A candidate is reaped only if ALL hold: Desktop-local shape - (never a fixed-port remote serve); ppid 1 (or 0); not self / parent / - HERMES_DESKTOP_CHILD_PID; not claimed by a valid ``backend.lock.json`` (SSH backends other - clients started legitimately sit at ppid 1 — killing them is an incident); older than - ``_REAP_MIN_AGE_SECONDS`` with a determinable age (Desktop writes the lock only after - HERMES_BACKEND_READY, so a live sibling in concurrent multi-profile startup is briefly - unowned and indistinguishable from a corpse). Best-effort; never raises. + When Electron dies uncleanly its ``serve --host 127.0.0.1 --port 0`` children are + reparented to pid 1 with their MCP trees alive; each Desktop boot then stacks a fresh + backend on the corpses until EMFILE. Reaped only if ALL hold: Desktop-local shape; ppid + 0/1; not self / parent / HERMES_DESKTOP_CHILD_PID; not claimed by a valid + ``backend.lock.json`` (SSH backends other clients started legitimately sit at ppid 1); + older than ``_REAP_MIN_AGE_SECONDS`` with a determinable age (Desktop writes the lock only + after HERMES_BACKEND_READY, so a live sibling mid-startup is briefly unowned). """ import signal as _signal import time as _time From b7c5e3a205dd9a4372c31cf5448c1a7ab2d44254 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 23:03:28 -0700 Subject: [PATCH 7/8] refactor(hermes_cli): cron doctor summary line width --- hermes_cli/cron.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hermes_cli/cron.py b/hermes_cli/cron.py index 760cabbce6..239f1177a7 100644 --- a/hermes_cli/cron.py +++ b/hermes_cli/cron.py @@ -568,8 +568,8 @@ def cron_doctor() -> int: if not findings: print(color("✓ Cron doctor found no issues", Colors.GREEN)) - summary = f" Checked {len(jobs)} active job(s)." if jobs else " No active jobs configured." - print(color(summary, Colors.DIM)) + note = f" Checked {len(jobs)} active job(s)." if jobs else " No active jobs configured." + print(color(note, Colors.DIM)) return 0 issue_count = sum(len(issues) for _, issues in findings) From f1d0eaaa85e20022e867d1862a254c9b0d683069 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 23:07:30 -0700 Subject: [PATCH 8/8] =?UTF-8?q?refactor(hermes=5Fcli):=20group=20D=20?= =?UTF-8?q?=E2=80=94=20defensive-layer=20collapse=20(lockfile=20validation?= =?UTF-8?q?,=20pending=20loader,=20gh=20candidates,=20env=20pid=20parsing)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- hermes_cli/copilot_auth.py | 9 ++++--- hermes_cli/dashboard_procs.py | 44 ++++++++++++++--------------------- hermes_cli/debug.py | 9 ++----- 3 files changed, 23 insertions(+), 39 deletions(-) diff --git a/hermes_cli/copilot_auth.py b/hermes_cli/copilot_auth.py index fca2f96b70..f7b0066389 100644 --- a/hermes_cli/copilot_auth.py +++ b/hermes_cli/copilot_auth.py @@ -96,11 +96,10 @@ def resolve_copilot_token() -> tuple[str, str]: def _gh_cli_candidates() -> list[str]: """Candidate ``gh`` binary paths, including common Homebrew installs.""" candidates: list[str] = [c for c in (shutil.which("gh"),) if c] - for candidate in ("/opt/homebrew/bin/gh", "/usr/local/bin/gh", - str(Path.home() / ".local" / "bin" / "gh")): - if (candidate not in candidates and os.path.isfile(candidate) - and os.access(candidate, os.X_OK)): - candidates.append(candidate) + candidates += [ + c for c in ("/opt/homebrew/bin/gh", "/usr/local/bin/gh", str(Path.home() / ".local/bin/gh")) + if c not in candidates and os.path.isfile(c) and os.access(c, os.X_OK) + ] return candidates diff --git a/hermes_cli/dashboard_procs.py b/hermes_cli/dashboard_procs.py index afde2e30dc..c0f61950a3 100644 --- a/hermes_cli/dashboard_procs.py +++ b/hermes_cli/dashboard_procs.py @@ -243,13 +243,8 @@ def _exclude_pids_from_env() -> set[int]: """PIDs Desktop marks as live backends (``HERMES_DESKTOP_CHILD_PID``, comma-separated).""" out: set[int] = set() for part in os.environ.get("HERMES_DESKTOP_CHILD_PID", "").split(","): - part = part.strip() - if not part: - continue - try: + with contextlib.suppress(ValueError): out.add(int(part)) - except ValueError: - continue return out @@ -265,18 +260,17 @@ def _kill_pids_windows(pids: list[int], killed: list[int], failed: list[tuple[in expected_start_time = pid_start_times.get(pid) if expected_start_time is None: failed.append((pid, "could not verify process identity")) - continue - if not pid_is_hermes(pid, expected_start_time=expected_start_time): + elif not pid_is_hermes(pid, expected_start_time=expected_start_time): failed.append((pid, "not hermes-owned or process identity changed")) - continue - result = subprocess.run(["taskkill", "/PID", str(pid), "/F"], stdout=subprocess.PIPE, - stderr=subprocess.PIPE, stdin=subprocess.DEVNULL, text=True, - encoding="utf-8", errors="replace", timeout=10, - creationflags=windows_hide_flags()) - if result.returncode == 0: - killed.append(pid) else: - failed.append((pid, (result.stderr or result.stdout or "").strip())) + result = subprocess.run( + ["taskkill", "/PID", str(pid), "/F"], stdout=subprocess.PIPE, + stderr=subprocess.PIPE, stdin=subprocess.DEVNULL, text=True, encoding="utf-8", + errors="replace", timeout=10, creationflags=windows_hide_flags()) + if result.returncode == 0: + killed.append(pid) + else: + failed.append((pid, (result.stderr or result.stdout or "").strip())) except (FileNotFoundError, subprocess.TimeoutExpired, OSError) as e: failed.append((pid, str(e))) @@ -552,17 +546,15 @@ def _valid_lockfile_payload(parsed: object, ownership_id: str) -> bool: or not _is_hex(parsed.get("tokenFingerprint"), 32) ): return False - pid = parsed.get("pid") - port = parsed.get("port") - if not isinstance(pid, int) or not 0 < pid <= 4194304: + pid, port = parsed.get("pid"), parsed.get("port") + if not (isinstance(pid, int) and 0 < pid <= 4194304): return False - if not isinstance(port, int) or not 0 <= port <= 65535: + if not (isinstance(port, int) and 0 <= port <= 65535): return False # String fields must be present and bounded (the writer enforces <=1024). - for field in ("profile", "hermesPath", "hermesHome", "logPath", "startedAt"): - value = parsed.get(field) - if not isinstance(value, str) or len(value) > 1024: - return False + if any(not isinstance(parsed.get(f), str) or len(parsed[f]) > 1024 + for f in ("profile", "hermesPath", "hermesHome", "logPath", "startedAt")): + return False # logPath is ``{lock_root}/{ownershipId}/{spawnNonce}.log``; only the suffix is checked so a # relocated HERMES_HOME can't falsely reject a legitimate backend (= re-introduce the kill). return parsed["logPath"].endswith(f"/{ownership_id}/{parsed['spawnNonce']}.log") @@ -576,10 +568,8 @@ def _lock_owned_serve_pids(base_dir: Path | None = None) -> set[int]: import json root = base_dir if base_dir is not None else _hermes_home_dir() / _REMOTE_LOCK_SUBDIR owned: set[int] = set() - if not root.is_dir(): - return owned try: - entries = list(root.iterdir()) + entries = list(root.iterdir()) if root.is_dir() else [] except OSError: return owned for entry in entries: diff --git a/hermes_cli/debug.py b/hermes_cli/debug.py index 436e10614f..f2edfb3003 100644 --- a/hermes_cli/debug.py +++ b/hermes_cli/debug.py @@ -56,11 +56,8 @@ def _pending_file() -> Path: def _load_pending() -> list[dict]: - path = _pending_file() - if not path.exists(): - return [] try: - data = json.loads(path.read_text(encoding="utf-8")) + data = json.loads(_pending_file().read_text(encoding="utf-8")) except (OSError, ValueError): return [] if not isinstance(data, list): @@ -265,10 +262,8 @@ def _missing_log_note(log_name: str) -> str: reason = _CLIENT_SIDE_LOGS.get(log_name) if reason is None: return "(file not found)" - primary = _primary_log_path(log_name) - where = f" — expected at {primary}" if primary else "" - return f"(not on this host: {reason}{where})" + return f"(not on this host: {reason}{f' — expected at {primary}' if primary else ''})" def _resolve_log_path(log_name: str) -> Optional[Path]: