<!-- task=m-iop-agent-cli-runtime/10+06_state_recovery plan=0 tag=API -->

# Code Review Reference - API

> **[IMPLEMENTING AGENT — READ FIRST]** Confirm `06` completion. Keep active files in place after recording actual evidence.

## Overview

date=2026-07-28
task=m-iop-agent-cli-runtime/10+06_state_recovery, plan=0, tag=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:
  - `state-recovery`: singleton/workspace lease, checkpoint, restart reconciliation
- Completion mode: check-on-pass

## Implementation Item Completion

| Item | Status |
|---|---|
| API-1 Implement the crash-safe state store and reconciliation boundary | [x] |

## Implementation Checklist

- [x] Add durable local StateStore format with schema/version/checksum and atomic CAS writes.
- [x] Add device singleton and workspace lease ownership/expiry/reconciliation without relaxing manager CAS semantics.
- [x] Persist process/session/overlay/change-set locators and failure budget as opaque, identity-checked records.
- [x] Block corrupt, stale or ambiguous checkpoint state with zero provider invocation.
- [x] Add restart, duplicate owner, cancel, corrupt checkpoint, partial archive and budget matrices.
- [x] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.

## Review-Only Checklist

> **[REVIEW AGENT ONLY]**

- [x] Append PASS/WARN/FAIL and verified routing signals.
- [x] Archive as `code_review_cloud_G10_0.log` and `plan_cloud_G10_0.log`.
- [ ] On PASS create `complete.log`, report `state-recovery`, then archive this subtask.

## Deviations from Plan

- The implementation expanded beyond the original file summary into `agenttask` types, dispatch, review, integration, test support, and the two affected inner contracts. These changes were required to preserve locator and failure-budget identity across every durable transition rather than storing detached state.
- `ProviderInvoker` now uses `Start`, checkpointed `Locators`, `Wait`, and `Cancel` instead of returning only a terminal `Submission`. This closes the crash window in which a live child existed before any recoverable process/session identity was durable.
- No external provider or standalone CLI process was exercised. This packet changes host-neutral state/recovery ports and uses deterministic temp roots and fake recovery observations as specified by the plan.

## Key Design Decisions

- `agentstate.Store` uses a schema-versioned JSON envelope with a monotonic CAS revision and SHA-256 checksum. Cross-instance CAS is serialized by an advisory lock; commits use a same-directory temporary file, file sync, atomic rename, and directory sync.
- The manager claims a durable device singleton before reconciliation and CAS-claims project, workspace invocation, and workspace integration leases. Foreign live owners block execution; expired leases can be reclaimed.
- Process, session, overlay, change-set, and completion locators remain opaque to the manager and carry exact project/workspace/work/attempt/kind/revision identity. Recovery decisions come from `RecoveryInspector`.
- Restart reconciliation retains proven-live work, resumes an exact recovered submission at review, and blocks stale, exited-without-result, partial-completion, corrupt, or ambiguous evidence without provider invocation.
- Failure budgets are durable per stage. Repeated review recovery stops at the configured limit with a non-retryable `failure_budget_exhausted` blocker.

## Reviewer Checkpoints

- Corrupt or ambiguous durable state never restarts a provider; leases and state transitions are CAS-safe.

## Verification Results

### `go test -count=1 -race ./packages/go/agentstate ./packages/go/agenttask`

```text
ok  	iop/packages/go/agentstate	1.291s
ok  	iop/packages/go/agenttask	1.232s
```

### `git diff --check`

PASS (no output).

### Additional verification

```text
$ command -v go && readlink -f "$(command -v go)" && go version && go env GOROOT
/config/.local/bin/go
/config/opt/go/bin/go
go version go1.26.2 linux/arm64
/config/opt/go

$ make proto
protoc \
	--go_out=. \
	--go_opt=module=iop \
	--proto_path=. \
	proto/iop/runtime.proto \
	proto/iop/node.proto \
	proto/iop/control.proto \
	proto/iop/job.proto

$ go test -count=1 ./packages/go/...
ok  	iop/packages/go/agentconfig	0.046s
ok  	iop/packages/go/agentguard	0.037s
ok  	iop/packages/go/agentprovider/catalog	0.093s
ok  	iop/packages/go/agentprovider/cli	32.668s
?   	iop/packages/go/agentprovider/cli/internal/testutil	[no test files]
ok  	iop/packages/go/agentprovider/cli/status	40.390s
ok  	iop/packages/go/agentruntime	0.821s
ok  	iop/packages/go/agentstate	0.168s
ok  	iop/packages/go/agenttask	0.201s
ok  	iop/packages/go/agentworkspace	1.665s
ok  	iop/packages/go/audit	0.042s
?   	iop/packages/go/auth	[no test files]
ok  	iop/packages/go/config	0.238s
?   	iop/packages/go/events	[no test files]
ok  	iop/packages/go/hostsetup	0.043s
?   	iop/packages/go/jobs	[no test files]
?   	iop/packages/go/metadata	[no test files]
ok  	iop/packages/go/observability	0.018s
?   	iop/packages/go/policy	[no test files]
ok  	iop/packages/go/streamgate	0.893s
?   	iop/packages/go/version	[no test files]

$ go vet ./packages/go/agentstate ./packages/go/agenttask
PASS (no output).

$ gofmt -d <modified Go files>
PASS (no output).

$ test -f agent-contract/inner/agent-runtime.md && test -f agent-contract/inner/iop-agent-cli-runtime.md
PASS (no output).

$ go test -count=20 ./packages/go/agentstate ./packages/go/agenttask
ok  	iop/packages/go/agentstate	1.438s
ok  	iop/packages/go/agenttask	3.283s
```

## Section Ownership

| Section | Owner |
|---|---|
| Implementation checklist/results/deviations/decisions | Implementing agent |
| Review checklist, verdict, archive and completion log | Review agent |

## Code Review Result

- Overall Verdict: FAIL

### Dimension Assessment

| Dimension | Assessment | Evidence |
|---|---|---|
| Correctness | Fail | A live manager can lose its device/project/workspace lease after the fixed TTL while it is still waiting on provider, review, or integration work, and it continues mutating state without a fencing-token check. |
| Completeness | Fail | The S09 singleton and workspace-owner invariant is not retained for the full duration of reconciliation. |
| Test coverage | Fail | The lease matrix covers a pre-seeded foreign lease and expiry takeover, but not expiry while the original owner is still executing an external call. |
| API contract | Fail | This violates the S09 no-duplicate-owner requirement and the durable lease ownership contract in `iop.agent-runtime` and `iop.agent-cli-runtime`. |
| Code quality | Pass | The state store, recovery records, and validation paths are structured and readable. |
| Implementation deviation | Pass | The expanded `agenttask` changes are connected to the planned durable locator and recovery boundary. |
| Verification trust | Pass | Fresh reviewer runs reproduced the reported race suite, shared-package suite, vet, formatting, proto generation, and diff checks; the blocking defect is an uncovered behavior rather than a fabricated command result. |
| Spec conformance | Fail | SDD S09 requires one live device/workspace invocation owner and no duplicate execution across restart and live-child recovery. |

### Findings

- Required — `packages/go/agenttask/intent.go:8`: lease claims write a single `ExpiresAt` value, but `Reconcile` can remain inside provider `Wait`, official review, or integration beyond that TTL and no renewal or fencing-token validation protects the original owner. A focused reviewer reproducer kept manager A in a live invocation, advanced manager B beyond the lease TTL, and observed manager B's `Reconcile` return `nil` instead of `ErrDeviceLeaseHeld`. Manager A can then continue committing results after manager B has replaced the durable device lease; the same single-claim pattern exists for project, workspace, and integration leases. Preserve a stable per-owner fencing token, renew every held lease for the entire external-operation lifetime, stop/cancel work when renewal is lost, require the token on release and post-call state commits, and add deterministic long-running invocation/review/integration expiry tests.

### Routing Signals

- `review_rework_count=1`
- `evidence_integrity_failure=false`

### Next Step

- Invoke the plan skill in `prepare-follow-up` mode with this Required finding and fresh verification evidence, then archive the current pair and materialize the freshly routed follow-up pair.
