hermes: harden worker isolation and blocked-task semantics
Three narrowly scoped Hermes reliability fixes backed by live evidence from the Cassandra/titan-iac proof run. Worker concurrency. Three simultaneous direct CLI workers on the 4-core hermes-agent node drove load to ~45 and made the hermes and oauth2-proxy containers fail their probes, leaving the pod 8/10 Ready; two workers stayed at 10/10. Cap HERMES_CLI_LANE_CONCURRENCY at 2 and lower the cli-lane-runner CPU limit from 3 to 2 so the dashboard and auth sidecars keep a guaranteed share of the node. Requests are unchanged: the pod still asks for 745m total, so placement does not move. Service links. Kubernetes injects a service-link variable pair for every service in the namespace, and hermes-claude-broker produces HERMES_CLAUDE_BROKER_PORT=tcp://10.43.31.76:9006 — a value the broker parses as an int. That contaminated worker and test environments even though the deployment already addresses every service by DNS name. Set enableServiceLinks: false on the hermes-agent pod spec. Blocked-task scheduling. create_task(initial_status="blocked") records a created event carrying status=blocked but never a blocked event, while _has_sticky_block() only inspects blocked/unblocked events. recompute_ready() considers blocked tasks, so an explicitly parked task with no incomplete parent auto-promoted on the next dispatcher cycle. Teach _has_sticky_block() to also recognize a created event whose payload status is blocked, which covers tasks created before this image patch without adding a persisted field. Dependency-driven promotion and the circuit-breaker failure-limit guard are untouched; unblock_task() still releases either kind of block. hermes-kanban-blocked-regression.py runs against the real upstream kanban_db API during the image build, so the build fails if any of these semantics regress. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
0dd6ea0f02
commit
4f8dcfbbf7
@ -629,16 +629,75 @@ source = source[:runs_start] + runs_source
|
|||||||
path.write_text(source)
|
path.write_text(source)
|
||||||
PY
|
PY
|
||||||
|
|
||||||
# A manual evidence-based close must not expose a parked task to the dispatcher
|
# ``create_task(initial_status="blocked")`` is an explicit operator park, but
|
||||||
# between separate `unblock` and `complete` commands. Upstream already permits
|
# upstream only treats later block_task() events as sticky. Recognize the
|
||||||
# direct completion from `blocked`; extend the same atomic path to `scheduled`
|
# existing created(status=blocked) event too. This also protects tasks created
|
||||||
# and make that contract visible in CLI help so agents do not create a ready
|
# before this image patch without making dependency or circuit-breaker blocks
|
||||||
# window that can launch redundant work.
|
# depend on a new persisted field.
|
||||||
RUN python - <<'PY'
|
RUN python - <<'PY'
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
db_path = Path("/opt/hermes/hermes_cli/kanban_db.py")
|
db_path = Path("/opt/hermes/hermes_cli/kanban_db.py")
|
||||||
db_source = db_path.read_text()
|
db_source = db_path.read_text()
|
||||||
|
sticky_doc_before = ''' The cheapest signal that distinguishes the two is the most recent
|
||||||
|
``"blocked"`` / ``"unblocked"`` event for the task. If the most
|
||||||
|
recent one is ``"blocked"`` (or there is a ``"blocked"`` event and
|
||||||
|
no ``"unblocked"`` event has fired since), the task is sticky and
|
||||||
|
``recompute_ready`` must *not* auto-promote it.
|
||||||
|
|
||||||
|
Returns ``False`` when there is no such event at all (e.g. the task
|
||||||
|
'''
|
||||||
|
sticky_doc_after = ''' The cheapest signal that distinguishes the two is the most recent
|
||||||
|
``"created"`` / ``"blocked"`` / ``"unblocked"`` event for the task.
|
||||||
|
A ``"created"`` event whose payload has ``status="blocked"`` is the
|
||||||
|
explicit park requested by ``create_task(initial_status="blocked")``.
|
||||||
|
A later ``"unblocked"`` event clears either kind of sticky block.
|
||||||
|
|
||||||
|
Returns ``False`` when there is no such event at all (e.g. the task
|
||||||
|
'''
|
||||||
|
if db_source.count(sticky_doc_before) != 1:
|
||||||
|
raise SystemExit(
|
||||||
|
"Hermes Kanban sticky-block documentation changed: expected 1, "
|
||||||
|
f"found {db_source.count(sticky_doc_before)}"
|
||||||
|
)
|
||||||
|
db_source = db_source.replace(sticky_doc_before, sticky_doc_after, 1)
|
||||||
|
|
||||||
|
sticky_before = ''' row = conn.execute(
|
||||||
|
"SELECT kind FROM task_events "
|
||||||
|
"WHERE task_id = ? AND kind IN ('blocked', 'unblocked') "
|
||||||
|
"ORDER BY id DESC LIMIT 1",
|
||||||
|
(task_id,),
|
||||||
|
).fetchone()
|
||||||
|
return bool(row) and row["kind"] == "blocked"
|
||||||
|
'''
|
||||||
|
sticky_after = ''' row = conn.execute(
|
||||||
|
"SELECT kind, payload FROM task_events "
|
||||||
|
"WHERE task_id = ? AND kind IN ('created', 'blocked', 'unblocked') "
|
||||||
|
"ORDER BY id DESC LIMIT 1",
|
||||||
|
(task_id,),
|
||||||
|
).fetchone()
|
||||||
|
if not row or row["kind"] == "unblocked":
|
||||||
|
return False
|
||||||
|
if row["kind"] == "blocked":
|
||||||
|
return True
|
||||||
|
try:
|
||||||
|
payload = json.loads(row["payload"] or "{}")
|
||||||
|
except (TypeError, ValueError):
|
||||||
|
return False
|
||||||
|
return payload.get("status") == "blocked"
|
||||||
|
'''
|
||||||
|
if db_source.count(sticky_before) != 1:
|
||||||
|
raise SystemExit(
|
||||||
|
"Hermes Kanban sticky-block gate changed: expected 1, "
|
||||||
|
f"found {db_source.count(sticky_before)}"
|
||||||
|
)
|
||||||
|
db_source = db_source.replace(sticky_before, sticky_after, 1)
|
||||||
|
|
||||||
|
# A manual evidence-based close must not expose a parked task to the dispatcher
|
||||||
|
# between separate `unblock` and `complete` commands. Upstream already permits
|
||||||
|
# direct completion from `blocked`; extend the same atomic path to `scheduled`
|
||||||
|
# and make that contract visible in CLI help so agents do not create a ready
|
||||||
|
# window that can launch redundant work.
|
||||||
state_before = ''' WHERE id = ?
|
state_before = ''' WHERE id = ?
|
||||||
AND status IN ('running', 'ready', 'blocked')
|
AND status IN ('running', 'ready', 'blocked')
|
||||||
'''
|
'''
|
||||||
@ -671,6 +730,10 @@ if cli_source.count(help_before) != 1:
|
|||||||
cli_path.write_text(cli_source.replace(help_before, help_after, 1))
|
cli_path.write_text(cli_source.replace(help_before, help_after, 1))
|
||||||
PY
|
PY
|
||||||
|
|
||||||
|
COPY dockerfiles/hermes-kanban-blocked-regression.py /tmp/hermes-kanban-blocked-regression.py
|
||||||
|
RUN /opt/hermes/.venv/bin/python /tmp/hermes-kanban-blocked-regression.py \
|
||||||
|
&& rm /tmp/hermes-kanban-blocked-regression.py
|
||||||
|
|
||||||
COPY dockerfiles/hermes-python-sandbox-tool.py /opt/hermes/tools/python_sandbox_tool.py
|
COPY dockerfiles/hermes-python-sandbox-tool.py /opt/hermes/tools/python_sandbox_tool.py
|
||||||
COPY dockerfiles/hermes-public-extract/__init__.py /opt/hermes/plugins/web/public_extract/__init__.py
|
COPY dockerfiles/hermes-public-extract/__init__.py /opt/hermes/plugins/web/public_extract/__init__.py
|
||||||
COPY dockerfiles/hermes-public-extract/plugin.yaml /opt/hermes/plugins/web/public_extract/plugin.yaml
|
COPY dockerfiles/hermes-public-extract/plugin.yaml /opt/hermes/plugins/web/public_extract/plugin.yaml
|
||||||
|
|||||||
@ -1,4 +1,5 @@
|
|||||||
**
|
**
|
||||||
|
!dockerfiles/hermes-kanban-blocked-regression.py
|
||||||
!dockerfiles/hermes-python-sandbox-tool.py
|
!dockerfiles/hermes-python-sandbox-tool.py
|
||||||
!dockerfiles/hermes-public-extract/
|
!dockerfiles/hermes-public-extract/
|
||||||
!dockerfiles/hermes-public-extract/**
|
!dockerfiles/hermes-public-extract/**
|
||||||
|
|||||||
110
dockerfiles/hermes-kanban-blocked-regression.py
Normal file
110
dockerfiles/hermes-kanban-blocked-regression.py
Normal file
@ -0,0 +1,110 @@
|
|||||||
|
"""Regression tests for Hermes Kanban blocked-task scheduling semantics."""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import os
|
||||||
|
import tempfile
|
||||||
|
import unittest
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
|
||||||
|
_HERMES_HOME = tempfile.TemporaryDirectory(prefix="hermes-kanban-test-home-")
|
||||||
|
os.environ["HERMES_HOME"] = _HERMES_HOME.name
|
||||||
|
|
||||||
|
from hermes_cli import kanban_db # noqa: E402
|
||||||
|
|
||||||
|
|
||||||
|
class BlockedTaskSchedulingTests(unittest.TestCase):
|
||||||
|
"""Exercise the real upstream database API against isolated databases."""
|
||||||
|
|
||||||
|
def setUp(self) -> None:
|
||||||
|
self._database_dir = tempfile.TemporaryDirectory(
|
||||||
|
prefix="hermes-kanban-test-db-"
|
||||||
|
)
|
||||||
|
self.connection = kanban_db.connect(
|
||||||
|
Path(self._database_dir.name) / "kanban.db"
|
||||||
|
)
|
||||||
|
|
||||||
|
def tearDown(self) -> None:
|
||||||
|
self.connection.close()
|
||||||
|
self._database_dir.cleanup()
|
||||||
|
|
||||||
|
def create_task(self, title: str, **kwargs: object) -> str:
|
||||||
|
return kanban_db.create_task(self.connection, title=title, **kwargs)
|
||||||
|
|
||||||
|
def status(self, task_id: str) -> str:
|
||||||
|
task = kanban_db.get_task(self.connection, task_id)
|
||||||
|
self.assertIsNotNone(task)
|
||||||
|
return task.status
|
||||||
|
|
||||||
|
def test_initial_block_without_parents_is_sticky(self) -> None:
|
||||||
|
task_id = self.create_task("operator parked", initial_status="blocked")
|
||||||
|
|
||||||
|
self.assertEqual(kanban_db.recompute_ready(self.connection), 0)
|
||||||
|
self.assertEqual(self.status(task_id), "blocked")
|
||||||
|
|
||||||
|
def test_initial_block_with_complete_parents_is_sticky(self) -> None:
|
||||||
|
parent_id = self.create_task("completed prerequisite")
|
||||||
|
self.assertTrue(kanban_db.complete_task(self.connection, parent_id))
|
||||||
|
task_id = self.create_task(
|
||||||
|
"operator parked after prerequisite",
|
||||||
|
initial_status="blocked",
|
||||||
|
parents=[parent_id],
|
||||||
|
)
|
||||||
|
|
||||||
|
self.assertEqual(kanban_db.recompute_ready(self.connection), 0)
|
||||||
|
self.assertEqual(self.status(task_id), "blocked")
|
||||||
|
|
||||||
|
def test_initial_block_is_released_by_an_explicit_unblock(self) -> None:
|
||||||
|
task_id = self.create_task("operator parked", initial_status="blocked")
|
||||||
|
|
||||||
|
self.assertTrue(kanban_db.unblock_task(self.connection, task_id))
|
||||||
|
self.assertEqual(self.status(task_id), "ready")
|
||||||
|
|
||||||
|
def test_dependency_block_promotes_after_parent_completes(self) -> None:
|
||||||
|
parent_id = self.create_task("incomplete prerequisite")
|
||||||
|
task_id = self.create_task("dependency gated", parents=[parent_id])
|
||||||
|
self.assertEqual(self.status(task_id), "todo")
|
||||||
|
|
||||||
|
self.assertTrue(kanban_db.complete_task(self.connection, parent_id))
|
||||||
|
self.assertEqual(self.status(task_id), "ready")
|
||||||
|
|
||||||
|
def test_explicit_block_and_unblock_remain_sticky_and_reversible(self) -> None:
|
||||||
|
task_id = self.create_task("worker handoff")
|
||||||
|
|
||||||
|
self.assertTrue(
|
||||||
|
kanban_db.block_task(
|
||||||
|
self.connection,
|
||||||
|
task_id,
|
||||||
|
reason="human review required",
|
||||||
|
)
|
||||||
|
)
|
||||||
|
self.assertEqual(kanban_db.recompute_ready(self.connection), 0)
|
||||||
|
self.assertEqual(self.status(task_id), "blocked")
|
||||||
|
self.assertTrue(kanban_db.unblock_task(self.connection, task_id))
|
||||||
|
self.assertEqual(self.status(task_id), "ready")
|
||||||
|
|
||||||
|
def test_circuit_breaker_block_is_not_auto_promoted(self) -> None:
|
||||||
|
task_id = self.create_task("repeated worker failure")
|
||||||
|
self.assertIsNotNone(kanban_db.claim_task(self.connection, task_id))
|
||||||
|
|
||||||
|
# Upstream has no public circuit-breaker entry point; exercise the
|
||||||
|
# dispatcher helper that owns its persisted failure-limit semantics.
|
||||||
|
self.assertTrue(
|
||||||
|
kanban_db._record_spawn_failure(
|
||||||
|
self.connection,
|
||||||
|
task_id,
|
||||||
|
"worker failed",
|
||||||
|
failure_limit=1,
|
||||||
|
)
|
||||||
|
)
|
||||||
|
self.assertEqual(self.status(task_id), "blocked")
|
||||||
|
self.assertEqual(
|
||||||
|
kanban_db.recompute_ready(self.connection, failure_limit=1),
|
||||||
|
0,
|
||||||
|
)
|
||||||
|
self.assertEqual(self.status(task_id), "blocked")
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
unittest.main(verbosity=2)
|
||||||
@ -91,6 +91,7 @@ spec:
|
|||||||
vault.hashicorp.com/agent-limits-mem: 128Mi
|
vault.hashicorp.com/agent-limits-mem: 128Mi
|
||||||
spec:
|
spec:
|
||||||
serviceAccountName: hermes-agent
|
serviceAccountName: hermes-agent
|
||||||
|
enableServiceLinks: false
|
||||||
automountServiceAccountToken: true
|
automountServiceAccountToken: true
|
||||||
securityContext:
|
securityContext:
|
||||||
fsGroup: 10000
|
fsGroup: 10000
|
||||||
@ -873,7 +874,7 @@ spec:
|
|||||||
- {name: CLAUDE_CONFIG_DIR, value: /runtime-access/claude}
|
- {name: CLAUDE_CONFIG_DIR, value: /runtime-access/claude}
|
||||||
- {name: KUBECONFIG, value: /opt/data/home/.kube/config}
|
- {name: KUBECONFIG, value: /opt/data/home/.kube/config}
|
||||||
- {name: PYTHONPATH, value: /opt/hermes}
|
- {name: PYTHONPATH, value: /opt/hermes}
|
||||||
- {name: HERMES_CLI_LANE_CONCURRENCY, value: "4"}
|
- {name: HERMES_CLI_LANE_CONCURRENCY, value: "2"}
|
||||||
- {name: HERMES_AUTO_ROUTER_PROFILE, value: agent}
|
- {name: HERMES_AUTO_ROUTER_PROFILE, value: agent}
|
||||||
- {name: PATH, value: /opt/coordinator:/opt/data/tools/bin:/opt/data/home/.local/bin:/opt/hermes/.venv/bin:/usr/local/bin:/usr/bin:/bin}
|
- {name: PATH, value: /opt/coordinator:/opt/data/tools/bin:/opt/data/home/.local/bin:/opt/hermes/.venv/bin:/usr/local/bin:/usr/bin:/bin}
|
||||||
securityContext:
|
securityContext:
|
||||||
@ -896,7 +897,7 @@ spec:
|
|||||||
- {name: tmp, mountPath: /tmp}
|
- {name: tmp, mountPath: /tmp}
|
||||||
resources:
|
resources:
|
||||||
requests: {cpu: 100m, memory: 256Mi}
|
requests: {cpu: 100m, memory: 256Mi}
|
||||||
limits: {cpu: "3", memory: 6Gi}
|
limits: {cpu: "2", memory: 6Gi}
|
||||||
- name: model-steward
|
- name: model-steward
|
||||||
image: registry.bstein.dev/bstein/hermes-agent@sha256:81970563e542f0720773e72297810b3a844b83e381e278f25c0916c78d930107
|
image: registry.bstein.dev/bstein/hermes-agent@sha256:81970563e542f0720773e72297810b3a844b83e381e278f25c0916c78d930107
|
||||||
imagePullPolicy: IfNotPresent
|
imagePullPolicy: IfNotPresent
|
||||||
|
|||||||
@ -120,6 +120,9 @@ def test_agent_image_completes_parked_kanban_tasks_atomically():
|
|||||||
"Atomically mark running, ready, blocked, or scheduled tasks done"
|
"Atomically mark running, ready, blocked, or scheduled tasks done"
|
||||||
in dockerfile
|
in dockerfile
|
||||||
)
|
)
|
||||||
|
assert "kind IN ('created', 'blocked', 'unblocked')" in dockerfile
|
||||||
|
assert "payload.get(\"status\") == \"blocked\"" in dockerfile
|
||||||
|
assert "hermes-kanban-blocked-regression.py" in dockerfile
|
||||||
|
|
||||||
|
|
||||||
def test_sandbox_shares_only_the_tenant_workspace_without_credentials():
|
def test_sandbox_shares_only_the_tenant_workspace_without_credentials():
|
||||||
|
|||||||
@ -1074,6 +1074,7 @@ def test_agent_uses_one_native_kanban_control_plane():
|
|||||||
|
|
||||||
deployment = _agent_deployment()
|
deployment = _agent_deployment()
|
||||||
pod = deployment["spec"]["template"]["spec"]
|
pod = deployment["spec"]["template"]["spec"]
|
||||||
|
assert pod["enableServiceLinks"] is False
|
||||||
names = {item["name"] for item in pod["containers"]}
|
names = {item["name"] for item in pod["containers"]}
|
||||||
assert "cli-lane-runner" in names
|
assert "cli-lane-runner" in names
|
||||||
assert "terminal" in names
|
assert "terminal" in names
|
||||||
@ -1083,6 +1084,22 @@ def test_agent_uses_one_native_kanban_control_plane():
|
|||||||
assert "herdr-dispatch" not in rendered
|
assert "herdr-dispatch" not in rendered
|
||||||
|
|
||||||
|
|
||||||
|
def test_cli_lane_reserves_cpu_headroom_for_ui_and_auth():
|
||||||
|
deployment = _agent_deployment()
|
||||||
|
containers = {
|
||||||
|
item["name"]: item
|
||||||
|
for item in deployment["spec"]["template"]["spec"]["containers"]
|
||||||
|
}
|
||||||
|
lane = containers["cli-lane-runner"]
|
||||||
|
environment = {item["name"]: item["value"] for item in lane["env"]}
|
||||||
|
|
||||||
|
assert environment["HERMES_CLI_LANE_CONCURRENCY"] == "2"
|
||||||
|
assert lane["resources"] == {
|
||||||
|
"requests": {"cpu": "100m", "memory": "256Mi"},
|
||||||
|
"limits": {"cpu": "2", "memory": "6Gi"},
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
def test_agent_root_is_stock_dashboard_and_terminal_is_a_separate_path():
|
def test_agent_root_is_stock_dashboard_and_terminal_is_a_separate_path():
|
||||||
deployment = _agent_deployment()
|
deployment = _agent_deployment()
|
||||||
pod = deployment["spec"]["template"]["spec"]
|
pod = deployment["spec"]["template"]["spec"]
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user