iop/agent-task/m-iop-agent-cli-runtime/plan_cloud_G06_1.log

<!-- task=m-iop-agent-cli-runtime plan=1 tag=REVIEW_API -->

# Plan - Fail-closed Milestone catalog and trustworthy S10 evidence

## For the Implementing Agent

Filling the implementation-owned sections in `CODE_REVIEW-*-G??.md` is mandatory. Implement the checklist, run every verification command, paste actual notes and output into the active review file, leave the active Plan/Review files in place, and report ready for review. Final verdicts, log renames, `complete.log`, and archive moves belong only to the code-review skill. If blocked, record only the exact blocker, attempted commands/output, and resume condition in implementation-owned evidence fields; do not ask the user, call user-input tools, create control-plane stop files, classify the next state, archive logs, or write `complete.log`.

## Background

The first review found that the new Milestone catalog silently omits malformed workflows, accepts glob-bearing identities, and reports an empty catalog as success instead of preserving the runtime's fail-closed contract. It also found that the named adapter and binary tests pass without proving the claimed two-work status projection or exact stdout/stderr transcript. This follow-up closes both Required findings without changing the established DTO or shared runtime ownership.

## Archive Evidence Snapshot

- `agent-task/m-iop-agent-cli-runtime/plan_local_G07_0.log`: first-pass S10 CLI catalog/status implementation plan.
- `agent-task/m-iop-agent-cli-runtime/code_review_cloud_G07_0.log`: FAIL with 2 Required, 0 Suggested, and 0 Nit findings.
- Required production finding: exact Milestone/task-group identifiers, archive collision suffixes, malformed workflows, archive traversal errors, and empty catalogs must fail closed or normalize only by the runtime-defined archive rule.
- Required evidence finding: `TestAdapterStatusPreservesAllWork` was vacuous, and the in-process/built-binary transcripts used substring assertions without proving per-work projection or exact stdout/stderr.
- Reviewer evidence: all three focused implementation suites passed, but focused negative reproducers proved malformed selected workflow returned empty success, `m-*` consumed another archived group, and an empty workspace returned empty success.
- Roadmap carryover: `cli-surface` remains the only unchecked Milestone Task and still requires exact `Roadmap Completion` plus trustworthy S10 evidence.

## 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:
  - `cli-surface`: headless binary, split config, Milestone discovery/selection/preview, lifecycle, and per-work overlay/integration/blocker observation
- Completion mode: check-on-pass

## Analysis

### Files Read

- Production: `apps/agent/internal/taskloop/workflow.go`, `apps/agent/internal/taskloop/module.go`, `apps/agent/cmd/agent/main.go`, `apps/agent/internal/command/service.go`, `apps/agent/internal/command/root.go`, `apps/agent/internal/bootstrap/module.go`, `packages/go/agenttask/types.go`.
- Tests: `apps/agent/internal/taskloop/workflow_test.go`, `apps/agent/internal/taskloop/module_test.go`, `apps/agent/cmd/agent/main_test.go`, `apps/agent/internal/command/root_test.go`, `apps/agent/internal/bootstrap/module_test.go`.
- Contract/roadmap: `agent-contract/index.md`, `agent-contract/inner/iop-agent-cli-runtime.md`, `agent-roadmap/phase/automation-runtime-bridge/PHASE.md`, `agent-roadmap/phase/automation-runtime-bridge/milestones/iop-agent-cli-runtime.md`, `agent-roadmap/sdd/automation-runtime-bridge/iop-agent-cli-runtime/SDD.md`.
- Rules/context: `agent-ops/rules/project/domain/agent/rules.md`, `agent-ops/rules/project/domain/testing/rules.md`, `agent-test/local/rules.md`, `agent-test/local/testing-smoke.md`, `agent-roadmap/current.md`.

### SDD Criteria

- SDD: `agent-roadmap/sdd/automation-runtime-bridge/iop-agent-cli-runtime/SDD.md`; status `[approved]`, lock released.
- Target: Acceptance Scenario `S10`, Milestone Task `cli-surface`.
- Evidence Map: binary entry point, split configuration, validation, discovery, selection, lifecycle, and status commands; completion requires `cli-surface` Roadmap Completion and a headless operation transcript.
- The S10 row makes catalog fail-closed behavior, list/select parity, per-work status fidelity, and compiled-binary stream assertions part of the implementation checklist and final verification.

### Verification Context

- Supplied handoff: none. Repository-native fallback used the active pair, current source/tests, S10 contract/SDD, agent/testing domain rules, and local testing profile.
- Environment: local checkout `/config/workspace/iop-s0`; branch `feature/iop-agent-cli-runtime`; HEAD `8760d165105fb03b0b8b62b55dd31c90f34daa44`; intentional task changes and unrelated pre-existing Agent-Ops changes are present.
- Toolchain preflight: `/config/.local/bin/go`; `go version go1.26.2 linux/arm64`; `GOROOT=/config/opt/go`.
- Reviewer commands: fresh API-1/API-2/API-3 focused suites passed. A focused negative reproducer failed as expected with three concrete violations: malformed selected workflow returned `[]` without error, `m-*` returned archived `m-one` work, and an empty workspace returned `[]` without error.
- Applied criteria: fresh `-count=1` tests, deterministic fixtures, no real provider process, built binary transcript, race/vet/build/Darwin cross-build, logged-smoke preflight, deterministic symbol search, and `git diff --check`.
- Constraints: do not launch a real provider CLI; do not modify shared manager state semantics, DTO shape, roadmap state, or Agent-Ops common files. Existing task-owned files remain in the write claim until PASS even when no new edit is expected.
- Gaps/confidence: the defects and missing assertions are directly reproducible; confidence is high.
- External Verification Preflight: not applicable. Darwin is a local cross-build, and live logged-in macOS/provider evidence remains the already completed S14 scope.

### Test Coverage Gaps

- Exact identity: no test rejects glob metacharacters, uppercase/underscore/non-canonical slugs, or path-like task-group inputs across selection and config-free inspection.
- Catalog failure: no test requires `Milestones` to propagate malformed selected workflow evidence, archive traversal errors, or the no-workflow condition.
- Archive collision: no test proves `m-<slug>_<n>` archive collision directories normalize to the original Milestone instead of becoming selectable pseudo-Milestones.
- Adapter projection: `TestAdapterStatusPreservesAllWork` creates no durable works and asserts only the project id.
- Transcript fidelity: in-process and built-binary tests use substring checks; built-binary output merges stdout/stderr and neither transcript proves the two-work status fields.

### Symbol References

- No existing public symbol is renamed or removed.
- Add one internal canonical Milestone/task-group parser and use it from `scanWorkflow`, `scanMilestones`, `InspectTaskGroup`, and selection through the existing `scanWorkflow` call.
- `MilestoneView`, `WorkView`, `MilestoneEntry`, `WorkStatusEntry`, and their current call sites remain unchanged.

### Split Judgment

Keep one plan. The production fix and evidence repair share one invariant: every catalog entry and operator-supplied task group must resolve to the same exact workflow parser, and the CLI evidence must prove that parser plus the resulting per-work projection through the compiled entry point. Splitting would permit another untrustworthy PASS boundary.

### Scope Rationale

- Keep `MilestoneEntry`, `StatusResponse`, `WorkStatusEntry`, and `WorkView` shapes unchanged; the review found missing validation/evidence, not a DTO design defect.
- Do not change `agenttask.Manager`, provider execution, lifecycle transitions, durable state schema, config schema, or local-control behavior.
- Do not update the active roadmap or SDD. PASS completion metadata remains runtime-owned.
- Do not modify Agent-Ops common/project skills or unrelated staged work.
- Keep prior task-owned source/contract files in `Modified Files Summary` so shared-checkout ownership is not released before final review; only the files named in REVIEW_API-1/2 are expected to receive new implementation edits.

### Final Routing

- `evaluation_mode=isolated-reassessment`; finalizer `finalize-task-policy.sh`, mode `pair`.
- Build closures: scope/context/verification/evidence/ownership/decision all `true`; capability gap absent.
- Build grade: scope `1`, state `0`, blast `1`, evidence `2`, verification `2` = `G06`; base `local-fit`, final `recovery-boundary`, lane `cloud`.
- Review closures: all `true`; grade scores `2/0/1/2/2` = `G07`; route `official-review`, lane `cloud`, adapter `codex`, model `gpt-5.6-sol`, reasoning `xhigh`.
- `large_indivisible_context=false`; positive loop risks: `boundary_contract`, `structured_interpretation` (`loop_risk_count=2`).
- Recovery signals: `review_rework_count=1`, `evidence_integrity_failure=true`; recovery boundary matched.
- Canonical files: `PLAN-cloud-G06.md`, `CODE_REVIEW-cloud-G07.md`.

## Implementation Checklist

- [ ] [REVIEW_API-1] Enforce one canonical Milestone/task-group identity and archive-collision parser across catalog, selection, and inspection; propagate malformed/traversal errors, reject empty catalogs, and add normal/boundary regressions.
- [ ] [REVIEW_API-2] Replace vacuous adapter/transcript evidence with deterministic two-work status assertions and exact in-process/built-binary stdout/stderr coverage for catalog, selection, lifecycle, and per-work projection.
- [ ] [REVIEW_API-3] Run the complete fresh local S10 verification matrix without real provider execution and record exact output.
- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.

### [REVIEW_API-1] Fail-closed Milestone identity and archive discovery

**Problem**

`apps/agent/internal/taskloop/workflow.go:206-255` admits any `m-` suffix that lacks separators, uses wildcard archive discovery, ignores archive stat/traversal errors, and discards `scanWorkflow` failures. `apps/agent/internal/taskloop/workflow.go:283-286` also labels its input exact while accepting glob metacharacters. The reviewer reproduced all three known variants: malformed selected workflow and empty catalog returned successful empty lists, while `m-*` consumed work from another archived Milestone.

Before (`apps/agent/internal/taskloop/workflow.go:206-255`):

```go
if entry.IsDir() && strings.HasPrefix(entry.Name(), taskGroupPrefix) {
    slug := strings.TrimPrefix(entry.Name(), taskGroupPrefix)
    if !strings.ContainsAny(slug, "/\\\x00\r\n") {
        candidates[slug] = struct{}{}
    }
}
// ...
units, _, err := scanWorkflow(root, slug)
if err != nil || len(units) == 0 {
    continue
}
```

**Solution**

- Define one ASCII canonical slug rule matching `[a-z0-9][a-z0-9-]*`; reject glob, separator, whitespace, uppercase, underscore, empty, and path-like identities before constructing any path or archive matcher.
- Parse archive group directories as either exact `m-<slug>` or the runtime archive collision form `m-<slug>_<positive integer>`, normalizing the latter to `<slug>` without exposing it as a selectable pseudo-Milestone.
- Enumerate active/year/month/archive directories with `os.ReadDir`/`Lstat`-style checks instead of user-influenced glob patterns; allow exact not-exist only where optional and propagate all other errors.
- Make catalog candidates fail closed when their workflow is malformed, return an explicit error when no workflow-backed Milestone exists, and keep deterministic deduplication/sorting.
- Apply the same canonical parser to `scanWorkflow`, `SelectMilestone` through that call, and `InspectTaskGroup`.

After:

```go
slug, err := parseMilestoneTaskGroup(group, false)
if err != nil {
    return nil, err
}
candidates, err := discoverMilestoneCandidates(root)
if err != nil {
    return nil, err
}
for _, slug := range candidates {
    units, _, err := scanWorkflow(root, slug)
    if err != nil {
        return nil, err
    }
    // append deterministic summary
}
if len(views) == 0 {
    return nil, errors.New("taskloop: no workflow-backed milestones")
}
```

**Modified Files and Checklist**

- [ ] `apps/agent/internal/taskloop/workflow.go`: canonical identity/archive parser, strict directory discovery, propagated errors, empty-catalog failure.
- [ ] `apps/agent/internal/taskloop/workflow_test.go`: exact identity, malformed/empty catalog, collision suffix, and list/select parity regression matrix.

**Test Strategy**

- Extend `TestWorkflowMilestonesListsSelectableTaskGroups` to select every returned id and require exact order/counts.
- Add `TestWorkflowMilestonesFailClosedBoundaryMatrix` for malformed selected workflow, no workflows, invalid active/archive identities, and deterministic errors.
- Add `TestInspectTaskGroupRejectsInvalidIdentifiers` for `m-*`, `m-foo?`, uppercase, underscore, whitespace, separators, and empty suffixes.
- Add `TestWorkflowMilestonesNormalizesArchiveCollisionSuffixes` for base plus `_1`/`_2` archive directories, one canonical list entry, and exact archived completion counts.

**Verification**

```bash
TMPDIR=/tmp GOTMPDIR=/tmp GOCACHE=/tmp/iop-cli-followup-cache go test -count=1 ./apps/agent/internal/taskloop -run '^(TestWorkflowMilestonesListsSelectableTaskGroups|TestWorkflowMilestonesFailClosedBoundaryMatrix|TestInspectTaskGroupRejectsInvalidIdentifiers|TestWorkflowMilestonesNormalizesArchiveCollisionSuffixes|TestWorkflowArchiveOnlyMilestoneRemainsSelectable|TestInspectTaskGroupArchiveOnly)$'
```

Expected: PASS; invalid identities and malformed/empty catalogs return deterministic errors, while base/collision archive directories produce one selectable Milestone.

### [REVIEW_API-2] Trustworthy adapter and compiled-binary S10 transcript

**Problem**

`apps/agent/cmd/agent/main_test.go:473-486` names a two-work preservation test but creates no work records and checks only `status.Project`. `apps/agent/cmd/agent/main_test.go:490-652` checks output substrings; the compiled-binary path uses `CombinedOutput`, so it cannot prove exact stdout or empty stderr and never asserts the claimed ordered work projection.

Before (`apps/agent/cmd/agent/main_test.go:473-486`):

```go
status, err := adapter.Status(ctx, command.StatusRequest{Config: paths, Project: "iop-s0"})
if err != nil {
    t.Fatalf("Status error: %v", err)
}
if status.Project != "iop-s0" {
    t.Fatalf("status = %#v", status)
}
```

**Solution**

- Seed or drive two durable `WorkRecord` values through the existing deterministic lifecycle fixture and call the real command adapter.
- Assert exact ordered ids, states, overlay modes, integration outcomes, durable dispatch ordinals, scoped blocker identity/retryability, and compatible top-level blocker aggregation.
- Give both transcript tests exact expected stdout and expected-empty stderr per step. For the subprocess test, attach separate buffers instead of `CombinedOutput`.
- Require exact two-entry Milestone catalog summaries, select only ids returned by that catalog, verify selection/lifecycle persistence across invocations, and include an exact per-work status step in the compiled-binary transcript.
- Keep provider-deny/no-op confinement boundaries; no real provider CLI may execute.

After:

```go
cmd.Stdout = &stdout
cmd.Stderr = &stderr
err := cmd.Run()
if err != nil || stdout.String() != step.wantStdout || stderr.String() != step.wantStderr {
    t.Fatalf("command %v: err=%v stdout=%q stderr=%q", step.args, err, stdout.String(), stderr.String())
}
```

**Modified Files and Checklist**

- [ ] `apps/agent/cmd/agent/main_test.go`: meaningful adapter state fixture/assertions and exact in-process/built-binary transcripts.

**Test Strategy**

- Rewrite `TestAdapterStatusPreservesAllWork` to assert two durable work entries and both scoped/top-level blockers.
- Strengthen `TestRunMilestoneListAndSelectionShareCatalog` to parse or exactly compare the catalog and select every listed id.
- Strengthen `TestRunFullHeadlessS10Transcript` and `TestBuiltBinaryHeadlessS10Transcript` with exact stdout/stderr, lifecycle persistence, and two-work status output.
- Reuse proof-owned fake/no-op child processes only; assert no real provider invocation.

**Verification**

```bash
TMPDIR=/tmp GOTMPDIR=/tmp GOCACHE=/tmp/iop-cli-followup-cache go test -count=1 ./apps/agent/internal/command ./apps/agent/cmd/agent -run '^(TestCommandMatrix|TestCommandJSONOutputMatrix|TestStatusIncludesOverlayIntegrationAndBlockers|TestStableTextAndJSONOutput|TestRunMilestoneListAndSelectionShareCatalog|TestAdapterStatusPreservesAllWork|TestRunFullHeadlessS10Transcript|TestBuiltBinaryHeadlessS10Transcript)$'
```

Expected: PASS with exact deterministic text/JSON/stdout/stderr and no real provider process.

### [REVIEW_API-3] Fresh S10 closure verification

**Problem**

The first-pass focused commands passed despite not exercising the failing catalog boundaries or claimed adapter projection. S10 completion therefore needs a fresh matrix whose named tests contain meaningful assertions and whose full-cycle step is the compiled binary.

**Solution**

- Run every command below with fresh Go test execution and task-specific caches outside the repository.
- Preserve deterministic fake/no-op provider boundaries.
- Record raw stdout/stderr and any exact omission reason in the active review file.

**Modified Files and Checklist**

- [ ] `agent-task/m-iop-agent-cli-runtime/CODE_REVIEW-cloud-G07.md`: complete implementation notes, decisions, deviations, and exact verification output.
- [ ] Retain ownership of the previously modified S10 source/contract files listed below until final review; no additional edit is expected unless a regression exposes a necessary correction.

**Test Strategy**

- No separate test file is added for this item; REVIEW_API-1/2 own all new regressions.
- Fresh focused, race, full agent, vet, build, cross-build, binary, smoke-preflight, symbol, and diff checks provide integrated evidence.

**Verification**

```bash
TMPDIR=/tmp GOTMPDIR=/tmp GOCACHE=/tmp/iop-cli-followup-cache go test -count=1 ./apps/agent/internal/taskloop ./apps/agent/internal/command ./apps/agent/cmd/agent
```

Expected: PASS with fresh execution.

## Modified Files Summary

| File | Item |
|------|------|
| `apps/agent/internal/taskloop/workflow.go` | REVIEW_API-1 |
| `apps/agent/internal/taskloop/workflow_test.go` | REVIEW_API-1 |
| `apps/agent/cmd/agent/main_test.go` | REVIEW_API-2 |
| `apps/agent/internal/taskloop/module.go` | REVIEW_API-3 ownership carryover; no planned edit |
| `apps/agent/internal/command/service.go` | REVIEW_API-3 ownership carryover; no planned behavior edit |
| `apps/agent/internal/command/root.go` | REVIEW_API-3 ownership carryover; no planned behavior edit |
| `apps/agent/internal/command/root_test.go` | REVIEW_API-3 ownership carryover; no planned edit |
| `apps/agent/cmd/agent/main.go` | REVIEW_API-3 ownership carryover; no planned edit |
| `agent-contract/inner/iop-agent-cli-runtime.md` | REVIEW_API-3 ownership carryover; no planned edit |
| `agent-task/m-iop-agent-cli-runtime/CODE_REVIEW-cloud-G07.md` | REVIEW_API-3 |

## Final Verification

Fresh execution is required; cached Go test output is not acceptable. Unit/integration tests must not start a real provider CLI.

```bash
gofmt -w apps/agent/internal/taskloop/*.go apps/agent/cmd/agent/*.go apps/agent/internal/command/*.go
```

Expected: formatting completes without error.

```bash
TMPDIR=/tmp GOTMPDIR=/tmp GOCACHE=/tmp/iop-cli-followup-cache go test -count=1 ./apps/agent/internal/taskloop ./apps/agent/internal/command ./apps/agent/cmd/agent
```

Expected: focused packages PASS.

```bash
TMPDIR=/tmp GOTMPDIR=/tmp GOCACHE=/tmp/iop-cli-followup-race-cache go test -count=1 -race ./apps/agent/internal/taskloop ./apps/agent/internal/command ./apps/agent/cmd/agent ./apps/agent/internal/bootstrap ./packages/go/agenttask ./packages/go/agentstate
```

Expected: race suites PASS using deterministic fakes/no-op children only.

```bash
TMPDIR=/tmp GOTMPDIR=/tmp GOCACHE=/tmp/iop-cli-followup-cache go test -count=1 ./apps/agent/...
```

Expected: all agent packages PASS, including strict catalog and compiled-binary transcript regressions.

```bash
TMPDIR=/tmp GOTMPDIR=/tmp GOCACHE=/tmp/iop-cli-followup-cache go vet ./apps/agent/internal/taskloop ./apps/agent/internal/command ./apps/agent/cmd/agent ./apps/agent/internal/bootstrap ./packages/go/...
```

Expected: no vet findings.

```bash
make build-agent
build/bin/iop-agent --help
build/bin/iop-agent validate --repo-config configs/iop-agent.runtime.yaml --local-config configs/iop-agent.local.example.yaml --provider-catalog configs/iop-agent.providers.yaml
```

Expected: binary builds, full command surface is present, and tracked split configs validate.

```bash
TMPDIR=/tmp GOTMPDIR=/tmp GOCACHE=/tmp/iop-cli-followup-darwin-cache GOOS=darwin GOARCH=arm64 go build -trimpath -o /tmp/iop-agent-cli-followup-darwin-arm64 ./apps/agent/cmd/agent
```

Expected: a non-empty Darwin arm64 binary is produced outside the repository.

```bash
make test-iop-agent-logged-smoke-preflight
```

Expected: syntax/schema/self-test PASS and the Linux non-Darwin gate exits before provider login or launch.

```bash
rg --sort path -n '\b(MilestoneView|scanMilestones|scanWorkflow|InspectTaskGroup|MilestoneEntry|StatusResponse|WorkStatusEntry|WorkView)\b' apps/agent agent-contract/inner/iop-agent-cli-runtime.md
git diff --check
git status --short
```

Expected: identity/parser and projection call sites are intentional, no whitespace errors exist, and only declared task-owned files plus explicitly unrelated pre-existing changes are modified.

After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`.