Harden PromptKit upgrade integration

2026-08-25 23:46:21 +00:00
parent 7005688b80
commit 0bafbcb21f
12 changed files with 314 additions and 14 deletions
--- a/internal/adapters/promptkit/adapter_test.go
+++ b/internal/adapters/promptkit/adapter_test.go
@@ -239,6 +239,56 @@ func TestMaintainedWeatherLightLocalProfileExampleInspectsOffline(t *testing.T)
 	assertProfile(t, adapter, "weather-light", "", "weather-local")
 }

+func TestMaintainedWeatherLightLocalProfileExampleExecutesThroughProductionClient(t *testing.T) {
+	t.Setenv("WEATHERREPORTER_TEST_MISSING_KEY", "")
+	for _, test := range []struct {
+		name             string
+		credentialSource string
+	}{
+		{name: "without credential source"},
+		{name: "with blank optional credential source", credentialSource: "\napi_key_env: WEATHERREPORTER_TEST_MISSING_KEY\n"},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			var authorization string
+			server := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) {
+				authorization = request.Header.Get("Authorization")
+				writer.Header().Set("Content-Type", "application/json")
+				_, _ = fmt.Fprintf(writer, `{"choices":[{"message":{"content":%q}}],"usage":{"prompt_tokens":12,"completion_tokens":8,"total_tokens":20}}`, validResponse().Content)
+			}))
+			defer server.Close()
+
+			example, err := os.ReadFile(filepath.Join("..", "..", "..", "examples", "weather-light-local-profile.yml"))
+			if err != nil {
+				t.Fatal(err)
+			}
+			profile := strings.Replace(string(example), "http://127.0.0.1:11434/v1", server.URL+"/v1", 1) + test.credentialSource
+			adapter, err := New(Config{ProfileFile: writeProfileFile(t, profile)})
+			if err != nil {
+				t.Fatalf("New() error = %v", err)
+			}
+			request := testExecuteRequest()
+			request.ProfileID = "weather-light"
+			var preparation promptexec.Preparation
+			result, err := adapter.Execute(context.Background(), request, func(value promptexec.Preparation, _ *promptexec.PreparationDebug) error {
+				preparation = value
+				return nil
+			})
+			if err != nil {
+				t.Fatalf("Execute() error = %v", err)
+			}
+			if authorization != "" {
+				t.Fatalf("Authorization header = %q, want absent", authorization)
+			}
+			if preparation.ProfileID != "weather-light" || preparation.BackendID != "" || preparation.ModelName != "weather-local" || preparation.Output.RepairAttempts != 1 {
+				t.Fatalf("preparation = %#v", preparation)
+			}
+			if result == nil || result.ProfileID != "weather-light" || result.BackendID != "" || result.ModelName != "weather-local" || result.Validation.Status != promptexec.ValidationPassed || result.Validation.RepairAttempts != 0 {
+				t.Fatalf("execution = %#v", result)
+			}
+		})
+	}
+}
+
 func TestProfileResolutionFallsThroughOnlyWhenTheConfiguredIDIsAbsent(t *testing.T) {
 	absentAdapter, err := New(Config{ProfileDirectory: testProfileDirectory(t, map[string]string{"profile.yml": `id: other-profile
 backend: openrouter
--- a/internal/app/comparison_execution.go
+++ b/internal/app/comparison_execution.go
@@ -173,9 +173,14 @@ func comparisonExecutionMessage(err error) string {
 	if errors.Is(err, context.DeadlineExceeded) {
 		return "profile execution deadline exceeded"
 	}
+	operation := "profile execution"
 	var execution *profileExecutionError
 	if errors.As(err, &execution) {
-		return comparison.TruncateErrorMessage(execution.operation + " failed")
+		operation = execution.operation
 	}
-	return "profile execution failed"
+	var generation *promptexec.GenerationError
+	if errors.As(err, &generation) && generation.StatusCode() > 0 {
+		return comparison.TruncateErrorMessage(fmt.Sprintf("%s failed (HTTP %d)", operation, generation.StatusCode()))
+	}
+	return comparison.TruncateErrorMessage(operation + " failed")
 }
--- a/internal/app/comparison_execution_test.go
+++ b/internal/app/comparison_execution_test.go
@@ -4,9 +4,11 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"net/http"
 	"os"
 	"path/filepath"
 	"reflect"
+	"strings"
 	"sync"
 	"testing"
 	"time"
@@ -72,6 +74,66 @@ func TestExecuteComparisonProfilesContinuesAfterProfileFailure(t *testing.T) {
 	}
 }

+func TestExecuteComparisonProfilesPreservesIndependentRepairOutcomes(t *testing.T) {
+	prepared, prompt := preparedDailyProfile(t)
+	profiles := comparisonProfiles(4)
+	executor := newBarrierExecutor(profiles)
+	executor.setValidation(profiles[0].ProfileID, promptexec.ValidationPassed, 0)
+	executor.setValidation(profiles[1].ProfileID, promptexec.ValidationPassed, 1)
+	executor.setValidation(profiles[2].ProfileID, promptexec.ValidationFailed, 1)
+	executor.setError(profiles[3].ProfileID, errors.New("provider failure"))
+	results := startComparisonExecution(t, context.Background(), comparisonExecutionRequest{
+		Prepared: prepared, Inspection: comparisonInspection(prompt, profiles), ComparisonID: "comparison_daily", Executor: executor,
+	}, executor)
+	waitForProfileStarts(t, executor, profiles, results)
+	executor.releaseAll()
+	result := <-results
+	wantStatuses := []string{comparison.StatusSucceeded, comparison.StatusSucceeded, comparison.StatusFailed, comparison.StatusFailed}
+	wantValidations := []promptexec.ValidationStatus{promptexec.ValidationPassed, promptexec.ValidationPassed, promptexec.ValidationFailed, ""}
+	wantRepairs := []*int{intPointer(0), intPointer(1), intPointer(1), nil}
+	for index, outcome := range result.Outcomes {
+		if outcome.Status != wantStatuses[index] || outcome.ValidationStatus != wantValidations[index] || !reflect.DeepEqual(outcome.RepairAttempts, wantRepairs[index]) {
+			t.Fatalf("outcome[%d] = %#v, want status/validation/repairs %q/%q/%#v", index, outcome, wantStatuses[index], wantValidations[index], wantRepairs[index])
+		}
+	}
+}
+
+func TestExecuteComparisonProfilesCapturesConcurrentProviderFailures(t *testing.T) {
+	prepared, prompt := preparedDailyProfile(t)
+	profiles := comparisonProfiles(2)
+	debugWriter, err := promptdebug.NewPromptDebugWriter(t.TempDir())
+	if errors.Is(err, promptdebug.ErrSecureCaptureUnsupported) {
+		t.Skipf("secure prompt debug capture is unavailable: %v", err)
+	}
+	if err != nil {
+		t.Fatalf("NewPromptDebugWriter() error = %v", err)
+	}
+	markers := []string{"first-provider-private-marker", "second-provider-private-marker"}
+	statuses := []int{http.StatusTooManyRequests, http.StatusServiceUnavailable}
+	executor := newBarrierExecutor(profiles)
+	for index, profile := range profiles {
+		executor.setError(profile.ProfileID, promptexec.NewGenerationError(statuses[index], "provider_code", "provider_type", markers[index], nil))
+	}
+	results := startComparisonExecution(t, context.Background(), comparisonExecutionRequest{
+		Prepared: prepared, Inspection: comparisonInspection(prompt, profiles), ComparisonID: "comparison_daily", DebugWriter: debugWriter, Executor: executor,
+	}, executor)
+	waitForProfileStarts(t, executor, profiles, results)
+	executor.releaseAll()
+	result := <-results
+	for index, outcome := range result.Outcomes {
+		if outcome.Status != comparison.StatusFailed || outcome.Error == nil || outcome.Error.Category != string(promptexec.Generation) || outcome.Error.Message != fmt.Sprintf("execute prompt failed (HTTP %d)", statuses[index]) || strings.Contains(outcome.Error.Message, markers[index]) || outcome.LLMDebugPath == "" {
+			t.Fatalf("outcome[%d] = %#v", index, outcome)
+		}
+		failure, readErr := os.ReadFile(filepath.Join(outcome.LLMDebugPath, "failure.json"))
+		if readErr != nil {
+			t.Fatal(readErr)
+		}
+		if !strings.Contains(string(failure), markers[index]) || strings.Contains(string(failure), markers[1-index]) {
+			t.Fatalf("failure[%d] = %s", index, failure)
+		}
+	}
+}
+
 func TestExecuteComparisonProfilesPropagatesCancellationAndJoins(t *testing.T) {
 	prepared, prompt := preparedDailyProfile(t)
 	profiles := comparisonProfiles(4)
@@ -139,6 +201,8 @@ type barrierExecutor struct {
 	releases         map[string]chan struct{}
 	requests         map[string]promptexec.ExecuteRequest
 	errors           map[string]error
+	validations      map[string]promptexec.ValidationStatus
+	repairAttempts   map[string]int
 	profiles         map[string]ComparisonProfileInspection
 	inFlight         int
 	maximum          int
@@ -153,7 +217,7 @@ func newBarrierExecutor(profiles []ComparisonProfileInspection) *barrierExecutor
 	}
 	return &barrierExecutor{
 		started: make(chan string, len(profiles)), callbackFailures: make(chan error, len(profiles)), releases: releases,
-		requests: make(map[string]promptexec.ExecuteRequest, len(profiles)), errors: map[string]error{}, profiles: identities,
+		requests: make(map[string]promptexec.ExecuteRequest, len(profiles)), errors: map[string]error{}, validations: map[string]promptexec.ValidationStatus{}, repairAttempts: map[string]int{}, profiles: identities,
 	}
 }

@@ -195,15 +259,20 @@ func (e *barrierExecutor) Execute(ctx context.Context, req promptexec.ExecuteReq
 	e.mu.Lock()
 	e.inFlight--
 	err := e.errors[req.ProfileID]
+	validationStatus := e.validations[req.ProfileID]
+	repairAttempts := e.repairAttempts[req.ProfileID]
 	e.mu.Unlock()
 	if err != nil {
 		return nil, err
 	}
+	if validationStatus == "" {
+		validationStatus = promptexec.ValidationPassed
+	}
 	return &promptexec.Execution{
 		PromptID: req.PromptID, PromptVersion: req.PromptVersion, PromptHash: generationPromptHash,
 		ProfileID: req.ProfileID, BackendID: profile.BackendID, ModelName: profile.ModelName,
 		StartedAt: stamp, EndedAt: stamp, RawOutput: comparisonRawOutput(),
-		Validation: promptexec.NewValidation(promptexec.ValidationPassed, "json_schema", generationDefinitionForPrompt(req.PromptID).GeneratedTextSchemaID+".generated_text.schema.json", 0, nil),
+		Validation: promptexec.NewValidation(validationStatus, "json_schema", generationDefinitionForPrompt(req.PromptID).GeneratedTextSchemaID+".generated_text.schema.json", repairAttempts, nil),
 	}, nil
 }

@@ -220,6 +289,13 @@ func (e *barrierExecutor) setError(profileID string, err error) {
 	e.errors[profileID] = err
 }

+func (e *barrierExecutor) setValidation(profileID string, status promptexec.ValidationStatus, repairAttempts int) {
+	e.mu.Lock()
+	defer e.mu.Unlock()
+	e.validations[profileID] = status
+	e.repairAttempts[profileID] = repairAttempts
+}
+
 func (e *barrierExecutor) release(profileID string) {
 	close(e.releases[profileID])
 }
@@ -318,4 +394,8 @@ func bytesEqual(left, right []byte) bool {
 	return reflect.DeepEqual(left, right)
 }

+func intPointer(value int) *int {
+	return &value
+}
+
 var _ promptexec.Executor = (*barrierExecutor)(nil)
--- a/internal/app/comparison_test.go
+++ b/internal/app/comparison_test.go
@@ -87,6 +87,40 @@ func TestCompareDetailedPublishesPartialBundleAndReturnsAggregateError(t *testin
 	}
 }

+func TestCompareDetailedPublishesPostValidationProfileFailure(t *testing.T) {
+	bundle := generationBundle(t)
+	executor := &generationExecutor{complete: func(execution *promptexec.Execution) {
+		if execution.ProfileID == "weather-deep" {
+			execution.RawOutput = []byte(`{"summary":42}`)
+		}
+	}}
+	result, err := CompareDetailed(context.Background(), ComparisonRequest{
+		Config: comparisonConfig(), Report: ReportDaily, ProfileIDs: []string{"weather-light", "weather-deep"},
+		WorkingDir: t.TempDir(), Date: generationTime("2026-05-29T12:00:00-05:00"),
+		Clock:     timeutil.FixedClock{Time: generationTime("2026-05-29T08:30:00-05:00")},
+		Collector: &generationCollector{bundle: &bundle}, Executor: executor,
+	})
+	if err == nil || result == nil || result.Succeeded != 1 || result.Failed != 1 || result.ManifestPath == "" {
+		t.Fatalf("CompareDetailed() result/error = %#v/%v", result, err)
+	}
+	failure := result.Results[1]
+	if failure.Status != comparison.StatusFailed || failure.ValidationStatus != promptexec.ValidationPassed || failure.RepairAttempts == nil || *failure.RepairAttempts != 0 {
+		t.Fatalf("post-validation failure = %#v", failure)
+	}
+	data, readErr := os.ReadFile(result.ManifestPath)
+	if readErr != nil {
+		t.Fatal(readErr)
+	}
+	var manifest comparison.Manifest
+	if decodeErr := json.Unmarshal(data, &manifest); decodeErr != nil {
+		t.Fatal(decodeErr)
+	}
+	manifestFailure := manifest.Results[1]
+	if manifestFailure.ValidationStatus != "passed" || manifestFailure.RepairAttempts == nil || *manifestFailure.RepairAttempts != 0 {
+		t.Fatalf("published post-validation failure = %#v", manifestFailure)
+	}
+}
+
 func TestCompareDetailedRetainsCommittedPathsWhenBackupCleanupFails(t *testing.T) {
 	for _, test := range []struct {
 		name  string
--- a/internal/app/generation_test.go
+++ b/internal/app/generation_test.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
+	"net/http"
 	"os"
 	"path/filepath"
 	"strings"
@@ -549,6 +550,66 @@ func TestGenerateDetailedWritesRequestedPromptDebugArtifacts(t *testing.T) {
 	}
 }

+func TestGenerateDetailedCapturesProviderFailureOnlyInDebugArtifacts(t *testing.T) {
+	bundle := generationBundle(t)
+	debugRoot := t.TempDir()
+	const marker = "provider-private-generation-marker"
+	result, err := GenerateDetailed(context.Background(), GenerateRequest{
+		Config: generationConfig(), Report: ReportDaily,
+		Date: generationTime("2026-05-29T12:00:00-05:00"), Now: generationTime("2026-05-29T08:30:00-05:00"),
+		WorkingDir: t.TempDir(), LLMDebugDir: debugRoot, Collector: &generationCollector{bundle: &bundle},
+		Executor: &generationExecutor{executeErr: promptexec.NewGenerationError(http.StatusTooManyRequests, "rate_limit", "provider_error", marker, nil)},
+	})
+	if errors.Is(err, promptdebug.ErrSecureCaptureUnsupported) {
+		t.Skipf("secure prompt debug capture is unavailable: %v", err)
+	}
+	if err == nil || result == nil || result.LLMDebugPath == "" || promptexec.CategoryOf(err) != promptexec.Generation || !strings.Contains(err.Error(), "HTTP 429") || strings.Contains(err.Error(), marker) || result.OutputPath != "" {
+		t.Fatalf("GenerateDetailed() result/error = %#v/%v", result, err)
+	}
+	for _, name := range []string{"preparation.json", "failure.json"} {
+		if _, statErr := os.Stat(filepath.Join(result.LLMDebugPath, name)); statErr != nil {
+			t.Fatalf("debug artifact %q: %v", name, statErr)
+		}
+	}
+	data, readErr := os.ReadFile(filepath.Join(result.LLMDebugPath, "failure.json"))
+	if readErr != nil || !strings.Contains(string(data), marker) {
+		t.Fatalf("failure artifact = %q, error = %v", data, readErr)
+	}
+}
+
+func TestGenerateDetailedPreservesProviderFailureWhenFailureDebugWriteFails(t *testing.T) {
+	bundle := generationBundle(t)
+	debugRoot := t.TempDir()
+	const marker = "provider-private-write-failure-marker"
+	var setupErr error
+	executor := &generationExecutor{
+		executeErr: promptexec.NewGenerationError(http.StatusServiceUnavailable, "unavailable", "provider_error", marker, nil),
+		beforeExecute: func(promptexec.ExecuteRequest) {
+			setupErr = filepath.Walk(debugRoot, func(path string, info os.FileInfo, err error) error {
+				if err != nil {
+					return err
+				}
+				if info.Name() == "preparation.json" {
+					return os.Mkdir(filepath.Join(filepath.Dir(path), "failure.json"), 0o700)
+				}
+				return nil
+			})
+		},
+	}
+	result, err := GenerateDetailed(context.Background(), GenerateRequest{
+		Config: generationConfig(), Report: ReportDaily,
+		Date: generationTime("2026-05-29T12:00:00-05:00"), Now: generationTime("2026-05-29T08:30:00-05:00"),
+		WorkingDir: t.TempDir(), LLMDebugDir: debugRoot, Collector: &generationCollector{bundle: &bundle}, Executor: executor,
+	})
+	if errors.Is(err, promptdebug.ErrSecureCaptureUnsupported) {
+		t.Skipf("secure prompt debug capture is unavailable: %v", err)
+	}
+	var generationError *promptexec.GenerationError
+	if setupErr != nil || err == nil || result == nil || result.OutputPath != "" || promptexec.CategoryOf(err) != promptexec.Generation || !errors.As(err, &generationError) || generationError.StatusCode() != http.StatusServiceUnavailable || !strings.Contains(err.Error(), "HTTP 503") || strings.Contains(err.Error(), marker) {
+		t.Fatalf("GenerateDetailed() setup/result/error = %v/%#v/%v", setupErr, result, err)
+	}
+}
+
 func generationConfig() config.Config {
 	cfg := config.Defaults()
 	cfg.WeatherAPI.Timezone, cfg.Location.ID = "America/Chicago", "home"
--- a/internal/comparison/comparison.go
+++ b/internal/comparison/comparison.go
@@ -331,7 +331,7 @@ func (safeError SafeError) validate() error {
 }

 func isValidationStatus(status string) bool {
-	return status == "" || status == "failed" || status == "skipped"
+	return status == "" || status == "passed" || status == "failed" || status == "skipped"
 }

 func isSHA256(value string) bool {
--- a/internal/comparison/comparison_test.go
+++ b/internal/comparison/comparison_test.go
@@ -202,6 +202,9 @@ func TestManifestValidateRejectsInvariants(t *testing.T) {
 		{name: "successful result without passed validation", mutate: func(manifest *Manifest) { manifest.Results[0].ValidationStatus = "failed" }},
 		{name: "failed result with report", mutate: func(manifest *Manifest) { manifest.Results[1].ReportPath = "02-weather-deep.md" }},
 		{name: "failed result without error", mutate: func(manifest *Manifest) { manifest.Results[1].Error = nil }},
+		{name: "failed passed result without repair provenance", mutate: func(manifest *Manifest) {
+			manifest.Results[1].ValidationStatus = "passed"
+		}},
 		{name: "traversal report path", mutate: func(manifest *Manifest) { manifest.Results[0].ReportPath = "../report.md" }},
 		{name: "duplicate report path", mutate: func(manifest *Manifest) {
 			manifest.Results[1] = Result{Position: 2, ProfileID: "weather-deep", ModelName: "gpt-5", Status: StatusSucceeded, ValidationStatus: "passed", ReportPath: manifest.Results[0].ReportPath}
@@ -223,6 +226,17 @@ func TestManifestValidateRejectsInvariants(t *testing.T) {
 	}
 }

+func TestManifestValidateAcceptsPostValidationFailure(t *testing.T) {
+	t.Parallel()
+
+	manifest := validManifest()
+	manifest.Results[1].ValidationStatus = "passed"
+	manifest.Results[1].RepairAttempts = intPtr(0)
+	if err := manifest.Validate(); err != nil {
+		t.Fatalf("Manifest.Validate() rejected post-validation failure: %v", err)
+	}
+}
+
 func TestLogicalBundleValidate(t *testing.T) {
 	t.Parallel()

--- a/internal/comparison/publish.go
+++ b/internal/comparison/publish.go
@@ -519,8 +519,15 @@ func validateSafeErrorJSON(decoder *json.Decoder) error {
 }

 func validateRepairAttemptsJSON(decoder *json.Decoder) error {
+	var raw json.RawMessage
+	if err := decoder.Decode(&raw); err != nil {
+		return err
+	}
+	if bytes.Equal(bytes.TrimSpace(raw), []byte("null")) {
+		return fmt.Errorf("repairAttempts must be an integer")
+	}
 	var value int
-	if err := decoder.Decode(&value); err != nil {
+	if err := json.Unmarshal(raw, &value); err != nil {
 		return err
 	}
 	if value < 0 {
--- a/internal/comparison/publish_test.go
+++ b/internal/comparison/publish_test.go
@@ -1,6 +1,7 @@
 package comparison

 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"errors"
@@ -146,6 +147,23 @@ func TestRecognizeBundleRejectsUnsafeAndMalformedContents(t *testing.T) {
 			t.Helper()
 			appendManifestField(t, directory, `"SchemaVersion": "weatherreporter.comparison.v1"`)
 		}},
+		{name: "null repair attempts", mutate: func(t *testing.T, directory string) {
+			t.Helper()
+			path := filepath.Join(directory, ManifestFilename)
+			data, err := os.ReadFile(path)
+			if err != nil {
+				t.Fatal(err)
+			}
+			before := []byte("      \"status\": \"failed\",\n      \"error\": {")
+			after := []byte("      \"status\": \"failed\",\n      \"repairAttempts\": null,\n      \"error\": {")
+			data = bytes.Replace(data, before, after, 1)
+			if bytes.Equal(data, readFile(t, path)) {
+				t.Fatal("failed to add null repairAttempts")
+			}
+			if err := os.WriteFile(path, data, 0o600); err != nil {
+				t.Fatal(err)
+			}
+		}},
 		{name: "traversal report path", mutate: func(t *testing.T, directory string) {
 			t.Helper()
 			path := filepath.Join(directory, ManifestFilename)
@@ -202,6 +220,30 @@ func TestRecognizeBundleRejectsUnsafeAndMalformedContents(t *testing.T) {
 	}
 }

+func TestValidateRepairAttemptsJSONRejectsNonIntegerValues(t *testing.T) {
+	t.Parallel()
+
+	for _, value := range []string{"null", "-1", "1.5", `"1"`, "9223372036854775808", "{"} {
+		value := value
+		t.Run(value, func(t *testing.T) {
+			t.Parallel()
+			if err := validateRepairAttemptsJSON(json.NewDecoder(strings.NewReader(value))); err == nil {
+				t.Fatalf("validateRepairAttemptsJSON(%q) error = nil", value)
+			}
+		})
+	}
+}
+
+func TestValidateRepairAttemptsJSONAcceptsNonnegativeIntegers(t *testing.T) {
+	t.Parallel()
+
+	for _, value := range []string{"0", "1"} {
+		if err := validateRepairAttemptsJSON(json.NewDecoder(strings.NewReader(value))); err != nil {
+			t.Fatalf("validateRepairAttemptsJSON(%q) error = %v", value, err)
+		}
+	}
+}
+
 func appendManifestField(t *testing.T, directory, field string) {
 	t.Helper()
 	path := filepath.Join(directory, ManifestFilename)