feat(epic): liveness-observer 작업을 준비한다
This commit is contained in:
parent
d76779559f
commit
cc8b23c742
11 changed files with 2336 additions and 1 deletions
|
|
@ -28,7 +28,7 @@ Node는 원 요청의 liveness와 provider 전체 health를 분리해 직접 점
|
|||
- [x] SDD 잠금이 해제되어 있다.
|
||||
- [x] SDD 사용자 리뷰가 없거나 승인/해결되었다.
|
||||
- [x] Acceptance Scenario가 Milestone 기능 Task와 연결되어 있다.
|
||||
- [x] Evidence Map이 완료 시 `Roadmap Completion`과 최종 검증 evidence로 검증 가능하게 연결되어 있다.
|
||||
- [x] Evidence Map이 완료 시 `milestone-task`가 보존된 `complete.log`, workstate sync 집계와 최종 검증 evidence로 검증 가능하게 연결되어 있다.
|
||||
- 결정 필요: 없음
|
||||
|
||||
## 범위
|
||||
|
|
|
|||
|
|
@ -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`.**
|
||||
|
|
@ -0,0 +1,134 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/01_activity_contract plan=0 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=0, tag=API
|
||||
|
||||
## 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_0.log` and `PLAN-local-G06.md` → `plan_local_G06_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/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_0.log`.
|
||||
- [ ] Archive active `PLAN-*-G??.md` to `plan_local_G06_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/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 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 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._
|
||||
|
||||
### `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,334 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/01_activity_contract plan=0 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.
|
||||
|
||||
## 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-test/local/rules.md`
|
||||
- `agent-test/local/node-smoke.md`
|
||||
- `agent-test/local/platform-common-smoke.md`
|
||||
- `agent-test/local/edge-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`
|
||||
- `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.
|
||||
- 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.
|
||||
|
||||
### 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=first-pass`; `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 and reject/guard unexpected negative mixed-version inputs rather than disabling the observer.
|
||||
- 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`
|
||||
- `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, 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. `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`
|
||||
4. `go test -race -count=1 ./packages/go/execution`
|
||||
5. `go test -count=1 ./...`
|
||||
6. `make readability-audit`
|
||||
7. `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`.**
|
||||
|
|
@ -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,136 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/02+01_stall_watchdog plan=0 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=0, tag=API
|
||||
|
||||
## 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/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_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/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.
|
||||
- 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,331 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/02+01_stall_watchdog plan=0 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.
|
||||
|
||||
## 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=first-pass`; `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`. Resolve `attempt_id` from request metadata when present and otherwise use the existing run identity; 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;
|
||||
- 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,134 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/03+02_health_classification plan=0 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=0, tag=API
|
||||
|
||||
## 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, unavailable/exact target absent -> provider-unhealthy, and 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 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,284 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/03+02_health_classification plan=0 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 slice adds the bounded classifier, connection sequence source, and terminal enrichment for both normalized and raw tunnel paths without changing timer, fence, retry, or Edge projection ownership.
|
||||
|
||||
## 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/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.
|
||||
- Cohesion: probe outcome mapping and observation sequence must be attached atomically to the already-claimed stall terminal; separating them would emit incomplete or reordered evidence.
|
||||
- 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: one independent bounded target probe after stall claim, available/unavailable/unknown mapping, 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, provider adapter behavior changes, 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=first-pass`; `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 `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**
|
||||
|
||||
- [ ] `apps/node/internal/node/node.go`
|
||||
- [ ] `apps/node/internal/node/health_probe.go`
|
||||
- [ ] `apps/node/internal/node/liveness_watchdog.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; unavailable/network target result and exact target absent -> provider-unhealthy; 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; add no adapter implementation changes.
|
||||
|
||||
**Modified files**
|
||||
|
||||
- [ ] `apps/node/internal/node/health_probe_test.go`
|
||||
- [ ] `apps/node/internal/node/liveness_watchdog_test.go`
|
||||
- [ ] `apps/node/internal/transport/session_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 |
|
||||
|------|--------|-----------|
|
||||
| `apps/node/internal/node/node.go` | modify | API-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/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/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 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 in the review stub. External provider smoke is intentionally excluded because deterministic probe fixtures cover the classification contract without requiring credentials or mutating provider state.
|
||||
|
||||
**After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`.**
|
||||
Loading…
Reference in a new issue