Skip to content

🐛 fix(kanban): never spawn a tool-less worker; bound per-tick spawn burst - #20

Merged
cwest merged 2 commits into
cwest/integrationfrom
topic/toolless-spawn-guard-integration
Jun 29, 2026
Merged

🐛 fix(kanban): never spawn a tool-less worker; bound per-tick spawn burst#20
cwest merged 2 commits into
cwest/integrationfrom
topic/toolless-spawn-guard-integration

Conversation

@cwest

@cwest cwest commented Jun 29, 2026

Copy link
Copy Markdown
Owner

Summary

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 correctly
declaring a full toolset, then self-blocked. Root cause: under a
stuck→mass-spawn recovery, the dispatcher launched all ready cards in one tick;
_resolve_worker_cli_toolsets came up degenerate (None/empty) and
_default_spawn silently launched the worker without a --toolsets pin
(the old if worker_toolsets: guard), letting it fall back to a kanban-only
surface and waste a full LLM cycle.

The fix — two layers, smallest-footprint first

The single _default_spawn helper backs both the ready and review dispatch
paths, so fixing it once covers the whole class.

  1. Never spawn tool-less. _default_spawn now requires a non-empty
    resolved CLI toolset and raises when resolution is degenerate.
    dispatch_once's existing spawn-failure handler records the failure with
    release_claim=True, so 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 resolution failure — not a legitimately tool-less profile.

  2. Bound the per-tick spawn burst. New kanban.max_spawn_per_tick caps how
    many workers a single tick may launch (ready + review combined), distinct
    from max_spawn (a live concurrency cap). Prevents a stuck→recovery tick
    from dumping the whole ready queue at once — the condition under which
    workers raced into tool-less spawns. Wired through the gateway dispatcher and
    the CLI dispatch path; unset preserves historical unbounded behavior;
    invalid/<1 values normalize to None.

The new knob lives in config.yaml under kanban.max_spawn_per_tick
(DEFAULT_CONFIG) — no new HERMES_* env var.

Files

  • hermes_cli/kanban_db.py_default_spawn raise-on-degenerate guard;
    dispatch_once per-tick cap (applied to both the ready and review loops).
  • gateway/kanban_watchers.py — read kanban.max_spawn_per_tick, forward to
    dispatch_once.
  • hermes_cli/kanban.py — CLI dispatch path forwards the per-tick cap.
  • hermes_cli/config.pykanban.max_spawn_per_tick: None default.
  • Tests: test_kanban_worker_spawn_toolsets.py,
    test_kanban_core_functionality.py, test_kanban_db.py.
  • PATCHES.md — upstream-pending row.

Tests (behavior-contract, real path against temp HERMES_HOME)

  • A spawned worker's resolved toolset is pinned (--toolsets present).
  • A degenerate (None) and an empty-list resolution both raise and never
    Popen (no tool-less worker).
  • N>cap ready cards spawn at most cap per tick; the per-tick cap also counts
    review spawns.
  • The gateway forwards kanban.max_spawn_per_tick to dispatch_once.

RED→GREEN verified for both layers (reverting each production hunk turns the
matching tests red). Full dispatch/spawn/config/CLI/gateway-dispatch surface
green: 595 passed, 1 skipped, 0 regressions.

Notes

Recovered the original work by cherry-pick to preserve authorship (origin
commit 8fb892f1c); the spawn-env block reconciled cleanly alongside the
recent per-task max-iterations change.

cwest added 2 commits June 29, 2026 16:27
…urst

A dispatcher-spawned worker came up with ONLY the
base kanban_* coordination tools — no web/shell/git/file — despite its profile
correctly declaring a full toolset, then self-blocked ("only kanban_*
coordination tools"). Root cause: under a stuck->mass-spawn recovery the
dispatcher spawned all 20 ready cards in one tick; _resolve_worker_cli_toolsets
came up degenerate and _default_spawn silently launched the worker WITHOUT a
--toolsets pin (the `if worker_toolsets:` guard), letting it fall back to a
kanban-only surface and waste a full LLM cycle.

Two layers, smallest-footprint first, fixing the whole class (the single
_default_spawn helper covers both the ready and review dispatch call paths):

1. Never spawn tool-less. _default_spawn now REQUIRES a non-empty resolved CLI
   toolset and raises when resolution is degenerate. dispatch_once's existing
   spawn-failure handler records the failure with release_claim=True, so 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. Bound the per-tick spawn burst. New kanban.max_spawn_per_tick caps how many
   workers a single tick may launch (ready + review combined), distinct from
   max_spawn (a live concurrency cap). Prevents a stuck->recovery tick from
   dumping the whole ready queue at once — the condition under which workers
   raced into tool-less spawns. Wired through the gateway dispatcher and the
   CLI dispatch path; unset preserves historical unbounded behavior.

Behavior-contract tests: a spawned worker's resolved toolset is pinned and a
degenerate resolution reclaims the card (real _default_spawn against a temp
HERMES_HOME, not a mock); N>cap ready cards spawn at most cap per tick; and the
gateway forwards kanban.max_spawn_per_tick to dispatch_once.
Add the upstream-pending row for the dispatcher-safety fix recovered onto
integration: never spawn a tool-less kanban worker (guard the degenerate
toolset resolution) and bound the per-tick spawn burst via the new
kanban.max_spawn_per_tick config knob.

@cwest cwest left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code is right and the verification holds up. I reran the suite against a temp HERMES_HOME: the two spawn-guard tests, the reclaim-on-degenerate E2E, the three per-tick-cap tests, and the gateway config-propagation test all pass (7 passed). I also confirmed both layers RED to GREEN myself: reverting the _default_spawn raise turns the two guard tests red (Popen would run a tool-less worker), and removing the ready-loop per-tick break turns the burst-cap test red (5 spawn instead of 2). The reclaim path is real: _default_spawn's RuntimeError is caught in dispatch_once and routed through _record_spawn_failure(release_claim=True), so the card returns to ready rather than running crippled. The cap break sits before the claim/spawn and spawned only increments on success, so a reclaim doesn't burn cap budget. Whole-class coverage checks out: the single _default_spawn helper backs both dispatch paths and the cap guards both loops. mergeable, all checks green, no open threads.

One thing has to change before this lands. The recovered commit message body on the first commit (61db1de59) names a specific internal worker in its opening line. That detail shouldn't sit in a commit body on the remote. Reword that line to the generic form ("a dispatcher-spawned worker on the knowledge board came up with only the base coordination tools"), preserving the original authorship via --author and re-signing. The code stays byte-identical; only the message changes. Everything else is ready, so once that commit body is generalized this is good to go.

@cwest
cwest force-pushed the topic/toolless-spawn-guard-integration branch from b1409e2 to 6bca7c4 Compare June 29, 2026 21:12
@cwest
cwest marked this pull request as ready for review June 29, 2026 21:17

@cwest cwest left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No changes needed. The recovered dispatcher-safety fix holds up: the guard makes a degenerate toolset resolution raise rather than launch a worker on the kanban-only surface, and the per-tick cap keeps a recovery burst from emptying the ready queue in one go. The single helper covers both the ready and review spawn paths, so the whole class is closed, not just the reported site. The new behavior-contract tests run green against a temp HERMES_HOME, and the per-tick cap and gateway wiring are exercised end to end. mergeable, all required checks, and conversation resolution are clean. Ready to merge.

@cwest
cwest merged commit f1fc4b2 into cwest/integration Jun 29, 2026
8 checks passed
@cwest
cwest deleted the topic/toolless-spawn-guard-integration branch June 29, 2026 21:58
cwest added a commit that referenced this pull request Jul 1, 2026
_default_spawn now REQUIRES a non-empty resolved CLI toolset and raises when
resolution is degenerate, so the spawn-failure handler reclaims the card to
ready for a clean retry instead of running crippled. Adds a
kanban.max_spawn_per_tick config knob capping how many workers a single tick
may launch (ready + review combined), wired through the gateway dispatcher and
the CLI dispatch path; unset preserves historical unbounded behavior. No new
user-facing env var — the knob lives in config.yaml.

upstream-pending: fork PR #20
cwest added a commit that referenced this pull request Jul 26, 2026
_default_spawn now REQUIRES a non-empty resolved CLI toolset and raises when
resolution is degenerate, so the spawn-failure handler reclaims the card to
ready for a clean retry instead of running crippled. Adds a
kanban.max_spawn_per_tick config knob capping how many workers a single tick
may launch (ready + review combined), wired through the gateway dispatcher and
the CLI dispatch path; unset preserves historical unbounded behavior. No new
user-facing env var — the knob lives in config.yaml.

upstream-pending: fork PR #20
(cherry picked from commit 9ce3420)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant