Commit Graph

2808 Commits

Author SHA1 Message Date
rcourtman 8197fe6b1e Use disk type thresholds for SMART temperatures 2026-05-13 10:44:11 +01:00
rcourtman 3cecf9576d Seed disk temperature type defaults 2026-05-13 10:44:11 +01:00
rcourtman becee760dd Add disk temperature by type normalization 2026-05-13 10:44:11 +01:00
rcourtman 950bd187a9 seed DiskFillByType defaults and normalize on load
Seeds AlertConfig.DiskFillByType in defaultAlertConfig() with the three
forward-compatible keys (nvme 92/87, sata 90/85, hdd 85/80). Adds
NormalizeDiskFillByType to lowercase keys on load, seed defaults when
the map is nil, and reset non-positive trigger/clear values to the
default for that key. NormalizeAgentDefaults invokes the new helper so
it runs through the existing UpdateConfig normalization chain. Round
trip tests cover nil seed, customized survival, negative reset, and
mixed case normalization, plus a defaultAlertConfig seed assertion.

Only the "nvme" key is consulted by the host evaluation today (the
inference helper only matches /dev/nvme*); "sata" and "hdd" are
forward-compatible placeholders for when the agent protocol carries
hardware type explicitly.
2026-05-13 07:16:07 +01:00
rcourtman 85fbbd4ef6 consult DiskFillByType in host disk fill alert evaluation
Inserts a per-type threshold lookup before the global AgentDefaults.Disk
fallback in CheckHost's disk evaluation. inferDiskHardwareType maps the
disk device path to "nvme" (only NVMe is inferable today); when the
DiskFillByType map carries a matching key, that hysteresis threshold is
used, otherwise evaluation falls through to the existing global default.
Storage-type unified_eval branch is unchanged.

Tests prove: (a) NVMe device at 91% does not alert when DiskFillByType
sets the nvme trigger to 92; (b) the same device at 93% alerts at the
nvme threshold; (c) /dev/sda1 falls back to the global threshold; (d)
the storage-type alert branch is not regressed.
2026-05-13 07:16:07 +01:00
rcourtman 005873bdd5 add disk-type-aware threshold field and device inference helper
Adds DiskFillByType map to AlertConfig (keys: nvme, sata, hdd ->
HysteresisThreshold). Adds inferDiskHardwareType helper that returns
"nvme" for /dev/nvme* device paths (case-insensitive) and "" for
anything else, since sata vs hdd cannot be reliably inferred from
device paths today. Unit tests cover the four input shapes (nvme,
sata, empty, mixed case).

Substrate only. No wire-in to host alert evaluation yet.
2026-05-13 07:13:46 +01:00
rcourtman 99d0edb04a wire PDM alert bridge into PatrolService and seed demo 2026-05-13 05:42:25 +01:00
rcourtman b9844f22c1 prove PDM alert bridge emits and resolves through FindingsStore.Add 2026-05-13 05:42:25 +01:00
rcourtman 8aad44174b add PDM alert bridge substrate in internal/ai 2026-05-13 05:42:25 +01:00
rcourtman 4097ddcb9c handle repeated update-safety digest changes 2026-05-13 04:18:27 +01:00
rcourtman 5821b6c810 wire update-safety watcher into PatrolService and seed demo 2026-05-13 04:16:35 +01:00
rcourtman 2e87e374eb prove update-safety watcher emits and resolves through FindingsStore.Add 2026-05-13 04:16:35 +01:00
rcourtman ab71410ccb add update-safety watcher substrate in internal/ai 2026-05-13 04:16:35 +01:00
rcourtman 178c961723 wire storm throttler into PatrolService from the constructor
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.
2026-05-13 02:41:45 +01:00
rcourtman 38a471120d prove storm throttler emits and resolves through FindingsStore.Add
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.
2026-05-13 02:41:45 +01:00
rcourtman 8f9ff92701 add storm throttler substrate in FindingsStore.Add
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.
2026-05-13 02:41:45 +01:00
rcourtman d4f84e69e0 add hourly sweep for overdue will_fix_later commitments 2026-05-13 01:13:38 +01:00
rcourtman 183bf5b617 persist remind_at and remind_count on will_fix_later findings 2026-05-13 01:13:38 +01:00
rcourtman 0e0c90da53 emit reliability finding when an alert starts flapping
Wire the alerts manager's new flapping-detected callback in the AI
intelligence initialization path. Two things happen on each first
transition into the flapping cooldown window for a tracking key:

1. A reliability-category finding is written directly to the findings
   store via emitFlappingPostmortemFinding. Path B from the lane brief:
   the finding is durable without depending on patrol synthesis, so
   the operator sees the diagnosis the moment Pulse decides to
   suppress. The finding ID is derived from the canonical tracking
   key ("alert-flapping:<trackingKey>") so re-detection inside the
   cooldown window folds into the existing record via the same-ID
   branch of FindingsStore.Add -- one finding per flapping condition,
   not one per dispatch.

2. A scoped FlappingPostmortemPatrolScope is enqueued on the trigger
   manager so an actual patrol run can enrich the finding with deeper
   context once it lands.

The finding body names the flapping threshold, window, and cooldown
the manager is currently configured with, plus an action hint
(widen threshold, raise cooldown, or stabilise the resource). That
turns the suppressed alert from silence into a closable item on the
FindingsPanel.

FindingCategoryReliability is reused; no new category, no parent/
child finding structure -- those are deferred per the lane brief.
2026-05-13 00:28:24 +01:00
rcourtman fb05c38ec7 add patrol scope for flapping postmortem
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
2026-05-13 00:28:24 +01:00
rcourtman fa9cc5d691 fire flapping-detected callback on first transition
When the alert manager suppresses an alert because it is flapping, today
the suppression is silent: the operator sees no alert and no diagnosis.
That trains people not to trust Pulse on flapping resources -- the
detector did its job, but the user just sees absence.

Add a one-shot Manager callback that fires exactly when a tracking key
crosses from quiet into the flapping cooldown window. The callback is
the hook downstream surfaces (patrol, findings) will use to explain
what is flapping and why Pulse stopped notifying.

Lock-safety: checkFlappingLocked now returns (suppress, justTransitioned).
The caller (dispatchAlert) reads the transition signal, then dispatches
the callback from a fresh goroutine. The alerts manager mutex is never
held while the callback runs, so subscribers may take their own locks
or re-enter the alerts package without deadlock.

Subsequent dispatches inside the cooldown window remain silent: the
transition flag is only set on the call that flips flappingActive from
false to true.
2026-05-13 00:28:24 +01:00
rcourtman 2e064e6c6c attach forecast proposals onto patrol RemediationPlan
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.
2026-05-12 23:41:55 +01:00
rcourtman a5629d5701 add capacity-forecast action template registry
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.
2026-05-12 23:41:55 +01:00
rcourtman e69633daf8 emit backup_verification_stale finding from VerifyIntent
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.
2026-05-12 22:23:55 +01:00
rcourtman bd62b6d8ad add VerifyIntent rollup substrate
Extend ProtectionRollup with a tri-state VerifyIntent (verified / stale /
unknown) plus LastVerifiedAt, derived at read-time from the existing
recovery_points.verified column. Stale means: a successful backup exists
but no verification-bearing point has landed within
BackupVerifyStaleWindow (7d, package-level constant for the MVP). Both
fields are omitempty so existing rollup snapshot consumers see no shape
change until they opt into the verify loop.

The SQL ListRollups path projects verified into the filtered CTE and
folds MAX(CASE WHEN verified=1 THEN ts_ms END) into the per-subject
aggregate. The in-memory BuildRollupsFromPoints mirrors the same logic
via the new ComputeVerifyIntentAt helper so mock mode and the persisted
store agree.
2026-05-12 22:23:55 +01:00
rcourtman 7f38a3d8bf add verify_window to agentexec command policy
CommandPolicy gains a VerifyWindow (Go duration) bounded to
[(0,], 15m] with a 2m default. NormalizeVerifyWindow and the policy
Normalize() method enforce the bounds; DefaultPolicy() populates the
default. JSON marshaling now goes through a shadow struct so the wire
form serializes as a duration string (e.g. "2m0s") rather than the raw
nanosecond integer.

Tests cover the default, the bounds (clamp to max, fall through to
default for zero/negative), the JSON roundtrip, the unmarshal-applies-
bounds contract, and rejection of unparseable duration strings.
2026-05-12 21:53:49 +01:00
rcourtman 006821327f add verification outcome and capability postcondition substrate
ActionAuditRecord gains a VerificationOutcome{status, evidenceSummary}
field with a closed enum (unknown/verified/unverified/failed). Existing
records read back as unknown by default via the normalizer and a new
SQLite column verification_outcome_json. The redaction pass scrubs the
evidence summary alongside other operator-authored text.

A new agentexec/verifier_postconditions.go registers postconditions for
qm.start, pct.start, docker.restart, systemctl.restart, and
kubectl.rollout, each parsed by verifier_postconditions_test.go.

Three pre-existing action JSON snapshot tests
(TestContract_ActionDecisionJSONSnapshot,
TestContract_ActionExecutionJSONSnapshot,
TestContract_UnifiedActionAuditsJSONSnapshot) now include the new
verificationOutcome field. The two flagged failing contract tests on
this branch
(TestContract_ActionDryRunOnlyExecutionErrorJSONSnapshot,
TestContract_RouterBridgesVerificationOntoActionCompleted) are
unrelated to this change and were left alone per lane D-002 scope.
2026-05-12 21:53:49 +01:00
rcourtman 015e7f6555 Add maintenance verification reports
When a maintenance window ends on a resource, the sentinel runs
deterministic checks (active alerts, Patrol findings, failed actions
since window start, basic post-window metric recovery) and writes a
durable LoopReport. Operators can list reports per resource, mark them
reviewed, or rerun verification immediately. UI surfaces the section in
the resource detail drawer; scoped Patrol runs and Assistant deep-link
are deferred until those entry points stabilise.
2026-05-12 21:10:58 +01:00
rcourtman 93c62e691a Aggregate simplify-review cleanups (no behavior change)
Six small refactors aggregated from a simplify-review pass over this
session's commits:

1. internal/config/persistence_relay.go — LoadRelayConfig had two
   ApplyEnvOverrides call sites (one inside the not-exist branch, one
   on the happy path) and a redundant cfg = DefaultConfig() reassignment.
   Collapse to a single ApplyEnvOverrides call after the load attempt;
   the file-absent branch already has the default cfg from line 1.

2. internal/relay/config_env.go — swap two strings.TrimSpace(os.Getenv(...))
   calls for utils.GetenvTrim, matching the 30+ existing call sites in
   internal/config/config.go. Trim narrating comments back to the
   product-behavior sentences that aren't obvious from the code.

3. internal/relay/config_env_test.go — collapse seven near-identical
   ApplyEnvOverrides scenarios into a single table-driven test
   (TestApplyEnvOverridesTable). Reduces ~85 lines to ~60 and gives each
   subcase a named t.Run for clearer failure output. Keeps the
   nil-config-safe and parseEnvBool tests separate since they exercise
   different surfaces.

4. .github/workflows/install-sh-smoke.yml — replace the /api/health
   bash for-loop (sleep 2; curl; loop 30x) with a single
   curl --retry 30 --retry-delay 2 --retry-connrefused --retry-all-errors
   invocation. Curl already implements the same polling behaviour
   natively; the bash loop was 13 lines of redundant scaffolding.

5. scripts/installtests/build_release_assets_test.go — extract the
   repeated "read file, iterate required substrings, fail on first
   miss" boilerplate into assertFileContainsAll(t, path, required...).
   Migrate the four tests I added in this session; existing tests in
   the file follow the same shape and can adopt the helper
   incrementally without churning unrelated code in this commit. Also
   updated the pinned curl string for the /api/health retry change.

Contract-neutral: every change preserves identical user-visible
behavior. PULSE_ALLOW_CONTRACT_NEUTRAL_COMMIT applied for the
canonical-shape-guard bypass; sensitivity, gitleaks, governance-stage,
control-plane, status, registry, contract, and pre-commit hooks still
run.

Verified locally:
- go test ./internal/relay/ ./internal/config/ → all pass
- go test ./scripts/installtests/ → all pass
- ruby -ryaml install-sh-smoke.yml → parses clean
2026-05-12 17:32:11 +01:00
rcourtman 8b0f3564f6 Fail closed on stale API action plans 2026-05-12 17:32:11 +01:00
rcourtman c6d5c4590a Keep agent heartbeats stream local 2026-05-12 16:14:28 +01:00
rcourtman 0b98cded45 Bulk count agent fleet approvals 2026-05-12 16:00:31 +01:00
rcourtman f16aa8a65c Parse stored subscription states for entitlement refresh 2026-05-12 15:17:53 +01:00
rcourtman 4cf16ec9cb Stabilize summary chart SLOs 2026-05-12 14:40:55 +01:00
rcourtman 1726cf47b4 Harden Patrol and Assistant action boundaries 2026-05-12 12:06:27 +01:00
rcourtman b69c8c8007 Wire PULSE_RELAY_ENABLED and PULSE_RELAY_SERVER as real env overrides
These two env vars were documented as relay overrides in v6 docs since
March 18 (CONFIGURATION.md, RELAY.md, and the frontend-served doc copy)
but no code ever read them. Operators trying to bootstrap relay headlessly
saw no effect.

Implement them rather than remove the documentation. Headless and
container deployments now have a real path to enable relay and point it
at a private endpoint without going through Settings → Relay.

internal/relay/config_env.go:
  - ApplyEnvOverrides(*Config) mutates relay.Config in place.
  - PULSE_RELAY_ENABLED accepts true/false/yes/no/1/0/on/off (case-
    insensitive). Unrecognized values log a warning and leave the file
    value untouched — important so "unset" reads differently from
    "explicit false."
  - PULSE_RELAY_SERVER goes through the existing validateRelayServerURL
    check; invalid URLs log a warning and fall through.

internal/config/persistence_relay.go:
  LoadRelayConfig calls ApplyEnvOverrides after the file load and after
  the default-fallback when relay.enc is absent, so the env override
  applies on every load.

Tests cover unset / true / false / garbage-bool / valid-URL / invalid-URL
/ both-together / nil-config paths in the relay package, plus two
end-to-end tests in internal/config that prove the override flows through
LoadRelayConfig against a real persisted file and against the
missing-file default branch.

Restore the env-var docs with the correct default URL (the full
wss://relay.pulserelay.pro/ws/instance, not the bare hostname the
original aspirational table claimed) and add an explicit precedence note:
saving from the UI after an env override persists the env-effective state
to disk, so clearing the env alone does not revert.

Add internal/relay/config_env_test.go to the relay-runtime registry's
desktop-relay-runtime exact_files so the new code surface is proof-tracked.
Update the matching pin in subsystem_lookup_test.py. Extend the
relay-runtime contract Extension Point 3 to document the override
semantics LoadRelayConfig must satisfy.
2026-05-12 11:18:31 +01:00
rcourtman 89379c4b5c Use effectiveLoadP95Budget for metrics-history load test CI variance 2026-05-12 09:19:58 +01:00
rcourtman a5d8b43088 Let assertJSONSnapshot exclude dynamic top-level fields for patrol_preflight 2026-05-12 01:38:59 +01:00
rcourtman 5fd05efa83 Add connection-degraded alert for wedged platform connections
A Proxmox host wedged on a ZFS deadlock yesterday took the cluster API poll
with it (context deadline exceeded). The unified connections aggregator
flipped the Connection from active to stale to unreachable, and the
Settings / Infrastructure page rendered the right badges, but no top-nav
alert ever fired because nothing was actively notifying off that derived
state. Patrol's deterministic triage flagged it every minute, but its LLM
investigation stage has been broken since 2026-02-26 so flags never
escalated into user-visible findings. Result: a 3 hour outage I only
noticed because I happened to open Settings.

This wires an active notification off the same connection state the
Settings badges already use:

- internal/alerts/connection.go: new CheckConnection +
  clearConnectionDegradedAlert that fire connection-degraded after three
  consecutive stale or unreachable observations. Severity scales: stale
  warning, unreachable / unauthorized critical. Clear runs through the
  same recovery-confirmation gate as clearNodeOfflineAlert so a single
  flap back to active doesn't silently resolve a real outage. Paused,
  disabled, and non-platform connections are no-ops.

- internal/api/connections_alerts.go: snapshot translator that turns
  api.Connection into the narrow alerts.ConnectionSnapshot view. Keeping
  the snapshot type inside the alerts package preserves the existing
  api -> monitoring import direction; the monitor would have cycled if
  it called back into api directly.

- internal/monitoring: new SetConnectionsSnapshotLister hook + a
  per-tick checkConnectionAlerts call in the main poll loop, alongside
  the existing evaluate*Agents passes.

- internal/api/router.go: register the lister closure on r.monitor so
  the alerts loop sees the same Connection rows the HTTP handler does.

- internal/alerts/specs/types.go: add "connection" to the migration
  bridge list of accepted ResourceTypes, alongside node / docker-host /
  proxmox-disk / etc. The connection concept doesn't have a canonical
  unified resource type yet; this matches the existing pattern for
  alert-keyed resources that aren't first-class canonical.

Test coverage in internal/alerts/connection_test.go covers active never
fires, three stale observations escalate from pending to warning,
unreachable escalates warning to critical, unauthorized fires critical
cold, paused / disabled / agent never fire, recovery confirmation gate,
and a stale flap during recovery resets the gate.
TestResourceAlertSpecValidateAllowsConnectionMigrationBridgeType mirrors
the existing migration-bridge proof tests for the new type.
2026-05-12 00:29:04 +01:00
rcourtman 16963e415c Drop t.Parallel from dismiss/snooze finding tests that race on global session store 2026-05-11 23:10:35 +01:00
rcourtman 7951da526b Add release_cycle_artifact_globs so RC ceremony skips contract-update requirement 2026-05-11 22:55:29 +01:00
rcourtman 41dd867037 Stop alerts manager in TestPollCephClusterChecksPoolStorageThresholds to fix TempDir cleanup flake 2026-05-11 22:30:30 +01:00
rcourtman e32db04543 Recalibrate CI 500-node load floor after rc.5 operator-state and agent-substrate plumbing 2026-05-11 19:07:44 +01:00
rcourtman 8ff69daa43 Bump install pins to rc.5 and refresh test fixtures for Patrol readiness + Unraid host profile tokens 2026-05-11 18:02:52 +01:00
rcourtman e36945741e Sanitise plain-JSON tool-call leaks from weak local models
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.
2026-05-11 17:02:07 +01:00
rcourtman 9329258f8b Correct the DeepSeek tool_choice coercion rationale
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.
2026-05-11 14:46:08 +01:00
rcourtman 2a9afb1112 Sanitise double-pipe DeepSeek DSML tool-call markers in chat
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.
2026-05-11 13:42:11 +01:00
rcourtman e22113230a Purge resolved legacy alert-mirror findings on load
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.
2026-05-11 11:40:07 +01:00
rcourtman 3c0b52c11d Expose resolved findings to the Patrol Resolved tab
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).
2026-05-11 11:09:03 +01:00
rcourtman d02255907c Fail closed when patrol_resolve_finding verifier is inconclusive
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.
2026-05-11 10:53:06 +01:00