🐛 fix(kanban): treat a phantom/deleted PR URL as not_found so it never wedges active_pr - #78
Closed
cwest wants to merge 68 commits into
Closed
🐛 fix(kanban): treat a phantom/deleted PR URL as not_found so it never wedges active_pr#78cwest wants to merge 68 commits into
cwest wants to merge 68 commits into
Conversation
…ew skill Fork-only release engineering, kept indefinitely (permanent-local): - .github/workflows/fork-daily-sync.yml — daily upstream-sync workflow that fetches/rebases the patch queue onto the newest upstream tag, runs tests, and opens a review PR; scripts/fork_retire_patches.py supports it. - .github/workflows/fork-secret-scan.yml — gitleaks gate on every PR/push to cwest/integration, enforcing the fork's "no secrets, ever" invariant. - PATCHES.md — the manifest of everything the fork carries on top of its upstream base tag, plus the bucket model, auto-retire rule, and per-row behavior-keyed override; docs/patches/* hold per-patch rationale. - skills/github/github-code-review — homestead PR-review pipeline expansion (webhook review context, post-once idempotency, author-resolves-threads loop, humanizer/de-claude gate). Never sent upstream.
Count skills.external_dirs-granted skills in the profiles dashboard and `hermes profile list` so the count matches `hermes skills list` exactly. 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. Fork-specific behavior (the fork's external_dirs grant); kept indefinitely (fork PR #6).
…l join 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). Gives the mocked discord.AudioSource in the gateway conftest a real base class so the 19 TestVoiceMixerCore tests run without the real discord package. upstream-pending: PR NousResearch#44023
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. Ported into GatewayKanbanWatchersMixin._kanban_notifier_watcher. upstream-pending: PR NousResearch#44338 (partial carry — SendResult non-delivery only)
…ent_success) 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. Reads tasks.status and skips those two guards for status='review' while keeping rate_limit_cooldown and blocker_auth active. upstream-pending: PR NousResearch#46549
…cards Subscribe the configured report-back target so a card created without an originating session still delivers its terminal notification. upstream-pending: fork PR #9
Run the in-process kanban dispatcher on a dedicated single-thread executor so a busy default ThreadPoolExecutor (saturated by agent turns + 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; preserves _release_singleton_lock in both the CancelledError and normal-exit paths. Ships a starvation regression test that fails on the old shared-pool behaviour. upstream-pending: fork PR #4 (no upstream PR yet)
Deduplicate review cards on their PR URL in create_task so a repeated PR-review request does not open a second review card for the same PR. upstream-pending: fork PR #3
detect_crashed_workers grants a freshly-spawned worker a launch-window grace so its PID can become visible before liveness is checked — but it measured the grace from tasks.started_at, pinned to the task's first-ever start and never refreshed on re-claim. A card re-claimed for its next lane inherited a stale started_at, the grace had expired, and the new worker was reaped mid-init. Now 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 — with a fallback to tasks.started_at when no run row is present. upstream-pending (PR TBD)
…g lanes 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. Widens the crash scan to any card with a non-NULL worker_pid and, for a non-running card, does an in-place claim clear with a stale_claim_cleared event and NO lane change; the running path is byte-for-byte unchanged. upstream-pending: fork PR #16
A card in review is claimed by claim_review_task, which CAS-transitions review->running, so while the reviewer works the row status is running, indistinguishable from a build run. When detect_crashed_workers reaped such a crash it ran SET status='ready', losing the review lane (the implementer re-ran instead of the reviewer respawning). Reads the durable source_status='review' signal off the crashed run's claimed event and restores the card to review instead of ready; the breaker-trip WHERE-IN widens to include 'review' so a repeatedly-crashing reviewer still trips to blocked via the failure-count path. upstream-pending: fork PR #17
Pin a dispatcher-spawned worker's git identity to the host config at spawn time so worker commits carry the correct author instead of a container default. upstream-pending: fork PR #14
Make the active_pr respawn guard honor an explicit unblock so the review->author rework loop can spawn without waiting out the 24h PR window. 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 respawn, while URLs at/after still guard. Additive over NousResearch#46549 (which covers status='review' only; the author-rework card is in 'ready'). upstream-pending: PR NousResearch#46204
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. Board-internal (lives in the dispatcher, not the reviewer): auto_route_review_bounce scans blocked cards each dispatch_once tick and, for a card whose most-recent sticky blocked event carries the review-changes-requested reason prefix, reassigns it to the original author and unblocks it. Idempotent. upstream-pending: fork PR #18
_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
…(default-off) 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. New module gateway/kanban_transition_emit.py adds pure decision logic (should_emit_transition, build_transition_payload with a stable (board,task_id,kind,event_id) idempotency key) plus a fail-safe emit_transition coroutine that HMAC-signs and POSTs; it NEVER raises. The payload is classifiable by the webhook adapter, and transition wakes route back to the origin thread/session. Guarded by kanban.transition_emit.enabled (default OFF); when disabled the notifier path is byte-for-byte unchanged. No new core tool, no new model surface, no user-facing HERMES_* config var. upstream-pending: fork PR #21, #22, #23
Rebased the fork stack onto upstream main@9be292f1e and dropped the per-task --max-iterations override (former P16, upstream #19). The knob cut against the board's decompose-first design: a card that exhausts the global 90-turn ceiling is almost always a sizing failure (too big, should be split into smaller cards), not a budget failure. The escape hatch for a genuinely atomic-large task already exists natively — upstream bridges HERMES_MAX_ITERATIONS from agent.max_turns in config — so the per-task DB column was redundant carry with no upstream home. Stack is now 17 patches.
…hor map The fork now develops against upstream `main` HEAD (the community installer tracks main, not release tags), so the CI fork-point is a commit SHA, not a `vX.Y.Z` tag. Two CI gates assumed a tag base and broke after the rebase onto main@9be292f1e: - fork-secret-scan: the range step grep'd `Base tag: vX.Y.Z` and verified `refs/tags/<tag>`, which no longer matches a SHA base. Reworked to prefer a machine-readable `<!-- Base commit: <sha> -->` line in PATCHES.md (falling back to the legacy tag), emitting a generic `base_ref` used by the scan + summary steps. Scopes gitleaks to exactly the carried patch stack (18 commits). - check-attribution walks `merge-base(origin/main, HEAD)..HEAD`; with the fork's main mirror advanced to the rebase base, that range is now our patches only — add demi (christophergervais92) to release.py AUTHOR_MAP so the sole non-cwest author in range is mapped. PATCHES.md records the SHA base + notes the fork tracks main HEAD.
The notifier gate delivered only five terminal event kinds (completed/blocked/gave_up/crashed/timed_out) and silently dropped every other lane change — assigned, unblocked, and critically block_loop_detected, the auto-escalate-to-triage signal (the system asking for a human). A card could exhaust its block-recurrence limit, escalate to triage, and sit completely silent. Changes: - Widen the delivery gate to NOTIFY_KINDS: the terminal five plus the meaningful lane changes (block_loop_detected, unblocked, assigned, promoted, reclaimed, stale, dependency_wait). High-frequency bookkeeping kinds stay excluded so routine churn does not ping. - Add per-kind message wording for the new kinds, with a generic transition line for any remaining NOTIFY_KIND so a lane change is never silent. - Guarantee a delivery target for a card with NO subscription: lazily register a fallback subscription to the default channel, cursor-seeded to the latest notifiable event so only the live transition fires (no historical backfill flood). Channel/platform config-overridable via kanban.notify_fallback; empty chat_id opts out. - Seed support in add_notify_sub via initial_cursor (INSERT-only) so the fallback sub never replays a task's entire event history. - Extend the agent-run bridge default kinds to include block_loop_detected so the triage escalation wakes the orchestrator, carrying origin session/thread context. Tests: new E2E tests against a temp HERMES_KANBAN_DB prove an assigned lane change delivers, a block_loop_detected escalation delivers AND wakes, and a no-subscription card surfaces to the fallback channel. All RED before the change, GREEN after; the terminal-kind and origin-payload contract is unregressed.
* ✨ feat(kanban): wake origin thread session on every transition The transition-emit bridge stamped origin_* fields on its payload, but the webhook route ignored them and always minted a contextless webhook:<route>:<delivery> session — so a woken orchestrator never resumed the thread the work was born in. And the emit gate covered only blocked/ block_loop_detected, so a card moving to review (status_changed + assigned) woke nothing at all. - webhook route: when a payload carries origin_platform/chat_id/thread_id, build a SessionSource targeting that origin thread instead of the synthetic webhook session, so the run resumes the origin session and reports back there. Falls back to the webhook session when origin fields are absent (backward-compatible). The origin source is shaped to mirror the live inbound session key (chat_type=thread, chat_id+thread_id set, no per-user suffix) so the wake lands in the exact origin session, not a phantom one. - widen DEFAULT_EMIT_KINDS to include the actionable lane-move kinds (status_changed, assigned, unblocked) alongside blocked/block_loop_detected; completed stays out (done is a close-loop ping, not a reasoning task). - tests: origin routing + key-mirror invariant + emit-kind coverage; RED before, GREEN after; full gateway suite green. * 🐛 fix(kanban): wake orchestrator on terminal-failure transitions DEFAULT_EMIT_KINDS omitted the terminal-failure kinds gave_up, crashed, and timed_out. Those flow through the same gate as the chat-ping notifier (should_emit_transition is invoked inside the loop over NOTIFY_KINDS, whose TERMINAL_KINDS = completed, blocked, gave_up, crashed, timed_out), so a worker that gave up, crashed, or timed out pinged chat but woke no orchestrator — the same silent-escalation gap this change set closes, left open on the failure path. Add the three kinds to the default emit set so the failure path wakes the orchestrator exactly like blocked/block_loop_detected do. completed stays out by design (done is a close-loop chat ping, not a reasoning task). Add a default-coverage test so the failure-path wake cannot silently regress.
#28) A card moving ready→running→review fires a status_changed event, but the notifier's agent-wake (transition-emit) was nested inside the chat-ping delivery loop, which only claims events whose kind is in NOTIFY_KINDS. status_changed is deliberately excluded from NOTIFY_KINDS as high-frequency bookkeeping, so the wake could only ever fire for a kind that ALSO chat-pinged. A plain lane move therefore never woke the orchestrator — the wake and the ping were coupled to a single per-subscription cursor. Decouple them: - Add a second per-subscription cursor, kanban_notify_subs.last_emit_event_id (canonical CREATE + additive migration), so the wake path claims independently of the chat-ping cursor. - Add claim_unseen_emit_events_for_sub(), the wake twin of the ping claim, running on the emit cursor and filtering by emit_kinds. - In the notifier tick, claim emit events per subscription BEFORE the ping claim's early-continue and fire the wake from a dedicated emit_wakes loop, independent of whether any chat ping was delivered. A status_changed now wakes the origin thread session WITHOUT sending a chat ping. Tests: RED→GREEN unit coverage for the new column, the independent-cursor claim, and cursor isolation; full notifier + transition-emit + kanban_db suites pass (302).
…on wake (#29) An origin-routed kanban-transition wake arrives at the platform adapter as a plain TEXT MessageEvent. When the origin session is mid-turn, handle_message takes the busy branch and either text-debounces the event or newline-merges it into the in-progress/queued turn via merge_pending_message_event(merge_text=True) — so the wake is absorbed silently instead of producing its own identifiable turn (the busy-session swallow). Tag the origin-routed wake on the webhook side (metadata["kanban_transition_wake"] = True), keyed on the emitter-stamped origin_* fields so ordinary webhook routes are unaffected. A pure predicate is_transition_wake_event reads the 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 queues the wake without debounce/merge so the existing in-band drain cascade runs it as a distinct turn. Cache- and 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.
…banner (#30) A transition wake now leads its woken turn with a fixed-prefix, greppable banner stamping task_id + kind + from->to lane + event_id (`AUTONOMOUS-WAKE t_XXXX <kind> <from>-><to> evt=NNNN`), so the woken output is unmistakable and can never be conflated with a late-delivered prior reply. - kanban_transition_emit: add `build_wake_banner` (single source of truth for the banner shape) and stamp `wake_banner` + `from_lane`/`to_lane` into the emit payload. Lane ends degrade to `?` when a kind carries no from->to pair; the banner is deterministic per (task_id, kind, from, to, event_id) so each wake is unique and stable to grep. - kanban_watchers: extract the event's `{"from","to"}` hop and pass it through so status_changed/assigned wakes carry their real lane transition. - webhook route: when a payload carries `wake_banner`, guarantee the woken run's prompt LEADS with it verbatim as the first line, regardless of the route's prompt template. Scoped to payloads that carry the banner, so ordinary webhooks are untouched. Tests: RED->GREEN for the banner builder, payload stamping, and the route prepend (incl. empty-template and non-transition-untouched cases).
…mit <10s (#31) The transition-emit (agent-wake) and chat-ping delivery both fire from the notifier tick, so the tick gap bounds worst-case emit latency after a lane MOVE. The interval was a hardcoded 5.0s with a hardcoded 5s first-tick settle, so a cold start could push a transition's emit toward ~10s and the cadence was untunable without editing source. Make the tick cadence config-driven via kanban.notifier_interval_seconds with a near-real-time default of 2s and a hard >=1.0s floor (a sub-second value busy-polls the SQLite WAL), and cap the first-tick cold-start settle to the interval so a tightened cadence isn't undercut by a fixed 5s delay. An early tick before adapters connect is a harmless no-op (the tick skips when no adapters are active) and retries next interval. Delivery-filter anti-churn behavior is untouched: NOTIFY_KINDS / emit_kinds and the cursor-based dedup are unchanged - only the poll gap tightens. <!-- card:t_940f9205 -->
#32) A kanban-transition wake arriving while the origin session is mid-turn was silently swallowed: no turn_context line, banner never surfaced. Root cause is the runner busy handler. handle_message routes an inbound event to _handle_active_session_busy_message BEFORE the base adapter's wake-precedence enqueue. Under the default busy_input_mode='interrupt', that handler treated the wake like a user TEXT message — calling running_agent.interrupt(wake_text), which folds the banner into the finishing prior turn's response context (where it is discarded), and returning True. Returning True short-circuits handle_message before the adapter enqueues the wake un-merged for the post-turn drain, so the wake never becomes a conversation turn. Exempt a transition wake from the interrupt/steer/queue busy-input logic, the same way internal synthetic events are exempted: return False early so the base adapter queues the wake un-merged (its wake-precedence slot guard) and the existing post-turn cascade dispatches it as a DISTINCT turn once the in-flight turn ends. Adds a reproducing test: a wake mid-turn under interrupt/steer mode must not interrupt/steer and must fall through, and end-to-end the queued wake is drained as its own turn carrying the banner text.
#33) A kanban-transition wake queued while the origin session was mid-turn was drained and reprocessed correctly, but its own turn output was then nulled by the stale-response guard: when a later inbound message set the interrupt Event and queued a follow-up, the wake turn's banner-led response met the suppression predicate (response + interrupted + pending) and was discarded. The wake turn ran but delivered nothing — the 'no visible wake turn, no delivered banner' symptom. A transition-wake turn is an autonomous action, not a reply to a user prompt, so a later inbound message never makes it stale. Extract the suppression decision into should_suppress_stale_response() which exempts wake turns, and route the call site through it. Plain replies are still suppressed when superseded, preserving the original behaviour. Adds a regression test that fails against the pre-fix predicate (witnessed RED) and passes with the exemption, plus controls for the plain-reply path.
…ter (#34) * fix(gateway): dispatch origin-routed wake on the owning platform adapter An origin-routed kanban-transition wake carries a SessionSource whose platform is the ORIGIN platform (e.g. discord) and targets a session owned by that platform's adapter. The webhook receiver was processing the wake with self.handle_message(event) — running it on the WEBHOOK adapter, not the origin adapter. Each adapter keeps its own _active_sessions / _pending_messages. Processed on the webhook adapter, the wake never sees the origin session's busy state: it takes the cold path, spawns a turn via the shared runner, collides with the already-running origin turn at the runner's _running_agents guard, and is silently dropped — no turn_context line, banner never surfaces. This is why the adapter-level wake fixes never took effect: they ran on the wrong adapter instance. Dispatch an origin wake through gateway_runner.adapters[origin_platform] (the reference already wired for cross-platform delivery) so busy-detection, the wake-precedence pending slot, and the post-turn drain all operate on the same session state as the running origin turn. Fall back to the webhook adapter when the owning adapter can't be resolved (backward-compatible; ordinary non-origin webhook events are unaffected). Adds regression tests that drive the real handle_message on two independent adapter instances: proving the wake reaches the owning adapter's pending slot while the origin session is busy, and demonstrating the pre-fix bug where the wrong adapter leaves the owner's pending slot empty. * test(gateway): drive the real _handle_webhook origin-wake selection path Add two integration tests that POST an origin-wake payload through the live aiohttp route (WebhookAdapter._handle_webhook) with a mocked gateway_runner.adapters registry, and assert the SHIPPED dispatch-adapter selection routes to the owning platform adapter (not the webhook adapter), plus the fallback-to-webhook path when no owner is registered. Closes the coverage gap where the routing unit tests asserted against a hand-copied selection helper that could drift from production without failing. Verified the new integration test fails against the pre-fix dispatch (owner never called) and passes with it.
An origin-routed kanban-transition wake carries a user-less SessionSource (the webhook builds no user_id for a shared thread) and is HMAC-authenticated at the webhook route. When the origin session was busy, the busy handler's user-authorization gate (NousResearch#17775) ran first, saw user=None, deemed the wake 'unauthorized', and silently dropped it (return True) BEFORE the wake exemption further down — the final swallow that kept an origin-routed wake from ever becoming a turn while the session was busy. Exempt system-internal events (transition wakes and events flagged internal) from the user-authorization gate: they are not user messages and are already authenticated upstream. They remain exempted from interrupt/steer/fold below, so this only lets them reach the queue-as-distinct-turn path. Ordinary unauthorized user messages are still dropped (the NousResearch#17775 protection is intact). Adds a regression test (witnessed-RED against the pre-fix gate) plus a control proving an unauthorized non-system message is still dropped.
The kanban notifier read transition_emit_cfg (enabled / emit_kinds) and the notify_fallback target ONCE before its `while self._running` poll loop, then used the cached values inside every tick. Any change to kanban.transition_emit.emit_kinds or .enabled was therefore silently restart-gated: editing the live config had no effect until a gateway restart, with nothing signalling the setting was stale. Move the derivation into a small _resolve_transition_cfg() helper backed by a FRESH load_config() and call it at the top of each tick, mirroring how the tick interval is re-resolved per call. The _collect closure captures these names by reference, so rebinding them per tick makes the change visible to that tick's claim/emit gates. load_config() is a cheap in-memory read; behavior is unchanged when the config is unchanged. The helper falls back to safe defaults if a load fails so a transient config error never crashes the tick. A regression test flips emit_kinds mid-run across two ticks (via a fresh config object, as an on-disk edit produces) and asserts the widened gate is honored on the next tick with no restart.
…51) A card born in a thread must report back to THAT thread, but the wake was dispatching to the Home channel with thread=None — so from the watcher's seat the pipeline went dark: work ran, cards moved, follow-ups were commissioned, and none of it surfaced. Root cause: the transition-wake destination is read verbatim off the notify subscription row, and a thread-born card ends up with TWO rows — the correct origin sub (chat+thread) stamped at filing, plus a later thread-less channel sub (e.g. a review-stage defensive re-subscribe to the Home channel). The emit-wake loop fires one wake per row, so the card double-fires: one to the origin thread (correct) and one to Home/thread=None (dark). Fix at both the write and the egress: - add_notify_sub skips a thread-less sub when the same (task, platform) already has a thread-bearing sub, so the dark-routing duplicate is never written. A genuinely channel-born card (no thread sub) is unaffected. - dedupe_wake_subs collapses a card's subs to a single wake target per (task, platform), preferring the thread-bearing sub, and the notifier gates the agent-wake claim on it — so even a legacy DB that already has both rows fires exactly one wake, to the origin thread. Distinct real threads are preserved; only the thread-less duplicate is dropped. The chat-ping path (separate cursor) is untouched. Restart-gated: touches gateway notifier/emit + kanban_db; verify against a restarted gateway. Filing-side origin resolution (literal 'origin' sentinel, review-stage bare-channel subscribe) lives in a separate repo and is tracked as a gated follow-up. Tests: new unit coverage for dedupe_wake_subs + the add_notify_sub guard, and an integration test proving a card with dup subs fires ONE wake to the thread (RED-verified: two wakes without the gate).
…o-origin cards (#52) * 🔇 fix(kanban): suppress human-facing wakes for write-time-sweep and no-origin cards Automatic `curate: write-time sweep @ <sha>` bookkeeping cards, and any card with no real human origin, were firing transition wakes that resolved to the Home fallback channel and posted noise there (992-char post observed). Suppress at the source in the transition-emit path: - Add pure, unit-testable helpers in kanban_transition_emit: is_sweep_card_title (matches the `curate: write-time sweep @` prefix) and should_emit_wake, which gates the human-facing agent-run wake on card class + origin. A sweep card never wakes; a thread-less subscription pointed at the Home fallback channel is treated as no-origin (no origin => no human post). - Gate the notifier's emit-wake dispatch loop on should_emit_wake, skipping the POST (with a debug log) for suppressed cards. The chat-ping accounting is unaffected; only the agent-run wake egress is gated. The gate fails open so its own error can never drop a legitimate wake. - Genuine origin-born cards (real thread, or a real non-fallback channel) still fire, so the working synopsis/commissioning wakes are regression-guarded. RESTART-GATED: touches the gateway transition-emit / notifier path; verify against a restarted gateway. * 🧪 test(kanban): add runtime E2E proof for sweep/no-origin wake suppression Drive the real GatewayRunner._kanban_notifier_watcher tick against a seeded throwaway board carrying the incident card classes (curate: write-time sweep @ 9c63aa9 with a real non-Home origin sub, a thread-less Home-fallback no-origin card, and a real #research-thread origin card) and assert on the actual outbound wake dispatch (the emit_transition egress that produces the gateway's 'Sending response … to <Home>' log line): - the sweep card fires its chat ping but NO agent-run wake; - the no-origin Home-fallback card fires NO wake to Home; - NO wake POST lands on the Home channel 1515879019269197885; - the real #research origin card STILL wakes its origin thread; - chat-ping accounting is unaffected (all three cards ping). This exercises the runtime path through the wired should_emit_wake gate rather than a fresh-Python re-import of the pure function. The live-gateway variant of this proof additionally requires deploy + a gateway restart, since the change is restart-gated. Also document why the no-origin test is structural (chat == fallback_chat_id) rather than a read of the is_fallback flag: that flag lives only on the synthesized in-memory delivery sub, while the wake fires from a persisted notify-sub row that carries no such flag — so the structural derivation is what catches the persisted thread-less Home sub the flag would miss.
…as crashes (#54) The dispatcher still false-flagged cleanly-finished and transiently-failed workers as crashed/gave_up on two sibling paths that the rc=0 carve-out in detect_crashed_workers missed — the same bug class, uncovered on the neighbouring call paths. A. Transient endpoint-unreachable failures must be retried, not counted. A run that dies on APIConnectionError / connection-refused / a provider 5xx (classified timeout / overloaded / server_error / upstream_rate_limit after the in-process retries are exhausted) exited a plain 1, which the reap classifier read as a real crash and counted toward failure_limit — so a transient proxy blip could gave_up a card that would otherwise succeed. Add a KANBAN_TRANSIENT_EXIT_CODE (69, EX_UNAVAILABLE) sentinel plus a single source-of-truth kanban_worker_exit_code(failure_reason) helper the CLI worker exit path delegates to (replacing the inline rate-limit-only mapping). _classify_worker_exit maps 69 to a new "transient" kind, and detect_crashed_workers treats it exactly like the rate-limit carve-out: requeue to ready/review, count NO failure, record a distinct "transient" run outcome + event, and stamp last_failure_error so check_respawn_guard's cooldown spaces the retry (the cooldown gate now recognizes both rate_limited and transient outcomes). Surfaced via DispatchResult.transient and the _last_transient side-channel. B. A clean exit misread as a crash on the "pid not alive" / unknown reap path. The provably-done carve-out was gated on the reap-registry clean_exit only, so a worker that finished its lane cleanly (a draft-PR handoff, an 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, so _classify_worker_exit returns "unknown") fell into the generic crashed branch and counted a failure. Extend the _lane_work_provably_done carve-out from clean_exit to also cover the unknown kind, using the same durable proof signals (recent_success / active_pr). Non-regression preserved: a genuinely-incomplete quiet exit with no completion proof still crashes and auto-blocks after the limit, and a real non-zero crash (not the sentinel) still counts toward the breaker — the carve-outs are surgical, not a blanket "never count crashes". Adds 8 behavior-contract tests exercising the real dispatcher path against a temp board: transient-sentinel classification; transient requeue without counting a failure across many hits; transient cooldown defer/allow; unknown-exit-after-a-completed-run and unknown-exit-after-a-draft-PR not counted; unknown-exit-without-proof still crashes; a real crash still trips the breaker; and the failure-reason to exit-code mapping.
…istory (#56) A kanban-transition wake ran the woken turn against a session resolved purely from the wake's coordinate source (origin_chat_id/origin_thread_id). For a Discord thread the live session key is thread:<thread>:<thread> (chat==thread), but a persisted notify-sub can carry the PARENT channel as chat_id — so the coordinate-derived key diverges, get_or_create_session lands on a phantom/empty session, and load_transcript returns no live history. The woken turn then reasoned only from stale card/summary state and posted messages that contradict facts the live conversation had already established. The transition payload already ships the authoritative origin_session_id (= task.session_id) but the webhook dropped it. Stamp it onto the event and, in the turn loop right after get_or_create_session and before load_transcript, resume that persisted session via the existing SessionStore.switch_session (/resume) path so the woken turn loads the live thread's transcript. No-op when the ids already match (the healthy chat==thread case) and for non-wake turns; falls back to today's behavior on any missing id or switch failure, so nothing is lost. Addressing/routing is unchanged — this fixes context loading only. Adds a real-SessionStore end-to-end test proving the resumed turn's loaded history contains the previously established fact, plus unit coverage for the webhook stamp and the resume helper's no-op/fallback branches.
…ary (#55) * 📝 docs(kanban): spec origin inheritance + reassignability Autonomous-wake and terminal-state routing resolve a card's delivery surface from its kanban_notify_subs row. That row is stamped at card-create from the running process's own session identity, so a workstream that crosses a spawn boundary into a detached context (dispatched worker, delegated subagent, background process, nested create) loses the human origin and its wakes route to an inert surface. There is also no atomic way to re-point a card's origin to a new thread when a workstream forks. Spec an explicit, inheritable origin channel (HERMES_KANBAN_ORIGIN, deliberately outside _VAR_MAP so it is not subject to the session identity strip guards) that is captured at the root live session and propagated to children, plus a reassign primitive/tool to move a card's origin (optionally its descendants') to a new surface without replaying history. The already-merged active-origin delivery and progressive fallback are out of scope. Design gate — review before TDD. * ✨ feat(kanban): inherit + reassign card origin across the spawn boundary A card's origin — the delivery surface its transition wakes and completion notifications route to — is its kanban_notify_subs row, stamped at create time from the running process's own HERMES_SESSION_*. That is correct inside a live gateway session but wrong the moment a workstream crosses a spawn boundary into a detached context (dispatched worker, delegate_task subagent, background process, or a nested create from any of those): the session identity then names the detached run, not the human origin, so a wake for that work has nowhere real to land. There was also no way to re-point a card's origin when a workstream forks into a new thread. Add an explicit, inheritable origin channel and a reassign primitive: - HERMES_KANBAN_ORIGIN: a standalone ContextVar + os.environ mirror (NOT a _VAR_MAP member, so it is exempt from the per-message reset and the subprocess-env engaged-strip that session-identity vars get — it must SURVIVE the spawn boundary, the opposite requirement). set/get/capture helpers plus a root-capture at session bind and a handler-entry reset that mirrors the existing cross-session leak guards. - _maybe_auto_subscribe prefers the inherited origin over the running process's own session, so a child card created in a detached worker subscribes the human origin. Falls back verbatim to prior behaviour when no origin is inherited, so live-session-created cards are byte-identical. - The dispatcher seeds HERMES_KANBAN_ORIGIN into the worker env from the card's origin notify-sub (worker_origin_env), so descendant cards re-inherit it. - reassign_task_origin: atomically re-point a card's origin for a platform (delete same-platform subs + insert, cursor-seeded to the latest event so no history replay; idempotent; optional descendant cascade). Exposed as the orchestrator-only kanban_reassign_origin tool, which also refreshes the caller's origin so subsequently-created child cards inherit the new surface. Reuses the existing owning-adapter / async-delivery path (no parallel delivery), preserves prompt caching + role alternation, and keeps all HERMES_SESSION_* identity leak guards intact. Behaviour-contract tests C1–C4 + D1–D3 and an E2E that drives the real seed → create → subscribe → wake-payload chain and asserts the wake targets the inherited (and reassigned) origin surface.
#59) `complete_task` unconditionally set `status='done'` on any worker completion from the author lane (running/ready). `done` on this board means exactly one thing — the work was merged/accepted — so an author's end-of-lane completion landing there skips the review lane and the acceptance gate entirely. Code cards were only accidentally rescued: opening their PR fires the `github-prs` webhook, which moves the card to review. A board-driven review card (no PR-review webhook on its drafting step) had no such rescue, so its author's completion flipped it straight to `done` past the reviewer and past acceptance — a false-`done` that had to be reconciled by hand every cycle. Make the board-native path correct for every kind with a review lane. Before the `-> done` write, when the merge override is NOT set, the card is in the author lane (running/ready), and the card's own stamped owner map declares a `review` owner, MOVE the card to `status='review'` + that owner and emit a `status_changed` event instead of completing it. - No kind-default fallback: a card with no stamped review lane (legacy / un-stamped / plain task, research swarm root) completes to `done` exactly as before — the redirect never shunts an undeclared card. - Idempotent with the webhook path: once in `review` the card is no longer running/ready, so a second completion is a clean no-op and the `-> done` UPDATE cannot match it. - The acceptance-lane refusal, the merge override (`allow_acceptance_complete=True`), the hallucinated-cards gate, and `expected_run_id` atomicity are all preserved. Add `_review_owner_from_owner_map`, a reader that resolves `state_owners["review"]` from the card's submit-stage audit comment (the owner map lives in the audit trail, not a column), returning None when unstamped so the completion path is unchanged for such cards. <!-- card:t_a5a4fbf6 -->
…her (#58) * ✨ feat(kanban): team/kind-aware review-skill selection in the dispatcher The review-column dispatch force-loaded a single hardcoded review skill (sdlc-review, a code-review skill) for ANY review-status card, so a card whose review lane belongs to the writing team would be reviewed with a code lens instead of an editorial one. Add review_skills_for_card(): resolve the review-lane owner from the card's own stamped owner map (state_owners={...} in its submit-stage audit comment — the same signal the stage transition reads), falling back to the review card's assignee and then to the code-review default for legacy/un-stamped cards. A writing reviewer selects editorial-review; everything else keeps sdlc-review, so code cards are unchanged. No team/kind column is read or added — the owner map lives in the audit trail, keeping the selection cache-safe. Wire it into the review dispatch in place of the hardcoded skill list. Tests cover both selection paths (code -> sdlc, writing -> editorial), the assignee fallback, and the legacy default, plus end-to-end dispatch spawning with the correct skill. * 🐛 fix(kanban): scope review owner-map read to the submit-stage audit comment review_skills_for_card resolves the review-lane owner via _review_owner_from_owner_map, which previously scanned every comment for a state_owners={...} fragment. The reference reader it is asserted equivalent to (resolve_reviewer) is stricter: it filters to the submit-stage audit comment before parsing the map. The two readers only agreed by accident of comment order (the submit note sorts first). A later comment echoing a state_owners={...} fragment with a different review owner — or one preceding the submit note — could resolve a reviewer that disagrees with the lane the card was moved into, and thus the wrong review skill. Add a submit-stage guard (_SUBMIT_AUDIT_RE) so the dispatcher keys off the same authoritative comment as resolve_reviewer, making the two readers provably equivalent regardless of comment order. Add a regression test that stamps a conflicting owner map in a non-submit comment before the submit note and asserts the submit note wins.
…+ reference) (#60) Add a first-class `nano-banana` image_gen backend serving Google's Gemini image models (Nano Banana Pro / Nano Banana 2) through the local OpenAI-compatible proxy. It joins the existing ImageGenProvider framework and mirrors the shipped openrouter backend's chat-completions image protocol. Capability (v1, verified E2E against the live proxy): - Text-to-image, image editing (image_url source), and reference-image grounding (reference_image_urls, multiple, clamped to 3) for character/subject consistency. Local files are inlined as data-URI image_url parts. - Reads the generated image from choices[0].message.images[0].image_url.url (message.content is null on this protocol); POSTs to {base_url}/chat/completions with modalities:["image","text"]. - Pro (gemini-3-pro-image) is the quality default; Flash (gemini-3.1-flash-image) is a one-parameter fast path. Model routing is config-driven (kwarg > NANO_BANANA_IMAGE_MODEL env > image_gen.nano-banana.model > image_gen.model > default); Nano Banana 2 Lite is a documented catalog slot that drops in with zero code change when the proxy serves it. - image_config.aspect_ratio is honored on the generate path (1:1→1024x1024, 16:9→1376x768, 9:16→768x1376); edit-path dims track the source image. Auth is proxy/OpenAI-compatible by construction: credentials resolve via the shared runtime resolver against the vertex-llm-proxy custom_providers entry (overridable via image_gen.nano-banana.runtime). No Google credential is handled client-side. is_available() degrades gracefully (False, no crash) when the proxy or token is absent; generate() surfaces error_response for every failure mode. Also add a framework-level GC janitor over the shared cache dir ($HERMES_HOME/cache/images/, used by every backend). It runs opportunistically inside save_b64_image / save_url_image — so all backends get GC for free — pruning oldest-first when total size exceeds max_total_mb (default 2048) or a file's age exceeds max_age_days (default 30), never deleting the just-written file, emitting one INFO line per prune, best-effort (never fails a save). Caps are config-overridable under image_gen.cache in config.yaml.
) The outer feedback loop (feedback -> author on a card carrying an open PR) had no automatic active_pr respawn-guard-clearing path — only the inner review-bounce loop did. unblock_task matched only `status IN ('blocked','scheduled')`, so a card the block-loop breaker escalated to `triage` matched zero rows, returned False, and emitted no `unblocked` event. Because check_respawn_guard uses the latest `unblocked` event as the cutoff that clears the active_pr guard, a triaged card carrying an open-PR comment stayed respawn-guarded forever: the dispatcher refused to spawn the author every tick, and the standard block->unblock recovery silently no-oped from triage. Add `'triage'` to both `WHERE status IN (...)` clauses in unblock_task (the stale-run-pointer SELECT and the status UPDATE), so a triaged card transitions back to ready/todo (parent gate re-checked) and emits the `unblocked` cutoff, clearing active_pr exactly like a normal block->unblock. block_recurrences is deliberately still not reset, so the loop breaker is preserved — a genuine same-finding loop still escalates and the counter reset remains an explicit operator action. Reproduces the wedge RED-first (a triage card with an inflated counter and an open-PR comment stays active_pr-guarded), then GREEN. Adds behavior-contract tests: unblock from triage emits the cutoff and clears the guard; the parent gate is re-checked (undone parent -> todo); the loop counter survives the unblock. Syncs the now-stale unblock CLI comments/messages. Carried as a PATCHES.md row composing with the block-loop-breaker + reset-recovery rows.
…fault 4K) (#62) Add an output-resolution knob for the nano-banana Gemini image backend, config-only — no per-call parameter on the image_generate tool schema. image_gen.nano-banana.resolution (config.yaml) selects the size sent as image_config.image_size (1K/2K/4K, uppercase), defaulting to 4K. Verified against the live proxy that image_config.image_size is the field that changes output dimensions on the text-to-image path and composes with aspect_ratio (4K+16:9 -> 5504x3072, 4K+1:1 -> 4096x4096, 2K+16:9 -> 2752x1536, 1K+16:9 -> 1376x768). The proxy rejects image_config.resolution and ignores response_format.image_size, so only image_config.image_size is sent. Resolution precedence (first hit wins): NANO_BANANA_IMAGE_RESOLUTION env -> image_gen.nano-banana.resolution -> default 4K. Values are normalized to the uppercase ladder; an out-of-ladder value falls back to the default rather than 400ing the proxy. A per-model cap degrades gracefully (Lite = 1K clamps down and logs, instead of erroring), and if the proxy rejects image_size the request is retried once without it (current no-resolution behavior) so the generation still lands. An unrelated 400 (e.g. a safety block) is not masked by the fallback. Edits tend to preserve the source image's dimensions regardless, so this primarily affects text-to-image; documented in the backend docstring.
…generations (#63) gemini-3-pro-image silently returned ~1K (1408x768) regardless of the configured resolution because the chat/completions path has NO imageConfig->generationConfig mapping (verified against LiteLLM 1.91.2 and 1.92.0 source). LiteLLM maps resolution ONLY on /v1/images/generations. Route text-to-image to POST {base_url}/v1/images/generations with a NESTED imageConfig ({"imageSize": resolution, "aspectRatio": ratio}), which maps to Vertex generationConfig.imageConfig for pro/flash/lite (model-agnostic, no flag), so Pro (and all image models) honor 4K. Parse the images-API response (b64_json preferred, url fallback) and save it. Keep the EDIT / reference path on chat/completions: the images endpoint has no clean input-image contract at 1.92.0 (image input lives on the multipart /v1/images/edits route, unverified against this proxy), the chat edit path works today, and resolution matters less for edits since the model preserves source dimensions. The split is documented in the module docstring + comments. Preserve resolution-fallback resilience: if an older proxy rejects the nested imageConfig, retry once without it so generation still lands. Reuse _resolve_model/_resolve_resolution/_clamp_resolution and the aspect map unchanged; the Lite 1K cap still clamps on the images path. Tests mock the HTTP layer (no running proxy): assert text-to-image hits /v1/images/generations with the nested imageConfig carrying imageSize + aspectRatio, response parse (b64_json/url), imageConfig-rejection fallback, and that the edit/reference path still routes to chat/completions unchanged.
…dcoding .png (#64) save_b64_image hardcoded the `.png` extension regardless of the decoded payload, so a backend returning JPEG (Nano Banana Lite, gemini-3.1-flash-lite-image) was written as `*.png` — a mislabelled file that can confuse downstream consumers keying on the extension (Discord attachment sniffing, file tooling). Sniff the format from the payload's magic bytes (PNG/JPEG/WEBP/GIF, defaulting to png when unrecognised) and pick the extension from that. Extract a shared `_sniff_image_extension` helper and reuse it as save_url_image's final fallback so both helpers agree on detection. An explicit `extension=` arg still wins verbatim (leading dot normalised) for back-compat; all existing callers pass no extension and now get a truthful one. This is a generic framework fix benefiting every image_gen backend (nano-banana, openai, xai, fal, krea, openrouter, ...). No behavior change for PNG-returning models — Pro/Flash still save as `.png`.
…ve attachment (#65) * ✨ feat(gateway): auto-downscale oversized outbound images before native attachment Size-capped platforms silently drop an oversized native attachment: the message sends, the platform reports "Couldn't deliver the image attachment," and the file never arrives. A 4K render (Nano Banana Pro at 5504x3072) is routinely 15-20 MB, well past Discord's ~10 MB non-Nitro cap, so now that nano-banana defaults to 4K every such image sent to Discord is lost. Add an outbound image size cap + downscale-preview mechanism, mirroring the existing inbound media-size infrastructure in gateway/platforms/base.py: - get_outbound_image_max_bytes(platform): config-driven cap resolver reading gateway.max_outbound_image_bytes (global) and gateway.max_outbound_image_bytes_by_platform (per-platform override map). 0 disables; default 10 MiB (Discord non-Nitro), a safe conservative floor for platforms whose exact cap is unknown. - prepare_outbound_image(path, *, platform, max_bytes=None): when a local image exceeds the cap, write a downscaled JPEG preview into the image cache and return its path (long-edge/quality ladder: 2048/88 → 1568/82 → 1024/76 → 768/72 until it fits, else best-effort smallest). Preserves aspect ratio, never upscales, never mutates the source. Fail-open: any error returns the original path so a downscale bug can't block a delivery that would otherwise succeed. - BasePlatformAdapter.prepare_outbound_image_paths(): the single shared prep call every local-path image dispatch site funnels through, avoiding per-site drift. Wire it into the local-path image dispatch sites in gateway/run.py (primary post-stream batch + background-task send_image_file) and the base adapter's streaming batch path. Remote image URLs pass through untouched. The full-resolution original stays on disk; only the bytes sent to the platform change when over-cap. Generic across platforms, not Discord- or nano-banana-specific. Behavioral config lives in config.yaml (not a HERMES_* env var). Delivery-side companion to the inbound media cap and the outbound b64 format-sniff fix. Tests: 24 new tests covering the resolver (default/global/per-platform/ disabled/fail-open), the downscale helper (under-cap passthrough, over-cap preview under cap with aspect preserved, never-upscale, non-image/missing passthrough, PIL-failure fail-open), the shared prep helper, the dispatch contract (preview reaches the adapter, not the original), run.py wiring guards, and config defaults. Full gateway/platforms/config scope green (two pre-existing macOS env failures ruled out against the base). * 🐛 fix(gateway): wire remaining local-path image sends through outbound downscale The outbound downscale-preview helper only guarded the run.py and base.py streaming dispatch paths, leaving sibling local-path image send sites un-wired — an over-cap image shipped through those still silently dropped on size-capped platforms (Discord's ~10MB non-Nitro cap). Funnel every remaining local-path image send through prepare_outbound_image so the whole bug class is fixed, not just the reported sites: - kanban_watchers.py _deliver_kanban_artifacts: prep the file:// batch before send_multiple_images, so images shipped via completion artifacts are downscaled when over-cap (keyed by the delivery target's platform). - weixin.py: the adapter send() media path and both send_message-tool direct-send branches (live + fresh adapter). - yuanbao.py: the send_message-tool direct-send media path. Remote http(s) URL sends and the default send_multiple_images dispatcher (which unquotes an already-prepped file:// batch) are pass-through by design and left untouched. Fail-open preserved: prep returns the original path on any error, so a downscale bug can never block a delivery. Add a behavioral regression for the kanban artifact path (asserts the preview, not the oversized original, reaches the send when over-cap; the under-cap original passes through unchanged) plus source guards that the weixin/yuanbao/kanban paths reference the shared prep.
…ct is absent (#57) `done` on the board means exactly one thing: the declared reviewable artifact exists. A worker could build its deliverable in a git worktree, exit WITHOUT committing / pushing / opening a PR, and still have `kanban_complete` flip the card to `done` — a false state whose gated children auto-promote onto a foundation that does not exist. `complete_task` already guards two false-`done` classes before its write txn (phantom `created_cards`; the acceptance-lane park). This adds the third guard in the same shape: a required-artifact completion guard. - Signal (opt-in per card, zero schema footprint): `_card_requires_pr` returns True only for `workspace_kind == 'worktree'` (the sole kind that materializes an isolated linked git worktree on a branch — the implementer->PR shape) whose path is not anchored under `~/.hermes`. `scratch` and `dir` cards, and any `~/.hermes` edit-in-place workspace, are not PR-requiring and complete exactly as before. - Artifact check: `_card_has_pr_artifact` reuses the existing PR->card linkage — a resolvable `pull/<n>` URL in a task comment, matched by the same regex the `active_pr` respawn guard already trusts. - A guarded card with no PR is a clean no-op refusal: returns False, no task-state mutation, and an auditable `completion_refused_missing_pr` event with a `summary_preview` (mirrors `completion_refused_acceptance`). - The merge path (`allow_acceptance_complete=True`) bypasses the guard, exactly as it bypasses the acceptance guard. The guard lives inside `complete_task`, the single chokepoint every completion path calls (worker tool, CLI complete, swarm root helper, dashboard), so the whole class is covered by construction. Adds tests/hermes_cli/test_kanban_complete_missing_pr_guard.py (7 behavior-contract cases against a real temp kanban DB) and a PATCHES.md row.
…igible card (#67) The author-lane redirect in complete_task moves a running|ready author completion to the review lane instead of done — but only when the card's stamped owner map yields a review owner. A review-eligible card filed without a stage=submit owner map resolved None, the redirect precondition failed, and completion fell through to the -> done UPDATE with no signal: a code card with an open PR reaching done past the reviewer and past acceptance. done must mean "merged/accepted". Close the hole for the population identifiable without a card kind column (there is none — the owner map lives in the audit trail by design): when an author-lane completion cannot resolve a review owner AND the card is PR-requiring (an isolated git worktree owing a PR, the same _card_requires_pr signal the missing-PR guard uses), refuse with a clean no-op (return False, no state mutation) and an auditable completion_redirect_unresolved event — the same shape as the acceptance and missing-PR guards. Non-pipeline cards (scratch / dir / edit-in-place) with no review owner still complete to done unchanged. Also collapse the duplicate _review_owner_from_owner_map definition (and its duplicate owner-map regex) to one authoritative reader, removing a latent divergence bug. Adds behavior-contract regression tests (real imports, temp HERMES_HOME): an unstamped PR-requiring card refuses (not done) and emits the audit event; a stamped one still redirects to review; the merge override still reaches done; scratch/dir/edit-in-place cards still complete; the reader is defined once.
A clean bounce -> rework -> PASS -> acceptance cycle re-blocks a card with the SAME block kind as the earlier review bounce, so block_task's prev_kind == kind classifier counted the acceptance park as "the same failure repeating" and escalated a cleanly-accepted card to phantom triage once enough acceptance moves accumulated past BLOCK_RECURRENCE_LIMIT. An awaiting-casey-signoff block is a human sign-off park, not a failure. Add _is_acceptance_signoff_reason and force a fresh cause for such blocks in block_task so the loop counter resets to 1 and the card always lands in blocked for acceptance. Genuine failure/rework blocks are unaffected and still escalate to triage on a real same-cause loop.
…st event history (#69) A review-changes-requested bounce could sit blocked forever when the card's author was unresolvable from event history. auto_route_review_bounce resolved the author only via _resolve_review_author, which matches the assigned {from, to} event shape emitted by the one-card move helper. When the review move instead recorded the assigned {assignee: X} shape (a non-move_card reassignment), author resolution returned None and the card was left parked for a human — regardless of block kind, so a needs_input-kinded bounce with a review-changes-requested reason never routed to the author. Prefer the card's stamped state_owners[ready] owner (the implementing author) when resolving the routing target, falling back to event-history resolution for legacy / un-stamped cards. The owner map is authoritative and independent of the assigned-event shape, so the whole review-bounce routing class is fixed: the router matches on the reason prefix (any block kind) and now always finds the author from the map. A block whose reason is not the review-bounce prefix still parks for the human. Extract _owner_from_owner_map(conn, task_id, lane); _review_owner_from_owner_map delegates to it (still defined once) and a new _ready_owner_from_owner_map reads the ready lane. Behavior-contract tests: a needs_input bounce with an unresolvable event-history author routes via the owner map; a needs_input block with a non-bounce reason stays parked.
A delegate_task child inherits the parent's enabled toolsets AND the parent's HERMES_KANBAN_TASK env. A child that kept the kanban toolset could call kanban_complete with no explicit task_id, which falls back to HERMES_KANBAN_TASK and resolves the PARENT's card — falsely flipping it to done with the child's summary while the real deliverable was never produced (false-done, control-surface-bleed). Add "kanban" to the composite toolsets stripped from every child in _strip_blocked_tools, alongside delegation and code_execution. The strip is unconditional — orchestrator children re-gain delegation by role but never regain kanban, so no delegated agent holds card-terminal authority bound to its parent's card.
…70) A reviewer PASS parked the card for sign-off in two separate steps: a status flip to blocked and an assign to the acceptance owner. When only one half landed, the card stranded in review/owner — the reviewer's lane with the acceptance owner's name on it — and, because the acceptance notification rides the blocked event, no ping fired. Hand-reconciled via a housekeeping safety net after the fact. Add accept_task: a single atomic primitive that, in ONE write_txn, flips status to blocked, sets assignee to the acceptance owner, ends the reviewer's run, and emits the blocked event carrying the awaiting-casey-signoff reason. The reviewer PASS path (kanban_block with that reason) resolves the acceptance owner from the card's own owner map — profile-agnostic, no name baked into core — and routes to accept_task, so the transition can never land half-applied. A block whose reason is not the acceptance sign-off, or a card with no stamped acceptance owner, falls through to the normal block path unchanged. Behavior-contract tests cover the atomic landing, the single sign-off event, run/claim release, reason normalization, refusal off a non-review card, the unclaimed-review-lane path, and the reviewer toolset routing.
…dback bounce (#72) Routing an accepted card back to its author on outer-loop feedback had no code path that cleared the active_pr respawn guard. A raw move_card blocked/acceptance -> ready emits only status_changed, never the unblocked cutoff event check_respawn_guard honors, so the guard stayed active_pr (the PR is genuinely open) and the dispatcher refused to spawn the author every tick. Repeated churn inflated block_recurrences past BLOCK_RECURRENCE_LIMIT and escalated the card to triage, where a naive block->unblock cutoff silently no-ops. Add route_feedback_to_author, a sanctioned caller-driven primitive that performs the whole outer-loop bounce atomically, composed from existing building blocks so the inner review-bounce loop, the reviewer PASS -> acceptance path (accept_task), and the merge -> done path are all unchanged: 1. reset_block_recurrences (frees a triage card, prevents re-escalation), 2. reassign to the author (assigned event), fenced on a transitionable lane, 3. unblock_task (emits the unblocked cutoff AFTER the PR-URL comment ts, which is what clears the active_pr / recent_success guard), 4. recompute_ready + an audit comment naming the PR + feedback. Recovers a card from blocked, acceptance (blocked + acceptance owner), and triage. Idempotent: a second call on an already-routed card is a clean no-op. Behavior-contract tests assert the guard clears (check_respawn_guard is None), the card is spawnable via the real dispatch_once path, the triage counter resets, and a reverted guard-clear leaves active_pr and fails.
…ane (#74) `complete_task`'s author-lane redirect only refused fall-through to `done` when `_card_requires_pr(...)` was True, and that helper is True ONLY for `workspace_kind == 'worktree'`. A `dir`-workspace card that already owns an open PR — but has no stamped `state_owners` review owner — was invisible to that guard: `_review_owner_from_owner_map` returned None, the refusal branch did not match, and control fell through to the `-> done` UPDATE, landing the card in `done` past the reviewer and past acceptance. Broaden the refusal branch to also fire when `_card_has_pr_artifact(conn, task_id)` is True (a resolvable `pull/<n>` URL in a comment — the same signal the active_pr respawn guard trusts). A card that carries an open PR is review-eligible regardless of `workspace_kind`, so it must hand off to review or be refused, never fall through to a generic `done`. Cards that are neither PR-requiring nor own a PR (plain scratch tasks, dir builds with no PR, ~/.hermes edit-in-place cards) still complete to `done` unchanged. The merge path (`allow_acceptance_complete=True`) bypasses this guard as it does the acceptance and missing-PR guards. Adds regression coverage for a dir-workspace card owning a PR: refused when unstamped (returns False, status unchanged, `completion_redirect_unresolved` emitted), still redirects to review when stamped, and still completes to `done` under the merge override.
…-done (#75) An edit-in-place / no-PR card (workspace 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. detect_crashed_workers then misread that verb-less rc=0 exit as a protocol_violation, tripped failure_limit=1, and emitted a false gave_up, stranding a fully-completed card in blocked for a human to hand-reconcile. Root cause: _lane_work_provably_done — the helper both the clean_exit_after_done carve-out and check_respawn_guard trust — accepted exactly two proofs (a completed-run row in the success window, and a PR URL in a recent comment). A no-PR edit-in-place clean exit satisfies neither: it opens no PR (fails proof 2), and the verb-less exit is precisely why no outcome='completed' run row was written (fails proof 1). So the carve-out never fired for exactly the card shape that legitimately exits verb-less. Add a third proof, proof-gated and scoped to the no-PR shape via the shared _card_requires_pr predicate (not a new ad-hoc definition): for a card _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) 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, so this 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 unchanged). No new config, no new schema, no user-facing env var. Adds 3 behavior-contract tests (real path, temp HERMES_HOME): 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.
…it-after-done (#76) A code-author card whose worker opens a PR, pushes, and exits rc=0 without calling a terminal kanban verb reaches detect_crashed_workers' clean-exit- after-done branch: _lane_work_provably_done is True (a PR URL in a recent comment), so the reap correctly declines to count a failure. But it only RELEASED the card to `ready`, which is a dead end for a PR-open card — the active_pr respawn guard holds an open-PR card out of respawn (dup-PR risk) WITHOUT advancing it, so the card wedged in running->ready until an orchestrator hand-staged it to review. Fix: when the provably-done signal is a PR handoff (a pull/<n> URL via _card_has_pr_artifact) AND the card's submit-stage owner map declares a review owner, MOVE the card running->review + that reviewer atomically with the reap, emitting the same status_changed running->review event shape complete_task's author-lane handoff emits, and recording the run as completed. This is exactly the handoff the worker's clean exit skipped. The two other clean-exit-after-done shapes are unchanged: a completed-run proof with no PR, a no-PR edit-in-place card (the prior no-PR fix), and a PR card with no resolvable owner-map reviewer all still release as a benign no-op to ready/review. The auto-advance is gated on both a PR artifact and a resolvable owner-map reviewer, so it narrows to exactly the code-author-opened-a-PR case. Adds behavior/E2E tests exercising both shapes against a temp HERMES_HOME: a PR-open code card with an owner map lands in review + the reviewer with the transition events, and a PR card without an owner map falls back to the benign release-to-ready.
…utoff comment (#77) The active_pr respawn guard wedged a ready card that legitimately reuses an open PR for a revision round (the outer feedback loop). After a correct block -> route_feedback_to_author -> unblock (which emits the unblocked cutoff event), check_respawn_guard still returned active_pr every tick, so the dispatcher refused to spawn the author for hours (live 2026-07-22, PR NousResearch#143). Root cause: the 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 comment naming the existing PR URL at the same second as the unblocked event it emits. Kanban timestamps are second-granular, so that 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. The prior carve-out excluded only the literal 'dispatcher' author; it did not cover the orchestrator/recovery actors that drive the outer loop -- crucially route_feedback_to_author, which stamps its PR-URL audit comment with author='orchestrator'. Fix: _RESPAWN_GUARD_RECOVERY_AUTHOR_PREFIXES now covers (dispatcher, hollis, onecard, orchestrator) and excludes any comment whose author prefix-matches that set from the dup-PR scan (author NOT LIKE '<name>%', covering profile-suffixed variants). 'orchestrator' is the author the real outer-loop primitive emits, so a same-second reopen audit no longer re-trips the guard. None of these actors ever opens a PR, so this is analogous to the review-status carve-out. A genuine builder/author PR-URL comment at/after the cutoff is not in the set and still re-arms active_pr, so duplicate-PR protection is preserved; the review-status skip and closed/merged fail-open behavior are untouched. Single-file change plus behavior-contract regression tests, including one driven through route_feedback_to_author itself so the test cannot drift from the real recovery emitter. Full test_kanban_db.py and the kanban surface green, 0 regressions.
…r wedges active_pr
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". `_resolve_pr_state` failed open to "unknown" on
ANY nonzero exit, and the active_pr respawn guard treats "unknown" as still-
active, so a phantom PR URL in a task comment guarded the card on every tick
indefinitely (dozens of respawn_guarded {active_pr} events, no worker ever
spawned).
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).
Add a terminal "not_found" state: _resolve_pr_state returns it when gh exits
nonzero AND stderr carries the not-found signature (_PR_NOT_FOUND_STDERR_RE);
every other nonzero exit still returns "unknown" (fail-open preserved).
check_respawn_guard's dup-PR decision consults a single-source-of-truth
_RESPAWN_GUARD_INACTIVE_PR_STATES = {closed, merged, not_found}, so a
not-found PR no longer guards while open/unknown still do.
Tests: 3 new behavior-contract tests (not-found stderr maps to not_found; a
generic nonzero exit still maps to unknown; a not-found PR-URL comment from a
real builder clears the guard). Also makes the route-feedback fixture stub
_resolve_pr_state->open so its placeholder PR URL is deterministic instead of
resolving not_found against live GitHub.
cwest
marked this pull request as ready for review
July 23, 2026 22:03
Owner
Author
|
The fix separates a definitive not-found PR from a transient gh failure at exactly the right seam. The guard wiring is the only consumer of the resolver, and it now reads The test-fixture stub in the route-feedback suite is warranted, not scope creep: those tests stage a placeholder Ran the touched surface in a clean checkout at the head SHA: the three new contract tests pass, the full kanban_db suite is 283 passed, and the route-feedback file is 9 passed. Checks are green and the PR is mergeable. No changes needed. |
Owner
Author
|
Superseded: the branch had accumulated 68 commits from its base. The single behavioral change was rebuilt cleanly onto current integration and merged. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A PR URL that GitHub definitively does not have — one that was never created, or was deleted — makes
gh pr viewexit nonzero with the GraphQL signatureCould not resolve to a PullRequest._resolve_pr_statefailed open to"unknown"on any nonzero exit, and theactive_prrespawn guard treats"unknown"as still-active (conservative). So a phantom/stale PR URL sitting in a task comment guarded the card on every dispatcher tick, indefinitely — dozens ofrespawn_guarded {active_pr}events with no worker ever spawning.This is an orthogonal residual hole not covered by the earlier
active_prfixes (NousResearch#46204 explicit-unblock cutoff, NousResearch#46549 review-lane bypass, #77 recovery-actor re-arming comment): all of those still route through_resolve_pr_state, which fails open on the phantom-PR case.Root cause
Conflating a definitive not-found (terminal — a nonexistent PR can never be active work) with a transient
ghfailure (network / auth / gh-missing — genuinely unresolvable, must stay fail-open).Fix
_PR_NOT_FOUND_STDERR_RE(could not resolve to a pullrequest, case-insensitive)._resolve_pr_statereturns a new terminal state"not_found"whenghexits nonzero and stderr carries that signature. Every other nonzero exit still returns"unknown"— fail-open behavior is preserved for genuine transient failures.check_respawn_guard's dup-PR decision now consults a single-source-of-truth_RESPAWN_GUARD_INACTIVE_PR_STATES = frozenset({"closed", "merged", "not_found"}). A not-found PR no longer guards, exactly like a closed/merged one;openandunknownstill guard.Minimal, single-file production change in
hermes_cli/kanban_db.py.Tests
RED-then-GREEN, in
tests/hermes_cli/test_kanban_db.py:_resolve_pr_statemaps the GraphQL not-found stderr →not_found.unknown(fail-open intact).None.Plus one hermetic-fixture fix in
tests/hermes_cli/test_kanban_route_feedback_to_author.py: thekanban_homefixture now stubs_resolve_pr_state→"open", because its placeholder PR URL is non-existent and would otherwise now resolvenot_foundand clear the very guard those routing tests set out to exercise. The routing logic under test is orthogonal to live PR-state resolution.Verification
Real
kanban_db, tempHERMES_HOME, no mocks. Isolated kanban surface green (310 passed across the four core kanban test files run per-file). Zero new failures — the ~26 cross-test-pollution failures seen in a whole--k kanbansingle-process run reproduce identically on the clean base branch and are unrelated to this change (base: 892 passed / 26 failed; this branch: 895 passed / 26 failed = exactly the 3 new tests added).PATCHES.mdupdated with the carried-patch manifest row (permanent-local; fork-internal dispatcher invariant).