<!-- task=m-iop-owned-single-request-agent-execution/02+01_preset_binding plan=3 tag=REVIEW_API milestone-task=preset-binding -->

# Review Follow-up: Fail-closed Immutable Preset Admission

## For the Implementing Agent

Do not start until the packet 01 dependency command passes. Implement this plan exactly within the listed write boundary, run every verification command, fill all implementation-owned sections of `CODE_REVIEW-cloud-G07.md` with actual notes and stdout/stderr, keep the active pair in place, and report ready for review. Finalization belongs only to the code-review skill. If blocked, record exact blocker evidence, attempted commands/output, and the resume condition; do not ask the user, call user-input tools, create control-plane stop files, classify the next state, archive logs, or write `complete.log`.

## Archive Evidence Snapshot

- Superseded pair: `plan_local_G06_2.log`, `code_review_cloud_G07_2.log`.
- Archived verdict: FAIL with Required R1-R3 and no Suggested or Nit findings. R1 covers swallowed marked-preset compilation failures, R2 covers discarded fixed stage options and missing defense-in-depth checks, and R3 covers shallow copying of nested option values.
- Fresh reviewer verification passed focused, package, race, vet, full Edge, formatting, and diff-hygiene commands, but the green tests exercised helpers or vacuous values instead of the failing production boundaries.
- Split predecessor 01 is satisfied by `agent-task/archive/2026/08/m-iop-owned-single-request-agent-execution/01_preset_config/complete.log`.

## Finding Resolution Map

| Finding | Mode | Exact Fix / Dependency Evidence | Changed or Satisfied Precondition |
|---------|------|---------------------------------|-----------------------------------|
| Required R1 | direct-fix | Propagate marked-preset compiler rejection from managed and unmanaged production resolvers; add resolver and model-list regression tests in `route_resolution.go`, `principal_routes.go`, `openai_auth_routes_models_test.go`, and `principal_routes_test.go`. | Production resolution, rather than a helper-only assertion, becomes the fail-closed oracle. |
| Required R2 | direct-fix | Compile plan/work/review from the approved fixed stage configs, validate role/model/options shape against the light route and authorized dispatches, and assert real option values and refresh isolation in `single_request_preset_binding.go` and its tests. | Tests begin with non-empty approved options and reject duplicate, dynamic, or inconsistent shapes. |
| Required R3 | direct-fix | Recursively copy nested option maps, slices, arrays, pointers, and interface values in `single_request_types.go`; mutate nested source and clone values in service tests. | Copy assertions cross a nested reference boundary instead of checking only top-level map keys. |

## Background

The first implementation introduced a service-owned single-request binding, but marked-preset compiler errors are ignored by both production route paths. It also drops the approved plan/review options and retains nested option aliases. The follow-up closes those three defects at the existing admission boundary without changing the published contract or expanding into coordinator execution.

## Analysis

### Files Read

- `agent-ops/rules/project/rules.md`
- `agent-ops/rules/private/rules.md`
- `agent-ops/rules/common/rules-roadmap.md`
- `agent-ops/rules/common/rules-agent-spec.md`
- `agent-ops/rules/project/domain/edge/rules.md`
- `agent-ops/rules/project/domain/platform/rules.md`
- `agent-ops/rules/project/domain/testing/rules.md`
- `agent-test/local/rules.md`
- `agent-test/local/edge-smoke.md`
- `agent-test/local/platform-smoke.md`
- `agent-roadmap/current.md`
- `agent-roadmap/phase/knowledge-tool-optimization-extension/milestones/iop-owned-single-request-agent-execution.md`
- `agent-roadmap/sdd/knowledge-tool-optimization-extension/iop-owned-single-request-agent-execution/SDD.md`
- `agent-contract/index.md`
- `agent-contract/outer/anthropic-compatible-api.md`
- `agent-contract/inner/edge-config-runtime-refresh.md`
- `agent-spec/index.md`
- `agent-spec/input/openai-compatible-surface.md`
- `agent-spec/runtime/provider-pool-config-refresh.md`
- `packages/go/config/execution_preset_types.go`
- `apps/edge/internal/service/single_request_types.go`
- `apps/edge/internal/service/single_request_types_test.go`
- `apps/edge/internal/openai/route_resolution.go`
- `apps/edge/internal/openai/routes.go`
- `apps/edge/internal/openai/principal_routes.go`
- `apps/edge/internal/openai/principal_routes_test.go`
- `apps/edge/internal/openai/openai_auth_routes_models_test.go`
- `apps/edge/internal/openai/single_request_preset_binding.go`
- `apps/edge/internal/openai/single_request_preset_binding_test.go`
- `apps/edge/internal/openai/server.go`
- `agent-task/m-iop-owned-single-request-agent-execution/02+01_preset_binding/plan_local_G06_1.log`
- `agent-task/m-iop-owned-single-request-agent-execution/02+01_preset_binding/code_review_cloud_G07_1.log`
- `agent-task/m-iop-owned-single-request-agent-execution/02+01_preset_binding/plan_local_G06_2.log`
- `agent-task/m-iop-owned-single-request-agent-execution/02+01_preset_binding/code_review_cloud_G07_2.log`
- `agent-task/archive/2026/08/m-iop-owned-single-request-agent-execution/01_preset_config/complete.log`

### SDD Criteria

- SDD: `agent-roadmap/sdd/knowledge-tool-optimization-extension/iop-owned-single-request-agent-execution/SDD.md`; status approved and implementation lock released.
- First-line Milestone task: `preset-binding`; targeted Acceptance Scenario: S02.
- Evidence Map rows: preset decode/authorization, public model echo, workspace snapshot, and config-refresh isolation. They require the checklist to reject marked admission at the real resolver, preserve fixed plan/work/review options, and prove nested snapshot isolation with fresh tests.

### Verification Context

- No separate verification handoff was supplied. Repository-native evidence came from the managed-principal authorization matrix, legacy model-list tests, service binding tests, Edge local test rules, the approved SDD, and the current contract/spec documents.
- Fresh reviewer commands passed: focused service/openai tests, package tests, `go test -race`, `go vet ./apps/edge/...`, `go test ./apps/edge/... -count=1`, Go formatting inspection, and `git diff --check`.
- Precondition: the exact packet 01 archived `complete.log` above. Constraints: local deterministic verification only; no external runner/provider, coordinator, concrete workspace execution, Node wire, or SSE lifecycle is part of this packet.
- Gap: existing green tests call compiler helpers directly, assert nil options, and do not prove resolver/model-list rejection. Confidence is high because the production error-swallowing and shallow copy sites are direct and the revised commands force fresh execution with `-count=1`.

### Test Coverage Gaps

- Managed compilation failure is not exercised through `resolveRouteDispatchForPrincipal` or the managed model list.
- Unmanaged marked presets are rejected by a helper test but still resolve and advertise through production code.
- Plan/review `reasoning_effort=high`, work option absence, duplicate roles, dynamic model mismatch, and option mismatch are not asserted as concrete admitted values or fail-closed errors.
- Constructor and clone tests mutate only top-level option maps; nested maps and slices remain untested.

### Symbol References

- No symbol is renamed or removed. Keep `compileSingleRequestBinding`, `compileSingleRequestBindingForUnmanaged`, `resolveStageBinding`, and `cloneMapStringAny` in place; update their current call sites and behavior without creating a compatibility migration.

### Split Judgment

- Keep one plan because error propagation, fixed-stage compilation, and recursive copy safety form one immutable admission invariant: a marked preset is either fully authorized and frozen or not resolvable at all.
- The dependent directory `02+01_preset_binding` names predecessor index 01. It is satisfied by the unique archived evidence `agent-task/archive/2026/08/m-iop-owned-single-request-agent-execution/01_preset_config/complete.log`.

### Scope Rationale

- Modify only the existing service DTO/copy implementation, OpenAI resolver/compiler paths, their regression tests, and the active review evidence file.
- Do not change config validation, contracts, or specs: they already state the required fixed, immutable, no-fallback behavior. Do not add coordinator, provider, workspace, Node, HTTP response, or SSE execution behavior.
- Preserve unmarked legacy presets, ordinary routes, public model echo, managed credential identity, and current error privacy.

### Final Routing

- `evaluation_mode=isolated-reassessment`; `finalizer=finalize-task-policy.sh`; `finalizer_mode=pair`; status routed with no missing evidence, blocker, or capability gap.
- Build closures: scope/context/verification/evidence/ownership/decision all true. Scores 2/1/1/2/1 = G07; base `local-fit`, recovery boundary matched because `review_rework_count=1` and `evidence_integrity_failure=true`; final route `worker/cloud/G07`; canonical filename `PLAN-cloud-G07.md`.
- Build signals: `large_indivisible_context=false`; positive loop risks `boundary_contract`, `concurrent_consistency`, and `variant_product` (3); risk boundary false; recovery boundary true.
- Review closures: scope/context/verification/evidence/ownership/decision all true. Scores 2/1/1/2/1 = G07; route `official-review`, catalog `review/cloud/G07`; canonical filename `CODE_REVIEW-cloud-G07.md`.

## Dependencies and Execution Order

1. Verify the unique predecessor completion evidence.
2. Fix production fail-closed resolution before relying on model-list and resolver tests.
3. Preserve and validate fixed stage options, then make the service copy recursively isolated.
4. Run all fresh focused, race, package, vet, full Edge, formatting, and hygiene checks.

## Implementation Checklist

- [ ] Propagate marked-preset compilation failures from managed and unmanaged production resolution, omit rejected presets from model lists, and preserve unmarked preset behavior.
- [ ] Compile and validate exact fixed plan/work/review role, model, and option values through authorized managed dispatches, with non-vacuous refresh and invalid-shape tests.
- [ ] Recursively isolate nested stage option values during construction and cloning, with source-to-binding and clone-to-original mutation tests.
- [ ] Run the dependency, formatting, focused, package, race, vet, full Edge regression, and diff-hygiene verification commands exactly as written.
- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.

## Implementation Plan

### [REVIEW_API-1] Fail closed at real route admission

**Problem**

- `apps/edge/internal/openai/route_resolution.go:195` calls the unmanaged compiler but ignores its error and returns a successful preset dispatch.
- `apps/edge/internal/openai/principal_routes.go:153` attaches a binding only on success but returns the marked preset normally on every compiler failure. Both model-list implementations therefore advertise a marked preset that cannot produce a valid immutable admission.

**Solution**

Before (`apps/edge/internal/openai/route_resolution.go:195`):

```go
if _, err := compileSingleRequestBindingForUnmanaged(model, preset); err == nil {
    // No binding to attach.
}
return disp, true
```

After:

```go
if preset.SingleRequest != nil {
    if _, err := compileSingleRequestBindingForUnmanaged(model, preset); err != nil {
        return routeDispatch{}, false
    }
}
return disp, true
```

For managed resolution, propagate any marked compiler error as the existing public `ErrRouteNotFound`, never return a marked dispatch with `SingleRequest == nil`, and attach the non-nil binding on success. Keep the nil-policy legacy path unchanged. The existing advertised-model helpers will then omit rejected entries through their resolver calls without a new listing policy.

**Modified Files and Checklist**

- [ ] `apps/edge/internal/openai/route_resolution.go` — reject unmanaged marked presets at the production resolver.
- [ ] `apps/edge/internal/openai/principal_routes.go` — reject managed marked presets when immutable compilation fails.
- [ ] `apps/edge/internal/openai/openai_auth_routes_models_test.go` — add unmanaged resolver and `/v1/models` no-fallback regression coverage.
- [ ] `apps/edge/internal/openai/principal_routes_test.go` — add managed resolver and model-list no-fallback regression coverage.

**Test Strategy**

- Add `TestUnmanagedSingleRequestPresetFailsClosed`: a marked preset must be absent from `/v1/models` and `resolveRouteDispatch` must return `ok=false`; an unmarked legacy preset remains listed/resolvable.
- Add `TestManagedSingleRequestPresetFailsClosed`: an authorized principal with an invalid marked stage shape must not see the virtual model and `resolveRouteDispatchForPrincipal` must return `ErrRouteNotFound`.
- Rerun `TestVirtualPresetModelAuthorizationMatrix` unchanged to preserve valid managed public identity and authorization behavior.

**Verification**

- `go test ./apps/edge/internal/openai -run 'Test(UnmanagedSingleRequestPresetFailsClosed|ManagedSingleRequestPresetFailsClosed|VirtualPresetModelAuthorizationMatrix)' -count=1`
- Expected: valid managed presets resolve; invalid managed and all unmanaged marked presets fail closed without listing fallback; unmarked legacy behavior remains green.

### [REVIEW_API-2] Preserve the approved fixed stage shape

**Problem**

- `apps/edge/internal/openai/single_request_preset_binding.go:45` passes only stage model strings into the binding resolver.
- `apps/edge/internal/openai/single_request_preset_binding.go:102` derives no values from the approved stage config and `extractStageOptions` always returns nil. The compiler never enforces the declared duplicate, dynamic, or option-inconsistent errors.

**Solution**

Before (`apps/edge/internal/openai/single_request_preset_binding.go:45`):

```go
planBinding, err := resolveStageBinding(sr.Stages.Plan.Model, bindings, view)
```

After, pass the approved config value and copy its options into the service DTO:

```go
planBinding, err := resolveStageBinding("plan", sr.Stages.Plan, bindings, view)
```

Validate the defense-in-depth shape before construction: exactly one light route; ordered unique `plan`, `work`, `review` roles; each route model/options exactly matching `SingleRequest.Stages`; selector exactly matching plan; high reasoning on plan/review and no reasoning option on work; one managed, same-principal dispatch whose `ModelGroupKey` equals each fixed canonical model. Map missing, duplicate, dynamic model, authorization, and option inconsistency to the existing errors. Copy options from the approved stage config, not provider dispatch metadata.

**Modified Files and Checklist**

- [ ] `apps/edge/internal/openai/single_request_preset_binding.go` — validate the complete fixed shape and preserve approved options.
- [ ] `apps/edge/internal/openai/single_request_preset_binding_test.go` — assert concrete options, invalid shapes, real managed resolution, and refresh isolation.

**Test Strategy**

- Extend `TestSingleRequestPresetBindingManaged` to require plan/review `reasoning_effort=high` and no work `reasoning_effort`.
- Extend `TestSingleRequestPresetBindingRejectsInvalidDefenseInDepth` with duplicate role, route/policy model mismatch, selector mismatch, plan/review option mismatch, and work reasoning-option cases, checking the intended existing error values.
- Make `TestSingleRequestPresetBindingRefreshIsolation` mutate `SingleRequest.Stages` option values after a real managed resolution and assert the already admitted non-empty options remain unchanged.

**Verification**

- `go test ./apps/edge/internal/openai -run 'TestSingleRequestPresetBinding' -count=1`
- Expected: approved options survive admission, invalid fixed shapes fail with the expected typed errors, and refresh mutation cannot alter the admitted binding.

### [REVIEW_API-3] Deep-copy nested option values

**Problem**

- `apps/edge/internal/service/single_request_types.go:179` promises a deep copy, but `cloneMapStringAny` assigns each nested value directly. Nested maps and slices therefore remain shared after construction and `Clone`.

**Solution**

Before (`apps/edge/internal/service/single_request_types.go:185`):

```go
for k, v := range m {
    out[k] = v
}
```

After, recursively clone each value using service-private helpers equivalent to the existing config copier:

```go
for k, v := range m {
    out[k] = cloneValueAny(v)
}
```

Handle nil, interface, pointer, map, slice, and array values recursively while leaving scalar values unchanged. Preserve concrete collection types so callers receive the same option shape without retaining mutable references.

**Modified Files and Checklist**

- [ ] `apps/edge/internal/service/single_request_types.go` — recursively copy supported nested option values.
- [ ] `apps/edge/internal/service/single_request_types_test.go` — prove nested constructor and clone isolation in both mutation directions.

**Test Strategy**

- Extend `TestSingleRequestBindingDefensiveCopyOptions` with nested `map[string]any` and slice values, mutate the caller-owned values, and require the constructed binding to retain originals.
- Extend `TestSingleRequestBindingCloneIsolation` by mutating nested values on the clone and original independently and asserting no cross-object change.

**Verification**

- `go test ./apps/edge/internal/service -run 'TestSingleRequestBinding(CloneIsolation|DefensiveCopyOptions)' -count=1`
- Expected: nested maps and slices never alias across caller, binding, or clone boundaries.

## Modified Files Summary

| File | Item |
|------|------|
| `apps/edge/internal/openai/route_resolution.go` | REVIEW_API-1 |
| `apps/edge/internal/openai/principal_routes.go` | REVIEW_API-1 |
| `apps/edge/internal/openai/openai_auth_routes_models_test.go` | REVIEW_API-1 |
| `apps/edge/internal/openai/principal_routes_test.go` | REVIEW_API-1 |
| `apps/edge/internal/openai/single_request_preset_binding.go` | REVIEW_API-2 |
| `apps/edge/internal/openai/single_request_preset_binding_test.go` | REVIEW_API-2 |
| `apps/edge/internal/service/single_request_types.go` | REVIEW_API-3 |
| `apps/edge/internal/service/single_request_types_test.go` | REVIEW_API-3 |
| `agent-task/m-iop-owned-single-request-agent-execution/02+01_preset_binding/CODE_REVIEW-cloud-G07.md` | REVIEW_API-1, REVIEW_API-2, REVIEW_API-3 |

## Final Verification

1. `test -f agent-task/m-iop-owned-single-request-agent-execution/01_preset_config/complete.log || test "$(compgen -G 'agent-task/archive/*/*/m-iop-owned-single-request-agent-execution/01_preset_config/complete.log' | wc -l)" -eq 1`
   - Expected: exactly one active or archived predecessor `complete.log` satisfies index 01.
2. `test -z "$(gofmt -l apps/edge/internal/openai/route_resolution.go apps/edge/internal/openai/principal_routes.go apps/edge/internal/openai/openai_auth_routes_models_test.go apps/edge/internal/openai/principal_routes_test.go apps/edge/internal/openai/single_request_preset_binding.go apps/edge/internal/openai/single_request_preset_binding_test.go apps/edge/internal/service/single_request_types.go apps/edge/internal/service/single_request_types_test.go)"`
   - Expected: no path output; all changed Go files are formatted.
3. `go test ./apps/edge/internal/service -run 'TestSingleRequestBinding' -count=1`
   - Expected: all service binding validity and nested isolation tests pass freshly.
4. `go test ./apps/edge/internal/openai -run 'Test(SingleRequestPresetBinding|UnmanagedSingleRequestPresetFailsClosed|ManagedSingleRequestPresetFailsClosed|VirtualPresetModelAuthorizationMatrix)' -count=1`
   - Expected: actual managed/unmanaged admission, fixed options, invalid defenses, listing, public identity, and refresh isolation pass freshly.
5. `go test ./apps/edge/internal/service ./apps/edge/internal/openai -count=1`
   - Expected: both changed packages pass without cached results.
6. `go test -race -count=1 ./apps/edge/internal/openai ./apps/edge/internal/service`
   - Expected: both changed packages pass under the race detector.
7. `go vet ./apps/edge/...`
   - Expected: no diagnostics.
8. `go test ./apps/edge/... -count=1`
   - Expected: the full Edge profile passes without cached results.
9. `git diff --check`
   - Expected: no whitespace errors.

**After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`.**
