mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-08-04 13:23:28 +00:00
Keep the Patrol readiness check alive through proxies and classify cancellation honestly
Fixes #1640. Three defects around POST /api/ai/patrol/readiness on slow local hardware behind a reverse proxy: 1. The handler ran up to four sequential provider calls (~45s and more on slow Ollama boxes) while writing nothing to the response, so any intermediary with a ~30s read timeout severed the request mid-run. The handler now commits headers up front and streams flushed newline keepalives every 10s while the evaluation runs, then appends the normal JSON payload. Leading newlines are insignificant JSON whitespace, so existing clients parse the response unchanged. 2. A severed connection cancels the request context, and patrolRuntimeFailureFromError classified the resulting context.Canceled as a generic "Provider analysis error", blaming the provider and model for an infrastructure event. Mid-run cancellation is now classified as the new "interrupted" cause: the overall status and every unfinished dimension and autonomy mode report not assessed, per-scenario evidence completed before the interruption is preserved in the returned result, and the readiness cache keeps the last completed evaluation. context.DeadlineExceeded keeps its provider-path timeout classification. 3. createAPIErrorFromResponse pre-seeded the error message with the raw response body, making its non-JSON guard dead code, so full HTML proxy error pages became Error.message and were rendered into the readiness result boxes. Non-JSON bodies now surface only when they are short plain text; anything with markup or excessive length collapses to a generic status-derived message. Regression tests: internal/ai/issue1640_readiness_cancellation_test.go, internal/api/issue1640_readiness_transport_test.go, and frontend-modern/src/utils/__tests__/apiClient.issue1640.test.ts, all registered in the subsystem verification registry. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
193c96afc3
commit
8d0d74e35c
15 changed files with 488 additions and 13 deletions
|
|
@ -2143,7 +2143,11 @@ Agent` secondary handoff against the live setup wizard instead of relying
|
|||
lifecycle. `POST /api/ai/patrol/readiness` may exercise only synthetic
|
||||
provider tools and in-memory tool results under settings-write authority;
|
||||
it must not request `agent:exec`, dispatch an agent command, inspect agent
|
||||
inventory, or turn a successful model probe into command authority.
|
||||
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
|
||||
partial probe evidence grant no agent capability and must not be read as
|
||||
agent lifecycle signals.
|
||||
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
|
||||
|
|
|
|||
|
|
@ -3939,6 +3939,20 @@ relevant timeout budget changes. Monitor/Watch and Approval/Ask First may be
|
|||
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
|
||||
`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).
|
||||
|
||||
## Current State
|
||||
|
||||
### Agent surfaces expose execution-time readiness refusal
|
||||
|
|
|
|||
|
|
@ -3633,6 +3633,16 @@ Failed evaluations carry per-scenario probe and validator evidence in
|
|||
reasons only — never fixture prompts or infrastructure content) so operators
|
||||
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).
|
||||
|
||||
Every mobile-facing contract change must update the canonical manifest,
|
||||
regenerate both repositories, keep the mobile consumer minimum compatible, and
|
||||
pass `generate_mobile_compatibility.py --check` plus
|
||||
|
|
|
|||
|
|
@ -2822,6 +2822,11 @@ for hosted tenant requests. Cloud and MSP surfaces may show failures from
|
|||
`frontend-modern/src/utils/apiClient.ts`, but they must resolve canonical JSON
|
||||
`error` / `message` fields before showing UI feedback instead of leaking raw
|
||||
response payloads while tenant-scoped org headers are still in flight.
|
||||
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).
|
||||
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
|
||||
|
|
|
|||
|
|
@ -1905,6 +1905,7 @@
|
|||
"internal/api/ai_handlers_patrol_actions_additional_test.go",
|
||||
"internal/api/ai_handlers_test.go",
|
||||
"internal/api/ai_intelligence_handlers_test.go",
|
||||
"internal/api/issue1640_readiness_transport_test.go",
|
||||
"internal/api/patrol_autopilot_test.go"
|
||||
]
|
||||
},
|
||||
|
|
@ -2871,6 +2872,7 @@
|
|||
"internal/api/contract_test.go",
|
||||
"internal/api/docker_agents_report_size_test.go",
|
||||
"internal/api/host_agent_removal_lifecycle_integration_test.go",
|
||||
"internal/api/issue1640_readiness_transport_test.go",
|
||||
"internal/api/metadata_handlers_test.go",
|
||||
"internal/api/patrol_autopilot_test.go",
|
||||
"pulse-enterprise:test/extensions_contract_test.go"
|
||||
|
|
@ -3953,6 +3955,7 @@
|
|||
"test_prefixes": [],
|
||||
"exact_files": [
|
||||
"frontend-modern/src/__tests__/App.architecture.test.ts",
|
||||
"frontend-modern/src/utils/__tests__/apiClient.issue1640.test.ts",
|
||||
"frontend-modern/src/utils/__tests__/apiClient.org.test.ts"
|
||||
]
|
||||
},
|
||||
|
|
|
|||
|
|
@ -1956,7 +1956,10 @@ Keep Patrol model-readiness context fixtures synthetic and isolated from
|
|||
storage/recovery state. The advisor may model backup failure and storage
|
||||
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.
|
||||
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.
|
||||
|
||||
26. Storage row presentation resolves its topology label from
|
||||
`storage.vdevLayout` first and falls back to `storage.topology`, so a
|
||||
|
|
|
|||
|
|
@ -0,0 +1,66 @@
|
|||
// Regression coverage for issue #1640: a reverse proxy that cuts a slow
|
||||
// Patrol readiness request answers with a full HTML error page. That body
|
||||
// must never become Error.message, because the message is rendered directly
|
||||
// into the readiness result boxes in AI settings.
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { apiErrorFromResponse } from '@/utils/apiClient';
|
||||
|
||||
const NGINX_GATEWAY_TIMEOUT = [
|
||||
'<html>',
|
||||
'<head><title>504 Gateway Time-out</title></head>',
|
||||
'<body>',
|
||||
'<center><h1>504 Gateway Time-out</h1></center>',
|
||||
'<hr><center>nginx</center>',
|
||||
'</body>',
|
||||
'</html>',
|
||||
].join('\r\n');
|
||||
|
||||
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 }),
|
||||
);
|
||||
expect(error.message).toBe('Request failed with status 504');
|
||||
expect(error.message).not.toContain('<');
|
||||
expect(error.status).toBe(504);
|
||||
});
|
||||
|
||||
it('replaces a long non-JSON body with a generic status message', async () => {
|
||||
const error = await apiErrorFromResponse(new Response('x'.repeat(500), { status: 502 }));
|
||||
expect(error.message).toBe('Request failed with status 502');
|
||||
});
|
||||
|
||||
it('keeps a short plain-text body as the message', async () => {
|
||||
const error = await apiErrorFromResponse(new Response('upstream unavailable', { status: 503 }));
|
||||
expect(error.message).toBe('upstream unavailable');
|
||||
});
|
||||
|
||||
it('extracts short plain text from a <pre> block but rejects markup inside it', async () => {
|
||||
const plain = await apiErrorFromResponse(
|
||||
new Response('<html><body><pre>proxy read timeout</pre></body></html>', { status: 504 }),
|
||||
);
|
||||
expect(plain.message).toBe('proxy read timeout');
|
||||
|
||||
const markup = await apiErrorFromResponse(
|
||||
new Response('<html><body><pre><b>504</b> upstream timed out</pre></body></html>', {
|
||||
status: 504,
|
||||
}),
|
||||
);
|
||||
expect(markup.message).toBe('Request failed with status 504');
|
||||
});
|
||||
|
||||
it('still prefers a JSON error payload over any fallback handling', async () => {
|
||||
const error = await apiErrorFromResponse(
|
||||
new Response(JSON.stringify({ error: 'Patrol model readiness failed' }), { status: 500 }),
|
||||
);
|
||||
expect(error.message).toBe('Patrol model readiness failed');
|
||||
});
|
||||
|
||||
it('uses the caller fallback message for unusable bodies', async () => {
|
||||
const error = await apiErrorFromResponse(
|
||||
new Response(NGINX_GATEWAY_TIMEOUT, { status: 504 }),
|
||||
'Pulse could not run the Patrol model readiness evaluation.',
|
||||
);
|
||||
expect(error.message).toBe('Pulse could not run the Patrol model readiness evaluation.');
|
||||
});
|
||||
});
|
||||
|
|
@ -113,7 +113,10 @@ async function createAPIErrorFromResponse(
|
|||
fallbackMessage?: string,
|
||||
): Promise<APIErrorShape> {
|
||||
const text = await response.text();
|
||||
let errorMessage = fallbackMessage || text;
|
||||
// Never pre-seed the message with the raw body: a reverse proxy error page
|
||||
// is a full HTML document, and whatever lands here becomes Error.message
|
||||
// rendered directly in the UI (#1640).
|
||||
let errorMessage = fallbackMessage || '';
|
||||
|
||||
let errorCode: string | undefined;
|
||||
let errorDetail: string | undefined;
|
||||
|
|
@ -137,14 +140,20 @@ async function createAPIErrorFromResponse(
|
|||
errorFeature = sanitizeBoundedText(jsonError.feature, 128) ?? undefined;
|
||||
errorUpgradeUrl = sanitizeBoundedText(jsonError.upgrade_url, 2048) ?? undefined;
|
||||
} 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);
|
||||
if (match) errorMessage = match[1];
|
||||
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();
|
||||
}
|
||||
|
||||
if (!text.includes('<') && text.length < 200) {
|
||||
errorMessage = text;
|
||||
} else if (!errorMessage && text.length > 200) {
|
||||
if (!errorMessage) {
|
||||
errorMessage = `Request failed with status ${response.status}`;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
144
internal/ai/issue1640_readiness_cancellation_test.go
Normal file
144
internal/ai/issue1640_readiness_cancellation_test.go
Normal file
|
|
@ -0,0 +1,144 @@
|
|||
package ai
|
||||
|
||||
// Regression coverage for issue #1640: a Patrol model readiness run cancelled
|
||||
// mid-flight (operator cancel or a reverse proxy dropping the connection)
|
||||
// must be classified as interrupted, not blamed on the provider or model, and
|
||||
// the per-scenario evidence completed before the cancellation must survive
|
||||
// into the returned result.
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/rcourtman/pulse-go-rewrite/internal/ai/providers"
|
||||
"github.com/rcourtman/pulse-go-rewrite/internal/config"
|
||||
)
|
||||
|
||||
func TestIssue1640ContextCanceledClassifiedAsInterrupted(t *testing.T) {
|
||||
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)
|
||||
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)
|
||||
}
|
||||
}
|
||||
|
||||
diagnostic := ClassifyPatrolRuntimeFailure(context.Canceled)
|
||||
if diagnostic.Cause != PatrolFailureCauseInterrupted {
|
||||
t.Fatalf("diagnostic cause = %q, want %q", diagnostic.Cause, PatrolFailureCauseInterrupted)
|
||||
}
|
||||
}
|
||||
|
||||
func TestIssue1640DeadlineExceededStaysProviderConnection(t *testing.T) {
|
||||
// A deadline is a real timeout on the provider path and must keep its
|
||||
// existing provider-connection classification.
|
||||
failure := patrolRuntimeFailureFromError(context.DeadlineExceeded)
|
||||
if failure.Cause != PatrolFailureCauseProviderConnection {
|
||||
t.Fatalf("deadline exceeded classified as %q, want %q", failure.Cause, PatrolFailureCauseProviderConnection)
|
||||
}
|
||||
}
|
||||
|
||||
// issue1640CancellingProvider answers the first readiness scenario correctly,
|
||||
// then cancels the run context before the second scenario completes —
|
||||
// simulating a proxy cutting the connection partway through a slow local run.
|
||||
type issue1640CancellingProvider struct {
|
||||
cancel context.CancelFunc
|
||||
calls int
|
||||
}
|
||||
|
||||
func (p *issue1640CancellingProvider) Chat(context.Context, providers.ChatRequest) (*providers.ChatResponse, error) {
|
||||
return nil, errors.New("readiness evaluator must use the streaming provider path")
|
||||
}
|
||||
|
||||
func (p *issue1640CancellingProvider) TestConnection(context.Context) error { return nil }
|
||||
func (p *issue1640CancellingProvider) Name() string { return "issue1640" }
|
||||
func (p *issue1640CancellingProvider) ListModels(context.Context) ([]providers.ModelInfo, error) {
|
||||
return []providers.ModelInfo{{ID: "test-model"}}, nil
|
||||
}
|
||||
func (p *issue1640CancellingProvider) SupportsThinking(string) bool { return false }
|
||||
|
||||
func (p *issue1640CancellingProvider) ChatStream(ctx context.Context, req providers.ChatRequest, callback providers.StreamCallback) error {
|
||||
p.calls++
|
||||
if p.calls == 1 {
|
||||
input := readinessArgumentsFromPrompt(req.Messages[0].Content)
|
||||
call := providers.ToolCall{ID: "observation-call", Name: patrolReadinessObservationTool, Input: input}
|
||||
callback(providers.StreamEvent{Type: "tool_start", Data: providers.ToolStartEvent{ID: call.ID, Name: call.Name, Input: call.Input}})
|
||||
callback(providers.StreamEvent{Type: "done", Data: providers.DoneEvent{StopReason: "tool_use", ToolCalls: []providers.ToolCall{call}, InputTokens: 100, OutputTokens: 10}})
|
||||
return nil
|
||||
}
|
||||
p.cancel()
|
||||
<-ctx.Done()
|
||||
return ctx.Err()
|
||||
}
|
||||
|
||||
func TestIssue1640MidRunCancellationKeepsPartialEvidence(t *testing.T) {
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
defer cancel()
|
||||
provider := &issue1640CancellingProvider{cancel: cancel}
|
||||
|
||||
result := runPatrolModelReadinessWithProvider(
|
||||
ctx, readinessTestConfig(), config.AIProviderOllama, "test-model", "ollama:test-model", provider,
|
||||
)
|
||||
|
||||
if result.Cause != PatrolFailureCauseInterrupted {
|
||||
t.Fatalf("mid-run cancellation classified as %q, want %q", result.Cause, PatrolFailureCauseInterrupted)
|
||||
}
|
||||
if result.Status != PatrolModelReadinessNotAssessed {
|
||||
t.Fatalf("interrupted run status = %q, want %q", result.Status, PatrolModelReadinessNotAssessed)
|
||||
}
|
||||
if result.Success || result.PatrolCapable {
|
||||
t.Fatalf("interrupted run must not verify the model: %+v", result)
|
||||
}
|
||||
|
||||
// Evidence from the completed scenario survives.
|
||||
tool := result.Dimensions.ToolProtocol
|
||||
if tool.Status != PatrolModelReadinessNotAssessed || tool.Passed != 1 {
|
||||
t.Fatalf("interrupted tool dimension must keep partial evidence, got %+v", tool)
|
||||
}
|
||||
if !strings.Contains(tool.Summary, "Interrupted after 1/3") {
|
||||
t.Fatalf("tool summary should report partial progress, got %q", tool.Summary)
|
||||
}
|
||||
if len(result.Details) == 0 || !strings.Contains(strings.Join(result.Details, "\n"), "context canceled") {
|
||||
t.Fatalf("per-scenario details must be preserved, got %v", result.Details)
|
||||
}
|
||||
|
||||
// The unfinished dimensions and modes report not assessed instead of
|
||||
// blaming the model.
|
||||
if result.Dimensions.ContextQuality.Status != PatrolModelReadinessNotAssessed {
|
||||
t.Fatalf("context dimension = %+v, want not assessed", result.Dimensions.ContextQuality)
|
||||
}
|
||||
if result.Dimensions.Latency.Status != PatrolModelReadinessNotAssessed {
|
||||
t.Fatalf("latency dimension = %+v, want not assessed", result.Dimensions.Latency)
|
||||
}
|
||||
if result.Modes.Monitor.Status != PatrolModeNotAssessed || result.Modes.Approval.Status != PatrolModeNotAssessed {
|
||||
t.Fatalf("interrupted run must leave modes unassessed: %+v", result.Modes)
|
||||
}
|
||||
if strings.Contains(result.Summary, "Provider analysis error") {
|
||||
t.Fatalf("interrupted run summary must not blame the provider: %q", result.Summary)
|
||||
}
|
||||
}
|
||||
|
||||
func TestIssue1640PreCancelledRunReportsInterrupted(t *testing.T) {
|
||||
cfg := readinessTestConfig()
|
||||
service := NewService(nil, nil)
|
||||
service.cfg = cfg
|
||||
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
cancel()
|
||||
result := service.RunPatrolModelReadiness(ctx, "", "")
|
||||
if result.Cause != PatrolFailureCauseInterrupted {
|
||||
t.Fatalf("pre-cancelled run classified as %q, want %q", result.Cause, PatrolFailureCauseInterrupted)
|
||||
}
|
||||
if result.Status != PatrolModelReadinessNotAssessed {
|
||||
t.Fatalf("pre-cancelled run status = %q, want %q", result.Status, PatrolModelReadinessNotAssessed)
|
||||
}
|
||||
}
|
||||
|
|
@ -390,7 +390,8 @@ func (s *Service) RunPatrolModelReadiness(ctx context.Context, providerName, mod
|
|||
}
|
||||
result.Provider, result.Model = config.ParseModelString(modelString)
|
||||
if err := ctx.Err(); err != nil {
|
||||
result.Cause = PatrolFailureCauseProviderConnection
|
||||
result.Cause = PatrolFailureCauseInterrupted
|
||||
result.Status = PatrolModelReadinessNotAssessed
|
||||
result.Summary = "The Patrol model readiness evaluation was cancelled."
|
||||
result.Recommendation = "Run the advisor again when you are ready."
|
||||
return finish()
|
||||
|
|
@ -424,6 +425,12 @@ func (s *Service) RunPatrolModelReadiness(ctx context.Context, providerName, mod
|
|||
if err != nil {
|
||||
failure := patrolRuntimeFailureFromError(err)
|
||||
result.Cause = failure.Cause
|
||||
if failure.Cause == PatrolFailureCauseInterrupted {
|
||||
result.Status = PatrolModelReadinessNotAssessed
|
||||
result.Summary = failure.Summary
|
||||
result.Recommendation = failure.Recommendation
|
||||
return finish()
|
||||
}
|
||||
result.Summary = "Patrol is already using the selected model."
|
||||
result.Recommendation = "Wait for the active Patrol work to finish, then run the advisor again."
|
||||
return finish()
|
||||
|
|
@ -473,6 +480,20 @@ func runPatrolModelReadinessWithProvider(ctx context.Context, cfg *config.AIConf
|
|||
if err := provider.TestConnection(ctx); err != nil {
|
||||
failure := patrolRuntimeFailureFromError(err)
|
||||
result.Cause = failure.Cause
|
||||
// A cancelled run carries no connectivity evidence: report it as not
|
||||
// assessed instead of blaming the provider (#1640).
|
||||
if failure.Cause == PatrolFailureCauseInterrupted {
|
||||
result.Status = PatrolModelReadinessNotAssessed
|
||||
result.Summary = failure.Summary
|
||||
result.Recommendation = failure.Recommendation
|
||||
result.Dimensions.Connectivity = PatrolModelReadinessDimension{
|
||||
Status: PatrolModelReadinessNotAssessed,
|
||||
Summary: "The evaluation was interrupted before connectivity could be assessed.",
|
||||
DurationMs: time.Since(connectionStarted).Milliseconds(),
|
||||
}
|
||||
result.DurationMs = time.Since(started).Milliseconds()
|
||||
return result
|
||||
}
|
||||
result.Status = PatrolModelReadinessFail
|
||||
result.Summary = failure.Summary
|
||||
result.Recommendation = failure.Recommendation
|
||||
|
|
@ -677,6 +698,19 @@ func runPatrolModelReadinessWithProvider(ctx context.Context, cfg *config.AIConf
|
|||
probeFailure = &failure
|
||||
toolSummary = failure.Summary
|
||||
}
|
||||
// A mid-run cancellation invalidates nothing the model already proved and
|
||||
// proves nothing about what it never attempted: keep completed per-scenario
|
||||
// evidence, report the unfinished dimensions as not assessed (#1640).
|
||||
interrupted := probeFailure != nil && probeFailure.Cause == PatrolFailureCauseInterrupted
|
||||
if interrupted {
|
||||
if toolPassed == len(scenarios) {
|
||||
toolStatus = PatrolModelReadinessPass
|
||||
toolSummary = "All scenarios passed before the run was interrupted; multi-turn continuation was not assessed."
|
||||
} else {
|
||||
toolStatus = PatrolModelReadinessNotAssessed
|
||||
toolSummary = fmt.Sprintf("Interrupted after %d/%d scenarios passed; the remaining scenarios were not assessed.", toolPassed, len(scenarios))
|
||||
}
|
||||
}
|
||||
result.Dimensions.ToolProtocol = PatrolModelReadinessDimension{
|
||||
Status: toolStatus,
|
||||
Summary: toolSummary,
|
||||
|
|
@ -691,6 +725,9 @@ func runPatrolModelReadinessWithProvider(ctx context.Context, cfg *config.AIConf
|
|||
if contextPassed == 2 {
|
||||
contextStatus = PatrolModelReadinessPass
|
||||
contextSummary = "Both Patrol-shaped fixtures selected the actionable resource without flagging healthy decoys."
|
||||
} else if interrupted {
|
||||
contextStatus = PatrolModelReadinessNotAssessed
|
||||
contextSummary = fmt.Sprintf("Interrupted after %d/2 context fixtures passed; the remaining fixtures were not assessed.", contextPassed)
|
||||
}
|
||||
if result.Metadata != nil && result.Metadata.ContextWindow > 0 && result.Metadata.ContextWindow < patrolReadinessMinimumContext {
|
||||
contextStatus = PatrolModelReadinessFail
|
||||
|
|
@ -712,7 +749,10 @@ func runPatrolModelReadinessWithProvider(ctx context.Context, cfg *config.AIConf
|
|||
projectedApproval := warmP50 * time.Duration(cfg.GetPatrolInvestigationBudget())
|
||||
latencyStatus := PatrolModelReadinessPass
|
||||
latencySummary := fmt.Sprintf("Warm median %s; projected 8-turn Watch-only loop %s.", formatReadinessDuration(warmP50), formatReadinessDuration(projectedWatch))
|
||||
if len(durations) == 0 || probeErr != nil {
|
||||
if interrupted {
|
||||
latencyStatus = PatrolModelReadinessNotAssessed
|
||||
latencySummary = "Latency was not fully measured because the evaluation was interrupted."
|
||||
} else if len(durations) == 0 || probeErr != nil {
|
||||
latencyStatus = PatrolModelReadinessFail
|
||||
latencySummary = "Latency could not be measured because the streaming probe did not complete."
|
||||
} else if projectedWatch > patrolReadinessWatchEnvelope {
|
||||
|
|
@ -763,6 +803,11 @@ func runPatrolModelReadinessWithProvider(ctx context.Context, cfg *config.AIConf
|
|||
result.Summary = probeFailure.Summary
|
||||
result.Recommendation = probeFailure.Recommendation
|
||||
}
|
||||
if interrupted {
|
||||
result.Status = PatrolModelReadinessNotAssessed
|
||||
result.Modes.Monitor = PatrolModeSuitability{Status: PatrolModeNotAssessed, Summary: "The evaluation was interrupted before Watch-only readiness could be assessed."}
|
||||
result.Modes.Approval = PatrolModeSuitability{Status: PatrolModeNotAssessed, Summary: "The evaluation was interrupted before Ask-first readiness could be assessed."}
|
||||
}
|
||||
if contextStatus == PatrolModelReadinessFail && toolStatus == PatrolModelReadinessPass {
|
||||
result.Cause = PatrolFailureCauseContextQualityFailed
|
||||
result.Summary = "The selected model did not pass Patrol's context-quality evaluation."
|
||||
|
|
|
|||
|
|
@ -279,8 +279,11 @@ func TestRunPatrolModelReadinessWithProvider_CancellationStopsStreamingProbe(t *
|
|||
if time.Since(started) > time.Second {
|
||||
t.Fatal("cancelled readiness evaluation did not stop promptly")
|
||||
}
|
||||
if result.Success || result.Dimensions.ToolProtocol.Status != PatrolModelReadinessFail {
|
||||
t.Fatalf("cancelled evaluation should not pass, got %+v", result)
|
||||
if result.Success || result.Dimensions.ToolProtocol.Status != PatrolModelReadinessNotAssessed {
|
||||
t.Fatalf("cancelled evaluation should not pass and must stay unassessed, got %+v", result)
|
||||
}
|
||||
if result.Cause != PatrolFailureCauseInterrupted {
|
||||
t.Fatalf("cancelled evaluation must be classified as interrupted, not %q", result.Cause)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -36,7 +36,12 @@ const (
|
|||
PatrolFailureCauseProviderRateLimited PatrolFailureCause = "provider_rate_limited"
|
||||
PatrolFailureCauseProviderAuth PatrolFailureCause = "provider_auth"
|
||||
PatrolFailureCauseProviderConnection PatrolFailureCause = "provider_connection"
|
||||
PatrolFailureCauseCircuitOpen PatrolFailureCause = "circuit_open"
|
||||
// PatrolFailureCauseInterrupted marks a run that was cancelled mid-flight
|
||||
// (operator cancel or a dropped client connection). It is deliberately not
|
||||
// a provider fault: an interrupted run carries no evidence about the
|
||||
// provider or model (#1640).
|
||||
PatrolFailureCauseInterrupted PatrolFailureCause = "interrupted"
|
||||
PatrolFailureCauseCircuitOpen PatrolFailureCause = "circuit_open"
|
||||
)
|
||||
|
||||
type PatrolConfigReadiness struct {
|
||||
|
|
|
|||
|
|
@ -1,6 +1,8 @@
|
|||
package ai
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"regexp"
|
||||
"strings"
|
||||
|
|
@ -115,6 +117,20 @@ func patrolMalformedToolHistory(lower string) bool {
|
|||
strings.Contains(lower, "responding to each")
|
||||
}
|
||||
|
||||
// patrolRunCancelled reports whether the upstream error is a mid-run
|
||||
// cancellation rather than a provider failure. context.Canceled reaches this
|
||||
// classifier when the operator cancels the run or the HTTP client connection
|
||||
// drops (a reverse proxy cutting a long request cancels r.Context()). It is
|
||||
// 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 {
|
||||
if errors.Is(err, context.Canceled) {
|
||||
return true
|
||||
}
|
||||
return strings.Contains(lower, "context canceled")
|
||||
}
|
||||
|
||||
func ClassifyPatrolRuntimeFailure(err error) PatrolRuntimeFailureDiagnostic {
|
||||
failure := patrolRuntimeFailureFromError(err)
|
||||
return PatrolRuntimeFailureDiagnostic{
|
||||
|
|
@ -137,6 +153,11 @@ func ClassifyProviderConnectionFailure(err error) PatrolRuntimeFailureDiagnostic
|
|||
}
|
||||
|
||||
switch failure.Cause {
|
||||
case PatrolFailureCauseInterrupted:
|
||||
diagnostic.Title = "Connection test interrupted"
|
||||
diagnostic.Summary = "Connection test interrupted"
|
||||
diagnostic.Description = "The connection test was cancelled before the provider finished responding."
|
||||
diagnostic.Recommendation = "Run the test again when you are ready."
|
||||
case PatrolFailureCauseMalformedToolHistory:
|
||||
diagnostic.Title = "Provider conversation state issue"
|
||||
diagnostic.Summary = "Provider conversation state issue"
|
||||
|
|
@ -211,6 +232,12 @@ func patrolRuntimeFailureFromError(err error) patrolRuntimeFailure {
|
|||
}
|
||||
|
||||
switch {
|
||||
case patrolRunCancelled(err, lower):
|
||||
failure.Title = "Pulse Patrol: Analysis interrupted"
|
||||
failure.Summary = "Analysis interrupted before completion"
|
||||
failure.Cause = PatrolFailureCauseInterrupted
|
||||
failure.Description = "The Patrol run was cancelled before the provider finished, either by an operator cancel or because the client connection closed mid-analysis. An interrupted run is not evidence about the provider or model."
|
||||
failure.Recommendation = "Run the analysis again when you are ready. If you did not cancel it, check for reverse proxies or load balancers that close long-running requests."
|
||||
case patrolMalformedToolHistory(lower):
|
||||
failure.Title = "Pulse Patrol: Malformed tool-call conversation history"
|
||||
failure.Summary = "Malformed tool-call conversation history"
|
||||
|
|
@ -329,6 +356,8 @@ func summarizePatrolRuntimeFailureDetail(raw string) string {
|
|||
}
|
||||
lower := strings.ToLower(raw)
|
||||
switch {
|
||||
case strings.Contains(lower, "context canceled"):
|
||||
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."
|
||||
case patrolToolChoiceValueRejected(lower):
|
||||
|
|
|
|||
|
|
@ -6201,10 +6201,48 @@ func patrolModelReadinessBudget(cfg *config.AIConfig) time.Duration {
|
|||
return budget
|
||||
}
|
||||
|
||||
// patrolModelReadinessKeepaliveInterval paces the whitespace keepalives
|
||||
// written while a readiness evaluation runs. A full advisor run makes four
|
||||
// sequential provider calls and can legitimately take minutes on slow local
|
||||
// hardware; without response bytes in flight, reverse proxies with ~30-second
|
||||
// read timeouts cut the connection mid-run (#1640). Ten seconds stays well
|
||||
// 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).
|
||||
func streamPatrolModelReadinessKeepalives(w http.ResponseWriter, interval time.Duration, evaluate func() ai.PatrolModelReadinessResult) ai.PatrolModelReadinessResult {
|
||||
flusher, _ := w.(http.Flusher)
|
||||
resultCh := make(chan ai.PatrolModelReadinessResult, 1)
|
||||
go func() { resultCh <- evaluate() }()
|
||||
|
||||
ticker := time.NewTicker(interval)
|
||||
defer ticker.Stop()
|
||||
for {
|
||||
select {
|
||||
case result := <-resultCh:
|
||||
return result
|
||||
case <-ticker.C:
|
||||
// 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()
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// HandlePatrolModelReadiness runs the explicit, multi-scenario Patrol model
|
||||
// advisor. Unlike the startup preflight, this uses Patrol's streaming
|
||||
// transport, exact typed arguments, two context fixtures, and a multi-turn tool
|
||||
// result. The request context makes the evaluation cancellable from the UI.
|
||||
// The response streams whitespace keepalives while the evaluation runs so
|
||||
// reverse proxies with short read timeouts do not sever the request (#1640).
|
||||
func (h *AISettingsHandler) HandlePatrolModelReadiness(w http.ResponseWriter, r *http.Request) {
|
||||
if r.Method != http.MethodPost {
|
||||
http.Error(w, "Method not allowed", http.StatusMethodNotAllowed)
|
||||
|
|
@ -6239,7 +6277,16 @@ func (h *AISettingsHandler) HandlePatrolModelReadiness(w http.ResponseWriter, r
|
|||
|
||||
ctx, cancel := context.WithTimeout(r.Context(), patrolModelReadinessBudget(aiService.GetConfig()))
|
||||
defer cancel()
|
||||
result := aiService.RunPatrolModelReadiness(ctx, body.Provider, body.Model)
|
||||
|
||||
// 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")
|
||||
result := streamPatrolModelReadinessKeepalives(w, patrolModelReadinessKeepaliveInterval, func() ai.PatrolModelReadinessResult {
|
||||
return aiService.RunPatrolModelReadiness(ctx, body.Provider, body.Model)
|
||||
})
|
||||
response := patrolModelReadinessSnapshot(&result, time.Now())
|
||||
if err := utils.WriteJSONResponse(w, response); err != nil {
|
||||
log.Error().Err(err).Msg("Failed to write Patrol model readiness response")
|
||||
|
|
|
|||
88
internal/api/issue1640_readiness_transport_test.go
Normal file
88
internal/api/issue1640_readiness_transport_test.go
Normal file
|
|
@ -0,0 +1,88 @@
|
|||
package api
|
||||
|
||||
// Regression coverage for issue #1640: POST /api/ai/patrol/readiness runs up
|
||||
// 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.
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/rcourtman/pulse-go-rewrite/internal/ai"
|
||||
"github.com/rcourtman/pulse-go-rewrite/internal/utils"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
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
|
||||
})
|
||||
response := patrolModelReadinessSnapshot(&result, time.Now())
|
||||
require.NoError(t, utils.WriteJSONResponse(rec, response))
|
||||
|
||||
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.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")
|
||||
|
||||
// The padding is insignificant JSON whitespace: the final payload still
|
||||
// parses as a single ordinary JSON document.
|
||||
var snapshot PatrolModelReadinessSnapshot
|
||||
require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &snapshot))
|
||||
require.NotNil(t, snapshot.PatrolModelReadinessResult)
|
||||
assert.Equal(t, "verified", snapshot.Summary)
|
||||
}
|
||||
|
||||
func TestIssue1640FastEvaluationEmitsNoPadding(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
result := streamPatrolModelReadinessKeepalives(rec, time.Hour, func() ai.PatrolModelReadinessResult {
|
||||
out := ai.PatrolModelReadinessResult{}
|
||||
out.ProbeVersion = ai.PatrolModelReadinessProbeVersion
|
||||
return out
|
||||
})
|
||||
response := patrolModelReadinessSnapshot(&result, time.Now())
|
||||
require.NoError(t, utils.WriteJSONResponse(rec, response))
|
||||
|
||||
assert.True(t, strings.HasPrefix(rec.Body.String(), "{"),
|
||||
"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")
|
||||
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
|
||||
}
|
||||
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")
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue