Remove hidden discovery provider fallback

This commit is contained in:
rcourtman 2026-06-08 03:48:33 +01:00
parent d9e9b952e7
commit 50295914c1
5 changed files with 88 additions and 56 deletions

View file

@ -1763,7 +1763,7 @@ deriving an older display status from `workflowStatusHistory`.
divergence must be sourced from canonical `ReadState.DockerHosts()` views,
with model-shaped data limited to the watcher adapter input rather than
direct `StateSnapshot.DockerHosts` reads in the Patrol run loop.
4. Keep discovery scheduling authoritative through `internal/config/ai.go`: `discovery_enabled` and `discovery_interval_hours` must govern both lightweight infrastructure discovery and deep service-discovery background loops. `internal/api/ai_handlers.go` must preserve an explicitly supplied `discovery_interval_hours: 0` as the manual-only setting and may only apply the 24-hour default when discovery is enabled without an explicit interval payload. Discovery analysis remains a Pulse tool-led model workflow: Pulse supplies agent/API/metrics evidence and cache orchestration, while the selected model provides the intelligence. Background, settings-triggered, or drawer-triggered discovery progress must describe that discovery analysis directly and must not imply a live Pulse Assistant chat transcript unless the run is actually executing inside the chat surface. Settings-triggered manual discovery is an explicit operator refresh and must not open, append to, or masquerade as a Pulse Assistant session. Assistant and Patrol access to discovery must stay behind the governed `pulse_discovery` tool, including an explicit forced refresh action for known resources, while settings-level manual runs must use the canonical `/api/discovery/run` new/changed/stale/repairable sweep rather than a frontend-only shortcut. Fresh-but-unidentified service-discovery records are not complete when canonical resource metadata, stored facts, or safe command evidence can deterministically identify a known workload and endpoint; cached reads and the canonical sweep must repair those records instead of presenting `Unknown Service` as fresh.
4. Keep discovery scheduling authoritative through `internal/config/ai.go`: `discovery_enabled` and `discovery_interval_hours` must govern both lightweight infrastructure discovery and deep service-discovery background loops. `internal/api/ai_handlers.go` must preserve an explicitly supplied `discovery_interval_hours: 0` as the manual-only setting and may only apply the 24-hour default when discovery is enabled without an explicit interval payload. Discovery analysis remains a Pulse tool-led model workflow: Pulse supplies agent/API/metrics evidence and cache orchestration, while the selected model provides the intelligence. Background, settings-triggered, or drawer-triggered discovery progress must describe that discovery analysis directly and must not imply a live Pulse Assistant chat transcript unless the run is actually executing inside the chat surface. Settings-triggered manual discovery is an explicit operator refresh and must not open, append to, or masquerade as a Pulse Assistant session. Assistant and Patrol access to discovery must stay behind the governed `pulse_discovery` tool, including an explicit forced refresh action for known resources, while settings-level manual runs must use the canonical `/api/discovery/run` new/changed/stale/repairable sweep rather than a frontend-only shortcut. Discovery analysis must use the selected discovery route only: provider-owned default resolution may choose a model before the request when no explicit route exists, but `AnalyzeForDiscovery` must not fall back to a stale default provider or another configured provider after selected-route provider construction or execution fails. Fresh-but-unidentified service-discovery records are not complete when canonical resource metadata, stored facts, or safe command evidence can deterministically identify a known workload and endpoint; cached reads and the canonical sweep must repair those records instead of presenting `Unknown Service` as fresh.
5. Preserve auditability for outbound model-bound context exports and keep the export record aligned with the prompt boundary that actually reaches the provider
External provider-bound unified-resource context must enforce the same
data-handling policy the export audit records: `local-only` resources are

View file

@ -0,0 +1,12 @@
import { describe, expect, it } from 'vitest';
import { aiHelpContent } from '../ai';
describe('AI help content', () => {
it('describes provider route recovery as explicit instead of automatic fallback', () => {
const providerHelp = aiHelpContent.find((item) => item.id === 'ai.providers.overview');
expect(providerHelp?.description).toContain('choose the route');
expect(providerHelp?.description).toContain('switch routes explicitly');
expect(providerHelp?.description).not.toMatch(/fallback to others|automatic provider fallback/i);
});
});

View file

@ -45,8 +45,8 @@ export const aiHelpContent: HelpContent[] = [
'- Natural language infrastructure queries\n' +
'- Automated troubleshooting suggestions\n' +
'- Alert investigation assistance\n\n' +
'You can configure multiple providers and Pulse will use the primary provider ' +
'with fallback to others if unavailable.',
'You can configure multiple providers and choose the route for Assistant, Patrol, and discovery. ' +
'Pulse retries the selected route when appropriate and asks you to switch routes explicitly if that provider is unavailable.',
related: ['ai.openai.baseUrl', 'ai.ollama.baseUrl', 'ai.ollama.keepAlive'],
addedInVersion: 'v4.0.0',
},

View file

@ -3525,12 +3525,13 @@ func (s *Service) GetKnowledgeStore() *knowledge.Store {
// It sends a prompt to the AI using the discovery model (optimized for cost).
// Dynamically creates a provider matching the discovery model to avoid using a
// stale or mismatched global provider (e.g., when the user selects a different
// model per-session in chat). Falls back to other configured providers on failure.
// model per-session in chat). Discovery uses the selected route only: provider
// failures fail closed instead of silently trying another configured provider.
func (s *Service) AnalyzeForDiscovery(ctx context.Context, prompt string) (string, error) {
s.mu.RLock()
cfg := s.cfg
costStore := s.costStore
fallbackProvider := s.provider // keep as last resort
defaultProvider := s.provider
s.mu.RUnlock()
if cfg == nil || !cfg.Enabled {
@ -3560,11 +3561,10 @@ func (s *Service) AnalyzeForDiscovery(ctx context.Context, prompt string) (strin
var providerErr error
provider, providerErr = providers.NewForModel(cfg, model)
if providerErr != nil {
log.Debug().Err(providerErr).Str("model", model).Msg("[Discovery] Could not create provider for discovery model, using default")
provider = fallbackProvider
return "", fmt.Errorf("discovery provider for selected model %q is not configured: %w", model, providerErr)
}
} else {
provider = fallbackProvider
provider = defaultProvider
}
if provider == nil {
@ -3583,55 +3583,8 @@ func (s *Service) AnalyzeForDiscovery(ctx context.Context, prompt string) (strin
discoveryReq = sanitizer(discoveryReq)
}
resp, err := provider.Chat(ctx, discoveryReq)
// If the primary provider fails (e.g., rate limited), try other configured providers
if err != nil {
primaryErr := err
primaryProvider, _ := config.ParseModelString(model)
for _, altProviderName := range cfg.GetConfiguredProviders() {
if altProviderName == primaryProvider {
continue // skip the one that just failed
}
if err := s.enforceBudget("discovery"); err != nil {
return "", err
}
altModel, resolveErr := ResolveConfiguredProviderModel(ctx, cfg, altProviderName)
if resolveErr != nil || altModel == "" {
continue
}
altProvider, createErr := providers.NewForModel(cfg, altModel)
if createErr != nil {
continue
}
log.Info().
Str("failed_provider", primaryProvider).
Str("fallback_provider", altProviderName).
Str("fallback_model", altModel).
Msg("[Discovery] Primary provider failed, trying fallback")
altReq := providers.ChatRequest{
Messages: messages,
Model: altModel,
MaxTokens: discoveryResponseTokenBudget,
}
if sanitizer := s.requestSanitizerForModel(altModel); sanitizer != nil {
altReq = sanitizer(altReq)
}
resp, err = altProvider.Chat(ctx, altReq)
if err == nil {
model = altModel
provider = altProvider
break
}
}
if err != nil {
return "", fmt.Errorf("discovery analysis failed: %w (primary error: %v)", err, primaryErr)
}
return "", fmt.Errorf("discovery analysis failed on selected provider route: %w", err)
}
// Track cost if cost store is available

View file

@ -2,6 +2,9 @@ package ai
import (
"context"
"errors"
"net/http"
"net/http/httptest"
"strings"
"testing"
"time"
@ -206,6 +209,70 @@ func TestService_AnalyzeForDiscovery(t *testing.T) {
}
}
func TestService_AnalyzeForDiscoveryDoesNotFallbackToAnotherConfiguredProvider(t *testing.T) {
var alternateProviderCalls int
alternateProviderServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
alternateProviderCalls++
w.Header().Set("Content-Type", "application/json")
_, _ = w.Write([]byte(`{
"id":"chatcmpl-fallback",
"object":"chat.completion",
"choices":[{"message":{"role":"assistant","content":"fallback discovery response"},"finish_reason":"stop"}],
"usage":{"prompt_tokens":1,"completion_tokens":1}
}`))
}))
defer alternateProviderServer.Close()
selectedProviderCalls := 0
svc := NewService(nil, nil)
svc.provider = &mockProvider{
chatFunc: func(ctx context.Context, req providers.ChatRequest) (*providers.ChatResponse, error) {
selectedProviderCalls++
return nil, errors.New("selected discovery route unavailable")
},
nameFunc: func() string { return "selected-provider" },
}
svc.cfg = &config.AIConfig{
Enabled: true,
OpenAIAPIKey: "openai-test",
OpenAIBaseURL: alternateProviderServer.URL,
}
_, err := svc.AnalyzeForDiscovery(context.Background(), "test discovery")
if err == nil || !strings.Contains(err.Error(), "selected discovery route unavailable") {
t.Fatalf("expected selected route failure, got %v", err)
}
if selectedProviderCalls != 1 {
t.Fatalf("selected provider calls = %d, want 1", selectedProviderCalls)
}
if alternateProviderCalls != 0 {
t.Fatalf("alternate provider calls = %d, want no hidden provider fallback", alternateProviderCalls)
}
}
func TestService_AnalyzeForDiscoveryExplicitModelProviderCreationFailsClosed(t *testing.T) {
defaultProviderCalls := 0
svc := NewService(nil, nil)
svc.provider = &mockProvider{
chatFunc: func(ctx context.Context, req providers.ChatRequest) (*providers.ChatResponse, error) {
defaultProviderCalls++
return &providers.ChatResponse{Content: "default provider response"}, nil
},
}
svc.cfg = &config.AIConfig{
Enabled: true,
DiscoveryModel: config.FormatModelString(config.AIProviderDeepSeek, config.DeepSeekModelV4Pro),
}
_, err := svc.AnalyzeForDiscovery(context.Background(), "test discovery")
if err == nil || !strings.Contains(err.Error(), "discovery provider for selected model") {
t.Fatalf("expected selected discovery provider configuration error, got %v", err)
}
if defaultProviderCalls != 0 {
t.Fatalf("default provider calls = %d, want no stale provider fallback", defaultProviderCalls)
}
}
func TestService_RecordIncidentRunbook(t *testing.T) {
svc := NewService(nil, nil)
canonicalStore := unifiedresources.NewMemoryStore()