<!-- task=m-iop-hot-path-one-shot-execution/09+06,08_artifact_pair plan=1 tag=REVIEW_API milestone-task=artifact-pair -->

# Code Review Reference - 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/09+06,08_artifact_pair, plan=1, tag=REVIEW_API

## Archive Evidence Snapshot

- Prior artifacts after review finalization: `agent-task/m-iop-hot-path-one-shot-execution/09+06,08_artifact_pair/plan_cloud_G08_0.log` and `agent-task/m-iop-hot-path-one-shot-execution/09+06,08_artifact_pair/code_review_cloud_G09_0.log`.
- Prior verdict: FAIL with 3 Required, 0 Suggested, and 0 Nit findings; `review_rework_count=1` and `evidence_integrity_failure=true`.
- Required findings: replace `hot_path_dispatch.go:810` HTTP 501 with a pinned-binding prepare/pair frontier; validate endpoint-native result status/body in `request_identity_ingress.go:34,110` before exactly-once frontier consumption; add the absent `artifact_pair.go`, `artifact_pair_test.go`, `TestArtifactPairFrontierMatrix`, and fresh implementation evidence.
- Affected files: `apps/edge/internal/openai/hot_path_dispatch.go`, `apps/edge/internal/openai/request_identity_ingress.go`, `apps/edge/internal/openai/server.go`, `apps/edge/internal/openai/artifact_pair.go`, and `apps/edge/internal/openai/artifact_pair_test.go`.
- Fresh review evidence: `go test -race -count=1 ./apps/edge/internal/openai -run 'Test(Workspace|ArtifactPair)'`, the shared race suite, `go vet ./apps/edge/internal/openai`, and `git diff --check` passed, but `go test ./apps/edge/internal/openai -list 'Test(Workspace|ArtifactPair)'` listed only five `TestWorkspace...` tests and no `TestArtifactPair...` test. The planned production and test files were absent.
- Roadmap carryover: approved SDD scenario S06 and Evidence Map row `artifact-pair` remain the sole scope; local/review model execution belongs to later milestone children.

## 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-G09.md` → `code_review_cloud_G09_1.log` and `PLAN-cloud-G09.md` → `plan_cloud_G09_1.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/09+06,08_artifact_pair/`. 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_API-1 Wire the pinned prepare/pair frontier | [x] |
| REVIEW_API-2 Add the S06 endpoint and rejection matrix | [x] |

## Implementation Checklist

- [x] Implement REVIEW_API-1 as one pinned, bounded, exactly-once prepare/pair frontier for Chat and Messages.
- [x] Implement REVIEW_API-2 with the complete deterministic S06 matrix and run every focused/common verification command.
- [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_G09_1.log`.
- [x] Archive active `PLAN-*-G??.md` to `plan_cloud_G09_1.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/09+06,08_artifact_pair/` to `agent-task/archive/YYYY/MM/m-iop-hot-path-one-shot-execution/09+06,08_artifact_pair/` 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

None.

## Key Design Decisions

- Compile and pin a workspace binding only when the admitted preset allows `light`; direct-only presets and general tool continuations retain the existing coordinator path.
- Keep the binding, selector stage, sealed encoded payloads, pending receipt hash, and consumed replay tombstones in one fixed-capacity mutex-protected store.
- Allocate distinct public tool IDs while retaining provider IDs in the sealed payload and coordinator mapping, and emit only mapped prepare or Plan/Review calls through the existing endpoint-native response writers.
- Validate the complete result set and every configured receipt while holding the artifact frontier lock, then consume the logical frontier. Prepare reactivates the original selector stage; pair success publishes one local-eligibility disposition without starting a workspace or local worker.
- Preserve rejected frontiers unchanged so missing, extra, duplicate, opaque, failed, mixed, alternate-ID, and replay attempts cannot advance coordinator or artifact state.

## Reviewer Checkpoints

- The initial request compiles and pins one immutable workspace binding, and later artifact turns cannot switch alternatives or request identities.
- The client receives only the exact request-directory prepare or exact Plan/Review pair through endpoint-native Chat/Messages response shapes; the Edge never executes a workspace tool.
- Endpoint-native result bodies and statuses match every stored encoded payload before the logical-request frontier is consumed.
- Prepare success resumes the retained selector stage; pair success makes local eligibility true exactly once, including under concurrent replay.
- Missing, extra, duplicate, opaque, failed, mixed, traversal, alternate-request, and replayed results fail without state advancement or downstream dispatch.
- Direct/general tool continuations preserve their existing coordinator behavior.

## Verification Results

### Dependency and named-test preflight

```bash
test -f agent-task/m-iop-hot-path-one-shot-execution/06+04,05_request_identity_ingress/complete.log || test -f agent-task/archive/2026/08/m-iop-hot-path-one-shot-execution/06+04,05_request_identity_ingress/complete.log
test -f agent-task/m-iop-hot-path-one-shot-execution/08+02,04,06_workspace_binding/complete.log || test -f agent-task/archive/2026/08/m-iop-hot-path-one-shot-execution/08+02,04,06_workspace_binding/complete.log
go test ./apps/edge/internal/openai -list 'TestArtifactPair' | rg --fixed-strings 'TestArtifactPairFrontierMatrix'
```

_Actual stdout/stderr:_

```text
TestArtifactPairFrontierMatrix
```

The two dependency checks produced no stdout/stderr and exited 0.

### Focused artifact and workspace race verification

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

_Actual stdout/stderr:_

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

### Shared package race verification

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

_Actual stdout/stderr:_

```text
ok  	iop/packages/go/streamgate	2.088s
ok  	iop/apps/edge/internal/openai	9.333s
ok  	iop/apps/edge/internal/service	7.005s
```

### Vet, formatting, and diff verification

```bash
go vet ./apps/edge/internal/openai
gofmt -d apps/edge/internal/openai/artifact_pair.go apps/edge/internal/openai/artifact_pair_test.go apps/edge/internal/openai/hot_path_dispatch.go apps/edge/internal/openai/request_identity_ingress.go apps/edge/internal/openai/server.go
git diff --check
```

_Actual stdout/stderr:_

```text
(no stdout/stderr; all commands 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 — artifact receipt validation can advance the internal record, but the production handlers neither consume the resulting phase nor preserve the fixed `light` transition through the next dispatch.
  - Completeness: Fail — pair success has no local-stage handoff/no-selector boundary, and successful direct completion does not release the artifact record introduced for mixed direct/light presets.
  - Test Coverage: Fail — `TestArtifactPairFrontierMatrix` exercises ingress/store helpers directly and does not cover the unconditional Chat/Messages provider redispatch or artifact-record lifecycle on direct completion.
  - API Contract: Fail — SDD S06 requires prepare to resume only the retained selector subphase and exact pair success to cross the local-stage frontier; the current handler path can reclassify after prepare and redispatch the selector after pair success.
  - Code Quality: Pass — the new store, mapping, and receipt checks are structured and free of task-local debug/dead-code findings; the blocking issues are lifecycle and integration behavior.
  - Implementation Deviation: Fail — the plan describes an integrated pinned prepare/pair frontier, but the implementation stops at metadata publication and helper-level tests without wiring the production disposition consumer.
  - Verification Trust: Pass — every claimed dependency, named-test, race, vet, formatting, and diff command was rerun successfully with fresh reviewer evidence.
  - Spec Conformance: Fail — the `artifact-pair` Evidence Map is not satisfied while pair success can dispatch the selector again and direct completion can exhaust the pinned frontier store.
- Findings:
  - Required — `apps/edge/internal/openai/request_identity_ingress.go:42` and `apps/edge/internal/openai/request_identity_ingress.go:139`: `applyArtifactDisposition` only writes metadata, and no production code reads `iop_artifact_disposition` or `iop_artifact_local_eligible`; Chat (`chat_handler.go:330`) and Messages (`anthropic_handler.go:61`) therefore unconditionally dispatch the selector again after an exact pair succeeds. The same missing phase gate lets a post-prepare selector response be reclassified as `direct`, despite SDD S06 fixing `light` and allowing only the pair-authoring subphase. Return/consume a typed artifact disposition at the handler boundary, resume the selector only for prepare, route pair success to the local-stage handoff without another selector dispatch, reject a `pair_ready` downgrade to direct, and add Chat/Messages handler-level tests that assert the exact service-call sequence.
  - Required — `apps/edge/internal/openai/hot_path_direct.go:71`: a successful no-tool direct terminal calls only `requestCoordinator.terminal`, so the artifact record pinned for every preset that allows `light` remains in `artifactFrontiers`; after `defaultArtifactFrontierCapacity` such direct requests, `artifactFrontierStore.pin` rejects otherwise valid traffic with `artifact frontier capacity reached`. Close successful direct requests through `terminalPresetRequest` (or otherwise remove the matching artifact record atomically) and add a lifecycle regression proving repeated direct completion does not grow or exhaust the store.
- Routing Signals:
  - `review_rework_count=2`
  - `evidence_integrity_failure=false`
- Next Step: Invoke the plan skill with these raw findings and create the freshly routed follow-up pair; no user-review gate applies.
