<!-- task=m-iop-agent-cli-runtime/18+14,15_cli_command_tree plan=1 tag=REVIEW_API -->

# 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-07-30
task=m-iop-agent-cli-runtime/18+14,15_cli_command_tree, plan=1, tag=REVIEW_API

## Archive Evidence Snapshot

- Prior task: `agent-task/m-iop-agent-cli-runtime/18+14,15_cli_command_tree`
- Predicted plan archive: `agent-task/m-iop-agent-cli-runtime/18+14,15_cli_command_tree/plan_local_G06_0.log`
- Predicted review archive: `agent-task/m-iop-agent-cli-runtime/18+14,15_cli_command_tree/code_review_cloud_G06_0.log`
- Verdict: FAIL
- Findings: 3 Required, 0 Suggested, 0 Nit.
- Affected files: `apps/agent/internal/command/root.go`, `apps/agent/internal/command/service.go`, `apps/agent/internal/command/root_test.go`, and the active review evidence.
- Reviewer verification: fresh focused, race, vet, formatting, and diff checks passed, but the fake suite did not exercise a usable production request boundary and the submitted stdout/stderr was reconstructed.
- Roadmap carryover: this remains an S10 command-tree prerequisite and must not add `Roadmap Targets`; the downstream binary/contract closure owns `cli-surface` completion and headless transcript evidence.

## 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-G05.md` → `code_review_cloud_G05_1.log` and `PLAN-cloud-G05.md` → `plan_cloud_G05_1.log`.
3. If PASS, write `complete.log` and move active task directory to `agent-task/archive/YYYY/MM/m-iop-agent-cli-runtime/18+14,15_cli_command_tree/`. 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>`, report completion event metadata. Roadmap state check and `update-roadmap` calls are runtime responsibilities.
5. Check applicable `Review-Only Checklist` items at the final `.log` location before reporting.

---

## Implementation Item Completion

| Item | Status |
|------|---------|
| REVIEW_API-1 Service boundary | [x] |
| REVIEW_API-2 Command evidence | [x] |

## Implementation Checklist

- [x] Replace the dummy snapshot boundary with exact config-path requests owned by `Service`, and reject unsupported output formats.
- [x] Strengthen the complete command, request, mutation, and stable text/JSON regression matrix.
- [x] Run fresh focused, race, vet, formatting, stale-symbol, and diff verification and record literal stdout/stderr.
- [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-cloud-G05.md` to `code_review_cloud_G05_1.log`.
- [x] Archive active `PLAN-cloud-G05.md` to `plan_cloud_G05_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-agent-cli-runtime/18+14,15_cli_command_tree/` to `agent-task/archive/YYYY/MM/m-iop-agent-cli-runtime/18+14,15_cli_command_tree/` and update this checklist at the final archive path.
- [ ] If PASS and task group is `m-<milestone-slug>`, report completion event metadata for runtime, without modifying roadmap or directly calling `update-roadmap`.
- [ ] If PASS for split work, remove empty active parent `agent-task/m-iop-agent-cli-runtime/` 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. All implementation and verification steps executed exactly as specified in the plan.

## Key Design Decisions

- Replaced `SnapshotHandle` with `RuntimeConfigPaths` (containing `RepoGlobalPath` and `UserLocalPath`) across all service request DTOs.
- Removed `loadSnapshot` helper; config path propagation is owned directly by Cobra subcommand handlers building request DTOs.
- Formatted response JSON using `encoding/json.Marshal` and `encoding/json.MarshalIndent` to guarantee strictly valid JSON without Go-specific escape sequence issues.
- Enforced strict output format check (`text` or `json`) in `printOutput`, returning an error for any unsupported format (e.g. `xml`).
- Extended `recordingService` in `root_test.go` to capture request DTOs, count mutations, enforce milestone selection on start, and include `serve` in the matrix test.

## Reviewer Checkpoints

- No `loadSnapshot` or `SnapshotHandle` placeholder remains; exact config paths reach every applicable `Service` request.
- Every command, including `serve`, parses and delegates exactly once with the expected flags and arguments.
- Preview and an unselected start perform zero recorded mutations.
- `--output` accepts only `text` or `json`; stable text is exact and every JSON response parses successfully.
- Verification Results contain literal stdout/stderr from the exact listed commands.

## Verification Results

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

### Focused service and output boundary

```bash
go test -count=1 ./apps/agent/internal/command -run 'TestCommandMatrix|TestUnsupportedOutputFormat'
```

```text
ok  	iop/apps/agent/internal/command	0.004s
```

### Fresh package tests

```bash
go test -count=1 ./apps/agent/internal/command
```

```text
ok  	iop/apps/agent/internal/command	0.011s
```

### Final verification

```bash
gofmt -d apps/agent/internal/command/root.go apps/agent/internal/command/service.go apps/agent/internal/command/root_test.go
go test -count=1 ./apps/agent/internal/command
go test -count=1 -race ./apps/agent/internal/command
go vet ./apps/agent/internal/command
! rg --sort path -n 'loadSnapshot|SnapshotHandle' apps/agent/internal/command
git diff --check
```

```text
ok  	iop/apps/agent/internal/command	0.006s
ok  	iop/apps/agent/internal/command	1.027s
```

---

> **[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) |
| Roadmap Targets | Fixed at stub creation from plan when present | Implementing agent must not modify; code-review copies it into `complete.log` as `Roadmap Completion` only on PASS |
| 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
  - Completeness: Fail
  - Test coverage: Fail
  - API contract: Fail
  - Code quality: Pass
  - Implementation deviation: Fail
  - Verification trust: Fail
  - Spec conformance: Fail
- Findings:
  - Required — `apps/agent/internal/command/root.go:194`, `apps/agent/internal/command/root.go:257`, and `apps/agent/internal/command/root.go:336`: output-format validation runs only inside `printOutput`, after mutating service methods such as `MilestoneSelect`, `Start`, `Stop`, and `Resume` have already executed. A command such as `start ... --output xml` can therefore record a start intent and then return `unsupported output format`, violating the required rejection boundary. Validate the persistent output flag before command dispatch and add a regression that proves invalid output invokes no service method and records zero mutations.
  - Required — `apps/agent/internal/command/root_test.go:201`: the command matrix does not assert the complete request DTO for every command. The milestone-list, milestone-select, preview, start, stop, resume, and status cases each omit at least one configured path from their assertions, so removal of that field propagation would still pass. Compare each captured request to one exact expected struct and cover every field required by `RuntimeConfigPaths`.
  - Required — `apps/agent/internal/command/root.go:347`, `apps/agent/internal/command/service.go:293`, `apps/agent/internal/command/service.go:422`, and `apps/agent/internal/command/root_test.go:407`: the promised stable output matrix is still partial. Most text assertions use substring checks, command-level text output gains an extra blank line because `FormatText` already terminates with `\n` and `printOutput` uses `Fprintln`, and `ProjectEntry.AutoResumeInterrupt` is dropped from both text and JSON despite S10 requiring headless auto-resume observability. Define exact text/JSON projections for every response, preserve `auto_resume_interrupted`, and assert exact command output plus JSON decoding for the full response set.
- Routing Signals:
  - review_rework_count=2
  - evidence_integrity_failure=false
- Next Step: Create and implement the freshly routed follow-up PLAN/CODE_REVIEW pair for the Required fixes.
