<!-- task=m-iop-owned-single-request-agent-execution/14+05,12,13_observation_timing plan=2 tag=REVIEW_API milestone-task=cleanup-observation -->

# Repair Single-request Lifecycle Observation Wiring

## For the Implementing Agent

Implement R1-R3 exactly within the listed write boundary, run every verification command, and fill the implementation-owned sections of `CODE_REVIEW-cloud-G07.md` with actual notes and stdout/stderr. Keep the active pair in place and report ready for review. 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`; finalization belongs to the code-review skill.

## Background

The previous implementation added a closed observation accumulator but did not attach it to real `Service.StartSingleRequest` handles. Its production hooks also omit tool events and cleanup timing and do not preserve one semantic stage across tool pauses. This follow-up connects the accumulator to the real lifecycle and replaces manual-only timing evidence with deterministic integration coverage.

## Archive Evidence Snapshot

- Closed pair: `agent-task/m-iop-owned-single-request-agent-execution/14+05,12,13_observation_timing/plan_local_G06_1.log` and `agent-task/m-iop-owned-single-request-agent-execution/14+05,12,13_observation_timing/code_review_cloud_G07_1.log`.
- Verdict: FAIL with Required R1-R3, zero Suggested findings, and zero Nit findings.
- Affected behavior: service observer/clock injection, semantic stage timing across internal tools, actual tool outcome/duration emission, cleanup timing, terminal closure, and lifecycle-level evidence.
- Fresh reviewer verification: packets 05/12/13 were uniquely complete; focused race test passed in 1.046s; service package passed in 6.211s; vet and `git diff --check` passed. Those commands did not exercise an attached production timing accumulator.
- Roadmap carryover: `milestone-task=cleanup-observation`; approved SDD scenario S07 still requires linked raw-free stage/tool/cleanup/total timing and outcomes.

## Finding Resolution Map

| Finding | Mode | Exact fix | Changed precondition |
|---------|------|-----------|----------------------|
| R1 | `direct-fix` | Snapshot observer/clock in `apps/edge/internal/service/service.go`, default the clock safely, and initialize the request accumulator in `apps/edge/internal/service/single_request.go` before the accepted event and executor launch. | Real service requests, rather than only direct accumulator tests, own a non-nil timing accumulator. |
| R2 | `direct-fix` | Repair stage/tool/cleanup lifecycle ownership in `apps/edge/internal/service/single_request.go`, `apps/edge/internal/service/single_request_tool_loop.go`, and `apps/edge/internal/service/single_request_observation.go`. | Each semantic stage, actual tool, cleanup gate, and terminal has one correctly bounded observation across success/error/cancel. |
| R3 | `direct-fix` | Replace state-only/manual-only claims with deterministic real-lifecycle assertions in `apps/edge/internal/service/single_request_observation_test.go`. | Fresh tests fail when service injection or any required lifecycle seam is absent. |

`ownership_closed=true`: all findings are repository-local direct fixes and require no external decision, authorization, or unordered dependency.

## Analysis

### Files Read

- `apps/edge/internal/service/service.go`
- `apps/edge/internal/service/single_request.go`
- `apps/edge/internal/service/single_request_tool_loop.go`
- `apps/edge/internal/service/single_request_observation.go`
- `apps/edge/internal/service/single_request_test.go`
- `apps/edge/internal/service/single_request_workspace_test.go`
- `apps/edge/internal/service/single_request_tool_loop_test.go`
- `apps/edge/internal/service/single_request_cleanup_test.go`
- `apps/edge/internal/service/single_request_observation_test.go`
- `agent-spec/runtime/edge-node-execution.md`
- `agent-roadmap/phase/knowledge-tool-optimization-extension/milestones/iop-owned-single-request-agent-execution.md`
- `agent-roadmap/sdd/knowledge-tool-optimization-extension/iop-owned-single-request-agent-execution/SDD.md`
- `agent-test/local/rules.md`
- `agent-test/local/edge-smoke.md`
- `agent-task/m-iop-owned-single-request-agent-execution/14+05,12,13_observation_timing/plan_local_G06_1.log`
- `agent-task/m-iop-owned-single-request-agent-execution/14+05,12,13_observation_timing/code_review_cloud_G07_1.log`

### SDD Criteria

- SDD: `agent-roadmap/sdd/knowledge-tool-optimization-extension/iop-owned-single-request-agent-execution/SDD.md`, status `[승인됨]`, lock released.
- Header scope: `milestone-task=cleanup-observation`.
- Target scenario: S07 requires request-owned cleanup plus raw-free stage/tool/total timing and outcomes across success, error, and cancellation.
- Evidence Map row S07 requires cleanup race, user-result preservation, and raw-free timing/log/metric allowlist evidence. This packet supplies the service-local lifecycle/timing portion; existing packet 13 evidence owns cleanup preservation, and production adapters remain a later packet.
- The checklist therefore requires real service/handle lifecycle emission and deterministic success/error/cancel/tool/cleanup assertions rather than accumulator-only calls.

### Verification Context

- No separate verification handoff was supplied.
- Repository-native sources: `agent-test/local/rules.md`, `agent-test/local/edge-smoke.md`, the active plan commands, Go package layout, related service tests, and approved SDD S07.
- Preconditions: packets 05, 12, and 13 each have one active/archive `complete.log`; the reviewer re-ran and passed those checks.
- Commands: fresh focused race test, fresh service package test, vet, and whitespace validation.
- Constraints: preserve unrelated dirty work; do not use Agent-Ops dispatcher as a product test harness; do not add production metrics/log adapters, wire, API, config, or external smoke.
- External Verification Preflight: not applicable. S07 service timing is deterministic in the current checkout; actual Claude/Mac S12 smoke is a separate Milestone task and is not claimed here.
- Gap: current passing tests bypass the real observer injection and lifecycle seams.
- Confidence: high; static call-site search proves the accumulator constructor is test-only and cleanup/tool emission hooks are disconnected.

### Test Coverage Gaps

- Service entrypoint injection: uncovered; no test asserts observer events from `Service.StartSingleRequest`.
- Semantic stage timing: uncovered; manual tests do not execute real state transitions.
- Tool observation: uncovered; no production tool DTO exists and tests assert only accumulator totals.
- Cleanup timing/outcome: uncovered; cleanup tests assert lifecycle ordering but not observation.
- Failure/cancel terminal closure and observer failure isolation: only state-only or accumulator-only tests exist; integrated observation assertions are missing.

### Symbol References

- No symbol is renamed or removed. Existing call sites of `startSingleRequestWithToolLoop` are in `single_request_cleanup_test.go` and `single_request.go`; preserve the compatibility wrapper or update those exact call sites.
- `SetSingleRequestObserver`, `SetSingleRequestClock`, and `singleRequestObserverSnapshot` currently have no production-lifecycle consumer outside their definitions.

### Split Judgment

- Keep one plan. Service injection, semantic stage pause/resume, tool completion, cleanup completion, terminal timing, and the integration oracle form one exactly-once lifecycle invariant; splitting would leave an independently unjudgeable intermediate state.

### Scope Rationale

- Include only the service-owned closed observation boundary, lifecycle hooks, and deterministic service tests.
- Exclude Prometheus/zap adapters, bootstrap wiring, Node logs, HTTP correlation, API/wire/config/schema changes, living-spec updates, dashboards, and actual Claude/Mac smoke. Those are not needed to close R1-R3 and remain outside this packet.

### Final Routing

- `evaluation_mode=isolated-reassessment`; finalizer `finalize-task-policy.sh` in `pair` mode.
- Build closures: scope/context/verification/evidence/ownership/decision are all true. Scores `1/2/1/2/1 = G07`; base `local-fit`, final basis `recovery-boundary`, lane `cloud`, catalog `worker/cloud/G07`, filename `PLAN-cloud-G07.md`.
- Review closures: scope/context/verification/evidence/ownership/decision are all true. Scores `1/2/1/2/1 = G07`; basis `official-review`, lane `cloud`, catalog `review/cloud/G07`, filename `CODE_REVIEW-cloud-G07.md`.
- `large_indivisible_context=false`; positive loop risks are `temporal_state`, `concurrent_consistency`, `boundary_contract`, and `variant_product` (`loop_risk_count=4`).
- Recovery signals: `review_rework_count=1`, `evidence_integrity_failure=true`; capability gap: none.

## Implementation Checklist

- [ ] Attach one failure-isolated timing accumulator to every admitted service request before the accepted event and executor launch, with a safe real-clock default.
- [ ] Emit each semantic provider stage, actual Node tool, cleanup gate, and terminal exactly once with closed success/error/cancel values while excluding tool time from stage pure time.
- [ ] Prove the real service/handle lifecycle for success, error, cancellation, tool pause/resume, cleanup success/failure, terminal races, observer failure, and sentinel exclusion with deterministic tests.
- [ ] Run dependency, focused race, package, vet, and whitespace verification with cache bypass where supported.
- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.

## Implementation Plan

### [REVIEW_API-1] Wire and correct lifecycle timing ownership

**Problem**

- `apps/edge/internal/service/service.go:116` omits observer and clock from the request snapshot, and `apps/edge/internal/service/single_request.go:164` never initializes `h.timing`.
- `apps/edge/internal/service/single_request.go:337` ends a stage only on tool resume and overwrites active stage starts on ordinary transitions.
- `apps/edge/internal/service/single_request_observation.go:531` records tool duration without emitting a tool event, and `apps/edge/internal/service/single_request.go:464` never calls cleanup timing hooks.

**Solution**

Replace the disconnected construction:

```go
// service.go:116-139 and single_request.go:164-185, before
executor := s.singleRequestExecutor
return startSingleRequestWithToolLoop(ctx, executor, continuation, s, req)

h := &singleRequestHandle{
    // no timing accumulator
}
```

with one immutable observer/clock snapshot and accumulator created before the accepted event:

```go
observer, clock := s.singleRequestObserverSnapshot()
return startSingleRequestWithToolLoopObserved(ctx, executor, continuation, s, req, observer, clock)

h := &singleRequestHandle{
    timing: newSingleRequestTimingAccumulator(clock, observer),
}
```

Keep the existing helper signature as a compatibility wrapper for current tests, or update every exact call site. Default a nil clock inside the constructor to `singleRequestRealClock{}` before calling `Now`.

Model timing by semantic stage identity, not raw envelope count. Enter a stage once; pause it on `internal_tool`; emit one actual tool event with closed outcome/error and duration at tool completion; resume the saved stage without ending it; close the stage only when its canonical stage changes or the request fails, cancels, or enters finalizing. Start cleanup once in `requestTerminalCleanupLocked`, finish it once in `completeTerminalCleanupLocked`, and close outstanding stage/tool state before the terminal total. Observer errors and panics remain isolated from locks and lifecycle outcomes.

**Modified Files and Checklist**

- [ ] `apps/edge/internal/service/service.go` — snapshot observer/clock with the admitted service dependencies and pass them to request construction.
- [ ] `apps/edge/internal/service/single_request.go` — initialize timing and own semantic stage, cleanup, failure/cancel, and terminal transitions.
- [ ] `apps/edge/internal/service/single_request_tool_loop.go` — finish each actual tool observation at the common success/error/cancel outcome seams.
- [ ] `apps/edge/internal/service/single_request_observation.go` — provide nil-safe clock construction and exactly-once stage/tool/cleanup accumulation/emission.

**Test Strategy**

- Production code changes require the integration regressions in REVIEW_API-2; do not add a second test file or mock production adapters.

**Verification**

- `go test -race ./apps/edge/internal/service -run 'TestSingleRequestObservation' -count=1`
- Expected: real lifecycle observation tests pass without races or duplicated events.

### [REVIEW_API-2] Replace manual-only evidence with lifecycle integration tests

**Problem**

- `apps/edge/internal/service/single_request_observation_test.go:295` calls the coordinator but never injects/captures observations, while timing assertions directly invoke accumulator hooks in a sequence that production does not use.

**Solution**

Add deterministic tests through `Service.StartSingleRequest` and the existing scripted tool/cleanup fixtures. Inject a manual clock and capturing or failing observer before request start. Advance the clock at controlled executor, tool-runtime, cleanup, and acknowledgement gates. Assert exact event order/counts, stage identities, tool/cleanup outcomes, terminal outcome, no duplicate terminal under races, observer failure isolation, sentinel exclusion, and `stage_active + tool + cleanup <= total`. Ensure the test fails if accumulator construction, tool emission, cleanup hooks, or stage-close hooks are removed.

**Modified Files and Checklist**

- [ ] `apps/edge/internal/service/single_request_observation_test.go` — add real lifecycle success/error/cancel/tool/cleanup/race/failure-isolation assertions and retain useful accumulator unit coverage.

**Test Strategy**

- Add `TestSingleRequestObservationLifecycleIntegration` with table-driven success, cleanup failure, provider failure, and cancellation cases.
- Add or extend `TestSingleRequestObservationToolTimingExcludedFromStage` to use the real service/tool loop and assert one stage event across tool pause/resume.
- Extend terminal race and observer panic/error tests to assert request outcomes remain unchanged and only one terminal event is emitted through the real handle.

**Verification**

- `go test -race ./apps/edge/internal/service -run 'TestSingleRequestObservation' -count=1`
- Expected: deterministic integrated lifecycle assertions pass fresh and under the race detector.

## Modified Files Summary

| File | Item |
|------|------|
| `apps/edge/internal/service/service.go` | REVIEW_API-1 |
| `apps/edge/internal/service/single_request.go` | REVIEW_API-1 |
| `apps/edge/internal/service/single_request_tool_loop.go` | REVIEW_API-1 |
| `apps/edge/internal/service/single_request_observation.go` | REVIEW_API-1 |
| `apps/edge/internal/service/single_request_observation_test.go` | REVIEW_API-2 |
| `agent-task/m-iop-owned-single-request-agent-execution/14+05,12,13_observation_timing/CODE_REVIEW-cloud-G07.md` | REVIEW_API-1, REVIEW_API-2 |

## Final Verification

1. `test -f agent-task/m-iop-owned-single-request-agent-execution/05+03_single_ingress/complete.log || test "$(compgen -G 'agent-task/archive/*/*/m-iop-owned-single-request-agent-execution/05+03_single_ingress/complete.log' | wc -l)" -eq 1`
2. `test -f agent-task/m-iop-owned-single-request-agent-execution/12+05,06,08,11_internal_tool_loop/complete.log || test "$(compgen -G 'agent-task/archive/*/*/m-iop-owned-single-request-agent-execution/12+05,06,08,11_internal_tool_loop/complete.log' | wc -l)" -eq 1`
3. `test -f agent-task/m-iop-owned-single-request-agent-execution/13+12_workspace_cleanup/complete.log || test "$(compgen -G 'agent-task/archive/*/*/m-iop-owned-single-request-agent-execution/13+12_workspace_cleanup/complete.log' | wc -l)" -eq 1`
4. `go test -race ./apps/edge/internal/service -run 'TestSingleRequestObservation' -count=1`
5. `go test ./apps/edge/internal/service -count=1`
6. `go vet ./apps/edge/internal/service`
7. `git diff --check`

Expected: predecessor checks are unique; real service requests emit exact closed stage/tool/cleanup/terminal timing across success/error/cancel; observer failure cannot alter lifecycle outcomes; the service package remains clean. Cached test output is not acceptable.

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