<!-- task=m-node-provider-execution-liveness-recovery/01_activity_contract plan=3 tag=REVIEW_API milestone-task=activity-contract -->

# PLAN — Repair Node Provider Activity Contract

## For the Implementing Agent

> **MANDATORY:** Implement only this review-derived checklist, preserve unrelated user changes, and keep edits inside the `activity-contract` slice. Do not update roadmap state, create another plan, commit, push, archive files, create `complete.log`, or perform the official review. After implementation, fill every implementation-owned section of `CODE_REVIEW-cloud-G08.md` and leave both active files in place.

## Background

The previous implementation added provider config, candidate, and protobuf fields for `response_stall_timeout_ms`, plus shared activity classifier scaffolding. Official review found that the effective timeout is dropped before both normalized and tunnel adapters, normalized invalid wire values are silently defaulted, and non-terminal usage activity can be classified as no progress. The implementation evidence also overstates queued/tunnel coverage and incorrectly dismisses new readability regressions. This follow-up repairs the same atomic contract boundary; it does not implement a watchdog, timer, cancellation, retry, or health overlay.

## Archive Evidence Snapshot

- Prior plan: `agent-task/m-node-provider-execution-liveness-recovery/01_activity_contract/plan_local_G06_2.log`.
- Prior review: `agent-task/m-node-provider-execution-liveness-recovery/01_activity_contract/code_review_cloud_G06_2.log`.
- Prior verdict: FAIL with 4 Required findings, 0 Suggested findings, and 0 Nit findings.
- Passing reviewer checks: generators, client tests, targeted/full/race Go tests, vet, test-only E2E, real Edge/Node reconnect diagnostic, and `git diff --check`.
- Failing reviewer check: `make readability-audit` exited 2 and reported new/increased entries in the current slice.
- Mandatory carryover: preserve already-correct config/protobuf/candidate/refresh work while repairing runtime retention, raw-wire validation, activity classification, deterministic coverage, documentation, and evidence integrity.

## 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-ops/rules/project/domain/edge/rules.md`
- `agent-ops/rules/project/domain/client/rules.md`
- `agent-test/local/rules.md`
- `agent-test/local/node-smoke.md`
- `agent-test/local/platform-common-smoke.md`
- `agent-test/local/edge-smoke.md`
- `agent-test/local/client-smoke.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-spec/runtime/provider-pool-config-refresh.md`
- `agent-contract/index.md`
- `agent-contract/inner/execution-runtime.md`
- `agent-contract/inner/edge-node-runtime-wire.md`
- `agent-contract/inner/edge-config-runtime-refresh.md`
- `packages/go/execution/liveness.go`
- `packages/go/execution/liveness_test.go`
- `packages/go/execution/types.go`
- `packages/go/config/provider_types.go`
- `packages/go/config/provider_catalog_validation_config_test.go`
- `proto/iop/runtime.proto`
- `apps/edge/internal/configrefresh/classify.go`
- `apps/edge/internal/configrefresh/provider_classify_test.go`
- `apps/edge/internal/service/model_queue_types.go`
- `apps/edge/internal/service/provider_resolution.go`
- `apps/edge/internal/service/provider_pool.go`
- `apps/edge/internal/service/provider_tunnel.go`
- `apps/edge/internal/service/run_submit.go`
- `apps/edge/internal/service/run_types.go`
- `apps/edge/internal/service/run_wire.go`
- `apps/edge/internal/service/provider_scheduling_advanced_test.go`
- `apps/edge/internal/service/run_command_test.go`
- `apps/edge/internal/service/run_dispatch_internal_test.go`
- `apps/node/internal/node/runtime_bridge.go`
- `apps/node/internal/node/run_handler.go`
- `apps/node/internal/node/tunnel_handler.go`
- `apps/node/internal/node/runtime_bridge_test.go`
- `apps/node/internal/node/provider_tunnel_test.go`
- `apps/node/internal/router/router.go`
- `apps/node/internal/router/router_test.go`
- `configs/edge.yaml`
- `Makefile`
- `scripts/dev/edge-node-reconnect-diagnostic.sh`
- `agent-task/m-node-provider-execution-liveness-recovery/01_activity_contract/PLAN-local-G06.md`
- `agent-task/m-node-provider-execution-liveness-recovery/01_activity_contract/CODE_REVIEW-cloud-G06.md`

### SDD Criteria

- SDD: `agent-roadmap/sdd/operational-observability-provider-management/node-provider-execution-liveness-recovery/SDD.md`; approved, user-reviewed where required, and implementation lock released.
- Scenario: S01, milestone task `activity-contract`.
- This task contributes default/override/invalid timeout evidence and normalized/tunnel start/progress/terminal classification evidence.
- The dependent watchdog task owns fake-clock deadline movement, timer lifecycle, cancellation, and transport recovery evidence. This follow-up must not claim those behaviors as implemented.
- Required invariants: zero/omitted uses `300000`, safe positive values pass through unchanged, negative/overflow values are rejected before provider invocation, terminal type takes precedence over payload, and hard/queue/heartbeat/client-idle timeouts retain separate ownership.

### Verification Context

- Environment: local Go module `/config/workspace/iop-s1/go.mod`; generated Go and Dart protobuf bindings are checked in.
- Fresh reviewer evidence already proves the broad build is green, but those suites do not exercise the missing adapter-visible propagation and raw normalized rejection boundaries.
- `make readability-audit` is a ratchet check: unrelated baseline failures may remain, but this follow-up must eliminate new/increased entries attributable to current-slice changes and record the exact residual output.
- `IOP_DEV_RECONNECT_BIND_TIMEOUT=45 ./scripts/dev/edge-node-reconnect-diagnostic.sh` is the credential-free real-process Edge/Node cycle and remains mandatory.
- No external provider credentials, deployment, migration, or user input is required.

### Test Coverage Gaps

- No adapter-capture test proves normalized `ExecutionSpec.ResponseStallTimeoutMS` receives zero-defaulted or positive values.
- No tunnel-adapter capture proves `ProviderTunnelRequest.ResponseStallTimeoutMS` receives the effective value.
- Normalized negative and overflow wire values are not rejected before router/provider invocation.
- Current tests do not prove queued winner re-resolution, shared-adapter/different-provider values, or direct/legacy default behavior at the runtime consumer.
- The protobuf test does not perform an actual int64 marshal/unmarshal boundary round trip.
- Activity tables omit non-terminal usage with non-zero token counts and terminal-with-payload precedence combinations.
- New tests use sleep-based synchronization and introduce readability ratchet regressions.

### Symbol References

- `packages/go/execution/liveness.go:25-36,103-135` — invalid defaulting and token-count-derived activity classification.
- `packages/go/execution/types.go:18-29` — normalized `ExecutionSpec` currently drops the timeout.
- `apps/node/internal/node/runtime_bridge.go:10-24,57-78` — normalized wire mapping and validation helpers.
- `apps/node/internal/node/run_handler.go:17-39` — normalized handler omits raw-wire validation.
- `apps/node/internal/router/router.go:35-55` — runtime request-to-spec mapping omits the timeout.
- `apps/node/internal/node/tunnel_handler.go:16-53` — tunnel validation result is discarded and the runtime request omits the timeout.
- `apps/edge/internal/service/run_types.go:14-74` — submit DTO has the field but dispatch DTO does not.
- `apps/edge/internal/service/provider_resolution.go`, `provider_pool.go`, `provider_tunnel.go`, and `run_submit.go` — initial and queued winning-provider dispatch facts.
- `apps/edge/internal/service/provider_scheduling_advanced_test.go:857-1056` — claimed initial/queued and normalized/tunnel coverage is incomplete.
- `agent-contract/inner/edge-config-runtime-refresh.md:61,69` — field semantics conflict with the stale no-wire-change statement.

### Split Judgment

- Classification: large review rework across config, generated wire bindings, Edge selection/dispatch, Node runtime boundaries, tests, and living documentation.
- Cohesion: indivisible. The safety invariant is only true if raw validation, zero defaulting, selected-candidate retention, both wire surfaces, both Node consumers, and observable dispatch metadata agree in one change.
- Dependency: this is the foundation task for the later stall-watchdog slice; splitting another producer after that indexed consumer would create an invalid partial contract.
- Scope remains the existing `01_activity_contract` task and `activity-contract` milestone task. No new split task is created.

### Scope Rationale

- In scope: effective timeout validation/defaulting, immutable retention through candidate/DTO/wire/Node runtime, activity classification, deterministic boundary tests, readability cleanup for current-slice regressions, matching contracts/specs, and exact verification evidence.
- Out of scope: starting or resetting timers, watchdog ownership, cancellation, synthesized terminal events, attempt fencing, retry/recovery, health classification, metrics, or roadmap changes.
- Prefer extending the listed existing source and test files. New production files are not needed; any new focused test file must be added to Modified Files Summary before implementation proceeds.

### Final Routing

- `evaluation_mode=isolated-reassessment`; `finalizer=finalize-task-policy.sh`; `finalizer_mode=pair`.
- Build score: `scope=2`, `state=0`, `blast=2`, `evidence=2`, `verification=2` -> G08.
- Build signals: `base_route_basis=local-fit`, `large_indivisible_context=false`, loop risk `boundary_contract` (`count=1`), `review_rework_count=1`, `evidence_integrity_failure=true`; recovery boundary matched.
- Build route: `route_basis=recovery-boundary`, lane `cloud`, file `PLAN-cloud-G08.md`.
- Review score: `scope=2`, `state=0`, `blast=2`, `evidence=2`, `verification=2` -> G08.
- Review route: `route_basis=official-review`, lane `cloud`, adapter/model `codex/gpt-5.6-sol`, reasoning `xhigh`, file `CODE_REVIEW-cloud-G08.md`.

## Implementation Checklist

- [ ] [REVIEW_API-1] Repair raw timeout validation, zero defaulting, and effective-value retention through normalized and tunnel runtime consumers.
- [ ] [REVIEW_API-2] Correct normalized provider activity classification and terminal precedence.
- [ ] [REVIEW_TEST-1] Add deterministic adapter-visible, queue, protobuf, validation, and classifier coverage without readability regressions.
- [ ] [REVIEW_DOC-1] Synchronize matching contracts, living specs, and the example with the repaired behavior.
- [ ] [REVIEW_VERIFY-1] Run all final verification commands and preserve exact, trustworthy evidence.
- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.

### [REVIEW_API-1] Retain one validated effective timeout through both runtime paths

**Problem**

The normalized wire mapper silently defaults invalid values, the handler does not validate raw input, the router drops the field because `ExecutionSpec` lacks it, and the tunnel handler validates but discards the value. `RunDispatch` also cannot report the immutable selected value.

**Solution**

- Establish one validate-then-normalize path: raw zero maps to `DefaultResponseStallTimeoutMS`; safe positive values pass through; negative and duration-overflow values return an error before router or provider invocation. Do not expose an effective helper that silently converts invalid values.
- Add `ResponseStallTimeoutMS int64` to `ExecutionSpec` and copy it from `RunRequest` in the router. Validate normalized protobuf input before creating/resolving the runtime request.
- Assign the validated effective value to `runtime.ProviderTunnelRequest` before tunnel adapter lookup/invocation.
- Add `ResponseStallTimeoutMS int64` to `RunDispatch` and populate it from the actual request/selected attempt for direct, initial, and queued dispatches on normalized and tunnel surfaces.
- Keep `TimeoutSec`, queue timeout, heartbeat/disconnect deadlines, and client response-idle timeout semantically separate.

**Modified files**

- [ ] `packages/go/execution/liveness.go`
- [ ] `packages/go/execution/types.go`
- [ ] `apps/edge/internal/service/provider_resolution.go`
- [ ] `apps/edge/internal/service/provider_pool.go`
- [ ] `apps/edge/internal/service/provider_tunnel.go`
- [ ] `apps/edge/internal/service/run_submit.go`
- [ ] `apps/edge/internal/service/run_types.go`
- [ ] `apps/edge/internal/service/run_wire.go`
- [ ] `apps/node/internal/node/runtime_bridge.go`
- [ ] `apps/node/internal/node/run_handler.go`
- [ ] `apps/node/internal/node/tunnel_handler.go`
- [ ] `apps/node/internal/router/router.go`

**Test Strategy**

Required. Capture the exact value passed to normalized and tunnel adapters for omitted/zero, positive, negative, and overflow inputs. Assert invalid raw values produce an error and no router/provider invocation, and assert `TimeoutSec` is unchanged.

**Verification**

- `go test -count=1 ./packages/go/execution ./apps/edge/internal/service ./apps/node/internal/node ./apps/node/internal/router`
- `go test -race -count=1 ./packages/go/execution ./apps/node/internal/node`

### [REVIEW_API-2] Correct normalized provider activity classification

**Problem**

The classifier treats non-zero token counters as terminal usage and consequently returns `none` for non-terminal delta/reasoning events carrying usage. Terminality belongs to the event type, not token values.

**Solution**

- Preserve terminal event-type precedence for complete, error, and cancelled events even when payload or usage is present.
- For non-terminal delta/reasoning events, treat non-empty delta/message and supported usage observations as progress without deriving terminality from token counts.
- Remove or narrow the exported/internal terminal-usage helper if it no longer expresses a valid contract; do not leave a misleading public API solely for tests.
- Preserve start and unknown/empty semantics and the existing tunnel classifier rules.

**Modified files**

- [ ] `packages/go/execution/liveness.go`
- [ ] `packages/go/execution/liveness_test.go`

**Test Strategy**

Required. Use compact tables for nil/zero/non-zero usage, empty/non-empty delta and message, every terminal kind with payload/usage, start, and unknown events. No wall-clock sleeps.

**Verification**

- `go test -count=1 ./packages/go/execution`
- `go test -race -count=1 ./packages/go/execution`

### [REVIEW_TEST-1] Prove both adapter boundaries and queue re-resolution deterministically

**Problem**

Broad suites pass while the required boundaries remain untested. Existing new tests claim queued and tunnel coverage that they do not execute, use sleep-based synchronization, and increase readability thresholds.

**Solution**

- Add actual protobuf marshal/unmarshal cases for zero, positive, negative, and safe-boundary int64 values on both request messages.
- Capture normalized `ExecutionSpec` and tunnel `ProviderTunnelRequest` at the adapter boundary. Prove zero/default and positive propagation, invalid-wire rejection before invocation, and separation from hard timeout.
- Exercise both initial and queued winner selection, including a queued re-resolution where the selected provider changes, and prove two providers sharing one adapter retain distinct timeout values.
- Cover direct/legacy zero at the actual runtime consumer and timeout-only refresh classification with omitted/explicit-zero equivalence.
- Replace `time.Sleep` synchronization with channels or another deterministic barrier. Split helpers/tables into the closest existing files so `make readability-audit` reports no new/increased current-slice violations.
- Keep generator output generated by Make targets only.

**Modified files**

- [ ] `packages/go/config/provider_catalog_validation_config_test.go`
- [ ] `apps/edge/internal/configrefresh/provider_classify_test.go`
- [ ] `apps/edge/internal/service/provider_scheduling_advanced_test.go`
- [ ] `apps/edge/internal/service/run_command_test.go`
- [ ] `apps/edge/internal/service/run_dispatch_internal_test.go`
- [ ] `apps/node/internal/node/runtime_bridge_test.go`
- [ ] `apps/node/internal/node/provider_tunnel_test.go`
- [ ] `apps/node/internal/router/router_test.go`
- [ ] `proto/iop/runtime.proto`
- [ ] `proto/gen/iop/runtime.pb.go`
- [ ] `apps/client/lib/gen/proto/iop/runtime.pb.dart`
- [ ] `apps/client/lib/gen/proto/iop/runtime.pbjson.dart`

**Test Strategy**

Required and deterministic. Each named boundary must fail if the timeout field is removed or ignored. Use exact adapter captures and invocation counts rather than only inspecting intermediate DTOs.

**Verification**

- `make proto`
- `make proto-dart`
- `make client-test`
- `go test -count=1 ./packages/go/config ./apps/edge/internal/configrefresh ./apps/edge/internal/service ./apps/node/internal/node ./apps/node/internal/router`

### [REVIEW_DOC-1] Align contracts and living specifications

**Problem**

One inner contract still says the request messages and wire schema are unchanged, while living spec bodies do not describe the added field or its ownership and rejection behavior.

**Solution**

- Update the execution runtime contract with validate-before-normalize semantics, adapter-visible retention, and corrected non-terminal usage classification.
- Update the Edge-Node wire contract with both additive int64 fields, zero/default compatibility, invalid raw rejection, and Node retention on normalized and tunnel paths.
- Update the config/refresh contract to remove the stale no-wire-change statement while preserving the Edge-local attribution-field distinction.
- Update both matching living spec bodies and change records with provider config ownership, selected-candidate propagation, restart-required refresh, Node retention, and timeout separation. Keep timer/watchdog lifecycle explicitly out of scope.
- Preserve the provider-first example with a valid value.

**Modified files**

- [ ] `agent-contract/inner/execution-runtime.md`
- [ ] `agent-contract/inner/edge-node-runtime-wire.md`
- [ ] `agent-contract/inner/edge-config-runtime-refresh.md`
- [ ] `agent-spec/runtime/edge-node-execution.md`
- [ ] `agent-spec/runtime/provider-pool-config-refresh.md`
- [ ] `configs/edge.yaml`

**Test Strategy**

No standalone documentation test. Cross-check every statement against the schema, runtime mappings, refresh classifier, and adapter-capture tests.

**Verification**

- `git diff --check`

### [REVIEW_VERIFY-1] Produce exact reviewable evidence

**Problem**

The failed review record understated changed-file counts and incorrectly described new readability findings as entirely pre-existing, reducing evidence trust.

**Solution**

- Run every Final Verification command against the finished worktree and record command, exit status, and concise exact output in the new review stub.
- For readability, distinguish exact unrelated baseline entries from current-slice entries; the latter must be zero. Do not claim a non-zero audit result is clean.
- Record generated-file changes and final `git diff --stat`; do not hand-edit generated files.

**Modified files**

- [ ] `agent-task/m-node-provider-execution-liveness-recovery/01_activity_contract/CODE_REVIEW-cloud-G08.md`

**Test Strategy**

Required as verification evidence. A command failure or unexpected generated file is a real deviation to record, not text to reinterpret.

**Verification**

- All commands in Final Verification.

## Modified Files Summary

| Path | Action | Checklist |
|---|---|---|
| `packages/go/execution/liveness.go` | modify | REVIEW_API-1, REVIEW_API-2 |
| `packages/go/execution/liveness_test.go` | modify | REVIEW_API-2 |
| `packages/go/execution/types.go` | modify | REVIEW_API-1 |
| `packages/go/config/provider_types.go` | preserve/modify if validation cleanup requires | REVIEW_API-1 |
| `packages/go/config/provider_catalog_validation_config_test.go` | modify | REVIEW_TEST-1 |
| `proto/iop/runtime.proto` | preserve/modify if schema correction requires | REVIEW_TEST-1 |
| `proto/gen/iop/runtime.pb.go` | regenerate | REVIEW_TEST-1 |
| `apps/client/lib/gen/proto/iop/runtime.pb.dart` | regenerate | REVIEW_TEST-1 |
| `apps/client/lib/gen/proto/iop/runtime.pbjson.dart` | regenerate | REVIEW_TEST-1 |
| `apps/edge/internal/configrefresh/classify.go` | preserve/modify for readability | REVIEW_TEST-1 |
| `apps/edge/internal/configrefresh/provider_classify_test.go` | modify | REVIEW_TEST-1 |
| `apps/edge/internal/service/model_queue_types.go` | preserve/modify for propagation | REVIEW_API-1 |
| `apps/edge/internal/service/provider_resolution.go` | modify | REVIEW_API-1 |
| `apps/edge/internal/service/provider_pool.go` | modify | REVIEW_API-1 |
| `apps/edge/internal/service/provider_tunnel.go` | modify | REVIEW_API-1 |
| `apps/edge/internal/service/run_submit.go` | modify | REVIEW_API-1 |
| `apps/edge/internal/service/run_types.go` | modify | REVIEW_API-1 |
| `apps/edge/internal/service/run_wire.go` | modify | REVIEW_API-1 |
| `apps/edge/internal/service/provider_scheduling_advanced_test.go` | modify | REVIEW_TEST-1 |
| `apps/edge/internal/service/run_command_test.go` | modify | REVIEW_TEST-1 |
| `apps/edge/internal/service/run_dispatch_internal_test.go` | modify | REVIEW_TEST-1 |
| `apps/node/internal/node/runtime_bridge.go` | modify | REVIEW_API-1 |
| `apps/node/internal/node/run_handler.go` | modify | REVIEW_API-1 |
| `apps/node/internal/node/tunnel_handler.go` | modify | REVIEW_API-1 |
| `apps/node/internal/node/runtime_bridge_test.go` | modify | REVIEW_TEST-1 |
| `apps/node/internal/node/provider_tunnel_test.go` | modify | REVIEW_TEST-1 |
| `apps/node/internal/router/router.go` | modify | REVIEW_API-1 |
| `apps/node/internal/router/router_test.go` | modify | REVIEW_TEST-1 |
| `agent-contract/inner/execution-runtime.md` | modify | REVIEW_DOC-1 |
| `agent-contract/inner/edge-node-runtime-wire.md` | modify | REVIEW_DOC-1 |
| `agent-contract/inner/edge-config-runtime-refresh.md` | modify | REVIEW_DOC-1 |
| `agent-spec/runtime/edge-node-execution.md` | modify | REVIEW_DOC-1 |
| `agent-spec/runtime/provider-pool-config-refresh.md` | modify | REVIEW_DOC-1 |
| `configs/edge.yaml` | preserve/modify if example correction requires | REVIEW_DOC-1 |
| `agent-task/m-node-provider-execution-liveness-recovery/01_activity_contract/CODE_REVIEW-cloud-G08.md` | update evidence | REVIEW_VERIFY-1 |

## Final Verification

1. `go version && go env GOMOD`
2. `flutter --version`
3. `make proto`
4. `make proto-dart`
5. `make client-test`
6. `go test -count=1 ./packages/go/execution ./packages/go/streamgate ./packages/go/config`
7. `go test -count=1 ./packages/go/execution ./apps/node/...`
8. `go test -count=1 ./packages/go/streamgate ./apps/edge/internal/openai ./apps/edge/internal/service ./apps/edge/internal/controlplane`
9. `go test -count=1 ./packages/go/execution ./packages/go/config ./apps/edge/internal/service ./apps/edge/internal/configrefresh ./apps/node/internal/node ./apps/node/internal/router`
10. `go test -race -count=1 ./packages/go/execution ./apps/node/internal/node`
11. `go vet ./packages/go/execution ./packages/go/config ./apps/edge/internal/service ./apps/edge/internal/configrefresh ./apps/node/internal/node ./apps/node/internal/router`
12. `go test -count=1 ./...`
13. `./scripts/e2e-smoke.sh`
14. `IOP_DEV_RECONNECT_BIND_TIMEOUT=45 ./scripts/dev/edge-node-reconnect-diagnostic.sh`
15. `make readability-audit`
16. `gofmt -l packages/go/execution packages/go/config apps/edge/internal/configrefresh apps/edge/internal/service apps/node/internal/node apps/node/internal/router`
17. `git diff --check`
18. `git diff --stat`

Record each command, exit status, and concise output in `CODE_REVIEW-cloud-G08.md`. If a generator changes an unlisted file or readability reports a new/increased current-slice entry, stop and reconcile the documented scope through the owning runtime.

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