fix(providers): fix GitHub Copilot provider — broken session lifecycle and empty responses by saisankargochhayat · Pull Request #553 · sipeed/picoclaw

saisankargochhayat · 2026-02-20T21:03:01Z

🐛 Bug Fixes

The GitHub Copilot provider had two critical bugs causing it to always return empty responses.

Bug 1: defer client.Stop() kills the session immediately
NewGitHubCopilotProvider called defer client.Stop() inside the constructor — so by the time any Chat() call was made, the gRPC connection was already torn down. Every message silently failed.

Bug 2: session.Send() returns a UUID, not response text
Send() is async and only returns a message ID. The actual response text comes via events. SendAndWait() must be used instead — it blocks until the assistant is idle and returns the final SessionEvent containing .Data.Content.

Changes

Remove defer client.Stop() from constructor
Store client as a struct field (was a local variable with no cleanup path)
Replace session.Send() with session.SendAndWait() to get actual response text
Add proper error handling for CreateSession (was silently ignoring error with _)
Add Close() method for proper client lifecycle management
Add 15s connection timeout to fail fast if the Copilot CLI is unreachable

Type of Change

Bug fix (non-breaking change which fixes an issue)

AI Code Generation

Mostly AI-generated (AI draft, Human verified/modified)

Test Environment

OS: Windows 11
Model/Provider: GitHub Copilot / claude-sonnet-4.6
Channels: Discord

Copilot

Pull request overview

Fixes the GitHub Copilot LLM provider session lifecycle so it can successfully connect to the Copilot CLI, maintain a usable session, and return actual assistant content instead of empty responses.

Changes:

Persist the Copilot client on the provider struct (instead of a short-lived local), and remove the constructor-time defer client.Stop().
Add a 15s startup timeout and proper error handling around Start() / CreateSession().
Switch from async Send() to blocking SendAndWait() and return the resulting event content.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-20T21:05:40Z

pkg/providers/github_copilot_provider.go

+		connectCtx, connectCancel := context.WithTimeout(context.Background(), 15*time.Second)
+		defer connectCancel()
+		if err := client.Start(connectCtx); err != nil {
+			return nil, fmt.Errorf("can't connect to Github Copilot: %w", err)


The error message uses "Github" instead of the project’s consistent "GitHub" capitalization. Please update the string to "GitHub Copilot" (and keep wrapping the underlying error).

Suggested change

return nil, fmt.Errorf("can't connect to Github Copilot: %w", err)

return nil, fmt.Errorf("can't connect to GitHub Copilot: %w", err)

pkg/providers/github_copilot_provider.go

Copilot · 2026-02-20T21:05:41Z

pkg/providers/github_copilot_provider.go

+func (p *GitHubCopilotProvider) Close() {
+	if p.client != nil {
+		p.client.Stop()
+	}


Close() is added, but LLMProvider doesn’t include it and there are no call sites in the repo invoking (*GitHubCopilotProvider).Close(), so the Copilot client connection will never be stopped during normal operation. Consider making providers that own resources implement io.Closer (e.g., Close() error) and have the agent/provider factory/loop call it when shutting down.

Suggested change

func (p *GitHubCopilotProvider) Close() {

if p.client != nil {

p.client.Stop()

}

func (p *GitHubCopilotProvider) Close() error {

if p.client != nil {

p.client.Stop()

}

return nil

Copilot · 2026-02-20T21:05:41Z

pkg/providers/github_copilot_provider.go

 	fullcontent, _ := json.Marshal(out)

-	content, _ := p.session.Send(ctx, copilot.MessageOptions{
+	event, err := p.session.SendAndWait(ctx, copilot.MessageOptions{
 		Prompt: string(fullcontent),
 	})


json.Marshal(out) errors are ignored. If marshaling fails, fullcontent will be nil and the prompt becomes an empty string, which can lead to confusing downstream behavior. Please handle the marshal error and return it to the caller.

Copilot · 2026-02-20T21:05:41Z

pkg/providers/github_copilot_provider.go

@@ -65,13 +81,20 @@ func (p *GitHubCopilotProvider) Chat(ctx context.Context, messages []Message, to

 	fullcontent, _ := json.Marshal(out)



Chat() calls p.session.SendAndWait(...) without verifying p.session is non-nil. Given the constructor can currently return session == nil for some modes, add a guard (and a clear error) before using the session to avoid a nil-pointer panic.

Suggested change

if p.session == nil {

return nil, fmt.Errorf("copilot session is not initialized")

}

Copilot · 2026-02-20T21:05:42Z

pkg/providers/github_copilot_provider.go

+		return nil, fmt.Errorf("copilot error: %w", err)
+	}
+
+	if event == nil || event.Data.Content == nil {


The nil-check can panic: event.Data may be nil even when event is non-nil, so event.Data.Content will dereference a nil pointer. Please check event == nil || event.Data == nil || event.Data.Content == nil (or equivalent) before accessing .Content.

Suggested change

if event == nil || event.Data.Content == nil {

if event == nil || event.Data == nil || event.Data.Content == nil {

…ponse handling - Store client as struct field to prevent premature garbage collection - Remove defer client.Stop() from constructor which was killing the gRPC client immediately after NewGitHubCopilotProvider returned, causing all subsequent Chat() calls to silently fail - Add proper error handling for CreateSession (was ignoring error with _) - Replace session.Send() with session.SendAndWait(): Send() only returns a message UUID, not the response text; SendAndWait() blocks until the assistant is idle and returns the actual SessionEvent with content - Add Close() method for proper client lifecycle management - Add 15s connection timeout to fail fast if CLI is unreachable

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-20T21:28:03Z

pkg/providers/github_copilot_provider.go


 	case "stdio":
-		// todo
+		//todo


Comment formatting should follow Go conventions with "TODO:" (uppercase with colon) instead of "//todo". See pkg/providers/factory_test.go:292 and :296 for examples of the correct format.

Suggested change

//todo

// TODO:

Copilot · 2026-02-20T21:28:03Z

pkg/providers/github_copilot_provider.go

 }

 func NewGitHubCopilotProvider(uri string, connectMode string, model string) (*GitHubCopilotProvider, error) {
+


Extra blank line added. The function should follow the same formatting pattern as other providers in the codebase (e.g., claude_cli_provider.go:18-24, codex_cli_provider.go) where there's no blank line after the function signature.

Copilot · 2026-02-20T21:28:03Z

pkg/providers/github_copilot_provider.go

 }

 func (p *GitHubCopilotProvider) GetDefaultModel() string {
+


Extra blank line added. This should follow the same pattern as other GetDefaultModel methods in the codebase (e.g., claude_cli_provider.go:62-65) where there's no blank line before the return statement.

Suggested change

Copilot · 2026-02-20T21:28:04Z

pkg/providers/github_copilot_provider.go

-		Content:      content,
+		Content:      *event.Data.Content,
 	}, nil
+


Extra blank line added after the return statement. This should follow the same pattern as other provider Chat methods (e.g., claude_cli_provider.go:59-60) where there's no blank line after the return statement.

Suggested change

Copilot · 2026-02-20T21:28:04Z

pkg/providers/github_copilot_provider.go

+			client.Stop()
+			return nil, fmt.Errorf("failed to create Copilot session: %w", err)
+		}



The switch statement has no default case to handle invalid connectMode values. If a user provides an unsupported connectMode (anything other than "stdio" or "grpc"), the function will silently return a provider with nil client and session, causing a nil pointer dereference later. A default case should return an error for unsupported modes.

Suggested change

default:

return nil, fmt.Errorf("unsupported connect mode %q", connectMode)

Copilot · 2026-02-20T21:28:04Z

pkg/providers/github_copilot_provider.go

 	"fmt"
+	"time"
+
+	json "encoding/json"


The import alias json "encoding/json" is unconventional. Throughout the codebase, imports use standard naming without aliases. For example, in antigravity_provider.go, claude_cli_provider.go, and other providers, the import is simply "encoding/json" without an alias. This should follow the same convention for consistency.

Suggested change

json "encoding/json"

"encoding/json"

Copilot · 2026-02-20T21:28:04Z

pkg/providers/github_copilot_provider.go

 	case "stdio":
-		// todo
+		//todo


When connectMode is "stdio", both session and client will be nil, but the constructor returns successfully. This means any subsequent call to Chat() will panic with a nil pointer dereference when calling p.session.SendAndWait(). The "stdio" case should either be implemented or explicitly return an error indicating it's not yet supported.

Copilot · 2026-02-20T21:28:04Z

pkg/providers/github_copilot_provider.go

+func (p *GitHubCopilotProvider) Close() {
+	if p.client != nil {
+		p.client.Stop()
+	}
+}
+


The Close() method is added but never called anywhere in the codebase. The LLMProvider interface (pkg/providers/types.go:22-31) does not include a Close() method, so this won't be invoked automatically. Providers are created once at application startup in cmd/picoclaw/cmd_agent.go:62 and cmd/picoclaw/cmd_gateway.go:47 and are never cleaned up. This means the gRPC client connection will remain open until the process terminates. Either the Close() method should be integrated into the application lifecycle (e.g., with a shutdown handler), or it can be removed if it's not needed given that the provider lives for the entire application lifetime.

Suggested change

func (p *GitHubCopilotProvider) Close() {

if p.client != nil {

p.client.Stop()

}

}

- Remove json import alias (use standard encoding/json) - Fix capitalization: 'Github' -> 'GitHub' in error message - Fix //todo -> // TODO: comment formatting - stdio case now returns explicit error (not yet implemented) - Add default case returning error for unsupported connect modes - Close() now returns error to satisfy io.Closer interface - Add nil guard for p.session in Chat() - Handle json.Marshal error instead of ignoring it - Remove extra blank lines for consistency with codebase style

…ll sites

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-20T21:44:51Z

pkg/providers/github_copilot_provider.go

 type GitHubCopilotProvider struct {
 	uri         string
-	connectMode string // `stdio` or `grpc``
+	connectMode string // `stdio` or `grpc`

+	client  *copilot.Client
 	session *copilot.Session
 }


The PR description claims to add a Close() method for proper client lifecycle management, but this method is not present in the implementation. Without a Close() method, the gRPC client connection will remain open for the lifetime of the provider instance, creating a resource leak. While this may be acceptable if provider instances are long-lived and reused, it's inconsistent with the claimed changes in the PR description.

Copilot · 2026-02-20T21:44:51Z

pkg/providers/github_copilot_provider.go

-			)
+		connectCtx, connectCancel := context.WithTimeout(context.Background(), 15*time.Second)
+		defer connectCancel()
+		if err := client.Start(connectCtx); err != nil {


When client.Start() fails, the client is not properly cleaned up before returning the error. While it may not be strictly necessary to call client.Stop() on a client that failed to start, for consistency and to ensure proper cleanup of any partially initialized resources, consider calling client.Stop() in the error path at line 37.

Suggested change

if err := client.Start(connectCtx); err != nil {

if err := client.Start(connectCtx); err != nil {

client.Stop()

Copilot · 2026-02-20T21:44:52Z

pkg/providers/github_copilot_provider.go

+		var err error
+		session, err = client.CreateSession(connectCtx, &copilot.SessionConfig{


The same context (connectCtx) with a 15-second timeout is reused for both client.Start() and client.CreateSession(). If client.Start() takes close to 15 seconds, CreateSession() may fail immediately due to timeout. Consider using separate contexts with appropriate timeouts for each operation, or at minimum using context.Background() for CreateSession after Start succeeds.

Suggested change

var err error

session, err = client.CreateSession(connectCtx, &copilot.SessionConfig{

sessionCtx, sessionCancel := context.WithTimeout(context.Background(), 15*time.Second)

defer sessionCancel()

var err error

session, err = client.CreateSession(sessionCtx, &copilot.SessionConfig{

Copilot AI review requested due to automatic review settings February 20, 2026 21:03

Copilot started reviewing on behalf of saisankargochhayat February 20, 2026 21:03 View session

Copilot AI reviewed Feb 20, 2026

View reviewed changes

saisankargochhayat force-pushed the fix/github-copilot-provider branch from a69774f to 9bf6199 Compare February 20, 2026 21:07

docs: add GitHub Copilot Docker setup guide

c7da41d

Copilot AI review requested due to automatic review settings February 20, 2026 21:23

Copilot started reviewing on behalf of saisankargochhayat February 20, 2026 21:23 View session

Copilot AI reviewed Feb 20, 2026

View reviewed changes

Sai Sankar Gochhayat added 2 commits February 20, 2026 13:38

fix(providers): remove Close() not in LLMProvider interface and no ca…

a9b10ce

…ll sites

Copilot AI review requested due to automatic review settings February 20, 2026 21:40

Copilot started reviewing on behalf of saisankargochhayat February 20, 2026 21:40 View session

Copilot AI reviewed Feb 20, 2026

View reviewed changes

	return nil, fmt.Errorf("can't connect to Github Copilot: %w", err)
	return nil, fmt.Errorf("can't connect to GitHub Copilot: %w", err)

		@@ -65,13 +81,20 @@ func (p *GitHubCopilotProvider) Chat(ctx context.Context, messages []Message, to

		fullcontent, _ := json.Marshal(out)

+	if p.session == nil {
+		return nil, fmt.Errorf("copilot session is not initialized")
+	}

	if event == nil \|\| event.Data.Content == nil {
	if event == nil \|\| event.Data == nil \|\| event.Data.Content == nil {

		}

		func NewGitHubCopilotProvider(uri string, connectMode string, model string) (*GitHubCopilotProvider, error) {



	default:
	return nil, fmt.Errorf("unsupported connect mode %q", connectMode)

	if err := client.Start(connectCtx); err != nil {
	if err := client.Start(connectCtx); err != nil {
	client.Stop()

		var err error
		session, err = client.CreateSession(connectCtx, &copilot.SessionConfig{

Comments

Conversation

saisankargochhayat commented Feb 20, 2026

🐛 Bug Fixes

Changes

Type of Change

AI Code Generation

Test Environment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant