Skip to content

Future-proof SDK policy wire types - #2385

Closed
roji wants to merge 1 commit into
mainfrom
roji-open-sdk-policy-types
Closed

Future-proof SDK policy wire types#2385
roji wants to merge 1 commit into
mainfrom
roji-open-sdk-policy-types

Conversation

@roji

@roji roji commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

Summary

Extracts the runtime-independent, SDK-owned prerequisite fixes from #2375:

  • decouple Go managed bypass policy values from the generated closed enum and preserve future string values
  • make Java managed bypass policy values open strings while retaining the generated-enum setter overload for source ergonomics
  • construct Rust ConnectRequest from Default so newly generated optional fields do not require handwritten updates
  • configure the existing Go invalid task-agent-model E2E snapshot fixture

@github/copilot remains unchanged. This PR does not update the runtime, generated sources, snapshots, version manifests, or Rust launcher behavior.

Validation

  • cd go && go test . -run '^TestSessionRequests_ManagedSettings$'
  • cd go && go test ./internal/e2e -run 'TestRPCTasksAndHandlersE2E/should report implemented error for invalid task agent model' -count=1
  • cd java && mvn spotless:check
  • cd java && mvn -pl sdk test -Dtest=ManagedSettingsTest
  • cd rust && cargo +nightly-2026-04-14 fmt --check
  • cd rust && cargo clippy --all-features --all-targets -- -D warnings
  • cd rust && cargo test --all-features

Use SDK-owned open strings for managed bypass policy values, preserve Java's generated-enum setter compatibility, initialize Rust connect requests from defaults, and activate the Go invalid-model fixture.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings August 23, 2026 22:03
@roji
roji requested a review from a team as a code owner August 23, 2026 22:03
@roji roji closed this Aug 23, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Cross-SDK Consistency Review

This PR makes disableBypassPermissionsMode an open string type in Go and Java (adding the allow-auto-only value and supporting arbitrary future values), but the same type remains closed/restricted in three other SDKs:

SDKs not yet updated

SDK Current state Issue
Python (python/copilot/client.py:259) Literal["disable"] | None Locked to "disable" only; cannot pass "allow-auto-only" or future modes
Node.js (nodejs/src/types.ts:2184) disableBypassPermissionsMode?: "disable" Same — TypeScript literal union, no "allow-auto-only"
Rust (rust/src/types.rs:1767) enum DisableBypassPermissionsMode { Disable } (closed enum, only one variant) Cannot represent allow-auto-only or future modes without an SDK release; no #[non_exhaustive] would help here since it's an input type — callers can't construct unknown variants
.NET (dotnet/src/Types.cs:3059) enum DisableBypassPermissionsMode { Disable } Same as Rust — closed enum, allow-auto-only is unrepresentable

Suggested alignment

To match the Go/Java approach in the other SDKs:

  • Python: Widen the type to str | None (or Literal["disable", "allow-auto-only"] | str | None) so callers can pass future values directly.
  • Node.js: Widen to "disable" | "allow-auto-only" | (string & {}) or a string type alias, and add the allow-auto-only literal to the union.
  • Rust: Add an AllowAutoOnly variant to the enum and change the field type to Option<String> (or use a newtype wrapper around String), matching the PR's stated goal of "callers can use newer modes without waiting for an SDK release."
  • .NET: Add an AllowAutoOnly member to the enum and consider a string? overload or a separate DisableBypassPermissionsModes constants class (similar to Java's approach) so callers aren't blocked on SDK releases.

These aren't blocking concerns for this PR if the allow-auto-only mode isn't shipped to the runtime yet (the PR description says the runtime is unchanged), but they would be worth tracking as follow-up work before allow-auto-only is activated.

Generated by SDK Consistency Review Agent for #2385 · sonnet46 31.2 AIC · ⌖ 5.6 AIC · ⊞ 6.6K ·

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

Future-proofs handwritten SDK code against upcoming policy and generated-type changes.

Changes:

  • Opens Go and Java bypass-policy wire values to future strings.
  • Builds Rust handshake requests from generated defaults.
  • Configures the existing Go invalid-model E2E fixture.
Show a summary per file
File Description
go/types.go Defines open policy values and constants.
go/client_test.go Tests future policy serialization.
go/internal/e2e/rpc_tasks_and_handlers_e2e_test.go Configures the invalid-model snapshot.
java/sdk/src/main/java/com/github/copilot/rpc/ManagedSettingsPermissions.java Stores bypass policy as a string.
java/sdk/src/main/java/com/github/copilot/rpc/DisableBypassPermissionsModes.java Adds known string constants.
java/sdk/src/test/java/com/github/copilot/ManagedSettingsTest.java Tests known and future policy values.
rust/src/lib.rs Default-initializes generated connect requests.

Review details

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

  • Files reviewed: 7/7 changed files
  • Comments generated: 3
  • Review effort level: Balanced


/** @return the bypass-permissions policy, or {@code null} when unset */
public DisableBypassPermissionsMode getDisableBypassPermissionsMode() {
public String getDisableBypassPermissionsMode() {
* bypass-permissions policy
* @return this policy
*/
public ManagedSettingsPermissions setDisableBypassPermissionsMode(String value) {
Comment thread go/types.go
//
// The runtime may introduce additional fail-closed modes. Values are serialized
// as strings so callers can use newer modes without waiting for an SDK release.
type DisableBypassPermissionsMode string
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