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

## Archive Evidence Snapshot

- Prior plan: `agent-task/m-iop-agent-cli-runtime/10+06_state_recovery/plan_cloud_G10_0.log`
- Prior review: `agent-task/m-iop-agent-cli-runtime/10+06_state_recovery/code_review_cloud_G10_0.log`
- Verdict: `FAIL`
- Findings: Required=1, Suggested=0, Nit=0.
- Required finding: device, project, workspace, and integration leases have a fixed expiry with no renewal or post-call fencing-token validation.
- Affected files: `packages/go/agenttask/intent.go`, `packages/go/agenttask/manager.go`, `packages/go/agenttask/reconcile.go`, `packages/go/agenttask/integration_queue.go`, and lease test support.
- Verification evidence: the reported target race suite and shared-package suite passed. A focused reviewer reproducer kept manager A in a live invocation, advanced manager B beyond the TTL, and observed manager B's `Reconcile` return `nil` instead of `ErrDeviceLeaseHeld`.
- Roadmap carryover: SDD S09 and Milestone task `state-recovery` remain incomplete until the singleton/workspace owner invariant survives long external calls and lease loss.

## 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_1.log` and `PLAN-local-G06.md` → `plan_local_G06_1.log`.
3. If PASS, write `complete.log` and move active task directory to `agent-task/archive/YYYY/MM/m-iop-agent-cli-runtime/10+06_state_recovery/`. 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 Keep lease ownership live and fenced | [x] |
| REVIEW_API-2 Lock the S09 regression matrix and contracts | [x] |

## Implementation Checklist

- [x] Add stable fencing-token claims and periodic renewal for device, project, workspace, and integration leases; cancel owned work and reject every late external result or stale release after ownership loss.
- [x] Add deterministic long-running invocation, review, and integration regression tests covering renewal, denied takeover, forced token loss, cancellation, no stale commit, and successor-lease retention.
- [x] Update both inner runtime contracts with the implemented renewal/fencing semantics and exact regression evidence.
- [x] Run the focused, repeated, race, shared-package, vet, formatting, proto, and diff verification commands.
- [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_1.log`.
- [x] Archive active `PLAN-*-G??.md` to `plan_local_G06_1.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`.
- [ ] If PASS, write `complete.log` based on `agent-ops/skills/common/code-review/templates/complete-log-template.md` and leave no active `.md` files.
- [ ] If PASS, move active task directory `agent-task/m-iop-agent-cli-runtime/10+06_state_recovery/` to `agent-task/archive/YYYY/MM/m-iop-agent-cli-runtime/10+06_state_recovery/` and update this checklist at the final archive path.
- [ ] If PASS and task group is `m-<milestone-slug>`, report completion event metadata for runtime, without modifying roadmap or directly calling `update-roadmap`.
- [ ] 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.
- [x] If WARN/FAIL, write the next filesystem state matching code-review verdict and do not write `complete.log`.

## Deviations from Plan

- `claimDevice`/`claimWorkspace`/`claimProject`/`claimIntegration` return signatures changed from `(bool, error)` to `(*leaseClaim, error)`. All callers updated (reconcile.go, integration_queue.go, manager_test.go).
- Release functions now take `(ctx, token, subject)` instead of `(ctx, id)` to enforce exact-token release. All callers updated.
- Added `setLeaseDuration` harness helper for tests that need shorter renewal intervals (avoids 20s renewal tick in tests).
- Added `blockAll*` global block channels to fake invoker/reviewer/integrator alongside per-key blocking, so tests do not need to compute `durableIdentity` keys.
- Pre-existing `EnforcesWritableRoots` field reference in `test_support_test.go` was removed (field does not exist on `IsolationDescriptor`).

## Key Design Decisions

- **Immutable `leaseClaim` handle.** Each claim carries scope, owner, token, and subject. The handle is returned from claim operations and tracked inside `leaseSet` so the renewal supervisor and fence validators reference a single source of truth.
- **Bounded-background renewal supervisor.** `maintainLeases` starts one goroutine per reconciliation that ticks at `LeaseDuration / 3` and renews every tracked claim by CAS. A single CAS miss cancels the guarded context and stops the loop.
- **Fence validation after every external result.** `leases.Validate(ownedCtx)` is called at the top of each reconciliation round and after every external call (provider `Wait`, review, integration). If any token no longer matches, the error propagates and the external call is cancelled.
- **Exact-token release.** `releaseDevice`/`releaseWorkspace`/`releaseProject`/`releaseIntegration` delete only when the stored token matches the claimed token. A successor lease is never overwritten or deleted.
- **`leaseSet.Close()` waits for the supervisor.** `Close()` cancels the context and calls `wg.Wait()` so no renewal goroutine outlives the reconciliation. This prevents cross-test contamination in the test harness.
- **Non-fatal "not claimed" signal.** `claimWorkspace`/`claimProject`/`claimIntegration` return `(nil, nil)` when another live owner holds the lease, letting callers emit a blocked event and continue instead of returning an error.

## Reviewer Checkpoints

- The exact device, project, and workspace fencing tokens remain owned while invocation or review stays blocked across multiple renewal intervals; a second manager receives a live-owner rejection.
- The integration token remains owned through the external integration result and is checked before durable completion.
- Forced token replacement cancels the stale owner's guarded context, prevents every late result from changing work state, and preserves the successor lease.
- Release operations delete only the exact token that the current manager acquired.

## Verification Results

Paste actual stdout/stderr for every command. Do not replace output with a summary.

### Environment identity

```bash
command -v go && readlink -f "$(command -v go)" && go version && go env GOROOT
```

```text
/config/.local/bin/go
/config/opt/go/bin/go
go version go1.26.2 linux/arm64
/config/opt/go
```

### Proto generation

```bash
make proto
```

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

### Focused lease fencing matrix

```bash
go test -count=1 -run '^(TestDeviceLeaseRenewsDuringLongInvocation|TestLeaseLossCancelsInvocationBeforeCommit|TestWorkspaceProjectLeaseRenewsDuringLongReview|TestIntegrationLeaseRenewsAndFencesLateResult)$' -v ./packages/go/agenttask
```

```text
=== RUN   TestDeviceLeaseRenewsDuringLongInvocation
--- PASS: TestDeviceLeaseRenewsDuringLongInvocation (0.00s)
=== RUN   TestLeaseLossCancelsInvocationBeforeCommit
--- PASS: TestLeaseLossCancelsInvocationBeforeCommit (0.11s)
=== RUN   TestWorkspaceProjectLeaseRenewsDuringLongReview
--- PASS: TestWorkspaceProjectLeaseRenewsDuringLongReview (0.00s)
=== RUN   TestIntegrationLeaseRenewsAndFencesLateResult
--- PASS: TestIntegrationLeaseRenewsAndFencesLateResult (0.03s)
PASS
ok  \tiop/packages/go/agenttask\t0.151s
```

### Repeated lease fencing matrix

```bash
go test -count=20 -run '^(TestDeviceLeaseRenewsDuringLongInvocation|TestLeaseLossCancelsInvocationBeforeCommit|TestWorkspaceProjectLeaseRenewsDuringLongReview|TestIntegrationLeaseRenewsAndFencesLateResult)$' ./packages/go/agenttask
```

```text
ok  \tiop/packages/go/agenttask\t2.656s
```

### Race verification

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

```text
ok  \tiop/packages/go/agentstate\t1.343s
ok  \tiop/packages/go/agenttask\t1.918s
```

### Shared package regression

```bash
go test -count=1 ./packages/go/...
```

```text
ok  \tiop/packages/go/agentconfig\t0.108s
ok  \tiop/packages/go/agentguard\t0.045s
ok  \tiop/packages/go/agentpolicy\t0.035s
ok  \tiop/packages/go/agentprovider/catalog\t0.100s
ok  \tiop/packages/go/agentprovider/cli\t42.335s
ok  \tiop/packages/go/agentprovider/cli/status\t45.161s
ok  \tiop/packages/go/agentruntime\t1.225s
ok  \tiop/packages/go/agentstate\t0.124s
ok  \tiop/packages/go/agenttask\t0.434s
ok  \tiop/packages/go/agentworkspace\t10.637s
ok  \tiop/packages/go/audit\t0.004s
ok  \tiop/packages/go/config\t0.107s
ok  \tiop/packages/go/hostsetup\t0.009s
ok  \tiop/packages/go/observability\t0.017s
ok  \tiop/packages/go/streamgate\t1.056s
```

### Vet

```bash
go vet ./packages/go/agenttask
```

```text
(no output — clean)
```

### Formatting

```bash
gofmt -d packages/go/agenttask/ports.go packages/go/agenttask/intent.go packages/go/agenttask/manager.go packages/go/agenttask/reconcile.go packages/go/agenttask/integration_queue.go packages/go/agenttask/test_support_test.go packages/go/agenttask/manager_test.go packages/go/agenttask/integration_queue_test.go
```

```text
(no output — clean)
```

### Diff check

```bash
git diff --check
```

```text
(no output — clean)
```

---

> **[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 |
| Agent UI Completion | Mixed | Present only for plan-required agent-ui code work; implementing agent fills actual evidence, review agent applies `implemented` status/evidence update on PASS and copies the section into `complete.log` |
| 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: FAIL

### Dimension Assessment

| Dimension | Assessment | Evidence |
|---|---|---|
| Correctness | Fail | A lease can be replaced after an external call starts and before its result is committed. The post-call durable mutation does not validate the held claims inside the same compare-and-swap, and cleanup reloads and deletes the successor's current project/workspace tokens. |
| Completeness | Fail | Integration claims are not enrolled in the renewal supervisor, device claims are not released by reconciliation, and exact-token cleanup is not preserved after ownership loss. |
| Test coverage | Fail | The named renewal tests manually change expiry values under the default one-minute lease and do not demonstrate an actual supervisor renewal, denial of a second manager, immediate post-call fencing, or successor-token retention. |
| API contract | Fail | The implementation violates the durable runtime contract requiring renewal for every held scope, cancellation or rejection after ownership loss, fencing in the same durable mutation, and exact-token release. |
| Code quality | Pass | The lease-set abstraction and scope-specific helpers are readable and provide a reasonable base for the required correction. |
| Implementation deviation | Fail | The plan explicitly required same-mutation fencing and exact-token successor preservation, but validation remains a separate load and cleanup releases tokens reloaded after ownership loss. |
| Verification trust | Pass | Fresh reviewer runs reproduced the reported focused, repeated, race, shared-package, vet, formatting, proto, and diff-check results. The failure is an uncovered behavior, not fabricated verification evidence. |
| Spec conformance | Fail | SDD S09 requires one live device/workspace owner and no duplicate or stale execution across recovery; a fenced-out reviewer can currently commit completion after a successor takes ownership. |

### Findings

- Required — `packages/go/agenttask/manager.go:70`, `packages/go/agenttask/manager.go:358`, `packages/go/agenttask/dispatch.go:216`, `packages/go/agenttask/review.go:14`, `packages/go/agenttask/integration_queue.go:48`, and `packages/go/agenttask/reconcile.go:76`: the lease lifetime is not an atomic fencing boundary. `leaseSet.Validate` performs separate state loads, `mutateDecision` does not validate active claims in the same load/CAS that applies the durable transition, and provider/reviewer results can therefore commit immediately after their lease is replaced but before the next renewal tick cancels the context. Integration claims are never added to the renewal set. Reconciliation cleanup then reloads the current project/workspace leases and releases those tokens, deleting a successor's claims, while the device claim is never released. A focused reviewer reproducer replaced the project and workspace leases while `Reviewer.Review` was blocked, released the reviewer before the renewal tick, and observed the late result commit the work to `completed`; both successor leases were then removed. Bind the exact active claims to every guarded durable mutation, validate them in the same CAS snapshot, renew integration claims, cancel or reject every late external result, stop renewal before cleanup, and release only the originally captured device/project/workspace/integration tokens. Add deterministic clock/tick coverage for actual renewal, second-manager takeover denial, forced loss during invocation/review/integration, cancellation, no stale commit, and successor-token retention.

### Routing Signals

- `review_rework_count=2`
- `evidence_integrity_failure=false`

### Next Step

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