<!-- task=m-iop-agent-cli-runtime/11+06_workspace_overlay plan=2 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=2, 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_G10_1.log`
- Prior review: `agent-task/m-iop-agent-cli-runtime/11+06_workspace_overlay/code_review_cloud_G10_1.log`
- Verdict: `FAIL`
- Required findings:
  - `packages/go/agentworkspace/confinement_linux.go:41` prefers Landlock although it does not restrict protected `chmod`, `chown`, `setxattr`, or `utime` operations.
  - `packages/go/agenttask/dispatch.go:147` passes the proof to arbitrary `ProviderInvoker.Start`; no production call site starts the provider through `InvocationConfinement.Start`.
- Suggested findings: None.
- Nit findings: None.
- Affected files: Linux confinement selection/probes, Manager/provider launch ports and fixtures, S18 tests, and the two runtime contracts.
- Fresh verification: focused overlay, retained-identity, guard/task target, overlay race, vet, formatting, Darwin cross-compile, and diff checks passed. A package-wide run later failed only in concurrently added `agentpolicy` tests outside this task; the target packages remained green.
- Roadmap carryover: S18 / `overlay-workspace` remains incomplete until all protected mutations are denied through the actual Manager launch path.

## 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_2.log` and `PLAN-cloud-G10.md` -> `plan_cloud_G10_2.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 Linux confinement metadata-complete | [x] |
| REVIEW_API-2 Make the Manager own the confined child start | [x] |

## Implementation Checklist

- [x] Replace incomplete Landlock preference with a metadata-complete fail-closed Linux policy and add protected metadata mutation probes/tests.
- [x] Move provider process start under a mandatory Manager-owned `InvocationConfinement.Start` boundary and add launch-order/identity regressions.
- [x] Synchronize runtime contracts and run fresh target, repetition, race, package-wide, cross-platform compile, vet, formatting, search, 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_2.log`.
- [x] Archive active `PLAN-*-G??.md` to `plan_cloud_G10_2.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

Implementation scope did not deviate. The previously concurrent
`failure_continuation_test.go` compile error was corrected outside this task;
the resumed vet and complete verification set passed without changes to target
selection, quota/failover, or their tests.

## Key Design Decisions

- Linux admits only the probed user/mount-namespace policy. The probe requires
  protected content writes plus `chmod`, `utime`, `chown`, and `setxattr` to
  fail without changing the protected file; Landlock is not an admissible
  fallback.
- `ProviderInvoker.Prepare` is side-effect-free. The Manager calls the exact
  confinement proof's `Start` once, then binds that returned child through
  `ProviderLaunch.BindStarted`; a bind failure kills and reaps the child.

## Reviewer Checkpoints

- The selected Linux backend denies both content and metadata mutations outside view/temp/cache, or `Prepare` fails closed.
- `agenttask.Manager` starts the intended child exactly once through the validated `InvocationConfinement.Start` proof before binding the durable invocation handle.
- Canonical, sibling, snapshot, overlay-record, and shared-Git evidence remains unchanged while view/temp/cache content and metadata remain writable.
- Exact retained root/config/grant replay behavior remains unchanged.

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

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

### Linux content and metadata confinement

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

```text
=== RUN   TestConfinementPlatformFailsWithTypedUnavailableError
--- PASS: TestConfinementPlatformFailsWithTypedUnavailableError (0.15s)
=== RUN   TestConfinementLinuxUsesMetadataCompleteMountNamespacePolicy
--- PASS: TestConfinementLinuxUsesMetadataCompleteMountNamespacePolicy (0.00s)
=== RUN   TestConfinementProofRejectsTamperedBinding
--- PASS: TestConfinementProofRejectsTamperedBinding (0.06s)
=== RUN   TestConfinementWriteHelper
--- PASS: TestConfinementWriteHelper (0.00s)
=== RUN   TestOverlayBackendConfinementDeniesActualAbsoluteWrites
    overlay_test.go:233: confined child output:
        === RUN   TestConfinementWriteHelper
            overlay_test.go:52: allowed write: child-view.txt
            overlay_test.go:52: allowed write: child-temp.txt
            overlay_test.go:52: allowed write: child-cache.txt
            overlay_test.go:64: denied write: canonical-child.txt: open /tmp/TestOverlayBackendConfinementDeniesActualAbsoluteWrites2362363215/001/workspace/canonical-child.txt: read-only file system
            overlay_test.go:64: denied write: sibling-child.txt: open /tmp/TestOverlayBackendConfinementDeniesActualAbsoluteWrites2362363215/001/runtime/tasks/a65d79ce0b1134859495bad94ddb8db17ca6105c4034ee39dc13b38df35e3a97/view/sibling-child.txt: read-only file system
            overlay_test.go:64: denied write: snapshot-child.txt: open /tmp/TestOverlayBackendConfinementDeniesActualAbsoluteWrites2362363215/001/runtime/snapshots/e7c3c40b84597736c4c4188ecb6ec0f19f2bae4f54ec5f0a045f1b7afae08589/snapshot-child.txt: read-only file system
            overlay_test.go:64: denied write: confinement-child: open /tmp/TestOverlayBackendConfinementDeniesActualAbsoluteWrites2362363215/001/workspace/.git/confinement-child: read-only file system
            overlay_test.go:75: allowed metadata chmod: child-view.txt
            overlay_test.go:75: allowed metadata utime: child-view.txt
            overlay_test.go:75: allowed metadata chown: child-view.txt
            overlay_test.go:75: allowed metadata setxattr: child-view.txt
            overlay_test.go:75: allowed metadata chmod: child-temp.txt
            overlay_test.go:75: allowed metadata utime: child-temp.txt
            overlay_test.go:75: allowed metadata chown: child-temp.txt
            overlay_test.go:75: allowed metadata setxattr: child-temp.txt
            overlay_test.go:75: allowed metadata chmod: child-cache.txt
            overlay_test.go:75: allowed metadata utime: child-cache.txt
            overlay_test.go:75: allowed metadata chown: child-cache.txt
            overlay_test.go:75: allowed metadata setxattr: child-cache.txt
            overlay_test.go:96: denied metadata chmod: shared.txt: chmod /tmp/TestOverlayBackendConfinementDeniesActualAbsoluteWrites2362363215/001/workspace/shared.txt: read-only file system
            overlay_test.go:96: denied metadata utime: shared.txt: chtimes /tmp/TestOverlayBackendConfinementDeniesActualAbsoluteWrites2362363215/001/workspace/shared.txt: read-only file system
            overlay_test.go:96: denied metadata chown: shared.txt: chown /tmp/TestOverlayBackendConfinementDeniesActualAbsoluteWrites2362363215/001/workspace/shared.txt: read-only file system
            overlay_test.go:96: denied metadata setxattr: shared.txt: read-only file system
            overlay_test.go:96: denied metadata chmod: shared.txt: chmod /tmp/TestOverlayBackendConfinementDeniesActualAbsoluteWrites2362363215/001/runtime/tasks/a65d79ce0b1134859495bad94ddb8db17ca6105c4034ee39dc13b38df35e3a97/view/shared.txt: read-only file system
            overlay_test.go:96: denied metadata utime: shared.txt: chtimes /tmp/TestOverlayBackendConfinementDeniesActualAbsoluteWrites2362363215/001/runtime/tasks/a65d79ce0b1134859495bad94ddb8db17ca6105c4034ee39dc13b38df35e3a97/view/shared.txt: read-only file system
            overlay_test.go:96: denied metadata chown: shared.txt: chown /tmp/TestOverlayBackendConfinementDeniesActualAbsoluteWrites2362363215/001/runtime/tasks/a65d79ce0b1134859495bad94ddb8db17ca6105c4034ee39dc13b38df35e3a97/view/shared.txt: read-only file system
            overlay_test.go:96: denied metadata setxattr: shared.txt: read-only file system
            overlay_test.go:96: denied metadata chmod: snapshot.json: chmod /tmp/TestOverlayBackendConfinementDeniesActualAbsoluteWrites2362363215/001/runtime/snapshots/e7c3c40b84597736c4c4188ecb6ec0f19f2bae4f54ec5f0a045f1b7afae08589/snapshot.json: read-only file system
            overlay_test.go:96: denied metadata utime: snapshot.json: chtimes /tmp/TestOverlayBackendConfinementDeniesActualAbsoluteWrites2362363215/001/runtime/snapshots/e7c3c40b84597736c4c4188ecb6ec0f19f2bae4f54ec5f0a045f1b7afae08589/snapshot.json: read-only file system
            overlay_test.go:96: denied metadata chown: snapshot.json: chown /tmp/TestOverlayBackendConfinementDeniesActualAbsoluteWrites2362363215/001/runtime/snapshots/e7c3c40b84597736c4c4188ecb6ec0f19f2bae4f54ec5f0a045f1b7afae08589/snapshot.json: read-only file system
            overlay_test.go:96: denied metadata setxattr: snapshot.json: read-only file system
            overlay_test.go:96: denied metadata chmod: overlay.json: chmod /tmp/TestOverlayBackendConfinementDeniesActualAbsoluteWrites2362363215/001/runtime/tasks/2c12b91e8492432844c5321b67955a36bc8264fc0ea971fbaa113623a1092eec/overlay.json: read-only file system
            overlay_test.go:96: denied metadata utime: overlay.json: chtimes /tmp/TestOverlayBackendConfinementDeniesActualAbsoluteWrites2362363215/001/runtime/tasks/2c12b91e8492432844c5321b67955a36bc8264fc0ea971fbaa113623a1092eec/overlay.json: read-only file system
            overlay_test.go:96: denied metadata chown: overlay.json: chown /tmp/TestOverlayBackendConfinementDeniesActualAbsoluteWrites2362363215/001/runtime/tasks/2c12b91e8492432844c5321b67955a36bc8264fc0ea971fbaa113623a1092eec/overlay.json: read-only file system
            overlay_test.go:96: denied metadata setxattr: overlay.json: read-only file system
            overlay_test.go:96: denied metadata chmod: config: chmod /tmp/TestOverlayBackendConfinementDeniesActualAbsoluteWrites2362363215/001/workspace/.git/config: read-only file system
            overlay_test.go:96: denied metadata utime: config: chtimes /tmp/TestOverlayBackendConfinementDeniesActualAbsoluteWrites2362363215/001/workspace/.git/config: read-only file system
            overlay_test.go:96: denied metadata chown: config: chown /tmp/TestOverlayBackendConfinementDeniesActualAbsoluteWrites2362363215/001/workspace/.git/config: read-only file system
            overlay_test.go:96: denied metadata setxattr: config: read-only file system
        --- PASS: TestConfinementWriteHelper (0.00s)
        PASS
--- PASS: TestOverlayBackendConfinementDeniesActualAbsoluteWrites (2.28s)
PASS
ok  	iop/packages/go/agentworkspace	2.491s
exit 0
```

### Manager-owned confinement launch

```bash
go test -count=1 -run '^(TestValidatePreparedIsolation|TestManager.*Confinement|TestDispatch.*Confinement)' -v ./packages/go/agenttask
```

```text
=== RUN   TestValidatePreparedIsolationRequiresExactConfinementProof
--- PASS: TestValidatePreparedIsolationRequiresExactConfinementProof (0.00s)
=== RUN   TestManagerOwnsConfinementStartBeforeBindingProviderInvocation
--- PASS: TestManagerOwnsConfinementStartBeforeBindingProviderInvocation (0.03s)
=== RUN   TestManagerConfinementLaunchFailuresDoNotBindOrLeakChildren
=== RUN   TestManagerConfinementLaunchFailuresDoNotBindOrLeakChildren/prepare_failure
=== RUN   TestManagerConfinementLaunchFailuresDoNotBindOrLeakChildren/proof_start_failure
=== RUN   TestManagerConfinementLaunchFailuresDoNotBindOrLeakChildren/bind_failure_reaps_child
--- PASS: TestManagerConfinementLaunchFailuresDoNotBindOrLeakChildren (0.03s)
    --- PASS: TestManagerConfinementLaunchFailuresDoNotBindOrLeakChildren/prepare_failure (0.00s)
    --- PASS: TestManagerConfinementLaunchFailuresDoNotBindOrLeakChildren/proof_start_failure (0.00s)
    --- PASS: TestManagerConfinementLaunchFailuresDoNotBindOrLeakChildren/bind_failure_reaps_child (0.03s)
PASS
ok  	iop/packages/go/agenttask	0.062s
exit 0
```

### Fresh target suites

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

```text
ok  	iop/packages/go/agentworkspace	3.005s
ok  	iop/packages/go/agentguard	0.022s
ok  	iop/packages/go/agenttask	2.150s
exit 0
```

### Repetition

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

```text
ok  	iop/packages/go/agentworkspace	32.221s
exit 0
```

### Race detector

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

```text
ok  	iop/packages/go/agentworkspace	4.707s
ok  	iop/packages/go/agenttask	2.330s
exit 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.013s
ok  	iop/packages/go/agentpolicy	0.011s
ok  	iop/packages/go/agentprovider/catalog	0.079s
ok  	iop/packages/go/agentprovider/cli	30.304s
?   	iop/packages/go/agentprovider/cli/internal/testutil	[no test files]
ok  	iop/packages/go/agentprovider/cli/status	39.941s
ok  	iop/packages/go/agentruntime	0.716s
ok  	iop/packages/go/agentstate	0.084s
ok  	iop/packages/go/agenttask	1.163s
ok  	iop/packages/go/agentworkspace	1.810s
ok  	iop/packages/go/audit	0.006s
?   	iop/packages/go/auth	[no test files]
ok  	iop/packages/go/config	0.106s
?   	iop/packages/go/events	[no test files]
ok  	iop/packages/go/hostsetup	0.012s
?   	iop/packages/go/jobs	[no test files]
?   	iop/packages/go/metadata	[no test files]
ok  	iop/packages/go/observability	0.027s
?   	iop/packages/go/policy	[no test files]
ok  	iop/packages/go/streamgate	0.888s
?   	iop/packages/go/version	[no test files]
exit 0
```

### Darwin cross-compile

```bash
tmp_build_dir=$(mktemp -d)
CGO_ENABLED=0 GOOS=darwin GOARCH=arm64 go test -c -o "$tmp_build_dir/agentworkspace.test" ./packages/go/agentworkspace
```

```text
exit 0
stdout/stderr: empty
```

### Vet

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

```text
exit 0
stdout/stderr: empty
```

### Formatting

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

```text
exit 0
stdout/stderr: empty
```

### Mandatory launch search

```bash
rg --sort path 'ProviderInvoker|\.invoker\.(Start|Prepare)|Confinement\.Start' packages/go
```

```text
packages/go/agenttask/dispatch.go:                prepared, err := m.invoker.Prepare(invokeCtx, dispatchRequest)
packages/go/agenttask/dispatch.go:                child, err := isolation.Confinement.Start(invokeCtx, command)
packages/go/agenttask/manager.go: invoker    ProviderInvoker
packages/go/agenttask/manager.go: invoker ProviderInvoker,
packages/go/agenttask/ports.go:type ProviderInvoker interface {
packages/go/agenttask/ports.go: // Manager invokes the returned command through InvocationConfinement.Start.
packages/go/agentworkspace/overlay_test.go: command, err := first.Confinement.Start(
exit 0
```

### Diff integrity

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

```text
exit 0
stdout/stderr: empty
```

---

> **[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 — an invoker can pass a pre-opened protected file as child stdio, and the confined child can mutate that canonical file despite the read-only mount namespace.
  - Completeness: Fail — S18 requires canonical files, sibling layers, snapshots, overlay records, and shared Git metadata to remain non-writable through the complete Manager-owned launch boundary.
  - Test coverage: Fail — the current OS helper opens protected paths only after confinement and never exercises inherited or host-forwarded writable descriptors.
  - API contract: Fail — `ConfinementCommand` permits arbitrary `io.Reader`/`io.Writer` handles although the standalone runtime contract explicitly forbids passing a pre-opened writable descriptor for a protected path.
  - Code quality: Pass — the reviewed changes are formatted, focused, and free of debug residue.
  - Implementation deviation: Fail — Manager owns `Start`, but the launch plan still controls the child's inherited stdio capabilities, so the claimed mandatory proof boundary is not complete.
  - Verification trust: Pass — fresh focused, target, repetition, race, package-wide, Darwin cross-compile, vet, formatting, search, and diff checks matched the recorded outputs; the missing descriptor case is a coverage gap rather than fabricated evidence.
  - Spec conformance: Fail — S18's canonical-unchanged trace and the contract's no-pre-opened-writable-descriptor rule are violated by the current launch API.
- Findings:
  - Required — `packages/go/agenttask/ports.go:209` and `packages/go/agentworkspace/confinement.go:163`: `ConfinementCommand` accepts arbitrary `Stdin`, `Stdout`, and `Stderr` interfaces and `ConfinementProof.Start` attaches them to the child after the caller has opened them. A focused reviewer regression opened a canonical file with `O_WRONLY|O_APPEND`, supplied it as `Stdout`, started `sh -c "printf 'bypass\n'"` through the real proof, and observed the canonical file change while the mount namespace was read-only. This directly violates `agent-contract/inner/iop-agent-cli-runtime.md:75-76` and SDD S18. Remove caller-supplied inherited I/O from the launch command; have the confinement owner create safe stdin/stdout/stderr pipes (or an equivalently non-forgeable started-process handle) and pass those handles to `ProviderLaunch.BindStarted`, then add real-proof and Manager-level regressions proving a pre-opened protected descriptor cannot reach the child or alter canonical evidence.
- Routing Signals:
  - `review_rework_count=3`
  - `evidence_integrity_failure=false`
- Next Step: Invoke the plan skill with this raw finding, complete isolated reassessment, archive this pair, and materialize the routed follow-up pair.
