From b1915cb02da6e5367bb3bd309b9fe9540a4cf626 Mon Sep 17 00:00:00 2001 From: Erosika Date: Tue, 8 Sep 2026 16:31:18 -0400 Subject: [PATCH] fix(bot-mode): keep the relay waiter watching past the Desktop deliver deadline The sender-side waiter gave up at 900s while the Desktop held bot_relay.deliver open for 1500s, so a turn finishing between minute 15 and minute 25 wrote a reply nobody read. REPLY_WAIT_SECONDS now rebuilds the Desktop budget from the same numbers and waits 60s past it. The two turn constants move into tools/bot_relay.py so the gateway handler and the waiter share one definition. --- .../hermes-bots/relay-deliver-budget.test.ts | 25 ++++++++----- apps/desktop/src/plugins/hermes-bots/relay.ts | 7 ++-- tests/tools/test_bot_relay.py | 35 +++++++++++++++++++ tools/bot_relay.py | 12 +++++-- tui_gateway/methods_bot_relay.py | 10 ++---- 5 files changed, 69 insertions(+), 20 deletions(-) diff --git a/apps/desktop/src/plugins/hermes-bots/relay-deliver-budget.test.ts b/apps/desktop/src/plugins/hermes-bots/relay-deliver-budget.test.ts index 78827f97bb..97c82ad8fe 100644 --- a/apps/desktop/src/plugins/hermes-bots/relay-deliver-budget.test.ts +++ b/apps/desktop/src/plugins/hermes-bots/relay-deliver-budget.test.ts @@ -13,7 +13,7 @@ import { describe, expect, it } from 'vitest' const relaySource = readFileSync(join(process.cwd(), 'src/plugins/hermes-bots/relay.ts'), 'utf8') const repoRoot = join(process.cwd(), '..', '..') const configDefaults = readFileSync(join(repoRoot, 'hermes_cli/config_defaults.py'), 'utf8') -const relayHandler = readFileSync(join(repoRoot, 'tui_gateway/methods_bot_relay.py'), 'utf8') +const relayPlumbing = readFileSync(join(repoRoot, 'tools/bot_relay.py'), 'utf8') function tsConstant(name: string): number { const match = relaySource.match(new RegExp(`const ${name} = ([0-9_]+)`)) @@ -22,6 +22,13 @@ function tsConstant(name: string): number { return Number(match![1].replaceAll('_', '')) } +function pyConstant(name: string): number { + const match = relayPlumbing.match(new RegExp(`^${name}\\s*=\\s*(\\d+)`, 'm')) + expect(match, `${name} must exist as a literal in tools/bot_relay.py`).toBeTruthy() + + return Number(match![1]) +} + describe('bot_relay.deliver budget mirrors', () => { it('mirrors the backend turn-lock default', () => { const lockWaitMatch = configDefaults.match(/"turn_wait_seconds":\s*(\d+)/) @@ -31,15 +38,17 @@ describe('bot_relay.deliver budget mirrors', () => { }) it('mirrors the backend per-attempt turn timeout', () => { - // The backend names both numbers explicitly (methods_bot_relay.py) so the mirror is a + // The backend names both numbers explicitly (tools/bot_relay.py) so the mirror is a // constant-to-constant check, not a count of textual subprocess.run(...) call sites. - const attemptTimeout = relayHandler.match(/^TURN_ATTEMPT_TIMEOUT_SECONDS\s*=\s*(\d+)/m) - const maxAttempts = relayHandler.match(/^TURN_MAX_ATTEMPTS\s*=\s*(\d+)/m) + expect(tsConstant('RELAY_TURN_ATTEMPT_MS')).toBe(pyConstant('TURN_ATTEMPT_TIMEOUT_SECONDS') * 1000) + expect(tsConstant('RELAY_TURN_MAX_ATTEMPTS')).toBe(pyConstant('TURN_MAX_ATTEMPTS')) + }) - expect(attemptTimeout, 'TURN_ATTEMPT_TIMEOUT_SECONDS must exist in methods_bot_relay.py').toBeTruthy() - expect(maxAttempts, 'TURN_MAX_ATTEMPTS must exist in methods_bot_relay.py').toBeTruthy() - expect(tsConstant('RELAY_TURN_ATTEMPT_MS')).toBe(Number(attemptTimeout![1]) * 1000) - expect(tsConstant('RELAY_TURN_MAX_ATTEMPTS')).toBe(Number(maxAttempts![1])) + it('shares its settlement margin with the sender-side waiter budget', () => { + // tests/tools/test_bot_relay.py checks that REPLY_WAIT_SECONDS exceeds the rebuilt sum. + expect(tsConstant('RELAY_DELIVER_SETTLEMENT_MARGIN_MS')).toBe( + pyConstant('DESKTOP_DELIVER_SETTLEMENT_MARGIN_SECONDS') * 1000 + ) }) it('keeps the client deadline strictly greater than the backend ceiling', () => { diff --git a/apps/desktop/src/plugins/hermes-bots/relay.ts b/apps/desktop/src/plugins/hermes-bots/relay.ts index df92c8f173..36031bf0eb 100644 --- a/apps/desktop/src/plugins/hermes-bots/relay.ts +++ b/apps/desktop/src/plugins/hermes-bots/relay.ts @@ -49,10 +49,10 @@ const RELAY_DRAIN_INTERVAL_MS = 30_000 // // These three are mirrors of backend values, so a change there must not // silently invalidate this constant: relay-deliver-budget.test.ts reads -// hermes_cli/config_defaults.py and tui_gateway/methods_bot_relay.py and fails -// if the mirrors drift or the margin stops being positive. +// hermes_cli/config_defaults.py and tools/bot_relay.py and fails if the +// mirrors drift or the margin stops being positive. const RELAY_TURN_LOCK_WAIT_MS = 120_000 // bot_mode.turn_wait_seconds default -const RELAY_TURN_ATTEMPT_MS = 600_000 // subprocess.run(..., timeout=600) +const RELAY_TURN_ATTEMPT_MS = 600_000 // tools/bot_relay.py TURN_ATTEMPT_TIMEOUT_SECONDS const RELAY_TURN_MAX_ATTEMPTS = 2 // first attempt + the policy-gated re-run const RELAY_DELIVER_BACKEND_CEILING_MS = RELAY_TURN_LOCK_WAIT_MS + RELAY_TURN_ATTEMPT_MS * RELAY_TURN_MAX_ATTEMPTS @@ -60,6 +60,7 @@ const RELAY_DELIVER_BACKEND_CEILING_MS = RELAY_TURN_LOCK_WAIT_MS + RELAY_TURN_AT // Settlement + transport headroom on top of the ceiling, so a backend that // answers at its own limit still wins the race against this timer. const RELAY_DELIVER_SETTLEMENT_MARGIN_MS = 180_000 +// tools/bot_relay.py REPLY_WAIT_SECONDS rebuilds this sum and waits past it for the timeout reply below. const RELAY_DELIVER_TIMEOUT_MS = RELAY_DELIVER_BACKEND_CEILING_MS + RELAY_DELIVER_SETTLEMENT_MARGIN_MS // Push path (#93091): the gateway broadcasts `bot_relay.outbox.pending` when // an envelope lands on disk; a burst of signals inside this window collapses diff --git a/tests/tools/test_bot_relay.py b/tests/tools/test_bot_relay.py index 9172cce445..dd9f8e7ffa 100644 --- a/tests/tools/test_bot_relay.py +++ b/tests/tools/test_bot_relay.py @@ -154,6 +154,41 @@ def test_waiter_command_quotes_and_targets_reply_file(root): assert "rm -rf" not in cmd # sanity: single quoted -c payload +_DESKTOP_RELAY_TS = Path(__file__).resolve().parents[2] / "apps" / "desktop" / "src" / "plugins" / "hermes-bots" / "relay.ts" + + +def _desktop_relay_ms(name: str) -> int: + """Literal ``const = N`` from relay.ts. No shared anchor links a TS constant to a Python + one, so this regex is the seam, mirroring relay-deliver-budget.test.ts on the Desktop side.""" + match = re.search(rf"^const {name} = ([0-9_]+)", _DESKTOP_RELAY_TS.read_text(encoding="utf-8"), re.M) + assert match, f"{name} must stay a literal in relay.ts" + return int(match.group(1).replace("_", "")) + + +def test_waiter_outlives_the_desktop_deliver_deadline(): + """The Desktop posts its timeout reply when RELAY_DELIVER_TIMEOUT_MS passes. A waiter that gave + up first left that reply, and any turn finishing after minute 15, in a file nobody read (#93911).""" + desktop_budget_s = ( + _desktop_relay_ms("RELAY_TURN_LOCK_WAIT_MS") + + _desktop_relay_ms("RELAY_TURN_ATTEMPT_MS") * _desktop_relay_ms("RELAY_TURN_MAX_ATTEMPTS") + + _desktop_relay_ms("RELAY_DELIVER_SETTLEMENT_MARGIN_MS") + ) // 1000 + assert bot_relay.DESKTOP_DELIVER_TIMEOUT_SECONDS == desktop_budget_s + assert bot_relay.REPLY_WAIT_SECONDS > desktop_budget_s + assert bot_relay.TURN_WAIT_SECONDS_FALLBACK * 1000 == _desktop_relay_ms("RELAY_TURN_LOCK_WAIT_MS") + + +def test_waiter_give_up_message_states_the_real_budget(root): + import shlex + + env = {"id": "d" * 32, "target_handle": "researcher", "target_connection": "ssh-vps"} + parts = shlex.split(bot_relay.waiter_command(root, env)) + code = parts[parts.index("-c") + 1] + assert f"deadline = time.time() + {bot_relay.REPLY_WAIT_SECONDS}\n" in code + assert f"within {bot_relay.REPLY_WAIT_SECONDS}s" in code + assert "within 900s" not in code + + def test_waiter_picks_up_reply_within_a_sub_second_cadence(root): """The reply file is written once; the waiter must notice it fast, not on a multi-second sleep (dead air the sender's completion notification diff --git a/tools/bot_relay.py b/tools/bot_relay.py index 08b45fef25..43ec860ecb 100644 --- a/tools/bot_relay.py +++ b/tools/bot_relay.py @@ -41,8 +41,16 @@ LOCKS_DIR = "locks" # Config fallbacks (real knobs: ``bot_mode.turn_wait_seconds`` / ``bot_mode.envelope_ttl_seconds``). TURN_WAIT_SECONDS_FALLBACK = 120 DEFAULT_ENVELOPE_TTL_SECONDS = 900 # older envelopes are refused at drain with 'queued_expired' -# Waiter give-up budget: cross-connection turns can be slow — generous, but bounded. -REPLY_WAIT_SECONDS = 900 +# Per-attempt turn timeout and attempt ceiling for bot_relay.deliver (tui_gateway/methods_bot_relay.py). +TURN_ATTEMPT_TIMEOUT_SECONDS = 600 +TURN_MAX_ATTEMPTS = 2 # first attempt + the policy-gated re-run +# Mirrors RELAY_DELIVER_TIMEOUT_MS in apps/desktop/src/plugins/hermes-bots/relay.ts; both test suites pin it. +DESKTOP_DELIVER_SETTLEMENT_MARGIN_SECONDS = 180 +DESKTOP_DELIVER_TIMEOUT_SECONDS = ( + TURN_WAIT_SECONDS_FALLBACK + TURN_ATTEMPT_TIMEOUT_SECONDS * TURN_MAX_ATTEMPTS + DESKTOP_DELIVER_SETTLEMENT_MARGIN_SECONDS +) +# The Desktop posts its own timeout reply at that deadline, so the waiter must still be watching then. +REPLY_WAIT_SECONDS = DESKTOP_DELIVER_TIMEOUT_SECONDS + 60 # Envelopes/replies older than this are stale artifacts (Desktop closed) and are swept. STALE_AFTER_SECONDS = 6 * 3600 # Only a recent roster is authoritative for the fail-fast offline check: the diff --git a/tui_gateway/methods_bot_relay.py b/tui_gateway/methods_bot_relay.py index df11a0db72..1ebbddcc30 100644 --- a/tui_gateway/methods_bot_relay.py +++ b/tui_gateway/methods_bot_relay.py @@ -10,6 +10,9 @@ import os import subprocess from pathlib import Path +# Defined beside the sender-side waiter budget so the two Python sides cannot drift (#93911). +from tools.bot_relay import TURN_ATTEMPT_TIMEOUT_SECONDS + from .method_ctx import HandlerRegistry _registry = HandlerRegistry() @@ -22,13 +25,6 @@ def _relay_root() -> Path: return home.parent.parent if home.parent.name == "profiles" else home -# Per-attempt turn timeout and attempt ceiling for bot_relay.deliver. The Desktop client mirrors -# both (apps/desktop/src/plugins/hermes-bots/relay.ts: RELAY_TURN_ATTEMPT_MS / RELAY_TURN_MAX_ATTEMPTS) -# and its relay-deliver-budget test reads these two lines, so a change here must be deliberate (#93911). -TURN_ATTEMPT_TIMEOUT_SECONDS = 600 -TURN_MAX_ATTEMPTS = 2 # first attempt + the policy-gated re-run - - def _run_delivery(profile: str, tmp: str) -> subprocess.CompletedProcess: from tools.bot_relay import local_delivery_command return subprocess.run(