Skip to content

refactor(sandbox)!: remove the sandbox priority knob - #50

Merged
polakamtejas merged 2 commits into
mainfrom
tejas/remove-priority
Oct 5, 2026
Merged

polakamtejas merged 2 commits into
mainfrom
tejas/remove-priority

Conversation

@polakamtejas

@polakamtejas polakamtejas commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

priority was a sandbox scheduling hint ("0=interactive, 1=non_interactive"). It was threaded from the CLI, the deploy steps, the rubrics judge, run overrides and the explorer down to every SandboxProvider create method, but no provider in this repo reads it:

  • local and modal ignored it.
  • modal_vm and e2b documented it as inert (e2b did del priority).

Its only consumer is a hosted backend's scheduling tiers, which aren't part of this repo. This removes it.

What's removed

  • Sandbox providers: priority is gone from SandboxProvider.create_sandbox / create_vm / create_container and from the four built-ins.
  • Envs and env providers: gone from MCPServerEnv / WebsiteEnv / MultiEnv.deploy, the gateway and server env providers, so env-provider plugins no longer receive it in their options.
  • Steps: deploy_env, deploy_agent and deploy_sandbox lose priority, the rubrics verifier loses judge_priority, and none of them read user_overrides["priority"]. A stored step document that still carries either key loads, and the key isn't written back (new tst/unit/task_step/stored_priority_test.py).
  • CLI: --priority on env deploy and a2a-agent deploy.
  • A2A agents: A2AAgent.deploy(priority=).
  • Explorer: RunRequest.priority, and the UI no longer sends priority: 0 on every run.

Breaking for downstream

  • Plugin API: check_plugin_api.py reports the 3 removed SandboxProvider parameters, which the ! marks. A provider that still declares priority keeps working; core just stops passing it.
  • CLI: --priority now fails with "No such option".
  • Explorer API: a run request with priority in the body returns 422 (RunRequest forbids extra keys).
  • Python callers: passing priority= to an env's or agent's deploy(), a step constructor, or a sandbox provider raises TypeError.

Testing

  • pytest tst/unit packages/agentenv-protocol/tests -n auto: 5802 passed. The only failures are the 2 test_mcp_cli_builder tests, because the generated CLI's interpreter has no click (same as on refactor(attribution)!: remove project_id and the LiteLLM cost attribution built on it #45).
  • UI: npx tsc --noEmit, npm run test:smoke (13 ran, the 2 skips need trajectory fixtures) and npm run build:static pass.
  • Live: agent-env env deploy --priority 1 and a2a-agent deploy --priority 1 give "No such option"; agent-env run hello passes; RunRequest(priority=1) is rejected as extra_forbidden.
  • git grep -i priority now finds only the new test and unrelated uses: Next.js's <Image priority> prop, "priority order" wording in comments, the protocol package's enum test, mock-service seed data and an email fixture.

🤖 Generated with Claude Code

RetriggerConfidence Score: 5/5

The PR appears safe to merge; the remaining smoke-test gap is non-blocking.

Fix All in CursorFindings

  1. P2 Run test misses rejected field ▶
Fix with agent prompt
### Issue 1
src/agent_env/explorer/ui/src/lib/task-runner-run.smoke.ts:undefined-62
The smoke test checks the version but no longer checks that `buildRunBody` leaves out `priority`. If a later change adds it back, this test will pass while the explorer API rejects the run with 422. Please assert that the key is absent.

```suggestion
  assert(body.version === 3, 'run body: task version');
  assert(!('priority' in body), 'run body: priority is not sent');
```

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

The PR removes the unused sandbox priority setting from deployment and run paths because the built-in providers did not use it to schedule work.

  • Sandbox and environment deploy APIs, task steps, CLI commands, and A2A agent deployment no longer carry priority.
  • Explorer no longer accepts or sends priority when starting runs; older stored step documents still load without keeping the removed key.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart LR
  CLI[CLI and Explorer] --> Steps[Deploy steps]
  Steps --> Envs[Envs and agents]
  Envs --> Providers[Sandbox providers]
  Stored[Older stored steps] --> Steps
Loading

Reviews (2) · Last reviewed commit: "Merge remote-tracking branch 'origin/mai..."

priority was threaded from the CLI, the deploy steps, the rubrics judge, run overrides and the
explorer down to every SandboxProvider create method, but no provider in this repo reads it
(local and modal ignored it, modal_vm and e2b documented it as inert). Its only consumer, a
hosted backend's scheduling tiers, isn't part of this repo. Stored step documents that still
carry priority or judge_priority load and drop the key.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@polakamtejas
polakamtejas requested a review from a team as a code owner October 2, 2026 16:57
Comment thread src/agent_env/explorer/ui/src/lib/task-runner-run.smoke.ts
@earakely-scale

Copy link
Copy Markdown
Collaborator

🤖 Automated Claude routine — Env Pod PR Context Review. This is a bot, not Edgar Arakelyan; not a human review.

One piece of context on the blast radius, offered because it sits just outside the diff: the "Breaking for downstream" list covers the plugin API and stored step documents, but a caller that constructs a step with priority= isn't an ignored key — it's a TypeError.

  • In this PR, tst/unit/task_step/stored_priority_test.py pins the tolerant path only: from_dict({**document, "priority": 1}). That is genuinely tolerant, because from_dict reads named keys.
  • The constructors are not. After this PR DeployEnvTaskStep.__init__ (src/agent_env/task_step/task_steps/deploy_env.py:23), DeployAgentTaskStep.__init__ (deploy_agent.py:97), DeploySandboxTaskStep.__init__ (deploy_sandbox.py:22) and RubricsVerifierTaskStep.__init__ take no priority / judge_priority and no **kwargs (checked by AST over this head, 358e52f).
  • The second half is on main, outside the diff: TaskStep.put (src/agent_env/task_step/task_step.py:162-173) pops only retry_config and then calls cls(**kwargs) unfiltered.

So DeployEnvTaskStep.put(id=…, env_id=…, priority=0) goes from accepted to TypeError: __init__() got an unexpected keyword argument 'priority', as does DeployAgentTaskStep.put(…, priority=…) and the direct constructor. An out-of-repo composer that builds task steps in code upgrades into a hard failure, while the stored-document path this PR tests stays fine — worth a line in the breaking notes even if the intent is that those callers drop the argument.

For precedent on tolerating it instead: deploy_env.py:156 already probes inspect.Parameter.VAR_KEYWORD before forwarding litellm_api_key, so the repo has a shape for "accept and ignore" when the other end may be older.


Generated by Claude Code

@polakamtejas
polakamtejas enabled auto-merge (squash) October 3, 2026 14:15
# Conflicts:
#	src/agent_env/explorer/routers/runs.py
#	src/agent_env/explorer/ui/src/lib/task-runner-run.smoke.ts
#	src/agent_env/providers/sandbox_providers/modal_sandbox.py
#	tst/unit/env/envs/test_mcp_server_container_mode.py
#	tst/unit/env/envs/test_mcp_server_plugin_provider.py
#	tst/unit/providers/env_providers/env_server_provider_test.py
@polakamtejas
polakamtejas merged commit d0140ed into main Oct 5, 2026
14 checks passed
@polakamtejas
polakamtejas deleted the tejas/remove-priority branch October 5, 2026 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants