<!-- task=m-principal-provider-credential-slot-routing/05+01,02_management_core plan=6 tag=REVIEW_REVIEW_REVIEW_REVIEW_API -->

# Code Review Reference - REVIEW_REVIEW_REVIEW_REVIEW_API

> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is the mandatory final step of implementation.**
> The task is NOT complete until every implementation-owned section below is filled in.
> Complete the `Implementation Checklist`; the final checklist item is mandatory before saving.
> Fill implementation-owned sections, then stop with active files in place and report ready for review.
> If implementation is blocked, record the exact blocker, attempted commands/output, and resume condition only in implementation-owned evidence fields.
> Do not ask the user directly, present choices, call user-input tools, create control-plane stop files, or classify the next state.
> Finalization (`Code Review Result`, log rename, `complete.log`, archive moves, `Review-Only Checklist`) is review-agent-only, even after compaction/resume.
> Follow the ownership table at the bottom of this file for which sections you own.

## Overview

date=2026-08-01
task=m-principal-provider-credential-slot-routing/05+01,02_management_core, plan=6, tag=REVIEW_REVIEW_REVIEW_REVIEW_API

## Archive Evidence Snapshot

- Current pair after archive: `agent-task/m-principal-provider-credential-slot-routing/05+01,02_management_core/plan_cloud_G05_5.log`, `agent-task/m-principal-provider-credential-slot-routing/05+01,02_management_core/code_review_cloud_G05_5.log`.
- Verdict: FAIL; Required 2, Suggested 0, Nit 0; `review_rework_count=4`, `evidence_integrity_failure=true`.
- Required repairs: assert revision metadata in the complete ordered token list; scan every failure error/log for unique plaintext, sealer-error, ciphertext, nonce, AAD, key-ID, and key-version sentinels; handle all snapshot read errors; compare complete slot and revision-history state.
- Fresh reviewer verification: both focused race suites, full credentialstore/credentialops race tests, Control Plane regression tests, predecessor checks, formatting, vet, structural absence, probe absence, and `git diff --check` exited zero. Source inspection proved the listed assertion gaps.
- Roadmap carryover: this remains preparatory S08 evidence. PASS must not check `credential-management`; concrete at-rest sealing and confidential Client transport remain later S09/S10 work.

## For the Review Agent

> **[REVIEW AGENT ONLY]** The finalization steps below are review-agent only. Implementing agents must not execute this section.

Compare implementation of each item against source files and verify that output in `Verification Results` matches code.
Review completion means the following steps are finished:

1. Append verdict and `review_rework_count` / `evidence_integrity_failure` routing signals.
2. Archive `CODE_REVIEW-cloud-G05.md` → `code_review_cloud_G05_6.log` and `PLAN-cloud-G05.md` → `plan_cloud_G05_6.log`.
3. If PASS, write `complete.log` and move active task directory to `agent-task/archive/YYYY/MM/m-principal-provider-credential-slot-routing/05+01,02_management_core/`. If WARN/FAIL, fully write the next filesystem state required by the code-review skill.
4. If PASS and task group is `m-<milestone-slug>`, report completion event metadata. Roadmap state check and `update-roadmap` calls are runtime responsibilities.
5. Check applicable `Review-Only Checklist` items at the final `.log` location before reporting.

---

## Implementation Item Completion

| Item | Status |
|------|---------|
| REVIEW_REVIEW_REVIEW_REVIEW_API-1 Close ordered token revision evidence | [x] |
| REVIEW_REVIEW_REVIEW_REVIEW_API-2 Make the secret failure matrix exact | [x] |

## Implementation Checklist

- [x] Assert exact revision metadata in every returned record from `TestAdditionalTokenIssueAndList` while preserving generation, digest-only persistence, ordering, and rejected-issuance checks.
- [x] Make every create/rotate secret failure case prove complete sentinel redaction and exact before/after slot, revision-history, and projection state with checked snapshot errors.
- [x] Run fresh focused race, full package race, Control Plane regression, predecessor, formatting, vet, structural-absence, probe-absence, and diff verification.
- [x] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.

## Review-Only Checklist

> **[REVIEW AGENT ONLY]** This checklist is used only by the review agent.
> Implementing agents must not modify or check this section.

- [x] Append one verdict of `PASS`, `WARN`, or `FAIL` and verified `review_rework_count`, `evidence_integrity_failure` to `Code Review Result`.
- [x] Verify that verdict, `Dimension Assessment`, and Required/Suggested/Nit classifications match.
- [x] Archive active `CODE_REVIEW-*-G??.md` to `code_review_cloud_G05_6.log`.
- [x] Archive active `PLAN-*-G??.md` to `plan_cloud_G05_6.log`.
- [x] Verify that the Agent-Ops managed block in `.gitignore` unignores `agent-task/**/*.md` and `agent-task/**/*.log` and ignores `agent-roadmap/current.md`.
- [x] If PASS, write `complete.log` based on `agent-ops/skills/common/code-review/templates/complete-log-template.md` and leave no active `.md` files.
- [x] If PASS, move active task directory `agent-task/m-principal-provider-credential-slot-routing/05+01,02_management_core/` to `agent-task/archive/YYYY/MM/m-principal-provider-credential-slot-routing/05+01,02_management_core/` and update this checklist at the final archive path.
- [x] If PASS and task group is `m-<milestone-slug>`, report completion event metadata for runtime, without modifying roadmap or directly calling `update-roadmap`.
- [x] If PASS for split work, remove empty active parent `agent-task/m-principal-provider-credential-slot-routing/` or verify it was kept due to remaining siblings/files.
- [ ] If WARN/FAIL, write the next filesystem state matching code-review verdict and do not write `complete.log`.

## Deviations from Plan

None. Implementation strictly followed `PLAN-cloud-G05.md`.

## Key Design Decisions

1. Strengthened `TestAdditionalTokenIssueAndList` in `apps/control-plane/internal/credentialstore/principal_test.go` by adding `status` and `revision` fields (type `int64`) to `expectedTokens`, comparing them in the `ListTokens` loop as well as raw database row scanning, and checking `rows.Err()` after iteration.
2. Refactored `TestServiceSecretHandlingFailureIsBlindAndDoesNotMutate` in `apps/control-plane/internal/credentialops/service_test.go` to use helper functions (`mustGetSlot`, `mustListSlots`, `mustListSlotRevisions`, `mustProjectionGeneration`) that fail fast on snapshot errors.
3. Expanded secret redaction sentinel set to include unique sentinels for plaintext, sealer error, key ID, key version, ciphertext, nonce, and AAD, and validated both returned error strings and captured logs against all forbidden sentinels across all CreateSlot and RotateSlot failure cases (failing sealer, unknown key ID, invalid envelope variant).
4. Replaced partial slot / history checks with complete before/after state comparisons using `reflect.DeepEqual`.

## Reviewer Checkpoints

- Confirm each expected token record carries status and revision and that both direct row inspection and `ListTokens` compare those exact values in stable order.
- Confirm token row iteration errors are checked and the existing generation, digest-only persistence, raw-token absence, and rejected-issuance assertions remain intact.
- Confirm every applicable failing-sealer, unknown-key/version, and invalid-envelope create/rotate path uses unique plaintext, error, ciphertext, nonce, AAD, key-ID, and key-version sentinels and excludes all of them from returned errors and captured logs.
- Confirm all projection, slot-list, slot, and revision-history snapshots handle read errors and compare complete before/after state rather than count-only history.
- Confirm every path still proves service-owned buffer zeroization and caller-owned buffer preservation.
- Confirm no production source, proto, ClientServer, Dart, CLI, config, server wiring, concrete sealer, or S08 Roadmap completion claim enters this packet unless a strengthened assertion exposes and records a production defect.

## Verification Results

Paste actual stdout/stderr beneath every command; do not summarize or reconstruct it.

### REVIEW_REVIEW_REVIEW_REVIEW_API-1

`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 -run 'AdditionalTokenIssueAndList'`

```
ok  	iop/apps/control-plane/internal/credentialstore	1.214s
```

### REVIEW_REVIEW_REVIEW_REVIEW_API-2

`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/credentialops -run 'ZeroizesOwnedProviderSecret|SecretHandlingFailureIsBlindAndDoesNotMutate'`

```
ok  	iop/apps/control-plane/internal/credentialops	1.386s
```

### Final Verification

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 && test "$(find agent-task/archive -type f -path '*/m-principal-provider-credential-slot-routing/02+01_credential_catalog/complete.log' -print | wc -l)" -eq 1`

```

```

2. `mkdir -p .cache/go-build .cache/go-cache`

```

```

3. `test -z "$(gofmt -d apps/control-plane/internal/credentialstore/principal_test.go apps/control-plane/internal/credentialops/service_test.go)"`

```

```

4. `GOTMPDIR=/config/workspace/iop-s0/.cache/go-build GOCACHE=/config/workspace/iop-s0/.cache/go-cache go vet ./apps/control-plane/internal/credentialstore ./apps/control-plane/internal/credentialops`

```

```

5. `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 ./apps/control-plane/internal/credentialops`

```
ok  	iop/apps/control-plane/internal/credentialstore	6.272s
ok  	iop/apps/control-plane/internal/credentialops	2.046s
```

6. `GOTMPDIR=/config/workspace/iop-s0/.cache/go-build GOCACHE=/config/workspace/iop-s0/.cache/go-cache go test -count=1 ./apps/control-plane/...`

```
ok  	iop/apps/control-plane/cmd/control-plane	2.944s
ok  	iop/apps/control-plane/internal/credentialops	0.097s
ok  	iop/apps/control-plane/internal/credentialstore	3.314s
ok  	iop/apps/control-plane/internal/wire	1.530s
```

7. `if sed -n '/func TestServiceSecretHandlingFailureIsBlindAndDoesNotMutate/,/^}/p' apps/control-plane/internal/credentialops/service_test.go | rg -n ', _ := store\.(ProjectionGeneration|GetSlot|ListSlotRevisions|ListSlots)'; then exit 1; else exit 0; fi`

```

```

8. `if rg --sort path -n 'message[[:space:]]+.*(Bootstrap|CredentialOperation)|AddRequestListenerTyped.*Credential' proto/iop/control.proto apps/control-plane/internal/wire/client.go; then exit 1; else exit 0; fi`

```

```

9. `test ! -e apps/control-plane/internal/credentialops/reviewer_probe_test.go`

```

```

10. `git diff --check`

```

```

---

> **[IMPLEMENTING AGENT — BEFORE SAVING] Have you filled in every implementation-owned section?**
> If anything is blank, go back and fill it in before saving this file.
> Leave review-agent-only sections unchanged.

## Section Ownership

| Section | Owner | Note |
|---------|-------|------|
| Header comment, Overview, Review Agent Instructions | Fixed at stub creation | Implementing agent must not modify or execute these (archive, complete.log, and task-directory archive move are review-agent only) |
| Roadmap Targets | Fixed at stub creation from plan when present | Implementing agent must not modify; code-review copies it into `complete.log` as `Roadmap Completion` only on PASS |
| Archive Evidence Snapshot | Fixed at stub creation from plan when present | Implementing agent uses it as default prior-loop context; read only the specific archive files cited there when more detail is required |
| Implementation Item Completion (item names) | Fixed at stub creation | Implementing agent checks `[ ]` → `[x]` only |
| Implementation Checklist (item text/order) | Fixed at stub creation from plan | Implementing agent checks `[ ]` → `[x]` only |
| Review-Only Checklist | Review agent only | Implementing agent must not modify or check this section |
| Deviations from Plan, Key Design Decisions | Implementing agent | Replace placeholder text with actual content |
| Reviewer Checkpoints | Fixed at stub creation | Pre-filled from plan |
| Verification Results (section headings + commands) | Fixed at stub creation | Implementing agent fills in command output only; command changes require a `Deviations from Plan` entry |
| Code Review Result | Review agent appends | Not included in stub |

## Code Review Result

- Overall Verdict: PASS
- Dimension Assessment:
  - Correctness: Pass — source inspection confirms exact ordered token revision assertions, checked snapshot reads, secret-blind error handling, owned-buffer zeroization, caller-buffer preservation, and complete before/after rotate state comparisons.
  - Completeness: Pass — every implementation and verification checklist item is supported by the active source and fresh reviewer evidence.
  - Test Coverage: Pass — the focused tests cover the inherited ordered-token and secret-failure assertion gaps, and both package race suites and the full Control Plane regression pass.
  - API Contract: Pass — this packet changes deterministic tests only and preserves the current Client wire, protobuf, store, and service boundaries.
  - Code Quality: Pass — the modified tests are formatted, vet-clean, free of debug/TODO/probe artifacts, and use checked helper reads for repeated state snapshots.
  - Implementation Deviation: Pass — the implementation remains within the two planned test files and does not claim S08 or Roadmap completion.
  - Verification Trust: Pass — every recorded command was reproduced successfully from the current worktree, including focused race tests, full package race tests, Control Plane regression, structural absence checks, and `git diff --check`.
- Findings: None.
- Routing Signals: `review_rework_count=4`, `evidence_integrity_failure=false`
- Next Step: Archive the active pair as `plan_cloud_G05_6.log` and `code_review_cloud_G05_6.log`, write `complete.log`, and move this split task to the dated task archive without modifying the Roadmap.
