<!-- task=m-iop-agent-cli-runtime/22+16_local_control plan=2 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/22+16_local_control, plan=2, tag=REVIEW_API

## Roadmap Targets

- Milestone: `agent-roadmap/phase/automation-runtime-bridge/milestones/iop-agent-cli-runtime.md`
- Milestone link: [Milestone 문서](agent-roadmap/phase/automation-runtime-bridge/milestones/iop-agent-cli-runtime.md)
- Task ids:
  - `local-control`: daemon-owned same-user local proto-socket lifecycle, state, events, and control endpoints
- Completion mode: check-on-pass

## Archive Evidence Snapshot

- Prior plan: `agent-task/m-iop-agent-cli-runtime/22+16_local_control/plan_cloud_G08_1.log`
- Prior review: `agent-task/m-iop-agent-cli-runtime/22+16_local_control/code_review_cloud_G08_1.log`
- Verdict: `FAIL`; Required `2`, Suggested `0`, Nit `0`.
- Findings: `Ledger.BeginCommand` still returns a matching incomplete response without checking `durableCommand.Complete`; `Service.handleProjectMutation` returns that provisional `"accepted"` response; the blocking-concurrency and accepted-but-incomplete restart regressions are absent; all implementation and verification fields in the review artifact were left unfilled.
- Affected files: `apps/agent/internal/localcontrol/ledger.go`, `apps/agent/internal/localcontrol/service.go`, `apps/agent/internal/localcontrol/ledger_test.go`, `apps/agent/internal/localcontrol/service_test.go`, and the active review artifact.
- Verification evidence: `go test -list` found only the completed-restart, conflict, and response-kind-only concurrency tests. The focused plan pattern passed without running an accepted-but-incomplete regression, so it is not completion evidence.
- Roadmap carryover: approved SDD scenario S11 and Milestone task `local-control` remain incomplete until durable command convergence and fresh final verification are proven.

## 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-G07.md` → `code_review_cloud_G07_2.log` and `PLAN-cloud-G07.md` → `plan_cloud_G07_2.log`.
3. If PASS, write `complete.log` and move active task directory to `agent-task/archive/YYYY/MM/m-iop-agent-cli-runtime/22+16_local_control/`. 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 Durable ownership and replay | [x] |
| REVIEW_API-2 Concurrency and recovery regressions | [x] |
| REVIEW_API-3 S11 evidence | [x] |

## Implementation Checklist

- [x] Distinguish new/recovery ownership, incomplete waiting, and completed replay so no incomplete durable response is exposed and every identical request converges on one stored final result.
- [x] Add deterministic blocking-concurrency and accepted-but-incomplete restart regressions that prove final response, event, revision/cursor, conflict, cancellation, and effective-mutation behavior.
- [x] Run fresh focused, race, contract, protobuf-determinism, Darwin cross-build, shared-package, and whole-Go verification on stable source.
- [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_G07_2.log`.
- [x] Archive active `PLAN-*-G??.md` to `plan_cloud_G07_2.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`.
- [x] If PASS, write `complete.log` based on `agent-ops/skills/common/code-review/templates/complete-log-template.md` and leave no active `.md` files.
- [x] If PASS, move active task directory `agent-task/m-iop-agent-cli-runtime/22+16_local_control/` to `agent-task/archive/YYYY/MM/m-iop-agent-cli-runtime/22+16_local_control/` and update this checklist at the final archive path.
- [x] If PASS and task group is `m-<milestone-slug>`, report completion event metadata for runtime, without modifying roadmap or directly calling `update-roadmap`.
- [x] 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.
- [ ] If WARN/FAIL, write the next filesystem state matching code-review verdict and do not write `complete.log`.

## Deviations from Plan

The unchanged `go test -count=1 ./packages/go/...` and `go test -count=1 ./...` commands were run in persistent local terminal sessions because the normal command collection window ends before the `agentprovider/cli` package completes. Both commands completed with exit code 0; no verification command or source scope was omitted.

## Key Design Decisions

- `Ledger.BeginCommand` now classifies a matching durable record as completed replay, same-process waiter, or recovery owner. Only a complete record is decoded and returned to a client.
- A process-local flight is created only after durable acceptance succeeds. Owner release closes its flight after completion or failure, allowing waiters to reload the durable result or elect exactly one recovery owner.
- Waiter cancellation is local to `WaitCommand`; it neither cancels the owner context nor mutates the durable command record.
- The restart regression deliberately invokes the idempotent controller once before simulated loss of `FinishCommand`, then proves recovery reuses the same command ID while producing one effective mutation and one durable event.

## Reviewer Checkpoints

- No incomplete durable command response is exposed as a completed replay.
- Concurrent identical requests wait for and receive the same stored final response without cancelling the owner.
- A cancelled waiter exits without changing owner completion, durable state, or other waiters.
- Restart recovery reuses the stable command ID and causes no second effective mutation.
- A different immutable hash conflicts with zero conflicting mutation even while the original command is incomplete.
- Exactly one completed event, coherent state revision, and replay cursor are retained and broadcast.
- Fresh race, contract, protobuf determinism, Darwin arm64 cross-build, shared-package, and whole-Go evidence is present.

## Verification Results

Paste actual stdout/stderr under each command. Do not summarize reconstructed output. Record command substitutions or omissions in `Deviations from Plan`.

### REVIEW_API-1 Durable ownership and replay

```bash
gofmt -d apps/agent/internal/localcontrol/ledger.go apps/agent/internal/localcontrol/service.go
go test -count=1 ./apps/agent/internal/localcontrol -run 'TestConcurrentIdenticalCommandConvergesOnCompletedResponse|TestAcceptedButIncompleteCommandReconcilesAfterRestart|TestCommandIdempotency|TestCommandIDConflict'
```

```text
ok  	iop/apps/agent/internal/localcontrol	0.023s
```

### REVIEW_API-2 Concurrency and recovery regressions

```bash
go test -count=1 ./apps/agent/internal/localcontrol -run 'TestConcurrentIdenticalCommandConvergesOnCompletedResponse|TestAcceptedButIncompleteCommandReconcilesAfterRestart|TestCommandIdempotency|TestCommandIDConflict'
go test -count=1 -race ./apps/agent/internal/localcontrol ./packages/go/agentstate
```

```text
ok  	iop/apps/agent/internal/localcontrol	0.023s
ok  	iop/apps/agent/internal/localcontrol	1.089s
ok  	iop/packages/go/agentstate	1.285s
```

### REVIEW_API-3 S11 evidence

```bash
go test -count=1 ./apps/agent/internal/localcontrol
go vet ./apps/agent/internal/localcontrol ./packages/go/...
git diff --check
```

```text
ok  	iop/apps/agent/internal/localcontrol	0.050s
```

### Final Verification

```bash
command -v go
go version
go env GOROOT
git status --short
gofmt -w apps/agent/internal/localcontrol/ledger.go apps/agent/internal/localcontrol/service.go apps/agent/internal/localcontrol/service_test.go apps/agent/internal/localcontrol/ledger_test.go
go test -list 'Test(ConcurrentIdenticalCommandConvergesOnCompletedResponse|AcceptedButIncompleteCommandReconcilesAfterRestart|CommandIdempotency|CommandIDConflict)' ./apps/agent/internal/localcontrol
go test -count=1 ./apps/agent/internal/localcontrol -run 'TestConcurrentIdenticalCommandConvergesOnCompletedResponse|TestAcceptedButIncompleteCommandReconcilesAfterRestart|TestCommandIdempotency|TestCommandIDConflict'
go test -count=1 ./apps/agent/internal/localcontrol
go test -count=1 -race ./apps/agent/internal/localcontrol ./packages/go/agentstate
go vet ./apps/agent/internal/localcontrol ./packages/go/...
cp proto/gen/iop/agent.pb.go /tmp/iop-agent-localcontrol-agent.pb.go
make proto
cmp -s proto/gen/iop/agent.pb.go /tmp/iop-agent-localcontrol-agent.pb.go
GOOS=darwin GOARCH=arm64 go test -c -o /tmp/localcontrol-darwin.test ./apps/agent/internal/localcontrol
go test -count=1 ./packages/go/...
go test -count=1 ./...
rg -n --sort path 'S11|agent.proto|localcontrol|same OS user|SO_PEERCRED|LOCAL_PEERCRED|getpeereid' agent-contract/inner/iop-agent-cli-runtime.md apps/agent/internal/localcontrol proto/iop/agent.proto
git diff --check
git status --short
```

```text
/config/.local/bin/go
go version go1.26.2 linux/arm64
/config/opt/go
TestCommandIdempotencySurvivesRestart
TestAcceptedButIncompleteCommandReconcilesAfterRestart
TestCommandIDConflictHasZeroMutation
TestConcurrentIdenticalCommandConvergesOnCompletedResponse
ok  	iop/apps/agent/internal/localcontrol	0.004s
ok  	iop/apps/agent/internal/localcontrol	0.017s
ok  	iop/apps/agent/internal/localcontrol	0.040s
ok  	iop/apps/agent/internal/localcontrol	1.105s
ok  	iop/packages/go/agentstate	1.144s
protoc \
	--go_out=. \
	--go_opt=module=iop \
	--proto_path=. \
	proto/iop/agent.proto \
	proto/iop/runtime.proto \
	proto/iop/node.proto \
	proto/iop/control.proto \
	proto/iop/job.proto
ok  	iop/apps/agent/cmd/agent	0.051s
ok  	iop/apps/agent/internal/bootstrap	0.009s
ok  	iop/apps/agent/internal/command	0.169s
ok  	iop/apps/agent/internal/host	0.010s
ok  	iop/apps/agent/internal/localcontrol	0.095s
ok  	iop/apps/agent/internal/projectlog	5.596s
ok  	iop/apps/control-plane/cmd/control-plane	0.153s
ok  	iop/apps/control-plane/internal/wire	1.509s
ok  	iop/apps/edge/cmd/edge	0.220s
ok  	iop/apps/edge/internal/bootstrap	7.834s
ok  	iop/apps/edge/internal/configrefresh	0.076s
ok  	iop/apps/edge/internal/controlplane	4.461s
ok  	iop/apps/edge/internal/edgecmd	0.096s
ok  	iop/apps/edge/internal/edgevalidate	0.056s
ok  	iop/apps/edge/internal/events	0.006s
ok  	iop/apps/edge/internal/input	0.010s
ok  	iop/apps/edge/internal/input/a2a	0.011s
ok  	iop/apps/edge/internal/node	0.008s
ok  	iop/apps/edge/internal/openai	7.419s
ok  	iop/apps/edge/internal/opsconsole	0.009s
ok  	iop/apps/edge/internal/service	6.401s
ok  	iop/apps/edge/internal/transport	4.746s
ok  	iop/apps/node/cmd/node	0.015s
ok  	iop/apps/node/internal/adapters	0.015s
?   	iop/apps/node/internal/adapters/mock	[no test files]
ok  	iop/apps/node/internal/adapters/ollama	0.015s
ok  	iop/apps/node/internal/adapters/openai_compat	0.137s
ok  	iop/apps/node/internal/adapters/vllm	0.134s
ok  	iop/apps/node/internal/bootstrap	1.492s
ok  	iop/apps/node/internal/node	0.854s
ok  	iop/apps/node/internal/router	0.515s
ok  	iop/apps/node/internal/store	0.123s
ok  	iop/apps/node/internal/transport	5.658s
?   	iop/apps/worker/cmd/worker	[no test files]
ok  	iop/cmd/iop-provider-smoke	0.020s
ok  	iop/packages/go/agentconfig	0.044s
ok  	iop/packages/go/agentguard	0.022s
ok  	iop/packages/go/agentpolicy	0.022s
ok  	iop/packages/go/agentprovider/catalog	0.074s
ok  	iop/packages/go/agentprovider/cli	33.549s
?   	iop/packages/go/agentprovider/cli/internal/testutil	[no test files]
ok  	iop/packages/go/agentprovider/cli/status	40.833s
ok  	iop/packages/go/agentruntime	0.674s
ok  	iop/packages/go/agentstate	0.113s
ok  	iop/packages/go/agenttask	3.234s
ok  	iop/packages/go/agentworkspace	21.200s
ok  	iop/packages/go/audit	0.003s
?   	iop/packages/go/auth	[no test files]
ok  	iop/packages/go/config	0.121s
?   	iop/packages/go/events	[no test files]
ok  	iop/packages/go/hostsetup	0.005s
?   	iop/packages/go/jobs	[no test files]
?   	iop/packages/go/metadata	[no test files]
ok  	iop/packages/go/observability	0.017s
?   	iop/packages/go/policy	[no test files]
ok  	iop/packages/go/streamgate	1.356s
?   	iop/packages/go/version	[no test files]
?   	iop/proto/gen/iop	[no test files]
ok  	iop/scripts/inventory-query	0.009s

Persistent-session completion for `go test -count=1 ./packages/go/...`:

ok  	iop/packages/go/agentconfig	0.044s
ok  	iop/packages/go/agentguard	0.022s
ok  	iop/packages/go/agentpolicy	0.022s
ok  	iop/packages/go/agentprovider/catalog	0.074s
ok  	iop/packages/go/agentprovider/cli	30.527s
?   	iop/packages/go/agentprovider/cli/internal/testutil	[no test files]
ok  	iop/packages/go/agentprovider/cli/status	40.186s
ok  	iop/packages/go/agentruntime	0.764s
ok  	iop/packages/go/agentstate	0.218s
ok  	iop/packages/go/agenttask	1.540s
ok  	iop/packages/go/agentworkspace	12.797s
ok  	iop/packages/go/audit	0.011s
?   	iop/packages/go/auth	[no test files]
ok  	iop/packages/go/config	0.167s
?   	iop/packages/go/events	[no test files]
ok  	iop/packages/go/hostsetup	0.021s
?   	iop/packages/go/jobs	[no test files]
?   	iop/packages/go/metadata	[no test files]
ok  	iop/packages/go/observability	0.031s
?   	iop/packages/go/policy	[no test files]
ok  	iop/packages/go/streamgate	0.892s
?   	iop/packages/go/version	[no test files]
```

---

> **[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: PASS
- Dimension Assessment:
  - Correctness: Pass — incomplete durable records now elect one owner or join one process-local flight, and callers expose only the stored completed response.
  - Completeness: Pass — durable ownership, waiter cancellation isolation, restart recovery, final response/event persistence, and replay convergence are implemented.
  - Test Coverage: Pass — deterministic concurrency and accepted-but-incomplete restart regressions cover final response equality, conflict, cancellation, revision/cursor, retained event, and effective-mutation behavior.
  - API Contract: Pass — identical command replays converge on one completed result, conflicting immutable arguments perform no mutation, and recovery reuses the stable command ID.
  - Spec Conformance: Pass — the targeted S11 durable command-convergence evidence is present alongside the existing same-user/other-user local-control boundary coverage.
  - Code Quality: Pass — lifecycle state remains private to local control, synchronization is bounded, and no stale provisional-response path remains.
  - Implementation Deviation: Pass — persistent terminal sessions changed only command collection; the planned source, test, build, contract, and repository verification scope was retained.
  - Verification Trust: Pass — fresh reviewer runs reproduced focused, package, race, vet, protobuf determinism, Darwin arm64 cross-build, shared-package, whole-Go, and repeated race-stress success on byte-stable claimed files with this task as the sole active write claim.
- Findings: None
- Routing Signals:
  - `review_rework_count=2`
  - `evidence_integrity_failure=false`
- Next Step: Archive the passing pair, write `complete.log`, move the task to the July 2026 archive, and emit Milestone completion metadata for runtime processing.
