diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index d06a1e8..aaf687c 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -1,6 +1,6 @@ # Configuration Collections Implementation Review Findings -Status: Finding 1 completed; Findings 2–3 open. Review completed 2026-10-17. +Status: Findings 1–2 completed; Finding 3 open. Review completed 2026-10-17. Reviewed implementation after `89af2a1` through `0221cd4` against the accepted [feature roadmap](configuration-collections.md), [implementation plan](implementation.md), @@ -89,7 +89,7 @@ and public API tests, configuration vet, and `git diff --check`. ## Finding 2: Detach The Optional Label In Internal Prepared Snapshots -Priority: P3. Status: open. +Priority: P3. Status: completed 2026-10-17. Location: [clonePreparedRun](../../internal/usecase/prepared_execution.go), lines 226–244; also its use by `PrepareExecution` and `PreparedExecution.Details`. @@ -121,6 +121,14 @@ non-nil version label. Mutation of one returned internal snapshot must not chang retained details, the execution snapshot, or a later result. Reuse existing nil/empty public metadata coverage rather than reproducing it at every layer. +Completion: the prepared-run copier now detaches the optional version label with +the existing nil-aware helper. Extended the owning prepared-execution snapshot +test to mutate source and returned labels and verify later details, execution +metadata, and post-execution details remain unchanged. The regression failed +before the fix and passes afterward. Validation passed with `GOWORK=off`: +the focused regression, `go test -race ./...`, `go vet ./...`, all three offline +examples, and `git diff --check`. + ## Finding 3: Document Engine Ownership In Terms Of The Shared Assembly Priority: P3. Status: open. diff --git a/internal/usecase/prepared_execution.go b/internal/usecase/prepared_execution.go index 4a2b499..5f2464c 100644 --- a/internal/usecase/prepared_execution.go +++ b/internal/usecase/prepared_execution.go @@ -229,6 +229,7 @@ func clonePreparedRun(source *domain.PreparedRun) (*domain.PreparedRun, error) { } copied := *source + copied.PromptVersion = copyStringPointer(source.PromptVersion) extraParams, err := jsonvalue.CopyMap(source.EffectiveModelParams.ExtraParams) if err != nil { return nil, err diff --git a/internal/usecase/prepared_execution_test.go b/internal/usecase/prepared_execution_test.go index c6f345b..f511e44 100644 --- a/internal/usecase/prepared_execution_test.go +++ b/internal/usecase/prepared_execution_test.go @@ -662,6 +662,7 @@ func TestRunnerPreparedExecutionDetachesTransferredSnapshotSources(t *testing.T) "properties": map[string]any{"value": map[string]any{"type": "string"}}, } definition := promptDef(domain.FormatJSON, domain.ValidationJSONSchema, 0) + wantPromptVersion := *definition.Version definition.Validation.SchemaPath = "schema.json" control := &domain.CacheControl{Type: domain.CacheControlEphemeral, TTL: "1h"} rendered := &domain.RenderedPrompt{Messages: []domain.RenderedMessage{{ @@ -697,6 +698,7 @@ func TestRunnerPreparedExecutionDetachesTransferredSnapshotSources(t *testing.T) } mutateExtra(profileExtras, "mutated profile source") mutateExtra(requestExtras, "mutated request source") + *definition.Version = "mutated definition label" rendered.Messages[0].Content = "mutated renderer source" control.TTL = "mutated renderer cache metadata" schemaDocument["properties"].(map[string]any)["value"].(map[string]any)["type"] = "integer" @@ -711,11 +713,21 @@ func TestRunnerPreparedExecutionDetachesTransferredSnapshotSources(t *testing.T) details.StructuredOutput.JSONSchema.Schema.(map[string]any)["properties"].(map[string]any)["value"].(map[string]any)["type"] != "string" { t.Fatalf("prepared details retained a source alias: %#v", details) } + if details.PromptVersion == nil || *details.PromptVersion != wantPromptVersion { + t.Fatalf("prepared details did not preserve the captured version: %+v", details) + } + *details.PromptVersion = "mutated details label" + if again := prepared.Details(); again == nil || again.PromptVersion == nil || *again.PromptVersion != wantPromptVersion { + t.Fatalf("details version mutation escaped into the retained snapshot: %+v", again) + } result, err := runner.RunPrepared(context.Background(), prepared) if err != nil || result == nil || !result.Validation.IsValid || result.InputHashes["transcript"] != "captured-input-hash" { t.Fatalf("run prepared snapshot=(%+v, %v)", result, err) } + if result.PromptVersion == nil || *result.PromptVersion != wantPromptVersion { + t.Fatalf("details version mutation escaped into execution metadata: %+v", result) + } if len(llmClient.lastReq.Prompt.Messages) != 1 || llmClient.lastReq.StructuredOutput == nil || llmClient.lastReq.StructuredOutput.JSONSchema == nil || extraLabel(llmClient.lastReq.Target.ExtraParams) != wantLabel || @@ -725,6 +737,7 @@ func TestRunnerPreparedExecutionDetachesTransferredSnapshotSources(t *testing.T) t.Fatalf("execution snapshot retained a source alias: %#v", llmClient.lastReq) } if completedDetails := prepared.Details(); completedDetails == nil || + completedDetails.PromptVersion == nil || *completedDetails.PromptVersion != wantPromptVersion || len(completedDetails.Messages) != 1 || completedDetails.Messages[0].Content != "captured rendering" || extraLabel(completedDetails.EffectiveModelParams.ExtraParams) != wantLabel { t.Fatalf("completed details changed after source mutation: %#v", completedDetails)