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

# Make Client Receipt Completion and Ambiguous Close Fault-Atomic

## For the Implementing Agent

Complete every implementation item and run every verification command below. Fill all implementation-owned sections in `CODE_REVIEW-cloud-G08.md` with actual notes and stdout/stderr, leave the active pair in place, and report ready for review. Final verdict classification, log/archive transitions, `complete.log`, and control-plane stop files belong only to the official reviewer. If blocked, record only the exact blocker, attempted commands/output, and resume condition in implementation-owned evidence; do not ask the user, call user-input tools, classify the next state, archive files, or write `complete.log`.

## Background

The third S15 review passed every submitted focused, race, package, vet, Darwin arm64, formatting, and diff command, but reviewer fault injection found a false-success receipt replay after a completion save failed and the launched process was aborted. Durable validation still accepts impossible action/state receipt projections, and ambiguous adopted identities can make `Close` report success without reaping or wait forever beyond its context.

## 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.

## 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

## Analysis

### Files Read

- `agent-task/m-iop-agent-cli-runtime/23+14,16,22_client_process_manager/PLAN-cloud-G08.md`
- `agent-task/m-iop-agent-cli-runtime/23+14,16,22_client_process_manager/CODE_REVIEW-cloud-G08.md`
- `agent-task/m-iop-agent-cli-runtime/23+14,16,22_client_process_manager/plan_local_G07_1.log`
- `agent-task/m-iop-agent-cli-runtime/23+14,16,22_client_process_manager/code_review_cloud_G07_1.log`
- `agent-task/m-iop-agent-cli-runtime/23+14,16,22_client_process_manager/code_review_cloud_G10_0.log`
- `agent-roadmap/phase/automation-runtime-bridge/milestones/iop-agent-cli-runtime.md`
- `agent-roadmap/sdd/automation-runtime-bridge/iop-agent-cli-runtime/SDD.md`
- `agent-contract/inner/agent-runtime.md`
- `agent-contract/inner/iop-agent-cli-runtime.md`
- `apps/agent/internal/clientprocess/types.go`
- `apps/agent/internal/clientprocess/store.go`
- `apps/agent/internal/clientprocess/manager.go`
- `apps/agent/internal/clientprocess/process.go`
- `apps/agent/internal/clientprocess/store_test.go`
- `apps/agent/internal/clientprocess/manager_test.go`
- `apps/agent/internal/localcontrol/ledger.go`
- `apps/agent/internal/localcontrol/client_operations.go`
- `apps/agent/internal/localcontrol/client_operations_test.go`

### SDD Criteria

The approved and unlocked SDD at `agent-roadmap/sdd/automation-runtime-bridge/iop-agent-cli-runtime/SDD.md` maps S15 to `client-process-manager`. S15 requires daemon-owned singleton client lifecycle and Unity-to-Flutter detail routing; its Evidence Map requires fixture process ownership, crash/reconnect coverage, and a PID/start/focus lifecycle trace. Completion-save fault atomicity and bounded ambiguous close are required so that the trace cannot report an accepted process result after abort or a successful close without exact exit evidence.

### Verification Context

No external handoff was supplied. Repository-native inputs were the Agent, platform-common, testing, and local test rules; the approved SDD; both matching runtime contracts; current source/tests; and the three archived review verdicts for this task. Local preflight resolved `/config/.local/bin/go`, Go `1.26.2 linux/arm64`, and GOROOT `/config/opt/go`. Fresh reviewer commands passed the submitted focused/repeated race, package, vet, Darwin arm64 cross-build, formatting, `./packages/go/...`, `./apps/agent/...`, and diff checks. The reviewer fault regression failed deterministically on the current checkout. No external runner, credential, service, UI, port, or generated repository-local tool is required.

### Test Coverage Gaps

- No test fails the client-record CAS after the process side effect but before a completed receipt becomes durable, then retries the same `command_id`.
- `TestRecordRejectsInvalidCommandReceiptProjection` does not cover completed `start`, `focus`, or `detail` receipts paired with impossible stopped/crashed lifecycle states or invalid changed flags.
- No test closes a manager while a retained identity is ambiguous before adoption or becomes ambiguous after an adopted watcher starts.

### Symbol References

No public or private symbol is renamed. `validCompletedReceipt` is used by durable decode and receipt replay; completion receipt writes occur in `Start`, `Stop`, `Focus`, `StartOrFocusFlutter`, and `startLocked`. `startAdoptedWatcher` is created only by `reconcileLocked`, while `waitDone` is consumed by stop and exit paths.

### Split Judgment

Keep one plan. The completed receipt, current slot projection, process abort/reap state, and adopted watcher shutdown all share the same checksum-covered per-kind record and close ordering. Splitting validation, persistence rollback, and closure could independently pass while leaving a replayable false success or a close-time identity leak.

The encoded predecessors remain satisfied by the same task-group completion evidence already recorded in the prior plan:

- `14`: `agent-task/archive/2026/07/m-iop-agent-cli-runtime/14+13_cli_config_fixtures/complete.log`
- `16`: `agent-task/archive/2026/07/m-iop-agent-cli-runtime/16+13,15_bootstrap_composition/complete.log`
- `22`: `agent-task/archive/2026/07/m-iop-agent-cli-runtime/22+16_local_control/complete.log`

### Scope Rationale

Do not change protobuf fields, local-control ledger capacity/schema, client configuration, daemon bootstrap composition, Flutter/Unity applications, direct client IPC, or shared `agenttask` state. This packet changes only clientprocess receipt rollback/validation, close/watcher behavior, deterministic regressions, and S15 contract evidence.

### Final Routing

- evaluation_mode: `isolated-reassessment`
- finalizer: `finalize-task-policy.sh pair`
- build closures: scope/context/verification/evidence/ownership/decision all `true`; grade scores `2/2/1/2/1` = `G08`; base `local-fit`; route `recovery-boundary`; lane `cloud`; filename `PLAN-cloud-G08.md`
- review closures: scope/context/verification/evidence/ownership/decision all `true`; grade scores `2/2/1/2/1` = `G08`; route `official-review`; lane `cloud`; filename `CODE_REVIEW-cloud-G08.md`
- large_indivisible_context: `false`
- matched loop risks: `temporal_state`, `concurrent_consistency`, `boundary_contract` (count `3`)
- recovery signals: `review_rework_count=3`, `evidence_integrity_failure=false`
- capability-gap evidence: none

## Implementation Checklist

- [ ] Make every completed client receipt save fault-atomic so failed persistence cannot replay a non-durable or aborted success.
- [ ] Enforce action-specific lifecycle, connection, and changed-result invariants for completed receipt projections.
- [ ] Make ambiguous pre-adoption and adopted identity closure bounded, error-reporting, and evidence-preserving.
- [ ] Refresh S15 contract evidence and pass fresh fault, focused, repeated race, package, vet, Darwin cross-build, formatting, and diff verification.
- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.

### [REVIEW_REVIEW_REVIEW_API-1] Make Completion Receipt Persistence Fault-Atomic

#### Problem

`apps/agent/internal/clientprocess/manager.go:719-725` installs a completed receipt and live identity before saving. If that save fails, it aborts the child but leaves the completed receipt in memory, so `manager.go:980-984` replays success for the aborted process on the same `command_id`.

#### Solution

Capture the last successfully persisted pending projection and revision before applying each completed receipt. If the completion save fails, restore that exact pending projection in memory after any required process abort/cleanup so later retries remain fail-closed. Apply the helper consistently to start/already-running, stop/already-stopped, focus, and detail completion paths.

Before (`apps/agent/internal/clientprocess/manager.go:719`):

```go
m.setReceiptLocked(slot, commandID, receiptAction, CommandReceiptCompleted, res)
if err := m.persistLocked(ctx, slot); err != nil {
	_ = process.Abort()
	return Result{}, err
}
```

After:

```go
pendingRecord, pendingRevision := cloneRecord(slot.record), slot.revision
m.setReceiptLocked(slot, commandID, receiptAction, CommandReceiptCompleted, res)
if err := m.persistLocked(ctx, slot); err != nil {
	_ = process.Abort()
	slot.record, slot.revision = pendingRecord, pendingRevision
	return Result{}, err
}
```

#### Modified Files and Checklist

- [ ] `apps/agent/internal/clientprocess/manager.go` — restore the exact last durable pending projection after any completion receipt save failure.
- [ ] `apps/agent/internal/clientprocess/manager_test.go` — add table-driven failure injection for start, stop, focus, and detail completion saves; prove start aborts and no same-command retry returns a non-durable success.

#### Test Strategy

Add `TestCommandReceiptCompletionSaveFailureStaysPending` with a deterministic nth-CAS store and action cases. Assert the first call returns the injected error, start aborts its child, a same-command retry returns `ErrCommandPending`, and a new manager loaded from the store also remains fail-closed.

#### Verification

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

Expected: every failed completion save retains a pending durable receipt, aborts any uncommitted start, and never replays success.

### [REVIEW_REVIEW_REVIEW_API-2] Validate Completed Receipt Projections by Action

#### Problem

`apps/agent/internal/clientprocess/types.go:117-140` accepts any recognized lifecycle state for completed `start`, `focus`, and `detail` receipts. Corrupt durable records such as `start + stopped` or `focus + crashed` therefore pass `apps/agent/internal/clientprocess/store.go:147-162` and replay as accepted results.

#### Solution

Define exact state/result combinations produced by each command: start must be starting or connected; stop must be stopped and disconnected; focus must be starting or connected and changed; detail-start must be starting and changed; detail-focus must be starting or connected and changed. Keep the existing connection coherence checks and reject every other projection.

Before (`apps/agent/internal/clientprocess/types.go:130`):

```go
case "start":
	return receipt.Result.Action == "start"
case "focus":
	return receipt.Result.Action == "focus"
```

After:

```go
case "start":
	return receipt.Result.Action == "start" &&
		(receipt.Result.State == StateStarting ||
			receipt.Result.State == StateConnected)
case "focus":
	return receipt.Result.Action == "focus" &&
		receipt.Result.Changed &&
		(receipt.Result.State == StateStarting ||
			receipt.Result.State == StateConnected)
```

#### Modified Files and Checklist

- [ ] `apps/agent/internal/clientprocess/types.go` — encode strict command-action/result-state/changed invariants.
- [ ] `apps/agent/internal/clientprocess/store_test.go` — extend the invalid receipt table across start, stop, focus, detail-start, and detail-focus combinations.

#### Test Strategy

Extend `TestRecordRejectsInvalidCommandReceiptProjection` with valid and invalid state/connection/changed tables for every receipt action. Existing manager-generated receipt cases must continue to pass durable save/load.

#### Verification

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

Expected: manager-produced receipts validate and every impossible action/state projection is rejected.

### [REVIEW_REVIEW_REVIEW_API-3] Bound Ambiguous Identity Closure Without False Exit

#### Problem

`apps/agent/internal/clientprocess/manager.go:580-589` suppresses `ErrIdentityAmbiguous`. `manager.go:856-873` then keeps an adopted watcher polling forever after manager cancellation while `Close` waits unconditionally, so ambiguity can yield false success before adoption or an unbounded hang after adoption.

#### Solution

Return `ErrIdentityAmbiguous` as close evidence while continuing shutdown of independent slots. On manager cancellation, let an adopted watcher exit without calling `finishExit` or closing the process-exit channel when identity remains unproven. Preserve the durable identity/blocker, allow the watcher WaitGroup to drain, and ensure `Close` returns within its supplied context without recording a false stopped/exited transition.

Before (`apps/agent/internal/clientprocess/manager.go:860`):

```go
case <-m.ctx.Done():
	for {
		observation, err := m.backend.Inspect(context.Background(), identity)
		if err == nil && observation.State == IdentityLive {
			time.Sleep(m.pollInterval)
			continue
		}
		if err != nil || observation.State == IdentityAmbiguous {
			time.Sleep(m.pollInterval)
			continue
		}
		m.finishExit(kind, slot, identity, done)
		return
	}
```

After:

```go
case <-m.ctx.Done():
	return // retain identity; cancellation is not exit evidence
```

#### Modified Files and Checklist

- [ ] `apps/agent/internal/clientprocess/manager.go` — surface ambiguous stop evidence and terminate adopted watcher ownership without fabricating exit.
- [ ] `apps/agent/internal/clientprocess/manager_test.go` — cover ambiguity before adoption and after a live identity has been adopted; assert bounded return and retained identity.

#### Test Strategy

Add `TestClosePreservesAmbiguousIdentityBeforeAdoption` and `TestClosePreservesAmbiguousAdoptedIdentity`. Use a deterministic backend that changes from live to ambiguous, run `Close` with a bounded context, require `ErrIdentityAmbiguous`, require the watcher to join, and assert the durable identity/state remain unchanged rather than stopped.

#### Verification

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

Expected: close never reports a false exit, never hangs after manager cancellation, and existing exact adopted cleanup remains green.

### [REVIEW_REVIEW_REVIEW_API-4] Refresh S15 Evidence Anchors

#### Problem

The S15 row in `agent-contract/inner/iop-agent-cli-runtime.md` does not name completion-save fault atomicity, strict receipt projection, or ambiguous close regressions, so its exact evidence list would remain weaker than the corrected lifecycle contract.

#### Solution

Add the exact new test names and state that non-durable completion results never replay, ambiguous close preserves identity evidence and returns a bounded error, and only proven exit permits durable reaping.

#### Modified Files and Checklist

- [ ] `agent-contract/inner/iop-agent-cli-runtime.md` — add the new S15 fault/validation/close anchors and matching lifecycle wording.

#### Test Strategy

No separate document test is added. A deterministic anchor search plus the focused/race/package commands verifies that every named test exists and exercises the production boundary.

#### Verification

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

Expected: every test definition and the S15 contract row appear in deterministic path order.

## Dependencies and Execution Order

Implement fault-atomic receipt persistence first, strict validation second, ambiguous close third, and refresh the S15 evidence row last. All changes remain inside the existing `23+14,16,22_client_process_manager` task boundary.

## Modified Files Summary

| File | Item |
|------|------|
| `apps/agent/internal/clientprocess/manager.go` | REVIEW_REVIEW_REVIEW_API-1, REVIEW_REVIEW_REVIEW_API-3 |
| `apps/agent/internal/clientprocess/manager_test.go` | REVIEW_REVIEW_REVIEW_API-1, REVIEW_REVIEW_REVIEW_API-3 |
| `apps/agent/internal/clientprocess/types.go` | REVIEW_REVIEW_REVIEW_API-2 |
| `apps/agent/internal/clientprocess/store_test.go` | REVIEW_REVIEW_REVIEW_API-2 |
| `agent-contract/inner/iop-agent-cli-runtime.md` | REVIEW_REVIEW_REVIEW_API-4 |
| `agent-task/m-iop-agent-cli-runtime/23+14,16,22_client_process_manager/CODE_REVIEW-cloud-G08.md` | REVIEW_REVIEW_REVIEW_API-1, REVIEW_REVIEW_REVIEW_API-2, REVIEW_REVIEW_REVIEW_API-3, REVIEW_REVIEW_REVIEW_API-4 |

## 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
```

Expected: completion-save failures remain pending and never replay aborted success; impossible receipt projections are rejected; ambiguous close returns bounded evidence without false exit; existing S15 lifecycle, fresh/race/repeated/package/vet/Darwin/anchor/format/diff checks pass. Cached Go test output is not accepted. External provider smoke, UI full-cycle execution, and Edge-Node diagnostics remain not applicable because this packet changes no external provider, UI, wire, or daemon composition.

After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`.
