iop/agent-task/m-iop-agent-cli-runtime/PLAN-cloud-G07.md

Plan - Canonical Milestone prefix round-trip and exact compiled 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 second review confirmed that the fail-closed catalog tests pass but found a remaining ambiguity between canonical Milestone slugs and m- task-group identifiers: a valid slug beginning with m- is stripped twice and cannot round-trip through catalog and selection. It also confirmed that the named S10 tests still do not provide the exact two-work compiled-binary transcript or exact field assertions claimed by the review evidence. This follow-up closes those two Required findings without changing the command DTOs or shared runtime ownership.

Archive Evidence Snapshot

• agent-task/m-iop-agent-cli-runtime/plan_local_G07_0.log and agent-task/m-iop-agent-cli-runtime/code_review_cloud_G07_0.log: first review loop, FAIL with 2 Required production/evidence findings.
• agent-task/m-iop-agent-cli-runtime/plan_cloud_G06_1.log and agent-task/m-iop-agent-cli-runtime/code_review_cloud_G07_1.log: second review loop, FAIL with 2 Required, 0 Suggested, and 0 Nit findings.
• Required correctness finding: scanWorkflow accepts a slug but also strips m- as though every input were a task-group identifier. The reviewer fixture m-m-foo failed catalog discovery with taskloop: selected milestone "foo" has no active or archived task artifacts.
• Required evidence finding: the adapter test does not assert exact ordinals, both overlays, or exact blocker fields; the built-binary transcript only shows works: 0 and still validates stdout by substring for one step.
• Reviewer verification: the API-1, API-2, and API-3 named suites passed fresh on the current checkout, proving that the remaining defects are test-oracle gaps rather than command failures.
• Roadmap carryover: cli-surface remains the only unchecked Milestone Task and still requires exact Roadmap Completion plus trustworthy SDD S10 headless evidence.

Roadmap Targets

• Milestone: agent-roadmap/phase/automation-runtime-bridge/milestones/iop-agent-cli-runtime.md
• Milestone link: Milestone document
• 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 and callers: apps/agent/internal/taskloop/workflow.go, apps/agent/internal/taskloop/module.go, apps/agent/cmd/agent/main.go.
• Tests: apps/agent/internal/taskloop/workflow_test.go, apps/agent/cmd/agent/main_test.go.
• Contract and design: 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.
• Loop evidence: agent-task/m-iop-agent-cli-runtime/PLAN-cloud-G06.md, agent-task/m-iop-agent-cli-runtime/CODE_REVIEW-cloud-G07.md, agent-task/m-iop-agent-cli-runtime/code_review_cloud_G07_0.log.
• Rules and local 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, no unresolved user review.
• Target: Acceptance Scenario S10, Milestone Task cli-surface.
• Evidence Map: binary entry point and split-config command integration must produce cli-surface Roadmap Completion and a headless operation transcript.
• The implementation checklist therefore requires one unambiguous catalog/selection/inspection identity round-trip, exact two-work status evidence, and exact compiled-binary stdout/stderr before the S10 completion event can be trusted.

Verification Context

• Supplied handoff: none. Repository-native fallback used the active pair, current source/tests, the S10 contract/SDD, agent/testing domain rules, and the local testing profile.
• Environment: local checkout /config/workspace/iop-s0; branch dev; HEAD 8760d165105fb03b0b8b62b55dd31c90f34daa44; intentional S10 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.
• Fresh reviewer commands passed: API-1 (taskloop, 0.094s), API-2 (command, 0.012s; cmd/agent, 9.434s), API-3 (taskloop, 12.380s; command, 0.018s; cmd/agent, 14.444s), plus git diff --check.
• Focused reviewer reproducer: a valid m-m-foo task group caused scanMilestones to fail with taskloop: selected milestone "foo" has no active or archived task artifacts.
• Constraints: do not launch a real provider CLI; use the existing proof-owned no-op/fake provider fixture; do not change shared manager state semantics, public DTOs, config schema, roadmap state, SDD state, or Agent-Ops common files.
• Gap and confidence: both defects are directly visible in source and one is reproduced; confidence is high.
• External Verification Preflight: not applicable. Darwin remains a local cross-build, and live logged-in macOS/provider evidence is the already completed S14 scope.
• Agent spec: agent-spec/index.md has no matching standalone iop-agent CLI spec; code, contract, SDD, and tests remain authoritative.

Test Coverage Gaps

• Prefix slug round-trip: no test uses a canonical Milestone slug beginning with m-, so catalog discovery, exact selection, persisted selection, and InspectTaskGroup do not prove one identity.
• Adapter projection: TestAdapterStatusPreservesAllWork uses nonzero ordinal predicates and omits work 2 overlay plus exact scoped blocker message/retry assertions.
• Built binary: TestBuiltBinaryHeadlessS10Transcript never seeds durable work state, so every status assertion reports works: 0.
• Exact streams: the compiled validate step uses strings.Contains instead of complete stdout equality.

Symbol References

• No public symbol is renamed or removed.
• scanWorkflow call sites are Workflow.Snapshot, Runtime.SelectMilestone, scanMilestones, and InspectTaskGroup.
• InspectTaskGroup is called by the config-free CLI adapter path in apps/agent/cmd/agent/main.go and its workflow tests.
• Keep MilestoneView, WorkView, MilestoneEntry, StatusResponse, and WorkStatusEntry unchanged.

Split Judgment

Keep one plan. Slug/group normalization and exact compiled status evidence form one indivisible invariant: every catalog id must select the same workflow and the product binary must display the exact durable work projection produced by that workflow. Splitting would create another boundary where tests could pass without proving the end-to-end identity and status contract.

Scope Rationale

• Change production behavior only in workflow.go; do not change command DTOs, shared agenttask.Manager, lifecycle transitions, durable schema, provider execution, local control, or config schema.
• Change tests only in workflow_test.go and main_test.go; reuse the existing deterministic lifecycle fixture and built binary.
• Keep previously modified S10 source/contract files in the write claim until final PASS, but do not edit them unless a new focused regression proves a necessary correction.
• Do not update the roadmap, SDD, agent-spec, or Agent-Ops skills. Runtime completion-event handling owns roadmap updates after PASS.

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 scores: scope 1, state 1, blast 1, evidence 2, verification 2 = G07; base local-fit, final recovery-boundary, lane cloud.
• Review closures: scope/context/verification/evidence/ownership/decision all true; grade scores 1/1/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=2, evidence_integrity_failure=true; recovery boundary matched.
• Canonical files: PLAN-cloud-G07.md, CODE_REVIEW-cloud-G07.md.

Implementation Checklist

• [REVIEW_REVIEW_API-1] Separate canonical Milestone slug parsing from m- task-group parsing and add an m-foo catalog/selection/inspection round-trip regression.
• [REVIEW_REVIEW_API-2] Make adapter and compiled-binary S10 evidence assert the exact ordered two-work projection and exact stdout/stderr for every transcript step.
• [REVIEW_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_REVIEW_API-1] Unambiguous Milestone slug and task-group parsing

Problem

apps/agent/internal/taskloop/workflow.go:179-184 accepts a Milestone slug but strips the m- task-group prefix whenever the slug starts with those characters. Because [a-z0-9][a-z0-9-]* permits m-foo, discovery returns a valid id that scanWorkflow immediately changes to foo.

Before (apps/agent/internal/taskloop/workflow.go:179):

func scanWorkflow(root, milestone string) ([]agenttask.WorkUnit, string, error) {
	slug := milestone
	if strings.HasPrefix(milestone, taskGroupPrefix) {
		slug = strings.TrimPrefix(milestone, taskGroupPrefix)
	}
	canonicalSlug, err := parseMilestoneSlug(slug)

Solution

• Make scanWorkflow accept and validate only a canonical Milestone slug; never strip a prefix there.
• Keep task-group parsing at the boundary that actually accepts a group: InspectTaskGroup strips exactly one m-, validates the remainder, and passes the canonical slug inward.
• Keep active/archive directory parsers responsible for their own directory grammar and return canonical slugs.
• Add a regression using active group m-m-foo, catalog id m-foo, exact selection m-foo, persisted workflow snapshot, and inspection group m-m-foo.

After:

func scanWorkflow(root, milestone string) ([]agenttask.WorkUnit, string, error) {
	canonicalSlug, err := parseMilestoneSlug(milestone)
	if err != nil {
		return nil, "", fmt.Errorf("taskloop: selected milestone %q: %w", milestone, err)
	}

Modified Files and Checklist

• apps/agent/internal/taskloop/workflow.go: remove ambiguous prefix stripping from the canonical slug path.
• apps/agent/internal/taskloop/workflow_test.go: add TestWorkflowMilestonePrefixSlugRoundTrip covering catalog, exact selection, workflow snapshot, and config-free inspection.

Test Strategy

• Write TestWorkflowMilestonePrefixSlugRoundTrip with one m-m-foo fixture.
• Assert the catalog returns exactly m-foo, SelectMilestone("m-foo") succeeds, the saved selection remains m-foo, WorkflowSnapshot returns its work, and InspectTaskGroup("m-m-foo") returns the same work.
• Retain the invalid identifier and archive-collision boundary matrices.

Verification

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

Expected: PASS; m-foo round-trips as a slug while m-m-foo is accepted only as its task-group identifier.

[REVIEW_REVIEW_API-2] Exact adapter and compiled-binary two-work evidence

Problem

apps/agent/cmd/agent/main_test.go:521-533 uses nonzero predicates instead of exact ordinals and omits work 2's overlay and exact blocker content. apps/agent/cmd/agent/main_test.go:650-724 supplies no durable works to the built binary, asserts only works: 0, and treats validate as a substring exception.

Before (apps/agent/cmd/agent/main_test.go:650):

{
	name:       "validate",
	args:       append([]string{"validate"}, mutationBase...),
	wantStdout: "", // substring validated inside test body
	wantStderr: "",
}
// ...
if step.name == "validate" {
	if !strings.Contains(stdout.String(), "validate: ok") {

Solution

• Extract one deterministic lifecycle-state seeding helper used by the adapter assertion and the compiled-binary transcript.
• Compare the complete ordered []command.WorkStatusEntry value, including ids, states, both overlays, integrations, exact ordinals, and exact blocker code/message/retryability; compare the compatible top-level blocker list exactly.
• Run the built binary against the same checksum-protected seeded state and add an exact status step containing both work rows and the scoped plus top-level blocker output.
• Derive the deterministic validation revision from the fixture snapshot and compare the entire validate stdout; preserve separate exact stdout/stderr for every subprocess step.
• Keep all provider execution behind the existing proof-owned no-op/fake ports and assert the expected dispatch/review/validation counts before invoking the read-only binary.

After:

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: exact adapter equality, reusable deterministic state seed, exact validation output, and compiled-binary two-work status transcript.

Test Strategy

• Strengthen TestAdapterStatusPreservesAllWork to compare exact complete work and blocker DTOs.
• Strengthen TestBuiltBinaryHeadlessS10Transcript to seed two durable works before subprocess reads, assert exact validate output, and include exact per-work status output.
• Keep TestRunFullHeadlessS10Transcript exact and confirm all transcript steps use separate stdout/stderr.
• Do not launch a real provider CLI.

Verification

TMPDIR=/tmp GOTMPDIR=/tmp GOCACHE=/tmp/iop-cli-recovery2-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 DTOs and subprocess stdout/stderr, including two work entries from the compiled binary.

[REVIEW_REVIEW_API-3] Fresh S10 recovery closure

Problem

The named focused suites pass while the parser and transcript assertions remain incomplete. Final S10 closure therefore requires fresh execution after the new regression and exact compiled-binary oracle are present.

Solution

• Run every command below with 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 focused regression proves it necessary.

Test Strategy

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

Verification

TMPDIR=/tmp GOTMPDIR=/tmp GOCACHE=/tmp/iop-cli-recovery2-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_REVIEW_API-1
apps/agent/internal/taskloop/workflow_test.go	REVIEW_REVIEW_API-1
apps/agent/cmd/agent/main_test.go	REVIEW_REVIEW_API-2
apps/agent/internal/taskloop/module.go	REVIEW_REVIEW_API-3 ownership carryover; no planned edit
apps/agent/internal/command/service.go	REVIEW_REVIEW_API-3 ownership carryover; no planned behavior edit
apps/agent/internal/command/root.go	REVIEW_REVIEW_API-3 ownership carryover; no planned behavior edit
apps/agent/internal/command/root_test.go	REVIEW_REVIEW_API-3 ownership carryover; no planned edit
apps/agent/cmd/agent/main.go	REVIEW_REVIEW_API-3 ownership carryover; no planned edit
agent-contract/inner/iop-agent-cli-runtime.md	REVIEW_REVIEW_API-3 ownership carryover; no planned edit
agent-task/m-iop-agent-cli-runtime/CODE_REVIEW-cloud-G07.md	REVIEW_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.

gofmt -w apps/agent/internal/taskloop/workflow.go apps/agent/internal/taskloop/workflow_test.go apps/agent/cmd/agent/main_test.go

Expected: formatting completes without error.

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

Expected: focused packages PASS.

TMPDIR=/tmp GOTMPDIR=/tmp GOCACHE=/tmp/iop-cli-recovery2-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.

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

Expected: all agent packages PASS, including the prefix-slug and exact compiled-binary transcript regressions.

TMPDIR=/tmp GOTMPDIR=/tmp GOCACHE=/tmp/iop-cli-recovery2-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.

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, the full command surface is present, and tracked split configs validate.

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

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

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.

rg --sort path -n '\b(parseMilestoneSlug|parseActiveGroupDir|parseArchiveGroupDir|scanWorkflow|scanMilestones|InspectTaskGroup|MilestoneView|WorkStatusEntry|TestAdapterStatusPreservesAllWork|TestBuiltBinaryHeadlessS10Transcript)\b' apps/agent agent-contract/inner/iop-agent-cli-runtime.md
git diff --check
git status --short

Expected: parser/identity 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.