Go SDK API review fixes by SteveSandersonMS · Pull Request #1360 · github/copilot-sdk

SteveSandersonMS · 2026-05-21T14:24:20Z

Applies the equivalent of the C# (#1343) and TypeScript (#1357) API review fixes to the Go SDK, plus the Go-specific items from api_review_go.md.

This is a breaking-change PR by design, intentionally landed in one pass before the SDK is declared stable. All unit tests, e2e tests, samples, docs, and the Go scenario apps under test/scenarios/**/go were migrated in lockstep with the source changes. (E2E suites still need the local nodejs CLI install to actually run — they migrate cleanly but I haven't executed them locally.)

Cross-language parity note for reviewers

Please assess consistency against C# and TypeScript only at this point. Python and Rust haven't been touched yet; once we're happy with the Go shape we'll port the equivalent changes there.

Phases

Each phase is a self-contained commit. After every phase: go build ./... && go vet ./... && go test . ./rpc (and go build ./... in go/samples/ and every scenario go/).

Phase A — SessionConfig / ResumeSessionConfig renames
- OnExitPlanMode → OnExitPlanModeRequest, OnAutoModeSwitch → OnAutoModeSwitchRequest
- ExitPlanModeHandler → ExitPlanModeRequestHandler, AutoModeSwitchHandler → AutoModeSwitchRequestHandler
- Session.GetMessages → Session.GetEvents
- ResumeSessionConfig.DisableResume → SuppressResumeEvent
- Streaming bool → *bool on both configs
Phase B + C — small wire/shape tweaks
- ProviderConfig.MaxInputTokens → MaxPromptTokens (matches the wire name maxPromptTokens)
- MCPStdioServerConfig.Tools / MCPHTTPServerConfig.Tools: JSON tag → tools,omitempty. nil slice now means "all tools" (matches TS/C#).
Phase D — RuntimeConnection discriminated config (the largest change)
- Replaces CLIPath / CLIArgs / CLIUrl / UseStdio / Port / TCPConnectionToken / AutoStart / AutoRestart on ClientOptions with a single Connection RuntimeConnection field, constructed from one of:
  - StdioConnection{Path, Args}
  - TcpConnection{Port, ConnectionToken, Path, Args}
  - UriConnection{URL, ConnectionToken}
- CopilotHome → BaseDirectory
- Remote → EnableRemoteSessions
- Client.ActualPort() → Client.RuntimePort()
- Drop public ConnectionState, State* constants, Client.State() — purely-internal state remains
- LogLevel no longer defaults to "info": when empty, the SDK does not pass --log-level to the runtime at all (matches TS).
Phase E — SessionMetadata / SessionLifecycleEventMetadata: StartTime / ModifiedTime string → time.Time (wire is ISO 8601, parsed by time.Time's default JSON unmarshal)
Phase F — All six hook input types: Timestamp int64 → time.Time, with MarshalJSON / UnmarshalJSON pairs that serialize to/from Unix milliseconds on the wire
Phase G-K — misc cleanups
- G: InputOptions → UiInputOptions
- H: Convenience Session.SendPrompt(ctx, prompt) and Session.SendPromptAndWait(ctx, prompt)
- I: SessionFsProvider method renames (Go-specific): Mkdir → MakeDirectory, Readdir → ReadDirectory, ReaddirWithTypes → ReadDirectoryWithTypes, Rm → Remove. The adapter still implements the wire-protocol method names on rpc.SessionFsHandler unchanged.
- J: CreateSessionFsHandler field → CreateSessionFsProvider
- K: Remove deprecated Session.Destroy() (callers must use Session.Disconnect())
Phase L — Docs, samples, scenarios

Items explicitly NOT in this PR

Documented in the session plan:

SessionConfigBase extraction — skipped. Go has no struct inheritance; embedding would change JSON marshal behaviour and zero-value semantics, and the duplication is small. Documented divergence from TS/C#.
Sealing types — N/A in Go.
Removing named handler types in favour of plain func(...) — named handler types are idiomatic in Go and not noise the way C# delegate declarations are.
Discriminated lifecycle event type split (SessionCreatedEvent etc.) — keeping the flat SessionLifecycleEvent with a Type discriminator. Without sum-type narrowing in Go, a discriminated interface adds friction (forced type-switches at every callsite) without changing the wire shape.
LogLevel value-object — kept as string (TS/Go), but did drop the "info" default as part of Phase D.
AsyncDisposable — N/A; Disconnect() / Stop() are idiomatic.
_internalConnection / joinSession — feature not exposed in Go yet.

Already correct (no change needed)

GitHubToken casing, no "local" MCP alias, hidden session constructor, clean Stop() exit, MCP broadcast handling.

- OnExitPlanMode -> OnExitPlanModeRequest - OnAutoModeSwitch -> OnAutoModeSwitchRequest - ExitPlanModeHandler -> ExitPlanModeRequestHandler - AutoModeSwitchHandler -> AutoModeSwitchRequestHandler - Session.GetMessages -> Session.GetEvents - ResumeSessionConfig.DisableResume -> SuppressResumeEvent - Streaming bool -> *bool on SessionConfig and ResumeSessionConfig Wire-level RPC method (session.getMessages) and internal request types are unchanged; only the public Go surface is renamed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

B. ProviderConfig.MaxInputTokens -> MaxPromptTokens (matches the wire name 'maxPromptTokens' exactly). C. MCPStdioServerConfig.Tools / MCPHTTPServerConfig.Tools: change JSON tag from 'tools' to 'tools,omitempty'. Now nil/omitted slice means 'all tools' (CLI default), matching TS/Rust/C# semantics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Replaces the flat connection fields on ClientOptions with a single discriminated Connection RuntimeConnection field constructed from one of: StdioConnection{Path, Args} TcpConnection{Port, ConnectionToken, Path, Args} UriConnection{URL, ConnectionToken} Removes: - CLIPath, CLIArgs, CLIUrl, UseStdio, Port, TCPConnectionToken - AutoStart, AutoRestart - public ConnectionState type, State* constants, and Client.State() method Renames: - CopilotHome -> BaseDirectory - Remote -> EnableRemoteSessions - Client.ActualPort() -> Client.RuntimePort() LogLevel no longer defaults to 'info'. When empty, the SDK does not pass --log-level to the runtime, matching the TS SDK. All unit tests, e2e tests, samples, and the Go scenario apps are migrated. README updated. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

SessionMetadata and SessionLifecycleEventMetadata: StartTime and ModifiedTime change from string to time.Time. The runtime emits these as ISO 8601 strings, which time.Time's default JSON unmarshal handles natively. Matches the equivalent C# (#1343) / TS (#1357) changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

All six hook input types (PreToolUseHookInput, PostToolUseHookInput, UserPromptSubmittedHookInput, SessionStartHookInput, SessionEndHookInput, ErrorOccurredHookInput): Timestamp changes from int64 to time.Time. Each type gains a MarshalJSON/UnmarshalJSON pair that serializes Timestamp as Unix milliseconds on the wire, matching the runtime protocol. Mirrors the equivalent C# (#1343) UnixMillisecondsDateTimeOffsetConverter and TS (#1357) Date-typed hook timestamps. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

G. InputOptions -> UiInputOptions on the Session.UI().Input convenience. H. Add Session.SendPrompt(ctx, prompt) and Session.SendPromptAndWait(ctx, prompt) convenience wrappers around the MessageOptions-based methods (mirrors the string overloads added in C#/TS). I. SessionFsProvider interface method renames (Go-specific): Mkdir -> MakeDirectory, Readdir -> ReadDirectory, ReaddirWithTypes -> ReadDirectoryWithTypes, Rm -> Remove. The adapter still implements the wire-protocol method names (rpc.SessionFsHandler) unchanged. J. SessionConfig/ResumeSessionConfig.CreateSessionFsHandler -> CreateSessionFsProvider (matches the SessionFsProvider type it returns). K. Remove deprecated Session.Destroy(); callers must use Session.Disconnect(). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Update Go code samples in docs/ to use the new ClientOptions.Connection shape (StdioConnection / UriConnection). Also migrate the streaming scenario to copilot.Bool(true) for the new *bool Streaming field. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

CI docs-validate extracts Go code blocks and compiles them. Update the remaining 4 sites (docs/getting-started.md x3, docs/features/streaming-events.md) to use copilot.Bool(true) now that SessionConfig.Streaming is *bool. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

This PR applies the cross-language (C# / TypeScript) API review decisions to the Go SDK, including the new discriminated RuntimeConnection configuration model, multiple naming/shape alignments, and synchronized updates across Go tests, scenarios, samples, and documentation.

Changes:

Introduces RuntimeConnection (StdioConnection / TcpConnection / UriConnection) and migrates callers away from CLIPath/CLIUrl/UseStdio/Port fields.
Updates Go SDK API shapes for parity (e.g., Streaming *bool, hook timestamps to time.Time, GetMessages → GetEvents, provider/session fs renames).
Migrates Go E2E/unit tests, scenario apps, samples, and docs to the new API.

Show a summary per file

File	Description
test/scenarios/transport/tcp/go/main.go	Updates scenario to use `ClientOptions.Connection` with `UriConnection`.
test/scenarios/transport/reconnect/go/main.go	Updates scenario to use `ClientOptions.Connection` with `UriConnection`.
test/scenarios/sessions/streaming/go/main.go	Updates scenario to use `Streaming: copilot.Bool(true)`.
test/scenarios/bundling/container-proxy/go/main.go	Updates scenario to use `ClientOptions.Connection` with `UriConnection`.
test/scenarios/bundling/app-direct-server/go/main.go	Updates scenario to use `ClientOptions.Connection` with `UriConnection`.
test/scenarios/bundling/app-backend-to-server/go/main.go	Updates backend scenario to use `ClientOptions.Connection` with `UriConnection`.
go/types.go	Adds `RuntimeConnection` types; aligns config/event types (timestamps, streaming pointers, renames) and MCP tool-list tagging.
go/types_test.go	Updates provider-config JSON test for `MaxPromptTokens`.
go/session.go	Adds `SendPrompt`/`SendPromptAndWait`, renames APIs (`GetEvents`, handler types, `UiInputOptions`), removes deprecated `Destroy`.
go/session_fs_provider.go	Renames session fs provider methods and updates adapter to call renamed methods.
go/samples/manual_tool_resume/main.go	Migrates sample to `StdioConnection{Path: ...}`.
go/samples/chat.go	Migrates sample to `StdioConnection{Path: ...}`.
go/README.md	Documents new `Connection` model, `RuntimePort`, `Streaming *bool`, and related renames.
go/internal/e2e/testharness/helper.go	Migrates helper to `GetEvents`.
go/internal/e2e/testharness/context.go	Updates test harness default client options to use `StdioConnection`.
go/internal/e2e/suspend_e2e_test.go	Migrates suspend tests to `UriConnection` + connection token.
go/internal/e2e/streaming_fidelity_e2e_test.go	Migrates streaming tests to `Streaming: copilot.Bool(...)` and `GetEvents`.
go/internal/e2e/session_fs_sqlite_e2e_test.go	Migrates fs sqlite tests to renamed fs methods + `CreateSessionFsProvider`.
go/internal/e2e/session_fs_e2e_test.go	Migrates fs tests to new connection model, `RuntimePort`, renamed fs APIs, and `GetEvents`.
go/internal/e2e/session_e2e_test.go	Migrates session tests to `GetEvents` and `time.Time` metadata assertions.
go/internal/e2e/session_config_e2e_test.go	Migrates config tests to `GetEvents`.
go/internal/e2e/rpc_shell_and_fleet_e2e_test.go	Migrates polling logic to `GetEvents`.
go/internal/e2e/rpc_session_state_e2e_test.go	Migrates session state tests to `GetEvents`.
go/internal/e2e/rpc_mcp_and_skills_e2e_test.go	Updates CLI arg injection to flow via `StdioConnection.Args`.
go/internal/e2e/rpc_event_side_effects_e2e_test.go	Migrates event side-effect tests to `GetEvents`.
go/internal/e2e/rpc_e2e_test.go	Migrates RPC E2E tests to `StdioConnection`.
go/internal/e2e/per_session_auth_e2e_test.go	Migrates auth test client creation to `StdioConnection`.
go/internal/e2e/pending_work_resume_e2e_test.go	Migrates resume tests to `UriConnection`, `RuntimePort`, `SuppressResumeEvent`, `GetEvents`.
go/internal/e2e/multi_client_e2e_test.go	Migrates multi-client tests to `TcpConnection`/`UriConnection` and `RuntimePort`.
go/internal/e2e/mode_handlers_e2e_test.go	Migrates mode handler names to `OnExitPlanModeRequest` / `OnAutoModeSwitchRequest`.
go/internal/e2e/event_fidelity_e2e_test.go	Migrates fidelity checks to `GetEvents` and updated error messages.
go/internal/e2e/error_resilience_e2e_test.go	Migrates resilience checks to `GetEvents`.
go/internal/e2e/connection_token_test.go	Migrates token tests to `TcpConnection`/`UriConnection` and `RuntimePort`.
go/internal/e2e/commands_and_elicitation_e2e_test.go	Migrates commands/UI elicitation multi-client tests to new connection model and renames.
go/internal/e2e/client_options_e2e_test.go	Updates options tests to new connection model and removes state-based assertions.
go/internal/e2e/client_lifecycle_e2e_test.go	Removes state assertions after `State()` API removal.
go/internal/e2e/client_e2e_test.go	Migrates client E2E tests to new connection model and removes state assertions.
go/internal/e2e/agent_and_compact_rpc_e2e_test.go	Migrates agent-selection tests to `StdioConnection`.
go/internal/e2e/abort_e2e_test.go	Migrates abort test to `Streaming: copilot.Bool(true)`.
go/client.go	Implements `RuntimeConnection` resolution, drops public state API, updates runtime spawning/args/log-level semantics.
go/client_test.go	Updates unit tests for new connection model (but currently has formatting issues).
docs/troubleshooting/debugging.md	Updates Go guidance to refer to `Connection (UriConnection)` (but still claims no extra-args support).
docs/setup/local-cli.md	Updates Go examples to `Connection`, but also changes some cross-language wording.
docs/setup/bundled-cli.md	Updates Go note to refer to `Connection`.
docs/setup/backend-services.md	Updates Go backend-services example to use `UriConnection`.
docs/getting-started.md	Updates Go examples to use `Streaming: copilot.Bool(true)` and `UriConnection`.
docs/features/streaming-events.md	Updates Go streaming example to use `Streaming: copilot.Bool(true)`.

Copilot's findings

Comments suppressed due to low confidence (1)

docs/setup/local-cli.md:16

The prose here says Node.js/Python/.NET can override the bundled CLI using the Connection option, but the Node and Python examples below still use cliPath / cli_path (and the diagram arrow is labeled cliPath). Consider rewording to be language-specific (e.g., Node: cliPath, Python: cli_path, .NET: Connection) to avoid confusing readers.

## How it works

By default, the Node.js, Python, and .NET SDKs include their own CLI dependency (see [Default Setup](./bundled-cli.md)). If you need to override this—for example, to use a system-installed CLI—you can use the `Connection` option.

```mermaid
flowchart LR
    subgraph YourMachine["Your Machine"]
        App["Your App"] --> SDK["SDK Client"]
        SDK -- "cliPath" --> CLI["Copilot CLI<br/>(your own binary)"]
        CLI --> Keychain["🔐 System Keychain<br/>(stored credentials)"]

Files reviewed: 47/47 changed files
Comments generated: 4

+// The Tools field controls which tools from the server are exposed:
+//   - nil (omitted from the wire): all tools (CLI default)
+//   - []string{"*"}: explicit "all tools"
+//   - []string{}: no tools
+//   - []string{"foo","bar"}: only those tools
 type MCPStdioServerConfig struct {
-	Tools   []string          `json:"tools"`
+	Tools   []string          `json:"tools,omitempty"`
 	Timeout int               `json:"timeout,omitempty"`


 // MCPHTTPServerConfig configures a remote MCP server (HTTP or SSE).
+//
+// See [MCPStdioServerConfig] for the semantics of the Tools field.
 type MCPHTTPServerConfig struct {
-	Tools   []string          `json:"tools"`
+	Tools   []string          `json:"tools,omitempty"`
 	Timeout int               `json:"timeout,omitempty"`


 func TestClient_URLParsing(t *testing.T) {
 	t.Run("should parse port-only URL format", func(t *testing.T) {
 		client := NewClient(&ClientOptions{
-			CLIUrl: "8080",
+			Connection: UriConnection{URL: "8080"},
 		})
-
 		if client.actualPort != 8080 {
 			t.Errorf("Expected port 8080, got %d", client.actualPort)
 		}


 func main() {
 	// The Go SDK does not currently support passing extra CLI arguments.
 	// For custom log directories, run the CLI manually with --log-dir
-	// and connect via CLIUrl option.
+	// and connect via Connection (UriConnection).
 }


github-actions · 2026-05-21T15:03:24Z

Cross-SDK Consistency Review ✅

I reviewed this PR's Go SDK changes against the TypeScript (#1357) and C# (#1343) API reviews, per the author's request. All substantive changes are well-aligned.

Consistent across Go, TypeScript, and C#

Change	Go (this PR)	TypeScript	C#
Handler renames	`OnExitPlanModeRequest`, `OnAutoModeSwitchRequest`	same (camelCase)	same (PascalCase)
Resume option	`SuppressResumeEvent`	`suppressResumeEvent`	`SuppressResumeEvent`
Streaming field	`*bool` (pointer)	optional	nullable `bool?`
Provider field	`MaxPromptTokens`	`maxPromptTokens`	`MaxPromptTokens`
ClientOptions	`Connection RuntimeConnection` (via `StdioConnection{}`, `TcpConnection{}`, `UriConnection{}`)	`RuntimeConnection.forStdio/forTcp/forUri()`	`RuntimeConnection.ForStdio/ForTcp/ForUri()`
Client option	`BaseDirectory`	`baseDirectory`	`BaseDirectory`
Client option	`EnableRemoteSessions`	`enableRemoteSessions`	`EnableRemoteSessions`
Client method	`RuntimePort() int`	(private only)	`int? RuntimePort`
Session method	`GetEvents()`	`getEvents()`	`GetEventsAsync()`
Input type	`UiInputOptions`	`UiInputOptions`	`UiInputOptions`

Language-idiomatic differences (not issues)

RuntimeConnection: Go uses plain structs implementing an interface (idiomatic Go type constraints) rather than factory methods on an abstract base—the conceptual model is identical.
SessionFsProvider renames (MakeDirectory, ReadDirectory, Remove): these are Go-specific and consistent with Go naming conventions; the wire-protocol names are unchanged.
time.Time for timestamps: using Go's native JSON unmarshalling is the right Go-idiomatic choice; TS/C# use Date/DateTimeOffset.
Named handler types (ExitPlanModeRequestHandler etc.): appropriate in Go where first-class function types are the norm.

Acknowledged gaps (not in scope for this PR)

Python and Rust are explicitly deferred to a follow-up, as documented in the PR description. No action needed here.

One pre-existing minor inconsistency (not introduced by this PR)

TypeScript's CopilotClient tracks runtimePort as a private field with no public getter, while .NET exposes public int? RuntimePort and Go now exposes RuntimePort() int. This existed before this PR and would be worth a small follow-up to add a public getter to the TypeScript SDK for parity.

Bottom line: this PR successfully applies the equivalent of the C# and TypeScript API review fixes to the Go SDK. The changes are consistent and well-reasoned. 🎉

Generated by SDK Consistency Review Agent for issue #1360 · ● 1.8M · ◷

SteveSandersonMS and others added 7 commits May 21, 2026 14:45

This comment has been minimized.

Sign in to view

SteveSandersonMS marked this pull request as ready for review May 21, 2026 14:50

SteveSandersonMS requested a review from a team as a code owner May 21, 2026 14:50

Copilot AI review requested due to automatic review settings May 21, 2026 14:50

Copilot started reviewing on behalf of SteveSandersonMS May 21, 2026 14:50 View session

Apply gofmt

341a8ea

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI reviewed May 21, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Go SDK API review fixes#1360

Go SDK API review fixes#1360
SteveSandersonMS wants to merge 9 commits into
mainfrom
stevesandersonms/go-sdk-api-review

SteveSandersonMS commented May 21, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

Copilot AI left a comment

Uh oh!

github-actions Bot commented May 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

SteveSandersonMS commented May 21, 2026

Cross-language parity note for reviewers

Phases

Items explicitly NOT in this PR

Already correct (no change needed)

Uh oh!

This comment has been minimized.

This comment has been minimized.

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

github-actions Bot commented May 21, 2026

Cross-SDK Consistency Review ✅

Consistent across Go, TypeScript, and C#

Language-idiomatic differences (not issues)

Acknowledged gaps (not in scope for this PR)

One pre-existing minor inconsistency (not introduced by this PR)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants