<!-- task=m-iop-hot-path-one-shot-execution/11+09,10_cleanup plan=1 tag=REVIEW_API milestone-task=cleanup -->

# Code Review Reference - REVIEW_API

> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is the mandatory final step of implementation.**
> Fill item statuses, deviations, decisions, and actual output, then stop with active files and report ready. Record blockers only in implementation evidence. Do not ask the user, create control state, classify, archive, mutate roadmap state, or write `complete.log`; review owns finalization.

## Overview

date=2026-08-03
task=m-iop-hot-path-one-shot-execution/11+09,10_cleanup, plan=1, tag=REVIEW_API

## For the Review Agent

> **[REVIEW AGENT ONLY]** Implementers must not execute this section.

Compare source and fresh evidence against the routed FAIL findings. Append verdict/signals, archive the pair, and on PASS write `complete.log`, preserve metadata, archive the directory, and update the final `.log` checklist. WARN/FAIL must create the exact mandatory next state.

## Archive Evidence Snapshot

- Archived plan: `agent-task/m-iop-hot-path-one-shot-execution/11+09,10_cleanup/plan_cloud_G09_0.log`
- Archived review: `agent-task/m-iop-hot-path-one-shot-execution/11+09,10_cleanup/code_review_cloud_G10_0.log`
- Verdict: FAIL
- Finding counts: Required 3, Suggested 0, Nit 0.
- Required source gaps: `hot_path_review.go` returns a logical terminal before an exact delete receipt; `request_coordinator.go` retains terminal state and silently deletes expired state without active-state protection or raw-free orphan observations.
- Required evidence gap: both implementation items and their focused/common race and vet outputs were left incomplete because the implementer checked only obsolete active predecessor paths.
- Predecessor correction: both exact archived predecessor `complete.log` files above report PASS.
- Roadmap carryover: milestone task `cleanup`, approved/unlocked SDD scenario and Evidence Map row S09 only.

## Implementation Item Completion

| Item | Status |
|------|--------|
| REVIEW_API-1 Commit terminal intent only after exact cleanup acknowledgement | [x] |
| REVIEW_API-2 Bound inactive state TTL and emit raw-free orphan responsibility | [x] |

## Implementation Checklist

- [x] Hold one success or primary-error terminal intent behind a canonical exact delete receipt and make cleanup/finalization exactly once across duplicates and races.
- [x] Preserve primary error identity, convert successful work plus cleanup failure to the standard endpoint error, and stop without hidden model/tool cleanup after cancellation or disconnect.
- [x] Reclaim only bounded inactive server state by TTL, protect active work, remove matching hot-path records safely, and emit fixed raw-free orphan responsibility observations.
- [x] Add deterministic cleanup, TTL, redaction, cancellation, and concurrency tests for both compatible endpoint flows.
- [x] Run every focused and final verification command exactly as written and fill all implementation-owned sections in this file with actual output.

## Review-Only Checklist

> **[REVIEW AGENT ONLY]** Implementers must not modify or check this section.

- [x] Append one PASS/WARN/FAIL verdict with verified `review_rework_count` and `evidence_integrity_failure`.
- [x] Verify verdict, Dimension Assessment, and Required/Suggested/Nit classifications match.
- [x] Archive the active review to `code_review_cloud_G10_1.log`.
- [x] Archive the active plan to `plan_cloud_G10_1.log`.
- [x] Verify the Agent-Ops `.gitignore` block.
- [ ] On PASS write `complete.log` from `agent-ops/skills/common/code-review/templates/complete-log-template.md` and leave no active `.md`.
- [ ] On PASS archive to `agent-task/archive/YYYY/MM/m-iop-hot-path-one-shot-execution/11+09,10_cleanup/` and update this checklist there.
- [ ] On PASS preserve/report `milestone-task=cleanup` without direct roadmap mutation.
- [ ] On PASS remove the active parent only if no siblings/files remain.
- [x] On WARN/FAIL create the mandatory next state without `complete.log`.

## Deviations from Plan

No product-scope deviation. Supporting edits beyond the summary table were required in `chat_handler.go` and `anthropic_handler.go` to consume cleanup/terminal dispositions before provider dispatch, in `workspace_tool_codec.go` to expose immutable issue-correlation validation without adding a receipt dialect, and in existing coordinator/direct tests to reflect immediate terminal record removal and explicit TTL sweep ownership.

The first exact `go test -count=1 ./apps/edge/...` run failed only because the host mounts `/tmp` with `noexec`, so `TestActualNodeReconnectReadyPumpsQueuedWaiterExactlyOnce` could build but could not execute its temporary `iop-node` (`permission denied`). The same command passed on the current checkout after exporting an untracked executable temporary directory under `/config/workspace` as `TMPDIR`; both the initial failure and passing rerun are preserved below.

## Key Design Decisions

- The light record persists exactly one immutable success or primary endpoint error intent before issuing one caller-executed delete through the pinned workspace binding. The coordinator owns the cleanup stage and exact public/provider call mapping.
- Cleanup admission reuses the canonical delete encoder, payload correlation digest, reserved request directory, configured result matcher, and exact continuation lineage. An exact failed or mismatched receipt converts only a pending success to the standard endpoint cleanup error; an existing primary error retains its original status, type, and sanitized message.
- Cleanup receipt commit and coordinator removal occur in one coordinator critical section. The light record is removed under its own lock and the artifact record is removed before the stored terminal is written, so duplicate and concurrent continuations have one terminal winner.
- A cancelled context marks the coordinator record disconnected and issues neither a delete call nor another provider call. The bounded TTL observer later owns server-state reclamation without claiming workspace deletion.
- TTL selection is deterministic and bounded, skips active state, removes coordinator state before releasing its lock, and removes matching light/artifact state afterward. The orphan log allowlist is fixed to request ID, canonical directory, prior state, stage, terminal class, and fixed reason; no prompt, content, result, principal, or credential is emitted.

## Reviewer Checkpoints

- Success or primary-error terminal intent is stored before one canonical delete issue and is externally committed only after exact receipt handling.
- A mismatched/failed receipt cannot become success; an existing primary endpoint error retains its identity; duplicate/concurrent results have one winner.
- Disconnect/cancellation emits no subsequent cleanup/model call, and malformed or unknown continuations never trigger a blind delete.
- TTL work is bounded, skips active in-flight state, coordinates matching server-store removal, and never claims caller workspace deletion.
- Orphan observations include only fixed request id, canonical reserved path, prior state/stage or terminal class, and reason; sentinel raw data is absent.
- Chat Completions and Anthropic endpoint paths preserve their existing public error envelopes while sharing the same logical cleanup invariants.

## Verification Results

Paste actual stdout/stderr and exit status below each command block.

### REVIEW_API-1 item verification

```bash
go test -race -count=1 ./apps/edge/internal/openai -run '^TestHotPathCleanup'
```

_Actual stdout/stderr:_

```text
ok  	iop/apps/edge/internal/openai	1.681s
exit status 0
```

### REVIEW_API-2 item verification

```bash
go test -race -count=1 ./apps/edge/internal/openai -run '^Test(LogicalRequestTTL|HotPathCleanup)'
```

_Actual stdout/stderr:_

```text
ok  	iop/apps/edge/internal/openai	1.688s
exit status 0
```

### Dependency and test registration checks

```bash
test -f agent-task/archive/2026/08/m-iop-hot-path-one-shot-execution/09+06,08_artifact_pair/complete.log
test -f agent-task/archive/2026/08/m-iop-hot-path-one-shot-execution/10+07,09_light_flow/complete.log
go test ./apps/edge/internal/openai -list 'Test(LogicalRequestTTL|HotPathCleanup)' | rg '^Test(HotPathCleanup|LogicalRequestTTL)'
```

_Actual stdout/stderr:_

```text
The two dependency checks produced no stdout and exited 0.
TestHotPathCleanupTerminalMatrix
TestHotPathCleanupPrimaryErrorPrecedence
TestHotPathCleanupConcurrentExactlyOnce
TestHotPathCleanupCancellationStopsWork
TestLogicalRequestTTLSweep
TestLogicalRequestTTLActiveSurvives
TestLogicalRequestTTLFinalizeRace
TestLogicalRequestTTLObservationRedaction
exit status 0
```

### Common race and Edge tests

```bash
go test -race -count=1 ./packages/go/streamgate ./packages/go/config ./apps/edge/internal/openai ./apps/edge/internal/service
go test -count=1 ./apps/edge/...
```

_Actual stdout/stderr:_

```text
ok  	iop/packages/go/streamgate	2.014s
ok  	iop/packages/go/config	1.611s
ok  	iop/apps/edge/internal/openai	11.236s
ok  	iop/apps/edge/internal/service	7.051s
exit status 0

Initial full-Edge run with the host default TMPDIR:
ok  	iop/apps/edge/cmd/edge	0.296s
ok  	iop/apps/edge/internal/authprojection	0.086s
--- FAIL: TestActualNodeReconnectReadyPumpsQueuedWaiterExactlyOnce (4.85s)
    reconnect_readiness_integration_test.go:81: start actual iop-node: fork/exec /tmp/TestActualNodeReconnectReadyPumpsQueuedWaiterExactlyOnce162188885/001/iop-node: permission denied
FAIL
FAIL	iop/apps/edge/internal/bootstrap	5.440s
ok  	iop/apps/edge/internal/configrefresh	0.180s
ok  	iop/apps/edge/internal/controlplane	6.721s
ok  	iop/apps/edge/internal/edgecmd	0.229s
ok  	iop/apps/edge/internal/edgevalidate	0.160s
ok  	iop/apps/edge/internal/events	0.100s
ok  	iop/apps/edge/internal/input	0.224s
ok  	iop/apps/edge/internal/input/a2a	0.178s
ok  	iop/apps/edge/internal/node	0.147s
ok  	iop/apps/edge/internal/openai	7.968s
ok  	iop/apps/edge/internal/opsconsole	0.139s
ok  	iop/apps/edge/internal/service	6.009s
ok  	iop/apps/edge/internal/transport	4.863s
FAIL
exit status 1

Passing rerun after exporting an untracked executable TMPDIR under /config/workspace:
ok  	iop/apps/edge/cmd/edge	0.960s
ok  	iop/apps/edge/internal/authprojection	0.121s
ok  	iop/apps/edge/internal/bootstrap	5.836s
ok  	iop/apps/edge/internal/configrefresh	0.760s
ok  	iop/apps/edge/internal/controlplane	6.765s
ok  	iop/apps/edge/internal/edgecmd	0.452s
ok  	iop/apps/edge/internal/edgevalidate	0.183s
ok  	iop/apps/edge/internal/events	0.145s
ok  	iop/apps/edge/internal/input	0.269s
ok  	iop/apps/edge/internal/input/a2a	0.229s
ok  	iop/apps/edge/internal/node	0.148s
ok  	iop/apps/edge/internal/openai	8.181s
ok  	iop/apps/edge/internal/opsconsole	0.160s
ok  	iop/apps/edge/internal/service	6.116s
ok  	iop/apps/edge/internal/transport	4.995s
exit status 0
```

### Vet, format, and diff

```bash
go vet ./apps/edge/...
gofmt -d apps/edge/internal/openai/hot_path_cleanup.go apps/edge/internal/openai/hot_path_cleanup_test.go apps/edge/internal/openai/hot_path_light.go apps/edge/internal/openai/hot_path_light_test.go apps/edge/internal/openai/hot_path_review.go apps/edge/internal/openai/hot_path_review_test.go apps/edge/internal/openai/artifact_pair.go apps/edge/internal/openai/artifact_pair_test.go apps/edge/internal/openai/request_identity_ingress.go apps/edge/internal/openai/request_coordinator.go apps/edge/internal/openai/request_coordinator_ttl.go apps/edge/internal/openai/request_coordinator_ttl_test.go
git diff --check
```

_Actual stdout/stderr:_

```text
go vet ./apps/edge/...: no stdout/stderr; exit status 0.
gofmt -d ...: no stdout/stderr; exit status 0.
git diff --check: no stdout/stderr; exit status 0.
```

---

> **[IMPLEMENTING AGENT — BEFORE SAVING] Have you filled in every implementation-owned section?** Leave review-only sections unchanged.

## Section Ownership

| Section | Owner | Note |
|---------|-------|------|
| Fixed structure, item names/checklist/checkpoints/commands | Fixed | Do not rewrite |
| Item status, deviations, decisions, actual output | Implementer | Must complete |
| Review checklist and verdict/finalization | Review agent | Implementer must not modify |

## Code Review Result

- Overall Verdict: FAIL
- Dimension Assessment:
  - Correctness: Fail — exact prepare-receipt failure and post-artifact local/review stage failure do not enter the required primary-error cleanup transaction.
  - Completeness: Fail — the primary-error path is implemented only for a resumed artifact-pair frontier with an already committed selector response identity.
  - Test Coverage: Fail — the cleanup matrix covers pair-write failure but omits prepare failure and cleanup-capable local/review stage errors.
  - API Contract: Fail — SDD S09 requires artifact-bearing error paths to attempt caller-executed cleanup while preserving the primary endpoint error.
  - Code Quality: Pass — the reviewed cleanup/TTL code is formatted, race-clean under the listed suites, and contains no debug or stale-symbol residue.
  - Implementation Deviation: Fail — the plan requires exact correlated artifact generation failures and primary endpoint errors to share the cleanup transaction, but the implementation terminates known variants directly.
  - Verification Trust: Fail — every listed command passes, but a focused reviewer reproducer contradicts the claimed primary-error production path.
  - Spec Conformance: Fail — S09 error best-effort cleanup evidence is incomplete even though success, receipt-failure, cancellation, TTL, and redaction evidence pass.
- Findings:
  - Required — `apps/edge/internal/openai/hot_path_cleanup.go:121` and `apps/edge/internal/openai/hot_path_light.go:745`: primary-error cleanup requires `intent.Output.ResponseID` or `selectorCommit.ResponseID`, while prepare failure occurs before selector commit and non-cancelled local/review errors call `terminalPresetRequest` directly. A focused reviewer test on both OpenAI and Anthropic returned HTTP 400 with `cleanup response identity is unavailable` after an exact failed prepare receipt instead of issuing the delete frontier. Persist the selector response identity before the prepare frontier, extend primary-error cleanup to replace the exact active light stage as well as a resumed artifact frontier, route cleanup-capable local/review errors through that transaction, and add deterministic prepare/local/review primary-error tests that assert delete acknowledgement and original error precedence.
- Routing Signals:
  - `review_rework_count=2`
  - `evidence_integrity_failure=true`
- Next Step: Invoke the plan skill for a FAIL follow-up using this raw reviewer evidence; do not create `USER_REVIEW.md`.
