<!-- task=m-iop-agent-cli-runtime/23+14,16,22_client_process_manager plan=3 tag=REVIEW_REVIEW_REVIEW_API -->

# Code Review Reference - REVIEW_REVIEW_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/23+14,16,22_client_process_manager, plan=3, tag=REVIEW_REVIEW_REVIEW_API

## Roadmap Targets

- Milestone: `agent-roadmap/phase/automation-runtime-bridge/milestones/iop-agent-cli-runtime.md`
- Milestone link: [Milestone document](agent-roadmap/phase/automation-runtime-bridge/milestones/iop-agent-cli-runtime.md)
- Task ids:
  - `client-process-manager`: daemon-owned Flutter/Unity start, stop, reconnect, crash recovery, and detail routing
- Completion mode: check-on-pass

## Archive Evidence Snapshot

- Prior plan: `agent-task/m-iop-agent-cli-runtime/23+14,16,22_client_process_manager/plan_cloud_G08_2.log`
- Prior review: `agent-task/m-iop-agent-cli-runtime/23+14,16,22_client_process_manager/code_review_cloud_G08_2.log`
- Verdict: FAIL
- Findings: 3 Required, 0 Suggested, 0 Nit.
- Affected boundary: completion-receipt persistence rollback, action-specific receipt projection validation, and ambiguous pre-adoption/adopted close behavior.
- Verified evidence: all submitted focused, repeated race, package, vet, Darwin arm64 cross-build, formatting, `./packages/go/...`, `./apps/agent/...`, and diff checks passed. A reviewer-only second-client-save fault regression failed because retry replayed `start` success after the child had been aborted.
- Roadmap carryover: S15 / `client-process-manager` remains incomplete until completion-save failure is fail-closed, receipt projections are action-state strict, and ambiguous identity closure returns bounded evidence without false exit.

## 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-G08.md` → `code_review_cloud_G08_3.log` and `PLAN-cloud-G08.md` → `plan_cloud_G08_3.log`.
3. If PASS, write `complete.log` and move active task directory to `agent-task/archive/YYYY/MM/m-iop-agent-cli-runtime/23+14,16,22_client_process_manager/`. 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_REVIEW_REVIEW_API-1 Completion receipt fault atomicity | [x] |
| REVIEW_REVIEW_REVIEW_API-2 Strict receipt projection validation | [x] |
| REVIEW_REVIEW_REVIEW_API-3 Bounded ambiguous identity closure | [x] |
| REVIEW_REVIEW_REVIEW_API-4 S15 evidence anchors | [x] |

## Implementation Checklist

- [x] Make every completed client receipt save fault-atomic so failed persistence cannot replay a non-durable or aborted success.
- [x] Enforce action-specific lifecycle, connection, and changed-result invariants for completed receipt projections.
- [x] Make ambiguous pre-adoption and adopted identity closure bounded, error-reporting, and evidence-preserving.
- [x] Refresh S15 contract evidence and pass fresh fault, focused, repeated race, package, vet, Darwin cross-build, formatting, and diff verification.
- [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_G08_3.log`.
- [x] Archive active `PLAN-*-G??.md` to `plan_cloud_G08_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`.
- [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/23+14,16,22_client_process_manager/` to `agent-task/archive/YYYY/MM/m-iop-agent-cli-runtime/23+14,16,22_client_process_manager/` 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

No deviations from the plan.

## Key Design Decisions

- `processSlot` retains a clone of its most recently successful durable record. Completion receipt persistence restores that exact pending projection and revision if the completed CAS fails, including when an asynchronous exit persistence raced the command completion.
- Every externally addressed completion path now persists a pending receipt before writing a completed receipt, including already-running start and already-stopped stop results.
- Completed receipt validation is command-action specific. Focus and detail-focus require `Changed=true`; detail-start requires `starting`; and only starting/connected or stopped/disconnected projections are accepted as applicable.
- `Close` joins `ErrIdentityAmbiguous` into its result. Cancellation makes an adopted watcher relinquish ownership without converting unproven identity state into a false exit.

## Reviewer Checkpoints

- Every completion save failure restores the exact last durable pending receipt and revision before a retry can inspect the slot.
- A failed start completion aborts its child, and neither the same manager nor a restarted manager can replay that non-durable success.
- Completed start, stop, focus, detail-start, and detail-focus receipts accept only the lifecycle/connection/changed combinations emitted by production paths.
- Ambiguous identity close returns bounded error evidence before and after adoption, preserves the durable identity, and never calls the proven-exit cleanup path.
- S15 contract anchors name the new fault, projection, and ambiguous-close tests while the existing PID/start/focus lifecycle trace remains green.

## Verification Results

Run every command exactly as written and paste actual stdout/stderr. If a command changes, record the replacement and reason in `Deviations from Plan`. Cached Go test output is not accepted.

### Completion receipt persistence failure

```bash
go test -count=1 ./apps/agent/internal/clientprocess -run 'TestCommandReceiptCompletionSaveFailureStaysPending'
```

```text
ok  	iop/apps/agent/internal/clientprocess	0.162s
```

### Strict completed receipt projections

```bash
go test -count=1 ./apps/agent/internal/clientprocess -run 'TestRecordRejectsInvalidCommandReceiptProjection|TestStoreRoundTripAndConflict'
```

```text
ok  	iop/apps/agent/internal/clientprocess	0.008s
```

### Ambiguous identity closure

```bash
go test -count=25 -race ./apps/agent/internal/clientprocess -run 'TestClosePreservesAmbiguousIdentity|TestAdoptedCleanupRequiresObservedExit|TestConcurrentClose'
```

```text
ok  	iop/apps/agent/internal/clientprocess	10.326s
```

### S15 evidence anchors

```bash
rg -n --sort path 'TestCommandReceiptCompletionSaveFailureStaysPending|TestRecordRejectsInvalidCommandReceiptProjection|TestClosePreservesAmbiguousIdentityBeforeAdoption|TestClosePreservesAmbiguousAdoptedIdentity' apps/agent/internal/clientprocess agent-contract/inner/iop-agent-cli-runtime.md
```

```text
apps/agent/internal/clientprocess/manager_test.go:1307:func TestCommandReceiptCompletionSaveFailureStaysPending(t *testing.T) {
apps/agent/internal/clientprocess/manager_test.go:1481:func TestClosePreservesAmbiguousIdentityBeforeAdoption(t *testing.T) {
apps/agent/internal/clientprocess/manager_test.go:1485:func TestClosePreservesAmbiguousAdoptedIdentity(t *testing.T) {
apps/agent/internal/clientprocess/store_test.go:165:func TestRecordRejectsInvalidCommandReceiptProjection(t *testing.T) {
agent-contract/inner/iop-agent-cli-runtime.md:49:| S15 | `TestManagerOwnsSingletonAndReapsClient`, `TestDuplicateLaunchConvergesAfterManagerRestart`, `TestDaemonSurvivesCrashAndBoundedRestart`, `TestS15ClientLifecycleTrace`, `TestReconcileBlocksAmbiguousIdentityWithoutLaunch`, `TestClientOperationMatrix`, `TestUnityDetailStartsOrFocusesFlutter`, `TestRejectedClientCommandHasZeroProcessCalls`, `TestAcceptedIncompleteClientCommandReusesExactManagerReceiptAfterStateChange`, `TestCommandReceiptCapacityMatchesLedger`, `TestStartConfiguredLaunchesAfterDaemonRestart`, `TestCloseStopsCurrentGenerationAfterPriorLifecycleReceipts`, `TestConcurrentCloseCancelsInFlightFocus`, `TestConcurrentCloseCancelsInFlightDetail`, `TestConcurrentCloseFencesConnectionMutation`, `TestCommandReceiptCompletionSaveFailureStaysPending`, `TestRecordRejectsInvalidCommandReceiptProjection`, `TestClosePreservesAmbiguousIdentityBeforeAdoption`, and `TestClosePreservesAmbiguousAdoptedIdentity`; fresh focused/race suites and Darwin arm64 cross-build | `client-process-manager` proves one PID/start identity per kind, exact reaping, live adoption without duplicate launch, bounded crash restart, disconnect/reconnect without daemon cancellation, fail-closed ambiguous recovery, and Unity detail routing only to daemon-owned Flutter start/focus. Completed external client receipts replay only an immutable accepted result for the matching command action and strict action-specific lifecycle projection; a completion save failure leaves the durable pending receipt in place and never replays an aborted or non-durable success. Ambiguous close preserves the retained identity, returns bounded error evidence, and permits durable reaping only after a proven exit; daemon lifecycle generations do not create or reuse external command receipts, and close cancels admitted mutations before reaping the current identity. |
```

### Final verification

```bash
gofmt -w apps/agent/internal/clientprocess/types.go apps/agent/internal/clientprocess/manager.go apps/agent/internal/clientprocess/store_test.go apps/agent/internal/clientprocess/manager_test.go
go test -count=1 ./apps/agent/internal/clientprocess -run 'TestCommandReceiptCompletionSaveFailureStaysPending|TestRecordRejectsInvalidCommandReceiptProjection|TestClosePreservesAmbiguousIdentity'
go test -count=1 ./apps/agent/internal/clientprocess ./apps/agent/internal/localcontrol -run 'TestAcceptedIncompleteClient|TestCommandReceipt|TestStartConfiguredLaunchesAfterDaemonRestart|TestCloseStopsCurrentGenerationAfterPriorLifecycleReceipts|TestConcurrentClose|TestAdoptedCleanupRequiresObservedExit'
go test -count=1 ./packages/go/agentconfig ./apps/agent/internal/clientprocess ./apps/agent/internal/localcontrol
go test -count=1 -race ./apps/agent/internal/clientprocess ./apps/agent/internal/localcontrol ./packages/go/agentstate
go test -count=25 -race ./apps/agent/internal/clientprocess -run 'TestCommandReceiptCompletionSaveFailureStaysPending|TestClosePreservesAmbiguousIdentity|TestConcurrentClose|TestAdoptedCleanupRequiresObservedExit'
go vet ./packages/go/agentconfig ./apps/agent/internal/clientprocess ./apps/agent/internal/localcontrol
review_darwin_dir=$(mktemp -d) && review_darwin_bin="$review_darwin_dir/clientprocess.test" && GOOS=darwin GOARCH=arm64 go test -c -o "$review_darwin_bin" ./apps/agent/internal/clientprocess && test -s "$review_darwin_bin"
rg -n --sort path 'TestCommandReceiptCompletionSaveFailureStaysPending|TestRecordRejectsInvalidCommandReceiptProjection|TestClosePreservesAmbiguousIdentityBeforeAdoption|TestClosePreservesAmbiguousAdoptedIdentity' apps/agent/internal/clientprocess agent-contract/inner/iop-agent-cli-runtime.md
go test -count=1 ./packages/go/...
go test -count=1 ./apps/agent/...
git diff --check
```

```text
gofmt completed without output.
ok  	iop/apps/agent/internal/clientprocess	0.091s
ok  	iop/apps/agent/internal/clientprocess	0.527s
ok  	iop/apps/agent/internal/localcontrol	0.088s
ok  	iop/packages/go/agentconfig	0.036s
ok  	iop/apps/agent/internal/clientprocess	1.152s
ok  	iop/apps/agent/internal/localcontrol	0.265s
ok  	iop/apps/agent/internal/clientprocess	8.330s
ok  	iop/apps/agent/internal/localcontrol	1.515s
ok  	iop/packages/go/agentstate	1.205s
ok  	iop/apps/agent/internal/clientprocess	11.944s
go vet completed without output.
Darwin arm64 test compilation completed without output and produced a non-empty binary.
apps/agent/internal/clientprocess/manager_test.go:1307:func TestCommandReceiptCompletionSaveFailureStaysPending(t *testing.T) {
apps/agent/internal/clientprocess/manager_test.go:1481:func TestClosePreservesAmbiguousIdentityBeforeAdoption(t *testing.T) {
apps/agent/internal/clientprocess/manager_test.go:1485:func TestClosePreservesAmbiguousAdoptedIdentity(t *testing.T) {
apps/agent/internal/clientprocess/store_test.go:165:func TestRecordRejectsInvalidCommandReceiptProjection(t *testing.T) {
agent-contract/inner/iop-agent-cli-runtime.md:49:| S15 | `TestManagerOwnsSingletonAndReapsClient`, `TestDuplicateLaunchConvergesAfterManagerRestart`, `TestDaemonSurvivesCrashAndBoundedRestart`, `TestS15ClientLifecycleTrace`, `TestReconcileBlocksAmbiguousIdentityWithoutLaunch`, `TestClientOperationMatrix`, `TestUnityDetailStartsOrFocusesFlutter`, `TestRejectedClientCommandHasZeroProcessCalls`, `TestAcceptedIncompleteClientCommandReusesExactManagerReceiptAfterStateChange`, `TestCommandReceiptCapacityMatchesLedger`, `TestStartConfiguredLaunchesAfterDaemonRestart`, `TestCloseStopsCurrentGenerationAfterPriorLifecycleReceipts`, `TestConcurrentCloseCancelsInFlightFocus`, `TestConcurrentCloseCancelsInFlightDetail`, `TestConcurrentCloseFencesConnectionMutation`, `TestCommandReceiptCompletionSaveFailureStaysPending`, `TestRecordRejectsInvalidCommandReceiptProjection`, `TestClosePreservesAmbiguousIdentityBeforeAdoption`, and `TestClosePreservesAmbiguousAdoptedIdentity`; fresh focused/race suites and Darwin arm64 cross-build | `client-process-manager` proves one PID/start identity per kind, exact reaping, live adoption without duplicate launch, bounded crash restart, disconnect/reconnect without daemon cancellation, fail-closed ambiguous recovery, and Unity detail routing only to daemon-owned Flutter start/focus. Completed external client receipts replay only an immutable accepted result for the matching command action and strict action-specific lifecycle projection; a completion save failure leaves the durable pending receipt in place and never replays an aborted or non-durable success. Ambiguous close preserves the retained identity, returns bounded error evidence, and permits durable reaping only after a proven exit; daemon lifecycle generations do not create or reuse external command receipts, and close cancels admitted mutations before reaping the current identity. |
ok  	iop/packages/go/agentconfig	0.039s
ok  	iop/packages/go/agentguard	0.011s
ok  	iop/packages/go/agentpolicy	0.042s
ok  	iop/packages/go/agentprovider/catalog	0.081s
ok  	iop/packages/go/agentprovider/cli	30.904s
?   	iop/packages/go/agentprovider/cli/internal/testutil	[no test files]
ok  	iop/packages/go/agentprovider/cli/status	40.003s
ok  	iop/packages/go/agentruntime	0.620s
ok  	iop/packages/go/agentstate	0.145s
ok  	iop/packages/go/agenttask	1.016s
ok  	iop/packages/go/agentworkspace	11.927s
ok  	iop/packages/go/audit	0.004s
?   	iop/packages/go/auth	[no test files]
ok  	iop/packages/go/config	0.131s
?   	iop/packages/go/events	[no test files]
ok  	iop/packages/go/hostsetup	0.012s
?   	iop/packages/go/jobs	[no test files]
?   	iop/packages/go/metadata	[no test files]
ok  	iop/packages/go/observability	0.022s
?   	iop/packages/go/policy	[no test files]
ok  	iop/packages/go/streamgate	0.897s
?   	iop/packages/go/version	[no test files]
ok  	iop/apps/agent/cmd/agent	0.013s
ok  	iop/apps/agent/internal/bootstrap	0.004s
ok  	iop/apps/agent/internal/clientprocess	2.107s
ok  	iop/apps/agent/internal/command	0.024s
ok  	iop/apps/agent/internal/host	0.004s
ok  	iop/apps/agent/internal/localcontrol	0.506s
ok  	iop/apps/agent/internal/projectlog	3.759s
git diff --check completed without output.
```

---

> **[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
  - Completeness: Pass
  - Test coverage: Pass
  - API contract: Pass
  - Code quality: Pass
  - Implementation deviation: Pass
  - Verification trust: Pass
  - Spec conformance: Pass
- Findings: None
- Routing Signals:
  - `review_rework_count=3`
  - `evidence_integrity_failure=false`
- Next Step: Archive the active pair, write `complete.log`, and emit the milestone-task completion metadata for runtime processing.
