From 9345c67854f6ef1ef4f68ac4b9c3ae266ecceccf Mon Sep 17 00:00:00 2001 From: emozilla Date: Fri, 18 Sep 2026 14:37:08 -0400 Subject: [PATCH 001/190] fix(webhook): per-route toolsets bind to the authenticated route, not a split of chat_id (GHSA-2fmg-cjqm-hhrj) toolsets_for_source recovered the route for the per-route toolsets grant by splitting the session chat_id "webhook:{route}:{delivery_id}" on ":", while authentication used the exact URL segment. A route named "build:external" therefore resolved to route "build" and inherited its toolsets: a caller holding the weak route's HMAC secret got the privileged sibling's terminal/ file tools. Reproduced on main through the real aiohttp handler with a signed request. Key the lookup on source.user_id instead, which _dispatch_agent_run already stamps as exactly "webhook:{route_name}" from the authenticated segment. No split at all: delivery_id is caller-supplied (X-GitHub-Delivery, svix-id, X-Request-ID), so any parse of chat_id, including rsplit, stays attacker- influenced. Routes without ":" resolve identically before and after; a ":" route with no colliding sibling now gets the toolsets it configured, where the old code silently fell back to the platform default. Tests go through the HTTP handler, HMAC, and the gateway resolver: the ":" route gets its own toolsets (red on base), and a crafted delivery id on the privileged route cannot name another route (pins against a future rsplit). Reported-by: pinarsadioglu (GHSA-2fmg-cjqm-hhrj) --- gateway/platforms/webhook.py | 10 ++-- tests/gateway/test_webhook_route_toolsets.py | 61 ++++++++++++++++++++ 2 files changed, 67 insertions(+), 4 deletions(-) diff --git a/gateway/platforms/webhook.py b/gateway/platforms/webhook.py index c8005a0df3..a9c6fa514d 100644 --- a/gateway/platforms/webhook.py +++ b/gateway/platforms/webhook.py @@ -323,11 +323,13 @@ class WebhookAdapter(BasePlatformAdapter): def toolsets_for_source(self, source) -> Optional[List[str]]: """Per-route ``toolsets`` override (config.yaml or a manual key in webhook_subscriptions.json — deliberately NOT settable via `hermes webhook subscribe`, so an agent-created subscription - cannot self-grant tools).""" - parts = str(getattr(source, "chat_id", "") or "").split(":", 2) - if len(parts) < 2 or parts[0] != "webhook": + cannot self-grant tools). Keyed on ``user_id`` (exactly ``webhook:{route}`` as authenticated), not + ``chat_id``, whose caller-supplied delivery id and ``:``-bearing route names make any split ambiguous + (GHSA-2fmg-cjqm-hhrj).""" + user_id = str(getattr(source, "user_id", "") or "") + if not user_id.startswith("webhook:"): return None - route_config = self._routes.get(parts[1]) + route_config = self._routes.get(user_id[len("webhook:"):]) toolsets = route_config.get("toolsets") if isinstance(route_config, dict) else None if not isinstance(toolsets, list): return None diff --git a/tests/gateway/test_webhook_route_toolsets.py b/tests/gateway/test_webhook_route_toolsets.py index d1a19f07f3..a830444f47 100644 --- a/tests/gateway/test_webhook_route_toolsets.py +++ b/tests/gateway/test_webhook_route_toolsets.py @@ -5,8 +5,22 @@ platform-level ``platform_toolsets.webhook`` resolution for runs triggered by that route only. The gateway validates the override through the same ``_get_platform_tools`` path as platform config, so restricted/unknown names behave identically to a manually configured platform toolset list. + +The grant must bind to the route whose secret authenticated the request +(GHSA-2fmg-cjqm-hhrj): route names may contain ``:`` and the delivery id in the +session key is caller-supplied, so nothing may be recovered by splitting ``chat_id``. """ +import asyncio +import hashlib +import hmac +import json + +import pytest +from aiohttp import web +from aiohttp.test_utils import TestClient, TestServer + +from gateway.config import PlatformConfig from gateway.platforms.base import BasePlatformAdapter from gateway.platforms.webhook import WebhookAdapter from gateway.run import GatewayRunner @@ -16,6 +30,8 @@ from hermes_cli.tools_config import _get_platform_tools class _Src: def __init__(self, chat_id): self.chat_id = chat_id + # What _dispatch_agent_run stamps: exactly the authenticated route, no delivery id. + self.user_id = chat_id.rsplit(":", 1)[0] if chat_id.startswith("webhook:") else None def _make_adapter(routes): @@ -135,3 +151,48 @@ class TestGatewayResolveEnabledToolsetsForSource: gr, cfg, _Src("webhook:mon:d"), "webhook" ) assert cfg["platform_toolsets"]["webhook"] == ["web"] + + +class TestToolsetsBindToAuthenticatedRoute: + """Regression for GHSA-2fmg-cjqm-hhrj, end to end through the real HTTP handler.""" + + ROUTES = { + "build": {"secret": "strong-secret", "toolsets": ["terminal", "file"]}, + "build:external": {"secret": "weak-secret", "toolsets": ["web"], "prompt": "{text}"}, + } + + async def _dispatch(self, path, secret, delivery_id): + adapter = WebhookAdapter(PlatformConfig( + enabled=True, extra={"host": "127.0.0.1", "port": 0, "routes": self.ROUTES})) + captured = [] + + async def _capture(event): + captured.append(event.source) + + adapter.handle_message = _capture + app = web.Application() + app.router.add_post("/webhooks/{route_name}", adapter._handle_webhook) + body = json.dumps({"text": "hi"}).encode() + sig = "sha256=" + hmac.new(secret.encode(), body, hashlib.sha256).hexdigest() + async with TestClient(TestServer(app)) as cli: + resp = await cli.post(path, data=body, headers={ + "X-Hub-Signature-256": sig, "Content-Type": "application/json", + "X-GitHub-Delivery": delivery_id}) + assert resp.status == 202 + await asyncio.sleep(0.05) + assert len(captured) == 1 + gr = _make_runner(adapter) + return GatewayRunner._resolve_enabled_toolsets_for_source(gr, BASE_CONFIG, captured[0], "webhook") + + @pytest.mark.asyncio + async def test_colon_route_gets_its_own_toolsets_not_its_prefix_routes(self): + res = await self._dispatch("/webhooks/build:external", "weak-secret", "d1") + assert "web" in res + assert "terminal" not in res and "file" not in res + + @pytest.mark.asyncio + async def test_caller_supplied_delivery_id_cannot_name_another_route(self): + # Delivery id is attacker-controlled; a right-split would read "build:external" here. + res = await self._dispatch("/webhooks/build", "strong-secret", "external:d1") + assert "terminal" in res and "file" in res + assert "web" not in res From a2ed82c88d946529264af49059b171f8d4107ce8 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Fri, 18 Sep 2026 01:55:50 -0700 Subject: [PATCH 002/190] fix(desktop): keep the message reaction picker open while the pointer moves toward it (#114130) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clicking the reaction button on an assistant message opened the picker, but the very next pointer movement over the transcript closed it, so no emoji could ever be chosen. The group-hover / PopoverAnchor mechanism in the report is not the cause: the anchor is already kept live while open (styles.css `[data-state='open']`) and Radix does not close on a hidden anchor. The picker died to the floating composer's focus-follow: Radix autofocuses the popover content when it opens, `trackPointer` pulled that focus back into the pane composer on `pointermove`, and the popover's `onFocusOutside` dismissed it. Guard `focusSelectedComposer()` — the chokepoint every focus-follow branch funnels through — against focus that sits inside an open floating layer (popover, menu, listbox, dialog, popper wrapper). The selector is the one `composer-focus-keys` already uses for its overlay gate, hoisted to the leaf `combo.ts` as `OVERLAY_SURFACE` so both consumers share it. Tests: focus-follow leaves focus inside a floating layer (with the plain-control control case), and the assistant reaction picker survives a pointermove over its message with a registered floating composer. Both red on origin/main. --- .../floating-target-floating-layer.test.ts | 66 +++++++++++++++++++ .../src/app/chat/composer/floating-target.ts | 14 +++- .../reaction-picker-focus-follow.test.tsx | 65 ++++++++++++++++++ apps/desktop/src/lib/keybinds/combo.ts | 7 ++ .../src/lib/keybinds/composer-focus-keys.ts | 5 +- 5 files changed, 151 insertions(+), 6 deletions(-) create mode 100644 apps/desktop/src/app/chat/composer/floating-target-floating-layer.test.ts create mode 100644 apps/desktop/src/components/assistant-ui/thread/reaction-picker-focus-follow.test.tsx diff --git a/apps/desktop/src/app/chat/composer/floating-target-floating-layer.test.ts b/apps/desktop/src/app/chat/composer/floating-target-floating-layer.test.ts new file mode 100644 index 0000000000..e2433bebcc --- /dev/null +++ b/apps/desktop/src/app/chat/composer/floating-target-floating-layer.test.ts @@ -0,0 +1,66 @@ +// @vitest-environment jsdom +import { afterEach, describe, expect, it } from 'vitest' + +import { registerFloatingComposer } from './floating-target' + +/** The owner's composer host, a chat surface with a plain control, and a + * body-portaled Radix popper layer holding a control — the shapes the + * window-level focus-follow has to tell apart. */ +function mount() { + const host = document.createElement('div') + host.dataset.composerOwner = 'surface-1' + const editor = document.createElement('div') + editor.dataset.slot = 'composer-rich-input' + editor.tabIndex = -1 + host.appendChild(editor) + document.body.appendChild(host) + + const surface = document.createElement('div') + surface.dataset.chatSurface = '' + surface.dataset.composerSurfaceId = 'surface-1' + const plainButton = document.createElement('button') + surface.appendChild(plainButton) + document.body.appendChild(surface) + + const layer = document.createElement('div') + layer.setAttribute('data-radix-popper-content-wrapper', '') + const layerButton = document.createElement('button') + layer.appendChild(layerButton) + document.body.appendChild(layer) + + return { editor, layerButton, plainButton, surface } +} + +/** Button-up movement over the chat surface — every hover of the transcript. */ +function movePointerOver(target: Element, x: number) { + target.dispatchEvent(new PointerEvent('pointermove', { bubbles: true, buttons: 0, clientX: x, clientY: 40 })) +} + +/** Radix moves focus into a popover/menu when it opens and dismisses it as soon + * as focus leaves. The focus-follow used to pull that focus back into the pane + * composer on the next pointermove, so the message reaction picker closed + * before the pointer could reach it. */ +describe('floating composer focus-follow vs an open floating layer', () => { + let unregister: (() => void) | undefined + + afterEach(() => { + unregister?.() + unregister = undefined + document.body.innerHTML = '' + }) + + it('leaves focus inside a floating layer, but still follows the pointer otherwise', () => { + const { editor, layerButton, plainButton, surface } = mount() + unregister = registerFloatingComposer('surface-1', { groupId: 'g1', target: 'main' }) + + layerButton.focus() + movePointerOver(surface, 10) + expect(document.activeElement).toBe(layerButton) + + // Control: focus on an ordinary control in the surface is not "owned" — + // hovering the transcript hands the caret to the composer as before. + plainButton.focus() + movePointerOver(surface, 20) + expect(document.activeElement).toBe(editor) + }) +}) diff --git a/apps/desktop/src/app/chat/composer/floating-target.ts b/apps/desktop/src/app/chat/composer/floating-target.ts index 6bf5f2451a..4fe29bf1d9 100644 --- a/apps/desktop/src/app/chat/composer/floating-target.ts +++ b/apps/desktop/src/app/chat/composer/floating-target.ts @@ -1,7 +1,7 @@ import { flushSync } from 'react-dom' import { $activeTreeGroup, $hoveredTreeGroup, noteActiveTreeGroup } from '@/components/pane-shell/tree/store' -import { isEditableTarget } from '@/lib/keybinds/combo' +import { isEditableTarget, OVERLAY_SURFACE } from '@/lib/keybinds/combo' import { $composerPopout } from '@/store/composer-popout' import { $floatingComposerOwner, type FloatingComposerOwner } from './floating-state' @@ -32,6 +32,13 @@ const inInlineEdit = (el: Element | null) => Boolean(el?.closest(EDIT_COMPOSER_R const keepsOwnFocus = (el: Element | null) => inInlineEdit(el) || (isEditableTarget(el) && !el?.closest('[data-slot="composer-rich-input"]')) +/** Focus inside an open floating layer — a popover, menu, listbox or dialog + * portaled over the transcript — is the user's own as well. Radix moves focus + * into such a layer when it opens and dismisses it the moment focus leaves, so + * pulling the caret back to the pane composer on the very next pointermove + * closed the message reaction picker before the pointer could reach it. */ +const inFloatingLayer = (el: Element | null) => Boolean(el?.closest(OVERLAY_SURFACE)) + function rememberCaret(editor: EventTarget | null) { const selection = window.getSelection() @@ -66,11 +73,12 @@ function selectionOutsideComposer(): boolean { } /** Every focus-follow branch (pointermove and focusin) funnels here, so the - * selection guard lives at this chokepoint rather than at one call site. */ + * selection and floating-layer guards live at this chokepoint rather than at + * one call site. */ function focusSelectedComposer() { const owner = $floatingComposerOwner.get() - if (!owner || selectionOutsideComposer()) { + if (!owner || selectionOutsideComposer() || inFloatingLayer(document.activeElement)) { return } diff --git a/apps/desktop/src/components/assistant-ui/thread/reaction-picker-focus-follow.test.tsx b/apps/desktop/src/components/assistant-ui/thread/reaction-picker-focus-follow.test.tsx new file mode 100644 index 0000000000..dc3f1c804c --- /dev/null +++ b/apps/desktop/src/components/assistant-ui/thread/reaction-picker-focus-follow.test.tsx @@ -0,0 +1,65 @@ +import { AssistantRuntimeProvider, type ThreadMessage, useExternalStoreRuntime } from '@assistant-ui/react' +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' + +import { registerFloatingComposer } from '@/app/chat/composer/floating-target' +import { $reactionsEnabled } from '@/store/reactions-enabled' + +import { assistantMessage, stubThreadEnvironment } from '../test-utils' + +import { Thread } from '.' + +stubThreadEnvironment() + +/** The transcript inside a chat surface that owns a floating composer — the + * shape of every chat pane, and the one the window-level focus-follow acts on. */ +function Harness() { + const runtime = useExternalStoreRuntime({ + messages: [assistantMessage()], + isRunning: false, + onNew: async () => {} + }) + + return ( + +
+ +
+
+
+
+ + ) +} + +let unregister: (() => void) | undefined + +beforeEach(() => { + $reactionsEnabled.set(true) + unregister = registerFloatingComposer('surface-1', { groupId: 'g1', target: 'main' }) +}) + +afterEach(() => { + unregister?.() + unregister = undefined + cleanup() + $reactionsEnabled.set(false) +}) + +describe('assistant reaction picker', () => { + it('stays open while the pointer moves over the message toward it', async () => { + render() + + const message = (await screen.findByText('done')).closest('[data-slot="aui_assistant-message-root"]') + const trigger = message?.querySelector('[data-slot="aui_msg-reactions"]') + + expect(trigger).toBeTruthy() + fireEvent.click(trigger!) + expect(await screen.findByRole('button', { name: '👍' })).toBeTruthy() + + fireEvent.pointerMove(message!, { buttons: 0, clientX: 40, clientY: 40 }) + + expect(screen.queryByRole('button', { name: '👍' })).not.toBeNull() + expect(trigger?.getAttribute('data-state')).toBe('open') + }) +}) diff --git a/apps/desktop/src/lib/keybinds/combo.ts b/apps/desktop/src/lib/keybinds/combo.ts index d5cedeeb90..4222816e42 100644 --- a/apps/desktop/src/lib/keybinds/combo.ts +++ b/apps/desktop/src/lib/keybinds/combo.ts @@ -235,6 +235,13 @@ export function isFocusWithin(selector: string): boolean { return document.activeElement?.closest(selector) != null } +// Overlays that cover the whole window (portaled to the body, or the overlay +// shell itself): dialogs, menus, listboxes, every Radix popper layer. One +// anywhere means the composer is behind it — its keys, and any focus the +// user has inside it, are the overlay's own. +export const OVERLAY_SURFACE = + '[role="dialog"],[role="alertdialog"],[role="menu"],[role="listbox"],[data-radix-popper-content-wrapper],[data-overlay-surface]' + // True when focus is in a text-entry surface, so bare-key shortcuts don't fire // while the user is typing. export function isEditableTarget(target: EventTarget | null): boolean { diff --git a/apps/desktop/src/lib/keybinds/composer-focus-keys.ts b/apps/desktop/src/lib/keybinds/composer-focus-keys.ts index 8cc7d7bdcd..b56ad899fd 100644 --- a/apps/desktop/src/lib/keybinds/composer-focus-keys.ts +++ b/apps/desktop/src/lib/keybinds/composer-focus-keys.ts @@ -11,7 +11,7 @@ import { queryAllVisible } from '@/components/pane-shell/pane-visibility' import { $activeTreeGroup, $hoveredTreeGroup } from '@/components/pane-shell/tree/store' import { switcherActive } from '@/store/session-switcher' -import { isEditableTarget, isFocusWithin } from './combo' +import { isEditableTarget, isFocusWithin, OVERLAY_SURFACE } from './combo' /** `composer.focus` defaults that need the surface/target gate. */ export const isComposerFocusSoftCombo = (combo: string) => combo === '/' || combo === 'enter' @@ -41,8 +41,7 @@ const ENTER_ACTIVATES = [ // Overlays that cover the whole window (portaled to the body, or the overlay // shell itself) — one anywhere means the composer is behind it. -const BLOCKING_OVERLAY = - '[role="dialog"],[role="alertdialog"],[role="menu"],[role="listbox"],[data-radix-popper-content-wrapper],[data-overlay-surface]' +const BLOCKING_OVERLAY = OVERLAY_SURFACE // Blockers that live INSIDE a chat surface. Inactive tabs stay mounted, so this // one has to be visible-scoped: a clarify card waiting in a background thread From 9d5a076c4c81763f9259cf26d735054c5a6ef9ac Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Fri, 18 Sep 2026 04:50:36 -0700 Subject: [PATCH 003/190] refactor(desktop): read OVERLAY_SURFACE directly in composer-focus-keys The hoist into combo.ts left a BLOCKING_OVERLAY alias behind for its single use site; drop the alias. --- apps/desktop/src/lib/keybinds/composer-focus-keys.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/apps/desktop/src/lib/keybinds/composer-focus-keys.ts b/apps/desktop/src/lib/keybinds/composer-focus-keys.ts index b56ad899fd..1c01f3ec4c 100644 --- a/apps/desktop/src/lib/keybinds/composer-focus-keys.ts +++ b/apps/desktop/src/lib/keybinds/composer-focus-keys.ts @@ -39,10 +39,6 @@ const ENTER_ACTIVATES = [ '[role="treeitem"]' ].join(',') -// Overlays that cover the whole window (portaled to the body, or the overlay -// shell itself) — one anywhere means the composer is behind it. -const BLOCKING_OVERLAY = OVERLAY_SURFACE - // Blockers that live INSIDE a chat surface. Inactive tabs stay mounted, so this // one has to be visible-scoped: a clarify card waiting in a background thread // must not take the foreground composer's letter keys. @@ -155,7 +151,7 @@ export function composerFocusBlockedBySurface(): boolean { switcherActive() || $workspaceIsPage.get() || isFocusWithin('[data-terminal]') || - Boolean(document.querySelector(BLOCKING_OVERLAY)) + Boolean(document.querySelector(OVERLAY_SURFACE)) ) } From e1374093aa07ec794e10297773031b59da33b4af Mon Sep 17 00:00:00 2001 From: John Paul Soliva Date: Thu, 17 Sep 2026 14:02:07 +0900 Subject: [PATCH 004/190] fix(desktop): never register a turn lease that nothing can release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A routed prompt holds one lease per (route, runtime session) until the turn settles, and for a streaming turn only a Secondary's own terminal-event listener ends it: releaseTerminalTurnLease invokes `g.turnLeases.get(key)?.()`, so a lease that is not the mapped one is never called at all. Two ways a hold ends up in that state, both closed here. The function already skips the primary profile, and its comment says why: a no-op lease leaves a phantom key that suppresses the real hold if the route is later re-homed as a secondary. But the primary profile is not the only route served by the primary socket. retainGatewayForAgent returns a no-op for a shared-remote collapse and for a build with no registry dialing, and gatewayForProfile returns one for a shared-primary route; none of them creates a Secondary. A lease registered on any of those is stored and can never be released — and because the map is keyed per session, it silently suppresses every later hold on that session, including after the route becomes a real pooled secondary. The shared-remote probe explicitly expects that transition: it prefers the primary "until a later probe can prove isolation". The next turn then runs with no hold at all, so the socket is free to be reaped mid-turn and the gateway interrupts the turn as client_gone, which is the failure this mechanism exists to prevent. The duplicate-lease guard is also a check-then-act: it runs before the retain awaits and the map is written after it, so two submits for one (route, session) can both pass. The loser's hold is then orphaned — its release is never the mapped one, so activeRequests stays above zero and the pooled socket can never be reclaimed. Both are decided by outcome rather than by re-probing: no pooled entry for the scope means nothing can release the key, and a key already present means someone else owns the turn. Either way the route is released and the caller gets a no-op. These are the function's first tests. One takes a lease while the route rides the primary, leaves it unreleased as a streaming turn would, then makes registry dialing available and asserts the next turn really holds a socket. The other races two submits across the dial, fires the terminal event the gateway's own listener would, and asserts the socket is then reclaimable. Each fails without its own guard. --- .../gateway-connection-lifecycle.test.ts | 73 ++++++++++++++++++- apps/desktop/src/store/gateway.ts | 25 +++++++ 2 files changed, 97 insertions(+), 1 deletion(-) diff --git a/apps/desktop/src/store/gateway-connection-lifecycle.test.ts b/apps/desktop/src/store/gateway-connection-lifecycle.test.ts index 7bab024916..b712b897a9 100644 --- a/apps/desktop/src/store/gateway-connection-lifecycle.test.ts +++ b/apps/desktop/src/store/gateway-connection-lifecycle.test.ts @@ -17,6 +17,7 @@ const gatewayMocks = vi.hoisted(() => { return { connect: vi.fn(async (_wsUrl: string): Promise => undefined), + eventHandlers: [] as ((event: unknown) => void)[], instances } }) @@ -37,7 +38,11 @@ vi.mock('@/hermes', () => ({ await gatewayMocks.connect(wsUrl) this.connectionState = 'open' } - onEvent = vi.fn(() => () => {}) + onEvent = vi.fn((handler: (event: unknown) => void) => { + gatewayMocks.eventHandlers.push(handler) + + return () => {} + }) onState = vi.fn(() => () => {}) constructor() { gatewayMocks.instances.push(this as never) @@ -67,6 +72,7 @@ const { pruneSecondaryGateways, reconnectSecondaryGateways, retainGatewayForAgent, + retainGatewayForSessionTurn, retireLocalProfileGateways, setPrimaryGateway } = await import('./gateway') @@ -94,11 +100,76 @@ beforeEach(() => { afterEach(() => { closeSecondaryGateways() gatewayMocks.instances.length = 0 + gatewayMocks.eventHandlers.length = 0 vi.clearAllMocks() vi.useRealTimers() delete (window as unknown as { hermesDesktop?: unknown }).hermesDesktop }) +describe('retainGatewayForSessionTurn', () => { + it('lets the next turn take a real hold after a turn that rode the primary socket', async () => { + // A routed prompt holds one lease per (route, runtime session) until the turn settles, and for a + // streaming turn only a Secondary's terminal-event listener ends it. When the route is served by + // the primary socket there is no Secondary, so a lease registered then can never be released — + // and the map is keyed per session, so it silently suppresses every later hold on that session, + // including after the route is dialed as a real secondary. The socket is then free to be reaped + // mid-turn, which is the interruption this whole mechanism exists to prevent. + installDesktop({ getConnection: vi.fn() }) // no getConnectionFor: nothing to hold, no Secondary + + // The streaming turn deliberately does NOT release: its release would arrive as a terminal event. + await retainGatewayForSessionTurn('homelab', 'writer', 'session-1') + + installDesktop({ + getConnection: vi.fn(), + getConnectionFor: vi.fn(async ({ connectionId, profile }: { connectionId: string; profile: string }) => + descriptorFor(connectionId, profile) + ) + }) + const dialedBefore = gatewayMocks.instances.length + + await retainGatewayForSessionTurn('homelab', 'writer', 'session-1') + + expect(gatewayMocks.instances.length).toBeGreaterThan(dialedBefore) + }) + + it('does not orphan a hold when two submits for one session race the dial', async () => { + // The duplicate-lease guard runs BEFORE the retain awaits, and the map is written after it, so + // two submits for the same (route, session) can both pass. Only the mapped release is ever + // invoked — releaseTerminalTurnLease does `g.turnLeases.get(key)?.()` — so the loser's hold is + // never released and the socket can never be reclaimed. + let openDial = () => {} + + const dialed = new Promise(resolve => { + openDial = resolve + }) + + gatewayMocks.connect.mockImplementationOnce(async () => dialed) + installDesktop({ + getConnectionFor: vi.fn(async ({ connectionId, profile }: { connectionId: string; profile: string }) => + descriptorFor(connectionId, profile) + ) + }) + + const both = Promise.all([ + retainGatewayForSessionTurn('homelab', 'writer', 'session-2'), + retainGatewayForSessionTurn('homelab', 'writer', 'session-2') + ]) + + openDial() + await both + + // The terminal event releases the one lease the map holds, exactly as the gateway's own + // listener does; a leaked second hold would keep activeRequests above zero. + for (const handler of gatewayMocks.eventHandlers) { + handler({ session_id: 'session-2', type: 'session.reclaimed' }) + } + + pruneSecondaryGateways(new Set()) + + expect(gatewayMocks.instances[0].close).toHaveBeenCalled() + }) +}) + describe('a redial of the active route', () => { it('is not cancelled by a prune that runs while it is dialing', async () => { // Editing the connection you are viewing defers the redial until its lease drops, then diff --git a/apps/desktop/src/store/gateway.ts b/apps/desktop/src/store/gateway.ts index 27a134f455..81c49a688c 100644 --- a/apps/desktop/src/store/gateway.ts +++ b/apps/desktop/src/store/gateway.ts @@ -1500,6 +1500,31 @@ export async function retainGatewayForSessionTurn( } const releaseRoute = await retainGatewayForAgent(connectionId, profile) + + // Only a Secondary's own terminal-event listener releases this lease, so a route with no + // Secondary can never release one: retainGatewayForAgent and gatewayForProfile both hand back a + // no-op exactly when the route rides the primary socket (shared-remote collapse, shared-primary + // route, or a build without registry dialing), and none of those creates an entry. Storing the + // key there leaves the same phantom the primary-profile guard above avoids, and it would suppress + // the real hold once that route IS dialed as a secondary — which the shared-remote probe + // explicitly expects ("prefer the primary until a later probe can prove isolation"). Decide by + // outcome rather than re-probing every no-op case. + if (!g.secondaries.has(scope)) { + releaseRoute() + + return () => undefined + } + + // Re-check after the await: the guard above the retain ran before it, so a second submit for the + // same (route, session) can arrive while this one suspends and both pass it. Only the release + // stored in the map is ever invoked — releaseTerminalTurnLease does `g.turnLeases.get(key)?.()` — + // so the loser's hold would never be released and the socket could never be reclaimed. + if (g.turnLeases.has(key)) { + releaseRoute() + + return () => undefined + } + let released = false const release = () => { From 8bbc02a02a4cecaae5a9b0cff4c6f6c30d5cf325 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Fri, 18 Sep 2026 01:57:27 -0700 Subject: [PATCH 005/190] test(desktop): move the turn-lease tests to the end of the lifecycle file Keeps the block off the shared anchor after afterEach that the sibling prune-redial fix also extends, so the two PRs merge independently. --- .../gateway-connection-lifecycle.test.ts | 128 +++++++++--------- 1 file changed, 64 insertions(+), 64 deletions(-) diff --git a/apps/desktop/src/store/gateway-connection-lifecycle.test.ts b/apps/desktop/src/store/gateway-connection-lifecycle.test.ts index b712b897a9..6b2c7fa382 100644 --- a/apps/desktop/src/store/gateway-connection-lifecycle.test.ts +++ b/apps/desktop/src/store/gateway-connection-lifecycle.test.ts @@ -106,70 +106,6 @@ afterEach(() => { delete (window as unknown as { hermesDesktop?: unknown }).hermesDesktop }) -describe('retainGatewayForSessionTurn', () => { - it('lets the next turn take a real hold after a turn that rode the primary socket', async () => { - // A routed prompt holds one lease per (route, runtime session) until the turn settles, and for a - // streaming turn only a Secondary's terminal-event listener ends it. When the route is served by - // the primary socket there is no Secondary, so a lease registered then can never be released — - // and the map is keyed per session, so it silently suppresses every later hold on that session, - // including after the route is dialed as a real secondary. The socket is then free to be reaped - // mid-turn, which is the interruption this whole mechanism exists to prevent. - installDesktop({ getConnection: vi.fn() }) // no getConnectionFor: nothing to hold, no Secondary - - // The streaming turn deliberately does NOT release: its release would arrive as a terminal event. - await retainGatewayForSessionTurn('homelab', 'writer', 'session-1') - - installDesktop({ - getConnection: vi.fn(), - getConnectionFor: vi.fn(async ({ connectionId, profile }: { connectionId: string; profile: string }) => - descriptorFor(connectionId, profile) - ) - }) - const dialedBefore = gatewayMocks.instances.length - - await retainGatewayForSessionTurn('homelab', 'writer', 'session-1') - - expect(gatewayMocks.instances.length).toBeGreaterThan(dialedBefore) - }) - - it('does not orphan a hold when two submits for one session race the dial', async () => { - // The duplicate-lease guard runs BEFORE the retain awaits, and the map is written after it, so - // two submits for the same (route, session) can both pass. Only the mapped release is ever - // invoked — releaseTerminalTurnLease does `g.turnLeases.get(key)?.()` — so the loser's hold is - // never released and the socket can never be reclaimed. - let openDial = () => {} - - const dialed = new Promise(resolve => { - openDial = resolve - }) - - gatewayMocks.connect.mockImplementationOnce(async () => dialed) - installDesktop({ - getConnectionFor: vi.fn(async ({ connectionId, profile }: { connectionId: string; profile: string }) => - descriptorFor(connectionId, profile) - ) - }) - - const both = Promise.all([ - retainGatewayForSessionTurn('homelab', 'writer', 'session-2'), - retainGatewayForSessionTurn('homelab', 'writer', 'session-2') - ]) - - openDial() - await both - - // The terminal event releases the one lease the map holds, exactly as the gateway's own - // listener does; a leaked second hold would keep activeRequests above zero. - for (const handler of gatewayMocks.eventHandlers) { - handler({ session_id: 'session-2', type: 'session.reclaimed' }) - } - - pruneSecondaryGateways(new Set()) - - expect(gatewayMocks.instances[0].close).toHaveBeenCalled() - }) -}) - describe('a redial of the active route', () => { it('is not cancelled by a prune that runs while it is dialing', async () => { // Editing the connection you are viewing defers the redial until its lease drops, then @@ -1029,3 +965,67 @@ it('does not let a removed connection repopulate the auth rejection', async () = await expect(requestGatewayForAgent('cloud', 'default', 'session.list')).rejects.toThrow() expect(getGatewayWsUrlFor).toHaveBeenCalledTimes(2) }) + +describe('retainGatewayForSessionTurn', () => { + it('lets the next turn take a real hold after a turn that rode the primary socket', async () => { + // A routed prompt holds one lease per (route, runtime session) until the turn settles, and for a + // streaming turn only a Secondary's terminal-event listener ends it. When the route is served by + // the primary socket there is no Secondary, so a lease registered then can never be released — + // and the map is keyed per session, so it silently suppresses every later hold on that session, + // including after the route is dialed as a real secondary. The socket is then free to be reaped + // mid-turn, which is the interruption this whole mechanism exists to prevent. + installDesktop({ getConnection: vi.fn() }) // no getConnectionFor: nothing to hold, no Secondary + + // The streaming turn deliberately does NOT release: its release would arrive as a terminal event. + await retainGatewayForSessionTurn('homelab', 'writer', 'session-1') + + installDesktop({ + getConnection: vi.fn(), + getConnectionFor: vi.fn(async ({ connectionId, profile }: { connectionId: string; profile: string }) => + descriptorFor(connectionId, profile) + ) + }) + const dialedBefore = gatewayMocks.instances.length + + await retainGatewayForSessionTurn('homelab', 'writer', 'session-1') + + expect(gatewayMocks.instances.length).toBeGreaterThan(dialedBefore) + }) + + it('does not orphan a hold when two submits for one session race the dial', async () => { + // The duplicate-lease guard runs BEFORE the retain awaits, and the map is written after it, so + // two submits for the same (route, session) can both pass. Only the mapped release is ever + // invoked — releaseTerminalTurnLease does `g.turnLeases.get(key)?.()` — so the loser's hold is + // never released and the socket can never be reclaimed. + let openDial = () => {} + + const dialed = new Promise(resolve => { + openDial = resolve + }) + + gatewayMocks.connect.mockImplementationOnce(async () => dialed) + installDesktop({ + getConnectionFor: vi.fn(async ({ connectionId, profile }: { connectionId: string; profile: string }) => + descriptorFor(connectionId, profile) + ) + }) + + const both = Promise.all([ + retainGatewayForSessionTurn('homelab', 'writer', 'session-2'), + retainGatewayForSessionTurn('homelab', 'writer', 'session-2') + ]) + + openDial() + await both + + // The terminal event releases the one lease the map holds, exactly as the gateway's own + // listener does; a leaked second hold would keep activeRequests above zero. + for (const handler of gatewayMocks.eventHandlers) { + handler({ session_id: 'session-2', type: 'session.reclaimed' }) + } + + pruneSecondaryGateways(new Set()) + + expect(gatewayMocks.instances[0].close).toHaveBeenCalled() + }) +}) From 3aa76b2ce51a484d7e09b0aadab38b32190ed16d Mon Sep 17 00:00:00 2001 From: KoNit-K Date: Thu, 17 Sep 2026 08:12:48 +0800 Subject: [PATCH 006/190] fix(kanban): report failed auto-heartbeats --- tests/tools/test_delegate_kanban_isolation.py | 26 +++++++++++++++++++ tools/kanban_tools.py | 10 ++++--- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/tests/tools/test_delegate_kanban_isolation.py b/tests/tools/test_delegate_kanban_isolation.py index 9d053d0570..960ae29864 100644 --- a/tests/tools/test_delegate_kanban_isolation.py +++ b/tests/tools/test_delegate_kanban_isolation.py @@ -174,6 +174,32 @@ def test_delegate_child_execute_code_env_bridges_contextvar_and_scrubs_kanban( assert env["HERMES_KANBAN_WORKSPACE"] == str(tmp_path / "parent-workspace") +def test_auto_heartbeat_reports_failure_without_mutating_fenced_child_board( + monkeypatch, + tmp_path, +): + """An inherited child marker fences both bridge writes instead of faking success.""" + kb, tid, _workspace, _attachments_root = _make_running_kanban_task(monkeypatch, tmp_path) + from hermes_cli import kanban_db_connect as kbc + from tools import kanban_tools + + conn = kbc.connect() + try: + task_before = kb.get_task(conn, tid) + events_before = kb.list_events(conn, tid) + monkeypatch.setenv("HERMES_DELEGATED_CHILD_CONTEXT", str(tmp_path / ".hermes")) + monkeypatch.setattr(kanban_tools, "_auto_heartbeat_last_attempt", 0.0) + + assert kanban_tools.heartbeat_current_worker_from_env() is False + + task_after = kb.get_task(conn, tid) + assert task_after.claim_expires == task_before.claim_expires + assert task_after.last_heartbeat_at == task_before.last_heartbeat_at + assert kb.list_events(conn, tid) == events_before + finally: + conn.close() + + def test_delegate_child_kanban_cli_cannot_delete_parent_board( monkeypatch, tmp_path, diff --git a/tools/kanban_tools.py b/tools/kanban_tools.py index 6390577f96..3fd9a58ced 100644 --- a/tools/kanban_tools.py +++ b/tools/kanban_tools.py @@ -450,8 +450,8 @@ _auto_heartbeat_last_attempt: float = 0.0 def heartbeat_current_worker_from_env() -> bool: - """Claim extension + board heartbeat for the current worker; True iff a write was - attempted. ``HERMES_KANBAN_RUN_ID`` pins the run row so a reclaimed stale run is not + """Claim extension + board heartbeat for the current worker; True iff both writes + succeed. ``HERMES_KANBAN_RUN_ID`` pins the run row so a reclaimed stale run is not heartbeated; ``HERMES_KANBAN_CLAIM_LOCK`` absent -> default claimer (local workers).""" global _auto_heartbeat_last_attempt tid = os.environ.get("HERMES_KANBAN_TASK") @@ -464,13 +464,15 @@ def heartbeat_current_worker_from_env() -> bool: with _board(None, quiet_close=True) as (kb, conn): ops = ((kb.heartbeat_claim, {"claimer": os.environ.get("HERMES_KANBAN_CLAIM_LOCK")}), (kbd.heartbeat_worker, {"note": None, "expected_run_id": _worker_run_id(tid)})) + succeeded = True for fn, kwargs in ops: op = fn.__name__ try: - fn(conn, tid, **kwargs) + succeeded = bool(fn(conn, tid, **kwargs)) and succeeded except Exception: logger.debug("auto-heartbeat: %s failed", op, exc_info=True) - return True + succeeded = False + return succeeded except Exception: logger.debug("auto-heartbeat: bridge failed", exc_info=True) return False From 63fb1a7d36d3593ea636bc8abb0d928cae33f1f0 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Fri, 18 Sep 2026 00:44:46 -0700 Subject: [PATCH 007/190] fix(kanban): warn once when an inherited delegation fence rejects the worker's auto-heartbeat A worker process that carries HERMES_DELEGATED_CHILD_CONTEXT next to its HERMES_KANBAN_TASK is fenced by kanban_path_is_fenced's env-marker branch: both auto-heartbeat writes raised PermissionError at DEBUG only, so the board showed a worker that never beats while the process was alive (the salvaged commit already made the return value honest, but _touch_activity ignores it). Log the refusal once per process at WARNING with the cause, and skip the bridge for an in-process delegate child BEFORE stamping the rate-limit window so a chatty child cannot starve the worker's own heartbeat. No self-write grant is added: the marker + TASK combination means "descendant, not worker" by design (b578261584e, 8a8c3634e8d), and the only in-tree grant path, kanban_db_dispatch._default_spawn, already pops the marker. The real-spawn grant test now proves that from a dispatcher that itself carries the marker: the worker's auto-heartbeat lands and its handoff completes. Docs: worker guide notes the automatic claim extension and what the refusal warning means. --- tests/cron/test_cron_kanban_env_isolation.py | 12 ++++++++--- tests/tools/test_delegate_kanban_isolation.py | 21 +++++++++++++++++-- tools/kanban_tools.py | 21 ++++++++++++++++++- website/docs/user-guide/features/kanban.md | 2 ++ 4 files changed, 50 insertions(+), 6 deletions(-) diff --git a/tests/cron/test_cron_kanban_env_isolation.py b/tests/cron/test_cron_kanban_env_isolation.py index 9e8badc95f..a177de9805 100644 --- a/tests/cron/test_cron_kanban_env_isolation.py +++ b/tests/cron/test_cron_kanban_env_isolation.py @@ -400,17 +400,23 @@ def test_dispatcher_grants_only_the_assigned_worker_scope(tmp_path, monkeypatch) root = str(Path(__file__).resolve().parents[2]) worker.write_text( f"#!{sys.executable}\nimport sys, os, json;sys.path.insert(0, {root!r})\n" - "from tools.kanban_tools import _handle_complete\n" - f"result=_handle_complete({{'summary':'assigned worker'}});open({str(output)!r}, 'w').write(result)\n" + "from tools.kanban_tools import _handle_complete, heartbeat_current_worker_from_env\n" + "beat=heartbeat_current_worker_from_env()\n" + f"result=json.loads(_handle_complete({{'summary':'assigned worker'}}));result['beat']=beat\n" + f"open({str(output)!r}, 'w').write(json.dumps(result))\n" ) worker.chmod(0o700) monkeypatch.setenv("HERMES_BIN", str(worker)) # Building a new worker under an existing task must replace, not inherit, its scope. monkeypatch.setenv("HERMES_KANBAN_TASK", "prior-task") + # A dispatcher launched from an agent's shell carries the descendant fence itself; the worker it + # grants a task to must not (an inherited marker fences the worker's own heartbeat + handoff). + monkeypatch.setenv("HERMES_DELEGATED_CHILD_CONTEXT", str(tmp_path)) pid = _default_spawn(task, str(tmp_path), board="default") assert pid is not None os.waitpid(pid, 0) # windows-footgun: ok — Linux-only real dispatcher spawn - assert json.loads(output.read_text())["ok"] + result = json.loads(output.read_text()) + assert result["ok"] and result["beat"] is True, result assert kb.get_task(conn, tid).status == "done" assert os.environ["HERMES_KANBAN_TASK"] == "prior-task" conn.close() diff --git a/tests/tools/test_delegate_kanban_isolation.py b/tests/tools/test_delegate_kanban_isolation.py index 960ae29864..28658cdfb1 100644 --- a/tests/tools/test_delegate_kanban_isolation.py +++ b/tests/tools/test_delegate_kanban_isolation.py @@ -2,6 +2,7 @@ from __future__ import annotations import json +import logging import os import shlex import sys @@ -177,9 +178,13 @@ def test_delegate_child_execute_code_env_bridges_contextvar_and_scrubs_kanban( def test_auto_heartbeat_reports_failure_without_mutating_fenced_child_board( monkeypatch, tmp_path, + caplog, ): - """An inherited child marker fences both bridge writes instead of faking success.""" + """An inherited child marker fences both bridge writes instead of faking success, and says + so once at WARNING (the DEBUG-only refusal hid a starving claim for a day). An in-process + delegate child stays fenced too, quietly, without consuming the worker's heartbeat window.""" kb, tid, _workspace, _attachments_root = _make_running_kanban_task(monkeypatch, tmp_path) + from agent.delegation_context import delegated_child_context from hermes_cli import kanban_db_connect as kbc from tools import kanban_tools @@ -189,8 +194,20 @@ def test_auto_heartbeat_reports_failure_without_mutating_fenced_child_board( events_before = kb.list_events(conn, tid) monkeypatch.setenv("HERMES_DELEGATED_CHILD_CONTEXT", str(tmp_path / ".hermes")) monkeypatch.setattr(kanban_tools, "_auto_heartbeat_last_attempt", 0.0) + monkeypatch.setattr(kanban_tools, "_auto_heartbeat_fence_warned", False) - assert kanban_tools.heartbeat_current_worker_from_env() is False + with caplog.at_level(logging.WARNING, logger="tools.kanban_tools"): + assert kanban_tools.heartbeat_current_worker_from_env() is False + monkeypatch.setattr(kanban_tools, "_auto_heartbeat_last_attempt", 0.0) + assert kanban_tools.heartbeat_current_worker_from_env() is False + fence_warnings = [r for r in caplog.records if "HERMES_DELEGATED_CHILD_CONTEXT" in r.getMessage()] + assert len(fence_warnings) == 1 and tid in fence_warnings[0].getMessage() + + monkeypatch.delenv("HERMES_DELEGATED_CHILD_CONTEXT") + monkeypatch.setattr(kanban_tools, "_auto_heartbeat_last_attempt", 0.0) + with delegated_child_context("child-1"): + assert kanban_tools.heartbeat_current_worker_from_env() is False + assert kanban_tools._auto_heartbeat_last_attempt == 0.0 task_after = kb.get_task(conn, tid) assert task_after.claim_expires == task_before.claim_expires diff --git a/tools/kanban_tools.py b/tools/kanban_tools.py index 3fd9a58ced..3292eadd38 100644 --- a/tools/kanban_tools.py +++ b/tools/kanban_tools.py @@ -447,17 +447,22 @@ def _goal_gate(tool_name: str, task, tid: str, evidence: str) -> None: # for the explicit tool which carries a model-supplied note. _AUTO_HEARTBEAT_MIN_INTERVAL_SECONDS = 60.0 _auto_heartbeat_last_attempt: float = 0.0 +_auto_heartbeat_fence_warned = False def heartbeat_current_worker_from_env() -> bool: """Claim extension + board heartbeat for the current worker; True iff both writes succeed. ``HERMES_KANBAN_RUN_ID`` pins the run row so a reclaimed stale run is not heartbeated; ``HERMES_KANBAN_CLAIM_LOCK`` absent -> default claimer (local workers).""" - global _auto_heartbeat_last_attempt + global _auto_heartbeat_last_attempt, _auto_heartbeat_fence_warned tid = os.environ.get("HERMES_KANBAN_TASK") now = time.monotonic() if not tid or (now - _auto_heartbeat_last_attempt) < _AUTO_HEARTBEAT_MIN_INTERVAL_SECONDS: return False + if _is_delegated_child_context(): + # An in-process delegate child's activity is not the worker's liveness; checked before + # stamping the window so a chatty child cannot starve the worker's own heartbeat. + return False _auto_heartbeat_last_attempt = now try: from hermes_cli import kanban_db_dispatch as kbd @@ -469,6 +474,20 @@ def heartbeat_current_worker_from_env() -> bool: op = fn.__name__ try: succeeded = bool(fn(conn, tid, **kwargs)) and succeeded + except PermissionError as exc: + # The board fence rejected the worker's own liveness write: this process + # inherited HERMES_DELEGATED_CHILD_CONTEXT next to HERMES_KANBAN_TASK, so it is + # a delegate descendant, not the dispatcher's worker (kanban_complete refuses + # too). Loud once: at DEBUG the board just showed a worker that never beats. + succeeded = False + if not _auto_heartbeat_fence_warned: + _auto_heartbeat_fence_warned = True + logger.warning( + "kanban auto-heartbeat for task %s refused (%s): this process carries " + "HERMES_DELEGATED_CHILD_CONTEXT together with HERMES_KANBAN_TASK, so the board " + "treats it as a delegate_task descendant and its claim will not be extended by " + "activity. Only the dispatcher's own spawn grants worker scope; do not copy a " + "worker's environment into a hand-launched process.", tid, exc) except Exception: logger.debug("auto-heartbeat: %s failed", op, exc_info=True) succeeded = False diff --git a/website/docs/user-guide/features/kanban.md b/website/docs/user-guide/features/kanban.md index 96e36a702f..837799590e 100644 --- a/website/docs/user-guide/features/kanban.md +++ b/website/docs/user-guide/features/kanban.md @@ -553,6 +553,8 @@ Every profile that works kanban tasks automatically gets the worker lifecycle 3. Call `kanban_heartbeat(note="...")` every few minutes during long operations. **If your work may run longer than 1 hour, call `kanban_heartbeat` at least once an hour** — the dispatcher reclaims tasks that have been running past `kanban.dispatch_stale_timeout_seconds` (default 4 h) with no heartbeat in the last hour, on the assumption the worker crashed without cleanup. A reclaim is benign (the task goes back to `ready` for re-dispatch without a failure-counter tick) but you lose your current run's progress. 4. Complete with `kanban_complete(summary="...", metadata={...})`, hand a code change off for same-card review with `kanban_request_review(summary="...")`, or `kanban_block(reason="...")` if stuck. +Normal tool activity also extends the claim automatically (the worker mirrors its in-process liveness onto the board about once a minute). That bridge only works for a process the dispatcher spawned itself: a process that carries `HERMES_DELEGATED_CHILD_CONTEXT` next to `HERMES_KANBAN_TASK` (a `delegate_task` descendant, or a hand-launched copy of a worker's environment) is fenced from the board — its auto-heartbeat logs one `kanban auto-heartbeat for task … refused` warning and `kanban_complete` / `kanban_request_review` refuse. Fix the launch (let the dispatcher spawn the worker) rather than exporting the marker away. + That final terminal board call (`kanban_complete` / `kanban_request_review` / `kanban_block`; reviewers end with `kanban_complete` or `kanban_request_changes`) is part of the worker From e3e086689973a8cc78591cf9b7c053e910733bcf Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Fri, 18 Sep 2026 04:02:58 -0700 Subject: [PATCH 008/190] fix(kanban): fence-warned flag reset must not mask the heartbeat assertion `monkeypatch.setattr(kanban_tools, "_auto_heartbeat_fence_warned", False)` raised AttributeError against a module without the flag, so a regression that dropped the warn-once flag failed on the attribute probe instead of the user-visible symptom (`heartbeat_current_worker_from_env()` returning True). `raising=False` lets the symptom assertion bite (main's kanban_tools.py: AssertionError `assert True is False`). --- tests/tools/test_delegate_kanban_isolation.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/tools/test_delegate_kanban_isolation.py b/tests/tools/test_delegate_kanban_isolation.py index 28658cdfb1..71bc8a10da 100644 --- a/tests/tools/test_delegate_kanban_isolation.py +++ b/tests/tools/test_delegate_kanban_isolation.py @@ -194,7 +194,9 @@ def test_auto_heartbeat_reports_failure_without_mutating_fenced_child_board( events_before = kb.list_events(conn, tid) monkeypatch.setenv("HERMES_DELEGATED_CHILD_CONTEXT", str(tmp_path / ".hermes")) monkeypatch.setattr(kanban_tools, "_auto_heartbeat_last_attempt", 0.0) - monkeypatch.setattr(kanban_tools, "_auto_heartbeat_fence_warned", False) + # raising=False: a regression that drops the module flag must fail on the + # symptom assertions below, not on this attribute probe. + monkeypatch.setattr(kanban_tools, "_auto_heartbeat_fence_warned", False, raising=False) with caplog.at_level(logging.WARNING, logger="tools.kanban_tools"): assert kanban_tools.heartbeat_current_worker_from_env() is False From 8f6f9e5b36da7435e26b79e7b35a4f235eb175ce Mon Sep 17 00:00:00 2001 From: funky-xamarin <30426178+Wenfengcheng@users.noreply.github.com> Date: Wed, 16 Sep 2026 15:18:50 +0800 Subject: [PATCH 009/190] fix(cron): order status next run by actual instant --- hermes_cli/cron.py | 14 +++++- tests/hermes_cli/test_cron_status_next_run.py | 47 +++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) create mode 100644 tests/hermes_cli/test_cron_status_next_run.py diff --git a/hermes_cli/cron.py b/hermes_cli/cron.py index 02c7a94a18..4de42aa0dd 100644 --- a/hermes_cli/cron.py +++ b/hermes_cli/cron.py @@ -466,10 +466,20 @@ def _print_active_jobs_summary(jobs) -> None: if not jobs: print(" No active jobs") return - next_runs = [j.get("next_run_at") for j in jobs if j.get("next_run_at")] + from datetime import timezone + from cron.jobs import _parse_aware + + next_runs = [] + for job in jobs: + raw = job.get("next_run_at") + parsed = _parse_aware(raw) + if parsed is not None: + # A shared ZoneInfo compares wall times across a DST fold; use UTC + # for ordering, but keep the stored timestamp for display. + next_runs.append((parsed.astimezone(timezone.utc), raw)) print(f" {len(jobs)} active job(s)") if next_runs: - print(f" Next run: {min(next_runs)}") + print(f" Next run: {min(next_runs, key=lambda run: run[0])[1]}") # Post-downtime late fires show at status level, not just per-job in `cron list`. late = [j for j in jobs if isinstance(j.get("last_dispatch"), dict) and j["last_dispatch"].get("kind") in ("late", "catch_up")] diff --git a/tests/hermes_cli/test_cron_status_next_run.py b/tests/hermes_cli/test_cron_status_next_run.py new file mode 100644 index 0000000000..b73370499a --- /dev/null +++ b/tests/hermes_cli/test_cron_status_next_run.py @@ -0,0 +1,47 @@ +"""Cron status orders persisted next runs by instant, including a DST fold.""" + +from datetime import datetime +from zoneinfo import ZoneInfo + +import pytest + +from cron import jobs as job_store +from hermes_cli.cron import _print_active_jobs_summary + + +@pytest.mark.parametrize("later,earlier", [ + ("2026-11-01T01:15:00-05:00", "2026-11-01T01:45:00-04:00"), + ("2026-11-02T09:00:00-05:00", "2026-11-02T08:00:00-05:00"), + ("2026-11-02T08:00:00-05:00", "2026-11-02T12:30:00+00:00"), +]) +def test_status_earliest_persisted_instant(later, earlier, monkeypatch, capsys): + # Only freeze the clock: creation, persistence, normalization and listing are real. + now = datetime(2026, 10, 31, tzinfo=ZoneInfo("America/New_York")) + monkeypatch.setattr(job_store, "_hermes_now", lambda: now) + late_job = job_store.create_job(prompt="later", schedule=later) + early_job = job_store.create_job(prompt="earlier", schedule=earlier) + jobs = job_store.list_jobs() + assert {job["id"] for job in jobs} == {late_job["id"], early_job["id"]} + expected = next(job["next_run_at"] for job in jobs if job["id"] == early_job["id"]) + + _print_active_jobs_summary(jobs) + + out = capsys.readouterr().out + assert "2 active job(s)" in out + assert f"Next run: {expected}\n" in out + assert job_store.list_jobs() == jobs # Rendering never reschedules a job. + + +@pytest.mark.parametrize("jobs,expected", [ + ([], " No active jobs\n"), + ([{}, {"next_run_at": None}, {"next_run_at": ""}], " 3 active job(s)\n"), + ([{"next_run_at": "bad"}, {"next_run_at": 123}], " 2 active job(s)\n"), + ([{"next_run_at": "bad"}, {"next_run_at": "2026-11-01T06:15:00Z"}], + " 2 active job(s)\n Next run: 2026-11-01T06:15:00Z\n"), + ([{"next_run_at": "2026-11-01T06:15:00Z"}, + {"next_run_at": "2026-11-01T01:15:00-05:00"}], + " 2 active job(s)\n Next run: 2026-11-01T06:15:00Z\n"), +]) +def test_status_missing_invalid_and_equivalent_instants(jobs, expected, capsys): + _print_active_jobs_summary(jobs) + assert capsys.readouterr().out == expected From a20a71a3e3f1a507b40b94ce48784faeccc4a97f Mon Sep 17 00:00:00 2001 From: liuhao1024 Date: Fri, 18 Sep 2026 02:14:35 +0800 Subject: [PATCH 010/190] fix(cli): flag overdue next_run_at and stale ticker in cron status When the scheduler host dies, a job's next_run_at is stranded in the past and `hermes cron status` still printed it as an upcoming "Next run", hiding the outage (#114309): the report's only signal was the gateway line, while the schedule line kept presenting a 7h-old timestamp as future. - _print_active_jobs_summary: when the earliest next_run_at already passed, print a loud OVERDUE line (age + scheduler question) instead of a bare "Next run: "; future timestamps render unchanged. - cron_status gateway-down branch: when ticker_heartbeat is stale, print when the scheduler last ticked so the frozen state is first-class visible. Fixes #114309 --- hermes_cli/cron.py | 36 ++++++++++++++++++++++++++- tests/hermes_cli/test_cron.py | 46 +++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/hermes_cli/cron.py b/hermes_cli/cron.py index 4de42aa0dd..fc15702fc5 100644 --- a/hermes_cli/cron.py +++ b/hermes_cli/cron.py @@ -4,6 +4,7 @@ import contextlib import json import re import sys +from datetime import datetime, timezone from pathlib import Path from typing import Any, Dict, Iterable, List, Optional @@ -96,6 +97,21 @@ def _dispatch_kind_label(kind) -> Optional[str]: return {"catch_up": "catch-up after missed fire", "late": "late"}.get(kind) +def _next_run_overdue_seconds(next_run_at: str) -> Optional[float]: + """Seconds the timestamp has already been in the past; None if malformed. + + next_run_at is written as timezone-aware ISO strings; a naive value only ever comes + from a hand-edited jobs.json and is read as UTC. + """ + try: + dt = datetime.fromisoformat(str(next_run_at).replace("Z", "+00:00")) + except ValueError: + return None + if dt.tzinfo is None: + dt = dt.replace(tzinfo=timezone.utc) + return (datetime.now(timezone.utc) - dt).total_seconds() + + def _dispatch_display(dispatch: dict) -> Optional[str]: """One-line scheduled-vs-actual dispatch summary; None when the stamp is malformed. @@ -442,6 +458,15 @@ def cron_status(): else: print(color("✗ Gateway is not running — cron jobs will NOT fire", Colors.RED)) active = get_active_profile_name() + # When scheduling last worked before the host went away: without this, a + # 7h-overdue job still reads as a normal upcoming "Next run" (#114309). + with contextlib.suppress(Exception): + from cron.jobs import TICKER_INTERVAL_SECONDS, get_ticker_heartbeat_age + hb_age = get_ticker_heartbeat_age() + if hb_age is not None and hb_age > TICKER_INTERVAL_SECONDS * 3 + 20: + print(color(" Scheduler last ticked " + f"{_format_lateness(hb_age)} ago — jobs that came due " + "since then have not fired.", Colors.YELLOW)) print("\n To enable automatic execution for this profile:\n" " hermes gateway install # Install as a user service\n" " sudo hermes gateway install --system # Linux servers: boot-time system service\n" @@ -479,7 +504,16 @@ def _print_active_jobs_summary(jobs) -> None: next_runs.append((parsed.astimezone(timezone.utc), raw)) print(f" {len(jobs)} active job(s)") if next_runs: - print(f" Next run: {min(next_runs, key=lambda run: run[0])[1]}") + earliest = min(next_runs, key=lambda run: run[0])[1] + overdue_by = _next_run_overdue_seconds(earliest) + if overdue_by is not None and overdue_by > 0: + # #114309: a dead scheduler leaves next_run_at stranded in the past; presenting it + # as an upcoming "Next run" hides the outage. + print(color(f" ⚠ Next run {earliest} is OVERDUE — passed " + f"{_format_lateness(overdue_by)} ago but the job has not fired " + "(is the scheduler running?)", Colors.YELLOW)) + else: + print(f" Next run: {earliest}") # Post-downtime late fires show at status level, not just per-job in `cron list`. late = [j for j in jobs if isinstance(j.get("last_dispatch"), dict) and j["last_dispatch"].get("kind") in ("late", "catch_up")] diff --git a/tests/hermes_cli/test_cron.py b/tests/hermes_cli/test_cron.py index 96d4e63dd9..8680689340 100644 --- a/tests/hermes_cli/test_cron.py +++ b/tests/hermes_cli/test_cron.py @@ -1,7 +1,9 @@ """Tests for hermes_cli.cron command handling.""" import argparse +import time from argparse import Namespace +from datetime import datetime, timedelta, timezone from types import SimpleNamespace import pytest @@ -590,6 +592,50 @@ class TestSlashCronListLastStatus: assert "(ok)" in out +class TestStatusSurfacesDeadScheduler: + """#114309 — with the ticker dead and a job's next_run_at stranded in the past, `cron + status` must not present the stale timestamp as an upcoming "Next run": flag it as + OVERDUE and say when the scheduler last ticked.""" + + def _dead_gateway(self, monkeypatch): + monkeypatch.setattr("hermes_cli.gateway.find_gateway_pids", lambda: []) + monkeypatch.setattr( + "hermes_cli.gateway.named_profile_served_by_running_multiplexer", lambda: None + ) + monkeypatch.setattr("gateway.status.is_gateway_runtime_lock_active", lambda: False) + + def test_overdue_next_run_and_stale_heartbeat_are_loud( + self, tmp_cron_dir, capsys, monkeypatch + ): + job = create_job(prompt="Hourly", schedule="every 60m") + self._dead_gateway(monkeypatch) + overdue = datetime.now(timezone.utc) - timedelta(hours=7) + jobs = load_jobs() + jobs[[j["id"] for j in jobs].index(job["id"])]["next_run_at"] = overdue.isoformat() + save_jobs(jobs) + (tmp_cron_dir / "cron" / "ticker_heartbeat").write_text(str(time.time() - 25 * 3600)) + + cron_command(Namespace(cron_command="status")) + + out = capsys.readouterr().out + assert "Gateway is not running" in out + assert "Scheduler last ticked" in out + assert "OVERDUE" in out + # The stale timestamp must no longer read as an upcoming run. + assert "Next run:" not in out + + def test_future_next_run_stays_plain(self, tmp_cron_dir, capsys, monkeypatch): + create_job(prompt="Hourly", schedule="every 60m") + self._dead_gateway(monkeypatch) + + cron_command(Namespace(cron_command="status")) + + out = capsys.readouterr().out + assert "Next run:" in out + assert "OVERDUE" not in out + assert "Scheduler last ticked" not in out + + class TestSlashCronRunSkipped: """``/cron run`` on a job whose claim is refused (paused here; a live claim held by another run is the same shape) must print the refusal, never ``Triggered … next scheduler tick``.""" From f9683271448162920e12c6e9a4a07130a3113409 Mon Sep 17 00:00:00 2001 From: liuhao1024 Date: Fri, 18 Sep 2026 04:25:26 +0800 Subject: [PATCH 011/190] fix(cli): share the doctor's overdue grace in cron status Address review feedback: `cron doctor` tolerates a 15-minute grace window (_OVERDUE_GRACE_SECONDS) before calling a next_run_at overdue, but the new status OVERDUE line fired on any `scheduled < now`, so a job a few minutes behind the ticker's own cadence could flash OVERDUE while doctor still called the same job healthy. - Gate the status OVERDUE line on the same _OVERDUE_GRACE_SECONDS so status, list, and doctor tell one consistent story. - Reuse _next_run_overdue_seconds inside _next_run_overdue_issue, dropping the duplicated timestamp parsing. - Add a regression test: a next_run_at 5 minutes in the past stays a plain 'Next run' line. --- hermes_cli/cron.py | 17 ++++++++--------- tests/hermes_cli/test_cron.py | 17 +++++++++++++++++ 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/hermes_cli/cron.py b/hermes_cli/cron.py index fc15702fc5..bb1cc84e1b 100644 --- a/hermes_cli/cron.py +++ b/hermes_cli/cron.py @@ -506,9 +506,11 @@ def _print_active_jobs_summary(jobs) -> None: if next_runs: earliest = min(next_runs, key=lambda run: run[0])[1] overdue_by = _next_run_overdue_seconds(earliest) - if overdue_by is not None and overdue_by > 0: + if overdue_by is not None and overdue_by > _OVERDUE_GRACE_SECONDS: # #114309: a dead scheduler leaves next_run_at stranded in the past; presenting it - # as an upcoming "Next run" hides the outage. + # as an upcoming "Next run" hides the outage. Same 15m grace as `cron doctor` + # (_OVERDUE_GRACE_SECONDS) so a job a few minutes behind the ticker's own + # cadence doesn't flash OVERDUE here while doctor still calls it healthy. print(color(f" ⚠ Next run {earliest} is OVERDUE — passed " f"{_format_lateness(overdue_by)} ago but the job has not fired " "(is the scheduler running?)", Colors.YELLOW)) @@ -552,19 +554,16 @@ def _script_health_issue(script: str) -> Optional[str]: # A busy tick can push dispatch a few minutes late; only a next_run_at parked well in the past # means the job is silently not firing (ticker dead, gateway down, wedged fire-claim). +# `cron status`'s OVERDUE line shares this grace so status, list, and doctor tell one +# consistent story about when a job counts as overdue. _OVERDUE_GRACE_SECONDS = 15 * 60 def _next_run_overdue_issue(next_run: str) -> Optional[str]: """Issue string when ``next_run_at`` is parked in the past.""" - from datetime import datetime, timezone - try: - dt = datetime.fromisoformat(next_run.replace("Z", "+00:00")) - except ValueError: + overdue_s = _next_run_overdue_seconds(next_run) + if overdue_s is None: return f"next_run_at is not a valid timestamp: {next_run!r}" - if dt.tzinfo is None: - dt = dt.replace(tzinfo=timezone.utc) - overdue_s = (datetime.now(timezone.utc) - dt).total_seconds() if overdue_s <= _OVERDUE_GRACE_SECONDS: return None amount = f"{overdue_s / 3600:.1f}h" if overdue_s >= 3600 else f"{overdue_s / 60:.0f}m" diff --git a/tests/hermes_cli/test_cron.py b/tests/hermes_cli/test_cron.py index 8680689340..0df0fe05c9 100644 --- a/tests/hermes_cli/test_cron.py +++ b/tests/hermes_cli/test_cron.py @@ -635,6 +635,23 @@ class TestStatusSurfacesDeadScheduler: assert "OVERDUE" not in out assert "Scheduler last ticked" not in out + def test_overdue_within_doctor_grace_stays_plain(self, tmp_cron_dir, capsys, monkeypatch): + # status shares `cron doctor`'s 15-minute grace (_OVERDUE_GRACE_SECONDS): a job only + # a few minutes behind the ticker's own cadence is not an outage yet, and status must + # not flash OVERDUE while doctor calls the same job healthy. + job = create_job(prompt="Hourly", schedule="every 60m") + self._dead_gateway(monkeypatch) + within_grace = datetime.now(timezone.utc) - timedelta(minutes=5) + jobs = load_jobs() + jobs[[j["id"] for j in jobs].index(job["id"])]["next_run_at"] = within_grace.isoformat() + save_jobs(jobs) + + cron_command(Namespace(cron_command="status")) + + out = capsys.readouterr().out + assert "Next run:" in out + assert "OVERDUE" not in out + class TestSlashCronRunSkipped: """``/cron run`` on a job whose claim is refused (paused here; a live claim held by another From eddf7283e4b178ddca0dea37b1a5744f9ee8e992 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Fri, 18 Sep 2026 00:52:39 -0700 Subject: [PATCH 012/190] fix(cron): label overdue next runs in cron list, dashboard and Desktop; one parser for the instant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `hermes cron list`, the web dashboard Cron page and the Desktop cron panel/sidebar all rendered a `next_run_at` parked hours in the past as an ordinary upcoming "Next run" — the only user-visible trace of a scheduler that stopped ticking (#114309). Every surface now labels a slot past `cron doctor`'s 15-minute grace as overdue (CLI `Overdue:` row with the lateness, web `Overdue since`, Desktop `Overdue since` label on the detail panel and sidebar meta) while paused/disabled/completed jobs keep the plain label because they are not expected to fire. The CLI's overdue check now parses through `cron.jobs._parse_aware` and `hermes_time.now` so status/list/doctor and the ticker agree on the instant (mixed offsets, DST folds, legacy naive stamps read as system-local like the scheduler does), and both the ordering and the subtraction normalise to UTC: Python compares same-tzinfo datetimes by wall clock, which is wrong across a DST fold. Status still orders the soonest run by instant (#113874) and prints the stored stamp. Tests: the salvaged status tests are trimmed to two invariants (overdue + stale heartbeat is loud on status AND list; within-grace stays plain on both), the DST-fold ordering tests freeze the CLI clock as well so their 2026-11 fixtures never start reading as overdue, and one vitest each pins the web and Desktop helpers. Co-authored-by: funky-xamarin <30426178+Wenfengcheng@users.noreply.github.com> Co-authored-by: joaomarcos --- .../app/chat/sidebar/cron-jobs-section.tsx | 10 +++- .../src/app/cron/cron-job-model.test.ts | 19 +++++++ apps/desktop/src/app/cron/index.tsx | 7 ++- apps/desktop/src/app/cron/job-state.ts | 25 +++++++++ apps/desktop/src/i18n/en.ts | 2 + apps/desktop/src/i18n/types.ts | 1 + hermes_cli/cron.py | 55 ++++++++++++------- tests/hermes_cli/test_cron.py | 54 ++++++++---------- tests/hermes_cli/test_cron_status_next_run.py | 15 +++-- web/src/i18n/en.ts | 2 + web/src/i18n/types.ts | 1 + web/src/lib/cron-job.test.ts | 20 +++++++ web/src/lib/cron-job.ts | 20 +++++++ web/src/pages/CronPage.tsx | 16 +++++- website/docs/user-guide/features/cron.md | 2 + 15 files changed, 187 insertions(+), 62 deletions(-) diff --git a/apps/desktop/src/app/chat/sidebar/cron-jobs-section.tsx b/apps/desktop/src/app/chat/sidebar/cron-jobs-section.tsx index b8feea2416..2e6575f43c 100644 --- a/apps/desktop/src/app/chat/sidebar/cron-jobs-section.tsx +++ b/apps/desktop/src/app/chat/sidebar/cron-jobs-section.tsx @@ -20,7 +20,7 @@ import { notify, notifyError } from '@/store/notifications' import { $selectedStoredSessionId } from '@/store/session' import type { CronJob } from '@/types/hermes' -import { jobState, jobTitle, STATE_DOT } from '../../cron/job-state' +import { jobState, jobTitle, nextRunOverdueMs, STATE_DOT } from '../../cron/job-state' import { SidebarPanelLabel } from '../../shell/sidebar-label' import { SidebarRowBody, SidebarRowLabel, SidebarRowLead, SidebarRowShell } from './chrome' @@ -244,7 +244,13 @@ function CronJobSidebarRow({ const label = jobTitle(job) const isPaused = state === 'paused' - const meta = INACTIVE_STATES.has(state) ? (c.states[state] ?? state) : next !== null ? relativeTime(next, nowMs) : '—' + const overdue = nextRunOverdueMs(job, nowMs) !== null + + const meta = INACTIVE_STATES.has(state) + ? (c.states[state] ?? state) + : next !== null + ? `${overdue ? `${c.overdueSince.replace(/:$/, '')} ` : ''}${relativeTime(next, nowMs)}` + : '—' // Pause/resume and delete aren't threaded through the sidebar's prop chain, so // drive them against the shared $cronJobs atom directly (same path the cron diff --git a/apps/desktop/src/app/cron/cron-job-model.test.ts b/apps/desktop/src/app/cron/cron-job-model.test.ts index c76df80abf..97af7ac7b9 100644 --- a/apps/desktop/src/app/cron/cron-job-model.test.ts +++ b/apps/desktop/src/app/cron/cron-job-model.test.ts @@ -8,6 +8,7 @@ import { toggleCronDeliveryTarget, validateCronEditor } from './cron-job-model' +import { nextRunOverdueMs } from './job-state' describe('jobIsScriptOnly', () => { it('is true when no_agent is set and a script is present', () => { @@ -143,3 +144,21 @@ describe('cronEditorUpdates', () => { expect('provider' in updates).toBe(false) }) }) + +describe('nextRunOverdueMs', () => { + const now = Date.parse('2026-09-17T20:35:00+04:00') + + it('flags an active job whose stored slot sits past the scheduler grace (#114309)', () => { + const job = { enabled: true, next_run_at: '2026-09-17T13:34:18+04:00', state: 'scheduled' } + + expect(nextRunOverdueMs(job, now)).toBe(now - Date.parse(job.next_run_at)) + }) + + it('keeps upcoming, within-grace, paused and unparseable slots as plain next runs', () => { + expect(nextRunOverdueMs({ enabled: true, next_run_at: '2026-09-17T21:00:00+04:00' }, now)).toBeNull() + expect(nextRunOverdueMs({ enabled: true, next_run_at: '2026-09-17T20:30:00+04:00' }, now)).toBeNull() + expect(nextRunOverdueMs({ enabled: true, next_run_at: '2026-09-17T13:34:18+04:00', state: 'paused' }, now)).toBeNull() + expect(nextRunOverdueMs({ enabled: false, next_run_at: '2026-09-17T13:34:18+04:00' }, now)).toBeNull() + expect(nextRunOverdueMs({ enabled: true, next_run_at: 'not-a-date' }, now)).toBeNull() + }) +}) diff --git a/apps/desktop/src/app/cron/index.tsx b/apps/desktop/src/app/cron/index.tsx index 9bbf2e44ff..cfd9f9138c 100644 --- a/apps/desktop/src/app/cron/index.tsx +++ b/apps/desktop/src/app/cron/index.tsx @@ -83,7 +83,7 @@ import { toggleCronDeliveryTarget, validateCronEditor } from './cron-job-model' -import { jobState, jobTitle, STATE_DOT } from './job-state' +import { jobState, jobTitle, nextRunOverdueMs, STATE_DOT } from './job-state' const DEFAULT_DELIVER = 'local' @@ -818,7 +818,10 @@ function CronJobDetail({ busy, c, job, onEdit, onOpenSession, onPauseResume, onT rows={[ { label: c.frequencyLabel, value: jobScheduleDisplay(job) }, { label: c.last.replace(/:$/, ''), value: formatTime(job.last_run_at) }, - { label: c.next.replace(/:$/, ''), value: formatTime(job.next_run_at) }, + { + label: (nextRunOverdueMs(job) === null ? c.next : c.overdueSince).replace(/:$/, ''), + value: formatTime(job.next_run_at) + }, { label: c.deliverLabel, value: c.deliveryLabels[deliver] ?? deliver }, ...(modelOverride ? [{ label: c.modelLabel, value: modelOverride }] : []) ]} diff --git a/apps/desktop/src/app/cron/job-state.ts b/apps/desktop/src/app/cron/job-state.ts index b7dd139cc4..bf7348f67d 100644 --- a/apps/desktop/src/app/cron/job-state.ts +++ b/apps/desktop/src/app/cron/job-state.ts @@ -27,3 +27,28 @@ export function jobTitle(job: CronJob): string { return pick(job.name) || clip(pick(job.prompt)) || clip(pick(job.script)) || job.id || 'Cron job' } + +// Mirrors hermes_cli/cron.py `_OVERDUE_GRACE_SECONDS`: a busy tick can dispatch a few minutes late. +export const NEXT_RUN_OVERDUE_GRACE_MS = 15 * 60 * 1000 + +// Milliseconds a job's stored next_run_at has sat in the past beyond that grace, or null when +// the slot is upcoming, within grace, unparseable, or the job is not expected to fire. A slot +// parked in the past is the only user-visible trace of a dead scheduler (#114309), so no +// surface may present it as an upcoming "Next". +export function nextRunOverdueMs(job: Pick, nowMs = Date.now()): null | number { + const state = jobState(job as CronJob) + + if (state === 'paused' || state === 'completed' || state === 'disabled' || !job.next_run_at) { + return null + } + + const at = Date.parse(job.next_run_at) + + if (Number.isNaN(at)) { + return null + } + + const overdue = nowMs - at + + return overdue > NEXT_RUN_OVERDUE_GRACE_MS ? overdue : null +} diff --git a/apps/desktop/src/i18n/en.ts b/apps/desktop/src/i18n/en.ts index fcd287befc..f54b272c54 100644 --- a/apps/desktop/src/i18n/en.ts +++ b/apps/desktop/src/i18n/en.ts @@ -2632,6 +2632,8 @@ export const en: Translations = { emptyTitleSearch: 'No matches', last: 'Last:', next: 'Next:', + // Replaces `next` when the stored next_run_at is already past the scheduler grace (#114309). + overdueSince: 'Overdue since:', noRuns: 'No runs yet', manage: 'Manage', showRuns: 'Show runs', diff --git a/apps/desktop/src/i18n/types.ts b/apps/desktop/src/i18n/types.ts index 7429e74e96..30c2474084 100644 --- a/apps/desktop/src/i18n/types.ts +++ b/apps/desktop/src/i18n/types.ts @@ -2254,6 +2254,7 @@ export interface Translations { emptyTitleSearch: string last: string next: string + overdueSince: string noRuns: string manage: string showRuns: string diff --git a/hermes_cli/cron.py b/hermes_cli/cron.py index bb1cc84e1b..fa459339d2 100644 --- a/hermes_cli/cron.py +++ b/hermes_cli/cron.py @@ -4,7 +4,7 @@ import contextlib import json import re import sys -from datetime import datetime, timezone +from datetime import timezone from pathlib import Path from typing import Any, Dict, Iterable, List, Optional @@ -97,19 +97,35 @@ def _dispatch_kind_label(kind) -> Optional[str]: return {"catch_up": "catch-up after missed fire", "late": "late"}.get(kind) -def _next_run_overdue_seconds(next_run_at: str) -> Optional[float]: - """Seconds the timestamp has already been in the past; None if malformed. +def _next_run_overdue_seconds(next_run_at: Any) -> Optional[float]: + """Seconds the stored ``next_run_at`` is already in the past (negative while still + upcoming); None when it is not a parseable ISO timestamp. - next_run_at is written as timezone-aware ISO strings; a naive value only ever comes - from a hand-edited jobs.json and is read as UTC. + Parses through the scheduler's own ``_parse_aware`` so the CLI and the ticker agree on + the instant (mixed UTC offsets, DST folds, legacy naive stamps read as system-local). """ - try: - dt = datetime.fromisoformat(str(next_run_at).replace("Z", "+00:00")) - except ValueError: + from cron.jobs import _parse_aware + from hermes_time import now + dt = _parse_aware(next_run_at) + if dt is None: return None - if dt.tzinfo is None: - dt = dt.replace(tzinfo=timezone.utc) - return (datetime.now(timezone.utc) - dt).total_seconds() + # Same-tzinfo subtraction is wall-clock arithmetic in Python; compare instants. + return (now().astimezone(timezone.utc) - dt.astimezone(timezone.utc)).total_seconds() + + +def _next_run_row(job: Dict[str, Any]) -> tuple[str, str]: + """``("Next run" | "Overdue", value)`` for one job. + + A stamp parked past `cron doctor`'s grace on a job that is supposed to fire is the only + user-visible trace of a dead scheduler; never present it as an upcoming run (#114309). + """ + stamp = job.get("next_run_at", "?") + overdue_s = _next_run_overdue_seconds(stamp) + if (overdue_s is None or overdue_s <= _OVERDUE_GRACE_SECONDS + or not job.get("enabled", True) or job.get("state") in {"paused", "completed"}): + return ("Next run", stamp) + return ("Overdue", color(f"{stamp} ({_format_lateness(overdue_s)} ago — the job has not fired; " + "is the scheduler running?)", Colors.YELLOW)) def _dispatch_display(dispatch: dict) -> Optional[str]: @@ -223,7 +239,7 @@ def _job_rows(job: Dict[str, Any]) -> List[tuple[str, str]]: ("Name", job.get("name", "(unnamed)")), ("Schedule", job.get("schedule_display", job.get("schedule", {}).get("value", "?"))), ("Repeat", f"{repeat_info.get('completed', 0)}/{repeat_times}" if repeat_times else "∞"), - ("Next run", job.get("next_run_at", "?")), + _next_run_row(job), ("Deliver", deliver if isinstance(deliver, str) else ", ".join(deliver)), ] + [(label, value) for label, value in optional if value] @@ -491,17 +507,14 @@ def _print_active_jobs_summary(jobs) -> None: if not jobs: print(" No active jobs") return - from datetime import timezone from cron.jobs import _parse_aware - next_runs = [] - for job in jobs: - raw = job.get("next_run_at") - parsed = _parse_aware(raw) - if parsed is not None: - # A shared ZoneInfo compares wall times across a DST fold; use UTC - # for ordering, but keep the stored timestamp for display. - next_runs.append((parsed.astimezone(timezone.utc), raw)) + # Stored stamps carry mixed UTC offsets (an interval job keeps last_run_at's offset, a cron + # job its configured zone), so order by instant, never by ISO text; display the stored stamp. + # `_parse_aware` hands back one shared ZoneInfo, and Python compares same-tzinfo datetimes + # by wall clock (wrong across a DST fold) — normalise to UTC before ordering. + next_runs = [(parsed.astimezone(timezone.utc), j["next_run_at"]) for j in jobs + if (parsed := _parse_aware(j.get("next_run_at"))) is not None] print(f" {len(jobs)} active job(s)") if next_runs: earliest = min(next_runs, key=lambda run: run[0])[1] diff --git a/tests/hermes_cli/test_cron.py b/tests/hermes_cli/test_cron.py index 0df0fe05c9..15c1104df9 100644 --- a/tests/hermes_cli/test_cron.py +++ b/tests/hermes_cli/test_cron.py @@ -594,8 +594,8 @@ class TestSlashCronListLastStatus: class TestStatusSurfacesDeadScheduler: """#114309 — with the ticker dead and a job's next_run_at stranded in the past, `cron - status` must not present the stale timestamp as an upcoming "Next run": flag it as - OVERDUE and say when the scheduler last ticked.""" + status` / `cron list` must not present the stale timestamp as an upcoming "Next run": + flag it as overdue and say when the scheduler last ticked.""" def _dead_gateway(self, monkeypatch): monkeypatch.setattr("hermes_cli.gateway.find_gateway_pids", lambda: []) @@ -604,36 +604,30 @@ class TestStatusSurfacesDeadScheduler: ) monkeypatch.setattr("gateway.status.is_gateway_runtime_lock_active", lambda: False) + def _park_next_run(self, job_id, when): + jobs = load_jobs() + jobs[[j["id"] for j in jobs].index(job_id)]["next_run_at"] = when.isoformat() + save_jobs(jobs) + def test_overdue_next_run_and_stale_heartbeat_are_loud( self, tmp_cron_dir, capsys, monkeypatch ): job = create_job(prompt="Hourly", schedule="every 60m") self._dead_gateway(monkeypatch) - overdue = datetime.now(timezone.utc) - timedelta(hours=7) - jobs = load_jobs() - jobs[[j["id"] for j in jobs].index(job["id"])]["next_run_at"] = overdue.isoformat() - save_jobs(jobs) + self._park_next_run(job["id"], datetime.now(timezone.utc) - timedelta(hours=7)) (tmp_cron_dir / "cron" / "ticker_heartbeat").write_text(str(time.time() - 25 * 3600)) cron_command(Namespace(cron_command="status")) + status_out = capsys.readouterr().out + cron_command(Namespace(cron_command="list", all=False, json=False)) + list_out = capsys.readouterr().out - out = capsys.readouterr().out - assert "Gateway is not running" in out - assert "Scheduler last ticked" in out - assert "OVERDUE" in out - # The stale timestamp must no longer read as an upcoming run. - assert "Next run:" not in out - - def test_future_next_run_stays_plain(self, tmp_cron_dir, capsys, monkeypatch): - create_job(prompt="Hourly", schedule="every 60m") - self._dead_gateway(monkeypatch) - - cron_command(Namespace(cron_command="status")) - - out = capsys.readouterr().out - assert "Next run:" in out - assert "OVERDUE" not in out - assert "Scheduler last ticked" not in out + assert "Gateway is not running" in status_out + assert "Scheduler last ticked" in status_out + assert "OVERDUE" in status_out and "7h ago" in status_out + # The stale timestamp must no longer read as an upcoming run on either surface. + assert "Next run:" not in status_out + assert "Overdue:" in list_out and "Next run:" not in list_out def test_overdue_within_doctor_grace_stays_plain(self, tmp_cron_dir, capsys, monkeypatch): # status shares `cron doctor`'s 15-minute grace (_OVERDUE_GRACE_SECONDS): a job only @@ -641,16 +635,16 @@ class TestStatusSurfacesDeadScheduler: # not flash OVERDUE while doctor calls the same job healthy. job = create_job(prompt="Hourly", schedule="every 60m") self._dead_gateway(monkeypatch) - within_grace = datetime.now(timezone.utc) - timedelta(minutes=5) - jobs = load_jobs() - jobs[[j["id"] for j in jobs].index(job["id"])]["next_run_at"] = within_grace.isoformat() - save_jobs(jobs) + self._park_next_run(job["id"], datetime.now(timezone.utc) - timedelta(minutes=5)) cron_command(Namespace(cron_command="status")) + status_out = capsys.readouterr().out + cron_command(Namespace(cron_command="list", all=False, json=False)) + list_out = capsys.readouterr().out - out = capsys.readouterr().out - assert "Next run:" in out - assert "OVERDUE" not in out + assert "Next run:" in status_out and "OVERDUE" not in status_out + assert "Scheduler last ticked" not in status_out # no heartbeat file → nothing to date + assert "Next run:" in list_out and "Overdue:" not in list_out class TestSlashCronRunSkipped: diff --git a/tests/hermes_cli/test_cron_status_next_run.py b/tests/hermes_cli/test_cron_status_next_run.py index b73370499a..4f3e2f7639 100644 --- a/tests/hermes_cli/test_cron_status_next_run.py +++ b/tests/hermes_cli/test_cron_status_next_run.py @@ -9,15 +9,22 @@ from cron import jobs as job_store from hermes_cli.cron import _print_active_jobs_summary +@pytest.fixture(autouse=True) +def _frozen_clock(monkeypatch): + # Freeze both the scheduler's clock (normalisation) and the CLI's (overdue check): the + # fixtures below are 2026-11 instants and must never start reading as overdue. + now = datetime(2026, 10, 31, tzinfo=ZoneInfo("America/New_York")) + monkeypatch.setattr(job_store, "_hermes_now", lambda: now) + monkeypatch.setattr("hermes_time.now", lambda: now) + + @pytest.mark.parametrize("later,earlier", [ ("2026-11-01T01:15:00-05:00", "2026-11-01T01:45:00-04:00"), ("2026-11-02T09:00:00-05:00", "2026-11-02T08:00:00-05:00"), ("2026-11-02T08:00:00-05:00", "2026-11-02T12:30:00+00:00"), ]) -def test_status_earliest_persisted_instant(later, earlier, monkeypatch, capsys): - # Only freeze the clock: creation, persistence, normalization and listing are real. - now = datetime(2026, 10, 31, tzinfo=ZoneInfo("America/New_York")) - monkeypatch.setattr(job_store, "_hermes_now", lambda: now) +def test_status_earliest_persisted_instant(later, earlier, capsys): + # Only the clock is frozen: creation, persistence, normalization and listing are real. late_job = job_store.create_job(prompt="later", schedule=later) early_job = job_store.create_job(prompt="earlier", schedule=earlier) jobs = job_store.list_jobs() diff --git a/web/src/i18n/en.ts b/web/src/i18n/en.ts index 159b8f2080..d5e4daeffb 100644 --- a/web/src/i18n/en.ts +++ b/web/src/i18n/en.ts @@ -308,6 +308,8 @@ export const en: Translations = { noJobs: "No cron jobs configured. Create one above.", last: "Last", next: "Next", + /** Replaces `next` when the stored next_run_at is already past the scheduler grace. */ + overdueSince: "Overdue since", pause: "Pause", resume: "Resume", triggerNow: "Trigger now", diff --git a/web/src/i18n/types.ts b/web/src/i18n/types.ts index 0346d7a885..a97b79a21e 100644 --- a/web/src/i18n/types.ts +++ b/web/src/i18n/types.ts @@ -322,6 +322,7 @@ export interface Translations { noJobs: string; last: string; next: string; + overdueSince?: string; pause: string; resume: string; triggerNow: string; diff --git a/web/src/lib/cron-job.test.ts b/web/src/lib/cron-job.test.ts index 25420d0a3a..f0d7424714 100644 --- a/web/src/lib/cron-job.test.ts +++ b/web/src/lib/cron-job.test.ts @@ -5,6 +5,7 @@ import { cronJobHasExecutionContent, cronJobFormFromJob, cronLastResult, + cronNextRunOverdueMs, splitCronList, type CronJobFormState, } from "./cron-job"; @@ -200,3 +201,22 @@ describe("cronLastResult", () => { ).toEqual({ status: "blocked_config", tone: "warning", detail: "missing API key" }); }); }); + +describe("cronNextRunOverdueMs", () => { + const now = Date.parse("2026-09-17T20:35:00+04:00"); + + it("flags an active job whose stored slot sits past the scheduler grace (#114309)", () => { + const job = { next_run_at: "2026-09-17T13:34:18+04:00", enabled: true, state: "scheduled" }; + expect(cronNextRunOverdueMs(job, now)).toBe(now - Date.parse(job.next_run_at)); + }); + + it("keeps upcoming, within-grace, paused and unparseable slots as plain next runs", () => { + expect(cronNextRunOverdueMs({ next_run_at: "2026-09-17T21:00:00+04:00", enabled: true }, now)).toBeNull(); + expect(cronNextRunOverdueMs({ next_run_at: "2026-09-17T20:30:00+04:00", enabled: true }, now)).toBeNull(); + expect( + cronNextRunOverdueMs({ next_run_at: "2026-09-17T13:34:18+04:00", enabled: true, state: "paused" }, now), + ).toBeNull(); + expect(cronNextRunOverdueMs({ next_run_at: "2026-09-17T13:34:18+04:00", enabled: false }, now)).toBeNull(); + expect(cronNextRunOverdueMs({ next_run_at: "not-a-date", enabled: true }, now)).toBeNull(); + }); +}); diff --git a/web/src/lib/cron-job.ts b/web/src/lib/cron-job.ts index abcbcd3351..2e9b103f7b 100644 --- a/web/src/lib/cron-job.ts +++ b/web/src/lib/cron-job.ts @@ -148,3 +148,23 @@ export function cronLastResult( : asString(job.last_error).trim() || asString(job.last_delivery_error).trim(); return { status, tone, detail: detail || null }; } + +/** Mirrors hermes_cli/cron.py `_OVERDUE_GRACE_SECONDS`: a busy tick can run a few minutes late. */ +export const CRON_NEXT_RUN_OVERDUE_GRACE_MS = 15 * 60 * 1000; + +/** + * Milliseconds a job's stored `next_run_at` has sat in the past beyond the doctor grace, or + * null when the job is upcoming, within grace, not expected to fire, or has no parseable stamp. + * A stamp parked in the past is the only user-visible trace of a dead scheduler (#114309), so + * the dashboard must never present it as an upcoming "Next". + */ +export function cronNextRunOverdueMs( + job: Pick, + nowMs: number = Date.now(), +): number | null { + if (job.enabled === false || job.state === "paused" || job.state === "completed") return null; + const at = Date.parse(asString(job.next_run_at)); + if (Number.isNaN(at)) return null; + const overdue = nowMs - at; + return overdue > CRON_NEXT_RUN_OVERDUE_GRACE_MS ? overdue : null; +} diff --git a/web/src/pages/CronPage.tsx b/web/src/pages/CronPage.tsx index 38f64f4661..6869775bf1 100644 --- a/web/src/pages/CronPage.tsx +++ b/web/src/pages/CronPage.tsx @@ -21,6 +21,7 @@ import type { import { buildCronJobPayload, cronJobHasExecutionContent, + cronNextRunOverdueMs, cronJobFormFromJob, cronLastResult, focusCronField, @@ -1181,9 +1182,18 @@ export default function CronPage() { {t.cron.last}: {formatTime(job.last_run_at)} - - {t.cron.next}: {formatTime(job.next_run_at)} - + {cronNextRunOverdueMs(job) === null ? ( + + {t.cron.next}: {formatTime(job.next_run_at)} + + ) : ( + + {t.cron.overdueSince ?? en.cron.overdueSince!}: {formatTime(job.next_run_at)} + + )}
{job.last_delivery_error && (

diff --git a/website/docs/user-guide/features/cron.md b/website/docs/user-guide/features/cron.md index f4f4aafdd4..b260492397 100644 --- a/website/docs/user-guide/features/cron.md +++ b/website/docs/user-guide/features/cron.md @@ -347,6 +347,8 @@ hermes cron list hermes cron status ``` +`hermes cron status` reports whether the scheduler is alive (gateway process, ticker heartbeat, last successful tick) and the soonest scheduled run across your active jobs, ordered by actual instant even when jobs store different UTC offsets. A `next_run_at` that is already more than 15 minutes in the past is never shown as an upcoming "Next run": `cron status` prints `⚠ Next run

+ {(t.cron.schedulerLastTicked ?? en.cron.schedulerLastTicked!).replace( + "{when}", + cronAgoLabel(schedulerStaleAgeS), + )} +

+ )} + setView(v as "jobs" | "blueprints")} diff --git a/website/docs/user-guide/features/cron.md b/website/docs/user-guide/features/cron.md index 7256397c68..9d7681f806 100644 --- a/website/docs/user-guide/features/cron.md +++ b/website/docs/user-guide/features/cron.md @@ -347,7 +347,7 @@ hermes cron list hermes cron status ``` -`hermes cron status` reports whether the scheduler is alive (gateway process, ticker heartbeat, last successful tick) and the soonest scheduled run across your active jobs, ordered by actual instant even when jobs store different UTC offsets. A `next_run_at` that is already more than 15 minutes in the past is never shown as an upcoming "Next run": `cron status` prints `⚠ Next run