hermes: make pool lease recovery and release isolation safe
Independent review t_5975c06a blocked this branch on a P1: a Kanban write that
failed while a lease expired left a `lease_failed` row that was invisible to
every pass, immortal to garbage collection, and fatal to the coordinator. It
poisoned `reconcile()` forever with a conflicting-duplicate primary key,
produced a spurious capability `block_task` from `dispatch()`, and -- because
startup maintenance ran unguarded before the port bound, against a store on a
PVC -- crash-looped the coordinator with no automatic recovery.
`lease_failed` is now a retryable state that every maintenance pass drains, and
a row only reaches a terminal state on authoritative evidence about its exact
Kanban run, so nothing is collected before its outcome is known and nothing is
silently dropped. Each row, task, and board is processed in isolation, and a
coordinator-side fault is never converted into a Kanban mutation. Startup runs
through the same guarded cycle as the steady-state loop.
The wire protocol and the durable store are now separate modules, and the
maintenance passes moved out of the coordinator, so each file stays under the
managed line ceiling with room for the recovery logic.
Also closes three consequential handoff risks the same review raised:
* mediator-N pinned itself hard to worker-N while sharing a ReadWriteOnce
claim, so a drain or preemption that moved only the lower-priority worker
deadlocked the ordinal on Multi-Attach until an operator deleted a Pod. The
shared workspace is now ReadWriteMany (as the hermes-chat tenant workspaces
already are on the same class), colocation is a preference, and the mediator
shares the worker's preemption priority, so each Pod reschedules on its own.
* the broker permits only branch creation, so a retry that added commits could
never submit and the run's work was discarded with the failure. Submission
now targets a fresh attempt- or content-scoped ref in the same reviewed
namespace -- never an update -- and is idempotent under replay. A refused
submission downgrades the result and says why instead of unwinding the run.
* the provider CLIs were reinstalled into an emptyDir on every Pod start inside
the 10m Flux health window for the whole hermes app. They now install once
per pinned version onto a durable volume, re-verified against the real
binaries and time-bounded, and the best-effort pool no longer gates the
health of the app its dependents wait on.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-17 16:31:15 +00:00
|
|
|
"""Lease-failure recovery contracts for the execution-pool coordinator.
|
|
|
|
|
|
|
|
|
|
The independent review reproduced a wedge in which a Kanban write that failed
|
|
|
|
|
during lease expiry left a ``lease_failed`` row that was invisible, immortal,
|
|
|
|
|
and fatal. These tests pin the recovered behaviour: the row stays retryable, is
|
|
|
|
|
only collected on authoritative terminal evidence, and survives a restart on the
|
|
|
|
|
PVC the store lives on.
|
|
|
|
|
"""
|
|
|
|
|
|
|
|
|
|
from __future__ import annotations
|
|
|
|
|
|
|
|
|
|
import sys
|
|
|
|
|
from pathlib import Path
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
ROOT = Path(__file__).parents[2]
|
|
|
|
|
SCRIPTS = ROOT / "services/hermes/scripts"
|
|
|
|
|
SCM_SCRIPTS = ROOT / "services/hermes/scm-common/scripts"
|
|
|
|
|
sys.path[:0] = [str(SCRIPTS), str(SCM_SCRIPTS)]
|
|
|
|
|
|
|
|
|
|
import execution_pool_coordinator as coordinator # noqa: E402
|
|
|
|
|
import execution_pool_maintenance as maintenance # noqa: E402
|
|
|
|
|
import execution_pool_store as pool_store # noqa: E402
|
|
|
|
|
from testing.tests.test_hermes_execution_pool_coordinator_v2 import ( # noqa: E402
|
|
|
|
|
MASTER,
|
|
|
|
|
assignment_payload,
|
|
|
|
|
binding,
|
|
|
|
|
install_kanban,
|
|
|
|
|
task,
|
|
|
|
|
)
|
|
|
|
|
from testing.tests.test_hermes_execution_pool_support import ( # noqa: E402
|
|
|
|
|
LOCKED,
|
|
|
|
|
age,
|
|
|
|
|
clear_pool_deferrals, # noqa: F401 - autouse fixture, resolved by name
|
|
|
|
|
exhausted_store,
|
|
|
|
|
states,
|
|
|
|
|
)
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def test_locked_board_at_lease_expiry_stays_retryable_and_never_wedges(
|
|
|
|
|
tmp_path, monkeypatch, capsys
|
|
|
|
|
):
|
|
|
|
|
"""The exact review repro: a locked board must not strand the run."""
|
|
|
|
|
live = task()
|
|
|
|
|
kanban = install_kanban(monkeypatch, [live], tmp_path)
|
|
|
|
|
monkeypatch.setattr(
|
|
|
|
|
coordinator, "assignment_payload",
|
|
|
|
|
lambda _db, _connection, _task, _board: assignment_payload(),
|
|
|
|
|
)
|
|
|
|
|
|
|
|
|
|
def locked(*_args, **_kwargs):
|
|
|
|
|
raise LOCKED
|
|
|
|
|
|
|
|
|
|
kanban.block_task = locked
|
|
|
|
|
store = exhausted_store(tmp_path / "wedge.db")
|
|
|
|
|
pool = coordinator.Coordinator(MASTER, store)
|
|
|
|
|
|
|
|
|
|
pool.expire_leases()
|
|
|
|
|
assert states(store) == [("23", 0, 3, "lease_failed")]
|
|
|
|
|
assert store.available_ordinals() == [0, 1, 2]
|
|
|
|
|
assert "lease park" in capsys.readouterr().err
|
|
|
|
|
|
|
|
|
|
# 1. The row is never collected while its outcome is unconfirmed.
|
|
|
|
|
age(store, 10 * 365 * 86400)
|
|
|
|
|
assert store.garbage_collect(0) == 0
|
|
|
|
|
assert states(store) == [("23", 0, 3, "lease_failed")]
|
|
|
|
|
|
|
|
|
|
# 2. reconcile() no longer hits the conflicting-duplicate primary key.
|
|
|
|
|
for _cycle in range(3):
|
|
|
|
|
pool.reconcile()
|
|
|
|
|
assert states(store) == [("23", 0, 3, "lease_failed")]
|
|
|
|
|
|
|
|
|
|
# 3. dispatch() makes no spurious Kanban mutation.
|
|
|
|
|
monkeypatch.setattr(
|
|
|
|
|
maintenance.cli_lane_dispatch, "claim_ready",
|
|
|
|
|
lambda *_args, **_kwargs: [("metis", live.id)],
|
|
|
|
|
)
|
|
|
|
|
pool.dispatch()
|
|
|
|
|
assert kanban.blocked == []
|
|
|
|
|
assert states(store) == [("23", 0, 3, "lease_failed")]
|
|
|
|
|
|
|
|
|
|
# 4. Once the board recovers, the retry records the exact run and settles.
|
|
|
|
|
kanban.block_task = lambda _connection, task_id, **values: (
|
|
|
|
|
kanban.blocked.append((task_id, values)) or True
|
|
|
|
|
)
|
|
|
|
|
pool.expire_leases()
|
|
|
|
|
assert kanban.blocked[-1][1] == {
|
|
|
|
|
"reason": "Distributed worker lease expired after 3 fenced attempts",
|
|
|
|
|
"kind": "transient",
|
|
|
|
|
"expected_run_id": 23,
|
|
|
|
|
}
|
|
|
|
|
assert states(store) == [("23", 0, 3, "finalized")]
|
|
|
|
|
|
|
|
|
|
# 5. Only now, with authoritative evidence, is the row collectable.
|
|
|
|
|
age(store, 10 * 365 * 86400)
|
|
|
|
|
assert store.garbage_collect(0) == 1
|
|
|
|
|
assert states(store) == []
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def test_refused_exact_run_park_keeps_the_row_for_the_next_pass(tmp_path, monkeypatch):
|
|
|
|
|
"""No terminal state without evidence, even when the write is refused."""
|
|
|
|
|
live = task()
|
|
|
|
|
kanban = install_kanban(monkeypatch, [live], tmp_path)
|
|
|
|
|
kanban.block_task = lambda _connection, task_id, **values: (
|
|
|
|
|
kanban.blocked.append((task_id, values)) or False
|
|
|
|
|
)
|
|
|
|
|
store = exhausted_store(tmp_path / "refused.db")
|
|
|
|
|
pool = coordinator.Coordinator(MASTER, store)
|
|
|
|
|
|
|
|
|
|
pool.expire_leases()
|
|
|
|
|
assert states(store) == [("23", 0, 3, "lease_failed")]
|
|
|
|
|
assert len(kanban.blocked) == 1
|
|
|
|
|
|
|
|
|
|
# The next pass retries rather than giving up on the run.
|
|
|
|
|
pool.expire_leases()
|
|
|
|
|
assert len(kanban.blocked) == 2
|
|
|
|
|
assert states(store) == [("23", 0, 3, "lease_failed")]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def test_lease_failure_settles_stale_when_the_run_moved_or_vanished(
|
|
|
|
|
tmp_path, monkeypatch
|
|
|
|
|
):
|
|
|
|
|
moved = task(current_run_id=99)
|
|
|
|
|
kanban = install_kanban(monkeypatch, [moved], tmp_path)
|
|
|
|
|
store = exhausted_store(tmp_path / "moved.db")
|
|
|
|
|
coordinator.Coordinator(MASTER, store).expire_leases()
|
|
|
|
|
assert states(store) == [("23", 0, 3, "stale")]
|
|
|
|
|
assert kanban.blocked == []
|
|
|
|
|
|
|
|
|
|
kanban.tasks.clear()
|
|
|
|
|
gone = exhausted_store(tmp_path / "gone.db", binding(task_id="t_gone", attempt=3))
|
|
|
|
|
coordinator.Coordinator(MASTER, gone).expire_leases()
|
|
|
|
|
assert states(gone) == [("23", 0, 3, "stale")]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def test_lease_failure_settles_finalized_when_kanban_already_parked_the_run(
|
|
|
|
|
tmp_path, monkeypatch
|
|
|
|
|
):
|
|
|
|
|
parked = task(status="blocked")
|
|
|
|
|
kanban = install_kanban(monkeypatch, [parked], tmp_path)
|
|
|
|
|
store = exhausted_store(tmp_path / "parked.db")
|
|
|
|
|
coordinator.Coordinator(MASTER, store).expire_leases()
|
|
|
|
|
assert states(store) == [("23", 0, 3, "finalized")]
|
|
|
|
|
assert kanban.blocked == []
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def test_noncanonical_run_lease_failure_is_released_without_a_board_call(
|
|
|
|
|
tmp_path, monkeypatch
|
|
|
|
|
):
|
|
|
|
|
kanban = install_kanban(monkeypatch, [task()], tmp_path)
|
|
|
|
|
store = exhausted_store(
|
|
|
|
|
tmp_path / "bad-run.db", binding(run_id="run-bad", attempt=3)
|
|
|
|
|
)
|
|
|
|
|
coordinator.Coordinator(MASTER, store).expire_leases()
|
|
|
|
|
assert states(store) == [("run-bad", 0, 3, "stale")]
|
|
|
|
|
assert kanban.blocked == []
|
|
|
|
|
|
|
|
|
|
|
2026-09-13 17:54:54 -05:00
|
|
|
def test_lease_expiry_releases_only_a_trusted_publication_retry(tmp_path, monkeypatch):
|
|
|
|
|
"""An OOM-style lease expiry returns one receipt before parking the native run."""
|
|
|
|
|
kanban = install_kanban(monkeypatch, [task()], tmp_path)
|
|
|
|
|
store = pool_store.PoolStore(tmp_path / "resume-lease.db")
|
|
|
|
|
exact = binding(attempt=3)
|
|
|
|
|
store.add(exact, assignment_payload(scm_resume={"source": {"worker_ordinal": 0}}))
|
|
|
|
|
store.offer(0)
|
|
|
|
|
with store._connect() as connection:
|
|
|
|
|
connection.execute("UPDATE assignments SET lease_until=1")
|
|
|
|
|
released = []
|
|
|
|
|
monkeypatch.setattr(
|
|
|
|
|
coordinator.supervisor_state, "reissue_publication_retry",
|
|
|
|
|
lambda *values: released.append(values) or True,
|
|
|
|
|
)
|
|
|
|
|
coordinator.Coordinator(MASTER, store).expire_leases()
|
|
|
|
|
assert released and released[0][2] == "23"
|
|
|
|
|
assert kanban.blocked[-1][1]["kind"] == "transient"
|
|
|
|
|
|
|
|
|
|
|
hermes: make pool lease recovery and release isolation safe
Independent review t_5975c06a blocked this branch on a P1: a Kanban write that
failed while a lease expired left a `lease_failed` row that was invisible to
every pass, immortal to garbage collection, and fatal to the coordinator. It
poisoned `reconcile()` forever with a conflicting-duplicate primary key,
produced a spurious capability `block_task` from `dispatch()`, and -- because
startup maintenance ran unguarded before the port bound, against a store on a
PVC -- crash-looped the coordinator with no automatic recovery.
`lease_failed` is now a retryable state that every maintenance pass drains, and
a row only reaches a terminal state on authoritative evidence about its exact
Kanban run, so nothing is collected before its outcome is known and nothing is
silently dropped. Each row, task, and board is processed in isolation, and a
coordinator-side fault is never converted into a Kanban mutation. Startup runs
through the same guarded cycle as the steady-state loop.
The wire protocol and the durable store are now separate modules, and the
maintenance passes moved out of the coordinator, so each file stays under the
managed line ceiling with room for the recovery logic.
Also closes three consequential handoff risks the same review raised:
* mediator-N pinned itself hard to worker-N while sharing a ReadWriteOnce
claim, so a drain or preemption that moved only the lower-priority worker
deadlocked the ordinal on Multi-Attach until an operator deleted a Pod. The
shared workspace is now ReadWriteMany (as the hermes-chat tenant workspaces
already are on the same class), colocation is a preference, and the mediator
shares the worker's preemption priority, so each Pod reschedules on its own.
* the broker permits only branch creation, so a retry that added commits could
never submit and the run's work was discarded with the failure. Submission
now targets a fresh attempt- or content-scoped ref in the same reviewed
namespace -- never an update -- and is idempotent under replay. A refused
submission downgrades the result and says why instead of unwinding the run.
* the provider CLIs were reinstalled into an emptyDir on every Pod start inside
the 10m Flux health window for the whole hermes app. They now install once
per pinned version onto a durable volume, re-verified against the real
binaries and time-bounded, and the best-effort pool no longer gates the
health of the app its dependents wait on.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-17 16:31:15 +00:00
|
|
|
def test_a_durable_row_in_any_state_blocks_conflicting_re_adoption(tmp_path, monkeypatch):
|
|
|
|
|
"""known_runs() covers terminal and retrying rows, not just live ones."""
|
|
|
|
|
live = task()
|
|
|
|
|
install_kanban(monkeypatch, [live], tmp_path)
|
|
|
|
|
monkeypatch.setattr(
|
|
|
|
|
coordinator, "assignment_payload",
|
|
|
|
|
lambda _db, _connection, _task, _board: assignment_payload(context="different"),
|
|
|
|
|
)
|
|
|
|
|
store = pool_store.PoolStore(tmp_path / "known.db")
|
|
|
|
|
store.add(binding(), assignment_payload())
|
|
|
|
|
pool = coordinator.Coordinator(MASTER, store)
|
|
|
|
|
for state in ("lease_failed", "finalized", "stale"):
|
|
|
|
|
with store._connect() as connection:
|
|
|
|
|
connection.execute("UPDATE assignments SET state=?", (state,))
|
|
|
|
|
assert store.known_runs() == {("metis", "t_deadbeef", "23")}
|
|
|
|
|
pool.reconcile()
|
|
|
|
|
assert states(store) == [("23", 0, 1, state)]
|