Discovery wraps every probe in `docker exec <container> sh -c '...'`.
The agentexec command policy lists `^docker\s+exec\s` as RequireApproval
(a sound default for user-driven docker exec) and Discovery has no path
to mint or supply an ApprovalID. Result: every probe was rejected, the
scanner returned empty CommandOutputs, and the AI fell back to
"Unknown Infrastructure Resource" at confidence 0. The Discovery sub-tab
rendered empty after a "successful" run.
Add a Trusted bool to ExecuteCommandPayload on both the server-facing
agentexec type and the agent's wire struct. When set, the approval gate
is skipped on both ends and the server does not attempt to auto-mint an
approval grant (which would fail with "approval id is required").
PolicyBlock still applies; this is not a way to run arbitrary commands.
Only the discoveryCommandAdapter sets Trusted=true. The flag is never
populated from a deserialised HTTP body or any user-driven path. Patrol
fixes, Assistant remediation, and AI tool calls continue to flow through
the governed approval-record path with a real ApprovalID.
Contracts: amend agent-lifecycle Completion Obligations and Current
State to document the lone exception to the on-agent approval rail, and
amend ai-runtime to fence the Trusted flag to the discovery adapter
only.
- add a forced run action to pulse_discovery for known resources
- make discovery progress describe model-backed evidence analysis rather than a live Assistant chat
- keep shared select hydration stable for persisted discovery intervals
The substrate has been in place for two commits but no live code path
constructs a throttler, so production behaviour is unchanged. This
commit closes the loop: `NewPatrolService` instantiates a
`findingStormThrottler` and calls
`p.findings.SetStormThrottler(p.stormThrottler)` immediately after
the `FindingsStore` is constructed, so every patrol emission flows
through the observer from the first run onward.
Constructor-internal wire-up keeps the throttler intrinsic to the
service rather than a deployment-time concern. No `router.go` seam,
no contract test pin, no goroutine to stop; the throttler holds
in-memory state and is garbage-collected when the PatrolService is
replaced.
The field sits next to `unifiedResourceProvider` to match the brief's
location and keep adjacent observer/provider seams visually grouped.
Commit 1 landed the substrate but only exercised it via direct calls
on the throttler. This commit drives a real `FindingsStore.Add` end
to end with the throttler wired via `SetStormThrottler`, so the hook
inside `Add`'s new-finding branch is on the test path.
Covers the four behaviours the brief calls out:
* Two distinct findings on the same resource within the window keep
the storm finding absent.
* The third distinct finding within the window emits a single storm
finding (severity warning, category reliability,
Source="finding-storm", IsActive).
* A fourth distinct finding within the same window does NOT spawn a
duplicate — the stable ID lands the re-entry on the existing-
finding branch and bumps TimesRaised.
* Cycle guard: emissions whose own Source matches the storm-finding
source flow through Add without tripping a storm.
* Filter: emissions against the synthetic patrol-runtime resource
("ai-service") flow through Add without tripping a storm.
* Resolve: after a synthetic quiet window the throttler returns a
sentinel that `ResolveWithReason` converts into an auto-resolved
storm finding with the expected reason string.
The resolve case is exercised by calling `observeLocked` directly
with a synthetic future time (then dispatching the sentinel through
`ResolveWithReason`) to avoid a 120s wall-clock wait in the test
binary; the production hook in `Add` follows the same path on the
next real emission against the cluster.
No production-code changes in this commit.
A single noisy resource that trips several patrol detectors in a tight
window currently emits one finding per symptom. The findings panel
fans those out, which is the right surface for the detectors but the
wrong surface for the operator: a flapping VM with CPU, swap, and
disk pressure should read as one "this resource is in a bad state"
event, not three concurrent rows competing for attention.
This commit lands the throttler substrate. The new
`findingStormThrottler` is invoked from `(*FindingsStore).Add`'s
new-finding branch (while `s.mu` is held), clusters emissions by
`Finding.ResourceID`, and when a cluster crosses
`stormThreshold` (3) within `stormWindow` (60s) returns a storm
finding for the caller to re-enter through `s.Add`. The stable ID
`finding-storm:<resourceID>` dedups subsequent emissions through the
existing-finding branch, and the cycle guard on `Source ==
"finding-storm"` keeps the storm finding's own emission from
recursing back into the observer.
Auto-resolve is lazy by design: on the next observe trip where a
cluster has fallen below threshold AND the storm finding has been
quiet for at least `2 * stormWindow`, the observer returns a
sentinel that the caller routes through `ResolveWithReason`. No
sweep goroutine, no extra wiring surface — the substrate is
contained in `internal/ai/` and exposes a single nil-safe setter
(`SetStormThrottler`).
This commit is observer-only: nothing constructs a throttler yet, so
in-tree behavior is unchanged. Wire-up lands in a follow-up.
Introduce TriggerReasonAlertFlapping and a matching scope factory so
the alert manager's new flapping-detected hook can enqueue a scoped
patrol that produces a postmortem finding instead of letting the
suppression go silent.
Priority is set to triggerPriorityAnomaly: a suppressed flap is
operationally similar to an anomaly (signal Pulse already saw, that
the user has not yet). Depth is Quick so the patrol stays focused on
the flapping resource.
The reason is wired through both gates:
- isEventDrivenTrigger now treats it as event-driven so it is
subject to the scoped trigger config
- allows() routes it under AlertTriggersEnabled, since flapping is
derived from alert state, not from baseline anomaly detection
When a capacity finding has a registered template in the forecast
registry, generateRemediationPlan attaches the deterministic proposal
to the existing aicontracts.RemediationPlan via a new optional
ProposedActionPlan field. Reuses the existing remediation engine
plumbing - no parallel approval surface.
The wire-in best-effort extracts current value from the finding title
and ships canonical thresholds for storage / guest disk so the
capacity-forecast approval card has enough context to render without
depending on the forecast service being configured.
ProposedActionPlan / ProposedActionPreflight / ProposedMetricSummary
are wire-side projections defined in pkg/aicontracts so the contract
package stays free of an internal/ dependency.
Deterministic remediation proposals for capacity findings, keyed by
(resourceType, metric). Templates ship Allowed=false, RequiresApproval=true,
and emit a preflight-only ActionPlan until a Pulse write capability is
wired for the resource type. Registers PBS datastore prune+GC, ZFS pool
snapshot prune, and VM/CT disk expand variants.
CapacityActionPlanSource = "capacity_forecast" is the wire-side marker
the FindingsPanel approval card variant keys off.
BuildBackupVerificationStaleFinding inspects a ProtectionRollup, and when
VerifyIntent is "stale" returns a backup-category Finding compatible with
the existing patrol intake (FindingsStore.Add /
PatrolService.recordFinding). Dedup keys go through the same
generateFindingID helper the LLM "case backup:" branch uses, so a
stale-still-stale tick updates rather than duplicates.
Severity is Watch by default and escalates to Warning once the last
successful backup is itself older than twice the staleness window —
the MVP's "multiple consecutive stale windows" heuristic without a
separate historical counter.
Tests cover: emission for a stale rollup; nil for verified / unknown /
nil rollups; severity escalation across multiple windows; dedup in the
FindingsStore across two patrol ticks; and the resolution path
(Resolve(auto=true) + auto_resolved lifecycle event) once verification
reappears.
Small Ollama models (qwen2.5:11b, qwen2.5:14b, similar) frequently emit
Pulse tool invocations as plain JSON inside content instead of routing
through the structured tool_calls channel. Users saw raw payloads like
`{"name": "pulse_query", "parameters": {...}}` as the assistant's final
response.
Extend cleanToolCallArtifacts and containsToolCallMarker with a new pass
that detects this leak shape, gated on a closed allowlist of canonical
tool names sourced from the runtime registry. The allowlist auto-syncs
when registerTools() gains a tool, so no separate hand-maintained list.
Anchored on (?:^|\n) and a leading `"name"` key so prose containing JSON
fragments, unrelated objects (`{"foo":"bar"}`), or named resources
(`{"name":"my-vm","cpu":50}`) are left untouched.
The previous comment claimed DeepSeek's API aliases v4-flash/v4-pro to
deepseek-reasoner, justifying the auto coercion via the legacy
reasoner's known 400 behavior. That had the alias direction inverted:
per DeepSeek's pricing page, deepseek-chat and deepseek-reasoner are
deprecated aliases for v4-flash's non-thinking and thinking modes
respectively, not the other way around.
The coercion itself is empirically correct, though. Live preflight
against deepseek-v4-flash with tool_choice=required produces a
deterministic HTTP 400 ("provider rejected forced tool selection") in
275ms. The behavior is consistent across the DeepSeek user community
- multiple downstream projects (pydantic-ai #5193, claude-code-router
#1378, opencode #24190, others) confirm V4 models reject forced tool
selection despite DeepSeek's chat-completion docs listing required as
a supported value. Server reality disagrees with documentation.
This commit updates only the comments in openai.go and openai_test.go
to point at the empirical evidence and the community confirmation.
The coercion behavior is unchanged.
Found by exercising pulse_summarize in real chat: the user asked a
question, the model called the tool successfully (response came back
with narrative_source: ai), but the chat panel ended with raw DSML
text and "Assistant response is ready" — no actual prose answer.
Root cause was in agentic_sanitize.go. The fast-path string list
only checked single-pipe DSML variants ("<｜DSML｜...>"), but
deepseek-v4-flash emits the double-pipe form ("<｜｜DSML｜｜...>").
The opening sequence didn't match any marker, so cleanToolCallArtifacts
returned the content unchanged. The chat orchestrator then showed
the raw DSML to the user as if it were the assistant's answer.
Fix:
- Add double-pipe variants (Unicode and ASCII) to the fast-path
marker list. Six new entries, mirroring the existing single-pipe
entries.
- Add a backstop regex (dsmlRe) that matches any pipe-count variant
via `</?[\|｜]+/?DSML[\|｜]*`. Future model behaviour with triple
or higher pipe counts gets caught without another fast-path edit.
- containsToolCallMarker gets the same coverage so streaming
detection stops forwarding content the moment the marker
appears, regardless of pipe count.
Tests in agentic_sanitize_test.go gain three new cases for cleanup
(double-pipe Unicode, double-pipe ASCII, triple-pipe regex
backstop) and two for detection (double-pipe Unicode, double-pipe
ASCII). All passing.
Process note: this bug was invisible to existing tests because the
test suite only covered the single-pipe variants that were
documented in the marker list. The double-pipe form only appears
when an actual model emits it. Same pattern as the JSON-casing fix
earlier — exercise the surface against a real LLM, find what tests
in isolation can't see.
The previous rip retired only active "Active alert detected" findings
from the now-removed detectAlertSignals -> SignalActiveAlert emitter.
Resolved instances were left in place, polluting the Resolved tab and
inflating the regressed total on the trust strip (a stale 8 resources
each marked "regressed 3x" with descriptions like "Active warning
alert: Container 'ollama' is powered off"). They have no canonical
operator value -- the Alerts surface is the source of truth for
currently-firing alerts -- so on load we now purge them entirely
rather than keeping them around as Resolved noise. Active mirrors are
still retired (auto-resolved with a clear reason) so operators see
why the finding closed; resolved mirrors disappear silently because
they were already in the terminal state. Idempotent.
Extends TestFindingsStore_SetPersistence_RetiresLegacyAlertMirrorFindings
with a fixture for the resolved-mirror case and asserts both the
in-memory purge and the persisted state no longer carries it.
The trust strip on the Patrol page credits "N auto-resolved" but
the Resolved tab next to it sat empty — operators could see the
count but not click through to audit which findings had been
resolved or by what mechanism. The /api/ai/patrol/findings
endpoint only returned active findings, so the frontend filter
(status === 'resolved' || 'dismissed' || 'snoozed') had nothing
to render.
Adds the audit-trail accessor end to end:
- PatrolService.GetAllFindingsIncludingResolved returns active +
resolved + dismissed + snoozed findings at warning severity or
higher, sorted with active first then by severity then recency.
Two separate severity orderings — filter (info=0..critical=3,
used with >= against the warning floor) and sort
(critical=0..info=3, used with < to surface critical first).
Conflating them initially let watch findings leak through the
warning floor; the test fixture catches that.
- HandleGetPatrolFindings honors a new include_resolved=1 query
parameter that routes to the new accessor. Default behaviour
(active only) is unchanged for clients that just want the live
findings list.
- Frontend getPatrolFindings accepts an options object with
includeResolved and loadPatrolFindings threads it through.
- FindingsPanel triggers an includeResolved load whenever the
Resolved filter becomes active for the Patrol-source view.
Test: TestPatrolService_GetAllFindingsIncludingResolved_IncludesResolvedAndDismissedSortsActiveFirst
covers active-first ordering, inclusion of resolved + dismissed,
and the warning severity floor (watch-level findings must not
leak through).
ResolveFinding adapter previously logged a warning and allowed the
LLM's resolve to proceed when the deterministic verifier returned
an error (timeout, executor unavailable, etc.). That's fail-open:
any verifier failure let the auto_resolved → re-detected cycle
continue, exactly the pattern the rest of this branch's
patrol_resolve_finding work spent commits closing. The "Backup
failed" finding on the live preview still cycled once post-
migration because of this path — verifier returned an
ErrVerificationUnknown and resolve was permitted.
Resolution of an event/persistent category finding is effectively
permanent (next detection registers as a regression and inflates
counters and pollutes the trust strip). When the deterministic
verifier cannot confidently say the failure signal is gone, we
don't have grounds to honor the LLM's judgment — the LLM's
"current investigation didn't surface a fresh failure" is exactly
the unreliable signal that produced bogus cycles.
Switches the inconclusive-verifier branch from log-and-allow to
log-and-reject, returning an error to the tool so the LLM can
retry or escalate to the operator. The verifier-still-detects-
signal path stays as-is (it was already fail-closed).
Test: TestPatrolFindingCreatorAdapter_ResolveFinding_RejectsWhenVerifierIsInconclusive
exercises the path by calling ResolveFinding on a backup-failed
finding through a PatrolService with no chat service wired
(getExecutorForVerification returns ErrVerificationUnknown). Asserts
the error mentions 'inconclusive' and that ResolvedAt remains nil.
Contract: extends the deterministic-resolve-gate clause in the
ai-runtime canonical-files completion-obligations to name the
fail-closed-on-inconclusive policy explicitly.
The parent-package audit (TestCostRecordingCoverage in
internal/ai/cost_recording_audit_test.go) only walks
internal/ai/*.go — sub-packages were unguarded, which is the gap
that let the chat cost-recording bug land in the first place (the
agentic loop in internal/ai/chat/ called ChatStream without anyone
recording cost; fixed in a0b3bc7ed).
Adds a parallel audit at internal/ai/chat/cost_recording_audit_test.go
covering both .Chat() and .ChatStream() callers. Same shape as the
parent audit but extended to include the streaming variant, since
the chat package is built around the agentic loop's ChatStream
call site.
The chat package's orchestrator/loop split means recording lives
in chat.Service.recordChatTurnCost — the orchestrator that owns
the loop — rather than inside the loop methods that call
ChatStream. Two doc-comment exemptions cover that:
- AgenticLoop.ExecuteWithTools / executeWithTools (agentic.go)
- AgenticLoop.ensureFinalTextResponse (agentic_final.go)
Both name the orchestrator that records, so a future contributor
moving recording elsewhere has a clear pointer to update.
A new ChatStream caller added to the chat package without either
recording or an exemption marker will now fail this audit, the
same way the parent audit would have caught QuickAnalysis or
Narrate if they had been written without recording.
The patrol-main chat session was reused across every scheduled
Patrol run with no upper bound. After a month of runs the file
had grown to 16 MB / 3,593 messages, and every AddMessage
rewrote the whole file to disk — so the I/O cost per Patrol
run was scaling with total session age, not with the run's own
output. Across all chat sessions on this dev instance, the
ai_sessions directory hit 676 MB / 1,629 files.
The stateless-Patrol-input fix (commit 43760fb0d) stopped
loading the session back into the agentic loop, but Patrol
still wrote each run's messages to the session for the Pulse
Assistant sidebar's forensic view. That write path is what
this commit bounds.
ExecutePatrolStream now calls SessionStore.TrimMessages(200)
after each run, keeping roughly the last two runs' worth of
messages — enough for the sidebar to show recent activity, far
short of unbounded growth. The next Patrol run on a bloated
session will drop the historical 3,000+ messages down to 200
on its first write, so existing storage debt clears on its
own without a separate migration.
User-driven chat sessions are unaffected: TrimMessages with
keepMostRecent <= 0 is a no-op, and callers that want full
history retention simply don't call it. Only Patrol's
forensic session is capped.
The canonical Patrol forensic log is the PatrolRunRecord
history surfaced at /api/ai/patrol/runs — that's the durable
record with structured fields. The chat-session-shaped file
is a sidebar convenience, not the source of truth.
Three tests guard the boundary:
- TrimMessages keeps the most recent N (50 messages
trimmed to 10 → messages 40-49 remain)
- TrimMessages is a no-op below threshold (5 messages,
cap 200 → 5 messages remain)
- TrimMessages with non-positive keep is a no-op (3
messages, cap 0 or -5 → 3 messages remain)
ai-runtime contract updated.
chat.Service.ExecuteStream was a long-standing cost-ledger gap: the
agentic loop accumulated token counts via stream callbacks (see
GetTotalInputTokens / GetTotalOutputTokens in agentic_control.go)
and surfaced them in the SSE done envelope to the frontend, but
nothing on the server side recorded a cost.UsageEvent. Patrol,
discovery, QuickAnalysis, and the report narrators all record; only
chat — the bulk of AI token spend — did not. The operator's AI
usage dashboard was therefore understating cost dramatically.
Found while extending the cost-recording mindset across subpackages
after fixing QuickAnalysis (08491b9f4). Initially spawned as a
separate task but the right shape and scope became clear, so landing
it directly here.
Pipeline:
- Service.CostStore() exposes the per-tenant cost store handle.
- chat.Config gains optional CostStore *cost.Store field, threaded
into chat.Service.costStore at NewService time.
- chat.Service.recordChatTurnCost records a UsageEvent with
UseCase="chat" after every loop.ExecuteWithTools return (success
OR error — operator was billed regardless of clean response).
Skips when costStore is nil or zero tokens accumulated.
- ai_handler.go's two chatCfg construction sites populate CostStore
via h.resolveCostStore(ctx).
- router wires the resolver to AISettingsHandler.GetAIService(ctx).CostStore()
with no Enabled gate — even brief chat usage while AI was being
configured should appear in the dashboard.
ExecutePatrolStream is deliberately not changed. It creates a
separate tempLoop and its caller (patrol_ai.go) records cost via
its own helper at line 887. Recording in ExecuteStream only avoids
double-counting on the patrol-via-chat path.
Tests in chat/cost_recording_test.go cover: recording when store
configured, no-op when store nil, no-op on zero tokens (early
failures), graceful handling of model strings missing the
provider prefix.
Defense-in-depth for the malformed-history bug pattern. The
Patrol fix made patrol-main runs stateless, but Assistant
chat sessions are inherently multi-turn and must keep their
history. Any chat session that ends mid-tool-call — network
drop, ctx timeout, browser crash, uncaught panic, any
interrupt that fires between "model emits tool_calls" and
"agentic loop appends all tool results" — leaves the
persisted session with orphan tool_call_ids. The next message
that loads this history is rejected with the same provider
error that flapped Patrol for 33 days:
An assistant message with 'tool_calls' must be followed by
tool messages responding to each 'tool_call_id'.
For Patrol this was fixable by ignoring the session. For
Assistant it isn't; the conversation context is the product.
convertToProviderMessages now ends with a repairOrphanToolCalls
pass that scans every assistant message with tool_calls and
inserts synthetic is_error tool result messages immediately
after the assistant turn for any tool_call_id that has no
matching downstream result. The synthetic content is marked
is_error=true and explains the interruption so the model can
retry the same call or proceed without that data — preserving
conversational continuity while satisfying the provider's
structural-validity check.
This guards every conversation that crosses
convertToProviderMessages, not just Assistant chat. If Patrol
ever changes back to loading session history, the same safety
net applies. If a new entry point appears for some other LLM
flow, it gets the repair for free.
Three tests guard the boundary:
- Orphan injection (3 tool_calls, only 1 result → 2
synthetic results, marked is_error with interrupted
explanation, ordering preserved)
- Clean no-op (all tool_calls fulfilled → no synthetic
messages, no is_error pollution)
- Existing truncation test still passes (assistant message
with both tool_calls and own tool_result → no repair
needed, tool_call_id matches in same message)
ai-runtime contract updated.
The overall-health "Recent Patrol errors" coverage factor in
summarizeRecentPatrolCoverage was anchoring the score to a
stale ratio: it counted errors across the last 10 runs without
weighting recency. After Pulse fixed two compounding Patrol
bugs today, four consecutive successful runs (50+ tool calls
each) followed six earlier failures. The assessment kept
showing C/65 with the prediction "most recent Patrol runs
encountered errors (6 of 10)" — directly contradicting the
fact that *every* recent run had succeeded.
Operators reading that score would conclude Pulse Patrol is
still broken. It isn't. The fix dragged the grade.
This commit adds a recovery-suppression check: count trailing
successful full Patrol runs from the most-recent end of the
window (GetAll returns newest-first), skipping non-full runs.
When three or more consecutive trailing successes exist —
roughly a 9-hour clean stretch at the default 3-hour cadence —
the error penalty drops entirely. The score reflects current
reality.
Three is conservative: a single recovery run could be a
transient win; three consecutive demonstrate the underlying
fix is sticking. Below the threshold, the existing ratio-tiered
penalty still applies so partially-recovered states still
register.
Two tests guard the boundary:
- 6 historical errors + 3 trailing successes → no coverage
factor (suppressed)
- 6 historical errors + 2 trailing successes → coverage
factor remains (recovery incomplete)
Live verified after this commit lands: the assessment that's
been stuck at C/65 since the malformed-history fix will
recompute to A/B grade as soon as the trailing 3 successful
runs are recognized by the same recent-runs query.
ai-runtime contract updated.
The reporting feature now ships across two surfaces (PDF/CSV export
and pulse_summarize chat tool) and three modes (single-resource,
fleet, summarize). Without usage telemetry we can't tell whether the
work earns its place — operator demand, AI-vs-heuristic adoption,
range/format preferences are all invisible. Stops further feature
investment from being pure speculation.
Three new info-level log events, structured so an agent can grep
transcripts and group by dimension without a separate metrics
pipeline (matches the "agent owns ops analysis, human gets outcomes"
posture in MEMORY.md):
reporting.single.generated — single-resource PDF/CSV
reporting.fleet.generated — multi-resource fleet PDF/CSV
reporting.summarize.invoked — pulse_summarize chat tool (both modes)
Common dimensions: org_id, format/action, range, ai_configured,
findings_configured, window_start/end. Single-resource adds
resource_type + metric_type + bytes; fleet adds resource_count +
bytes; summarize adds resource_type + resource_count (fleet mode) +
narrative_source (so we can audit AI-fallback rate).
Includes rangeLabel() helper that maps a window to the canonical
catalog range token (24h/7d/30d) with a 1h tolerance, falling back
to "<hours>h" so non-standard windows still group. Tested.
TestReportingTelemetryEventNames pins the canonical event names as
a contract — an agent grepping logs depends on them being stable;
changing them silently would break audit tooling on the consumer
side.
The reporting engine already logs the resolved narrative source
(heuristic/ai) at debug level via the existing "Generating report"
line, useful for diagnosing why a specific report fell back. Kept
at debug; the new info-level events cover the operator surface.
v1 of pulse_summarize (1fe5d6853) shipped with heuristic narrative
only. The follow-up wiring promised in that commit now lands: the
chat session carries optional report-narration providers that the
tool's handler reads when building requests, so AI-narrated synthesis
flows into chat using the same provider, sanitizer, model selection,
cost ledger, and budget gate the report PDF endpoint already uses.
Pipeline:
- pkg/reporting Narrator / FleetNarrator / FindingsProvider interfaces
are already implemented by internal/ai.Service. No new
implementations.
- tools.ExecutorConfig + PulseToolExecutor gain three optional fields
(ReportNarrator, ReportFleetNarrator, ReportFindingsProvider).
Clone() copies them so per-session executors inherit the wiring.
- chat.Config gains the same three fields; NewService threads them
into ExecutorConfig.
- tools_summarize.go reads e.reportNarrator/FleetNarrator/
FindingsProvider and populates MetricReportRequest /
MultiReportRequest. The engine already accepts these on the request
and falls back to heuristic when they are nil — no engine changes
needed.
- AIHandler gains SetReportNarratorResolver(ctx -> narrators); both
per-tenant and default chat.Config construction sites invoke the
resolver. Router wires the resolver to AISettingsHandler.GetAIService
with the same Enabled-gate the reporting handler uses.
Unconfigured tenants are unchanged: the resolver returns nil, the
tool returns heuristic narrative — identical to today. Configured
tenants get AI synthesis in chat that matches what their report PDF
already carries, billed and budget-gated the same way.
Backup-failed was flapping detected → auto-resolved → re-detected
ten times in a single day. Each cycle the LLM saw "PBS backups
look healthy in my current snapshot" during a Patrol pass, called
patrol_resolve_finding(backup-failed), and the adapter at
patrol_findings.go:985 called Resolve(findingID, true) directly —
no category check, no evidence verification.
The contract docs at findings.go:52-67 explicitly say event /
persistent categories (backup, reliability, security, general)
"stay active until explicitly resolved — either by the LLM calling
patrol_resolve_finding with evidence, or by operator action." That
"with evidence" was never enforced.
This commit enforces it. The adapter now checks two conditions
before honoring an LLM resolve:
- finding.Category does NOT support stale-auto-resolve (per the
contract function CategorySupportsStaleAutoResolve), AND
- a deterministic verifier exists for finding.Key (currently
smart-failure and backup-failed)
When both are true, the adapter runs VerifyFixResolved on the
finding's resource. If the verifier still detects the failure
signal, the LLM gets an error explaining why the resolve was
rejected and that the underlying issue must be fixed first. If
the verifier confirms the signal has cleared, the resolve
proceeds with grounded evidence.
Categories that support stale-auto-resolve (performance, capacity)
bypass the gate entirely — the LLM can resolve them based on
absence per the existing contract. Keys without a verifier also
fall through to current behavior so we don't block resolves for
categories we haven't built verifiers for yet.
New PatrolService.hasDeterministicVerifierForKey() helper keeps
the gate's verifier list in lockstep with the switch in
verifyFixDeterministically.
Tests cover the three branches:
- performance category → gate skipped, resolve proceeds
- reliability + no verifier → gate falls through, resolve proceeds
- hasDeterministicVerifierForKey for known and unknown keys
ai-runtime contract updated.
The reporting synthesis layer (observations, recommendations,
outliers, period comparison) shipped trapped behind the PDF/CSV
export. Operators who chat with Assistant could not ask "what's been
happening with pve1 this week" — the data path existed but had no
non-PDF surface. This commit adds a single new tool, pulse_summarize,
that wraps the engine's non-rendering entry points (NarrativeFor /
FleetNarrativeFor) so that question gets answered in chat.
The tool takes an action parameter (resource | fleet) and routes
accordingly:
- resource mode requires resource_type + resource_id and returns the
same Narrative the single-resource report carries (health status,
observations, recommendations, period comparison).
- fleet mode requires resource_type + a comma-separated resource_ids
string (PropertySchema does not currently support array items, and
CSV is LLM-friendly enough) and returns the FleetNarrative
(outliers, patterns, recommendations). Capped at the same
multi-report ceiling (50) as the API endpoint.
The tool is read-only — no control level requirement, no approval
gate — and uses the global reporting engine the rest of the app
already shares. Returns a JSON envelope so chat can render it or
hand it back to the model for follow-up framing.
v1 ships with heuristic narrative only. The AI narrator wiring
through the chat session (Narrator/FleetNarrator/FindingsProvider
threaded via chat.Config -> tools.ExecutorConfig -> PulseToolExecutor)
is a focused follow-up; it lets the same tool inherit the per-tenant
AI service the report PDF endpoint already uses. The seam is
already in place because NarrativeFor/FleetNarrativeFor take an
optional narrator on the request — v1 passes nil, v2 populates it.
The single-resource and fleet narrator prompts both grounded their
claims in structured data, but neither prevented the model from
classifying observations at warning or critical severity based on
metric inference alone. That left a subtle gap: an AI narrator
noticing memory creep across a window could promote it to warning
even when Patrol — the canonical detection layer — had not flagged
it. That competes with Patrol rather than summarizing its work,
and it lets the report PDF silently shadow-classify in a way that
diverges from the findings store.
Add an explicit detection-boundary instruction to both prompts:
warning or critical severity may only be assigned when backed by
a Patrol finding, an alert, or a hard-threshold breach visible in
the input (cpu max > 90, memory avg > 85, disk avg > 85, failed
or high-wear disks, storage pools at >= 90%). Patterns the model
sees in metric data without that backing are constrained to info
severity. Recommendations follow the same rule. The narrative
remains a retrospective summary of Patrol's classified state, not
a parallel classifier.
This is a prompt-only change. The deterministic data surface and
the heuristic fallback narrator are unaffected; the heuristic
narrators already classify only on the same threshold rules listed
above, so the AI narrator is now constrained to the same evidence
surface its fallback uses.
Service.Narrate (b84b87d8d) and Service.NarrateFleet (d4463a615)
fixed missing cost-record calls in the report-narrative path. Auditing
the rest of internal/ai for the same bug class found one more:
Service.QuickAnalysis. It is used for alert auto-resolve and similar
lightweight decisions, so production token spend on auto-resolve
analysis was invisible in the AI usage dashboard.
Mirror the same fix: capture costStore under the read lock alongside
provider/cfg, and after provider.Chat returns, record a UsageEvent
labelled with the request's UseCase (defaulting to "quick_analysis"
when the caller leaves it blank). Recording happens before the
empty-content guard so failed-but-billed calls are still visible.
Adds cost_recording_audit_test.go: an AST-level audit that walks
internal/ai/*.go (excluding _test.go and sub-packages), finds every
function calling .Chat() on a providers.Provider value, and asserts
each function body also references .Record() on a cost store.
Exemption is allowed via a //cost-recording-exempt: <reason> doc
comment. RunPatrolToolPreflight is annotated as exempt — it is a
connectivity self-test, not user workload, and should not pollute
the operator's cost dashboard.
The audit is intentionally local (function-scoped, not
interprocedural). A passthrough wrapper that calls a recording
function rather than calling Record itself would need an explicit
exemption naming the wrapped callee. Keeping the scan local makes
new Chat callers loud rather than letting silent gaps creep in via
indirection.
Future Chat callers must either record cost or carry the exemption
marker. The audit fails CI otherwise, so the regression that shipped
in b2bd9d114 (Narrate) and would have shipped again in d4463a615
(fleet) cannot recur silently.
rcourtman