fix(kanban): dispatch_profiles is config-only and fail-closed; trim tests

Follow-up to the salvaged #111004 commit, aligning it with the shape agreed
on #110995:

- Drop the HERMES_KANBAN_DISPATCH_PROFILES env bridge: non-secret behaviour
  lives in config.yaml only, like every other kanban.* key.
- Read the key via load_config_readonly() with the same fail-open config
  read as the sibling kanban.* readers (configured_max_in_progress).
- Fail closed when the key is set: the "none" sentinel is gone (an empty
  list already claims nothing), and an assignee that is not a valid
  profile id is never claimable instead of being lower-cased into the
  allowlist.
- Trim the regression file to two invariants (allowlist without `default`
  buckets the card as nonspawnable AND turns has_spawnable_ready off;
  unset key keeps upstream behaviour). Both drive the real dispatch tick
  against a real config.yaml + kanban.db; the first is red on origin/main.
- Docs: move the "Shared boards across homes" section out of the
  gateway-dispatcher paragraph, state that `default` collides by
  construction, add the config-reference row.
This commit is contained in:
teknium1
2026-09-14 18:09:03 -07:00
committed by Teknium
parent 2d46af3fa2
commit 923960fa4b
4 changed files with 77 additions and 111 deletions

View File

@@ -1771,12 +1771,11 @@ DEFAULT_CONFIG = {
# fan-out workflows that would otherwise saturate one profile's local model / API quota / browser
# pool while leaving other profiles idle. See #21582.
"max_in_progress_per_profile": None,
# Per-home claim allowlist for shared boards (#110995): profile names this
# home's dispatcher may claim, as a list or comma-separated string. Unset
# (None) = any existing profile is claimable (upstream behavior);
# ["none"] = claim nothing. On a shared kanban.db, every home's
# profile_exists("default") is True, so without this each home can claim
# default-assigned cards. Runtime override: HERMES_KANBAN_DISPATCH_PROFILES.
# Per-home claim allowlist for boards shared across Hermes homes (#110995): profile names
# this home's dispatcher may claim (list or comma-separated string). None = any existing
# profile is claimable. Set = fail-closed (an empty list claims nothing). Every home has a
# root profile named "default", so on a shared kanban.db every home can otherwise claim
# default-assigned cards.
"dispatch_profiles": None,
# Auto-run the decomposer on Triage tasks every tick. False = manual via `hermes kanban
# decompose <id>` or the dashboard's Decompose button.

View File

@@ -1220,11 +1220,10 @@ def _profile_exists_fn() -> Optional[Callable[[str], bool]]:
imported (local import avoids a cycle; callers fall back to trusting the
assignee).
When a per-home claim allowlist is configured (``kanban.dispatch_profiles``
or ``HERMES_KANBAN_DISPATCH_PROFILES``, #110995), the returned predicate
additionally requires the assignee to be listed — so a card assigned to
``default`` is only claimable by homes that opted into it. Foreign
assignees land in the existing ``skipped_nonspawnable`` bucket.
When ``kanban.dispatch_profiles`` is set (#110995) the returned predicate
additionally requires the assignee to be listed, fail-closed — so a card
assigned to ``default`` is only claimable by homes that opted into it.
Foreign assignees land in the existing ``skipped_nonspawnable`` bucket.
"""
try:
from hermes_cli.profiles import normalize_profile_name, profile_exists
@@ -1238,61 +1237,42 @@ def _profile_exists_fn() -> Optional[Callable[[str], bool]]:
try:
canon = normalize_profile_name(name)
except ValueError:
canon = (name or "").strip().lower()
return False
return canon in allowlist and bool(profile_exists(name))
return _gated
# Env-var bridge for the per-home kanban dispatch claim allowlist (#110995).
# Non-secret behavioral settings live in config.yaml; this is the fleet-friendly
# runtime override (containers set env per home more easily than per-home
# config.yaml edits), mirroring terminal.cwd -> TERMINAL_CWD.
KANBAN_DISPATCH_PROFILES_ENV = "HERMES_KANBAN_DISPATCH_PROFILES"
def _dispatch_profile_allowlist(normalize_profile_name) -> Optional[frozenset]:
"""Per-home claim allowlist for the kanban dispatcher (#110995).
"""Per-home claim allowlist ``kanban.dispatch_profiles`` (#110995).
On a shared board (one ``kanban.db`` mounted across several Hermes homes),
every home's ``profile_exists`` returns True for ``default`` — the root
profile every home has — so a card assigned to ``default`` is claimable by
every home's dispatcher. A home opts out of foreign claims by declaring
which assignees it may claim, canonically in config.yaml:
which assignees it may claim::
kanban:
dispatch_profiles: ["sage", "researcher"] # or "sage,researcher"
(``HERMES_KANBAN_DISPATCH_PROFILES`` overrides config at runtime.)
Returns ``None`` when neither is set (upstream behavior: any existing
profile is claimable). The special value ``none`` (or an empty value)
yields an empty allowlist — the home claims nothing.
Returns ``None`` when the key is unset (upstream behavior: any existing
profile is claimable). A set value is fail-closed: an empty list claims
nothing. Config read is fail-open like the sibling ``kanban.*`` readers.
"""
raw = os.environ.get(KANBAN_DISPATCH_PROFILES_ENV)
if raw is None:
try:
from hermes_cli.config import load_config
cfg = load_config()
kanban_cfg = cfg.get("kanban", {}) if isinstance(cfg, dict) else {}
raw = kanban_cfg.get("dispatch_profiles")
except Exception:
return None
try:
from hermes_cli.config import load_config_readonly
raw = (load_config_readonly() or {}).get("kanban", {}).get("dispatch_profiles")
except Exception:
return None
if raw is None:
return None
if isinstance(raw, (list, tuple)):
names = [str(n) for n in raw]
else:
names = str(raw).split(",")
names = [n.strip() for n in names if n.strip()]
if not names or all(n.casefold() == "none" for n in names):
return frozenset()
names = [str(n) for n in raw] if isinstance(raw, (list, tuple)) else str(raw).split(",")
allowed = set()
for n in names:
try:
allowed.add(normalize_profile_name(n))
except ValueError:
allowed.add(n.strip().lower())
continue
return frozenset(allowed)

View File

@@ -1,69 +1,54 @@
"""Per-home kanban dispatch claim allowlist.
"""kanban.dispatch_profiles: per-home claim allowlist for shared boards (#110995).
Regression tests for #110995: on a shared kanban board (one kanban.db mounted
across several Hermes homes) every home's ``profile_exists("default")`` is
unconditionally True, so any home's dispatcher could claim cards assigned to
``default``. ``kanban.dispatch_profiles`` (or the
``HERMES_KANBAN_DISPATCH_PROFILES`` env bridge) declares which assignees this
home may claim; anything else lands in ``skipped_nonspawnable``.
On a board shared across Hermes homes (one kanban.db mounted in several
containers) every home's ``profile_exists("default")`` is True, so any home's
dispatcher could claim cards assigned to ``default``. The allowlist wraps the
same predicate consumed by the spawn gate and the spawnable telemetry, so a
foreign assignee lands in ``skipped_nonspawnable`` and does not keep the
gateway wake-up loop hot.
"""
from __future__ import annotations
import os
from pathlib import Path
import pytest
from hermes_cli import kanban_db as kb
from hermes_cli import kanban_db_connect as kbc
from hermes_cli import kanban_db_dispatch as kbd
@pytest.fixture
def every_home_has_default(monkeypatch):
"""Reproduce the incident premise: ``profile_exists("default")`` is True."""
from hermes_cli import profiles
monkeypatch.setattr(profiles, "profile_exists", lambda name: True)
def kanban_home(tmp_path, monkeypatch):
"""Isolated HERMES_HOME with an empty kanban DB."""
home = tmp_path / ".hermes"
home.mkdir()
monkeypatch.setenv("HERMES_HOME", str(home))
monkeypatch.setattr(Path, "home", lambda: tmp_path)
kb.init_db()
return home
# Env bridge for the claim allowlist (mirrors
# kanban_db_dispatch.KANBAN_DISPATCH_PROFILES_ENV; spelled out so the tests
# stay red-on-base against code that lacks the constant).
_DISPATCH_PROFILES_ENV = "HERMES_KANBAN_DISPATCH_PROFILES"
def test_allowlist_without_default_skips_default_card(kanban_home, all_assignees_spawnable):
"""Fail-closed: the allowlist is set and ``default`` is not on it, so the
card is neither spawnable (wake-up telemetry) nor claimed (spawn gate),
even though every profile exists locally."""
(kanban_home / "config.yaml").write_text(
"kanban:\n dispatch_profiles:\n - sage\n", encoding="utf-8",
)
with kbc.connect() as conn:
tid = kb.create_task(conn, title="foreign card", assignee="default")
assert kbd.has_spawnable_ready(conn) is False
res = kbd.dispatch_once(conn, dry_run=True)
assert res.spawned == []
assert res.skipped_nonspawnable == [tid]
@pytest.fixture
def no_env_bridge(monkeypatch):
monkeypatch.delenv(_DISPATCH_PROFILES_ENV, raising=False)
def test_allowlist_env_restricts_default_claim(monkeypatch, every_home_has_default):
monkeypatch.setenv(_DISPATCH_PROFILES_ENV, "sage,researcher")
claim = kbd._profile_exists_fn()
assert claim is not None
assert claim("sage") is True
assert claim("researcher") is True
# The incident: this home must NOT claim another home's "default".
assert claim("default") is False
def test_allowlist_env_none_claims_nothing(monkeypatch, every_home_has_default):
monkeypatch.setenv(_DISPATCH_PROFILES_ENV, "none")
claim = kbd._profile_exists_fn()
assert claim is not None
assert claim("sage") is False
assert claim("default") is False
def test_allowlist_config_key_end_to_end(every_home_has_default, no_env_bridge):
"""Real config.yaml -> real load_config() -> gated predicate."""
home = Path(os.environ["HERMES_HOME"])
(home / "config.yaml").write_text("kanban:\n dispatch_profiles:\n - sage\n")
claim = kbd._profile_exists_fn()
assert claim is not None
assert claim("sage") is True
assert claim("default") is False
def test_unset_allowlist_preserves_upstream(every_home_has_default, no_env_bridge):
claim = kbd._profile_exists_fn()
assert claim is not None
assert claim("default") is True
def test_unset_allowlist_keeps_default_claimable(kanban_home, all_assignees_spawnable):
"""No key = upstream behaviour: any existing profile, ``default`` included."""
with kbc.connect() as conn:
tid = kb.create_task(conn, title="local card", assignee="default")
assert kbd.has_spawnable_ready(conn) is True
res = kbd.dispatch_once(conn, dry_run=True)
assert [t for t, _a, _w in res.spawned] == [tid]
assert res.skipped_nonspawnable == []

View File

@@ -285,23 +285,10 @@ kanban:
dispatch_profiles: null # default: this home may claim cards for any
# existing profile. Set to a list (or
# comma-separated string) of profile names to
# restrict which assignees this home claims.
# ["none"] claims nothing. Override at
# runtime with HERMES_KANBAN_DISPATCH_PROFILES.
# restrict which assignees this home claims;
# fail-closed, an empty list claims nothing.
```
### Shared boards across homes
Mounting one `kanban.db` in several Hermes homes (containers, fleet hosts) shares
the board, but profile names are home-local: every home has a root profile named
`default`, and the dispatcher's spawn gate checks `profile_exists(assignee)`
against the *claiming* home. Without further configuration, every home's
dispatcher considers a card assigned to `default` claimable, so the wrong home
can claim and run it. Either give each home unique profile names, or set
`kanban.dispatch_profiles` per home to declare exactly which assignees that home
may claim — anything else lands in the dispatcher's `skipped_nonspawnable`
bucket instead of spawning.
Override the config flag at runtime via `HERMES_KANBAN_DISPATCH_IN_GATEWAY=0`
for debugging. Standard gateway supervision applies: run `hermes gateway
start` directly, or wire the gateway up as a systemd user unit (see the
@@ -316,6 +303,20 @@ the old standalone daemon alive for one release cycle, but running both
a gateway-embedded dispatcher AND a standalone daemon against the same
`kanban.db` causes claim races and is not supported.
### Shared boards across homes
Mounting one `kanban.db` in several Hermes homes (containers, fleet hosts) shares
the board, but profile names are home-local and every home has a root profile
named `default` — so `default` collides by construction. The dispatcher's spawn
gate checks `profile_exists(assignee)` against the *claiming* home, and without
further configuration every home's dispatcher considers a card assigned to
`default` claimable, so the wrong home can claim and run it. Either give each
home unique profile names and never assign cards to `default` on a shared board,
or set `kanban.dispatch_profiles` per home to declare exactly which assignees
that home may claim — anything else lands in the dispatcher's
`skipped_nonspawnable` bucket instead of spawning, and no longer counts as
spawnable work for the gateway's wake-up probe.
### Idempotent create (for automation / webhooks)
```bash
@@ -898,6 +899,7 @@ All commands are also available as a slash command in the interactive CLI and in
|------------|---------|--------------|
| `kanban.max_in_progress` | unset (unlimited) | Caps the number of simultaneously running tasks. When the board already has N running, the dispatcher skips spawning more — useful for slow workers (local LLMs, resource-constrained hosts) so they finish what they have before more pile up and time out. Invalid or below-1 values log a warning and behave as unlimited. |
| `kanban.max_in_progress_per_profile` | unset (unlimited) | Per-profile variant of `max_in_progress` — caps how many tasks any single assignee profile may run concurrently. Useful when one profile is slow or rate-limited but others should keep flowing. Applies alongside the board-wide `max_in_progress`; both must allow a spawn for it to proceed. |
| `kanban.dispatch_profiles` | unset (any existing profile) | Per-home claim allowlist for boards shared across Hermes homes. When set, this home's dispatcher only claims cards whose assignee is listed (fail-closed; an empty list claims nothing); other assignees land in `skipped_nonspawnable`. See [Shared boards across homes](#shared-boards-across-homes). |
| `kanban.auto_promote_children` | `true` | After `decompose_triage_task()` produces children with no parent-blocker dependencies, they're automatically promoted to `ready` so the dispatcher can pick them up. Set to `false` to require manual review — children stay in `todo` until you promote them. |
| `kanban.default_workdir` | unset | Board-level default working directory applied to new tasks when neither `--workspace` nor the task itself overrides it. Per-task `workspace:` still wins. |