Skip to content

SDKs: Add sandbox config - #2371

Open
jcoc611-microsoft wants to merge 10 commits into
mainfrom
user/juosori/Sandbox-RustSDK
Open

SDKs: Add sandbox config#2371
jcoc611-microsoft wants to merge 10 commits into
mainfrom
user/juosori/Sandbox-RustSDK

Conversation

@jcoc611-microsoft

@jcoc611-microsoft jcoc611-microsoft commented Aug 21, 2026

Copy link
Copy Markdown

Summary

Add sandbox configuration support to session creation and resume APIs across all six SDK languages.

  • Expose the generated SandboxConfig and related policy types through each SDK's public session configuration API.
  • Forward the optional configuration as sandboxConfig for create and resume requests, while preserving omission when unset.
  • Apply configured sandbox options through session.options.update before returning a created or resumed session, ensuring current CLI versions activate the requested policy.
  • Add create/resume serialization coverage across Rust, Node.js, .NET, Python, Go, and Java.
  • Add behavior-based E2E coverage for Node.js, .NET, Python, Go, and Java using the shared replay fixture.
  • Verify actual policy effects: enabled create blocks a write outside automatic grants, disabled create permits it, and enabled resume blocks it again.

Testing

  • Linux bubblewrap E2Es passed for Node.js, .NET, Python, Go, and Java against Copilot CLI 1.0.81-6.
  • Node.js focused client test passed.
  • Python focused client test, Ruff format, and Ruff lint passed.
  • Go focused client test and gofmt passed.
  • .NET compiled on net8.0; the exact sandbox E2E passed in Linux.
  • Java options-update unit tests, the exact sandbox E2E, Spotless, and Checkstyle passed.
  • Rust compiled with Rust 1.94 on Linux; sandbox serialization test and formatting check passed.

@jcoc611-microsoft
jcoc611-microsoft requested review from jmoseley and stephentoub and a balanced review from Copilot and removed request for stephentoub August 21, 2026 21:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds Rust SDK sandbox configuration support for session creation and resume.

Changes:

  • Exposes sandbox policy types publicly.
  • Forwards optional sandbox configuration in create/resume requests.
  • Adds nested serialization and omission coverage.
Show a summary per file
File Description
rust/src/wire.rs Adds optional sandbox configuration to wire payloads.
rust/src/types.rs Extends public configs, conversions, exports, redacted debug output, and tests.

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Balanced

@jcoc611-microsoft
jcoc611-microsoft marked this pull request as ready for review August 21, 2026 21:40
@jcoc611-microsoft
jcoc611-microsoft requested a review from a team as a code owner August 21, 2026 21:40
@SteveSandersonMS
SteveSandersonMS marked this pull request as draft August 25, 2026 15:14
@SteveSandersonMS

SteveSandersonMS commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Thanks for adding the Rust support. Would you be willing to apply the same improvement across the other five SDK languages (Node, C#, Python, Go, and Java), with at least one end-to-end test for each language?

We aim to keep all six SDKs in sync in capabilities and usage patterns, so landing this only for Rust would leave an avoidable cross-language gap. Please move the PR back out of draft status once the other implementations and E2E coverage are ready for review.

@jcoc611-microsoft jcoc611-microsoft changed the title Rust SDK: Add sandbox config SDKs: Add sandbox config Aug 25, 2026
@jcoc611-microsoft
jcoc611-microsoft marked this pull request as ready for review August 25, 2026 17:30
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Comment thread java/sdk/src/main/java/com/github/copilot/CopilotClient.java Fixed
@github-actions

This comment has been minimized.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Generated by SDK Consistency Review Agent for #2371 · sonnet46 99.5 AIC · ⌖ 5.84 AIC · ⊞ 6.6K

Comment thread nodejs/src/types.ts
Comment thread rust/src/types.rs
Comment thread python/copilot/client.py Outdated
Comment thread go/types.go
@github-actions

This comment has been minimized.

Juan Osorio added 2 commits August 25, 2026 15:31
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

Cross-SDK Consistency Review ✅ (with one minor suggestion)

This PR successfully adds sandboxConfig support to all six SDK implementations (Node.js, Python, Go, .NET, Java, Rust) with consistent API naming and behavior. The feature is implemented in both create_session and resume_session across all SDKs, and the session.options.update post-create patch is correctly applied everywhere.

One minor Rust inconsistency

The Rust SDK has ergonomic builder methods for similar experimental fields (e.g., with_session_limits() on both SessionConfig and ResumeSessionConfig), but the PR adds sandbox_config as a plain public field without a corresponding with_sandbox_config() builder method. I've left an inline comment on rust/src/types.rs with the suggested addition. This is a minor ergonomics gap rather than a functional issue.

Everything else looks consistent ✅

SDK sandboxConfig in create sandboxConfig in resume options.update patch Types exported
Node.js
Python
Go via generated rpc
.NET
Java
Rust

Generated by SDK Consistency Review Agent for #2371 · sonnet46 55.6 AIC · ⌖ 5.56 AIC · ⊞ 6.6K ·

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Generated by SDK Consistency Review Agent for #2371 · sonnet46 55.6 AIC · ⌖ 5.56 AIC · ⊞ 6.6K

Comment thread rust/src/types.rs
/// **Experimental.** Limits applied to this session's current accounting window.
pub session_limits: Option<SessionLimitsConfig>,
/// **Experimental.** Resolved sandbox configuration applied when the session is created.
pub sandbox_config: Option<SandboxConfig>,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cross-SDK consistency suggestion: The Rust SessionConfig has a with_session_limits() builder method for the analogous session_limits field (at line 3103), but no corresponding with_sandbox_config() builder method is added for sandbox_config.

For consistency with the existing ergonomic builder pattern, consider adding:

/// **Experimental.** Set the sandbox configuration for this session.
pub fn with_sandbox_config(mut self, config: SandboxConfig) -> Self {
    self.sandbox_config = Some(config);
    self
}

The test at the bottom of this file also directly assigns resume_config.sandbox_config = Some(sandbox_config) instead of using a builder call, which would read more naturally as resume_config.with_sandbox_config(sandbox_config) once the builder is added.

The same applies to ResumeSessionConfig.sandbox_config (line 3400).

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.

4 participants