SDKs: Add sandbox config - #2371
Conversation
There was a problem hiding this comment.
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
|
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. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Generated by SDK Consistency Review Agent for #2371 · sonnet46 99.5 AIC · ⌖ 5.84 AIC · ⊞ 6.6K
This comment has been minimized.
This comment has been minimized.
…RustSDK # Conflicts: # .github/workflows/java-sdk-tests.yml
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Cross-SDK Consistency Review ✅ (with one minor suggestion)This PR successfully adds One minor Rust inconsistencyThe Rust SDK has ergonomic builder methods for similar experimental fields (e.g., Everything else looks consistent ✅
|
There was a problem hiding this comment.
Generated by SDK Consistency Review Agent for #2371 · sonnet46 55.6 AIC · ⌖ 5.56 AIC · ⊞ 6.6K
| /// **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>, |
There was a problem hiding this comment.
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).
Summary
Add sandbox configuration support to session creation and resume APIs across all six SDK languages.
SandboxConfigand related policy types through each SDK's public session configuration API.sandboxConfigfor create and resume requests, while preserving omission when unset.session.options.updatebefore returning a created or resumed session, ensuring current CLI versions activate the requested policy.Testing
1.0.81-6.net8.0; the exact sandbox E2E passed in Linux.