Skip to content

Commit 29174c3

Browse files
authored
fix(workflow): dedupe progress rows, close leaked coroutines, cap runaway loops (#191)
- render_progress() re-listed agents truncated by the per-phase max_agents cap as duplicate "unphased" rows; now filters by phase membership instead of a rendered-id set. - parallel() left already-created agent() coroutines unclosed when its argument validation raised, leaking "never awaited" warnings. - The Python port dropped the reference implementation's 1000-agent lifetime backstop, so a workflow script with an unbounded loop and no token_budget could spawn subagents forever; restored the cap.
1 parent 3c44c7e commit 29174c3

5 files changed

Lines changed: 75 additions & 3 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ GitHub Releases page; `0.8.0` is the new starting line.
1515

1616
## Unreleased
1717

18+
- Fix Workflow progress rendering duplicating agents truncated by the per-phase
19+
display cap, close leaked `agent()` coroutines when `parallel()` rejects its
20+
arguments, and add a 1000-agent lifetime backstop against runaway workflow loops.
1821
- Fix stale update-success notices so restarting into an older Homebrew install
1922
shows `/update` again instead of a permanent "Restart to apply" banner.
2023

src/pythinker_code/tools/workflow/display.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,6 @@ def render_progress(snapshot: WorkflowSnapshot, max_agents: int = 6, max_logs: i
8484
lines = [
8585
f"◆ Workflow: {snapshot.name} ({snapshot.done_count}/{len(snapshot.agents)} done{state})"
8686
]
87-
rendered: set[int] = set()
8887
phase_order = list(snapshot.phases)
8988
if snapshot.current_phase and snapshot.current_phase not in phase_order:
9089
phase_order.append(snapshot.current_phase)
@@ -95,9 +94,10 @@ def render_progress(snapshot: WorkflowSnapshot, max_agents: int = 6, max_logs: i
9594
done = sum(1 for a in agents if a.status == "done")
9695
lines.append(f" {phase} {done}/{len(agents)}")
9796
for agent in agents[-max_agents:]:
98-
rendered.add(agent.id)
9997
lines.append(f" #{agent.id} {_STATUS_ICON.get(agent.status, '?')} {agent.label}")
100-
unphased = [a for a in snapshot.agents if a.id not in rendered]
98+
# Membership in phase_order, not an id set of rendered rows: agents cut by
99+
# the per-phase max_agents tail must stay truncated, not reappear here.
100+
unphased = [a for a in snapshot.agents if a.phase not in phase_order]
101101
for agent in unphased[-max_agents:]:
102102
lines.append(f" #{agent.id} {_STATUS_ICON.get(agent.status, '?')} {agent.label}")
103103
for message in snapshot.logs[-max_logs:]:

src/pythinker_code/tools/workflow/engine.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,12 @@ class WorkflowRuntimeError(Exception):
191191
"""Raised when a workflow script misuses a primitive at runtime."""
192192

193193

194+
# Lifetime cap on agent() calls per workflow run — a runaway-loop backstop
195+
# (e.g. `while True: await agent(...)` with no token_budget set), mirroring
196+
# the reference implementation. Set far above any real workflow.
197+
MAX_TOTAL_AGENTS = 1000
198+
199+
194200
class AgentOptions:
195201
__slots__ = ("label", "phase", "schema", "model", "agent_type")
196202

@@ -406,6 +412,11 @@ async def agent(prompt: Any, options: Any = None) -> Any:
406412
# batch instead of the whole dispatched set.
407413
if token_budget is not None and budget.remaining() <= 0:
408414
raise WorkflowRuntimeError("workflow token budget exhausted")
415+
if state["agent_count"] >= MAX_TOTAL_AGENTS:
416+
raise WorkflowRuntimeError(
417+
f"workflow exceeded the {MAX_TOTAL_AGENTS}-agent lifetime cap; "
418+
"this is a runaway-loop backstop"
419+
)
409420
state["agent_count"] += 1
410421
# Captured into a local now, before the first `await` below: other
411422
# concurrently-dispatched agent() calls can advance
@@ -440,9 +451,14 @@ async def agent(prompt: Any, options: Any = None) -> Any:
440451

441452
async def parallel(items: Sequence[Any]) -> list[Any]:
442453
if not isinstance(items, (list, tuple)):
454+
# Close before raising: the argument may itself be (or contain)
455+
# already-created agent() coroutines that would otherwise leak
456+
# as "never awaited" warnings.
457+
_close_coroutines(items)
443458
raise WorkflowRuntimeError("parallel() expects a list of awaitables")
444459
for item in items:
445460
if callable(item) and not inspect.isawaitable(item):
461+
_close_coroutines(items)
446462
raise WorkflowRuntimeError(
447463
"parallel() expects awaitables, not functions: "
448464
"use parallel([agent('...'), agent('...')])"

tests/tools/test_workflow_display.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,26 @@ def test_render_progress_shows_recent_log_messages():
5252
assert "second checkpoint" in text
5353

5454

55+
def test_render_progress_truncated_phase_agents_are_not_duplicated():
56+
# Regression guard: agents cut by the per-phase max_agents tail must stay
57+
# truncated, not reappear at the bottom as "unphased" rows.
58+
snap = WorkflowSnapshot(name="n", description="d")
59+
for i in range(1, 9):
60+
snap.start_agent(i, f"scan {i}", "Scan")
61+
snap.end_agent(i)
62+
text = render_progress(snap, max_agents=6)
63+
assert "#1 " not in text
64+
assert "#2 " not in text
65+
assert text.count("#8 ") == 1
66+
67+
68+
def test_render_progress_still_shows_phaseless_agents():
69+
snap = WorkflowSnapshot(name="n", description="d")
70+
snap.start_agent(1, "loner", None)
71+
text = render_progress(snap)
72+
assert "loner" in text
73+
74+
5575
def test_render_progress_truncates_to_max_logs():
5676
snap = WorkflowSnapshot(name="n", description="d")
5777
for i in range(5):

tests/tools/test_workflow_engine.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,3 +250,36 @@ async def test_cancellation_marks_running_cancelled_and_reraises():
250250
await task
251251
assert set(started) == {"a", "b"}
252252
assert set(skipped) == {"a", "b"} # in-flight agents reported as cancelled, none completed
253+
254+
255+
@pytest.mark.asyncio
256+
async def test_agent_lifetime_cap_stops_runaway_loop(monkeypatch):
257+
from pythinker_code.tools.workflow import engine
258+
259+
monkeypatch.setattr(engine, "MAX_TOTAL_AGENTS", 3)
260+
runner, calls = make_runner()
261+
script = 'meta = {"name": "n", "description": "d"}\nwhile True:\n await agent("go")\n'
262+
with pytest.raises(WorkflowRuntimeError, match="lifetime cap"):
263+
await run_workflow(script, agent_runner=runner, cwd=".")
264+
assert len(calls) == 3
265+
266+
267+
@pytest.mark.asyncio
268+
async def test_parallel_validation_failure_closes_pending_coroutines():
269+
# Regression guard: mixing a plain function into parallel() raises, but the
270+
# already-created agent() coroutines in the same list must be closed, not
271+
# leaked as "coroutine was never awaited" warnings at GC time.
272+
import gc
273+
import warnings
274+
275+
runner, calls = make_runner()
276+
script = (
277+
'meta = {"name": "n", "description": "d"}\nawait parallel([agent("a"), len])\nreturn None\n'
278+
)
279+
with warnings.catch_warnings(record=True) as caught:
280+
warnings.simplefilter("always")
281+
with pytest.raises(WorkflowRuntimeError, match="not functions"):
282+
await run_workflow(script, agent_runner=runner, cwd=".")
283+
gc.collect()
284+
assert not [w for w in caught if "never awaited" in str(w.message)]
285+
assert calls == []

0 commit comments

Comments
 (0)