<!-- task=m-iop-agent-cli-runtime/11+06_workspace_overlay 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/11+06_workspace_overlay, 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:
  - `overlay-workspace`: pinned base and task writable layer
- Completion mode: check-on-pass

## Archive Evidence Snapshot

- Prior task: `agent-task/m-iop-agent-cli-runtime/11+06_workspace_overlay`
- Prior plan: `agent-task/m-iop-agent-cli-runtime/11+06_workspace_overlay/plan_cloud_G09_0.log`
- Prior review: `agent-task/m-iop-agent-cli-runtime/11+06_workspace_overlay/code_review_cloud_G09_0.log`
- Verdict: `FAIL`
- Required findings:
  - `packages/go/agentworkspace/overlay.go:241` self-attests `EnforcesWritableRoots`; an admitted callback can write the canonical absolute path.
  - `packages/go/agentworkspace/snapshot.go:57` omits canonical/config/grant identity; replay can return root B with root A's retained view.
- Suggested findings: None.
- Nit findings: None.
- Affected files: `packages/go/agentworkspace/overlay.go`, `packages/go/agentworkspace/snapshot.go`, `packages/go/agentworkspace/overlay_test.go`, and the strict admission/dispatch types required to carry verified confinement identity.
- Fresh verification: target suites, 20-count overlay repetition, overlay race test, all `packages/go/...`, vet, gofmt, and `git diff --check` passed; focused reviewer probes reproduced both Required findings.
- Roadmap carryover: S18 / `overlay-workspace` remains incomplete until actual absolute writes are denied and retained overlays cannot be rebound.

## 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-G10.md` -> `code_review_cloud_G10_1.log` and `PLAN-cloud-G10.md` -> `plan_cloud_G10_1.log`.
3. If PASS, write `complete.log` and move active task directory to `agent-task/archive/YYYY/MM/m-iop-agent-cli-runtime/11+06_workspace_overlay/`. 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 Make writable-root enforcement executable and identity-bound | [x] |
| REVIEW_API-2 Bind retained overlays to the exact canonical/config/grant base | [x] |

## Implementation Checklist

- [x] Replace self-attested writable-root enforcement with a concrete invocation confinement proof bound to the exact overlay/profile revision, and add offline child-process tests that allow task view/temp/cache writes while denying canonical, sibling-layer, snapshot, and shared-Git writes.
- [x] Bind canonical root plus configuration/grant revisions into workspace snapshot and overlay identities, reject retained-record root/revision rebinding, and add an exact root-A/root-B replay regression.
- [x] Run fresh target, repetition, race, package-wide, vet, 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_G10_1.log`.
- [x] Archive active `PLAN-*-G??.md` to `plan_cloud_G10_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/11+06_workspace_overlay/` to `agent-task/archive/YYYY/MM/m-iop-agent-cli-runtime/11+06_workspace_overlay/` 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

- Linux uses Landlock first and a verified unprivileged user/mount-namespace policy when Landlock is unavailable. This container selected the mount-namespace policy, so protected writes failed with the kernel's `EROFS` result instead of `EACCES` or `EPERM`. The production child still crossed an OS-enforced boundary: view/temp/cache writes succeeded, canonical/sibling/snapshot/shared-Git writes failed, and canonical evidence remained unchanged.
- The Linux availability probe was strengthened beyond the planned feature-presence check. It now launches confined `touch` children and requires both an allowed-root success and a protected-root denial before `Prepare` can return an admissible descriptor.
- `golang.org/x/sys` was already present in `go.sum`; only its existing version was promoted from indirect to direct in `go.mod`.
- `packages/go/agenttask/confinement_dispatch_test.go` was added to isolate missing and rebound proof regression coverage. No protobuf source changed, so `make proto` was not run.
- The shared worktree contained concurrent Agent Task runtime fixture changes. A transient duplicate method and an unconditional nil-channel wait were observed during verification; the duplicate was removed by the concurrent writer, and the wait branch was stabilized in the already planned `test_support_test.go` fixture before all final commands were rerun. Unrelated shared changes were preserved.

## Key Design Decisions

- `InvocationConfinement` is an executable launcher carried from `PreparedIsolation` through `DispatchRequest`. Its immutable binding covers the overlay, base snapshot, canonical/runtime/snapshot/task roots, exact view/temp/cache roots, config/grant/profile revisions, and platform policy revision. Dispatch validates the same proof after permit revalidation and immediately before provider start.
- `ConfinementProof.Start` constructs and starts the wrapper before returning, preventing a caller from replacing the sandbox executable or arguments between validation and launch. Unsupported hosts and failed policy probes return `ErrConfinementUnavailable` before an isolation descriptor exists.
- Linux prefers Landlock ABI 3 or newer, including refer and truncate restrictions. The fallback creates an unprivileged user/mount namespace, makes the full mount tree recursively read-only, restores write access only on the three task-owned bind mounts, locks privilege transitions, and drops capabilities. macOS uses a probed `sandbox-exec` file-write policy; other platforms fail closed.
- Snapshot schema v2 hashes canonical root, config revision, and grant revision with the Git/tree fingerprint. Overlay schema v2 persists and hashes the same base identity. A separate deterministic confinement revision avoids an overlay-revision cycle while still binding the exact overlay revision and platform policy.
- Retained lookup validates record checksum, layout, snapshot checksum, canonical root, config/grant/profile revisions, and confinement identity before returning the original overlay. Any rebind fails without rewriting retained evidence.
- Contracts were synchronized to distinguish admission capability from executable confinement and to require provider invokers to launch children through the exact proof.

## Reviewer Checkpoints

- The exact production confinement path, not descriptor mutation, produces `EACCES` or `EPERM` for canonical, sibling, snapshot, and shared-Git writes.
- View, temp, and cache writes remain available and isolated for concurrent tasks.
- Unsupported confinement fails closed before an admissible descriptor or provider invocation.
- Exact replay returns the retained overlay; root/config/grant rebinding fails without changing retained evidence.

## Verification Results

Paste actual stdout/stderr and exit status under every command. Do not replace output with a summary. If a command changes, record the reason under `Deviations from Plan`.

### Focused confinement verification

```bash
go test -count=1 -run '^(TestOverlayBackendConfinement|TestOverlayBackendConcurrent)' -v ./packages/go/agentworkspace
```

```text
=== RUN   TestOverlayBackendConfinementDeniesActualAbsoluteWrites
    overlay_test.go:155: confined child output:
        === RUN   TestConfinementWriteHelper
            overlay_test.go:47: allowed write: child-view.txt
            overlay_test.go:47: allowed write: child-temp.txt
            overlay_test.go:47: allowed write: child-cache.txt
            overlay_test.go:59: denied write: canonical-child.txt: open /tmp/TestOverlayBackendConfinementDeniesActualAbsoluteWrites4076591/001/workspace/canonical-child.txt: read-only file system
            overlay_test.go:59: denied write: sibling-child.txt: open /tmp/TestOverlayBackendConfinementDeniesActualAbsoluteWrites4076591/001/runtime/tasks/a65d79ce0b1134859495bad94ddb8db17ca6105c4034ee39dc13b38df35e3a97/view/sibling-child.txt: read-only file system
            overlay_test.go:59: denied write: snapshot-child.txt: open /tmp/TestOverlayBackendConfinementDeniesActualAbsoluteWrites4076591/001/runtime/snapshots/7914cb16e6b17550b1c4eba636275bd6ff03f3579fd546e5f9ba85062cdac753/snapshot-child.txt: read-only file system
            overlay_test.go:59: denied write: confinement-child: open /tmp/TestOverlayBackendConfinementDeniesActualAbsoluteWrites4076591/001/workspace/.git/confinement-child: read-only file system
        --- PASS: TestConfinementWriteHelper (0.00s)
        PASS
--- PASS: TestOverlayBackendConfinementDeniesActualAbsoluteWrites (10.78s)
=== RUN   TestOverlayBackendConcurrentTasksSharePinnedDirtyBaseAndIsolateWrites
--- PASS: TestOverlayBackendConcurrentTasksSharePinnedDirtyBaseAndIsolateWrites (9.10s)
PASS
ok  	iop/packages/go/agentworkspace	19.892s
exit status: 0
```

### Strict admission/dispatch regression

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

```text
ok  	iop/packages/go/agentguard	0.074s
ok  	iop/packages/go/agenttask	0.327s
exit status: 0
```

### Retained identity rebinding verification

```bash
go test -count=1 -run '^TestOverlayBackend(RejectsRetainedIdentityRebinding|IdempotencyAndFailureRetention)$' -v ./packages/go/agentworkspace
```

```text
=== RUN   TestOverlayBackendIdempotencyAndFailureRetention
--- PASS: TestOverlayBackendIdempotencyAndFailureRetention (0.70s)
=== RUN   TestOverlayBackendRejectsRetainedIdentityRebinding
--- PASS: TestOverlayBackendRejectsRetainedIdentityRebinding (0.08s)
PASS
ok  	iop/packages/go/agentworkspace	0.790s
exit status: 0
```

### Host Go 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
exit status: 0
```

### Fresh target suites

```bash
go test -count=1 ./packages/go/agentworkspace ./packages/go/agentguard ./packages/go/agenttask
```

```text
ok  	iop/packages/go/agentworkspace	24.013s
ok  	iop/packages/go/agentguard	0.016s
ok  	iop/packages/go/agenttask	0.308s
exit status: 0
```

### Repetition

```bash
go test -count=20 ./packages/go/agentworkspace
```

```text
ok  	iop/packages/go/agentworkspace	286.081s
exit status: 0
```

### Race detector

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

```text
ok  	iop/packages/go/agentworkspace	14.868s
ok  	iop/packages/go/agenttask	1.506s
exit status: 0
```

### Shared Go packages

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

```text
ok  	iop/packages/go/agentconfig	0.059s
ok  	iop/packages/go/agentguard	0.090s
ok  	iop/packages/go/agentprovider/catalog	0.149s
ok  	iop/packages/go/agentprovider/cli	55.752s
?   	iop/packages/go/agentprovider/cli/internal/testutil	[no test files]
ok  	iop/packages/go/agentprovider/cli/status	48.812s
ok  	iop/packages/go/agentruntime	2.235s
ok  	iop/packages/go/agentstate	0.300s
ok  	iop/packages/go/agenttask	0.351s
ok  	iop/packages/go/agentworkspace	20.236s
ok  	iop/packages/go/audit	0.056s
?   	iop/packages/go/auth	[no test files]
ok  	iop/packages/go/config	0.342s
?   	iop/packages/go/events	[no test files]
ok  	iop/packages/go/hostsetup	0.014s
?   	iop/packages/go/jobs	[no test files]
?   	iop/packages/go/metadata	[no test files]
ok  	iop/packages/go/observability	0.058s
?   	iop/packages/go/policy	[no test files]
ok  	iop/packages/go/streamgate	1.308s
?   	iop/packages/go/version	[no test files]
exit status: 0
```

### Vet

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

```text
(no output)
exit status: 0
```

### Formatting

```bash
gofmt -d packages/go/agentworkspace packages/go/agentguard packages/go/agenttask
```

```text
(no output)
exit status: 0
```

### Enforcement construction search

```bash
rg --sort path 'EnforcesWritableRoots: true|WritableRootConfinement: true' packages/go
```

```text
packages/go/agentguard/admission_integration_test.go:				WritableRootConfinement: true,
packages/go/agenttask/test_support_test.go:			Unattended: true, ApprovalBypass: true, WritableRootConfinement: true,
packages/go/agentworkspace/overlay_test.go:				WritableRootConfinement: true,
packages/go/agentworkspace/overlay_test.go:				WritableRootConfinement: true,
exit status: 0
```

### Diff integrity

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

```text
(no output)
exit status: 0
```

---

> **[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:
  - Correctness: Fail — the preferred Linux Landlock backend still permits protected-file metadata mutation, and the Manager dispatch path does not itself start the provider through the confinement proof.
  - Completeness: Fail — S18 requires every unattended provider child to be unable to mutate the canonical root, sibling layers, snapshots, or shared Git metadata through the actual dispatch path.
  - Test coverage: Fail — the child probe covers create/write denial only and calls `Confinement.Start` directly; it does not cover protected metadata changes or the Manager-to-invoker launch boundary.
  - API contract: Fail — the implementation contradicts the contracts that only view/temp/cache are mutable and that an invoker launches exclusively through `InvocationConfinement.Start`.
  - Code quality: Pass — the reviewed implementation is formatted, focused, and free of debug residue.
  - Implementation deviation: Fail — the required production invocation path was replaced by a direct proof test while `ProviderInvoker.Start` remains free to launch or simulate an unconfined child.
  - Verification trust: Pass — fresh focused, target, race, vet, formatting, and diff checks matched the recorded evidence; the later package-wide `agentpolicy` failures came from an unrelated concurrent sibling change and are excluded from this verdict.
  - Spec conformance: Fail — S18's canonical-unchanged and no-cross-write guarantees include file-mode/symlink identity and the real unattended dispatch path.
- Findings:
  - Required — `packages/go/agentworkspace/confinement_linux.go:41`: Linux prefers Landlock after a `touch`-only probe, but Landlock does not restrict `chmod(2)`, `chown(2)`, `setxattr(2)`, or `utime(2)`. A provider on a Landlock-capable host can therefore change the canonical tree, snapshot, sibling layer, or shared Git metadata without opening a file for write, invalidating the mode/timestamp identity that S18 requires to remain unchanged. Select only a backend that enforces read-only metadata outside view/temp/cache (or fail closed), extend the platform probe and child regression to attempt protected metadata mutations, and align both runtime contracts with the backend actually capable of that guarantee.
  - Required — `packages/go/agenttask/dispatch.go:147`: the Manager validates and passes the proof to arbitrary `ProviderInvoker.Start`, but no production code calls `DispatchRequest.Confinement.Start`; the only real child test calls the proof directly in `packages/go/agentworkspace/overlay_test.go:132`, while the manager fake merely self-validates and returns a simulated invocation. An invoker can ignore the proof and raw-start a provider after admission. Move the actual child start under a mandatory Manager-owned confinement boundary (for example, make the invoker return an immutable launch plan that the Manager starts through the proof), then add a Manager-level child regression that allows view/temp/cache writes and denies canonical/sibling/snapshot/shared-Git writes and metadata changes.
- Routing Signals:
  - `review_rework_count=2`
  - `evidence_integrity_failure=false`
- Next Step: Invoke the plan skill with these raw findings, complete isolated reassessment, archive this pair, and materialize the routed follow-up pair.
