mirror of
https://github.com/usestrix/strix.git
synced 2026-08-28 05:25:00 +00:00
fix(shell): validate shell yields against the PTY ceiling
The PTY layer clamps any yield above its maximum, so a larger configured value bought nothing and looked effective. Bound both yield settings by that constant instead, and take the exec default from it.
This commit is contained in:
parent
7ef555ad19
commit
4dcd543db7
2 changed files with 26 additions and 7 deletions
|
|
@ -4,6 +4,7 @@ from __future__ import annotations
|
|||
|
||||
from typing import Literal
|
||||
|
||||
from agents.sandbox.session.pty_types import PTY_YIELD_TIME_MS_MAX
|
||||
from pydantic import AliasChoices, Field
|
||||
from pydantic_settings import BaseSettings, SettingsConfigDict
|
||||
|
||||
|
|
@ -110,19 +111,27 @@ class ShellSettings(BaseSettings):
|
|||
Raising these defaults lets one call return a meaningful result instead of a
|
||||
no-op round-trip. An explicit ``yield_time_ms`` from the model always wins.
|
||||
|
||||
The SDK's PTY layer clamps every yield to 30s, so a larger value here would
|
||||
be silently ineffective; keep both yields at or below that ceiling.
|
||||
The SDK's PTY layer clamps every yield to ``PTY_YIELD_TIME_MS_MAX``, so both
|
||||
yields are validated against that ceiling instead of being silently reduced.
|
||||
"""
|
||||
|
||||
model_config = _BASE_CONFIG
|
||||
|
||||
# Default yield for exec_command when the model omits yield_time_ms. 30s is
|
||||
# the most the PTY layer honours, so this sits right at that ceiling.
|
||||
exec_yield_ms: int = Field(default=30_000, gt=0, alias="STRIX_SHELL_EXEC_YIELD_MS")
|
||||
# Default yield for exec_command when the model omits yield_time_ms. The
|
||||
# default sits at the ceiling: waiting is cheaper than another poll turn.
|
||||
exec_yield_ms: int = Field(
|
||||
default=PTY_YIELD_TIME_MS_MAX,
|
||||
gt=0,
|
||||
le=PTY_YIELD_TIME_MS_MAX,
|
||||
alias="STRIX_SHELL_EXEC_YIELD_MS",
|
||||
)
|
||||
# Default yield for an empty (polling) write_stdin call. The SDK already
|
||||
# floors an empty poll at 5s; this trades a little latency for far fewer turns.
|
||||
write_stdin_poll_yield_ms: int = Field(
|
||||
default=20_000, gt=0, alias="STRIX_SHELL_WRITE_STDIN_POLL_YIELD_MS"
|
||||
default=20_000,
|
||||
gt=0,
|
||||
le=PTY_YIELD_TIME_MS_MAX,
|
||||
alias="STRIX_SHELL_WRITE_STDIN_POLL_YIELD_MS",
|
||||
)
|
||||
# Cap on a bare `sleep N` hand-wait (seconds); larger sleeps are clamped.
|
||||
max_sleep_seconds: int = Field(default=60, gt=0, alias="STRIX_SHELL_MAX_SLEEP_SECONDS")
|
||||
|
|
|
|||
|
|
@ -8,11 +8,12 @@ from typing import Any, cast
|
|||
|
||||
import pytest
|
||||
from agents.sandbox.errors import InvalidManifestPathError
|
||||
from agents.sandbox.session.pty_types import PTY_YIELD_TIME_MS_MAX
|
||||
from agents.tool import CustomTool, FunctionTool
|
||||
from pydantic import BaseModel, ValidationError
|
||||
|
||||
from strix.agents import factory
|
||||
from strix.config import load_settings
|
||||
from strix.config import ShellSettings, load_settings
|
||||
|
||||
|
||||
def _capturing_exec_tool(captured: dict[str, str]) -> FunctionTool:
|
||||
|
|
@ -363,3 +364,12 @@ async def test_invalid_workdir_is_rendered_as_a_message() -> None:
|
|||
assert isinstance(result, str)
|
||||
assert "workdir must be a path inside /workspace" in result
|
||||
assert "'../etc'" in result
|
||||
|
||||
|
||||
@pytest.mark.parametrize("field", ["exec_yield_ms", "write_stdin_poll_yield_ms"])
|
||||
def test_shell_settings_reject_a_yield_above_the_pty_ceiling(field: str) -> None:
|
||||
"""A yield the PTY layer would clamp is a misconfiguration, not a longer wait."""
|
||||
with pytest.raises(ValidationError):
|
||||
ShellSettings(**{field: PTY_YIELD_TIME_MS_MAX + 1})
|
||||
|
||||
assert getattr(ShellSettings(**{field: PTY_YIELD_TIME_MS_MAX}), field) == PTY_YIELD_TIME_MS_MAX
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue