<!-- task=m-principal-provider-credential-slot-routing/02+01_credential_catalog plan=3 tag=REVIEW_API -->

# Fail Closed Credential Catalog Migration and Freeze Profile Resolution

## For the Implementing Agent

Filling implementation-owned sections of `CODE_REVIEW-cloud-G08.md` is mandatory. Run every verification command, paste actual output, keep the active pair in place, and report ready for review; only code-review may finalize, rename logs, write `complete.log`, or archive. If blocked, record only exact blocker evidence, attempted commands/output, and the resume condition in implementation-owned fields; do not ask the user, call user-input tools, create stop files, classify the next state, archive logs, or write `complete.log`.

## Background

The catalog now rejects invalid tuples on new mutations, but the intermediate-schema migration preserves legacy active bindings that the new compatibility rules reject. Route resolution also reads a public mutable profile snapshot, so unrelated in-process mutation can change credential routing after startup. This follow-up closes both bypasses with one compatibility source and deterministic regression evidence.

## Archive Evidence Snapshot

- Closing pair: `agent-task/m-principal-provider-credential-slot-routing/02+01_credential_catalog/plan_cloud_G08_2.log`, `agent-task/m-principal-provider-credential-slot-routing/02+01_credential_catalog/code_review_cloud_G08_2.log`.
- Verdict: FAIL with two Required findings, zero Suggested findings, and zero Nits.
- Required repair: reject or deterministically deactivate incompatible active bindings during intermediate-catalog migration, and isolate credential routing from the exported mutable built-in profile snapshot.
- Fresh evidence: all planned focused, race, repeated, Control Plane regression, vet, predecessor, and diff commands passed. Two temporary reviewer regressions failed: `Open` preserved an incompatible legacy route as active, and deleting `openai` from `config.BuiltInProtocolProfiles` broke an otherwise valid `CreateRoute`. The temporary reviewer file was removed.
- Roadmap carryover: S04 `slot-store` and S05 `model-binding` remain incomplete until this follow-up passes.

## Roadmap Targets

- Milestone: `agent-roadmap/phase/operational-observability-provider-management/milestones/principal-provider-credential-slot-routing.md`
- Milestone link: [Milestone document](agent-roadmap/phase/operational-observability-provider-management/milestones/principal-provider-credential-slot-routing.md)
- Task ids:
  - `slot-store`: principal-scoped provider credential slot ledger
  - `model-binding`: one route to one slot/profile/upstream-model tuple
- Completion mode: check-on-pass

## Analysis

### Files Read

- `apps/control-plane/internal/credentialstore/schema.go`
- `apps/control-plane/internal/credentialstore/store.go`
- `apps/control-plane/internal/credentialstore/secret_envelope.go`
- `apps/control-plane/internal/credentialstore/slot.go`
- `apps/control-plane/internal/credentialstore/slot_test.go`
- `apps/control-plane/internal/credentialstore/route.go`
- `apps/control-plane/internal/credentialstore/route_test.go`
- `apps/control-plane/cmd/control-plane/server.go`
- `apps/control-plane/cmd/control-plane/credential_store_test.go`
- `packages/go/config/protocol_profile.go`
- `agent-roadmap/phase/operational-observability-provider-management/PHASE.md`
- `agent-roadmap/phase/operational-observability-provider-management/milestones/principal-provider-credential-slot-routing.md`
- `agent-roadmap/sdd/operational-observability-provider-management/principal-provider-credential-slot-routing/SDD.md`
- `agent-spec/control/control-plane-operations.md`

### SDD Criteria

- Approved and unlocked SDD: `agent-roadmap/sdd/operational-observability-provider-management/principal-provider-credential-slot-routing/SDD.md`.
- Targeted scenarios: S04 / `slot-store` and S05 / `model-binding`; the S04-S08 Evidence Map row requires compatible multi-profile binding and fail-closed alias/binding isolation.
- The checklist therefore applies the same closed compatibility source to new mutations and migrated active rows, and the final verification proves that legacy invalid tuples cannot remain active while valid migrated bindings remain usable.

### Verification Context

- No external handoff was supplied. Repository-native sources were `agent-test/local/rules.md`, `agent-test/local/control-plane-smoke.md`, the Control Plane/testing domain rules, the approved SDD, the profile catalog, the current package tests, and the exact predecessor completion log.
- Local preflight: `/config/.local/bin/go`, Go 1.26.2 linux/arm64, `GOROOT=/config/opt/go`, repo root `/config/workspace/iop-s0`. Use ignored `.cache/go-build` and `.cache/go-cache`; fresh `-count=1` results are required.
- Fresh baseline: focused slot/route suites, package vet, package race, 25-repeat fail-closed/CAS tests, `./apps/control-plane/...`, predecessor count, and `git diff --check` passed.
- Reviewer evidence: an intermediate SQLite catalog with an active `unknown-provider`/`openai` binding reopened successfully and retained an active route; mutating the exported built-in profile snapshot removed `openai` from credential routing. Both reviewer regressions failed and their temporary source was removed.
- No external service, provider credential, Docker runner, remote host, or user authorization is required. Confidence: high.

### Test Coverage Gaps

- Intermediate migration covers credential-kind backfill and optional alias, but it does not cover an incompatible persisted active route or an active slot whose bindings violate the new closed catalog.
- Route tests cover unknown tuples on `CreateRoute`, but they do not prove that credential routing is isolated from mutation of `config.BuiltInProtocolProfiles`.

### Symbol References

- No symbol is renamed or removed. `resolveSlotProfile` remains package-local and is called by route create/update/enable and binding validation.

### Split Judgment

- Keep one packet. Migration validation must call the same immutable compatibility resolver as route create/update/enable; splitting would leave one active-state path able to accept a tuple that the other rejects.
- Runtime predecessor `01_principal_store` is satisfied by `agent-task/archive/2026/08/m-principal-provider-credential-slot-routing/01_principal_store/complete.log`.

### Scope Rationale

- Do not modify built-in protocol profile definitions, module dependencies, Control Plane management transports, Edge projections, credential leases, discovery, or adapter injection. This repair only closes the S04/S05 durable catalog bypasses.
- Do not modify predecessor principal/token code or unrelated shared-worktree task-loop artifacts. The later `contract-ops` roadmap task owns living-spec and wire-contract synchronization.

### Final Routing

- `status=routed`, `evaluation_mode=isolated-reassessment`, finalizer=`finalize-task-policy.sh`, mode=`pair`.
- Build closures: scope/context/verification/evidence/ownership/decision all true. Scores `2+1+2+2+1=G08`; base=`local-fit`, route basis=`recovery-boundary`, lane=`cloud`, filename=`PLAN-cloud-G08.md`.
- Review closures: scope/context/verification/evidence/ownership/decision all true. Scores `2+1+2+2+1=G08`; route=`official-review`, lane=`cloud`, adapter=`codex`, model=`gpt-5.6-sol`, reasoning=`xhigh`, filename=`CODE_REVIEW-cloud-G08.md`.
- `large_indivisible_context=false`; positive loop risks=`temporal_state`, `boundary_contract`, `variant_product` (3); `review_rework_count=2`; `evidence_integrity_failure=false`; recovery boundary matched; capability gap none.

## Dependencies and Execution Order

1. `01_principal_store` is satisfied by the exact archived completion path recorded above.
2. Freeze the credential-store profile catalog before using the resolver to validate migrated active rows.
3. Keep every change within `02+01_credential_catalog`; no new split dependency is introduced.

## Implementation Checklist

- [ ] Fail closed when intermediate catalogs contain active slot/route bindings that violate the current vendor/credential-kind/profile-auth contract, while preserving valid migrated bindings with deterministic dialect-safe tests.
- [ ] Resolve route compatibility from a credential-store-owned immutable profile catalog and prove exported compatibility-snapshot mutation cannot change route behavior.
- [ ] Run fresh focused, repeated, race, Control Plane regression, predecessor, reviewer-artifact absence, and diff verification.
- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.

### [REVIEW_API-1] Fence incompatible active rows during catalog migration

**Problem:** `apps/control-plane/internal/credentialstore/schema.go:245-284` backfills `credential_kind` and relaxes the slot schema, but it never validates existing active routes against `checkSlotProfileCompatibility`. A row accepted by the prior permissive implementation therefore remains active after `Open` even when every new mutation rejects the same tuple.

**Solution:** After dialect-specific slot backfill/rebuild, validate every active route joined to its slot with the same package-owned profile resolver used by route mutations. Reject migration/startup with a redacted typed compatibility error when an active route targets a non-active slot or an incompatible vendor/kind/profile-auth tuple; also reject an active slot that has no compatible non-revoked binding. Keep the check idempotent and portable through `bindQuery`, and preserve the existing valid intermediate-catalog migration fixture.

Before (`apps/control-plane/internal/credentialstore/schema.go:192-197`):

```go
if err := migrateCredentialSlotContract(ctx, db, dialect); err != nil {
    return fmt.Errorf("migrate credential slot contract: %w", err)
}
```

After contract:

```go
if err := migrateCredentialSlotContract(ctx, db, dialect); err != nil {
    return fmt.Errorf("migrate credential slot contract: %w", err)
}
if err := validateMigratedCredentialBindings(ctx, db, dialect); err != nil {
    return fmt.Errorf("validate migrated credential bindings: %w", err)
}
```

**Modified Files and Checklist:**

- [ ] `apps/control-plane/internal/credentialstore/schema.go`: add dialect-portable, idempotent active-slot/route compatibility validation after the credential-kind migration.
- [ ] `apps/control-plane/internal/credentialstore/route.go`: expose one package-local immutable resolver path for both migration and route mutations without duplicating compatibility rules.
- [ ] `apps/control-plane/internal/credentialstore/slot_test.go`: extend the valid intermediate migration and add `TestCredentialSlotMigrationRejectsIncompatibleActiveBinding` with no raw secret output.

**Test Strategy:** Update `TestCredentialSlotMigrationPreservesIntermediateCatalog` so every migrated active slot has a compatible binding. Add `TestCredentialSlotMigrationRejectsIncompatibleActiveBinding` using an intermediate SQLite schema with an active unknown-vendor/OpenAI route; assert `Open` fails with `ErrIncompatibleProfile` and does not expose a serving-ready store.

**Verification:** `GOTMPDIR=/config/workspace/iop-s0/.cache/go-build GOCACHE=/config/workspace/iop-s0/.cache/go-cache go test -count=1 ./apps/control-plane/internal/credentialstore -run 'CredentialSlotMigration'` must pass.

### [REVIEW_API-2] Isolate credential routing from the mutable compatibility snapshot

**Problem:** `apps/control-plane/internal/credentialstore/route.go:192-200` calls `config.ResolveProtocolProfile` with `config.BuiltInProtocolProfiles`. `packages/go/config/protocol_profile.go:276-283` documents that exported map as a mutable compatibility snapshot and provides `BuiltInProtocolProfileCatalog()` for an independent process-owned clone.

**Solution:** Initialize a package-private credential profile catalog from `config.BuiltInProtocolProfileCatalog()` and use it for every slot/profile resolution, including migration validation. Do not mutate the snapshot after initialization; `ResolveProtocolProfile` already deep-copies the selected profile.

Before (`apps/control-plane/internal/credentialstore/route.go:192-199`):

```go
func resolveSlotProfile(slot CredentialSlotRecord, profileID string) (config.ConcreteProtocolProfile, error) {
    profile, err := config.ResolveProtocolProfile(profileID, slot.Vendor, config.BuiltInProtocolProfiles)
    if err != nil {
        return config.ConcreteProtocolProfile{}, fmt.Errorf("%w: %v", ErrIncompatibleProfile, err)
    }
```

After contract:

```go
var credentialProtocolProfiles = config.BuiltInProtocolProfileCatalog()

func resolveSlotProfile(slot CredentialSlotRecord, profileID string) (config.ConcreteProtocolProfile, error) {
    profile, err := config.ResolveProtocolProfile(profileID, slot.Vendor, credentialProtocolProfiles)
```

**Modified Files and Checklist:**

- [ ] `apps/control-plane/internal/credentialstore/route.go`: own and consume an isolated built-in profile snapshot for compatibility resolution.
- [ ] `apps/control-plane/internal/credentialstore/route_test.go`: add `TestRouteResolutionUsesImmutableProfileCatalog`, mutate/restore only the exported compatibility snapshot, and prove valid route creation remains stable.

**Test Strategy:** Add the named regression using the deterministic fake key registry and a valid OpenAI bearer slot. Delete or alter `config.BuiltInProtocolProfiles["openai"]` for the test, restore it with cleanup, and assert `CreateRoute` still resolves the package-owned profile without a data race.

**Verification:** `GOTMPDIR=/config/workspace/iop-s0/.cache/go-build GOCACHE=/config/workspace/iop-s0/.cache/go-cache go test -count=1 ./apps/control-plane/internal/credentialstore -run 'ImmutableProfileCatalog|Compatibility'` must pass.

## Modified Files Summary

| File | Items |
|------|-------|
| `apps/control-plane/internal/credentialstore/schema.go` | REVIEW_API-1 |
| `apps/control-plane/internal/credentialstore/route.go` | REVIEW_API-1, REVIEW_API-2 |
| `apps/control-plane/internal/credentialstore/slot_test.go` | REVIEW_API-1 |
| `apps/control-plane/internal/credentialstore/route_test.go` | REVIEW_API-2 |
| `agent-task/m-principal-provider-credential-slot-routing/02+01_credential_catalog/CODE_REVIEW-cloud-G08.md` | REVIEW_API-1, REVIEW_API-2 |

## Final Verification

Run from `/config/workspace/iop-s0`; cached Go test results are not acceptable.

1. `test "$(find agent-task/archive -type f -path '*/m-principal-provider-credential-slot-routing/01_principal_store/complete.log' -print | wc -l)" -eq 1` — exactly one archived predecessor completion exists.
2. `mkdir -p .cache/go-build .cache/go-cache` — executable Go temp/cache paths exist.
3. `GOTMPDIR=/config/workspace/iop-s0/.cache/go-build GOCACHE=/config/workspace/iop-s0/.cache/go-cache go vet ./apps/control-plane/internal/credentialstore` — no vet findings.
4. `GOTMPDIR=/config/workspace/iop-s0/.cache/go-build GOCACHE=/config/workspace/iop-s0/.cache/go-cache go test -count=1 ./apps/control-plane/internal/credentialstore -run 'CredentialSlotMigration|ImmutableProfileCatalog|Compatibility'` — migration and immutable-catalog regressions pass.
5. `GOTMPDIR=/config/workspace/iop-s0/.cache/go-build GOCACHE=/config/workspace/iop-s0/.cache/go-cache go test -count=25 ./apps/control-plane/internal/credentialstore -run 'MigrationRejectsIncompatible|ImmutableProfileCatalog'` — the two reviewer regressions remain stable across repetition.
6. `GOTMPDIR=/config/workspace/iop-s0/.cache/go-build GOCACHE=/config/workspace/iop-s0/.cache/go-cache go test -count=1 -race ./apps/control-plane/internal/credentialstore` — S04/S05 migration, compatibility, lifecycle, CAS, isolation, and reopen fixtures pass under the race detector.
7. `GOTMPDIR=/config/workspace/iop-s0/.cache/go-build GOCACHE=/config/workspace/iop-s0/.cache/go-cache go test -count=1 ./apps/control-plane/...` — Control Plane regression suite passes.
8. `test ! -e apps/control-plane/internal/credentialstore/reviewer_probe_test.go` — no repository-local reviewer probe remains.
9. `git diff --check` — no whitespace errors.

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