Skip to content

Commit d0a1c2e

Browse files
committed
Address greptile: give the debug spawns the same stream limit
The retry bound counts every consecutive ValueError, including limit overruns, which do make progress. That is harmless where the limit is 8 MiB, but the debug spawns were left on asyncio's 64 KiB default, so an agent emitting enough consecutive large lines under --debug exhausts the bound, the reader re-raises, and the child deadlocks on a full pipe. The two changes were individually defensible and together reintroduced the bug being fixed. Move SUBPROCESS_STREAM_LIMIT to cli/utils/cli_utils.py, which imports only typer and rich, so both handlers can share it. It could not live in either handler: run_handlers imports cli.debug, so the reverse import would cycle. Extends the wiring test to all four spawn sites rather than two, so a new one cannot be added on the default limit without failing.
1 parent a4cd872 commit d0a1c2e

4 files changed

Lines changed: 36 additions & 8 deletions

File tree

src/agentex/lib/cli/debug/debug_handlers.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
pass
1717

1818
from agentex.lib.utils.logging import make_logger
19+
from agentex.lib.cli.utils.cli_utils import SUBPROCESS_STREAM_LIMIT
1920

2021
from .debug_config import DebugConfig, resolve_debug_port
2122

@@ -66,6 +67,7 @@ async def start_temporal_worker_debug(
6667
env=debug_env,
6768
stdout=asyncio.subprocess.PIPE,
6869
stderr=asyncio.subprocess.STDOUT,
70+
limit=SUBPROCESS_STREAM_LIMIT,
6971
)
7072

7173

@@ -119,6 +121,7 @@ async def start_acp_server_debug(
119121
env=debug_env,
120122
stdout=asyncio.subprocess.PIPE,
121123
stderr=asyncio.subprocess.STDOUT,
124+
limit=SUBPROCESS_STREAM_LIMIT,
122125
)
123126

124127

src/agentex/lib/cli/handlers/run_handlers.py

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
from agentex.lib.cli.debug import DebugConfig, start_acp_server_debug, start_temporal_worker_debug
1313
from agentex.lib.utils.logging import make_logger
1414
from agentex.config.agent_manifest import AgentManifest
15+
from agentex.lib.cli.utils.cli_utils import SUBPROCESS_STREAM_LIMIT
1516
from agentex.lib.cli.utils.path_utils import (
1617
get_file_paths,
1718
calculate_uvicorn_target_for_local,
@@ -23,11 +24,6 @@
2324
logger = make_logger(__name__)
2425
console = Console()
2526

26-
# asyncio's StreamReader defaults to 64 KiB, and a single log line above that makes
27-
# readline() raise. Agents legitimately emit large lines (serialized charts, payloads
28-
# echoed by validation errors), so give the reader room before it has to drop one.
29-
SUBPROCESS_STREAM_LIMIT = 8 * 1024 * 1024
30-
3127
# How many consecutive unreadable lines to skip before giving up on the stream.
3228
# Skipping is only known-safe for the limit-overrun case; this bounds the damage
3329
# if some other error repeats without consuming anything.

src/agentex/lib/cli/utils/cli_utils.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,18 @@
55

66
console = Console()
77

8+
# asyncio's StreamReader defaults to 64 KiB, and a single log line above that makes
9+
# readline() raise. Agents legitimately emit large lines (serialized charts, payloads
10+
# echoed back by validation errors), so give the reader room before it has to drop one.
11+
#
12+
# Lives here rather than beside its users so that both the normal spawns in
13+
# cli/handlers/run_handlers.py and the debug spawns in cli/debug/debug_handlers.py can
14+
# import it: run_handlers imports cli.debug, so the constant cannot live in either one.
15+
# Keep the two in step. A subprocess left on the asyncio default overruns far more
16+
# easily, and enough consecutive overruns exhaust the reader's retry bound and stop it
17+
# draining, which is the deadlock the bound is there to avoid.
18+
SUBPROCESS_STREAM_LIMIT = 8 * 1024 * 1024
19+
820

921
def handle_questionary_cancellation(
1022
result: str | None, operation: str = "operation"

tests/lib/cli/test_run_handlers_streaming.py

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,12 @@
1414

1515
import pytest
1616

17+
from agentex.lib.cli.debug import DebugMode, DebugConfig
1718
from agentex.lib.cli.handlers import run_handlers
19+
from agentex.lib.cli.debug.debug_handlers import (
20+
start_acp_server_debug,
21+
start_temporal_worker_debug,
22+
)
1823
from agentex.lib.cli.handlers.run_handlers import (
1924
SUBPROCESS_STREAM_LIMIT,
2025
start_acp_server,
@@ -144,10 +149,15 @@ async def readline(self) -> bytes:
144149
await task
145150

146151

147-
async def test_agent_subprocesses_are_spawned_with_the_larger_limit(
152+
async def test_every_spawn_uses_the_larger_limit(
148153
monkeypatch: pytest.MonkeyPatch, tmp_path: Any
149154
) -> None:
150-
"""The helpers must pass limit=, or large lines are dropped in production."""
155+
"""Every spawn must pass limit=, including the debug ones.
156+
157+
A subprocess left on asyncio's default overruns far more easily, and enough
158+
consecutive overruns exhaust MAX_CONSECUTIVE_READ_ERRORS and stop the reader
159+
draining, which is the deadlock the bound exists to avoid.
160+
"""
151161
seen: list[int | None] = []
152162

153163
async def fake_exec(*_args: Any, **kwargs: Any) -> None:
@@ -159,5 +169,12 @@ async def fake_exec(*_args: Any, **kwargs: Any) -> None:
159169
await start_acp_server(tmp_path / "acp.py", 8000, {}, tmp_path)
160170
await start_temporal_worker(tmp_path / "run_worker.py", {}, tmp_path)
161171

162-
assert seen == [SUBPROCESS_STREAM_LIMIT, SUBPROCESS_STREAM_LIMIT]
172+
# BOTH, since each helper refuses unless its own mode is enabled.
173+
debug_config = DebugConfig(
174+
enabled=True, mode=DebugMode.BOTH, port=5678, wait_for_attach=False, auto_port=False
175+
)
176+
await start_acp_server_debug(tmp_path / "acp.py", 8000, {}, debug_config)
177+
await start_temporal_worker_debug(tmp_path / "run_worker.py", {}, debug_config)
178+
179+
assert seen == [SUBPROCESS_STREAM_LIMIT] * 4, f"a spawn is missing limit=: {seen}"
163180
assert SUBPROCESS_STREAM_LIMIT > 64 * 1024, "asyncio's default is what breaks readline()"

0 commit comments

Comments
 (0)