feat(providers): implement stdio mode for GitHub Copilot provider
Implement the previously stubbed stdio connect mode for the GitHub
Copilot provider, allowing picoclaw to automatically spawn and
communicate with the Copilot CLI subprocess via stdin/stdout.
Changes:
- github_copilot_provider.go: implement stdio mode using
copilot.ClientOptions{CLIPath}, sharing Start/CreateSession/Chat
flow with grpc mode
- factory_provider.go: skip default apiBase for stdio mode (SDK
uses 'copilot' from PATH)
- factory.go: skip default apiBase for stdio mode in legacy path
- github_copilot_provider_test.go: add 9 test cases for stdio mode
- factory_test.go: add 3 test cases for copilot stdio factory logic
This commit is contained in:
parent
de2ccb5da4
commit
dce22494ee
5 changed files with 202 additions and 35 deletions
|
|
@ -192,12 +192,13 @@ func resolveProviderSelection(cfg *config.Config) (providerSelection, error) {
|
|||
}
|
||||
case "github_copilot", "copilot":
|
||||
sel.providerType = providerTypeGitHubCopilot
|
||||
if cfg.Providers.GitHubCopilot.APIBase != "" {
|
||||
sel.apiBase = cfg.Providers.GitHubCopilot.APIBase
|
||||
} else {
|
||||
sel.connectMode = cfg.Providers.GitHubCopilot.ConnectMode
|
||||
sel.apiBase = cfg.Providers.GitHubCopilot.APIBase
|
||||
// For grpc mode (or default), use localhost:4321 if no address is specified.
|
||||
// For stdio mode, apiBase is the optional CLI binary path (empty = "copilot" from PATH).
|
||||
if sel.apiBase == "" && sel.connectMode != "stdio" {
|
||||
sel.apiBase = "localhost:4321"
|
||||
}
|
||||
sel.connectMode = cfg.Providers.GitHubCopilot.ConnectMode
|
||||
return sel, nil
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -154,15 +154,17 @@ func CreateProviderFromConfig(cfg *config.ModelConfig) (LLMProvider, string, err
|
|||
return NewCodexCliProvider(workspace), modelID, nil
|
||||
|
||||
case "github-copilot", "copilot":
|
||||
apiBase := cfg.APIBase
|
||||
if apiBase == "" {
|
||||
apiBase = "localhost:4321"
|
||||
}
|
||||
connectMode := cfg.ConnectMode
|
||||
if connectMode == "" {
|
||||
connectMode = "grpc"
|
||||
}
|
||||
provider, err := NewGitHubCopilotProvider(apiBase, connectMode, modelID)
|
||||
uri := cfg.APIBase
|
||||
// For grpc mode, default to localhost:4321 if no address is specified.
|
||||
// For stdio mode, uri is the optional CLI binary path (empty = "copilot" from PATH).
|
||||
if uri == "" && connectMode == "grpc" {
|
||||
uri = "localhost:4321"
|
||||
}
|
||||
provider, err := NewGitHubCopilotProvider(uri, connectMode, modelID)
|
||||
if err != nil {
|
||||
return nil, "", err
|
||||
}
|
||||
|
|
|
|||
|
|
@ -54,6 +54,34 @@ func TestResolveProviderSelection(t *testing.T) {
|
|||
wantType: providerTypeGitHubCopilot,
|
||||
wantAPIBase: "localhost:4321",
|
||||
},
|
||||
{
|
||||
name: "copilot stdio mode does not set default apiBase",
|
||||
setup: func(cfg *config.Config) {
|
||||
cfg.Agents.Defaults.Provider = "copilot"
|
||||
cfg.Providers.GitHubCopilot.ConnectMode = "stdio"
|
||||
},
|
||||
wantType: providerTypeGitHubCopilot,
|
||||
},
|
||||
{
|
||||
name: "copilot stdio mode preserves custom cli path",
|
||||
setup: func(cfg *config.Config) {
|
||||
cfg.Agents.Defaults.Provider = "copilot"
|
||||
cfg.Providers.GitHubCopilot.ConnectMode = "stdio"
|
||||
cfg.Providers.GitHubCopilot.APIBase = "/usr/local/bin/copilot"
|
||||
},
|
||||
wantType: providerTypeGitHubCopilot,
|
||||
wantAPIBase: "/usr/local/bin/copilot",
|
||||
},
|
||||
{
|
||||
name: "copilot grpc mode with custom apiBase",
|
||||
setup: func(cfg *config.Config) {
|
||||
cfg.Agents.Defaults.Provider = "copilot"
|
||||
cfg.Providers.GitHubCopilot.ConnectMode = "grpc"
|
||||
cfg.Providers.GitHubCopilot.APIBase = "myhost:5000"
|
||||
},
|
||||
wantType: providerTypeGitHubCopilot,
|
||||
wantAPIBase: "myhost:5000",
|
||||
},
|
||||
{
|
||||
name: "explicit deepseek provider uses deepseek defaults",
|
||||
setup: func(cfg *config.Config) {
|
||||
|
|
|
|||
|
|
@ -9,6 +9,10 @@ import (
|
|||
copilot "github.com/github/copilot-sdk/go"
|
||||
)
|
||||
|
||||
// GitHubCopilotProvider provides LLM capabilities via the GitHub Copilot SDK.
|
||||
// It supports two connection modes:
|
||||
// - "stdio": spawns a local Copilot CLI process and communicates via stdin/stdout (default SDK behavior)
|
||||
// - "grpc": connects to an external Copilot CLI server over TCP
|
||||
type GitHubCopilotProvider struct {
|
||||
uri string
|
||||
connectMode string // "stdio" or "grpc"
|
||||
|
|
@ -19,45 +23,57 @@ type GitHubCopilotProvider struct {
|
|||
mu sync.Mutex
|
||||
}
|
||||
|
||||
// NewGitHubCopilotProvider creates a new GitHub Copilot provider.
|
||||
//
|
||||
// Parameters:
|
||||
// - uri: for "grpc" mode, the address of an external CLI server (e.g. "localhost:4321");
|
||||
// for "stdio" mode, the path to the Copilot CLI binary (empty string uses the default "copilot" from PATH)
|
||||
// - connectMode: "stdio" or "grpc" (defaults to "grpc" if empty)
|
||||
// - model: the model identifier to use for the session
|
||||
func NewGitHubCopilotProvider(uri string, connectMode string, model string) (*GitHubCopilotProvider, error) {
|
||||
if connectMode == "" {
|
||||
connectMode = "grpc"
|
||||
}
|
||||
|
||||
var client *copilot.Client
|
||||
|
||||
switch connectMode {
|
||||
case "stdio":
|
||||
// TODO: Implement stdio mode for GitHub Copilot provider
|
||||
// See https://github.com/github/copilot-sdk/blob/main/docs/getting-started.md for details
|
||||
return nil, fmt.Errorf("stdio mode not implemented for GitHub Copilot provider; please use 'grpc' mode instead")
|
||||
opts := &copilot.ClientOptions{}
|
||||
if uri != "" {
|
||||
opts.CLIPath = uri
|
||||
}
|
||||
client = copilot.NewClient(opts)
|
||||
case "grpc":
|
||||
client := copilot.NewClient(&copilot.ClientOptions{
|
||||
client = copilot.NewClient(&copilot.ClientOptions{
|
||||
CLIUrl: uri,
|
||||
})
|
||||
if err := client.Start(context.Background()); err != nil {
|
||||
return nil, fmt.Errorf(
|
||||
"can't connect to Github Copilot: %w; `https://github.com/github/copilot-sdk/blob/main/docs/getting-started.md#connecting-to-an-external-cli-server` for details",
|
||||
err,
|
||||
)
|
||||
}
|
||||
|
||||
session, err := client.CreateSession(context.Background(), &copilot.SessionConfig{
|
||||
Model: model,
|
||||
Hooks: &copilot.SessionHooks{},
|
||||
})
|
||||
if err != nil {
|
||||
client.Stop()
|
||||
return nil, fmt.Errorf("create session failed: %w", err)
|
||||
}
|
||||
|
||||
return &GitHubCopilotProvider{
|
||||
uri: uri,
|
||||
connectMode: connectMode,
|
||||
client: client,
|
||||
session: session,
|
||||
}, nil
|
||||
default:
|
||||
return nil, fmt.Errorf("unknown connect mode: %s", connectMode)
|
||||
}
|
||||
|
||||
if err := client.Start(context.Background()); err != nil {
|
||||
return nil, fmt.Errorf(
|
||||
"can't connect to GitHub Copilot (%s mode): %w",
|
||||
connectMode, err,
|
||||
)
|
||||
}
|
||||
|
||||
session, err := client.CreateSession(context.Background(), &copilot.SessionConfig{
|
||||
Model: model,
|
||||
Hooks: &copilot.SessionHooks{},
|
||||
})
|
||||
if err != nil {
|
||||
client.Stop()
|
||||
return nil, fmt.Errorf("create session failed: %w", err)
|
||||
}
|
||||
|
||||
return &GitHubCopilotProvider{
|
||||
uri: uri,
|
||||
connectMode: connectMode,
|
||||
client: client,
|
||||
session: session,
|
||||
}, nil
|
||||
}
|
||||
|
||||
func (p *GitHubCopilotProvider) Close() {
|
||||
|
|
|
|||
120
pkg/providers/github_copilot_provider_test.go
Normal file
120
pkg/providers/github_copilot_provider_test.go
Normal file
|
|
@ -0,0 +1,120 @@
|
|||
package providers
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/sipeed/picoclaw/pkg/config"
|
||||
)
|
||||
|
||||
func TestNewGitHubCopilotProvider_UnknownConnectMode(t *testing.T) {
|
||||
_, err := NewGitHubCopilotProvider("localhost:4321", "unknown", "gpt-4.1")
|
||||
if err == nil {
|
||||
t.Fatal("expected error for unknown connect mode, got nil")
|
||||
}
|
||||
if got := err.Error(); got != "unknown connect mode: unknown" {
|
||||
t.Fatalf("error = %q, want %q", got, "unknown connect mode: unknown")
|
||||
}
|
||||
}
|
||||
|
||||
func TestNewGitHubCopilotProvider_DefaultConnectMode(t *testing.T) {
|
||||
// When connectMode is empty, it should default to "grpc" and attempt to connect.
|
||||
// Since there's no server, we expect a connection error (not a "not implemented" error).
|
||||
_, err := NewGitHubCopilotProvider("localhost:19999", "", "gpt-4.1")
|
||||
if err == nil {
|
||||
t.Fatal("expected connection error, got nil")
|
||||
}
|
||||
// Should NOT get "not implemented" error — that was the old behavior before stdio was implemented
|
||||
if got := err.Error(); got == "stdio mode not implemented for GitHub Copilot provider; please use 'grpc' mode instead" {
|
||||
t.Fatal("got old 'not implemented' error; stdio mode should be implemented now")
|
||||
}
|
||||
}
|
||||
|
||||
func TestNewGitHubCopilotProvider_StdioModeAccepted(t *testing.T) {
|
||||
// Stdio mode should no longer return "not implemented".
|
||||
// It will fail to start because the copilot CLI binary is likely not installed,
|
||||
// but the error should be about starting the CLI process, not about the mode
|
||||
// being unimplemented.
|
||||
_, err := NewGitHubCopilotProvider("", "stdio", "gpt-4.1")
|
||||
if err == nil {
|
||||
// If it succeeds, that's fine too (copilot CLI is installed).
|
||||
return
|
||||
}
|
||||
got := err.Error()
|
||||
if got == "stdio mode not implemented for GitHub Copilot provider; please use 'grpc' mode instead" {
|
||||
t.Fatal("got old 'not implemented' error; stdio mode should be implemented now")
|
||||
}
|
||||
// Expect a startup/connection error, not an implementation error
|
||||
t.Logf("expected startup error (copilot CLI likely not installed): %v", err)
|
||||
}
|
||||
|
||||
func TestNewGitHubCopilotProvider_StdioWithCustomCLIPath(t *testing.T) {
|
||||
// When a custom CLI path is provided in stdio mode, the error should reference
|
||||
// the startup failure, not "not implemented".
|
||||
_, err := NewGitHubCopilotProvider("/nonexistent/copilot", "stdio", "gpt-4.1")
|
||||
if err == nil {
|
||||
t.Fatal("expected error for nonexistent CLI path, got nil")
|
||||
}
|
||||
got := err.Error()
|
||||
if got == "stdio mode not implemented for GitHub Copilot provider; please use 'grpc' mode instead" {
|
||||
t.Fatal("got old 'not implemented' error; stdio mode should be implemented now")
|
||||
}
|
||||
t.Logf("got expected startup error: %v", err)
|
||||
}
|
||||
|
||||
func TestGitHubCopilotProvider_GetDefaultModel(t *testing.T) {
|
||||
p := &GitHubCopilotProvider{}
|
||||
if got := p.GetDefaultModel(); got != "gpt-4.1" {
|
||||
t.Fatalf("GetDefaultModel() = %q, want %q", got, "gpt-4.1")
|
||||
}
|
||||
}
|
||||
|
||||
func TestGitHubCopilotProvider_CloseNilClient(t *testing.T) {
|
||||
// Close on a provider with nil client should not panic
|
||||
p := &GitHubCopilotProvider{}
|
||||
p.Close() // should not panic
|
||||
}
|
||||
|
||||
func TestGitHubCopilotProvider_ChatNilSession(t *testing.T) {
|
||||
p := &GitHubCopilotProvider{}
|
||||
_, err := p.Chat(nil, nil, nil, "gpt-4.1", nil)
|
||||
if err == nil {
|
||||
t.Fatal("expected error for nil session, got nil")
|
||||
}
|
||||
if got := err.Error(); got != "provider closed" {
|
||||
t.Fatalf("error = %q, want %q", got, "provider closed")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCreateProviderFromConfig_CopilotStdioMode(t *testing.T) {
|
||||
cfg := &config.ModelConfig{
|
||||
Model: "copilot/gpt-4.1",
|
||||
ConnectMode: "stdio",
|
||||
}
|
||||
|
||||
// This will try to start the copilot CLI which likely isn't installed,
|
||||
// but the error should be a startup error, not "unknown protocol" or "not implemented".
|
||||
_, _, err := CreateProviderFromConfig(cfg)
|
||||
if err == nil {
|
||||
return // copilot CLI is installed, all good
|
||||
}
|
||||
got := err.Error()
|
||||
if got == "stdio mode not implemented for GitHub Copilot provider; please use 'grpc' mode instead" {
|
||||
t.Fatal("got old 'not implemented' error; stdio should be supported now")
|
||||
}
|
||||
t.Logf("expected startup error: %v", err)
|
||||
}
|
||||
|
||||
func TestCreateProviderFromConfig_CopilotGrpcModeDefaultAPIBase(t *testing.T) {
|
||||
cfg := &config.ModelConfig{
|
||||
Model: "github-copilot/gpt-4.1",
|
||||
ConnectMode: "grpc",
|
||||
}
|
||||
|
||||
// Will fail to connect, but should attempt localhost:4321
|
||||
_, _, err := CreateProviderFromConfig(cfg)
|
||||
if err == nil {
|
||||
return
|
||||
}
|
||||
// The error should mention the connection failure, not "not implemented"
|
||||
t.Logf("expected connection error: %v", err)
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue