From 2b24c375e04fa23128861814743e031ddf13061d Mon Sep 17 00:00:00 2001 From: rcourtman Date: Fri, 5 Jun 2026 09:57:57 +0100 Subject: [PATCH] Abstain instead of fabricating discovery when commands can't run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a deep scan was attempted for a workload (container/VM) but produced no command output — e.g. the host agent rejects exec — the AI analyzer was still asked to identify the service from metadata alone. It confabulated confident, false identities: an ESPHome LXC and an influxdb-telegraf LXC were both "identified" as Pi-hole at 0.95 confidence, with invented facts carrying fabricated command sources (source: "pihole -v" for a command that never ran) and a docker exec CLI for an LXC. When a command scan was attempted for a command-dependent resource type but yielded nothing, abstain: skip the analyzer entirely and return a not-determined result (no service, no facts, zero confidence) with guidance to enable Pulse Commands. Host agents are unaffected (identified from their own metadata), and deployments without command scanning keep the metadata-only path. Abstaining also avoids a wasted model call. Verified live: re-running discovery on the esphome LXC now returns an empty, zero-confidence result with the correct container CLI instead of fabricated Pi-hole facts. Adds a regression test. --- internal/servicediscovery/service.go | 113 ++++++++++++++++------ internal/servicediscovery/service_test.go | 44 +++++++++ 2 files changed, 129 insertions(+), 28 deletions(-) diff --git a/internal/servicediscovery/service.go b/internal/servicediscovery/service.go index 8ef2ecf79..fc070ddbc 100644 --- a/internal/servicediscovery/service.go +++ b/internal/servicediscovery/service.go @@ -1763,39 +1763,62 @@ func (s *Service) DiscoverResource(ctx context.Context, req DiscoveryRequest) (* } } - // Build prompt and analyze - prompt := s.buildDeepAnalysisPrompt(analysisReq) + // When a resource type relies on command-execution evidence but the scan + // produced none (e.g. the host agent has "Pulse Commands" disabled), do NOT + // ask the model to identify the service. With only metadata it confabulates + // confident, false identities — inventing facts with fabricated command + // sources (a metadata-only scan once "identified" Pi-hole inside an ESPHome + // LXC at 0.95 confidence). Abstain instead; deterministic metadata identity + // is still applied below via applyKnownServiceIdentity. + // Abstain only when a command scan was actually attempted (scanner present + // and command scanning enabled) for a workload that needs command evidence, + // yet produced no output — i.e. we expected to look inside the workload and + // couldn't (e.g. the host agent rejected exec). In that state the model has + // nothing real to go on and confabulates; "not determined" is the honest + // answer. Deployments without command scanning keep the metadata-only path. + commandScanAttempted := s.scanner != nil && s.IsCommandScanningEnabled() + metadataOnly := resourceIdentityNeedsCommandEvidence(req.ResourceType) && + commandScanAttempted && + len(analysisReq.CommandOutputs) == 0 - // Broadcast progress: AI analysis starting - s.broadcastProgress(&DiscoveryProgress{ - ResourceID: resourceID, - Status: DiscoveryStatusRunning, - CurrentStep: "Analyzing discovery evidence with the selected model...", - PercentComplete: 75, - }) + var result *AIAnalysisResponse + if metadataOnly { + result = metadataOnlyDiscoveryAbstention() + } else { + // Build prompt and analyze + prompt := s.buildDeepAnalysisPrompt(analysisReq) - analyzeCtx, cancel := context.WithTimeout(ctx, s.aiAnalysisTimeout) - defer cancel() + // Broadcast progress: AI analysis starting + s.broadcastProgress(&DiscoveryProgress{ + ResourceID: resourceID, + Status: DiscoveryStatusRunning, + CurrentStep: "Analyzing discovery evidence with the selected model...", + PercentComplete: 75, + }) - response, err := analyzer.AnalyzeForDiscovery(analyzeCtx, prompt) - if err != nil { - if errors.Is(err, context.DeadlineExceeded) { - inProg.err = fmt.Errorf("AI analysis timed out after %s", s.aiAnalysisTimeout) + analyzeCtx, cancel := context.WithTimeout(ctx, s.aiAnalysisTimeout) + defer cancel() + + response, err := analyzer.AnalyzeForDiscovery(analyzeCtx, prompt) + if err != nil { + if errors.Is(err, context.DeadlineExceeded) { + inProg.err = fmt.Errorf("AI analysis timed out after %s", s.aiAnalysisTimeout) + return nil, inProg.err + } + inProg.err = fmt.Errorf("AI analysis failed: %w", err) return nil, inProg.err } - inProg.err = fmt.Errorf("AI analysis failed: %w", err) - return nil, inProg.err - } - result := s.parseAIResponse(response) - if result == nil { - // Truncate response for error message - truncated := response - if len(truncated) > 500 { - truncated = truncated[:500] + "..." + result = s.parseAIResponse(response) + if result == nil { + // Truncate response for error message + truncated := response + if len(truncated) > 500 { + truncated = truncated[:500] + "..." + } + inProg.err = fmt.Errorf("failed to parse AI response: %s", truncated) + return nil, inProg.err } - inProg.err = fmt.Errorf("failed to parse AI response: %s", truncated) - return nil, inProg.err } // Resolve hostname from metadata if not provided in request @@ -1857,8 +1880,10 @@ func (s *Service) DiscoverResource(ctx context.Context, req DiscoveryRequest) (* Msg("Parsed Docker bind mounts from on-demand discovery") } } - } else if scanError != nil { - // Add note to reasoning when we couldn't run commands + } else if scanError != nil && !metadataOnly { + // Add note to reasoning when we couldn't run commands. Skipped when we + // abstained (metadataOnly), since the abstention reasoning already + // explains the missing-commands state without duplication. metadataNote := "[Note: Discovery was limited to metadata-only analysis because command execution was unavailable. " if strings.Contains(scanError.Error(), "no connected agent") { metadataNote += "To enable full discovery with command execution, ensure the host agent has 'Pulse Commands' enabled in Settings → Infrastructure.]" @@ -2602,6 +2627,38 @@ Respond with ONLY valid JSON.`, strings.Join(sections, "\n\n")) } // parseAIResponse parses the AI's JSON response. +// resourceIdentityNeedsCommandEvidence reports whether a resource type's service +// identity must come from inside the workload (command output) rather than from +// its own metadata. Workloads (containers and VMs) qualify; host agents do not — +// they are identified from their own rich metadata, so metadata-only analysis is +// legitimate for them. Used to decide when to abstain rather than let the model +// fabricate an identity with no command evidence. +func resourceIdentityNeedsCommandEvidence(rt ResourceType) bool { + switch rt { + case ResourceTypeSystemContainer, + ResourceTypeVM, + ResourceTypeDocker, + ResourceTypeDockerSystemContainer, + ResourceTypeDockerVM, + ResourceTypeK8s: + return true + default: + return false + } +} + +// metadataOnlyDiscoveryAbstention returns an empty, zero-confidence analysis +// used when discovery has no command-execution evidence. It deliberately carries +// no service identity, facts, paths, or ports so the UI shows "not determined" +// instead of a fabricated guess. CLIAccess is left empty so the platform-correct +// access command is generated downstream rather than invented by the model. +func metadataOnlyDiscoveryAbstention() *AIAnalysisResponse { + return &AIAnalysisResponse{ + Confidence: 0, + Reasoning: "Discovery could not run commands on this resource, so its service was not identified. Enable \"Pulse Commands\" for the host agent (Settings → Infrastructure) to run real discovery instead of guessing.", + } +} + func (s *Service) parseAIResponse(response string) *AIAnalysisResponse { log.Debug().Str("raw_response", response).Msg("discovery raw response") payload, ok := jsonresponse.ExtractObject(response) diff --git a/internal/servicediscovery/service_test.go b/internal/servicediscovery/service_test.go index cbf512e81..1462b209f 100644 --- a/internal/servicediscovery/service_test.go +++ b/internal/servicediscovery/service_test.go @@ -280,6 +280,50 @@ func TestService_DiscoverResource_ProgressDescribesModelAnalysisNotAssistantChat } } +func TestService_DiscoverResource_AbstainsWithoutCommandEvidence(t *testing.T) { + store, err := NewStore(t.TempDir()) + if err != nil { + t.Fatalf("NewStore error: %v", err) + } + store.crypto = nil + + // Command scanning is enabled with a scanner, but the scan yields nothing + // (nil executor → no command output) — mirroring a host agent that rejects + // exec. The stub returns a fully fabricated Pi-hole identity. The service + // must abstain: the analyzer must NOT be called and the fabrication must NOT + // be adopted. Guards the metadata-only hallucination bug where an ESPHome LXC + // was "identified" as Pi-hole at 0.95 confidence. + service := NewService(store, NewDeepScanner(nil), DefaultConfig()) + service.SetCommandScanningEnabled(true) + analyzer := &stubAnalyzer{ + response: `{"service_type":"pi-hole","service_name":"Pi-hole","confidence":0.95,"facts":[{"key":"core_version","value":"5.18.2","source":"pihole -v"}],"config_paths":["/etc/pihole/"],"reasoning":"identified Pi-hole"}`, + } + service.SetAIAnalyzer(analyzer) + + d, err := service.DiscoverResource(context.Background(), DiscoveryRequest{ + ResourceType: ResourceTypeSystemContainer, + TargetID: "delly", + ResourceID: "102", + Hostname: "esphome", + Force: true, + }) + if err != nil { + t.Fatalf("DiscoverResource error: %v", err) + } + if d == nil { + t.Fatal("expected a discovery result") + } + if analyzer.calls != 0 { + t.Fatalf("analyzer must not be called without command evidence; got %d calls", analyzer.calls) + } + if d.ServiceName == "Pi-hole" || d.ServiceType == "pi-hole" { + t.Fatalf("must not adopt fabricated identity, got type=%q name=%q", d.ServiceType, d.ServiceName) + } + if !strings.Contains(d.AIReasoning, "Pulse Commands") { + t.Fatalf("reasoning should guide enabling Pulse Commands, got %q", d.AIReasoning) + } +} + // readStateFromSnapshot builds a ReadState backed by a ResourceRegistry // from a local servicediscovery StateSnapshot. This lets tests populate // state using the familiar local types while exercising the ReadState path.