<!-- task=m-iop-agent-cli-runtime/22+16_local_control plan=1 tag=REVIEW_API -->

# Convergent Local-Control Command Completion

## For the Implementing Agent

Fill all implementation-owned sections in `CODE_REVIEW-*-G??.md` with fresh command output, keep the active pair in place, and report ready for review. If blocked, record the exact blocker, attempted commands/output, and resume condition only in implementation-owned evidence fields. Do not ask the user, call user-input tools, create control-plane stop files, classify next state, archive logs, or write `complete.log`; finalization is code-review-skill only.

## Background

The first S11 review proved that identical concurrent `command_id` requests can return different results: the owner returns the completed host result while a duplicate returns a provisional durable acceptance. The local-control contract requires every identical replay to converge on the original completed result without a second effective mutation, including recovery from an accepted-but-unfinished durable record.

## Archive Evidence Snapshot

- Prior plan: `agent-task/m-iop-agent-cli-runtime/22+16_local_control/plan_cloud_G10_0.log`
- Prior review: `agent-task/m-iop-agent-cli-runtime/22+16_local_control/code_review_cloud_G10_0.log`
- Verdict: `FAIL`; Required `1`, Suggested `0`, Nit `0`.
- Finding: `Ledger.BeginCommand` exposes an incomplete `"accepted"` response because the existing-record path does not check `durableCommand.Complete`; a blocking-controller reproducer returned final state `"project.start"` to the owner and provisional state `"accepted"` to the duplicate.
- Affected files: `apps/agent/internal/localcontrol/ledger.go`, `apps/agent/internal/localcontrol/service.go`, `apps/agent/internal/localcontrol/service_test.go`, and `apps/agent/internal/localcontrol/ledger_test.go`.
- Verification evidence: the repository package, race, vet, protobuf determinism, Darwin arm64 cross-build, shared-package, and whole-Go suites passed; the focused reviewer reproducer failed response equality and was removed after capture.
- Roadmap carryover: SDD scenario S11 and task `local-control` remain incomplete until durable command convergence is proven.

## Roadmap Targets

- Milestone: `agent-roadmap/phase/automation-runtime-bridge/milestones/iop-agent-cli-runtime.md`
- Milestone link: [Milestone 문서](agent-roadmap/phase/automation-runtime-bridge/milestones/iop-agent-cli-runtime.md)
- Task ids:
  - `local-control`: daemon-owned same-user local proto-socket lifecycle, state, events, and control endpoints
- Completion mode: check-on-pass

## Analysis

### Files Read

- `agent-task/m-iop-agent-cli-runtime/22+16_local_control/PLAN-cloud-G10.md`
- `agent-task/m-iop-agent-cli-runtime/22+16_local_control/CODE_REVIEW-cloud-G10.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`
- `agent-contract/inner/iop-agent-cli-runtime.md`
- `agent-contract/inner/agent-runtime.md`
- `proto/iop/agent.proto`
- `apps/agent/internal/localcontrol/protocol.go`
- `apps/agent/internal/localcontrol/ledger.go`
- `apps/agent/internal/localcontrol/service.go`
- `apps/agent/internal/localcontrol/server.go`
- `apps/agent/internal/localcontrol/peercred.go`
- `apps/agent/internal/localcontrol/peercred_linux.go`
- `apps/agent/internal/localcontrol/peercred_darwin.go`
- `apps/agent/internal/localcontrol/peercred_unsupported.go`
- `apps/agent/internal/localcontrol/protocol_test.go`
- `apps/agent/internal/localcontrol/ledger_test.go`
- `apps/agent/internal/localcontrol/service_test.go`
- `apps/agent/internal/localcontrol/server_test.go`
- `packages/go/agentstate/store.go`
- `packages/go/agentstate/store_test.go`
- `../proto-socket/go/communicator.go`
- `../proto-socket/go/tcp_client.go`
- `Makefile`

### SDD Criteria

- SDD: `agent-roadmap/sdd/automation-runtime-bridge/iop-agent-cli-runtime/SDD.md`; status `[승인됨]`, lock released.
- Target: S11 → Milestone task `local-control`.
- Evidence Map: a local-control contract, OS-user socket boundary tests, no token fallback, denied cross-user dispatch, durable command convergence, and snapshot recovery.
- This follow-up narrows the checklist to the failed S11 command-id convergence row and preserves the fresh package/race, contract, and Darwin evidence required for Roadmap Completion.

### Verification Context

- No external handoff was supplied. Repository-native sources were `agent-test/local/rules.md`, `agent-test/local/platform-common-smoke.md`, `agent-test/local/testing-smoke.md`, the active plan, SDD, contract, source, and tests.
- Local preflight: repo `/config/workspace/iop-s0`, branch `feature/iop-agent-cli-runtime`, reviewed HEAD `4e23ded3f482bf61b6de3651dbc913c2e3384193`, Linux arm64, Go `1.26.2` at `/config/.local/bin/go`, GOROOT `/config/opt/go`, no external service or credential.
- Fresh reviewer commands passed: `make proto` determinism, localcontrol package and focused tests, localcontrol/agentstate race, localcontrol and shared-package vet, Darwin arm64 test cross-build, `go test -count=1 ./packages/go/...`, `go test -count=1 ./...`, and `git diff --check`.
- The deterministic blocking-controller reproducer failed because the duplicate observed provisional `"accepted"` while the owner observed final `"project.start"`.
- The current checkout includes intentional sibling task changes. Final evidence must be captured on a stable source state with the task write claim retained; record worktree status before and after verification. Cached Go results are not accepted.
- Full daemon-cycle verification is unavailable in this packet because local-control composition remains owned by `24+19,21,22,23,24_daemon_wiring`; package integration and cross-platform build are the current S11 execution surface.

### Test Coverage Gaps

- `TestConcurrentIdenticalCommandDispatchesExactlyOnce` proves one controller call but checks only response kind, so it misses response divergence and event/cursor mismatch.
- `TestCommandIdempotencySurvivesRestart` restarts only after `FinishCommand`; it does not cover a durable accepted-but-unfinished record.
- Sequential command conflicts, completed restart replay, replay gaps, authorization, framing, and server broadcast already have deterministic coverage and must remain unchanged.

### Symbol References

None. No symbol rename or removal is required.

### Split Judgment

Keep one packet. Durable record state, in-process duplicate coordination, restart reconciliation, final response persistence, and regression tests form one command-convergence invariant. Predecessor `16+13,15_bootstrap_composition` is satisfied by `agent-task/archive/2026/07/m-iop-agent-cli-runtime/16+13,15_bootstrap_composition/complete.log`.

### Scope Rationale

Do not change protobuf fields, peer credential code, socket framing, replay retention policy, daemon wiring, client-process operations, shared `agentstate.Store`, Control Plane paths, or roadmap files. The current contract already requires completed replay convergence; only the local ledger/service lifecycle and its tests need repair.

### Final Routing

- evaluation_mode: `isolated-reassessment`
- finalizer: `finalize-task-policy.sh pair`
- closures: build/review scope, context, verification, evidence, ownership, and decisions are closed.
- build grade scores: scope `1`, state/concurrency `2`, blast/irreversibility `2`, evidence/diagnosis `2`, verification `1` → `G08`.
- build base: `local-fit`; final route: `recovery-boundary`, `cloud-G08`, `PLAN-cloud-G08.md`.
- review grade scores: scope `1`, state/concurrency `2`, blast/irreversibility `2`, evidence/diagnosis `2`, verification `1` → `G08`.
- review route: `official-review`, `cloud-G08`, `CODE_REVIEW-cloud-G08.md`.
- large_indivisible_context: `false`
- positive loop risks: `temporal_state`, `concurrent_consistency`, `boundary_contract` (count `3`)
- recovery signals: `review_rework_count=1`, `evidence_integrity_failure=true`
- capability-gap evidence: none.

## Implementation Checklist

- [ ] Prevent incomplete durable command records from being returned as final results, and make concurrent/restarted identical requests converge on one stored completed response without a second effective mutation.
- [ ] Add deterministic blocking concurrency and accepted-but-unfinished restart regressions that assert complete response, event, cursor, and conflict behavior.
- [ ] Run fresh focused, race, contract, protobuf, Darwin cross-build, shared-package, and whole-Go verification on stable source.
- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.

### [REVIEW_API-1] Separate Command Ownership from Completed Replay

#### Problem

`apps/agent/internal/localcontrol/ledger.go:141-155` returns `record.Response` for every matching existing command without checking `record.Complete`. `apps/agent/internal/localcontrol/service.go:237-258` stores a provisional `"accepted"` response before dispatch and treats that existing response as terminal, so an identical concurrent request can return before the owner records the final controller result.

#### Solution

Represent a matching command as one of: new/recovery owner, incomplete in-flight waiter, or completed replay. Only the completed state may expose a response. Coordinate same-process duplicates by command ID and immutable hash so one owner performs or reconciles the controller call, waiters honor context cancellation, and every waiter reloads the completed durable response. After restart, reconcile an incomplete acceptance through the same stable command ID and the idempotent controller boundary, or fail closed without returning the provisional response; never report provisional acceptance as completed success.

Before (`apps/agent/internal/localcontrol/ledger.go:141-155`):

```go
if record, exists := state.Commands[commandID]; exists {
	if record.RequestHash != requestHash {
		return nil, false, protocolError(
			ErrorCommandIDConflict,
			"command_id was already used with different immutable arguments",
		)
	}
	response, err := decodeStoredEnvelope(record.Response)
	if err != nil {
		return nil, false, protocolError(
			ErrorInternal,
			"durable command state is invalid",
		)
	}
	return response, false, nil
}
```

After:

```go
if record, exists := state.Commands[commandID]; exists {
	if record.RequestHash != requestHash {
		return commandConflict()
	}
	if record.Complete {
		return completedReplay(record)
	}
	return incompleteCommand(commandID, requestHash)
}
```

The concrete coordination type may remain private, but it must release waiters on every completion/error path and preserve the current stored-response and event/cursor format.

#### Modified Files and Checklist

- [ ] `apps/agent/internal/localcontrol/ledger.go` — distinguish incomplete ownership/waiting from completed replay and coordinate completion safely.
- [ ] `apps/agent/internal/localcontrol/service.go` — drive one owner/recovery path and return only completed durable responses.

#### Test Strategy

Production behavior changes; regression tests are mandatory in REVIEW_API-2. Existing sequential replay and conflict tests remain the compatibility oracle.

#### Verification

```bash
gofmt -d apps/agent/internal/localcontrol/ledger.go apps/agent/internal/localcontrol/service.go
go test -count=1 ./apps/agent/internal/localcontrol -run 'TestConcurrentIdenticalCommand|TestCommandIdempotency|TestCommandIDConflict'
```

Expected: formatting is clean and all command lifecycle tests pass with no provisional response.

### [REVIEW_API-2] Prove Concurrent and Restart Convergence

#### Problem

`apps/agent/internal/localcontrol/service_test.go:315-350` checks only response kind and one host call. `apps/agent/internal/localcontrol/ledger_test.go:15-57` restarts only after a completed command. Neither test observes the incomplete durable state that caused the review failure.

#### Solution

Add a blocking controller fixture that holds the owner after durable acceptance, starts identical concurrent requests, then releases the owner. Assert every response is protobuf-equal to the completed stored result, exactly one live event/cursor is committed, and one effective mutation occurs. Add a restart fixture that seeds or observes an accepted-but-unfinished durable record, reopens the ledger/service, reconciles with the stable command ID, and proves final replay convergence without a second effective mutation. Retain a different-hash conflict assertion with zero conflicting mutation.

#### Modified Files and Checklist

- [ ] `apps/agent/internal/localcontrol/service_test.go` — assert completed response equality for blocked concurrent duplicates.
- [ ] `apps/agent/internal/localcontrol/ledger_test.go` — cover accepted-but-unfinished restart reconciliation and stable final replay.

#### Test Strategy

Add deterministic tests named for the behavior, using channels rather than sleeps. Run them under `-race`; assert payload, state revision, replay daemon/cursor, event sequence, and effective mutation count.

#### Verification

```bash
go test -count=1 ./apps/agent/internal/localcontrol -run 'TestConcurrentIdenticalCommand|TestAcceptedButIncomplete|TestCommandIdempotency|TestCommandIDConflict'
go test -count=1 -race ./apps/agent/internal/localcontrol ./packages/go/agentstate
```

Expected: every focused assertion passes and the race detector reports no issue.

### [REVIEW_API-3] Re-establish S11 Evidence

#### Problem

The prior package and repository suites passed while omitting the failed response-convergence assertion, so S11 completion evidence cannot rely on those results alone.

#### Solution

Run every final command fresh after the regression tests pass, retain exact stdout/stderr in the active review artifact, and record omitted full-cycle scope explicitly. Confirm the protobuf output and contract anchors remain unchanged.

#### Modified Files and Checklist

- [ ] `agent-task/m-iop-agent-cli-runtime/22+16_local_control/CODE_REVIEW-cloud-G08.md` — record implementation decisions, deviations, and actual verification output.

#### Test Strategy

No additional test file; REVIEW_API-2 supplies the missing deterministic oracle. This item validates the complete S11 package and repository surface.

#### Verification

```bash
go test -count=1 ./apps/agent/internal/localcontrol
go vet ./apps/agent/internal/localcontrol ./packages/go/...
git diff --check
```

Expected: all commands exit zero and exact output is recorded.

## Dependencies and Execution Order

`16+13,15_bootstrap_composition` is satisfied by `agent-task/archive/2026/07/m-iop-agent-cli-runtime/16+13,15_bootstrap_composition/complete.log`. Implement REVIEW_API-1 before REVIEW_API-2, then capture REVIEW_API-3 evidence.

## Modified Files Summary

| File | Item |
|------|------|
| `apps/agent/internal/localcontrol/ledger.go` | REVIEW_API-1 |
| `apps/agent/internal/localcontrol/service.go` | REVIEW_API-1 |
| `apps/agent/internal/localcontrol/service_test.go` | REVIEW_API-2 |
| `apps/agent/internal/localcontrol/ledger_test.go` | REVIEW_API-2 |
| `agent-task/m-iop-agent-cli-runtime/22+16_local_control/CODE_REVIEW-cloud-G08.md` | REVIEW_API-3 |

## Final Verification

```bash
command -v go
go version
go env GOROOT
git status --short
gofmt -w apps/agent/internal/localcontrol/ledger.go apps/agent/internal/localcontrol/service.go apps/agent/internal/localcontrol/service_test.go apps/agent/internal/localcontrol/ledger_test.go
go test -count=1 ./apps/agent/internal/localcontrol -run 'TestConcurrentIdenticalCommand|TestAcceptedButIncomplete|TestCommandIdempotency|TestCommandIDConflict'
go test -count=1 ./apps/agent/internal/localcontrol
go test -count=1 -race ./apps/agent/internal/localcontrol ./packages/go/agentstate
go vet ./apps/agent/internal/localcontrol ./packages/go/...
make proto
GOOS=darwin GOARCH=arm64 go test -c -o /tmp/localcontrol-darwin.test ./apps/agent/internal/localcontrol
go test -count=1 ./packages/go/...
go test -count=1 ./...
rg -n --sort path 'S11|agent.proto|localcontrol|same OS user|SO_PEERCRED|LOCAL_PEERCRED|getpeereid' agent-contract/inner/iop-agent-cli-runtime.md apps/agent/internal/localcontrol proto/iop/agent.proto
git diff --check
git status --short
```

Expected: fresh focused/package/race/shared/repository tests and vet pass, proto generation is stable, Darwin arm64 cross-build succeeds, contract anchors remain present, the reviewer regression is covered, and the worktree shows only intentional task/sibling changes. Repository Edge-Node diagnostics, auxiliary E2E smoke, and a full `iop-agent` daemon cycle remain out of scope because this packet does not wire the package into the daemon.

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