atlas-iac/testing/tests/test_hermes_execution_pool_submission.py

251 lines
9.3 KiB
Python
Raw Normal View History

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
"""Retry-submission contracts against the creation-only Atlas SCM broker.
The broker deliberately permits only new namespaced branch creation, so a pool
retry that adds commits to an already-published branch could never submit: the
push was refused, the worker unwound, and the run's actual work was discarded
along with it. Submission now targets a fresh attempt- or content-scoped ref, and
a refused submission downgrades the result instead of losing it.
"""
from __future__ import annotations
import sys
from pathlib import Path
import pytest
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_client as client # noqa: E402
import execution_pool_protocol as protocol # noqa: E402
import execution_pool_scm as scm # noqa: E402
import receive_pack_scan # noqa: E402
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
import scm_broker # noqa: E402
from testing.tests.test_hermes_scm_broker_support import _receive_command # noqa: E402
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
from testing.tests.test_hermes_execution_pool_mediator import ( # noqa: E402
KEY,
RESULT,
assignment,
binding,
payload,
)
def attempt_assignment(attempt):
"""One signed assignment envelope bound to an exact retry attempt."""
return protocol.sign_envelope(
KEY, "assignment", binding(attempt=attempt), payload()
)
def test_submission_refs_are_creation_only_and_attempt_scoped():
first = scm.submission_refs("feature/pool", 1, "a" * 40)
assert first[0] == "feature/pool"
assert first[1] == "feature/pool-attempt-1"
assert first[2] == f"feature/pool-{'a' * 12}"
retry = scm.submission_refs("feature/pool", 3, "b" * 40)
assert retry[1] == "feature/pool-attempt-3"
# Every candidate stays inside a reviewed namespace the broker accepts, and
# none of them is a protected or base ref.
for candidate in first + retry:
assert receive_pack_scan.FEATURE_REF_RE.fullmatch(f"refs/heads/{candidate}")
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
assert candidate not in {"main", "master"}
def test_submission_refs_drop_oversized_names_and_fail_closed_when_empty():
long_branch = "feature/" + "a" * 185
candidates = scm.submission_refs(long_branch, 2, "c" * 40)
assert candidates == (long_branch,)
with pytest.raises(protocol.ProtocolError, match="reviewed branch name"):
scm.submission_refs("main", 1, "d" * 40)
def test_the_broker_accepts_the_attempt_ref_as_a_creation_and_still_refuses_updates():
zero, head = b"0" * 40, b"1" * 40
ref = b"refs/heads/feature/pool-attempt-2"
scm_broker._validate_receive_pack(_receive_command(zero, head, ref), "token")
update = _receive_command(b"2" * 40, head, ref)
with pytest.raises(scm_broker.PolicyError, match="feature-branch creation"):
scm_broker._validate_receive_pack(update, "token")
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 submit_harness(tmp_path, monkeypatch, remote_refs, *, attempt=1):
"""A Boundary whose broker advertises exactly ``remote_refs``."""
monkeypatch.setattr(scm, "WORKSPACE_ROOT", tmp_path / "workspace")
monkeypatch.setattr(scm, "SCM_ROOT", tmp_path / "state")
monkeypatch.setattr(scm, "ORDINAL", 0)
exact = attempt_assignment(attempt)
destination = scm.workspace_path(exact)
destination.mkdir(parents=True)
protocol.atomic_json(scm._state_path(exact), {"baseline_sha": "a" * 40})
head = "b" * 40
monkeypatch.setattr(scm, "_workspace_identity", lambda *_a: head)
calls: list[tuple[str, ...]] = []
def run(*arguments, **_kwargs):
calls.append(arguments)
if arguments[0] == "ls-remote":
return "\n".join(
f"{sha}\trefs/heads/{ref}" for ref, sha in remote_refs.items()
)
if arguments[0] == "rev-list":
return "2"
return ""
monkeypatch.setattr(scm, "_run", run)
boundary = scm.Boundary(KEY)
monkeypatch.setattr(boundary, "_draft", lambda *args: f"https://scm/pulls/{args[1]}")
return boundary, exact, calls, head
def test_a_retry_publishes_an_attempt_ref_instead_of_updating_the_existing_one(
tmp_path, monkeypatch
):
boundary, exact, calls, _head = submit_harness(
tmp_path, monkeypatch, {"wt/t_deadbeef": "c" * 40}, attempt=2
)
result = boundary.submit(exact, {"title": "retry", "body": "evidence"})
pushes = [item for item in calls if item[0] == "push"]
assert pushes == [("push", "hermes-broker", "HEAD:refs/heads/wt/t_deadbeef-attempt-2")]
assert result["branch"] == "wt/t_deadbeef-attempt-2"
assert result["pull_request"] == "https://scm/pulls/wt/t_deadbeef-attempt-2"
def test_an_already_published_head_is_adopted_without_a_second_push(
tmp_path, monkeypatch
):
boundary, exact, calls, head = submit_harness(
tmp_path, monkeypatch, {"wt/t_deadbeef-attempt-2": "b" * 40}, attempt=2
)
assert head == "b" * 40
result = boundary.submit(exact, {"title": "replay", "body": "evidence"})
assert [item for item in calls if item[0] == "push"] == []
assert result["branch"] == "wt/t_deadbeef-attempt-2"
assert result["pull_request"].endswith("wt/t_deadbeef-attempt-2")
def test_a_run_with_no_new_commits_still_reports_work_a_prior_attempt_published(
tmp_path, monkeypatch
):
boundary, exact, calls, _head = submit_harness(
tmp_path, monkeypatch, {"wt/t_deadbeef": "b" * 40}
)
real_run = scm._run
def run(*arguments, **kwargs):
if arguments[0] == "rev-list":
return "0"
return real_run(*arguments, **kwargs)
monkeypatch.setattr(scm, "_run", run)
result = boundary.submit(exact, {"title": "nothing new", "body": "evidence"})
assert [item for item in calls if item[0] == "push"] == []
assert result["pull_request"].endswith("wt/t_deadbeef")
def test_a_run_that_produced_nothing_at_all_submits_nothing(tmp_path, monkeypatch):
boundary, exact, calls, _head = submit_harness(tmp_path, monkeypatch, {})
real_run = scm._run
def run(*arguments, **kwargs):
if arguments[0] == "rev-list":
return "0"
return real_run(*arguments, **kwargs)
monkeypatch.setattr(scm, "_run", run)
result = boundary.submit(exact, {})
assert [item for item in calls if item[0] == "push"] == []
assert result["pull_request"] == "" and result["branch"] == "wt/t_deadbeef"
def test_submission_fails_closed_when_every_candidate_ref_is_taken(
tmp_path, monkeypatch
):
taken = {
"wt/t_deadbeef": "c" * 40,
"wt/t_deadbeef-attempt-1": "d" * 40,
f"wt/t_deadbeef-{'b' * 12}": "e" * 40,
}
boundary, exact, calls, _head = submit_harness(tmp_path, monkeypatch, taken)
with pytest.raises(protocol.ProtocolError, match="every reviewed branch name"):
boundary.submit(exact, {"title": "blocked", "body": "evidence"})
assert [item for item in calls if item[0] == "push"] == []
def test_remote_head_parsing_ignores_anything_that_is_not_a_branch(
tmp_path, monkeypatch
):
monkeypatch.setattr(
scm, "_run",
lambda *_a, **_k: "aaa\trefs/tags/v1\nbbb\trefs/heads/feature/x\ngarbage\n",
)
assert scm._remote_heads(tmp_path, ("feature/x",)) == {"feature/x": "bbb"}
class FailingSCM:
def __init__(self, error):
self.error = error
self.calls = 0
def submit(self, _assignment, _request):
self.calls += 1
raise self.error
def test_a_refused_submission_downgrades_the_result_instead_of_discarding_it():
failing = FailingSCM(protocol.ProtocolError("every reviewed branch name is taken"))
boundary = client.ClientBoundary(KEY, failing)
boundary.current = assignment()
boundary._post = lambda _path, _envelope: protocol.sign_envelope(
KEY, "ack", binding(), {"accepted": True, "duplicate": False}
)
finished = boundary.finish(
{
"binding": binding(),
"payload": {"structured": dict(RESULT), "returncode": 0},
"title": "safe", "body": "evidence",
}
)
assert failing.calls == 1
structured = finished["structured"]
# The run is recorded rather than unwound, and it says exactly why.
assert structured["status"] == "blocked"
assert structured["changed_files"] == ["safe.py"]
assert len(structured["blockers"]) == 1
assert "commits remain on this ordinal's workspace" in structured["blockers"][0]
assert "every reviewed branch name is taken" in structured["blockers"][0]
assert finished["ack"]["accepted"] is True
def test_a_successful_submission_records_both_the_draft_and_the_exact_branch():
class Recording:
def submit(self, _assignment, _request):
return {
"workspace": "/workspace/runs/metis/t_deadbeef/42",
"branch": "wt/t_deadbeef-attempt-2",
"pull_request": "https://scm/pulls/9",
}
boundary = client.ClientBoundary(KEY, Recording())
boundary.current = assignment()
boundary._post = lambda _path, _envelope: protocol.sign_envelope(
KEY, "ack", binding(), {"accepted": True, "duplicate": False}
)
finished = boundary.finish(
{
"binding": binding(),
"payload": {"structured": dict(RESULT), "returncode": 0},
"title": "safe", "body": "evidence",
}
)
assert finished["structured"]["status"] == "completed"
assert finished["structured"]["artifacts"] == [
"https://scm/pulls/9", "branch:wt/t_deadbeef-attempt-2",
]