Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions PATCHES.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,4 @@ default; do **not** retire such a row on a PR-merge signal. See the #44338 row.
| fork PR (TBD — no upstream PR; fork-internal review-lane semantics) | Fix the kanban crash-detector dropping a crashed **reviewer** back into the build lane. A card moved to `review` (a worker opened a PR and parked it) is claimed by `claim_review_task`, which CAS-transitions `review → running` and records a `claimed` event carrying `source_status: "review"` — so while the reviewer works the row status is `running`, indistinguishable from a build run. PR #16 only handled a dead worker on a card parked in a NON-`running` lane; it did NOT cover a reviewer that dies while actively `running`. When `detect_crashed_workers` reaped such a crash it ran the running-crash UPDATE `SET status='ready'`, losing the review lane: (i) the normal `ready` dispatch then re-ran the IMPLEMENTER instead of respawning the reviewer (PR-under-review silently falls back into the build lane), and (ii) `check_respawn_guard` recomputed `is_review=False`, re-tripping the `recent_success`/`active_pr` guards (the original build run is a recent `completed` run + left a PR-URL comment) and deferring respawn for the full window. The fix reads the durable `source_status: "review"` signal off the crashed run's `claimed` event (scoped to `current_run_id`; no new schema column) and, on a genuine crash, restores the card to `review` instead of `ready` — claim still cleared, CAS still guarded on `status='running'`, crash event / run outcome / failure counter / circuit breaker bookkeeping unchanged. To keep a flaky reviewer from looping forever in `review`, the crash-path breaker-trip UPDATE in `_record_task_failure` widens its WHERE-IN from `('ready','running')` to `('ready','running','review')`, so a repeatedly-crashing reviewer still trips to `blocked` via the normal failure-count path. Rate-limited (cooldown defer) and protocol-violation (immediate trip) sub-cases unchanged — only the lane on a genuine crash changes. Single-file change in `hermes_cli/kanban_db.py` + 4 regression tests in `tests/hermes_cli/test_kanban_db.py` (crashed reviewer → `review`; crashed build run → `ready` unchanged; repeatedly-crashing reviewer → `blocked`; restored `review` card is free of the `recent_success`/`active_pr` guards). Fork-internal review-lane semantics (the `claim_review_task` source_status signal is fork machinery) — not surfaced to NousResearch. **Retire trigger:** never auto-retires on an upstream PR-merge signal (no upstream PR); remove only if the fork's review-lane dispatch is retired or upstream adopts equivalent review-lane-aware crash recovery. | permanent-local | v2026.6.19 |
| fork PR (TBD — no upstream PR; fork-internal review-lane semantics) | Auto-route a reviewer's `review-changes-requested` block back to the original author from the housekeeping tick, closing the reviewer→author hop the GitHub `pull_request_review` webhook cannot close when reviewer and author share one GitHub identity. The reviewer (lamport) runs as the `cwest` identity that OWNS the team PRs, so GitHub rejects his `REQUEST_CHANGES` with HTTP 422 and he falls back to a `COMMENT` event — which is NOT `changes_requested`, so the webhook router never bounces and the card sits `blocked` until a human hand-routes it. The fix is board-internal and lives entirely in the dispatcher, NOT the reviewer (the reviewer's terminal action stays a clean `kanban_block`, preserving the lane-corruption-safe design): `auto_route_review_bounce` scans `blocked` cards on each `dispatch_once` tick (before `recompute_ready`), and for any card whose most-recent sticky `blocked` event carries the `review-changes-requested` reason prefix, reassigns it to the original author (resolved from the `assigned` event history — the `from` of the move whose `to` is the current reviewer — never a literal profile name) and unblocks it via the existing `unblock_task`, which clears the `active_pr`/`recent_success` respawn guards exactly like a manual block→unblock cutoff, plus a `dispatcher`-authored `[audit]` comment naming the PR and verdict gist. Idempotent (the route flips the card off `blocked`, so a later tick won't re-fire; two ticks → one route, one comment). The `awaiting-casey-signoff` PASS/acceptance block is excluded by the prefix match (must stay `blocked`+casey), as are non-review and circuit-breaker (`gave_up`) blocks. Two `check_respawn_guard` carve-outs support the route: the `recent_success` guard now honors a trailing `unblocked` event (the bounced build run is the work being reworked — it must not veto respawn), and the dup-PR scan excludes the dispatcher's own same-second audit comment. Toggle `kanban.auto_route_review_bounce` (default ON) gates the whole path; wired through `dispatch_once`, the `hermes kanban dispatch` CLI, and the gateway dispatcher watcher. Changes in `hermes_cli/kanban_db.py`, `hermes_cli/kanban.py`, `gateway/kanban_watchers.py` + 8 regression tests in `tests/hermes_cli/test_kanban_auto_route_review_bounce.py` (core route; route via real `dispatch_once`; acceptance block does NOT route; idempotency; routed card dispatchable; non-review block; circuit-breaker block; toggle off). Upstream has no equivalent (no `review-changes-requested`/`auto_route`/`bounce` concept in upstream `hermes_cli/*.py`) — cwest-team review-loop tooling. **Retire trigger:** never auto-retires (permanent-local); remove only if the fork's review-lane dispatch is retired. | permanent-local | v2026.6.19 |
| fork PR (TBD — upstream PR not yet opened) | Add a per-task inner-iteration budget override so a large kanban card can raise the worker's `agent.max_turns` ceiling instead of timing out at the global default (90). The dispatcher's worker-spawn env block set the goal-loop turn budget and terminal timeouts but never set `HERMES_MAX_ITERATIONS`, so every spawned worker fell back to the global 90 regardless of task size; a large task (e.g. a ~1900-line doc reconcile) exhausted 90 iterations and timed out, forcing manual recovery. The existing `--goal-max-turns`/`goal_max_turns` knob bounds the OUTER goal-loop budget and does not feed the inner `agent.max_turns`. The fix mirrors the `goal_max_turns` plumbing end-to-end: a new nullable `max_iterations INTEGER` column on `tasks` (schema + additive migration), a `--max-iterations N` flag on `hermes kanban create`, surfacing in `hermes kanban show` (+ `--json`), and — in the worker-spawn env block in `hermes_cli/kanban_db.py` — `env["HERMES_MAX_ITERATIONS"] = str(task.max_iterations)` only when the card carries a value (a plain card leaves the env clean so the global default is preserved). NO new user-facing env var: `HERMES_MAX_ITERATIONS` already exists and is already honored in `cli.py`'s budget fallback chain (`config.yaml agent.max_turns` > `HERMES_MAX_ITERATIONS` > 90); this wires the existing internal env var from a per-task config column. Tests: DB persist/default/legacy-migration, spawn-env set-when-present / clean-when-absent, and a CLI behavior-contract test (flag → DB round-trip + `--json` surface) in `tests/hermes_cli/test_kanban_max_iterations.py`, plus an E2E against a temp `HERMES_HOME` exercising the real `hermes kanban create`/`show` path. **Retire trigger:** open the upstream PR, then auto-retire when it merges in a tagged release ≥ base; until then carry as upstream-pending. | upstream-pending | v2026.6.19 |
| fork PR (TBD — upstream PR not yet opened) | Never spawn a tool-less kanban worker, and bound the per-tick spawn burst. A dispatcher-spawned worker came up with ONLY the base `kanban_*` coordination tools (no web/shell/git/file) despite its profile declaring a full toolset, then self-blocked; root cause was a stuck→mass-spawn recovery tick that launched all ready cards at once, under which `_resolve_worker_cli_toolsets` came up degenerate (None/empty) and `_default_spawn` silently launched the worker WITHOUT a `--toolsets` pin (the `if worker_toolsets:` guard let it fall back to a kanban-only surface). Two layers, smallest-footprint first, covering the whole class via the single `_default_spawn` helper (shared by the ready and review dispatch paths): (1) `_default_spawn` now REQUIRES a non-empty resolved CLI toolset and raises `RuntimeError` when resolution is degenerate, so `dispatch_once`'s existing spawn-failure handler records the failure with `release_claim=True` and the card is reclaimed to `ready` for a clean retry instead of running crippled (`_resolve_worker_cli_toolsets` always recovers at least the kanban lifecycle surface for a real profile home, so None/empty is a genuine failure, not a legitimately tool-less profile); (2) a new `kanban.max_spawn_per_tick` config knob caps how many workers a single tick may launch (ready + review combined), distinct from `max_spawn` (a live concurrency cap), wired through the gateway dispatcher (`gateway/kanban_watchers.py`) and the CLI dispatch path (`hermes_cli/kanban.py`); unset (None) preserves historical unbounded behavior, invalid/<1 values normalize to None. NO new user-facing env var: the knob lives in `config.yaml` under `kanban.max_spawn_per_tick` (`hermes_cli/config.py` DEFAULT_CONFIG), per the `.env`-is-secrets-only rule. Behavior-contract tests against the real `_default_spawn`/`dispatch_once` with a temp `HERMES_HOME` (not mocks of the unit under test): a spawned worker's resolved toolset is pinned; a degenerate (None) and an empty-list resolution both raise and never `Popen`; N>cap ready cards spawn at most cap per tick; the per-tick cap also counts review spawns; and the gateway forwards `kanban.max_spawn_per_tick` to `dispatch_once`. Recovered from stranded fork PR #10 (origin commit `8fb892f1c`, authored Casey West) by cherry-pick onto current integration; the spawn-env block reconciled cleanly alongside the #19 max-iterations work. **Retire trigger:** open the upstream PR, then auto-retire when it merges in a tagged release ≥ base; until then carry as upstream-pending. | upstream-pending | v2026.6.19 |
12 changes: 12 additions & 0 deletions gateway/kanban_watchers.py
Original file line number Diff line number Diff line change
Expand Up @@ -736,6 +736,17 @@ async def _kanban_dispatcher_watcher(self) -> None:
if max_spawn is not None:
logger.info(f"kanban dispatcher: max_spawn={max_spawn}")

# Read max_spawn_per_tick — the per-tick burst bound (distinct from
# max_spawn, which is a live concurrency cap). Caps how many workers a
# single tick may launch so a stuck->recovery tick can't dump the whole
# ready queue at once (incident 2026-06-26). Invalid/<1 values are
# normalized to None (= unbounded) inside dispatch_once.
max_spawn_per_tick = kanban_cfg.get("max_spawn_per_tick", None)
if max_spawn_per_tick is not None:
logger.info(
f"kanban dispatcher: max_spawn_per_tick={max_spawn_per_tick}"
)

# Cap the number of simultaneously running tasks so slow workers
# (local LLMs, resource-constrained hosts) don't pile up and time
# out. When set, the dispatcher skips spawning when the board
Expand Down Expand Up @@ -932,6 +943,7 @@ def _tick_once_for_board(slug: str) -> "Optional[object]":
conn,
board=slug,
max_spawn=max_spawn,
max_spawn_per_tick=max_spawn_per_tick,
max_in_progress=max_in_progress,
failure_limit=failure_limit,
stale_timeout_seconds=stale_timeout_seconds,
Expand Down
9 changes: 9 additions & 0 deletions hermes_cli/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -2343,6 +2343,15 @@ def _ensure_hermes_home_managed(home: Path):
# otherwise saturate one profile's local model / API quota /
# browser pool while leaving other profiles idle.
"max_in_progress_per_profile": None,
# Per-tick spawn burst bound (incident 2026-06-26). Distinct from
# max_spawn (a live concurrency cap counting all running workers):
# this caps how many workers a SINGLE dispatcher tick may launch
# (ready + review combined). Prevents a stuck->recovery tick from
# dumping the whole ready queue at once — the burst that raced
# workers into spawning tool-less. Unset (None) means "no per-tick
# bound" (historical behavior). Invalid/<1 values are treated as
# None. Both caps apply together when set.
"max_spawn_per_tick": None,
# When true, the kanban dispatcher auto-runs the decomposer on
# tasks that land in Triage (every dispatcher tick). When false,
# decomposition is manual via `hermes kanban decompose <id>` or
Expand Down
6 changes: 6 additions & 0 deletions hermes_cli/kanban.py
Original file line number Diff line number Diff line change
Expand Up @@ -2149,17 +2149,23 @@ def _coerce_positive_int(value):
max_spawn = cli_max if cli_max is not None else _coerce_positive_int(
_kanban_cfg.get("max_spawn")
)
# Per-tick burst bound (distinct from max_spawn live-concurrency cap).
max_spawn_per_tick = _coerce_positive_int(
_kanban_cfg.get("max_spawn_per_tick")
)
except Exception:
default_assignee = None
max_in_progress_per_profile = None
max_in_progress = None
auto_route_review_bounce_enabled = True
max_spawn = getattr(args, "max", None)
max_spawn_per_tick = None
with kb.connect_closing() as conn:
res = kb.dispatch_once(
conn,
dry_run=args.dry_run,
max_spawn=max_spawn,
max_spawn_per_tick=max_spawn_per_tick,
max_in_progress=max_in_progress,
failure_limit=getattr(args, "failure_limit", kb.DEFAULT_SPAWN_FAILURE_LIMIT),
default_assignee=default_assignee,
Expand Down
49 changes: 47 additions & 2 deletions hermes_cli/kanban_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -6593,6 +6593,7 @@ def dispatch_once(
ttl_seconds: Optional[int] = None,
dry_run: bool = False,
max_spawn: Optional[int] = None,
max_spawn_per_tick: Optional[int] = None,
max_in_progress: Optional[int] = None,
failure_limit: int = DEFAULT_SPAWN_FAILURE_LIMIT,
stale_timeout_seconds: int = 0,
Expand Down Expand Up @@ -6625,6 +6626,15 @@ def dispatch_once(
a 60-second tick interval could grow concurrency by N every minute on a
busy board and accumulate without bound.

``max_spawn_per_tick`` is a **per-tick burst bound** (distinct from
``max_spawn``): it caps how many workers a single tick may launch,
counting ready + review spawns together. Defense-in-depth against the
stuck->mass-spawn recovery pattern (incident 2026-06-26: the dispatcher
sat stuck for 16 ticks, then spawned all 20 ready cards in ONE tick).
Dumping the whole ready queue at once is the condition under which workers
raced and came up tool-less. ``None`` (the default) preserves the
historical unbounded per-tick behavior; both caps apply when set.

``spawn_fn`` defaults to ``_default_spawn``. Tests pass a stub.
``board`` pins workspace/log/db resolution for this tick to a specific
board. When omitted, the current-board resolution chain is used.
Expand Down Expand Up @@ -6700,6 +6710,18 @@ def dispatch_once(
if max_spawn is None or max_spawn > remaining:
max_spawn = remaining
spawned = 0
# Per-tick burst bound (#incident-2026-06-26). When set, no single tick
# launches more than this many workers (ready + review combined). Normalize
# invalid/<1 values to None (= unbounded) so a typo in config can't wedge
# the dispatcher into never spawning.
_per_tick_cap: Optional[int] = None
if max_spawn_per_tick is not None:
try:
_candidate = int(max_spawn_per_tick)
except (TypeError, ValueError):
_candidate = 0
if _candidate >= 1:
_per_tick_cap = _candidate
# Per-profile concurrency cap (#21582): when set, track how many
# workers each assignee already has in flight, and refuse to spawn
# when this would push that assignee past the cap. Prevents
Expand Down Expand Up @@ -6737,6 +6759,8 @@ def dispatch_once(
# there, with the existing diagnostic.
_default_assignee_resolved = True
for row in ready_rows:
if _per_tick_cap is not None and spawned >= _per_tick_cap:
break
if max_spawn is not None and running_count + spawned >= max_spawn:
break
row_assignee = row["assignee"]
Expand Down Expand Up @@ -6922,6 +6946,8 @@ def dispatch_once(
"ORDER BY priority DESC, created_at ASC"
).fetchall()
for row in review_rows:
if _per_tick_cap is not None and spawned >= _per_tick_cap:
break
if max_spawn is not None and running_count + spawned >= max_spawn:
break
if not row["assignee"]:
Expand Down Expand Up @@ -7480,9 +7506,28 @@ def _default_spawn(
cmd.extend(["--skills", sk])
if task.model_override:
cmd.extend(["-m", task.model_override])
# Resolve the assignee profile's CLI toolset and pin it explicitly so the
# worker never falls back to a stale root/active-profile config. This MUST
# succeed: a worker spawned without its toolset comes up with only the
# base kanban_* coordination tools (no web/shell/file/git), can't do its
# job, and self-blocks — wasting a full LLM cycle. Under a stuck->mass-spawn
# burst (incident 2026-06-26) resolution can come up degenerate; rather
# than silently launching a crippled worker, FAIL the spawn so the caller
# (dispatch_once) records a spawn failure and RECLAIMS the card to ``ready``
# for a clean retry on the next tick. _resolve_worker_cli_toolsets always
# recovers at least the kanban lifecycle surface for a real profile home,
# so a None/empty result here is a genuine resolution failure, not a
# legitimately tool-less profile.
worker_toolsets = _resolve_worker_cli_toolsets(env.get("HERMES_HOME"))
if worker_toolsets:
cmd.extend(["--toolsets", ",".join(worker_toolsets)])
if not worker_toolsets:
raise RuntimeError(
f"kanban worker spawn aborted for task {task.id} (profile "
f"{profile_arg!r}): could not resolve a non-empty CLI toolset from "
f"HERMES_HOME={env.get('HERMES_HOME')!r}. Refusing to spawn a "
"tool-less worker (only kanban_* coordination tools); the card will "
"be reclaimed for a clean retry."
)
cmd.extend(["--toolsets", ",".join(worker_toolsets)])
cmd.extend([
"chat",
"-q", prompt,
Expand Down
Loading
Loading