Protect retained metadata from custom JSON encoders

docs/internal/llm.md

@@ -41,6 +41,10 @@ then:
    the complete-request failure remains a request failure;
 6. resolves authentication and freezes the URL, body, and optional credential.
 
+Provider values are detached with trusted copying before encoding. Custom JSON
+encoding methods and per-value encoding-failure diagnostics operate on those
+private copies rather than retained prepared or diagnostic metadata.
+
 Each execution of that prepared request creates a fresh HTTP request, body
 reader, and attempt timeout context. It closes the response body and returns
 the HTTP status, decoded response or error, reported token values and presence,

docs/roadmap/remediation.md

@@ -7,17 +7,29 @@ against [responsibility-simplification.md](responsibility-simplification.md)
 and its accepted audit decisions. The implementation spans `448e64d` through
 `5ee602cc4b5a3afe4e7d361eb77181ef0940c7cd`, after planning commit `de46935`.
 
-The targeted architecture is substantially implemented. One ownership regression
-remains open below. No additional unintended restriction or public guarantee was
-identified; removing copier-derived schema limits is the selected acceptance
-change, while runtime provider-value support remains in place. The earlier
+The targeted architecture is implemented, and the ownership regression below
+is now addressed. Its evidence and review table describe the pre-fix baseline;
+the completion marker records the correction. No additional unintended
+restriction or public guarantee was identified. Removing copier-derived schema
+limits remains the selected acceptance change, while runtime provider-value
+support remains in place. The earlier
 33-stage review and its five completed findings are retained separately below.
 
 ### R1. Isolate Custom JSON Encoders From Retained Metadata [P2]
 
-**Status:** Open. **Stages:** 2, 13–15; particularly the shared prepared snapshot
+**Status:** Complete. **Stages:** 2, 13–15; particularly the shared prepared snapshot
 and removal of private smoke-target detachment.
 
+Completion: built-in generation preparation uses trusted detachment before
+payload construction, so whole-payload encoding and per-value encoding-failure
+diagnostics cannot mutate retained provider values through their receivers.
+The single prepared snapshot and named/typed custom encoding remain intact.
+Public regressions failed before the fix and now protect prepared details,
+result settings, smoke settings, caller input, custom wire output, and failure
+classification without dispatch. Go 1.27.3 `go test -race ./...` and
+`go vet ./...` passed, including the existing retry, repair, and concurrent
+prepared-lifecycle coverage. Internal model-client documentation was updated.
+
 **Locations:** [provider payload construction](../../internal/llm/openai_compatible_client.go),
 `openAIChatRequestPayload`, lines 468–475, and `PrepareGeneration`, lines 133–139;
 [prepared metadata sharing](../../internal/usecase/prepared_execution.go),
@@ -94,6 +106,9 @@ injected initial/repair mutation and concurrent prepared lifecycle protection.
 
 ### Stage-By-Stage Review
 
+This is the pre-remediation review record; R1's completion above addresses its
+identified gap without changing the other selected boundaries.
+
 | Stage | Current evidence and result |
 | --- | --- |
 | 1 | Constructor routing defaults and their only error path are removed. Resolved endpoint/model checks and HTTP-client timeout ownership remain. No additional finding. |
@@ -114,9 +129,10 @@ injected initial/repair mutation and concurrent prepared lifecycle protection.
 | 16 | Canonical format, integration, model-client, runner, source, and component guidance reflects the selected changes without new resource/provider promises. The immutability description is not fully realized until R1 is fixed. |
 | 17 | Fresh full race tests and vet pass, but those checks do not cover R1. Documentation/history links and delivered scope were reviewed; completion still requires the ownership correction. |
 
-### Current Validation And Limits
+### Review Validation And Limits
 
-Using Go 1.27.3 with `GOTOOLCHAIN=local`, `GOWORK=off`, and `GOPROXY=off`:
+At the reviewed pre-fix baseline, using Go 1.27.3 with `GOTOOLCHAIN=local`,
+`GOWORK=off`, and `GOPROXY=off`:
 
 - `go test -race ./...` passed, including the maintained offline examples.
 - `go vet ./...` passed.
@@ -124,9 +140,10 @@ Using Go 1.27.3 with `GOTOOLCHAIN=local`, `GOWORK=off`, and `GOPROXY=off`:
   errors and unchanged caller values; the two-file historical overlay prevented
   both mutations.
 
-No production code or repository tests were changed. The source review and
-targeted probe establish R1 despite the passing existing suite. The review does
-not claim universal callback safety, provider compatibility, or defect-free code.
+The review changed no production code or repository tests. Its source analysis
+and targeted probe established R1 despite the passing existing suite; the
+completion marker above records the subsequent fix and validation. The review
+does not claim universal callback safety, provider compatibility, or defect-free code.
 
 ## Earlier Audit Remediation Review: Scope And Status
 

internal/llm/openai_compatible_client.go

@@ -17,6 +17,7 @@ import (
 
 	"go.maximumdirect.net/promptkit/internal/defaults"
 	"go.maximumdirect.net/promptkit/internal/domain"
+	"go.maximumdirect.net/promptkit/internal/jsonvalue"
 )
 
 var (
@@ -125,6 +126,9 @@ func (c *OpenAICompatibleClient) PrepareGeneration(req domain.GenerateRequest) (
 		return nil, fmt.Errorf("%w: invalid endpoint path: %v", ErrInvalidRequest, err)
 	}
 
+	// Custom JSON encoders may mutate their receivers. Both payload encoding
+	// and encoding-failure diagnostics must use values detached from metadata.
+	req.Target.ExtraParams = jsonvalue.CloneTrusted(req.Target.ExtraParams).(map[string]any)
 	wirePayload, err := openAIChatRequestPayload(req)
 	if err != nil {
 		return nil, fmt.Errorf("%w: %v", ErrInvalidRequest, err)

provider_encoding_ownership_contract_test.go

@@ -0,0 +1,110 @@
+package promptkit_test
+
+import (
+	"context"
+	"encoding/json"
+	"errors"
+	"net/http"
+	"reflect"
+	"testing"
+
+	"go.maximumdirect.net/promptkit"
+)
+
+type receiverMutatingProviderMap map[string]string
+
+func (value receiverMutatingProviderMap) MarshalJSON() ([]byte, error) {
+	value["value"] = "encoded"
+	if value["fail"] == "yes" {
+		return nil, errors.New("synthetic encoding failure")
+	}
+	return json.Marshal(map[string]string(value))
+}
+
+func TestBuiltInEncodingPreservesPreparedProviderValues(t *testing.T) {
+	for _, fail := range []bool{false, true} {
+		name := "successful encoding"
+		if fail {
+			name = "encoding failure diagnostics"
+		}
+		t.Run(name, func(t *testing.T) {
+			calls := 0
+			config := simpleConfig("message")
+			config.HTTPClient = &http.Client{Transport: encodingOwnershipTransport(t, &calls)}
+			engine, err := promptkit.NewEngine(config)
+			if err != nil {
+				t.Fatal(err)
+			}
+			options := receiverMutatingProviderMap{"value": "original"}
+			if fail {
+				options["fail"] = "yes"
+			}
+			prepared, err := engine.PrepareExecution(context.Background(), promptkit.RunRequest{
+				PromptID: "prompt", Execution: &promptkit.ExecutionTargetOverride{
+					ExtraParams: map[string]any{"options": options},
+				},
+			})
+			if err != nil {
+				t.Fatal(err)
+			}
+			before := prepared.Details()
+			result, err := engine.RunPrepared(context.Background(), prepared)
+			if fail {
+				if !errors.Is(err, promptkit.ErrInvalidRequest) || result != nil || calls != 0 {
+					t.Fatalf("encoding failure: result=%+v error=%v dispatches=%d", result, err, calls)
+				}
+			} else if err != nil || result == nil || calls != 1 {
+				t.Fatalf("successful encoding: result=%+v error=%v dispatches=%d", result, err, calls)
+			} else if result.EffectiveModelParams.ExtraParams["options"].(receiverMutatingProviderMap)["value"] != "original" {
+				t.Fatal("custom encoder changed result settings")
+			}
+			if !reflect.DeepEqual(before, prepared.Details()) {
+				t.Fatal("custom encoder changed retained prepared details")
+			}
+			if options["value"] != "original" {
+				t.Fatal("custom encoder changed caller input")
+			}
+		})
+	}
+}
+
+func TestBuiltInEncodingPreservesSmokeProviderValues(t *testing.T) {
+	calls := 0
+	config := simpleConfig("unused")
+	config.HTTPClient = &http.Client{Transport: encodingOwnershipTransport(t, &calls)}
+	engine, err := promptkit.NewEngine(config)
+	if err != nil {
+		t.Fatal(err)
+	}
+	options := receiverMutatingProviderMap{"value": "original"}
+	report, err := engine.SmokeTest(context.Background(), promptkit.SmokeTestRequest{
+		Kind: promptkit.SmokeTestTextInference, ProfileID: "profile",
+		Execution: &promptkit.ExecutionTargetOverride{ExtraParams: map[string]any{"options": options}},
+	})
+	if err != nil || report == nil || !report.Completed || calls != 1 {
+		t.Fatalf("smoke result=%+v error=%v dispatches=%d", report, err, calls)
+	}
+	if report.Target.EffectiveSettings.ExtraParams["options"].(receiverMutatingProviderMap)["value"] != "original" {
+		t.Fatal("custom encoder changed smoke report settings")
+	}
+	if options["value"] != "original" {
+		t.Fatal("custom encoder changed caller input")
+	}
+}
+
+func encodingOwnershipTransport(t *testing.T, calls *int) roundTripFunc {
+	t.Helper()
+	return func(request *http.Request) (*http.Response, error) {
+		*calls += 1
+		var payload struct {
+			Options map[string]string `json:"options"`
+		}
+		if err := json.NewDecoder(request.Body).Decode(&payload); err != nil {
+			return nil, err
+		}
+		if payload.Options["value"] != "encoded" {
+			t.Errorf("custom wire encoding was lost: options=%v", payload.Options)
+		}
+		return observedHTTPResponse(http.StatusOK, `{"choices":[{"message":{"content":"PROMPTKIT_OK"}}]}`), nil
+	}
+}