From 68d3d85f1a0f0be4517f1d5bb59cf098a3378975 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Fri, 30 Oct 2026 14:15:54 -0700 Subject: [PATCH] fix(computer): a recording of an env that declares none fails as unsupported, not on a missing key _grant read the card's recording params before anything checked the card declared recording, so starting or ending a recording of an env without one (a website browser with no recorder) raised KeyError: 'max_duration_s'. It now raises EnvCapabilityUnsupported, as DeployedEnv.invoke does. Co-Authored-By: Claude Opus 5.5 --- .../task_step/task_steps/computer_recording.py | 13 +++++++++---- tst/unit/task_step/computer_recording_test.py | 17 ++++++++++++++++- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/agent_env/task_step/task_steps/computer_recording.py b/src/agent_env/task_step/task_steps/computer_recording.py index 4841cec61..7044fdda8 100644 --- a/src/agent_env/task_step/task_steps/computer_recording.py +++ b/src/agent_env/task_step/task_steps/computer_recording.py @@ -23,6 +23,7 @@ from agent_env.artifact.store import get_artifact_store from agent_env.config import get_config from agent_env.entity.refs import EntityRef +from agent_env.env.env import EnvCapabilityUnsupported from agent_env.providers.sandbox_providers.sandbox import NetworkPolicy from agent_env.store.base import GrantUnavailableError from agent_env.store.ids import derive_id, is_local_id, validate_local_id @@ -178,7 +179,7 @@ async def end_recording(context: TaskStepContext, name: str, reason: str) -> Non try: deployed, environment_name = _deployed(context, entry["env_id"]), entry.get("environment_name") manifest = RecordingManifest.model_validate(await deployed.invoke(COMPUTER_RECORDING, "end", { - "recording_id": entry["recording_id"], "upload": _grant(deployed, entry["namespace"], environment_name), + "recording_id": entry["recording_id"], "upload": _grant(deployed, entry["namespace"], environment_name, "end"), "reason": reason}, environment_name=environment_name)) except Exception as e: logger.warning("end_computer_recording %s: %s; registering what its namespace holds", name, e) @@ -228,11 +229,15 @@ def _namespace(request_id: str) -> str: return config.get_object_store().object_url(f"{config.get_artifact_key_prefix()}{_KEY_PREFIX}/{key}") -def _grant(deployed, namespace: str, environment_name: Optional[str] = None) -> dict: +def _grant(deployed, namespace: str, environment_name: Optional[str] = None, method: str = "start") -> dict: """A grant that uploads under ``namespace``, lasting the recorded env's ``max_duration_s`` and a margin, or the - longest the store issues.""" + longest the store issues. EnvCapabilityUnsupported when the env's card, or its child's, declares no recording.""" card = deployed.environment_card if environment_name is None else deployed.get_child_env_card(environment_name) - params = (find_extension(card or {}, COMPUTER_RECORDING) or {}).get("params") or {} + extension = find_extension(card or {}, COMPUTER_RECORDING) + if extension is None: + raise EnvCapabilityUnsupported(COMPUTER_RECORDING, method, deployed.env_id, + getattr(deployed, "instance_id", None), environment_name) + params = extension.get("params") or {} store = get_config().get_object_store() sandbox_type = getattr(deployed, "sandbox_type", None) if not issues_grants_to(store, sandbox_type): diff --git a/tst/unit/task_step/computer_recording_test.py b/tst/unit/task_step/computer_recording_test.py index 42e6199c6..6b05e0feb 100644 --- a/tst/unit/task_step/computer_recording_test.py +++ b/tst/unit/task_step/computer_recording_test.py @@ -319,10 +319,25 @@ async def test_a_child_env_is_recorded_through_its_own_card_from_start_to_end(st assert start["upload"]["root_path"] == end["upload"]["root_path"] +@pytest.mark.asyncio +@pytest.mark.parametrize("children", [[{"name": "website-browser", "capabilities": {"extensions": []}}], []]) +async def test_a_child_env_that_declares_no_recording_fails_the_start_saying_so(store, children): + """A browser without a recorder, as each stage's first website browser was: its card has no recording, or there's no + card for it at all.""" + gateway = _Gateway(store) + gateway.environment_card["children_environments"] = children + context = await _start(env_id="shop", environment_name="website-browser").execute(_context(gateway)) + entry = context.metadata[RECORDINGS_KEY]["desk"] + assert entry["state"] == "failed" and gateway.calls == [] + assert entry["error"].startswith( + f"EnvCapabilityUnsupported: env 'shop' child env 'website-browser' does not offer 'start' on {COMPUTER_RECORDING}") + + @pytest.mark.asyncio async def test_a_start_without_a_child_env_reads_the_envs_own_card_as_before(store): context = await _start(env_id="shop").execute(_context(_Gateway(store))) entry = context.metadata[RECORDINGS_KEY]["desk"] - assert entry["state"] == "failed" and "max_duration_s" in entry["error"] # its own card declares no recording + assert entry["state"] == "failed" # its own card declares no recording + assert entry["error"].startswith("EnvCapabilityUnsupported: env 'shop' does not offer 'start' on ") context = await _start().execute(_context(_Computer(store))) assert "environment_name" not in context.metadata[RECORDINGS_KEY]["desk"]