<!-- task=m-iop-agent-cli-runtime/24+19,21,22,23_daemon_wiring plan=2 tag=REVIEW_TEST -->

# Close Daemon Lifecycle Review Findings

## For the Implementing Agent

Run every verification command, fill the implementation-owned sections in `CODE_REVIEW-*-G??.md` with actual notes and stdout/stderr, keep the active pair in place, and report ready for review. Finalization belongs only to the code-review skill. 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 stop files, classify the next state, archive logs, or write `complete.log`.

## Background

The first daemon composition starts the expected host adapters, but it reports fictitious task-runtime state, leaks an earlier client when a later configured launch fails, and has no production signal-cancellation path. This follow-up keeps unavailable task-loop behavior fail-closed, makes startup rollback fault-atomic, and proves graceful shutdown through the built binary without pulling the separately planned task-loop cutover into this packet.

## Archive Evidence Snapshot

- Current pair after finalization: `agent-task/m-iop-agent-cli-runtime/24+19,21,22,23_daemon_wiring/plan_local_G08_1.log` and `agent-task/m-iop-agent-cli-runtime/24+19,21,22,23_daemon_wiring/code_review_cloud_G08_1.log`.
- Verdict: FAIL; Required=3, Suggested=0, Nit=0.
- Required findings: task-runtime reads/mutations emit success without `agenttask.Manager`; partial client launch is not rolled back; production `main` never converts SIGINT/SIGTERM into daemon cancellation.
- Affected files: `apps/agent/internal/bootstrap/module.go`, `apps/agent/internal/bootstrap/module_test.go`, `apps/agent/cmd/agent/main.go`, and `apps/agent/cmd/agent/main_test.go`.
- Fresh baseline passed focused race tests, adjacent adapter tests, vet, agent build, Darwin arm64 cross-build, help, and `git diff --check`. The focused reviewer rollback reproducer failed with `first client remains alive after daemon startup failure`.
- Roadmap carryover: this task remains Milestone-adjacent and has no `Roadmap Targets`; authenticated S14 evidence remains in child 25, and full task-loop/cutover ownership remains in child 26.

## Analysis

### Files Read

- `agent-task/m-iop-agent-cli-runtime/24+19,21,22,23_daemon_wiring/PLAN-local-G08.md`
- `agent-task/m-iop-agent-cli-runtime/24+19,21,22,23_daemon_wiring/CODE_REVIEW-cloud-G08.md`
- `agent-task/m-iop-agent-cli-runtime/24+19,21,22,23_daemon_wiring/plan_cloud_G07_0.log`
- `agent-task/m-iop-agent-cli-runtime/24+19,21,22,23_daemon_wiring/code_review_cloud_G07_0.log`
- `apps/agent/internal/bootstrap/module.go`
- `apps/agent/internal/bootstrap/module_test.go`
- `apps/agent/cmd/agent/main.go`
- `apps/agent/cmd/agent/main_test.go`
- `apps/agent/internal/host/host.go`
- `apps/agent/internal/host/ports.go`
- `apps/agent/internal/clientprocess/manager.go`
- `apps/agent/internal/clientprocess/process.go`
- `apps/agent/internal/localcontrol/service.go`
- `apps/agent/internal/localcontrol/ledger.go`
- `apps/agent/internal/projectlog/store.go`
- `packages/go/agenttask/ports.go`
- `packages/go/agenttask/manager.go`
- `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/agent-runtime.md`
- `agent-contract/inner/iop-agent-cli-runtime.md`
- `agent-task/archive/2026/07/m-iop-agent-cli-runtime/19+14,16,18_cli_binary_contract/complete.log`
- `agent-task/archive/2026/07/m-iop-agent-cli-runtime/21+13,17,20_project_log_sink/complete.log`
- `agent-task/archive/2026/07/m-iop-agent-cli-runtime/22+16_local_control/complete.log`
- `agent-task/archive/2026/07/m-iop-agent-cli-runtime/23+14,16,22_client_process_manager/complete.log`
- `agent-task/m-iop-agent-cli-runtime/25+19,21,22,23,24_logged_smoke_closure/PLAN-cloud-G09.md`
- `agent-task/m-iop-agent-cli-runtime/26+19,21,22,23,25_parity_cutover/PLAN-cloud-G10.md`

### SDD Criteria

- SDD: `agent-roadmap/sdd/automation-runtime-bridge/iop-agent-cli-runtime/SDD.md`; status `[승인됨]`, lock released.
- S10 / `cli-surface`: this packet preserves the partial fail-closed command boundary and adds production signal/cleanup evidence, but does not claim the full headless runtime transcript.
- S11 / `local-control`: task-runtime operations must delegate to the shared runtime; while that owner is absent, they must return an error rather than fabricate state or mutation success.
- S15 / `client-process-manager`: daemon-owned clients must be cleaned up on startup failure and must never own or terminate daemon/runtime lifecycle.
- The S10, S11, and S15 Evidence Map rows drive the built-binary signal regression, unavailable-operation assertions, partial-launch rollback test, race suite, and Darwin cross-build. No feature Task is checked by this regression-only packet.

### Verification Context

- No verification handoff was supplied. Repository-native evidence came from the local test rules, Agent/testing domain rules, contracts, related tests, and fresh commands.
- Runner: current checkout `/config/workspace/iop-s0`, Linux arm64; Go resolves to `/config/.local/bin/go`, `go1.26.2 linux/arm64`, GOROOT `/config/opt/go`.
- Source basis: the intentional dirty shared checkout containing the active Milestone work. The exact four claimed files are the verification focus; fresh tests use `-count=1`.
- Fresh baseline: focused race suites, adjacent adapter packages, vet, `make build-agent`, Darwin arm64 build, built help, and `git diff --check` all passed.
- Reviewer reproducer: a fake backend started Flutter and failed Unity; daemon startup returned an error but the Flutter process remained alive.
- No remote service, credential, provider login, or macOS execution is required. Darwin verification is compile-only. The authenticated macOS run belongs to child 25.
- The full production task-loop adapters and parity cutover belong to child 26; this packet must not duplicate those files.
- Confidence: high for the three local lifecycle failures and their deterministic regressions.

### Test Coverage Gaps

- No test rejects fixed `idle`/`clear` task-runtime projections or fictitious project mutation success from the daemon bridge.
- Generic host rollback tests do not cover a component whose own `Start` partially succeeds before returning an error; the reviewer reproducer proves the leak.
- `TestRunServeCancellationAndCleanup` injects a cancellable context and does not exercise the production `main` signal path or a built process.

### Symbol References

- No symbol is removed yet.
- `daemonStateBridge` is constructed only in `apps/agent/internal/bootstrap/module.go`.
- `run` is used by `main` and CLI tests; `runWithContext` is the existing injectable cancellation seam.

### Split Judgment

- Keep one packet. Fail-closed task-runtime presentation, client-start rollback, and signal-driven close are one daemon ownership invariant and share the same construction and shutdown evidence.
- Predecessors 19, 21, 22, and 23 are satisfied by the four exact archived `complete.log` files listed under `Files Read`.

### Scope Rationale

- Exclude `apps/agent/internal/taskloop/**` and command task-loop cutover files because child 26 owns their production implementation.
- Exclude the authenticated harness, schema, and Makefile smoke target because child 25 owns S14 evidence.
- Do not change `clientprocess.Manager` APIs: the bootstrap adapter can close the partially started manager and join cleanup errors at its component boundary.
- Do not change contracts or roadmap state; the fixes enforce existing lifecycle and fail-closed requirements.

### Final Routing

- evaluation_mode: `isolated-reassessment`
- finalizer: `finalize-task-policy.sh pair`
- build closures: scope/context/verification/evidence/ownership/decision all `true`; grade scores `2/2/1/1/2`; base/route `local-fit`; lane/grade `local-G08`; filename `PLAN-local-G08.md`
- review closures: scope/context/verification/evidence/ownership/decision all `true`; grade scores `2/2/1/1/2`; route `official-review`; lane/grade `cloud-G08`; filename `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=false`
- capability-gap evidence: none

## Implementation Checklist

- [ ] Keep unwired task-runtime reads and project mutations fail-closed while preserving only host/client projections backed by composed state, and verify no success response or state-change event is emitted.
- [ ] Make configured client startup fault-atomic, preserve launch and cleanup error identities, and add the partial-launch rollback regression.
- [ ] Add signal-aware production daemon cancellation, return cleanup failures, and add a built-binary signal shutdown regression.
- [ ] Run the complete fresh focused, race, adjacent-package, vet, build, Darwin cross-build, whole-Go, help, and diff verification.
- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.

### [REVIEW_TEST-1] Fail Closed Without Task Runtime Ownership

#### Problem

`apps/agent/internal/bootstrap/module.go:329-350,372-408` returns fixed task-runtime state and successful project mutation results even though no `agenttask.Manager` is composed. The CLI correctly reports that the lifecycle is unavailable, so the local-control boundary disagrees with the production owner.

#### Solution

Keep `runtime.status` and `process.status` backed by actual composed host/client state. Return one stable unavailable error from project, overlay, integration, blocker, start, stop, and resume task-runtime methods until child 26 supplies the authoritative owner.

Before (`apps/agent/internal/bootstrap/module.go:329`):

```go
func (b *daemonStateBridge) OverlayStatus(...) (localcontrol.StatusSnapshot, error) {
	return localcontrol.StatusSnapshot{State: "idle"}, nil
}
```

After:

```go
var errTaskRuntimeUnavailable = errors.New("bootstrap: task runtime is unavailable")

func (b *daemonStateBridge) OverlayStatus(...) (localcontrol.StatusSnapshot, error) {
	return localcontrol.StatusSnapshot{}, errTaskRuntimeUnavailable
}
```

Apply the same fail-closed rule to all task-runtime reads and mutations; do not add a no-op or success fallback.

#### Modified Files and Checklist

- [ ] `apps/agent/internal/bootstrap/module.go` — remove fictitious task-runtime projections and mutation success.
- [ ] `apps/agent/internal/bootstrap/module_test.go` — add service-level unavailable read/mutation assertions and prove no mutation event is appended.

#### Test Strategy

Add `TestDaemonTaskRuntimeFailsClosedWithoutOwner`. Construct the production bridge/service over a temporary durable ledger, issue project status/start requests, require typed error responses, and verify replay contains no state-change event. Also assert runtime/process status still reflects actual composed state.

#### Verification

```bash
go test -count=1 ./apps/agent/internal/bootstrap -run '^TestDaemonTaskRuntimeFailsClosedWithoutOwner$'
```

Expected: unavailable task-runtime requests fail closed and actual host/client projections remain readable.

### [REVIEW_TEST-2] Roll Back a Partially Started Client Component

#### Problem

`apps/agent/internal/bootstrap/module.go:275-280` returns `StartConfigured` errors directly. `StartConfigured` may already have started an earlier client, while `host.Host` only stops components whose `Start` returned success.

#### Solution

On `StartConfigured` failure, close the manager with a non-cancelled cleanup context and return `errors.Join(launchErr, cleanupErr)`. This makes the failing component responsible for its own partial side effects and preserves both error identities.

Before (`apps/agent/internal/bootstrap/module.go:275`):

```go
func (c *clientProcessAdapter) Start(ctx context.Context) error {
	return c.manager.StartConfigured(ctx)
}
```

After:

```go
func (c *clientProcessAdapter) Start(ctx context.Context) error {
	if err := c.manager.StartConfigured(ctx); err != nil {
		return errors.Join(err, c.manager.Close(context.WithoutCancel(ctx)))
	}
	return nil
}
```

#### Modified Files and Checklist

- [ ] `apps/agent/internal/bootstrap/module.go` — close partial client state and join errors.
- [ ] `apps/agent/internal/bootstrap/module_test.go` — add a two-client fake backend with first-success/second-failure and cleanup-error cases.

#### Test Strategy

Add `TestDaemonPartialClientLaunchFailureRollsBack`. Require the first process to be aborted/reaped, the second launch error to remain discoverable with `errors.Is`, optional cleanup error identity to be preserved, no local-control socket to remain, and host status to record failed startup.

#### Verification

```bash
go test -count=1 ./apps/agent/internal/bootstrap -run '^TestDaemonPartialClientLaunchFailureRollsBack$'
```

Expected: both launch and cleanup evidence are preserved and no earlier client survives.

### [REVIEW_TEST-3] Handle Production Termination Signals

#### Problem

`apps/agent/cmd/agent/main.go:42-47` runs production commands with `context.Background`. The cancellable seam used by tests is never connected to SIGINT/SIGTERM, and `Serve` ignores `daemon.Close` errors.

#### Solution

Create the production context with `signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)`, keep `runWithContext` injectable, and make `Serve` close with a non-cancelled bounded context after cancellation so cleanup errors affect the exit code.

Before (`apps/agent/cmd/agent/main.go:42`):

```go
func run(args []string, stdout, stderr io.Writer) int {
	return runWithContext(context.Background(), args, stdout, stderr)
}

func main() {
	os.Exit(run(os.Args[1:], os.Stdout, os.Stderr))
}
```

After:

```go
func runMain(args []string, stdout, stderr io.Writer) int {
	ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
	defer stop()
	return runWithContext(ctx, args, stdout, stderr)
}

func main() {
	os.Exit(runMain(os.Args[1:], os.Stdout, os.Stderr))
}
```

#### Modified Files and Checklist

- [ ] `apps/agent/cmd/agent/main.go` — add production signal cancellation and observable bounded cleanup.
- [ ] `apps/agent/cmd/agent/main_test.go` — build a temporary binary, wait for socket readiness, send SIGTERM, and assert bounded clean exit and socket removal.

#### Test Strategy

Add `TestBuiltBinarySignalShutdownCleansDaemon`. Use `t.TempDir` for binary/config/state, start the real built executable, wait for the owner-only socket, signal the process, require exit code 0 within the deadline, and verify the socket plus any fake configured client are gone. Keep the existing injected-context cancellation test.

#### Verification

```bash
go test -count=1 ./apps/agent/cmd/agent -run '^Test(BuiltBinarySignalShutdownCleansDaemon|RunServeCancellationAndCleanup)$'
```

Expected: injected and real production cancellation both close the daemon cleanly.

## Dependencies and Execution Order

Predecessors `19+14,16,18_cli_binary_contract`, `21+13,17,20_project_log_sink`, `22+16_local_control`, and `23+14,16,22_client_process_manager` are satisfied by their archived `complete.log` files. Implement REVIEW_TEST-1 and REVIEW_TEST-2 before REVIEW_TEST-3, then run final verification on the integrated lifecycle.

## Modified Files Summary

| File | Item |
|------|------|
| `apps/agent/internal/bootstrap/module.go` | REVIEW_TEST-1, REVIEW_TEST-2 |
| `apps/agent/internal/bootstrap/module_test.go` | REVIEW_TEST-1, REVIEW_TEST-2 |
| `apps/agent/cmd/agent/main.go` | REVIEW_TEST-3 |
| `apps/agent/cmd/agent/main_test.go` | REVIEW_TEST-3 |

## Final Verification

```bash
gofmt -w apps/agent/internal/bootstrap/module.go apps/agent/internal/bootstrap/module_test.go apps/agent/cmd/agent/main.go apps/agent/cmd/agent/main_test.go
go test -count=1 ./apps/agent/internal/bootstrap -run 'TestDaemon(TaskRuntimeFailsClosedWithoutOwner|PartialClientLaunchFailureRollsBack|CompositionAndLifecycle|ClientExitIsolation)'
go test -count=1 ./apps/agent/cmd/agent -run 'Test(BuiltBinarySignalShutdownCleansDaemon|RunServeCancellationAndCleanup)'
go test -count=1 -race ./apps/agent/internal/bootstrap ./apps/agent/cmd/agent ./apps/agent/internal/command
go test -count=1 ./apps/agent/internal/clientprocess ./apps/agent/internal/localcontrol ./apps/agent/internal/projectlog ./packages/go/agentstate
go vet ./apps/agent/internal/bootstrap ./apps/agent/cmd/agent ./apps/agent/internal/command
make build-agent
review_tmp_dir=$(mktemp -d)
GOOS=darwin GOARCH=arm64 go build -trimpath -o "$review_tmp_dir/iop-agent-darwin-arm64" ./apps/agent/cmd/agent
test -s "$review_tmp_dir/iop-agent-darwin-arm64"
build/bin/iop-agent --help >/dev/null
go test -count=1 ./...
git diff --check
```

Expected: fail-closed task-runtime behavior, fault-atomic startup, graceful built-binary shutdown, fresh race/adjacent/whole-Go suites, vet, builds, help, and diff checks all pass without external credentials. Cached output is not accepted.

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