Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 28 additions & 9 deletions src/agents/sandbox/session/sandbox_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

from ...run_config import SandboxArchiveLimits, SandboxConcurrencyLimits
from ...tracing import Span, custom_span, get_current_trace
from ..errors import OpName, SandboxError
from ..errors import OpName, SandboxError, WorkspaceReadNotFoundError
from ..files import FileEntry
from ..types import ExecResult, ExposedPortEndpoint, User
from .base_sandbox_session import BaseSandboxSession
Expand All @@ -39,6 +39,7 @@ def instrumented_op(
) = None,
ok: Callable[[object], bool] | None = None,
outputs: Callable[[object], tuple[bytes | None, bytes | None]] | None = None,
expected_errors: tuple[type[BaseException], ...] = (),
) -> Callable[[F], F]:
"""Decorator to emit SandboxSessionEvents around a SandboxSession operation."""

Expand All @@ -64,6 +65,7 @@ def _finish_cb(res: object) -> dict[str, object]:
finish_data=finish_cb,
ok=ok,
outputs=outputs,
expected_errors=expected_errors,
)

return cast(F, _wrapped)
Expand Down Expand Up @@ -378,6 +380,7 @@ async def _annotate(
finish_data: Callable[[T], dict[str, object]] | None = None,
ok: Callable[[T], bool] | None = None,
outputs: Callable[[T], tuple[bytes | None, bytes | None]] | None = None,
expected_errors: tuple[type[BaseException], ...] = (),
) -> T:
span_cm = (
custom_span(
Expand All @@ -403,13 +406,25 @@ async def _annotate(
value = await run()
except Exception as e:
duration_ms = (time.monotonic() - t0) * 1000.0
self._apply_trace_finish_data(
span=trace_span,
op=op,
ok=False,
data=start_data,
exc=e,
)
# Expected errors are reported through the audit finish event but must not
# flag the trace span as errored (for example a probe read of a not-yet
# materialized file).
if isinstance(e, expected_errors):
self._apply_trace_finish_data(
span=trace_span,
op=op,
ok=True,
data=start_data,
exc=None,
)
else:
self._apply_trace_finish_data(
span=trace_span,
op=op,
ok=False,
data=start_data,
exc=e,
)
await self._emit_finish_event(
op=op,
span_id=span_id,
Expand Down Expand Up @@ -599,7 +614,11 @@ async def mkdir(
) -> None:
await self._inner.mkdir(path, parents=parents, user=user)

@instrumented_op("read", data=_read_start_data)
@instrumented_op(
"read",
data=_read_start_data,
expected_errors=(WorkspaceReadNotFoundError,),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep direct read failures visible in traces

By putting WorkspaceReadNotFoundError on the read decorator, every failed session.read() that raises this error is treated as a non-error span, not just the lazy-skill existence probe. In a caller trace around await session.read(Path("missing.txt")) (and Docker also maps any non-zero cat read, such as permission denied, to this error), the operation still raises and the audit finish event has ok=False, but the trace contains no error, so direct sandbox read failures disappear from trace-based debugging. Make the suppression a per-probe/per-call opt-in instead of global to read.

Useful? React with 👍 / 👎.

)
async def read(self, path: Path, *, user: str | User | None = None) -> io.IOBase:
return await self._inner.read(path, user=user)

Expand Down
52 changes: 51 additions & 1 deletion tests/sandbox/capabilities/test_skills_capability.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,17 @@
from agents.sandbox import Manifest, SandboxPathGrant
from agents.sandbox.capabilities import LocalDirLazySkillSource, Skill, Skills
from agents.sandbox.entries import Dir, File, LocalDir
from agents.sandbox.errors import SkillsConfigError
from agents.sandbox.errors import SkillsConfigError, WorkspaceReadNotFoundError
from agents.sandbox.files import EntryKind, FileEntry
from agents.sandbox.session.base_sandbox_session import BaseSandboxSession
from agents.sandbox.session.sandbox_session import SandboxSession
from agents.sandbox.snapshot import NoopSnapshot
from agents.sandbox.types import ExecResult, Permissions, User
from agents.sandbox.workspace_paths import coerce_posix_path
from agents.tool import FunctionTool
from agents.tool_context import ToolContext
from agents.tracing import trace
from tests.testing_processor import fetch_ordered_spans
from tests.utils.factories import TestSessionState


Expand Down Expand Up @@ -127,6 +130,16 @@ async def ls(
return entries


class _NotFoundReadSkillsSession(_SkillsSession):
async def read(self, path: Path, *, user: object = None) -> io.BytesIO:
self.read_users.append(_user_name(user))
normalized = self.normalize_path(path)
try:
return io.BytesIO(normalized.read_bytes())
except FileNotFoundError as e:
raise WorkspaceReadNotFoundError(path=path, cause=e) from e


class TestSkillValidation:
def test_rejects_directory_content_artifact(self) -> None:
with pytest.raises(SkillsConfigError):
Expand Down Expand Up @@ -686,3 +699,40 @@ async def test_lazy_metadata_cache_is_reset_on_bind(self, tmp_path: Path) -> Non
"- cached-skill: old description (file: .agents/dynamic-skill)" in second_instructions
)
assert "- cached-skill: new description (file: .agents/dynamic-skill)" in third_instructions

@pytest.mark.asyncio
async def test_first_time_load_skill_does_not_flag_read_span_as_errored(
self, tmp_path: Path
) -> None:
workspace_root = tmp_path / "workspace"
workspace_root.mkdir()
src_root = tmp_path / "skills"
skill_dir = src_root / "dynamic-skill"
skill_dir.mkdir(parents=True)
(skill_dir / "SKILL.md").write_text("# dynamic skill\n", encoding="utf-8")

capability = Skills(lazy_from=LocalDirLazySkillSource(source=LocalDir(src=src_root)))
inner = _NotFoundReadSkillsSession(
_source_granted_manifest(workspace_root, source=src_root)
)
session = SandboxSession(inner)
capability.bind(session)

with trace("skills_lazy_load_test"):
output = await capability.load_skill("dynamic-skill")

assert output == {
"status": "loaded",
"skill_name": "dynamic-skill",
"path": ".agents/dynamic-skill",
}
loaded_skill = workspace_root / ".agents" / "dynamic-skill" / "SKILL.md"
assert loaded_skill.read_text(encoding="utf-8") == "# dynamic skill\n"

read_spans = [
span
for span in fetch_ordered_spans()
if span.span_data.export().get("name") == "sandbox.read"
]
assert len(read_spans) == 1
assert read_spans[0].error is None
14 changes: 4 additions & 10 deletions tests/sandbox/test_session_sinks.py
Original file line number Diff line number Diff line change
Expand Up @@ -435,24 +435,18 @@ async def test_sandbox_session_error_events_and_traces_include_retryability(
assert isinstance(read_finish, SandboxSessionFinishEvent)
assert read_finish.error_retryable is False

# A not-found read is an expected error, so the audit event still records retryability
# while the trace span is not flagged as errored.
spans = fetch_normalized_spans()
read_span = next(
child for child in spans[0]["children"] if child["data"]["name"] == "sandbox.read"
)
span_data = read_span["data"]
assert isinstance(span_data, dict)
span_payload = span_data["data"]
assert isinstance(span_payload, dict)
assert span_payload["error_retryable"] is False
assert "error" not in read_span

raw_read_span = next(
span for span in fetch_ordered_spans() if span.span_data.export()["name"] == "sandbox.read"
)
span_error = raw_read_span.error
assert span_error is not None
error_payload = span_error["data"]
assert isinstance(error_payload, dict)
assert error_payload["error_retryable"] is False
assert raw_read_span.error is None


@pytest.mark.asyncio
Expand Down