<!-- task=m-iop-agent-cli-runtime/26+19,21,22,23,25_parity_cutover plan=3 tag=REVIEW_REFACTOR -->

# Code Review Reference - REVIEW_REFACTOR

> **[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-31
task=m-iop-agent-cli-runtime/26+19,21,22,23,25_parity_cutover, plan=3, tag=REVIEW_REFACTOR

## 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:
  - `parity-cutover`: classify Python/Node behavior and cut production task-loop ownership to Go with zero unclassified/runtime dependency/duplicate
- Completion mode: check-on-pass

## Archive Evidence Snapshot

- Prior task: `agent-task/m-iop-agent-cli-runtime/26+19,21,22,23,25_parity_cutover`.
- Prior verdict: FAIL in `code_review_cloud_G08_2.log`; Required=3, Suggested=0, Nit=0.
- Prior plan: `plan_cloud_G08_2.log`.
- Required finding 1: `canonicalizeCandidate` ignored a broken symlink leaf after `EvalSymlinks` returned `ENOENT`; a reviewer probe for an external target exited 0.
- Required finding 2: `cutoverProductionDocuments` and its injected-caller test supplied a fixed path list, so a new active common/project/rules document could call the retained dispatcher without failing the production guard.
- Required finding 3: retained-mode validation requires every reference path to exist, while the recorded transition requires deletion followed by the same parity suite; no disposed state can produce the promised S13 evidence.
- Fresh reviewer evidence: focused, race, vet, parity, build, built validator/parity/dry-run, broad zero-match, and full Go suites passed with `/tmp` cache roots. Fresh `gofmt -d` contradicted the recorded formatting claim; the reviewer repaired only that non-behavioral drift.
- Roadmap carryover: approved SDD S13 and its Evidence Map still require symlink-resolved write-set containment, trustworthy zero Python production callers, and executable checksum-bound physical-disposal evidence.

## 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-G08.md` → `code_review_cloud_G08_3.log` and `PLAN-cloud-G08.md` → `plan_cloud_G08_3.log`.
3. If PASS, write `complete.log` and move active task directory to `agent-task/archive/YYYY/MM/m-iop-agent-cli-runtime/26+19,21,22,23,25_parity_cutover/`. 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_REFACTOR-1 Symlink-complete PLAN containment | [x] |
| REVIEW_REFACTOR-2 Production caller discovery | [x] |
| REVIEW_REFACTOR-3 Executable disposal lifecycle | [x] |

## Implementation Checklist

- [x] Make PLAN write-claim canonicalization resolve nonexistent paths through every symlink component, normalize contained broken-symlink targets, reject external or looping aliases, and add deterministic regression coverage.
- [x] Replace fixed production document enumeration with deterministic active-root discovery for Python callers while retaining bounded static-route and exact Go-validator ownership checks, with an unlisted injected-caller regression.
- [x] Add an explicit retained/disposed disposal lifecycle that validates exact hashes before deletion, exact absence after transition, mixed-state rejection, and stable operator/contract evidence without deleting live fixtures.
- [x] Run fresh formatting, focused, race, vet, parity, built-command, zero-match, and full Go verification with repository-external cache/tmp roots.
- [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_G08_3.log`.
- [x] Archive active `PLAN-*-G??.md` to `plan_cloud_G08_3.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/26+19,21,22,23,25_parity_cutover/` to `agent-task/archive/YYYY/MM/m-iop-agent-cli-runtime/26+19,21,22,23,25_parity_cutover/` 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

None.

## Key Design Decisions

- The canonicalizer follows a dangling leaf symlink before retaining a truly
  nonexistent suffix, so containment is checked against its physical target.
- Production caller discovery walks the declared active roots, excludes test,
  hidden-cache, and retained-reference-script directories, and leaves static
  provider/model checks bounded to the S13 ownership documents.
- The parity manifest has strict `retained` and `disposed` states. Retained
  mode verifies the exact inventory and checksums; disposed mode requires the
  recorded inventory and discovery result to be empty.

## Reviewer Checkpoints

- Broken leaf symlinks are resolved to their physical target even when absent; contained targets normalize and external or looping targets fail.
- Nested safe new-file paths remain valid and runtime workflow admission uses the same canonicalizer as the standalone validator.
- Python caller ownership scans discover new production files below the declared active roots without a hand-maintained file list.
- Test fixtures and the exact retained reference subtree remain excluded, while unrelated production scripts remain guarded.
- Static provider/model/capacity checks stay bounded to S13 orchestration ownership documents, and common plan/review skills retain the exact Go validator command.
- Retained mode requires exact discovered inventory and checksum equality; disposed mode requires exact absence, zero retained discovery, and rejects partial or mixed state.
- The nine live retained fixtures are not edited or deleted in this follow-up.
- SDD S13 and `iop.agent-cli-runtime` describe the same two-phase disposal evidence.

## Verification Results

### PLAN containment

```bash
TMPDIR=/tmp/t GOCACHE=/tmp/iop-s0-parity-third-gocache GOTMPDIR=/tmp/t \
  go test -count=1 ./apps/agent/internal/taskloop -run 'Test.*ValidatePlan'
```

```text
TMPDIR=/tmp/t GOCACHE=/tmp/iop-s0-parity-third-gocache GOTMPDIR=/tmp/t \
  go test -count=1 ./apps/agent/internal/taskloop -run 'Test.*ValidatePlan'
ok  iop/apps/agent/internal/taskloop
```

### Production caller and disposal lifecycle

```bash
TMPDIR=/tmp/t GOCACHE=/tmp/iop-s0-parity-third-gocache GOTMPDIR=/tmp/t \
  go test -count=1 ./apps/agent/internal/taskloop -run 'Test.*(Parity|Disposition|Disposal|Cutover)'
```

```text
TMPDIR=/tmp/t GOCACHE=/tmp/iop-s0-parity-third-gocache GOTMPDIR=/tmp/t \
  go test -count=1 ./apps/agent/internal/taskloop -run 'Test.*(Parity|Disposition|Disposal|Cutover)'
ok  iop/apps/agent/internal/taskloop  0.348s
```

### Final verification

```bash
mkdir -p /tmp/iop-s0-parity-third-gocache /tmp/t
gofmt -w apps/agent/internal/taskloop/workflow.go apps/agent/internal/taskloop/workflow_test.go apps/agent/internal/taskloop/parity.go apps/agent/internal/taskloop/parity_test.go apps/agent/internal/taskloop/cutover_test.go
test -z "$(gofmt -d apps/agent/internal/taskloop/workflow.go apps/agent/internal/taskloop/workflow_test.go apps/agent/internal/taskloop/parity.go apps/agent/internal/taskloop/parity_test.go apps/agent/internal/taskloop/cutover_test.go)"
TMPDIR=/tmp/t GOCACHE=/tmp/iop-s0-parity-third-gocache GOTMPDIR=/tmp/t go test -count=1 ./apps/agent/internal/taskloop ./apps/agent/internal/command ./apps/agent/cmd/agent
TMPDIR=/tmp/t GOCACHE=/tmp/iop-s0-parity-third-gocache GOTMPDIR=/tmp/t go test -count=1 -race ./apps/agent/internal/taskloop ./apps/agent/internal/command ./apps/agent/cmd/agent
TMPDIR=/tmp/t GOCACHE=/tmp/iop-s0-parity-third-gocache GOTMPDIR=/tmp/t go vet ./apps/agent/internal/taskloop ./apps/agent/internal/command ./apps/agent/cmd/agent
TMPDIR=/tmp/t GOCACHE=/tmp/iop-s0-parity-third-gocache GOTMPDIR=/tmp/t make test-iop-agent-parity
TMPDIR=/tmp/t GOCACHE=/tmp/iop-s0-parity-third-gocache GOTMPDIR=/tmp/t make build-agent
build/bin/iop-agent task-loop validate-plan --workspace /config/workspace/iop-s0 agent-task/m-iop-agent-cli-runtime/26+19,21,22,23,25_parity_cutover/PLAN-cloud-G08.md
build/bin/iop-agent task-loop parity --disposal-manifest
build/bin/iop-agent task-loop --dry-run --task-group m-iop-agent-cli-runtime
rg -n --sort path 'python3 .*orchestrate-agent-task-loop|dispatch\.py --workspace|dispatch\.py' agent-ops/skills/common agent-ops/skills/project agent-ops/rules --glob '!**/tests/**' --glob '!agent-ops/skills/project/orchestrate-agent-task-loop/scripts/**'
TMPDIR=/tmp/t GOCACHE=/tmp/iop-s0-parity-third-gocache GOTMPDIR=/tmp/t go test -count=1 ./packages/go/... ./apps/agent/...
git diff --check
```

```text
gofmt check: exit 0
go test ./apps/agent/internal/taskloop ./apps/agent/internal/command ./apps/agent/cmd/agent: exit 0
ok  iop/apps/agent/internal/taskloop  6.111s
ok  iop/apps/agent/internal/command  0.016s
ok  iop/apps/agent/cmd/agent  4.604s
go test -race ./apps/agent/internal/taskloop ./apps/agent/internal/command ./apps/agent/cmd/agent: exit 0
go vet ./apps/agent/internal/taskloop ./apps/agent/internal/command ./apps/agent/cmd/agent: exit 0
make test-iop-agent-parity: exit 0
ok  iop/apps/agent/internal/taskloop  0.280s
make build-agent: exit 0
build/bin/iop-agent task-loop validate-plan ...PLAN-cloud-G08.md: exit 0
build/bin/iop-agent task-loop parity --disposal-manifest: exit 0
parity: ok
  schema_version: 1
  behaviors: 13
  disposal_state: retained
  disposal_files: 9
  deletion_gate: milestone-completion-transition
build/bin/iop-agent task-loop --dry-run --task-group m-iop-agent-cli-runtime: exit 0
task-loop dry_run=true projects=1
  - project=workspace group=m-iop-agent-cli-runtime status=read-only next=26
  exit_code: 0
production caller rg search: exit 1 (no matches; expected)
go test ./packages/go/... ./apps/agent/...: exit 0
git diff --check: exit 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 |
| 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 — deep literal new-file paths are rejected after the non-symlink component count reaches the symlink-resolution limit, and disposed manifests can discard part of the checksum-bound historical inventory.
  - Completeness: Fail — the implementation does not preserve both promised invariants across all admitted nested paths and the retained-to-disposed evidence transition.
  - Test Coverage: Fail — the current matrices stop at a three-component new path and retain the complete disposal list during disposed-state validation, so neither reviewer reproducer is covered.
  - API Contract: Fail — the shared `PlanWriteSet` boundary admits contained literal files without a documented depth limit, while SDD S13 and `iop.agent-cli-runtime` require checksum-bound physical-disposal evidence to remain trustworthy after transition.
  - Code Quality: Pass — the changes are focused, formatted, and free of debug or dead-code drift.
  - Implementation Deviation: Pass — the implementation stayed within the planned taskloop and S13 contract boundaries.
  - Verification Trust: Fail — all recorded commands pass on the current checkout, but fresh reviewer execution contradicts the claimed production-path completeness for deep nested targets and immutable disposal evidence.
  - Spec Conformance: Fail — approved SDD S13 and its Evidence Map are not satisfied while the disposed verifier can forget recorded fixtures, and the shared PLAN containment contract is incomplete for valid deep new paths.
- Findings:
  - Required — `apps/agent/internal/taskloop/workflow.go:581` and `apps/agent/internal/taskloop/workflow_test.go:199`: `canonicalizeCandidateSeen` increments `depth` for every nonexistent parent component and compares that value with `maxCandidateSymlinkResolutions`, so a contained literal path with 256 nonexistent directory components fails with `too many symlink resolutions` even though it has no symlink. The reviewer reproducer observed this exact failure. Track actual symlink traversals separately from reconstruction of the nonexistent suffix, or iteratively resolve the nearest existing ancestor, and add a regression proving a deep contained new-file path succeeds while real symlink loops still fail.
  - Required — `apps/agent/internal/taskloop/parity.go:166` and `apps/agent/internal/taskloop/parity_test.go:122`: disposed mode validates absence only for the entries still present in `Disposal.ReferenceFiles`; after deleting every fixture, truncating that list makes validation return success and silently drops checksum-bound historical evidence. The reviewer reproducer observed the truncated inventory pass. Require exact equality between the classified reference-source inventory and the disposal inventory before branching on state, retain checksum metadata across the transition, and add a disposed-state regression that rejects a missing historical entry.
- Routing Signals:
  - `review_rework_count=3`
  - `evidence_integrity_failure=true`
- Next Step: Invoke the plan skill with these raw findings and materialize the freshly routed follow-up pair; do not write `complete.log` or update the roadmap.
