diff --git a/cmd/eval/main.go b/cmd/eval/main.go index d27d6f793..cdfef5ebc 100644 --- a/cmd/eval/main.go +++ b/cmd/eval/main.go @@ -9,7 +9,7 @@ // // Options: // -// -scenario string Scenario to run: smoke, readonly, enforce, routing, routing-recovery, logs, readonly-recovery, search-id, disambiguate, context-target, discovery, writeverify, strict, strict-block, strict-recovery, readonly-guardrails, noninteractive, approval, approval-approve, approval-deny, approval-combo, patrol, patrol-basic, patrol-investigation, patrol-finding-quality, patrol-signal-coverage, matrix, all (default "smoke") +// -scenario string Scenario to run: smoke, readonly, routing, routing-recovery, logs, readonly-recovery, search-id, disambiguate, context-target, discovery, writeverify, strict, strict-block, strict-recovery, readonly-guardrails, noninteractive, approval, approval-approve, approval-deny, approval-combo, patrol, patrol-basic, patrol-investigation, patrol-finding-quality, patrol-signal-coverage, matrix, all (default "smoke") // -url string Pulse API base URL (default "http://127.0.0.1:7655") // -user string Username for auth (default "admin") // -pass string Password for auth (default "admin") @@ -36,7 +36,7 @@ import ( ) func main() { - scenario := flag.String("scenario", "smoke", "Scenario to run: smoke, readonly, enforce, routing, routing-recovery, logs, readonly-recovery, search-id, disambiguate, context-target, discovery, writeverify, guest-control, guest-idempotent, guest-discovery, guest-natural, guest-multi, readonly-filtering, read-loop-recovery, ambiguous-intent, strict, strict-block, strict-recovery, readonly-guardrails, noninteractive, approval, approval-approve, approval-deny, approval-combo, patrol, patrol-basic, patrol-investigation, patrol-finding-quality, patrol-signal-coverage, matrix, all") + scenario := flag.String("scenario", "smoke", "Scenario to run: smoke, readonly, routing, routing-recovery, logs, readonly-recovery, search-id, disambiguate, context-target, discovery, writeverify, guest-control, guest-idempotent, guest-discovery, guest-natural, guest-multi, readonly-model-choice, read-loop-recovery, ambiguous-intent, strict, strict-block, strict-recovery, readonly-guardrails, noninteractive, approval, approval-approve, approval-deny, approval-combo, patrol, patrol-basic, patrol-investigation, patrol-finding-quality, patrol-signal-coverage, matrix, all") url := flag.String("url", "http://127.0.0.1:7655", "Pulse API base URL") user := flag.String("user", "admin", "Username for auth") pass := flag.String("pass", "admin", "Password for auth") @@ -179,7 +179,6 @@ func listScenarios() { fmt.Println(" Basic:") fmt.Println(" smoke - Quick smoke test (1 step)") fmt.Println(" readonly - Read-only infrastructure test (3 steps)") - fmt.Println(" enforce - Explicit tool enforcement (1 step)") fmt.Println(" routing - Routing validation test (2 steps)") fmt.Println(" routing-recovery - Routing mismatch recovery (2 steps)") fmt.Println(" logs - Log tailing/bounded command test (2 steps)") @@ -196,8 +195,8 @@ func listScenarios() { fmt.Println(" guest-natural - Natural language variations (turn off, shut down, 4 steps)") fmt.Println(" guest-multi - Multi-mention status query (2 resources, 1 step)") fmt.Println() - fmt.Println(" Safety & Filtering:") - fmt.Println(" readonly-filtering - Control tools excluded from read-only queries (3 steps)") + fmt.Println(" Safety:") + fmt.Println(" readonly-model-choice - Model chooses read tools for read-only queries (3 steps)") fmt.Println(" read-loop-recovery - Model produces text after budget blocks (2 steps)") fmt.Println(" ambiguous-intent - Ambiguous requests default to read-only (3 steps)") fmt.Println() @@ -262,8 +261,6 @@ func getScenarios(name string) []eval.Scenario { return []eval.Scenario{eval.QuickSmokeTest()} case "readonly": return []eval.Scenario{eval.ReadOnlyInfrastructureScenario()} - case "enforce": - return []eval.Scenario{eval.ExplicitToolEnforcementScenario()} case "routing": return []eval.Scenario{eval.RoutingValidationScenario()} case "routing-recovery": @@ -293,9 +290,9 @@ func getScenarios(name string) []eval.Scenario { case "guest-multi": return []eval.Scenario{eval.GuestControlMultiMentionScenario()} - // Safety & filtering scenarios - case "readonly-filtering": - return []eval.Scenario{eval.ReadOnlyToolFilteringScenario()} + // Safety scenarios + case "readonly-model-choice": + return []eval.Scenario{eval.ReadOnlyModelChoiceScenario()} case "read-loop-recovery": return []eval.Scenario{eval.ReadLoopRecoveryScenario()} case "ambiguous-intent": @@ -342,7 +339,6 @@ func getScenarios(name string) []eval.Scenario { return []eval.Scenario{ eval.QuickSmokeTest(), eval.ReadOnlyInfrastructureScenario(), - eval.ExplicitToolEnforcementScenario(), eval.RoutingValidationScenario(), eval.RoutingMismatchRecoveryScenario(), eval.LogTailingScenario(), @@ -372,7 +368,7 @@ func getScenarios(name string) []eval.Scenario { eval.GuestControlDiscoveryScenario(), eval.GuestControlNaturalLanguageScenario(), eval.GuestControlMultiMentionScenario(), - eval.ReadOnlyToolFilteringScenario(), + eval.ReadOnlyModelChoiceScenario(), eval.ReadLoopRecoveryScenario(), eval.AmbiguousIntentScenario(), eval.StrictResolutionScenario(), @@ -386,7 +382,6 @@ func getScenarios(name string) []eval.Scenario { return []eval.Scenario{ eval.QuickSmokeTest(), eval.ReadOnlyInfrastructureScenario(), - eval.ExplicitToolEnforcementScenario(), eval.RoutingValidationScenario(), eval.RoutingMismatchRecoveryScenario(), eval.LogTailingScenario(), @@ -408,7 +403,7 @@ func getScenarios(name string) []eval.Scenario { eval.GuestControlDiscoveryScenario(), eval.GuestControlNaturalLanguageScenario(), eval.GuestControlMultiMentionScenario(), - eval.ReadOnlyToolFilteringScenario(), + eval.ReadOnlyModelChoiceScenario(), eval.ReadLoopRecoveryScenario(), eval.AmbiguousIntentScenario(), eval.StrictResolutionScenario(), diff --git a/docs/release-control/v6/internal/status.json b/docs/release-control/v6/internal/status.json index 9829e6e26..12c174ed4 100644 --- a/docs/release-control/v6/internal/status.json +++ b/docs/release-control/v6/internal/status.json @@ -2855,7 +2855,7 @@ }, { "id": "RA29", - "summary": "Interactive Pulse Assistant chat behaves like a governed LLM tool surface, not a Pulse-authored intent router: the operator's selected model receives the user turn and the governed tools, decides whether tools are needed, and Pulse only enforces approvals, resource resolution, FSM gates, and tool policy after that model choice. The chat surface must echo user messages before network/session creation finishes and must not emit Pulse-owned explore or synthetic investigating workflow events before the model acts.", + "summary": "Interactive Pulse Assistant chat behaves like a governed LLM tool surface, not a Pulse-authored intent router: the operator's selected model receives the user turn and governed tools, decides whether tools are needed, and Pulse must not use prompt heuristics to force tool_choice, force a named tool, retry because an expected tool was not used, or hide tools from the model by keyword detection. Pulse only enforces approvals, resource resolution, FSM gates, and tool policy after that model choice. The chat surface must echo user messages before network/session creation finishes and must not emit Pulse-owned explore or synthetic investigating workflow events before the model acts.", "kind": "invariant", "blocking_level": "repo-ready", "proof_type": "automated", diff --git a/docs/release-control/v6/internal/subsystems/ai-runtime.md b/docs/release-control/v6/internal/subsystems/ai-runtime.md index 74336cdec..cdaf668f0 100644 --- a/docs/release-control/v6/internal/subsystems/ai-runtime.md +++ b/docs/release-control/v6/internal/subsystems/ai-runtime.md @@ -119,7 +119,7 @@ runtime cost control, and shared AI transport surfaces. ## Completion Obligations -1. Update this contract when canonical AI runtime or transport entry points move, including transport-level provider request-shape changes such as DeepSeek `tool_choice` coercion, runtime-failure classification splits (for example separating forced tool selection rejection, no tool-capable endpoint, and generic model-level lack of tool support into distinct causes), Patrol-specific verification surfaces such as `POST /api/ai/patrol/preflight` that exercise the full chat-completions path with a minimal tool definition rather than only listing models, Patrol-preflight cache observability where the AI Service caches the most recent preflight outcome (success, soft warning, or classified failure) and the AI settings response surfaces it as `patrol_preflight` so the UI can hydrate a "last verified" indicator without forcing operators to re-run preflight on every page load, the auto-trigger contract on `HandleUpdateAISettings` where the save handler runs `TriggerPatrolPreflightAsync` only when the change actually moved Patrol transport (model swap, provider key for that model changed, or assistant just enabled with a Patrol model) so routine settings saves do not burn provider tokens, the startup-seed contract where the AI Service handler dispatches the same async preflight on Pulse boot when assistant is enabled and a Patrol model is configured so the cache is populated for the first `/api/settings/ai` poll after a restart instead of blanking back to "never verified", the readiness-integration contract where the `tools` check in the Patrol readiness payload consults the cached preflight and surfaces the classified evidence (success, soft warning, or failure with classified summary plus "last preflight ") for the configured provider+model when available (falling back to the static `PatrolToolReadinessForModel` classifier only when the cache is empty or holds a result for a different model), the stateless-Patrol-input contract where `ExecutePatrolStream` must pass only the current run's user prompt into the agentic loop rather than reloading the persisted `patrol-main` session history (so a prior run that ended with orphan `tool_calls` cannot poison every subsequent run with malformed conversation structure), and the deterministic-resolve-gate contract where the `patrol_resolve_finding` tool adapter rejects LLM-driven resolves of event/persistent category findings (`backup`, `reliability`, `security`, `general`) when a deterministic verifier exists for the finding's key and that verifier either still detects the failure signal **or returns an inconclusive result** — preventing the LLM from optimistically resolving a finding its current investigation simply didn't re-surface, which was the source of the "Backup failed" flap (detected → auto-resolved → re-detected ten times in a day before this gate). The fail-closed-on-inconclusive policy treats verifier errors (timeouts, executor unavailability, transport faults) as "we don't know" rather than "go ahead": resolution of an event/persistent finding is effectively permanent (next detection registers as a regression and inflates counters), so the safe default is to refuse and require either a successful re-verification or operator action, the assessment-recovery contract where the overall-health "Recent Patrol errors" coverage factor in `summarizeRecentPatrolCoverage` suppresses the score penalty once three consecutive trailing successful full Patrol runs exist at the most-recent end of the recent-runs window — so the grade reflects current reality after a Patrol-affecting bug is fixed rather than dragging stale failures forward for the ~9 hours it takes scheduled runs to age them out of the trailing-10 ratio, the orphan-tool-call-repair contract where `convertToProviderMessages` injects synthetic is_error tool result messages for any `tool_call_id` in an assistant message that has no matching downstream tool result, so a chat session that ended mid-tool-call (network drop, ctx timeout, browser crash) cannot poison its next message with the structural-violation error the provider rejects — the synthetic content is marked is_error=true and explains the interruption so the model can retry the call or proceed without the data, and the patrol-session-bound contract where `ExecutePatrolStream` calls `SessionStore.TrimMessages` after persisting each run's messages to cap the patrol-main session at 200 messages (roughly two recent runs' worth) — without the bound the file grew unbounded at every scheduled run, reaching 16 MB and 3,593 messages within a month and making every `AddMessage` rewrite linearly more expensive; the canonical Patrol forensic log is the `PatrolRunRecord` history surfaced at `/api/ai/patrol/runs`, not the chat-session-shaped file +1. Update this contract when canonical AI runtime or transport entry points move, including transport-level provider request-shape changes such as OpenAI-compatible `tool_choice` handling, runtime-failure classification splits (for example separating tool-choice request rejection, no tool-capable endpoint, and generic model-level lack of tool support into distinct causes), Patrol-specific verification surfaces such as `POST /api/ai/patrol/preflight` that exercise the full chat-completions path with a minimal tool definition rather than only listing models, Patrol-preflight cache observability where the AI Service caches the most recent preflight outcome (success, soft warning, or classified failure) and the AI settings response surfaces it as `patrol_preflight` so the UI can hydrate a "last verified" indicator without forcing operators to re-run preflight on every page load, the auto-trigger contract on `HandleUpdateAISettings` where the save handler runs `TriggerPatrolPreflightAsync` only when the change actually moved Patrol transport (model swap, provider key for that model changed, or assistant just enabled with a Patrol model) so routine settings saves do not burn provider tokens, the startup-seed contract where the AI Service handler dispatches the same async preflight on Pulse boot when assistant is enabled and a Patrol model is configured so the cache is populated for the first `/api/settings/ai` poll after a restart instead of blanking back to "never verified", the readiness-integration contract where the `tools` check in the Patrol readiness payload consults the cached preflight and surfaces the classified evidence (success, soft warning, or failure with classified summary plus "last preflight ") for the configured provider+model when available (falling back to the static `PatrolToolReadinessForModel` classifier only when the cache is empty or holds a result for a different model), the stateless-Patrol-input contract where `ExecutePatrolStream` must pass only the current run's user prompt into the agentic loop rather than reloading the persisted `patrol-main` session history (so a prior run that ended with orphan `tool_calls` cannot poison every subsequent run with malformed conversation structure), and the deterministic-resolve-gate contract where the `patrol_resolve_finding` tool adapter rejects LLM-driven resolves of event/persistent category findings (`backup`, `reliability`, `security`, `general`) when a deterministic verifier exists for the finding's key and that verifier either still detects the failure signal **or returns an inconclusive result** — preventing the LLM from optimistically resolving a finding its current investigation simply didn't re-surface, which was the source of the "Backup failed" flap (detected → auto-resolved → re-detected ten times in a day before this gate). The fail-closed-on-inconclusive policy treats verifier errors (timeouts, executor unavailability, transport faults) as "we don't know" rather than "go ahead": resolution of an event/persistent finding is effectively permanent (next detection registers as a regression and inflates counters), so the safe default is to refuse and require either a successful re-verification or operator action, the assessment-recovery contract where the overall-health "Recent Patrol errors" coverage factor in `summarizeRecentPatrolCoverage` suppresses the score penalty once three consecutive trailing successful full Patrol runs exist at the most-recent end of the recent-runs window — so the grade reflects current reality after a Patrol-affecting bug is fixed rather than dragging stale failures forward for the ~9 hours it takes scheduled runs to age them out of the trailing-10 ratio, the orphan-tool-call-repair contract where `convertToProviderMessages` injects synthetic is_error tool result messages for any `tool_call_id` in an assistant message that has no matching downstream tool result, so a chat session that ended mid-tool-call (network drop, ctx timeout, browser crash) cannot poison its next message with the structural-violation error the provider rejects — the synthetic content is marked is_error=true and explains the interruption so the model can retry the call or proceed without the data, and the patrol-session-bound contract where `ExecutePatrolStream` calls `SessionStore.TrimMessages` after persisting each run's messages to cap the patrol-main session at 200 messages (roughly two recent runs' worth) — without the bound the file grew unbounded at every scheduled run, reaching 16 MB and 3,593 messages within a month and making every `AddMessage` rewrite linearly more expensive; the canonical Patrol forensic log is the `PatrolRunRecord` history surfaced at `/api/ai/patrol/runs`, not the chat-session-shaped file 2. Keep AI runtime and shared API proof routing aligned in `registry.json` 3. Preserve explicit coverage for chat, Patrol, remediation, and cost-control behavior when AI runtime changes Patrol runtime failures are part of that runtime contract: provider, model, @@ -978,9 +978,11 @@ Interactive Assistant chat must not put a Pulse-authored intent router, scout model, or explore pre-pass in front of the operator's selected model. The runtime may assemble governed context and expose the approved tool list, but the selected model owns the decision to answer directly, ask a question, read -context, or request an action. Pulse enforcement starts after that model choice: -approval mode, FSM gates, strict resource resolution, and tool policy remain -the safety boundary. +context, or request an action. Pulse must not use prompt heuristics to force +`tool_choice=any`, force a named tool, retry because an expected tool was not +used, or hide tools from the model based on keyword detection. Pulse +enforcement starts after that model choice: approval mode, FSM gates, strict +resource resolution, and tool policy remain the safety boundary. The same runtime ownership now includes the customer-facing AI usage and cost surface. `frontend-modern/src/components/AI/AICostDashboard.tsx` is the @@ -1725,15 +1727,11 @@ process the buffered frame set and route tool-call assembly plus final done event emission through the same canonical finalizer used for `[DONE]` instead of dropping the last chunk or leaving tool calls unfinalized on clean close. That same provider-transport boundary owns OpenAI-compatible tool protocol -adaptation. For direct DeepSeek provider paths, the shared OpenAI-compatible -client must coerce specific or required `tool_choice` values to provider- -supported auto tool selection for every DeepSeek model ID, including current -V4 tool-capable models, legacy aliases, and unknown direct DeepSeek IDs. -DeepSeek's API server-side aliases V4 IDs to `deepseek-reasoner`, which -rejects forced `tool_choice` with HTTP 400, so coercing to auto for all -direct DeepSeek paths keeps Patrol functional regardless of how DeepSeek -routes the requested ID and keeps any provider errors as model or readiness -diagnostics instead of forced-tool protocol noise. +adaptation. Pulse must keep normal tool selection automatic/model-owned for +OpenAI-compatible providers, including direct DeepSeek paths. Text-only +`tool_choice=none` remains allowed only as a safety brake when Pulse is +ending a turn after loop, budget, or verification gates; it must not be used +as an intent classifier. Reasoning-backed provider turns that return tool calls with `reasoning_content` must preserve that reasoning state on the following tool-result turn when the provider requires it, so Assistant and Patrol can complete multi-turn tool use diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index dff3ba8b6..9f0a6d3a2 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -538,6 +538,11 @@ the canonical monitored-system blocked payload. and the Patrol verification summary derived from run history, so the page also states whether recent Patrol evidence came from a successful full patrol or only from scoped/erroring runs instead of leaving verification scope implicit and the same-day activity-mix explanation derived from that governed run history, so when a recent full patrol is followed by alert-triggered or anomaly-triggered scoped work the verification surface can explain the mix directly instead of reconstructing it from page-local timing heuristics and the Patrol status recency split, so `last_patrol_at` remains reserved for completed full Patrol sweeps while scoped runs and verification checks advance `last_activity_at` without claiming a fresh full-estate verification pass + and the Patrol Assistant handoff model, so frontend handoff prompts pass + current finding context, safe action posture, and resource references as + bounded request metadata while leaving tool selection and remediation + reasoning to the configured LLM instead of serializing a frontend-authored + tool route or fix plan into the API request and the canonical alert-triggered Patrol enqueue path in `internal/api/router.go`, so alert-fired Patrol work flows through the unified alert bridge and trigger manager instead of being duplicated by monitor callback wiring and the shared `frontend-modern/src/components/Infrastructure/ResourceChangeSummary.tsx` card, so canonical recent-change timelines stay rendered through one governed frontend card instead of separate page-local list loops and the shared `frontend-modern/src/utils/resourceChangePresentation.ts` formatter used by the summary page and resource drawer, so canonical change wording does not drift across surfaces diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index 592dfe829..174f6acf6 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -249,7 +249,11 @@ button/link shape, but they must not create a second full-width action band or nest another card inside the primary Pulse surface. If the same assessment opens Assistant, the Patrol-to-Assistant handoff must carry that exact recommendation as safe bounded metadata so the drawer briefing and first-turn -prompt explain the same operator-facing priority. +prompt explain the same operator-facing priority. Feature-owned Assistant +handoff prompts may provide source context and safe metadata, but the shared +drawer boundary must not turn those prompts into frontend-authored tool routes +or remediation plans; the configured model owns tool choice and diagnostic +reasoning after the request reaches the AI runtime. 1. Add shared primitives in `frontend-modern/src/components/shared/` Framed product table surfaces must consume the shared `TableCard` frame and diff --git a/docs/release-control/v6/internal/subsystems/patrol-intelligence.md b/docs/release-control/v6/internal/subsystems/patrol-intelligence.md index becaf13d9..79c212f7a 100644 --- a/docs/release-control/v6/internal/subsystems/patrol-intelligence.md +++ b/docs/release-control/v6/internal/subsystems/patrol-intelligence.md @@ -181,9 +181,12 @@ Patrol-specific presentation helpers. decision framing, and governed approval/action posture instead of behaving like a generic chat over a pasted incident dump. Patrol is the scheduled probe, context assembler, and execution-governance owner; the configured LLM - is the diagnostic and remediation-reasoning owner. Visible Patrol Assistant - handoffs must not make the operator think Pulse has already produced the - correct fix. The visible Assistant drawer briefing opened from a Patrol + is the diagnostic and remediation-reasoning owner. Patrol handoff prompts may + provide system context, resource posture, action posture, and governed tools, + but must not force active tool use, name a required tool path, or present a + Patrol-authored remediation answer for the LLM to execute. Visible Patrol + Assistant handoffs must not make the operator think Pulse has already + produced the correct fix. The visible Assistant drawer briefing opened from a Patrol finding must be compact and source-named: current status/risk, one primary subject, and any approval-required boundary, with richer evidence, action artifacts, command counts, and prompt suggestions staying in model-only or diff --git a/frontend-modern/src/api/__tests__/patrolPreflight.test.ts b/frontend-modern/src/api/__tests__/patrolPreflight.test.ts index 90d7ba12e..0e55e6631 100644 --- a/frontend-modern/src/api/__tests__/patrolPreflight.test.ts +++ b/frontend-modern/src/api/__tests__/patrolPreflight.test.ts @@ -39,9 +39,9 @@ describe('runPatrolPreflight', () => { success: false, tool_call_observed: false, duration_ms: 312, - message: 'Provider rejected forced tool selection', + message: 'Provider rejected tool-choice request', cause: 'tool_choice_rejected', - recommendation: 'Pulse will retry with automatic tool selection on the next Patrol run.', + recommendation: 'Retry with automatic tool selection.', }); const result = await runPatrolPreflight({ provider: 'deepseek', model: 'deepseek-v4-flash' }); @@ -142,12 +142,11 @@ describe('runPatrolPreflight', () => { model: 'deepseek-v4-flash', tool_call_observed: false, duration_ms: 312, - message: 'Provider rejected forced tool selection', + message: 'Provider rejected tool-choice request', cause: 'tool_choice_rejected', summary: - 'Pulse Patrol reached the provider and the model accepts tools, but the provider rejected the specific tool-selection coercion Pulse sent.', - recommendation: - 'Pulse will retry with automatic tool selection on the next Patrol run.', + 'Pulse Patrol reached the provider and the model accepts tools, but the provider rejected a tool_choice transport field.', + recommendation: 'Retry with automatic tool selection.', recorded_at: '2026-05-10T15:30:42Z', recorded_at_unix: 1778430642, }); diff --git a/frontend-modern/src/features/patrol/__tests__/patrolInvestigationContextModel.test.ts b/frontend-modern/src/features/patrol/__tests__/patrolInvestigationContextModel.test.ts index a4f58f897..772752e1c 100644 --- a/frontend-modern/src/features/patrol/__tests__/patrolInvestigationContextModel.test.ts +++ b/frontend-modern/src/features/patrol/__tests__/patrolInvestigationContextModel.test.ts @@ -1130,12 +1130,11 @@ describe('patrolInvestigationContextModel', () => { expect(prompt).not.toContain('Investigate this Patrol finding now'); }); - it('seeds an investigate-intent prompt that instructs active tool use, not just narration', () => { + it('seeds an investigate-intent prompt that lets the model decide whether tools are needed', () => { // Investigate is the action counterpart to Explain. Where Explain says // "tell me what we know," Investigate says "go find out what's true - // right now" — the prompt must explicitly direct the LLM toward its - // Pulse tools (metrics, alerts, state) rather than just summarizing - // the attached briefing. + // right now" — the prompt should provide the available context without + // forcing a specific tool path before the model has reasoned about it. const prompt = buildPatrolAssistantFindingPrompt({ title: 'Backup job failing', subject: 'vm-101', @@ -1146,9 +1145,9 @@ describe('patrolInvestigationContextModel', () => { expect(prompt).toContain('Investigate this Patrol finding now'); expect(prompt).toContain('Backup job failing'); expect(prompt).toContain('vm-101'); - // Active-tool-use instruction — the differentiator vs Explain. - expect(prompt.toLowerCase()).toContain('actively use the pulse tools'); - expect(prompt.toLowerCase()).toMatch(/metrics|alerts|resource state|recent changes/); + // Model-owned tool choice — the differentiator vs Explain. + expect(prompt.toLowerCase()).toContain('decide whether the available pulse tools are needed'); + expect(prompt.toLowerCase()).toContain('fresh evidence'); // Synthesis instruction — root cause + confidence + safe next step. expect(prompt.toLowerCase()).toContain('root cause'); expect(prompt.toLowerCase()).toContain('confidence'); diff --git a/frontend-modern/src/features/patrol/patrolInvestigationContextModel.ts b/frontend-modern/src/features/patrol/patrolInvestigationContextModel.ts index a87aa2622..1b138953f 100644 --- a/frontend-modern/src/features/patrol/patrolInvestigationContextModel.ts +++ b/frontend-modern/src/features/patrol/patrolInvestigationContextModel.ts @@ -88,11 +88,10 @@ export interface PatrolInvestigationRecordPresentation { // 'discuss' = open-ended chat entry, operator drives. // 'explain' = action-style "do the explaining," auto-sent. -// 'investigate' = action-style "run the investigation now using tools" — -// the prompt explicitly invites the LLM to query metrics/alerts/state -// rather than just narrate the attached context, and the surface -// auto-sends the prompt so the work is already in flight on the screen -// the operator lands on. +// 'investigate' = action-style "work out what's true now" — the prompt +// gives the LLM context and lets it decide whether fresh tool evidence is +// needed before synthesis. The surface auto-sends the prompt so the work is +// already in flight on the screen the operator lands on. // 'why' = diagnostic "why did this happen" — focuses the LLM on cause // rather than current state: recent changes around detection time, // learned correlations, prior incident memory, regression history. @@ -548,9 +547,8 @@ export function buildPatrolAssistantFindingPrompt( } else if (input.intent === 'investigate') { prompt = `Investigate this Patrol finding now: "${title}" on ${subject}. ` + - 'Do not just summarize the attached context — actively use the Pulse tools available to you ' + - '(metrics, alerts, resource state, recent changes, correlations) to gather fresh evidence ' + - 'about the current state of the affected resource and any related resources. ' + + 'Use the attached context and decide whether the available Pulse tools are needed to gather fresh evidence ' + + 'about the current state of the affected resource and related resources. ' + 'Then synthesize: what is the root cause given the current evidence, ' + 'what is your confidence, what is the safe next step, and is the recommended action still right? ' + 'If any command-running step is involved, surface it for governed approval rather than executing on your own judgment.'; @@ -563,14 +561,12 @@ export function buildPatrolAssistantFindingPrompt( 'Then answer: what most likely caused this to fire now, ' + 'what evidence in the attached context supports that cause, ' + 'and what would have to be true for the cause to recur. ' + - 'If the cause requires verification through a Pulse tool call, do that; do not run anything ' + - 'that changes state without operator approval.'; + 'If the cause requires verification, decide whether the available Pulse tools are needed; do not run anything that changes state without operator approval.'; } else if (input.intent === 'verify_fix') { prompt = `Verify the fix applied to this Patrol finding: "${title}" on ${subject}. ` + 'A remediation step has been executed against this finding — confirm whether the underlying ' + - 'condition has actually cleared. Use the Pulse tools (metrics, resource state, recent alerts, ' + - 'service health) to check the current evidence against the original signal that fired this ' + + 'condition has actually cleared. Decide whether the available Pulse tools are needed to check current evidence against the original signal that fired this ' + 'finding, not against an unrelated state. ' + 'Then answer: is the condition cleared, what evidence supports that judgment, ' + 'how confident are you, and is there any residual risk the operator should monitor for. ' + diff --git a/internal/ai/chat/agentic.go b/internal/ai/chat/agentic.go index 380ed3bc4..0558215a9 100644 --- a/internal/ai/chat/agentic.go +++ b/internal/ai/chat/agentic.go @@ -260,10 +260,6 @@ func (a *AgenticLoop) executeWithTools(ctx context.Context, sessionID string, me toolsSucceededThisEpisode := false // Track if any tool executed successfully this episode writeCompletedLastTurn := false // When true, force text-only response on next turn toolBlockedLastTurn := false // When true, force text-only response after budget/loop block - preferredToolName := "" - preferredToolRetried := false - singleToolRequested := isSingleToolRequest(providerMessages) - singleToolEnforced := false // Fresh-data intent: if the user's latest message indicates they want // fresh/updated data, bypass the knowledge gate so tools re-execute. @@ -338,18 +334,14 @@ func (a *AgenticLoop) executeWithTools(ctx context.Context, sessionID string, me ExecutionID: a.executionID, } - // Determine tool_choice based on turn, intent, and explicit tool requests. - // We only force tool use when: - // 1. Tools are available - // 2. It's the first turn - // 3. The user's message indicates they need live data or an action - // This prevents forcing tool calls on conceptual questions like "What is TCP?" - forcedTextOnly := false + // Tool selection is model-owned. Pulse exposes the governed tool manifest + // and only applies text-only brakes for loop/budget/FSM safety. + textOnlySafetyBrake := false if turn >= maxTurns-1 { // Last turn before hitting the limit — force a text-only response so // the model summarizes its findings instead of silently stopping. req.ToolChoice = &providers.ToolChoice{Type: providers.ToolChoiceNone} - forcedTextOnly = true + textOnlySafetyBrake = true log.Warn(). Int("turn", turn). Int("max_turns", maxTurns). @@ -360,7 +352,7 @@ func (a *AgenticLoop) executeWithTools(ctx context.Context, sessionID string, me // Force text-only response so the model summarizes the result instead of // making more tool calls (which often return stale cached data and cause loops). req.ToolChoice = &providers.ToolChoice{Type: providers.ToolChoiceNone} - forcedTextOnly = true + textOnlySafetyBrake = true writeCompletedLastTurn = false log.Debug(). Str("session_id", sessionID). @@ -370,38 +362,11 @@ func (a *AgenticLoop) executeWithTools(ctx context.Context, sessionID string, me // The model already has the data it gathered — force it to produce a text // response instead of continuing to call tools that will just be blocked again. req.ToolChoice = &providers.ToolChoice{Type: providers.ToolChoiceNone} - forcedTextOnly = true + textOnlySafetyBrake = true toolBlockedLastTurn = false log.Debug(). Str("session_id", sessionID). Msg("[AgenticLoop] Tool calls blocked last turn — forcing text-only response") - } else if len(tools) > 0 { - if preferredToolName == "" { - preferredToolName = getPreferredTool(providerMessages, tools) - } - if preferredToolName != "" { - req.ToolChoice = &providers.ToolChoice{Type: providers.ToolChoiceTool, Name: preferredToolName} - if singleToolRequested { - singleToolEnforced = true - } - log.Debug(). - Str("session_id", sessionID). - Str("tool", preferredToolName). - Msg("[AgenticLoop] Explicit tool request - forcing tool") - } else if turn == 0 && requiresToolUse(providerMessages) { - // First turn with action intent: force the model to use a tool - req.ToolChoice = &providers.ToolChoice{Type: providers.ToolChoiceAny} - log.Debug(). - Str("session_id", sessionID). - Msg("[AgenticLoop] First turn with action intent - forcing tool use") - } else { - req.ToolChoice = &providers.ToolChoice{Type: providers.ToolChoiceAuto} - if turn == 0 { - log.Debug(). - Str("session_id", sessionID). - Msg("[AgenticLoop] First turn appears conceptual - using auto tool choice") - } - } } // Pre-request context validation: catch overflow from message history growth. @@ -440,7 +405,7 @@ func (a *AgenticLoop) executeWithTools(ctx context.Context, sessionID string, me req.Tools = nil req.ToolChoice = &providers.ToolChoice{Type: providers.ToolChoiceNone} - forcedTextOnly = true + textOnlySafetyBrake = true // Final check: if system + messages alone still exceed limit, // prune old messages as last resort. @@ -649,11 +614,11 @@ func (a *AgenticLoop) executeWithTools(ctx context.Context, sessionID string, me return resultMessages, fmt.Errorf("provider error: %w", err) } - // Guard: if we forced text-only but the model still returned tool calls + // Guard: if a safety brake requested text-only but the model still returned tool calls // (some providers like Gemini can hallucinate function calls from conversation // history even when tools are not offered in the request), strip them so the // model's text content is treated as the final response. - if forcedTextOnly && len(toolCalls) > 0 { + if textOnlySafetyBrake && len(toolCalls) > 0 { log.Warn(). Str("session_id", sessionID). Int("stripped_tool_calls", len(toolCalls)). @@ -724,19 +689,6 @@ func (a *AgenticLoop) executeWithTools(ctx context.Context, sessionID string, me // No tool calls breaks the "consecutive all-error tool turns" streak. consecutiveAllErrorTurns = 0 - // If the user explicitly requested a tool and the model didn't comply, retry once. - if preferredToolName != "" && !preferredToolRetried { - preferredToolRetried = true - - retryPrompt := fmt.Sprintf("Tool required: use %s for this request.", preferredToolName) - if len(resultMessages) > 0 { - resultMessages[len(resultMessages)-1].Content = retryPrompt - } - - turn++ - continue - } - // === FSM ENFORCEMENT GATE 2: Check if final answer is allowed === a.mu.Lock() fsm := a.sessionFSM @@ -788,8 +740,7 @@ func (a *AgenticLoop) executeWithTools(ctx context.Context, sessionID string, me } } - // Detect phantom execution: model claims to have done something without tool calls - // This is especially important for providers that can't force tool use (e.g., Ollama) + // Detect phantom execution: model claims to have done something without tool calls. // IMPORTANT: Skip this check if tools already succeeded this episode - the model is // legitimately summarizing tool results, not hallucinating. log.Debug(). @@ -834,10 +785,6 @@ func (a *AgenticLoop) executeWithTools(ctx context.Context, sessionID string, me // Phase 1: Pre-check (sequential) — FSM, loop detection, knowledge gate // Phase 2: Execute (parallel) — actual tool calls via goroutines // Phase 3: Post-process (sequential) — streaming, FSM transitions, KA extraction - if len(toolCalls) > 0 && preferredToolName != "" { - // Clear preferred tool once the model has used any tool. - preferredToolName = "" - } firstToolResultText := "" budgetBlockedThisTurn := 0 anyToolSucceededThisTurn := false @@ -1654,7 +1601,7 @@ func (a *AgenticLoop) executeWithTools(ctx context.Context, sessionID string, me a.mu.Lock() autonomousMode := a.autonomousMode a.mu.Unlock() - if !autonomousMode && !singleToolRequested && consecutiveToolOnlyTurns >= maxConsecutiveToolOnlyTurns { + if !autonomousMode && consecutiveToolOnlyTurns >= maxConsecutiveToolOnlyTurns { toolBlockedLastTurn = true log.Warn(). Int("consecutive_tool_only_turns", consecutiveToolOnlyTurns). @@ -1663,13 +1610,6 @@ func (a *AgenticLoop) executeWithTools(ctx context.Context, sessionID string, me Msg("[AgenticLoop] Consecutive tool-only turns exceeded threshold — forcing text-only response") } - if singleToolEnforced && len(toolCalls) > 0 { - // Single tool request completed - ensure we have a proper response - // Don't just return raw tool output, let ensureFinalTextResponse synthesize if needed - resultMessages = a.ensureFinalTextResponse(ctx, sessionID, resultMessages, providerMessages, callback) - return resultMessages, nil - } - // Track cumulative tool calls and inject wrap-up nudge/escalation if threshold exceeded totalToolCalls += len(toolCalls) if !wrapUpNudgeFired && totalToolCalls >= wrapUpNudgeAfterCalls { diff --git a/internal/ai/chat/agentic_additional_test.go b/internal/ai/chat/agentic_additional_test.go index 172c37fe6..f7d14c010 100644 --- a/internal/ai/chat/agentic_additional_test.go +++ b/internal/ai/chat/agentic_additional_test.go @@ -142,7 +142,7 @@ func TestEnsureFinalTextResponse(t *testing.T) { t.Fatalf("expected summary message to be appended") } if provider.lastRequest.ToolChoice == nil || provider.lastRequest.ToolChoice.Type != providers.ToolChoiceNone { - t.Fatalf("expected summary call to enforce text-only tool choice") + t.Fatalf("expected summary call to use the text-only safety brake") } provider.chatStream = func(ctx context.Context, req providers.ChatRequest, callback providers.StreamCallback) error { diff --git a/internal/ai/chat/agentic_test.go b/internal/ai/chat/agentic_test.go index 65a1606f4..45beb147c 100644 --- a/internal/ai/chat/agentic_test.go +++ b/internal/ai/chat/agentic_test.go @@ -353,60 +353,6 @@ func TestWaitForApprovalDecision(t *testing.T) { }) } -func TestRequiresToolUse(t *testing.T) { - tests := []struct { - name string - message string - expected bool - }{ - // Should require tools - action requests - {"@mention", "@jellyfin status", true}, - {"check status", "check the status of my server", true}, - {"restart request", "please restart nginx", true}, - {"status query", "is homepage running?", true}, - {"logs request", "show me the logs for influxdb", true}, - {"cpu query", "what's the cpu usage on pve-node?", true}, - {"memory query", "how much memory is traefik using?", true}, - {"container query", "list my docker containers", true}, - {"my infrastructure", "what's happening on my server?", true}, - {"troubleshoot my", "troubleshoot my plex server", true}, - {"my docker", "show me my docker containers", true}, - - // Should NOT require tools (conceptual questions) - {"what is tcp", "what is tcp?", false}, - {"explain docker", "explain how docker networking works", false}, - {"general question", "how do i configure nginx?", false}, - {"theory question", "what's the difference between lxc and vm?", false}, - {"empty message", "", false}, - {"greeting", "hello", false}, - {"thanks", "thanks for your help!", false}, - {"explain proxmox", "explain what proxmox is", false}, - {"describe kubernetes", "describe how kubernetes pods work", false}, - - // Edge cases from feedback - these are conceptual despite mentioning infra terms - {"is docker hard", "is docker networking hard?", false}, - {"best way cpu", "what's the best way to monitor CPU usage?", false}, - {"best practice", "what are the best practices for container security?", false}, - {"should i use", "should i use kubernetes or docker swarm?", false}, - - // Edge cases - conceptual patterns with action keywords should still be action - // ONLY when they reference MY specific infrastructure - {"what is status", "what is the status of my server", true}, - {"what is running", "what is running on my host", true}, - {"my cpu usage", "what is my cpu usage", true}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - messages := []providers.Message{ - {Role: "user", Content: tt.message}, - } - result := requiresToolUse(messages) - assert.Equal(t, tt.expected, result, "message: %q", tt.message) - }) - } -} - func TestHasPhantomExecution(t *testing.T) { tests := []struct { name string diff --git a/internal/ai/chat/agentic_tool_choice.go b/internal/ai/chat/agentic_tool_choice.go deleted file mode 100644 index 2d8fdd7b0..000000000 --- a/internal/ai/chat/agentic_tool_choice.go +++ /dev/null @@ -1,311 +0,0 @@ -package chat - -import ( - "strings" - - "github.com/rcourtman/pulse-go-rewrite/internal/ai/providers" -) - -// requiresToolUse determines if the user's message requires live data or an action. -// Returns true for messages that need infrastructure access (check status, restart, etc.) -// Returns false for conceptual questions (What is TCP?, How does Docker work?) -func requiresToolUse(messages []providers.Message) bool { - // Find the last user message - var lastUserContent string - for i := len(messages) - 1; i >= 0; i-- { - if messages[i].Role == "user" && messages[i].ToolResult == nil { - lastUserContent = strings.ToLower(messages[i].Content) - break - } - } - - if lastUserContent == "" { - return false - } - - // First, check for explicit conceptual question patterns - // These should NOT require tools even if they mention infrastructure terms - conceptualPatterns := []string{ - "what is ", "what's the difference", "what are the", - "explain ", "how does ", "how do i ", "how to ", - "why do ", "why does ", "why is it ", - "tell me about ", "describe ", - "can you explain", "help me understand", - "difference between", "best way to", "best practice", - "is it hard", "is it difficult", "is it easy", - "should i ", "would you recommend", "what do you think", - } - - for _, pattern := range conceptualPatterns { - if strings.Contains(lastUserContent, pattern) { - // Exception: questions about MY specific infrastructure state are action queries - // e.g., "what is the status of my server" or "what is my CPU usage" - hasMyInfra := strings.Contains(lastUserContent, "my ") || - strings.Contains(lastUserContent, "on my") || - strings.Contains(lastUserContent, "@") - hasStateQuery := strings.Contains(lastUserContent, "status") || - strings.Contains(lastUserContent, "doing") || - strings.Contains(lastUserContent, "running") || - strings.Contains(lastUserContent, "using") || - strings.Contains(lastUserContent, "usage") - - if hasMyInfra && hasStateQuery { - return true // Explicit state query about user's infrastructure - } - - // Exception: explicit resource references should trigger tools even in "tell me about" queries. - resourceNouns := []string{ - "container", "system-container", "vm", "node", "pod", "deployment", "service", "host", "cluster", - } - hasResourceNoun := false - for _, noun := range resourceNouns { - if strings.Contains(lastUserContent, noun) { - hasResourceNoun = true - break - } - } - explicitIndicator := strings.Contains(lastUserContent, "@") || - strings.Contains(lastUserContent, "\"") || - strings.Contains(lastUserContent, "-") || - strings.Contains(lastUserContent, "_") || - strings.Contains(lastUserContent, "/") - - if hasResourceNoun && explicitIndicator { - return true // Treat as action: specific resource is referenced - } - - return false - } - } - - // Pattern 1: @mentions indicate infrastructure references - if strings.Contains(lastUserContent, "@") { - return true - } - - // Pattern 2: Action verbs that require live data - // These are more specific to avoid matching conceptual discussions - actionPatterns := []string{ - // Direct action commands - "restart ", "start ", "stop ", "reboot ", "shutdown ", - "kill ", "terminate ", - // Status checks (specific phrasing) - "check ", "check the", "status of", "is it running", "is it up", "is it down", - "is running", "is stopped", "is down", - // "is X running?" pattern - " running?", " up?", " down?", " stopped?", - // Live data queries - "show me the", "list my", "list the", "list all", - "what's the cpu", "what's the memory", "what's the disk", - "cpu usage", "memory usage", "disk usage", "storage usage", - "how much memory", "how much cpu", "how much disk", - // Logs and debugging - "show logs", "show the logs", "check logs", "view logs", - "why is my", "why did my", "troubleshoot my", "debug my", "diagnose my", - // Discovery of MY resources - "where is my", "which of my", "find my", - // Questions about "my" specific infrastructure - "my server", "my container", "my vm", "my host", "my infrastructure", - "my node", "my cluster", "my proxmox", "my docker", - // Inventory-style queries - "what nodes do i have", "what proxmox nodes", - "what containers do i have", "what vms do i have", - "what is running on", "what's running on", - } - - for _, pattern := range actionPatterns { - if strings.Contains(lastUserContent, pattern) { - return true - } - } - - // Logs or journal queries should always hit tools. - if strings.Contains(lastUserContent, "logs") || - strings.Contains(lastUserContent, " log") || - strings.Contains(lastUserContent, "journal") || - strings.Contains(lastUserContent, "journald") { - return true - } - - // Default: assume conceptual question, don't force tools - return false -} - -// getPreferredTool returns a tool name if the user explicitly requested one. -// Only returns tools that are available for this request. -func getPreferredTool(messages []providers.Message, tools []providers.Tool) string { - var lastUserContent string - for i := len(messages) - 1; i >= 0; i-- { - if messages[i].Role == "user" && messages[i].ToolResult == nil { - lastUserContent = strings.ToLower(messages[i].Content) - break - } - } - if lastUserContent == "" { - return "" - } - - toolSet := make(map[string]bool, len(tools)) - for _, tool := range tools { - if tool.Name != "" { - toolSet[tool.Name] = true - } - } - - // Explicit tool mentions - explicitTools := []string{ - "pulse_read", - "pulse_control", - "pulse_query", - "pulse_discovery", - "pulse_docker", - "pulse_kubernetes", - "pulse_metrics", - "pulse_storage", - } - for _, tool := range explicitTools { - if strings.Contains(lastUserContent, tool) && toolSet[tool] { - return tool - } - } - - // Natural language aliases - if (strings.Contains(lastUserContent, "read-only tool") || strings.Contains(lastUserContent, "read only tool")) && toolSet["pulse_read"] { - return "pulse_read" - } - if strings.Contains(lastUserContent, "control tool") && toolSet["pulse_control"] { - return "pulse_control" - } - if strings.Contains(lastUserContent, "query tool") && toolSet["pulse_query"] { - return "pulse_query" - } - - // Context carryover: if we injected an explicit target and logs are requested, force pulse_read. - if strings.Contains(lastUserContent, "explicit target") && - (strings.Contains(lastUserContent, "log") || strings.Contains(lastUserContent, "journal")) && - toolSet["pulse_read"] { - return "pulse_read" - } - - return "" -} - -// isSingleToolRequest detects user instructions to use exactly one tool call. -func isSingleToolRequest(messages []providers.Message) bool { - var lastUserContent string - for i := len(messages) - 1; i >= 0; i-- { - if messages[i].Role == "user" && messages[i].ToolResult == nil { - lastUserContent = strings.ToLower(messages[i].Content) - break - } - } - if lastUserContent == "" { - return false - } - - patterns := []string{ - "only that tool once", - "only this tool once", - "call only that tool once", - "call only this tool once", - "call only that tool", - "call only this tool", - "call only one tool", - "only one tool", - "single tool", - "use only that tool", - "use only this tool", - "do not call any other tools", - "don't call any other tools", - "no other tools", - } - - for _, pattern := range patterns { - if strings.Contains(lastUserContent, pattern) { - return true - } - } - - return false -} - -// hasWriteIntent checks if the user's message contains explicit write/control intent. -// Returns true if the user is asking for an action (stop, start, restart, run command, etc.). -// Returns false if the intent is read-only (status check, logs, monitoring). -// This is used to structurally block control tools on read-only requests. -func hasWriteIntent(messages []providers.Message) bool { - var lastUserContent string - for i := len(messages) - 1; i >= 0; i-- { - if messages[i].Role == "user" && messages[i].ToolResult == nil { - lastUserContent = strings.ToLower(messages[i].Content) - break - } - } - if lastUserContent == "" { - return false - } - - // If the user explicitly forbids modifications, treat as read-only even if - // write verbs appear in the negated phrase (e.g. "do not restart/stop/edit"). - readOnlyGuardPhrases := []string{ - "do not restart", - "don't restart", - "do not stop", - "don't stop", - "do not shutdown", - "don't shutdown", - "do not shut down", - "don't shut down", - "do not edit", - "don't edit", - "do not modify", - "don't modify", - "do not change", - "don't change", - "read-only", - "read only", - "without changing anything", - "without modifying anything", - "no changes", - } - for _, phrase := range readOnlyGuardPhrases { - if strings.Contains(lastUserContent, phrase) { - return false - } - } - - // Explicit write/control action verbs (used only for autonomous mode gating) - writePatterns := []string{ - "stop ", "start ", "restart ", "reboot ", "shutdown ", "shut down", - "kill ", "terminate ", - "turn off", "turn on", "bring up", "bring down", "bring back", - "run command", "run the command", "execute ", - "using the control tool", "use pulse_control", - "using pulse_control", - // File editing - "edit ", "modify ", "change ", "update ", "write ", - "use pulse_file_edit", - // Configuration changes - "set ", "disable ", "enable ", "configure ", - "remove ", "add ", "create ", "delete ", "resize ", - "migrate ", "clone ", "move ", "assign ", - } - - for _, pattern := range writePatterns { - if strings.Contains(lastUserContent, pattern) { - return true - } - } - - return false -} - -// isWriteTool returns true if the tool name is a write/control tool that modifies infrastructure. -func isWriteTool(name string) bool { - switch name { - case "pulse_control", "pulse_docker", "pulse_file_edit": - return true - default: - return false - } -} diff --git a/internal/ai/chat/agentic_tool_choice_test.go b/internal/ai/chat/agentic_tool_choice_test.go deleted file mode 100644 index 09da7b828..000000000 --- a/internal/ai/chat/agentic_tool_choice_test.go +++ /dev/null @@ -1,35 +0,0 @@ -package chat - -import ( - "testing" - - "github.com/rcourtman/pulse-go-rewrite/internal/ai/providers" -) - -func TestHasWriteIntent_NegatedWriteVerbsRemainReadOnly(t *testing.T) { - tests := []string{ - "Give me a read-only health summary. Do not restart, stop, or edit anything.", - "Check the status, don't modify or change anything.", - "Please review this without changing anything.", - } - - for _, prompt := range tests { - if hasWriteIntent([]providers.Message{{Role: "user", Content: prompt}}) { - t.Fatalf("expected read-only intent for prompt %q", prompt) - } - } -} - -func TestHasWriteIntent_ExplicitActionStillDetected(t *testing.T) { - tests := []string{ - "Restart jellyfin now", - "Please run command 'reboot' on node-1", - "Use pulse_control to stop container 101", - } - - for _, prompt := range tests { - if !hasWriteIntent([]providers.Message{{Role: "user", Content: prompt}}) { - t.Fatalf("expected write intent for prompt %q", prompt) - } - } -} diff --git a/internal/ai/chat/context_prefetch.go b/internal/ai/chat/context_prefetch.go index 15cd2908f..9e36b6a65 100644 --- a/internal/ai/chat/context_prefetch.go +++ b/internal/ai/chat/context_prefetch.go @@ -148,8 +148,8 @@ func (p *ContextPrefetcher) Prefetch(ctx context.Context, message string, struct for _, name := range unresolvedMentions { sb.WriteString(fmt.Sprintf("'%s' was NOT found in Pulse monitoring. It is not a tracked VM, container, Docker container, or host.\n", name)) } - sb.WriteString("Do NOT use pulse_discovery to search for these resources — they are not in the system.\n") - sb.WriteString("Instead: use pulse_control directly if you know the host where the service runs, or ask the user for the location.\n") + sb.WriteString("Pulse discovery cannot resolve these names as monitored resources.\n") + sb.WriteString("If their location is already known, command-capable investigation may still apply; otherwise ask the user for the location.\n") return &PrefetchedContext{ Summary: sb.String(), @@ -1049,7 +1049,7 @@ func (p *ContextPrefetcher) formatContextSummary(mentions []ResourceMention, dis if mentionUsesDockerRouting(mention) { sb.WriteString(fmt.Sprintf("## %s (Docker container)\n", mention.Name)) - // Show the full routing chain unambiguously + // Show the full routing chain unambiguously. if mention.DockerHostType == "system-container" { sb.WriteString(fmt.Sprintf("Location: Docker on \"%s\" (container %d) on node \"%s\"\n", mention.DockerHostName, mention.DockerHostVMID, mention.NodeName)) @@ -1060,8 +1060,7 @@ func (p *ContextPrefetcher) formatContextSummary(mentions []ResourceMention, dis sb.WriteString(fmt.Sprintf("Location: Docker on host \"%s\"\n", mention.DockerHostName)) } - // THE target_host - this is the critical routing info - sb.WriteString(fmt.Sprintf(">>> target_host: \"%s\" <<<\n", mention.TargetHost)) + sb.WriteString(fmt.Sprintf("target_host: \"%s\"\n", mention.TargetHost)) // Bind mounts - clarify these are on the LXC/VM filesystem if len(mention.BindMounts) > 0 { @@ -1088,7 +1087,7 @@ func (p *ContextPrefetcher) formatContextSummary(mentions []ResourceMention, dis } } } else { - sb.WriteString("You have the resource location and target_host. Proceed directly with pulse_docker or pulse_control — do NOT call pulse_discovery.\n") + sb.WriteString("Resource location and target_host are already resolved; discovery is not required to identify this resource.\n") } sb.WriteString("\n") @@ -1104,9 +1103,9 @@ func (p *ContextPrefetcher) formatContextSummary(mentions []ResourceMention, dis if (mention.ResourceType == "system-container" || mention.ResourceType == "vm") && mention.ResourceID != "" { sb.WriteString(fmt.Sprintf("VMID: %s\n", mention.ResourceID)) if mention.SupportsControl { - sb.WriteString(fmt.Sprintf("To control this guest, use: pulse_control type=\"guest\", guest_id=\"%s\", action=\"start|stop|shutdown|restart\"\n", mention.ResourceID)) + sb.WriteString(fmt.Sprintf("Shared guest control actions are available with guest_id=\"%s\" and action=\"start|stop|shutdown|restart\".\n", mention.ResourceID)) } else { - sb.WriteString("This guest is read-only in Pulse. Do NOT use pulse_control for this resource.\n") + sb.WriteString("This guest is read-only in Pulse; shared write/control actions are unavailable for this resource.\n") } } @@ -1188,8 +1187,8 @@ func (p *ContextPrefetcher) formatContextSummary(mentions []ResourceMention, dis if hint.ref != "" { sb.WriteString(fmt.Sprintf("resource_id: \"%s\"\n", hint.ref)) } - if instruction := hint.prefetchInstruction(); instruction != "" { - sb.WriteString(instruction + "\n") + if contextLine := hint.prefetchContext(); contextLine != "" { + sb.WriteString(contextLine + "\n") } default: targetHost := firstNonEmptyTrimmed(hint.ref, mention.Name) @@ -1197,9 +1196,9 @@ func (p *ContextPrefetcher) formatContextSummary(mentions []ResourceMention, dis sb.WriteString(fmt.Sprintf("target_host: \"%s\"\n", targetHost)) } if resourceRequiresReadOnlyGuidance(mention.ResourceType, mention.SupportsControl) { - sb.WriteString("Use pulse_query or pulse_read only — do NOT call pulse_control or pulse_discovery.\n") + sb.WriteString("Capability context: this resource is read-only in Pulse; shared control and discovery routing are not valid for it.\n") } else { - sb.WriteString("Proceed directly with pulse_control — do NOT call pulse_discovery.\n") + sb.WriteString("Resource location is already resolved; discovery is not required to identify this resource.\n") } } } diff --git a/internal/ai/chat/context_prefetch_additional_test.go b/internal/ai/chat/context_prefetch_additional_test.go index aa8aa8b78..d53e229a3 100644 --- a/internal/ai/chat/context_prefetch_additional_test.go +++ b/internal/ai/chat/context_prefetch_additional_test.go @@ -562,8 +562,8 @@ func TestContextPrefetcher_FormatContextSummary_VMwareGuestStaysReadOnly(t *test if !strings.Contains(summary, "resource_id: \"app-01\"") { t.Fatalf("expected VMware guest summary to expose canonical resource_id routing, got %q", summary) } - if !strings.Contains(summary, "Use pulse_query action=get resource_id=\"app-01\"") { - t.Fatalf("expected VMware summary to direct the assistant to pulse_query, got %q", summary) + if !strings.Contains(summary, "Status, alerts, recent activity, and metrics are available through resource_id=\"app-01\"") { + t.Fatalf("expected VMware summary to expose resource_id read context, got %q", summary) } } @@ -597,8 +597,8 @@ func TestContextPrefetcher_FormatContextSummary_VMwareHostAndStorageStayReadOnly if strings.Contains(summary, "target_host:") { t.Fatalf("expected VMware host and datastore summary to avoid target_host routing, got %q", summary) } - if got := strings.Count(summary, "Use pulse_query action=get resource_id="); got != 2 { - t.Fatalf("expected resource_id-based read guidance for VMware host and datastore, got count=%d summary=%q", got, summary) + if got := strings.Count(summary, "Status, alerts, recent activity, and metrics are available through resource_id="); got != 2 { + t.Fatalf("expected resource_id-based read context for VMware host and datastore, got count=%d summary=%q", got, summary) } } @@ -628,8 +628,8 @@ func TestContextPrefetcher_FormatContextSummary_TrueNASAppUsesNativeResourceRead if !strings.Contains(summary, "resource_id: \"Nextcloud\"") { t.Fatalf("expected native TrueNAS app summary to expose resource_id routing, got %q", summary) } - if !strings.Contains(summary, "pulse_read action=logs resource_id=\"Nextcloud\"") { - t.Fatalf("expected native TrueNAS app summary to expose native log routing, got %q", summary) + if !strings.Contains(summary, "Shared reads address this resource by resource_id=\"Nextcloud\"") { + t.Fatalf("expected native TrueNAS app summary to expose native resource context, got %q", summary) } } diff --git a/internal/ai/chat/read_routing_hints.go b/internal/ai/chat/read_routing_hints.go index ad1bc8282..305840bbe 100644 --- a/internal/ai/chat/read_routing_hints.go +++ b/internal/ai/chat/read_routing_hints.go @@ -65,40 +65,40 @@ func (h readRoutingHint) targetHintSuffix() string { return "" } -func (h readRoutingHint) recentLogsInstruction(resourceLabel string) string { +func (h readRoutingHint) recentLogsContext(resourceLabel string) string { switch h.mode { case readRoutingTargetHost: if h.ref == "" { return "" } - return fmt.Sprintf("Instruction: Show logs for %s (last 50 lines). Use pulse_read action=logs target_host=\"%s\" lines=50.", resourceLabel, h.ref) + return fmt.Sprintf("Log routing context for %s: target_host=%q.", resourceLabel, h.ref) case readRoutingNativeResource: if h.ref == "" { return "" } - return fmt.Sprintf("Instruction: Show logs for %s (last 50 lines). Use pulse_read action=logs resource_id=\"%s\" lines=50.", resourceLabel, h.ref) + return fmt.Sprintf("Log routing context for %s: resource_id=%q.", resourceLabel, h.ref) case readRoutingQueryOnly: if h.ref == "" { return "" } - return fmt.Sprintf("Instruction: %s does not support shared log reads. Use pulse_query action=get resource_id=\"%s\" to inspect current status, alerts, recent activity, and metrics instead.", resourceLabel, h.ref) + return fmt.Sprintf("Read capability context for %s: shared log reads are not supported; current status, alerts, recent activity, and metrics are available through resource_id=%q.", resourceLabel, h.ref) default: return "" } } -func (h readRoutingHint) prefetchInstruction() string { +func (h readRoutingHint) prefetchContext() string { switch h.mode { case readRoutingNativeResource: if h.ref == "" { - return "Native API-backed app. Use shared resource_id-based reads instead of Docker-style target routing." + return "Native API-backed app. Shared reads address this resource by resource_id rather than Docker-style target routing." } - return fmt.Sprintf("Native API-backed app. Use pulse_read action=logs resource_id=\"%s\" or pulse_query action=config resource_id=\"%s\". Do NOT use pulse_discovery or target_host.", h.ref, h.ref) + return fmt.Sprintf("Native API-backed app. Shared reads address this resource by resource_id=%q; target_host is not valid for this resource.", h.ref) case readRoutingQueryOnly: if h.ref == "" { - return "API-backed read-only resource. Use pulse_query get/search on the shared resource identity. Do NOT use target_host, pulse_control, or pulse_discovery." + return "API-backed read-only resource. Shared reads address this resource by resource_id; target_host and control routing are not valid for this resource." } - return fmt.Sprintf("API-backed read-only resource. Use pulse_query action=get resource_id=\"%s\" for status, alerts, recent activity, and metrics. Do NOT use target_host, pulse_control, or pulse_discovery.", h.ref) + return fmt.Sprintf("API-backed read-only resource. Status, alerts, recent activity, and metrics are available through resource_id=%q; target_host and control routing are not valid for this resource.", h.ref) default: return "" } diff --git a/internal/ai/chat/service.go b/internal/ai/chat/service.go index d3a9dafc2..9a8379878 100644 --- a/internal/ai/chat/service.go +++ b/internal/ai/chat/service.go @@ -765,11 +765,11 @@ func (s *Service) ExecuteStream(ctx context.Context, req ExecuteRequest, callbac } // Run agentic loop - filteredTools := s.filterToolsForPrompt(ctx, req.Prompt, autonomousMode, false) + filteredTools := s.toolsForExecutionMode(autonomousMode, false) log.Debug(). Str("session_id", session.ID). Int("tools_count", len(filteredTools)). - Msg("[ChatService] Filtered tools, starting agentic loop") + Msg("[ChatService] Prepared governed tool manifest, starting agentic loop") // Set session-scoped FSM on agentic loop for workflow enforcement // This ensures structural guarantees: discover before write, verify after write @@ -1969,8 +1969,8 @@ func (s *Service) ExecutePatrolStream(ctx context.Context, req PatrolRequest, ca // this run's user prompt; the session is just a forensic log. messages := []Message{userMsg} - // Get all tools (patrol runs in autonomous mode) - filteredTools := s.filterToolsForPrompt(ctx, req.Prompt, true, true) + // Get governed tools for the Patrol run. + filteredTools := s.toolsForExecutionMode(true, true) // Run the agentic loop resultMessages, err := tempLoop.ExecuteWithTools(ctx, session.ID, messages, filteredTools, callback) @@ -2102,7 +2102,7 @@ func (s *Service) ListAvailableTools(ctx context.Context, prompt string) []strin return nil } - tools := s.filterToolsForPrompt(ctx, prompt, s.isAutonomousModeEnabled(), false) + tools := s.toolsForExecutionMode(s.isAutonomousModeEnabled(), false) names := make([]string, 0, len(tools)) for _, tool := range tools { if tool.Name == "" { @@ -2652,10 +2652,9 @@ func (s *Service) applyChatContextSettings() { // buildSystemPrompt builds the base system prompt for the AI. // Mode-specific context (autonomous vs controlled) is added dynamically by the AgenticLoop. // -// Philosophy: This prompt provides IDENTITY and CONTEXT only, not behavioral steering. -// Behavioral guarantees (tool use, no hallucination) are enforced structurally via: -// - tool_choice API parameter (forces tool calls when needed) -// - Phantom execution detection (catches false claims at runtime) +// Philosophy: This prompt provides identity, context, and tool policy. Tool +// selection remains model-owned; Pulse enforces safety after a model choice via +// tool policy, approvals, FSM verification gates, and phantom execution checks. func (s *Service) buildSystemPrompt() string { return `You are Pulse AI, a knowledgeable infrastructure assistant. You pair-program with the user on their homelab and infrastructure tasks. @@ -2670,22 +2669,22 @@ func (s *Service) buildSystemPrompt() string { ## DOCKER BIND MOUNTS - Container files are often mapped to host paths via bind mounts - To edit a container's config, find the bind mount and edit the host path -- Use pulse_discovery to find bind mount mappings +- Bind mount mappings may be available in Pulse discovery context or tools when needed ## TOOL SELECTION - Tool action modes and approval policies are generated from Pulse's tool registry. Treat that manifest as the source of truth for whether a tool is read-only, mixed, or write-capable. - pulse_control and pulse_file_edit are WRITE tools — they change infrastructure state. - pulse_docker, pulse_kubernetes, pulse_alerts, and pulse_knowledge are MIXED tools — their read subactions are safe, but their write or decision-recording subactions require the governed path described in the manifest. - Not every VM or container supports control. Some API-backed platforms are read-only even when the resource type is "vm" or "system-container". -- ONLY use write tools when the user explicitly asks you to perform an action. -- For status checks or monitoring, use pulse_query or pulse_read instead. -- If you are missing critical information (target, risky choice, preference), use pulse_question to ask structured questions. -- Do not use pulse_question in autonomous mode; proceed with safe defaults and clearly state assumptions instead. +- Write tools are allowed only when the user explicitly asks you to perform an action. +- Status checks and monitoring are read-oriented; do not change state unless the user asked for a state change. +- If you are missing critical information (target, risky choice, preference), pulse_question is available for structured clarification. +- pulse_question is not available in autonomous mode; proceed with safe defaults and clearly state assumptions instead. ## HOW TO RESPOND You are like a colleague doing pair programming on infrastructure tasks. Tool calls are your internal investigation — the user sees your final synthesized response. -1. INVESTIGATE THOROUGHLY: Use tools to gather the information you need. Don't stop after the first tool call if more context would help. +1. INVESTIGATE THOROUGHLY: Decide whether tool evidence is needed, then gather enough information to answer well. Don't stop after the first tool call if more context would help. 2. SYNTHESIZE YOUR FINDINGS: After using tools, explain what you learned and did. Don't just confirm "done" — provide context that helps the user understand the outcome. @@ -2885,8 +2884,8 @@ func (s *Service) injectRecentContextIfNeeded(prompt, sessionID string, messages if instructionTarget == "" { instructionTarget = primary } - if instruction := readHint.recentLogsInstruction(instructionTarget); instruction != "" { - summary += "\n" + instruction + if contextLine := readHint.recentLogsContext(instructionTarget); contextLine != "" { + summary += "\n" + contextLine } } @@ -2906,111 +2905,38 @@ func (s *Service) injectRecentContextIfNeeded(prompt, sessionID string, messages messages[lastIdx].Content = summary + "\n\n---\nExplicit target: " + primaryName + "\nUser question (targeted): " + messages[lastIdx].Content } -func (s *Service) filterToolsForPrompt(ctx context.Context, prompt string, autonomousMode bool, patrolMode bool) []providers.Tool { +func (s *Service) toolsForExecutionMode(autonomousMode bool, patrolMode bool) []providers.Tool { mcpTools := s.executor.ListTools() providerTools := ConvertMCPToolsToProvider(mcpTools) - // For patrol (autonomous mode), use config flags instead of keyword detection. - // Patrol seed context can mention all resource types, which defeats keyword filtering. + // Patrol may be scoped by explicit product configuration, but never by + // prompt keyword inference. The selected Patrol model owns tool choice. if patrolMode { filtered := s.filterToolsForPatrol(providerTools) - - // Keep write-intent gating for autonomous runs. - if !hasWriteIntent(convertPromptToMessages(prompt)) { - nonWrite := make([]providers.Tool, 0, len(filtered)) - for _, tool := range filtered { - if !isWriteTool(tool.Name) { - nonWrite = append(nonWrite, tool) - } - } - filtered = nonWrite - } - log.Debug(). Int("total_tools", len(providerTools)). - Int("filtered_tools", len(filtered)). - Bool("autonomous_patrol_filter", true). - Msg("[filterToolsForPrompt] Filtered tools for patrol using config flags") + Int("tool_manifest_count", len(filtered)). + Bool("patrol_scope_filter", true). + Msg("[toolsForExecutionMode] Built Patrol tool manifest from configured subsystem scope") return filtered } - if !autonomousMode { - tools := append([]providers.Tool{}, providerTools...) - tools = append(tools, userQuestionTool()) - log.Debug(). - Int("total_tools", len(providerTools)). - Int("filtered_tools", len(tools)). - Msg("[filterToolsForPrompt] Exposing governed interactive chat tools") - return tools - } - - // Keep write-intent gating for autonomous runs where commands execute - // without approval. Interactive chat does not use prompt-owned routing: - // the selected model sees the governed tools and chooses what to call. - readOnly := autonomousMode && !hasWriteIntent(convertPromptToMessages(prompt)) - - // Determine which specialty tools are relevant based on prompt keywords. - // Core tools are always included; specialty tools only when topic-relevant. - // This reduces token consumption on every request. - lowerPrompt := strings.ToLower(prompt) - includeK8s := promptMentionsAny(lowerPrompt, k8sKeywords) - includePMG := promptMentionsAny(lowerPrompt, pmgKeywords) - includeStorage := promptMentionsAny(lowerPrompt, storageKeywords) || promptMentionsBroadInfra(lowerPrompt) - includeDocker := promptMentionsAny(lowerPrompt, dockerKeywords) - - // If no specialty keywords detected, include everything (safe default). - noSpecialtyDetected := !includeK8s && !includePMG && !includeStorage && !includeDocker - - filtered := make([]providers.Tool, 0, len(providerTools)) - for _, tool := range providerTools { - // Remove write tools for read-only prompts - if readOnly && isWriteTool(tool.Name) { - continue - } - // Conditionally include specialty tools - if !noSpecialtyDetected && isSpecialtyTool(tool.Name) { - switch tool.Name { - case "pulse_kubernetes": - if !includeK8s { - continue - } - case "pulse_pmg": - if !includePMG { - continue - } - case "pulse_storage": - if !includeStorage { - continue - } - case "pulse_docker": - if !includeDocker && readOnly { - // Only filter docker for read-only; write-intent already implies docker may be needed - continue - } - } - } - filtered = append(filtered, tool) - } - - log.Debug(). - Int("total_tools", len(providerTools)). - Int("filtered_tools", len(filtered)). - Bool("read_only", readOnly). - Bool("specialty_filter_active", !noSpecialtyDetected). - Str("prompt_prefix", truncateForLog(prompt, 80)). - Msg("[filterToolsForPrompt] Filtered tools for prompt") - + filtered := append([]providers.Tool{}, providerTools...) // pulse_question is interactive; exclude it for autonomous runs (Pulse Patrol). if !autonomousMode { filtered = append(filtered, userQuestionTool()) } + log.Debug(). + Int("total_tools", len(providerTools)). + Int("tool_manifest_count", len(filtered)). + Bool("autonomous", autonomousMode). + Msg("[toolsForExecutionMode] Exposing governed tool manifest") return filtered } -// filterToolsForPatrol filters tools for patrol runs using AI config flags -// instead of keyword-based detection. This prevents the seed context -// (which mentions all resource types) from causing all tools to be included. +// filterToolsForPatrol applies explicit Patrol subsystem scope settings. It +// must not inspect prompt text or infer which tool the model should use. func (s *Service) filterToolsForPatrol(providerTools []providers.Tool) []providers.Tool { s.mu.RLock() cfg := s.cfg @@ -3049,69 +2975,6 @@ func (s *Service) filterToolsForPatrol(providerTools []providers.Tool) []provide return filtered } -// isSpecialtyTool returns true for tools that are only relevant to specific topics. -func isSpecialtyTool(name string) bool { - switch name { - case "pulse_kubernetes", "pulse_pmg", "pulse_storage", "pulse_docker": - return true - default: - return false - } -} - -// Keyword lists for specialty tool detection -var ( - k8sKeywords = []string{ - "k8s", "kubernetes", "kubectl", "pod", "pods", "deployment", "deployments", - "namespace", "namespaces", "replica", "replicas", "cluster", - "node pool", "daemonset", "statefulset", "ingress", "helm", - } - pmgKeywords = []string{ - "mail", "email", "spam", "pmg", "mail gateway", "postfix", - "smtp", "queue", "bounce", "quarantine", - } - storageKeywords = []string{ - "backup", "backups", "snapshot", "snapshots", "storage", "disk", - "ceph", "zfs", "raid", "replication", "pbs", "lvm", - "smart", "pool", "pools", "s3", "nfs", - } - dockerKeywords = []string{ - "docker", "container", "containers", "swarm", "compose", - "image", "images", "registry", - } -) - -// promptMentionsAny checks if the lowercased prompt contains any of the keywords. -func promptMentionsAny(lowerPrompt string, keywords []string) bool { - for _, kw := range keywords { - if strings.Contains(lowerPrompt, kw) { - return true - } - } - return false -} - -// promptMentionsBroadInfra returns true for broad infrastructure questions -// where storage tools should remain available. -func promptMentionsBroadInfra(lowerPrompt string) bool { - broadPatterns := []string{ - "infrastructure", "overview", "health check", "full status", - "everything", "all systems", "entire", - } - for _, p := range broadPatterns { - if strings.Contains(lowerPrompt, p) { - return true - } - } - return false -} - -// convertPromptToMessages wraps a prompt string into a providers.Message slice -// for use with hasWriteIntent. -func convertPromptToMessages(prompt string) []providers.Message { - return []providers.Message{{Role: "user", Content: prompt}} -} - func (s *Service) isAutonomousModeEnabled() bool { s.mu.RLock() defer s.mu.RUnlock() diff --git a/internal/ai/chat/service_patrol_additional_test.go b/internal/ai/chat/service_patrol_additional_test.go index 949b8db40..faa4688b5 100644 --- a/internal/ai/chat/service_patrol_additional_test.go +++ b/internal/ai/chat/service_patrol_additional_test.go @@ -561,12 +561,3 @@ func TestService_ExecuteMCPTool(t *testing.T) { t.Fatalf("expected error for missing resource_id") } } - -func TestIsSpecialtyTool(t *testing.T) { - if !isSpecialtyTool("pulse_storage") { - t.Fatalf("expected pulse_storage to be specialty tool") - } - if isSpecialtyTool("pulse_query") { - t.Fatalf("expected pulse_query to be non-specialty tool") - } -} diff --git a/internal/ai/chat/service_recent_context_test.go b/internal/ai/chat/service_recent_context_test.go index eae8a3362..23e26550a 100644 --- a/internal/ai/chat/service_recent_context_test.go +++ b/internal/ai/chat/service_recent_context_test.go @@ -66,11 +66,8 @@ func TestInjectRecentContextIfNeeded_InjectsSummaryAndInstruction(t *testing.T) if !strings.Contains(content, "Use target_host=\"host-1\".") { t.Fatalf("expected target host hint, got: %s", content) } - if !strings.Contains(content, "Instruction: Show logs for api (last 50 lines).") { - t.Fatalf("expected log instruction, got: %s", content) - } - if !strings.Contains(content, "Use pulse_read action=logs target_host=\"host-1\" lines=50.") { - t.Fatalf("expected log tool instruction, got: %s", content) + if !strings.Contains(content, "Log routing context for api: target_host=\"host-1\".") { + t.Fatalf("expected log routing context, got: %s", content) } if !strings.Contains(content, "Explicit target: api") { t.Fatalf("expected explicit target, got: %s", content) @@ -127,8 +124,8 @@ func TestInjectRecentContextIfNeeded_PrimaryNameFallback(t *testing.T) { if !strings.Contains(content, "Explicit target: node:alpha") { t.Fatalf("expected fallback explicit target, got: %s", content) } - if !strings.Contains(content, "Use pulse_read action=logs target_host=\"node:alpha\" lines=50.") { - t.Fatalf("expected log instruction to use primary name, got: %s", content) + if !strings.Contains(content, "Log routing context for node:alpha: target_host=\"node:alpha\".") { + t.Fatalf("expected log context to use primary name, got: %s", content) } } @@ -161,8 +158,8 @@ func TestInjectRecentContextIfNeeded_UsesResourceIDHintForTrueNASAppLogs(t *test if !strings.Contains(content, "Use resource_id=\"Nextcloud\".") { t.Fatalf("expected resource_id hint, got: %s", content) } - if !strings.Contains(content, "Use pulse_read action=logs resource_id=\"Nextcloud\" lines=50.") { - t.Fatalf("expected resource-targeted log instruction, got: %s", content) + if !strings.Contains(content, "Log routing context for Nextcloud: resource_id=\"Nextcloud\".") { + t.Fatalf("expected resource-targeted log context, got: %s", content) } } @@ -197,10 +194,10 @@ func TestInjectRecentContextIfNeeded_UsesQueryResourceIDHintForVMwareLogs(t *tes if strings.Contains(content, "target_host=\"esxi-01.lab.local\"") { t.Fatalf("expected VMware recent context to avoid target_host log routing, got: %s", content) } - if !strings.Contains(content, "does not support shared log reads") { + if !strings.Contains(content, "shared log reads are not supported") { t.Fatalf("expected VMware log limitation guidance, got: %s", content) } - if !strings.Contains(content, "Use pulse_query action=get resource_id=\"esxi-01.lab.local\"") { - t.Fatalf("expected VMware recent context to redirect to pulse_query, got: %s", content) + if !strings.Contains(content, "current status, alerts, recent activity, and metrics are available through resource_id=\"esxi-01.lab.local\"") { + t.Fatalf("expected VMware recent context to expose resource_id read context, got: %s", content) } } diff --git a/internal/ai/chat/service_test.go b/internal/ai/chat/service_test.go index 62e39e255..ce9c2fb13 100644 --- a/internal/ai/chat/service_test.go +++ b/internal/ai/chat/service_test.go @@ -435,8 +435,7 @@ func TestService_SettersAndUpdateControlSettings(t *testing.T) { assert.True(t, hasTool(service.executor.ListTools(), "pulse_control")) } -func TestService_FilterToolsForPrompt_ReadOnlyFiltersWriteTools(t *testing.T) { - // Read-only prompts should not include write/control tools. +func TestService_ToolsForExecutionMode_ExposesGovernedTools(t *testing.T) { service := NewService(Config{ AIConfig: &config.AIConfig{ControlLevel: config.ControlLevelControlled}, StateProvider: &mockStateProvider{}, @@ -448,14 +447,13 @@ func TestService_FilterToolsForPrompt_ReadOnlyFiltersWriteTools(t *testing.T) { require.True(t, hasTool(service.executor.ListTools(), "pulse_docker")) require.True(t, hasTool(service.executor.ListTools(), "pulse_query")) - // Read-only prompts in autonomous mode should exclude write tools - filtered := service.filterToolsForPrompt(context.Background(), "run uptime", true, false) - assert.False(t, hasProviderTool(filtered, "pulse_control")) - assert.False(t, hasProviderTool(filtered, "pulse_docker")) + filtered := service.toolsForExecutionMode(true, false) + assert.True(t, hasProviderTool(filtered, "pulse_control")) + assert.True(t, hasProviderTool(filtered, "pulse_docker")) assert.True(t, hasProviderTool(filtered, "pulse_query")) } -func TestService_FilterToolsForPrompt_WriteIntentIncludesWriteTools(t *testing.T) { +func TestService_ToolsForExecutionMode_InteractiveAddsQuestionTool(t *testing.T) { service := NewService(Config{ AIConfig: &config.AIConfig{ControlLevel: config.ControlLevelControlled}, StateProvider: &mockStateProvider{}, @@ -465,10 +463,11 @@ func TestService_FilterToolsForPrompt_WriteIntentIncludesWriteTools(t *testing.T require.True(t, hasTool(service.executor.ListTools(), "pulse_docker")) require.True(t, hasTool(service.executor.ListTools(), "pulse_query")) - filtered := service.filterToolsForPrompt(context.Background(), "restart vm 101", false, false) + filtered := service.toolsForExecutionMode(false, false) assert.True(t, hasProviderTool(filtered, "pulse_control")) assert.True(t, hasProviderTool(filtered, "pulse_docker")) assert.True(t, hasProviderTool(filtered, "pulse_query")) + assert.True(t, hasProviderTool(filtered, pulseQuestionToolName)) } func TestService_Execute_NonStreaming(t *testing.T) { diff --git a/internal/ai/chat/service_tooling_test.go b/internal/ai/chat/service_tooling_test.go index 2668dcd2d..ba00496c7 100644 --- a/internal/ai/chat/service_tooling_test.go +++ b/internal/ai/chat/service_tooling_test.go @@ -94,7 +94,7 @@ func toolNameSet(list []providers.Tool) map[string]bool { return set } -func TestFilterToolsForPrompt_ReadOnlyAndSpecialty(t *testing.T) { +func TestToolsForExecutionMode_InteractiveExposesGovernedTools(t *testing.T) { exec := tools.NewPulseToolExecutor(tools.ExecutorConfig{ StateProvider: fakeStateProvider{}, AgentServer: fakeAgentServer{}, @@ -110,16 +110,7 @@ func TestFilterToolsForPrompt_ReadOnlyAndSpecialty(t *testing.T) { }, } - readOnlyTools := svc.filterToolsForPrompt(context.Background(), "show node status", true, false) - readOnlySet := toolNameSet(readOnlyTools) - if readOnlySet["pulse_control"] || readOnlySet["pulse_file_edit"] || readOnlySet["pulse_docker"] { - t.Fatalf("expected write tools to be filtered for read-only prompt") - } - if !readOnlySet["pulse_storage"] || !readOnlySet["pulse_kubernetes"] || !readOnlySet["pulse_pmg"] { - t.Fatalf("expected specialty tools to remain when no specialty keyword detected") - } - - interactiveTools := svc.filterToolsForPrompt(context.Background(), "check kubernetes pods", false, false) + interactiveTools := svc.toolsForExecutionMode(false, false) interactiveSet := toolNameSet(interactiveTools) if !interactiveSet["pulse_kubernetes"] || !interactiveSet["pulse_storage"] || @@ -132,7 +123,7 @@ func TestFilterToolsForPrompt_ReadOnlyAndSpecialty(t *testing.T) { } } -func TestFilterToolsForPrompt_BroadInfraKeepsStorage(t *testing.T) { +func TestToolsForExecutionMode_AutonomousNonPatrolExposesGovernedTools(t *testing.T) { exec := tools.NewPulseToolExecutor(tools.ExecutorConfig{ StateProvider: fakeStateProvider{}, AgentServer: fakeAgentServer{}, @@ -140,13 +131,16 @@ func TestFilterToolsForPrompt_BroadInfraKeepsStorage(t *testing.T) { }) svc := &Service{executor: exec} - toolsList := svc.filterToolsForPrompt(context.Background(), "full status overview", false, false) + toolsList := svc.toolsForExecutionMode(true, false) set := toolNameSet(toolsList) if !set["pulse_storage"] { - t.Fatalf("expected storage tool to be kept for broad infrastructure prompt") + t.Fatalf("expected storage tool to be exposed without prompt keyword routing") } if !set["pulse_control"] || !set["pulse_file_edit"] || !set["pulse_docker"] { - t.Fatalf("expected interactive mode to keep write tools") + t.Fatalf("expected autonomous non-patrol mode to expose governed write tools") + } + if set[pulseQuestionToolName] { + t.Fatalf("expected autonomous mode to exclude the interactive clarification tool") } } @@ -172,7 +166,7 @@ func TestBuildSystemPrompt_DoesNotClaimGenericVMControl(t *testing.T) { } } -func TestFilterToolsForPrompt_RecoveryOnlyKeepsStorage(t *testing.T) { +func TestToolsForExecutionMode_RecoveryOnlyKeepsStorage(t *testing.T) { exec := tools.NewPulseToolExecutor(tools.ExecutorConfig{ RecoveryPointsProvider: &fakeRecoveryPointsProvider{points: []recovery.RecoveryPoint{{ ID: "pve-snapshot:before-upgrade", @@ -185,14 +179,14 @@ func TestFilterToolsForPrompt_RecoveryOnlyKeepsStorage(t *testing.T) { }) svc := &Service{executor: exec} - toolsList := svc.filterToolsForPrompt(context.Background(), "list recovery snapshots for guest 100", false, false) + toolsList := svc.toolsForExecutionMode(false, false) set := toolNameSet(toolsList) if !set["pulse_storage"] { t.Fatalf("expected storage tool to be kept when recovery points are the only storage source") } } -func TestFilterToolsForPrompt_AutonomousNonPatrol(t *testing.T) { +func TestToolsForExecutionMode_PatrolScopeUsesConfigNotPrompt(t *testing.T) { exec := tools.NewPulseToolExecutor(tools.ExecutorConfig{ StateProvider: fakeStateProvider{}, AgentServer: fakeAgentServer{}, @@ -207,15 +201,17 @@ func TestFilterToolsForPrompt_AutonomousNonPatrol(t *testing.T) { }, } - // Use write intent so read-only write-tool gating does not hide docker. - filtered := svc.filterToolsForPrompt(context.Background(), "restart docker containers and check storage pools", true, false) + filtered := svc.toolsForExecutionMode(true, true) set := toolNameSet(filtered) - if !set["pulse_docker"] { - t.Fatalf("expected pulse_docker to be included for autonomous non-patrol runs") + if set["pulse_docker"] { + t.Fatalf("expected pulse_docker to follow disabled Patrol subsystem scope") } - if !set["pulse_storage"] { - t.Fatalf("expected pulse_storage to be included for autonomous non-patrol runs") + if set["pulse_storage"] { + t.Fatalf("expected pulse_storage to follow disabled Patrol subsystem scope") + } + if !set["pulse_query"] { + t.Fatalf("expected core read/query tools to remain available to the Patrol model") } } diff --git a/internal/ai/chat/types.go b/internal/ai/chat/types.go index 5e68f96e0..8f905e7d9 100644 --- a/internal/ai/chat/types.go +++ b/internal/ai/chat/types.go @@ -1243,7 +1243,7 @@ type ResourceNotResolvedError struct { } func (e *ResourceNotResolvedError) Error() string { - return "resource not resolved: " + e.ResourceID + " - use pulse_query first to discover this resource" + return "resource not resolved: " + e.ResourceID + " - resource discovery is required before this action" } // ActionNotAllowedError indicates an action isn't permitted for a resource diff --git a/internal/ai/eval/eval.go b/internal/ai/eval/eval.go index 7a34b3695..8db847cae 100644 --- a/internal/ai/eval/eval.go +++ b/internal/ai/eval/eval.go @@ -31,7 +31,6 @@ type Config struct { // Retry behavior for transient eval failures. StepRetries int RetryOnPhantom bool - RetryOnExplicitTool bool RetryOnStreamFailure bool RetryOnEmptyResponse bool RetryOnToolErrors bool @@ -54,7 +53,6 @@ func DefaultConfig() Config { RequestTimeout: 5 * time.Minute, StepRetries: 2, RetryOnPhantom: true, - RetryOnExplicitTool: true, RetryOnStreamFailure: true, RetryOnEmptyResponse: true, RetryOnToolErrors: true, @@ -429,11 +427,6 @@ func (r *Runner) shouldRetryStep(result *StepResult, step Step) (bool, string) { } } - // If an explicit tool was requested and no tool calls occurred, retry once. - if r.config.RetryOnExplicitTool && len(result.ToolCalls) == 0 && requiresExplicitTool(step.Prompt) { - return true, "no_tool_calls_for_explicit_tool" - } - if r.config.RetryOnToolErrors && len(result.ToolCalls) > 0 && !hasSuccessfulToolCallRetry(result.ToolCalls) { if hasRetryableToolError(result.ToolCalls) { return true, "tool_error" @@ -443,32 +436,6 @@ func (r *Runner) shouldRetryStep(result *StepResult, step Step) (bool, string) { return false, "" } -func requiresExplicitTool(prompt string) bool { - prompt = strings.ToLower(prompt) - explicitTools := []string{ - "pulse_read", - "pulse_control", - "pulse_query", - "pulse_discovery", - "pulse_docker", - "pulse_kubernetes", - "pulse_metrics", - "pulse_storage", - } - for _, tool := range explicitTools { - if strings.Contains(prompt, tool) { - return true - } - } - if strings.Contains(prompt, "read-only tool") || strings.Contains(prompt, "read only tool") { - return true - } - if strings.Contains(prompt, "control tool") || strings.Contains(prompt, "query tool") { - return true - } - return false -} - func applyEvalEnvOverrides(config *Config) { if config == nil { return @@ -490,12 +457,6 @@ func applyEvalEnvOverrides(config *Config) { config.RetryOnPhantom = true } - if value, ok := envBool("EVAL_RETRY_ON_EXPLICIT_TOOL"); ok { - config.RetryOnExplicitTool = value - } else if !config.RetryOnExplicitTool { - config.RetryOnExplicitTool = true - } - if value, ok := envBool("EVAL_RETRY_ON_STREAM_FAILURE"); ok { config.RetryOnStreamFailure = value } else if !config.RetryOnStreamFailure { diff --git a/internal/ai/eval/runner_additional_test.go b/internal/ai/eval/runner_additional_test.go index 9d15035f6..514bafb4e 100644 --- a/internal/ai/eval/runner_additional_test.go +++ b/internal/ai/eval/runner_additional_test.go @@ -191,7 +191,6 @@ func TestRunner_RetryLogic_Advanced(t *testing.T) { runner.config.RetryOnRateLimit = true runner.config.RetryOnPhantom = true runner.config.RetryOnToolErrors = true - runner.config.RetryOnExplicitTool = true tests := []struct { name string @@ -237,16 +236,6 @@ func TestRunner_RetryLogic_Advanced(t *testing.T) { }, shouldRetry: false, }, - { - name: "Explicit Tool Requested but Missing", - step: Step{Prompt: "Please use pulse_read to check files"}, - result: &StepResult{ - Content: "some content", - ToolCalls: []ToolCallEvent{}, - }, - shouldRetry: true, - retryReason: "no_tool_calls_for_explicit_tool", - }, { name: "Tool Error (Retryable: timeout)", result: &StepResult{ @@ -454,7 +443,6 @@ func TestApplyEvalEnvOverrides_Comprehensive(t *testing.T) { "EVAL_HTTP_TIMEOUT": "60", "EVAL_STEP_RETRIES": "3", "EVAL_RETRY_ON_PHANTOM": "true", - "EVAL_RETRY_ON_EXPLICIT_TOOL": "true", "EVAL_RETRY_ON_STREAM_FAILURE": "false", "EVAL_RETRY_ON_EMPTY_RESPONSE": "false", "EVAL_RETRY_ON_TOOL_ERRORS": "true", @@ -477,7 +465,6 @@ func TestApplyEvalEnvOverrides_Comprehensive(t *testing.T) { assert.Equal(t, 60*time.Second, cfg.RequestTimeout) assert.Equal(t, 3, cfg.StepRetries) assert.True(t, cfg.RetryOnPhantom) - assert.True(t, cfg.RetryOnExplicitTool) assert.False(t, cfg.RetryOnStreamFailure) assert.False(t, cfg.RetryOnEmptyResponse) assert.True(t, cfg.RetryOnToolErrors) diff --git a/internal/ai/eval/runner_test.go b/internal/ai/eval/runner_test.go index 46495fab3..56416b1af 100644 --- a/internal/ai/eval/runner_test.go +++ b/internal/ai/eval/runner_test.go @@ -35,22 +35,6 @@ func TestSanitizeFilename(t *testing.T) { } } -func TestRequiresExplicitTool(t *testing.T) { - tests := []struct { - prompt string - expected bool - }{ - {"use pulse_read please", true}, - {"check the system", false}, - {"use a read-only tool", true}, - {"use a control tool", true}, - } - - for _, tc := range tests { - assert.Equal(t, tc.expected, requiresExplicitTool(tc.prompt), "Prompt: %s", tc.prompt) - } -} - func TestApplyEvalEnvOverrides(t *testing.T) { os.Setenv("EVAL_STEP_RETRIES", "5") os.Setenv("EVAL_RETRY_ON_PHANTOM", "false") diff --git a/internal/ai/eval/scenarios.go b/internal/ai/eval/scenarios.go index e39f912b7..b668b6d7c 100644 --- a/internal/ai/eval/scenarios.go +++ b/internal/ai/eval/scenarios.go @@ -162,26 +162,6 @@ func ReadOnlyInfrastructureScenario() Scenario { } } -// ExplicitToolEnforcementScenario ensures the assistant uses only the requested tool. -func ExplicitToolEnforcementScenario() Scenario { - return Scenario{ - Name: "Explicit Tool Enforcement", - Description: "Ensures explicit tool requests are followed and no extra tools are used", - Steps: []Step{ - { - Name: "List nodes with explicit tool", - Prompt: "Use pulse_query action=list type=nodes and nothing else. Return the node names.", - Assertions: []Assertion{ - AssertNoError(), - AssertAnyToolUsed(), - AssertOnlyToolsUsed("pulse_query"), - AssertToolInputContains("pulse_query", "nodes"), - }, - }, - }, - } -} - // RoutingValidationScenario tests that the assistant correctly routes commands // to containers vs their parent hosts. func RoutingValidationScenario() Scenario { @@ -1368,15 +1348,13 @@ func GuestControlMultiMentionScenario() Scenario { } } -// ReadOnlyToolFilteringScenario tests that read-only queries do NOT receive control tools. -// This validates the filterToolsForPrompt() structural fix: when the user asks a general -// monitoring question (no write verbs), pulse_control/pulse_docker/pulse_file_edit should -// not be in the tool set at all. -func ReadOnlyToolFilteringScenario() Scenario { +// ReadOnlyModelChoiceScenario tests that models choose read tools for +// read-only questions while Pulse exposes the full governed tool manifest. +func ReadOnlyModelChoiceScenario() Scenario { t := loadEvalTargets() return Scenario{ - Name: "Read-Only Tool Filtering", - Description: "Tests that control tools are excluded from read-only queries", + Name: "Read-Only Model Choice", + Description: "Tests that the selected model chooses read tools for read-only queries", Steps: []Step{ { Name: "General monitoring query", @@ -1465,10 +1443,9 @@ func ReadLoopRecoveryScenario() Scenario { } } -// AmbiguousIntentScenario tests that ambiguous requests default to read-only behavior. -// Phrases like "check on", "look at", "handle" don't contain explicit write verbs, -// so hasWriteIntent() should return false and control tools should be filtered out. -// This prevents models from interpreting vague requests as restart/stop commands. +// AmbiguousIntentScenario tests that ambiguous requests result in read-only +// model behavior. Pulse does not filter tools from the prompt; write safety +// is enforced by tool policy and approval gates after the model chooses. func AmbiguousIntentScenario() Scenario { t := loadEvalTargets() mention := StepMention{ diff --git a/internal/ai/patrol_ai.go b/internal/ai/patrol_ai.go index 8b2c93de1..4ed8418c5 100644 --- a/internal/ai/patrol_ai.go +++ b/internal/ai/patrol_ai.go @@ -1086,26 +1086,25 @@ You have access to the following tools to investigate infrastructure: You are provided with the current state of the user's infrastructure below, including resource metrics, storage health, backup status, disk health, active alerts, baselines, and connection health. This gives you a complete point-in-time snapshot without needing to query for it. -The seed context includes service identity (from discovery) and reachability data when available. Guests marked UNREACHABLE are running according to Proxmox but did not respond to ICMP ping from their host node. This may indicate a network issue, guest crash, or firewall blocking ICMP. Use pulse_read to check guest logs or pulse_discovery for service details. +The seed context includes service identity (from discovery) and reachability data when available. Guests marked UNREACHABLE are running according to Proxmox but did not respond to ICMP ping from their host node. This may indicate a network issue, guest crash, or firewall blocking ICMP. Logs or service-discovery details can help distinguish those causes when the model decides more evidence is needed. **Step 1 — Analyze the snapshot.** Scan the data for anything notable: high usage, backup gaps, disk health issues, resources above baseline, stopped resources that should be running, storage trending full, unreachable guests, etc. -**Step 2 — Investigate deeper.** For anything notable you spotted, use your tools to understand whether it's actually a problem: -- Use **pulse_metrics** with historical windows (1h, 6h, 24h) to check if a high metric is trending up or just a momentary spike. A resource at 60% and rising is more interesting than one sitting steady at 75%. -- Use **pulse_read** to check logs on resources that look unhealthy or abnormal. -- Use **pulse_storage** to check snapshot ages, replication status, or backup job details. -- Use **pulse_query** to check resource configuration for misconfigurations. -- Use **pulse_pmg** to check mail queues or spam volume if mail flow looks abnormal. +**Step 2 — Investigate deeper when needed.** For anything notable you spotted, decide whether additional tool evidence is needed before treating it as a real problem. Useful evidence may include: +- Historical metrics windows (1h, 6h, 24h) to see whether a high metric is trending up or just a momentary spike. A resource at 60% and rising is more interesting than one sitting steady at 75%. +- Logs from resources that look unhealthy or abnormal. +- Snapshot ages, replication status, RAID details, or backup job details. +- Resource configuration details that could explain misconfiguration. +- Mail queue or spam-volume data if mail flow looks abnormal. **Step 3 — Report or resolve findings.** Report findings for confirmed issues. Resolve active findings that are no longer issues based on current data. Always call patrol_get_findings before reporting or resolving findings. -The snapshot eliminates routine data gathering, but you must still investigate to distinguish real problems from noise. Do not skip investigation — a snapshot alone cannot tell you whether a metric is stable or rapidly changing. +The snapshot eliminates routine data gathering. When a notable signal needs current or historical confirmation, gather enough evidence to distinguish real problems from noise before reporting it. ## Efficiency Rules - Do NOT call the same tool with the same parameters twice in a single patrol run. - Keep track of what you've already checked. If you've already retrieved metrics for a resource, use the data you have. -- Always call at least one investigation tool (pulse_query, pulse_metrics, pulse_storage, or pulse_read) in every patrol run, even if everything appears healthy. ## Severity & Thresholds @@ -1167,7 +1166,7 @@ Keep the summary factual, terse, and scannable. Do NOT repeat your investigation ## Auto-Fix Mode -Auto-fix is enabled. You may use pulse_control and pulse_read tools to attempt automatic remediation. +Auto-fix is enabled. Governed read and control tools are available for remediation when the model determines they are needed. Safe operations you can perform autonomously: - Restart services (systemctl restart) @@ -1185,18 +1184,14 @@ Always: ## Observe Only Mode -You are in observation mode. Use read-only tools to gather diagnostic information but DO NOT modify anything. Report findings with clear recommendations for the user to review and action manually.` +You are in observation mode. Read-only diagnostic evidence is available, but do not modify anything. Report findings with clear recommendations for the user to review and action manually.` } const triageSystemPreamble = `You are Pulse Patrol, an autonomous infrastructure analysis agent. Deterministic triage has already scanned all resources against thresholds, baselines, backup schedules, disk health, and connectivity. The flagged items are listed in your seed context under "Deterministic Triage Results". -Your job is to investigate each flagged item deeper using tools: -- Use pulse_metrics with historical windows to check if an elevated metric is trending up or stable -- Use pulse_read to check logs on flagged resources -- Use pulse_storage to verify backup/replication/RAID details -- Use pulse_query to check resource configuration +Your job is to assess each flagged item. Available evidence sources include historical metrics, logs, backup/replication/RAID details, and resource configuration. After investigation, report confirmed issues via patrol_report_finding and resolve any active findings that are no longer problems. diff --git a/internal/ai/patrol_ai_more_test.go b/internal/ai/patrol_ai_more_test.go index 2ef26d8ad..ae04d0161 100644 --- a/internal/ai/patrol_ai_more_test.go +++ b/internal/ai/patrol_ai_more_test.go @@ -193,7 +193,7 @@ func TestGetPatrolSystemPrompt_ModeSwitch(t *testing.T) { svc := &Service{cfg: &config.AIConfig{PatrolAutoFix: true}} ps := NewPatrolService(svc, nil) prompt := ps.getPatrolSystemPrompt() - if !strings.Contains(prompt, "Auto-Fix Mode") || !strings.Contains(prompt, "pulse_control") { + if !strings.Contains(prompt, "Auto-Fix Mode") || !strings.Contains(prompt, "Governed read and control tools are available") { t.Fatalf("expected auto-fix prompt, got: %s", prompt) } diff --git a/internal/ai/patrol_preflight.go b/internal/ai/patrol_preflight.go index 79e26b70a..4b91b9ead 100644 --- a/internal/ai/patrol_preflight.go +++ b/internal/ai/patrol_preflight.go @@ -15,10 +15,10 @@ import ( // // Unlike a connection test, which only lists models, the preflight // exercises the full chat-completions path with a minimal tool -// definition. This surfaces real failure modes — provider rejecting the -// tool_choice value, no tool-capable endpoint available, model genuinely -// lacking tool support — at configuration time instead of waiting for -// the next scheduled Patrol run to silently fail. +// definition. This surfaces real failure modes — no tool-capable endpoint +// available, model genuinely lacking tool support, or the model declining +// the tool — at configuration time instead of waiting for the next +// scheduled Patrol run to silently fail. type PatrolPreflightResult struct { Success bool Provider string @@ -197,8 +197,7 @@ func (s *Service) RunPatrolToolPreflight(ctx context.Context, providerName, mode }, }, }, - ToolChoice: &providers.ToolChoice{Type: providers.ToolChoiceAny}, - MaxTokens: 256, + MaxTokens: 256, } resp, err := provider.Chat(ctx, req) diff --git a/internal/ai/patrol_preflight_test.go b/internal/ai/patrol_preflight_test.go index e40062719..ce9268e3b 100644 --- a/internal/ai/patrol_preflight_test.go +++ b/internal/ai/patrol_preflight_test.go @@ -327,10 +327,8 @@ func TestTriggerPatrolPreflightAsync_PopulatesCacheInBackground(t *testing.T) { } func TestRunPatrolToolPreflight_RequestShapeIncludesVerifyTool(t *testing.T) { - // Locks the preflight request shape so a future refactor can't - // silently strip the tool definition or tool_choice (which would - // turn preflight into a connection-test that misses the original - // failure mode). + // Locks the preflight request shape so a future refactor can't silently + // strip the tool definition, while keeping tool selection model-owned. var captured map[string]interface{} h := newPatrolPreflightTestService(t, "openai:gpt-4o-mini", func(w http.ResponseWriter, r *http.Request) { _ = json.NewDecoder(r.Body).Decode(&captured) @@ -350,7 +348,7 @@ func TestRunPatrolToolPreflight_RequestShapeIncludesVerifyTool(t *testing.T) { if fn["name"] != patrolPreflightToolName { t.Fatalf("expected tool name %q, got %v", patrolPreflightToolName, fn["name"]) } - if captured["tool_choice"] == nil { - t.Fatalf("expected tool_choice to be set in preflight request, got %v", captured) + if captured["tool_choice"] != nil { + t.Fatalf("expected preflight to keep tool selection automatic, got %v", captured["tool_choice"]) } } diff --git a/internal/ai/patrol_runtime_failure.go b/internal/ai/patrol_runtime_failure.go index cb40f1c00..3257d9876 100644 --- a/internal/ai/patrol_runtime_failure.go +++ b/internal/ai/patrol_runtime_failure.go @@ -76,10 +76,9 @@ type PatrolRuntimeFailureDiagnostic struct { } // patrolToolChoiceValueRejected reports whether the upstream error indicates -// the provider rejected the specific tool_choice value Pulse sent (for -// example, "deepseek-reasoner does not support this tool_choice"). This is -// distinct from the model truly lacking tool support: the model accepts -// tools but not the requested coercion. +// the provider rejected a tool_choice transport field. This is distinct from +// the model truly lacking tool support: the provider accepted tools but not +// that request-shape detail. func patrolToolChoiceValueRejected(lower string) bool { if !strings.Contains(lower, "tool_choice") { return false @@ -153,11 +152,11 @@ func patrolRuntimeFailureFromError(err error) patrolRuntimeFailure { failure.Description = "Pulse Patrol reached the provider, but the conversation it sent had an assistant message containing tool_calls without matching tool result messages for every tool_call_id. The provider rejects this structure. This usually means a previous Patrol run ended after the model emitted tool calls but before all results were captured, leaving orphan tool_calls in persisted state that the next run reused." failure.Recommendation = "Pulse should treat each Patrol run as stateless. If the failure persists across runs, restart Pulse to clear any in-memory session state and report the issue." case patrolToolChoiceValueRejected(lower): - failure.Title = "Pulse Patrol: Provider rejected forced tool selection" - failure.Summary = "Provider rejected forced tool selection" + failure.Title = "Pulse Patrol: Provider rejected tool-choice request" + failure.Summary = "Provider rejected tool-choice request" failure.Cause = PatrolFailureCauseToolChoiceRejected - failure.Description = "Pulse Patrol reached the provider and the model accepts tools, but the provider rejected the specific tool-selection coercion Pulse sent. This usually means the routed model accepts tools yet does not honour a request to force a particular tool, only automatic selection." - failure.Recommendation = "Pulse will retry with automatic tool selection on the next Patrol run. If the failure persists, switch Patrol to a different model or provider where forced tool selection is accepted, or report the model in question." + failure.Description = "Pulse Patrol reached the provider and the model accepts tools, but the provider rejected a tool_choice transport field. Patrol should keep model tool selection automatic and avoid provider-specific coercion." + failure.Recommendation = "Retry with automatic tool selection. If the failure persists, switch Patrol to a model or provider route with reliable tool-call support, or report the model in question." case patrolNoToolCapableEndpoint(lower): failure.Title = "Pulse Patrol: No tool-capable provider endpoint available" failure.Summary = "No tool-capable provider endpoint available" @@ -267,7 +266,7 @@ func summarizePatrolRuntimeFailureDetail(raw string) string { case patrolMalformedToolHistory(lower): return "Pulse sent a malformed tool-call conversation. Each Patrol run should be stateless; restart Pulse if the failure persists." case patrolToolChoiceValueRejected(lower): - return "Provider rejected Pulse's forced tool selection. Pulse will retry with automatic tool selection on the next Patrol run." + return "Provider rejected a tool-choice transport setting. Patrol should use automatic model-owned tool selection." case patrolNoToolCapableEndpoint(lower): return "Provider has no tool-capable endpoint for the selected model. Review provider routing or privacy filters." case strings.Contains(lower, "tool_choice") || diff --git a/internal/ai/patrol_runtime_failure_test.go b/internal/ai/patrol_runtime_failure_test.go index ab07c8ea2..0b5a3b0f8 100644 --- a/internal/ai/patrol_runtime_failure_test.go +++ b/internal/ai/patrol_runtime_failure_test.go @@ -69,17 +69,17 @@ func TestPatrolRuntimeFailureFromError_ClassifiesNoToolCapableEndpoint(t *testin } func TestPatrolRuntimeFailureFromError_ClassifiesToolChoiceValueRejected(t *testing.T) { - // Direct DeepSeek path: provider accepts tools but rejects forced - // tool_choice. Pre-fix this misclassified as "model does not support - // tools" and pointed operators at the wrong remediation. + // Direct DeepSeek path: provider accepts tools but rejects the + // tool_choice request shape. Keep this distinct from "model does not + // support tools" so operators see the right remediation. err := errors.New(`agentic patrol failed: provider error: API error (400): deepseek-reasoner does not support this tool_choice`) failure := patrolRuntimeFailureFromError(err) - if failure.Title != "Pulse Patrol: Provider rejected forced tool selection" { + if failure.Title != "Pulse Patrol: Provider rejected tool-choice request" { t.Fatalf("unexpected title %q", failure.Title) } - if failure.Summary != "Provider rejected forced tool selection" { + if failure.Summary != "Provider rejected tool-choice request" { t.Fatalf("unexpected summary %q", failure.Summary) } if failure.Cause != PatrolFailureCauseToolChoiceRejected { diff --git a/internal/ai/providers/anthropic.go b/internal/ai/providers/anthropic.go index 555e1e925..e8b9f39ed 100644 --- a/internal/ai/providers/anthropic.go +++ b/internal/ai/providers/anthropic.go @@ -72,11 +72,10 @@ type anthropicRequest struct { ToolChoice *anthropicToolChoice `json:"tool_choice,omitempty"` } -// anthropicToolChoice controls how Claude selects tools -// See: https://docs.anthropic.com/en/docs/build-with-claude/tool-use/implement-tool-use#forcing-tool-use +// anthropicToolChoice controls whether tools are available or disabled. +// Pulse uses automatic selection by default and none only as a safety brake. type anthropicToolChoice struct { - Type string `json:"type"` // "auto", "any", "tool", or "none" - Name string `json:"name,omitempty"` // Only used when Type is "tool" + Type string `json:"type"` // "auto" or "none" } type anthropicMessage struct { @@ -256,14 +255,11 @@ func (c *AnthropicClient) Chat(ctx context.Context, req ChatRequest) (*ChatRespo anthropicReq.Tools[len(anthropicReq.Tools)-1].CacheControl = &anthropicCacheControl{Type: "ephemeral"} } - // Add tool_choice if specified - // This controls whether Claude MUST use tools vs just being able to - // See: https://docs.anthropic.com/en/docs/build-with-claude/tool-use/implement-tool-use#forcing-tool-use - // Anthropic may reject tool_choice if tools are not provided. + // Add tool_choice if specified. Pulse only sends automatic selection or + // text-only safety brakes. if shouldAddTools && req.ToolChoice != nil { anthropicReq.ToolChoice = &anthropicToolChoice{ Type: string(req.ToolChoice.Type), - Name: req.ToolChoice.Name, } } @@ -645,12 +641,11 @@ func (c *AnthropicClient) ChatStream(ctx context.Context, req ChatRequest, callb anthropicReq.Tools[len(anthropicReq.Tools)-1].CacheControl = &anthropicCacheControl{Type: "ephemeral"} } - // Add tool_choice if specified (same as non-streaming) - // Anthropic may reject tool_choice if tools are not provided. + // Add tool_choice if specified (same as non-streaming). Pulse only sends + // automatic selection or text-only safety brakes. if shouldAddTools && req.ToolChoice != nil { anthropicReq.ToolChoice = &anthropicToolChoice{ Type: string(req.ToolChoice.Type), - Name: req.ToolChoice.Name, } } diff --git a/internal/ai/providers/gemini.go b/internal/ai/providers/gemini.go index d8e04e278..c5733a0fa 100644 --- a/internal/ai/providers/gemini.go +++ b/internal/ai/providers/gemini.go @@ -257,26 +257,14 @@ func sanitizeGeminiContents(contents []geminiContent) []geminiContent { return result } -// convertToolChoiceToGemini converts our ToolChoice to Gemini's mode string -// Gemini uses: AUTO (default), ANY (force tool use), NONE (no tools) +// convertToolChoiceToGemini converts our ToolChoice to Gemini's mode string. +// Pulse omits automatic tool config so tool use stays model-owned. // See: https://ai.google.dev/api/caching#FunctionCallingConfig func convertToolChoiceToGemini(tc *ToolChoice) string { - if tc == nil { - return "AUTO" - } - switch tc.Type { - case ToolChoiceAuto: - return "AUTO" - case ToolChoiceNone: + if tc != nil && tc.Type == ToolChoiceNone { return "NONE" - case ToolChoiceAny: - return "ANY" - case ToolChoiceTool: - // Gemini doesn't support forcing a specific tool, fall back to ANY - return "ANY" - default: - return "AUTO" } + return "" } // Chat sends a chat request to the Gemini API @@ -407,13 +395,10 @@ func (c *GeminiClient) Chat(ctx context.Context, req ChatRequest) (*ChatResponse } if len(funcDecls) > 0 { geminiReq.Tools = []geminiToolDef{{FunctionDeclarations: funcDecls}} - - // Add tool_config based on ToolChoice - // Gemini uses: AUTO (default), ANY (force tool use), NONE (no tools) - geminiReq.ToolConfig = &geminiToolConfig{ - FunctionCallingConfig: &geminiFunctionCallingConfig{ - Mode: convertToolChoiceToGemini(req.ToolChoice), - }, + if mode := convertToolChoiceToGemini(req.ToolChoice); mode != "" { + geminiReq.ToolConfig = &geminiToolConfig{ + FunctionCallingConfig: &geminiFunctionCallingConfig{Mode: mode}, + } } log.Debug().Int("tool_count", len(funcDecls)).Strs("tool_names", func() []string { @@ -806,12 +791,10 @@ func (c *GeminiClient) ChatStream(ctx context.Context, req ChatRequest, callback } if len(funcDecls) > 0 { geminiReq.Tools = []geminiToolDef{{FunctionDeclarations: funcDecls}} - - // Add tool_config based on ToolChoice (same as non-streaming) - geminiReq.ToolConfig = &geminiToolConfig{ - FunctionCallingConfig: &geminiFunctionCallingConfig{ - Mode: convertToolChoiceToGemini(req.ToolChoice), - }, + if mode := convertToolChoiceToGemini(req.ToolChoice); mode != "" { + geminiReq.ToolConfig = &geminiToolConfig{ + FunctionCallingConfig: &geminiFunctionCallingConfig{Mode: mode}, + } } // Log tool names for debugging tool selection issues diff --git a/internal/ai/providers/openai.go b/internal/ai/providers/openai.go index 08d9a3f39..468c173da 100644 --- a/internal/ai/providers/openai.go +++ b/internal/ai/providers/openai.go @@ -89,7 +89,7 @@ type openaiRequest struct { MaxCompletionTokens int `json:"max_completion_tokens,omitempty"` // For o1/o3 models Temperature float64 `json:"temperature,omitempty"` Tools []openaiTool `json:"tools,omitempty"` - ToolChoice interface{} `json:"tool_choice,omitempty"` // "auto", "none", or specific tool + ToolChoice interface{} `json:"tool_choice,omitempty"` // "auto" or "none" } // openaiTool represents a function tool in OpenAI format @@ -180,37 +180,6 @@ func (c *OpenAIClient) shouldSendReasoningContent() bool { return c.isDeepSeek() } -func (c *OpenAIClient) supportsForcedToolChoice(model string) bool { - // Empirical: DeepSeek's API returns HTTP 400 ("provider rejected - // forced tool selection") when tool_choice=required is sent against - // the canonical chat models, including deepseek-v4-flash. DeepSeek's - // official chat-completion docs list `required` as a supported value - // with no model-specific restriction (verified at - // https://api-docs.deepseek.com/api/create-chat-completion on - // 2026-05-11), but their server behavior disagrees with their docs. - // A live preflight against deepseek-v4-flash with required produced - // `cause: tool_choice_rejected` in 275ms — a deterministic 400, not - // a transient. The legacy `deepseek-reasoner` id is documented as a - // thinking-mode alias for v4-flash on the pricing page and shares - // the same behavior. - // - // Until DeepSeek's API matches their docs, coerce forced tool_choice - // to "auto" for any DeepSeek model so Patrol stays functional. With - // auto, the model is free to skip the tool call and preflight may - // record tool_call_observed=false; that is handled as a soft warning - // ("Model did not emit a tool call during preflight") rather than a - // hard failure, which is the correct surface for a model that - // supports tools but is not under forced selection. - return !c.isDeepSeek() -} - -func (c *OpenAIClient) toolChoiceForModel(model string, choice *ToolChoice) interface{} { - if !c.supportsForcedToolChoice(model) { - return "auto" - } - return convertToolChoiceToOpenAI(choice) -} - func normalizeOpenAICompatibleModelName(model string) string { model = strings.ToLower(strings.TrimSpace(model)) for _, prefix := range []string{"openai:", "openrouter:", "deepseek:"} { @@ -233,32 +202,15 @@ func (c *OpenAIClient) requiresMaxCompletionTokens(model string) bool { return strings.HasPrefix(model, "o1") || strings.HasPrefix(model, "o3") || strings.HasPrefix(model, "o4") } -// convertToolChoiceToOpenAI converts our ToolChoice to OpenAI's format -// OpenAI uses "required" instead of Anthropic's "any" to force tool use +// convertToolChoiceToOpenAI converts our ToolChoice to OpenAI's format. +// Pulse omits automatic tool_choice so tool use stays model-owned, and only +// uses text-only safety brakes when tools are removed from the request. // See: https://platform.openai.com/docs/api-reference/chat/create#chat-create-tool_choice func convertToolChoiceToOpenAI(tc *ToolChoice) interface{} { - if tc == nil { - return "auto" - } - switch tc.Type { - case ToolChoiceAuto: - return "auto" - case ToolChoiceNone: + if tc != nil && tc.Type == ToolChoiceNone { return "none" - case ToolChoiceAny: - // OpenAI uses "required" to force the model to use one of the provided tools - return "required" - case ToolChoiceTool: - // Force a specific tool - return map[string]interface{}{ - "type": "function", - "function": map[string]string{ - "name": tc.Name, - }, - } - default: - return "auto" } + return nil } // Chat sends a chat request to the OpenAI API @@ -379,12 +331,9 @@ func (c *OpenAIClient) Chat(ctx context.Context, req ChatRequest) (*ChatResponse }) } if len(openaiReq.Tools) > 0 { - // Map ToolChoice to OpenAI format - // OpenAI uses "required" instead of Anthropic's "any" - // DeepSeek V4 supports the OpenAI tool_choice contract. Unknown - // direct DeepSeek model IDs fall back to auto so provider errors - // become model/readiness diagnostics instead of forced-tool noise. - openaiReq.ToolChoice = c.toolChoiceForModel(model, req.ToolChoice) + if toolChoice := convertToolChoiceToOpenAI(req.ToolChoice); toolChoice != nil { + openaiReq.ToolChoice = toolChoice + } } } @@ -786,11 +735,9 @@ func (c *OpenAIClient) ChatStream(ctx context.Context, req ChatRequest, callback }) } if len(openaiReq.Tools) > 0 { - // Map ToolChoice to OpenAI format (same as non-streaming) - // DeepSeek V4 supports the OpenAI tool_choice contract. Unknown - // direct DeepSeek model IDs fall back to auto so provider errors - // become model/readiness diagnostics instead of forced-tool noise. - openaiReq.ToolChoice = c.toolChoiceForModel(model, req.ToolChoice) + if toolChoice := convertToolChoiceToOpenAI(req.ToolChoice); toolChoice != nil { + openaiReq.ToolChoice = toolChoice + } } } diff --git a/internal/ai/providers/openai_test.go b/internal/ai/providers/openai_test.go index ec4894f93..04029d845 100644 --- a/internal/ai/providers/openai_test.go +++ b/internal/ai/providers/openai_test.go @@ -400,115 +400,6 @@ func TestOpenAIClient_ChatStream_ToolChoiceNone_DropsTools(t *testing.T) { assert.True(t, doneCalled) } -func TestOpenAIClient_Chat_DeepSeekCoercesForcedToolChoiceToAuto(t *testing.T) { - // Empirical: DeepSeek's API rejects tool_choice=required with HTTP 400 - // for the canonical chat models (verified via live preflight against - // deepseek-v4-flash on 2026-05-11, deterministic 400 in 275ms). DeepSeek's - // official docs claim required is supported, but server behavior - // disagrees. Pulse coerces forced tool_choice to "auto" for any DeepSeek - // model so Patrol stays functional; the soft "model did not emit a tool - // call" warning surface handles auto-mode preflight outcomes correctly. - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - var got map[string]interface{} - require.NoError(t, json.NewDecoder(r.Body).Decode(&got)) - assert.Equal(t, "deepseek-v4-flash", got["model"]) - assert.Equal(t, "auto", got["tool_choice"]) - require.Len(t, got["tools"], 1) - - _ = json.NewEncoder(w).Encode(openaiResponse{ - ID: "chatcmpl-deepseek-tools", - Model: "deepseek-v4-flash", - Choices: []openaiChoice{ - { - Message: openaiRespMsg{Role: "assistant", Content: "ok"}, - FinishReason: "stop", - }, - }, - }) - })) - defer server.Close() - - client := NewOpenAIClient("sk-test", "deepseek-v4-flash", server.URL, 0) - client.baseURL = strings.TrimSuffix(server.URL, "/") + "/v1/chat/completions#deepseek.com" - - _, err := client.Chat(context.Background(), ChatRequest{ - Messages: []Message{{Role: "user", Content: "Use the tool"}}, - Tools: []Tool{ - {Name: "ping", Description: "Ping", InputSchema: map[string]interface{}{"type": "object"}}, - }, - ToolChoice: &ToolChoice{Type: ToolChoiceTool, Name: "ping"}, - }) - require.NoError(t, err) -} - -func TestOpenAIClient_ChatStream_DeepSeekCoercesAnyToolChoiceToAuto(t *testing.T) { - // Streaming variant of the same coercion. See the non-streaming test - // for the empirical-evidence note. - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - var got map[string]interface{} - require.NoError(t, json.NewDecoder(r.Body).Decode(&got)) - assert.Equal(t, "deepseek-v4-flash", got["model"]) - assert.Equal(t, "auto", got["tool_choice"]) - require.Len(t, got["tools"], 1) - - w.Header().Set("Content-Type", "text/event-stream") - w.WriteHeader(http.StatusOK) - fmt.Fprintf(w, "data: %s\n\n", `{"id":"chatcmpl-1","choices":[{"delta":{"content":"ok"}}],"object":"chat.completion.chunk"}`) - fmt.Fprintf(w, "data: [DONE]\n\n") - w.(http.Flusher).Flush() - })) - defer server.Close() - - client := NewOpenAIClient("sk-test", "deepseek-v4-flash", server.URL, 0) - client.baseURL = strings.TrimSuffix(server.URL, "/") + "/v1/chat/completions#deepseek.com" - - err := client.ChatStream(context.Background(), ChatRequest{ - Messages: []Message{{Role: "user", Content: "Use the tool"}}, - Tools: []Tool{ - {Name: "ping", Description: "Ping", InputSchema: map[string]interface{}{"type": "object"}}, - }, - ToolChoice: &ToolChoice{Type: ToolChoiceAny}, - }, func(event StreamEvent) {}) - require.NoError(t, err) -} - -func TestOpenAIClient_Chat_UnknownDeepSeekModelCoercesForcedToolChoiceToAuto(t *testing.T) { - // Unknown DeepSeek model ids share the canonical-models' coercion - // behavior. Until DeepSeek's documented tool_choice=required support - // actually works server-side, every DeepSeek request gets auto. - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - var got map[string]interface{} - require.NoError(t, json.NewDecoder(r.Body).Decode(&got)) - assert.Equal(t, "deepseek-v4-flush7pro", got["model"]) - assert.Equal(t, "auto", got["tool_choice"]) - require.Len(t, got["tools"], 1) - - _ = json.NewEncoder(w).Encode(openaiResponse{ - ID: "chatcmpl-deepseek-unknown-tools", - Model: "deepseek-v4-flush7pro", - Choices: []openaiChoice{ - { - Message: openaiRespMsg{Role: "assistant", Content: "ok"}, - FinishReason: "stop", - }, - }, - }) - })) - defer server.Close() - - client := NewOpenAIClient("sk-test", "deepseek-v4-flush7pro", server.URL, 0) - client.baseURL = strings.TrimSuffix(server.URL, "/") + "/v1/chat/completions#deepseek.com" - - _, err := client.Chat(context.Background(), ChatRequest{ - Messages: []Message{{Role: "user", Content: "Use the tool"}}, - Tools: []Tool{ - {Name: "ping", Description: "Ping", InputSchema: map[string]interface{}{"type": "object"}}, - }, - ToolChoice: &ToolChoice{Type: ToolChoiceTool, Name: "ping"}, - }) - require.NoError(t, err) -} - func TestOpenAIClient_Chat_DeepSeekPreservesReasoningContentForToolTurns(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { var got openaiRequest @@ -728,7 +619,7 @@ func TestOpenAIClient_Chat_Success(t *testing.T) { require.Len(t, req.Tools, 1) assert.Equal(t, "function", req.Tools[0].Type) assert.Equal(t, "get_time", req.Tools[0].Function.Name) - assert.Equal(t, "auto", req.ToolChoice) + assert.Nil(t, req.ToolChoice) _ = json.NewEncoder(w).Encode(openaiResponse{ ID: "chatcmpl-1", diff --git a/internal/ai/providers/provider.go b/internal/ai/providers/provider.go index b70e6bcab..3b21b3a5e 100644 --- a/internal/ai/providers/provider.go +++ b/internal/ai/providers/provider.go @@ -79,20 +79,13 @@ func (t Tool) NormalizeCollections() Tool { type ToolChoiceType string const ( - // ToolChoiceAuto lets the model decide whether to use tools (default) - ToolChoiceAuto ToolChoiceType = "auto" - // ToolChoiceAny forces the model to use one of the provided tools - ToolChoiceAny ToolChoiceType = "any" // ToolChoiceNone prevents the model from using any tools ToolChoiceNone ToolChoiceType = "none" - // ToolChoiceTool forces the model to use a specific tool (set ToolName) - ToolChoiceTool ToolChoiceType = "tool" ) // ToolChoice controls how the model selects tools type ToolChoice struct { Type ToolChoiceType `json:"type"` - Name string `json:"name,omitempty"` // Only used when Type is ToolChoiceTool } // ChatRequest represents a request to the AI provider @@ -104,7 +97,7 @@ type ChatRequest struct { Temperature float64 `json:"temperature,omitempty"` System string `json:"system,omitempty"` // System prompt (Anthropic style) Tools []Tool `json:"tools,omitempty"` // Available tools - ToolChoice *ToolChoice `json:"tool_choice,omitempty"` // How to select tools (nil = auto) + ToolChoice *ToolChoice `json:"tool_choice,omitempty"` // nil = model-owned automatic selection; none = text-only safety brake } func (r ChatRequest) NormalizeCollections() ChatRequest { diff --git a/internal/ai/tools/tools_control.go b/internal/ai/tools/tools_control.go index 8a4c6bc02..54f2a4eb3 100644 --- a/internal/ai/tools/tools_control.go +++ b/internal/ai/tools/tools_control.go @@ -23,7 +23,7 @@ func (e *PulseToolExecutor) registerControlTools() { e.registry.Register(RegisteredTool{ Definition: Tool{ Name: "pulse_control", - Description: `WRITE operations: control canonical resources that explicitly advertise shared Pulse actions (for example Proxmox guests and supported app-containers) or execute state-modifying commands. Some canonical resources are read-only and will reject pulse_control even when their type is vm or system-container. For read-only operations use pulse_read. For Docker-only workflows use pulse_docker.`, + Description: `WRITE operations: control canonical resources that explicitly advertise shared Pulse actions (for example Proxmox guests and supported app-containers) or execute state-modifying commands. Some canonical resources are read-only and will reject pulse_control even when their type is vm or system-container. Read-only and Docker-only workflows are exposed through their own governed tools.`, InputSchema: InputSchema{ Type: "object", Properties: map[string]PropertySchema{ @@ -115,7 +115,7 @@ func (e *PulseToolExecutor) executeControlResource(ctx context.Context, args map if validation.ErrorMsg != "" { return NewErrorResult(errors.New(validation.ErrorMsg)), nil } - return NewErrorResult(fmt.Errorf("resource '%s' has not been discovered in this session. Use pulse_query to find it first", resourceRef)), nil + return NewErrorResult(fmt.Errorf("resource '%s' has not been discovered in this session. Resource discovery is required first", resourceRef)), nil } if validation.ErrorMsg != "" { return NewErrorResult(errors.New(validation.ErrorMsg)), nil diff --git a/internal/ai/tools/tools_discovery.go b/internal/ai/tools/tools_discovery.go index dcd7059a6..585fabb6f 100644 --- a/internal/ai/tools/tools_discovery.go +++ b/internal/ai/tools/tools_discovery.go @@ -14,7 +14,7 @@ func (e *PulseToolExecutor) registerDiscoveryTools() { e.registry.Register(RegisteredTool{ Definition: Tool{ Name: "pulse_discovery", - Description: `Get AI-discovered service details (log paths, config locations, ports). action="get" triggers discovery for a resource (requires resource_type, resource_id, target_id). action="list" searches existing discoveries. Use pulse_query action="search" first to find resource details.`, + Description: `Get AI-discovered service details (log paths, config locations, ports). action="get" triggers discovery for a resource (requires resource_type, resource_id, target_id). action="list" searches existing discoveries. Resource details must already be known from the current context or a prior resource lookup.`, InputSchema: InputSchema{ Type: "object", Properties: map[string]PropertySchema{ @@ -308,7 +308,7 @@ func (e *PulseToolExecutor) executeGetDiscovery(ctx context.Context, args map[st return CallToolResult{ Content: []Content{{ Type: "text", - Text: fmt.Sprintf("Discovery temporarily unavailable: %v. Do NOT retry this call. Use pulse_control or a different approach to investigate the resource.", err), + Text: fmt.Sprintf("Discovery temporarily unavailable: %v. Immediate retry is unlikely to add information; command-capable investigation or another approach may be needed.", err), }}, IsError: true, }, nil @@ -322,7 +322,7 @@ func (e *PulseToolExecutor) executeGetDiscovery(ctx context.Context, args map[st "target_id": targetID, "cli_access": cliAccess, "message": fmt.Sprintf("Discovery failed: %v", err), - "hint": "Use pulse_control with type='command' to investigate. Try checking /var/log/ for logs.", + "hint": "Command-capable investigation may still be possible. Common log locations include /var/log/.", }), nil } } @@ -336,7 +336,7 @@ func (e *PulseToolExecutor) executeGetDiscovery(ctx context.Context, args map[st "target_id": targetID, "cli_access": cliAccess, "message": "Discovery returned no data. The resource may not be accessible.", - "hint": "Use pulse_control with type='command' to investigate. Try listing /var/log/ or checking running processes.", + "hint": "Command-capable investigation may still be possible. Common next evidence includes /var/log/ entries or running-process state.", }), nil } diff --git a/internal/ai/tools/tools_file.go b/internal/ai/tools/tools_file.go index dcb99e6b8..f1d546c61 100644 --- a/internal/ai/tools/tools_file.go +++ b/internal/ai/tools/tools_file.go @@ -90,7 +90,7 @@ Examples: Governance: ToolGovernance{ ActionMode: ToolActionWrite, ApprovalPolicy: "hidden in read-only mode; approval required in controlled mode", - Summary: "Reads or changes files through the governed file-edit path; use pulse_read for read-only file inspection.", + Summary: "Reads or changes files through the governed file-edit path; read-only file inspection is exposed through the read-only tool surface.", }, }) } diff --git a/internal/ai/tools/tools_query.go b/internal/ai/tools/tools_query.go index 63e9ca955..580c16aa8 100644 --- a/internal/ai/tools/tools_query.go +++ b/internal/ai/tools/tools_query.go @@ -51,7 +51,7 @@ func (e *ErrStrictResolution) ToToolResponse() ToolResponse { map[string]interface{}{ "resource_id": e.ResourceID, "action": e.Action, - "recovery_hint": "Use pulse_query action=search to discover the resource first", + "recovery_hint": "Resource discovery is required before resource-specific actions.", "auto_recoverable": true, // Signal to agentic loop that auto-discovery can help }, ) @@ -1508,13 +1508,13 @@ func isPhase1GuardrailFailure(reason string) bool { func getPhase1Hint(reason, baseHint string) string { switch { case strings.Contains(reason, "sudo"): - return baseHint + ". Remove sudo to use pulse_read; use pulse_control for privileged operations." + return baseHint + ". Read-only execution does not accept sudo; privileged operations require the governed control path." case strings.Contains(reason, "redirect"): - return baseHint + ". Remove redirects (>, >>, <, <<, <<<) to use pulse_read." + return baseHint + ". Read-only execution does not accept redirects (>, >>, <, <<, <<<)." case strings.Contains(reason, "tee"): - return baseHint + ". Remove tee to use pulse_read; tee writes to files." + return baseHint + ". Read-only execution does not accept tee because tee writes to files." case strings.Contains(reason, "substitution"): - return baseHint + ". Remove $() or backticks to use pulse_read." + return baseHint + ". Read-only execution does not accept $() or backticks." case strings.Contains(reason, "chaining"): return baseHint + ". Run commands separately instead of chaining with ; && ||." case strings.Contains(reason, "piped input"): @@ -1529,7 +1529,7 @@ func getPhase1Hint(reason, baseHint string) string { case strings.Contains(reason, "unbounded") || strings.Contains(reason, "streaming"): return baseHint + ". Add line limit: journalctl -n 100 -f or tail -n 50 -f, or wrap with timeout." default: - return baseHint + ". Remove redirects, chaining, sudo, or subshells to use pulse_read." + return baseHint + ". Read-only execution does not accept redirects, chaining, sudo, or subshells." } } @@ -1651,7 +1651,7 @@ func (e *PulseToolExecutor) validateResolvedResource(resourceName, action string err := &ErrStrictResolution{ ResourceID: resourceName, Action: action, - Message: fmt.Sprintf("Resource '%s' has not been discovered. Use pulse_query to find resources before performing '%s' action.", resourceName, action), + Message: fmt.Sprintf("Resource '%s' has not been discovered. Resource discovery is required before performing '%s' action.", resourceName, action), } return ValidationResult{ ErrorMsg: err.Message, @@ -1662,7 +1662,7 @@ func (e *PulseToolExecutor) validateResolvedResource(resourceName, action string return ValidationResult{} } return ValidationResult{ - ErrorMsg: fmt.Sprintf("Resource '%s' has not been discovered. Use pulse_query to find resources first.", resourceName), + ErrorMsg: fmt.Sprintf("Resource '%s' has not been discovered. Resource discovery is required first.", resourceName), } } @@ -1721,7 +1721,7 @@ func (e *PulseToolExecutor) validateResolvedResource(resourceName, action string err := &ErrStrictResolution{ ResourceID: resourceName, Action: action, - Message: fmt.Sprintf("Resource '%s' has not been discovered in this session. Use pulse_query action=search to find it before performing '%s' action.", resourceName, action), + Message: fmt.Sprintf("Resource '%s' has not been discovered in this session. Resource discovery is required before performing '%s' action.", resourceName, action), } return ValidationResult{ ErrorMsg: err.Message, @@ -1735,7 +1735,7 @@ func (e *PulseToolExecutor) validateResolvedResource(resourceName, action string } return ValidationResult{ - ErrorMsg: fmt.Sprintf("Resource '%s' has not been discovered in this session. Use pulse_query action=search to find it first.", resourceName), + ErrorMsg: fmt.Sprintf("Resource '%s' has not been discovered in this session. Resource discovery is required first.", resourceName), } } @@ -1781,11 +1781,11 @@ func (e *PulseToolExecutor) validateResolvedResourceForExec(resourceName, comman e.telemetryCallback.RecordStrictResolutionBlock("validateResolvedResourceForExec", "exec (read-only)") } return ValidationResult{ - ErrorMsg: "No resources discovered in this session. Use pulse_query to discover resources first.", + ErrorMsg: "No resources discovered in this session. Resource discovery is required first.", StrictError: &ErrStrictResolution{ ResourceID: resourceName, Action: "exec (read-only)", - Message: fmt.Sprintf("Resource '%s' cannot be accessed. No resources have been discovered in this session. Use pulse_query action=search to discover available resources.", resourceName), + Message: fmt.Sprintf("Resource '%s' cannot be accessed. No resources have been discovered in this session. Resource discovery is required first.", resourceName), }, } } diff --git a/internal/ai/tools/tools_read.go b/internal/ai/tools/tools_read.go index ef5d4d764..ac6f42ad3 100644 --- a/internal/ai/tools/tools_read.go +++ b/internal/ai/tools/tools_read.go @@ -142,7 +142,7 @@ func (e *PulseToolExecutor) executeReadExec(ctx context.Context, args map[string intentResult := ClassifyExecutionIntent(command) if intentResult.Intent == IntentWriteOrUnknown { hint := GetReadOnlyViolationHint(command, intentResult) - alternative := "Use pulse_control type=command for write operations" + alternative := "Write operations require the governed control path" details := map[string]interface{}{ "command": truncateCommand(command, 100), @@ -164,7 +164,7 @@ func (e *PulseToolExecutor) executeReadExec(ctx context.Context, args map[string return NewToolResponseResult(NewToolBlockedError( "READ_ONLY_VIOLATION", - fmt.Sprintf("Command '%s' is not read-only. Use pulse_control for write operations.", truncateCommand(command, 50)), + fmt.Sprintf("Command '%s' is not read-only. Write operations require the governed control path.", truncateCommand(command, 50)), details, )), nil } @@ -465,7 +465,7 @@ func (e *PulseToolExecutor) executeNativeAppContainerReadLogs(ctx context.Contex if validation.ErrorMsg != "" { return NewErrorResult(fmt.Errorf("%s", validation.ErrorMsg)), nil } - return NewErrorResult(fmt.Errorf("resource '%s' has not been discovered in this session. Use pulse_query action=search to find it first", resourceRef)), nil + return NewErrorResult(fmt.Errorf("resource '%s' has not been discovered in this session. Resource discovery is required first", resourceRef)), nil } if validation.ErrorMsg != "" { log.Warn(). @@ -569,7 +569,7 @@ func blockedNativeReadLogsResult(resolved ResolvedResourceInfo, resourceRef, con details["suggested_tool"] = "pulse_read" details["suggested_arguments"] = suggestedArgs details["recovery_hint"] = fmt.Sprintf( - "Use pulse_read action=logs target_host=%q container=%q for app logs on this platform.", + "App logs on this platform are addressed with target_host=%q and container=%q.", targetHost, containerName, ) @@ -598,7 +598,7 @@ func blockedNativeReadLogsResult(resolved ResolvedResourceInfo, resourceRef, con details["suggested_tool"] = "pulse_query" details["suggested_arguments"] = suggestedArgs details["recovery_hint"] = fmt.Sprintf( - "Use pulse_query action=get resource_type=%q resource_id=%q to inspect status, alerts, activity, and metrics for this resource.", + "Status, alerts, activity, and metrics are addressed with resource_type=%q and resource_id=%q.", queryType, queryResourceID, ) diff --git a/internal/ai/tools/tools_read_test.go b/internal/ai/tools/tools_read_test.go index d33faaedf..dc7916c69 100644 --- a/internal/ai/tools/tools_read_test.go +++ b/internal/ai/tools/tools_read_test.go @@ -282,7 +282,7 @@ func TestExecuteReadLogs_VMwareResourcesReturnStructuredQueryGuidance(t *testing hint, ok := payload.Error.Details["recovery_hint"].(string) require.True(t, ok) - assert.Contains(t, hint, `pulse_query action=get`) + assert.Contains(t, hint, `Status, alerts, activity, and metrics are addressed`) assert.Contains(t, hint, `resource_type="`+tc.wantKind+`"`) suggestedArgs, ok := payload.Error.Details["suggested_arguments"].(map[string]interface{}) diff --git a/internal/ai/tools/tools_summarize.go b/internal/ai/tools/tools_summarize.go index cdfa2288c..202642c17 100644 --- a/internal/ai/tools/tools_summarize.go +++ b/internal/ai/tools/tools_summarize.go @@ -50,7 +50,7 @@ Time window defaults to the last 7 days; supported ranges: 24h, 7d, 30d.`, }, "resource_ids": { Type: "string", - Description: "For action=fleet: comma-separated list of resource identifiers to include (e.g. \"instance:pve1:101,instance:pve1:102\"). Use pulse_query to enumerate resources of a type first if you need the full set.", + Description: "For action=fleet: comma-separated list of resource identifiers to include (e.g. \"instance:pve1:101,instance:pve1:102\"). Resource enumeration may be needed first if the full set is not already known.", }, "range": { Type: "string",