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

# Code Review Reference - 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-03
task=m-iop-hot-path-one-shot-execution/11+09,10_cleanup, plan=3, tag=REVIEW_REVIEW_REVIEW_API

## Archive Evidence Snapshot

- Current review archive after finalization: `agent-task/m-iop-hot-path-one-shot-execution/11+09,10_cleanup/code_review_cloud_G09_2.log`.
- Earlier reviews: `agent-task/m-iop-hot-path-one-shot-execution/11+09,10_cleanup/code_review_cloud_G10_0.log` and `agent-task/m-iop-hot-path-one-shot-execution/11+09,10_cleanup/code_review_cloud_G10_1.log`.
- Current verdict: FAIL; findings: Required 1, Suggested 0, Nit 0.
- Required gap: `artifact_pair.go` promotes only explicit-error receipt failures to the primary-error cleanup transaction and immediately rejects other correlation-valid matcher failures.
- Reviewer reproduction: an exact successful Plan result plus Review result `{"written":false}` returned HTTP 400 on both OpenAI and Anthropic with no delete frontier.
- Trusted passing evidence: focused primary-error race tests, cleanup/TTL race tests, common package race tests, full Edge tests with an executable temporary directory, vet, format, and diff checks all passed; those suites omit the reproduced partial-pair matcher-failure variant.
- Affected implementation area: `artifact_pair.go`, the obsolete explicit-error classifier in `workspace_tool_codec.go`, and focused cleanup tests.
- Roadmap carryover: milestone task `cleanup`, approved and unlocked SDD Acceptance Scenario/Evidence Map row S09 only.

## 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-G08.md` → `code_review_cloud_G08_3.log` and `PLAN-cloud-G07.md` → `plan_cloud_G07_3.log`.
3. If PASS, write `complete.log` and move active task directory to `agent-task/archive/YYYY/MM/m-iop-hot-path-one-shot-execution/11+09,10_cleanup/`. 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>`, preserve the first-line `milestone-task` metadata in `complete.log` and report it for the runtime aggregation event. Roadmap state evaluation belongs to `sync-milestone-workstate`.
5. Check applicable `Review-Only Checklist` items at the final `.log` location before reporting.

---

## Implementation Item Completion

| Item | Status |
|------|---------|
| REVIEW_REVIEW_REVIEW_API-1 — Admit every correlation-valid artifact receipt failure to primary cleanup | [x] |
| REVIEW_REVIEW_REVIEW_API-2 — Close partial-pair matcher-failure evidence | [x] |

## Implementation Checklist

- [x] Promote every receipt mismatch with valid lineage, pending-call identity, and immutable issue correlation to the stored artifact primary error while preserving immediate rejection for invalid correlation.
- [x] Remove the obsolete explicit-error-only artifact classifier without changing receipt matcher or endpoint error semantics.
- [x] Add deterministic OpenAI and Anthropic partial-pair matcher-failure coverage for delete issue, cleanup acknowledgement failure, original error precedence, and provider-call count.
- [x] Run all focused and final verification commands with fresh output.
- [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_G08_3.log`.
- [x] Archive active `PLAN-*-G??.md` to `plan_cloud_G07_3.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-iop-hot-path-one-shot-execution/11+09,10_cleanup/` to `agent-task/archive/YYYY/MM/m-iop-hot-path-one-shot-execution/11+09,10_cleanup/` and update this checklist at the final archive path.
- [ ] If PASS and task group is `m-<milestone-slug>`, preserve and report `milestone-task` metadata for runtime aggregation, without modifying roadmap or directly calling `update-roadmap`.
- [ ] If PASS for split work, remove empty active parent `agent-task/m-iop-hot-path-one-shot-execution/` 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

- Cleanup-entry gate (REVIEW_REVIEW_REVIEW_API-1). The plan's "After" snippet gated cleanup entry on `matchResultCorrelation(...) != ""` alone. Implemented verbatim, that promotes *every* correlation-valid receipt failure — including an opaque/unparseable result body — into primary-error cleanup, because `matchResultCorrelation` only validates immutable issue identity and never inspects the body. That breaks the pre-existing fail-closed contract verified by `TestArtifactPairFrontierMatrix/{openai,anthropic}/reject opaque` and `TestArtifactPairFailureCleanupKeepsMalformedFailClosed/{openai,anthropic}/malformed result`, both of which require an unparseable body to return HTTP 400 with no delete frontier. The plan's own Final Verification runs the full `./apps/edge/...` suite and requires exit 0, so those tests must stay green.
- Resolution: the gate is `if matchResultCorrelation(...) != "" || !workspaceResultIsExact(result) { reject }`. `workspaceResultIsExact` is a new predicate in `workspace_tool_codec.go` that returns true only when the caller body decodes into the normalized `{status,result}` envelope. It replaces the removed explicit-error-only `workspaceResultExplicitlyFailed` (which admitted only bodies carrying an explicit error signal) and broadens admission to *any* exact (parseable) correlation-valid receipt-matcher failure, including `{"written":false}`, while keeping opaque/malformed bodies fail-closed. `matchResultReceipt`, `matchResultCorrelation`, lineage/owner/principal/expected-set validation, and the endpoint error envelopes are unchanged.
- No verification commands were changed; every command matches the stub. The post-loop light-flow guard (`if primaryFailure != nil && (lightFlows == nil || !lightFlows.has(...))`) is left unchanged per the plan's `artifact_pair.go:444-455` scope; it still requires an active light flow before a stored primary error can enter cleanup.

## Key Design Decisions

- Trust boundary is body parseability, not identity alone. An "exact caller-reported operation failure" (the phrase already in the `matchResultCorrelation` doc comment) is distinguished from "malformed/opaque/untrusted continuation input" by whether the body decodes into the normalized envelope. Identity correlation alone is insufficient because a valid call id can accompany an unparseable body; gating solely on it would authorize a delete frontier from untrusted input.
- Regression evidence reuses the existing precedence fixture. REVIEW_REVIEW_REVIEW_API-2 adds a `pair-matcher-failure` frontier whose Plan result is `{"written":true}` and Review result is `{"written":false}` (correlation-valid, matcher-only failure, no explicit error signal). The unchanged table body then asserts, on both OpenAI and Anthropic: one canonical `delete_file` frontier at the pair selector's response ID (`chatcmpl-scripted-pair` / `msg-scripted-pair`); both matching (`{"written":true}`) and failing (`{"written":false,"error":"delete-denied"}`) cleanup acknowledgements returning the original HTTP 400 `invalid_request_error` "artifact receipt rejected"; no "workspace cleanup failed"; no leaked "denied"; exactly two selector provider calls; and full coordinator/light/artifact state removal via `assertCleanupStoresRemoved`.
- Original-error precedence is preserved by the existing `consumeCleanupLocked`, which only substitutes the standard cleanup error when `intent.Error == nil`. Because the stored primary error is non-nil, a failed cleanup acknowledgement never overwrites the original artifact error — the matcher-failure variant exercises exactly this path and is asserted to keep the HTTP 400 body.

## Reviewer Checkpoints

- Confirm a valid request lineage, pending call, and immutable issue correlation are sufficient to route any receipt-matcher failure to primary cleanup, without trusting the result as success.
- Confirm wrong call identity, mutated issue correlation, owner/principal mismatch, or lineage mismatch still fails immediately and cannot authorize a delete frontier.
- Confirm a partially successful Plan/Review pair with `{"written":false}` issues exactly one canonical delete frontier on OpenAI and Anthropic.
- Confirm matching and failed cleanup acknowledgements retain the original artifact HTTP status/type/message and never expose `workspace cleanup failed`.
- Confirm provider-call counts, cancellation, TTL/redaction, duplicate cleanup, and existing explicit-error variants remain unchanged.

## Verification Results

Paste actual stdout/stderr for every command below. Do not summarize or reconstruct output. If a command changes, record the replacement and reason in `Deviations from Plan` before pasting its output.

### REVIEW_REVIEW_REVIEW_API-1 — correlated receipt classification

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

_Actual stdout/stderr:_

```text
ok  	iop/apps/edge/internal/openai	1.286s
```

### REVIEW_REVIEW_REVIEW_API-2 — registration and complete primary-error matrix

```bash
go test ./apps/edge/internal/openai -list '^TestHotPathCleanupPrimaryError' | rg '^TestHotPathCleanupPrimaryError'
go test -race -count=1 ./apps/edge/internal/openai -run '^TestHotPathCleanupPrimaryError'
```

_Actual stdout/stderr:_

```text
TestHotPathCleanupPrimaryErrorPrecedence
TestHotPathCleanupPrimaryErrorStageMatrix
TestHotPathCleanupPrimaryErrorStartFailure
ok  	iop/apps/edge/internal/openai	1.520s
```

### Final — prerequisites, focused suites, common race suites, and full Edge

```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 '^TestHotPathCleanupPrimaryError' | rg '^TestHotPathCleanupPrimaryError'
go test -race -count=1 ./apps/edge/internal/openai -run '^TestHotPathCleanupPrimaryError'
go test -race -count=1 ./apps/edge/internal/openai -run '^Test(LogicalRequestTTL|HotPathCleanup)'
go test -race -count=1 ./packages/go/streamgate ./packages/go/config ./apps/edge/internal/openai ./apps/edge/internal/service
edge_test_tmpdir="$(mktemp -d /config/workspace/iop-edge-test.XXXXXX)"
chmod 700 "$edge_test_tmpdir"
TMPDIR="$edge_test_tmpdir" go test -count=1 ./apps/edge/...
edge_test_status=$?
rmdir "$edge_test_tmpdir"
exit "$edge_test_status"
```

_Actual stdout/stderr:_

```text
TestHotPathCleanupPrimaryErrorPrecedence
TestHotPathCleanupPrimaryErrorStageMatrix
TestHotPathCleanupPrimaryErrorStartFailure
ok  	iop/apps/edge/internal/openai	1.492s
ok  	iop/apps/edge/internal/openai	1.875s
ok  	iop/packages/go/streamgate	1.977s
ok  	iop/packages/go/config	1.587s
ok  	iop/apps/edge/internal/openai	12.623s
ok  	iop/apps/edge/internal/service	7.007s
ok  	iop/apps/edge/cmd/edge	0.790s
ok  	iop/apps/edge/internal/authprojection	0.080s
ok  	iop/apps/edge/internal/bootstrap	6.615s
ok  	iop/apps/edge/internal/configrefresh	0.631s
ok  	iop/apps/edge/internal/controlplane	6.668s
ok  	iop/apps/edge/internal/edgecmd	0.340s
ok  	iop/apps/edge/internal/edgevalidate	0.117s
ok  	iop/apps/edge/internal/events	0.073s
ok  	iop/apps/edge/internal/input	0.141s
ok  	iop/apps/edge/internal/input/a2a	0.107s
ok  	iop/apps/edge/internal/node	0.106s
ok  	iop/apps/edge/internal/openai	7.992s
ok  	iop/apps/edge/internal/opsconsole	0.128s
ok  	iop/apps/edge/internal/service	6.028s
ok  	iop/apps/edge/internal/transport	4.956s
```

### Final — static checks

Run this block in a new shell after the full Edge command.

```bash
go vet ./apps/edge/...
gofmt -d apps/edge/internal/openai/artifact_pair.go apps/edge/internal/openai/workspace_tool_codec.go apps/edge/internal/openai/hot_path_cleanup_test.go
git diff --check
```

_Actual stdout/stderr:_

```text
(no output; go vet, gofmt -d, and git diff --check each exited 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) |
| 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: Fail — an empty success-status artifact result is classified as exact and authorizes a delete frontier even though the existing receipt contract defines a bodyless result as opaque.
  - Completeness: Fail — the new exactness predicate closes the non-empty matcher-failure case but does not preserve the empty-result fail-closed boundary stated by its own contract.
  - Test Coverage: Fail — the primary-error matrix covers `{"written":false}` and malformed non-JSON bodies, but it omits a correlation-valid empty result on both public handlers.
  - API Contract: Fail — an opaque caller result can now advance the artifact transaction into caller-executed cleanup instead of returning the endpoint-standard validation error without a delete frontier.
  - Code Quality: Pass — the reviewed files are formatted and vet-clean, the planned focused/race/full Edge suites pass, and the obsolete classifier has no stale source reference.
  - Implementation Deviation: Fail — the documented deviation says opaque or malformed results stay fail-closed, but `workspaceResultIsExact` accepts the empty-body branch of `normalizeResultEnvelope`.
  - Verification Trust: Fail — the claimed opaque-result preservation is contradicted by a fresh OpenAI/Anthropic public-handler reproducer even though every listed command exits 0.
  - Spec Conformance: Fail — the SDD requires opaque receipt evidence to remain outside trusted artifact progression while S09 cleanup applies only after a trustworthy caller-executed artifact outcome.
- Findings:
  - Required — `apps/edge/internal/openai/workspace_tool_codec.go:425`: `workspaceResultIsExact` delegates directly to `normalizeResultEnvelope`, whose empty-body branch succeeds with `result=nil`; consequently a successful Plan receipt plus an empty Review receipt produced HTTP 200 with a canonical `delete_file` frontier on both OpenAI and Anthropic in the reviewer reproducer. Preserve an explicit status/error signal as an exact failure, but reject a success-status result with an empty body before JSON normalization; add a focused classifier table and both-endpoint public-handler regression that assert HTTP 400, no delete frontier, and exactly two selector calls while retaining the existing `{"written":false}` cleanup path.
- Routing Signals:
  - `review_rework_count=4`
  - `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`.
