<!-- task=m-iop-owned-single-request-agent-execution/18+17_plan_stage plan=4 tag=REVIEW_API milestone-task=plan-stage -->

# 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.
> Execute the plan's selected root cause, scope, files, and dependency decisions as written. Do not choose another owner, narrow/expand the write boundary, or replace a fix with another verification attempt.
> 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-07
task=m-iop-owned-single-request-agent-execution/18+17_plan_stage, plan=4, tag=REVIEW_API

## Archive Evidence Snapshot

- Prior plan: `agent-task/m-iop-owned-single-request-agent-execution/18+17_plan_stage/plan_cloud_G05_3.log`.
- Prior review: `agent-task/m-iop-owned-single-request-agent-execution/18+17_plan_stage/code_review_cloud_G05_3.log`.
- Verdict: `FAIL`; Required R1-R2, Suggested 0, Nit 0; `review_rework_count=3`, `evidence_integrity_failure=true`.
- R1: the private provider decoder accepts object-valued Chat content as JSON text, and the Plan decoder accepts duplicate known keys with last-value-wins semantics.
- R2: the checked matrix still omits invalid nested shapes, `USAGE`, complete frozen request/credential assertions, duplicate Plan keys, and rejected artifact no-write evidence.
- Fresh reviewer evidence: all eleven recorded commands passed, while a temporary focused reproducer failed because object-valued content and duplicate `plan` keys were accepted. The temporary test file was removed and `git diff --check` remained clean.
- Roadmap carryover: this packet contributes only `milestone-task=plan-stage` and SDD S08. Work, Review/repair, composite activation, generic error/cancel integration, and S12 Claude/Mac qualification remain deferred.

## 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-G04.md` → `code_review_cloud_G04_4.log` and `PLAN-cloud-G04.md` → `plan_cloud_G04_4.log`.
3. If PASS, write `complete.log` and move active task directory to `agent-task/archive/YYYY/MM/m-iop-owned-single-request-agent-execution/18+17_plan_stage/`. 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 Enforce strict private JSON shapes | [x] |
| REVIEW_API-2 Make every checked evidence row executable | [x] |

## Implementation Checklist

- [x] Reject non-string or structurally ambiguous Chat results and duplicate provider/Plan JSON keys before any PLAN artifact write, with deterministic regression cases.
- [x] Complete frozen Run/Tunnel/credential/predicate, `USAGE` frame, nested response-shape, and rejected artifact no-write assertions.
- [x] Run dependency, discovery, focused, vet, broader Edge, no-activation, spec, formatting, and diff-hygiene checks 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_G04_4.log`.
- [x] Archive active `PLAN-*-G??.md` to `plan_cloud_G04_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-iop-owned-single-request-agent-execution/18+17_plan_stage/` to `agent-task/archive/YYYY/MM/m-iop-owned-single-request-agent-execution/18+17_plan_stage/` 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-owned-single-request-agent-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

- Implemented `validateSingleRequestJSON` in `apps/edge/internal/openai/single_request_provider_stage.go` to recursively validate JSON objects and reject duplicate keys at any nesting level before typed decoding.
- Created private `singleRequestChatResponse`, `singleRequestChatChoice`, and `singleRequestChatMessage` structs with `Content *string` to strictly require JSON string content and fail closed on non-string (object, array, null) content.
- Applied `validateSingleRequestJSON` in `renderSingleRequestPlan` to reject duplicate keys in Plan stage result payloads.
- Updated fake `planController` in test to track write attempts separately from persisted content, verifying that failed artifact writes leave content empty.

## Reviewer Checkpoints

- Verify R1 rejects non-string/null Chat content, nested unknown fields, and duplicate keys at every owned object level before Plan rendering or artifact dispatch.
- Verify R2 asserts every Run/Tunnel/default field, the complete credential snapshot, a selective candidate predicate, explicit `USAGE` rejection, and zero persisted artifact content after a rejected write.
- Verify every acquired tunnel closes on success and failure, every exposed error remains a stable sentinel, and all failed Plan cases leave no persisted artifact content.
- Verify canonical discovery selects both complete families and the spec still defers Work, Review/repair, activation, generic error/cancel integration, and S12.
- Verify no shared Chat behavior, outer executor, service contract, roadmap state, or external qualification is changed.

## Verification Results

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

### 1. Dependency evidence

`bash -c 'set -euo pipefail; shopt -s nullglob; candidates=(agent-task/m-iop-owned-single-request-agent-execution/17_*/complete.log agent-task/m-iop-owned-single-request-agent-execution/17+*/complete.log agent-task/archive/*/*/m-iop-owned-single-request-agent-execution/17_*/complete.log agent-task/archive/*/*/m-iop-owned-single-request-agent-execution/17+*/complete.log); printf "%s\n" "${candidates[@]}"; ((${#candidates[@]} == 1))'`

Expected: exactly one predecessor completion path and exit zero.

```text
agent-task/archive/2026/08/m-iop-owned-single-request-agent-execution/17_internal_artifact_wire/complete.log
```

### 2. Focused test discovery

`bash -c 'set -euo pipefail; listed=$(go test ./apps/edge/internal/openai -list "TestSingleRequest(ProviderStage|PlanStage)"); printf "%s\n" "$listed"; rg -q "^TestSingleRequestProviderStage" <<<"$listed"; rg -q "^TestSingleRequestPlanStage" <<<"$listed"'`

Expected: both canonical test families are listed and the command exits zero.

```text
TestSingleRequestProviderStageUsesFrozenOptionsAndDispatch
TestSingleRequestProviderStageRejectsResponseEnvelope
TestSingleRequestProviderStageRejectsFrameFailures
TestSingleRequestProviderStageRejectsMismatchLimitAndContext
TestSingleRequestPlanStageWritesArtifact
TestSingleRequestPlanStageFailsClosed
ok  	iop/apps/edge/internal/openai	0.052s
```

### 3. Mandatory regression discovery

`rg --sort path -n 'non-string-content-object|duplicate-message-key|usage-frame|duplicate-plan-key|duplicate-verification-key|artifact-write-failure-rejects' apps/edge/internal/openai/single_request_provider_stage_test.go apps/edge/internal/openai/single_request_plan_stage_test.go`

Expected: every mandatory named regression is printed.

```text
apps/edge/internal/openai/single_request_provider_stage_test.go
231:			name: "non-string-content-object",
255:			name: "duplicate-message-key",
299:		"usage-frame": func() chan *iop.ProviderTunnelFrame {

apps/edge/internal/openai/single_request_plan_stage_test.go
96:		{"duplicate-plan-key", `{"plan":"A","plan":"B","verification":"V"}`},
97:		{"duplicate-verification-key", `{"plan":"P","verification":"V1","verification":"V2"}`},
190:	t.Run("artifact-write-failure-rejects", func(t *testing.T) {
```

### 4. Provider-stage matrix

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

Expected: the full provider authority/response/frame/deadline matrix passes freshly.

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

### 5. Plan-stage matrix

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

Expected: the full strict Plan/render/envelope/artifact matrix passes freshly.

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

### 6. Focused admission and S08 integration

`go test ./apps/edge/internal/service ./apps/edge/internal/openai -run 'TestSingleRequest(Binding|PresetBinding|ProviderStage|PlanStage)' -count=1`

Expected: admission and S08 integration fixtures pass freshly.

```text
ok  	iop/apps/edge/internal/service	0.029s
ok  	iop/apps/edge/internal/openai	0.048s
```

### 7. Vet

`go vet ./apps/edge/internal/service ./apps/edge/internal/openai`

Expected: both packages vet cleanly.

```text
```

### 8. Edge regression

`go test ./apps/edge/... -count=1`

Expected: all Edge packages pass freshly.

```text
ok  	iop/apps/edge/cmd/edge	0.080s
ok  	iop/apps/edge/internal/authprojection	0.016s
ok  	iop/apps/edge/internal/bootstrap	0.458s
ok  	iop/apps/edge/internal/configrefresh	0.112s
ok  	iop/apps/edge/internal/controlplane	6.620s
ok  	iop/apps/edge/internal/edgecmd	0.110s
ok  	iop/apps/edge/internal/edgevalidate	0.066s
ok  	iop/apps/edge/internal/events	0.040s
ok  	iop/apps/edge/internal/input	0.097s
ok  	iop/apps/edge/internal/input/a2a	0.084s
ok  	iop/apps/edge/internal/node	0.097s
ok  	iop/apps/edge/internal/openai	8.067s
ok  	iop/apps/edge/internal/opsconsole	0.087s
ok  	iop/apps/edge/internal/service	6.504s
ok  	iop/apps/edge/internal/transport	4.800s
```

### 9. No incomplete production activation

`test ! -e apps/edge/internal/openai/single_request_executor.go && bash -c 'set -euo pipefail; if rg --sort path -n "NewSingleRequestExecutor|SetSingleRequestExecutor" apps/edge/internal/input/manager.go; then exit 1; else test $? -eq 1; fi'`

Expected: exit zero with no output.

```text
```

### 10. Spec synchronization

`rg --sort path -n 'Plan stage|plan\.md|not installed|deferred' agent-spec/runtime/edge-node-execution.md`

Expected: output limits the component to implemented-but-not-installed Plan behavior and deferred later stages/S12.

```text
83:    notes: Private fixed Plan stage runner, strict result decoding, and PLAN artifact write
185:| single-request observation evidence | Stage-pure timing, tool/cleanup/total counts, cardinality-bounded labels, Node logs, and raw-free correlation are documented for the single-request path. `iop_anthropic_single_request_ingress_total` is strictly unlabeled. Internal tool names, raw arguments, and private results are absent from public output and log projections. Actual Claude/Mac timing evidence is explicitly deferred to `claude-smoke` (SDD S12); deterministic coordinator/tool-loop tests do not imply external qualification. |
190:| request-owned internal artifacts | `SingleRequestController` exposes closed plan/review read/write operations. Artifact calls and model workspace tools share one serialized lazy `WorkspaceOpen`, the exact admitted Node generation, the active stage deadline, the immutable output bound, in-flight work accounting, and one terminal cleanup. Node alone maps selectors to `plan.md` and `review.md`, and inventoried descriptor-relative reads fail closed on identity replacement. |
191:| Plan stage | A private, not installed Plan runner emits the `planning` envelope, sends the immutable task through the frozen Gemini Chat binding with `reasoning_effort=high`, requires one strict small `plan`/`verification` JSON result, and writes deterministic bounded Markdown through `SingleRequestArtifactPlan`. |
213:- Request-owned plan and review artifact access is implemented between the controller and the same dedicated workspace wire. Only `SingleRequestArtifactPlan` and `SingleRequestArtifactReview` are accepted. Artifact and model-tool callers share one serialized open attempt and the same opened cleanup gate; terminal and cancellation paths wait for in-flight artifact work before issuing exactly one cleanup. Edge bounds writes before dispatch and reads before acceptance, validates the echoed kind/operation and canonical terminal, and never reselects after a generation mismatch. Node maps the closed selectors to `plan.md` and `review.md`, validates the inventoried parent/file identity with descriptor-relative no-follow reads, and never grants the public workspace tool surface access to `.iop`.
214:- The private Plan stage is implemented but not installed in an outer executor. Its provider codec accepts only frozen Plan options and selected dispatch facts, uses the admitted stage deadline and exact output limit, accepts only `RESPONSE_START`, zero or more `BODY`, then `END`, and projects all provider failures to a generic internal failure. The fixed Plan prompt requests a small plan plus verification criteria and writes only the closed PLAN artifact.
223:Single-request lifecycle observation evidence links ingress=1, request-total=1, terminal=1, stage/tool/cleanup counts, and raw-free correlation for one real POST. `iop_anthropic_single_request_ingress_total` is strictly unlabeled (no request_id, stage_id, provider identity, content, or workspace reference). Internal tool names, raw arguments, and private results are absent from public output and log projections. Stage-pure timing, cardinality-bounded labels, and privacy semantics are documented here. Actual Claude/Mac timing evidence is explicitly deferred to `claude-smoke` (SDD S12); deterministic coordinator/tool-loop tests do not imply external qualification.
242:            Node->>Node: map selector to plan.md or review.md and validate inventory
301:- `go test -count=1 ./apps/edge/internal/openai -run 'TestSingleRequest(ProviderStage|PlanStage)'` — deterministic frozen provider codec and Plan stage evidence, including high reasoning, ordered tunnel frames, strict JSON, planning envelope, and `plan.md` artifact selection.
313:- The private Plan stage is implemented but not installed as a composite executor. Work, Review/repair, generic error/cancel integration, outer activation, and actual Claude/Mac qualification remain deferred; this deterministic component does not establish S12 evidence.
314:- Single-request observation evidence (ingress=1, request-total=1, terminal=1, stage/tool/cleanup counts, raw-free correlation) is documented and tested. `iop_anthropic_single_request_ingress_total` is strictly unlabeled. Internal tool names, raw arguments, and private results are absent from public output and log projections. Actual Claude/Mac timing evidence is explicitly deferred to `claude-smoke` (SDD S12); deterministic coordinator/tool-loop tests do not imply external qualification.
326:- 2026-08-06: Added implemented Edge workspace admission for single-request execution: an opaque `workspace_ref` binds to one configured ready Node generation and a closed capability projection before executor startup. Admission rejects unavailable, foreign, pending, malformed, and stale candidates without fallback or reselection; Node-private wire, executor, filesystem path, and symlink enforcement remain deferred.
327:- 2026-08-06: Added the dedicated Edge-Node workspace wire. `NodeConfigPayload` now delivers the approved catalog; `WorkspaceOpen`/`Tool`/`Cancel`/`Cleanup` messages have closed typed outcomes, immutable coordinator identities, parser registration, and an optional Node handler. Edge dispatch is generation-fenced and context cancellation sends one typed cancel. Node filesystem and process execution are intentionally deferred.
328:- 2026-08-06: Completed the reviewed workspace file boundary repair. Edge now sends only frozen request authority, Node admits immutable catalog subsets/lower limits, and structured write reaches the file executor while legacy incomplete input remains rejected. The Go 1.24-compatible descriptor-relative no-follow write path validates before effects, bounded list processing retains fixed state, startup errors are path-free, and composition proves handler-before-ready plus workspace-before-session/store teardown. Command execution/cancellation and cleanup remain deferred.
329:- 2026-08-07: Implemented exact-template workspace COMMAND and typed cancellation. The Node uses an inherited-root `fchdir`/`exec` shim, minimal allowlisted environment, a shared draining stdout/stderr cap, and one process-group result owner across exit, timeout, context cancel, and exact request/tool cancel. Focused race tests cover non-zero exit, output overflow, descendant termination, cross-request isolation, and configured-root rename/replacement. Artifact cleanup remains deferred.
332:- 2026-08-07: Added the closed request-owned PLAN/REVIEW artifact wire and controller lifecycle. Artifact calls share the model-tool lazy open and terminal cleanup gate, use the frozen Node generation and immutable bounds, and map only inside Node to inventoried `plan.md`/`review.md` files. Provider-specific stage drivers and actual Claude qualification remain deferred.
333:- 2026-08-08: Synchronized single-request lifecycle observation evidence: stage-pure timing (planning/working/reviewing/repairing/finalizing/completed/failed/cancelled), tool/cleanup/total counts, cardinality-bounded labels, Node logs, and raw-free correlation. External Claude/Mac timing evidence is explicitly deferred to `claude-smoke` (SDD S12). Deterministic coordinator/tool-loop tests cover the full single-request path without implying external qualification.
334:- 2026-08-07: Added the private Plan stage and its fail-closed provider codec. The component uses only frozen Gemini dispatch/options, ordered bounded tunnel decoding, strict small plan/verification JSON, and the closed `SingleRequestArtifactPlan` write. It is not installed; Work, Review/repair, activation, and S12 qualification remain deferred.
```

### 11. Formatting

`gofmt -d apps/edge/internal/openai/single_request_provider_stage.go apps/edge/internal/openai/single_request_provider_stage_test.go apps/edge/internal/openai/single_request_plan_stage.go apps/edge/internal/openai/single_request_plan_stage_test.go`

Expected: exit zero with no output.

```text
```

### 12. Diff hygiene

`git diff --check`

Expected: exit zero with no output.

```text
```

---

> **[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 — case-folded JSON member names can target the same Go struct field and retain last-value-wins behavior at both the provider response and Plan result boundaries.
  - Completeness: Fail — exact duplicate-key cases are covered, but the strict owned-object contract still accepts non-canonical case variants and case-folded duplicate aliases.
  - Test coverage: Fail — all recorded suites and the focused race test pass, while a fresh reviewer reproducer proves `plan` plus `Plan` and `content` plus `Content` are accepted.
  - API contract: Fail — SDD S08 and the current plan require a strict small `plan`/`verification` result before the internal PLAN write; case-insensitive struct matching leaves that boundary ambiguous.
  - Code quality: Pass — the owned files are formatted, vet-clean, contain no debug residue, and preserve the private uninstalled component boundary.
  - Implementation deviation: Fail — the implementation claims duplicate-key-aware strict decoding, but its raw-key validator and later case-insensitive typed decoder enforce different key identities.
  - Verification trust: Fail — the twelve recorded commands pass freshly, but their checked strict-JSON claim is contradicted by the reviewer reproducer; `evidence_integrity_failure=true`.
  - Spec conformance: Fail — the living spec calls the Plan result strict, while case-mutated names can still overwrite the same typed fields and reach artifact rendering.
- **Findings**:
  - **Required R1** — `apps/edge/internal/openai/single_request_provider_stage.go:210`, `apps/edge/internal/openai/single_request_provider_stage.go:269`, and `apps/edge/internal/openai/single_request_plan_stage.go:67`: `validateJSONValue` treats raw JSON names case-sensitively, but `encoding/json` matches tagged struct fields case-insensitively. A fresh focused reproducer showed that `{"plan":"first","Plan":"second","verification":"check"}` renders successfully and a provider message containing both `"content"` and `"Content"` is accepted with the latter value. Enforce exact canonical field names for every owned provider response/choice/message and Plan object before typed decoding, reject case-mutated names and case-folded aliases, and add deterministic regressions proving generic rejection, tunnel close, and zero artifact persistence.
- **Routing Signals**: `review_rework_count=4`, `evidence_integrity_failure=true`
- **Next Step**: Invoke the plan skill in `prepare-follow-up` mode with Required R1, archive this pair only after the routed follow-up is fully prepared, then materialize the new active PLAN/CODE_REVIEW pair without writing `complete.log`.
