diff --git a/PATCHES.md b/PATCHES.md index fec1b3383680f..7bec8a2513c5f 100644 --- a/PATCHES.md +++ b/PATCHES.md @@ -90,3 +90,30 @@ 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) | Stop the kanban `active_pr` respawn guard from re-arming on the SANCTIONED RECOVERY'S OWN unblock-cutoff comment, so a `ready` card that legitimately REUSES an open PR for a revision round (the OUTER feedback loop, heavy on WRITING cards) can spawn its author. Live symptom 2026-07-22 (card `t_11132357`, PR #143): after a correct `block` → `route_feedback_to_author` → `unblock_task` (which emits the `unblocked` cutoff event), `check_respawn_guard` (`hermes_cli/kanban_db.py`) STILL returned `active_pr` every tick and the dispatcher refused to spawn the author for hours. Root cause (distinct from #46204 and row 87 — a boundary bug those did not cover): the `active_pr` dup-PR scan clears only for PR-URL comments STRICTLY BEFORE `pr_cutoff = max(window, latest_unblock_ts)`, but the sanctioned recovery writes its OWN audit/steer comment that NAMES the existing PR URL AT THE SAME SECOND as the `unblocked` event it emits (verified: latest `unblocked` ts == latest pull/143 comment ts). Kanban timestamps are second-granular, so that recovery comment lands at `created_at == pr_cutoff` and the `>=` window scan re-counts it as a fresh active PR — the recovery's own cutoff-marking comment re-arms the guard it is trying to clear, and no operator move can win (any comment the recovery writes must reference the PR). The prior narrow carve-out excluded only the literal `dispatcher` author (its auto-route audit is the same class); it did not cover the homestead orchestrator/recovery actors that drive the OUTER loop — crucially the `orchestrator` author that `route_feedback_to_author` itself stamps on its outer-loop reopen audit comment. Fix (minimal, extends the existing exclusion — does NOT weaken genuine dup-PR protection): a new module-level `_RESPAWN_GUARD_RECOVERY_AUTHOR_PREFIXES = ("dispatcher", "hollis", "onecard", "orchestrator")` and the dup-PR comment scan excludes any comment whose author PREFIX-matches that set (`author NOT LIKE '%'`, so profile-suffixed variants like `dispatcher-default` are covered). None of these actors ever opens a PR, so excluding their at/after-cutoff PR-URL audit is exactly analogous to the review-status carve-out; a genuine builder/author PR-URL comment (e.g. `eckert`) at/after the cutoff is NOT in the set and STILL re-arms `active_pr` (the real duplicate-PR risk is preserved), and the `review`-status skip + closed/merged fail-open behavior are untouched. Single-file change in `hermes_cli/kanban_db.py` + 4 behavior-contract regression tests in `tests/hermes_cli/test_kanban_db.py` (RED-then-GREEN: recovery-actor PR-URL audit at the same-second cutoff → guard clears to None; prefixed recovery actor → clears; one test driven through `route_feedback_to_author` itself so it cannot drift from the real `orchestrator` emitter → clears; NON-recovery author PR-URL comment at/after cutoff → STILL `active_pr`). Real `kanban_db`, temp `HERMES_HOME`, no mocks; full `test_kanban_db.py` + the kanban surface green, 0 regressions. **Retire trigger:** never auto-retires on an upstream PR-merge signal (no upstream PR — fork-internal orchestrator recovery machinery; `hollis`/`onecard` are homestead actors, not upstream concepts); remove only if the fork's outer-feedback-loop recovery flow is retired. | permanent-local | main@2ba1e028e | | fork PR (TBD — no upstream PR; fork-local vision cost lever) | Add an optional image-resolution cost lever to the auxiliary vision path (`tools/vision_tools.py`). `vision_analyze` had ZERO image-resolution control: every image was sent at the provider default, so a coarse question ("what's in this picture?", "is the chart up?") cost the same as reading fine print. The Gemini/Vertex `generateContent` API exposes `generationConfig.mediaResolution`, which controls how many tokens an image is tiled into. Adds (1) a `config.yaml` setting `auxiliary.vision.media_resolution` ("" \| low \| medium \| high), defaulting to "" = unchanged behavior — no new `HERMES_*` env var, per the `.env`-is-secrets-only rule; (2) an optional `media_resolution` argument on `vision_analyze` (per-call override that beats the config value) — the ONLY new tool-schema surface, one optional enum arg, so existing callers are byte-for-byte unchanged; (3) provider-aware, fail-soft injection: three pure helpers (`_normalize_media_resolution` maps low/medium/high → the `MEDIA_RESOLUTION_*` enum and treats unset/default/unknown as a no-op; `_provider_model_is_gemini` gates on the RESOLVED vision provider+model; `_build_media_resolution_extra_body` emits the field ONLY on a Gemini/Vertex route and returns `{}` for every other provider — never errors, never sends an unknown field a strict provider would reject) wired into `vision_analyze_tool` after resolving the effective vision route via `resolve_vision_provider_client`. Wire shape is a FLAT `extra_body={"media_resolution": "MEDIA_RESOLUTION_LOW"}` — verified live to be the shape LiteLLM's Vertex adapter maps onto `generationConfig.mediaResolution` (nested `google`/`generationConfig` shapes are silently ignored by LiteLLM and 400 on direct Vertex). The native fast path (image bytes handed to the main model) is intentionally untouched — the knob is a parameter of the AUXILIARY vision call, which the native path does not make. **Measured live** (gemini-3.6-flash via the local LiteLLM→Vertex proxy, same 2400x1600 PNG, through the real `vision_analyze_tool` path): default → IMAGE tokens **1,080**; `low` → **260** (~4.15x fewer, correct answer); `high` → **1,080** (== default, so the win is only in the LOW direction — no gain from raising it). Tests: `tests/tools/test_vision_media_resolution.py` (28 — value normalization, provider-aware `{}` vs field, config+override precedence, fail-soft; plus 5 E2E cases through the real `vision_analyze_tool` asserting default sends NO extra_body, low/high inject the flat field on a Gemini route, a non-Gemini route is a no-op, and a provider-resolution failure does not break the call). Full vision + config surface green (315), 0 regressions. Clean upstream candidate — an image-resolution cost lever is generally useful — tracked upstream-pending. **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 | main@2ba1e028e | | fork PR (TBD — upstream PR not yet opened; clean upstream candidate) | Make the Discord adapter's `send` (`plugins/platforms/discord/adapter.py`) return a documented `SendResult(success=False, ...)` for a malformed (non-numeric) `thread_id` or `chat_id` instead of letting `int()` raise `ValueError` into the catch-all handler. Two failure modes in `send` had two different contracts: a well-formed-but-missing thread/channel returned a clean `SendResult` (`Thread not found` / `Channel not found`), but a MALFORMED id raised out of `int(thread_id)` / `int(chat_id)` and was only caught by the outer `except Exception`, which logs `Failed to send Discord message` at ERROR **with a full traceback** and stamps a generic failure. Live symptom 2026-07-26: a notify subscription carrying `thread_id='NEW'` (a non-numeric id that can never become valid) drove the kanban notifier's exponential backoff (16s, 32s, 64s, …) forever, logging 18 tracebacks in ~90s before the bad subscription was removed by hand. Fix: wrap each `int()` coercion in `try/except (TypeError, ValueError)` and return `SendResult(success=False, error=f"Invalid thread_id {thread_id!r}: not a numeric Discord ID")` (and the `chat_id` sibling), naming the offending value so the bad subscription is identifiable from the log alone, failing via the documented contract (no traceback spam) rather than by exception. Well-formed-but-missing behavior is byte-for-byte unchanged. Single-file change + 3 regression tests in `tests/gateway/test_discord_send.py` (malformed `thread_id` and `chat_id` each return a failed result naming the value, do NOT reach the real channel lookup, and do NOT emit the catch-all `Failed to send Discord message` ERROR; a well-formed-but-missing `thread_id` still returns `Thread not found`). Full discord test surface green (612 tests across 44 files). NOT-carried by design: the card's optional "notifier drops a permanently-unfixable subscription after N failures" idea is out of scope for this adapter fix and left as separate follow-up. Clean upstream candidate (upstream `adapter.py` carries the identical defect). **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 | main@2ba1e028e | +||||||| parent of b0b146806f (🐛 fix(kanban): treat a phantom/deleted PR URL as not_found so it never wedges active_pr) +| [#44023](https://github.com/NousResearch/hermes-agent/pull/44023) | Make `VoiceMixer` a real `discord.AudioSource` subclass so Discord voice playback can consume it directly (adds the `discord` import and changes the class base in `plugins/platforms/discord/voice_mixer.py`). Companion test commit gives the mocked `discord.AudioSource` in the gateway conftest a real base class so the 19 `TestVoiceMixerCore` tests run without the real `discord` package (they skip when `numpy` from the `voice` extra is absent). PR **OPEN** upstream. | upstream-pending | main@9be292f1e | +| [#44338](https://github.com/NousResearch/hermes-agent/pull/44338) | Fix kanban notifier SendResult non-delivery: treat `SendResult(success=False)` from `adapter.send` as a delivery failure (not a delivered ping), keep the subscription alive on send failure, rewind the pre-send claim so the terminal blocked/completed event is retried, and back off per-subscription (exponential, capped at 1h) so a dead chat is not hammered every tick. Re-ported by hand into `GatewayKanbanWatchersMixin._kanban_notifier_watcher` in `gateway/kanban_watchers.py` — upstream's god-file Phase 3 refactor extracted the notifier loop out of `gateway/run.py` (its v2026.6.5 home) into that mixin, so the carry now targets the loop's new location. Carries only the SendResult-non-delivery fix (upstream commits `b6fe83ffa`, `45459de8c`); the PR's later child-event escalation work (`cc328e2e6`, `2021d6bc4`) is deliberately not carried. **Retire trigger (behavior-keyed, NOT PR-merge-keyed):** #44338 was **closed administratively** (fork CI gating), not rejected, so the default "auto-retire when the PR merges" rule can never fire and must not be used for this row. Retire this row **only when upstream `gateway/kanban_watchers.py` implements ALL of: (i) `SendResult(success=False)` failure-detection AND (ii) keep-subscription-alive-on-permanent-failure AND (iii) bounded exponential backoff.** Watch #45940 and #46443, but do **NOT** drop on #45940 merge alone — #45940 only adds detection (i) and would regress behaviors (ii) keep-alive and (iv) backoff if this carry were dropped on its merge. Verify all three behaviors are present in the base tag's `gateway/kanban_watchers.py` before retiring. Full rationale: `docs/patches/2026-06-18-pr44338-partial-carry.md`. | upstream-pending | main@9be292f1e | +| [#46549](https://github.com/NousResearch/hermes-agent/pull/46549) | Fix the kanban dispatcher wedging review tasks out of respawn: `check_respawn_guard` applied the `recent_success` and `active_pr` guards — which exist only to stop a *builder* re-opening a duplicate PR — to review-lane spawns too, so a card in `status='review'` (whose build run already completed and left a PR-URL comment) got blocked from spawning its reviewer for up to 24h. The fix reads `tasks.status` and skips those two guards for `status='review'` while keeping `rate_limit_cooldown` and `blocker_auth` active. Carried whole and unmodified via `git cherry-pick -x` (upstream commit `32502fb01`, author `demi`) — clean apply, no hand-port; our base already has the `check_respawn_guard` shape the PR targets. Auto-retires when #46549 merges upstream. Note: the bypass keys on `status='review'`, the canonical review-lane state (23/24 historical review cards flow through it); a one-off review card hand-parked in `status='ready'` is out of scope by design — it is staged into `review` instead of widening this patch. Full rationale: `docs/patches/2026-06-18-pr46549-review-respawn-guard.md`. PR **OPEN** upstream; auto-retires when #46549 merges. | upstream-pending | main@9be292f1e | +| [#46204](https://github.com/NousResearch/hermes-agent/pull/46204) | Make the kanban `active_pr` respawn guard honor an explicit unblock so the review→author **rework** loop can spawn without waiting out the 24h PR window. The guard fires when a GitHub PR URL appears in a recent comment; when a reviewer bounces a card back to the author to fix the **same** PR, the card lands in `status='ready'` still carrying the build-handoff PR-URL comment, so `active_pr` wedged it out of spawning for up to 24h (live symptom on card `t_2c6bd5f2` / PR #68). The fix uses the latest `unblocked` task event as an additional lower bound on the PR-comment scan window (`pr_cutoff = max(window, latest_unblock_ts)`): PR URLs posted *before* a deliberate unblock no longer veto the respawn, while URLs at/after still guard (duplicate-PR protection intact); same-second comments stay guarded conservatively. **Additive over #46549, not redundant:** #46549 bypasses the guard for `status='review'` only — the author-rework card is in `ready`, which #46549 explicitly does not cover. **Adapted (not a clean cherry-pick):** our base already carries #46549's `if not is_review:` wrapper around the `active_pr` block, so the cutoff was hand-applied inside that block (logic byte-equivalent to upstream; placement differs to compose with #46549). Carries the matching docstring + `website/docs/user-guide/features/kanban.md` updates. Author `dannyfranca` credited. 3 new invariant tests + full `test_kanban_db.py` (226) green. Full rationale: `docs/patches/2026-06-28-pr46204-active-pr-unblock-cutoff.md`. PR **OPEN** upstream; auto-retires when #46204 merges (the upstream version supersedes the hand-applied cutoff on rebase). | upstream-pending | main@9be292f1e | +| fork PR [cwest#4](https://github.com/cwest/hermes-agent/pull/4) (no upstream PR yet) | Run the in-process kanban dispatcher on a dedicated single-thread executor so a busy default `ThreadPoolExecutor` (saturated by agent turns + their nested tool/sub-agent/vision/compression fan-out) can never starve the dispatcher tick. Routes every dispatcher offload (zombie reaper, auto-decompose, `_tick_once`, `_ready_nonempty`) through the private executor; `dispatch_interval_seconds` and the bad-tick health window are untouched. Re-applied by hand into `GatewayKanbanWatchersMixin` in `gateway/kanban_watchers.py` (upstream's god-file Phase 3 refactor moved the dispatcher loop there from `gateway/run.py`); preserves upstream's `_release_singleton_lock` in both the CancelledError and normal-exit paths. Ships a starvation regression test (`test_gateway_dispatcher_tick_not_starved_by_busy_default_executor`) that fails on the old shared-pool behaviour. Cherry-picked from `0b97e99e8`. **Retire trigger:** fork PR #4 is OPEN against `main` and has **no upstream NousResearch PR number yet** — so the default PR-merge rule cannot key on an upstream PR. Retire only when the dedicated-executor fix (or an equivalent that keeps the in-process dispatcher off the shared default pool) is present in the base tag's `gateway/kanban_watchers.py`. | upstream-pending | main@9be292f1e | +| fork PR [cwest#6](https://github.com/cwest/hermes-agent/pull/6) (fork-local, MERGED to `cwest/integration`) | Count `skills.external_dirs`-granted skills in the profiles dashboard and `hermes profile list` so the count matches `hermes skills list` exactly. `_count_skills` previously only `rglob`-ed `/skills` (no symlink following, no `external_dirs`), so fork profiles that source all skills from the shared `~/.hermes/skills` registry showed "Skills: 0". Extracts `_scan_skill_dirs(local, external)`, adds `count_profile_skills` + `get_external_skills_dirs_for`, and makes `_count_skills` delegate to that single source of truth (symlink-following + frontmatter-name dedup). Also adds `casey@geeknest.com → cwest` to the `scripts/release.py` author-map (re-appended at the dict tail after the rebase conflict, all upstream author entries preserved). Tests: `TestCountProfileSkills` (external-only profile, symlinked packages, local-precedence dedup, missing-config). Fork-specific behavior (the fork's `external_dirs` grant) — kept indefinitely; not sent upstream. | permanent-local | main@9be292f1e | +| fork-local (no PR; closed fork PR [cwest#2](https://github.com/cwest/hermes-agent/pull/2)) | Optional per-route action allow-list filter for webhook routes (`feat(webhook)`), a local feature for the fork's webhook deployment. Kept indefinitely as a deliberate local divergence. | permanent-local | main@9be292f1e | +| fork-local (CI infra) | Daily upstream-sync GitHub Actions workflow that fetches/rebases the patch queue onto the newest upstream tag, runs tests, and opens a review PR (`.github/workflows/fork-daily-sync.yml`). Fork-only release engineering; never sent upstream. | permanent-local | main@9be292f1e | +| fork-local (CI infra) | Secret-scan gate (gitleaks) that runs on every PR/push to `cwest/integration`, enforcing the fork's "no secrets, ever" invariant on our own commits (`.github/workflows/fork-secret-scan.yml`). Fork-only release engineering; never sent upstream. | permanent-local | main@9be292f1e | +| fork-local (this manifest) | `PATCHES.md` itself — the manifest of everything the fork carries on top of its upstream base tag, plus the bucket model, the auto-retire rule, and the per-row behavior-keyed override. Kept indefinitely. | permanent-local | main@9be292f1e | +| fork PR (TBD — upstream PR not yet opened) | Make the kanban crash-detector clear a dead worker's stale claim regardless of lane, so a worker that dies while its card sits in a NON-`running` lane (most commonly `review`, after the implementer opened a PR and the card moved on) no longer wedges that lane for the full 1h stale-claim TTL. `detect_crashed_workers` scanned only `WHERE t.status = 'running'`, so a card already in `review` carrying a dead `worker_pid` + `claim_lock` was invisible to the fast reaper; the review-column dispatch query gates on `claim_lock IS NULL`, so the next worker (the reviewer) could not spawn until `release_stale_claims`' TTL eventually freed it (observed live twice on PR #70 rework rounds; card `t_a29b93c2` had to be hand-cleared). The fix widens the scan to any card with a non-NULL `worker_pid` (status added to the projection) and, after the existing host-local + launch-grace + dead-PID checks pass, branches on status: a non-`running` card gets an **in-place claim clear** (`claim_lock`/`claim_expires`/`worker_pid` = NULL) with a `stale_claim_cleared` event and NO lane change (a dead worker in `review` stays in `review` so the reviewer re-spawns; it is not yanked to `ready`), and NO run open/close, crash event, or failure-counter/breaker tick (those are `running`-crash semantics). The `running` path is byte-for-byte unchanged — its UPDATE stays guarded `status = 'running'` and only fires for `running` cards, since non-`running` ones `continue` out above. Launch-grace (measured from the active `task_runs` row) and the host-local claim check still apply, so a freshly-spawned worker is not reaped mid-init. Single-file change in `hermes_cli/kanban_db.py` + 4 regression tests in `tests/hermes_cli/test_kanban_db.py` (dead worker in `review` → claim cleared, status stays `review`, no failure count; review dispatch then permitted; within-grace claim NOT cleared; foreign-host claim untouched). Clean upstream candidate — a reaper that ignores non-running lanes is a general dispatcher bug, not homestead-specific. **Retire trigger:** carried as upstream-pending with NO upstream PR (deliberate — we do not surface this to NousResearch). Auto-retires if/when an equivalent lane-agnostic stale-claim clear appears in the base tag's `detect_crashed_workers` independently (the rebase will drop or empty the hunk); otherwise it stays as a carried fix. | upstream-pending | main@9be292f1e | +| fork PR (TBD — upstream PR not yet opened) | Fix the kanban crash-detector reaping a re-claimed worker mid-init. `detect_crashed_workers` grants a freshly-spawned worker a launch-window grace (default 30s, `HERMES_KANBAN_CRASH_GRACE_SECONDS`) so its PID can become visible on `/proc` before liveness is checked — but it measured the grace from `tasks.started_at`, which is pinned to the task's first-ever start (`COALESCE` on every claim) and never refreshed on re-claim. A card re-claimed for its next lane (implement → review) therefore inherited a stale `started_at`, the grace had long since expired, and the new worker was reaped as `crashed (pid not alive)` before it finished initializing (plugin load); with `failure_limit=2` the card then landed in `blocked`/`gave_up` without the work ever being attempted. The fix measures the grace from the active `task_runs` row via `COALESCE(r.started_at, t.started_at)` joined on `current_run_id` — the **exact per-attempt pattern `enforce_max_runtime` already uses** (see `test_max_runtime_uses_current_run_start_after_retry`), so retries/re-claims get a fresh window; falls back to `tasks.started_at` when no run row is present (preserves first-claim behavior). Single-file change in `hermes_cli/kanban_db.py` + regression test `test_detect_crashed_workers_grace_uses_current_run_start_after_reclaim`. Proven live: under the fix the auto-dispatcher spawned a worker on a card with a 3600s-stale `started_at` and it reached its agent loop and completed (run 14, 12s) where pre-fix runs crashed mid-init. **Retire trigger:** open the upstream PR (phase 2), then auto-retire when it merges in a tagged release ≥ base; until then carry as upstream-pending. | upstream-pending | main@9be292f1e | +| fork-local (no upstream PR) | Homestead-specific expansion of the `skills/github/github-code-review` skill for the kanban PR-review pipeline: the automated/non-interactive (webhook) review context, post-once idempotency guards (head-SHA dedup, blocked-timeout-is-unknown-not-failed, write-payload-to-file-before-POST), the humanizer/de-claude gate, and four reference files (`responding-to-and-resolving-review-threads` — the AUTHOR-resolves-threads loop incl. the Casey-2026-06-24 author-vs-reviewer decision; `webhook-triggered-reviews`; `consolidating-duplicate-reviews`; `editing-a-posted-review`) plus `scripts/commentable_lines.py`. This is the durable git home for material that previously lived ONLY in the `~/.hermes` deploy (silent drift). Deliberate divergence from upstream's generic review skill — homestead/kanban-specific, never sent upstream. **Retire trigger:** never auto-retires (permanent-local); remove only if the homestead PR-review pipeline is retired. | permanent-local | main@9be292f1e | +| 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 | main@9be292f1e | +| 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 | main@9be292f1e | +| 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 on the current base. **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 | main@9be292f1e | +| fork PR (TBD — no upstream PR; fork-internal event-driven orchestration) | Optional, default-OFF bridge that lets a kanban lifecycle transition wake the orchestrator as an agent RUN (not merely a chat ping), by POSTing the transition to a loopback webhook route — mirroring how a GitHub `pull_request` event triggers a review run. The native notifier loop (`GatewayKanbanWatchersMixin._kanban_notifier_watcher`) already delivers a terminal kanban event as a *message* to subscribed sources (ping a chat when a card reaches acceptance/done); it does NOT trigger reasoning. New module `gateway/kanban_transition_emit.py` adds pure, unit-testable decision logic (`should_emit_transition`, `build_transition_payload` with a stable `(board,task_id,kind,event_id)` idempotency key) plus a tiny fail-safe coroutine `emit_transition` that GitHub-style HMAC-signs (`X-Hub-Signature-256: sha256=`, matching the webhook adapter's validator) and POSTs to `http://127.0.0.1:8644/webhooks/kanban-transition`; it NEVER raises (a bridge failure must not break the notifier tick — the chat ping already happened). `gateway/kanban_watchers.py` gains a minimal additive call AFTER the existing chat-ping delivery, guarded by `kanban.transition_emit.enabled` (default OFF) and a bridged internal secret `HERMES_KANBAN_TRANSITION_SECRET` (read at gateway cold boot): when disabled the notifier path is byte-for-byte unchanged. NO new core tool, NO new model surface (reuses the existing webhook adapter + a route + a skill), NO user-facing `HERMES_*` config var (the flag lives in `config.yaml` under `kanban.transition_emit`; the env var is a secret bridge only). The route + its `kanban-transition-orchestrate` skill + the `config.yaml` flag live in `cwest/hermes-config` (the homestead config repo); this fork carries only the gateway-side emitter + the flag read. Tests: `tests/gateway/test_kanban_transition_emit.py` (decision logic, payload/idempotency, HMAC signature, fail-safe POST) + `tests/gateway/test_kanban_transition_emit_http.py` (real aiohttp loopback round-trip, 2xx/non-2xx, signature header). Restart-gated only for the internal secret env var; the `kanban.transition_emit` config (`enabled`/`emit_kinds`) is RE-READ per notifier tick (see the per-tick `_resolve_transition_cfg()` in `_kanban_notifier_watcher`) so widening/narrowing the wake gate hot-loads on the next tick with no gateway restart. Cache-/alternation-safe (no system-prompt or message-history mutation; the agent run is a fresh scoped session, exactly like the github-prs route). **Retire trigger:** never auto-retires on an upstream PR-merge signal (no upstream PR — fork-internal orchestration machinery); remove only if the homestead event-driven orchestration model is retired. | permanent-local | main@9be292f1e | +| fork PR (TBD — no upstream PR; fork-internal event-driven orchestration) | Make a kanban-transition WAKE resume the orchestrator as a DISTINCT turn even when the origin session is mid-turn (companion to the transition-emit bridge + origin-routing rows above). Root cause (verified live 2026-07-01): the origin-routed wake arrives at the adapter as a plain TEXT `MessageEvent`, so when the origin session is active `handle_message` (`gateway/platforms/base.py`) drops it into the busy branch, where it is either text-debounced (`_queue_text_debounce`) or newline-merged into the in-progress/queued turn via `merge_pending_message_event(merge_text=True)` — the wake is absorbed silently instead of producing its own identifiable turn (the busy-session swallow). Fix: the webhook route detects the wake by the emitter-stamped `origin_*` fields (`_is_transition_wake_payload`) and tags the inbound `MessageEvent` with `metadata["kanban_transition_wake"]=True`. A pure predicate `is_transition_wake_event` reads that flag; a wake-precedence branch in `merge_pending_message_event` keeps the wake in the single pending slot UN-merged and UN-clobbered (a pending wake is never overwritten by a later non-wake, and an incoming wake replaces a pending non-wake intact — a dropped autonomy wake is worse than a user follow-up the user can resend), and the busy branch in `handle_message` queues the wake without debounce/merge so the existing in-band drain cascade runs it as a DISTINCT turn. Cache-/alternation-safe: no system-prompt or message-history mutation, no new drain site, no synthetic mid-loop user message, `_active_sessions` lifecycle untouched — the wake is delivered as an ordinary next-turn user message via the same cascade every follow-up uses. SCOPE: distinct-turn delivery only; the self-announcing wake banner and latency tightening are separate follow-up work. Files: `gateway/platforms/base.py` (`is_transition_wake_event`, `merge_pending_message_event` wake-precedence, busy-branch bypass), `gateway/platforms/webhook.py` (`_is_transition_wake_payload` + wake tagging) + tests (`tests/gateway/test_transition_wake_busy_session.py`, one added case in `tests/gateway/test_webhook_origin_routing.py`). **Retire trigger:** never auto-retires on an upstream PR-merge signal (no upstream PR — fork-internal orchestration machinery); remove only if the homestead event-driven orchestration model is retired. | permanent-local | main@9be292f1e | +| fork PR (TBD — no upstream PR; fork-internal review-lane semantics) | Stop the kanban block-loop breaker from false-tripping on a healthy multi-round review cycle. The loop breaker in `block_task` (`hermes_cli/kanban_db.py`) increments `block_recurrences` whenever a task is re-blocked for the SAME `block_kind` after an unblock, routing to `triage` at `BLOCK_RECURRENCE_LIMIT` (2) to break a cron-unblock↔worker-re-block loop. But a legitimate review cycle (`review-changes-requested` → rework → a NEW, different finding → `review-changes-requested` again) is blocked with the same kind every round, so the second healthy round hit the limit and routed the card to `triage`/`needs_input` — taking it OFF the `auto_route_review_bounce` path (which scans `WHERE status='blocked'`), stranding it with no worker (observed live twice, 2026-07-02 and 2026-07-05 on card `t_0d57d36d`; required manual block→unblock rescue both times). The classifier — not the budget — was the bug (raising the limit only delays the false-trip). Fix: keying on the reviewer's `review-changes-requested` bounce reason, a review re-block whose normalized finding text MATERIALLY DIFFERS from the prior round's `blocked` event reason is treated as a fresh cause (`same_cause=False`, counter resets to 1) — a distinct finding is progress, not a loop. A review bounce repeating the IDENTICAL finding still counts (a reviewer bouncing the same unfixed finding forever IS a real loop worth escalating). Two small helpers added next to the existing `_is_review_bounce_reason`: `_last_block_reason` (most-recent `blocked` event reason, NOT gated on stickiness — the prior round was already unblocked, so `_latest_sticky_block_reason` returns None) and `_review_bounce_finding` (strips the prefix + separator, collapses whitespace, lowercases so trivial jitter is not a new finding). Non-review `needs_input`/`capability`/un-typed blocks are entirely unaffected (the reset is gated on `_is_review_bounce_reason(reason)`), so the breaker's real purpose is preserved. Single-file change in `hermes_cli/kanban_db.py` + 6 regression tests in `tests/hermes_cli/test_kanban_block_kinds.py` (distinct-finding round stays blocked; 3-round distinct cycle never triages; distinct-then-same escalates; same-finding repeat still triages; whitespace/case jitter is not a material change; non-review same-kind loop still trips). Upstream has no equivalent (no `block_recurrences`/`review-changes-requested`/typed-block concept in upstream `hermes_cli/*.py`) — cwest-team review-loop tooling. **Retire trigger:** never auto-retires (permanent-local); remove only if the fork's typed-block loop-breaker / review-lane dispatch is retired. | permanent-local | main@9be292f1e | +| fork PR (TBD — no upstream PR; fork-internal review-lane semantics) | Provide a sanctioned recovery for a card whose `block_recurrences` loop counter is ALREADY inflated (follow-up to the block-loop-breaker row above). The breaker resets `block_recurrences` ONLY inside `complete_task`, so a card whose counter was inflated by prior buggy-code runs or an operator's repeated block→unblock while diagnosing had NO sanctioned way back to the normal flow — every `block`→`unblock` re-tripped straight to `triage` (live symptom 2026-07-05 on card `t_0d57d36d`, counter stuck at 5; the only escape was the non-obvious `move_card`-to-`blocked`-then-`unblock` workaround that bypasses `block_task`'s counter logic). Two sanctioned paths, both in `hermes_cli/kanban_db.py` + CLI, both keyed on the SAME PR #48 `_review_bounce_finding` classifier so the breaker is never weakened: (1) new core API `reset_block_recurrences(conn, id, *, actor, reason)` zeroes the counter and emits a `block_recurrences_reset` audit event (does NOT touch status/block_kind/claim state — a pure counter reset), surfaced as `hermes kanban unblock --reset-loop ` which resets FIRST (so a card already escalated to `triage`, where `unblock_task` does not apply, is still recovered) then attempts the normal unblock; (2) `auto_route_review_bounce` now resets the counter to 0 when a genuine author-rework transition routes the card back — i.e. when the current bounce's finding MATERIALLY DIFFERS from the prior round's (a fresh cycle), while a bounce repeating the IDENTICAL finding LEAVES the counter intact so a real same-unfixed-finding loop still escalates to triage. New helper `_prior_block_reason` returns the SECOND-most-recent `blocked` event reason (the router reads a card already re-blocked, so the newest `blocked` event is the CURRENT round; the prior round is the one before it — distinct from `block_task`'s use of `_last_block_reason`, which fires BEFORE the new block event is written). A plain `unblock` (no `--reset-loop`) is byte-for-byte unchanged: the counter deliberately survives an ordinary unblock (the existing anti-amnesia design). Tests: `tests/hermes_cli/test_kanban_reset_block_loop.py` (10 — reset zeroes + audits; missing-task no-op; already-zero still audits; inflated card recovers to `blocked` not `triage` after reset; same-finding loop STILL escalates; auto-route resets on different finding / preserves on same; CLI `--reset-loop` recovers a blocked card, recovers a triage card, and plain unblock leaves the counter). Real `kanban_db`, temp board, no mocks. **Retire trigger:** open the upstream PR (phase 2), then auto-retire when it merges in a tagged release ≥ base; until then carry as upstream-pending, dropped only when the fork's block-loop-breaker / review-lane dispatch is retired. | permanent-local | main@9be292f1e | +| fork PR (TBD — no upstream PR; fork-internal dispatcher semantics) | Fix the two SIBLING crash-classification paths #47 missed, so the kanban dispatcher stops false-flagging cleanly-finished and transiently-failed workers as `crashed`/`gave_up`. #47 (`426a8a9c7`) carved out the provably-done case for a reap-registry `clean_exit` (rc=0 captured), but two same-class paths in `detect_crashed_workers` (`hermes_cli/kanban_db.py`) still counted a false failure: **(A) transient endpoint-unreachable failures** — a run that died on `APIConnectionError` / connection-refused / provider 5xx (classified `timeout`/`overloaded`/`server_error`/`upstream_rate_limit` after in-process retries) exited plain `1`, which the reaper read as a real crash toward `failure_limit`; a proxy blip thus `gave_up` a card that would otherwise succeed. Fix: new `KANBAN_TRANSIENT_EXIT_CODE` (69, EX_UNAVAILABLE) sentinel + a single-source-of-truth `kanban_worker_exit_code(failure_reason)` helper the CLI kanban-worker exit path delegates to (replacing the inline rate-limit-only mapping); `_classify_worker_exit` maps 69 → a new `transient` kind; `detect_crashed_workers` treats it exactly like the rate-limit carve-out (requeue to `ready`/`review`, NO failure counted, distinct `transient` run outcome + event, stamped `last_failure_error` so the respawn-guard cooldown spaces the retry), surfaced via `DispatchResult.transient` + the `_last_transient` side-channel; `check_respawn_guard`'s cooldown gate now recognizes both `rate_limited` and `transient` outcomes. **(B) clean exit misread as a crash on the `pid not alive`/`unknown` reap path** — #47's provably-done carve-out was gated on `kind == "clean_exit"` only, so a worker that finished its lane cleanly (draft-PR handoff / edit-in-place card) but whose exit was NOT captured in the reap registry (reaped by init, or gone between the reap tick and the liveness check → `_classify_worker_exit` returns `unknown`) fell into the generic `crashed` branch and counted a failure. Fix: extend the `_lane_work_provably_done` carve-out from `kind == "clean_exit"` to `kind in ("clean_exit", "unknown")`, using the SAME durable proof signals (recent_success / active_pr) #47 trusts. Non-regression preserved: a genuine mid-work death with NO proof still `crashed`→`gave_up` after the limit, and a real non-zero crash (not the sentinel) still counts. Two-file change (`hermes_cli/kanban_db.py` + `cli.py`) + 8 behavior-contract tests in `tests/hermes_cli/test_kanban_db.py` (transient-sentinel classify; transient requeue-without-failure across 6 hits; transient cooldown defer/allow; unknown-exit-after-completed-run and unknown-exit-after-draft-PR not counted; unknown-exit-without-proof still crashes; real-crash non-regression; exit-code mapping). **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 | main@9be292f1e | +| fork PR (TBD — no upstream PR; fork-internal review-lane semantics) | Make `unblock_task` also transition a card out of `triage`, so the OUTER feedback loop (Casey-feedback -> author on a card carrying an open PR) self-heals the `active_pr` respawn-guard wedge that only the INNER review-bounce loop was covered for (composes with rows 84/85 -- the block-loop breaker + reset-recovery -- extending them to the non-review author-bounce case). Live symptom 2026-07-11: a writing card (`t_f6126dcd`, author orwell) carrying an OPEN PR needed to bounce back to its author for a Casey-feedback revision; the hand-move `blocked->todo`+author did not clear the `active_pr` guard (a raw status flip emits no `unblocked` cutoff event), so `hermes kanban dispatch` refused to spawn the author every tick (`respawn_guarded {active_pr}`). Repeated churn inflated `block_recurrences` past `BLOCK_RECURRENCE_LIMIT`, escalating the card to `triage`, where the standard block->unblock cutoff SILENTLY no-oped -- `unblock_task` (`hermes_cli/kanban_db.py`) matched only `status IN ('blocked','scheduled')`, so a triage card matched zero rows, returned False, emitted no `unblocked` event, and the guard stayed tripped forever (only a hand `reset_block_recurrences` + status=blocked + `unblock_task` rescued it). Root cause: the INNER review loop self-heals via `auto_route_review_bounce` (reassigns + `unblock_task`, emitting the `unblocked` cutoff that `check_respawn_guard` honors), but the outer feedback loop has no `review-changes-requested` reason so it is never routed, and the only skill-layer primitive (`onecard_common.move_card`) is a raw status flip that emits no cutoff. Fix (minimal, well-scoped): add `'triage'` to both `WHERE status IN (...)` clauses in `unblock_task` -- the stale-run-pointer SELECT and the status UPDATE -- so a card escalated to `triage` transitions back to `ready`/`todo` (parent-gate re-checked) and emits the `unblocked` cutoff, clearing `active_pr` the same way a normal block->unblock does. Preserves the loop breaker: `block_recurrences` is deliberately NOT reset (counter still survives the unblock; reset only on completion or the explicit `reset_block_recurrences` API), so a genuine same-finding loop still escalates. This supersedes row 85's parenthetical about a card already escalated to `triage` where `unblock_task` did not apply -- with this patch `unblock --reset-loop` fully recovers a triaged card (reset + lane flip). Single-file change in `hermes_cli/kanban_db.py` (+ stale-comment sync in `hermes_cli/kanban.py`) + 5 behavior-contract regression tests in `tests/hermes_cli/test_kanban_reset_block_loop.py` (RED-then-GREEN reproducing the exact triage-with-open-PR wedge; parent-gate re-check; counter preserved; the pre-fix wedge documented). Clean upstream candidate -- a sanctioned unblock that no-ops from triage is a real bug -- but tracked fork-internal for now. **Retire trigger:** open the upstream PR, then auto-retire when it merges in a tagged release >= base; until then carry as permanent-local. | permanent-local | main@9be292f1e | +| fork PR (TBD — no upstream PR; fork-internal completion-lane semantics) | Add a **required-artifact completion guard** to `complete_task` (`hermes_cli/kanban_db.py`) — the third false-`done` guard, beside the phantom-`created_cards` and acceptance-lane guards, in the same before-write-txn shape. Root cause (live 2026-07-08, card `t_e3e5bc16`): a worker can build its deliverable in a git worktree but exit WITHOUT committing / pushing / opening a PR, and its `kanban_complete` still flips the card to `done`; that card wrote three research files into its worktree, opened no PR, completed anyway, and its gated children auto-promoted onto a foundation that did not exist (one began running before it was caught). `done` MUST mean "the declared reviewable artifact exists"; a `done` card with no PR is a false state, and no guard covered "the card's own contract required a PR and none exists." Fix (opt-in per card, derived from the workspace — zero schema footprint, no `requires_pr` column, no migration, existing cards classified correctly with no backfill): a new `_card_requires_pr(workspace_kind, workspace_path)` returns True only for `workspace_kind == 'worktree'` (the sole kind that materializes an isolated linked git worktree on a `topic/*` branch via `_ensure_git_worktree` — precisely the implementer→PR shape; `branch_name` is worktree-only) whose `workspace_path` is NOT anchored under any `~/.hermes` tree (`get_default_hermes_root()` and `$HOME/.hermes` both excluded, so edit-in-place config / live-install worktrees stay completable); `scratch` (throwaway tmp dir) and `dir` (plain shared directory — persistent build dirs, `cwest/hermes-config` edits) are NOT PR-requiring and complete exactly as before. The artifact check `_card_has_pr_artifact(conn, id)` reuses the existing PR→card linkage — a resolvable `pull/` URL in any task comment (the implementer's ready-for-review handoff), matched by the SAME `_RESPAWN_GUARD_PR_URL_RE` the `active_pr` respawn guard and `_lane_work_provably_done` already trust (extend, not duplicate). For a guarded card with no PR, `complete_task` is a clean no-op refusal (returns `False`, no task-state mutation) that emits an auditable `completion_refused_missing_pr` event with a `summary_preview`, mirroring `completion_refused_acceptance`. Casey's merge path (`allow_acceptance_complete=True`, the `github-pr-closed` webhook) BYPASSES the guard exactly as it bypasses the acceptance guard. The guard lives inside `complete_task` — the single chokepoint every completion path calls (worker `kanban_complete`, `hermes kanban complete`, the swarm root helper, the dashboard) — so the whole class is covered by construction. Tests: `tests/hermes_cli/test_kanban_complete_missing_pr_guard.py` (7 behavior-contract cases against a real temp kanban DB — worktree+no-PR refused, refusal emits the audit event + no `completed` event, `dir`/`scratch` cards complete with no PR, worktree+PR-comment completes, `~/.hermes` worktree completes, merge-override completes without a PR). Full `test_kanban_db.py` (271) + the kanban surface green; no regressions. **Retire trigger:** never auto-retires on an upstream PR-merge signal (no upstream PR — fork-internal completion-lane invariant); remove only if the fork's one-card completion discipline is retired. | permanent-local | main@9be292f1e | +| fork PR (TBD — no upstream PR; fork-internal dispatcher semantics) | Recognize a **no-PR edit-in-place clean exit as provably-done** so the kanban crash-detector stops false-`gave_up`ing a finished edit-in-place card (the last defect from the code-enforcement batch; live 2026-07-19 on cards `t_3f07b138` and `t_ae057580`, both false-`gave_up`'d with deliverables complete and green on disk). Root cause: `_lane_work_provably_done` (`hermes_cli/kanban_db.py`) — the SAME helper both `detect_crashed_workers`' `clean_exit_after_done` carve-out and `check_respawn_guard` trust — accepted exactly TWO proofs (a `task_runs` row with `outcome='completed'` in the success window, and a GitHub PR URL in a recent comment). An edit-in-place / no-PR card (`workspace_kind='scratch'`/`'dir'`, or a `~/.hermes` workdir) completes its lane by exiting rc=0 WITHOUT a terminal kanban verb — by design, since `done ≡ merged` only applies to PR-backed cards and there is no PR to open — so it satisfies NEITHER proof (opens no PR → fails proof 2; the verb-less exit is exactly why no `outcome='completed'` run row is written → fails proof 1). The carve-out therefore never fired for exactly the card shape that legitimately exits verb-less → guaranteed false `protocol_violation`/`gave_up`, stranding a completed card in `blocked` for a human to hand-reconcile. Fix: add a THIRD proof, proof-gated and scoped to the no-PR shape via the SHARED `_card_requires_pr(workspace_kind, workspace_path)` predicate (the same predicate the required-artifact completion guard uses — not a new ad-hoc definition): for a card that `_card_requires_pr` classifies as NOT PR-requiring, a durable self-verification / lane-done HANDOFF comment within the reused `_RESPAWN_GUARD_SUCCESS_WINDOW` (matched by a conservative, line-anchored `_LANE_DONE_HANDOFF_RE` — `review-required:` / `self-verified` / `lane-done` / `handoff` / `verified green|on disk` / `completed lane`) reads as landed-work proof. Absence of proof KEEPS the strict protocol-violation behavior (a genuinely-incomplete quiet exit on a no-PR card STILL counts — the carve-out is proof-gated and cannot mask real breakage), the existing two proofs are unchanged, and a PR-requiring worktree card is still held to a real PR/completed-run artifact (PR-backed behavior byte-for-byte unchanged — the handoff-comment match never leaks into it). No new config, no new schema, no user-facing env var — reuses the existing window constant and the shared shape predicate. Single-file change in `hermes_cli/kanban_db.py` + 3 behavior-contract regression tests in `tests/hermes_cli/test_kanban_db.py` (no-PR edit-in-place done+handoff → benign no-op surfaced via the `_last_clean_exit_after_done` side-channel; no-PR edit-in-place with no landed-work proof → still `protocol_violation`/`gave_up`; PR-requiring card with a handoff comment but no PR → still `protocol_violation`). Real `kanban_db`, temp `HERMES_HOME`, no mocks; full `test_kanban_db.py` (274) + the kanban surface green, ruff clean, 0 regressions. **Retire trigger:** never auto-retires on an upstream PR-merge signal (no upstream PR — fork-internal dispatcher/lifecycle invariant); remove only if the fork's no-PR edit-in-place card model is retired. | permanent-local | main@9be292f1e | +| fork PR (TBD — no upstream PR; fork-internal review-lane semantics) | Stop the kanban `active_pr` respawn guard from re-arming on the SANCTIONED RECOVERY'S OWN unblock-cutoff comment, so a `ready` card that legitimately REUSES an open PR for a revision round (the OUTER feedback loop, heavy on WRITING cards) can spawn its author. Live symptom 2026-07-22 (card `t_11132357`, PR #143): after a correct `block` → `route_feedback_to_author` → `unblock_task` (which emits the `unblocked` cutoff event), `check_respawn_guard` (`hermes_cli/kanban_db.py`) STILL returned `active_pr` every tick and the dispatcher refused to spawn the author for hours. Root cause (distinct from #46204 and row 87 — a boundary bug those did not cover): the `active_pr` dup-PR scan clears only for PR-URL comments STRICTLY BEFORE `pr_cutoff = max(window, latest_unblock_ts)`, but the sanctioned recovery writes its OWN audit/steer comment that NAMES the existing PR URL AT THE SAME SECOND as the `unblocked` event it emits (verified: latest `unblocked` ts == latest pull/143 comment ts). Kanban timestamps are second-granular, so that recovery comment lands at `created_at == pr_cutoff` and the `>=` window scan re-counts it as a fresh active PR — the recovery's own cutoff-marking comment re-arms the guard it is trying to clear, and no operator move can win (any comment the recovery writes must reference the PR). The prior narrow carve-out excluded only the literal `dispatcher` author (its auto-route audit is the same class); it did not cover the homestead orchestrator/recovery actors that drive the OUTER loop — crucially the `orchestrator` author that `route_feedback_to_author` itself stamps on its outer-loop reopen audit comment. Fix (minimal, extends the existing exclusion — does NOT weaken genuine dup-PR protection): a new module-level `_RESPAWN_GUARD_RECOVERY_AUTHOR_PREFIXES = ("dispatcher", "hollis", "onecard", "orchestrator")` and the dup-PR comment scan excludes any comment whose author PREFIX-matches that set (`author NOT LIKE '%'`, so profile-suffixed variants like `dispatcher-default` are covered). None of these actors ever opens a PR, so excluding their at/after-cutoff PR-URL audit is exactly analogous to the review-status carve-out; a genuine builder/author PR-URL comment (e.g. `eckert`) at/after the cutoff is NOT in the set and STILL re-arms `active_pr` (the real duplicate-PR risk is preserved), and the `review`-status skip + closed/merged fail-open behavior are untouched. Single-file change in `hermes_cli/kanban_db.py` + 4 behavior-contract regression tests in `tests/hermes_cli/test_kanban_db.py` (RED-then-GREEN: recovery-actor PR-URL audit at the same-second cutoff → guard clears to None; prefixed recovery actor → clears; one test driven through `route_feedback_to_author` itself so it cannot drift from the real `orchestrator` emitter → clears; NON-recovery author PR-URL comment at/after cutoff → STILL `active_pr`). Real `kanban_db`, temp `HERMES_HOME`, no mocks; full `test_kanban_db.py` + the kanban surface green, 0 regressions. **Retire trigger:** never auto-retires on an upstream PR-merge signal (no upstream PR — fork-internal orchestrator recovery machinery; `hollis`/`onecard` are homestead actors, not upstream concepts); remove only if the fork's outer-feedback-loop recovery flow is retired. | permanent-local | main@9be292f1e | +| fork PR (TBD — no upstream PR; fork-internal dispatcher semantics) | Stop a PHANTOM or DELETED PR URL in a task comment from wedging a card forever on the `active_pr` respawn guard. Companion to the #46204 / #46549 / #77 `active_pr` fixes, closing an orthogonal residual hole none of them covered: those handle a LIVE-but-stale open PR, a review-lane bypass, an explicit-unblock cutoff, and a recovery-actor's own re-arming comment respectively — but ALL of them still route through `_resolve_pr_state` (`hermes_cli/kanban_db.py`), which fails OPEN to `"unknown"` on ANY `gh` nonzero exit. A PR URL that GitHub definitively does NOT have — never created, or deleted — makes `gh pr view` exit nonzero with the GraphQL signature `Could not resolve to a PullRequest`; the fail-open path read that as `unknown`, and the guard treats `unknown` as still-active (conservative), so the phantom URL guarded the card on every tick indefinitely (live symptom: an `orchestrator-recovery` card whose audit comment named a `pull/143` that never existed accrued dozens of `respawn_guarded {active_pr}` events with no worker ever spawning). Root cause: conflating a DEFINITIVE not-found (terminal — a nonexistent PR can never be active work) with a TRANSIENT `gh` failure (network / auth / gh-missing — genuinely unresolvable, must stay fail-open). Fix (minimal, well-scoped): a new module-level `_PR_NOT_FOUND_STDERR_RE` (`could not resolve to a pullrequest`, case-insensitive) lets `_resolve_pr_state` return a new terminal state `"not_found"` when `gh` exits nonzero AND stderr carries that signature — every OTHER nonzero exit still returns `"unknown"` (fail-open preserved); `check_respawn_guard`'s dup-PR decision now consults a single-source-of-truth `_RESPAWN_GUARD_INACTIVE_PR_STATES = frozenset({"closed","merged","not_found"})` so a not-found PR no longer guards, exactly like a closed/merged one, while `open` and `unknown` still guard. Single-file change in `hermes_cli/kanban_db.py` + 3 behavior-contract regression tests in `tests/hermes_cli/test_kanban_db.py` (`_resolve_pr_state` maps the GraphQL not-found stderr → `not_found`; a generic nonzero exit with no not-found signature STILL → `unknown` fail-open; a not-found PR-URL comment from a real builder → guard clears to None) plus one hermetic-fixture fix in `tests/hermes_cli/test_kanban_route_feedback_to_author.py` (the `kanban_home` fixture now stubs `_resolve_pr_state`→`open`, since its placeholder PR URL is non-existent and would otherwise now resolve `not_found` and clear the very guard those routing tests set out to exercise — the routing logic is orthogonal to live PR-state resolution). Real `kanban_db`, temp `HERMES_HOME`, no mocks; the isolated kanban surface green, 0 new failures (the ~26 pre-existing cross-test-pollution failures in a whole-`-k kanban` single-process run reproduce identically on the clean base and are unrelated to this change). **Retire trigger:** never auto-retires on an upstream PR-merge signal (no upstream PR — fork-internal dispatcher/recovery invariant); remove only if the fork's `active_pr` respawn-guard machinery is retired. | permanent-local | main@9be292f1e | diff --git a/hermes_cli/kanban_db.py b/hermes_cli/kanban_db.py index e464f9a55b2d9..b2af8b5206853 100644 --- a/hermes_cli/kanban_db.py +++ b/hermes_cli/kanban_db.py @@ -9477,15 +9477,44 @@ def schedule_task( ) +# gh's stderr signature for a PR URL that GitHub definitively does NOT have — +# never created, or already deleted. The GraphQL resolver emits "Could not +# resolve to a PullRequest with the number of N.". Matched case-insensitively +# and tolerant of the exact phrasing so a definitive not-found is separated from +# a transient gh failure (network / auth / gh-missing), which must stay +# fail-open. See :func:`_resolve_pr_state`. +_PR_NOT_FOUND_STDERR_RE = re.compile( + r"could not resolve to a pullrequest", + re.IGNORECASE, +) + +# PR states that are NOT active work, so an active_pr respawn guard must NOT +# fire on them. ``closed`` / ``merged`` are self-explanatory; ``not_found`` is a +# phantom or deleted PR (:data:`_PR_NOT_FOUND_STDERR_RE`) which — before this +# carve-out — fell through the fail-open path and wedged the card forever. +# ``open`` and ``unknown`` (unresolvable, fail-open) are deliberately absent: +# both still guard. +_RESPAWN_GUARD_INACTIVE_PR_STATES = frozenset({"closed", "merged", "not_found"}) + + def _resolve_pr_state(pr_url: str) -> str: """Return the live GitHub state of ``pr_url`` for the active_pr respawn guard. - One of ``"open"``, ``"closed"``, ``"merged"``, or ``"unknown"``. Backed by - ``gh pr view --json state`` (``gh``'s uppercase ``OPEN``/``CLOSED``/ - ``MERGED`` are lowercased). Fail-open: any subprocess failure -- ``gh`` not - installed, not authenticated, network error, timeout, unparseable output -- - resolves to ``"unknown"`` so the caller keeps guarding conservatively rather - than dropping the guard on a transient hiccup. + One of ``"open"``, ``"closed"``, ``"merged"``, ``"not_found"``, or + ``"unknown"``. Backed by ``gh pr view --json state`` (``gh``'s + uppercase ``OPEN``/``CLOSED``/``MERGED`` are lowercased). + + ``"not_found"`` is returned when ``gh`` exits nonzero AND its stderr carries + GitHub's definitive "does not exist" signature (``Could not resolve to a + PullRequest``). A PR URL that was never created, or was deleted, is NOT a + transient hiccup: it can never be active work, so the caller treats + ``not_found`` like ``closed``/``merged`` and does not guard. This is what + keeps a phantom/stale PR URL in a comment from wedging a card forever. + + Fail-open otherwise: any *other* subprocess failure -- ``gh`` not installed, + not authenticated, network error, timeout, unparseable output -- resolves to + ``"unknown"`` so the caller keeps guarding conservatively rather than + dropping the guard on a transient hiccup. Isolated as a module-level function (not inlined) so ``check_respawn_guard`` can be tested without a live network call by monkeypatching this hook, and @@ -9502,6 +9531,12 @@ def _resolve_pr_state(pr_url: str) -> str: except Exception: return "unknown" if result.returncode != 0: + # A definitive "this PR does not exist" is terminal, not transient: + # never created or already deleted -> never active work. Distinguish it + # from a transient gh failure (which stays fail-open as "unknown") by the + # GraphQL not-found signature on stderr. + if _PR_NOT_FOUND_STDERR_RE.search(result.stderr or ""): + return "not_found" return "unknown" try: state = json.loads(result.stdout or "{}").get("state") @@ -11435,8 +11470,11 @@ def check_respawn_guard(conn: sqlite3.Connection, task_id: str) -> Optional[str] state = _resolve_pr_state(pr_url) pr_state_cache[pr_url] = state # open or unknown (unresolvable -> fail-open) still guards; - # a confirmed closed/merged PR does not. - if state not in ("closed", "merged"): + # a confirmed closed / merged / not_found PR does not: those + # three are terminal states that can never be active work + # (not_found = a phantom or deleted PR that would otherwise wedge + # the card forever via the fail-open path). + if state not in _RESPAWN_GUARD_INACTIVE_PR_STATES: return "active_pr" return None diff --git a/tests/hermes_cli/test_kanban_db.py b/tests/hermes_cli/test_kanban_db.py index 010a3bb01323d..ecdf7e6ec09be 100644 --- a/tests/hermes_cli/test_kanban_db.py +++ b/tests/hermes_cli/test_kanban_db.py @@ -3853,6 +3853,64 @@ def _fake_run(*a, **k): assert kb._resolve_pr_state("https://github.com/o/r/pull/7") == "unknown" +# --------------------------------------------------------------------------- +# not_found: a PR URL that GitHub definitively reports does not exist (never +# created, or deleted) is NOT a transient hiccup and must NOT fail open. gh +# returns a nonzero exit whose stderr carries the GraphQL signature "Could not +# resolve to a PullRequest". Conflating that DEFINITIVE not-found with a +# transient error (network / auth / gh-missing) is what let a phantom PR URL in +# a task comment wedge a card forever on the active_pr respawn guard. A +# not_found PR is terminal like closed/merged: it can never be active work. +# --------------------------------------------------------------------------- + +def test_resolve_pr_state_not_found_on_graphql_no_pr(monkeypatch): + """A definitive 'Could not resolve to a PullRequest' resolves to 'not_found'.""" + def _fake_run(*a, **k): + return types.SimpleNamespace( + returncode=1, + stdout="", + stderr=( + "GraphQL: Could not resolve to a PullRequest with the number " + "of 143. (repository.pullRequest)" + ), + ) + monkeypatch.setattr(kb.subprocess, "run", _fake_run) + assert kb._resolve_pr_state("https://github.com/o/r/pull/143") == "not_found" + + +def test_resolve_pr_state_transient_error_still_unknown(monkeypatch): + """A generic nonzero exit (no not-found signature) still fails open to 'unknown'.""" + def _fake_run(*a, **k): + return types.SimpleNamespace( + returncode=1, stdout="", stderr="error connecting to api.github.com", + ) + monkeypatch.setattr(kb.subprocess, "run", _fake_run) + assert kb._resolve_pr_state("https://github.com/o/r/pull/7") == "unknown" + + +def test_respawn_guard_not_found_pr_not_guarded(kanban_home, monkeypatch): + """A builder's PR URL whose PR does not exist on GitHub never blocks respawn. + + Regression for the phantom-PR livelock: a comment carried a PR URL that was + never actually created (or was deleted). The old fail-open path treated the + gh error as 'unknown' and guarded forever, stranding the card across dozens + of respawn_guarded ticks. A definitively not-found PR is terminal like + closed/merged — the card must dispatch. The comment is authored by a real + builder ('eckert', NOT a recovery-actor) so it genuinely reaches the + live-state resolution instead of being excluded by the recovery-author + carve-out. + """ + monkeypatch.setattr(kb, "_resolve_pr_state", lambda url: "not_found") + with kb.connect() as conn: + t = kb.create_task(conn, title="phantom-pr", assignee="alice") + kb.add_comment( + conn, t, "eckert", + "PR opened: https://github.com/totemx-AI/subsidysmart/pull/143", + ) + reason = kb.check_respawn_guard(conn, t) + assert reason is None + + # Review tasks bypass the dup-PR guards (recent_success / active_pr) but a # build-lane task in the same shape must still be guarded. The invariant being # asserted: review spawns are never wedged by the build-only dup-PR guards, diff --git a/tests/hermes_cli/test_kanban_route_feedback_to_author.py b/tests/hermes_cli/test_kanban_route_feedback_to_author.py index 02754af6891c4..34fb0409ee605 100644 --- a/tests/hermes_cli/test_kanban_route_feedback_to_author.py +++ b/tests/hermes_cli/test_kanban_route_feedback_to_author.py @@ -32,11 +32,21 @@ @pytest.fixture def kanban_home(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: - """Isolated HERMES_HOME with an empty kanban DB.""" + """Isolated HERMES_HOME with an empty kanban DB. + + Also stubs ``_resolve_pr_state`` to ``"open"`` so the open-PR cards these + tests stage are treated as genuinely active work without a live ``gh`` call. + Without this the guard would resolve the fixture's placeholder PR URL against + real GitHub -- a non-existent PR now resolves to ``not_found`` (terminal, not + active), which would clear the very guard these routing tests set out to + exercise. The routing logic under test is orthogonal to live PR-state + resolution, so pinning it keeps these tests hermetic and deterministic. + """ home = tmp_path / ".hermes" home.mkdir() monkeypatch.setenv("HERMES_HOME", str(home)) monkeypatch.setattr(Path, "home", lambda: tmp_path) + monkeypatch.setattr(kb, "_resolve_pr_state", lambda url: "open") kb.init_db() return home