<!-- task=m-node-provider-execution-liveness-recovery/02+01_stall_watchdog plan=3 tag=REVIEW_API milestone-task=activity-contract,stall-watchdog -->

# PLAN — Repair Node Stall Watchdog Ownership and Evidence

## For the Implementing Agent

> **MANDATORY:** Implement only this follow-up checklist and preserve unrelated worktree changes. Run every verification command, fill all implementation-owned sections of `CODE_REVIEW-cloud-G09.md` with literal results, keep the active pair in place, and report ready for review. If blocked, record the exact blocker, commands/output, and resume condition only in implementation-owned evidence. Do not ask the user, call user-input tools, create stop files, classify next state, archive logs, or write `complete.log`; finalization belongs to the official code-review agent.

## Background

The watchdog implementation passes package, race, full-suite, and local process diagnostics, but official review found two correctness defects and no integrated S01/S02 temporal evidence. Credential validation can leak tunnel admission, and a frame already past the tunnel gate can be delivered after the watchdog terminal. The same change also increased directly touched readability metrics beyond their ratcheted values.

## Archive Evidence Snapshot

- Current pair will archive as `agent-task/m-node-provider-execution-liveness-recovery/02+01_stall_watchdog/plan_cloud_G08_2.log` and `agent-task/m-node-provider-execution-liveness-recovery/02+01_stall_watchdog/code_review_cloud_G08_2.log`.
- Prior verdict: FAIL. Required=4, Suggested=0, Nit=0.
- Required fixes: release admission on every pre-provider tunnel failure; serialize tunnel frame acceptance/send with stall terminal authority; add deterministic normalized/tunnel/session watchdog evidence; remove directly increased readability violations without editing the baseline.
- Fresh reviewer evidence: focused tests, `go test -race -count=3`, vet, `go test -count=1 ./...`, `./scripts/e2e-smoke.sh`, and the reconnect diagnostic passed. `make readability-audit` failed with directly increased `Node.OnRunRequest`, `Node.OnProviderTunnelRequest`, `newSession`, and `node-core-readability` values plus unrelated worktree findings.
- Roadmap carryover: preserve `milestone-task=activity-contract,stall-watchdog`; satisfy approved SDD S01/S02 evidence only and do not update roadmap state.

## Dependencies and Execution Order

- `01_activity_contract` is satisfied by `agent-task/archive/2026/08/m-node-provider-execution-liveness-recovery/01_activity_contract/complete.log`.
- Complete FIX-1 and FIX-2 before TEST-1; finish DOC-1 after code and deterministic evidence agree.

## Analysis

### Files Read

- `AGENTS.md`
- `agent-ops/rules/project/rules.md`
- `agent-ops/rules/common/rules-roadmap.md`
- `agent-ops/rules/common/rules-agent-spec.md`
- `agent-ops/rules/project/domain/node/rules.md`
- `agent-ops/rules/project/domain/platform-common/rules.md`
- `agent-ops/rules/project/domain/testing/rules.md`
- `agent-test/local/rules.md`
- `agent-test/local/node-smoke.md`
- `agent-test/local/platform-common-smoke.md`
- `agent-test/local/testing-smoke.md`
- `agent-roadmap/phase/operational-observability-provider-management/PHASE.md`
- `agent-roadmap/phase/operational-observability-provider-management/milestones/node-provider-execution-liveness-recovery.md`
- `agent-roadmap/sdd/operational-observability-provider-management/node-provider-execution-liveness-recovery/SDD.md`
- `agent-spec/index.md`
- `agent-spec/runtime/edge-node-execution.md`
- `agent-contract/index.md`
- `agent-contract/inner/execution-runtime.md`
- `agent-contract/inner/edge-node-runtime-wire.md`
- `apps/node/internal/node/node.go`
- `apps/node/internal/node/liveness_watchdog.go`
- `apps/node/internal/node/run_handler.go`
- `apps/node/internal/node/runtime_sink.go`
- `apps/node/internal/node/tunnel_handler.go`
- `apps/node/internal/node/liveness_watchdog_test.go`
- `apps/node/internal/node/run_cancel_test.go`
- `apps/node/internal/node/provider_tunnel_test.go`
- `apps/node/internal/transport/session.go`
- `apps/node/internal/transport/session_test.go`
- `packages/go/execution/liveness.go`
- `packages/go/execution/failure.go`
- `packages/go/execution/types.go`
- `agent-task/m-node-provider-execution-liveness-recovery/02+01_stall_watchdog/PLAN-cloud-G08.md`
- `agent-task/m-node-provider-execution-liveness-recovery/02+01_stall_watchdog/CODE_REVIEW-cloud-G08.md`
- `agent-task/archive/2026/08/m-node-provider-execution-liveness-recovery/01_activity_contract/complete.log`

### SDD Criteria

- SDD: `agent-roadmap/sdd/operational-observability-provider-management/node-provider-execution-liveness-recovery/SDD.md`; status `[승인됨]`, lock released, no user review.
- Header ids remain `activity-contract,stall-watchdog`; both ids exist in the selected Milestone.
- Target S01: provider activity only resets the clock, terminal stops it, and earlier deadline/transport loss retains its boundary.
- Target S02: threshold, event/cancel/close races produce one terminal and one confirmed/unconfirmed local fence, with late output rejected.
- Evidence Map rows S01/S02 require fake-clock normalized/tunnel lifecycle tables, threshold races, exactly-once terminal, confirmed/unconfirmed fixtures, and late-event fencing. TEST-1 and the final repeated/race commands are derived directly from those rows.

### Verification Context

- No external handoff was supplied. Repository-native local rules and the current checkout are authoritative.
- Current environment preflight passed: Go `go1.26.2 linux/arm64`, module `/config/workspace/iop-s1/go.mod`; no external provider or credential is required.
- Fresh reviewer commands passed: focused package tests, race count 3, vet, full Go suite, auxiliary E2E, reconnect diagnostic, formatting, and `git diff --check`.
- Required gap: current tests never drive either handler through watchdog expiry or close grace. `make readability-audit` fails partly for unrelated changes, so success is a deterministic comparison against ratcheted values for the touched functions and `node-core-readability`; the baseline must not change.
- Fresh execution is required (`-count=1`, repeated temporal tests, and race). Go test cache output is not acceptable for acceptance evidence.
- Confidence: high; both defects follow directly from ownership and lock ordering in the reviewed code, and all required runners are available locally.

### Test Coverage Gaps

- Credential failure after `admission.acquire`: uncovered; add capacity-1 failure-then-success regression.
- Tunnel frame accepted before stall claim: uncovered; add a channel-controlled blocked sender proving terminal cannot overtake an accepted frame and no frame follows terminal.
- Watchdog threshold/progress/terminal races on normalized and tunnel handlers: uncovered; add injected-clock handler fixtures.
- Exact 5s close grace, confirmed/unconfirmed cleanup, ticket/run/drain/credential lifetime, and release-once: uncovered; add manual-clock and channel ownership assertions.
- Caller identity spoof resistance in normalized domain/wire and tunnel metadata: only constructor metadata is covered; extend through handler/protobuf output.
- Session disconnect context propagation: implementation exists but `session_test.go` has no lifetime assertion; add run and tunnel listener context cancellation evidence.
- Current package/race/E2E tests cover ordinary execution and reconnect but cannot substitute for these deterministic S01/S02 cases.

### Symbol References

- No public symbol is renamed or removed.
- Internal coordination remains at `newAttemptObserver`, `attemptObserver.observe/claimFence`, `terminalDeferringSink.Emit/claimStall/Flush`, `tunnelSink.EmitTunnelFrame/claimStall`, and the run/tunnel session listeners. Update all package-local call sites if helpers move between existing files.

### Split Judgment

- Keep one atomic follow-up. Admission ownership, terminal send serialization, coordinator extraction, fake-clock evidence, and readability ratchet form one exactly-once lifecycle invariant; no child has a safe independent PASS state.
- Runtime predecessor index `01` is satisfied by the archived `complete.log` listed above.

### Scope Rationale

- In scope: Node normalized/tunnel watchdog coordination, session listener lifetime wiring, deterministic tests, living spec evidence, and task-local readability conformance.
- Excluded: config/protobuf/provider-pool propagation already completed by `01_activity_contract`; provider health probing, Edge health overlay, recovery eligibility/retry, metrics, and roadmap state.
- Do not modify `scripts/readability_baseline.json`, `scripts/readability_read_sets.json`, unrelated `agent-ops` files, or unrelated Edge transport findings.

### Final Routing

- `evaluation_mode=isolated-reassessment`; `finalizer=finalize-task-policy.sh`, `finalizer_mode=pair`.
- Build closures: scope/context/verification/evidence/ownership/decision all true; no capability gap.
- Build scores: scope=2, state=2, blast=1, evidence=2, verification=2 => G09; base/route basis `grade-boundary`, lane `cloud`, `PLAN-cloud-G09.md`.
- Build signals: `large_indivisible_context=false`; positive risks `temporal_state`, `concurrent_consistency`, `boundary_contract`, `variant_product` (4); `review_rework_count=1`; `evidence_integrity_failure=true`; risk and recovery boundaries matched without replacing the grade basis.
- Review closures: scope/context/verification/evidence/ownership/decision all true; scores 2/2/1/2/2 => G09; `official-review`, cloud, Codex `gpt-5.6-sol` xhigh, `CODE_REVIEW-cloud-G09.md`.

## Implementation Checklist

- [ ] [FIX-1] Release tunnel admission on every pre-provider error and serialize accepted frames with watchdog terminal authority.
- [ ] [FIX-2] Extract focused shared watchdog/session lifecycle helpers while preserving deadline, cancel, disconnect, cleanup, and metadata contracts.
- [ ] [TEST-1] Add deterministic S01/S02 normalized, tunnel, transport, close-grace, ownership, spoof-resistance, and regression evidence.
- [ ] [DOC-1] Reconcile the living spec and prove touched readability metrics do not exceed their baseline values.
- [ ] Run every command in Final Verification and record literal output in `CODE_REVIEW-cloud-G09.md`.
- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.

### [FIX-1] Restore tunnel admission and terminal ordering

**Problem**

At `apps/node/internal/node/tunnel_handler.go:58`, ticket ownership begins before credential validation, but returns at lines 74-90 bypass cleanup. At lines 212-225, a frame passes the fence under `tunnelSink.mu` and is sent after unlocking, so lines 228-239 can claim and send a stall terminal first.

**Solution**

Before (`tunnel_handler.go:58-90,212-239`):

```go
ticket, err := admission.acquire()
// credential validation returns without ticket.release()

s.mu.Unlock()
return s.emit(ctx, frame)
```

After:

```go
ticket, err := admission.acquire()
preProviderOwned := true
defer func() {
    if preProviderOwned {
        ticket.release()
    }
}()
// transfer ownership only to the provider lifecycle cleanup

// One emission authority serializes classification, fence claim, and Send.
```

- Use an explicit ownership transfer or idempotent cleanup primitive; never release a running unconfirmed attempt early.
- Serialize accepted tunnel sends with stall/provider terminal claims so no accepted frame can appear after terminal.
- Preserve credential zeroization only after real provider return and keep confirmed/unconfirmed semantics unchanged.

**Modified Files and Checklist**

- [ ] `apps/node/internal/node/tunnel_handler.go` — close pre-provider ticket paths and serialize tunnel emissions.
- [ ] `apps/node/internal/node/liveness_watchdog.go` — host shared ownership/emission helpers when extraction reduces handler/read-set size.

**Test Strategy**

Required. Add `TestTunnelCredentialFailureReleasesAdmission` in `provider_tunnel_test.go` with MaxConcurrency=1 and failing credential preflight followed by a valid request. Add `TestTunnelSinkStallClaimSerializesAcceptedFrame` in `liveness_watchdog_test.go` with a blocked sender and channel ordering; assert exactly one terminal and no post-terminal BODY/USAGE/END.

**Verification**

- `go test -count=10 ./apps/node/internal/node -run 'Test(TunnelCredentialFailureReleasesAdmission|TunnelSinkStallClaimSerializesAcceptedFrame)$'`
- `go test -race -count=3 ./apps/node/internal/node -run 'Test(TunnelCredentialFailureReleasesAdmission|TunnelSinkStallClaimSerializesAcceptedFrame)$'`

### [FIX-2] Extract lifecycle coordination without changing boundaries

**Problem**

`Node.OnRunRequest` and `Node.OnProviderTunnelRequest` are each 166 LOC, `newSession` is 112 LOC, and the Node core read set grew by 133 LOC. The coordinator logic is duplicated, and session listeners have no focused lifetime helper or direct cancellation evidence.

**Solution**

Before (`run_handler.go:106-175`, `tunnel_handler.go:132-180`, `session.go:42-152`):

```go
run := func() error {
    // provider goroutine, cleanup, watchdog, grace, terminal, context race
}
// newSession registers every listener inline.
```

After:

```go
// Existing liveness_watchdog.go owns focused coordinator/cleanup helpers.
// OnRunRequest and OnProviderTunnelRequest retain setup and delegate lifecycle.
// newSession constructs state and delegates listener registration helpers.
```

- Extract shared/focused helpers into existing files; do not add framework abstractions or alter public contracts.
- Keep hard deadline/caller cancel/session disconnect precedence, provider-return ownership, terminal-before/after admission ordering, and background behavior.
- Fence or suppress provider output after the request/session boundary is terminal so a dead session cannot be revived for delivery.
- Do not edit readability baselines or unrelated source.

**Modified Files and Checklist**

- [ ] `apps/node/internal/node/liveness_watchdog.go` — focused coordinator and cleanup ownership helpers.
- [ ] `apps/node/internal/node/run_handler.go` — delegate normalized lifecycle.
- [ ] `apps/node/internal/node/runtime_sink.go` — keep sink surface small; move watchdog-only methods if useful.
- [ ] `apps/node/internal/node/tunnel_handler.go` — delegate tunnel lifecycle.
- [ ] `apps/node/internal/transport/session.go` — extract listener registration and connection lifetime helpers.

**Test Strategy**

Required through TEST-1. Existing ordinary execution/cancel/reconnect tests remain regression coverage; new deterministic tests cover extracted concurrency behavior.

**Verification**

- `go test -count=1 ./apps/node/internal/node ./apps/node/internal/transport`
- `go vet ./apps/node/internal/node ./apps/node/internal/transport`

### [TEST-1] Prove S01/S02 lifecycle and ownership deterministically

**Problem**

`liveness_watchdog_test.go:30-64` never fires the observer timer or invokes a handler. The checked TEST-1 claim lacks threshold, grace, lifecycle, wire, spoof, resource, and disconnect assertions.

**Solution**

- Extend the internal manual clock to record every duration, support multiple concurrent timers, and advance timers without `time.Sleep`.
- Use channel-controlled normalized and tunnel providers to cover progress reset, terminal stop, exact threshold, terminal/event/cancel/deadline races, and the exact `defaultAttemptCloseGrace=5s` boundary.
- Assert one terminal, confirmed only after provider return inside grace, unconfirmed ownership retention until eventual return, late output drop, release exactly once, and no Node retry/recovery metadata.
- Assert caller metadata cannot spoof `run_id`/`attempt_id`; compare normalized `Failure.Metadata`, normalized protobuf metadata, and tunnel ERROR metadata with independent cloned maps.
- Test session run and tunnel listener contexts are canceled on remote disconnect and do not deliver a new terminal on the dead session.
- Use synchronization channels/manual clock only; no scheduler sleeps in new temporal tests.

**Modified Files and Checklist**

- [ ] `apps/node/internal/node/liveness_watchdog_test.go` — shared clock, sink ordering, normalized/tunnel lifecycle and ownership tables.
- [ ] `apps/node/internal/node/run_cancel_test.go` — adjacent normalized handler assertions only where external-package fixtures are required.
- [ ] `apps/node/internal/node/provider_tunnel_test.go` — credential admission regression and adjacent tunnel wire assertions.
- [ ] `apps/node/internal/transport/session_test.go` — run/tunnel listener lifetime cancellation on disconnect.

**Test Strategy**

Required; these named tests are the S01/S02 Evidence Map oracle. Keep existing E2E as secondary regression evidence.

**Verification**

- `go test -count=10 ./apps/node/internal/node ./apps/node/internal/transport`
- `go test -race -count=3 ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/transport`

### [DOC-1] Reconcile living spec and readability evidence

**Problem**

The living spec claims a completed Node watchdog but its source evidence/change record only anchors the predecessor activity contract. The readability audit also reports increases in directly touched functions and `node-core-readability`.

**Solution**

- Add the watchdog implementation/test evidence and a dated watchdog/fence change record to the existing spec after code and tests agree.
- Run the audit without changing its baseline/read-set files. Parse `build/readability-audit.json` against `scripts/readability_baseline.json` and require the touched function values and `node-core-readability` total to be no greater than baseline; unrelated worktree failures remain explicitly reported.

**Modified Files and Checklist**

- [ ] `agent-spec/runtime/edge-node-execution.md` — current watchdog evidence and change record.

**Test Strategy**

No separate document test. Contract conformance is mapped to TEST-1 and deterministic readability comparison.

**Verification**

- `git diff --check`

## Modified Files Summary

| Path | Action | Checklist |
|------|--------|-----------|
| `apps/node/internal/node/liveness_watchdog.go` | modify | FIX-1, FIX-2 |
| `apps/node/internal/node/run_handler.go` | modify | FIX-2 |
| `apps/node/internal/node/runtime_sink.go` | modify | FIX-2 |
| `apps/node/internal/node/tunnel_handler.go` | modify | FIX-1, FIX-2 |
| `apps/node/internal/transport/session.go` | modify | FIX-2 |
| `apps/node/internal/node/liveness_watchdog_test.go` | modify | TEST-1 |
| `apps/node/internal/node/run_cancel_test.go` | modify | TEST-1 |
| `apps/node/internal/node/provider_tunnel_test.go` | modify | TEST-1 |
| `apps/node/internal/transport/session_test.go` | modify | TEST-1 |
| `agent-spec/runtime/edge-node-execution.md` | modify | DOC-1 |
| `agent-task/m-node-provider-execution-liveness-recovery/02+01_stall_watchdog/CODE_REVIEW-cloud-G09.md` | update evidence | all |

## Final Verification

1. `go version && go env GOMOD`
2. `go test -count=10 ./apps/node/internal/node -run 'Test(TunnelCredentialFailureReleasesAdmission|TunnelSinkStallClaimSerializesAcceptedFrame|RunWatchdogLifecycle|TunnelWatchdogLifecycle)$'`
3. `go test -count=10 ./apps/node/internal/transport -run 'TestSessionLifetimeCancels(Run|Tunnel)Handler$'`
4. `go test -count=1 ./packages/go/execution ./packages/go/streamgate ./packages/go/config`
5. `go test -count=1 ./packages/go/execution ./apps/node/...`
6. `go test -count=10 ./apps/node/internal/node ./apps/node/internal/transport`
7. `go test -race -count=3 ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/transport`
8. `go vet ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/transport`
9. `go test -count=1 ./...`
10. `./scripts/e2e-smoke.sh`
11. `IOP_DEV_RECONNECT_BIND_TIMEOUT=45 ./scripts/dev/edge-node-reconnect-diagnostic.sh`
12. `make readability-audit || test $? -eq 2`
13. `python3 - <<'PY'
import json

audit = json.load(open('build/readability-audit.json', encoding='utf-8'))
baseline = json.load(open('scripts/readability_baseline.json', encoding='utf-8'))
targets = {
    ('apps/node/internal/node/run_handler.go', 'Node.OnRunRequest'),
    ('apps/node/internal/node/tunnel_handler.go', 'Node.OnProviderTunnelRequest'),
    ('apps/node/internal/transport/session.go', 'newSession'),
}
base_functions = {(x['path'], x.get('function')): x['value'] for x in baseline['function_thresholds']}
current_functions = {(x['path'], x.get('function')): x['value'] for x in audit['violations'] if x.get('metric') == 'function_loc'}
bad = {key: current_functions.get(key, 0) for key in targets if current_functions.get(key, 0) > base_functions[key]}
base_sets = {x['task_id']: x['value'] for x in baseline['task_read_set_totals']}
current_sets = {x['task_id']: x['total_loc'] for x in audit['task_read_sets']}
if current_sets['node-core-readability'] > base_sets['node-core-readability']:
    bad['node-core-readability'] = current_sets['node-core-readability']
if bad:
    raise SystemExit(f'touched readability regression: {bad}')
print('touched readability regression: none')
PY`
14. `test -z "$(gofmt -l apps/node/internal/node/liveness_watchdog.go apps/node/internal/node/run_handler.go apps/node/internal/node/runtime_sink.go apps/node/internal/node/tunnel_handler.go apps/node/internal/node/liveness_watchdog_test.go apps/node/internal/node/run_cancel_test.go apps/node/internal/node/provider_tunnel_test.go apps/node/internal/transport/session.go apps/node/internal/transport/session_test.go)"`
15. `git diff --check`

Expected: commands 1-11 and 13-15 exit 0. Command 12 may exit 0 or the known Make exit 2 only; command 13 must prove no touched function/read-set increase and the review must record any remaining unrelated audit findings literally. Do not modify readability baselines to obtain this result.

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