go: preserve tri-state session flags#1536
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR updates session configuration options to support tri-state behavior (unset vs explicitly true/false) for config discovery and pending work continuation, allowing clients to preserve runtime defaults while still being able to explicitly disable features.
Changes:
- Converted
EnableConfigDiscoveryandContinuePendingWorkfromboolto*bool(tri-state) in public config types. - Updated client request construction to forward pointer values directly.
- Adjusted samples/e2e usage and added unit tests to validate JSON omit/forward behavior.
Show a summary per file
| File | Description |
|---|---|
| go/types.go | Changes config fields to *bool and updates GoDoc to describe tri-state semantics. |
| go/client.go | Forwards *bool values into RPC request structs (instead of only sending true). |
| go/client_test.go | Adds tests for JSON forwarding/omission of enableConfigDiscovery and continuePendingWork. |
| go/samples/manual_tool_resume/main.go | Updates sample to pass copilot.Bool(true) for new pointer field. |
| go/internal/e2e/*_e2e_test.go | Updates e2e configs to use copilot.Bool(...) for new pointer fields. |
Copilot's findings
- Files reviewed: 8/8 changed files
- Comments generated: 3
Comment on lines
+904
to
+911
| // EnableConfigDiscovery, when non-nil, controls automatic discovery of MCP server configurations | ||
| // (e.g. .mcp.json, .vscode/mcp.json) and skill directories from the working directory | ||
| // and merges them with any explicitly provided MCPServers and SkillDirectories, with | ||
| // explicit values taking precedence on name collision. | ||
| // Nil leaves the runtime default unchanged; use Bool(false) to explicitly disable discovery. | ||
| // Custom instruction files (.github/copilot-instructions.md, AGENTS.md, etc.) are | ||
| // always loaded from the working directory regardless of this setting. | ||
| EnableConfigDiscovery bool | ||
| EnableConfigDiscovery *bool |
Comment on lines
+500
to
+508
| t.Run("create omits enableConfigDiscovery when unset", func(t *testing.T) { | ||
| req := createSessionRequest{} | ||
| data, _ := json.Marshal(req) | ||
| var m map[string]any | ||
| json.Unmarshal(data, &m) | ||
| if _, ok := m["enableConfigDiscovery"]; ok { | ||
| t.Error("Expected enableConfigDiscovery to be omitted when unset") | ||
| } | ||
| }) |
Comment on lines
+525
to
+533
| t.Run("resume omits enableConfigDiscovery when unset", func(t *testing.T) { | ||
| req := resumeSessionRequest{SessionID: "s1"} | ||
| data, _ := json.Marshal(req) | ||
| var m map[string]any | ||
| json.Unmarshal(data, &m) | ||
| if _, ok := m["enableConfigDiscovery"]; ok { | ||
| t.Error("Expected enableConfigDiscovery to be omitted when unset") | ||
| } | ||
| }) |
SteveSandersonMS
approved these changes
Jun 1, 2026
This was referenced Jun 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SessionConfig.EnableConfigDiscovery,ResumeSessionConfig.EnableConfigDiscovery, andResumeSessionConfig.ContinuePendingWorkfrom plainboolto*bool.session.create/session.resume, so explicitfalseis preserved instead of being omitted.Impact
This is a breaking Go API change for callers assigning plain booleans to these fields. Callers now use
copilot.Bool(true)orcopilot.Bool(false)when they want to override the runtime default, and leave the field nil when they want the runtime default.The behavioral impact is that Go callers can now intentionally send
enableConfigDiscovery=falseandcontinuePendingWork=false. Previously those false values were indistinguishable from unset, which meant the SDK silently omitted them and could not express user intent.Why
The wire schema treats these options as optional booleans with tri-state semantics: unset means defer to runtime default, true means opt in, and false means explicitly opt out. The Go public API used plain booleans, collapsing unset and false into the same zero value. Aligning the API with the schema avoids surprising behavior for configuration discovery and pending-work resume flows.
Validation
Not run, per request.