Skip to content

Commit 2ffde24

Browse files
authored
fix: page ready agents during health-check startup (#345)
## Summary - page through READY agents when starting health-check workflows at startup - avoid loading non-ready agents just to filter them in Python - add a focused unit test covering pagination beyond one page ## Tests - uv run ruff check src/temporal/run_healthcheck_workflow.py tests/unit/temporal/test_run_healthcheck_workflow.py - uv run --group test pytest tests/unit/temporal/test_run_healthcheck_workflow.py -q <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR fixes the health-check startup path to page through only `READY` agents server-side, replacing a full table scan followed by in-Python filtering. A new `READY_AGENT_PAGE_SIZE = 200` constant drives offset-based pagination with a stable `order_by="id"` sort, and two focused unit tests verify both the multi-page and exact-page-size boundary paths. - **`run_healthcheck_workflow.py`**: Replaces a single `agent_repo.list()` call (no filter) with a `while True` pagination loop filtered on `status=READY`, breaking on an empty page or a partial page. - **`test_run_healthcheck_workflow.py`**: New test file with a helper that monkeypatches all external dependencies; covers multi-page traversal and the exact-multiple-of-page-size edge case. <details><summary><h3>Confidence Score: 5/5</h3></summary> Safe to merge — the change is a straightforward pagination refactor with no mutations, no new external dependencies, and deterministic termination. The pagination loop correctly handles all exit conditions (empty page, partial page), uses a stable immutable sort key, and pushes filtering to the database. Both new tests exercise the logic end-to-end with full dependency isolation. No regressions expected. No files require special attention. </details> <details><summary><h3>Important Files Changed</h3></summary> | Filename | Overview | |----------|----------| | agentex/src/temporal/run_healthcheck_workflow.py | Pagination loop is correct — stable sort key (`order_by="id"`), server-side status filter, and two break conditions handle both partial-page and empty-page termination. No issues found. | | agentex/tests/unit/temporal/test_run_healthcheck_workflow.py | New unit tests cover the multi-page and exact-page-size boundary cases with thorough monkeypatching; helper is structured cleanly as a reusable async coroutine. | </details> <details><summary><h3>Flowchart</h3></summary> <a href="#gh-light-mode-only"> ```mermaid %%{init: {'theme': 'neutral'}}%% flowchart TD A[main starts] --> B[load GlobalDependencies] B --> C{env configured?} C -- no --> Z[return] C -- yes --> D{health-check enabled?} D -- no --> Z D -- yes --> E{Temporal configured?} E -- no --> Z E -- yes --> F[init AgentRepository + TemporalAdapter] F --> G[page_number = 1] G --> H[agent_repo.list status=READY limit=200 page=N order_by=id] H --> I{agents empty?} I -- yes --> Z I -- no --> J[for each agent: start_workflow healthcheck_workflow_agentId] J --> K{len agents < 200?} K -- yes --> Z K -- no --> L[page_number += 1] L --> H ``` </a> <a href="#gh-dark-mode-only"> ```mermaid %%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A[main starts] --> B[load GlobalDependencies] B --> C{env configured?} C -- no --> Z[return] C -- yes --> D{health-check enabled?} D -- no --> Z D -- yes --> E{Temporal configured?} E -- no --> Z E -- yes --> F[init AgentRepository + TemporalAdapter] F --> G[page_number = 1] G --> H[agent_repo.list status=READY limit=200 page=N order_by=id] H --> I{agents empty?} I -- yes --> Z I -- no --> J[for each agent: start_workflow healthcheck_workflow_agentId] J --> K{len agents < 200?} K -- yes --> Z K -- no --> L[page_number += 1] L --> H ``` </a> </details> <sub>Reviews (5): Last reviewed commit: ["Merge branch &#39;main&#39; into fix/page-ready-..."](5fa8d7e) | [Re-trigger Greptile](https://app.greptile.com/api/retrigger?id=41266248)</sub> <!-- /greptile_comment -->
1 parent 38f917b commit 2ffde24

2 files changed

Lines changed: 189 additions & 24 deletions

File tree

agentex/src/temporal/run_healthcheck_workflow.py

Lines changed: 38 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
from src.utils.logging import make_logger
1919

2020
logger = make_logger(__name__)
21+
READY_AGENT_PAGE_SIZE = 200
2122

2223

2324
async def main() -> None:
@@ -45,37 +46,50 @@ async def main() -> None:
4546
logger.error("Temporal is not configured, skipping workflow creation")
4647
return
4748

48-
# Initialize repository and list agents
49+
# Initialize repository and list ready agents
4950
engine = database_async_read_write_engine()
5051
session_maker = database_async_read_write_session_maker(engine)
5152
read_only_session_maker = database_async_read_only_session_maker(engine)
5253
agent_repo = AgentRepository(session_maker, read_only_session_maker)
53-
agents = await agent_repo.list()
5454

5555
adapter = TemporalAdapter(temporal_client=global_dependencies.temporal_client)
5656
logger.info(f"Adding Health Check workflows to task queue: {task_queue}")
57-
# Try to add health check workflows to task queue for each agent
58-
for agent in agents:
59-
if agent.status != AgentStatus.READY:
60-
logger.info(
61-
f"Agent {agent.id} is not ready, skipping health check workflow"
62-
)
63-
continue
64-
try:
65-
await adapter.start_workflow(
66-
workflow_id=f"healthcheck_workflow_{agent.id}",
67-
workflow=HealthCheckWorkflow,
68-
args=[{"agent_id": agent.id, "acp_url": agent.acp_url}],
69-
task_queue=task_queue,
70-
)
71-
except TemporalWorkflowAlreadyExistsError:
72-
# Expected if workflow is already running for existing agent registration
73-
logger.info(f"Health check workflow already exists for agent {agent.id}")
74-
except Exception as e:
75-
# Unexpected error, don't raise here to continue with the next agent
76-
logger.error(
77-
f"Failed to start health check workflow for agent {agent.id}: {e}"
78-
)
57+
58+
page_number = 1
59+
while True:
60+
agents = await agent_repo.list(
61+
filters={"status": AgentStatus.READY},
62+
limit=READY_AGENT_PAGE_SIZE,
63+
page_number=page_number,
64+
order_by="id",
65+
order_direction="asc",
66+
)
67+
if not agents:
68+
break
69+
70+
# Try to add health check workflows to task queue for each ready agent
71+
for agent in agents:
72+
try:
73+
await adapter.start_workflow(
74+
workflow_id=f"healthcheck_workflow_{agent.id}",
75+
workflow=HealthCheckWorkflow,
76+
args=[{"agent_id": agent.id, "acp_url": agent.acp_url}],
77+
task_queue=task_queue,
78+
)
79+
except TemporalWorkflowAlreadyExistsError:
80+
# Expected if workflow is already running for existing agent registration
81+
logger.info(
82+
f"Health check workflow already exists for agent {agent.id}"
83+
)
84+
except Exception as e:
85+
# Unexpected error, don't raise here to continue with the next agent
86+
logger.error(
87+
f"Failed to start health check workflow for agent {agent.id}: {e}"
88+
)
89+
90+
if len(agents) < READY_AGENT_PAGE_SIZE:
91+
break
92+
page_number += 1
7993

8094

8195
if __name__ == "__main__":
Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
from types import SimpleNamespace
2+
3+
import pytest
4+
from src.temporal import run_healthcheck_workflow
5+
6+
7+
async def _run_main_with_pages(monkeypatch, pages):
8+
class FakeGlobalDependencies:
9+
temporal_client = object()
10+
11+
async def load(self):
12+
return None
13+
14+
class FakeAgentRepository:
15+
def __init__(self, *args):
16+
self.calls = []
17+
18+
async def list(
19+
self,
20+
filters,
21+
limit,
22+
page_number,
23+
order_by,
24+
order_direction,
25+
):
26+
self.calls.append(
27+
{
28+
"filters": filters,
29+
"limit": limit,
30+
"page_number": page_number,
31+
"order_by": order_by,
32+
"order_direction": order_direction,
33+
}
34+
)
35+
return pages.get(page_number, [])
36+
37+
class FakeTemporalAdapter:
38+
def __init__(self, temporal_client):
39+
self.temporal_client = temporal_client
40+
self.started_workflows = []
41+
42+
async def start_workflow(self, **kwargs):
43+
self.started_workflows.append(kwargs)
44+
45+
fake_repo = FakeAgentRepository()
46+
fake_adapter = FakeTemporalAdapter(object())
47+
fake_env = SimpleNamespace(
48+
ENABLE_HEALTH_CHECK_WORKFLOW=True,
49+
AGENTEX_SERVER_TASK_QUEUE="agentex-server",
50+
)
51+
52+
monkeypatch.setattr(
53+
run_healthcheck_workflow,
54+
"GlobalDependencies",
55+
FakeGlobalDependencies,
56+
)
57+
monkeypatch.setattr(
58+
run_healthcheck_workflow.EnvironmentVariables,
59+
"refresh",
60+
lambda: fake_env,
61+
)
62+
monkeypatch.setattr(
63+
run_healthcheck_workflow.TemporalClientFactory,
64+
"is_temporal_configured",
65+
lambda env: True,
66+
)
67+
monkeypatch.setattr(
68+
run_healthcheck_workflow,
69+
"database_async_read_write_engine",
70+
lambda: object(),
71+
)
72+
monkeypatch.setattr(
73+
run_healthcheck_workflow,
74+
"database_async_read_write_session_maker",
75+
lambda engine: object(),
76+
)
77+
monkeypatch.setattr(
78+
run_healthcheck_workflow,
79+
"database_async_read_only_session_maker",
80+
lambda engine: object(),
81+
)
82+
monkeypatch.setattr(
83+
run_healthcheck_workflow,
84+
"AgentRepository",
85+
lambda *args: fake_repo,
86+
)
87+
monkeypatch.setattr(
88+
run_healthcheck_workflow,
89+
"TemporalAdapter",
90+
lambda temporal_client: fake_adapter,
91+
)
92+
93+
await run_healthcheck_workflow.main()
94+
95+
return fake_repo, fake_adapter
96+
97+
98+
def _agents(count: int, prefix: str = "agent"):
99+
return [
100+
SimpleNamespace(id=f"{prefix}-{i}", acp_url=f"http://{prefix}-{i}")
101+
for i in range(count)
102+
]
103+
104+
105+
def _expected_call(page_number: int):
106+
return {
107+
"filters": {"status": run_healthcheck_workflow.AgentStatus.READY},
108+
"limit": run_healthcheck_workflow.READY_AGENT_PAGE_SIZE,
109+
"page_number": page_number,
110+
"order_by": "id",
111+
"order_direction": "asc",
112+
}
113+
114+
115+
@pytest.mark.asyncio
116+
@pytest.mark.unit
117+
async def test_main_pages_ready_agents_for_healthcheck(monkeypatch):
118+
final_agent = SimpleNamespace(id="agent-final", acp_url="http://agent-final")
119+
fake_repo, fake_adapter = await _run_main_with_pages(
120+
monkeypatch,
121+
{
122+
1: _agents(run_healthcheck_workflow.READY_AGENT_PAGE_SIZE),
123+
2: [final_agent],
124+
},
125+
)
126+
127+
assert fake_repo.calls == [_expected_call(1), _expected_call(2)]
128+
assert len(fake_adapter.started_workflows) == (
129+
run_healthcheck_workflow.READY_AGENT_PAGE_SIZE + 1
130+
)
131+
assert (
132+
fake_adapter.started_workflows[-1]["workflow_id"]
133+
== "healthcheck_workflow_agent-final"
134+
)
135+
136+
137+
@pytest.mark.asyncio
138+
@pytest.mark.unit
139+
async def test_main_checks_empty_page_for_exact_page_size(monkeypatch):
140+
fake_repo, fake_adapter = await _run_main_with_pages(
141+
monkeypatch,
142+
{
143+
1: _agents(run_healthcheck_workflow.READY_AGENT_PAGE_SIZE),
144+
2: [],
145+
},
146+
)
147+
148+
assert fake_repo.calls == [_expected_call(1), _expected_call(2)]
149+
assert len(fake_adapter.started_workflows) == (
150+
run_healthcheck_workflow.READY_AGENT_PAGE_SIZE
151+
)

0 commit comments

Comments
 (0)