<!-- task=m-node-provider-execution-liveness-recovery/10+09_stall_recovery plan=8 tag=REVIEW_API milestone-task=bounded-retry -->

# 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-05
task=m-node-provider-execution-liveness-recovery/10+09_stall_recovery, plan=8, tag=REVIEW_API

## Archive Evidence Snapshot

- The reviewed plan=7 pair is archived in this task directory as `plan_cloud_G10_7.log` and `code_review_cloud_G10_7.log` with verdict `FAIL`.
- Required R1: `newOpenAIResponsesRecoveryAdmissionBuilder` constructs a normalized dispatch context before replacement path selection, and `newResponsesDispatchContext` rejects the exact `stream=true` replay body even when the next candidate is another tunnel.
- Required R2: `TestOpenAIStallRecoveryMatrix` hard-codes `stream=false` for every recovery product and therefore does not cover the supported Responses streaming-tunnel recovery path.
- All eleven declared verification commands passed on fresh reviewer reruns. A focused production-handler probe failed for semantic false and true with one dispatch and `recovery_failed`. Routing signals are `review_rework_count=5` and `evidence_integrity_failure=true`.

## 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-G10.md` → `code_review_cloud_G10_8.log` and `PLAN-cloud-G09.md` → `plan_cloud_G09_8.log`.
3. If PASS, write `complete.log` and move active task directory to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/10+09_stall_recovery/`. If WARN/FAIL, fully write the next filesystem state required by the code-review skill.
4. If PASS and task group is `m-node-provider-execution-liveness-recovery`, 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 Defer Responses normalized validation until candidate selection | [x] |
| REVIEW_API-2 Add the missing Responses streaming-tunnel recovery product | [x] |

## Implementation Checklist

- [x] REVIEW_API-1 makes Responses exact replay candidate-dependent: tunnel replacements retain `stream=true`, normalized replacements perform the existing strict validation only in `PrepareRun`, and every admitted attempt binds the matching request context.
- [x] REVIEW_API-2 extends the production stall matrix with semantic-false and semantic-true Responses streaming-tunnel recovery rows that prove replacement identity, provider avoidance, shared budget, close/cancel behavior, sanitized output, and exactly one successful SSE terminal.
- [x] Run and record every exact final verification command separately after both implementation items are complete.
- [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_G10_8.log`.
- [x] Archive active `PLAN-*-G??.md` to `plan_cloud_G09_8.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-node-provider-execution-liveness-recovery/10+09_stall_recovery/` to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/10+09_stall_recovery/` and update this checklist at the final archive path.
- [ ] If PASS and task group is `m-node-provider-execution-liveness-recovery`, 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-node-provider-execution-liveness-recovery/` 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

- Public Responses replay bodies are decoded into a tunnel-capable attempt context before provider-pool selection. The raw rebuilt body remains the tunnel body source, so `stream=true` and provider extension fields survive replacement admission.
- Strict public Responses decoding and `newResponsesDispatchContext` construction occur inside `PrepareRun` only for a selected normalized candidate. Direct normalized recovery and private continuation handling retain their previous validation and construction paths.
- Attempt state is bound to the tunnel-capable context before admission and rebound to the strict normalized context from `PrepareRun`, ensuring the event-source factory always observes the context matching the selected replacement path.
- The recovery matrix now enumerates supported endpoint/path/stream/semantic products explicitly. The two Responses streaming-tunnel rows use a deterministic Responses SSE success fixture and assert the rewritten model/body, request metadata, provider avoidance, bounded two-dispatch lifecycle, zero duplicate cancellation, exactly-once transport close, sanitized output, one `response.completed`, and one `[DONE]`.

## Reviewer Checkpoints

- Exact public Responses replay is decoded without normalized-only validation before provider-path selection.
- A replacement tunnel preserves `stream=true`, rebuilt body/model, request metadata, and the attempt context used by the tunnel event source.
- A replacement normalized path still calls `newResponsesDispatchContext` inside `PrepareRun` and rejects unsupported public streaming without weakening the normalized API contract.
- Private continuation and direct normalized recovery behavior remain unchanged.
- The production matrix contains semantic-false and semantic-true `recover/responses/provider_tunnel/stream=true` rows and asserts two dispatches, failed-provider avoidance, distinct identity, shared budget, exactly-once close, no duplicate cancel, no raw leakage, one `response.completed`, and one `[DONE]`.
- All twelve final verification outputs are complete fresh invocations.

## Verification Results

Paste complete stdout/stderr for each exact command. Do not summarize or combine command results.

### Verification 1

Command:

```bash
go test -count=10 ./apps/edge/internal/openai -run 'TestOpenAIStallRecoveryMatrix/recover/responses/provider_tunnel/stream=true'
```

Output:

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

### Verification 2

Command:

```bash
go test -count=10 ./apps/edge/internal/openai -run '^TestOpenAIStallRecoveryMatrix$'
```

Output:

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

### Verification 3

Command:

```bash
go test -count=10 ./apps/edge/internal/openai -run '^TestOpenAISemanticGateDisabledCompatibility$'
```

Output:

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

### Verification 4

Command:

```bash
go test -count=20 ./apps/edge/internal/openai -run '^(TestOpenAIStallRecoveryFilter|TestOpenAIAttemptControllerConfirmedStall|TestOpenAIAttemptDispatcherStalledProvider)$'
```

Output:

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

### Verification 5

Command:

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

Output:

```text
ok  	iop/packages/go/streamgate	0.925s
ok  	iop/apps/edge/internal/openai	7.479s
ok  	iop/apps/edge/internal/service	6.006s
ok  	iop/apps/edge/internal/controlplane	6.618s
```

### Verification 6

Command:

```bash
go test -race -count=3 ./apps/edge/internal/service
```

Output:

```text
ok  	iop/apps/edge/internal/service	19.138s
```

### Verification 7

Command:

```bash
go test -race -count=3 ./apps/edge/internal/openai
```

Output:

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

### Verification 8

Command:

```bash
go vet ./packages/go/streamgate ./apps/edge/internal/service ./apps/edge/internal/openai ./apps/edge/internal/controlplane
```

Output:

```text
No stdout or stderr.
```

### Verification 9

Command:

```bash
./scripts/e2e-smoke.sh
```

Output:

```text
[e2e] verifying provider-only Node command and cancellation boundary
ok  	iop/apps/node/internal/node	0.040s
[e2e] verifying Edge dispatch, provider tunnel, queue, and reconnect fencing
ok  	iop/apps/edge/internal/service	4.433s
ok  	iop/apps/edge/internal/transport	0.252s
[e2e] provider-only Edge-Node smoke PASSED
```

### Verification 10

Command:

```bash
IOP_VLLM_MODE=fake ./scripts/e2e-openai-vllm.sh
```

Output:

```text
[openai-vllm] OpenAI-compatible vLLM serving test PASSED (mode=fake).
```

### Verification 11

Command:

```bash
./scripts/e2e-provider-capacity-smoke.sh
```

Output:

```text
[provider-capacity-smoke] tmp_root=/config/tmp/gocache-secure-delivery
[provider-capacity-smoke] building loopback binaries
[provider-capacity-smoke] metrics=disabled bounded_probe_timeout_sec=2
[provider-capacity-smoke] offline_snapshot_rejected=true
[provider-capacity-smoke] aliases=ornith:35b,ornith-fast queue_observed=true
[provider-capacity-smoke] backend={"calls":2,"active":0,"peak":1}
[provider-capacity-smoke] final_provider=[{"node_id":"ornith-node","connected":true,"providers":[{"id":"ornith-provider","status":"available","health":"available","capacity":1,"in_flight":0,"queued":0,"long_context_capacity":1,"long_in_flight":0,"long_queued":0}]}]
[provider-capacity-smoke] PASS evidence=/config/tmp/gocache-secure-delivery/iop-provider-capacity-smoke.qMvA9l
```

### Verification 12

Command:

```bash
git diff --check
```

Output:

```text
No stdout or stderr.
```

---

> **[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 — a Responses recovery that switches from an initial provider tunnel to a normalized replacement drops the ingress `TimeoutSec`, `MaxQueue`, and `QueueTimeoutMS` values from the selected run request.
  - Completeness: Fail — candidate-dependent context binding is incomplete because the normalized replacement receives only prompt, input, metadata, token estimate, and context class.
  - Test Coverage: Fail — every recovery-matrix success attempt uses the same execution path as its failed attempt, so the tunnel-to-normalized transition that exposes the loss is not exercised.
  - API Contract: Fail — the configured request timeout can silently change from the ingress value to the service default when recovery selects the normalized path.
  - Code Quality: Pass — the candidate-dependent decoding change is cohesive, and no independent formatting, dead-code, or debug-output defect was found.
  - Implementation Deviation: Fail — the plan requires every admitted replacement to bind the complete matching request context, but the normalized overlay is partial.
  - Verification Trust: Fail — all twelve declared commands pass on fresh reviewer reruns, but a focused production-handler probe contradicts the claim that every admitted attempt preserves the matching request context.
  - Spec Conformance: Fail — SDD S05 requires request-local bounded recovery under the original timeout/cancellation boundary; the replacement run can instead inherit a zero timeout and be normalized to a different service default.
- Findings:
  - Required R1 — `apps/edge/internal/openai/responses_stream_gate.go:1079`: the recovery `PrepareRun` overlay omits `TimeoutSec`, `MaxQueue`, and `QueueTimeoutMS`, although `newResponsesDispatchContext` populates them at `apps/edge/internal/openai/responses_handler.go:283` and the initial Responses provider-pool `PrepareRun` copies them at lines 439-441. For an initial tunnel with ingress timeout 5 followed by a normalized replacement, the scripted production-handler path records `TimeoutSec=0`; the service then substitutes its default timeout instead of preserving the request-local value. Copy all request-owned normalized execution fields from `attemptDC.submitReq`, matching the initial Responses preparation path, and add deterministic production-handler recovery rows whose failed and successful attempts use different provider paths. At minimum, assert tunnel-to-normalized preservation of timeout/queue context together with provider avoidance, distinct attempt identity, bounded dispatch, exactly-once close, sanitized output, and one terminal; cover the reverse path where it verifies candidate-specific tunnel context without duplicating existing same-path rows.
- Reviewer Verification:
  - All twelve declared verification commands passed on fresh reviewer reruns, including package tests, race runs, vet, Edge-Node smoke, fake-vLLM full-cycle smoke, provider-capacity smoke, and `git diff --check`.
  - Focused temporary regression probe: `go test -count=1 ./apps/edge/internal/openai -run '^TestReviewProbeResponsesTunnelToNormalizedRecoveryContext$'` failed with `replacement TimeoutSec=0, want ingress timeout 5`; the temporary probe file was removed after diagnosis.
- Routing Signals:
  - `review_rework_count=6`
  - `evidence_integrity_failure=true`
- Next Step: Archive this reviewed pair and create a freshly routed follow-up PLAN/CODE_REVIEW pair that directly resolves Required R1.
