From 9cd5523f147a5fd3630b1a794777f883504b43d3 Mon Sep 17 00:00:00 2001 From: knQzx <75641500+knQzx@users.noreply.github.com> Date: Mon, 20 Jul 2026 08:50:34 +0200 Subject: [PATCH] sandbox: do not flag expected not-found reads as errored spans Let instrumented ops declare expected exceptions via expected_errors so a raised expected error is still recorded on the audit finish event but does not mark the trace span as errored. Apply it to read for WorkspaceReadNotFoundError, which a first-time lazy load_skill probe read triggers by design. --- src/agents/sandbox/session/sandbox_session.py | 37 +++++++++---- .../capabilities/test_skills_capability.py | 52 ++++++++++++++++++- tests/sandbox/test_session_sinks.py | 14 ++--- 3 files changed, 83 insertions(+), 20 deletions(-) diff --git a/src/agents/sandbox/session/sandbox_session.py b/src/agents/sandbox/session/sandbox_session.py index 1d41bc02fb..6d4a4d17c0 100644 --- a/src/agents/sandbox/session/sandbox_session.py +++ b/src/agents/sandbox/session/sandbox_session.py @@ -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 @@ -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.""" @@ -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) @@ -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( @@ -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, @@ -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,), + ) async def read(self, path: Path, *, user: str | User | None = None) -> io.IOBase: return await self._inner.read(path, user=user) diff --git a/tests/sandbox/capabilities/test_skills_capability.py b/tests/sandbox/capabilities/test_skills_capability.py index 2603077777..4316726990 100644 --- a/tests/sandbox/capabilities/test_skills_capability.py +++ b/tests/sandbox/capabilities/test_skills_capability.py @@ -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 @@ -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): @@ -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 diff --git a/tests/sandbox/test_session_sinks.py b/tests/sandbox/test_session_sinks.py index 09efbd83ca..eeb25dd3a3 100644 --- a/tests/sandbox/test_session_sinks.py +++ b/tests/sandbox/test_session_sinks.py @@ -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