From 4d972a68ee9621774c0ba00863d0578066d38875 Mon Sep 17 00:00:00 2001 From: "courtmanr@gmail.com" Date: Tue, 28 Jul 2026 13:56:58 +0100 Subject: [PATCH] Gate Patrol readiness on the completed overall verdict (#1640) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "Patrol tools" readiness check read the cached model-readiness snapshot's tool-protocol dimension on its own. Since the interrupted-run handling landed (8d0d74e35, b78330405), a run cancelled after every tool scenario already passed keeps ToolProtocol at pass while the overall status reports not_assessed, so the check reported "Patrol ready" from an evaluation that never completed. The check now requires the snapshot's own overall verdict (Success) before reporting ready. A snapshot carrying no verdict at all — overall status not_assessed, or the interrupted or internal_error cause — is not turned into a failure either: it falls back to the base-config classifier exactly as an absent snapshot does, capped at a warning. That cap matters because not_ready is a blocking status in this payload: it clears readiness.ready, which disables the Patrol run control in usePatrolIntelligenceState and drops the page into the setup-only view. #1640 promises a severed or cancelled check never blames the model and never blocks Patrol from running in Watch mode, and the runtime gate on POST /api/ai/patrol/run (PatrolRuntimeReadiness) already treats an unassessed mode as a warning, so a blocking tools check would have contradicted the route that actually runs Patrol. A completed run whose tool protocol passed while the overall verdict fell short now warns instead of claiming ready. It must not block either: the dimension that actually failed carries the verdict on its own check (context quality blocks, latency warns), so blocking here would have turned today's latency warning into a hard stop. Regression tests: internal/api/issue1640_readiness_gate_test.go covers the gate across interrupted, internal-error, completed-pass, completed-fail, and short-of-pass snapshots, asserting the resulting runnability of the readiness payload; internal/ai/issue1640_readiness_gate_test.go drives a real evaluation that is cancelled at the continuation probe to produce the ToolProtocol=pass / status=not_assessed snapshot end to end and pins that PatrolRuntimeReadiness keeps Patrol runnable. The new API test file is registered in the subsystem verification registry. Co-Authored-By: Claude Fable 5 --- .../v6/internal/subsystems/agent-lifecycle.md | 6 +- .../v6/internal/subsystems/ai-runtime.md | 16 ++ .../v6/internal/subsystems/api-contracts.md | 14 ++ .../v6/internal/subsystems/registry.json | 2 + .../internal/subsystems/storage-recovery.md | 5 +- internal/ai/issue1640_readiness_gate_test.go | 78 +++++++ internal/api/ai_handlers.go | 107 +++++++-- internal/api/issue1640_readiness_gate_test.go | 204 ++++++++++++++++++ .../canonical_completion_guard_test.py | 1 + 9 files changed, 419 insertions(+), 14 deletions(-) create mode 100644 internal/ai/issue1640_readiness_gate_test.go create mode 100644 internal/api/issue1640_readiness_gate_test.go diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index 4fb18c6b22..aa252a89a2 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -2159,7 +2159,11 @@ Agent` secondary handoff against the live setup wizard instead of relying partial probe evidence grant no agent capability and must not be read as 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. + treated as an agent-readiness verdict in either direction. The readiness + checks derived from that snapshot follow the same rule: a passing + tool-protocol dimension on an evaluation that never completed is not a + readiness verdict, and no readiness check may promote a synthetic probe + dimension into agent command authority. 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 aac0ef6d99..99cac90d98 100644 --- a/docs/release-control/v6/internal/subsystems/ai-runtime.md +++ b/docs/release-control/v6/internal/subsystems/ai-runtime.md @@ -3968,6 +3968,22 @@ 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. +Readiness is claimed from the completed overall verdict, never from a single +dimension. An interrupted run keeps its tool-protocol dimension at `pass` when +every tool scenario finished before the cancellation, so any gate that reads +`Dimensions.ToolProtocol` alone reports "Patrol ready" from an evaluation that +never completed; the `tools` readiness check must require the snapshot's own +overall verdict (`Success`) before reporting ready. A snapshot carrying no +verdict — overall status `not_assessed`, or the `interrupted` or +`internal_error` cause — is not a failure either. It falls back to the same +base-config classifier an absent snapshot uses, capped at a warning so an +unfinished run can neither claim readiness nor block Patrol: a not-ready +readiness status disables the Patrol run control, and a severed or cancelled +check must leave Watch-mode Patrol runnable (#1640). A completed run whose tool +protocol passed while the overall verdict fell short warns on the tools check +rather than blocking, because the dimension that actually failed carries the +verdict on its own check. + ## 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 ddf254a693..07ad3e5cbf 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -3688,6 +3688,20 @@ 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. +The `patrol_readiness` payload on `/api/settings/ai` and `/api/ai/patrol/status` +derives its `tools` check from that snapshot's completed overall verdict, not +from the tool-protocol dimension alone: an interrupted evaluation keeps the +dimension at `pass` when the scenarios finished before the cancellation, so a +dimension-only gate reports `status: ready` for a check that never completed. +A snapshot with no verdict — overall status `not_assessed`, or the +`interrupted` or `internal_error` cause — carries no failure either: the check +falls back to the same base-config classifier an absent snapshot uses, capped +at `warning`. `not_ready` is a blocking status in this payload (`ready: false` +suppresses the Patrol run control), so an unfinished or Pulse-side-failed check +must never produce it, and a completed run whose tool protocol passed while the +overall verdict fell short warns here while the dimension that failed carries +the verdict on its own check (#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/registry.json b/docs/release-control/v6/internal/subsystems/registry.json index e4891ec240..6503c5d24f 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_gate_test.go", "internal/api/issue1640_readiness_transport_test.go", "internal/api/patrol_autopilot_test.go" ] @@ -2872,6 +2873,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_gate_test.go", "internal/api/issue1640_readiness_transport_test.go", "internal/api/metadata_handlers_test.go", "internal/api/patrol_autopilot_test.go", diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index f487613cbd..4a66ac072d 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -1975,7 +1975,10 @@ readiness run keeps the same isolation: preserved partial scenario evidence is 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. +assessed and carries no storage or backup state at all. Readiness checks read +that snapshot's completed overall verdict rather than a single dimension, so a +tool-protocol pass on an unfinished run cannot be presented as verified Patrol +capability over storage or recovery data. 26. Storage row presentation resolves its topology label from `storage.vdevLayout` first and falls back to `storage.topology`, so a diff --git a/internal/ai/issue1640_readiness_gate_test.go b/internal/ai/issue1640_readiness_gate_test.go new file mode 100644 index 0000000000..79f432f726 --- /dev/null +++ b/internal/ai/issue1640_readiness_gate_test.go @@ -0,0 +1,78 @@ +package ai + +// Regression coverage for issue #1640: an evaluation interrupted after every +// tool scenario already passed keeps the tool-protocol dimension at pass while +// the overall run reports not assessed. That snapshot must never be read as a +// Patrol verdict — neither as proof the model is ready nor as a failure that +// blocks Patrol from running in Watch mode. + +import ( + "context" + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/ai/providers" + "github.com/rcourtman/pulse-go-rewrite/internal/config" +) + +// issue1640ContinuationCancellingProvider answers every readiness scenario +// correctly and cancels the run only once the multi-turn continuation probe +// starts — the one ordering that leaves ToolProtocol at pass on an interrupted +// run. +type issue1640ContinuationCancellingProvider struct { + *scriptedReadinessProvider + cancel context.CancelFunc +} + +func (p *issue1640ContinuationCancellingProvider) ChatStream(ctx context.Context, req providers.ChatRequest, callback providers.StreamCallback) error { + if req.Messages[len(req.Messages)-1].ToolResult != nil { + p.cancel() + <-ctx.Done() + return ctx.Err() + } + return p.scriptedReadinessProvider.ChatStream(ctx, req, callback) +} + +func TestIssue1640InterruptedRunKeepsToolPassWithoutAVerdict(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + provider := &issue1640ContinuationCancellingProvider{ + scriptedReadinessProvider: &scriptedReadinessProvider{contextWindow: 32768}, + cancel: cancel, + } + cfg := readinessTestConfig() + + result := runPatrolModelReadinessWithProvider( + ctx, cfg, config.AIProviderOllama, "test-model", "ollama:test-model", provider, + ) + + // The shape the readiness gate has to reason about: a passing tool + // dimension attached to a run that never reached a verdict. + if result.Dimensions.ToolProtocol.Status != PatrolModelReadinessPass { + t.Fatalf("tool dimension = %+v, want pass (all scenarios completed before the cancel)", result.Dimensions.ToolProtocol) + } + if result.Status != PatrolModelReadinessNotAssessed { + t.Fatalf("interrupted run status = %q, want %q", result.Status, PatrolModelReadinessNotAssessed) + } + if result.Success || result.PatrolCapable { + t.Fatalf("an interrupted run must not verify the model: %+v", result) + } + if result.Cause != PatrolFailureCauseInterrupted { + t.Fatalf("interrupted run cause = %q, want %q", result.Cause, PatrolFailureCauseInterrupted) + } + + // The runtime gate on POST /api/ai/patrol/run reads the same snapshot: an + // interrupted check may not claim readiness, and may not block the run. + service := NewService(config.NewConfigPersistence(t.TempDir()), nil) + service.cfg = cfg + result.CacheKey = patrolModelReadinessCacheKey(cfg, result.Provider, result.Model) + service.recordPatrolModelReadiness(result, time.Now()) + + readiness := service.PatrolRuntimeReadiness() + if readiness.Status == PatrolReadinessReady { + t.Fatalf("an interrupted evaluation must not report Patrol ready: %+v", readiness) + } + if !readiness.Ready { + t.Fatalf("an interrupted evaluation must not block Patrol from running: %+v", readiness) + } +} diff --git a/internal/api/ai_handlers.go b/internal/api/ai_handlers.go index 7e7b8acc4e..25c8c0890f 100644 --- a/internal/api/ai_handlers.go +++ b/internal/api/ai_handlers.go @@ -5316,24 +5316,16 @@ func resolvePatrolToolsCheck(aiService *ai.Service, provider, model string) (sta if staticStatus != patrolReadinessReady { staticAction = "open_provider_settings" } + static := patrolToolsCheck{Status: staticStatus, Cause: staticCause, Message: staticMessage, Action: staticAction} if aiService == nil { - return staticStatus, staticCause, staticMessage, staticAction + return static.Status, static.Cause, static.Message, static.Action } if advisor, recordedAt := aiService.CachedPatrolModelReadiness(); advisor != nil && !recordedAt.IsZero() { _, bareModel := config.ParseModelString(model) if strings.EqualFold(advisor.Provider, provider) && advisor.Model == bareModel { - age := formatPatrolPreflightAge(time.Since(recordedAt)) - if advisor.Dimensions.ToolProtocol.Status == ai.PatrolModelReadinessPass { - return patrolReadinessReady, ai.PatrolFailureCauseNone, - fmt.Sprintf("Exact streaming tool protocol passed %d/%d scenarios %s.", advisor.Dimensions.ToolProtocol.Passed, advisor.Dimensions.ToolProtocol.Attempts, age), "" - } - cause := advisor.Cause - if cause == ai.PatrolFailureCauseNone || cause == "" { - cause = ai.PatrolFailureCauseModelToolSupportUnverified - } - return patrolReadinessNotReady, cause, - fmt.Sprintf("%s (last readiness evaluation %s).", advisor.Dimensions.ToolProtocol.Summary, age), "open_provider_settings" + resolved := patrolToolsCheckFromModelReadiness(advisor, formatPatrolPreflightAge(time.Since(recordedAt)), static) + return resolved.Status, resolved.Cause, resolved.Message, resolved.Action } } cached, recordedAt := aiService.CachedPatrolPreflight() @@ -5370,6 +5362,97 @@ func resolvePatrolToolsCheck(aiService *ai.Service, provider, model string) (sta return resolvedStatus, cached.Cause, fmt.Sprintf("%s (last preflight %s).", msg, age), "open_provider_settings" } +// patrolToolsCheck is the resolved "Patrol tools" readiness check: the shape +// `resolvePatrolToolsCheck` returns, named so the snapshot gate can be tested +// without standing up an AI service and its readiness cache. +type patrolToolsCheck struct { + Status string + Cause ai.PatrolFailureCause + Message string + Action string +} + +// patrolToolsCheckFromModelReadiness maps a cached model-readiness snapshot +// onto the tools check. Readiness is claimed only from a completed evaluation +// whose overall verdict passed: an interrupted run keeps the tool-protocol +// dimension at pass when every scenario completed before the interruption +// (`internal/ai/patrol_model_readiness.go`) while the overall status is +// not assessed, so reading the dimension alone reported "Patrol ready" from a +// check that never finished. +// +// An inconclusive snapshot is not a verdict in either direction. It must not +// record a failure either, because a not-ready tools check blocks the Patrol +// run control in the UI — #1640 promises an interrupted or cancelled check +// never blames the model and never blocks Patrol from running in Watch mode. +// So an inconclusive snapshot falls back to the base-config classifier exactly +// as an absent snapshot does (mirroring `PatrolRuntimeReadiness` with a nil +// snapshot), capped at a warning so an unfinished run cannot claim readiness. +func patrolToolsCheckFromModelReadiness(advisor *ai.PatrolModelReadinessResult, age string, static patrolToolsCheck) patrolToolsCheck { + if advisor == nil { + return static + } + tool := advisor.Dimensions.ToolProtocol + if patrolModelReadinessInconclusive(advisor) { + // A config-level block stands on its own evidence: the incomplete run + // says nothing that could clear it. + if static.Status == patrolReadinessNotReady { + return static + } + return patrolToolsCheck{ + Status: patrolReadinessWarning, + Cause: patrolToolsCheckCause(advisor), + Message: fmt.Sprintf("The last Patrol readiness evaluation did not complete (%s), so Patrol's tool protocol is unverified. %s", age, strings.TrimSpace(tool.Summary)), + Action: "open_provider_settings", + } + } + if advisor.Success && tool.Status == ai.PatrolModelReadinessPass { + return patrolToolsCheck{ + Status: patrolReadinessReady, + Cause: ai.PatrolFailureCauseNone, + Message: fmt.Sprintf("Exact streaming tool protocol passed %d/%d scenarios %s.", tool.Passed, tool.Attempts, age), + } + } + if tool.Status == ai.PatrolModelReadinessPass { + // The evaluation finished and the tool protocol itself passed, but the + // run did not verify the model. Whichever dimension failed carries the + // verdict on its own check (context quality blocks, latency warns), so + // the tools check reports what it measured without double-blocking. + return patrolToolsCheck{ + Status: patrolReadinessWarning, + Cause: patrolToolsCheckCause(advisor), + Message: fmt.Sprintf("%s The evaluation did not verify the model for Patrol (last readiness evaluation %s).", strings.TrimSpace(tool.Summary), age), + Action: "open_provider_settings", + } + } + return patrolToolsCheck{ + Status: patrolReadinessNotReady, + Cause: patrolToolsCheckCause(advisor), + Message: fmt.Sprintf("%s (last readiness evaluation %s).", strings.TrimSpace(tool.Summary), age), + Action: "open_provider_settings", + } +} + +// patrolModelReadinessInconclusive reports whether a snapshot carries no +// verdict about the model at all: a run that was cut short (interrupted) or a +// Pulse-side defect on the evaluation path (internal_error). Both leave the +// overall status at not assessed or blame Pulse rather than the model (#1640). +func patrolModelReadinessInconclusive(advisor *ai.PatrolModelReadinessResult) bool { + if advisor == nil { + return false + } + return advisor.Status == ai.PatrolModelReadinessNotAssessed || + advisor.Cause == ai.PatrolFailureCauseInterrupted || + advisor.Cause == ai.PatrolFailureCauseInternalError +} + +func patrolToolsCheckCause(advisor *ai.PatrolModelReadinessResult) ai.PatrolFailureCause { + cause := advisor.Cause + if cause == ai.PatrolFailureCauseNone || cause == "" { + return ai.PatrolFailureCauseModelToolSupportUnverified + } + return cause +} + func formatPatrolPreflightAge(age time.Duration) string { switch { case age < time.Minute: diff --git a/internal/api/issue1640_readiness_gate_test.go b/internal/api/issue1640_readiness_gate_test.go new file mode 100644 index 0000000000..34ca4ac0b1 --- /dev/null +++ b/internal/api/issue1640_readiness_gate_test.go @@ -0,0 +1,204 @@ +package api + +// Regression coverage for issue #1640: the "Patrol tools" readiness check read +// the cached snapshot's tool-protocol dimension on its own, so an evaluation +// that was interrupted after every tool scenario passed — tool protocol pass, +// overall status not assessed — reported "Patrol ready" from a check that +// never completed. Readiness now requires the completed overall verdict, and +// an inconclusive snapshot may not record a failure either: a not-ready tools +// check disables the Patrol run control, and #1640 promises an interrupted or +// cancelled check never blocks Patrol from running in Watch mode. + +import ( + "testing" + + "github.com/rcourtman/pulse-go-rewrite/internal/ai" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// issue1640Snapshot builds a cached readiness snapshot with the given overall +// verdict and tool-protocol dimension, leaving the rest of the shape as the +// evaluator produces it. +func issue1640Snapshot(status string, success bool, cause ai.PatrolFailureCause, toolStatus, toolSummary string) *ai.PatrolModelReadinessResult { + snapshot := &ai.PatrolModelReadinessResult{ + ProbeVersion: ai.PatrolModelReadinessProbeVersion, + Status: status, + Success: success, + Cause: cause, + Provider: "ollama", + Model: "test-model", + } + snapshot.Dimensions.ToolProtocol = ai.PatrolModelReadinessDimension{ + Status: toolStatus, + Summary: toolSummary, + Attempts: 3, + Passed: 3, + } + return snapshot +} + +func issue1640OllamaStaticCheck() patrolToolsCheck { + return patrolToolsCheck{ + Status: patrolReadinessWarning, + Cause: ai.PatrolFailureCauseModelToolSupportUnverified, + Message: "Ollama connectivity alone does not prove tool support.", + Action: "open_provider_settings", + } +} + +func TestIssue1640ToolsCheckRequiresCompletedVerdict(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + snapshot *ai.PatrolModelReadinessResult + static patrolToolsCheck + wantStatus string + wantRunnable bool + wantCause ai.PatrolFailureCause + wantMessagePart string + }{ + { + // The regression: every tool scenario passed, then the run was cut + // short. The dimension says pass; the run has no verdict. + name: "interrupted run with a passing tool dimension is not readiness", + snapshot: issue1640Snapshot( + ai.PatrolModelReadinessNotAssessed, false, ai.PatrolFailureCauseInterrupted, + ai.PatrolModelReadinessPass, + "All scenarios passed before the run was interrupted; multi-turn continuation was not assessed.", + ), + static: issue1640OllamaStaticCheck(), + wantStatus: patrolReadinessWarning, + wantRunnable: true, + wantCause: ai.PatrolFailureCauseInterrupted, + wantMessagePart: "did not complete", + }, + { + name: "interrupted run with a partial tool dimension does not blame the model", + snapshot: issue1640Snapshot( + ai.PatrolModelReadinessNotAssessed, false, ai.PatrolFailureCauseInterrupted, + ai.PatrolModelReadinessNotAssessed, + "Interrupted after 1/3 scenarios passed; the remaining scenarios were not assessed.", + ), + static: issue1640OllamaStaticCheck(), + wantStatus: patrolReadinessWarning, + wantRunnable: true, + wantCause: ai.PatrolFailureCauseInterrupted, + wantMessagePart: "did not complete", + }, + { + // A recovered panic on the evaluation path is a Pulse defect, not a + // model verdict, and carries the same non-evidence status. + name: "internal error carries no verdict either", + snapshot: issue1640Snapshot( + ai.PatrolModelReadinessFail, false, ai.PatrolFailureCauseInternalError, + ai.PatrolModelReadinessNotAssessed, "Not assessed.", + ), + static: issue1640OllamaStaticCheck(), + wantStatus: patrolReadinessWarning, + wantRunnable: true, + wantCause: ai.PatrolFailureCauseInternalError, + }, + { + name: "completed passing evaluation reports ready", + snapshot: issue1640Snapshot( + ai.PatrolModelReadinessPass, true, ai.PatrolFailureCauseNone, + ai.PatrolModelReadinessPass, + "Exact tool protocol passed 3/3 scenarios.", + ), + static: issue1640OllamaStaticCheck(), + wantStatus: patrolReadinessReady, + wantRunnable: true, + wantCause: ai.PatrolFailureCauseNone, + wantMessagePart: "passed 3/3 scenarios", + }, + { + name: "completed failing evaluation blocks", + snapshot: issue1640Snapshot( + ai.PatrolModelReadinessWarning, false, ai.PatrolFailureCauseModelUnsupportedTools, + ai.PatrolModelReadinessFail, + "Exact tool protocol passed 0/3 scenarios.", + ), + static: issue1640OllamaStaticCheck(), + wantStatus: patrolReadinessNotReady, + wantRunnable: false, + wantCause: ai.PatrolFailureCauseModelUnsupportedTools, + wantMessagePart: "last readiness evaluation", + }, + { + // A completed run whose tool protocol passed but whose overall + // verdict did not: the dimension that failed carries the verdict on + // its own check, so the tools check reports what it measured + // without turning a latency warning into a hard block. + name: "completed run short of an overall pass warns instead of claiming ready", + snapshot: issue1640Snapshot( + ai.PatrolModelReadinessWarning, false, ai.PatrolFailureCauseLatencyUnsuitable, + ai.PatrolModelReadinessPass, + "Exact tool protocol passed 3/3 scenarios.", + ), + static: issue1640OllamaStaticCheck(), + wantStatus: patrolReadinessWarning, + wantRunnable: true, + wantCause: ai.PatrolFailureCauseLatencyUnsuitable, + wantMessagePart: "did not verify the model", + }, + { + // An incomplete run proves nothing that could clear a config-level + // block, so the base classifier's verdict stands. + name: "config-level block survives an inconclusive snapshot", + snapshot: issue1640Snapshot( + ai.PatrolModelReadinessNotAssessed, false, ai.PatrolFailureCauseInterrupted, + ai.PatrolModelReadinessPass, "All scenarios passed before the run was interrupted.", + ), + static: patrolToolsCheck{ + Status: patrolReadinessNotReady, + Cause: ai.PatrolFailureCauseModelUnsupportedTools, + Message: "The selected Patrol model is a reasoning-only model family.", + Action: "open_provider_settings", + }, + wantStatus: patrolReadinessNotReady, + wantRunnable: false, + wantCause: ai.PatrolFailureCauseModelUnsupportedTools, + wantMessagePart: "reasoning-only", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got := patrolToolsCheckFromModelReadiness(tt.snapshot, "2m ago", tt.static) + + assert.Equal(t, tt.wantStatus, got.Status) + assert.Equal(t, tt.wantCause, got.Cause) + if tt.wantMessagePart != "" { + assert.Contains(t, got.Message, tt.wantMessagePart) + } + require.NotEmpty(t, got.Message) + + // The readiness payload the Patrol page consumes: a not-ready check + // is what disables the run control, so assert the runnability the + // operator actually gets. + readiness := summarizePatrolReadiness("ollama", "ollama:test-model", []PatrolReadinessCheck{{ + ID: "tools", + Status: got.Status, + Cause: patrolFailureCauseResponse(got.Cause), + Label: "Patrol tools", + Message: got.Message, + Action: got.Action, + }}) + assert.Equal(t, tt.wantStatus, readiness.Status) + assert.Equal(t, tt.wantRunnable, readiness.Ready) + }) + } +} + +// An absent snapshot keeps the base-config classifier untouched — the fallback +// an inconclusive snapshot mirrors. +func TestIssue1640AbsentSnapshotKeepsBaseConfigReadiness(t *testing.T) { + t.Parallel() + + static := issue1640OllamaStaticCheck() + assert.Equal(t, static, patrolToolsCheckFromModelReadiness(nil, "just now", static)) +} diff --git a/scripts/release_control/canonical_completion_guard_test.py b/scripts/release_control/canonical_completion_guard_test.py index b356a87dd7..a161fac686 100644 --- a/scripts/release_control/canonical_completion_guard_test.py +++ b/scripts/release_control/canonical_completion_guard_test.py @@ -1171,6 +1171,7 @@ class CanonicalCompletionGuardTest(unittest.TestCase): "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_gate_test.go", "internal/api/issue1640_readiness_transport_test.go", "internal/api/metadata_handlers_test.go", "internal/api/patrol_autopilot_test.go",