<!-- task=m-iop-owned-single-request-agent-execution/14+05,12,13_observation_timing plan=3 tag=REVIEW_API milestone-task=cleanup-observation -->

# Code Review Reference - REVIEW_API

> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is the mandatory final step of implementation.**
> Complete the `Implementation Checklist`, fill implementation-owned evidence, keep active files in place, and leave finalization to the review agent.

## Overview

date=2026-08-07
task=m-iop-owned-single-request-agent-execution/14+05,12,13_observation_timing, plan=3, tag=REVIEW_API

## Archive Evidence Snapshot

- Closed pair: `agent-task/m-iop-owned-single-request-agent-execution/14+05,12,13_observation_timing/plan_cloud_G07_2.log` and `agent-task/m-iop-owned-single-request-agent-execution/14+05,12,13_observation_timing/code_review_cloud_G07_2.log`.
- Verdict: FAIL with Required R4-R6, zero Suggested findings, and zero Nit findings.
- Affected behavior: in-flight tool/stage settlement, terminal error classification, and request-local linkage of raw-free lifecycle timing/outcomes.
- Fresh reviewer verification: packet 05/12/13 checks passed; focused race passed in 1.067s; the service package passed in 6.244s; vet and `git diff --check` passed. A temporary real-service tool-failure reproducer was removed after reporting `ToolCount=0`, terminal `ErrorClass="provider"`, and correlations `stage="single_request.stage.plan" tool="" terminal=""`.
- Roadmap carryover: `milestone-task=cleanup-observation`; approved SDD scenario S07 requires linked raw-free stage/tool/cleanup/total timing and outcomes.

## For the Review Agent

The review agent compares every item against source and verifies the recorded command output.

## Implementation Item Completion

| Item | Status |
|------|---------|
| REVIEW_API-1 Close tool/stage ordering and terminal error ownership | [x] |
| REVIEW_API-2 Generate one raw-free correlation per request accumulator | [x] |

## Implementation Checklist

- [x] Settle each in-flight Node tool before its semantic stage emits on error/cancel, count it once, and resume stage timing only while the same stage remains active.
- [x] Preserve exact terminal error classes with typed sentinel classification before cleanup joins secondary failures.
- [x] Attach one bounded raw-input-independent request correlation to every request/stage/tool/cleanup/terminal DTO and prove within-request equality plus cross-request separation.
- [x] Run dependency, focused race, package, vet, and whitespace verification with cache bypass where supported.
- [x] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.

## Review-Only Checklist

> **[REVIEW AGENT ONLY]** Implementing agents must not modify 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_G07_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 to `agent-task/archive/YYYY/MM/m-iop-owned-single-request-agent-execution/14+05,12,13_observation_timing/` and update this checklist there.
- [ ] If PASS and task group is `m-iop-owned-single-request-agent-execution`, preserve and report `milestone-task` metadata without modifying roadmap.
- [ ] If PASS for split work, remove the empty active parent or verify it remains due to siblings.
- [x] If WARN/FAIL, write the next filesystem state matching the verdict and do not write `complete.log`.

## Deviations from Plan

None.

## Key Design Decisions

- The accumulator retains a pending semantic-stage close while a Node tool is in flight. Tool emission and stage `ToolCount` settlement occur together when that tool exits; only a still-active stage resumes its provider timer.
- The handle captures the primary terminal error class using `errors.Is` when failure or cancellation wins. Cleanup changes the class to `workspace_cleanup` only when it converts an otherwise successful finalizing request.
- Each accumulator creates one `sr-<hex>` correlation from random bytes, with a process-local atomic fallback. The value is bounded, raw-input-independent, and copied to every lifecycle DTO.

## Reviewer Checkpoints

- Confirm an in-flight tool emits exactly once before its deferred error/cancel stage close, contributes `ToolCount=1`, and cannot restart timing after the stage is terminal.
- Confirm the primary typed terminal class survives cleanup error joining and cleanup converts only pending success to `workspace_cleanup`.
- Confirm every request/stage/tool/cleanup/terminal DTO from one accumulator has the same non-empty bounded correlation, while separate requests differ and no caller id/sentinel is embedded.
- Confirm observer error/panic remains isolated and no API, wire, config, adapter, or external-smoke scope was added.

## Verification Results

### 1. Packet 05 dependency

```text
(no output; exit 0)
```

### 2. Packet 12 dependency

```text
(no output; exit 0)
```

### 3. Packet 13 dependency

```text
(no output; exit 0)
```

### 4. Focused race test

`go test -race ./apps/edge/internal/service -run 'TestSingleRequestObservation' -count=1`

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

### 5. Package regression

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

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

### 6. Vet

```text
(no output; exit 0)
```

### 7. Whitespace

```text
(no output; exit 0)
```

## Section Ownership

| Section | Owner | Note |
|---------|-------|------|
| Header, Overview, Review Agent Instructions | Fixed | Implementation does not modify finalization fields. |
| Implementation completion/checklist | Implementing agent | Items were checked after implementation and verification. |
| Review-Only Checklist and Code Review Result | Review agent | Finalization ownership. |
| Deviations, design decisions, verification output | Implementing agent | Actual implementation evidence. |

## Code Review Result

- Overall Verdict: FAIL
- Dimension Assessment:
  - Correctness: Fail
  - Completeness: Fail
  - Test Coverage: Fail
  - API Contract: Pass
  - Code Quality: Pass
  - Implementation Deviation: Fail
  - Verification Trust: Fail
  - Spec Conformance: Fail
- Findings:
  - Required R7 — `apps/edge/internal/service/single_request.go:223`, `apps/edge/internal/service/single_request.go:632`, `apps/edge/internal/service/single_request_tool_loop.go:232`, `apps/edge/internal/service/single_request_tool_loop.go:250`, and `apps/edge/internal/service/single_request_tool_loop.go:315` collapse actual request, stage, and in-flight tool deadlines into `ErrSingleRequestInternalToolBudget`, so the captured terminal class is `internal_tool_budget` instead of the plan-required `timeout`. The active test only checks `singleRequestErrorClassFromErr(context.DeadlineExceeded)` directly and does not exercise any production deadline owner. A fresh service-lifecycle reproducer failed with `terminal error class="internal_tool_budget", want "timeout"`; the temporary probe was removed. Preserve the existing budget class for iteration/output exhaustion, classify real deadline winners as `timeout` before cleanup joins secondary errors, and add lifecycle regressions for request wall-clock, stage timer, and in-flight tool deadlines.
- Routing Signals:
  - `review_rework_count=3`
  - `evidence_integrity_failure=true`
- Next Step: Create and implement the routed follow-up plan for R7 under the same task path; no user-review gate applies.
