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

# PLAN — Repair Stale Watchdog Expiry and Confirmed Tunnel Ownership Ordering

## 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-G08.md` with literal results, 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. 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 now releases failed credential admissions, serializes tunnel sends with terminal claims, passes the repeated/race/full-suite checks, and meets the touched readability ratchet. Official review still found that an already-consumed timer tick can fence an attempt after intervening provider progress and that a confirmed tunnel terminal can become observable before its Node-owned resources are released. These two ordering defects prevent the S01/S02 evidence from closing.

## Archive Evidence Snapshot

- The current pair will archive as `agent-task/m-node-provider-execution-liveness-recovery/02+01_stall_watchdog/plan_cloud_G09_3.log` and `agent-task/m-node-provider-execution-liveness-recovery/02+01_stall_watchdog/code_review_cloud_G09_3.log`.
- Prior verdict: FAIL. Required=2, Suggested=0, Nit=0.
- Required fixes: invalidate an expiry after intervening normalized/tunnel progress; release confirmed tunnel admission, run-manager, and credential ownership before publishing the confirmed terminal.
- Fresh reviewer verification passed the focused repeated tests, session lifetime tests, `go test -race -count=3`, vet, Node packages, `go test -count=1 ./...`, `./scripts/e2e-smoke.sh`, reconnect diagnostic, formatting, and `git diff --check`. The touched readability comparison passed; the repository audit retained unrelated worktree ratchet failures.
- 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

- Runtime predecessor `01_activity_contract` remains satisfied by `agent-task/archive/2026/08/m-node-provider-execution-liveness-recovery/01_activity_contract/complete.log`.
- Implement FIX-1 and FIX-2 before TEST-1 so the temporal fixtures assert the final shared ordering contract.

## Analysis

### Files Read

- `agent-task/m-node-provider-execution-liveness-recovery/02+01_stall_watchdog/PLAN-cloud-G09.md`
- `agent-task/m-node-provider-execution-liveness-recovery/02+01_stall_watchdog/CODE_REVIEW-cloud-G09.md`
- `agent-task/m-node-provider-execution-liveness-recovery/02+01_stall_watchdog/code_review_cloud_G08_2.log`
- `agent-task/archive/2026/08/m-node-provider-execution-liveness-recovery/01_activity_contract/complete.log`
- `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-contract/inner/execution-runtime.md`
- `agent-contract/inner/edge-node-runtime-wire.md`
- `agent-spec/runtime/edge-node-execution.md`
- `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/runtime_bridge.go`
- `apps/node/internal/node/node.go`
- `apps/node/internal/node/run_manager.go`
- `apps/node/internal/transport/session.go`
- `packages/go/execution/liveness.go`
- `packages/go/execution/failure.go`
- `packages/go/execution/types.go`
- `apps/node/internal/node/liveness_watchdog_test.go`
- `apps/node/internal/node/provider_tunnel_test.go`
- `apps/node/internal/node/run_cancel_test.go`
- `apps/node/internal/transport/session_test.go`
- `packages/go/execution/liveness_test.go`

### 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 active Milestone.
- S01 requires provider progress to reset the clock and earlier request/session boundaries to retain their classification.
- S02 requires threshold and timer/event/cancel/close races to yield exactly one terminal and a trustworthy confirmed/unconfirmed local fence.
- Evidence Map rows S01/S02 require fake-clock normalized/tunnel lifecycle and race evidence. FIX-1, FIX-2, TEST-1, and the repeated/race commands are derived directly from those rows.

### Verification Context

- No external handoff was supplied. Repository-native local rules, the current checkout, the approved SDD, contracts, and current tests are authoritative.
- Current preflight: Go `go1.26.2 linux/arm64`; module `/config/workspace/iop-s1/go.mod`; no external provider or credential is required.
- Fresh reviewer checks passed: focused count-10 tests, session lifetime count-10 tests, race count 3, vet, Node packages, full Go suite, auxiliary E2E, reconnect diagnostic, touched readability comparison, formatting, and diff checks.
- The current tests do not cover an expiry already consumed by `awaitAttempt` while progress wins the sink authority, or ownership state at the instant a confirmed tunnel terminal becomes observable.
- Fresh execution is required; Go test cache output is not acceptable. Confidence: high, because both failures follow from deterministic line ordering and have channel/manual-clock reproducers.

### Test Coverage Gaps

- Stale expiry after intervening normalized progress: not covered; current code can fence immediately after a valid reset.
- Stale expiry after intervening tunnel progress or a blocked accepted send: not covered; the current sink test expects the stale claim to succeed.
- Confirmed tunnel terminal visibility versus admission/run/credential cleanup: not covered; current lifecycle test checks ownership only after the handler returns.
- Confirmed/unconfirmed close grace, late-output drop, caller/deadline/session precedence, credential preflight admission, metadata cloning, and ordinary regression paths already have coverage and must remain green.

### Symbol References

- No public symbol is renamed or removed.
- Package-local `attemptClock`, `attemptObserver.observe`, `attemptObserver.claimFence`, `awaitAttempt`, `terminalDeferringSink.claimStall`, and `tunnelSink.claimStall` are referenced only in `apps/node/internal/node/liveness_watchdog.go` and `apps/node/internal/node/liveness_watchdog_test.go`; update every package-local call when the expiry validity input changes.

### Split Judgment

- Keep one atomic follow-up. Expiry validity and confirmed cleanup-before-terminal are two halves of the same terminal-authority invariant, and their deterministic tests must observe the shared sink/cleanup ordering in one independently passing packet.
- Predecessor index `01` is satisfied by the archived `complete.log` listed above.

### Scope Rationale

- In scope: shared Node watchdog time/epoch validation, confirmed tunnel cleanup ordering, and deterministic normalized/tunnel regressions.
- Excluded: activity/config/protobuf propagation, provider health probing, Edge health overlay/recovery, metrics, contract wording, living spec wording, readability baselines/read sets, and roadmap state. Existing contracts/spec already state the desired behavior.

### Final Routing

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

## Implementation Checklist

- [ ] [FIX-1] Reject a consumed watchdog expiry after intervening normalized or tunnel progress while preserving exactly-once terminal/fence behavior.
- [ ] [FIX-2] Close confirmed tunnel admission, run-manager, and credential ownership before publishing the confirmed stall terminal; retain unconfirmed ownership until provider return.
- [ ] [TEST-1] Add deterministic normalized/tunnel stale-expiry and confirmed-terminal ownership-order regressions and rerun the S01/S02 repeated/race evidence.
- [ ] Run every command in Final Verification and record literal output in `CODE_REVIEW-cloud-G08.md`.
- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.

### [FIX-1] Invalidate stale expiry claims after progress

**Problem**

At `apps/node/internal/node/liveness_watchdog.go:159-162`, `awaitAttempt` consumes `observer.expired()` before acquiring a sink's emission authority. At lines 343-352 and 413-420, both sink claims call `observer.claimFence()` without proving that no progress reset occurred after that tick. A progress emission can therefore reset the timer and finish before the old tick still fences the attempt.

**Solution**

Before (`liveness_watchdog.go:159-162,343-352,413-420`):

```go
case <-observer.expired():
    if !contextStillActive(execCtx) || !claimStall() {
        continue
    }
```

After:

```go
case expiry := <-observer.expired():
    if !contextStillActive(execCtx) || !claimStall(expiry) {
        continue
    }
```

- Bind an expiry to the activity state that produced it, using an epoch-bearing signal or monotonic `Now`/last-progress check through the injected clock.
- Make both normalized and tunnel claims reject the expiry when progress won the emission authority after the tick.
- Preserve provider terminal precedence, context/deadline/session precedence, exact close grace, and exactly-once fencing.

**Modified Files and Checklist**

- [ ] `apps/node/internal/node/liveness_watchdog.go` — add expiry validity and thread it through shared claim coordination.

**Test Strategy**

Required through TEST-1. Add channel-controlled handler-level tests that consume/fire the old timer, let normalized/tunnel progress win the sink authority, assert no immediate stall, then fire the reset timer and assert one stall terminal.

**Verification**

- `go test -count=10 ./apps/node/internal/node -run 'Test(Run|Tunnel)WatchdogStaleExpiryYieldsToProgress$'`

### [FIX-2] Publish confirmed tunnel terminal only after local cleanup

**Problem**

At `apps/node/internal/node/liveness_watchdog.go:295-300`, the confirmed path sends the stall terminal before `cleanup.run`. The terminal can become visible while the admission ticket, run handle, and plaintext credential material are still owned, contradicting `attempt_fence=confirmed` and allowing a concurrent next dispatch to observe stale local capacity.

**Solution**

Before (`liveness_watchdog.go:295-300`):

```go
_ = sink.emitClaimedTerminal(context.Background(), terminal)
if result.providerReturned {
    cleanup.run()
} else {
    cleanup.afterProviderReturn(providerDone)
}
```

After:

```go
if result.providerReturned {
    cleanup.run()
} else {
    cleanup.afterProviderReturn(providerDone)
}
return sink.emitClaimedTerminal(context.Background(), terminal)
```

- Release confirmed local ownership only after actual provider return and before the terminal send.
- Keep unconfirmed cleanup deferred until actual provider return and keep late frames fenced.
- Preserve terminal send/error behavior unless a concrete existing contract requires propagation changes.

**Modified Files and Checklist**

- [ ] `apps/node/internal/node/liveness_watchdog.go` — reorder confirmed cleanup and terminal publication without early unconfirmed release.

**Test Strategy**

Required through TEST-1. Add a sender that inspects or blocks at terminal visibility and proves ticket/run/credential cleanup already completed for confirmed, while the existing unconfirmed fixture continues to prove retention.

**Verification**

- `go test -count=10 ./apps/node/internal/node -run 'TestTunnelConfirmedFenceClosesOwnershipBeforeTerminal$'`

### [TEST-1] Add deterministic S01/S02 ordering regressions

**Problem**

`apps/node/internal/node/liveness_watchdog_test.go:312-345` verifies only that the send lock is held and then expects a claim to succeed immediately after progress. Lines 535-559 check confirmed ownership after handler return, not when the terminal becomes observable. Neither fixture detects the two review failures.

**Solution**

- Extend the existing manual clock only as needed to represent expiry validity deterministically; do not add scheduler sleeps to temporal assertions.
- Add `TestRunWatchdogStaleExpiryYieldsToProgress` and `TestTunnelWatchdogStaleExpiryYieldsToProgress` with blocked send/progress ordering, no terminal from the old tick, a new full threshold, and exactly one final terminal.
- Add `TestTunnelConfirmedFenceClosesOwnershipBeforeTerminal` with capacity-1 admission, run-manager state, credential bytes, provider return within exactly `defaultAttemptCloseGrace`, and terminal visibility assertions.
- Keep existing confirmed/unconfirmed, deadline/cancel/session, credential failure, metadata spoof/cloning, and late-output tests unchanged unless helper signatures require mechanical updates.

**Modified Files and Checklist**

- [ ] `apps/node/internal/node/liveness_watchdog_test.go` — deterministic stale-expiry and cleanup-before-terminal fixtures.

**Test Strategy**

Required. The new named tests are the direct S01/S02 regression oracle; repeated and race runs prove deterministic ordering and shared-state safety.

**Verification**

- `go test -count=10 ./apps/node/internal/node -run 'Test((Run|Tunnel)WatchdogStaleExpiryYieldsToProgress|TunnelConfirmedFenceClosesOwnershipBeforeTerminal|RunWatchdogLifecycle|TunnelWatchdogLifecycle|TunnelSinkStallClaimSerializesAcceptedFrame)$'`
- `go test -race -count=3 ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/transport`

## Modified Files Summary

| Path | Action | Checklist |
|------|--------|-----------|
| `apps/node/internal/node/liveness_watchdog.go` | modify | FIX-1, FIX-2 |
| `apps/node/internal/node/liveness_watchdog_test.go` | modify | TEST-1 |
| `agent-task/m-node-provider-execution-liveness-recovery/02+01_stall_watchdog/CODE_REVIEW-cloud-G08.md` | update evidence | all |

## Final Verification

1. `go version && go env GOMOD`
2. `go test -count=10 ./apps/node/internal/node -run 'Test((Run|Tunnel)WatchdogStaleExpiryYieldsToProgress|TunnelConfirmedFenceClosesOwnershipBeforeTerminal|RunWatchdogLifecycle|TunnelWatchdogLifecycle|TunnelSinkStallClaimSerializesAcceptedFrame|TunnelCredentialFailureReleasesAdmission)$'`
3. `go test -count=10 ./apps/node/internal/transport -run 'TestSessionLifetimeCancels(Run|Tunnel)Handler$'`
4. `go test -race -count=3 ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/transport`
5. `go vet ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/transport`
6. `go test -count=1 ./packages/go/execution ./apps/node/...`
7. `go test -count=1 ./...`
8. `./scripts/e2e-smoke.sh`
9. `IOP_DEV_RECONNECT_BIND_TIMEOUT=45 ./scripts/dev/edge-node-reconnect-diagnostic.sh`
10. `make readability-audit || test $? -eq 2`
11. `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`
12. `test -z "$(gofmt -l apps/node/internal/node/liveness_watchdog.go apps/node/internal/node/liveness_watchdog_test.go)"`
13. `git diff --check`

Expected: commands 1-9 and 11-13 exit 0. Command 10 may exit 0 or the known Make exit 2 only; command 11 must prove no touched-function/read-set regression and remaining unrelated audit findings must be recorded literally. The new ordering tests must fail on the reviewed implementation and pass after the fix. Do not modify readability baselines.

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