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

# PLAN — Close Response-Stall Boundary Review Gaps

## 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 preceding repair retained the response-stall timeout through the main provider-pool and Node runtime paths and corrected activity classification. Official review still found four contract gaps: the shared validator uses half of the actual safe `time.Duration` range, tunnel validation errors lose their run/tunnel identity, direct calls can inject a provider-owned non-zero wire value, and the recorded queued/shared-adapter coverage does not exist. This follow-up closes those exact gaps without entering watchdog, timer, cancellation, retry, health, or roadmap work.

## Archive Evidence Snapshot

- Prior plan: `agent-task/m-node-provider-execution-liveness-recovery/01_activity_contract/plan_cloud_G08_3.log`.
- Prior review: `agent-task/m-node-provider-execution-liveness-recovery/01_activity_contract/code_review_cloud_G08_3.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, the clean E2E rerun, the real Edge/Node reconnect diagnostic, formatting, and `git diff --check`.
- Failing reviewer evidence: the temporary exact-boundary test rejects safe value `9223372036854`; `make readability-audit` also retains unrelated Edge transport and central AgentOps failures.
- Mandatory carryover: use the exact duration boundary, retain tunnel rejection correlation, enforce direct wire zero, prove queued winner/shared-adapter identity on both request surfaces, and record only evidence actually exercised.

## 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/config/provider_types.go`
- `packages/go/config/provider_stall_timeout_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_stall_timeout_test.go`
- `apps/edge/internal/service/provider_scheduling_advanced_test.go`
- `apps/edge/internal/service/run_dispatch_internal_test.go`
- `apps/node/internal/node/runtime_bridge.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/transport/session.go`
- `agent-task/m-node-provider-execution-liveness-recovery/01_activity_contract/PLAN-cloud-G08.md`
- `agent-task/m-node-provider-execution-liveness-recovery/01_activity_contract/CODE_REVIEW-cloud-G08.md`

### SDD Criteria

- SDD: `agent-roadmap/sdd/operational-observability-provider-management/node-provider-execution-liveness-recovery/SDD.md`; approved and implementation lock released.
- Scenario: S01, milestone task `activity-contract`.
- This task contributes the exact default/override/invalid boundary and immutable selected-provider evidence for normalized and tunnel requests.
- Required invariants: every millisecond value through `math.MaxInt64 / time.Millisecond` is safe, the next value is rejected before provider invocation, direct/non-pool requests send zero, a queued attempt uses its final winning provider's value, and rejection remains observable through the original correlation identity.
- The dependent watchdog task still owns clock/timer movement, cancellation, synthesized terminal events, and recovery behavior.

### Verification Context

- Environment: local Go module `/config/workspace/iop-s1/go.mod`; checked-in protobuf bindings are regenerated only through Make targets.
- Broad suites pass but do not establish the failed exact boundary or the missing ownership variants.
- A temporary reviewer-only focused test reproduced safe-boundary rejection and was removed after execution.
- `make readability-audit` is a ratchet check. Existing unrelated failures may be recorded exactly; no new/increased current-slice entry is acceptable.
- No external provider credentials, deployment, migration, or user input is required.

### Test Coverage Gaps

- The validator tests use an arbitrary overflow value rather than the exact maximum safe millisecond value and its successor.
- Negative/overflow tunnel tests use a nil session, so they cannot observe whether the error frame keeps the original identifiers.
- No direct normalized or tunnel test supplies a non-zero DTO value and proves the actual protobuf still sends zero.
- No queued timeout test forces re-resolution to a different winner after waiting.
- No test distinguishes provider-specific timeout values when two providers share the same adapter instance.
- The implementation record claims all of these variants despite their absence.

### Symbol References

- `packages/go/execution/liveness.go:12-16,34-50` — incorrect safe duration bound and shared validation gate.
- `packages/go/execution/liveness_test.go:12-54` — current default/positive/negative/overflow cases miss the exact edge.
- `apps/node/internal/node/runtime_bridge.go:88-98` — validation error returns an identity-empty tunnel runtime request.
- `apps/node/internal/node/tunnel_handler.go:25-29,147-161` — the empty request is used to build the pre-execution ERROR frame.
- `apps/node/internal/transport/session.go:75-91` — handler errors are only logged after the frame path; there is no alternate correlated response.
- `apps/edge/internal/service/provider_tunnel.go:61-74` — tunnel frames route exclusively by `tunnel_id`.
- `apps/edge/internal/service/run_submit.go:55-60,166-198` — direct normalized dispatch builds from the caller DTO unchanged.
- `apps/edge/internal/service/provider_tunnel.go:194-198,291-318,490-526` — direct tunnel dispatch copies the caller DTO timeout.
- `apps/edge/internal/service/run_submit.go:79-99` and `provider_tunnel.go:201-246` — queued selection already writes the final candidate value; tests must prove re-resolution and identity.
- `agent-contract/inner/edge-node-runtime-wire.md:46` — direct wire-zero, Node default, raw rejection, and selected-provider ownership contract.

### Split Judgment

- Classification: bounded review rework across one shared validator, two direct Edge boundaries, one Node rejection mapping, and focused tests.
- Cohesion: indivisible. The public contract is only repaired when exact numeric validity, correlation, direct ownership, and queued provider identity agree.
- Scope remains the existing `01_activity_contract` task and `activity-contract` milestone task. No new split task is created.

### Scope Rationale

- In scope: exact duration arithmetic, correlation-preserving tunnel rejection, direct wire-zero enforcement, deterministic initial/queued/shared-adapter tests on normalized and tunnel surfaces, and trustworthy verification evidence.
- Out of scope: schema changes, new config fields, watchdog timers, cancellation/retry/health behavior, client idle semantics, roadmap changes, commit, or push.
- Prefer the listed existing source and focused test files. Do not broaden into unrelated queue or transport refactors.

### Final Routing

- `evaluation_mode=isolated-reassessment`; `finalizer=finalize-task-policy.sh`; `finalizer_mode=pair`.
- Build score: `scope=2`, `state=1`, `blast=2`, `evidence=1`, `verification=2` -> G08.
- Build signals: `base_route_basis=local-fit`, `large_indivisible_context=false`, loop risks `boundary_contract` and `variant_product` (`count=2`), `review_rework_count=2`, `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=1`, `blast=2`, `evidence=1`, `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_REVIEW_API-1] Correct the exact safe duration boundary and preserve validate-before-normalize behavior.
- [ ] [REVIEW_REVIEW_API-2] Preserve raw tunnel correlation identity through pre-execution validation errors.
- [ ] [REVIEW_REVIEW_API-3] Enforce direct wire-zero ownership while retaining the final queued provider's value.
- [ ] [REVIEW_REVIEW_TEST-1] Add deterministic exact-boundary, direct, queued, and shared-adapter evidence for both request surfaces.
- [ ] [REVIEW_REVIEW_VERIFY-1] Run final verification and record exact, non-overstated evidence.
- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.

### [REVIEW_REVIEW_API-1] Use the true `time.Duration` millisecond bound

**Problem**

The shared validator uses `(1 << 62) / time.Millisecond`, rejecting valid positive values well below `math.MaxInt64 / time.Millisecond` despite the documented safe-positive pass-through contract.

**Solution**

- Define the maximum safe millisecond value from `math.MaxInt64 / int64(time.Millisecond)` without converting an overflowing value to `time.Duration` first.
- Preserve zero defaulting, positive pass-through, typed validation errors, and the single validate-then-normalize entry point.
- Keep the source comment exact: the bound exists solely to prevent duration conversion overflow.

**Modified files**

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

**Test Strategy**

Required. Assert the exact maximum safe millisecond value is accepted and preserved, the next millisecond is rejected, zero defaults, and negative remains rejected. Exercise the shared validator and config validation/effective helper.

**Verification**

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

### [REVIEW_REVIEW_API-2] Keep tunnel error frames correlated

**Problem**

Tunnel timeout validation fails before runtime request construction, so `sendTunnelError` receives empty identity and Edge cannot route the rejection to the waiting tunnel subscriber.

**Solution**

- Construct or preserve `RunID` and `TunnelID` from the raw protobuf before timeout validation can return.
- Continue rejecting invalid raw values before router lookup, credential consumption, capacity admission, or adapter invocation.
- Send exactly one pre-execution ERROR frame with the original identifiers and retain the current returned validation error for transport logging.

**Modified files**

- [ ] `apps/node/internal/node/runtime_bridge.go`
- [ ] `apps/node/internal/node/provider_tunnel_test.go`

**Test Strategy**

Required. Use the real session pipe to observe negative and overflow rejections. Assert original run/tunnel ids, ERROR kind, validation text, a single frame, and zero adapter calls. Include the exact safe boundary as an accepted adapter-visible value.

**Verification**

- `go test -count=1 ./apps/node/internal/node -run 'StallTimeout|ProviderTunnelRequest'`
- `go test -race -count=1 ./apps/node/internal/node`

### [REVIEW_REVIEW_API-3] Make timeout ownership explicit at Edge dispatch boundaries

**Problem**

Direct normalized and tunnel builders copy any non-zero caller DTO value even though only provider-pool selection owns a non-zero effective timeout. This conflicts with direct wire-zero compatibility and leaves queued identity unproven.

**Solution**

- Force `ResponseStallTimeoutMS` to zero at the direct normalized and direct tunnel dispatch boundaries before protobuf construction.
- Keep provider-pool initial and queued paths authoritative: after final admission/re-resolution, overwrite the request with the selected candidate's effective value immediately before building the protobuf.
- Keep `RunDispatch.ResponseStallTimeoutMS` observable as the effective default for direct calls and the selected provider value for provider-pool calls.
- Do not change the public protobuf schema or permit prepare hooks/caller DTOs to override the final provider selection.

**Modified files**

- [ ] `apps/edge/internal/service/run_submit.go`
- [ ] `apps/edge/internal/service/provider_tunnel.go`
- [ ] `apps/edge/internal/service/provider_stall_timeout_test.go`

**Test Strategy**

Required. Supply a non-zero value to each direct DTO and prove the marshaled protobuf carries zero while dispatch metadata reports `300000`. Prove provider-pool initial and queued requests carry the final selected provider value.

**Verification**

- `go test -count=1 ./apps/edge/internal/service -run 'ResponseStallTimeout|ProviderStallTimeout'`

### [REVIEW_REVIEW_TEST-1] Prove queue re-resolution and shared-adapter identity

**Problem**

Existing tests cover only one initially selected provider. They cannot fail if queue re-resolution retains a stale timeout or if timeout identity collapses to adapter identity.

**Solution**

- Build compact deterministic helpers around `net.Pipe`, captured protobuf channels, and queue-state barriers; do not use fixed sleeps as synchronization.
- Configure two provider records that share one enabled adapter instance but have distinct ids, served targets, capacities, and timeout values.
- Hold or disable the initial candidate so the request queues, change candidate availability/config through the existing service/store boundary, release admission, and assert the final winner's provider id, target, timeout, wire message, and `RunDispatch` agree.
- Cover normalized and tunnel provider-pool surfaces. Preserve initial-selection assertions so both initial and queued behavior remain explicit.
- Keep helper/test sizes within the readability ratchet; prefer table-driven surface variants and small assertions.

**Modified files**

- [ ] `apps/edge/internal/service/provider_stall_timeout_test.go`

**Test Strategy**

Required and deterministic. Run the new queue/shared-adapter cases repeatedly so stale selection or ordering defects cannot hide behind a single pass.

**Verification**

- `go test -count=10 ./apps/edge/internal/service -run 'ResponseStallTimeout|ProviderStallTimeout'`

### [REVIEW_REVIEW_VERIFY-1] Preserve trustworthy follow-up evidence

**Problem**

The failed review record claimed variants not present in the test tree. Passing broad suites therefore did not establish the required contract.

**Solution**

- Run every Final Verification command against the finished worktree and record command, exit status, and concise actual output in the new review stub.
- Name the exact tests that establish safe-boundary, correlated-error, direct-zero, queued-winner, and shared-adapter behavior.
- Record any unrelated readability baseline or transient smoke failure exactly; never convert a failing command into a pass or claim absent coverage.
- Confirm generator output is still limited to checked-in bindings and no reviewer-only temporary test remains.

**Modified files**

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

**Test Strategy**

Required as evidence integrity. Every checklist claim must map to a named test or captured protobuf assertion.

**Verification**

- All commands in Final Verification.

## Modified Files Summary

| Path | Action | Checklist |
|---|---|---|
| `packages/go/execution/liveness.go` | modify | REVIEW_REVIEW_API-1 |
| `packages/go/execution/liveness_test.go` | modify | REVIEW_REVIEW_API-1 |
| `packages/go/config/provider_stall_timeout_test.go` | modify | REVIEW_REVIEW_API-1 |
| `apps/node/internal/node/runtime_bridge.go` | modify | REVIEW_REVIEW_API-2 |
| `apps/node/internal/node/provider_tunnel_test.go` | modify | REVIEW_REVIEW_API-2 |
| `apps/edge/internal/service/run_submit.go` | modify | REVIEW_REVIEW_API-3 |
| `apps/edge/internal/service/provider_tunnel.go` | modify | REVIEW_REVIEW_API-3 |
| `apps/edge/internal/service/provider_stall_timeout_test.go` | modify | REVIEW_REVIEW_API-3, REVIEW_REVIEW_TEST-1 |
| `agent-task/m-node-provider-execution-liveness-recovery/01_activity_contract/CODE_REVIEW-cloud-G08.md` | update evidence | REVIEW_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 ./apps/node/internal/node -run 'StallTimeout|ProviderTunnelRequest'`
8. `go test -count=1 ./apps/edge/internal/service -run 'ResponseStallTimeout|ProviderStallTimeout'`
9. `go test -count=10 ./apps/edge/internal/service -run 'ResponseStallTimeout|ProviderStallTimeout'`
10. `go test -count=1 ./packages/go/execution ./apps/node/...`
11. `go test -count=1 ./packages/go/streamgate ./apps/edge/internal/openai ./apps/edge/internal/service ./apps/edge/internal/controlplane`
12. `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`
13. `go test -race -count=1 ./packages/go/execution ./apps/node/internal/node`
14. `go vet ./packages/go/execution ./packages/go/config ./apps/edge/internal/service ./apps/edge/internal/configrefresh ./apps/node/internal/node ./apps/node/internal/router`
15. `go test -count=1 ./...`
16. `./scripts/e2e-smoke.sh`
17. `IOP_DEV_RECONNECT_BIND_TIMEOUT=45 ./scripts/dev/edge-node-reconnect-diagnostic.sh`
18. `make readability-audit`
19. `gofmt -l packages/go/execution packages/go/config apps/edge/internal/service apps/node/internal/node`
20. `git diff --check`
21. `git diff --stat`

Record each command, exit status, concise output, exact changed/generated-file state, and any deviation in `CODE_REVIEW-cloud-G08.md`. If readability reports a new/increased current-slice entry or a required named boundary test is absent, stop and repair the implementation or record an exact blocker.

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