Skip to content

feat: add shell tool wire types - #264

Open
bojiang3 wants to merge 4 commits into
vllm-project:mainfrom
bojiang3:feat/shell-wire-types
Open

feat: add shell tool wire types#264
bojiang3 wants to merge 4 commits into
vllm-project:mainfrom
bojiang3:feat/shell-wire-types

Conversation

@bojiang3

@bojiang3 bojiang3 commented Sep 7, 2026

Copy link
Copy Markdown

Summary

  • add typed support for the current OpenAI shell tool declaration and local environment shape
  • add typed shell_call and shell_call_output items, including command limits, stdout/stderr, and exit/timeout outcomes
  • preserve shell calls through request parsing, persistence, rehydration, and call/output pairing
  • implement a client-owned ShellHandler that validates environment.type: "local" and normalizes shell declarations to vLLM-compatible function tools
  • restore normalized blocking and streaming function calls to the public shell_call shape without ever executing commands in the gateway
  • reject unsupported shell environments and malformed model-generated actions instead of treating them as permission to execute
  • add a recorder for OpenAI-reference and gateway shell cassettes; recorded fixtures still require provider credentials/endpoints and are not hand-authored

Part of #170.

Safety boundary

This slice is client-owned only. ShellHandler intentionally does not implement GatewayExecutor, so declaring a shell tool cannot cause the gateway host to execute arbitrary commands. A separately configured, policy-enforced executor remains follow-up work.

Test Plan

  • cargo fmt --all -- --check
  • cargo test -p agentic-server-core (448 passed, 8 environment-dependent tests ignored)
  • cargo clippy -p agentic-server-core --all-targets -- -D warnings
  • focused shell tests cover local declaration normalization, unsupported environments, public blocking restoration, streaming lifecycle restoration, malformed-action failure, storage/rehydration, and call/output pairing

Cassette status

record_shell_cassettes.sh records matching streaming and non-streaming suites against OpenAI and the gateway using the repository's standard recorder. This environment has no OPENAI_API_KEY, and the configured gateway endpoint was unavailable, so this update deliberately does not include fabricated cassette YAML. The recorder is ready to run where those endpoints are available.

vec![]
}
Self::Shell(_) => {
tracing::debug!("shell tool skipped in normalize - handler not yet registered");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@bojiang3 shell tool type as below taken from openai examples in https://developers.openai.com/api/docs/guides/tools-shell
"tools": [ { "type": "shell", "environment": { "type": "container_auto", "network_policy": { "type": "allowlist", "allowed_domains": ["pypi.org", "files.pythonhosted.org", "github.com"] } } }

is not recognized by vLLM upstream. even if some of the types might be recognized by upstream still to keep agentic-api consistent and compatible with multiple upstreams for inference we always normalize/canonicalize any tool types into function types. so all tool support on agentic-api either client owned like shell or owned on gateway by agentic-api need to implement a ToolHandler interface so that we can normalize and validate the tools's schema for inference. kindly follow ARCHITECTURE.md and TERMINOLOGY.md.

there should be also request and responses cassettes recording for this tool type from openai as ground truth and agentic-api (gateway) to match the behavior of gateway with openai Responses API. you can read https://github.com/vllm-project/agentic-api/blob/main/crates/agentic-server-core/tests/cassettes/README.md how to record a cassettes and there are shell script examples like https://github.com/vllm-project/agentic-api/blob/main/crates/agentic-server-core/tests/cassettes/record_web_search_cassettes.sh, https://github.com/vllm-project/agentic-api/blob/main/crates/agentic-server-core/tests/cassettes/record_mcp_cassettes.sh
that use our python cassettes recorder.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed the handler/normalization feedback in 0aaebc9: shell now has a client-owned ShellHandler, validates local-only environments, registers as client-owned, normalizes to a vLLM-compatible function schema, and restores both blocking and streaming calls to typed shell_call items. Unsupported environments and malformed actions fail closed; no gateway command execution was added. I also added record_shell_cassettes.sh for matching OpenAI/gateway streaming and non-streaming recordings. I could not capture authentic YAML in this environment because OPENAI_API_KEY is unset and the configured gateway endpoint is unavailable, so I intentionally did not hand-author fixtures. Validation: full cargo test -p agentic-server-core passed (448 passed, 8 environment-dependent ignored), plus fmt and clippy with warnings denied.

@maralbahari maralbahari Sep 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@bojiang3 do you have resources to record cassettes for both gatewayand for openai ? openai would need api key! if you cant actually record cassettes , I will record and push in your branch then you can write integration test comparing openai ground truth to agentic-api behavior. cassettes are a required for landing features specially for tool supports.

@franciscojavierarceo franciscojavierarceo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

the local shell wire types, call/output pairing, and storage round-trip look consistent with this slice. i don't have additional findings beyond the existing handler and cassette feedback. this was a source review; i haven't independently run the tests.

bojiang3 added 2 commits September 7, 2026 15:33
Signed-off-by: bojiang3 <bojiang3@illinois.edu>
Signed-off-by: bojiang3 <bojiang3@illinois.edu>
@bojiang3
bojiang3 force-pushed the feat/shell-wire-types branch from 5cf6c7c to 0aaebc9 Compare September 7, 2026 22:56
@franciscojavierarceo

Copy link
Copy Markdown
Collaborator

the client-executed shell loop still breaks on continuation, and the public Rust API doesn't yet let someone plug in their own shell executor. on 0aaebc9, i found:

  • shell input deserialization retains type in the flattened extra map, then serialization writes the enum tag again. submitting a valid shell_call_output produces duplicate JSON keys and an upstream HTTP 400; the equivalent OpenAI continuation succeeds. we should consume the discriminator before deserializing the payload and test serialized bytes, since round-tripping through serde_json::Value hides this.
  • rehydrated shell calls and submitted outputs remain in shell format even though the upstream declaration is normalized to a function. we should lower both to matching function history at the inference boundary while preserving the public shell items in storage. this is a separate gap after fixing duplicate keys; the Rust reproduction confirms the mismatch, but i haven't tested a live vLLM endpoint.
  • the accumulator creates no in-flight entry for native shell_call items. the existing strict replay test rejects both recorded shell streams at response.output_item.done with “has no active output item.” we should handle that lifecycle so another framework can feed shell streams through decode_upstream.
  • finished calls remain in_progress in both blocking output and streaming item-done events, whereas the OpenAI recordings return completed. we should preserve the completion status instead of hardcoding InProgress.
  • tool_choice: {"type":"shell"} succeeds against OpenAI but receives HTTP 400 from the gateway. we should add the typed selector and normalize it to the internal function selector.

for pluggability, we should expose an optional typed shell executor registration through ExecutionContext, keeping client execution as the default. insert_shell_entry currently always chooses client execution, and GatewayExecutorRegistration only has web-search and MCP slots. a small adapter implemented outside core should be able to register, accept typed calls with cancellation and limits, return typed outputs, and complete a two-round blocking/streaming test without changing core. that would give users a concrete seam for their own sandbox or execution framework.

i captured eight local cassettes covering streaming, blocking, continuation, and explicit selection against OpenAI directly and this gateway backed by the same model. they aren't pushed yet. all 448 core unit tests pass, as do Clippy and formatting; the recordings expose the strict replay failure above, and five focused review assertions fail.

bojiang3 and others added 2 commits September 7, 2026 19:22
Signed-off-by: bojiang3 <bojiang3@illinois.edu>
- Prevent duplicate type tags in serialized shell inputs
- Convert shell calls and outputs to matching function history
- Support explicit shell tool selection and completed call status
- Translate shell command SSE events and validate native stream lifecycle
- Add two-turn cassettes and gateway/OpenAI compatibility tests

Signed-off-by: maral <maralbahari.98@gmail.com>
@maralbahari

Copy link
Copy Markdown
Collaborator

@bojiang3 @franciscojavierarceo
After recording the cassettes against the gateway and OpenAI, I found that the gateway’s shell behavior differed from OpenAI in streaming lifecycle, completion status, and continuation handling. This commit addresses those differences.

  • Streaming lifecycle: Added translation to response.shell_call_command.added, .delta, and .done, plus accumulation and validation of native shell streams. Validation checks command indices, event ordering, completion, and consistency between streamed commands and the final shell item.
  • Duplicate JSON keys: Shell input deserialization retained the type discriminator in the flattened extra map. Serialization then wrote both that retained value and the enum’s own type tag, producing duplicate keys and causing continuation requests to fail upstream. The discriminator is now consumed before payload deserialization. Regression tests inspect serialized bytes because converting through serde_json::Value can hide duplicate keys.
  • Continuation: Shell declarations are normalized to function tools for inference, so shell calls and submitted outputs must also become matching function-call history. Added typed conversions through the existing input and output-to-input paths while preserving public shell output items in storage.
  • Status and selection: Preserved completed status for finished shell calls and supported explicit tool_choice: {"type": "shell"}.
  • Execution ownership: The local shell tool is client-owned and client-executed. The gateway returns a shell_call; the client executes the commands and submits shell_call_output using the same call_id. Removed the optional gateway shell executor to keep this responsibility on the client.
  • Recording and coverage: Added two-turn scenarios for success, nonzero exit, timeout, and multiple commands in both streaming and blocking modes. Failed recordings retain their YAML, and Rust tests compare gateway behavior against OpenAI.

The latest recordings match on shell actions, limits, completion status, command lifecycle, and continuation behavior. Delta fragment sizes differ, but reconstruct the same commands. The recordings use simulated client command outputs.

@maralbahari

Copy link
Copy Markdown
Collaborator

@bojiang3 please resolve the conflicts. afterwards I'll re-record cassettes again to make sure there is no regression.

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.

3 participants