diff --git a/.github/agents/docs-maintenance.agent.md b/.github/agents/docs-maintenance.agent.md index 9b97fecf4..c5363e369 100644 --- a/.github/agents/docs-maintenance.agent.md +++ b/.github/agents/docs-maintenance.agent.md @@ -122,7 +122,6 @@ Every major SDK feature should be documented. Core features include: - Client initialization and configuration - Connection modes (stdio vs TCP) - Authentication options -- Auto-start and auto-restart behavior **Session Management:** - Creating sessions @@ -342,7 +341,7 @@ cat nodejs/src/types.ts | grep -A 10 "export interface ExportSessionOptions" ``` **Must match:** -- `CopilotClient` constructor options: `cliPath`, `cliUrl`, `useStdio`, `port`, `logLevel`, `autoStart`, `autoRestart`, `env`, `githubToken`, `useLoggedInUser` +- `CopilotClient` constructor options: `cliPath`, `cliUrl`, `useStdio`, `port`, `logLevel`, `autoStart`, `env`, `githubToken`, `useLoggedInUser` - `createSession()` config: `model`, `tools`, `hooks`, `systemMessage`, `mcpServers`, `availableTools`, `excludedTools`, `streaming`, `reasoningEffort`, `provider`, `infiniteSessions`, `customAgents`, `workingDirectory` - `CopilotSession` methods: `send()`, `sendAndWait()`, `getMessages()`, `disconnect()`, `abort()`, `on()`, `once()`, `off()` - Hook names: `onPreToolUse`, `onPostToolUse`, `onUserPromptSubmitted`, `onSessionStart`, `onSessionEnd`, `onErrorOccurred` @@ -360,7 +359,7 @@ cat python/copilot/types.py | grep -A 15 "class SessionHooks" ``` **Must match (snake_case):** -- `CopilotClient` options: `cli_path`, `cli_url`, `use_stdio`, `port`, `log_level`, `auto_start`, `auto_restart`, `env`, `github_token`, `use_logged_in_user` +- `CopilotClient` options: `cli_path`, `cli_url`, `use_stdio`, `port`, `log_level`, `auto_start`, `env`, `github_token`, `use_logged_in_user` - `create_session()` config keys: `model`, `tools`, `hooks`, `system_message`, `mcp_servers`, `available_tools`, `excluded_tools`, `streaming`, `reasoning_effort`, `provider`, `infinite_sessions`, `custom_agents`, `working_directory` - `CopilotSession` methods: `send()`, `send_and_wait()`, `get_messages()`, `disconnect()`, `abort()`, `export_session()` - Hook names: `on_pre_tool_use`, `on_post_tool_use`, `on_user_prompt_submitted`, `on_session_start`, `on_session_end`, `on_error_occurred` @@ -378,7 +377,7 @@ cat go/types.go | grep -A 15 "type SessionHooks struct" ``` **Must match (PascalCase for exported):** -- `ClientOptions` fields: `CLIPath`, `CLIUrl`, `UseStdio`, `Port`, `LogLevel`, `AutoStart`, `AutoRestart`, `Env`, `GithubToken`, `UseLoggedInUser` +- `ClientOptions` fields: `CLIPath`, `CLIUrl`, `UseStdio`, `Port`, `LogLevel`, `AutoStart`, `Env`, `GithubToken`, `UseLoggedInUser` - `SessionConfig` fields: `Model`, `Tools`, `Hooks`, `SystemMessage`, `MCPServers`, `AvailableTools`, `ExcludedTools`, `Streaming`, `ReasoningEffort`, `Provider`, `InfiniteSessions`, `CustomAgents`, `WorkingDirectory` - `Session` methods: `Send()`, `SendAndWait()`, `GetMessages()`, `Disconnect()`, `Abort()`, `ExportSession()` - Hook fields: `OnPreToolUse`, `OnPostToolUse`, `OnUserPromptSubmitted`, `OnSessionStart`, `OnSessionEnd`, `OnErrorOccurred` @@ -396,7 +395,7 @@ cat dotnet/src/Types.cs | grep -A 15 "public class SessionHooks" ``` **Must match (PascalCase):** -- `CopilotClientOptions` properties: `CliPath`, `CliUrl`, `UseStdio`, `Port`, `LogLevel`, `AutoStart`, `AutoRestart`, `Environment`, `GithubToken`, `UseLoggedInUser` +- `CopilotClientOptions` properties: `CliPath`, `CliUrl`, `UseStdio`, `Port`, `LogLevel`, `AutoStart`, `Environment`, `GithubToken`, `UseLoggedInUser` - `SessionConfig` properties: `Model`, `Tools`, `Hooks`, `SystemMessage`, `McpServers`, `AvailableTools`, `ExcludedTools`, `Streaming`, `ReasoningEffort`, `Provider`, `InfiniteSessions`, `CustomAgents`, `WorkingDirectory` - `CopilotSession` methods: `SendAsync()`, `SendAndWaitAsync()`, `GetMessagesAsync()`, `DisposeAsync()`, `AbortAsync()`, `ExportSessionAsync()` - Hook properties: `OnPreToolUse`, `OnPostToolUse`, `OnUserPromptSubmitted`, `OnSessionStart`, `OnSessionEnd`, `OnErrorOccurred` diff --git a/docs/setup/local-cli.md b/docs/setup/local-cli.md index 188c511d4..c9074af67 100644 --- a/docs/setup/local-cli.md +++ b/docs/setup/local-cli.md @@ -171,9 +171,6 @@ const client = new CopilotClient({ // Set working directory cwd: "/path/to/project", - - // Auto-restart CLI if it crashes (default: true) - autoRestart: true, }); ``` diff --git a/docs/troubleshooting/debugging.md b/docs/troubleshooting/debugging.md index 4bb261621..146d3fd5a 100644 --- a/docs/troubleshooting/debugging.md +++ b/docs/troubleshooting/debugging.md @@ -297,14 +297,7 @@ var client = new CopilotClient(new CopilotClientOptions copilot --server --stdio ``` -2. Enable auto-restart (enabled by default): - ```typescript - const client = new CopilotClient({ - autoRestart: true, - }); - ``` - -3. Check for port conflicts if using TCP mode: +2. Check for port conflicts if using TCP mode: ```typescript const client = new CopilotClient({ useStdio: false, diff --git a/dotnet/README.md b/dotnet/README.md index bdb3e8dab..132113038 100644 --- a/dotnet/README.md +++ b/dotnet/README.md @@ -73,7 +73,6 @@ new CopilotClient(CopilotClientOptions? options = null) - `UseStdio` - Use stdio transport instead of TCP (default: true) - `LogLevel` - Log level (default: "info") - `AutoStart` - Auto-start server (default: true) -- `AutoRestart` - Auto-restart on crash (default: true) - `Cwd` - Working directory for the CLI process - `Environment` - Environment variables to pass to the CLI process - `Logger` - `ILogger` instance for SDK logging diff --git a/dotnet/src/Client.cs b/dotnet/src/Client.cs index 5b7474a64..90d04051b 100644 --- a/dotnet/src/Client.cs +++ b/dotnet/src/Client.cs @@ -63,6 +63,7 @@ public sealed partial class CopilotClient : IDisposable, IAsyncDisposable private readonly CopilotClientOptions _options; private readonly ILogger _logger; private Task? _connectionTask; + private volatile bool _disconnected; private bool _disposed; private readonly int? _optionsPort; private readonly string? _optionsHost; @@ -199,6 +200,7 @@ public Task StartAsync(CancellationToken cancellationToken = default) async Task StartCoreAsync(CancellationToken ct) { _logger.LogDebug("Starting Copilot client"); + _disconnected = false; Task result; @@ -590,6 +592,7 @@ public ConnectionState State if (_connectionTask == null) return ConnectionState.Disconnected; if (_connectionTask.IsFaulted) return ConnectionState.Error; if (!_connectionTask.IsCompleted) return ConnectionState.Connecting; + if (_disconnected) return ConnectionState.Disconnected; return ConnectionState.Connected; } } @@ -1198,6 +1201,9 @@ private async Task ConnectToServerAsync(Process? cliProcess, string? rpc.AddLocalRpcMethod("hooks.invoke", handler.OnHooksInvoke); rpc.StartListening(); + // Transition state to Disconnected if the JSON-RPC connection drops + _ = rpc.Completion.ContinueWith(_ => _disconnected = true, TaskScheduler.Default); + _rpc = new ServerRpc(rpc); return new Connection(rpc, cliProcess, tcpClient, networkStream, stderrBuffer); diff --git a/dotnet/src/Types.cs b/dotnet/src/Types.cs index 633a97654..4ebad524e 100644 --- a/dotnet/src/Types.cs +++ b/dotnet/src/Types.cs @@ -50,8 +50,10 @@ protected CopilotClientOptions(CopilotClientOptions? other) { if (other is null) return; - AutoRestart = other.AutoRestart; AutoStart = other.AutoStart; +#pragma warning disable CS0618 // Obsolete member + AutoRestart = other.AutoRestart; +#pragma warning restore CS0618 CliArgs = (string[]?)other.CliArgs?.Clone(); CliPath = other.CliPath; CliUrl = other.CliUrl; @@ -99,9 +101,10 @@ protected CopilotClientOptions(CopilotClientOptions? other) /// public bool AutoStart { get; set; } = true; /// - /// Whether to automatically restart the CLI server if it exits unexpectedly. + /// Obsolete. This option has no effect. /// - public bool AutoRestart { get; set; } = true; + [Obsolete("AutoRestart has no effect and will be removed in a future release.")] + public bool AutoRestart { get; set; } /// /// Environment variables to pass to the CLI process. /// diff --git a/dotnet/test/CloneTests.cs b/dotnet/test/CloneTests.cs index cc6e5ad56..a0051ffbc 100644 --- a/dotnet/test/CloneTests.cs +++ b/dotnet/test/CloneTests.cs @@ -22,7 +22,7 @@ public void CopilotClientOptions_Clone_CopiesAllProperties() CliUrl = "http://localhost:8080", LogLevel = "debug", AutoStart = false, - AutoRestart = false, + Environment = new Dictionary { ["KEY"] = "value" }, GitHubToken = "ghp_test", UseLoggedInUser = false, @@ -38,7 +38,7 @@ public void CopilotClientOptions_Clone_CopiesAllProperties() Assert.Equal(original.CliUrl, clone.CliUrl); Assert.Equal(original.LogLevel, clone.LogLevel); Assert.Equal(original.AutoStart, clone.AutoStart); - Assert.Equal(original.AutoRestart, clone.AutoRestart); + Assert.Equal(original.Environment, clone.Environment); Assert.Equal(original.GitHubToken, clone.GitHubToken); Assert.Equal(original.UseLoggedInUser, clone.UseLoggedInUser); diff --git a/go/README.md b/go/README.md index 4cc73398c..a13cbbacc 100644 --- a/go/README.md +++ b/go/README.md @@ -138,7 +138,6 @@ Event types: `SessionLifecycleCreated`, `SessionLifecycleDeleted`, `SessionLifec - `UseStdio` (bool): Use stdio transport instead of TCP (default: true) - `LogLevel` (string): Log level (default: "info") - `AutoStart` (\*bool): Auto-start server on first use (default: true). Use `Bool(false)` to disable. -- `AutoRestart` (\*bool): Auto-restart on crash (default: true). Use `Bool(false)` to disable. - `Env` ([]string): Environment variables for CLI process (default: inherits from current process) - `GitHubToken` (string): GitHub token for authentication. When provided, takes priority over other auth methods. - `UseLoggedInUser` (\*bool): Whether to use logged-in user for authentication (default: true, but false when `GitHubToken` is provided). Cannot be used with `CLIUrl`. @@ -174,7 +173,7 @@ Event types: `SessionLifecycleCreated`, `SessionLifecycleDeleted`, `SessionLifec ### Helper Functions -- `Bool(v bool) *bool` - Helper to create bool pointers for `AutoStart`/`AutoRestart` options +- `Bool(v bool) *bool` - Helper to create bool pointers for `AutoStart` option ## Image Support diff --git a/go/client.go b/go/client.go index 021de2b14..df498be31 100644 --- a/go/client.go +++ b/go/client.go @@ -71,19 +71,19 @@ import ( // } // defer client.Stop() type Client struct { - options ClientOptions - process *exec.Cmd - client *jsonrpc2.Client - actualPort int - actualHost string - state ConnectionState - sessions map[string]*Session - sessionsMux sync.Mutex - isExternalServer bool - conn net.Conn // stores net.Conn for external TCP connections - useStdio bool // resolved value from options - autoStart bool // resolved value from options - autoRestart bool // resolved value from options + options ClientOptions + process *exec.Cmd + client *jsonrpc2.Client + actualPort int + actualHost string + state ConnectionState + sessions map[string]*Session + sessionsMux sync.Mutex + isExternalServer bool + conn net.Conn // stores net.Conn for external TCP connections + useStdio bool // resolved value from options + autoStart bool // resolved value from options + modelsCache []ModelInfo modelsCacheMux sync.Mutex lifecycleHandlers []SessionLifecycleHandler @@ -132,7 +132,6 @@ func NewClient(options *ClientOptions) *Client { isExternalServer: false, useStdio: true, autoStart: true, // default - autoRestart: true, // default } if options != nil { @@ -182,9 +181,6 @@ func NewClient(options *ClientOptions) *Client { if options.AutoStart != nil { client.autoStart = *options.AutoStart } - if options.AutoRestart != nil { - client.autoRestart = *options.AutoRestart - } if options.GitHubToken != "" { opts.GitHubToken = options.GitHubToken } @@ -1231,6 +1227,15 @@ func (c *Client) startCLIServer(ctx context.Context) error { // Create JSON-RPC client immediately c.client = jsonrpc2.NewClient(stdin, stdout) c.client.SetProcessDone(c.processDone, c.processErrorPtr) + c.client.SetOnClose(func() { + // Run in a goroutine to avoid deadlocking with Stop/ForceStop, + // which hold startStopMux while waiting for readLoop to finish. + go func() { + c.startStopMux.Lock() + defer c.startStopMux.Unlock() + c.state = StateDisconnected + }() + }) c.RPC = rpc.NewServerRpc(c.client) c.setupNotificationHandler() c.client.Start() @@ -1346,6 +1351,13 @@ func (c *Client) connectViaTcp(ctx context.Context) error { if c.processDone != nil { c.client.SetProcessDone(c.processDone, c.processErrorPtr) } + c.client.SetOnClose(func() { + go func() { + c.startStopMux.Lock() + defer c.startStopMux.Unlock() + c.state = StateDisconnected + }() + }) c.RPC = rpc.NewServerRpc(c.client) c.setupNotificationHandler() c.client.Start() diff --git a/go/internal/jsonrpc2/jsonrpc2.go b/go/internal/jsonrpc2/jsonrpc2.go index 09505c06d..827a15cb4 100644 --- a/go/internal/jsonrpc2/jsonrpc2.go +++ b/go/internal/jsonrpc2/jsonrpc2.go @@ -61,6 +61,7 @@ type Client struct { processDone chan struct{} // closed when the underlying process exits processError error // set before processDone is closed processErrorMu sync.RWMutex // protects processError + onClose func() // called when the read loop exits unexpectedly } // NewClient creates a new JSON-RPC client @@ -293,9 +294,22 @@ func (c *Client) sendMessage(message any) error { return nil } +// SetOnClose sets a callback invoked when the read loop exits unexpectedly +// (e.g. the underlying connection or process was lost). +func (c *Client) SetOnClose(fn func()) { + c.onClose = fn +} + // readLoop reads messages from stdout in a background goroutine func (c *Client) readLoop() { defer c.wg.Done() + defer func() { + // If still running, the read loop exited unexpectedly (process died or + // connection dropped). Notify the caller so it can update its state. + if c.onClose != nil && c.running.Load() { + c.onClose() + } + }() reader := bufio.NewReader(c.stdout) diff --git a/go/internal/jsonrpc2/jsonrpc2_test.go b/go/internal/jsonrpc2/jsonrpc2_test.go new file mode 100644 index 000000000..9f542049d --- /dev/null +++ b/go/internal/jsonrpc2/jsonrpc2_test.go @@ -0,0 +1,69 @@ +package jsonrpc2 + +import ( + "io" + "sync" + "testing" + "time" +) + +func TestOnCloseCalledOnUnexpectedExit(t *testing.T) { + stdinR, stdinW := io.Pipe() + stdoutR, stdoutW := io.Pipe() + defer stdinR.Close() + + client := NewClient(stdinW, stdoutR) + + var called bool + var mu sync.Mutex + client.SetOnClose(func() { + mu.Lock() + called = true + mu.Unlock() + }) + + client.Start() + + // Simulate unexpected process death by closing the stdout writer + stdoutW.Close() + + // Wait for readLoop to detect the close and invoke the callback + time.Sleep(200 * time.Millisecond) + + mu.Lock() + defer mu.Unlock() + if !called { + t.Error("expected onClose to be called when read loop exits unexpectedly") + } +} + +func TestOnCloseNotCalledOnIntentionalStop(t *testing.T) { + stdinR, stdinW := io.Pipe() + stdoutR, stdoutW := io.Pipe() + defer stdinR.Close() + defer stdoutW.Close() + + client := NewClient(stdinW, stdoutR) + + var called bool + var mu sync.Mutex + client.SetOnClose(func() { + mu.Lock() + called = true + mu.Unlock() + }) + + client.Start() + + // Intentional stop — should set running=false before closing stdout, + // so the readLoop should NOT invoke onClose. + client.Stop() + + time.Sleep(200 * time.Millisecond) + + mu.Lock() + defer mu.Unlock() + if called { + t.Error("onClose should not be called on intentional Stop()") + } +} diff --git a/go/types.go b/go/types.go index a139f294f..bf02e0eb8 100644 --- a/go/types.go +++ b/go/types.go @@ -38,8 +38,7 @@ type ClientOptions struct { // AutoStart automatically starts the CLI server on first use (default: true). // Use Bool(false) to disable. AutoStart *bool - // AutoRestart automatically restarts the CLI server if it crashes (default: true). - // Use Bool(false) to disable. + // Deprecated: AutoRestart has no effect and will be removed in a future release. AutoRestart *bool // Env is the environment variables for the CLI process (default: inherits from current process). // Each entry is of the form "key=value". @@ -65,7 +64,7 @@ type ClientOptions struct { } // Bool returns a pointer to the given bool value. -// Use for setting AutoStart or AutoRestart: AutoStart: Bool(false) +// Use for setting AutoStart: AutoStart: Bool(false) func Bool(v bool) *bool { return &v } diff --git a/nodejs/README.md b/nodejs/README.md index 78a535b76..13169e7b7 100644 --- a/nodejs/README.md +++ b/nodejs/README.md @@ -82,7 +82,6 @@ new CopilotClient(options?: CopilotClientOptions) - `useStdio?: boolean` - Use stdio transport instead of TCP (default: true) - `logLevel?: string` - Log level (default: "info") - `autoStart?: boolean` - Auto-start server (default: true) -- `autoRestart?: boolean` - Auto-restart on crash (default: true) - `githubToken?: string` - GitHub token for authentication. When provided, takes priority over other auth methods. - `useLoggedInUser?: boolean` - Whether to use logged-in user for authentication (default: true, but false when `githubToken` is provided). Cannot be used with `cliUrl`. diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index 954d88b59..31f24d645 100644 --- a/nodejs/src/client.ts +++ b/nodejs/src/client.ts @@ -243,7 +243,8 @@ export class CopilotClient { cliUrl: options.cliUrl, logLevel: options.logLevel || "debug", autoStart: options.autoStart ?? true, - autoRestart: options.autoRestart ?? true, + autoRestart: false, + env: options.env ?? process.env, githubToken: options.githubToken, // Default useLoggedInUser to false when githubToken is provided, otherwise true @@ -1259,8 +1260,6 @@ export class CopilotClient { } else { reject(new Error(`CLI server exited with code ${code}`)); } - } else if (this.options.autoRestart && this.state === "connected") { - void this.reconnect(); } }); @@ -1412,13 +1411,11 @@ export class CopilotClient { ); this.connection.onClose(() => { - if (this.state === "connected" && this.options.autoRestart) { - void this.reconnect(); - } + this.state = "disconnected"; }); this.connection.onError((_error) => { - // Connection errors are handled via autoRestart if enabled + this.state = "disconnected"; }); } @@ -1644,17 +1641,4 @@ export class CopilotClient { "resultType" in value ); } - - /** - * Attempt to reconnect to the server - */ - private async reconnect(): Promise { - this.state = "disconnected"; - try { - await this.stop(); - await this.start(); - } catch (_error) { - // Reconnection failed - } - } } diff --git a/nodejs/src/types.ts b/nodejs/src/types.ts index 99b9af75c..8b1c8ec01 100644 --- a/nodejs/src/types.ts +++ b/nodejs/src/types.ts @@ -72,8 +72,7 @@ export interface CopilotClientOptions { autoStart?: boolean; /** - * Auto-restart the CLI server if it crashes - * @default true + * @deprecated This option has no effect and will be removed in a future release. */ autoRestart?: boolean; diff --git a/nodejs/test/client.test.ts b/nodejs/test/client.test.ts index 7206c903b..d656b95e3 100644 --- a/nodejs/test/client.test.ts +++ b/nodejs/test/client.test.ts @@ -484,4 +484,23 @@ describe("CopilotClient", () => { expect(models).toEqual(customModels); }); }); + + describe("unexpected disconnection", () => { + it("transitions to disconnected when child process is killed", async () => { + const client = new CopilotClient(); + await client.start(); + onTestFinished(() => client.forceStop()); + + expect(client.getState()).toBe("connected"); + + // Kill the child process to simulate unexpected termination + const proc = (client as any).cliProcess as import("node:child_process").ChildProcess; + proc.kill(); + + // Wait for the connection.onClose handler to fire + await vi.waitFor(() => { + expect(client.getState()).toBe("disconnected"); + }); + }); + }); }); diff --git a/python/README.md b/python/README.md index 5b87bb04e..a585ea114 100644 --- a/python/README.md +++ b/python/README.md @@ -84,7 +84,6 @@ client = CopilotClient({ "cli_url": None, # Optional: URL of existing server (e.g., "localhost:8080") "log_level": "info", # Optional: log level (default: "info") "auto_start": True, # Optional: auto-start server (default: True) - "auto_restart": True, # Optional: auto-restart on crash (default: True) }) await client.start() @@ -111,7 +110,6 @@ await client.stop() - `use_stdio` (bool): Use stdio transport instead of TCP (default: True) - `log_level` (str): Log level (default: "info") - `auto_start` (bool): Auto-start server on first use (default: True) -- `auto_restart` (bool): Auto-restart on crash (default: True) - `github_token` (str): GitHub token for authentication. When provided, takes priority over other auth methods. - `use_logged_in_user` (bool): Whether to use logged-in user for authentication (default: True, but False when `github_token` is provided). Cannot be used with `cli_url`. diff --git a/python/copilot/client.py b/python/copilot/client.py index df09a755b..ada36bb16 100644 --- a/python/copilot/client.py +++ b/python/copilot/client.py @@ -189,7 +189,6 @@ def __init__(self, options: CopilotClientOptions | None = None): "use_stdio": False if opts.get("cli_url") else opts.get("use_stdio", True), "log_level": opts.get("log_level", "info"), "auto_start": opts.get("auto_start", True), - "auto_restart": opts.get("auto_restart", True), "use_logged_in_user": use_logged_in_user, } if opts.get("cli_args"): @@ -1406,6 +1405,7 @@ async def _connect_via_stdio(self) -> None: # Create JSON-RPC client with the process self._client = JsonRpcClient(self._process) + self._client.on_close = lambda: setattr(self, "_state", "disconnected") self._rpc = ServerRpc(self._client) # Set up notification handler for session events @@ -1493,6 +1493,7 @@ def wait(self, timeout=None): self._process = SocketWrapper(sock_file, sock) # type: ignore self._client = JsonRpcClient(self._process) + self._client.on_close = lambda: setattr(self, "_state", "disconnected") self._rpc = ServerRpc(self._client) # Set up notification handler for session events diff --git a/python/copilot/jsonrpc.py b/python/copilot/jsonrpc.py index fc8255274..287f1b965 100644 --- a/python/copilot/jsonrpc.py +++ b/python/copilot/jsonrpc.py @@ -60,6 +60,7 @@ def __init__(self, process): self._process_exit_error: str | None = None self._stderr_output: list[str] = [] self._stderr_lock = threading.Lock() + self.on_close: Callable[[], None] | None = None def start(self, loop: asyncio.AbstractEventLoop | None = None): """Start listening for messages in background thread""" @@ -211,6 +212,8 @@ def _read_loop(self): # Process exited or read failed - fail all pending requests if self._running: self._fail_pending_requests() + if self.on_close is not None: + self.on_close() def _fail_pending_requests(self): """Fail all pending requests when process exits""" diff --git a/python/copilot/types.py b/python/copilot/types.py index 33764e5d1..d3840904f 100644 --- a/python/copilot/types.py +++ b/python/copilot/types.py @@ -86,8 +86,6 @@ class CopilotClientOptions(TypedDict, total=False): # Mutually exclusive with cli_path, use_stdio log_level: LogLevel # Log level auto_start: bool # Auto-start the CLI server on first use (default: True) - # Auto-restart the CLI server if it crashes (default: True) - auto_restart: bool env: dict[str, str] # Environment variables for the CLI process # GitHub token to use for authentication. # When provided, the token is passed to the CLI server via environment variable. diff --git a/python/test_jsonrpc.py b/python/test_jsonrpc.py index 2533fc8a7..7c3c8dab2 100644 --- a/python/test_jsonrpc.py +++ b/python/test_jsonrpc.py @@ -7,6 +7,9 @@ import io import json +import os +import threading +import time import pytest @@ -265,3 +268,62 @@ def test_read_message_multiple_messages_in_sequence(self): result2 = client._read_message() assert result2 == message2 + + +class ClosingStream: + """Stream that immediately returns empty bytes (simulates process death / EOF).""" + + def readline(self): + return b"" + + def read(self, n: int) -> bytes: + return b"" + + +class TestOnClose: + """Tests for the on_close callback when the read loop exits unexpectedly.""" + + def test_on_close_called_on_unexpected_exit(self): + """on_close fires when the stream closes while client is still running.""" + import asyncio + + process = MockProcess() + process.stdout = ClosingStream() + + client = JsonRpcClient(process) + + called = threading.Event() + client.on_close = lambda: called.set() + + loop = asyncio.new_event_loop() + try: + client.start(loop=loop) + assert called.wait(timeout=2), "on_close was not called within 2 seconds" + finally: + loop.close() + + def test_on_close_not_called_on_intentional_stop(self): + """on_close should not fire when stop() is called intentionally.""" + import asyncio + + r_fd, w_fd = os.pipe() + process = MockProcess() + process.stdout = os.fdopen(r_fd, "rb") + + client = JsonRpcClient(process) + + called = threading.Event() + client.on_close = lambda: called.set() + + loop = asyncio.new_event_loop() + try: + client.start(loop=loop) + + # Intentional stop sets _running = False before the thread sees EOF + loop.run_until_complete(client.stop()) + os.close(w_fd) + + time.sleep(0.5) + assert not called.is_set(), "on_close should not be called on intentional stop" + finally: + loop.close()