Support optional connection token for TCP servers

[SteveSandersonMS]

Adds opt-in support for connecting to a Copilot CLI TCP server that requires a shared connection token.

Behaviour

• New CopilotClientOptions.tcpConnectionToken. Only meaningful in TCP mode (useStdio: false); passing it together with useStdio: true throws.
• When the SDK spawns its own CLI subprocess in TCP mode, it now auto-generates a UUID connection token by default and forwards it to the subprocess via the COPILOT_CONNECTION_TOKEN environment variable. Callers who set tcpConnectionToken explicitly override that.
• During connection setup the SDK calls a new connect JSON-RPC method to authenticate. If the server returns MethodNotFound (older runtime without the handler) the SDK falls back to the existing ping-based protocol-version check, so this is fully backwards compatible.

Tests

test/e2e/connection_token.test.ts — four end-to-end tests against a spawned CLI:

• explicit token round-trips successfully
• auto-generated UUID round-trips successfully
• a sibling client connecting with the wrong token is rejected
• a sibling client connecting with no token is rejected

Manual verification: a client built with these changes can still ping a legacy CLI server that has no connect handler.

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

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[github-actions]

Generated by SDK Consistency Review Agent for issue #1176 · ● 1.5M

[github-actions]

Minor cross-SDK inconsistency: The or uuid.uuid4().hex expression silently replaces an empty-string tcp_connection_token with an auto-generated UUID (Python's falsy "" semantics). Node.js and .NET both raise an explicit error for empty strings:

• Node.js: throw new Error("tcpConnectionToken must be a non-empty string")
• .NET: throw new ArgumentException("TcpConnectionToken must be a non-empty string")

To stay consistent, consider adding an explicit guard before this line:

if config.tcp_connection_token is not None and config.use_stdio:
    raise ValueError("tcp_connection_token cannot be used with use_stdio=True")
if config.tcp_connection_token == "":
    raise ValueError("tcp_connection_token must be a non-empty string")

This is an edge case (nobody should pass ""), but worth aligning for predictable cross-SDK behaviour.

[github-actions]

Generated by SDK Consistency Review Agent for issue #1176 · ● 706.2K

[github-actions]

Cross-SDK consistency: The auto-generated token uses uuid.uuid4().hex which produces a compact 32-character hex string without dashes (e.g. a3f8b7d2e4c6f8a0b2d4e6f8a0b2d4e6), while the other three SDKs all produce standard UUID format with dashes:

• Node.js: randomUUID() → a3f8b7d2-e4c6-f8a0-b2d4-e6f8a0b2d4e6
• Go: uuid.NewString() → a3f8b7d2-e4c6-f8a0-b2d4-e6f8a0b2d4e6
• .NET: Guid.NewGuid().ToString() → a3f8b7d2-e4c6-f8a0-b2d4-e6f8a0b2d4e6

This is also inconsistent with how the same file generates session IDs on line 1457, which correctly uses str(uuid.uuid4()).

Suggestion: replace uuid.uuid4().hex with str(uuid.uuid4()) for consistency across all SDKs.

[github-actions]

Generated by SDK Consistency Review Agent for issue #1176 · ● 1.2M

[github-actions]

🐛 Bug: wrong RPC class used — ServerRpc has no connect method

ServerRpc only has ping (and sub-APIs for models, tools, etc.). The newly generated InternalServerRpc is the class that exposes connect. As written, every call to start() will raise:

AttributeError: 'ServerRpc' object has no attribute 'connect'

making the Python SDK completely non-functional after this PR.

Fix — two changes needed:

1. Add InternalServerRpc to the import in the from .generated.rpc import (...) block (around line 35):

from .generated.rpc import (
    ClientSessionApiHandlers,
    ConnectRequest,
    InternalServerRpc,   # ← add this
    ServerRpc,
    register_client_session_api_handlers,
)

2. Replace ServerRpc with InternalServerRpc here:

connect_result = await InternalServerRpc(self._client).connect(
    ConnectRequest(token=self._effective_connection_token)
)

The pattern mirrors the other SDKs:

• Node.js uses createInternalServerRpc(this.connection) for the handshake
• Go uses NewInternalServerRpc(...)
• .NET calls InvokeRpcAsync<ConnectResult>(connection.Rpc, "connect", ...)

[github-actions]

Generated by SDK Consistency Review Agent for issue #1176 · ● 1M

[github-actions]

Cross-SDK consistency: UUID format and empty-string validation

Two small inconsistencies compared with the other SDK implementations:

1. Token format — uuid.uuid4().hex produces a 32-character lowercase hex string with no dashes (e.g. "a3b4c5d6e7f8..."). Every other SDK generates a standard UUID string with dashes:

SDK	Expression	Format
Node.js	randomUUID()	xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
Go	uuid.NewString()	xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
.NET	Guid.NewGuid().ToString()	xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
Python	uuid.uuid4().hex	xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx

If the CLI server ever validates token format (or if users compare auto-generated tokens across SDKs), this discrepancy can cause confusion or failures. Consider using str(uuid.uuid4()) to stay consistent.

2. Empty-string validation — because of the or short-circuit, passing tcp_connection_token="" will silently fall through to auto-generation rather than raising a ValueError. Both Node.js and .NET reject an explicitly supplied empty string with an error:

# Node.js equivalent check:
if options.tcpConnectionToken is not None and options.tcpConnectionToken.length === 0:
    throw new Error("tcpConnectionToken must be a non-empty string")

# .NET equivalent:
if TcpConnectionToken.Length == 0:
    raise ArgumentException("TcpConnectionToken must be a non-empty string")

Suggested fix covering both points:

token = config.tcp_connection_token
if token is not None:
    if token == "":
        raise ValueError("tcp_connection_token must be a non-empty string")
    self._effective_connection_token = token
else:
    self._effective_connection_token = str(uuid.uuid4())

[github-actions]

Generated by SDK Consistency Review Agent for issue #1176 · ● 749.9K

[github-actions]

Cross-SDK consistency: empty-string tcp_connection_token is handled differently here than in the Node.js and .NET SDKs.

Node.js and .NET both explicitly reject an empty-string token at construction time:

// Node.js
if (typeof options.tcpConnectionToken !== "string" || options.tcpConnectionToken.length === 0) {
    throw new Error("tcpConnectionToken must be a non-empty string");
}

// .NET
if (_options.TcpConnectionToken.Length == 0)
    throw new ArgumentException("TcpConnectionToken must be a non-empty string");

Python has two divergent behaviours:

1. ExternalServerConfig path (line 900): self._effective_connection_token = config.tcp_connection_token — an empty string is assigned verbatim and then sent as the token field in the connect RPC, which the server would likely reject with AUTHENTICATION_FAILED at runtime rather than at construction time.

2. SubprocessConfig path (line 909): config.tcp_connection_token or uuid.uuid4().hex — an empty string silently falls back to an auto-generated UUID, which is a quiet substitution rather than a clear error.

Suggestion: add a shared guard before the isinstance branch so both paths behave consistently with the other SDKs:

if config.tcp_connection_token is not None and len(config.tcp_connection_token) == 0:
    raise ValueError("tcp_connection_token must be a non-empty string")

(Go uses the idiomatic empty-string-as-absent pattern, so there's no mismatch there — but aligning Python with Node.js and .NET would be a small improvement.)

[github-actions]

Generated by SDK Consistency Review Agent for issue #1176 · ● 609K

[github-actions]

Generated by SDK Consistency Review Agent for issue #1176 · ● 913.5K

[SteveSandersonMS]

Won't fix — the empty/generic catch is intentional best-effort cleanup after a deliberately-failed StartAsync. Catching narrower types (e.g. InvalidOperationException) risks letting unrelated failures fail the test on the cleanup path. Added an explanatory comment in commit 4f681cd.

[SteveSandersonMS]

Won't fix — see reply on the parallel comment for line 80. Best-effort cleanup; intentional broad catch with explanatory comment.

[SteveSandersonMS]

Won't fix — same reasoning as the duplicate comments above. The catch is intentionally broad for best-effort cleanup; comment in commit 4f681cd explains why.

[github-actions]

Cross-SDK Consistency Review ✅

This PR adds TCP connection token support across all four SDKs simultaneously. The implementation is well-coordinated and highly consistent. Here's the breakdown:

✅ What's consistent across all SDKs

Aspect	Node.js	Python	Go	.NET
Option name	tcpConnectionToken	tcp_connection_token	TCPConnectionToken	TcpConnectionToken
Auto-generate token in TCP mode	✅ UUID	✅ UUID	✅ UUID	✅ GUID
connect handshake with fallback to ping	✅	✅	✅	✅
COPILOT_CONNECTION_TOKEN env var forwarded to CLI	✅	✅	✅	✅
Token in ExternalServerConfig/external URL mode	✅	✅	✅	✅
4 parallel E2E test scenarios	✅	✅	✅	✅
AUTHENTICATION_FAILED error on wrong/missing token	✅	✅	✅	✅

All naming conventions are idiomatic for each language. The feature parity is complete.

⚠️ Minor validation nuance worth noting

There's a small behavioural difference in how each SDK validates the "token combined with stdio mode" case:

• Python (SubprocessConfig): use_stdio defaults to True (a concrete default), so the check if config.tcp_connection_token is not None and config.use_stdio fires immediately if you set tcp_connection_token without explicitly setting use_stdio=False. This proactively prevents the silent no-op.

• Node.js / Go / .NET: useStdio/UseStdio defaults to null/nil/undefined, and validation only fires when explicitly set to true. Setting a token without explicitly opting into TCP mode is silently permitted (though the token would be sent via connect and likely just fall through to ping on a non-token-aware server).

Python's behaviour is arguably more correct here — if a caller provides a token, they almost certainly intend TCP mode, and an early error is more helpful than a silent no-op. It may be worth bringing the other three SDKs into alignment by also raising when the effective transport mode is stdio (not just when useStdio is explicitly true). That said, this is a minor UX nuance rather than a functional bug — the token would simply be unused when the SDK defaults to stdio, causing no security issue.

Summary

This is a thorough and well-implemented cross-SDK feature. No blocking consistency issues found. The optional follow-up above is a suggestion, not a requirement.

Generated by SDK Consistency Review Agent for issue #1176 · ● 707.4K · ◷