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

# Close Client Process Recovery and Ownership Gaps

## For the Implementing Agent

Complete all implementation-owned fields in `CODE_REVIEW-cloud-G07.md` with actual verification output, leave the active pair in place, and report ready for review. The official reviewer alone owns verdict classification, log/archive transitions, `complete.log`, and control-plane stop files. If blocked, record only the exact blocker, attempted commands/output, and resume condition in implementation-owned evidence; do not ask the user or classify the next state.

## Background

The first S15 review found that the happy-path and race suites pass but do not close three crash/ownership boundaries. Incomplete local-control recovery can repeat a non-idempotent focus command, config removal can orphan a retained client identity, and manager shutdown is not fenced against an in-flight launch or an adopted watcher that exits without proving process termination. The follow-up keeps these behaviors in one packet because command convergence, durable ownership, and exact process cleanup share the same client slot record.

## Archive Evidence Snapshot

- Prior plan: `agent-task/m-iop-agent-cli-runtime/23+14,16,22_client_process_manager/plan_cloud_G10_0.log`
- Prior review: `agent-task/m-iop-agent-cli-runtime/23+14,16,22_client_process_manager/code_review_cloud_G10_0.log`
- Verdict: FAIL
- Findings: 3 Required, 0 Suggested, 0 Nit.
- Affected boundary: `clientprocess.Manager`, its checksum-covered slot record, and `localcontrol.ClientOperations` accepted-incomplete recovery.
- Verified evidence: fresh focused packages, race packages, vet, formatting, Darwin arm64 cross-build, S15 trace, `./packages/go/...`, and `./apps/agent/...` all passed; the missing deterministic crash/config-removal/shutdown regressions are the blocker.
- Roadmap carryover: S15 / `client-process-manager` remains incomplete until all Required findings and its Evidence Map lifecycle trace pass.

## 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-G10.md`
- `agent-task/m-iop-agent-cli-runtime/23+14,16,22_client_process_manager/CODE_REVIEW-cloud-G10.md`
- `agent-roadmap/phase/automation-runtime-bridge/PHASE.md`
- `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`
- `packages/go/agentconfig/runtime_config.go`
- `packages/go/agentconfig/runtime_config_test.go`
- `packages/go/agentstate/store.go`
- `configs/iop-agent.local.example.yaml`
- `apps/agent/internal/clientprocess/types.go`
- `apps/agent/internal/clientprocess/process.go`
- `apps/agent/internal/clientprocess/store.go`
- `apps/agent/internal/clientprocess/manager.go`
- `apps/agent/internal/clientprocess/manager_test.go`
- `apps/agent/internal/clientprocess/store_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

`agent-roadmap/sdd/automation-runtime-bridge/iop-agent-cli-runtime/SDD.md` is approved and unlocked. Scenario S15 maps to `client-process-manager`; its Evidence Map requires fixture Flutter/Unity ownership, crash/reconnect, detail routing, and a PID/start/focus trace. The standalone contract additionally requires durable command-id convergence, one retained owner per kind, fail-closed ambiguity, exact reaping, and bounded restart. Those requirements produce the three implementation items and the repeated race/recovery verification below.

### Verification Context

No external handoff was supplied. Repository-native inputs were the Agent, platform-common, and testing domain rules; `agent-test/local/rules.md`; `agent-test/local/platform-common-smoke.md`; the approved SDD; the two matching contracts; current source/tests; and the archived predecessor `complete.log` files. Local preflight resolved `/config/.local/bin/go`, Go `1.26.2 linux/arm64`, GOROOT `/config/opt/go`, and the dirty current checkout. Fresh reviewer commands passed for focused packages, race packages, vet, `gofmt -d`, `git diff --check`, Darwin arm64 cross-compilation, S15 verbose trace, all shared Go packages, and all Agent packages.

The remaining gap is behavioral, not environmental: current tests do not exercise accepted-but-incomplete client recovery after the process mutation, retained ownership after client config removal, semantic record contradictions, an in-flight launch crossing `Close`, or adopted cleanup after manager-context cancellation. No external runner, credential, service, port, UI, or user action is required. Cached output is not accepted; every Go test command uses `-count=1`, and concurrency regressions also repeat under `-race`.

### Test Coverage Gaps

- Completed local-control replay is covered, but accepted-incomplete recovery after a completed or ambiguous process-side mutation is not.
- Config-preserving restart adoption is covered, but a live durable record whose launch policy was removed is not.
- Live/exited/stale/ambiguous identity observations are covered, but contradictory lifecycle/identity records are not rejected.
- Sequential post-close launch rejection is covered, but an in-flight launch crossing `Close` and adopted cleanup after parent cancellation are not.

### Symbol References

No symbol is renamed or removed. `LastCommandID` is written in `manager.go` but has no read site; `StartOrFocusFlutter` is called only by the local-control adapter and its tests.

### Split Judgment

Keep one plan. A command receipt, the durable process identity, and shutdown/reconciliation all mutate one checksum-covered per-kind slot and must agree transactionally before S15 can PASS. Splitting would permit a state schema or manager transition to pass without the matching recovery adapter and race evidence.

The subtask dependencies are satisfied:

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

### Scope Rationale

Do not wire the manager into the daemon bootstrap, change proto fields, implement Flutter/Unity applications, add direct client IPC, or expand the client config schema. Daemon composition remains owned by the downstream wiring packet. This follow-up changes only durable client command/ownership semantics, OS process cancellation, exact tests, the standalone contract evidence row, and the mandatory review artifact.

### Final Routing

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

## Implementation Checklist

- [x] Make accepted-incomplete client commands converge on a durable completed receipt without repeating a process mutation, and fail closed for a pending non-idempotent outcome.
- [x] Reconcile retained canonical client records independently of current launch configuration and reject contradictory lifecycle/identity projections.
- [x] Fence launch and client mutations against manager closure, cancel/reap an in-flight launch, and require exact exit evidence for adopted cleanup.
- [x] Update S15 contract evidence and pass fresh 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_API-1] Make Client Command Recovery Process-Idempotent

#### Problem

`apps/agent/internal/localcontrol/client_operations.go:106-166` deliberately reclaims an accepted-but-incomplete ledger command and calls the controller again. `apps/agent/internal/clientprocess/manager.go:266-348` persists `LastCommandID` after focus/detail but never reads any command receipt. A crash after the focus subprocess returns and before `Ledger.FinishCommand` commits therefore executes focus twice during recovery.

#### Solution

Add a bounded, strictly validated command receipt to the durable per-kind record. Persist a pending receipt before a non-idempotent process mutation, atomically persist its completed result with the lifecycle transition, return that stored result when the same command/action is recovered, and return a typed fail-closed error when a pending outcome cannot be proven. Do not evict a receipt while the local-control ledger can still retain the matching command.

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

```go
type Record struct {
	SchemaVersion uint32 `json:"schema_version"`
	// ...
	LastCommandID string `json:"last_command_id,omitempty"`
}
```

After:

```go
type CommandReceipt struct {
	CommandID string
	Action    string
	Status    CommandReceiptStatus
	Result    CommandResultSnapshot
}

type Record struct {
	SchemaVersion uint32                    `json:"schema_version"`
	// ...
	Commands      map[string]CommandReceipt `json:"commands,omitempty"`
}
```

The exact field layout may follow existing JSON style, but it must preserve action identity, pending/completed state, and enough immutable result data to reproduce the original local-control mutation response.

#### Modified Files and Checklist

- [ ] `apps/agent/internal/clientprocess/types.go` — define receipt/result states and the typed ambiguous-outcome error.
- [ ] `apps/agent/internal/clientprocess/store.go` — validate and deep-copy bounded receipts.
- [ ] `apps/agent/internal/clientprocess/manager.go` — begin/complete/replay receipts under the slot lock for start, stop, focus, and detail.
- [ ] `apps/agent/internal/localcontrol/client_operations.go` — map ambiguous process-command recovery to the stable retryable invalid-state response.
- [ ] `apps/agent/internal/localcontrol/client_operations_test.go` — cover accepted-incomplete recovery through the real manager boundary.

#### Test Strategy

Add `TestAcceptedIncompleteClientCommandReusesCompletedManagerReceipt` and `TestAcceptedIncompleteClientFocusFailsClosedWhenOutcomePending`. Use a deterministic fake backend with start/focus counters and a restarted ledger. Assert that a completed process receipt yields the original response with zero additional focus/start calls, while a persisted pending focus returns the typed blocker with zero calls.

#### Verification

```bash
go test -count=1 ./apps/agent/internal/clientprocess ./apps/agent/internal/localcontrol -run 'TestAcceptedIncompleteClient(Command|Focus)'
```

Expected: both recovery branches converge without a second process mutation.

### [REVIEW_API-2] Preserve Ownership Across Config Removal and Reject Invalid Records

#### Problem

`apps/agent/internal/clientprocess/manager.go:100-116` creates slots only from the current `specs` map. A live `client-process/flutter` or `client-process/unity` record is invisible after its launch policy is removed. `apps/agent/internal/clientprocess/store.go:113-133` also accepts `stopped` with `Identity != nil` and `connected` with `Connected=false`, so semantic corruption can enter live adoption/start paths.

#### Solution

Load both canonical client record keys on every manager construction and track launch-policy presence separately from retained process ownership. Reconcile/watch and permit exact stop/status for a retained identity even when new launch/focus/restart is unconfigured; never auto-restart without a current policy. Tighten record validation so `stopped` and `crashed` have no current identity, `connected` requires both `Connected=true` and an identity, and `starting` remains the only in-flight state that may have an optional identity.

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

```go
for rawKind, spec := range specs {
	kind := ClientKind(rawKind)
	// load only configured kinds
}
```

After:

```go
for _, kind := range canonicalClientKinds {
	record, revision, found, err := durable.load(ctx, kind)
	// retain configured/found ownership independently
}
```

#### Modified Files and Checklist

- [ ] `apps/agent/internal/clientprocess/types.go` — expose deterministic canonical-kind metadata if needed.
- [ ] `apps/agent/internal/clientprocess/store.go` — enforce lifecycle/identity projection invariants.
- [ ] `apps/agent/internal/clientprocess/manager.go` — retain configured/found slot state and reconcile all canonical kinds.
- [ ] `apps/agent/internal/clientprocess/store_test.go` — add invalid-record and removed-config recovery tests.
- [ ] `apps/agent/internal/clientprocess/manager_test.go` — prove a retained live helper remains owned, duplicate-free, and exactly stoppable after policy removal.

#### Test Strategy

Add `TestManagerReconcilesRetainedClientAfterConfigRemoval` with a real helper identity and the same state store across managers. Add table-driven `TestRecordRejectsContradictoryLifecycleProjection` for stopped/live, crashed/live, connected/false, and connected/no-identity variants. Assert no replacement launch, an adopted watcher or exact cleanup path, and fail-closed decode.

#### Verification

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

Expected: retained ownership remains manageable without a launch policy and invalid projections never enter reconciliation.

### [REVIEW_API-3] Fence Shutdown and Prove Adopted Exit

#### Problem

`apps/agent/internal/clientprocess/manager.go:161-183` checks openness before taking the slot lock, while `Close` marks the manager closed and then waits for the slot. An already-admitted `Start` can therefore return from the backend and publish a child after closure began. `apps/agent/internal/clientprocess/process.go:29-45` ignores the manager context, and `manager.go:573-599` closes the same `waitDone` channel on watcher cancellation and proven exit, allowing cleanup to mistake supervisor shutdown for child termination.

#### Solution

Revalidate the manager lifecycle while holding the slot lock immediately before every subprocess mutation and after any blocking launch returns. Make the OS launch context-aware and abort/reap an identity if the lifecycle fence closes before durable publication. Separate adopted watcher termination from proven process exit; stop/close must inspect the exact PID/start token until exited, then kill that identity after the bound timeout and verify exit before clearing durable ownership.

Before (`apps/agent/internal/clientprocess/process.go:29`):

```go
func (osProcessBackend) Start(_ context.Context, _ ClientKind, spec agentconfig.ClientProcessSpec) (OwnedProcess, error) {
	command := exec.Command(spec.Executable, spec.Args...)
```

After:

```go
func (osProcessBackend) Start(ctx context.Context, _ ClientKind, spec agentconfig.ClientProcessSpec) (OwnedProcess, error) {
	command := exec.CommandContext(ctx, spec.Executable, spec.Args...)
```

#### Modified Files and Checklist

- [ ] `apps/agent/internal/clientprocess/process.go` — bind child launch to the manager lifecycle and retain exact abort/reap behavior.
- [ ] `apps/agent/internal/clientprocess/manager.go` — add under-lock lifecycle fencing and separate owned/adopted exit waits.
- [ ] `apps/agent/internal/clientprocess/manager_test.go` — add deterministic close/launch and cancelled-adopted-watcher regressions.

#### Test Strategy

Add `TestConcurrentCloseAbortsInFlightLaunch` using a backend barrier after child creation and `TestAdoptedCleanupRequiresObservedExit` using an adopted backend that ignores TERM until exact Kill. Assert no live child or current identity remains, every created child is reaped, and post-close start/focus/detail/connection calls fail without mutation.

#### Verification

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

Expected: 25 fresh race-enabled repetitions pass with no leaked identity, duplicate close, or late mutation.

## Dependencies and Execution Order

Implement receipt schema and validation first, then manager replay/reconciliation, then lifecycle fencing and integration regressions. The directory dependencies are already satisfied by the three archived predecessor `complete.log` files listed in `Analysis > Split Judgment`; this follow-up adds no new runtime dependency.

## Modified Files Summary

| File | Item |
|------|------|
| `apps/agent/internal/clientprocess/types.go` | REVIEW_API-1, REVIEW_API-2 |
| `apps/agent/internal/clientprocess/store.go` | REVIEW_API-1, REVIEW_API-2 |
| `apps/agent/internal/clientprocess/manager.go` | REVIEW_API-1, REVIEW_API-2, REVIEW_API-3 |
| `apps/agent/internal/clientprocess/process.go` | REVIEW_API-3 |
| `apps/agent/internal/clientprocess/manager_test.go` | REVIEW_API-2, REVIEW_API-3 |
| `apps/agent/internal/clientprocess/store_test.go` | REVIEW_API-2 |
| `apps/agent/internal/localcontrol/client_operations.go` | REVIEW_API-1 |
| `apps/agent/internal/localcontrol/client_operations_test.go` | REVIEW_API-1 |
| `agent-contract/inner/iop-agent-cli-runtime.md` | REVIEW_API-1, REVIEW_API-2, REVIEW_API-3 |
| `agent-task/m-iop-agent-cli-runtime/23+14,16,22_client_process_manager/CODE_REVIEW-cloud-G07.md` | REVIEW_API-1, REVIEW_API-2, REVIEW_API-3 |

## Final Verification

```bash
gofmt -w 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/manager_test.go apps/agent/internal/clientprocess/store_test.go apps/agent/internal/localcontrol/client_operations.go apps/agent/internal/localcontrol/client_operations_test.go
go test -count=1 ./apps/agent/internal/clientprocess ./apps/agent/internal/localcontrol -run 'TestAcceptedIncompleteClient(Command|Focus)|TestManagerReconcilesRetainedClientAfterConfigRemoval|TestRecordRejectsContradictoryLifecycleProjection|TestConcurrentCloseAbortsInFlightLaunch|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 ./apps/agent/internal/localcontrol -run 'TestAcceptedIncompleteClient(Command|Focus)|TestConcurrentCloseAbortsInFlightLaunch|TestAdoptedCleanupRequiresObservedExit'
go vet ./packages/go/agentconfig ./apps/agent/internal/clientprocess ./apps/agent/internal/localcontrol
review_darwin_bin=$(mktemp /tmp/iop-clientprocess-darwin.XXXXXX.test) && trap 'rm -f "$review_darwin_bin"' EXIT && GOOS=darwin GOARCH=arm64 go test -c -o "$review_darwin_bin" ./apps/agent/internal/clientprocess && test -s "$review_darwin_bin"
go test -count=1 ./packages/go/...
go test -count=1 ./apps/agent/...
git diff --check
```

Expected: receipt recovery performs zero duplicate process mutations; retained unconfigured identities remain owned and exactly stoppable; invalid durable projections fail closed; concurrent close and adopted cleanup leave no live child; all fresh/race/repeated/package/vet/Darwin/format/diff checks pass. Edge-Node diagnostics and external full-cycle UI smoke remain not applicable because this packet changes no Edge route, external wire, client application, or daemon composition.

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