Clean up SDK shell tool failure modes
Three concrete wraps on exec_command / write_stdin via the existing
Shell capability configure_tools mechanism, plus one skill-doc fix.
All wraps fire on both Responses and chat-completions paths; the
chat-completions error-as-result wrap still stacks on top when needed.
- write_stdin: decode the common escape forms in `chars` (\uXXXX,
\xXX, \n \t \r \0 \a \b \v \f \\). Models routinely send the
literal six-char string `` intending the ASCII control byte;
the SDK takes chars verbatim so the byte never reaches the PTY and
documented mechanisms like Ctrl-C, arrows, and Escape silently
don't work. Allowlist regex over recognized escapes only —
unrecognized sequences like `\p` pass through untouched.
- exec_command: catch InvalidManifestPathError and rewrite to a
model-actionable message ("workdir must be a path inside
/workspace") using the exception's structured `context["rel"]` so
we don't need to string-match the SDK's wording.
- Both tools: catch pydantic ValidationError once at the wrap and
reformat into a short "{tool}: invalid arguments — {field}: {msg}"
string. Covers empty cmd, missing required fields, ge/min_length
violations on max_output_tokens and yield_time_ms — and any future
schema field the SDK adds.
Updated python.md guidance: the `shell=` parameter is for swapping
POSIX shells (bash/zsh/sh). Interpreters belong in `cmd` —
`cmd="python3 -c '...'"`, not `shell=python3`. The `shell=interpreter`
shortcut breaks in interpreter-specific ways (python needs `-c`,
node/ruby/perl need `-e`) so there's no clean code fix and we don't
try one.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
+101
-5
@@ -20,12 +20,15 @@ from __future__ import annotations
|
|||||||
import inspect
|
import inspect
|
||||||
import json
|
import json
|
||||||
import logging
|
import logging
|
||||||
|
import re
|
||||||
from typing import TYPE_CHECKING, Any
|
from typing import TYPE_CHECKING, Any
|
||||||
|
|
||||||
from agents.agent import ToolsToFinalOutputResult
|
from agents.agent import ToolsToFinalOutputResult
|
||||||
from agents.sandbox import SandboxAgent
|
from agents.sandbox import SandboxAgent
|
||||||
from agents.sandbox.capabilities import Filesystem, Shell
|
from agents.sandbox.capabilities import Filesystem, Shell
|
||||||
|
from agents.sandbox.errors import InvalidManifestPathError
|
||||||
from agents.tool import CustomTool, FunctionTool, Tool
|
from agents.tool import CustomTool, FunctionTool, Tool
|
||||||
|
from pydantic import ValidationError
|
||||||
|
|
||||||
from strix.agents.prompt import render_system_prompt
|
from strix.agents.prompt import render_system_prompt
|
||||||
from strix.tools.agents_graph.tools import (
|
from strix.tools.agents_graph.tools import (
|
||||||
@@ -180,10 +183,103 @@ def _configure_chat_completions_filesystem_tools(toolset: Any) -> None:
|
|||||||
setattr(toolset, name, _function_tool_with_error_result(tool))
|
setattr(toolset, name, _function_tool_with_error_result(tool))
|
||||||
|
|
||||||
|
|
||||||
def _configure_chat_completions_shell_tools(toolset: Any) -> None:
|
_CHARS_ESCAPE_RE = re.compile(r"\\(?:u[0-9a-fA-F]{4}|x[0-9a-fA-F]{2}|[0abtnvfr\\])")
|
||||||
|
_CHARS_ESCAPE_MAP = {
|
||||||
|
"\\\\": "\\",
|
||||||
|
"\\n": "\n",
|
||||||
|
"\\t": "\t",
|
||||||
|
"\\r": "\r",
|
||||||
|
"\\0": "\x00",
|
||||||
|
"\\a": "\x07",
|
||||||
|
"\\b": "\x08",
|
||||||
|
"\\v": "\x0b",
|
||||||
|
"\\f": "\x0c",
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def _decode_chars_escape(s: str) -> str:
|
||||||
|
if "\\" not in s:
|
||||||
|
return s
|
||||||
|
|
||||||
|
def sub(match: re.Match[str]) -> str:
|
||||||
|
token = match.group(0)
|
||||||
|
if token in _CHARS_ESCAPE_MAP:
|
||||||
|
return _CHARS_ESCAPE_MAP[token]
|
||||||
|
if token.startswith(("\\u", "\\x")):
|
||||||
|
return chr(int(token[2:], 16))
|
||||||
|
return token
|
||||||
|
|
||||||
|
return _CHARS_ESCAPE_RE.sub(sub, s)
|
||||||
|
|
||||||
|
|
||||||
|
def _format_validation_error(tool_name: str, exc: ValidationError) -> str:
|
||||||
|
parts: list[str] = []
|
||||||
|
for err in exc.errors():
|
||||||
|
loc = ".".join(str(x) for x in err.get("loc", ()))
|
||||||
|
msg = err.get("msg", "invalid")
|
||||||
|
parts.append(f"{loc}: {msg}" if loc else msg)
|
||||||
|
return f"{tool_name}: invalid arguments — " + "; ".join(parts)
|
||||||
|
|
||||||
|
|
||||||
|
def _wrap_exec_command(tool: FunctionTool) -> FunctionTool:
|
||||||
|
invoke_tool = tool.on_invoke_tool
|
||||||
|
|
||||||
|
async def invoke(ctx: Any, raw_input: str) -> Any:
|
||||||
|
try:
|
||||||
|
return await invoke_tool(ctx, raw_input)
|
||||||
|
except ValidationError as exc:
|
||||||
|
return _format_validation_error(tool.name, exc)
|
||||||
|
except InvalidManifestPathError as exc:
|
||||||
|
rel = exc.context.get("rel", "?")
|
||||||
|
return (
|
||||||
|
"exec_command: workdir must be a path inside /workspace "
|
||||||
|
"(or omitted to use the turn's cwd). "
|
||||||
|
f"Got: {rel!r}."
|
||||||
|
)
|
||||||
|
|
||||||
|
tool.on_invoke_tool = invoke
|
||||||
|
return tool
|
||||||
|
|
||||||
|
|
||||||
|
def _wrap_write_stdin(tool: FunctionTool) -> FunctionTool:
|
||||||
|
invoke_tool = tool.on_invoke_tool
|
||||||
|
|
||||||
|
async def invoke(ctx: Any, raw_input: str) -> Any:
|
||||||
|
try:
|
||||||
|
parsed = json.loads(raw_input)
|
||||||
|
except json.JSONDecodeError:
|
||||||
|
parsed = None
|
||||||
|
if isinstance(parsed, dict) and isinstance(parsed.get("chars"), str):
|
||||||
|
parsed["chars"] = _decode_chars_escape(parsed["chars"])
|
||||||
|
raw_input = json.dumps(parsed)
|
||||||
|
try:
|
||||||
|
return await invoke_tool(ctx, raw_input)
|
||||||
|
except ValidationError as exc:
|
||||||
|
return _format_validation_error(tool.name, exc)
|
||||||
|
|
||||||
|
tool.on_invoke_tool = invoke
|
||||||
|
return tool
|
||||||
|
|
||||||
|
|
||||||
|
def _configure_shell_tools(toolset: Any, *, chat_completions: bool) -> None:
|
||||||
for name, tool in vars(toolset).items():
|
for name, tool in vars(toolset).items():
|
||||||
if isinstance(tool, FunctionTool):
|
if not isinstance(tool, FunctionTool):
|
||||||
setattr(toolset, name, _function_tool_with_error_result(tool))
|
continue
|
||||||
|
wrapped = tool
|
||||||
|
if tool.name == "exec_command":
|
||||||
|
wrapped = _wrap_exec_command(wrapped)
|
||||||
|
elif tool.name == "write_stdin":
|
||||||
|
wrapped = _wrap_write_stdin(wrapped)
|
||||||
|
if chat_completions:
|
||||||
|
wrapped = _function_tool_with_error_result(wrapped)
|
||||||
|
setattr(toolset, name, wrapped)
|
||||||
|
|
||||||
|
|
||||||
|
def _make_shell_configurator(*, chat_completions: bool) -> Any:
|
||||||
|
def configure(toolset: Any) -> None:
|
||||||
|
_configure_shell_tools(toolset, chat_completions=chat_completions)
|
||||||
|
|
||||||
|
return configure
|
||||||
|
|
||||||
|
|
||||||
def _lifecycle_tool_completed(tool_name: str, output: Any) -> bool:
|
def _lifecycle_tool_completed(tool_name: str, output: Any) -> bool:
|
||||||
@@ -366,8 +462,8 @@ def build_strix_agent(
|
|||||||
),
|
),
|
||||||
),
|
),
|
||||||
Shell(
|
Shell(
|
||||||
configure_tools=(
|
configure_tools=_make_shell_configurator(
|
||||||
_configure_chat_completions_shell_tools if chat_completions_tools else None
|
chat_completions=chat_completions_tools,
|
||||||
),
|
),
|
||||||
),
|
),
|
||||||
],
|
],
|
||||||
|
|||||||
@@ -11,6 +11,14 @@ Prefer writing reusable scripts to `/workspace/scratch/<name>.py` and
|
|||||||
running them with `python3 /workspace/scratch/<name>.py`. For short
|
running them with `python3 /workspace/scratch/<name>.py`. For short
|
||||||
one-off transformations, `python3 -c` or a small here-document is fine.
|
one-off transformations, `python3 -c` or a small here-document is fine.
|
||||||
|
|
||||||
|
The `shell` parameter on `exec_command` is for swapping POSIX shells
|
||||||
|
(`bash`/`zsh`/`sh`), not for picking interpreters. Put the interpreter
|
||||||
|
invocation in `cmd` instead: `cmd="python3 -c '...'"`, not
|
||||||
|
`shell=python3, cmd="..."`. The `shell=<interpreter>` shortcut breaks
|
||||||
|
in subtle ways — `python3` works only with `login=False` (because the
|
||||||
|
SDK adds `-l`/`-i`), and other interpreters (`node`, `ruby`, `perl`)
|
||||||
|
take `-e` not `-c` so they fail even with `login=False`.
|
||||||
|
|
||||||
## Proxy Automation From Python
|
## Proxy Automation From Python
|
||||||
|
|
||||||
The sandbox image includes an installed `caido_api` module. Import it
|
The sandbox image includes an installed `caido_api` module. Import it
|
||||||
|
|||||||
Reference in New Issue
Block a user