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.
This commit is contained in:
@@ -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', () => {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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 <name> = 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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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(
|
||||
|
||||
Reference in New Issue
Block a user