diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index 1c918f08c..738ca0486 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -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 diff --git a/docs/release-control/v6/internal/subsystems/ai-runtime.md b/docs/release-control/v6/internal/subsystems/ai-runtime.md index 7717e44ee..fc6034012 100644 --- a/docs/release-control/v6/internal/subsystems/ai-runtime.md +++ b/docs/release-control/v6/internal/subsystems/ai-runtime.md @@ -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 diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index f58b9d0d8..bb8017414 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -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 diff --git a/docs/release-control/v6/internal/subsystems/cloud-paid.md b/docs/release-control/v6/internal/subsystems/cloud-paid.md index 722cca5a9..b3123cd35 100644 --- a/docs/release-control/v6/internal/subsystems/cloud-paid.md +++ b/docs/release-control/v6/internal/subsystems/cloud-paid.md @@ -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 diff --git a/docs/release-control/v6/internal/subsystems/registry.json b/docs/release-control/v6/internal/subsystems/registry.json index 39429ddd8..09c7b67eb 100644 --- a/docs/release-control/v6/internal/subsystems/registry.json +++ b/docs/release-control/v6/internal/subsystems/registry.json @@ -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" ] }, diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index 67949e322..2cb65c811 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -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 diff --git a/frontend-modern/src/utils/__tests__/apiClient.issue1640.test.ts b/frontend-modern/src/utils/__tests__/apiClient.issue1640.test.ts new file mode 100644 index 000000000..d64a138cd --- /dev/null +++ b/frontend-modern/src/utils/__tests__/apiClient.issue1640.test.ts @@ -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 = [ + '', + '
block but rejects markup inside it', async () => {
+ const plain = await apiErrorFromResponse(
+ new Response('proxy read timeout
', { status: 504 }),
+ );
+ expect(plain.message).toBe('proxy read timeout');
+
+ const markup = await apiErrorFromResponse(
+ new Response('504 upstream timed out
', {
+ 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.');
+ });
+});
diff --git a/frontend-modern/src/utils/apiClient.ts b/frontend-modern/src/utils/apiClient.ts
index 5b3a839dd..fc4b71623 100644
--- a/frontend-modern/src/utils/apiClient.ts
+++ b/frontend-modern/src/utils/apiClient.ts
@@ -113,7 +113,10 @@ async function createAPIErrorFromResponse(
fallbackMessage?: string,
): Promise {
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('') && text.includes('')) {
const match = text.match(/(.*?)<\/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}`;
}
}
diff --git a/internal/ai/issue1640_readiness_cancellation_test.go b/internal/ai/issue1640_readiness_cancellation_test.go
new file mode 100644
index 000000000..5d76e5449
--- /dev/null
+++ b/internal/ai/issue1640_readiness_cancellation_test.go
@@ -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)
+ }
+}
diff --git a/internal/ai/patrol_model_readiness.go b/internal/ai/patrol_model_readiness.go
index 8680c337b..47d59c748 100644
--- a/internal/ai/patrol_model_readiness.go
+++ b/internal/ai/patrol_model_readiness.go
@@ -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."
diff --git a/internal/ai/patrol_model_readiness_test.go b/internal/ai/patrol_model_readiness_test.go
index 12cc3c72d..23985aee3 100644
--- a/internal/ai/patrol_model_readiness_test.go
+++ b/internal/ai/patrol_model_readiness_test.go
@@ -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)
}
}
diff --git a/internal/ai/patrol_readiness.go b/internal/ai/patrol_readiness.go
index 541bc8990..284e08ba7 100644
--- a/internal/ai/patrol_readiness.go
+++ b/internal/ai/patrol_readiness.go
@@ -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 {
diff --git a/internal/ai/patrol_runtime_failure.go b/internal/ai/patrol_runtime_failure.go
index 4b8879301..813a5912f 100644
--- a/internal/ai/patrol_runtime_failure.go
+++ b/internal/ai/patrol_runtime_failure.go
@@ -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):
diff --git a/internal/api/ai_handlers.go b/internal/api/ai_handlers.go
index 5e4afe82c..c5e8b6d1f 100644
--- a/internal/api/ai_handlers.go
+++ b/internal/api/ai_handlers.go
@@ -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")
diff --git a/internal/api/issue1640_readiness_transport_test.go b/internal/api/issue1640_readiness_transport_test.go
new file mode 100644
index 000000000..2064dbb3f
--- /dev/null
+++ b/internal/api/issue1640_readiness_transport_test.go
@@ -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")
+}