chore(epic): liveness-observer 준비 결과를 검증한다
This commit is contained in:
parent
cc8b23c742
commit
0dfd64eef8
16 changed files with 2156 additions and 54 deletions
|
|
@ -1,4 +1,4 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/01_activity_contract plan=1 tag=API milestone-task=activity-contract -->
|
||||
<!-- task=m-node-provider-execution-liveness-recovery/01_activity_contract plan=2 tag=API milestone-task=activity-contract -->
|
||||
|
||||
# Code Review Reference - API
|
||||
|
||||
|
|
@ -14,14 +14,14 @@
|
|||
## Overview
|
||||
|
||||
date=2026-08-03
|
||||
task=m-node-provider-execution-liveness-recovery/01_activity_contract, plan=1, tag=API
|
||||
task=m-node-provider-execution-liveness-recovery/01_activity_contract, plan=2, tag=API
|
||||
|
||||
## Archive Evidence Snapshot
|
||||
|
||||
- Prior plan: `agent-task/m-node-provider-execution-liveness-recovery/01_activity_contract/plan_local_G06_0.log`.
|
||||
- Prior review stub: `agent-task/m-node-provider-execution-liveness-recovery/01_activity_contract/code_review_cloud_G06_0.log`.
|
||||
- Prior plan: `agent-task/m-node-provider-execution-liveness-recovery/01_activity_contract/plan_local_G06_1.log`.
|
||||
- Prior review stub: `agent-task/m-node-provider-execution-liveness-recovery/01_activity_contract/code_review_cloud_G06_1.log`.
|
||||
- Prior verdict: none; implementation and implementation-owned evidence had not started.
|
||||
- Required carryover: regenerate Go/Dart bindings as planned and also run `make client-test` because the checked-in Flutter binding surface changes.
|
||||
- Required carryover: keep `make client-test`; use a duration-safe `int64` config/wire/runtime value; run both auxiliary E2E and the credential-free real Edge/Node reconnect diagnostic.
|
||||
|
||||
## For the Review Agent
|
||||
|
||||
|
|
@ -31,7 +31,7 @@ Compare implementation of each item against source files and verify that output
|
|||
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-G06.md` → `code_review_cloud_G06_1.log` and `PLAN-local-G06.md` → `plan_local_G06_1.log`.
|
||||
2. Archive `CODE_REVIEW-cloud-G06.md` → `code_review_cloud_G06_2.log` and `PLAN-local-G06.md` → `plan_local_G06_2.log`.
|
||||
3. If PASS, write `complete.log` and move active task directory to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/01_activity_contract/`. If WARN/FAIL, fully write the next filesystem state required by the code-review skill.
|
||||
4. If PASS, preserve first-line `milestone-task=activity-contract` metadata in `complete.log` and report it for runtime aggregation. Roadmap state evaluation belongs to `sync-milestone-workstate`.
|
||||
5. Check applicable `Review-Only Checklist` items at the final `.log` location before reporting.
|
||||
|
|
@ -63,8 +63,8 @@ Review completion means the following steps are finished:
|
|||
|
||||
- [ ] Append one verdict of `PASS`, `WARN`, or `FAIL` and verified `review_rework_count`, `evidence_integrity_failure` to `Code Review Result`.
|
||||
- [ ] Verify that verdict, `Dimension Assessment`, and Required/Suggested/Nit classifications match.
|
||||
- [ ] Archive active `CODE_REVIEW-*-G??.md` to `code_review_cloud_G06_1.log`.
|
||||
- [ ] Archive active `PLAN-*-G??.md` to `plan_local_G06_1.log`.
|
||||
- [ ] Archive active `CODE_REVIEW-*-G??.md` to `code_review_cloud_G06_2.log`.
|
||||
- [ ] Archive active `PLAN-*-G??.md` to `plan_local_G06_2.log`.
|
||||
- [ ] Verify that the Agent-Ops managed block in `.gitignore` unignores `agent-task/**/*.md` and `agent-task/**/*.log` and ignores `agent-roadmap/current.md`.
|
||||
- [ ] If PASS, write `complete.log` based on `agent-ops/skills/common/code-review/templates/complete-log-template.md` and leave no active `.md` files.
|
||||
- [ ] If PASS, move active task directory `agent-task/m-node-provider-execution-liveness-recovery/01_activity_contract/` to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/01_activity_contract/` and update this checklist at the final archive path.
|
||||
|
|
@ -84,6 +84,7 @@ _Record key design decisions here._
|
|||
|
||||
- Confirm `start`, progress, terminal, empty, and terminal-with-payload precedence match SDD S01 exactly for both runtime events and tunnel frames.
|
||||
- Confirm omitted/zero/positive/negative config behavior and effective default `300000` across provider-pool and direct/legacy dispatches.
|
||||
- Confirm the value stays `int64` through config, protobuf, Edge DTO, and Node runtime boundaries, and negative or duration-overflowing values are rejected before provider invocation.
|
||||
- Confirm the selected provider candidate carries the value through immediate and queued re-resolution into both wire requests, including providers that share one adapter but use different overrides.
|
||||
- Confirm Node normalizes wire zero to the default, retains a positive immutable value in normalized/tunnel runtime types, and does not couple it to request hard timeout.
|
||||
- Confirm a negative wire value is rejected before router/provider invocation and cannot disable or silently default the observer.
|
||||
|
|
@ -94,6 +95,14 @@ _Record key design decisions here._
|
|||
|
||||
## Verification Results
|
||||
|
||||
### `go version && go env GOMOD`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `flutter --version`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `make proto`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
|
@ -106,11 +115,27 @@ _Implementing agent: record exit status and concise output._
|
|||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `go test -count=1 ./packages/go/execution ./packages/go/streamgate ./packages/go/config`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `go test -count=1 ./packages/go/execution ./apps/node/...`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `go test -count=1 ./packages/go/streamgate ./apps/edge/internal/openai ./apps/edge/internal/service ./apps/edge/internal/controlplane`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `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`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `go test -race -count=1 ./packages/go/execution`
|
||||
### `go test -race -count=1 ./packages/go/execution ./apps/node/internal/node`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `go vet ./packages/go/execution ./packages/go/config ./apps/edge/internal/service ./apps/edge/internal/configrefresh ./apps/node/internal/node ./apps/node/internal/router`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
|
|
@ -118,6 +143,14 @@ _Implementing agent: record exit status and concise output._
|
|||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `./scripts/e2e-smoke.sh`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `IOP_DEV_RECONNECT_BIND_TIMEOUT=45 ./scripts/dev/edge-node-reconnect-diagnostic.sh`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `make readability-audit`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/01_activity_contract plan=1 tag=API milestone-task=activity-contract -->
|
||||
<!-- task=m-node-provider-execution-liveness-recovery/01_activity_contract plan=2 tag=API milestone-task=activity-contract -->
|
||||
|
||||
# PLAN — Node Provider Activity Contract
|
||||
|
||||
|
|
@ -12,14 +12,14 @@ The approved liveness SDD requires one provider-originated activity contract for
|
|||
|
||||
The user supplied starting reference is `95a81ca65fdd24733ec06e4191551dd547c5902e`. Planning was performed against the current branch after its approved SDD/roadmap updates, with a clean worktree and passing targeted Go/race baselines.
|
||||
|
||||
This replan incorporates the explicit pre-implementation self-review. The original pair was structurally valid, but its verification set regenerated checked-in Flutter protobuf bindings without running the client domain's mandatory test target. No implementation had started, so the runtime contract and file scope remain unchanged while the missing client verification is added.
|
||||
The first refinement added the client-domain verification required after regenerating Flutter protobuf bindings. This second fresh-context replan fixes two remaining material gaps before implementation: timeout values now use an `int64` contract with duration-safe upper-bound validation instead of an overflow-prone `int`/`int32` path, and verification now includes a credential-free real Edge/Node process cycle in addition to the auxiliary test-only E2E script. No implementation or official review has started.
|
||||
|
||||
## Archive Evidence Snapshot
|
||||
|
||||
- Prior plan: `agent-task/m-node-provider-execution-liveness-recovery/01_activity_contract/plan_local_G06_0.log`.
|
||||
- Prior review stub: `agent-task/m-node-provider-execution-liveness-recovery/01_activity_contract/code_review_cloud_G06_0.log`.
|
||||
- Prior plan: `agent-task/m-node-provider-execution-liveness-recovery/01_activity_contract/plan_local_G06_1.log`.
|
||||
- Prior review stub: `agent-task/m-node-provider-execution-liveness-recovery/01_activity_contract/code_review_cloud_G06_1.log`.
|
||||
- Prior verdict: none; implementation and implementation-owned evidence had not started.
|
||||
- Required carryover: regenerate Go/Dart bindings as planned and also run `make client-test` because the checked-in Flutter binding surface changes.
|
||||
- Required carryover: keep `make client-test`; use one `int64` millisecond value across config, wire, and runtime, reject values that cannot safely become a Go duration, and run the real Edge/Node reconnect diagnostic.
|
||||
|
||||
## Analysis
|
||||
|
||||
|
|
@ -52,7 +52,8 @@ This replan incorporates the explicit pre-implementation self-review. The origin
|
|||
- `packages/go/execution/failure.go`
|
||||
- `packages/go/config/provider_types.go`
|
||||
- `proto/iop/runtime.proto`
|
||||
- `makefile`
|
||||
- `Makefile`
|
||||
- `scripts/dev/edge-node-reconnect-diagnostic.sh`
|
||||
- `apps/edge/internal/node/mapper.go`
|
||||
- `apps/edge/internal/configrefresh/classify.go`
|
||||
- `apps/node/internal/adapters/config_set.go`
|
||||
|
|
@ -88,10 +89,11 @@ This replan incorporates the explicit pre-implementation self-review. The origin
|
|||
- Environment: local Go 1.26.2, module `/config/workspace/iop-s1/go.mod`.
|
||||
- Required generators are present: `protoc`, `protoc-gen-go`, and `protoc-gen-dart`; `make -n proto` and `make -n proto-dart` resolve successfully.
|
||||
- The client domain owns the checked-in Dart binding output and requires `make client-test` after that output changes.
|
||||
- `IOP_DEV_RECONNECT_BIND_TIMEOUT=45 ./scripts/dev/edge-node-reconnect-diagnostic.sh` is the repository's credential-free real-process check: it starts the actual Edge and Node dev entrypoints with temporary mock-provider config, checks ordered payload/terminal/reconnect behavior, and cleans up its processes.
|
||||
- Baseline passed:
|
||||
- `go test -count=1 ./packages/go/execution ./packages/go/config ./apps/edge/internal/node ./apps/edge/internal/configrefresh ./apps/node/internal/adapters`
|
||||
- `go test -race -count=1 ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/transport`
|
||||
- No external provider, secret, deployment, migration, or field host is required.
|
||||
- No external provider, secret, deployment, migration, or field host is required. `./scripts/e2e-smoke.sh` remains useful auxiliary coverage but is test-only and is not substituted for the real-process diagnostic.
|
||||
|
||||
### Test Coverage Gaps
|
||||
|
||||
|
|
@ -102,6 +104,8 @@ This replan incorporates the explicit pre-implementation self-review. The origin
|
|||
- Refresh tests do not classify this field as `restart_required`.
|
||||
- Generated Go/Dart bindings cannot carry the field yet.
|
||||
- The original verification list regenerated Dart bindings but omitted the client test target required for changes under `apps/client`.
|
||||
- An unrestricted positive `int` narrowed to protobuf `int32`, or converted directly to `time.Duration`, can overflow and produce a non-positive watchdog deadline.
|
||||
- The previous pair lacked the domain-required real Edge/Node process full-cycle verification.
|
||||
|
||||
### Symbol References
|
||||
|
||||
|
|
@ -120,6 +124,7 @@ This replan incorporates the explicit pre-implementation self-review. The origin
|
|||
|
||||
- Classification: large. The slice changes config and protobuf wire contracts and generated bindings, so it cannot be direct-small even though the runtime classifier itself is pure.
|
||||
- Cohesion: the setting and activity classifier must land together because the watchdog needs one effective timeout and one source of truth for reset/terminal decisions.
|
||||
- Refinement retention: this already-refined fixed-index pair remains atomic. Splitting it now would place a new producer after the already-indexed `02+01_stall_watchdog` consumer and violate dependency ordering; config, wire, runtime, and classifier changes are also one contract boundary.
|
||||
- Dependency: none. This is the foundation for `02+01_stall_watchdog`.
|
||||
- Collision check: no active PLAN/CODE_REVIEW claims the target task ids or listed files at plan creation.
|
||||
|
||||
|
|
@ -186,11 +191,11 @@ Required. Use table tests for every event/frame kind, non-empty versus empty pay
|
|||
|
||||
**Solution**
|
||||
|
||||
- Add `ResponseStallTimeoutMS int` to `NodeProviderConf` with `mapstructure/yaml:"response_stall_timeout_ms"`, reject negative values in `Validate`, and expose an effective helper using the shared default.
|
||||
- Add additive, never-reused `int32 response_stall_timeout_ms` fields to both protobuf request messages. Regenerate Go and Dart outputs through repository Make targets; do not edit generated files manually.
|
||||
- Add `ResponseStallTimeoutMS int64` to `NodeProviderConf` with `mapstructure/yaml:"response_stall_timeout_ms"`, reject negative values and positive values that cannot safely convert to `time.Duration` milliseconds in `Validate`, and expose an effective helper using the shared default.
|
||||
- Add additive, never-reused `int64 response_stall_timeout_ms` fields to both protobuf request messages. Retain `int64` through Edge DTOs and Node runtime types; convert to duration only through the validated helper. Regenerate Go and Dart outputs through repository Make targets; do not edit generated files manually.
|
||||
- Extend `candidateNode` with the effective timeout and populate it in `applyProviderDispatchFields`, which is shared by initial resolution and queued re-resolution. Copy it into normalized and tunnel submit DTOs immediately after admission and before request construction. Do not derive it from adapter key or target, and do not expose mutable config pointers.
|
||||
- Extend `SubmitRunRequest`, `SubmitProviderTunnelRequest`, and `RunDispatch` so the selected immutable value can be built, reported, and tested on both surfaces. Direct/non-pool calls that do not name a provider carry zero on the wire and therefore use the documented default; they do not acquire a synthetic provider identity.
|
||||
- Extend host-neutral `RunRequest`, `ExecutionSpec`, and `ProviderTunnelRequest`, plus Node wire bridges/router, with the effective value. Normalize zero to `300000` at the Node boundary. If an unexpected negative wire value reaches Node, reject the request before router/provider invocation rather than disabling the observer or silently defaulting it.
|
||||
- Extend host-neutral `RunRequest`, `ExecutionSpec`, and `ProviderTunnelRequest`, plus Node wire bridges/router, with the effective `int64` value. Normalize zero to `300000` at the Node boundary. Reject negative or duration-overflowing wire values before router/provider invocation rather than disabling the observer or silently defaulting it.
|
||||
- Extend the config-refresh provider snapshot and comparison so `nodes[].providers[...].response_stall_timeout_ms` is `restart_required`, using effective values so omitted and explicit zero compare equal.
|
||||
|
||||
Before: the field is absent at every boundary. After: every dispatched attempt owns the selected provider's immutable positive timeout, including two providers that share an adapter but use different values.
|
||||
|
|
@ -216,7 +221,7 @@ Before: the field is absent at every boundary. After: every dispatched attempt o
|
|||
|
||||
**Test decision**
|
||||
|
||||
Required because this changes config and wire behavior. Cover omitted, explicit zero, positive override, negative rejection, immediate and queued provider-pool dispatch, normalized and tunnel paths, two providers sharing one adapter with different values, direct legacy default, and timeout-only restart-required refresh.
|
||||
Required because this changes config and wire behavior. Cover omitted, explicit zero, positive override, negative and duration-overflow rejection, `int64` protobuf round-trip, immediate and queued provider-pool dispatch, normalized and tunnel paths, two providers sharing one adapter with different values, direct legacy default, and timeout-only restart-required refresh.
|
||||
|
||||
**Verification**
|
||||
|
||||
|
|
@ -336,14 +341,22 @@ No separate doc test. Contract accuracy is checked against the schema/mapping te
|
|||
|
||||
## Final Verification
|
||||
|
||||
1. `make proto`
|
||||
2. `make proto-dart`
|
||||
3. `make client-test`
|
||||
4. `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`
|
||||
5. `go test -race -count=1 ./packages/go/execution`
|
||||
6. `go test -count=1 ./...`
|
||||
7. `make readability-audit`
|
||||
8. `git diff --check`
|
||||
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. `git diff --check`
|
||||
|
||||
Record command, exit status, and concise output in the review stub. If a generator changes any file not listed in Modified Files Summary, stop and reconcile the plan through the owning runtime instead of silently expanding scope.
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,147 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/01_activity_contract plan=1 tag=API milestone-task=activity-contract -->
|
||||
|
||||
# Code Review Reference - 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-03
|
||||
task=m-node-provider-execution-liveness-recovery/01_activity_contract, plan=1, tag=API
|
||||
|
||||
## Archive Evidence Snapshot
|
||||
|
||||
- Prior plan: `agent-task/m-node-provider-execution-liveness-recovery/01_activity_contract/plan_local_G06_0.log`.
|
||||
- Prior review stub: `agent-task/m-node-provider-execution-liveness-recovery/01_activity_contract/code_review_cloud_G06_0.log`.
|
||||
- Prior verdict: none; implementation and implementation-owned evidence had not started.
|
||||
- Required carryover: regenerate Go/Dart bindings as planned and also run `make client-test` because the checked-in Flutter binding surface changes.
|
||||
|
||||
## 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-G06.md` → `code_review_cloud_G06_1.log` and `PLAN-local-G06.md` → `plan_local_G06_1.log`.
|
||||
3. If PASS, write `complete.log` and move active task directory to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/01_activity_contract/`. If WARN/FAIL, fully write the next filesystem state required by the code-review skill.
|
||||
4. If PASS, preserve first-line `milestone-task=activity-contract` metadata in `complete.log` and report it for runtime aggregation. 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 |
|
||||
|------|---------|
|
||||
| API-1 — effective timeout and activity contract | [ ] |
|
||||
| API-2 — config/wire propagation | [ ] |
|
||||
| TEST-1 — deterministic contract/config tests and generated bindings | [ ] |
|
||||
| DOC-1 — matching contracts and example | [ ] |
|
||||
|
||||
## Implementation Checklist
|
||||
|
||||
- [ ] [API-1] Define the effective response-stall timeout and the shared normalized/tunnel provider-activity contract.
|
||||
- [ ] [API-2] Propagate `response_stall_timeout_ms` through provider-pool candidate resolution, normalized/tunnel wire requests, Node runtime types, and refresh classification.
|
||||
- [ ] [TEST-1] Add deterministic contract/config/mapping tests and regenerate checked-in Go/Dart bindings.
|
||||
- [ ] [DOC-1] Update the three matching inner contracts and the provider-first example without claiming watchdog behavior.
|
||||
- [ ] Run every command in Final Verification and record exact results in `CODE_REVIEW-cloud-G06.md`.
|
||||
- [ ] 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.
|
||||
|
||||
- [ ] Append one verdict of `PASS`, `WARN`, or `FAIL` and verified `review_rework_count`, `evidence_integrity_failure` to `Code Review Result`.
|
||||
- [ ] Verify that verdict, `Dimension Assessment`, and Required/Suggested/Nit classifications match.
|
||||
- [ ] Archive active `CODE_REVIEW-*-G??.md` to `code_review_cloud_G06_1.log`.
|
||||
- [ ] Archive active `PLAN-*-G??.md` to `plan_local_G06_1.log`.
|
||||
- [ ] Verify that the Agent-Ops managed block in `.gitignore` unignores `agent-task/**/*.md` and `agent-task/**/*.log` and ignores `agent-roadmap/current.md`.
|
||||
- [ ] If PASS, write `complete.log` based on `agent-ops/skills/common/code-review/templates/complete-log-template.md` and leave no active `.md` files.
|
||||
- [ ] If PASS, move active task directory `agent-task/m-node-provider-execution-liveness-recovery/01_activity_contract/` to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/01_activity_contract/` and update this checklist at the final archive path.
|
||||
- [ ] If PASS and task group is `m-node-provider-execution-liveness-recovery`, preserve and report `milestone-task` metadata for runtime aggregation, without modifying roadmap or directly calling `update-roadmap`.
|
||||
- [ ] 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
|
||||
|
||||
_Record any deviations from the plan and the rationale here._
|
||||
|
||||
## Key Design Decisions
|
||||
|
||||
_Record key design decisions here._
|
||||
|
||||
## Reviewer Checkpoints
|
||||
|
||||
- Confirm `start`, progress, terminal, empty, and terminal-with-payload precedence match SDD S01 exactly for both runtime events and tunnel frames.
|
||||
- Confirm omitted/zero/positive/negative config behavior and effective default `300000` across provider-pool and direct/legacy dispatches.
|
||||
- Confirm the selected provider candidate carries the value through immediate and queued re-resolution into both wire requests, including providers that share one adapter but use different overrides.
|
||||
- Confirm Node normalizes wire zero to the default, retains a positive immutable value in normalized/tunnel runtime types, and does not couple it to request hard timeout.
|
||||
- Confirm a negative wire value is rejected before router/provider invocation and cannot disable or silently default the observer.
|
||||
- Confirm timeout-only config changes are `restart_required`, with omitted and explicit zero equivalent.
|
||||
- Confirm protobuf field numbering is additive, all checked-in Go/Dart outputs came from repository generators, and no generated file was hand-edited.
|
||||
- Confirm `make client-test` passes after regenerating the checked-in Dart protobuf bindings.
|
||||
- Confirm contract/example text does not claim timer, probe, Edge overlay, or retry behavior and preserves hard-timeout/queue/heartbeat/CLI ownership.
|
||||
|
||||
## Verification Results
|
||||
|
||||
### `make proto`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `make proto-dart`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `make client-test`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `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`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `go test -race -count=1 ./packages/go/execution`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `go test -count=1 ./...`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `make readability-audit`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `git diff --check`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
---
|
||||
|
||||
> **[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 |
|
||||
|
|
@ -0,0 +1,350 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/01_activity_contract plan=1 tag=API milestone-task=activity-contract -->
|
||||
|
||||
# PLAN — Node Provider Activity Contract
|
||||
|
||||
## For the Implementing Agent
|
||||
|
||||
> **MANDATORY:** Implement only the checklist below, preserve unrelated user changes, and keep every edit inside the `activity-contract` slice. Do not update roadmap state, create follow-up plans, commit, push, or run an official code review. After implementation, fill every implementation-owned section of `CODE_REVIEW-cloud-G06.md` and leave both active files in place for the review agent.
|
||||
|
||||
## Background
|
||||
|
||||
The approved liveness SDD requires one provider-originated activity contract for normalized `RuntimeEvent` and raw `ProviderTunnelFrame`, plus a provider-level `response_stall_timeout_ms` that reaches Node for every provider-first and legacy route. Current runtime types expose events and frames but no shared activity classifier, while provider-pool candidate resolution and the normalized/tunnel request wire do not carry the selected provider's timeout. A static adapter-level value would be incorrect because multiple provider resources may share one legacy adapter with different overrides. This slice therefore resolves the setting on the selected provider candidate and carries it on each immutable request; direct/legacy requests use the Node-side default. It establishes the contract without starting a timer or changing execution lifecycle.
|
||||
|
||||
The user supplied starting reference is `95a81ca65fdd24733ec06e4191551dd547c5902e`. Planning was performed against the current branch after its approved SDD/roadmap updates, with a clean worktree and passing targeted Go/race baselines.
|
||||
|
||||
This replan incorporates the explicit pre-implementation self-review. The original pair was structurally valid, but its verification set regenerated checked-in Flutter protobuf bindings without running the client domain's mandatory test target. No implementation had started, so the runtime contract and file scope remain unchanged while the missing client verification is added.
|
||||
|
||||
## Archive Evidence Snapshot
|
||||
|
||||
- Prior plan: `agent-task/m-node-provider-execution-liveness-recovery/01_activity_contract/plan_local_G06_0.log`.
|
||||
- Prior review stub: `agent-task/m-node-provider-execution-liveness-recovery/01_activity_contract/code_review_cloud_G06_0.log`.
|
||||
- Prior verdict: none; implementation and implementation-owned evidence had not started.
|
||||
- Required carryover: regenerate Go/Dart bindings as planned and also run `make client-test` because the checked-in Flutter binding surface changes.
|
||||
|
||||
## 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/types.go`
|
||||
- `packages/go/execution/failure.go`
|
||||
- `packages/go/config/provider_types.go`
|
||||
- `proto/iop/runtime.proto`
|
||||
- `makefile`
|
||||
- `apps/edge/internal/node/mapper.go`
|
||||
- `apps/edge/internal/configrefresh/classify.go`
|
||||
- `apps/node/internal/adapters/config_set.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/run_types.go`
|
||||
- `apps/edge/internal/service/run_wire.go`
|
||||
- `apps/edge/internal/service/provider_tunnel.go`
|
||||
- `apps/node/internal/node/runtime_bridge.go`
|
||||
- `apps/node/internal/router/router.go`
|
||||
- `apps/node/internal/node/tunnel_handler.go`
|
||||
- `configs/edge.yaml`
|
||||
- `packages/go/config/provider_catalog_validation_config_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/router/router_test.go`
|
||||
- `apps/node/internal/node/provider_tunnel_test.go`
|
||||
- `apps/edge/internal/configrefresh/provider_classify_test.go`
|
||||
|
||||
### SDD Criteria
|
||||
|
||||
- SDD: `agent-roadmap/sdd/operational-observability-provider-management/node-provider-execution-liveness-recovery/SDD.md` (`승인됨`, implementation lock released).
|
||||
- Decision basis: D01 is resolved; this slice does not add Edge runtime health overlay ownership.
|
||||
- Scenario: S01 / milestone task `activity-contract` (`SDD.md:92`).
|
||||
- Evidence row: S01 requires config validation and fake-clock-ready normalized/tunnel activity, deadline, and transport assertions (`SDD.md:103`). This slice supplies config and pure activity evidence; the dependent watchdog plan supplies clock/deadline/transport lifecycle evidence.
|
||||
- Contract requirements: default/zero `300000`, positive override, negative error, legacy default, restart-required refresh (`SDD.md:67`); normalized start-point/progress/terminal semantics (`SDD.md:70`); tunnel response-start/header/body/usage and terminal semantics (`SDD.md:71`).
|
||||
|
||||
### Verification Context
|
||||
|
||||
- Environment: local Go 1.26.2, module `/config/workspace/iop-s1/go.mod`.
|
||||
- Required generators are present: `protoc`, `protoc-gen-go`, and `protoc-gen-dart`; `make -n proto` and `make -n proto-dart` resolve successfully.
|
||||
- The client domain owns the checked-in Dart binding output and requires `make client-test` after that output changes.
|
||||
- Baseline passed:
|
||||
- `go test -count=1 ./packages/go/execution ./packages/go/config ./apps/edge/internal/node ./apps/edge/internal/configrefresh ./apps/node/internal/adapters`
|
||||
- `go test -race -count=1 ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/transport`
|
||||
- No external provider, secret, deployment, migration, or field host is required.
|
||||
|
||||
### Test Coverage Gaps
|
||||
|
||||
- There is no table test that defines provider activity consistently across normalized and tunnel types.
|
||||
- Provider config tests do not cover stall-timeout default/override/negative semantics.
|
||||
- Provider-pool candidate and dispatch tests do not prove that the winning provider's effective value survives queue re-resolution and reaches normalized/tunnel wire requests when providers share an adapter.
|
||||
- Direct/legacy request builders and Node runtime mappers do not prove that wire zero becomes the default without mutating request hard timeout.
|
||||
- Refresh tests do not classify this field as `restart_required`.
|
||||
- Generated Go/Dart bindings cannot carry the field yet.
|
||||
- The original verification list regenerated Dart bindings but omitted the client test target required for changes under `apps/client`.
|
||||
|
||||
### Symbol References
|
||||
|
||||
- `packages/go/execution/types.go:31-54` — normalized event kinds and payload.
|
||||
- `packages/go/execution/types.go:228-253` — tunnel frame kinds and payload.
|
||||
- `packages/go/config/provider_types.go:91-98,100-128` — provider-first execution fields and validation.
|
||||
- `proto/iop/runtime.proto:53-83,99-132` — tunnel and normalized request wire schemas.
|
||||
- `apps/edge/internal/service/model_queue_types.go:71-106` — selected provider candidate snapshot.
|
||||
- `apps/edge/internal/service/provider_resolution.go:278-298,381-480` — initial and queued provider dispatch facts.
|
||||
- `apps/edge/internal/service/run_wire.go:37-68` and `provider_tunnel.go:502-537` — normalized/tunnel request construction.
|
||||
- `apps/node/internal/node/runtime_bridge.go:8-21` and `apps/node/internal/router/router.go:35-55` — wire-to-runtime normalized propagation.
|
||||
- `apps/node/internal/node/tunnel_handler.go:25-39` — wire-to-runtime tunnel propagation.
|
||||
- `apps/edge/internal/configrefresh/classify.go:89-133,274-281` — provider snapshot and restart-required comparisons.
|
||||
|
||||
### Split Judgment
|
||||
|
||||
- Classification: large. The slice changes config and protobuf wire contracts and generated bindings, so it cannot be direct-small even though the runtime classifier itself is pure.
|
||||
- Cohesion: the setting and activity classifier must land together because the watchdog needs one effective timeout and one source of truth for reset/terminal decisions.
|
||||
- Dependency: none. This is the foundation for `02+01_stall_watchdog`.
|
||||
- Collision check: no active PLAN/CODE_REVIEW claims the target task ids or listed files at plan creation.
|
||||
|
||||
### Scope Rationale
|
||||
|
||||
- In scope: activity semantics, config schema/default/validation, selected-candidate propagation on both request variants, Node runtime retention, refresh classification, generated bindings, tests, matching specs, and inner contracts.
|
||||
- Out of scope: timers, cancellation, terminal synthesis, attempt fencing, health probes, observation sequence, Edge health overlay, recovery/retry, and operational metrics.
|
||||
- New files are limited to the shared classifier and its focused test; existing config/mapping test files are extended instead of creating parallel suites.
|
||||
|
||||
### Final Routing
|
||||
|
||||
- `evaluation_mode=isolated-reassessment`; `finalizer=finalize-task-policy.sh`, `finalizer_mode=pair`.
|
||||
- Build closure: scope/context/verification/evidence/ownership/decision all closed and trusted; capability gap none.
|
||||
- Build score: `scope=2`, `state=0`, `blast=2`, `evidence=1`, `verification=1` -> G06; `base_route_basis=local-fit`, `route_basis=local-fit`, lane `local`, file `PLAN-local-G06.md`.
|
||||
- Build signals: `large_indivisible_context=false`, positive loop risk `boundary_contract` (`count=1`), `review_rework_count=0`, `evidence_integrity_failure=false`; risk/recovery boundary not matched.
|
||||
- Review closure: scope/context/verification/evidence/ownership/decision all closed and trusted; capability gap none.
|
||||
- Review score: `scope=2`, `state=0`, `blast=2`, `evidence=1`, `verification=1` -> G06; `route_basis=official-review`, lane `cloud`, adapter/model `codex/gpt-5.6-sol`, reasoning `xhigh`, file `CODE_REVIEW-cloud-G06.md`.
|
||||
|
||||
## Implementation Checklist
|
||||
|
||||
- [ ] [API-1] Define the effective response-stall timeout and the shared normalized/tunnel provider-activity contract.
|
||||
- [ ] [API-2] Propagate `response_stall_timeout_ms` through provider-pool candidate resolution, normalized/tunnel wire requests, Node runtime types, and refresh classification.
|
||||
- [ ] [TEST-1] Add deterministic contract/config/mapping tests and regenerate checked-in Go/Dart bindings.
|
||||
- [ ] [DOC-1] Update the three matching inner contracts and the provider-first example without claiming watchdog behavior.
|
||||
- [ ] Run every command in Final Verification and record exact results in `CODE_REVIEW-cloud-G06.md`.
|
||||
- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.
|
||||
|
||||
### [API-1] Define the effective timeout and provider-activity contract
|
||||
|
||||
**Problem**
|
||||
|
||||
`RuntimeEvent` and `ProviderTunnelFrame` expose provider output but have no single progress/terminal classifier (`packages/go/execution/types.go:31-54,228-253`). A watchdog implemented directly in handlers would duplicate subtly different rules.
|
||||
|
||||
**Solution**
|
||||
|
||||
Add `packages/go/execution/liveness.go` with:
|
||||
|
||||
- `DefaultResponseStallTimeoutMS = 300000` and an effective-value helper that maps `0` to the default, passes positive values, and does not silently accept negatives.
|
||||
- A small `ProviderActivityDisposition` enum (`none`, `start`, `progress`, `terminal`) and pure classifiers for `RuntimeEvent` and `ProviderTunnelFrame`. `start` lets the observer establish its initial baseline without conflating that transition with later progress resets.
|
||||
- Normalized rules: `start` is the start disposition; non-empty `delta`/`reasoning_delta` and non-terminal usage are progress; complete/error/cancelled are terminal before any usage check; empty/unknown events are none.
|
||||
- Tunnel rules: response-start (including headers), non-empty body, and usage are progress; end/error are terminal before payload checks; empty/unknown frames are none.
|
||||
|
||||
Before: handlers would need to switch independently on event/frame kinds. After: all later timers consume the same pure disposition and cannot treat heartbeat/socket/process activity as provider progress because those signals never enter these classifiers.
|
||||
|
||||
**Modified files**
|
||||
|
||||
- [ ] `packages/go/execution/liveness.go`
|
||||
- [ ] `packages/go/execution/liveness_test.go`
|
||||
|
||||
**Test decision**
|
||||
|
||||
Required. Use table tests for every event/frame kind, non-empty versus empty payloads, usage, terminal-with-payload precedence, and unknown values. The tests must use no wall-clock sleep.
|
||||
|
||||
**Verification**
|
||||
|
||||
- `go test -count=1 ./packages/go/execution`
|
||||
- `go test -race -count=1 ./packages/go/execution`
|
||||
|
||||
### [API-2] Carry the selected provider timeout on each request
|
||||
|
||||
**Problem**
|
||||
|
||||
`NodeProviderConf` ends at `request_timeout_ms` (`packages/go/config/provider_types.go:91-98`). Provider-pool candidate resolution selects a provider id independently from its adapter key, but `RunRequest` and `ProviderTunnelRequest` carry only adapter/target/timeouts unrelated to liveness. The watchdog therefore cannot distinguish different provider overrides when multiple resources share one legacy adapter.
|
||||
|
||||
**Solution**
|
||||
|
||||
- Add `ResponseStallTimeoutMS int` to `NodeProviderConf` with `mapstructure/yaml:"response_stall_timeout_ms"`, reject negative values in `Validate`, and expose an effective helper using the shared default.
|
||||
- Add additive, never-reused `int32 response_stall_timeout_ms` fields to both protobuf request messages. Regenerate Go and Dart outputs through repository Make targets; do not edit generated files manually.
|
||||
- Extend `candidateNode` with the effective timeout and populate it in `applyProviderDispatchFields`, which is shared by initial resolution and queued re-resolution. Copy it into normalized and tunnel submit DTOs immediately after admission and before request construction. Do not derive it from adapter key or target, and do not expose mutable config pointers.
|
||||
- Extend `SubmitRunRequest`, `SubmitProviderTunnelRequest`, and `RunDispatch` so the selected immutable value can be built, reported, and tested on both surfaces. Direct/non-pool calls that do not name a provider carry zero on the wire and therefore use the documented default; they do not acquire a synthetic provider identity.
|
||||
- Extend host-neutral `RunRequest`, `ExecutionSpec`, and `ProviderTunnelRequest`, plus Node wire bridges/router, with the effective value. Normalize zero to `300000` at the Node boundary. If an unexpected negative wire value reaches Node, reject the request before router/provider invocation rather than disabling the observer or silently defaulting it.
|
||||
- Extend the config-refresh provider snapshot and comparison so `nodes[].providers[...].response_stall_timeout_ms` is `restart_required`, using effective values so omitted and explicit zero compare equal.
|
||||
|
||||
Before: the field is absent at every boundary. After: every dispatched attempt owns the selected provider's immutable positive timeout, including two providers that share an adapter but use different values.
|
||||
|
||||
**Modified files**
|
||||
|
||||
- [ ] `packages/go/config/provider_types.go`
|
||||
- [ ] `packages/go/execution/types.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`
|
||||
- [ ] `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/run_types.go`
|
||||
- [ ] `apps/edge/internal/service/run_wire.go`
|
||||
- [ ] `apps/edge/internal/service/provider_tunnel.go`
|
||||
- [ ] `apps/node/internal/node/runtime_bridge.go`
|
||||
- [ ] `apps/node/internal/router/router.go`
|
||||
- [ ] `apps/node/internal/node/tunnel_handler.go`
|
||||
- [ ] `apps/edge/internal/configrefresh/classify.go`
|
||||
|
||||
**Test decision**
|
||||
|
||||
Required because this changes config and wire behavior. Cover omitted, explicit zero, positive override, negative rejection, immediate and queued provider-pool dispatch, normalized and tunnel paths, two providers sharing one adapter with different values, direct legacy default, and timeout-only restart-required refresh.
|
||||
|
||||
**Verification**
|
||||
|
||||
- `make proto`
|
||||
- `make proto-dart`
|
||||
- `make client-test`
|
||||
- `go test -count=1 ./packages/go/config ./apps/edge/internal/service ./apps/edge/internal/configrefresh ./apps/node/internal/node ./apps/node/internal/router`
|
||||
|
||||
### [TEST-1] Lock generated and mapping behavior
|
||||
|
||||
**Problem**
|
||||
|
||||
Existing tests cover adjacent request/queue fields but not this generic liveness value, and a generated binding drift could compile only one client surface.
|
||||
|
||||
**Solution**
|
||||
|
||||
Extend the closest existing tests with compact tables:
|
||||
|
||||
- config validation/effective-value cases;
|
||||
- selected candidate, queue re-resolution, normalized/tunnel request round-trip, shared-adapter/different-timeout assertions;
|
||||
- Node wire bridge/router/tunnel domain propagation and direct legacy default assertions;
|
||||
- refresh classification/effective-zero assertions;
|
||||
- Go protobuf round-trip assertion for the new field.
|
||||
|
||||
Run both generators and the client test target, then use `git diff --check`; never hand-edit generated code. Do not add fake timers here—the dependent watchdog plan owns time behavior.
|
||||
|
||||
**Modified files**
|
||||
|
||||
- [ ] `packages/go/config/provider_catalog_validation_config_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/router/router_test.go`
|
||||
- [ ] `apps/node/internal/node/provider_tunnel_test.go`
|
||||
- [ ] `apps/edge/internal/configrefresh/provider_classify_test.go`
|
||||
|
||||
**Test decision**
|
||||
|
||||
Required; all fixtures are deterministic and local.
|
||||
|
||||
**Verification**
|
||||
|
||||
- `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`
|
||||
- `go test -race -count=1 ./packages/go/execution`
|
||||
|
||||
### [DOC-1] Synchronize contracts and example
|
||||
|
||||
**Problem**
|
||||
|
||||
The matching contracts currently describe execution events, Edge-to-Node adapter payloads, and restart-required provider fields without the new timeout/activity rules.
|
||||
|
||||
**Solution**
|
||||
|
||||
Update the contracts in the same change as implementation:
|
||||
|
||||
- execution runtime: effective default and exact activity/terminal classifier semantics;
|
||||
- Edge-Node wire: per-attempt `RunRequest`/`ProviderTunnelRequest.response_stall_timeout_ms` propagation and mixed-version/default behavior;
|
||||
- Edge config/refresh: schema, zero/default equivalence, negative rejection, and restart-required classification.
|
||||
|
||||
Add one provider-first example field to `configs/edge.yaml`. State explicitly that request hard timeout, queue timeout, heartbeat/disconnect, and CLI `response_idle_timeout_ms` retain their existing ownership. Do not document timers, health classification, retry, or Edge overlay as implemented by this slice.
|
||||
|
||||
**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 decision**
|
||||
|
||||
No separate doc test. Contract accuracy is checked against the schema/mapping tests and diff.
|
||||
|
||||
**Verification**
|
||||
|
||||
- `git diff --check`
|
||||
|
||||
## Modified Files Summary
|
||||
|
||||
| Path | Action | Checklist |
|
||||
|------|--------|-----------|
|
||||
| `packages/go/execution/liveness.go` | add | API-1 |
|
||||
| `packages/go/execution/liveness_test.go` | add | API-1 |
|
||||
| `packages/go/config/provider_types.go` | modify | API-2 |
|
||||
| `packages/go/execution/types.go` | modify | API-2 |
|
||||
| `proto/iop/runtime.proto` | modify | API-2 |
|
||||
| `proto/gen/iop/runtime.pb.go` | regenerate | API-2 |
|
||||
| `apps/client/lib/gen/proto/iop/runtime.pb.dart` | regenerate | API-2 |
|
||||
| `apps/client/lib/gen/proto/iop/runtime.pbjson.dart` | regenerate | API-2 |
|
||||
| `apps/edge/internal/service/model_queue_types.go` | modify | API-2 |
|
||||
| `apps/edge/internal/service/provider_resolution.go` | modify | API-2 |
|
||||
| `apps/edge/internal/service/provider_pool.go` | modify | API-2 |
|
||||
| `apps/edge/internal/service/run_types.go` | modify | API-2 |
|
||||
| `apps/edge/internal/service/run_wire.go` | modify | API-2 |
|
||||
| `apps/edge/internal/service/provider_tunnel.go` | modify | API-2 |
|
||||
| `apps/node/internal/node/runtime_bridge.go` | modify | API-2 |
|
||||
| `apps/node/internal/router/router.go` | modify | API-2 |
|
||||
| `apps/node/internal/node/tunnel_handler.go` | modify | API-2 |
|
||||
| `apps/edge/internal/configrefresh/classify.go` | modify | API-2 |
|
||||
| `packages/go/config/provider_catalog_validation_config_test.go` | modify | TEST-1 |
|
||||
| `apps/edge/internal/service/provider_scheduling_advanced_test.go` | modify | TEST-1 |
|
||||
| `apps/edge/internal/service/run_command_test.go` | modify | TEST-1 |
|
||||
| `apps/edge/internal/service/run_dispatch_internal_test.go` | modify | TEST-1 |
|
||||
| `apps/node/internal/node/runtime_bridge_test.go` | modify | TEST-1 |
|
||||
| `apps/node/internal/router/router_test.go` | modify | TEST-1 |
|
||||
| `apps/node/internal/node/provider_tunnel_test.go` | modify | TEST-1 |
|
||||
| `apps/edge/internal/configrefresh/provider_classify_test.go` | modify | TEST-1 |
|
||||
| `agent-contract/inner/execution-runtime.md` | modify | DOC-1 |
|
||||
| `agent-contract/inner/edge-node-runtime-wire.md` | modify | DOC-1 |
|
||||
| `agent-contract/inner/edge-config-runtime-refresh.md` | modify | DOC-1 |
|
||||
| `agent-spec/runtime/edge-node-execution.md` | modify | DOC-1 |
|
||||
| `agent-spec/runtime/provider-pool-config-refresh.md` | modify | DOC-1 |
|
||||
| `configs/edge.yaml` | modify | DOC-1 |
|
||||
| `agent-task/m-node-provider-execution-liveness-recovery/01_activity_contract/CODE_REVIEW-cloud-G06.md` | update evidence | all |
|
||||
|
||||
## Final Verification
|
||||
|
||||
1. `make proto`
|
||||
2. `make proto-dart`
|
||||
3. `make client-test`
|
||||
4. `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`
|
||||
5. `go test -race -count=1 ./packages/go/execution`
|
||||
6. `go test -count=1 ./...`
|
||||
7. `make readability-audit`
|
||||
8. `git diff --check`
|
||||
|
||||
Record command, exit status, and concise output in the review stub. If a generator changes any file not listed in Modified Files Summary, stop and reconcile the plan through the owning runtime instead of silently expanding scope.
|
||||
|
||||
**After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`.**
|
||||
|
|
@ -1,4 +1,4 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/02+01_stall_watchdog plan=1 tag=API milestone-task=activity-contract,stall-watchdog -->
|
||||
<!-- task=m-node-provider-execution-liveness-recovery/02+01_stall_watchdog plan=2 tag=API milestone-task=activity-contract,stall-watchdog -->
|
||||
|
||||
# Code Review Reference - API
|
||||
|
||||
|
|
@ -14,14 +14,14 @@
|
|||
## Overview
|
||||
|
||||
date=2026-08-03
|
||||
task=m-node-provider-execution-liveness-recovery/02+01_stall_watchdog, plan=1, tag=API
|
||||
task=m-node-provider-execution-liveness-recovery/02+01_stall_watchdog, plan=2, tag=API
|
||||
|
||||
## Archive Evidence Snapshot
|
||||
|
||||
- Prior plan: `agent-task/m-node-provider-execution-liveness-recovery/02+01_stall_watchdog/plan_cloud_G08_0.log`.
|
||||
- Prior review stub: `agent-task/m-node-provider-execution-liveness-recovery/02+01_stall_watchdog/code_review_cloud_G08_0.log`.
|
||||
- Prior plan: `agent-task/m-node-provider-execution-liveness-recovery/02+01_stall_watchdog/plan_cloud_G08_1.log`.
|
||||
- Prior review stub: `agent-task/m-node-provider-execution-liveness-recovery/02+01_stall_watchdog/code_review_cloud_G08_1.log`.
|
||||
- Prior verdict: none; implementation and implementation-owned evidence had not started.
|
||||
- Required carryover: use the request's Node-owned `run_id` as terminal `attempt_id` and prove caller metadata cannot spoof it.
|
||||
- Required carryover: keep Node-owned identity; use exact injected-clock `5s` close grace; clone one safe metadata map onto normalized failure/event and tunnel terminal; verify normalized protobuf preservation and the real-process diagnostic.
|
||||
|
||||
## For the Review Agent
|
||||
|
||||
|
|
@ -31,7 +31,7 @@ Compare implementation of each item against source files and verify that output
|
|||
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-G08.md` → `code_review_cloud_G08_1.log` and `PLAN-cloud-G08.md` → `plan_cloud_G08_1.log`.
|
||||
2. Archive `CODE_REVIEW-cloud-G08.md` → `code_review_cloud_G08_2.log` and `PLAN-cloud-G08.md` → `plan_cloud_G08_2.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, preserve first-line `milestone-task=activity-contract,stall-watchdog` metadata in `complete.log` and report it for runtime aggregation. Roadmap state evaluation belongs to `sync-milestone-workstate`.
|
||||
5. Check applicable `Review-Only Checklist` items at the final `.log` location before reporting.
|
||||
|
|
@ -65,8 +65,8 @@ Review completion means the following steps are finished:
|
|||
|
||||
- [ ] Append one verdict of `PASS`, `WARN`, or `FAIL` and verified `review_rework_count`, `evidence_integrity_failure` to `Code Review Result`.
|
||||
- [ ] Verify that verdict, `Dimension Assessment`, and Required/Suggested/Nit classifications match.
|
||||
- [ ] Archive active `CODE_REVIEW-*-G??.md` to `code_review_cloud_G08_1.log`.
|
||||
- [ ] Archive active `PLAN-*-G??.md` to `plan_cloud_G08_1.log`.
|
||||
- [ ] Archive active `CODE_REVIEW-*-G??.md` to `code_review_cloud_G08_2.log`.
|
||||
- [ ] Archive active `PLAN-*-G??.md` to `plan_cloud_G08_2.log`.
|
||||
- [ ] Verify that the Agent-Ops managed block in `.gitignore` unignores `agent-task/**/*.md` and `agent-task/**/*.log` and ignores `agent-roadmap/current.md`.
|
||||
- [ ] If PASS, write `complete.log` based on `agent-ops/skills/common/code-review/templates/complete-log-template.md` and leave no active `.md` files.
|
||||
- [ ] 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.
|
||||
|
|
@ -88,14 +88,24 @@ _Record key design decisions here._
|
|||
- Trace one terminal authority across provider terminal, watchdog expiry, cancel/deadline/disconnect, and late output for both normalized and tunnel paths.
|
||||
- Confirm a timer signal rechecks request/session termination before claiming stall and preserves existing deadline/transport classification.
|
||||
- Confirm `attempt_fence=confirmed` requires provider return within bounded close grace; unconfirmed attempts keep admission, run-manager, drain, and credential ownership until real provider exit.
|
||||
- Confirm the bounded close grace is exactly `defaultAttemptCloseGrace=5s` and all tests advance the injected clock rather than sleeping.
|
||||
- Confirm `Failure.retryable` is true only for confirmed local fence and no Node retry or `recovery_eligible` appears.
|
||||
- Confirm normalized and tunnel terminal `run_id`/`attempt_id` come from the concrete Node-visible run identity and caller metadata cannot spoof either value.
|
||||
- Confirm one allowlisted map is cloned into normalized `Failure.Metadata`, normalized `RuntimeEvent.Metadata`, and tunnel ERROR metadata, and that normalized protobuf conversion preserves the same keys.
|
||||
- Inspect fake-clock/channel tests for threshold/event/cancel races, release exactly once, and absence of wall-clock sleeps; independently rerun race tests.
|
||||
- Confirm contracts document only Node watchdog/fence behavior and retain unknown provider health pending the next slice.
|
||||
|
||||
## Verification Results
|
||||
|
||||
### `go test -count=1 ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/transport`
|
||||
### `go version && go env GOMOD`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `go test -count=1 ./packages/go/execution ./packages/go/streamgate ./packages/go/config`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `go test -count=1 ./packages/go/execution ./apps/node/...`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
|
|
@ -107,6 +117,10 @@ _Implementing agent: record exit status and concise output._
|
|||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `go vet ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/transport`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `go test -count=1 ./...`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
|
@ -115,6 +129,10 @@ _Implementing agent: record exit status and concise output._
|
|||
|
||||
_Implementing agent: record exit status and concise output, or the exact environment-only blocker._
|
||||
|
||||
### `IOP_DEV_RECONNECT_BIND_TIMEOUT=45 ./scripts/dev/edge-node-reconnect-diagnostic.sh`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `make readability-audit`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/02+01_stall_watchdog plan=1 tag=API milestone-task=activity-contract,stall-watchdog -->
|
||||
<!-- task=m-node-provider-execution-liveness-recovery/02+01_stall_watchdog plan=2 tag=API milestone-task=activity-contract,stall-watchdog -->
|
||||
|
||||
# PLAN — Node Response Stall Watchdog
|
||||
|
||||
|
|
@ -12,14 +12,14 @@ Node currently invokes normalized providers and raw tunnel adapters synchronousl
|
|||
|
||||
This slice consumes the activity/config contract from `01_activity_contract`, installs a single Node-owned watchdog for both execution surfaces, and emits one stable `response_stalled` terminal with a confirmed or unconfirmed local fence. It deliberately emits `provider_health=unknown`; the dependent health-classification slice replaces that bounded fallback with target-aware probe evidence.
|
||||
|
||||
This replan incorporates the explicit pre-implementation self-review. The original pair incorrectly allowed caller-defined request metadata to override the Node-produced `attempt_id`. The Edge-Node wire contract explicitly says `RunRequest.metadata` is caller-defined and not a control surface, while `run_id` is the Node-visible identity of this concrete execution attempt. No implementation had started; the lifecycle design is retained and the identity rule plus spoof-resistance evidence are corrected.
|
||||
The first refinement corrected caller-metadata spoofing of Node-owned attempt identity. This second fresh-context replan closes the remaining material ambiguities before implementation: close grace is exactly `5s` on the injected clock, normalized terminal metadata must be attached to both `Failure.Metadata` and `RuntimeEvent.Metadata` so the existing protobuf mapper cannot drop it, tunnel and normalized terminals must use clones of the same safe map, and verification includes a credential-free real Edge/Node process cycle. No implementation or official review has started.
|
||||
|
||||
## Archive Evidence Snapshot
|
||||
|
||||
- Prior plan: `agent-task/m-node-provider-execution-liveness-recovery/02+01_stall_watchdog/plan_cloud_G08_0.log`.
|
||||
- Prior review stub: `agent-task/m-node-provider-execution-liveness-recovery/02+01_stall_watchdog/code_review_cloud_G08_0.log`.
|
||||
- Prior plan: `agent-task/m-node-provider-execution-liveness-recovery/02+01_stall_watchdog/plan_cloud_G08_1.log`.
|
||||
- Prior review stub: `agent-task/m-node-provider-execution-liveness-recovery/02+01_stall_watchdog/code_review_cloud_G08_1.log`.
|
||||
- Prior verdict: none; implementation and implementation-owned evidence had not started.
|
||||
- Required carryover: use the request's Node-owned `run_id` as terminal `attempt_id` and prove caller metadata cannot spoof it.
|
||||
- Required carryover: use Node-owned `run_id` as `attempt_id`; fix `defaultAttemptCloseGrace=5s`; clone one allowlisted metadata map onto normalized `Failure.Metadata`, normalized `RuntimeEvent.Metadata`, and the tunnel ERROR frame; prove wire preservation and spoof resistance.
|
||||
|
||||
## Dependencies
|
||||
|
||||
|
|
@ -52,6 +52,7 @@ At plan creation the predecessor is active and not complete. The implementing ru
|
|||
- `apps/node/internal/node/run_handler.go`
|
||||
- `apps/node/internal/node/run_manager.go`
|
||||
- `apps/node/internal/node/runtime_sink.go`
|
||||
- `apps/node/internal/node/runtime_bridge.go`
|
||||
- `apps/node/internal/node/tunnel_handler.go`
|
||||
- `apps/node/internal/node/run_cancel_test.go`
|
||||
- `apps/node/internal/node/provider_tunnel_test.go`
|
||||
|
|
@ -63,6 +64,7 @@ At plan creation the predecessor is active and not complete. The implementing ru
|
|||
- `packages/go/execution/emitter_test.go`
|
||||
- `agent-contract/inner/execution-runtime.md`
|
||||
- `agent-contract/inner/edge-node-runtime-wire.md`
|
||||
- `scripts/dev/edge-node-reconnect-diagnostic.sh`
|
||||
|
||||
### SDD Criteria
|
||||
|
||||
|
|
@ -80,7 +82,7 @@ At plan creation the predecessor is active and not complete. The implementing ru
|
|||
- `go test -race -count=1 ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/transport`
|
||||
- Local deterministic fixtures are sufficient; no external provider or credentials are required.
|
||||
- Timer tests must use an injected fake clock and synchronization channels, not wall-clock sleeps.
|
||||
- Runtime execution changes require targeted unit/race tests plus the repository full Go suite and local E2E smoke.
|
||||
- Runtime execution changes require targeted unit/race tests, vet/full Go suite, auxiliary E2E, and `IOP_DEV_RECONNECT_BIND_TIMEOUT=45 ./scripts/dev/edge-node-reconnect-diagnostic.sh`, which starts the real Edge and Node dev entrypoints against temporary mock config without credentials.
|
||||
|
||||
### Test Coverage Gaps
|
||||
|
||||
|
|
@ -89,6 +91,7 @@ At plan creation the predecessor is active and not complete. The implementing ru
|
|||
- Run/tunnel handlers call providers on the cleanup-owning goroutine, so an adapter that ignores cancellation can retain or prematurely release admission/run ownership incorrectly.
|
||||
- Session listeners pass `context.Background()` (`apps/node/internal/transport/session.go:50-52,75-87`), so disconnect does not cancel current request handlers.
|
||||
- No tests cover timer/activity boundary ordering, hard deadline/disconnect precedence, close grace, or confirmed/unconfirmed resource ownership.
|
||||
- `runEventToProto` serializes `RuntimeEvent.Metadata` and does not automatically forward `Failure.Metadata`; a plan that populates only the failure would silently lose safe liveness evidence on the normalized wire.
|
||||
|
||||
### Symbol References
|
||||
|
||||
|
|
@ -107,7 +110,8 @@ At plan creation the predecessor is active and not complete. The implementing ru
|
|||
- Classification: large. Correctness depends on temporal state, concurrent exactly-once claims, cancellation and transport precedence, and resource ownership across two execution variants.
|
||||
- Cohesion: normalized and tunnel paths must share one watchdog/fence primitive so they cannot diverge on activity or terminal semantics.
|
||||
- Predecessor: `01_activity_contract` is required and encoded in the directory name and Dependencies section.
|
||||
- Successor: `03+02_health_classification` will enrich the same terminal evidence but may not change timer/fence ownership.
|
||||
- Successors: refined `03+02_health_probe_contract` defines the fail-closed probe result and `04+03_health_evidence` enriches the same terminal evidence without changing timer/fence ownership.
|
||||
- Refinement retention: this already-refined pair remains atomic because observer state, normalized/tunnel terminal authority, cancellation/fence cleanup, and wire evidence form one S02 exactly-once invariant; no child would have an independently reviewable PASS boundary.
|
||||
- Collision check: no active plan claimed these paths when prepared; dependency sequencing prevents overlap with predecessor contract files.
|
||||
|
||||
### Scope Rationale
|
||||
|
|
@ -148,9 +152,9 @@ Add `apps/node/internal/node/liveness_watchdog.go` with package-private abstract
|
|||
- an injectable clock/timer interface with a real default on `Node` and a deterministic manual test implementation;
|
||||
- one attempt observer state machine that accepts the predecessor's `ProviderActivityDisposition`, resets on progress, stops on terminal, and exposes a single atomic/mutex-protected terminal/fence claim;
|
||||
- an emission authority wrapper for each sink that drops every provider event/frame after the terminal claim and reports activity to the observer before forwarding valid non-terminal output;
|
||||
- a bounded cancel/close grace timer owned by the same injected clock. On threshold, cancel provider execution, revoke provider emission authority immediately, and classify `confirmed` only when the provider call has returned within grace; otherwise classify `unconfirmed`;
|
||||
- `const defaultAttemptCloseGrace = 5 * time.Second` and a bounded cancel/close grace timer owned by the same injected clock. On threshold, cancel provider execution, revoke provider emission authority immediately, and classify `confirmed` only when the provider call has returned within that exact grace; otherwise classify `unconfirmed`;
|
||||
- after a timer signal, re-check request context and session lifetime before claiming stall so a simultaneous/earlier hard deadline, caller cancel, or disconnect is never reclassified;
|
||||
- a stable `FailureCodeResponseStalled` in `packages/go/execution/failure.go`, known-code encoding/decoding support, and a single metadata builder for `failure_code`, `provider_health=unknown`, `liveness_classification=health_unknown`, `idle_duration_ms`, `run_id`, `attempt_id`, `attempt_fence`, `adapter`, and `target`. Set both `run_id` and `attempt_id` from the concrete request's Node-owned run identity (`ExecutionSpec.RunID`/wire `run_id`). Never derive either field from caller-defined request metadata, even when metadata contains `run_id` or `attempt_id`; never include raw output, prompt, reasoning, credentials, or `recovery_eligible`.
|
||||
- a stable `FailureCodeResponseStalled` in `packages/go/execution/failure.go`, known-code encoding/decoding support, and one allowlisted metadata builder for `failure_code`, `provider_health=unknown`, `liveness_classification=health_unknown`, `idle_duration_ms`, `run_id`, `attempt_id`, `attempt_fence`, `adapter`, and `target`. Set both identities from the concrete request's Node-owned run identity (`ExecutionSpec.RunID`/wire `run_id`). Clone this map into both normalized `Failure.Metadata` and `RuntimeEvent.Metadata`, and into the tunnel ERROR metadata, so normalized protobuf conversion and raw tunnel transport preserve identical keys without shared mutable aliases. Never derive values from caller-defined metadata or include raw output, prompt, reasoning, credentials, or `recovery_eligible`.
|
||||
|
||||
The normalized terminal is an error `RuntimeEvent` carrying `Failure{Code: response_stalled, Retryable: fence == confirmed}`. The tunnel terminal is an ERROR frame with the same safe metadata and stable error text. The health successor will replace only the unknown classification fields.
|
||||
|
||||
|
|
@ -250,7 +254,7 @@ Add a package-internal fake clock plus channel-controlled providers and table te
|
|||
- exact threshold minus one tick versus threshold, event-at-threshold, provider terminal-at-threshold, caller cancel, hard deadline, and session disconnect;
|
||||
- provider returns within close grace (`confirmed`) and ignores cancel beyond grace (`unconfirmed`), including eventual return;
|
||||
- one normalized terminal and one tunnel ERROR only, late delta/frame dropped, retryable only when confirmed;
|
||||
- caller metadata containing spoofed `run_id` or `attempt_id` cannot override the Node-produced terminal identity on either surface;
|
||||
- caller metadata containing spoofed `run_id` or `attempt_id` cannot override the Node-produced terminal identity on either surface; normalized domain failure, normalized protobuf event, and tunnel frame all preserve the same allowlisted values;
|
||||
- admission count, run-manager presence, drain wait, credential lifetime, and release exactly once for both fence outcomes;
|
||||
- `go test -race` with repeated boundary cases; assertions use channels/manual clock, never scheduler sleeps.
|
||||
|
||||
|
|
@ -328,13 +332,17 @@ No separate doc test; review maps contract statements to deterministic tests.
|
|||
|
||||
## Final Verification
|
||||
|
||||
1. `go test -count=1 ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/transport`
|
||||
2. `go test -count=10 ./apps/node/internal/node ./apps/node/internal/transport`
|
||||
3. `go test -race -count=3 ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/transport`
|
||||
4. `go test -count=1 ./...`
|
||||
5. `./scripts/e2e-smoke.sh`
|
||||
6. `make readability-audit`
|
||||
7. `git diff --check`
|
||||
1. `go version && go env GOMOD`
|
||||
2. `go test -count=1 ./packages/go/execution ./packages/go/streamgate ./packages/go/config`
|
||||
3. `go test -count=1 ./packages/go/execution ./apps/node/...`
|
||||
4. `go test -count=10 ./apps/node/internal/node ./apps/node/internal/transport`
|
||||
5. `go test -race -count=3 ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/transport`
|
||||
6. `go vet ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/transport`
|
||||
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`
|
||||
11. `git diff --check`
|
||||
|
||||
Record exact results and any environment-only E2E limitation in the review stub. A failed required deterministic or race test is a blocker; do not substitute a live provider smoke for these fixtures.
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,144 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/02+01_stall_watchdog plan=1 tag=API milestone-task=activity-contract,stall-watchdog -->
|
||||
|
||||
# Code Review Reference - 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-03
|
||||
task=m-node-provider-execution-liveness-recovery/02+01_stall_watchdog, plan=1, tag=API
|
||||
|
||||
## Archive Evidence Snapshot
|
||||
|
||||
- Prior plan: `agent-task/m-node-provider-execution-liveness-recovery/02+01_stall_watchdog/plan_cloud_G08_0.log`.
|
||||
- Prior review stub: `agent-task/m-node-provider-execution-liveness-recovery/02+01_stall_watchdog/code_review_cloud_G08_0.log`.
|
||||
- Prior verdict: none; implementation and implementation-owned evidence had not started.
|
||||
- Required carryover: use the request's Node-owned `run_id` as terminal `attempt_id` and prove caller metadata cannot spoof it.
|
||||
|
||||
## 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-G08.md` → `code_review_cloud_G08_1.log` and `PLAN-cloud-G08.md` → `plan_cloud_G08_1.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, preserve first-line `milestone-task=activity-contract,stall-watchdog` metadata in `complete.log` and report it for runtime aggregation. 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 |
|
||||
|------|---------|
|
||||
| API-1 — shared observer and typed stall terminal | [ ] |
|
||||
| API-2 — normalized execution integration | [ ] |
|
||||
| API-3 — raw tunnel and session lifetime integration | [ ] |
|
||||
| TEST-1 — deterministic temporal/concurrency evidence | [ ] |
|
||||
| DOC-1 — watchdog execution/wire contracts | [ ] |
|
||||
|
||||
## Implementation Checklist
|
||||
|
||||
- [ ] [API-1] Add a shared fake-clock-capable attempt observer and typed response-stalled evidence contract.
|
||||
- [ ] [API-2] Integrate the observer into normalized execution with safe admission/run cleanup and late-event fencing.
|
||||
- [ ] [API-3] Integrate the same observer into raw tunnels and bind both request paths to session disconnect.
|
||||
- [ ] [TEST-1] Prove activity, precedence, threshold races, exactly-once terminal, confirmed/unconfirmed fence, and resource ownership deterministically.
|
||||
- [ ] [DOC-1] Update the matching execution spec and execution/Edge-Node wire contracts for implemented Node watchdog behavior only.
|
||||
- [ ] Run every command in Final Verification and record exact results in `CODE_REVIEW-cloud-G08.md`.
|
||||
- [ ] 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.
|
||||
|
||||
- [ ] Append one verdict of `PASS`, `WARN`, or `FAIL` and verified `review_rework_count`, `evidence_integrity_failure` to `Code Review Result`.
|
||||
- [ ] Verify that verdict, `Dimension Assessment`, and Required/Suggested/Nit classifications match.
|
||||
- [ ] Archive active `CODE_REVIEW-*-G??.md` to `code_review_cloud_G08_1.log`.
|
||||
- [ ] Archive active `PLAN-*-G??.md` to `plan_cloud_G08_1.log`.
|
||||
- [ ] Verify that the Agent-Ops managed block in `.gitignore` unignores `agent-task/**/*.md` and `agent-task/**/*.log` and ignores `agent-roadmap/current.md`.
|
||||
- [ ] If PASS, write `complete.log` based on `agent-ops/skills/common/code-review/templates/complete-log-template.md` and leave no active `.md` files.
|
||||
- [ ] 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.
|
||||
- [ ] If PASS and task group is `m-node-provider-execution-liveness-recovery`, preserve and report `milestone-task` metadata for runtime aggregation, without modifying roadmap or directly calling `update-roadmap`.
|
||||
- [ ] 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
|
||||
|
||||
_Record any deviations from the plan and the rationale here._
|
||||
|
||||
## Key Design Decisions
|
||||
|
||||
_Record key design decisions here._
|
||||
|
||||
## Reviewer Checkpoints
|
||||
|
||||
- Verify the predecessor `01_activity_contract` PASS evidence was consumed and no duplicate activity/default logic was introduced.
|
||||
- Trace one terminal authority across provider terminal, watchdog expiry, cancel/deadline/disconnect, and late output for both normalized and tunnel paths.
|
||||
- Confirm a timer signal rechecks request/session termination before claiming stall and preserves existing deadline/transport classification.
|
||||
- Confirm `attempt_fence=confirmed` requires provider return within bounded close grace; unconfirmed attempts keep admission, run-manager, drain, and credential ownership until real provider exit.
|
||||
- Confirm `Failure.retryable` is true only for confirmed local fence and no Node retry or `recovery_eligible` appears.
|
||||
- Confirm normalized and tunnel terminal `run_id`/`attempt_id` come from the concrete Node-visible run identity and caller metadata cannot spoof either value.
|
||||
- Inspect fake-clock/channel tests for threshold/event/cancel races, release exactly once, and absence of wall-clock sleeps; independently rerun race tests.
|
||||
- Confirm contracts document only Node watchdog/fence behavior and retain unknown provider health pending the next slice.
|
||||
|
||||
## Verification Results
|
||||
|
||||
### `go test -count=1 ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/transport`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `go test -count=10 ./apps/node/internal/node ./apps/node/internal/transport`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `go test -race -count=3 ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/transport`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `go test -count=1 ./...`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `./scripts/e2e-smoke.sh`
|
||||
|
||||
_Implementing agent: record exit status and concise output, or the exact environment-only blocker._
|
||||
|
||||
### `make readability-audit`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `git diff --check`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
---
|
||||
|
||||
> **[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 |
|
||||
|
|
@ -0,0 +1,341 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/02+01_stall_watchdog plan=1 tag=API milestone-task=activity-contract,stall-watchdog -->
|
||||
|
||||
# PLAN — Node Response Stall Watchdog
|
||||
|
||||
## For the Implementing Agent
|
||||
|
||||
> **MANDATORY:** Do not begin until the dependency below has a PASS `complete.log`. Implement only this checklist, preserve unrelated user changes, and keep every edit inside the `stall-watchdog` slice. Do not update roadmap state, create follow-up plans, commit, push, or run an official code review. After implementation, fill every implementation-owned section of `CODE_REVIEW-cloud-G08.md` and leave active files in place for the review agent.
|
||||
|
||||
## Background
|
||||
|
||||
Node currently invokes normalized providers and raw tunnel adapters synchronously inside handler-owned cleanup. The normalized path defers terminals until admission release, but the tunnel path sends frames directly. Neither path observes provider-originated progress, derives request contexts from transport lifetime, or has an attempt-generation fence. A no-progress timeout therefore cannot safely race provider output, caller deadline, disconnect, cancel, or a provider that ignores cancellation.
|
||||
|
||||
This slice consumes the activity/config contract from `01_activity_contract`, installs a single Node-owned watchdog for both execution surfaces, and emits one stable `response_stalled` terminal with a confirmed or unconfirmed local fence. It deliberately emits `provider_health=unknown`; the dependent health-classification slice replaces that bounded fallback with target-aware probe evidence.
|
||||
|
||||
This replan incorporates the explicit pre-implementation self-review. The original pair incorrectly allowed caller-defined request metadata to override the Node-produced `attempt_id`. The Edge-Node wire contract explicitly says `RunRequest.metadata` is caller-defined and not a control surface, while `run_id` is the Node-visible identity of this concrete execution attempt. No implementation had started; the lifecycle design is retained and the identity rule plus spoof-resistance evidence are corrected.
|
||||
|
||||
## Archive Evidence Snapshot
|
||||
|
||||
- Prior plan: `agent-task/m-node-provider-execution-liveness-recovery/02+01_stall_watchdog/plan_cloud_G08_0.log`.
|
||||
- Prior review stub: `agent-task/m-node-provider-execution-liveness-recovery/02+01_stall_watchdog/code_review_cloud_G08_0.log`.
|
||||
- Prior verdict: none; implementation and implementation-owned evidence had not started.
|
||||
- Required carryover: use the request's Node-owned `run_id` as terminal `attempt_id` and prove caller metadata cannot spoof it.
|
||||
|
||||
## Dependencies
|
||||
|
||||
- `agent-task/m-node-provider-execution-liveness-recovery/01_activity_contract/complete.log`
|
||||
|
||||
At plan creation the predecessor is active and not complete. The implementing runtime must wait for its PASS completion, then use the resulting activity helpers, effective timeout lookup, generated wire field, and updated contracts rather than duplicating them.
|
||||
|
||||
## 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-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/stream-evidence-gate.md`
|
||||
- `apps/node/internal/transport/session.go`
|
||||
- `apps/node/internal/transport/session_test.go`
|
||||
- `apps/node/internal/node/node.go`
|
||||
- `apps/node/internal/node/run_handler.go`
|
||||
- `apps/node/internal/node/run_manager.go`
|
||||
- `apps/node/internal/node/runtime_sink.go`
|
||||
- `apps/node/internal/node/tunnel_handler.go`
|
||||
- `apps/node/internal/node/run_cancel_test.go`
|
||||
- `apps/node/internal/node/provider_tunnel_test.go`
|
||||
- `apps/node/internal/node/node_test_support_test.go`
|
||||
- `packages/go/execution/types.go`
|
||||
- `packages/go/execution/failure.go`
|
||||
- `packages/go/execution/emitter.go`
|
||||
- `packages/go/execution/failure_test.go`
|
||||
- `packages/go/execution/emitter_test.go`
|
||||
- `agent-contract/inner/execution-runtime.md`
|
||||
- `agent-contract/inner/edge-node-runtime-wire.md`
|
||||
|
||||
### SDD Criteria
|
||||
|
||||
- SDD status: approved, D01 resolved, implementation lock released.
|
||||
- Scenarios: the integrated run/tunnel lifecycle completes S01 / `activity-contract` and implements S02 / `stall-watchdog` (`SDD.md:92-93`).
|
||||
- Evidence rows: S01 fake-clock activity/deadline/transport lifecycle evidence and S02 threshold, timer/event/cancel/close race, exactly-once terminal, confirmed/unconfirmed fence, and late-event fencing (`SDD.md:103-104`).
|
||||
- Precedence: request hard deadline or current connection heartbeat/disconnect that wins first keeps its existing boundary (`SDD.md:68,92`).
|
||||
- Output: normalized `RunEvent{type=error}` and tunnel `ProviderTunnelFrame{kind=ERROR}` exactly once, stable `response_stalled`, safe metadata, and `Retryable` true only when fence is confirmed (`SDD.md:75-77`).
|
||||
- Prohibitions: no provider-specific watchdogs, no Node retry, no recovery eligibility, no late attempt revival or double resource release (`SDD.md:81-86`).
|
||||
|
||||
### Verification Context
|
||||
|
||||
- Baseline passed before plan creation:
|
||||
- `go test -count=1 ./apps/node/internal/node ./apps/node/internal/transport`
|
||||
- `go test -race -count=1 ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/transport`
|
||||
- Local deterministic fixtures are sufficient; no external provider or credentials are required.
|
||||
- Timer tests must use an injected fake clock and synchronization channels, not wall-clock sleeps.
|
||||
- Runtime execution changes require targeted unit/race tests plus the repository full Go suite and local E2E smoke.
|
||||
|
||||
### Test Coverage Gaps
|
||||
|
||||
- `terminalDeferringSink` suppresses post-terminal normalized events but has no liveness generation or activity notification.
|
||||
- Raw `tunnelSink` does not fence late frames or claim one terminal.
|
||||
- Run/tunnel handlers call providers on the cleanup-owning goroutine, so an adapter that ignores cancellation can retain or prematurely release admission/run ownership incorrectly.
|
||||
- Session listeners pass `context.Background()` (`apps/node/internal/transport/session.go:50-52,75-87`), so disconnect does not cancel current request handlers.
|
||||
- No tests cover timer/activity boundary ordering, hard deadline/disconnect precedence, close grace, or confirmed/unconfirmed resource ownership.
|
||||
|
||||
### Symbol References
|
||||
|
||||
- `apps/node/internal/node/run_handler.go:28-90` — config lock, resolve/admit, timeout context, run registration.
|
||||
- `apps/node/internal/node/run_handler.go:97-142` — terminal-deferring sink, provider call, ticket/run cleanup, foreground/background return.
|
||||
- `apps/node/internal/node/run_handler.go:246-265` — synthetic terminal construction.
|
||||
- `apps/node/internal/node/tunnel_handler.go:41-78,107-154` — lookup/admission, sink, timeout context, run registration, direct tunnel call.
|
||||
- `apps/node/internal/node/tunnel_handler.go:157-226` — plain tunnel error and direct frame conversion.
|
||||
- `apps/node/internal/node/runtime_sink.go:25-82` — normalized terminal claim/defer behavior.
|
||||
- `apps/node/internal/node/run_manager.go:13-51,71-92` — cancellation handle and drain lifecycle.
|
||||
- `apps/node/internal/transport/session.go:50-87,211-225` — background request contexts and connection done/close boundary.
|
||||
- `packages/go/execution/failure.go:12-32,83-129` — stable failure vocabulary and normalization.
|
||||
|
||||
### Split Judgment
|
||||
|
||||
- Classification: large. Correctness depends on temporal state, concurrent exactly-once claims, cancellation and transport precedence, and resource ownership across two execution variants.
|
||||
- Cohesion: normalized and tunnel paths must share one watchdog/fence primitive so they cannot diverge on activity or terminal semantics.
|
||||
- Predecessor: `01_activity_contract` is required and encoded in the directory name and Dependencies section.
|
||||
- Successor: `03+02_health_classification` will enrich the same terminal evidence but may not change timer/fence ownership.
|
||||
- Collision check: no active plan claimed these paths when prepared; dependency sequencing prevents overlap with predecessor contract files.
|
||||
|
||||
### Scope Rationale
|
||||
|
||||
- In scope: Node request lifetime context, fake-clock watchdog, provider-call isolation, progress reset, exactly-once terminal, cancellation/close grace, late emission fence, confirmed/unconfirmed evidence, and safe cleanup.
|
||||
- Out of scope: actual target probe, Edge health overlay/binding, lease projection, ingress retry/recovery, metric surface, config schema (owned by predecessor), and provider-adapter-specific timers.
|
||||
- A provider that ignores cancellation remains locally fenced but holds its Node admission/run ownership until its goroutine actually exits. This preserves capacity and refresh-drain integrity while reporting `attempt_fence=unconfirmed`.
|
||||
|
||||
### Final Routing
|
||||
|
||||
- `evaluation_mode=isolated-reassessment`; `finalizer=finalize-task-policy.sh`, `finalizer_mode=pair`.
|
||||
- Build closure: scope/context/verification/evidence/ownership/decision all closed and trusted; capability gap none.
|
||||
- Build score: `scope=2`, `state=2`, `blast=1`, `evidence=1`, `verification=2` -> G08; `base_route_basis=local-fit`, `route_basis=risk-boundary`, lane `cloud`, file `PLAN-cloud-G08.md`.
|
||||
- Build signals: `large_indivisible_context=false`, positive loop risks `temporal_state`, `concurrent_consistency`, `boundary_contract`, `variant_product` (`count=4`), `review_rework_count=0`, `evidence_integrity_failure=false`; risk boundary matched, recovery boundary not matched.
|
||||
- Review closure: scope/context/verification/evidence/ownership/decision all closed and trusted; capability gap none.
|
||||
- Review score: `scope=2`, `state=2`, `blast=1`, `evidence=1`, `verification=2` -> G08; `route_basis=official-review`, lane `cloud`, adapter/model `codex/gpt-5.6-sol`, reasoning `xhigh`, file `CODE_REVIEW-cloud-G08.md`.
|
||||
|
||||
## Implementation Checklist
|
||||
|
||||
- [ ] [API-1] Add a shared fake-clock-capable attempt observer and typed response-stalled evidence contract.
|
||||
- [ ] [API-2] Integrate the observer into normalized execution with safe admission/run cleanup and late-event fencing.
|
||||
- [ ] [API-3] Integrate the same observer into raw tunnels and bind both request paths to session disconnect.
|
||||
- [ ] [TEST-1] Prove activity, precedence, threshold races, exactly-once terminal, confirmed/unconfirmed fence, and resource ownership deterministically.
|
||||
- [ ] [DOC-1] Update the matching execution spec and execution/Edge-Node wire contracts for implemented Node watchdog behavior only.
|
||||
- [ ] Run every command in Final Verification and record exact results in `CODE_REVIEW-cloud-G08.md`.
|
||||
- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.
|
||||
|
||||
### [API-1] Build one attempt observer and typed stall terminal
|
||||
|
||||
**Problem**
|
||||
|
||||
There is no owner for the no-progress clock or for racing provider terminal, watchdog, cancel, and late emission. Adding independent timers to `run_handler.go` and `tunnel_handler.go` would create variant drift and non-deterministic cleanup.
|
||||
|
||||
**Solution**
|
||||
|
||||
Add `apps/node/internal/node/liveness_watchdog.go` with package-private abstractions:
|
||||
|
||||
- an injectable clock/timer interface with a real default on `Node` and a deterministic manual test implementation;
|
||||
- one attempt observer state machine that accepts the predecessor's `ProviderActivityDisposition`, resets on progress, stops on terminal, and exposes a single atomic/mutex-protected terminal/fence claim;
|
||||
- an emission authority wrapper for each sink that drops every provider event/frame after the terminal claim and reports activity to the observer before forwarding valid non-terminal output;
|
||||
- a bounded cancel/close grace timer owned by the same injected clock. On threshold, cancel provider execution, revoke provider emission authority immediately, and classify `confirmed` only when the provider call has returned within grace; otherwise classify `unconfirmed`;
|
||||
- after a timer signal, re-check request context and session lifetime before claiming stall so a simultaneous/earlier hard deadline, caller cancel, or disconnect is never reclassified;
|
||||
- a stable `FailureCodeResponseStalled` in `packages/go/execution/failure.go`, known-code encoding/decoding support, and a single metadata builder for `failure_code`, `provider_health=unknown`, `liveness_classification=health_unknown`, `idle_duration_ms`, `run_id`, `attempt_id`, `attempt_fence`, `adapter`, and `target`. Set both `run_id` and `attempt_id` from the concrete request's Node-owned run identity (`ExecutionSpec.RunID`/wire `run_id`). Never derive either field from caller-defined request metadata, even when metadata contains `run_id` or `attempt_id`; never include raw output, prompt, reasoning, credentials, or `recovery_eligible`.
|
||||
|
||||
The normalized terminal is an error `RuntimeEvent` carrying `Failure{Code: response_stalled, Retryable: fence == confirmed}`. The tunnel terminal is an ERROR frame with the same safe metadata and stable error text. The health successor will replace only the unknown classification fields.
|
||||
|
||||
**Modified files**
|
||||
|
||||
- [ ] `packages/go/execution/failure.go`
|
||||
- [ ] `packages/go/execution/failure_test.go`
|
||||
- [ ] `apps/node/internal/node/node.go`
|
||||
- [ ] `apps/node/internal/node/liveness_watchdog.go`
|
||||
|
||||
**Test decision**
|
||||
|
||||
Required. Unit-test the state machine and failure round-trip independently before handler integration.
|
||||
|
||||
**Verification**
|
||||
|
||||
- `go test -count=1 ./packages/go/execution ./apps/node/internal/node`
|
||||
- `go test -race -count=1 ./packages/go/execution ./apps/node/internal/node`
|
||||
|
||||
### [API-2] Integrate normalized execution without releasing an unclosed attempt
|
||||
|
||||
**Problem**
|
||||
|
||||
The run closure invokes `adapter.Execute` directly and owns all defers (`apps/node/internal/node/run_handler.go:106-136`). It cannot emit a terminal while retaining admission/run ownership for an adapter that ignores cancel, and its existing terminal sink has only a boolean terminal observation.
|
||||
|
||||
**Solution**
|
||||
|
||||
Refactor normalized execution around an explicit provider-call result channel and exactly-once cleanup owner:
|
||||
|
||||
- create the observer after resolve/admission using the predecessor's effective timeout lookup;
|
||||
- execute the provider in one goroutine with the fenced activity sink;
|
||||
- have the coordinator select among provider return, observer expiry, request deadline/cancel, and session lifetime;
|
||||
- preserve existing complete/error/cancel synthesis when provider return or context termination wins; on stall, claim and queue the typed stall terminal exactly once;
|
||||
- release the admission ticket before flushing the terminal only after provider ownership is confirmed closed, preserving the existing Edge wake-up ordering;
|
||||
- for an unconfirmed provider, emit/flush the terminal but move ticket release, run-manager deregistration, `done` close, credential/cancel cleanup if applicable, and final provider-return drain to one detached cleanup closure. It must execute exactly once when the provider eventually returns; until then refresh drain and capacity continue to see the old attempt;
|
||||
- retain background request behavior and current store completion semantics, recording the stalled run as error without converting it to caller cancellation;
|
||||
- replace or extend `terminalDeferringSink` so provider terminal, watchdog terminal, and late provider output share one terminal authority.
|
||||
|
||||
Do not start a replacement attempt and do not infer response commit/recovery eligibility.
|
||||
|
||||
**Modified files**
|
||||
|
||||
- [ ] `apps/node/internal/node/run_handler.go`
|
||||
- [ ] `apps/node/internal/node/run_manager.go`
|
||||
- [ ] `apps/node/internal/node/runtime_sink.go`
|
||||
- [ ] `apps/node/internal/node/liveness_watchdog.go`
|
||||
|
||||
**Test decision**
|
||||
|
||||
Required. Include foreground and background execution, provider-emitted versus Node-synthesized terminal, cancel race, and admission/run ownership assertions.
|
||||
|
||||
**Verification**
|
||||
|
||||
- `go test -count=1 ./apps/node/internal/node`
|
||||
- `go test -race -count=1 ./apps/node/internal/node`
|
||||
|
||||
### [API-3] Integrate raw tunnels and transport lifetime
|
||||
|
||||
**Problem**
|
||||
|
||||
Raw tunnel frames are sent directly and cleanup is deferred around the provider call (`apps/node/internal/node/tunnel_handler.go:107-154`). Session listeners pass background contexts, so an Edge/Node heartbeat disconnect closes the session but not the active request handler.
|
||||
|
||||
**Solution**
|
||||
|
||||
- Give each `Session` a connection-lifetime context canceled exactly once when `Done()` closes. Derive run and tunnel listener contexts from it and cancel per-request children on handler return; leave command/cancel listener semantics unchanged unless required for leak-free shared context plumbing.
|
||||
- Add the same observer/fencing sink to tunnels. Response-start/header, non-empty body, and usage reset via the predecessor classifier; END/ERROR terminates; empty frames do nothing.
|
||||
- Run `TunnelProvider` through the same result-channel coordination and cleanup invariants as normalized execution. On stall emit exactly one ERROR frame with typed metadata, then drop all late frames. On unconfirmed close, retain admission ticket/run handle until actual adapter return.
|
||||
- When request hard deadline/caller cancellation/session disconnect wins first, cancel and finish through the existing error/transport boundary; do not synthesize `response_stalled`. A dead session must not be treated as confirmed provider progress or be revived for terminal delivery.
|
||||
- Keep tunnel credential material zeroization tied to the real provider ownership lifetime; never return while a still-running adapter retains plaintext and then zero the buffer underneath it.
|
||||
|
||||
**Modified files**
|
||||
|
||||
- [ ] `apps/node/internal/transport/session.go`
|
||||
- [ ] `apps/node/internal/node/tunnel_handler.go`
|
||||
- [ ] `apps/node/internal/node/liveness_watchdog.go`
|
||||
|
||||
**Test decision**
|
||||
|
||||
Required. Session tests prove disconnect cancellation and no leaked request context. Tunnel tests prove hard deadline/disconnect precedence, terminal once, late frame drop, and confirmed/unconfirmed retention.
|
||||
|
||||
**Verification**
|
||||
|
||||
- `go test -count=1 ./apps/node/internal/transport ./apps/node/internal/node`
|
||||
- `go test -race -count=1 ./apps/node/internal/transport ./apps/node/internal/node`
|
||||
|
||||
### [TEST-1] Exercise timer and cleanup boundaries without sleeps
|
||||
|
||||
**Problem**
|
||||
|
||||
The acceptance boundary is defined by race outcomes. Ordinary happy-path tests and real `time.Sleep` cannot prove deterministic ordering or absence of double cleanup.
|
||||
|
||||
**Solution**
|
||||
|
||||
Add a package-internal fake clock plus channel-controlled providers and table tests covering:
|
||||
|
||||
- start/no-reset, non-empty text/reasoning reset, response-start/header/body/usage reset, empty frame no-reset, and terminal stop;
|
||||
- exact threshold minus one tick versus threshold, event-at-threshold, provider terminal-at-threshold, caller cancel, hard deadline, and session disconnect;
|
||||
- provider returns within close grace (`confirmed`) and ignores cancel beyond grace (`unconfirmed`), including eventual return;
|
||||
- one normalized terminal and one tunnel ERROR only, late delta/frame dropped, retryable only when confirmed;
|
||||
- caller metadata containing spoofed `run_id` or `attempt_id` cannot override the Node-produced terminal identity on either surface;
|
||||
- admission count, run-manager presence, drain wait, credential lifetime, and release exactly once for both fence outcomes;
|
||||
- `go test -race` with repeated boundary cases; assertions use channels/manual clock, never scheduler sleeps.
|
||||
|
||||
Extend the closest existing integration tests rather than duplicating all transport fixtures. Use the new focused test file for the shared state machine and cross-surface tables.
|
||||
|
||||
**Modified files**
|
||||
|
||||
- [ ] `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_test.go`
|
||||
|
||||
**Test decision**
|
||||
|
||||
Required; this is the primary acceptance evidence for S01 lifecycle portions and S02.
|
||||
|
||||
**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] Document Node watchdog and wire terminal semantics
|
||||
|
||||
**Problem**
|
||||
|
||||
The execution and Edge-Node wire contracts do not describe `response_stalled`, local fence meaning, precedence, or the safe terminal metadata implemented here.
|
||||
|
||||
**Solution**
|
||||
|
||||
Update the matching execution spec and both contracts to match code:
|
||||
|
||||
- Node owns detection, cancel, local emission fence, and local execution/transport close classification;
|
||||
- exact activity reset and hard-deadline/disconnect precedence inherited from the predecessor;
|
||||
- normalized/tunnel terminal shapes and metadata, including `provider_health=unknown` until bounded classification completes in the next slice;
|
||||
- confirmed versus unconfirmed ownership and retryable-as-capability-only semantics;
|
||||
- no Node retry, no `recovery_eligible`, no Edge overlay or stale-binding application in this slice.
|
||||
|
||||
Do not update roadmap or spec state; implementation contracts change alongside code.
|
||||
|
||||
**Modified files**
|
||||
|
||||
- [ ] `agent-contract/inner/execution-runtime.md`
|
||||
- [ ] `agent-contract/inner/edge-node-runtime-wire.md`
|
||||
- [ ] `agent-spec/runtime/edge-node-execution.md`
|
||||
|
||||
**Test decision**
|
||||
|
||||
No separate doc test; review maps contract statements to deterministic tests.
|
||||
|
||||
**Verification**
|
||||
|
||||
- `git diff --check`
|
||||
|
||||
## Modified Files Summary
|
||||
|
||||
| Path | Action | Checklist |
|
||||
|------|--------|-----------|
|
||||
| `packages/go/execution/failure.go` | modify | API-1 |
|
||||
| `packages/go/execution/failure_test.go` | modify | API-1 |
|
||||
| `apps/node/internal/node/node.go` | modify | API-1 |
|
||||
| `apps/node/internal/node/liveness_watchdog.go` | add | API-1, API-2, API-3 |
|
||||
| `apps/node/internal/node/run_handler.go` | modify | API-2 |
|
||||
| `apps/node/internal/node/run_manager.go` | modify | API-2 |
|
||||
| `apps/node/internal/node/runtime_sink.go` | modify | API-2 |
|
||||
| `apps/node/internal/transport/session.go` | modify | API-3 |
|
||||
| `apps/node/internal/node/tunnel_handler.go` | modify | API-3 |
|
||||
| `apps/node/internal/node/liveness_watchdog_test.go` | add | 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-contract/inner/execution-runtime.md` | modify | DOC-1 |
|
||||
| `agent-contract/inner/edge-node-runtime-wire.md` | modify | DOC-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-G08.md` | update evidence | all |
|
||||
|
||||
## Final Verification
|
||||
|
||||
1. `go test -count=1 ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/transport`
|
||||
2. `go test -count=10 ./apps/node/internal/node ./apps/node/internal/transport`
|
||||
3. `go test -race -count=3 ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/transport`
|
||||
4. `go test -count=1 ./...`
|
||||
5. `./scripts/e2e-smoke.sh`
|
||||
6. `make readability-audit`
|
||||
7. `git diff --check`
|
||||
|
||||
Record exact results and any environment-only E2E limitation in the review stub. A failed required deterministic or race test is a blocker; do not substitute a live provider smoke for these fixtures.
|
||||
|
||||
**After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`.**
|
||||
|
|
@ -0,0 +1,124 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/03+02_health_probe_contract plan=2 tag=API milestone-task=health-classification -->
|
||||
|
||||
# Code Review Reference - API
|
||||
|
||||
> **[IMPLEMENTING AGENT — READ FIRST]** Fill every implementation-owned section after implementation. Do not append a verdict, archive files, write `complete.log`, commit, push, or change roadmap state.
|
||||
|
||||
## Overview
|
||||
|
||||
date=2026-08-03
|
||||
task=m-node-provider-execution-liveness-recovery/03+02_health_probe_contract, plan=2, tag=API
|
||||
|
||||
## Archive Evidence Snapshot
|
||||
|
||||
- Original pair: `plan_cloud_G08_0.log` / `code_review_cloud_G08_0.log`.
|
||||
- Semantic replacement: `plan_cloud_G08_1.log` / `code_review_cloud_G08_1.log`.
|
||||
- Prior verdict: none; implementation had not started.
|
||||
- Refine carryover: inconclusive prober failures remain unknown; only a valid matching unavailable result is provider-unhealthy.
|
||||
|
||||
## For the Review Agent
|
||||
|
||||
> **[REVIEW AGENT ONLY]** Compare each item to source and recorded output. Append a verdict only during the later official review.
|
||||
|
||||
1. On later review, archive this file to `code_review_cloud_G07_2.log` and the plan to `plan_local_G07_2.log`.
|
||||
2. PASS finalization preserves `milestone-task=health-classification`; roadmap aggregation remains owned by `sync-milestone-workstate`.
|
||||
|
||||
## Implementation Item Completion
|
||||
|
||||
| Item | Status |
|
||||
|------|---------|
|
||||
| API-1 — shared fail-closed outcome contract | [ ] |
|
||||
| API-2 — prober errors and bounded coordinator | [ ] |
|
||||
| TEST-1 — deterministic adapter/classifier evidence | [ ] |
|
||||
| DOC-1 — execution probe contract | [ ] |
|
||||
|
||||
## Implementation Checklist
|
||||
|
||||
- [ ] [API-1] Define stable shared health/liveness classification values and a pure fail-closed probe outcome normalizer.
|
||||
- [ ] [API-2] Make supported probers expose inconclusive errors and add one independent bounded exact-target Node probe coordinator.
|
||||
- [ ] [TEST-1] Prove adapter and classifier outcome semantics deterministically without live providers.
|
||||
- [ ] [DOC-1] Update the execution-runtime contract for the typed probe boundary only.
|
||||
- [ ] Run every command in Final Verification and record exact results in `CODE_REVIEW-cloud-G07.md`.
|
||||
- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.
|
||||
|
||||
## Review-Only Checklist
|
||||
|
||||
> **[REVIEW AGENT ONLY]** Implementing agents must leave this section unchanged.
|
||||
|
||||
- [ ] Append exactly one PASS/WARN/FAIL verdict with routing signals.
|
||||
- [ ] Verify evidence and dimension assessment match that verdict.
|
||||
- [ ] Archive active files to `code_review_cloud_G07_2.log` and `plan_local_G07_2.log`.
|
||||
- [ ] Verify the Agent-Ops managed `.gitignore` block.
|
||||
- [ ] If PASS, write `complete.log`, preserve milestone metadata, and archive this task directory.
|
||||
- [ ] If WARN/FAIL, write only the next state required by the code-review skill.
|
||||
|
||||
## Deviations from Plan
|
||||
|
||||
_Implementing agent: replace with actual deviations or `None`._
|
||||
|
||||
## Key Design Decisions
|
||||
|
||||
_Implementing agent: record actual decisions._
|
||||
|
||||
## Reviewer Checkpoints
|
||||
|
||||
- Verify the `02+01_stall_watchdog` dependency is PASS and this child does not re-own timer/fence/terminal sequencing.
|
||||
- Confirm supported probers return endpoint/network/HTTP/decode errors and reserve unavailable for a valid exact-target result.
|
||||
- Confirm available -> request-stalled, valid unavailable -> provider-unhealthy, and every unsupported/error/timeout/unknown/identity mismatch -> health-unknown.
|
||||
- Confirm probe context is independent, exactly bounded, and never calls progress/reset.
|
||||
- Confirm stable values live in the shared execution contract and arbitrary provider metadata is not copied.
|
||||
- Confirm command-handler external behavior remains compatible.
|
||||
|
||||
## Verification Results
|
||||
|
||||
### `go version && go env GOMOD`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `go test -count=1 ./packages/go/execution ./packages/go/streamgate ./packages/go/config`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `go test -count=1 ./packages/go/execution ./apps/node/...`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `go test -count=10 ./packages/go/execution ./apps/node/internal/node`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `go test -race -count=3 ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/adapters/ollama ./apps/node/internal/adapters/vllm ./apps/node/internal/adapters/openai_compat`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `go vet ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/adapters/ollama ./apps/node/internal/adapters/vllm ./apps/node/internal/adapters/openai_compat`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `go test -count=1 ./...`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `./scripts/e2e-smoke.sh`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `IOP_DEV_RECONNECT_BIND_TIMEOUT=45 ./scripts/dev/edge-node-reconnect-diagnostic.sh`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `make readability-audit`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `git diff --check`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
## Section Ownership
|
||||
|
||||
| Section | Owner |
|
||||
|---------|-------|
|
||||
| Header, overview, archive snapshot, checklist item text, reviewer checkpoints, verification headings | Fixed at stub creation |
|
||||
| Item/checklist status, deviations, decisions, verification output | Implementing agent |
|
||||
| Review-only checklist and verdict/finalization | Review agent only |
|
||||
|
|
@ -0,0 +1,170 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/03+02_health_probe_contract plan=2 tag=API milestone-task=health-classification -->
|
||||
|
||||
# PLAN — Node Health Probe Contract
|
||||
|
||||
## For the Implementing Agent
|
||||
|
||||
> **MANDATORY:** Do not begin until the dependency below has a PASS `complete.log`. Implement only this checklist, preserve unrelated user changes, and keep every edit inside this probe-contract slice. Do not update roadmap state, create follow-up plans, commit, push, or run an official code review. After implementation, fill every implementation-owned section of `CODE_REVIEW-cloud-G07.md` and leave both active files in place.
|
||||
|
||||
## Background
|
||||
|
||||
The watchdog predecessor ends a stalled attempt with fail-closed unknown health. Before that terminal can be enriched, Node needs a typed, target-aware probe contract whose unavailable result cannot be confused with endpoint, HTTP, decode, timeout, or identity errors. Current Ollama, vLLM, and OpenAI-compatible probers swallow several such errors into `StatusUnavailable, nil`; reusing them would turn inconclusive evidence into provider-wide unhealthy.
|
||||
|
||||
This child is the first result of applying refine-plans once to the unstarted semantic replacement. It makes adapter error semantics observable, defines stable shared classification values, and provides a bounded pure coordinator. It does not touch session sequencing, watchdog terminal assembly, timer/fence ownership, Edge overlay, retry, or recovery.
|
||||
|
||||
## Archive Evidence Snapshot
|
||||
|
||||
- Original pair: `agent-task/m-node-provider-execution-liveness-recovery/03+02_health_probe_contract/plan_cloud_G08_0.log` and `code_review_cloud_G08_0.log`.
|
||||
- Semantic replacement before refinement: `plan_cloud_G08_1.log` and `code_review_cloud_G08_1.log` in this directory.
|
||||
- Prior verdict: none; implementation and implementation-owned evidence had not started.
|
||||
- Refine carryover: explicit adapter-unavailable is unhealthy only when a valid exact-target result says unavailable; all transport/protocol/decode/timeout/unsupported/unknown/identity-inconclusive outcomes remain unknown.
|
||||
|
||||
## Dependencies
|
||||
|
||||
- `agent-task/m-node-provider-execution-liveness-recovery/02+01_stall_watchdog/complete.log`
|
||||
|
||||
Consume the predecessor activity/failure types after PASS. Its transitive `+01` dependency supplies the activity contract.
|
||||
|
||||
## Analysis
|
||||
|
||||
### Files Read
|
||||
|
||||
- `AGENTS.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-contract/inner/execution-runtime.md`
|
||||
- `packages/go/execution/types.go`
|
||||
- predecessor-planned `packages/go/execution/liveness.go`
|
||||
- `apps/node/internal/node/command_handler.go`
|
||||
- `apps/node/internal/node/node.go`
|
||||
- `apps/node/internal/adapters/ollama/ollama.go`
|
||||
- `apps/node/internal/adapters/ollama/provider.go`
|
||||
- `apps/node/internal/adapters/ollama/ollama_test.go`
|
||||
- `apps/node/internal/adapters/vllm/provider.go`
|
||||
- `apps/node/internal/adapters/vllm/vllm_test.go`
|
||||
- `apps/node/internal/adapters/openai_compat/provider.go`
|
||||
- `apps/node/internal/adapters/openai_compat/capabilities_test.go`
|
||||
- `scripts/dev/edge-node-reconnect-diagnostic.sh`
|
||||
|
||||
### SDD Criteria
|
||||
|
||||
- Approved SDD S03 requires an independent bounded exact-target probe and stable available/unavailable/unknown mapping.
|
||||
- Available maps to `request_stalled`; a valid exact-target unavailable result maps to `provider_unhealthy`; unsupported, timeout, error, unknown, and identity-inconclusive map to `health_unknown`.
|
||||
- Probe completion is evidence only and must never reset original request progress, change the attempt fence, or authorize retry.
|
||||
|
||||
### Verification Context
|
||||
|
||||
- Local Go module; no external provider or credentials are required.
|
||||
- Tests use local HTTP fixtures and injected probe/context functions, never live endpoints or wall-clock sleeps.
|
||||
- `./scripts/e2e-smoke.sh` is auxiliary test-only coverage. The credential-free real-process check is `IOP_DEV_RECONNECT_BIND_TIMEOUT=45 ./scripts/dev/edge-node-reconnect-diagnostic.sh`.
|
||||
|
||||
### Test Coverage Gaps
|
||||
|
||||
- Supported probers currently collapse endpoint/network/HTTP/decode errors into a normal unavailable result.
|
||||
- There is no stable shared liveness-classification vocabulary or pure outcome normalizer.
|
||||
- The capabilities command error mapping is not a safe liveness contract and must retain its current external behavior.
|
||||
- No bounded exact-target classifier proves error/timeout/identity mismatch remain unknown.
|
||||
|
||||
### Split Judgment
|
||||
|
||||
- This refined child is independently reviewable: adapter error semantics and pure classification can PASS without changing terminal timing or session state.
|
||||
- The child is local G07; its dependency on the cloud G08 watchdog serializes overlap with predecessor-created execution contracts.
|
||||
- The dependent `04+03_health_evidence` owns all connection sequence and terminal integration work.
|
||||
|
||||
### Scope Rationale
|
||||
|
||||
- In scope: shared constants/types, fail-closed outcome normalization, supported prober error propagation, bounded exact-target probe coordinator, focused tests, and the matching execution contract.
|
||||
- Out of scope: session counters, handler/watchdog edits, terminal metadata, Edge generation binding/overlay, candidate exclusion, retry, recovery, and configuration.
|
||||
|
||||
### Final Routing
|
||||
|
||||
- `evaluation_mode=isolated-reassessment`; pair finalizer.
|
||||
- Build score `scope=2,state=1,blast=1,evidence=1,verification=2` -> G07; local `PLAN-local-G07.md`.
|
||||
- Loop risks: `temporal_state`, `boundary_contract` (`count=2`); no recovery boundary or evidence-integrity failure.
|
||||
- Review uses official-review cloud G07 in `CODE_REVIEW-cloud-G07.md`.
|
||||
|
||||
## Implementation Checklist
|
||||
|
||||
- [ ] [API-1] Define stable shared health/liveness classification values and a pure fail-closed probe outcome normalizer.
|
||||
- [ ] [API-2] Make supported probers expose inconclusive errors and add one independent bounded exact-target Node probe coordinator.
|
||||
- [ ] [TEST-1] Prove adapter and classifier outcome semantics deterministically without live providers.
|
||||
- [ ] [DOC-1] Update the execution-runtime contract for the typed probe boundary only.
|
||||
- [ ] Run every command in Final Verification and record exact results in `CODE_REVIEW-cloud-G07.md`.
|
||||
- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.
|
||||
|
||||
### [API-1] Shared fail-closed outcome contract
|
||||
|
||||
Add the predecessor-compatible definitions to `packages/go/execution/liveness.go`: stable provider-health and liveness-classification constants, a typed probe outcome input, and a pure normalizer. A validated matching available result yields request-stalled; a validated matching unavailable result yields provider-unhealthy. Returned error, context cancellation/deadline, unsupported adapter, unknown status, empty/mismatched adapter or target, and malformed identity yield health-unknown. Do not copy arbitrary provider metadata.
|
||||
|
||||
**Modified files**
|
||||
|
||||
- [ ] `packages/go/execution/liveness.go`
|
||||
- [ ] `packages/go/execution/liveness_test.go`
|
||||
|
||||
**Test decision:** Required; table-test every outcome and identity combination.
|
||||
|
||||
### [API-2] Preserve adapter errors and bound the exact-target probe
|
||||
|
||||
Update Ollama, vLLM, and OpenAI-compatible `ProbeProvider` implementations so endpoint construction, request/network, non-success HTTP, and decode failures return their underlying error instead of manufacturing unavailable. A valid response that positively reports the exact target absent remains `StatusUnavailable, nil`; available remains available. Keep the capabilities command external mapping unchanged.
|
||||
|
||||
Add `apps/node/internal/node/health_probe.go` with a package-private five-second ceiling and an injectable context/probe hook. Root it independently from the canceled execution request, re-check its deadline/cancel result, validate adapter/target identity, and feed only the typed outcome normalizer. It returns evidence and never calls observer progress/reset.
|
||||
|
||||
**Modified files**
|
||||
|
||||
- [ ] `apps/node/internal/node/health_probe.go`
|
||||
- [ ] `apps/node/internal/node/health_probe_test.go`
|
||||
- [ ] `apps/node/internal/adapters/ollama/provider.go`
|
||||
- [ ] `apps/node/internal/adapters/ollama/ollama_test.go`
|
||||
- [ ] `apps/node/internal/adapters/vllm/provider.go`
|
||||
- [ ] `apps/node/internal/adapters/vllm/vllm_test.go`
|
||||
- [ ] `apps/node/internal/adapters/openai_compat/provider.go`
|
||||
- [ ] `apps/node/internal/adapters/openai_compat/capabilities_test.go`
|
||||
|
||||
**Test decision:** Required; local fixtures distinguish exact-target absence from network, HTTP, decode, timeout, unsupported, and identity mismatch.
|
||||
|
||||
### [TEST-1] Deterministic contract evidence
|
||||
|
||||
Assert that all three adapters surface inconclusive errors, exact-target absence stays explicit unavailable, the coordinator receives a live independent bounded context, and all fail-closed branches return only stable safe values. No test may contact a live provider or use scheduler sleeps.
|
||||
|
||||
### [DOC-1] Probe contract only
|
||||
|
||||
Document the typed three-way mapping, error propagation, exact identity validation, independent bound, and explicit exclusion of progress reset, terminal sequencing, Edge overlay, retry, and recovery.
|
||||
|
||||
**Modified files**
|
||||
|
||||
- [ ] `agent-contract/inner/execution-runtime.md`
|
||||
|
||||
## Modified Files Summary
|
||||
|
||||
| Path | Action | Checklist |
|
||||
|------|--------|-----------|
|
||||
| `packages/go/execution/liveness.go` | modify predecessor file | API-1 |
|
||||
| `packages/go/execution/liveness_test.go` | modify predecessor test | API-1 |
|
||||
| `apps/node/internal/node/health_probe.go` | add | API-2 |
|
||||
| `apps/node/internal/node/health_probe_test.go` | add | API-2, TEST-1 |
|
||||
| `apps/node/internal/adapters/ollama/provider.go` | modify | API-2 |
|
||||
| `apps/node/internal/adapters/ollama/ollama_test.go` | modify | TEST-1 |
|
||||
| `apps/node/internal/adapters/vllm/provider.go` | modify | API-2 |
|
||||
| `apps/node/internal/adapters/vllm/vllm_test.go` | modify | TEST-1 |
|
||||
| `apps/node/internal/adapters/openai_compat/provider.go` | modify | API-2 |
|
||||
| `apps/node/internal/adapters/openai_compat/capabilities_test.go` | modify | TEST-1 |
|
||||
| `agent-contract/inner/execution-runtime.md` | modify | DOC-1 |
|
||||
| `agent-task/m-node-provider-execution-liveness-recovery/03+02_health_probe_contract/CODE_REVIEW-cloud-G07.md` | update evidence | all |
|
||||
|
||||
## Final Verification
|
||||
|
||||
1. `go version && go env GOMOD`
|
||||
2. `go test -count=1 ./packages/go/execution ./packages/go/streamgate ./packages/go/config`
|
||||
3. `go test -count=1 ./packages/go/execution ./apps/node/...`
|
||||
4. `go test -count=10 ./packages/go/execution ./apps/node/internal/node`
|
||||
5. `go test -race -count=3 ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/adapters/ollama ./apps/node/internal/adapters/vllm ./apps/node/internal/adapters/openai_compat`
|
||||
6. `go vet ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/adapters/ollama ./apps/node/internal/adapters/vllm ./apps/node/internal/adapters/openai_compat`
|
||||
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`
|
||||
11. `git diff --check`
|
||||
|
||||
Record command, exit status, and concise output in the review stub. External provider smoke is intentionally excluded.
|
||||
|
||||
**After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`.**
|
||||
|
|
@ -0,0 +1,161 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/03+02_health_classification plan=1 tag=API milestone-task=health-classification -->
|
||||
|
||||
# Code Review Reference - 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-03
|
||||
task=m-node-provider-execution-liveness-recovery/03+02_health_classification, plan=1, tag=API
|
||||
|
||||
## Archive Evidence Snapshot
|
||||
|
||||
- Prior plan: `agent-task/m-node-provider-execution-liveness-recovery/03+02_health_probe_contract/plan_cloud_G08_0.log`.
|
||||
- Prior review stub: `agent-task/m-node-provider-execution-liveness-recovery/03+02_health_probe_contract/code_review_cloud_G08_0.log`.
|
||||
- Prior verdict: none; implementation and implementation-owned evidence had not started.
|
||||
- Required carryover: adapter transport/HTTP/decode errors are inconclusive, not unavailable; expose them and consume a typed fail-closed classifier before terminal enrichment.
|
||||
|
||||
## 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-G08.md` → `code_review_cloud_G08_0.log` and `PLAN-cloud-G08.md` → `plan_cloud_G08_0.log`.
|
||||
3. If PASS, write `complete.log` and move active task directory to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/03+02_health_classification/`. If WARN/FAIL, fully write the next filesystem state required by the code-review skill.
|
||||
4. If PASS, preserve first-line `milestone-task=health-classification` metadata in `complete.log` and report it for runtime aggregation. 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 |
|
||||
|------|---------|
|
||||
| API-1 — bounded target-aware classifier | [ ] |
|
||||
| API-2 — connection sequence and terminal enrichment | [ ] |
|
||||
| TEST-1 — classification/sequence/isolation evidence | [ ] |
|
||||
| DOC-1 — Node health evidence contracts | [ ] |
|
||||
|
||||
## Implementation Checklist
|
||||
|
||||
- [ ] [API-1] Add an independent bounded target-aware probe classifier with fail-closed unknown semantics.
|
||||
- [ ] [API-2] Generate connection-scoped monotonic observation sequence and enrich both stall terminal variants atomically.
|
||||
- [ ] [TEST-1] Verify all probe outcomes, sequence scope, identity evidence, terminal invariants, and no progress reset deterministically.
|
||||
- [ ] [DOC-1] Update the matching execution spec and execution/Edge-Node wire contracts for Node-produced health evidence and the explicit Edge ownership exclusion.
|
||||
- [ ] Run every command in Final Verification and record exact results in `CODE_REVIEW-cloud-G08.md`.
|
||||
- [ ] 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.
|
||||
|
||||
- [ ] Append one verdict of `PASS`, `WARN`, or `FAIL` and verified `review_rework_count`, `evidence_integrity_failure` to `Code Review Result`.
|
||||
- [ ] Verify that verdict, `Dimension Assessment`, and Required/Suggested/Nit classifications match.
|
||||
- [ ] Archive active `CODE_REVIEW-*-G??.md` to `code_review_cloud_G08_0.log`.
|
||||
- [ ] Archive active `PLAN-*-G??.md` to `plan_cloud_G08_0.log`.
|
||||
- [ ] Verify that the Agent-Ops managed block in `.gitignore` unignores `agent-task/**/*.md` and `agent-task/**/*.log` and ignores `agent-roadmap/current.md`.
|
||||
- [ ] If PASS, write `complete.log` based on `agent-ops/skills/common/code-review/templates/complete-log-template.md` and leave no active `.md` files.
|
||||
- [ ] If PASS, move active task directory `agent-task/m-node-provider-execution-liveness-recovery/03+02_health_classification/` to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/03+02_health_classification/` and update this checklist at the final archive path.
|
||||
- [ ] If PASS and task group is `m-node-provider-execution-liveness-recovery`, preserve and report `milestone-task` metadata for runtime aggregation, without modifying roadmap or directly calling `update-roadmap`.
|
||||
- [ ] 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
|
||||
|
||||
_Record any deviations from the plan and the rationale here._
|
||||
|
||||
## Key Design Decisions
|
||||
|
||||
_Record key design decisions here._
|
||||
|
||||
## Reviewer Checkpoints
|
||||
|
||||
- Verify the `02+01_stall_watchdog` predecessor completion log is PASS; its declared `+01` dependency supplies the activity contract, and this implementation enriches rather than re-owns timer/fence behavior.
|
||||
- Confirm the probe context is independent from canceled execution context, bounded, exact-target-aware, and never calls observer progress/reset.
|
||||
- Confirm available -> request-stalled, only a valid exact-target unavailable/absent result -> provider-unhealthy, and network/HTTP/decode/unsupported/timeout/error/unknown/identity-inconclusive -> health-unknown.
|
||||
- Confirm returned provider metadata cannot override Node-owned adapter/target identity or leak raw provider detail.
|
||||
- Confirm one shared atomic sequence per Session, unique under concurrent run/tunnel observations, reset on a new Session, and omitted for nil-session internal calls.
|
||||
- Confirm terminal/fence count, retryable behavior, and late-event fencing remain unchanged from the predecessor.
|
||||
- Confirm contracts explicitly leave reception-generation binding, stale validation, Edge health overlay, recovery, and retry to later milestone tasks.
|
||||
|
||||
## Verification Results
|
||||
|
||||
### `go version && go env GOMOD`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `go test -count=1 ./packages/go/execution ./packages/go/streamgate ./packages/go/config`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `go test -count=1 ./packages/go/execution ./apps/node/...`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `go test -count=1 ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/transport ./apps/node/internal/adapters/ollama ./apps/node/internal/adapters/vllm ./apps/node/internal/adapters/openai_compat`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `go test -count=10 ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/transport`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `go test -race -count=3 ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/transport ./apps/node/internal/adapters/ollama ./apps/node/internal/adapters/vllm ./apps/node/internal/adapters/openai_compat`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `go vet ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/transport ./apps/node/internal/adapters/ollama ./apps/node/internal/adapters/vllm ./apps/node/internal/adapters/openai_compat`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `go test -count=1 ./...`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `./scripts/e2e-smoke.sh`
|
||||
|
||||
_Implementing agent: record exit status and concise output, or the exact environment-only blocker._
|
||||
|
||||
### `IOP_DEV_RECONNECT_BIND_TIMEOUT=45 ./scripts/dev/edge-node-reconnect-diagnostic.sh`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `make readability-audit`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `git diff --check`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
---
|
||||
|
||||
> **[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 |
|
||||
|
|
@ -0,0 +1,310 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/03+02_health_classification plan=1 tag=API milestone-task=health-classification -->
|
||||
|
||||
# PLAN — Node Stall Health Classification
|
||||
|
||||
## For the Implementing Agent
|
||||
|
||||
> **MANDATORY:** Do not begin until the dependency log below exists and is PASS. Implement only this checklist, preserve unrelated user changes, and keep every edit inside the `health-classification` slice. Do not update roadmap state, create follow-up plans, commit, push, or run an official code review. After implementation, fill every implementation-owned section of `CODE_REVIEW-cloud-G08.md` and leave active files in place for the review agent.
|
||||
|
||||
## Background
|
||||
|
||||
The watchdog predecessor deliberately terminates stalled attempts with `provider_health=unknown`. The approved SDD requires Node to separate request liveness from provider health by probing the exact stalled adapter/target in an independent bounded context. Available, unavailable, unsupported, error, and timeout outcomes must map to a stable three-way classification, and evidence must carry adapter/target plus a monotonic sequence scoped to the current transport connection. Edge binding validation and runtime health overlay remain the next Epic's responsibility.
|
||||
|
||||
This semantic replan corrects a fail-open source-contract defect before implementation. The Ollama, vLLM, and OpenAI-compatible probers currently convert endpoint, HTTP, and decode failures into a normal unavailable result, which would let the liveness classifier mislabel inconclusive transport evidence as provider-wide unhealthy. The owning adapter contract must expose those errors, stable classification values must live in the shared execution package, and the terminal integration must consume only the typed normalized outcome. The pair remains unstarted and is then refined once into a probe-contract child and a dependent evidence-integration child.
|
||||
|
||||
## Dependencies
|
||||
|
||||
- `agent-task/m-node-provider-execution-liveness-recovery/02+01_stall_watchdog/complete.log`
|
||||
|
||||
At plan creation the predecessor is active. Its own `+01` dependency guarantees that `01_activity_contract` has already passed before this task can start. The implementing runtime must wait for `02+01_stall_watchdog` PASS completion and extend its final APIs; it must not copy anticipated structs from this plan if predecessor review changed names while preserving the contract.
|
||||
|
||||
## 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-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`
|
||||
- `packages/go/execution/types.go`
|
||||
- `apps/node/internal/node/command_handler.go`
|
||||
- `apps/node/internal/node/node.go`
|
||||
- `apps/node/internal/node/run_handler.go`
|
||||
- `apps/node/internal/node/tunnel_handler.go`
|
||||
- `apps/node/internal/transport/session.go`
|
||||
- `apps/node/internal/transport/session_test.go`
|
||||
- `apps/node/internal/adapters/mock/mock.go`
|
||||
- `apps/node/internal/adapters/ollama/ollama.go`
|
||||
- `apps/node/internal/adapters/ollama/provider.go`
|
||||
- `apps/node/internal/adapters/vllm/provider.go`
|
||||
- `apps/node/internal/adapters/openai_compat/provider.go`
|
||||
- `apps/node/internal/adapters/ollama/ollama_test.go`
|
||||
- `apps/node/internal/adapters/vllm/vllm_test.go`
|
||||
- `apps/node/internal/adapters/openai_compat/capabilities_test.go`
|
||||
- `apps/node/internal/node/run_cancel_test.go`
|
||||
- `apps/node/internal/node/provider_tunnel_test.go`
|
||||
- `apps/node/internal/node/node_test_support_test.go`
|
||||
- `agent-contract/inner/execution-runtime.md`
|
||||
- `agent-contract/inner/edge-node-runtime-wire.md`
|
||||
|
||||
### SDD Criteria
|
||||
|
||||
- SDD status: approved, D01 resolved, implementation lock released.
|
||||
- Decision D01: this slice owns Node-side probe classification and evidence generation only; Edge runtime health overlay binding, unhealthy/recovery application, and stale evidence rejection belong to `failure-handoff` (`SDD.md:78-80,125`).
|
||||
- Scenario: S03 / milestone task `health-classification` (`SDD.md:94`).
|
||||
- Evidence row: available/unavailable/unsupported/timeout target prober fixtures, request/provider separation, adapter/target/observation sequence, and proof that probe does not reset original progress (`SDD.md:105`).
|
||||
- Input/output: independent bounded `ProviderProber` context (`SDD.md:72`), three-way provider health and safe metadata (`SDD.md:75-78`).
|
||||
|
||||
### Verification Context
|
||||
|
||||
- Baseline Node, transport, execution, and race suites passed before plan creation.
|
||||
- Existing adapters already optionally implement `execution.ProviderProber`; no new network client or provider-specific interface is needed.
|
||||
- Tests must use injected probe functions/manual clocks and local fixtures only. They must not call real provider endpoints.
|
||||
- A package-private five-second probe ceiling is an implementation bound, not a new external config surface. Tests inject a shorter/manual bound without sleeping.
|
||||
|
||||
### Test Coverage Gaps
|
||||
|
||||
- `command_handler.go:63-74` probes capabilities but maps every probe error to unavailable and uses the caller context; that behavior is not safe to reuse for liveness evidence.
|
||||
- There is no independent probe coordinator or typed request-stalled/provider-unhealthy/health-unknown mapping.
|
||||
- `Session` has no connection-scoped health observation counter.
|
||||
- Watchdog terminals carry only the predecessor's unknown fallback and cannot distinguish unavailable from inconclusive probes.
|
||||
- No test proves that a successful probe does not reset or revive the stalled request.
|
||||
|
||||
### Symbol References
|
||||
|
||||
- `packages/go/execution/types.go:67-84,142-157` — status normalization and optional target-aware `ProviderProber`.
|
||||
- `apps/node/internal/node/command_handler.go:49-75` — existing capabilities probe, useful only as an adapter-interface reference.
|
||||
- `apps/node/internal/node/node.go:18-65` — Node dependencies and test injection point.
|
||||
- `apps/node/internal/transport/session.go:153-225` — per-connection state/lifetime boundary.
|
||||
- `apps/node/internal/adapters/mock/mock.go:33-43` — deterministic available probe behavior.
|
||||
- predecessor `liveness_watchdog.go` — terminal metadata/fence hook to enrich, with timer ownership left intact.
|
||||
|
||||
### Split Judgment
|
||||
|
||||
- Classification: large. It adds an external provider side effect, bounded temporal state, connection-scoped concurrency, and shared evidence across normalized/tunnel variants.
|
||||
- Refinement decision: split once. Adapter/prober error semantics plus a typed pure outcome classifier have an independently testable contract boundary; connection sequencing and watchdog terminal enrichment depend on that boundary and retain the temporal/concurrency work.
|
||||
- Dependency: `02+01_stall_watchdog` completion is mandatory and encoded by `03+02`; its transitive `+01` dependency preserves the activity-contract ordering without adding an undeclared direct dependency.
|
||||
- Collision check: no other active plan claimed this task id. Overlap with predecessor handler/watchdog files is intentionally serialized by completion dependencies.
|
||||
|
||||
### Scope Rationale
|
||||
|
||||
- In scope: make supported probers return endpoint/HTTP/decode errors instead of manufacturing unavailable, add stable shared classification constants and a pure fail-closed mapper, then consume that contract in one independent bounded target probe with safe adapter/target evidence, connection-scoped monotonic sequence, and normalized/tunnel terminal enrichment.
|
||||
- Out of scope: Edge registry generation binding, runtime health overlay, provider candidate exclusion/recovery, ingress retry, health recovery polling, metrics/ops evidence, and config knobs for probe timeout.
|
||||
- Probe results never alter the original observer's last-progress time, fence, cancellation result, or terminal count.
|
||||
|
||||
### Final Routing
|
||||
|
||||
- `evaluation_mode=isolated-reassessment`; `finalizer=finalize-task-policy.sh`, `finalizer_mode=pair`.
|
||||
- Build closure: scope/context/verification/evidence/ownership/decision all closed and trusted; capability gap none.
|
||||
- Build score: `scope=2`, `state=2`, `blast=1`, `evidence=1`, `verification=2` -> G08; `base_route_basis=local-fit`, `route_basis=risk-boundary`, lane `cloud`, file `PLAN-cloud-G08.md`.
|
||||
- Build signals: `large_indivisible_context=false`, positive loop risks `temporal_state`, `concurrent_consistency`, `boundary_contract`, `variant_product` (`count=4`), `review_rework_count=0`, `evidence_integrity_failure=false`; risk boundary matched, recovery boundary not matched.
|
||||
- Review closure: scope/context/verification/evidence/ownership/decision all closed and trusted; capability gap none.
|
||||
- Review score: `scope=2`, `state=2`, `blast=1`, `evidence=1`, `verification=2` -> G08; `route_basis=official-review`, lane `cloud`, adapter/model `codex/gpt-5.6-sol`, reasoning `xhigh`, file `CODE_REVIEW-cloud-G08.md`.
|
||||
|
||||
## Implementation Checklist
|
||||
|
||||
- [ ] [API-1] Add an independent bounded target-aware probe classifier with fail-closed unknown semantics.
|
||||
- [ ] [API-2] Generate connection-scoped monotonic observation sequence and enrich both stall terminal variants atomically.
|
||||
- [ ] [TEST-1] Verify all probe outcomes, sequence scope, identity evidence, terminal invariants, and no progress reset deterministically.
|
||||
- [ ] [DOC-1] Update the matching execution spec and execution/Edge-Node wire contracts for Node-produced health evidence and the explicit Edge ownership exclusion.
|
||||
- [ ] Run every command in Final Verification and record exact results in `CODE_REVIEW-cloud-G08.md`.
|
||||
- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.
|
||||
|
||||
### [API-1] Classify a bounded probe independently of the stalled request
|
||||
|
||||
**Problem**
|
||||
|
||||
The existing capabilities command calls `ProviderProber` in its request context and treats errors as unavailable (`apps/node/internal/node/command_handler.go:63-74`). A stalled-request classifier must not inherit canceled request context or promote unsupported/timeouts/errors to provider-wide unhealthy.
|
||||
|
||||
**Solution**
|
||||
|
||||
Add stable provider-health/liveness-classification values and a pure fail-closed outcome normalizer to the predecessor-created `packages/go/execution/liveness.go`. Update the Ollama, vLLM, and OpenAI-compatible probers so endpoint/network, non-success HTTP, and decode failures return an error; reserve a normal unavailable result for a valid exact-target absence.
|
||||
|
||||
Add `apps/node/internal/node/health_probe.go` with a package-private classifier:
|
||||
|
||||
- capture the resolved adapter and exact target from the stalled attempt;
|
||||
- if the adapter does not implement `ProviderProber`, return `provider_health=unknown` and `liveness_classification=health_unknown` without a network call;
|
||||
- otherwise run `ProbeProvider` in a new context rooted independently from the canceled request, bounded by a package-private `5 * time.Second` ceiling. Inject the context/timeout hook for deterministic tests; do not add a config or protobuf field;
|
||||
- after the call, check the probe context deadline/cancellation before interpreting the adapter result. Timeout/cancel, returned error, unknown/unrecognized status, or identity-inconclusive result maps to unknown;
|
||||
- a valid available result maps to `provider_health=available`, `liveness_classification=request_stalled`; a valid unavailable result—including an exact target reported absent/unserved—maps to `provider_health=unavailable`, `liveness_classification=provider_unhealthy`;
|
||||
- validate/canonicalize returned adapter instance and target against the requested adapter/target. Do not let returned metadata override safe Node-owned identity or copy arbitrary provider detail into terminal metadata;
|
||||
- the probe result is evidence only. It cannot call the predecessor observer's activity/reset method, change attempt fence, or trigger retry.
|
||||
|
||||
Run fence close-wait and health probe concurrently after the stall claim so their independent bounds do not add serial latency; assemble terminal evidence only when both bounded results are known.
|
||||
|
||||
**Modified files**
|
||||
|
||||
- [ ] `packages/go/execution/liveness.go`
|
||||
- [ ] `packages/go/execution/liveness_test.go`
|
||||
- [ ] `apps/node/internal/node/health_probe.go`
|
||||
- [ ] `apps/node/internal/node/liveness_watchdog.go`
|
||||
- [ ] `apps/node/internal/adapters/ollama/provider.go`
|
||||
- [ ] `apps/node/internal/adapters/vllm/provider.go`
|
||||
- [ ] `apps/node/internal/adapters/openai_compat/provider.go`
|
||||
|
||||
**Test decision**
|
||||
|
||||
Required. Use function adapters for success, absent target, unavailable, unknown, error, unsupported, and timeout. No external endpoints.
|
||||
|
||||
**Verification**
|
||||
|
||||
- `go test -count=1 ./apps/node/internal/node`
|
||||
- `go test -race -count=1 ./apps/node/internal/node`
|
||||
|
||||
### [API-2] Sequence and attach safe evidence on the current connection
|
||||
|
||||
**Problem**
|
||||
|
||||
Node evidence must be ordered within a connection, but neither `Node` nor `Session` owns a connection-scoped monotonic counter. A process-global counter cannot give Edge the reset boundary required by the next Epic.
|
||||
|
||||
**Solution**
|
||||
|
||||
- Add an atomic `uint64` health-observation counter to `transport.Session` and an increment method used only when a stall health observation is finalized. A newly created/reconnected Session starts at zero; the first emitted observation is one. Session close does not reuse the object or counter.
|
||||
- The watchdog terminal builder requests exactly one sequence per finalized stall, after classification and before the exactly-once terminal send. Normalized and tunnel attempts on the same Session share the counter and therefore cannot duplicate or decrease values under concurrency.
|
||||
- Attach `health_observation_seq` as base-10 text plus Node-owned `adapter` and `target` to the same metadata map used by the predecessor's normalized Failure/tunnel ERROR. Preserve `run_id`, `attempt_id`, `idle_duration_ms`, `attempt_fence`, stable failure code, and retryable semantics.
|
||||
- For internal/nil-session calls where no connection boundary exists, omit the sequence instead of inventing a process generation; production listener paths must always supply the current Session.
|
||||
- Do not add connection generation, provider id binding, stale-sequence rejection, or runtime health mutation. The next Epic binds this evidence to its reception connection and immutable dispatch.
|
||||
|
||||
**Modified files**
|
||||
|
||||
- [ ] `apps/node/internal/transport/session.go`
|
||||
- [ ] `apps/node/internal/node/run_handler.go`
|
||||
- [ ] `apps/node/internal/node/tunnel_handler.go`
|
||||
- [ ] `apps/node/internal/node/liveness_watchdog.go`
|
||||
|
||||
**Test decision**
|
||||
|
||||
Required. Cover sequential and concurrent run/tunnel observations on one session, reset on a new session, nil-session omission, and exactly-one increment per terminal.
|
||||
|
||||
**Verification**
|
||||
|
||||
- `go test -count=1 ./apps/node/internal/transport ./apps/node/internal/node`
|
||||
- `go test -race -count=3 ./apps/node/internal/transport ./apps/node/internal/node`
|
||||
|
||||
### [TEST-1] Prove request/provider separation and evidence invariants
|
||||
|
||||
**Problem**
|
||||
|
||||
A happy-path available probe alone cannot prove fail-closed classification, target-awareness, sequence ownership, or that probe activity is isolated from the dead attempt.
|
||||
|
||||
**Solution**
|
||||
|
||||
Add focused tests with the predecessor's fake clock and controlled providers:
|
||||
|
||||
- available -> request-stalled; only a valid exact-target unavailable/absent result -> provider-unhealthy; network, HTTP, decode, unsupported, deadline, canceled probe, returned error, unknown status, and identity mismatch -> health-unknown;
|
||||
- probe receives the stalled adapter/target and a live independent context even though execution context is canceled;
|
||||
- advancing/completing the probe never resets the original idle timer, suppresses the stall terminal, changes confirmed/unconfirmed fence, or emits a provider progress event;
|
||||
- normalized and tunnel metadata contain only stable safe keys, the expected identity/classification, and increasing sequence; raw body, reasoning, prompt, credential, provider detail, and `recovery_eligible` are absent;
|
||||
- concurrent observations on one session are unique/monotonic as a set, and a fresh session begins at one;
|
||||
- each surface still emits exactly one terminal and late provider emissions remain fenced.
|
||||
|
||||
Reuse existing Node transport fixtures and add focused local HTTP-fixture assertions for each supported adapter; never call a live provider.
|
||||
|
||||
**Modified files**
|
||||
|
||||
- [ ] `apps/node/internal/node/health_probe_test.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_test.go`
|
||||
- [ ] `apps/node/internal/adapters/ollama/ollama_test.go`
|
||||
- [ ] `apps/node/internal/adapters/vllm/vllm_test.go`
|
||||
- [ ] `apps/node/internal/adapters/openai_compat/capabilities_test.go`
|
||||
|
||||
**Test decision**
|
||||
|
||||
Required; this is the S03 evidence set.
|
||||
|
||||
**Verification**
|
||||
|
||||
- `go test -count=10 ./apps/node/internal/node ./apps/node/internal/transport`
|
||||
- `go test -race -count=3 ./apps/node/internal/node ./apps/node/internal/transport`
|
||||
|
||||
### [DOC-1] Document Node evidence and preserve the Edge boundary
|
||||
|
||||
**Problem**
|
||||
|
||||
After implementation the contracts must distinguish request stall from provider health and state exactly what the connection-scoped sequence does—and does not—authorize.
|
||||
|
||||
**Solution**
|
||||
|
||||
Update the matching execution spec and both contracts with:
|
||||
|
||||
- independent bounded probe input and three-way classification;
|
||||
- fail-closed unknown cases and exact target/adapter identity rules;
|
||||
- one connection-scoped monotonically increasing sequence, reset on new Session;
|
||||
- identical normalized/tunnel metadata and raw/secret exclusions;
|
||||
- an explicit statement that probe success is not original-request progress and does not imply retry;
|
||||
- an explicit ownership boundary: Edge reception-generation binding, immutable dispatch validation, stale evidence rejection, runtime health overlay, recovery, and candidate selection remain `failure-handoff`/later work.
|
||||
|
||||
Do not document Edge projection as implemented and do not modify roadmap/spec status.
|
||||
|
||||
**Modified files**
|
||||
|
||||
- [ ] `agent-contract/inner/execution-runtime.md`
|
||||
- [ ] `agent-contract/inner/edge-node-runtime-wire.md`
|
||||
- [ ] `agent-spec/runtime/edge-node-execution.md`
|
||||
|
||||
**Test decision**
|
||||
|
||||
No separate doc test; official review maps statements to the S03 fixtures.
|
||||
|
||||
**Verification**
|
||||
|
||||
- `git diff --check`
|
||||
|
||||
## Modified Files Summary
|
||||
|
||||
| Path | Action | Checklist |
|
||||
|------|--------|-----------|
|
||||
| `packages/go/execution/liveness.go` | modify predecessor file | API-1 |
|
||||
| `packages/go/execution/liveness_test.go` | modify predecessor test | API-1, TEST-1 |
|
||||
| `apps/node/internal/node/health_probe.go` | add | API-1 |
|
||||
| `apps/node/internal/node/liveness_watchdog.go` | modify | API-1, API-2 |
|
||||
| `apps/node/internal/adapters/ollama/provider.go` | modify | API-1 |
|
||||
| `apps/node/internal/adapters/ollama/ollama_test.go` | modify | TEST-1 |
|
||||
| `apps/node/internal/adapters/vllm/provider.go` | modify | API-1 |
|
||||
| `apps/node/internal/adapters/vllm/vllm_test.go` | modify | TEST-1 |
|
||||
| `apps/node/internal/adapters/openai_compat/provider.go` | modify | API-1 |
|
||||
| `apps/node/internal/adapters/openai_compat/capabilities_test.go` | modify | TEST-1 |
|
||||
| `apps/node/internal/transport/session.go` | modify | API-2 |
|
||||
| `apps/node/internal/node/run_handler.go` | modify | API-2 |
|
||||
| `apps/node/internal/node/tunnel_handler.go` | modify | API-2 |
|
||||
| `apps/node/internal/node/health_probe_test.go` | add | TEST-1 |
|
||||
| `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-contract/inner/execution-runtime.md` | modify | DOC-1 |
|
||||
| `agent-contract/inner/edge-node-runtime-wire.md` | modify | DOC-1 |
|
||||
| `agent-spec/runtime/edge-node-execution.md` | modify | DOC-1 |
|
||||
| `agent-task/m-node-provider-execution-liveness-recovery/03+02_health_classification/CODE_REVIEW-cloud-G08.md` | update evidence | all |
|
||||
|
||||
## Final Verification
|
||||
|
||||
1. `go version && go env GOMOD`
|
||||
2. `go test -count=1 ./packages/go/execution ./packages/go/streamgate ./packages/go/config`
|
||||
3. `go test -count=1 ./packages/go/execution ./apps/node/...`
|
||||
4. `go test -count=1 ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/transport ./apps/node/internal/adapters/ollama ./apps/node/internal/adapters/vllm ./apps/node/internal/adapters/openai_compat`
|
||||
5. `go test -count=10 ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/transport`
|
||||
6. `go test -race -count=3 ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/transport ./apps/node/internal/adapters/ollama ./apps/node/internal/adapters/vllm ./apps/node/internal/adapters/openai_compat`
|
||||
7. `go vet ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/transport ./apps/node/internal/adapters/ollama ./apps/node/internal/adapters/vllm ./apps/node/internal/adapters/openai_compat`
|
||||
8. `go test -count=1 ./...`
|
||||
9. `./scripts/e2e-smoke.sh`
|
||||
10. `IOP_DEV_RECONNECT_BIND_TIMEOUT=45 ./scripts/dev/edge-node-reconnect-diagnostic.sh`
|
||||
11. `make readability-audit`
|
||||
12. `git diff --check`
|
||||
|
||||
Record exact results in the review stub. External provider smoke is intentionally excluded; local fixtures cover classification and the repository diagnostic covers a real Edge/Node process cycle without credentials.
|
||||
|
||||
**After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`.**
|
||||
|
|
@ -0,0 +1,118 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/04+03_health_evidence plan=0 tag=API milestone-task=health-classification -->
|
||||
|
||||
# Code Review Reference - API
|
||||
|
||||
> **[IMPLEMENTING AGENT — READ FIRST]** Fill every implementation-owned section after implementation. Do not append a verdict, archive files, write `complete.log`, commit, push, or change roadmap state.
|
||||
|
||||
## Overview
|
||||
|
||||
date=2026-08-03
|
||||
task=m-node-provider-execution-liveness-recovery/04+03_health_evidence, plan=0, tag=API
|
||||
|
||||
## For the Review Agent
|
||||
|
||||
> **[REVIEW AGENT ONLY]** Compare every item to source and recorded output. Append a verdict only during later official review.
|
||||
|
||||
1. On later review, archive this file to `code_review_cloud_G08_0.log` and the plan to `plan_cloud_G08_0.log`.
|
||||
2. PASS finalization preserves `milestone-task=health-classification`; roadmap aggregation remains owned by `sync-milestone-workstate`.
|
||||
|
||||
## Implementation Item Completion
|
||||
|
||||
| Item | Status |
|
||||
|------|---------|
|
||||
| API-1 — Session observation sequence | [ ] |
|
||||
| API-2 — terminal evidence integration | [ ] |
|
||||
| TEST-1 — temporal/wire evidence | [ ] |
|
||||
| DOC-1 — Node evidence boundary | [ ] |
|
||||
|
||||
## Implementation Checklist
|
||||
|
||||
- [ ] [API-1] Add one connection-scoped atomic health-observation sequence source.
|
||||
- [ ] [API-2] Enrich the already-claimed normalized and tunnel stall terminals after bounded probe/fence completion.
|
||||
- [ ] [TEST-1] Prove sequence scope, concurrency, terminal invariants, metadata parity, and no progress reset.
|
||||
- [ ] [DOC-1] Update execution/wire contracts and the matching execution spec without claiming Edge projection.
|
||||
- [ ] Run every command in Final Verification and record exact results in `CODE_REVIEW-cloud-G08.md`.
|
||||
- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.
|
||||
|
||||
## Review-Only Checklist
|
||||
|
||||
> **[REVIEW AGENT ONLY]** Implementing agents must leave this section unchanged.
|
||||
|
||||
- [ ] Append exactly one PASS/WARN/FAIL verdict with routing signals.
|
||||
- [ ] Verify evidence and dimension assessment match the verdict.
|
||||
- [ ] Archive active files to `code_review_cloud_G08_0.log` and `plan_cloud_G08_0.log`.
|
||||
- [ ] Verify the Agent-Ops managed `.gitignore` block.
|
||||
- [ ] If PASS, write `complete.log`, preserve milestone metadata, and archive this task directory.
|
||||
- [ ] If WARN/FAIL, write only the next state required by the code-review skill.
|
||||
|
||||
## Deviations from Plan
|
||||
|
||||
_Implementing agent: replace with actual deviations or `None`._
|
||||
|
||||
## Key Design Decisions
|
||||
|
||||
_Implementing agent: record actual decisions._
|
||||
|
||||
## Reviewer Checkpoints
|
||||
|
||||
- Verify `03+02_health_probe_contract` is PASS and this pair consumes its reviewed typed outcome.
|
||||
- Confirm one atomic sequence belongs to each Session, starts at one, is unique/monotonic across concurrent run/tunnel observations, resets on new Session, and is omitted when unbound.
|
||||
- Confirm fixed close-grace and bounded probe run concurrently after stall claim rather than adding serial latency.
|
||||
- Confirm available never resets progress or revives the attempt and every path still emits exactly one terminal.
|
||||
- Confirm one allowlisted map is cloned into normalized Failure, normalized RuntimeEvent, and tunnel ERROR metadata and survives protobuf conversion.
|
||||
- Confirm fence/retryability, late-output drop, admission/run/credential ownership, and deadline/disconnect precedence are unchanged.
|
||||
- Confirm no Edge overlay, stale binding, candidate selection, retry, recovery, raw provider detail, or secret is introduced.
|
||||
|
||||
## Verification Results
|
||||
|
||||
### `go version && go env GOMOD`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `go test -count=1 ./packages/go/execution ./packages/go/streamgate ./packages/go/config`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `go test -count=1 ./packages/go/execution ./apps/node/...`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `go test -count=10 ./apps/node/internal/node ./apps/node/internal/transport`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `go test -race -count=3 ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/transport`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `go vet ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/transport`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `go test -count=1 ./...`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `./scripts/e2e-smoke.sh`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `IOP_DEV_RECONNECT_BIND_TIMEOUT=45 ./scripts/dev/edge-node-reconnect-diagnostic.sh`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `make readability-audit`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
### `git diff --check`
|
||||
|
||||
_Implementing agent: record exit status and concise output._
|
||||
|
||||
## Section Ownership
|
||||
|
||||
| Section | Owner |
|
||||
|---------|-------|
|
||||
| Header, overview, checklist item text, reviewer checkpoints, verification headings | Fixed at stub creation |
|
||||
| Item/checklist status, deviations, decisions, verification output | Implementing agent |
|
||||
| Review-only checklist and verdict/finalization | Review agent only |
|
||||
|
|
@ -0,0 +1,165 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/04+03_health_evidence plan=0 tag=API milestone-task=health-classification -->
|
||||
|
||||
# PLAN — Node Health Observation Evidence
|
||||
|
||||
## For the Implementing Agent
|
||||
|
||||
> **MANDATORY:** Do not begin until the dependency below has a PASS `complete.log`. Implement only this checklist, preserve unrelated user changes, and keep edits inside the terminal-evidence slice. Do not update roadmap state, create follow-up plans, commit, push, or run an official code review. Fill `CODE_REVIEW-cloud-G08.md` after implementation and leave active files in place.
|
||||
|
||||
## Background
|
||||
|
||||
The predecessor watchdog owns the one stall terminal, cancel/close fence, and safe metadata authority. The refined probe-contract child owns exact-target, fail-closed health classification. This dependent child joins those two established boundaries: it sequences finalized observations within the current transport Session and enriches the already-claimed normalized and tunnel terminals only after bounded probe and fence results are both known.
|
||||
|
||||
It must not reset progress, extend the watchdog deadline, change confirmed/unconfirmed fence meaning, revive output, retry, or project health at Edge.
|
||||
|
||||
## Dependencies
|
||||
|
||||
- `agent-task/m-node-provider-execution-liveness-recovery/03+02_health_probe_contract/complete.log`
|
||||
|
||||
The `+03` dependency transitively includes `02+01_stall_watchdog` and `01_activity_contract`. Consume reviewed APIs rather than anticipated names.
|
||||
|
||||
## Analysis
|
||||
|
||||
### Files Read
|
||||
|
||||
- `AGENTS.md`
|
||||
- target Milestone and approved liveness SDD
|
||||
- `agent-spec/runtime/edge-node-execution.md`
|
||||
- `agent-contract/inner/execution-runtime.md`
|
||||
- `agent-contract/inner/edge-node-runtime-wire.md`
|
||||
- `apps/node/internal/transport/session.go`
|
||||
- `apps/node/internal/transport/session_test.go`
|
||||
- `apps/node/internal/node/run_handler.go`
|
||||
- `apps/node/internal/node/tunnel_handler.go`
|
||||
- predecessor-planned `apps/node/internal/node/liveness_watchdog.go`
|
||||
- `apps/node/internal/node/runtime_bridge.go`
|
||||
- `apps/node/internal/node/run_cancel_test.go`
|
||||
- `apps/node/internal/node/provider_tunnel_test.go`
|
||||
- `scripts/dev/edge-node-reconnect-diagnostic.sh`
|
||||
|
||||
### SDD Criteria
|
||||
|
||||
- S03 requires identical safe normalized/tunnel evidence with adapter, exact target, and a monotonic observation sequence scoped to the current connection.
|
||||
- Probe success is not progress on the original request. Classification must not alter terminal count, fence, retryability, or cleanup.
|
||||
- Edge reception-generation binding, stale rejection, runtime overlay, recovery, and selection remain later Epic ownership.
|
||||
|
||||
### Verification Context
|
||||
|
||||
- Fake clock and channel-controlled predecessor fixtures own temporal assertions; no wall-clock sleeps.
|
||||
- One local real-process full cycle is required in addition to the auxiliary test-only E2E script.
|
||||
- No external provider, credentials, migrations, or deployments are required.
|
||||
|
||||
### Test Coverage Gaps
|
||||
|
||||
- Session has no observation counter reset boundary.
|
||||
- The predecessor terminal retains unknown health and has no sequence.
|
||||
- Concurrency tests do not prove unique sequence values across normalized and tunnel attempts.
|
||||
- No assertion combines bounded probe and close-fence completion without resetting progress or losing identical wire metadata.
|
||||
|
||||
### Split Judgment
|
||||
|
||||
- Large/indivisible: Session sequencing, concurrent probe/fence join, exactly-once terminal authority, and normalized/tunnel variants are one temporal consistency boundary.
|
||||
- This is the second and final child from one refine-plans application; no further split has an independent PASS state.
|
||||
- Write overlap with both predecessors is serialized by the explicit dependency chain.
|
||||
|
||||
### Scope Rationale
|
||||
|
||||
- In scope: Session counter, one sequence per finalized observation, bounded concurrent result join, normalized/tunnel metadata enrichment, deterministic integration tests, contracts/spec.
|
||||
- Out of scope: adapter prober behavior, activity timer rules, fence/retry ownership changes, Edge overlay/generation binding, recovery, metrics, and configuration.
|
||||
|
||||
### Final Routing
|
||||
|
||||
- `evaluation_mode=isolated-reassessment`; pair finalizer.
|
||||
- Build score `scope=2,state=2,blast=1,evidence=1,verification=2` -> cloud G08 `PLAN-cloud-G08.md` by risk boundary.
|
||||
- Loop risks: `temporal_state`, `concurrent_consistency`, `boundary_contract`, `variant_product` (`count=4`).
|
||||
- Review is official-review cloud G08 in `CODE_REVIEW-cloud-G08.md`.
|
||||
|
||||
## Implementation Checklist
|
||||
|
||||
- [ ] [API-1] Add one connection-scoped atomic health-observation sequence source.
|
||||
- [ ] [API-2] Enrich the already-claimed normalized and tunnel stall terminals after bounded probe/fence completion.
|
||||
- [ ] [TEST-1] Prove sequence scope, concurrency, terminal invariants, metadata parity, and no progress reset.
|
||||
- [ ] [DOC-1] Update execution/wire contracts and the matching execution spec without claiming Edge projection.
|
||||
- [ ] Run every command in Final Verification and record exact results in `CODE_REVIEW-cloud-G08.md`.
|
||||
- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.
|
||||
|
||||
### [API-1] Connection-scoped observation sequence
|
||||
|
||||
Add an atomic `uint64` counter to `transport.Session`. A new Session starts at zero and its first finalized health observation receives one. Normalized and tunnel attempts on the same Session share the source and receive unique, monotonically increasing values under concurrency. Increment exactly once after classification and before terminal send. Internal/nil or unbound Session paths omit `health_observation_seq`; never invent a process-global generation.
|
||||
|
||||
**Modified files**
|
||||
|
||||
- [ ] `apps/node/internal/transport/session.go`
|
||||
- [ ] `apps/node/internal/transport/session_test.go`
|
||||
|
||||
**Test decision:** Required; sequential/concurrent increments, new-session reset, nil omission, and overflow policy are explicit.
|
||||
|
||||
### [API-2] Join bounded evidence without changing terminal ownership
|
||||
|
||||
After the predecessor claims a stall, run its fixed close-grace wait and the reviewed health probe concurrently. Wait only for both bounded outcomes; do not extend either bound serially. Then allocate one sequence and build one allowlisted metadata map containing stable failure/classification, idle duration, Node-owned run/attempt identity, fence, adapter, target, and optional sequence.
|
||||
|
||||
Use cloned maps for normalized `Failure.Metadata`, normalized `RuntimeEvent.Metadata`, and tunnel ERROR metadata so the existing protobuf mapper preserves the same values without shared mutable aliases. Preserve retryable as `attempt_fence == confirmed`. Provider availability never resets the observer, suppresses the terminal, changes the fence, or starts another attempt. Late provider output remains fenced.
|
||||
|
||||
**Modified files**
|
||||
|
||||
- [ ] `apps/node/internal/node/liveness_watchdog.go`
|
||||
- [ ] `apps/node/internal/node/run_handler.go`
|
||||
- [ ] `apps/node/internal/node/tunnel_handler.go`
|
||||
|
||||
**Test decision:** Required for both execution surfaces and every health/fence combination.
|
||||
|
||||
### [TEST-1] Temporal and wire evidence
|
||||
|
||||
Extend predecessor fixtures to prove: available/request-stalled, valid unavailable/provider-unhealthy, and every unknown branch; independent live probe context after request cancel; probe completion never resets progress; exactly one terminal; identical safe normalized domain/protobuf/tunnel maps; sequence uniqueness on one Session and reset on another; nil omission; unchanged confirmed/unconfirmed retryability and ownership; late output drop. Use manual clocks and channels only.
|
||||
|
||||
**Modified files**
|
||||
|
||||
- [ ] `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_test.go`
|
||||
|
||||
### [DOC-1] Evidence boundary
|
||||
|
||||
Document three-way health evidence, exact identity, connection-scoped sequence semantics, normalized/tunnel parity, secret/raw exclusions, and that probe success is not progress or retry authority. Explicitly leave reception-generation binding, stale validation, Edge health overlay, recovery, and selection to later work.
|
||||
|
||||
**Modified files**
|
||||
|
||||
- [ ] `agent-contract/inner/execution-runtime.md`
|
||||
- [ ] `agent-contract/inner/edge-node-runtime-wire.md`
|
||||
- [ ] `agent-spec/runtime/edge-node-execution.md`
|
||||
|
||||
## Modified Files Summary
|
||||
|
||||
| Path | Action | Checklist |
|
||||
|------|--------|-----------|
|
||||
| `apps/node/internal/transport/session.go` | modify | API-1 |
|
||||
| `apps/node/internal/transport/session_test.go` | modify | API-1, TEST-1 |
|
||||
| `apps/node/internal/node/liveness_watchdog.go` | modify predecessor file | API-2 |
|
||||
| `apps/node/internal/node/run_handler.go` | modify predecessor file | API-2 |
|
||||
| `apps/node/internal/node/tunnel_handler.go` | modify predecessor file | API-2 |
|
||||
| `apps/node/internal/node/liveness_watchdog_test.go` | modify predecessor test | TEST-1 |
|
||||
| `apps/node/internal/node/run_cancel_test.go` | modify | TEST-1 |
|
||||
| `apps/node/internal/node/provider_tunnel_test.go` | modify | TEST-1 |
|
||||
| `agent-contract/inner/execution-runtime.md` | modify | DOC-1 |
|
||||
| `agent-contract/inner/edge-node-runtime-wire.md` | modify | DOC-1 |
|
||||
| `agent-spec/runtime/edge-node-execution.md` | modify | DOC-1 |
|
||||
| `agent-task/m-node-provider-execution-liveness-recovery/04+03_health_evidence/CODE_REVIEW-cloud-G08.md` | update evidence | all |
|
||||
|
||||
## Final Verification
|
||||
|
||||
1. `go version && go env GOMOD`
|
||||
2. `go test -count=1 ./packages/go/execution ./packages/go/streamgate ./packages/go/config`
|
||||
3. `go test -count=1 ./packages/go/execution ./apps/node/...`
|
||||
4. `go test -count=10 ./apps/node/internal/node ./apps/node/internal/transport`
|
||||
5. `go test -race -count=3 ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/transport`
|
||||
6. `go vet ./packages/go/execution ./apps/node/internal/node ./apps/node/internal/transport`
|
||||
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`
|
||||
11. `git diff --check`
|
||||
|
||||
Record exact results in the review stub. A deterministic/race failure is a blocker; do not substitute live-provider smoke.
|
||||
|
||||
**After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`.**
|
||||
Loading…
Reference in a new issue