mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-08-04 13:23:28 +00:00
Harden Patrol readiness streaming transport (#1640)
Follow-up to 8d0d74e35. The keepalive mechanism was right, the edges
were not.
1. The evaluation ran on a bare goroutine with no recover, so a panic in
provider streaming or validation took the whole Pulse process down.
Before that commit the same panic was on the request goroutine and the
recovery middleware turned it into a logged 500. The goroutine now
recovers, logs the panic with its stack, and answers with an ordinary
readiness result carrying the new internal_error cause and every
dimension reported as not assessed. A Pulse defect is not a model
verdict.
2. Headers were only Set, never committed, despite the comment, the
commit message, and api-contracts.md all claiming otherwise. The
status line went out with the first keepalive at +10s, so a proxy
with a sub-10s time-to-first-byte budget still severed the request.
The transport now writes and flushes WriteHeader(200) before the
ticker starts, matching the pattern the file already uses for SSE.
3. The flusher was resolved with a discarded ok, so a writer that is not
an http.Flusher silently buffered the keepalives and degraded back to
the original bug. It is now checked and logged; the response still
completes, so a warning is the right level here rather than the hard
failure the SSE handlers use.
4. TestIssue1640HandlerUsesKeepaliveTransport grepped the handler source
for substrings, which proves nothing about behaviour. Replaced with a
real httptest.NewServer test that runs a 300ms evaluation and asserts
the client sees the 200 and a body byte before the evaluation
completes, and that the padded body still parses as the expected JSON.
Added coverage for the panic path and the non-flushable writer, and
fixed the eager body[:1] that would panic when a transport regression
left the body empty.
5. The settings readiness banner had no not_assessed branch, so an
interrupted run still rendered the red "Patrol model not verified"
headline: the exact blame-the-model presentation the backend fix
removed. Tone and headline are now exported pure functions with a
neutral treatment for not_assessed and interrupted results, and an
interrupted run cannot claim verification from a max_verified_mode
recorded before the cancellation.
6. createAPIErrorFromResponse let a short plain-text body override an
explicit caller fallbackMessage. A caller passing a fallback knows
which operation it was performing; an intermediary writing the body
does not. Precedence is now canonical JSON, then caller fallback,
then body, with the HTML and oversize suppression unchanged.
7. patrolRunCancelled classified on the raw "context canceled" substring
as its first switch case. Ollama embeds that phrase in its own error
body when it aborts an upstream request, so a genuine provider
failure on a healthy run was classified interrupted and finish()
persisted it as not_assessed. Cancellation is now established from
the run itself (errors.Is(err, context.Canceled), or a cancelled run
context), never from error wording, and the readiness paths classify
through a context-aware entry point. context.DeadlineExceeded keeps
its provider-path timeout classification.
The readiness gate in HandlePatrolModelReadiness keying off ToolProtocol
alone is untouched, as agreed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
courtmanr@gmail.com
parent
b45bd66b94
commit
c3fb35c8f8
23 changed files with 704 additions and 114 deletions
|
|
@ -2154,9 +2154,12 @@ Agent` secondary handoff against the live setup wizard instead of relying
|
|||
it must not request `agent:exec`, dispatch an agent command, inspect agent
|
||||
inventory, or turn a successful model probe into command authority. The
|
||||
advisor's keepalive-streaming response transport changes request pacing
|
||||
only: keepalive bytes, interrupted-run classification, and preserved
|
||||
only: keepalive bytes, the up-front status commit, interrupted-run
|
||||
classification, panic recovery on the evaluation goroutine, and preserved
|
||||
partial probe evidence grant no agent capability and must not be read as
|
||||
agent lifecycle signals.
|
||||
agent lifecycle signals. A readiness result carrying the `interrupted` or
|
||||
`internal_error` cause is an absence of evidence, so it must never be
|
||||
treated as an agent-readiness verdict in either direction.
|
||||
20. Keep Docker container-update proof on the production recreate path. Unit
|
||||
coverage must include standalone and Compose-shaped host, shared-service,
|
||||
shared-container, bridge, and custom-network plans, generated and explicit
|
||||
|
|
|
|||
|
|
@ -3940,18 +3940,33 @@ verified independently, while Safe auto-fix and Autopilot remain explicitly
|
|||
unassessed until deterministic remediation evals exist.
|
||||
|
||||
A cancelled run is not provider evidence. The runtime failure classifier
|
||||
(`internal/ai/patrol_runtime_failure.go`) must classify mid-run cancellation
|
||||
(`context.Canceled`, whether wrapped or surfaced as provider error text) as
|
||||
(`internal/ai/patrol_runtime_failure.go`) must classify mid-run cancellation as
|
||||
`interrupted`, never as a provider connection or analysis fault;
|
||||
`context.DeadlineExceeded` keeps its provider-path timeout classification. An
|
||||
interrupted readiness evaluation reports the overall status and every
|
||||
unfinished dimension and autonomy mode as not assessed while preserving the
|
||||
per-scenario evidence completed before the interruption, and it must not
|
||||
overwrite the last completed evaluation in the readiness cache. The readiness
|
||||
handler streams insignificant JSON-whitespace keepalives while the evaluation
|
||||
runs so intermediaries with short read timeouts do not sever slow local-model
|
||||
runs; severed or operator-cancelled runs surface as interrupted, not as model
|
||||
verdicts (#1640).
|
||||
`context.DeadlineExceeded` keeps its provider-path timeout classification.
|
||||
Cancellation is established from the run itself, never from error wording: an
|
||||
error that wraps `context.Canceled`, or a run whose own context is cancelled.
|
||||
A raw `context canceled` substring in a provider error body is not evidence of
|
||||
anything, because providers embed that phrase when they abort an upstream
|
||||
request of their own while the Pulse run is healthy, and treating it as
|
||||
cancellation would persist a genuine provider failure as not assessed. Callers
|
||||
holding the run context must classify through the context-aware entry point so
|
||||
a torn-down request is not blamed on the provider, and the detail rewrite
|
||||
follows the same rule as the cause. An interrupted readiness evaluation reports
|
||||
the overall status and every unfinished dimension and autonomy mode as not
|
||||
assessed while preserving the per-scenario evidence completed before the
|
||||
interruption, and it must not overwrite the last completed evaluation in the
|
||||
readiness cache. The readiness handler streams insignificant JSON-whitespace
|
||||
keepalives while the evaluation runs so intermediaries with short read timeouts
|
||||
do not sever slow local-model runs; severed or operator-cancelled runs surface
|
||||
as interrupted, not as model verdicts (#1640).
|
||||
|
||||
A Pulse-side defect is not a model verdict either. A panic recovered on the
|
||||
readiness evaluation path produces a readiness result with the
|
||||
`internal_error` cause and every dimension and autonomy mode reported as not
|
||||
assessed, because nothing about the model was measured. `internal_error`
|
||||
carries the same non-evidence status as `interrupted`: neither may be
|
||||
presented, cached, or reasoned about as a statement about the provider or the
|
||||
selected model.
|
||||
|
||||
## Current State
|
||||
|
||||
|
|
|
|||
|
|
@ -3667,13 +3667,26 @@ can diagnose a failure from the snapshot alone.
|
|||
|
||||
The readiness response is a keepalive-padded JSON document. Because a full
|
||||
advisor run makes multiple sequential provider calls and can legitimately run
|
||||
for minutes on slow local hardware, the handler commits a 200 status with
|
||||
`Content-Type: application/json` up front and writes flushed newline
|
||||
keepalives while the evaluation runs, then appends the ordinary JSON payload.
|
||||
Leading newlines are insignificant JSON whitespace, so any standard JSON
|
||||
parser consumes the response unchanged; clients must not depend on the first
|
||||
response byte being `{`, and evaluation failures stay in-band in the payload
|
||||
rather than becoming non-200 statuses after the first keepalive (#1640).
|
||||
for minutes on slow local hardware, the handler writes and flushes the 200
|
||||
status line with `Content-Type: application/json`, `Cache-Control: no-store`,
|
||||
and `X-Accel-Buffering: no` before the evaluation starts, not merely before
|
||||
the first keepalive: an intermediary whose time-to-first-byte budget is
|
||||
shorter than the keepalive interval must still see the response begin
|
||||
immediately. It then writes flushed newline keepalives while the evaluation
|
||||
runs and appends the ordinary JSON payload. Leading newlines are
|
||||
insignificant JSON whitespace, so any standard JSON parser consumes the
|
||||
response unchanged; clients must not depend on the first response byte being
|
||||
`{`, and evaluation failures stay in-band in the payload rather than becoming
|
||||
non-200 statuses after the first keepalive (#1640).
|
||||
|
||||
The evaluation runs on its own goroutine, outside the reach of the server's
|
||||
per-request panic recovery, so the readiness transport must recover panics
|
||||
itself and answer with the ordinary 200 readiness payload carrying an
|
||||
`internal_error` cause. A defect on the evaluation path must never take the
|
||||
Pulse process down, and it must never be reported as a model or provider
|
||||
verdict. A response writer that does not implement `http.Flusher` degrades the
|
||||
keepalives rather than failing the request: the handler logs the degradation
|
||||
and still completes the response.
|
||||
|
||||
Every mobile-facing contract change must update the canonical manifest,
|
||||
regenerate both repositories, keep the mobile consumer minimum compatible, and
|
||||
|
|
|
|||
|
|
@ -2826,7 +2826,12 @@ When a response body is not canonical JSON at all — a hosted proxy, gateway,
|
|||
or load-balancer HTML error page — the shared client must not surface the raw
|
||||
body as the thrown error message. Only short plain-text bodies without markup
|
||||
may pass through; anything else collapses to a generic status-derived message
|
||||
so hosted intermediary HTML never reaches tenant-facing UI (#1640).
|
||||
so hosted intermediary HTML never reaches tenant-facing UI (#1640). Message
|
||||
precedence on that path is fixed: canonical JSON `error` / `message` first, a
|
||||
caller-supplied fallback next, and a non-JSON body last. A caller that passes
|
||||
a fallback knows which tenant-scoped operation it was performing, while a
|
||||
non-JSON body may have been written by an intermediary that has no idea, so a
|
||||
body-derived message must never displace an explicit fallback.
|
||||
That same boundary now also owns stable structured error metadata on the shared
|
||||
browser client. When backend routes return canonical JSON `code` plus
|
||||
string-valued `details`, `frontend-modern/src/utils/apiClient.ts` must preserve
|
||||
|
|
|
|||
|
|
@ -1883,7 +1883,16 @@ default` instead of fusing provider and badge text such as
|
|||
operator's unsaved dropdown selection rather than whatever was
|
||||
previously saved, and must surface a stale-cache warning when the
|
||||
form's selection differs from the cached result's model so the
|
||||
green badge cannot silently mislead)
|
||||
green badge cannot silently mislead). The readiness banner's tone and
|
||||
headline are pure functions exported from
|
||||
`AIModelSelectionSection.tsx` so this presentation is provable without
|
||||
mounting the settings shell, and a result that was never assessed —
|
||||
`status: not_assessed`, or the `interrupted` cause left by an operator
|
||||
cancel or a severed request — must render in a neutral "check did not
|
||||
complete" treatment. It must not use the failure treatment or the
|
||||
"model not verified" headline, because a run that measured nothing is
|
||||
not a verdict on the model, and it must not claim verification from a
|
||||
`max_verified_mode` recorded before the interruption (#1640)
|
||||
2. Keep top-level settings surfaces routed through the canonical settings shell
|
||||
and maintain both `frontend-modern/src/components/Settings/__tests__/settingsArchitecture.test.ts`
|
||||
plus `tests/integration/tests/15-settings-shell-consistency.spec.ts`
|
||||
|
|
|
|||
|
|
@ -4798,6 +4798,7 @@
|
|||
"test_prefixes": [],
|
||||
"exact_files": [
|
||||
"frontend-modern/src/components/Settings/__tests__/dataHandlingPanelModel.test.ts",
|
||||
"frontend-modern/src/components/Settings/__tests__/patrolReadinessBanner.issue1640.test.ts",
|
||||
"frontend-modern/src/components/Settings/__tests__/settingsArchitecture.test.ts",
|
||||
"frontend-modern/src/components/Settings/__tests__/useAuditLogPanelState.test.tsx"
|
||||
]
|
||||
|
|
|
|||
|
|
@ -1972,8 +1972,10 @@ pressure to assess context selection, but it must not read live backup records,
|
|||
invoke recovery APIs, mutate storage, or treat a synthetic pass as evidence
|
||||
that Safe auto-fix or Autopilot remediation is verified. An interrupted
|
||||
readiness run keeps the same isolation: preserved partial scenario evidence is
|
||||
synthetic probe output only, and the keepalive response transport introduces no
|
||||
storage or recovery side channel.
|
||||
synthetic probe output only, and the keepalive response transport, its up-front
|
||||
status commit, and its panic recovery introduce no storage or recovery side
|
||||
channel. A result recovered from a panic reports every dimension as not
|
||||
assessed and carries no storage or backup state at all.
|
||||
|
||||
26. Storage row presentation resolves its topology label from
|
||||
`storage.vdevLayout` first and falls back to `storage.topology`, so a
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ import { AIProviderConfigurationSection } from '@/components/Settings/AIProvider
|
|||
import { isModelProviderConfigured } from '@/components/Settings/aiSettingsModel';
|
||||
import { settingsTabPath } from '@/components/Settings/settingsNavigationModel';
|
||||
import type { AISettingsState } from '@/components/Settings/useAISettingsState';
|
||||
import type { PatrolModelReadinessSnapshot } from '@/types/ai';
|
||||
import { AIModelPicker } from '@/components/shared/AIModelPicker';
|
||||
import { formField, labelClass, controlClass } from '@/components/shared/Form';
|
||||
import {
|
||||
|
|
@ -58,6 +59,51 @@ const stripModelProvider = (modelId: string) => {
|
|||
return colon === -1 ? trimmed : trimmed.slice(colon + 1);
|
||||
};
|
||||
|
||||
export type PatrolReadinessBannerTone = 'idle' | 'success' | 'warning' | 'neutral' | 'error';
|
||||
|
||||
// An interrupted run — an operator cancel, or a proxy cutting a slow local
|
||||
// evaluation — measured nothing, so it is not a verdict on the model. The
|
||||
// backend already reports it as not assessed rather than a provider fault
|
||||
// (#1640); the banner has to match, because rendering it in the failure
|
||||
// treatment is the same blame-the-model presentation with a different coat of
|
||||
// paint.
|
||||
export const isPatrolReadinessUnassessed = (result: PatrolModelReadinessSnapshot) =>
|
||||
result.status === 'not_assessed' || result.cause === 'interrupted';
|
||||
|
||||
export const patrolReadinessBannerTone = (
|
||||
result: PatrolModelReadinessSnapshot | null | undefined,
|
||||
isStale: boolean,
|
||||
): PatrolReadinessBannerTone => {
|
||||
if (!result) return 'idle';
|
||||
if (isStale) return 'warning';
|
||||
if (isPatrolReadinessUnassessed(result)) return 'neutral';
|
||||
if (result.status === 'pass') return 'success';
|
||||
if (result.transport_healthy && !result.patrol_capable) return 'warning';
|
||||
if (result.status === 'warning') return 'warning';
|
||||
return 'error';
|
||||
};
|
||||
|
||||
export const patrolReadinessBannerHeadline = (
|
||||
result: PatrolModelReadinessSnapshot | null | undefined,
|
||||
options: { isStale: boolean; pendingModel: string; cachedModel: string },
|
||||
): string => {
|
||||
if (!result) return '';
|
||||
if (options.isStale) {
|
||||
return `Evaluation result is for ${options.cachedModel}, your current selection is ${options.pendingModel}`;
|
||||
}
|
||||
if (isPatrolReadinessUnassessed(result)) {
|
||||
return result.cause === 'interrupted'
|
||||
? 'Patrol model check did not complete'
|
||||
: 'Patrol model not assessed';
|
||||
}
|
||||
if (result.max_verified_mode === 'approval') return 'Verified for Watch only and Ask first';
|
||||
if (result.max_verified_mode === 'monitor') return 'Verified for Watch only';
|
||||
if (result.transport_healthy && !result.patrol_capable)
|
||||
return 'Provider connected; Patrol capability not verified';
|
||||
if (result.status === 'warning') return 'Patrol model needs attention';
|
||||
return 'Patrol model not verified';
|
||||
};
|
||||
|
||||
export const PatrolModelReadinessControl: Component<{ state: AISettingsState }> = (
|
||||
controlProps,
|
||||
) => {
|
||||
|
|
@ -74,15 +120,7 @@ export const PatrolModelReadinessControl: Component<{ state: AISettingsState }>
|
|||
return pending !== '' && cached !== '' && pending !== cached;
|
||||
};
|
||||
|
||||
const tone = () => {
|
||||
const r = result();
|
||||
if (!r) return 'idle';
|
||||
if (isStaleAgainstFormSelection()) return 'warning';
|
||||
if (r.status === 'pass') return 'success';
|
||||
if (r.transport_healthy && !r.patrol_capable) return 'warning';
|
||||
if (r.status === 'warning') return 'warning';
|
||||
return 'error';
|
||||
};
|
||||
const tone = () => patrolReadinessBannerTone(result(), isStaleAgainstFormSelection());
|
||||
|
||||
const toneClasses = () => {
|
||||
switch (tone()) {
|
||||
|
|
@ -90,6 +128,8 @@ export const PatrolModelReadinessControl: Component<{ state: AISettingsState }>
|
|||
return 'border-green-200 dark:border-green-800 bg-green-50 dark:bg-green-900 text-green-700 dark:text-green-300';
|
||||
case 'warning':
|
||||
return 'border-amber-200 dark:border-amber-800 bg-amber-50 dark:bg-amber-900 text-amber-700 dark:text-amber-300';
|
||||
case 'neutral':
|
||||
return 'border-border bg-surface-alt text-base-content';
|
||||
case 'error':
|
||||
return 'border-red-200 dark:border-red-800 bg-red-50 dark:bg-red-900 text-red-700 dark:text-red-300';
|
||||
default:
|
||||
|
|
@ -97,19 +137,12 @@ export const PatrolModelReadinessControl: Component<{ state: AISettingsState }>
|
|||
}
|
||||
};
|
||||
|
||||
const headline = () => {
|
||||
const r = result();
|
||||
if (!r) return '';
|
||||
if (isStaleAgainstFormSelection()) {
|
||||
return `Evaluation result is for ${cachedResultModel()}, your current selection is ${pendingFormModel()}`;
|
||||
}
|
||||
if (r.max_verified_mode === 'approval') return 'Verified for Watch only and Ask first';
|
||||
if (r.max_verified_mode === 'monitor') return 'Verified for Watch only';
|
||||
if (r.transport_healthy && !r.patrol_capable)
|
||||
return 'Provider connected; Patrol capability not verified';
|
||||
if (r.status === 'warning') return 'Patrol model needs attention';
|
||||
return 'Patrol model not verified';
|
||||
};
|
||||
const headline = () =>
|
||||
patrolReadinessBannerHeadline(result(), {
|
||||
isStale: isStaleAgainstFormSelection(),
|
||||
pendingModel: pendingFormModel(),
|
||||
cachedModel: cachedResultModel(),
|
||||
});
|
||||
|
||||
const detail = () => {
|
||||
const r = result();
|
||||
|
|
@ -117,6 +150,13 @@ export const PatrolModelReadinessControl: Component<{ state: AISettingsState }>
|
|||
if (isStaleAgainstFormSelection()) {
|
||||
return 'Click Check Patrol model to test the pending selection.';
|
||||
}
|
||||
if (isPatrolReadinessUnassessed(r) && r.cause === 'interrupted') {
|
||||
// The backend summary already explains the interruption without blaming
|
||||
// the model; append the retry prompt rather than a failure recommendation.
|
||||
return r.summary
|
||||
? `${r.summary} Run the check again when you are ready.`
|
||||
: 'The check was interrupted before it finished. Run it again when you are ready.';
|
||||
}
|
||||
return r.summary || '';
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,130 @@
|
|||
// Regression coverage for issue #1640: an interrupted Patrol readiness run —
|
||||
// an operator cancel, or a reverse proxy cutting a slow local evaluation —
|
||||
// measured nothing about the model. The backend reports it as not assessed
|
||||
// rather than a provider fault, and the settings banner must present it the
|
||||
// same way. Rendering it in the red "Patrol model not verified" treatment is
|
||||
// the blame-the-model presentation this fix exists to remove.
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import {
|
||||
isPatrolReadinessUnassessed,
|
||||
patrolReadinessBannerHeadline,
|
||||
patrolReadinessBannerTone,
|
||||
} from '../AIModelSelectionSection';
|
||||
import type { PatrolModelReadinessSnapshot } from '@/types/ai';
|
||||
|
||||
const dimension = (status: PatrolModelReadinessSnapshot['status']) => ({ status, summary: '' });
|
||||
const mode = () => ({ status: 'not_assessed' as const, summary: '' });
|
||||
|
||||
const snapshot = (
|
||||
overrides: Partial<PatrolModelReadinessSnapshot> = {},
|
||||
): PatrolModelReadinessSnapshot => ({
|
||||
probe_version: 'patrol-readiness/v1',
|
||||
success: false,
|
||||
transport_healthy: false,
|
||||
patrol_capable: false,
|
||||
status: 'fail',
|
||||
provider: 'ollama',
|
||||
model: 'qwen3:4b',
|
||||
duration_ms: 1200,
|
||||
summary: '',
|
||||
dimensions: {
|
||||
connectivity: dimension('not_assessed'),
|
||||
tool_protocol: dimension('not_assessed'),
|
||||
context_quality: dimension('not_assessed'),
|
||||
latency: dimension('not_assessed'),
|
||||
},
|
||||
modes: { monitor: mode(), approval: mode(), assisted: mode(), full: mode() },
|
||||
recorded_at: '2026-07-28T10:00:00Z',
|
||||
recorded_at_unix: 1_785_232_800,
|
||||
...overrides,
|
||||
});
|
||||
|
||||
const noStale = { isStale: false, pendingModel: 'qwen3:4b', cachedModel: 'qwen3:4b' };
|
||||
|
||||
describe('Patrol readiness banner presentation (#1640)', () => {
|
||||
it('presents an interrupted run neutrally instead of as a model failure', () => {
|
||||
const interrupted = snapshot({
|
||||
status: 'not_assessed',
|
||||
cause: 'interrupted',
|
||||
transport_healthy: false,
|
||||
summary: 'Analysis interrupted before completion.',
|
||||
});
|
||||
|
||||
expect(isPatrolReadinessUnassessed(interrupted)).toBe(true);
|
||||
expect(patrolReadinessBannerTone(interrupted, false)).toBe('neutral');
|
||||
|
||||
const headline = patrolReadinessBannerHeadline(interrupted, noStale);
|
||||
expect(headline).toBe('Patrol model check did not complete');
|
||||
expect(headline).not.toContain('not verified');
|
||||
});
|
||||
|
||||
it('presents any not_assessed run neutrally, whatever the cause', () => {
|
||||
const notAssessed = snapshot({ status: 'not_assessed', summary: 'Nothing was measured.' });
|
||||
expect(patrolReadinessBannerTone(notAssessed, false)).toBe('neutral');
|
||||
expect(patrolReadinessBannerHeadline(notAssessed, noStale)).toBe('Patrol model not assessed');
|
||||
});
|
||||
|
||||
it('never claims verification for an interrupted run that had partial evidence', () => {
|
||||
// The interrupted run may still carry a max_verified_mode set before the
|
||||
// cancellation. Not assessed outranks it: the run never finished.
|
||||
const interrupted = snapshot({
|
||||
status: 'not_assessed',
|
||||
cause: 'interrupted',
|
||||
max_verified_mode: 'monitor',
|
||||
});
|
||||
expect(patrolReadinessBannerHeadline(interrupted, noStale)).toBe(
|
||||
'Patrol model check did not complete',
|
||||
);
|
||||
expect(patrolReadinessBannerTone(interrupted, false)).toBe('neutral');
|
||||
});
|
||||
|
||||
it('keeps the existing presentation for real verdicts', () => {
|
||||
const passed = snapshot({
|
||||
status: 'pass',
|
||||
success: true,
|
||||
transport_healthy: true,
|
||||
patrol_capable: true,
|
||||
max_verified_mode: 'approval',
|
||||
});
|
||||
expect(patrolReadinessBannerTone(passed, false)).toBe('success');
|
||||
expect(patrolReadinessBannerHeadline(passed, noStale)).toBe(
|
||||
'Verified for Watch only and Ask first',
|
||||
);
|
||||
|
||||
const failed = snapshot({ status: 'fail', cause: 'model_unsupported_tools' });
|
||||
expect(patrolReadinessBannerTone(failed, false)).toBe('error');
|
||||
expect(patrolReadinessBannerHeadline(failed, noStale)).toBe('Patrol model not verified');
|
||||
|
||||
const warned = snapshot({ status: 'warning' });
|
||||
expect(patrolReadinessBannerTone(warned, false)).toBe('warning');
|
||||
expect(patrolReadinessBannerHeadline(warned, noStale)).toBe('Patrol model needs attention');
|
||||
|
||||
const transportOnly = snapshot({
|
||||
status: 'fail',
|
||||
transport_healthy: true,
|
||||
patrol_capable: false,
|
||||
});
|
||||
expect(patrolReadinessBannerTone(transportOnly, false)).toBe('warning');
|
||||
expect(patrolReadinessBannerHeadline(transportOnly, noStale)).toBe(
|
||||
'Provider connected; Patrol capability not verified',
|
||||
);
|
||||
});
|
||||
|
||||
it('keeps the stale-selection warning ahead of every other verdict', () => {
|
||||
const interrupted = snapshot({ status: 'not_assessed', cause: 'interrupted' });
|
||||
expect(patrolReadinessBannerTone(interrupted, true)).toBe('warning');
|
||||
expect(
|
||||
patrolReadinessBannerHeadline(interrupted, {
|
||||
isStale: true,
|
||||
pendingModel: 'qwen3:8b',
|
||||
cachedModel: 'qwen3:4b',
|
||||
}),
|
||||
).toBe('Evaluation result is for qwen3:4b, your current selection is qwen3:8b');
|
||||
});
|
||||
|
||||
it('stays idle with no result at all', () => {
|
||||
expect(patrolReadinessBannerTone(null, false)).toBe('idle');
|
||||
expect(patrolReadinessBannerTone(undefined, false)).toBe('idle');
|
||||
expect(patrolReadinessBannerHeadline(null, noStale)).toBe('');
|
||||
});
|
||||
});
|
||||
|
|
@ -909,6 +909,22 @@ describe('settings architecture guardrails', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('presents a readiness run that was never assessed neutrally instead of as a model failure', () => {
|
||||
// An operator cancel or a proxy severing a slow local evaluation measures
|
||||
// nothing about the model. Rendering that in the red "Patrol model not
|
||||
// verified" treatment is the blame-the-model presentation #1640 removed
|
||||
// from the backend, so the banner needs its own neutral branch. Tone and
|
||||
// headline are exported pure functions so the presentation is provable
|
||||
// without mounting the settings shell.
|
||||
expect(aiModelSelectionSectionSource).toContain('export const patrolReadinessBannerTone');
|
||||
expect(aiModelSelectionSectionSource).toContain('export const patrolReadinessBannerHeadline');
|
||||
expect(aiModelSelectionSectionSource).toContain('isPatrolReadinessUnassessed');
|
||||
expect(aiModelSelectionSectionSource).toContain("'not_assessed'");
|
||||
expect(aiModelSelectionSectionSource).toContain("'interrupted'");
|
||||
expect(aiModelSelectionSectionSource).toContain('Patrol model check did not complete');
|
||||
expect(aiModelSelectionSectionSource).toContain("case 'neutral':");
|
||||
});
|
||||
|
||||
it('keeps contextual settings feature gates free of retired commercial telemetry wrappers', () => {
|
||||
for (const source of [
|
||||
agentProfilesPanelSource,
|
||||
|
|
|
|||
|
|
@ -12,12 +12,23 @@ describe('apiClient structured error extraction', () => {
|
|||
expect(error.status).toBe(400);
|
||||
});
|
||||
|
||||
it('falls back to plain text when the response is not JSON', async () => {
|
||||
// A caller-supplied fallback outranks any non-JSON body. The body on that
|
||||
// path is unattributed text from whatever answered the request — often a
|
||||
// proxy rather than Pulse — while the caller knows what it was asking for
|
||||
// (#1640). Canonical JSON still outranks both.
|
||||
it('prefers caller fallback copy over a non-JSON body', async () => {
|
||||
const error = await apiErrorFromResponse(
|
||||
new Response('temporary failure', { status: 500 }),
|
||||
'Fallback message',
|
||||
);
|
||||
|
||||
expect(error.message).toBe('Fallback message');
|
||||
expect(error.status).toBe(500);
|
||||
});
|
||||
|
||||
it('falls back to plain text when the response is not JSON and no fallback was given', async () => {
|
||||
const error = await apiErrorFromResponse(new Response('temporary failure', { status: 500 }));
|
||||
|
||||
expect(error.message).toBe('temporary failure');
|
||||
expect(error.status).toBe(500);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -17,9 +17,7 @@ const NGINX_GATEWAY_TIMEOUT = [
|
|||
|
||||
describe('apiErrorFromResponse never surfaces raw HTML (#1640)', () => {
|
||||
it('replaces an HTML proxy error page with a generic status message', async () => {
|
||||
const error = await apiErrorFromResponse(
|
||||
new Response(NGINX_GATEWAY_TIMEOUT, { status: 504 }),
|
||||
);
|
||||
const error = await apiErrorFromResponse(new Response(NGINX_GATEWAY_TIMEOUT, { status: 504 }));
|
||||
expect(error.message).toBe('Request failed with status 504');
|
||||
expect(error.message).not.toContain('<');
|
||||
expect(error.status).toBe(504);
|
||||
|
|
@ -63,4 +61,51 @@ describe('apiErrorFromResponse never surfaces raw HTML (#1640)', () => {
|
|||
);
|
||||
expect(error.message).toBe('Pulse could not run the Patrol model readiness evaluation.');
|
||||
});
|
||||
|
||||
// Precedence, pinned end to end. A caller that passes a fallback knows what
|
||||
// it was asking for; a plain-text body from an anonymous intermediary does
|
||||
// not, so the fallback wins over any non-JSON body. Canonical JSON still
|
||||
// outranks both, because that message came from Pulse itself.
|
||||
describe('message precedence', () => {
|
||||
const FALLBACK = 'Pulse could not run the Patrol model readiness evaluation.';
|
||||
|
||||
it('prefers the caller fallback over a short plain-text body', async () => {
|
||||
const error = await apiErrorFromResponse(
|
||||
new Response('upstream unavailable', { status: 503 }),
|
||||
FALLBACK,
|
||||
);
|
||||
expect(error.message).toBe(FALLBACK);
|
||||
});
|
||||
|
||||
it('prefers the caller fallback over text extracted from a <pre> block', async () => {
|
||||
const error = await apiErrorFromResponse(
|
||||
new Response('<html><body><pre>proxy read timeout</pre></body></html>', { status: 504 }),
|
||||
FALLBACK,
|
||||
);
|
||||
expect(error.message).toBe(FALLBACK);
|
||||
});
|
||||
|
||||
it('prefers the caller fallback over the generic status message', async () => {
|
||||
const error = await apiErrorFromResponse(
|
||||
new Response('x'.repeat(500), { status: 502 }),
|
||||
FALLBACK,
|
||||
);
|
||||
expect(error.message).toBe(FALLBACK);
|
||||
});
|
||||
|
||||
it('still lets a canonical JSON error outrank the caller fallback', async () => {
|
||||
const error = await apiErrorFromResponse(
|
||||
new Response(JSON.stringify({ error: 'Patrol model readiness failed' }), { status: 500 }),
|
||||
FALLBACK,
|
||||
);
|
||||
expect(error.message).toBe('Patrol model readiness failed');
|
||||
});
|
||||
|
||||
it('falls back to the plain-text body only when no fallback was supplied', async () => {
|
||||
const error = await apiErrorFromResponse(
|
||||
new Response('upstream unavailable', { status: 503 }),
|
||||
);
|
||||
expect(error.message).toBe('upstream unavailable');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -142,15 +142,19 @@ async function createAPIErrorFromResponse(
|
|||
} catch {
|
||||
// Non-JSON body. Only surface it when it reads as a short plain-text
|
||||
// message; anything with markup or excessive length (proxy/gateway HTML
|
||||
// error pages) falls through to the generic status message.
|
||||
if (text.includes('<pre>') && text.includes('</pre>')) {
|
||||
const match = text.match(/<pre>(.*?)<\/pre>/s);
|
||||
const extracted = match ? match[1].trim() : '';
|
||||
if (extracted && !extracted.includes('<') && extracted.length < 200) {
|
||||
errorMessage = extracted;
|
||||
// error pages) is dropped. A body-derived message never displaces an
|
||||
// explicit caller fallback either: the caller knows what it was asking
|
||||
// for, an anonymous intermediary does not (#1640).
|
||||
if (!errorMessage) {
|
||||
if (text.includes('<pre>') && text.includes('</pre>')) {
|
||||
const match = text.match(/<pre>(.*?)<\/pre>/s);
|
||||
const extracted = match ? match[1].trim() : '';
|
||||
if (extracted && !extracted.includes('<') && extracted.length < 200) {
|
||||
errorMessage = extracted;
|
||||
}
|
||||
} else if (!text.includes('<') && text.trim() && text.length < 200) {
|
||||
errorMessage = text.trim();
|
||||
}
|
||||
} else if (!text.includes('<') && text.trim() && text.length < 200) {
|
||||
errorMessage = text.trim();
|
||||
}
|
||||
|
||||
if (!errorMessage) {
|
||||
|
|
|
|||
|
|
@ -18,18 +18,38 @@ import (
|
|||
)
|
||||
|
||||
func TestIssue1640ContextCanceledClassifiedAsInterrupted(t *testing.T) {
|
||||
live := context.Background()
|
||||
|
||||
// An error that actually wraps context.Canceled is unambiguous wherever it
|
||||
// surfaces, with or without a run context to consult.
|
||||
for _, err := range []error{
|
||||
context.Canceled,
|
||||
fmt.Errorf("streaming request failed: %w", context.Canceled),
|
||||
errors.New(`Post "http://ollama.internal:11434/api/chat": context canceled`),
|
||||
} {
|
||||
failure := patrolRuntimeFailureFromError(err)
|
||||
failure := patrolRuntimeFailureFromErrorCtx(live, err)
|
||||
if failure.Cause != PatrolFailureCauseInterrupted {
|
||||
t.Fatalf("error %q classified as %q, want %q", err, failure.Cause, PatrolFailureCauseInterrupted)
|
||||
}
|
||||
if strings.Contains(failure.Summary, "Provider analysis error") {
|
||||
t.Fatalf("cancellation must not be reported as a provider fault: %q", failure.Summary)
|
||||
}
|
||||
if plain := patrolRuntimeFailureFromError(err); plain.Cause != PatrolFailureCauseInterrupted {
|
||||
t.Fatalf("context-free classification of %q = %q, want %q", err, plain.Cause, PatrolFailureCauseInterrupted)
|
||||
}
|
||||
}
|
||||
|
||||
// A cancelled run context is itself the signal: whatever symptom a
|
||||
// torn-down request produces, it is not evidence about the provider.
|
||||
cancelledCtx, cancel := context.WithCancel(context.Background())
|
||||
cancel()
|
||||
for _, err := range []error{
|
||||
errors.New(`Post "http://ollama.internal:11434/api/chat": context canceled`),
|
||||
errors.New("unexpected EOF"),
|
||||
} {
|
||||
failure := patrolRuntimeFailureFromErrorCtx(cancelledCtx, err)
|
||||
if failure.Cause != PatrolFailureCauseInterrupted {
|
||||
t.Fatalf("error %q under a cancelled run classified as %q, want %q", err, failure.Cause, PatrolFailureCauseInterrupted)
|
||||
}
|
||||
}
|
||||
|
||||
diagnostic := ClassifyPatrolRuntimeFailure(context.Canceled)
|
||||
|
|
@ -38,6 +58,72 @@ func TestIssue1640ContextCanceledClassifiedAsInterrupted(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
// A provider is free to put "context canceled" in its own error body when it
|
||||
// aborts an upstream request of its own — Ollama does. With the run context
|
||||
// still live that is a genuine provider failure, and classifying it as
|
||||
// interrupted would make the readiness path persist it as "not assessed",
|
||||
// hiding a real fault behind a neutral verdict (#1640).
|
||||
func TestIssue1640ProviderErrorTextIsNotCancellationEvidence(t *testing.T) {
|
||||
live := context.Background()
|
||||
err := errors.New(`Post "http://ollama.internal:11434/api/chat": context canceled`)
|
||||
|
||||
for name, failure := range map[string]patrolRuntimeFailure{
|
||||
"with_live_ctx": patrolRuntimeFailureFromErrorCtx(live, err),
|
||||
"without_ctx": patrolRuntimeFailureFromError(err),
|
||||
"deadline_error": patrolRuntimeFailureFromErrorCtx(live, context.DeadlineExceeded),
|
||||
} {
|
||||
if failure.Cause == PatrolFailureCauseInterrupted {
|
||||
t.Fatalf("%s: provider error text alone must not be classified as interrupted: %+v", name, failure)
|
||||
}
|
||||
}
|
||||
|
||||
// The detail rewrite follows the same rule: it must not announce a
|
||||
// cancellation that never happened.
|
||||
if detail := summarizePatrolRuntimeFailureDetail(err.Error(), false); strings.Contains(detail, "The run was cancelled") {
|
||||
t.Fatalf("detail must not claim cancellation for a live run: %q", detail)
|
||||
}
|
||||
if detail := summarizePatrolRuntimeFailureDetail(err.Error(), true); !strings.Contains(detail, "The run was cancelled") {
|
||||
t.Fatalf("a genuinely cancelled run should get the cancellation detail, got %q", detail)
|
||||
}
|
||||
}
|
||||
|
||||
// The end-to-end effect of the rule above: a readiness run whose provider fails
|
||||
// with cancellation-flavoured text while the run context is healthy reports a
|
||||
// real failure, not "not assessed".
|
||||
func TestIssue1640LiveRunProviderFailureStaysAFailure(t *testing.T) {
|
||||
provider := &issue1640ConnectionFailureProvider{
|
||||
err: errors.New(`Post "http://ollama.internal:11434/api/chat": context canceled`),
|
||||
}
|
||||
|
||||
result := runPatrolModelReadinessWithProvider(
|
||||
context.Background(), readinessTestConfig(), config.AIProviderOllama, "test-model", "ollama:test-model", provider,
|
||||
)
|
||||
|
||||
if result.Cause == PatrolFailureCauseInterrupted {
|
||||
t.Fatalf("live-run provider failure classified as interrupted: %+v", result)
|
||||
}
|
||||
if result.Status != PatrolModelReadinessFail {
|
||||
t.Fatalf("live-run provider failure status = %q, want %q", result.Status, PatrolModelReadinessFail)
|
||||
}
|
||||
}
|
||||
|
||||
type issue1640ConnectionFailureProvider struct {
|
||||
err error
|
||||
}
|
||||
|
||||
func (p *issue1640ConnectionFailureProvider) Chat(context.Context, providers.ChatRequest) (*providers.ChatResponse, error) {
|
||||
return nil, p.err
|
||||
}
|
||||
func (p *issue1640ConnectionFailureProvider) TestConnection(context.Context) error { return p.err }
|
||||
func (p *issue1640ConnectionFailureProvider) Name() string { return "issue1640" }
|
||||
func (p *issue1640ConnectionFailureProvider) ListModels(context.Context) ([]providers.ModelInfo, error) {
|
||||
return []providers.ModelInfo{{ID: "test-model"}}, nil
|
||||
}
|
||||
func (p *issue1640ConnectionFailureProvider) SupportsThinking(string) bool { return false }
|
||||
func (p *issue1640ConnectionFailureProvider) ChatStream(context.Context, providers.ChatRequest, providers.StreamCallback) error {
|
||||
return p.err
|
||||
}
|
||||
|
||||
func TestIssue1640DeadlineExceededStaysProviderConnection(t *testing.T) {
|
||||
// A deadline is a real timeout on the provider path and must keep its
|
||||
// existing provider-connection classification.
|
||||
|
|
|
|||
|
|
@ -248,7 +248,7 @@ func normalizePatrolRunRecord(record PatrolRunRecord) PatrolRunRecord {
|
|||
record.FindingAssessments = append([]PatrolFindingAssessment(nil), record.FindingAssessments...)
|
||||
record.ErrorSummary = strings.TrimSpace(redactPatrolRuntimeFailureDetail(record.ErrorSummary))
|
||||
if strings.TrimSpace(record.ErrorDetail) != "" {
|
||||
record.ErrorDetail = truncateString(summarizePatrolRuntimeFailureDetail(record.ErrorDetail), patrolRuntimeFailureDetailLimit)
|
||||
record.ErrorDetail = truncateString(summarizePatrolRuntimeFailureDetail(record.ErrorDetail, false), patrolRuntimeFailureDetailLimit)
|
||||
}
|
||||
return record
|
||||
}
|
||||
|
|
|
|||
|
|
@ -118,7 +118,7 @@ func patrolRunAssistantHandoffResources(run PatrolRunRecord) []chat.HandoffResou
|
|||
|
||||
func patrolRunRuntimeFailureSummary(run PatrolRunRecord) string {
|
||||
summary := strings.TrimSpace(redactPatrolRuntimeFailureDetail(run.ErrorSummary))
|
||||
detail := strings.TrimSpace(summarizePatrolRuntimeFailureDetail(run.ErrorDetail))
|
||||
detail := strings.TrimSpace(summarizePatrolRuntimeFailureDetail(run.ErrorDetail, false))
|
||||
if summary != "" && detail != "" && summary != detail {
|
||||
return summary + ": " + truncatePatrolRunContextText(detail, 260)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -167,6 +167,18 @@ func emptyPatrolModelReadinessResult() PatrolModelReadinessResult {
|
|||
}
|
||||
}
|
||||
|
||||
// FailedPatrolModelReadinessResult builds a fully-formed failed readiness
|
||||
// result for a failure the evaluation itself could not report, such as a panic
|
||||
// recovered on the transport goroutine. Every dimension and mode reads "not
|
||||
// assessed" because nothing about the model was actually measured (#1640).
|
||||
func FailedPatrolModelReadinessResult(cause PatrolFailureCause, summary, recommendation string) PatrolModelReadinessResult {
|
||||
result := emptyPatrolModelReadinessResult()
|
||||
result.Cause = cause
|
||||
result.Summary = summary
|
||||
result.Recommendation = recommendation
|
||||
return result
|
||||
}
|
||||
|
||||
func clonePatrolModelReadinessResult(result *PatrolModelReadinessResult) *PatrolModelReadinessResult {
|
||||
if result == nil {
|
||||
return nil
|
||||
|
|
@ -423,7 +435,7 @@ func (s *Service) RunPatrolModelReadiness(ctx context.Context, providerName, mod
|
|||
|
||||
releaseSlot, err := s.acquireExecutionSlot(ctx, "patrol")
|
||||
if err != nil {
|
||||
failure := patrolRuntimeFailureFromError(err)
|
||||
failure := patrolRuntimeFailureFromErrorCtx(ctx, err)
|
||||
result.Cause = failure.Cause
|
||||
if failure.Cause == PatrolFailureCauseInterrupted {
|
||||
result.Status = PatrolModelReadinessNotAssessed
|
||||
|
|
@ -478,7 +490,7 @@ func runPatrolModelReadinessWithProvider(ctx context.Context, cfg *config.AIConf
|
|||
|
||||
connectionStarted := time.Now()
|
||||
if err := provider.TestConnection(ctx); err != nil {
|
||||
failure := patrolRuntimeFailureFromError(err)
|
||||
failure := patrolRuntimeFailureFromErrorCtx(ctx, err)
|
||||
result.Cause = failure.Cause
|
||||
// A cancelled run carries no connectivity evidence: report it as not
|
||||
// assessed instead of blaming the provider (#1640).
|
||||
|
|
@ -694,7 +706,7 @@ func runPatrolModelReadinessWithProvider(ctx context.Context, cfg *config.AIConf
|
|||
}
|
||||
var probeFailure *patrolRuntimeFailure
|
||||
if probeErr != nil {
|
||||
failure := patrolRuntimeFailureFromError(probeErr)
|
||||
failure := patrolRuntimeFailureFromErrorCtx(ctx, probeErr)
|
||||
probeFailure = &failure
|
||||
toolSummary = failure.Summary
|
||||
}
|
||||
|
|
|
|||
|
|
@ -41,7 +41,11 @@ const (
|
|||
// a provider fault: an interrupted run carries no evidence about the
|
||||
// provider or model (#1640).
|
||||
PatrolFailureCauseInterrupted PatrolFailureCause = "interrupted"
|
||||
PatrolFailureCauseCircuitOpen PatrolFailureCause = "circuit_open"
|
||||
// PatrolFailureCauseInternalError marks a failure inside Pulse itself (a
|
||||
// recovered panic on the evaluation path) rather than anything the provider
|
||||
// or model did. It must never be presented as a model verdict (#1640).
|
||||
PatrolFailureCauseInternalError PatrolFailureCause = "internal_error"
|
||||
PatrolFailureCauseCircuitOpen PatrolFailureCause = "circuit_open"
|
||||
)
|
||||
|
||||
type PatrolConfigReadiness struct {
|
||||
|
|
|
|||
|
|
@ -124,11 +124,21 @@ func patrolMalformedToolHistory(lower string) bool {
|
|||
// checked before every provider-fault pattern because a cancelled run says
|
||||
// nothing about the provider or model (#1640). context.DeadlineExceeded is
|
||||
// deliberately excluded: a deadline is a real timeout on the provider path.
|
||||
func patrolRunCancelled(err error, lower string) bool {
|
||||
//
|
||||
// The error text alone is never enough, so a raw "context canceled" substring
|
||||
// is not a signal on its own. Providers embed that exact phrase in their own
|
||||
// error bodies when they abort an upstream request while our run is perfectly
|
||||
// healthy (Ollama does), and misreading it as our cancellation makes the
|
||||
// readiness path persist a genuine provider failure as "not assessed". The
|
||||
// wording only counts once the run's own context is cancelled, at which point
|
||||
// the cancelled context is itself the signal: nothing that surfaces out of a
|
||||
// torn-down request (a bare EOF, a reset, a truncated stream) is evidence
|
||||
// about the provider either.
|
||||
func patrolRunCancelled(ctx context.Context, err error) bool {
|
||||
if errors.Is(err, context.Canceled) {
|
||||
return true
|
||||
}
|
||||
return strings.Contains(lower, "context canceled")
|
||||
return ctx != nil && errors.Is(ctx.Err(), context.Canceled)
|
||||
}
|
||||
|
||||
func ClassifyPatrolRuntimeFailure(err error) PatrolRuntimeFailureDiagnostic {
|
||||
|
|
@ -213,12 +223,22 @@ func ClassifyProviderConnectionFailure(err error) PatrolRuntimeFailureDiagnostic
|
|||
return diagnostic
|
||||
}
|
||||
|
||||
// patrolRuntimeFailureFromError classifies an error with no knowledge of the
|
||||
// run context. Cancellation is then recognised only when the error actually
|
||||
// wraps context.Canceled. Callers that hold the run's context should use
|
||||
// patrolRuntimeFailureFromErrorCtx so a torn-down run is not blamed on the
|
||||
// provider (#1640).
|
||||
func patrolRuntimeFailureFromError(err error) patrolRuntimeFailure {
|
||||
return patrolRuntimeFailureFromErrorCtx(context.Background(), err)
|
||||
}
|
||||
|
||||
func patrolRuntimeFailureFromErrorCtx(ctx context.Context, err error) patrolRuntimeFailure {
|
||||
raw := ""
|
||||
if err != nil {
|
||||
raw = strings.TrimSpace(err.Error())
|
||||
}
|
||||
detail := truncateString(summarizePatrolRuntimeFailureDetail(raw), patrolRuntimeFailureDetailLimit)
|
||||
cancelled := patrolRunCancelled(ctx, err)
|
||||
detail := truncateString(summarizePatrolRuntimeFailureDetail(raw, cancelled), patrolRuntimeFailureDetailLimit)
|
||||
lower := strings.ToLower(raw)
|
||||
|
||||
failure := patrolRuntimeFailure{
|
||||
|
|
@ -232,7 +252,7 @@ func patrolRuntimeFailureFromError(err error) patrolRuntimeFailure {
|
|||
}
|
||||
|
||||
switch {
|
||||
case patrolRunCancelled(err, lower):
|
||||
case cancelled:
|
||||
failure.Title = "Pulse Patrol: Analysis interrupted"
|
||||
failure.Summary = "Analysis interrupted before completion"
|
||||
failure.Cause = PatrolFailureCauseInterrupted
|
||||
|
|
@ -349,14 +369,18 @@ func redactPatrolRuntimeFailureDetail(raw string) string {
|
|||
return redacted
|
||||
}
|
||||
|
||||
func summarizePatrolRuntimeFailureDetail(raw string) string {
|
||||
// summarizePatrolRuntimeFailureDetail rewrites a raw provider error into
|
||||
// operator-facing detail. cancelled must be the caller's decision about whether
|
||||
// the run was actually cancelled: the "context canceled" wording on its own is
|
||||
// not proof, because providers put it in their own error bodies (#1640).
|
||||
func summarizePatrolRuntimeFailureDetail(raw string, cancelled bool) string {
|
||||
raw = strings.TrimSpace(raw)
|
||||
if raw == "" {
|
||||
return ""
|
||||
}
|
||||
lower := strings.ToLower(raw)
|
||||
switch {
|
||||
case strings.Contains(lower, "context canceled"):
|
||||
case cancelled:
|
||||
return "The run was cancelled before the provider finished. This is not evidence of a provider or model fault."
|
||||
case patrolMalformedToolHistory(lower):
|
||||
return "Pulse sent a malformed tool-call conversation. Each Patrol run should be stateless; restart Pulse if the failure persists."
|
||||
|
|
|
|||
|
|
@ -194,7 +194,7 @@ func TestSummarizePatrolRuntimeFailureDetail(t *testing.T) {
|
|||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
got := summarizePatrolRuntimeFailureDetail(tc.raw)
|
||||
got := summarizePatrolRuntimeFailureDetail(tc.raw, false)
|
||||
if got != tc.want {
|
||||
t.Fatalf("summarizePatrolRuntimeFailureDetail(%q) = %q, want %q", tc.raw, got, tc.want)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ import (
|
|||
"net/url"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"runtime/debug"
|
||||
"strconv"
|
||||
"strings"
|
||||
"sync"
|
||||
|
|
@ -6209,16 +6210,62 @@ func patrolModelReadinessBudget(cfg *config.AIConfig) time.Duration {
|
|||
// inside common proxy read-timeout defaults.
|
||||
const patrolModelReadinessKeepaliveInterval = 10 * time.Second
|
||||
|
||||
// streamPatrolModelReadinessKeepalives runs evaluate in the background and
|
||||
// writes a newline to w on every keepalive tick until the evaluation
|
||||
// completes, flushing each one so intermediaries see bytes flowing. A leading
|
||||
// newline is insignificant JSON whitespace: every JSON parser skips it, so the
|
||||
// final payload appended after the padding still parses as ordinary JSON and
|
||||
// existing clients need no protocol change (#1640).
|
||||
// streamPatrolModelReadinessKeepalives commits the readiness response headers,
|
||||
// runs evaluate in the background, and writes a newline to w on every keepalive
|
||||
// tick until the evaluation completes, flushing each one so intermediaries see
|
||||
// bytes flowing. A leading newline is insignificant JSON whitespace: every JSON
|
||||
// parser skips it, so the final payload appended after the padding still parses
|
||||
// as ordinary JSON and existing clients need no protocol change (#1640).
|
||||
//
|
||||
// The status line and headers are written and flushed before the ticker starts,
|
||||
// so the client sees a 200 immediately rather than at the first keepalive: a
|
||||
// proxy with a sub-keepalive time-to-first-byte timeout would otherwise still
|
||||
// sever the request. The status is always 200 because evaluation failures
|
||||
// travel in the result payload.
|
||||
func streamPatrolModelReadinessKeepalives(w http.ResponseWriter, interval time.Duration, evaluate func() ai.PatrolModelReadinessResult) ai.PatrolModelReadinessResult {
|
||||
flusher, _ := w.(http.Flusher)
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
w.Header().Set("Cache-Control", "no-store")
|
||||
// Ask nginx-style proxies not to buffer the keepalives back out of existence.
|
||||
w.Header().Set("X-Accel-Buffering", "no")
|
||||
|
||||
flusher, ok := w.(http.Flusher)
|
||||
if !ok {
|
||||
// Without a flusher the keepalives sit in the response buffer and this
|
||||
// whole mechanism silently degrades back to the original bug. The
|
||||
// response still completes, so warn and carry on rather than failing a
|
||||
// readiness run over the writer type.
|
||||
log.Warn().Msg("Patrol model readiness: response writer is not an http.Flusher, keepalives may be buffered")
|
||||
}
|
||||
commit := func() {
|
||||
if ok {
|
||||
flusher.Flush()
|
||||
}
|
||||
}
|
||||
|
||||
w.WriteHeader(http.StatusOK)
|
||||
commit()
|
||||
|
||||
resultCh := make(chan ai.PatrolModelReadinessResult, 1)
|
||||
go func() { resultCh <- evaluate() }()
|
||||
go func() {
|
||||
// The evaluation runs on its own goroutine, outside the reach of the
|
||||
// server's per-request panic recovery: an unrecovered panic in provider
|
||||
// streaming or validation would take the whole Pulse process down. Turn
|
||||
// it into a failed readiness result instead (#1640).
|
||||
defer func() {
|
||||
if recovered := recover(); recovered != nil {
|
||||
log.Error().
|
||||
Interface("panic", recovered).
|
||||
Str("stack", string(debug.Stack())).
|
||||
Msg("Patrol model readiness evaluation panicked")
|
||||
resultCh <- ai.FailedPatrolModelReadinessResult(
|
||||
ai.PatrolFailureCauseInternalError,
|
||||
"The Patrol model readiness evaluation failed unexpectedly inside Pulse.",
|
||||
"This is a Pulse defect rather than a provider or model verdict. Retry the check, and report the failure with the Pulse logs from this run if it repeats.",
|
||||
)
|
||||
}
|
||||
}()
|
||||
resultCh <- evaluate()
|
||||
}()
|
||||
|
||||
ticker := time.NewTicker(interval)
|
||||
defer ticker.Stop()
|
||||
|
|
@ -6230,9 +6277,7 @@ func streamPatrolModelReadinessKeepalives(w http.ResponseWriter, interval time.D
|
|||
// A failed write means the client is gone; the request context
|
||||
// cancellation already unwinds the evaluation, so keep draining.
|
||||
_, _ = w.Write([]byte("\n"))
|
||||
if flusher != nil {
|
||||
flusher.Flush()
|
||||
}
|
||||
commit()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -6278,12 +6323,8 @@ func (h *AISettingsHandler) HandlePatrolModelReadiness(w http.ResponseWriter, r
|
|||
ctx, cancel := context.WithTimeout(r.Context(), patrolModelReadinessBudget(aiService.GetConfig()))
|
||||
defer cancel()
|
||||
|
||||
// Headers must be committed before the first keepalive byte. The status is
|
||||
// always 200: evaluation failures travel in the result payload.
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
w.Header().Set("Cache-Control", "no-store")
|
||||
// Ask nginx-style proxies not to buffer the keepalives back out of existence.
|
||||
w.Header().Set("X-Accel-Buffering", "no")
|
||||
// streamPatrolModelReadinessKeepalives commits the proxy-friendly headers
|
||||
// and the 200 status line before the evaluation starts.
|
||||
result := streamPatrolModelReadinessKeepalives(w, patrolModelReadinessKeepaliveInterval, func() ai.PatrolModelReadinessResult {
|
||||
return aiService.RunPatrolModelReadiness(ctx, body.Provider, body.Model)
|
||||
})
|
||||
|
|
|
|||
|
|
@ -4,13 +4,14 @@ package api
|
|||
// to four sequential provider calls (~45s and more on slow local hardware)
|
||||
// and previously wrote nothing to the response until the evaluation finished,
|
||||
// so any reverse proxy with a ~30-second read timeout severed the request.
|
||||
// The handler now streams JSON-whitespace keepalives while the evaluation
|
||||
// runs; these tests pin that transport shape.
|
||||
// The handler now commits a 200 up front and streams JSON-whitespace
|
||||
// keepalives while the evaluation runs; these tests pin that transport shape.
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
|
@ -21,17 +22,31 @@ import (
|
|||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// firstByte reports the first byte of a body in a form that is safe to print
|
||||
// when the body is empty, so a transport regression fails the assertion
|
||||
// instead of panicking inside it.
|
||||
func firstByte(body string) string {
|
||||
if body == "" {
|
||||
return "<empty body>"
|
||||
}
|
||||
return body[:1]
|
||||
}
|
||||
|
||||
func passingReadinessResult(summary string) ai.PatrolModelReadinessResult {
|
||||
out := ai.PatrolModelReadinessResult{}
|
||||
out.ProbeVersion = ai.PatrolModelReadinessProbeVersion
|
||||
out.Status = ai.PatrolModelReadinessPass
|
||||
out.Summary = summary
|
||||
return out
|
||||
}
|
||||
|
||||
func TestIssue1640KeepalivesFlowWhileEvaluationRuns(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
result := streamPatrolModelReadinessKeepalives(rec, 5*time.Millisecond, func() ai.PatrolModelReadinessResult {
|
||||
time.Sleep(60 * time.Millisecond)
|
||||
out := ai.PatrolModelReadinessResult{}
|
||||
out.ProbeVersion = ai.PatrolModelReadinessProbeVersion
|
||||
out.Status = ai.PatrolModelReadinessPass
|
||||
out.Summary = "verified"
|
||||
return out
|
||||
return passingReadinessResult("verified")
|
||||
})
|
||||
response := patrolModelReadinessSnapshot(&result, time.Now())
|
||||
require.NoError(t, utils.WriteJSONResponse(rec, response))
|
||||
|
|
@ -39,7 +54,7 @@ func TestIssue1640KeepalivesFlowWhileEvaluationRuns(t *testing.T) {
|
|||
body := rec.Body.String()
|
||||
// Bytes were on the wire before the evaluation completed, and they were
|
||||
// flushed so intermediaries actually observed them.
|
||||
assert.True(t, strings.HasPrefix(body, "\n"), "expected keepalive padding before the payload, got %q", body[:1])
|
||||
assert.True(t, strings.HasPrefix(body, "\n"), "expected keepalive padding before the payload, got %q", firstByte(body))
|
||||
assert.GreaterOrEqual(t, strings.Count(body, "\n")-strings.Count(strings.TrimLeft(body, "\n"), "\n"), 2,
|
||||
"expected multiple keepalives during a slow evaluation")
|
||||
assert.True(t, rec.Flushed, "keepalives must be flushed, not buffered")
|
||||
|
|
@ -68,21 +83,130 @@ func TestIssue1640FastEvaluationEmitsNoPadding(t *testing.T) {
|
|||
"a fast evaluation should produce a plain JSON body")
|
||||
}
|
||||
|
||||
// The handler must actually route through the keepalive transport and commit
|
||||
// proxy-friendly headers before the first byte. Mirrors the source-shape
|
||||
// assertion style used for the preflight deadline delegation.
|
||||
func TestIssue1640HandlerUsesKeepaliveTransport(t *testing.T) {
|
||||
source, err := os.ReadFile("ai_handlers.go")
|
||||
// The transport must survive a real HTTP connection, not just a recorder: the
|
||||
// status line has to be committed and flushed before the evaluation starts,
|
||||
// because a proxy with a time-to-first-byte timeout shorter than the keepalive
|
||||
// interval would otherwise still sever a slow readiness run.
|
||||
func TestIssue1640ResponseStartsBeforeEvaluationCompletes(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
const evaluationDuration = 300 * time.Millisecond
|
||||
completed := make(chan time.Time, 1)
|
||||
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
result := streamPatrolModelReadinessKeepalives(w, 50*time.Millisecond, func() ai.PatrolModelReadinessResult {
|
||||
time.Sleep(evaluationDuration)
|
||||
completed <- time.Now()
|
||||
return passingReadinessResult("verified over the wire")
|
||||
})
|
||||
response := patrolModelReadinessSnapshot(&result, time.Now())
|
||||
if err := utils.WriteJSONResponse(w, response); err != nil {
|
||||
t.Errorf("write readiness response: %v", err)
|
||||
}
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
started := time.Now()
|
||||
resp, err := server.Client().Get(server.URL)
|
||||
require.NoError(t, err)
|
||||
text := string(source)
|
||||
start := strings.Index(text, "func (h *AISettingsHandler) HandlePatrolModelReadiness(")
|
||||
require.NotEqual(t, -1, start)
|
||||
end := strings.Index(text[start:], "\nfunc ")
|
||||
if end < 0 {
|
||||
end = len(text) - start
|
||||
defer func() { _ = resp.Body.Close() }()
|
||||
|
||||
// Response headers alone prove the status line was committed early: the
|
||||
// client's Get returns as soon as they arrive.
|
||||
assert.Equal(t, http.StatusOK, resp.StatusCode)
|
||||
assert.Equal(t, "application/json", resp.Header.Get("Content-Type"))
|
||||
assert.Equal(t, "no", resp.Header.Get("X-Accel-Buffering"))
|
||||
assert.Less(t, time.Since(started), evaluationDuration,
|
||||
"headers must reach the client before the evaluation finishes")
|
||||
|
||||
// Read the first body byte under a deadline well short of the evaluation.
|
||||
// A buffered or unstarted response cannot satisfy it.
|
||||
one := make([]byte, 1)
|
||||
readDone := make(chan struct{})
|
||||
var (
|
||||
firstByteAt time.Time
|
||||
readErr error
|
||||
)
|
||||
go func() {
|
||||
defer close(readDone)
|
||||
_, readErr = io.ReadFull(resp.Body, one)
|
||||
firstByteAt = time.Now()
|
||||
}()
|
||||
select {
|
||||
case <-readDone:
|
||||
case <-time.After(evaluationDuration - 100*time.Millisecond):
|
||||
t.Fatal("no body byte arrived before the evaluation completed")
|
||||
}
|
||||
body := text[start : start+end]
|
||||
require.Contains(t, body, "streamPatrolModelReadinessKeepalives(")
|
||||
require.Contains(t, body, `w.Header().Set("X-Accel-Buffering", "no")`)
|
||||
require.Contains(t, body, "patrolModelReadinessKeepaliveInterval")
|
||||
require.NoError(t, readErr)
|
||||
assert.Equal(t, "\n", string(one), "the first body byte should be a keepalive newline")
|
||||
|
||||
evaluationFinishedAt := <-completed
|
||||
assert.True(t, firstByteAt.Before(evaluationFinishedAt),
|
||||
"keepalive byte at %s must precede evaluation completion at %s", firstByteAt, evaluationFinishedAt)
|
||||
|
||||
rest, err := io.ReadAll(resp.Body)
|
||||
require.NoError(t, err)
|
||||
|
||||
// The padded body is still one ordinary JSON document.
|
||||
body := string(one) + string(rest)
|
||||
assert.True(t, strings.HasPrefix(body, "\n"), "expected leading keepalive whitespace, got %q", firstByte(body))
|
||||
var snapshot PatrolModelReadinessSnapshot
|
||||
require.NoError(t, json.Unmarshal([]byte(body), &snapshot))
|
||||
require.NotNil(t, snapshot.PatrolModelReadinessResult)
|
||||
assert.Equal(t, "verified over the wire", snapshot.Summary)
|
||||
assert.Equal(t, ai.PatrolModelReadinessPass, snapshot.Status)
|
||||
}
|
||||
|
||||
// A panic inside provider streaming or validation runs on the evaluation
|
||||
// goroutine, outside the server's per-request recovery. Unrecovered it takes
|
||||
// the whole Pulse process down; it must become a failed readiness result.
|
||||
func TestIssue1640EvaluationPanicBecomesFailedResult(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
result := streamPatrolModelReadinessKeepalives(rec, 5*time.Millisecond, func() ai.PatrolModelReadinessResult {
|
||||
panic("provider stream decoder exploded")
|
||||
})
|
||||
|
||||
assert.Equal(t, ai.PatrolModelReadinessFail, result.Status)
|
||||
assert.Equal(t, ai.PatrolFailureCauseInternalError, result.Cause)
|
||||
assert.NotEmpty(t, result.Summary)
|
||||
assert.False(t, result.Success)
|
||||
assert.False(t, result.PatrolCapable)
|
||||
// Nothing was measured, so nothing may be reported as measured.
|
||||
assert.Equal(t, ai.PatrolModelReadinessNotAssessed, result.Dimensions.Connectivity.Status)
|
||||
assert.Equal(t, ai.PatrolModeNotAssessed, result.Modes.Monitor.Status)
|
||||
|
||||
// The response still completes as a normal 200 JSON document.
|
||||
response := patrolModelReadinessSnapshot(&result, time.Now())
|
||||
require.NoError(t, utils.WriteJSONResponse(rec, response))
|
||||
assert.Equal(t, http.StatusOK, rec.Code)
|
||||
var snapshot PatrolModelReadinessSnapshot
|
||||
require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &snapshot))
|
||||
require.NotNil(t, snapshot.PatrolModelReadinessResult)
|
||||
assert.Equal(t, ai.PatrolModelReadinessFail, snapshot.Status)
|
||||
}
|
||||
|
||||
// A writer that cannot flush must not take the readiness run down with it: the
|
||||
// keepalives degrade to buffered writes, but the response still completes.
|
||||
func TestIssue1640NonFlushableWriterStillCompletes(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
w := newNonFlushingResponseWriter()
|
||||
if _, ok := http.ResponseWriter(w).(http.Flusher); ok {
|
||||
t.Fatal("test writer must not implement http.Flusher")
|
||||
}
|
||||
|
||||
result := streamPatrolModelReadinessKeepalives(w, 5*time.Millisecond, func() ai.PatrolModelReadinessResult {
|
||||
time.Sleep(20 * time.Millisecond)
|
||||
return passingReadinessResult("verified without a flusher")
|
||||
})
|
||||
response := patrolModelReadinessSnapshot(&result, time.Now())
|
||||
require.NoError(t, utils.WriteJSONResponse(w, response))
|
||||
|
||||
assert.Equal(t, http.StatusOK, w.statusCode)
|
||||
var snapshot PatrolModelReadinessSnapshot
|
||||
require.NoError(t, json.Unmarshal([]byte(w.body.String()), &snapshot))
|
||||
require.NotNil(t, snapshot.PatrolModelReadinessResult)
|
||||
assert.Equal(t, "verified without a flusher", snapshot.Summary)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2579,6 +2579,7 @@ None yet.
|
|||
"test_prefixes": [],
|
||||
"exact_files": [
|
||||
"frontend-modern/src/components/Settings/__tests__/dataHandlingPanelModel.test.ts",
|
||||
"frontend-modern/src/components/Settings/__tests__/patrolReadinessBanner.issue1640.test.ts",
|
||||
"frontend-modern/src/components/Settings/__tests__/settingsArchitecture.test.ts",
|
||||
"frontend-modern/src/components/Settings/__tests__/useAuditLogPanelState.test.tsx",
|
||||
],
|
||||
|
|
@ -2605,6 +2606,7 @@ None yet.
|
|||
"test_prefixes": [],
|
||||
"exact_files": [
|
||||
"frontend-modern/src/components/Settings/__tests__/dataHandlingPanelModel.test.ts",
|
||||
"frontend-modern/src/components/Settings/__tests__/patrolReadinessBanner.issue1640.test.ts",
|
||||
"frontend-modern/src/components/Settings/__tests__/settingsArchitecture.test.ts",
|
||||
"frontend-modern/src/components/Settings/__tests__/useAuditLogPanelState.test.tsx",
|
||||
],
|
||||
|
|
@ -2631,6 +2633,7 @@ None yet.
|
|||
"test_prefixes": [],
|
||||
"exact_files": [
|
||||
"frontend-modern/src/components/Settings/__tests__/dataHandlingPanelModel.test.ts",
|
||||
"frontend-modern/src/components/Settings/__tests__/patrolReadinessBanner.issue1640.test.ts",
|
||||
"frontend-modern/src/components/Settings/__tests__/settingsArchitecture.test.ts",
|
||||
"frontend-modern/src/components/Settings/__tests__/useAuditLogPanelState.test.tsx",
|
||||
],
|
||||
|
|
@ -2663,6 +2666,7 @@ None yet.
|
|||
"test_prefixes": [],
|
||||
"exact_files": [
|
||||
"frontend-modern/src/components/Settings/__tests__/dataHandlingPanelModel.test.ts",
|
||||
"frontend-modern/src/components/Settings/__tests__/patrolReadinessBanner.issue1640.test.ts",
|
||||
"frontend-modern/src/components/Settings/__tests__/settingsArchitecture.test.ts",
|
||||
"frontend-modern/src/components/Settings/__tests__/useAuditLogPanelState.test.tsx",
|
||||
],
|
||||
|
|
@ -2733,6 +2737,7 @@ None yet.
|
|||
"settings-shell-and-framing",
|
||||
[
|
||||
"frontend-modern/src/components/Settings/__tests__/dataHandlingPanelModel.test.ts",
|
||||
"frontend-modern/src/components/Settings/__tests__/patrolReadinessBanner.issue1640.test.ts",
|
||||
"frontend-modern/src/components/Settings/__tests__/settingsArchitecture.test.ts",
|
||||
"frontend-modern/src/components/Settings/__tests__/useAuditLogPanelState.test.tsx",
|
||||
],
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue