diff --git a/docs/api-reference.md b/docs/api-reference.md index 9bf1828..e930b1e 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -117,11 +117,19 @@ Response shape: "result": { "singleAgent": true, "multiAgent": true, - "providers": ["codex", "gemini", "opencode"], + "providerCatalog": [ + { "providerId": "codex", "label": "Codex" }, + { "providerId": "gemini", "label": "Gemini" }, + { "providerId": "opencode", "label": "OpenCode" } + ], "capabilities": { "single_agent": true, "multi_agent": true, - "providers": ["codex", "gemini", "opencode"] + "providerCatalog": [ + { "providerId": "codex", "label": "Codex" }, + { "providerId": "gemini", "label": "Gemini" }, + { "providerId": "opencode", "label": "OpenCode" } + ] } } } @@ -129,8 +137,9 @@ Response shape: Notes: -- `providers` comes from the synced external provider catalog registered through - `xworkmate.providers.sync` +- `providerCatalog` comes from the synced external provider catalog registered + through `xworkmate.providers.sync` +- provider order is bridge-owned and preserves the sync order - `multiAgent` is controlled by `ACP_MULTI_AGENT_ENABLED`, default `true` ### 3.2 `session.start` @@ -293,6 +302,15 @@ Purpose: - selected skills - install suggestion / unavailable state +Canonical use: + +- apps should use this method as the single preflight source for: + - effective execution target + - effective provider selection + - unavailable code / message +- apps should not re-derive provider availability or `auto` resolution from + `acp.capabilities` + Key input fields: - `taskPrompt` diff --git a/internal/acp/providers_sync.go b/internal/acp/providers_sync.go index 4d1daad..ef679d0 100644 --- a/internal/acp/providers_sync.go +++ b/internal/acp/providers_sync.go @@ -1,7 +1,6 @@ package acp import ( - "sort" "strings" ) @@ -40,11 +39,15 @@ func (s *Server) syncProviders(providers []syncedProvider) map[string]any { s.mu.Lock() defer s.mu.Unlock() s.providerCatalog = make(map[string]syncedProvider, len(providers)) + s.providerOrder = make([]string, 0, len(providers)) for _, provider := range providers { - if strings.TrimSpace(provider.ProviderID) == "" { + providerID := strings.TrimSpace(provider.ProviderID) + if providerID == "" { continue } - s.providerCatalog[provider.ProviderID] = provider + provider.ProviderID = providerID + s.providerCatalog[providerID] = provider + s.providerOrder = append(s.providerOrder, providerID) } return map[string]any{ "ok": true, @@ -63,32 +66,58 @@ func (s *Server) syncedProviderByID(providerID string) (syncedProvider, bool) { } func (s *Server) availableProviders() []string { - providers := make(map[string]struct{}) s.mu.Lock() - for _, provider := range s.providerCatalog { + defer s.mu.Unlock() + ordered := make([]string, 0, len(s.providerOrder)) + for _, providerID := range s.providerOrder { + provider, ok := s.providerCatalog[providerID] + if !ok { + continue + } if !provider.Enabled || strings.TrimSpace(provider.Endpoint) == "" { continue } - providers[provider.ProviderID] = struct{}{} + ordered = append(ordered, provider.ProviderID) } - s.mu.Unlock() - ordered := make([]string, 0, len(providers)) - for providerID := range providers { - ordered = append(ordered, providerID) - } - sort.Strings(ordered) return ordered } +func (s *Server) availableProviderCatalog() []map[string]any { + s.mu.Lock() + defer s.mu.Unlock() + result := make([]map[string]any, 0, len(s.providerOrder)) + for _, providerID := range s.providerOrder { + provider, ok := s.providerCatalog[providerID] + if !ok { + continue + } + if !provider.Enabled || strings.TrimSpace(provider.Endpoint) == "" { + continue + } + result = append(result, map[string]any{ + "providerId": provider.ProviderID, + "label": providerLabel(provider), + }) + } + return result +} + func syncedProvidersResult(providers []syncedProvider) []map[string]any { result := make([]map[string]any, 0, len(providers)) for _, provider := range providers { result = append(result, map[string]any{ "providerId": provider.ProviderID, - "label": provider.Label, + "label": providerLabel(provider), "endpoint": provider.Endpoint, "enabled": provider.Enabled, }) } return result } + +func providerLabel(provider syncedProvider) string { + if label := strings.TrimSpace(provider.Label); label != "" { + return label + } + return provider.ProviderID +} diff --git a/internal/acp/providers_sync_test.go b/internal/acp/providers_sync_test.go index 81da261..a976f51 100644 --- a/internal/acp/providers_sync_test.go +++ b/internal/acp/providers_sync_test.go @@ -41,19 +41,77 @@ func TestProvidersSyncUpdatesCapabilities(t *testing.T) { if rpcErr != nil { t.Fatalf("expected capabilities success, got %v", rpcErr) } - providers, _ := result["providers"].([]string) - if len(providers) == 0 { + providerCatalog, ok := result["providerCatalog"].([]map[string]any) + if !ok || len(providerCatalog) == 0 { t.Fatalf("expected synced provider in capabilities, got %#v", result) } - found := false - for _, provider := range providers { - if provider == "claude" { - found = true - break - } + if providerCatalog[0]["providerId"] != "claude" { + t.Fatalf("expected claude provider after sync, got %#v", providerCatalog) } - if !found { - t.Fatalf("expected claude provider after sync, got %#v", providers) + if providerCatalog[0]["label"] != "Claude" { + t.Fatalf("expected Claude label after sync, got %#v", providerCatalog) + } +} + +func TestProvidersSyncPreservesProviderCatalogOrder(t *testing.T) { + server := NewServer() + + _, rpcErr := server.handleRequest(shared.RPCRequest{ + Method: "xworkmate.providers.sync", + Params: map[string]any{ + "providers": []any{ + map[string]any{ + "providerId": "gemini", + "label": "Gemini", + "endpoint": "http://127.0.0.1:9001", + "authorizationHeader": "Bearer gemini", + "enabled": true, + }, + map[string]any{ + "providerId": "codex", + "label": "Codex", + "endpoint": "http://127.0.0.1:9002", + "authorizationHeader": "Bearer codex", + "enabled": true, + }, + map[string]any{ + "providerId": "opencode", + "label": "OpenCode", + "endpoint": "http://127.0.0.1:9003", + "authorizationHeader": "Bearer opencode", + "enabled": true, + }, + }, + }, + }, func(map[string]any) {}) + if rpcErr != nil { + t.Fatalf("expected sync success, got %v", rpcErr) + } + + result, rpcErr := server.handleRequest(shared.RPCRequest{ + Method: "acp.capabilities", + Params: map[string]any{}, + }, func(map[string]any) {}) + if rpcErr != nil { + t.Fatalf("expected capabilities success, got %v", rpcErr) + } + providerCatalog, ok := result["providerCatalog"].([]map[string]any) + if !ok { + t.Fatalf("expected providerCatalog array, got %#v", result) + } + if len(providerCatalog) != 3 { + t.Fatalf("expected 3 catalog entries, got %#v", providerCatalog) + } + gotOrder := []string{ + providerCatalog[0]["providerId"].(string), + providerCatalog[1]["providerId"].(string), + providerCatalog[2]["providerId"].(string), + } + wantOrder := []string{"gemini", "codex", "opencode"} + for index, want := range wantOrder { + if gotOrder[index] != want { + t.Fatalf("expected provider order %#v, got %#v", wantOrder, gotOrder) + } } } diff --git a/internal/acp/routing_test.go b/internal/acp/routing_test.go index f5a3276..4f5915c 100644 --- a/internal/acp/routing_test.go +++ b/internal/acp/routing_test.go @@ -325,6 +325,66 @@ func TestExecuteSessionTaskExplicitProviderRequiresAdvertisedBridgeProvider(t *t if got := response["unavailableCode"]; got != "PROVIDER_UNAVAILABLE" { t.Fatalf("expected PROVIDER_UNAVAILABLE, got %#v", response) } + if got := response["unavailableMessage"]; got != "explicit provider is unavailable" { + t.Fatalf("expected explicit provider unavailable message, got %#v", response) + } +} + +func TestExecuteSessionTaskAutoRoutingUsesBridgeSyncOrderForProviderResolution(t *testing.T) { + workspaceDir := filepath.Join(t.TempDir(), "workspace") + if err := os.MkdirAll(workspaceDir, 0o755); err != nil { + t.Fatalf("create workspace: %v", err) + } + + server := NewServer() + geminiProvider := newExternalSingleAgentProvider(t, "gemini", "gemini-output") + defer geminiProvider.Close() + codexProvider := newExternalSingleAgentProvider(t, "codex", "codex-output") + defer codexProvider.Close() + server.syncProviders([]syncedProvider{ + { + ProviderID: "gemini", + Label: "Gemini", + Endpoint: geminiProvider.URL, + Enabled: true, + }, + { + ProviderID: "codex", + Label: "Codex", + Endpoint: codexProvider.URL, + Enabled: true, + }, + }) + + response, rpcErr := server.executeSessionTask(task{ + req: shared.RPCRequest{ + Method: "session.start", + Params: map[string]any{ + "sessionId": "session-auto-order", + "threadId": "thread-auto-order", + "taskPrompt": "create a powerpoint deck for launch", + "workingDirectory": workspaceDir, + "routing": map[string]any{ + "routingMode": "auto", + "preferredGatewayTarget": "local", + "availableSkills": []any{ + map[string]any{ + "id": "pptx", + "label": "PPTX", + "description": "slides", + "installed": true, + }, + }, + }, + }, + }, + }) + if rpcErr != nil { + t.Fatalf("expected success, got rpc error: %v", rpcErr) + } + if got := response["resolvedProviderId"]; got != "gemini" { + t.Fatalf("expected resolved provider gemini from bridge order, got %#v", response) + } } func TestExecuteSessionTaskRequiresRouting(t *testing.T) { diff --git a/internal/acp/server.go b/internal/acp/server.go index b7cc9fc..3b133bb 100644 --- a/internal/acp/server.go +++ b/internal/acp/server.go @@ -50,6 +50,7 @@ type Server struct { queues map[string]chan task gateway *gatewayruntime.Manager providerCatalog map[string]syncedProvider + providerOrder []string authService *service.StaticTokenAuthService } @@ -108,6 +109,7 @@ func NewServer() *Server { queues: make(map[string]chan task), gateway: gatewayruntime.NewManager(), providerCatalog: make(map[string]syncedProvider), + providerOrder: nil, authService: service.NewStaticTokenAuthService(strings.TrimSpace(shared.EnvOrDefault("ACP_AUTH_TOKEN", ""))), } } @@ -296,20 +298,20 @@ func (s *Server) handleRequest( method := strings.TrimSpace(request.Method) switch method { case "acp.capabilities": - providers := s.availableProviders() - singleAgent := len(providers) > 0 + providerCatalog := s.availableProviderCatalog() + singleAgent := len(providerCatalog) > 0 multiAgent := shared.BoolArg( shared.EnvOrDefault("ACP_MULTI_AGENT_ENABLED", "true"), true, ) result := map[string]any{ - "singleAgent": singleAgent, - "multiAgent": multiAgent, - "providers": providers, + "singleAgent": singleAgent, + "multiAgent": multiAgent, + "providerCatalog": providerCatalog, "capabilities": map[string]any{ - "single_agent": singleAgent, - "multi_agent": multiAgent, - "providers": providers, + "single_agent": singleAgent, + "multi_agent": multiAgent, + "providerCatalog": providerCatalog, }, } return result, nil diff --git a/internal/acp/web_contract_test.go b/internal/acp/web_contract_test.go index 52a41c3..2b678ca 100644 --- a/internal/acp/web_contract_test.go +++ b/internal/acp/web_contract_test.go @@ -125,7 +125,7 @@ func TestHandleRPCCapabilitiesStillReturnsJSONResult(t *testing.T) { if got := recorder.Header().Get("Content-Type"); !strings.Contains(got, "application/json") { t.Fatalf("expected application/json content type, got %q", got) } - if !strings.Contains(recorder.Body.String(), `"providers"`) { + if !strings.Contains(recorder.Body.String(), `"providerCatalog"`) { t.Fatalf("expected capabilities response, got %q", recorder.Body.String()) } } diff --git a/internal/router/router.go b/internal/router/router.go index e1248f5..690c189 100644 --- a/internal/router/router.go +++ b/internal/router/router.go @@ -2,7 +2,6 @@ package router import ( "os" - "sort" "strings" "xworkmate-bridge/internal/memory" @@ -267,7 +266,6 @@ func normalizeProviders(values []string) []string { unique[providerID] = struct{}{} normalized = append(normalized, providerID) } - sort.Strings(normalized) return normalized }