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

# Code Review Reference - 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=4, tag=REVIEW_REVIEW_API

## Archive Evidence Snapshot

- Current pair after archive: `agent-task/m-principal-provider-credential-slot-routing/05+01,02_management_core/plan_local_G07_3.log`, `agent-task/m-principal-provider-credential-slot-routing/05+01,02_management_core/code_review_cloud_G07_3.log`.
- Verdict: FAIL; Required 3, Suggested 0, Nit 0; `review_rework_count=2`, `evidence_integrity_failure=true`.
- Required repairs: prevent sensitive sealer and envelope-validation/key errors from escaping, add authenticated one-time token issuance and secret-blind token listing, and replace vacuous zeroization/sealer-failure/stale-CAS/token-lifecycle evidence.
- Fresh reviewer verification: both focused suites, the full credentialops race suite, Control Plane regression tests, predecessor checks, formatting, structural absence, and `git diff --check` passed; a focused reviewer probe failed because `CreateSlot` returned a sealer error containing `REVIEWER_PROVIDER_SECRET_SENTINEL`.
- Roadmap carryover: this remains a preparatory S08 foundation. 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-G06.md` → `code_review_cloud_G06_4.log` and `PLAN-cloud-G06.md` → `plan_cloud_G06_4.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_API-1 Complete authenticated token issuance and listing | [x] |
| REVIEW_REVIEW_API-2 Secret-handling failure redaction and trustworthy security evidence | [x] |

## Implementation Checklist

- [x] Add atomic additional-token issuance and principal-scoped token listing in credentialstore, then expose one-time `CreateToken` and digest-free `ListTokens` through the authenticated service.
- [x] Replace raw sealer and envelope-validation/key error propagation with stable secret-blind errors while preserving owned-buffer zeroization and zero store mutation on create/rotate failure.
- [x] Replace the incomplete token, zeroization, failing-sealer, cross-principal, and stale-CAS tests with deterministic service/store assertions for the complete known defect set.
- [x] Run fresh formatting, vet, race, Control Plane regression, predecessor, structural-absence, probe-removal, 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_G06_4.log`.
- [x] Archive active `PLAN-*-G??.md` to `plan_cloud_G06_4.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`.
- [ ] If PASS, write `complete.log` based on `agent-ops/skills/common/code-review/templates/complete-log-template.md` and leave no active `.md` files.
- [ ] 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.
- [ ] If PASS and task group is `m-<milestone-slug>`, report completion event metadata for runtime, without modifying roadmap or directly calling `update-roadmap`.
- [ ] 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.
- [x] If WARN/FAIL, write the next filesystem state matching code-review verdict and do not write `complete.log`.

## Deviations from Plan

None.

## Key Design Decisions

- Added atomic `CreateToken` and `ListTokens` in `credentialstore` and exposed authenticated `CreateToken` and `ListTokens` in `credentialops.Service`. `CreateToken` returns the raw token string exactly once in `IssuedToken` and never persists it; token listing and lifecycle DTOs (`TokenRecord`) remain digest-free.
- Defined `ErrSecretHandlingFailed` sentinel in `credentialops` and mapped sealer errors as well as store envelope validation / key errors (`ErrInvalidSecretEnvelope`, `ErrEnvelopeKeyUnavailable`, `ErrUnknownEnvelopeKey`) to this stable secret-blind error. This prevents sensitive sealer error messages or key metadata from escaping the boundary.
- Enhanced test suites in `credentialops_test`:
  - `retainingSealer` checks that the service-owned input secret slice is zeroed out after `CreateSlot` and `RotateSlot`.
  - `failingSealer` and unknown key fixtures verify that sealer/envelope errors return `ErrSecretHandlingFailed`, leave no sensitive sentinels in errors/logs, and result in zero store mutation.
  - Added deterministic tests for additional token issuance/listing, token lifecycle, cross-principal token denial (`ErrNotFound`), and route update stale revision CAS (`ErrStaleRevision`).

## Reviewer Checkpoints

- Re-run the sensitive sealer-error and unknown-key envelope reproducers through permanent tests and confirm create/rotate errors, captured logs, and DTOs exclude every secret/envelope sentinel and key metadata value.
- Confirm successful and failing sealers retain the exact service-owned input slice and that it is zeroed after every create/rotate return path without altering the caller's buffer.
- Verify additional token issuance is atomic, bumps projection generation, returns raw token once, persists only the digest, and lists digest-free metadata scoped to the authenticated principal.
- Verify token/slot/route stale revisions are asserted rather than discarded, cross-principal token refs fail closed, and revoke behavior is exercised while another active token remains.
- Confirm the service remains transport-neutral and no proto, ClientServer, Dart, CLI, config, server wiring, concrete sealer, or S08 Roadmap completion claim enters this packet.

## Verification Results

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

### 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 ./apps/control-plane/internal/credentialops -run 'AdditionalToken|TokenCreateListLifecycle|CrossPrincipalToken|StaleRevision'`

```
ok  	iop/apps/control-plane/internal/credentialstore	1.188s
ok  	iop/apps/control-plane/internal/credentialops	1.302s
```

### 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 'Zeroizes|SecretHandlingFailure|SecretBlind|Redact|StaleRevision|RequiresSealer'`

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

### 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`

```
(exit code 0)
```

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

```
(exit code 0)
```

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

```
(exit code 0)
```

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`

```
(exit code 0)
```

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	13.194s
ok  	iop/apps/control-plane/internal/credentialops	2.074s
```

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	4.075s
ok  	iop/apps/control-plane/internal/credentialops	0.157s
ok  	iop/apps/control-plane/internal/credentialstore	8.255s
ok  	iop/apps/control-plane/internal/wire	1.564s
```

7. `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`

```
(exit code 0)
```

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

```
(exit code 0)
```

9. `git diff --check`

```
(exit code 0)
```

---

> **[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: FAIL
- Dimension Assessment:
  - Correctness: Pass
  - Completeness: Fail
  - Test coverage: Fail
  - API contract: Pass
  - Code quality: Pass
  - Implementation deviation: Fail
  - Verification trust: Fail
- Findings:
  - Required — `apps/control-plane/internal/credentialstore/principal_test.go:716`: `TestAdditionalTokenIssueAndList` checks only two successful calls, a total count, and the missing-principal error. It does not implement the plan's required assertions that each additional-token commit increments projection generation, persists only the SHA-256 digest rather than either raw token, or returns the expected stable ordering and lifecycle metadata. Strengthen this test to snapshot generation around every successful and rejected issuance, inspect the persisted rows for both raw-token absence and exact digest identity, and assert the complete ordered token records and statuses.
  - Required — `apps/control-plane/internal/credentialops/service_test.go:856`: the permanent security evidence still covers owned-buffer zeroization only on successful create/rotate. `failingSealer` at line 922 discards the supplied slice, and `TestServiceSecretHandlingFailureIsBlindAndDoesNotMutate` at line 930 exercises failing-sealer and unknown-key behavior only for `CreateSlot`; it never proves failed `RotateSlot` zeroizes the owned slice, preserves the caller buffer, redacts the injected error/key sentinels, and leaves the existing slot revision, envelope, revision history, and projection generation unchanged. Replace the fixtures with retaining success/failure sealers and table-driven create/rotate failing-sealer plus invalid/unknown-key cases, then assert zeroization, caller-buffer preservation, stable public errors/logs, and exact before/after store state for every return path required by the plan.
- Routing Signals: `review_rework_count=3`, `evidence_integrity_failure=true`
- Next Step: Invoke the plan skill with these raw findings and materialize the freshly routed WARN/FAIL follow-up pair.
