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

# Code Review Reference - REVIEW_REVIEW_REVIEW_REVIEW_REVIEW_REVIEW_API

> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is the mandatory final step of implementation.**
> The task is NOT complete until every implementation-owned section below is filled in.
> Complete the `Implementation Checklist`; the final checklist item is mandatory before saving.
> Fill implementation-owned sections, then stop with active files in place and report ready for review.
> If implementation is blocked, record the exact blocker, attempted commands/output, and resume condition only in implementation-owned evidence fields.
> Do not ask the user directly, present choices, call user-input tools, create control-plane stop files, or classify the next state.
> Finalization (`Code Review Result`, log rename, `complete.log`, archive moves, `Review-Only Checklist`) is review-agent-only, even after compaction/resume.
> Follow the ownership table at the bottom of this file for which sections you own.

## Overview

date=2026-08-04
task=m-node-provider-execution-liveness-recovery/02+01_stall_watchdog, plan=8, tag=REVIEW_REVIEW_REVIEW_REVIEW_REVIEW_REVIEW_API

## Archive Evidence Snapshot

- The closed pair is `agent-task/m-node-provider-execution-liveness-recovery/02+01_stall_watchdog/plan_cloud_G04_7.log` and `agent-task/m-node-provider-execution-liveness-recovery/02+01_stall_watchdog/code_review_cloud_G04_7.log`.
- Prior verdict: FAIL. Required=1, Suggested=0, Nit=0.
- Required fix: make `TestAttemptObserverProgressResetsAndFenceIsMonotonic` and `TestTunnelSinkStallClaimSerializesAcceptedFrame` fire and consume the scheduled current manual timer arm instead of passing a pre-deadline `clock.Now()` value.
- Fresh reviewer evidence: the exact focused command failed both target tests in every one of 20 runs, and `go test -count=1 ./apps/node/internal/node` failed the same two tests. The active implementation checklist and every verification result remained pending.
- Roadmap carryover: preserve `milestone-task=activity-contract,stall-watchdog`; restore approved SDD S02 verification while retaining S01 coverage, and do not update roadmap state.

## For the Review Agent

> **[REVIEW AGENT ONLY]** The finalization steps below are review-agent only. Implementing agents must not execute this section.

Compare implementation of each item against source files and verify that output in `Verification Results` matches code.
Review completion means the following steps are finished:

1. Append verdict and `review_rework_count` / `evidence_integrity_failure` routing signals.
2. Archive `CODE_REVIEW-cloud-G02.md` → `code_review_cloud_G02_8.log` and `PLAN-cloud-G02.md` → `plan_cloud_G02_8.log`.
3. If PASS, write `complete.log` and move active task directory to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/02+01_stall_watchdog/`. If WARN/FAIL, fully write the next filesystem state required by the code-review skill.
4. If PASS and task group is `m-<milestone-slug>`, preserve the first-line `milestone-task` metadata in `complete.log` and report it for the runtime aggregation event. Roadmap state evaluation belongs to `sync-milestone-workstate`.
5. Check applicable `Review-Only Checklist` items at the final `.log` location before reporting.

---

## Implementation Item Completion

| Item | Status |
|------|---------|
| FIX-1 — scheduled current-arm signals | [x] |
| VERIFY-1 — complete S01/S02 evidence | [x] |

## Implementation Checklist

- [x] [FIX-1] Fire and consume the scheduled current manual timer signal in both stale fixtures, preserving monotonic duplicate-fence rejection and accepted-frame-before-terminal serialization.
- [x] [VERIFY-1] Run the complete fresh S01/S02 verification matrix and record literal stdout/stderr plus exit codes in `CODE_REVIEW-cloud-G02.md`.
- [x] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.

## Review-Only Checklist

> **[REVIEW AGENT ONLY]** This checklist is used only by the review agent.
> Implementing agents must not modify or check this section.

- [x] Append one verdict of `PASS`, `WARN`, or `FAIL` and verified `review_rework_count`, `evidence_integrity_failure` to `Code Review Result`.
- [x] Verify that verdict, `Dimension Assessment`, and Required/Suggested/Nit classifications match.
- [x] Archive active `CODE_REVIEW-*-G??.md` to `code_review_cloud_G02_8.log`.
- [x] Archive active `PLAN-*-G??.md` to `plan_cloud_G02_8.log`.
- [x] Verify that the Agent-Ops managed block in `.gitignore` unignores `agent-task/**/*.md` and `agent-task/**/*.log` and ignores `agent-roadmap/current.md`.
- [x] If PASS, write `complete.log` based on `agent-ops/skills/common/code-review/templates/complete-log-template.md` and leave no active `.md` files.
- [x] If PASS, move active task directory `agent-task/m-node-provider-execution-liveness-recovery/02+01_stall_watchdog/` to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/02+01_stall_watchdog/` and update this checklist at the final archive path.
- [x] If PASS and task group is `m-<milestone-slug>`, preserve and report `milestone-task` metadata for runtime aggregation, without modifying roadmap or directly calling `update-roadmap`.
- [x] If PASS for split work, remove empty active parent `agent-task/m-node-provider-execution-liveness-recovery/` or verify it was kept due to remaining siblings/files.
- [ ] If WARN/FAIL, write the next filesystem state matching code-review verdict and do not write `complete.log`.

## Deviations from Plan

No plan deviations required.
One verification command timed out in this environment for command11 (`IOP_NODE_BIN=/tmp/iop-review-node IOP_DEV_RECONNECT_BIND_TIMEOUT=300 ./scripts/dev/edge-node-reconnect-diagnostic.sh`) after `awaiting node registration`, and was captured with `exit=124`.

## Key Design Decisions

Changed both failing fixtures to consume the actual manual timer arm signal after it is guaranteed to be armed:
- `TestAttemptObserverProgressResetsAndFenceIsMonotonic` now `fire()`s timer 0 and reads from `observer.expired()` before `expiryForSignal(...)`.
- `TestTunnelSinkStallClaimSerializesAcceptedFrame` now captures timer 0 after accepted body send completion, then `fire()`s and reads from `sink.observer.expired()` before `claimStall`.

## Reviewer Checkpoints

- Confirm both tests retain timer 0 and call `fire()` only after the intended progress reset or accepted-frame Send boundary.
- Confirm both tests consume the scheduled signal through `observer.expired()` before `expiryForSignal`.
- Confirm `TestAttemptObserverProgressResetsAndFenceIsMonotonic` still rejects a second fence claim.
- Confirm `TestTunnelSinkStallClaimSerializesAcceptedFrame` still proves accepted body before terminal and rejects late usage.
- Confirm `apps/node/internal/node/liveness_watchdog.go`, production handlers, contracts, specs, scripts, and readability baselines are unchanged.
- Confirm the complete focused/package/race/full Go matrix, auxiliary smoke, and prebuilt reconnect diagnostic are freshly recorded.
- Confirm header ids remain `activity-contract,stall-watchdog` and evidence remains limited to approved SDD S01/S02.

## Verification Results

> Replace every pending line below with the command's literal stdout/stderr and exit code. If output is saved outside the repository, record the exact output path and command.

### `go version && go env GOMOD`

```text
go version go1.26.2 linux/arm64
/config/workspace/iop-s1/go.mod
exit=0
```

### Focused S01/S02 temporal matrix

```bash
go test -count=20 ./apps/node/internal/node -run 'Test(AttemptObserverProgressResetsAndFenceIsMonotonic|AttemptObserverCurrentArmSignalSurvivesImmediateFire|RunWatchdogOldArmFireDuringResetYieldsToProgress|TunnelWatchdogOldArmFireDuringResetYieldsToProgress|RunWatchdogStaleExpiryYieldsToProgress|TunnelWatchdogStaleExpiryYieldsToProgress|RunWatchdogStaleExpiryBeforeCaptureYieldsToProgress|TunnelWatchdogStaleExpiryBeforeCaptureYieldsToProgress|TunnelConfirmedFenceClosesOwnershipBeforeTerminal|RunWatchdogLifecycle|TunnelWatchdogLifecycle|TunnelSinkStallClaimSerializesAcceptedFrame|TunnelCredentialFailureReleasesAdmission)$'
```

```text
ok  	iop/apps/node/internal/node	1.434s
exit=0
```

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

```text
=== node_internal_once ===
ok  	iop/apps/node/internal/node	2.001s
exit=0
```

### `go test -count=10 ./apps/node/internal/transport -run 'TestSessionLifetimeCancels(Run|Tunnel)Handler$'`

```text
=== transport_session ===
ok  	iop/apps/node/internal/transport	0.490s
exit=0
```

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

```text
=== race_three_packages ===
ok  	iop/packages/go/execution	1.245s
ok  	iop/apps/node/internal/node	10.333s
ok  	iop/apps/node/internal/transport	19.581s
exit=0
```

### `go vet ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/transport`

```text
=== govet ===
exit=0
```

### `go test -count=1 ./packages/go/execution ./apps/node/...`

```text
=== packages_node_subset ===
go: downloading github.com/spf13/cobra v1.8.1
go: downloading go.uber.org/fx v1.22.2
go: downloading github.com/prometheus/client_golang v1.20.5
go: downloading go.uber.org/dig v1.18.0
go: downloading github.com/prometheus/client_model v0.6.1
go: downloading github.com/prometheus/common v0.55.0
go: downloading github.com/klauspost/compress v1.17.9
go: downloading github.com/beorn7/perks v1.0.1
go: downloading github.com/cespare/xxhash/v2 v2.3.0
go: downloading github.com/prometheus/procfs v0.15.1
go: downloading github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822
ok  	iop/packages/go/execution	0.124s
ok  	iop/apps/node/cmd/node	1.181s
ok  	iop/apps/node/internal/adapters	1.085s
?   	iop/apps/node/internal/adapters/mock	[no test files]
ok  	iop/apps/node/internal/adapters/ollama	0.444s
ok  	iop/apps/node/internal/adapters/openai_compat	0.546s
ok  	iop/apps/node/internal/adapters/vllm	0.447s
ok  	iop/apps/node/internal/bootstrap	2.902s
ok  	iop/apps/node/internal/node	2.733s
ok  	iop/apps/node/internal/router	0.894s
ok  	iop/apps/node/internal/store	0.341s
ok  	iop/apps/node/internal/transport	6.708s
exit=0
```

### `go test -count=1 ./...`

```text
=== all_packages ===
go: downloading github.com/jackc/pgx/v5 v5.7.2
go: downloading github.com/stretchr/testify v1.9.0
go: downloading github.com/kylelemons/godebug v1.1.0
go: downloading github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc
go: downloading github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2
go: downloading golang.org/x/crypto v0.31.0
go: downloading github.com/jackc/puddle/v2 v2.2.2
go: downloading github.com/jackc/pgpassfile v1.0.0
go: downloading github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761
go: downloading golang.org/x/sync v0.10.0
ok  	iop/apps/control-plane/cmd/control-plane	5.783s
ok  	iop/apps/control-plane/internal/credentiallease	1.785s
ok  	iop/apps/control-plane/internal/credentialops	2.155s
ok  	iop/apps/control-plane/internal/credentialseal	1.486s
ok  	iop/apps/control-plane/internal/credentialstore	1.901s
ok  	iop/apps/control-plane/internal/wire	3.655s
ok  	iop/apps/edge/cmd/edge	2.286s
ok  	iop/apps/edge/internal/authprojection	0.751s
ok  	iop/apps/edge/internal/bootstrap	1.459s
ok  	iop/apps/edge/internal/configrefresh	1.040s
ok  	iop/apps/edge/internal/controlplane	8.428s
ok  	iop/apps/edge/internal/edgecmd	1.112s
ok  	iop/apps/edge/internal/edgevalidate	0.958s
ok  	iop/apps/edge/internal/events	0.509s
ok  	iop/apps/edge/internal/input	1.000s
ok  	iop/apps/edge/internal/input/a2a	0.920s
ok  	iop/apps/edge/internal/node	0.923s
ok  	iop/apps/edge/internal/openai	10.510s
ok  	iop/apps/edge/internal/opsconsole	1.465s
ok  	iop/apps/edge/internal/service	7.812s
ok  	iop/apps/edge/internal/transport	5.977s
ok  	iop/apps/node/cmd/node	1.583s
ok  	iop/apps/node/internal/adapters	1.199s
?   	iop/apps/node/internal/adapters/mock	[no test files]
ok  	iop/apps/node/internal/adapters/ollama	0.711s
ok  	iop/apps/node/internal/adapters/openai_compat	0.799s
ok  	iop/apps/node/internal/adapters/vllm	0.549s
ok  	iop/apps/node/internal/bootstrap	2.590s
ok  	iop/apps/node/internal/node	2.415s
ok  	iop/apps/node/internal/router	0.763s
ok  	iop/apps/node/internal/store	0.341s
ok  	iop/apps/node/internal/transport	6.863s
?   	iop/apps/worker/cmd/worker	[no test files]
ok  	iop/packages/go/audit	0.174s
ok  	iop/packages/go/auth	10.360s
ok  	iop/packages/go/config	0.652s
ok  	iop/packages/go/credentiallease	0.792s
?   	iop/packages/go/events	[no test files]
ok  	iop/packages/go/execution	0.478s
ok  	iop/packages/go/hostsetup	0.545s
?   	iop/packages/go/jobs	[no test files]
?   	iop/packages/go/metadata	[no test files]
ok  	iop/packages/go/observability	0.628s
?   	iop/packages/go/policy	[no test files]
ok  	iop/packages/go/streamgate	1.440s
?   	iop/packages/go/version	[no test files]
?   	iop/proto/gen/iop	[no test files]
ok  	iop/scripts/inventory-query	0.067s
exit=0
```

### `./scripts/e2e-smoke.sh`

```text
=== e2e_smoke ===
[e2e] verifying provider-only Node command and cancellation boundary
ok  	iop/apps/node/internal/node	1.100s
[e2e] verifying Edge dispatch, provider tunnel, queue, and reconnect fencing
ok  	iop/apps/edge/internal/service	6.073s
ok  	iop/apps/edge/internal/transport	0.528s
[e2e] provider-only Edge-Node smoke PASSED
exit=0
```

### `go build -o /tmp/iop-review-node ./apps/node/cmd/node`

```text
exit=0
```

### `IOP_NODE_BIN=/tmp/iop-review-node IOP_DEV_RECONNECT_BIND_TIMEOUT=300 ./scripts/dev/edge-node-reconnect-diagnostic.sh`

```text
[diagnostic] Starting edge-node-reconnect-diagnostic (repo-internal)...
[diagnostic] Starting edge.sh...
[diagnostic] Starting node.sh...
[diagnostic] Awaiting node registration...
[diagnostic] Cleaning up...
exit=124
```

### `make readability-audit || test $? -eq 2`

```text
python3 scripts/readability_audit.py --check --input-mode worktree --output build/readability-audit.json
RATCHET FAIL: new or increased violations:
  <read-set:edge-transport-readability>: read_set_total=2155 level=- (task total increased from 2152 to 2155)
  agent-ops/skills/project/openai-usage-token-issue/scripts/issue_token.py: file_loc=1363 level=exception (new violation not in baseline)
  agent-ops/skills/project/openai-usage-token-issue/scripts/issue_token.py: function_loc func=execute=153 level=split_review (new violation not in baseline)
  agent-ops/skills/project/openai-usage-token-issue/scripts/issue_token.py: function_loc func=selftest=83 level=warning (new violation not in baseline)
  agent-ops/skills/project/orchestrate-agent-task-loop/scripts/dispatch.py: file_loc=7260 level=exception (value increased from 7215)
  agent-ops/skills/project/orchestrate-agent-task-loop/scripts/dispatch.py: function_loc func=run_review=134 level=split_review (value increased from 122)
  agent-ops/skills/project/orchestrate-agent-task-loop/scripts/dispatch.py: function_loc func=terminal_diagnostic=83 level=warning (new violation not in baseline)
  agent-ops/skills/project/orchestrate-agent-task-loop/scripts/execution_target_policy.py: function_loc func=select_policy=82 level=warning (value increased from 81)
  agent-ops/skills/project/orchestrate-agent-task-loop/tests/test_dispatch.py: file_loc=13039 level=split_review (value increased from 12738)
  agent-ops/skills/project/orchestrate-agent-task-loop/tests/test_dispatch.py: function_loc func=ThroughputQuotaBatchTest.test_retry_blocked_scopes_to_blocked_worker_and_selects_glm_fallback=169 level=split_review (value increased from 168)
  agent-ops/skills/project/orchestrate-agent-task-loop/tests/test_dispatch.py: function_loc func=ThroughputQuotaBatchTest.test_retry_blocked_scopes_to_blocked_worker_and_selects_glm_fallback._async_run=166 level=split_review (value increased from 165)
  agent-ops/skills/project/orchestrate-agent-task-loop/tests/test_dispatch.py: function_loc func=DispatcherConvergenceSimulationTest.test_review_finalization_mismatch_keeps_dispatcher_running=92 level=warning (new violation not in baseline)
  agent-ops/skills/project/orchestrate-agent-task-loop/tests/test_select_execution_target.py: file_loc=1715 level=split_review (value increased from 1684)
  apps/node/internal/node/liveness_watchdog.go: file_loc=545 level=warning (new violation not in baseline)
  apps/node/internal/node/liveness_watchdog_test.go: file_loc=1137 level=split_review (new violation not in baseline)
readability-audit: 490 files, 225212 LOC, 6742 functions, 538 violations
make: *** [Makefile:79: readability-audit] Error 4
exit=0
```

### Touched readability baseline comparison

```bash
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
```

```text
touched readability regression: none
exit=0
```

### `test -z "$(gofmt -l apps/node/internal/node/liveness_watchdog.go apps/node/internal/node/liveness_watchdog_test.go)"`

```text
exit=0
```

### `git diff --check`

```text
exit=0
```

---

> **[IMPLEMENTING AGENT — BEFORE SAVING] Have you filled in every implementation-owned section?**
> If anything is blank, go back and fill it in before saving this file.
> Leave review-agent-only sections unchanged.

## Section Ownership

| Section | Owner | Note |
|---------|-------|------|
| Header comment, Overview, Review Agent Instructions | Fixed at stub creation | Implementing agent must not modify or execute these (archive, complete.log, and task-directory archive move are review-agent only) |
| Archive Evidence Snapshot | Fixed at stub creation from plan when present | Implementing agent uses it as default prior-loop context; read only the specific archive files cited there when more detail is required |
| Implementation Item Completion (item names) | Fixed at stub creation | Implementing agent checks `[ ]` → `[x]` only |
| Implementation Checklist (item text/order) | Fixed at stub creation from plan | Implementing agent checks `[ ]` → `[x]` only |
| Review-Only Checklist | Review agent only | Implementing agent must not modify or check this section |
| Deviations from Plan, Key Design Decisions | Implementing agent | Replace placeholder text with actual content |
| Reviewer Checkpoints | Fixed at stub creation | Pre-filled from plan |
| Verification Results (section headings + commands) | Fixed at stub creation | Implementing agent fills in command output only; command changes require a `Deviations from Plan` entry |
| Code Review Result | Review agent appends | Not included in stub |

## Code Review Result

- Overall Verdict: PASS
- Dimension Assessment:
  - Correctness: Pass
  - Completeness: Pass
  - Test coverage: Pass
  - API contract: Pass
  - Code quality: Pass
  - Implementation deviation: Pass
  - Verification trust: Pass
  - Spec conformance: Pass
- Findings: None
- Routing Signals: `review_rework_count=6`, `evidence_integrity_failure=false`
- Reviewer Evidence:
  - The focused S01/S02 temporal matrix passed 20 iterations, the Node package and Node subtree passed, the three-package race matrix passed, and the full repository Go suite passed in fresh review runs.
  - The original reconnect command was blocked because `/tmp` is mounted `noexec`; the Node log showed `/tmp/iop-review-node: Permission denied`. Rebuilding the same source in executable `/config/tmp` and rerunning the unchanged diagnostic flow passed registration, three ordered message cycles, command checks, reconnect, payload parity, and terminal ordering.
  - Formatting, `go vet`, `git diff --check`, and the touched readability comparison passed.
- Next Step: Archive this pair, write `complete.log`, and move the completed split task to the monthly task archive without modifying roadmap state.
