<!-- task=m-iop-agent-cli-runtime/16+13,15_bootstrap_composition plan=0 tag=API -->

# Code Review Reference - API

> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is mandatory.**
> Complete the checklist and implementation-owned evidence, leave active files in place, and report ready for review. Finalization is review-agent-only.

## Overview

date=2026-07-29
task=m-iop-agent-cli-runtime/16+13,15_bootstrap_composition, plan=0, tag=API

## For the Review Agent

> **[REVIEW AGENT ONLY]** Implementing agents must not execute finalization.

Compare bootstrap composition and actual output to source. Append verdict/signals; archive `CODE_REVIEW-cloud-G05.md` → `code_review_cloud_G05_0.log` and `PLAN-local-G05.md` → `plan_local_G05_0.log`; on PASS write `complete.log` and archive the task; on WARN/FAIL materialize the next state.

## Implementation Item Completion

| Item | Status |
|------|--------|
| API-3 Bootstrap composition | [x] |

## Implementation Checklist

- [x] Add and test bootstrap composition with deterministic startup rollback and reverse shutdown.
- [x] Run fresh focused, race, 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]** Implementing agents must not modify this checklist.

- [x] Append one verdict and verified routing signals.
- [x] Verify verdict, dimensions, and finding classifications.
- [x] Archive `CODE_REVIEW-cloud-G05.md` as `code_review_cloud_G05_0.log`.
- [x] Archive `PLAN-local-G05.md` as `plan_local_G05_0.log`.
- [x] Verify the Agent-Ops managed `.gitignore` block.
- [ ] If PASS, write canonical `complete.log` and leave no active Markdown files.
- [ ] If PASS, move this task directory to the dated archive and update this checklist there.
- [ ] If PASS, remove an empty split parent or prove remaining siblings/files require it.
- [x] If WARN/FAIL, write the exact next state without `complete.log`.

## Deviations from Plan

None.

## Key Design Decisions

1. `Module` wraps `host.Host` and delegates `Run`/`Close` directly — no duplicate lifecycle policy. Construction validates all inputs before any side effect.
2. `resolveComponentName` falls back to a synthetic name (`component-xxx`) when a component does not implement `host.Namer` or returns an empty name, ensuring every component has a unique, traceable identity.
3. Startup failure rollback uses `context.WithoutCancel` for the rollback context so already-started components receive a clean cancellation signal without inheriting the failed launch context's deadline.
4. `ErrDuplicateName` is exported as a sentinel for callers that want to branch on duplicate-name validation failures specifically.

## Reviewer Checkpoints

- Construction is side-effect free and rejects invalid dependencies.
- Run/Close delegates to the completed lifecycle without duplicating its policy.
- Startup failure cleanup and reverse shutdown retain exact errors.

## Verification Results

### Formatting

```bash
gofmt -w apps/agent/internal/bootstrap/*.go
```

```
gofmt: OK
```

### Focused and race tests

```bash
go test -count=1 ./apps/agent/internal/host ./apps/agent/internal/bootstrap
go test -count=1 -race ./apps/agent/internal/host ./apps/agent/internal/bootstrap
```

```
ok  	iop/apps/agent/internal/host	0.002s
ok  	iop/apps/agent/internal/bootstrap	0.002s
ok  	iop/apps/agent/internal/host	1.006s
ok  	iop/apps/agent/internal/bootstrap	1.006s
```

### Static checks

```bash
go vet ./apps/agent/internal/host ./apps/agent/internal/bootstrap
git diff --check
```

```
(no output — no issues)
(no output — no issues)
```

---

> **[IMPLEMENTING AGENT — BEFORE SAVING]** Fill every implementation-owned section and leave review-only content unchanged.

## Section Ownership

| Section | Owner | Note |
|---------|-------|------|
| Header, Overview, Review Instructions | Fixed | Implementer must not modify |
| Implementation Item Completion, Implementation Checklist | Implementing agent | Check status only |
| Review-Only Checklist | Review agent | Implementer must not modify |
| Deviations, Decisions, Verification Results | Implementing agent | Record actual evidence |
| Reviewer Checkpoints | Fixed | Reviewer verifies them |
| Code Review Result and finalization | Review agent | Append/execute only after review |

## Code Review Result

### Overall Verdict

FAIL

### Dimension Assessment

| Dimension | Assessment | Evidence |
|-----------|------------|----------|
| Correctness | Fail | `NewModule` panics for a typed-nil `host.Component` and can accept two components that `host.Host` later resolves to the same status identity. |
| Completeness | Fail | The required nil/duplicate dependency validation is not complete across Go interface nils and the actual host naming boundary. |
| Test Coverage | Fail | The required validation tests cover only a nil interface and duplicate explicit names; they omit typed-nil inputs, unnamed/empty-name components, host-name collisions, and sentinel identity. |
| API Contract | Fail | `ErrDuplicateName` is exported for `errors.Is` branching but duplicate-name errors do not wrap or return it. |
| Code Quality | Pass | The implementation is formatted, scoped to the planned files, and contains no debug residue or unrelated framework additions. |
| Implementation Deviation | Fail | The implementation notes claim a usable duplicate-name sentinel and unique traceable fallback identities, but the production behavior does not provide either guarantee. |
| Verification Trust | Pass | Fresh focused tests, race tests, vet, formatting, and `git diff --check` reproduced the reported passing outputs; the defect is an uncovered case rather than fabricated command evidence. |

### Findings

- **Required** — `apps/agent/internal/bootstrap/module.go:39`: dependency identity validation is not sound across the boundary it constructs. A typed-nil component reaches `Name()` and panics; `bootstrap.resolveComponentName` assigns the first unnamed component `component-x` while `host.resolveName` assigns it `component-`, so `NewModule(reviewProbeUnnamedComponent{}, named("component-"))` succeeds and produces duplicate host identities; and the duplicate error at line 47 does not wrap the exported `ErrDuplicateName` at line 87. Centralize or preserve one resolved-name algorithm through host construction, reject typed-nil components before invoking their methods, return/wrap `ErrDuplicateName`, and add deterministic regression tests for all three observed variants.

Reviewer reproducer:

```text
typed_nil_is_rejected_without_panic: panic: runtime error: invalid memory address or nil pointer dereference
duplicate_error_exposes_sentinel: bootstrap: duplicate component name "same" does not match ErrDuplicateName
validated_names_match_host_status_identities: accepted [component- component-]
```

### Routing Signals

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

### Next Step

Invoke the plan skill in `prepare-follow-up` mode with the raw dependency-validation failures and fresh verification evidence. Archive this pair only after the routed follow-up PLAN validates successfully.
