feat(epic): recovery-handoff 작업을 준비한다
This commit is contained in:
parent
56e7d78af3
commit
86d1f4db9f
16 changed files with 3143 additions and 0 deletions
|
|
@ -0,0 +1,228 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/05+04_failure_wire plan=1 tag=API milestone-task=failure-handoff -->
|
||||
|
||||
# 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-05
|
||||
task=m-node-provider-execution-liveness-recovery/05+04_failure_wire, plan=1, tag=API
|
||||
|
||||
## Archive Evidence Snapshot
|
||||
|
||||
- Predecessor: `agent-task/archive/2026/08/m-node-provider-execution-liveness-recovery/04+03_health_evidence/complete.log`; final verdict PASS, no remaining nits or follow-up.
|
||||
- Carryover: Node emits the stable health/fence metadata and connection-scoped monotonic `health_observation_seq`; normalized and tunnel paths passed focused, package, race, vet, repository, smoke, and reconnect verification.
|
||||
- Affected foundation: `packages/go/execution`, Node liveness mappers, Provider Execution Runtime contract, Edge-Node Runtime Wire contract, and the living execution spec.
|
||||
- Self-review source: `plan_local_G06_0.log` and `code_review_cloud_G06_0.log` in this task directory. They contain an unimplemented plan/stub pair and no official verdict, Required/Suggested/Nit finding, code change, or verification evidence.
|
||||
- Replan carryover: preserve the optional raw-free failure envelope scope. The prior pair omitted the repository-wide generated-consumer compile check and a deterministic Edge -> Node -> provider full-cycle, and its copied archive snapshot drifted between PLAN and review; this pair repairs those material evidence gaps.
|
||||
|
||||
|
||||
## 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-G07.md` → `code_review_cloud_G07_1.log` and `PLAN-local-G07.md` → `plan_local_G07_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/05+04_failure_wire/`. If WARN/FAIL, fully write the next filesystem state required by the code-review skill.
|
||||
4. If PASS and task group is `m-node-provider-execution-liveness-recovery`, preserve the first-line `milestone-task` metadata in `complete.log` and report it for the runtime aggregation event. Roadmap state evaluation belongs to `sync-milestone-workstate`.
|
||||
5. Check applicable `Review-Only Checklist` items at the final `.log` location before reporting.
|
||||
|
||||
---
|
||||
|
||||
## Implementation Item Completion
|
||||
|
||||
| Item | Status |
|
||||
|------|---------|
|
||||
| API-1: Add the typed failure wire model | [ ] |
|
||||
| API-2: Preserve typed failures across both Node paths | [ ] |
|
||||
|
||||
## Implementation Checklist
|
||||
|
||||
- [ ] API-1 adds one safe optional non-recursive failure message to the protobuf/runtime models and regenerates checked-in Go and Dart bindings without changing existing field numbers.
|
||||
- [ ] API-2 maps the typed failure on normalized and tunnel terminals, adds absent/present raw-free round-trip tests, and synchronizes the runtime/wire contracts and living spec.
|
||||
- [ ] Run protobuf generation, focused, repository/package, client, race, vet, provider-only smoke, fake-provider full-cycle, and diff verification commands and confirm fresh uncached PASS output.
|
||||
- [ ] 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_G07_1.log`.
|
||||
- [ ] Archive active `PLAN-*-G??.md` to `plan_local_G07_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/05+04_failure_wire/` to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/05+04_failure_wire/` 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 protobuf field numbers remain append-only, generated Go/Dart descriptors match, and optional absence preserves legacy clients.
|
||||
- Confirm normalized and tunnel `response_stalled` envelopes clone only stable allowlisted failure metadata and never include raw output or `recovery_eligible`.
|
||||
- Confirm focused round-trips, repository-wide consumers, provider smoke, and fake-provider full-cycle all pass with fresh output.
|
||||
|
||||
## Verification Results
|
||||
|
||||
> Implementing agent: run each command exactly as written and paste its actual stdout/stderr under `Output`. Record any replacement command and reason in `Deviations from Plan`.
|
||||
|
||||
### Verification 1
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
make proto && make proto-dart
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 2
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go test -count=1 ./apps/node/internal/node -run '^(TestRuntimeEventToProtoPreservesTypedFailure|TestStallMetadata.*)$' && go test -count=1 ./apps/edge/internal/transport -run '^(TestEdgeParserMap_.*)$'
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 3
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
make client-test
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 4
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go test -count=1 ./packages/go/execution ./apps/node/... ./apps/edge/internal/transport ./apps/control-plane/...
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 5
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go test -race -count=1 ./packages/go/execution ./apps/node/internal/node ./apps/edge/internal/transport
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 6
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go vet ./packages/go/execution ./apps/node/... ./apps/edge/internal/transport ./apps/control-plane/...
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 7
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go test -count=1 ./...
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 8
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
./scripts/e2e-smoke.sh
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 9
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
IOP_VLLM_MODE=fake ./scripts/e2e-openai-vllm.sh
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 10
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
git diff --check
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
---
|
||||
|
||||
> **[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,224 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/05+04_failure_wire plan=1 tag=API milestone-task=failure-handoff -->
|
||||
|
||||
# Typed Execution Failure Wire Contract
|
||||
|
||||
## For the Implementing Agent
|
||||
|
||||
Implement only the items below, run every verification command, and fill all implementation-owned sections of `CODE_REVIEW-cloud-G07.md` with actual notes and raw command output. Keep the active files in place and report ready for review; finalization belongs to the code-review skill. If blocked, record the exact blocker, attempted commands/output, and resume condition only in implementation-owned evidence fields. Do not ask the user, call user-input tools, create control-plane stop files, classify the next state, archive logs, or write `complete.log`.
|
||||
|
||||
## Background
|
||||
|
||||
Node already creates a typed `response_stalled` failure, but `RunEvent` serializes only its error string and `ProviderTunnelFrame` has no typed failure field. S04 therefore cannot preserve identical normalized/tunnel failure semantics without first establishing a backward-compatible wire foundation.
|
||||
|
||||
## Archive Evidence Snapshot
|
||||
|
||||
- Predecessor: `agent-task/archive/2026/08/m-node-provider-execution-liveness-recovery/04+03_health_evidence/complete.log`; final verdict PASS, no remaining nits or follow-up.
|
||||
- Carryover: Node emits the stable health/fence metadata and connection-scoped monotonic `health_observation_seq`; normalized and tunnel paths passed focused, package, race, vet, repository, smoke, and reconnect verification.
|
||||
- Affected foundation: `packages/go/execution`, Node liveness mappers, Provider Execution Runtime contract, Edge-Node Runtime Wire contract, and the living execution spec.
|
||||
- Self-review source: `plan_local_G06_0.log` and `code_review_cloud_G06_0.log` in this task directory. They contain an unimplemented plan/stub pair and no official verdict, Required/Suggested/Nit finding, code change, or verification evidence.
|
||||
- Replan carryover: preserve the optional raw-free failure envelope scope. The prior pair omitted the repository-wide generated-consumer compile check and a deterministic Edge -> Node -> provider full-cycle, and its copied archive snapshot drifted between PLAN and review; this pair repairs those material evidence gaps.
|
||||
|
||||
## Analysis
|
||||
|
||||
### Files Read
|
||||
|
||||
- `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.pbenum.dart`, `apps/client/lib/gen/proto/iop/runtime.pbjson.dart`, `apps/client/lib/gen/proto/iop/runtime.pbserver.dart`
|
||||
- `Makefile`, `agent-test/local/node-smoke.md`, `agent-test/local/edge-smoke.md`, `agent-test/local/client-smoke.md`, `agent-test/local/control-plane-smoke.md`
|
||||
- `packages/go/execution/failure.go`, `packages/go/execution/types.go`, `packages/go/execution/liveness.go`, `packages/go/execution/liveness_test.go`
|
||||
- `apps/node/internal/node/runtime_bridge.go`, `apps/node/internal/node/runtime_bridge_test.go`
|
||||
- `apps/node/internal/node/liveness_watchdog.go`, `apps/node/internal/node/liveness_health_evidence.go`, `apps/node/internal/node/liveness_health_evidence_test.go`, `apps/node/internal/node/provider_tunnel_liveness_test.go`
|
||||
- `apps/edge/internal/transport/server.go`, `apps/edge/internal/transport/connection_handlers.go`, `apps/edge/internal/transport/server_test.go`
|
||||
- `agent-contract/inner/execution-runtime.md`, `agent-contract/inner/edge-node-runtime-wire.md`
|
||||
- `agent-spec/runtime/edge-node-execution.md`
|
||||
|
||||
### SDD Criteria
|
||||
|
||||
- SDD: `agent-roadmap/sdd/operational-observability-provider-management/node-provider-execution-liveness-recovery/SDD.md`; status `[승인됨]`, and this pair's first-line id is `milestone-task=failure-handoff`.
|
||||
- Acceptance Scenario S04 and Evidence Map S04 require RunEvent/ProviderTunnelFrame round-trips to retain stable code, health, idle duration, attempt identity, fence, and sequence without raw output or Edge-owned `recovery_eligible`.
|
||||
- Those rows define API-1's common typed message and API-2's two-path round-trip tests plus contract/spec synchronization.
|
||||
|
||||
### Verification Context
|
||||
|
||||
- Handoff supplied target Milestone/Epic, allowed ids, active task group, and starting HEAD `56e7d78af3cda4a8d6a85af091ad26bce935f8b6`; checkout matched and worktree had no tracked/user changes.
|
||||
- Repository-native baseline passed: `go test -count=1 ./packages/go/execution ./apps/node/internal/node ./apps/edge/internal/transport ./apps/edge/internal/service ./apps/edge/internal/openai` with Go 1.26.2 and the repository `go.mod`.
|
||||
- Preconditions: archived predecessor PASS above; protobuf regeneration must use the repository's existing Go and Dart generation paths and checked-in output. `protoc` 29.3 and `protoc-gen-go` v1.36.11 are available, but `protoc-gen-dart` is not currently installed; before `make proto-dart`, install the repository-declared generator with `flutter pub global activate protoc_plugin`, then regenerate and verify the client bindings. No external runtime runner, host, port, or artifact is required.
|
||||
- Gap: no current protobuf round-trip asserts typed `ExecutionFailure`; confidence is high because both mapper boundaries and parser fixtures are local and deterministic. Repository-wide Go tests and the fake-vLLM full-cycle are required after generation so wire changes are not accepted on focused fixtures alone.
|
||||
|
||||
### Test Coverage Gaps
|
||||
|
||||
- Normalized failures: Node tests cover the in-memory typed failure but not protobuf preservation.
|
||||
- Tunnel failures: tests cover safe metadata and terminal ordering but the model has no typed failure to assert.
|
||||
- Compatibility: existing parser separation is covered, but the new optional fields need absent/present boundary cases.
|
||||
|
||||
### Symbol References
|
||||
|
||||
- No symbol is renamed or removed. New optional `Failure` fields add call sites only in Node mappers and tests.
|
||||
|
||||
### Split Judgment
|
||||
|
||||
- `05+04_failure_wire`: stable typed wire contract; predecessor `04+03_health_evidence` is satisfied by the archived PASS `complete.log` above.
|
||||
- `06+05_health_overlay`: consumes typed wire evidence and produces reception-fenced runtime health projection; waits for this subtask's `complete.log`.
|
||||
- `07+06_retry_candidate_policy`: consumes overlay availability and produces request-local avoid-provider selection; waits for `06+05_health_overlay`.
|
||||
- `08+07_stall_recovery`: consumes typed failure and candidate policy in the OpenAI StreamGate host; waits for `07+06_retry_candidate_policy`.
|
||||
|
||||
### Scope Rationale
|
||||
|
||||
This packet does not interpret failures at Edge, mutate provider health, release leases, select retry candidates, or dispatch recovery. Those responsibilities are deliberately assigned to 06-08 so this packet remains a compatibility-testable wire foundation.
|
||||
|
||||
### Final Routing
|
||||
|
||||
- `evaluation_mode=isolated-reassessment`; finalizer=`finalize-task-policy.sh pair`.
|
||||
- Build closure true, scores `(2,0,2,1,2)`, grade G07, route `local-fit` -> `PLAN-local-G07.md`.
|
||||
- Review closure true, scores `(2,0,2,1,2)`, grade G07, route `official-review` -> `CODE_REVIEW-cloud-G07.md` (`codex`, `gpt-5.6-sol`, `xhigh`).
|
||||
- `large_indivisible_context=false`; positive loop risk: `boundary_contract` (1). `review_rework_count=0`, `evidence_integrity_failure=false`; no capability gap.
|
||||
|
||||
## Implementation Checklist
|
||||
|
||||
- [ ] API-1 adds one safe optional non-recursive failure message to the protobuf/runtime models and regenerates checked-in Go and Dart bindings without changing existing field numbers.
|
||||
- [ ] API-2 maps the typed failure on normalized and tunnel terminals, adds absent/present raw-free round-trip tests, and synchronizes the runtime/wire contracts and living spec.
|
||||
- [ ] Run protobuf generation, focused, repository/package, client, race, vet, provider-only smoke, fake-provider full-cycle, and diff verification commands and confirm fresh uncached PASS output.
|
||||
- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.
|
||||
|
||||
### [API-1] Add the typed failure wire model
|
||||
|
||||
**Problem:** `proto/iop/runtime.proto:30-43` carries `RunEvent.error`/metadata but no typed failure, and `proto/iop/runtime.proto:145-163` plus `packages/go/execution/types.go:239-253` have the same gap for tunnels. The Node-owned `execution.Failure` is consequently flattened before Edge can apply S04.
|
||||
|
||||
**Solution:** Add an optional protobuf `ExecutionFailure` matching the existing non-recursive `execution.Failure` shape (`code`, safe `message`, `retryable`, metadata), then add optional fields to both terminal envelopes using new field numbers. Mirror the field on the in-memory tunnel type and regenerate both Go and Dart bindings. Do not invent a recursive cause that the source runtime model does not own.
|
||||
|
||||
Before (`proto/iop/runtime.proto:30`):
|
||||
|
||||
```proto
|
||||
message RunEvent {
|
||||
string run_id = 1;
|
||||
string type = 2;
|
||||
string delta = 3;
|
||||
string message = 4;
|
||||
string error = 5;
|
||||
Usage usage = 6;
|
||||
map<string, string> metadata = 7;
|
||||
int64 timestamp = 8;
|
||||
string session_id = 9;
|
||||
bool background = 10;
|
||||
string node_id = 11;
|
||||
string node_alias = 12;
|
||||
}
|
||||
```
|
||||
|
||||
After:
|
||||
|
||||
```proto
|
||||
message ExecutionFailure {
|
||||
string code = 1;
|
||||
string message = 2;
|
||||
bool retryable = 3;
|
||||
map<string, string> metadata = 4;
|
||||
}
|
||||
|
||||
message RunEvent {
|
||||
// fields 1-12 unchanged
|
||||
ExecutionFailure failure = 13;
|
||||
}
|
||||
|
||||
message ProviderTunnelFrame {
|
||||
// fields 1-14 unchanged
|
||||
ExecutionFailure failure = 15;
|
||||
}
|
||||
```
|
||||
|
||||
**Modified Files and Checklist:**
|
||||
|
||||
- [ ] `proto/iop/runtime.proto`: append the common message and optional envelope fields without renumbering.
|
||||
- [ ] `proto/gen/iop/runtime.pb.go`: regenerate through the repository protobuf workflow; do not hand-diverge descriptors.
|
||||
- [ ] `apps/client/lib/gen/proto/iop/runtime.pb.dart`: regenerate the Dart runtime message bindings.
|
||||
- [ ] `apps/client/lib/gen/proto/iop/runtime.pbenum.dart`: regenerate the Dart enum companion output.
|
||||
- [ ] `apps/client/lib/gen/proto/iop/runtime.pbjson.dart`: regenerate the Dart descriptor/JSON output.
|
||||
- [ ] `apps/client/lib/gen/proto/iop/runtime.pbserver.dart`: regenerate the Dart server companion output.
|
||||
- [ ] `packages/go/execution/types.go`: add the tunnel-side typed failure pointer with ownership comments.
|
||||
|
||||
**Test Strategy:** API-2 owns normal and boundary round-trips; existing generated-code compilation is also exercised by every package command.
|
||||
|
||||
**Verification:** `make proto && make proto-dart && go test -count=1 ./packages/go/execution ./apps/node/internal/node ./apps/edge/internal/transport && make client-test` must PASS after installing `protoc_plugin` if the preflight gap remains.
|
||||
|
||||
### [API-2] Preserve typed failures across both Node paths
|
||||
|
||||
**Problem:** `apps/node/internal/node/runtime_bridge.go:34-57` emits only `Failure.Message` as `RunEvent.error`, while `apps/node/internal/node/liveness_watchdog.go:476-515` copies tunnel error and metadata without typed code/retryability. Existing tests therefore cannot distinguish a confirmed `response_stalled` from an unrelated string error.
|
||||
|
||||
**Solution:** Introduce one clone-safe failure mapper used by normalized and tunnel conversion. It emits a typed protobuf failure only for `FailureCodeResponseStalled`, clones only the SDD's allowlisted liveness keys, and never forwards arbitrary `Failure.Metadata`; nil and every other failure code leave the optional field absent and retain the legacy error string. Prove present/absent protobuf round-trips contain no `recovery_eligible`, raw output, or arbitrary metadata.
|
||||
|
||||
Before (`apps/node/internal/node/liveness_watchdog.go:494`):
|
||||
|
||||
```go
|
||||
return &iop.ProviderTunnelFrame{
|
||||
RunId: frame.RunID, TunnelId: frame.TunnelID, Kind: protoKind,
|
||||
Error: frame.Error, Metadata: cloneStringMap(frame.Metadata),
|
||||
}
|
||||
```
|
||||
|
||||
After:
|
||||
|
||||
```go
|
||||
return &iop.ProviderTunnelFrame{
|
||||
RunId: frame.RunID, TunnelId: frame.TunnelID, Kind: protoKind,
|
||||
Error: frame.Error, Failure: executionFailureToProto(frame.Failure),
|
||||
Metadata: cloneStringMap(frame.Metadata),
|
||||
}
|
||||
```
|
||||
|
||||
**Modified Files and Checklist:**
|
||||
|
||||
- [ ] `apps/node/internal/node/runtime_bridge.go`: map only allowlisted `response_stalled` failures for normalized events while preserving legacy error fallback for every failure.
|
||||
- [ ] `apps/node/internal/node/liveness_watchdog.go`: attach the same typed failure to stalled tunnel frames and protobuf conversion.
|
||||
- [ ] `apps/node/internal/node/runtime_bridge_test.go`: assert present/absent normalized conversion, non-stall compatibility, and defensive metadata cloning.
|
||||
- [ ] `apps/node/internal/node/liveness_health_evidence_test.go`: assert normalized/tunnel semantic parity and raw-free metadata.
|
||||
- [ ] `apps/edge/internal/transport/server_test.go`: assert protobuf parser round-trip of both optional failure fields and unchanged message separation.
|
||||
- [ ] `agent-contract/inner/execution-runtime.md`: document typed failure ownership and legacy string compatibility.
|
||||
- [ ] `agent-contract/inner/edge-node-runtime-wire.md`: document field semantics, optionality, and safe metadata boundary.
|
||||
- [ ] `agent-spec/runtime/edge-node-execution.md`: reflect the implemented two-path failure envelope.
|
||||
|
||||
**Test Strategy:** Write `TestRuntimeEventToProtoPreservesTypedFailure`, extend `TestStallMetadata...` with normalized/tunnel parity, and add `TestEdgeParserMap_TypedExecutionFailureRoundTrip`; cover nil failure, a non-stall typed failure remaining wire-absent, retryable hint, cloned allowlisted metadata, and forbidden metadata absence.
|
||||
|
||||
**Verification:** `go test -count=1 ./apps/node/internal/node -run '^(TestRuntimeEventToProtoPreservesTypedFailure|TestStallMetadata.*)$' && go test -count=1 ./apps/edge/internal/transport -run '^(TestEdgeParserMap_.*)$'` must PASS.
|
||||
|
||||
## Dependencies and Execution Order
|
||||
|
||||
1. `04+03_health_evidence` is satisfied by `agent-task/archive/2026/08/m-node-provider-execution-liveness-recovery/04+03_health_evidence/complete.log`.
|
||||
2. Implement API-1 before API-2. This subtask must produce `complete.log` before `06+05_health_overlay` starts.
|
||||
|
||||
## Modified Files Summary
|
||||
|
||||
| File | Item |
|
||||
|------|------|
|
||||
| `proto/iop/runtime.proto` | API-1 |
|
||||
| `proto/gen/iop/runtime.pb.go` | API-1 |
|
||||
| `apps/client/lib/gen/proto/iop/runtime.pb.dart` | API-1 |
|
||||
| `apps/client/lib/gen/proto/iop/runtime.pbenum.dart` | API-1 |
|
||||
| `apps/client/lib/gen/proto/iop/runtime.pbjson.dart` | API-1 |
|
||||
| `apps/client/lib/gen/proto/iop/runtime.pbserver.dart` | API-1 |
|
||||
| `packages/go/execution/types.go` | API-1 |
|
||||
| `apps/node/internal/node/runtime_bridge.go` | API-2 |
|
||||
| `apps/node/internal/node/liveness_watchdog.go` | API-2 |
|
||||
| `apps/node/internal/node/runtime_bridge_test.go` | API-2 |
|
||||
| `apps/node/internal/node/liveness_health_evidence_test.go` | API-2 |
|
||||
| `apps/edge/internal/transport/server_test.go` | API-2 |
|
||||
| `agent-contract/inner/execution-runtime.md` | API-2 |
|
||||
| `agent-contract/inner/edge-node-runtime-wire.md` | API-2 |
|
||||
| `agent-spec/runtime/edge-node-execution.md` | API-2 |
|
||||
| `agent-task/m-node-provider-execution-liveness-recovery/05+04_failure_wire/CODE_REVIEW-cloud-G07.md` | API-1, API-2 |
|
||||
|
||||
## Final Verification
|
||||
|
||||
Fresh Go output is required; cached output is not acceptable. If `protoc-gen-dart` is still absent, first run `flutter pub global activate protoc_plugin`; this is a tool precondition, not a repository change.
|
||||
|
||||
1. `make proto && make proto-dart` — PASS; checked-in Go and Dart bindings match `runtime.proto`.
|
||||
2. `go test -count=1 ./apps/node/internal/node -run '^(TestRuntimeEventToProtoPreservesTypedFailure|TestStallMetadata.*)$' && go test -count=1 ./apps/edge/internal/transport -run '^(TestEdgeParserMap_.*)$'` — PASS and every named new test runs in its owning package.
|
||||
3. `make client-test` — PASS.
|
||||
4. `go test -count=1 ./packages/go/execution ./apps/node/... ./apps/edge/internal/transport ./apps/control-plane/...` — PASS for the Node, Edge wire, and Control Plane consumers.
|
||||
5. `go test -race -count=1 ./packages/go/execution ./apps/node/internal/node ./apps/edge/internal/transport` — PASS with no race report.
|
||||
6. `go vet ./packages/go/execution ./apps/node/... ./apps/edge/internal/transport ./apps/control-plane/...` — no diagnostics.
|
||||
7. `go test -count=1 ./...` — PASS; every checked-in Go protobuf consumer compiles and its tests pass.
|
||||
8. `./scripts/e2e-smoke.sh` — PASS as the repository-native normalized execution smoke after wire regeneration.
|
||||
9. `IOP_VLLM_MODE=fake ./scripts/e2e-openai-vllm.sh` — PASS as a credential-free Edge -> Node -> provider full-cycle using the regenerated wire.
|
||||
10. `git diff --check` — no whitespace errors.
|
||||
|
||||
After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`.
|
||||
|
|
@ -0,0 +1,202 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/05+04_failure_wire plan=0 tag=API milestone-task=failure-handoff -->
|
||||
|
||||
# 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-05
|
||||
task=m-node-provider-execution-liveness-recovery/05+04_failure_wire, plan=0, tag=API
|
||||
|
||||
## Archive Evidence Snapshot
|
||||
|
||||
- Predecessor: `agent-task/archive/2026/08/m-node-provider-execution-liveness-recovery/04+03_health_evidence/complete.log`; final verdict PASS, no remaining nits or follow-up.
|
||||
- Carryover: Node emits stable health/fence metadata and connection-scoped monotonic `health_observation_seq`; normalized and tunnel paths passed focused, package, race, vet, repository, smoke, and reconnect verification.
|
||||
- Affected foundation: `packages/go/execution`, Node liveness mappers, Go/Dart protobuf bindings, Provider Execution Runtime contract, Edge-Node Runtime Wire contract, and the living execution spec.
|
||||
|
||||
## 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/05+04_failure_wire/`. If WARN/FAIL, fully write the next filesystem state required by the code-review skill.
|
||||
4. If PASS and task group is `m-node-provider-execution-liveness-recovery`, preserve the first-line `milestone-task` metadata in `complete.log` and report it for the runtime aggregation event. Roadmap state evaluation belongs to `sync-milestone-workstate`.
|
||||
5. Check applicable `Review-Only Checklist` items at the final `.log` location before reporting.
|
||||
|
||||
---
|
||||
|
||||
## Implementation Item Completion
|
||||
|
||||
| Item | Status |
|
||||
|------|---------|
|
||||
| API-1: Typed failure wire model | [ ] |
|
||||
| API-2: Normalized/tunnel typed failure mapping | [ ] |
|
||||
|
||||
## Implementation Checklist
|
||||
|
||||
- [ ] API-1 adds one safe optional non-recursive failure message to the protobuf/runtime models and regenerates checked-in Go and Dart bindings without changing existing field numbers.
|
||||
- [ ] API-2 maps the typed failure on normalized and tunnel terminals, adds absent/present raw-free round-trip tests, and synchronizes the runtime/wire contracts and living spec.
|
||||
- [ ] Run protobuf generation, focused, package, client, race, vet, smoke, and diff verification commands and confirm fresh uncached PASS output.
|
||||
- [ ] 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/05+04_failure_wire/` to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/05+04_failure_wire/` 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 protobuf field numbers are append-only, the message matches the non-recursive runtime `Failure` shape, and generated Go/Dart descriptors match `runtime.proto`.
|
||||
- Confirm normalized and tunnel paths emit typed wire data only for `response_stalled`, while every legacy error string remains compatible.
|
||||
- Confirm tests prove nil/non-stall/present boundaries, clone only allowlisted liveness metadata, and exclude `recovery_eligible`, raw output, and arbitrary metadata.
|
||||
|
||||
## Verification Results
|
||||
|
||||
> Implementing agent: run each command exactly as written and paste its actual stdout/stderr under `Output`. Record any replacement command and reason in `Deviations from Plan`.
|
||||
|
||||
### Verification 1
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
make proto && make proto-dart
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 2
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go test -count=1 ./apps/node/internal/node -run '^(TestRuntimeEventToProtoPreservesTypedFailure|TestStallMetadata.*)$' && go test -count=1 ./apps/edge/internal/transport -run '^(TestEdgeParserMap_.*)$'
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 3
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
make client-test
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 4
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go test -count=1 ./packages/go/execution ./apps/node/... ./apps/edge/internal/transport ./apps/control-plane/...
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 5
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go test -race -count=1 ./packages/go/execution ./apps/node/internal/node ./apps/edge/internal/transport
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 6
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go vet ./packages/go/execution ./apps/node/... ./apps/edge/internal/transport ./apps/control-plane/...
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 7
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
./scripts/e2e-smoke.sh
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 8
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
git diff --check
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
---
|
||||
|
||||
> **[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,220 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/05+04_failure_wire plan=0 tag=API milestone-task=failure-handoff -->
|
||||
|
||||
# Typed Execution Failure Wire Contract
|
||||
|
||||
## For the Implementing Agent
|
||||
|
||||
Implement only the items below, run every verification command, and fill all implementation-owned sections of `CODE_REVIEW-cloud-G06.md` with actual notes and raw command output. Keep the active files in place and report ready for review; finalization belongs to the code-review skill. If blocked, record the exact blocker, attempted commands/output, and resume condition only in implementation-owned evidence fields. Do not ask the user, call user-input tools, create control-plane stop files, classify the next state, archive logs, or write `complete.log`.
|
||||
|
||||
## Background
|
||||
|
||||
Node already creates a typed `response_stalled` failure, but `RunEvent` serializes only its error string and `ProviderTunnelFrame` has no typed failure field. S04 therefore cannot preserve identical normalized/tunnel failure semantics without first establishing a backward-compatible wire foundation.
|
||||
|
||||
## Archive Evidence Snapshot
|
||||
|
||||
- Predecessor: `agent-task/archive/2026/08/m-node-provider-execution-liveness-recovery/04+03_health_evidence/complete.log`; final verdict PASS, no remaining nits or follow-up.
|
||||
- Carryover: Node emits the stable health/fence metadata and connection-scoped monotonic `health_observation_seq`; normalized and tunnel paths passed focused, package, race, vet, repository, smoke, and reconnect verification.
|
||||
- Affected foundation: `packages/go/execution`, Node liveness mappers, Provider Execution Runtime contract, Edge-Node Runtime Wire contract, and the living execution spec.
|
||||
|
||||
## Analysis
|
||||
|
||||
### Files Read
|
||||
|
||||
- `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.pbenum.dart`, `apps/client/lib/gen/proto/iop/runtime.pbjson.dart`, `apps/client/lib/gen/proto/iop/runtime.pbserver.dart`
|
||||
- `Makefile`, `agent-test/local/node-smoke.md`, `agent-test/local/edge-smoke.md`, `agent-test/local/client-smoke.md`, `agent-test/local/control-plane-smoke.md`
|
||||
- `packages/go/execution/failure.go`, `packages/go/execution/types.go`, `packages/go/execution/liveness.go`, `packages/go/execution/liveness_test.go`
|
||||
- `apps/node/internal/node/runtime_bridge.go`, `apps/node/internal/node/runtime_bridge_test.go`
|
||||
- `apps/node/internal/node/liveness_watchdog.go`, `apps/node/internal/node/liveness_health_evidence.go`, `apps/node/internal/node/liveness_health_evidence_test.go`, `apps/node/internal/node/provider_tunnel_liveness_test.go`
|
||||
- `apps/edge/internal/transport/server.go`, `apps/edge/internal/transport/connection_handlers.go`, `apps/edge/internal/transport/server_test.go`
|
||||
- `agent-contract/inner/execution-runtime.md`, `agent-contract/inner/edge-node-runtime-wire.md`
|
||||
- `agent-spec/runtime/edge-node-execution.md`
|
||||
|
||||
### SDD Criteria
|
||||
|
||||
- SDD: `agent-roadmap/sdd/operational-observability-provider-management/node-provider-execution-liveness-recovery/SDD.md`; status `[승인됨]`, and this pair's first-line id is `milestone-task=failure-handoff`.
|
||||
- Acceptance Scenario S04 and Evidence Map S04 require RunEvent/ProviderTunnelFrame round-trips to retain stable code, health, idle duration, attempt identity, fence, and sequence without raw output or Edge-owned `recovery_eligible`.
|
||||
- Those rows define API-1's common typed message and API-2's two-path round-trip tests plus contract/spec synchronization.
|
||||
|
||||
### Verification Context
|
||||
|
||||
- Handoff supplied target Milestone/Epic, allowed ids, active task group, and starting HEAD `56e7d78af3cda4a8d6a85af091ad26bce935f8b6`; checkout matched and worktree had no tracked/user changes.
|
||||
- Repository-native baseline passed: `go test -count=1 ./packages/go/execution ./apps/node/internal/node ./apps/edge/internal/transport ./apps/edge/internal/service ./apps/edge/internal/openai` with Go 1.26.2 and the repository `go.mod`.
|
||||
- Preconditions: archived predecessor PASS above; protobuf regeneration must use the repository's existing Go and Dart generation paths and checked-in output. `protoc` 29.3 and `protoc-gen-go` v1.36.11 are available, but `protoc-gen-dart` is not currently installed; before `make proto-dart`, install the repository-declared generator with `flutter pub global activate protoc_plugin`, then regenerate and verify the client bindings. No external runtime runner, host, port, or artifact is required.
|
||||
- Gap: no current protobuf round-trip asserts typed `ExecutionFailure`; confidence is high because both mapper boundaries and parser fixtures are local and deterministic.
|
||||
|
||||
### Test Coverage Gaps
|
||||
|
||||
- Normalized failures: Node tests cover the in-memory typed failure but not protobuf preservation.
|
||||
- Tunnel failures: tests cover safe metadata and terminal ordering but the model has no typed failure to assert.
|
||||
- Compatibility: existing parser separation is covered, but the new optional fields need absent/present boundary cases.
|
||||
|
||||
### Symbol References
|
||||
|
||||
- No symbol is renamed or removed. New optional `Failure` fields add call sites only in Node mappers and tests.
|
||||
|
||||
### Split Judgment
|
||||
|
||||
- `05+04_failure_wire`: stable typed wire contract; predecessor `04+03_health_evidence` is satisfied by the archived PASS `complete.log` above.
|
||||
- `06+05_health_overlay`: consumes typed wire evidence and produces reception-fenced runtime health projection; waits for this subtask's `complete.log`.
|
||||
- `07+06_retry_candidate_policy`: consumes overlay availability and produces request-local avoid-provider selection; waits for `06+05_health_overlay`.
|
||||
- `08+07_stall_recovery`: consumes typed failure and candidate policy in the OpenAI StreamGate host; waits for `07+06_retry_candidate_policy`.
|
||||
|
||||
### Scope Rationale
|
||||
|
||||
This packet does not interpret failures at Edge, mutate provider health, release leases, select retry candidates, or dispatch recovery. Those responsibilities are deliberately assigned to 06-08 so this packet remains a compatibility-testable wire foundation.
|
||||
|
||||
### Final Routing
|
||||
|
||||
- `evaluation_mode=first-pass`; finalizer=`finalize-task-policy.sh pair`.
|
||||
- Build closure true, scores `(2,0,2,1,1)`, grade G06, route `local-fit` -> `PLAN-local-G06.md`.
|
||||
- Review closure true, scores `(2,0,2,1,1)`, grade G06, route `official-review` -> `CODE_REVIEW-cloud-G06.md` (`codex`, `gpt-5.6-sol`, `xhigh`).
|
||||
- `large_indivisible_context=false`; positive loop risk: `boundary_contract` (1). `review_rework_count=0`, `evidence_integrity_failure=false`; no capability gap.
|
||||
|
||||
## Implementation Checklist
|
||||
|
||||
- [ ] API-1 adds one safe optional non-recursive failure message to the protobuf/runtime models and regenerates checked-in Go and Dart bindings without changing existing field numbers.
|
||||
- [ ] API-2 maps the typed failure on normalized and tunnel terminals, adds absent/present raw-free round-trip tests, and synchronizes the runtime/wire contracts and living spec.
|
||||
- [ ] Run protobuf generation, focused, package, client, race, vet, smoke, and diff verification commands and confirm fresh uncached PASS output.
|
||||
- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.
|
||||
|
||||
### [API-1] Add the typed failure wire model
|
||||
|
||||
**Problem:** `proto/iop/runtime.proto:30-43` carries `RunEvent.error`/metadata but no typed failure, and `proto/iop/runtime.proto:145-163` plus `packages/go/execution/types.go:239-253` have the same gap for tunnels. The Node-owned `execution.Failure` is consequently flattened before Edge can apply S04.
|
||||
|
||||
**Solution:** Add an optional protobuf `ExecutionFailure` matching the existing non-recursive `execution.Failure` shape (`code`, safe `message`, `retryable`, metadata), then add optional fields to both terminal envelopes using new field numbers. Mirror the field on the in-memory tunnel type and regenerate both Go and Dart bindings. Do not invent a recursive cause that the source runtime model does not own.
|
||||
|
||||
Before (`proto/iop/runtime.proto:30`):
|
||||
|
||||
```proto
|
||||
message RunEvent {
|
||||
string run_id = 1;
|
||||
string type = 2;
|
||||
string delta = 3;
|
||||
string message = 4;
|
||||
string error = 5;
|
||||
Usage usage = 6;
|
||||
map<string, string> metadata = 7;
|
||||
int64 timestamp = 8;
|
||||
string session_id = 9;
|
||||
bool background = 10;
|
||||
string node_id = 11;
|
||||
string node_alias = 12;
|
||||
}
|
||||
```
|
||||
|
||||
After:
|
||||
|
||||
```proto
|
||||
message ExecutionFailure {
|
||||
string code = 1;
|
||||
string message = 2;
|
||||
bool retryable = 3;
|
||||
map<string, string> metadata = 4;
|
||||
}
|
||||
|
||||
message RunEvent {
|
||||
// fields 1-12 unchanged
|
||||
ExecutionFailure failure = 13;
|
||||
}
|
||||
|
||||
message ProviderTunnelFrame {
|
||||
// fields 1-14 unchanged
|
||||
ExecutionFailure failure = 15;
|
||||
}
|
||||
```
|
||||
|
||||
**Modified Files and Checklist:**
|
||||
|
||||
- [ ] `proto/iop/runtime.proto`: append the common message and optional envelope fields without renumbering.
|
||||
- [ ] `proto/gen/iop/runtime.pb.go`: regenerate through the repository protobuf workflow; do not hand-diverge descriptors.
|
||||
- [ ] `apps/client/lib/gen/proto/iop/runtime.pb.dart`: regenerate the Dart runtime message bindings.
|
||||
- [ ] `apps/client/lib/gen/proto/iop/runtime.pbenum.dart`: regenerate the Dart enum companion output.
|
||||
- [ ] `apps/client/lib/gen/proto/iop/runtime.pbjson.dart`: regenerate the Dart descriptor/JSON output.
|
||||
- [ ] `apps/client/lib/gen/proto/iop/runtime.pbserver.dart`: regenerate the Dart server companion output.
|
||||
- [ ] `packages/go/execution/types.go`: add the tunnel-side typed failure pointer with ownership comments.
|
||||
|
||||
**Test Strategy:** API-2 owns normal and boundary round-trips; existing generated-code compilation is also exercised by every package command.
|
||||
|
||||
**Verification:** `make proto && make proto-dart && go test -count=1 ./packages/go/execution ./apps/node/internal/node ./apps/edge/internal/transport && make client-test` must PASS after installing `protoc_plugin` if the preflight gap remains.
|
||||
|
||||
### [API-2] Preserve typed failures across both Node paths
|
||||
|
||||
**Problem:** `apps/node/internal/node/runtime_bridge.go:34-57` emits only `Failure.Message` as `RunEvent.error`, while `apps/node/internal/node/liveness_watchdog.go:476-515` copies tunnel error and metadata without typed code/retryability. Existing tests therefore cannot distinguish a confirmed `response_stalled` from an unrelated string error.
|
||||
|
||||
**Solution:** Introduce one clone-safe failure mapper used by normalized and tunnel conversion. It emits a typed protobuf failure only for `FailureCodeResponseStalled`, clones only the SDD's allowlisted liveness keys, and never forwards arbitrary `Failure.Metadata`; nil and every other failure code leave the optional field absent and retain the legacy error string. Prove present/absent protobuf round-trips contain no `recovery_eligible`, raw output, or arbitrary metadata.
|
||||
|
||||
Before (`apps/node/internal/node/liveness_watchdog.go:494`):
|
||||
|
||||
```go
|
||||
return &iop.ProviderTunnelFrame{
|
||||
RunId: frame.RunID, TunnelId: frame.TunnelID, Kind: protoKind,
|
||||
Error: frame.Error, Metadata: cloneStringMap(frame.Metadata),
|
||||
}
|
||||
```
|
||||
|
||||
After:
|
||||
|
||||
```go
|
||||
return &iop.ProviderTunnelFrame{
|
||||
RunId: frame.RunID, TunnelId: frame.TunnelID, Kind: protoKind,
|
||||
Error: frame.Error, Failure: executionFailureToProto(frame.Failure),
|
||||
Metadata: cloneStringMap(frame.Metadata),
|
||||
}
|
||||
```
|
||||
|
||||
**Modified Files and Checklist:**
|
||||
|
||||
- [ ] `apps/node/internal/node/runtime_bridge.go`: map only allowlisted `response_stalled` failures for normalized events while preserving legacy error fallback for every failure.
|
||||
- [ ] `apps/node/internal/node/liveness_watchdog.go`: attach the same typed failure to stalled tunnel frames and protobuf conversion.
|
||||
- [ ] `apps/node/internal/node/runtime_bridge_test.go`: assert present/absent normalized conversion, non-stall compatibility, and defensive metadata cloning.
|
||||
- [ ] `apps/node/internal/node/liveness_health_evidence_test.go`: assert normalized/tunnel semantic parity and raw-free metadata.
|
||||
- [ ] `apps/edge/internal/transport/server_test.go`: assert protobuf parser round-trip of both optional failure fields and unchanged message separation.
|
||||
- [ ] `agent-contract/inner/execution-runtime.md`: document typed failure ownership and legacy string compatibility.
|
||||
- [ ] `agent-contract/inner/edge-node-runtime-wire.md`: document field semantics, optionality, and safe metadata boundary.
|
||||
- [ ] `agent-spec/runtime/edge-node-execution.md`: reflect the implemented two-path failure envelope.
|
||||
|
||||
**Test Strategy:** Write `TestRuntimeEventToProtoPreservesTypedFailure`, extend `TestStallMetadata...` with normalized/tunnel parity, and add `TestEdgeParserMap_TypedExecutionFailureRoundTrip`; cover nil failure, a non-stall typed failure remaining wire-absent, retryable hint, cloned allowlisted metadata, and forbidden metadata absence.
|
||||
|
||||
**Verification:** `go test -count=1 ./apps/node/internal/node -run '^(TestRuntimeEventToProtoPreservesTypedFailure|TestStallMetadata.*)$' && go test -count=1 ./apps/edge/internal/transport -run '^(TestEdgeParserMap_.*)$'` must PASS.
|
||||
|
||||
## Dependencies and Execution Order
|
||||
|
||||
1. `04+03_health_evidence` is satisfied by `agent-task/archive/2026/08/m-node-provider-execution-liveness-recovery/04+03_health_evidence/complete.log`.
|
||||
2. Implement API-1 before API-2. This subtask must produce `complete.log` before `06+05_health_overlay` starts.
|
||||
|
||||
## Modified Files Summary
|
||||
|
||||
| File | Item |
|
||||
|------|------|
|
||||
| `proto/iop/runtime.proto` | API-1 |
|
||||
| `proto/gen/iop/runtime.pb.go` | API-1 |
|
||||
| `apps/client/lib/gen/proto/iop/runtime.pb.dart` | API-1 |
|
||||
| `apps/client/lib/gen/proto/iop/runtime.pbenum.dart` | API-1 |
|
||||
| `apps/client/lib/gen/proto/iop/runtime.pbjson.dart` | API-1 |
|
||||
| `apps/client/lib/gen/proto/iop/runtime.pbserver.dart` | API-1 |
|
||||
| `packages/go/execution/types.go` | API-1 |
|
||||
| `apps/node/internal/node/runtime_bridge.go` | API-2 |
|
||||
| `apps/node/internal/node/liveness_watchdog.go` | API-2 |
|
||||
| `apps/node/internal/node/runtime_bridge_test.go` | API-2 |
|
||||
| `apps/node/internal/node/liveness_health_evidence_test.go` | API-2 |
|
||||
| `apps/edge/internal/transport/server_test.go` | API-2 |
|
||||
| `agent-contract/inner/execution-runtime.md` | API-2 |
|
||||
| `agent-contract/inner/edge-node-runtime-wire.md` | API-2 |
|
||||
| `agent-spec/runtime/edge-node-execution.md` | API-2 |
|
||||
| `agent-task/m-node-provider-execution-liveness-recovery/05+04_failure_wire/CODE_REVIEW-cloud-G06.md` | API-1, API-2 |
|
||||
|
||||
## Final Verification
|
||||
|
||||
Fresh Go output is required; cached output is not acceptable. If `protoc-gen-dart` is still absent, first run `flutter pub global activate protoc_plugin`; this is a tool precondition, not a repository change.
|
||||
|
||||
1. `make proto && make proto-dart` — PASS; checked-in Go and Dart bindings match `runtime.proto`.
|
||||
2. `go test -count=1 ./apps/node/internal/node -run '^(TestRuntimeEventToProtoPreservesTypedFailure|TestStallMetadata.*)$' && go test -count=1 ./apps/edge/internal/transport -run '^(TestEdgeParserMap_.*)$'` — PASS and every named new test runs in its owning package.
|
||||
3. `make client-test` — PASS.
|
||||
4. `go test -count=1 ./packages/go/execution ./apps/node/... ./apps/edge/internal/transport ./apps/control-plane/...` — PASS for the Node, Edge wire, and Control Plane consumers.
|
||||
5. `go test -race -count=1 ./packages/go/execution ./apps/node/internal/node ./apps/edge/internal/transport` — PASS with no race report.
|
||||
6. `go vet ./packages/go/execution ./apps/node/... ./apps/edge/internal/transport ./apps/control-plane/...` — no diagnostics.
|
||||
7. `./scripts/e2e-smoke.sh` — PASS as the repository-native normalized execution smoke after wire regeneration.
|
||||
8. `git diff --check` — no whitespace errors.
|
||||
|
||||
After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`.
|
||||
|
|
@ -0,0 +1,206 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/06+05_health_overlay plan=1 tag=REFACTOR milestone-task=failure-handoff -->
|
||||
|
||||
# Code Review Reference - REFACTOR
|
||||
|
||||
> **[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-05
|
||||
task=m-node-provider-execution-liveness-recovery/06+05_health_overlay, plan=1, tag=REFACTOR
|
||||
|
||||
## Archive Evidence Snapshot
|
||||
|
||||
- Prior pair: `plan_local_G08_0.log` and `code_review_cloud_G08_0.log` in this task directory. It was unimplemented and has no official verdict, Required/Suggested/Nit finding, code change, or verification evidence.
|
||||
- Material self-review finding: the prior overlay could be lowered by terminal evidence, but its only recovery input was a test helper; production had no bounded status-probe response carrying the same connection-scoped sequence, so S04 recovery could not occur outside fixtures.
|
||||
- Replan carryover: retain reception/lease fencing and release-once scope, add a real exact-target CAPABILITIES status-probe path, and add queue/full-cycle verification. Predecessor `05+04_failure_wire` remains active and must produce `complete.log` before implementation.
|
||||
|
||||
|
||||
## 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-G09.md` → `code_review_cloud_G09_1.log` and `PLAN-cloud-G09.md` → `plan_cloud_G09_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/06+05_health_overlay/`. If WARN/FAIL, fully write the next filesystem state required by the code-review skill.
|
||||
4. If PASS and task group is `m-node-provider-execution-liveness-recovery`, preserve the first-line `milestone-task` metadata in `complete.log` and report it for the runtime aggregation event. Roadmap state evaluation belongs to `sync-milestone-workstate`.
|
||||
5. Check applicable `Review-Only Checklist` items at the final `.log` location before reporting.
|
||||
|
||||
---
|
||||
|
||||
## Implementation Item Completion
|
||||
|
||||
| Item | Status |
|
||||
|------|---------|
|
||||
| REFACTOR-1: Carry authoritative reception identity | [ ] |
|
||||
| REFACTOR-2: Apply a lease-bound runtime health overlay | [ ] |
|
||||
| REFACTOR-3: Feed recovery from the bounded status probe | [ ] |
|
||||
|
||||
## Implementation Checklist
|
||||
|
||||
- [ ] REFACTOR-1 propagates authoritative receiving node/generation for RunEvent and tunnel callbacks and binds it atomically to the current registry owner without trusting wire identity.
|
||||
- [ ] REFACTOR-2 validates immutable lease identity, applies sequence-fenced runtime unhealthy/recovery transitions, gates admission/snapshots, annotates every confirmed bound stall for Edge-local recovery (including unknown health), and releases valid terminal leases exactly once.
|
||||
- [ ] REFACTOR-3 turns the existing exact-target CAPABILITIES probe into fail-closed sequenced evidence and applies only an unambiguous current-generation higher-sequence available response to overlay recovery.
|
||||
- [ ] Add focused stale-owner, missing/ambiguous identity, mismatch, sequence, production-probe recovery, normalized/tunnel, and release-race tests; synchronize contracts/specs without mutating config health semantics.
|
||||
- [ ] Run focused, package, race, vet, provider-only/local-capacity/full-cycle, live preflight/scenario, and diff verification commands with fresh output.
|
||||
- [ ] 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_G09_1.log`.
|
||||
- [ ] Archive active `PLAN-*-G??.md` to `plan_cloud_G09_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/06+05_health_overlay/` to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/06+05_health_overlay/` 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 transport derives node/generation from the receiving client and stale clients cannot reach correctness callbacks.
|
||||
- Confirm lease/overlay transitions are provider/adapter/target/generation/sequence fenced, preserve config health, and release a valid terminal exactly once.
|
||||
- Confirm CAPABILITIES reuses Node `ProbeHealth` plus the Session sequence and only an unambiguous current-generation higher-sequence available response can recover the overlay.
|
||||
- Confirm local queue/full-cycle evidence passes and live provider-pool preflight/scenario is PASS or recorded as an exact verification blocker.
|
||||
|
||||
## Verification Results
|
||||
|
||||
> Implementing agent: run each command exactly as written and paste its actual stdout/stderr under `Output`. Record any replacement command and reason in `Deviations from Plan`.
|
||||
|
||||
### Verification 1
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go test -count=1 ./packages/go/execution ./apps/node/... ./apps/edge/internal/node ./apps/edge/internal/transport ./apps/edge/internal/bootstrap ./packages/go/streamgate ./apps/edge/internal/openai ./apps/edge/internal/service ./apps/edge/internal/controlplane
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 2
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go test -count=20 ./apps/edge/internal/service -run '^(TestProviderHealthOverlay|TestReceived.*Failure|Test.*ReleaseOnce)' && go test -count=10 ./apps/node/internal/node ./apps/edge/internal/service -run '^(TestCapabilitiesHealthEvidence|TestProviderHealthOverlayCapabilitiesRecovery)'
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 3
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go test -race -count=3 ./apps/node/internal/node ./apps/edge/internal/node ./apps/edge/internal/transport ./apps/edge/internal/service
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 4
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go vet ./packages/go/execution ./apps/node/... ./apps/edge/internal/node ./apps/edge/internal/transport ./apps/edge/internal/bootstrap ./packages/go/streamgate ./apps/edge/internal/openai ./apps/edge/internal/service ./apps/edge/internal/controlplane
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 5
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
./scripts/e2e-smoke.sh
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 6
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
./scripts/e2e-provider-capacity-smoke.sh
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 7
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
bash scripts/e2e-long-context-admission-smoke.sh --preflight && bash scripts/e2e-long-context-admission-smoke.sh --scenario normal-10
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 8
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
git diff --check
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
---
|
||||
|
||||
> **[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,228 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/06+05_health_overlay plan=1 tag=REFACTOR milestone-task=failure-handoff -->
|
||||
|
||||
# Reception-Fenced Provider Health Overlay
|
||||
|
||||
## For the Implementing Agent
|
||||
|
||||
Implement only the items below after the predecessor PASS, run every verification command, and fill all implementation-owned sections of `CODE_REVIEW-cloud-G09.md` with actual notes and raw command output. Keep active files in place and report ready for review; finalization belongs to the code-review skill. If blocked, record exact blocker evidence, attempted commands/output, and resume conditions only. Do not ask the user, call user-input tools, create control-plane stop files, classify the next state, archive logs, or write `complete.log`.
|
||||
|
||||
## Background
|
||||
|
||||
Typed failures are not authoritative merely because they name a node or provider: Edge must bind them to the actual receiving connection and the immutable lease that dispatched the attempt. S04 also requires a generation/sequence-fenced runtime overlay that changes admission and snapshots without mutating config health, while every valid terminal still releases its old lease exactly once.
|
||||
|
||||
## Archive Evidence Snapshot
|
||||
|
||||
- Prior pair: `plan_local_G08_0.log` and `code_review_cloud_G08_0.log` in this task directory. It was unimplemented and has no official verdict, Required/Suggested/Nit finding, code change, or verification evidence.
|
||||
- Material self-review finding: the prior overlay could be lowered by terminal evidence, but its only recovery input was a test helper; production had no bounded status-probe response carrying the same connection-scoped sequence, so S04 recovery could not occur outside fixtures.
|
||||
- Replan carryover: retain reception/lease fencing and release-once scope, add a real exact-target CAPABILITIES status-probe path, and add queue/full-cycle verification. Predecessor `05+04_failure_wire` remains active and must produce `complete.log` before implementation.
|
||||
|
||||
## Analysis
|
||||
|
||||
### Files Read
|
||||
|
||||
- `apps/edge/internal/node/registry.go`, `apps/edge/internal/node/registry_test.go`
|
||||
- `apps/edge/internal/transport/server.go`, `apps/edge/internal/transport/connection_handlers.go`, `apps/edge/internal/transport/server_test.go`
|
||||
- `apps/edge/internal/bootstrap/runtime.go`, `apps/edge/internal/bootstrap/runtime_refresh_test.go`
|
||||
- `apps/edge/internal/service/service.go`, `apps/edge/internal/service/provider_tunnel.go`, `apps/edge/internal/service/provider_pool.go`, `apps/edge/internal/service/provider_resolution.go`
|
||||
- `apps/edge/internal/service/model_queue_types.go`, `apps/edge/internal/service/model_queue_admission.go`, `apps/edge/internal/service/model_queue_release.go`, `apps/edge/internal/service/model_queue_snapshot.go`, `apps/edge/internal/service/model_queue_test_support_test.go`, `apps/edge/internal/service/model_queue_admission_test.go`, `apps/edge/internal/service/queue_dispatch_test.go`
|
||||
- `apps/edge/internal/service/node_command.go`, `apps/node/internal/node/command_handler.go`, `apps/node/internal/node/command_test.go`, `apps/node/internal/node/health_probe.go`, `apps/node/internal/node/health_probe_test.go`, `apps/node/internal/transport/session.go`
|
||||
- `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`
|
||||
- `agent-test/local/edge-smoke.md`, `agent-test/local/node-smoke.md`, `scripts/e2e-smoke.sh`, `scripts/e2e-provider-capacity-smoke.sh`, `scripts/e2e-long-context-admission-smoke.sh`
|
||||
|
||||
### SDD Criteria
|
||||
|
||||
- SDD: `agent-roadmap/sdd/operational-observability-provider-management/node-provider-execution-liveness-recovery/SDD.md`; status `[승인됨]`, and this pair's first-line id is `milestone-task=failure-handoff`.
|
||||
- Acceptance Scenario S04 and Evidence Map S04 require absent provider identity, stale connection/sequence, and identity mismatch to leave projection unchanged; only current bound fresh evidence may mark/recover overlay health, and terminal lease release is exactly once. A validated `unknown` probe does not change provider-wide health but still preserves a confirmed request-local stall handoff so the ingress owner may try a different provider.
|
||||
- The S04 transition table fixes semantics: `unavailable` lowers; higher-sequence same-generation `available` from a later bounded exact-target status probe recovers prior unavailable; request-stalled/available and unknown do not lower. REFACTOR-1 covers reception/binding, REFACTOR-2 covers transition/admission/snapshot/release, REFACTOR-3 provides that production probe input, and the final commands include race/ordering/full-cycle fixtures.
|
||||
|
||||
### Verification Context
|
||||
|
||||
- Handoff supplied starting HEAD `56e7d78af3cda4a8d6a85af091ad26bce935f8b6`; baseline package tests passed fresh. This plan assumes `05+04_failure_wire/complete.log` exists and its optional failure fields compile.
|
||||
- Current transport callbacks pass only a message although listener closures retain `*toki.TcpClient`; the registry already owns monotonic connection generations and compare-by-client fencing primitives.
|
||||
- Existing queue leases hold node/provider/generation but omit adapter/target; provider resources hold immutable config capacity/enable plus connection generation but no observed health sequence.
|
||||
- The existing CAPABILITIES command already reaches `ProviderProber`, but it bypasses the fail-closed `ProbeHealth` normalizer, does not allocate `Session.NextHealthObservationSeq`, and Edge returns the result without applying it. That path is the bounded on-demand S04 recovery input after this replan; ambiguous adapter/target -> provider binding or a stale response must be a no-op.
|
||||
- External verification preflight was run from `/config/workspace/iop-s1` at HEAD `56e7d78af3cda4a8d6a85af091ad26bce935f8b6`: `bash scripts/e2e-long-context-admission-smoke.sh --preflight` passed `configs/edge.yaml` validation but returned rc=3 because `http://toki-labs.com:18083/v1/models` and the runner-local status URL were unreachable. No binary/artifact override or token was present; the script assumes the configured dev provider pool and its documented host/ports. The implementer must rerun preflight on a source-synchronized authorized dev runner, then run an applicable `--scenario`; inability is a verification blocker, not permission to claim PASS. Deterministic local coverage remains `e2e-provider-capacity-smoke.sh` plus focused/race tests.
|
||||
- Confidence is medium because registry, queue, transport, command response, Node sequence, and live provider-pool behavior now form one temporal boundary.
|
||||
|
||||
### Test Coverage Gaps
|
||||
|
||||
- Transport proves RunEvent/tunnel separation but not authoritative client/generation context or stale-owner drops.
|
||||
- Queue tests prove generation-fenced capacity and release-once, but not adapter/target identity or health sequence transitions.
|
||||
- Snapshot tests project config/disconnect health only; runtime unhealthy/recovery overlay is absent.
|
||||
- CAPABILITIES tests prove probing but not fail-closed health classification, shared observation sequence, stale-response rejection, or Edge overlay recovery.
|
||||
|
||||
### Symbol References
|
||||
|
||||
- Preserve existing one-argument `Service.HandleRunLifecycleEvent` and `Service.RouteProviderTunnelFrame` for direct callers/tests. Add reception-aware siblings for bootstrap wiring; no symbol is renamed or removed.
|
||||
- Transport setter callback types change internally; call sites are `apps/edge/internal/bootstrap/runtime.go` and `apps/edge/internal/transport/server_test.go`.
|
||||
|
||||
### Split Judgment
|
||||
|
||||
- Stable predecessor contract: `05+04_failure_wire` supplies optional typed failure fields. Its `complete.log` is currently missing in the active sibling, so implementation must wait for PASS.
|
||||
- This packet supplies immutable reception/binding validation and overlay projection. `07+06_retry_candidate_policy` depends on its candidate eligibility; `08+07_stall_recovery` depends transitively on both.
|
||||
- The packet is cohesive because the same queue lock must order evidence validation, overlay transition, lease release, and the next admission pump; splitting that invariant would create an unsafe intermediate state.
|
||||
|
||||
### Scope Rationale
|
||||
|
||||
Do not create retry intents, pick alternate providers, consume StreamGate budget, add metrics, or mutate Node/config health. Retry selection belongs to 07, OpenAI recovery to 08, and `ops-evidence` is outside this Epic's allowed task ids.
|
||||
|
||||
### Final Routing
|
||||
|
||||
- `evaluation_mode=isolated-reassessment`; finalizer=`finalize-task-policy.sh pair`.
|
||||
- Build closure true, scores `(2,2,2,1,2)`, grade G09, route `grade-boundary` -> `PLAN-cloud-G09.md`.
|
||||
- Review closure true, scores `(2,2,2,1,2)`, grade G09, route `official-review` -> `CODE_REVIEW-cloud-G09.md` (`codex`, `gpt-5.6-sol`, `xhigh`).
|
||||
- `large_indivisible_context=false`; positive loop risks: `temporal_state`, `concurrent_consistency`, `boundary_contract`, `variant_product` (4). `review_rework_count=0`, `evidence_integrity_failure=false`; no capability gap.
|
||||
|
||||
## Implementation Checklist
|
||||
|
||||
- [ ] REFACTOR-1 propagates authoritative receiving node/generation for RunEvent and tunnel callbacks and binds it atomically to the current registry owner without trusting wire identity.
|
||||
- [ ] REFACTOR-2 validates immutable lease identity, applies sequence-fenced runtime unhealthy/recovery transitions, gates admission/snapshots, annotates every confirmed bound stall for Edge-local recovery (including unknown health), and releases valid terminal leases exactly once.
|
||||
- [ ] REFACTOR-3 turns the existing exact-target CAPABILITIES probe into fail-closed sequenced evidence and applies only an unambiguous current-generation higher-sequence available response to overlay recovery.
|
||||
- [ ] Add focused stale-owner, missing/ambiguous identity, mismatch, sequence, production-probe recovery, normalized/tunnel, and release-race tests; synchronize contracts/specs without mutating config health semantics.
|
||||
- [ ] Run focused, package, race, vet, provider-only/local-capacity/full-cycle, live preflight/scenario, and diff verification commands with fresh output.
|
||||
- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.
|
||||
|
||||
### [REFACTOR-1] Carry authoritative reception identity
|
||||
|
||||
**Problem:** `apps/edge/internal/transport/connection_handlers.go:15-54` captures the receiving client but invokes callbacks with only the wire message. A spoofed/stale event can therefore be processed without proving which registered connection delivered it.
|
||||
|
||||
**Solution:** Add an atomic registry lookup that returns a cloned current owner only when the supplied client still owns it. Change transport's internal lifecycle/tunnel callback contracts to include that authoritative node id and generation, drop callbacks from unregistered/stale clients, and wire the new service entry points from bootstrap. Keep observability fanout message-only.
|
||||
|
||||
Before (`apps/edge/internal/transport/connection_handlers.go:29`):
|
||||
|
||||
```go
|
||||
if lifecycle != nil {
|
||||
lifecycle(e)
|
||||
}
|
||||
```
|
||||
|
||||
After:
|
||||
|
||||
```go
|
||||
owner, ok := s.registry.CurrentOwnerForClient(client)
|
||||
if ok && lifecycle != nil {
|
||||
lifecycle(owner.NodeID, owner.ConnectionGeneration, e)
|
||||
}
|
||||
```
|
||||
|
||||
**Modified Files and Checklist:**
|
||||
|
||||
- [ ] `apps/edge/internal/node/registry.go`: add lock-safe current-owner-by-client lookup returning a clone.
|
||||
- [ ] `apps/edge/internal/node/registry_test.go`: prove current owner success and stale/unregistered client rejection across reconnect.
|
||||
- [ ] `apps/edge/internal/transport/server.go`: type reception-aware lifecycle/tunnel callbacks.
|
||||
- [ ] `apps/edge/internal/transport/connection_handlers.go`: resolve current owner at receipt and fail closed for stale clients before correctness callbacks.
|
||||
- [ ] `apps/edge/internal/transport/server_test.go`: assert authoritative node/generation and no callback from stale connection while observability separation remains intact.
|
||||
- [ ] `apps/edge/internal/bootstrap/runtime.go`: wire reception-aware service methods.
|
||||
|
||||
**Test Strategy:** Extend transport/registry fixtures with two clients for one node generation; assert only the live receiver reaches lifecycle/tunnel callbacks and wire metadata cannot substitute another owner.
|
||||
|
||||
**Verification:** `go test -count=1 ./apps/edge/internal/node ./apps/edge/internal/transport ./apps/edge/internal/bootstrap -run 'CurrentOwner|Reception|Lifecycle|Tunnel'` must PASS (Go treats unmatched package patterns as no tests, but every named new test must run in its owning package).
|
||||
|
||||
### [REFACTOR-2] Apply a lease-bound runtime health overlay
|
||||
|
||||
**Problem:** `apps/edge/internal/service/model_queue_types.go:173-186` cannot verify adapter/target, `providerResourceState` at lines 463-477 has no runtime observation state, and `model_queue_admission.go:75-113` plus `model_queue_snapshot.go:47-71` consult only config/connectivity. `Service.HandleRunLifecycleEvent` at `service.go:106-115` releases before classifying typed evidence.
|
||||
|
||||
**Solution:** Extend the immutable lease with dispatch adapter/target and maintain a separate `(node_id, connection_generation, provider_id)` overlay state under the queue lock. For a terminal owned by the receiving generation, compare provider/adapter/target and strictly increasing observation sequence; only `unavailable` sets unhealthy and a later same-generation, higher-sequence `available` clears it. Unknown/request-stalled leaves provider-wide projection unchanged. Attach Edge-local `provider_id`, normalized `provider_health`, and `recovery_eligible=true` to every confirmed, current, identity-bound `response_stalled` terminal, including `unknown`; this marker authorizes only ingress evaluation and never same-provider fallback. Missing stable provider identity, unconfirmed fence, stale owner/sequence, or binding mismatch remains terminal-only. Then release through the existing idempotent lease transition and pump. Apply the same path before routing tunnel ERROR frames.
|
||||
|
||||
Before (`apps/edge/internal/service/service.go:110`):
|
||||
|
||||
```go
|
||||
func (s *Service) HandleRunLifecycleEvent(event *iop.RunEvent) {
|
||||
if event == nil || s.queue == nil || !isTerminalRunEvent(event) { return }
|
||||
s.queue.releaseRun(event.GetRunId(), event.GetType())
|
||||
}
|
||||
```
|
||||
|
||||
After:
|
||||
|
||||
```go
|
||||
func (s *Service) HandleReceivedRunLifecycle(nodeID string, generation uint64, event *iop.RunEvent) {
|
||||
s.queue.applyTerminalEvidenceAndRelease(nodeID, generation, event)
|
||||
}
|
||||
```
|
||||
|
||||
**Modified Files and Checklist:**
|
||||
|
||||
- [ ] `apps/edge/internal/service/model_queue_types.go`: add immutable adapter/target binding and generation-scoped overlay sequence/health state separate from config.
|
||||
- [ ] `apps/edge/internal/service/model_queue_admission.go`: mint full bindings and reject runtime-unhealthy candidates under the existing queue lock.
|
||||
- [ ] `apps/edge/internal/service/model_queue_release.go`: atomically validate terminal evidence, transition overlay, annotate eligible failure, release once, and pump.
|
||||
- [ ] `apps/edge/internal/service/model_queue_snapshot.go`: project effective runtime unhealthy/recovery without changing catalog config.
|
||||
- [ ] `apps/edge/internal/service/service.go`: expose reception-aware normalized lifecycle handling while retaining the compatibility wrapper.
|
||||
- [ ] `apps/edge/internal/service/provider_tunnel.go`: validate/annotate terminal ERROR before request routing; duplicate stream cleanup remains a no-op release.
|
||||
- [ ] `apps/edge/internal/service/provider_health_overlay_test.go`: add the S04 table and normalized/tunnel release-race fixtures.
|
||||
- [ ] `agent-contract/inner/execution-runtime.md`: document lease binding, Edge-local eligibility annotation, and release ordering.
|
||||
- [ ] `agent-contract/inner/edge-node-runtime-wire.md`: document reception identity as out-of-band authority and stale evidence rejection.
|
||||
- [ ] `agent-contract/inner/edge-config-runtime-refresh.md`: distinguish immutable config health from runtime overlay.
|
||||
- [ ] `agent-spec/runtime/edge-node-execution.md`: reflect reception fencing and terminal handoff.
|
||||
- [ ] `agent-spec/runtime/provider-pool-config-refresh.md`: reflect effective admission/snapshot overlay behavior.
|
||||
|
||||
**Test Strategy:** Create table tests for missing provider id, wrong node/provider/adapter/target, stale generation, equal/lower sequence, unavailable transition, unknown/request-stalled overlay no-op, higher-sequence available recovery, and a new connection generation. Prove available, unavailable, and unknown confirmed bound stalls all receive the raw-free request-local handoff marker, while only unavailable/available mutate overlay state. Run normalized and tunnel terminal duplicates concurrently and assert one decrement, no negative count, and no newer lease release.
|
||||
|
||||
**Verification:** `go test -count=20 ./apps/edge/internal/service -run '^(TestProviderHealthOverlay|TestReceived.*Failure|Test.*ReleaseOnce)'` must PASS.
|
||||
|
||||
### [REFACTOR-3] Feed recovery from the bounded status probe
|
||||
|
||||
**Problem:** `apps/node/internal/node/command_handler.go:49-102` directly calls `ProbeProvider`, maps errors to unavailable, and emits neither normalized `provider_health` nor the Session-owned observation sequence. `apps/edge/internal/service/node_command.go:53-132` returns CAPABILITIES results without binding the response to its sending connection or applying it to the runtime overlay. The prior plan could therefore recover only through tests.
|
||||
|
||||
**Solution:** Pass the transport Session into CAPABILITIES handling and reuse `ProbeHealth(caps.AdapterName, caps.InstanceKey, exactTarget, ResolveProbeFunc(adapter))`. Allocate `health_observation_seq` from that same Session and return only stable adapter/instance/target, normalized health/status, and sequence fields. On Edge, retain the resolved entry's node id and connection generation through the synchronous response, map adapter/target to exactly one current provider resource under the queue lock, and apply only `available` with a strictly greater sequence to clear an unavailable overlay. Unknown/unavailable status probes never clear it; stale generation, mismatched response identity, empty target, or zero/malformed/ambiguous provider mapping is a no-op. This does not auto-retry or mutate catalog/config health.
|
||||
|
||||
**Modified Files and Checklist:**
|
||||
|
||||
- [ ] `apps/node/internal/node/command_handler.go`: use `ProbeHealth`, Session sequence, and stable result keys for CAPABILITIES without returning raw probe detail as correctness evidence.
|
||||
- [ ] `apps/node/internal/node/command_test.go`: cover exact available, timeout/error/unsupported/mismatch -> unknown, and monotonic CAPABILITIES evidence on one Session.
|
||||
- [ ] `apps/edge/internal/service/node_command.go`: carry authoritative node/generation from request dispatch and offer successful CAPABILITIES evidence to the queue only after response validation.
|
||||
- [ ] `apps/edge/internal/service/model_queue_release.go`: add the shared locked probe-evidence transition used by production and tests, with unambiguous provider binding and sequence fencing.
|
||||
- [ ] `apps/edge/internal/service/provider_health_overlay_test.go`: drive the real CAPABILITIES response path for recover, stale generation, lower/equal sequence, ambiguous adapter/target, and unknown/unavailable no-clear cases.
|
||||
- [ ] `agent-contract/inner/execution-runtime.md`, `agent-contract/inner/edge-node-runtime-wire.md`, `agent-spec/runtime/edge-node-execution.md`: document status-probe evidence ownership and fail-closed recovery conditions.
|
||||
|
||||
**Test Strategy:** Use a real Node command handler/session fixture and the Edge command service seam rather than calling an overlay test helper directly. Assert one unavailable terminal lowers admission/snapshot, a later current-generation exact available CAPABILITIES response with greater sequence restores it, and every stale/ambiguous/inconclusive response leaves state unchanged.
|
||||
|
||||
**Verification:** `go test -count=10 ./apps/node/internal/node ./apps/edge/internal/service -run '^(TestCapabilitiesHealthEvidence|TestProviderHealthOverlayCapabilitiesRecovery)'` must PASS every iteration and every named test must execute in its owning package.
|
||||
|
||||
## Dependencies and Execution Order
|
||||
|
||||
1. `05+04_failure_wire` must first produce `agent-task/m-node-provider-execution-liveness-recovery/05+04_failure_wire/complete.log`; it is active/missing at plan creation.
|
||||
2. Implement REFACTOR-1, then REFACTOR-2, then REFACTOR-3. This subtask must PASS before `07+06_retry_candidate_policy` starts.
|
||||
|
||||
## Modified Files Summary
|
||||
|
||||
| File | Item |
|
||||
|------|------|
|
||||
| `apps/edge/internal/node/registry.go` | REFACTOR-1 |
|
||||
| `apps/edge/internal/node/registry_test.go` | REFACTOR-1 |
|
||||
| `apps/edge/internal/transport/server.go` | REFACTOR-1 |
|
||||
| `apps/edge/internal/transport/connection_handlers.go` | REFACTOR-1 |
|
||||
| `apps/edge/internal/transport/server_test.go` | REFACTOR-1 |
|
||||
| `apps/edge/internal/bootstrap/runtime.go` | REFACTOR-1 |
|
||||
| `apps/edge/internal/service/model_queue_types.go` | REFACTOR-2 |
|
||||
| `apps/edge/internal/service/model_queue_admission.go` | REFACTOR-2 |
|
||||
| `apps/edge/internal/service/model_queue_release.go` | REFACTOR-2, REFACTOR-3 |
|
||||
| `apps/edge/internal/service/model_queue_snapshot.go` | REFACTOR-2 |
|
||||
| `apps/edge/internal/service/service.go` | REFACTOR-2 |
|
||||
| `apps/edge/internal/service/provider_tunnel.go` | REFACTOR-2 |
|
||||
| `apps/edge/internal/service/provider_health_overlay_test.go` | REFACTOR-2 |
|
||||
| `apps/node/internal/node/command_handler.go` | REFACTOR-3 |
|
||||
| `apps/node/internal/node/command_test.go` | REFACTOR-3 |
|
||||
| `apps/edge/internal/service/node_command.go` | REFACTOR-3 |
|
||||
| `agent-contract/inner/execution-runtime.md` | REFACTOR-2, REFACTOR-3 |
|
||||
| `agent-contract/inner/edge-node-runtime-wire.md` | REFACTOR-2, REFACTOR-3 |
|
||||
| `agent-contract/inner/edge-config-runtime-refresh.md` | REFACTOR-2 |
|
||||
| `agent-spec/runtime/edge-node-execution.md` | REFACTOR-2, REFACTOR-3 |
|
||||
| `agent-spec/runtime/provider-pool-config-refresh.md` | REFACTOR-2 |
|
||||
| `agent-task/m-node-provider-execution-liveness-recovery/06+05_health_overlay/CODE_REVIEW-cloud-G09.md` | REFACTOR-1, REFACTOR-2 |
|
||||
|
||||
## Final Verification
|
||||
|
||||
Fresh Go output is required; cached output is not acceptable.
|
||||
|
||||
1. `go test -count=1 ./packages/go/execution ./apps/node/... ./apps/edge/internal/node ./apps/edge/internal/transport ./apps/edge/internal/bootstrap ./packages/go/streamgate ./apps/edge/internal/openai ./apps/edge/internal/service ./apps/edge/internal/controlplane` — PASS, including Node and Edge local profiles.
|
||||
2. `go test -count=20 ./apps/edge/internal/service -run '^(TestProviderHealthOverlay|TestReceived.*Failure|Test.*ReleaseOnce)' && go test -count=10 ./apps/node/internal/node ./apps/edge/internal/service -run '^(TestCapabilitiesHealthEvidence|TestProviderHealthOverlayCapabilitiesRecovery)'` — PASS on every iteration and all named tests execute.
|
||||
3. `go test -race -count=3 ./apps/node/internal/node ./apps/edge/internal/node ./apps/edge/internal/transport ./apps/edge/internal/service` — PASS with no race report.
|
||||
4. `go vet ./packages/go/execution ./apps/node/... ./apps/edge/internal/node ./apps/edge/internal/transport ./apps/edge/internal/bootstrap ./packages/go/streamgate ./apps/edge/internal/openai ./apps/edge/internal/service ./apps/edge/internal/controlplane` — no diagnostics.
|
||||
5. `./scripts/e2e-smoke.sh` — PASS for provider-only Edge/Node dispatch, tunnel, queue, and reconnect fencing.
|
||||
6. `./scripts/e2e-provider-capacity-smoke.sh` — PASS for a deterministic local capacity-1 provider pool and zeroed final counters.
|
||||
7. `bash scripts/e2e-long-context-admission-smoke.sh --preflight && bash scripts/e2e-long-context-admission-smoke.sh --scenario normal-10` — PASS on the authorized synchronized dev runner; if reachability/identity remains blocked, record rc/output and do not claim completion.
|
||||
8. `git diff --check` — no whitespace errors.
|
||||
|
||||
After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`.
|
||||
|
|
@ -0,0 +1,175 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/06+05_health_overlay plan=0 tag=REFACTOR milestone-task=failure-handoff -->
|
||||
|
||||
# Code Review Reference - REFACTOR
|
||||
|
||||
> **[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-05
|
||||
task=m-node-provider-execution-liveness-recovery/06+05_health_overlay, plan=0, tag=REFACTOR
|
||||
|
||||
|
||||
|
||||
## 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-local-G08.md` → `plan_local_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/06+05_health_overlay/`. If WARN/FAIL, fully write the next filesystem state required by the code-review skill.
|
||||
4. If PASS and task group is `m-node-provider-execution-liveness-recovery`, preserve the first-line `milestone-task` metadata in `complete.log` and report it for the runtime aggregation event. Roadmap state evaluation belongs to `sync-milestone-workstate`.
|
||||
5. Check applicable `Review-Only Checklist` items at the final `.log` location before reporting.
|
||||
|
||||
---
|
||||
|
||||
## Implementation Item Completion
|
||||
|
||||
| Item | Status |
|
||||
|------|---------|
|
||||
| REFACTOR-1: Authoritative reception identity | [ ] |
|
||||
| REFACTOR-2: Lease-bound runtime health overlay | [ ] |
|
||||
|
||||
## Implementation Checklist
|
||||
|
||||
- [ ] REFACTOR-1 propagates authoritative receiving node/generation for RunEvent and tunnel callbacks and binds it atomically to the current registry owner without trusting wire identity.
|
||||
- [ ] REFACTOR-2 validates immutable lease identity, applies sequence-fenced runtime unhealthy/recovery transitions, gates admission/snapshots, annotates every confirmed bound stall for Edge-local recovery (including unknown health), and releases valid terminal leases exactly once.
|
||||
- [ ] Add focused stale-owner, missing identity, mismatch, sequence, recovery, normalized/tunnel, and release-race tests; synchronize contracts/specs without mutating config health semantics.
|
||||
- [ ] Run the focused, package, race, vet, and diff verification commands with fresh output.
|
||||
- [ ] 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_local_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/06+05_health_overlay/` to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/06+05_health_overlay/` 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 transport derives node/generation from the receiving client under registry ownership and stale clients cannot reach correctness callbacks.
|
||||
- Confirm overlay keys and transitions are connection/sequence fenced, config remains immutable, and unknown evidence leaves provider-wide health unchanged while retaining an alternate-provider-only recovery handoff.
|
||||
- Confirm normalized/tunnel terminals release only their bound old lease once and cannot release a newer generation.
|
||||
|
||||
## Verification Results
|
||||
|
||||
> Implementing agent: run each command exactly as written and paste its actual stdout/stderr under `Output`. Record any replacement command and reason in `Deviations from Plan`.
|
||||
|
||||
### Verification 1
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go test -count=1 ./apps/edge/internal/node ./apps/edge/internal/transport ./apps/edge/internal/bootstrap -run 'CurrentOwner|Reception|Lifecycle|Tunnel'
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 2
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go test -count=20 ./apps/edge/internal/service -run '^(TestProviderHealthOverlay|TestReceived.*Failure|Test.*ReleaseOnce)'
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 3
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go test -count=1 ./apps/edge/internal/node ./apps/edge/internal/transport ./apps/edge/internal/bootstrap ./packages/go/streamgate ./apps/edge/internal/openai ./apps/edge/internal/service ./apps/edge/internal/controlplane
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 4
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go test -race -count=3 ./apps/edge/internal/node ./apps/edge/internal/transport ./apps/edge/internal/service
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 5
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go vet ./apps/edge/internal/node ./apps/edge/internal/transport ./apps/edge/internal/bootstrap ./packages/go/streamgate ./apps/edge/internal/openai ./apps/edge/internal/service ./apps/edge/internal/controlplane
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 6
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
git diff --check
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
---
|
||||
|
||||
> **[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,192 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/06+05_health_overlay plan=0 tag=REFACTOR milestone-task=failure-handoff -->
|
||||
|
||||
# Reception-Fenced Provider Health Overlay
|
||||
|
||||
## For the Implementing Agent
|
||||
|
||||
Implement only the items below after the predecessor PASS, run every verification command, and fill all implementation-owned sections of `CODE_REVIEW-cloud-G08.md` with actual notes and raw command output. Keep active files in place and report ready for review; finalization belongs to the code-review skill. If blocked, record exact blocker evidence, attempted commands/output, and resume conditions only. Do not ask the user, call user-input tools, create control-plane stop files, classify the next state, archive logs, or write `complete.log`.
|
||||
|
||||
## Background
|
||||
|
||||
Typed failures are not authoritative merely because they name a node or provider: Edge must bind them to the actual receiving connection and the immutable lease that dispatched the attempt. S04 also requires a generation/sequence-fenced runtime overlay that changes admission and snapshots without mutating config health, while every valid terminal still releases its old lease exactly once.
|
||||
|
||||
## Analysis
|
||||
|
||||
### Files Read
|
||||
|
||||
- `apps/edge/internal/node/registry.go`, `apps/edge/internal/node/registry_test.go`
|
||||
- `apps/edge/internal/transport/server.go`, `apps/edge/internal/transport/connection_handlers.go`, `apps/edge/internal/transport/server_test.go`
|
||||
- `apps/edge/internal/bootstrap/runtime.go`, `apps/edge/internal/bootstrap/runtime_refresh_test.go`
|
||||
- `apps/edge/internal/service/service.go`, `apps/edge/internal/service/provider_tunnel.go`, `apps/edge/internal/service/provider_pool.go`, `apps/edge/internal/service/provider_resolution.go`
|
||||
- `apps/edge/internal/service/model_queue_types.go`, `apps/edge/internal/service/model_queue_admission.go`, `apps/edge/internal/service/model_queue_release.go`, `apps/edge/internal/service/model_queue_snapshot.go`, `apps/edge/internal/service/model_queue_test_support_test.go`, `apps/edge/internal/service/model_queue_admission_test.go`, `apps/edge/internal/service/queue_dispatch_test.go`
|
||||
- `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`
|
||||
- `agent-test/local/edge-smoke.md`
|
||||
|
||||
### SDD Criteria
|
||||
|
||||
- SDD: `agent-roadmap/sdd/operational-observability-provider-management/node-provider-execution-liveness-recovery/SDD.md`; status `[승인됨]`, and this pair's first-line id is `milestone-task=failure-handoff`.
|
||||
- Acceptance Scenario S04 and Evidence Map S04 require absent provider identity, stale connection/sequence, and identity mismatch to leave projection unchanged; only current bound fresh evidence may mark/recover overlay health, and terminal lease release is exactly once. A validated `unknown` probe does not change provider-wide health but still preserves a confirmed request-local stall handoff so the ingress owner may try a different provider.
|
||||
- The S04 transition table fixes semantics: `unavailable` lowers; higher-sequence same-generation `available` recovers prior unavailable; request-stalled/available and unknown do not lower. REFACTOR-1 covers reception/binding, REFACTOR-2 covers transition/admission/snapshot/release, and the final commands include race/ordering fixtures.
|
||||
|
||||
### Verification Context
|
||||
|
||||
- Handoff supplied starting HEAD `56e7d78af3cda4a8d6a85af091ad26bce935f8b6`; baseline package tests passed fresh. This plan assumes `05+04_failure_wire/complete.log` exists and its optional failure fields compile.
|
||||
- Current transport callbacks pass only a message although listener closures retain `*toki.TcpClient`; the registry already owns monotonic connection generations and compare-by-client fencing primitives.
|
||||
- Existing queue leases hold node/provider/generation but omit adapter/target; provider resources hold immutable config capacity/enable plus connection generation but no observed health sequence.
|
||||
- No external verification is needed. Gap is limited to missing reception-aware fixtures and overlay transition/race tests; confidence is high because registry, queue, and transport are all in-process and have deterministic test seams.
|
||||
|
||||
### Test Coverage Gaps
|
||||
|
||||
- Transport proves RunEvent/tunnel separation but not authoritative client/generation context or stale-owner drops.
|
||||
- Queue tests prove generation-fenced capacity and release-once, but not adapter/target identity or health sequence transitions.
|
||||
- Snapshot tests project config/disconnect health only; runtime unhealthy/recovery overlay is absent.
|
||||
|
||||
### Symbol References
|
||||
|
||||
- Preserve existing one-argument `Service.HandleRunLifecycleEvent` and `Service.RouteProviderTunnelFrame` for direct callers/tests. Add reception-aware siblings for bootstrap wiring; no symbol is renamed or removed.
|
||||
- Transport setter callback types change internally; call sites are `apps/edge/internal/bootstrap/runtime.go` and `apps/edge/internal/transport/server_test.go`.
|
||||
|
||||
### Split Judgment
|
||||
|
||||
- Stable predecessor contract: `05+04_failure_wire` supplies optional typed failure fields. Its `complete.log` is currently missing in the active sibling, so implementation must wait for PASS.
|
||||
- This packet supplies immutable reception/binding validation and overlay projection. `07+06_retry_candidate_policy` depends on its candidate eligibility; `08+07_stall_recovery` depends transitively on both.
|
||||
- The packet is cohesive because the same queue lock must order evidence validation, overlay transition, lease release, and the next admission pump; splitting that invariant would create an unsafe intermediate state.
|
||||
|
||||
### Scope Rationale
|
||||
|
||||
Do not create retry intents, pick alternate providers, consume StreamGate budget, add metrics, or mutate Node/config health. Retry selection belongs to 07, OpenAI recovery to 08, and `ops-evidence` is outside this Epic's allowed task ids.
|
||||
|
||||
### Final Routing
|
||||
|
||||
- `evaluation_mode=first-pass`; finalizer=`finalize-task-policy.sh pair`.
|
||||
- Build closure true, scores `(2,2,1,1,2)`, grade G08, route `local-fit` -> `PLAN-local-G08.md`.
|
||||
- Review closure true, scores `(2,2,1,1,2)`, grade G08, route `official-review` -> `CODE_REVIEW-cloud-G08.md` (`codex`, `gpt-5.6-sol`, `xhigh`).
|
||||
- `large_indivisible_context=false`; positive loop risks: `temporal_state`, `concurrent_consistency`, `boundary_contract` (3). `review_rework_count=0`, `evidence_integrity_failure=false`; no capability gap.
|
||||
|
||||
## Implementation Checklist
|
||||
|
||||
- [ ] REFACTOR-1 propagates authoritative receiving node/generation for RunEvent and tunnel callbacks and binds it atomically to the current registry owner without trusting wire identity.
|
||||
- [ ] REFACTOR-2 validates immutable lease identity, applies sequence-fenced runtime unhealthy/recovery transitions, gates admission/snapshots, annotates every confirmed bound stall for Edge-local recovery (including unknown health), and releases valid terminal leases exactly once.
|
||||
- [ ] Add focused stale-owner, missing identity, mismatch, sequence, recovery, normalized/tunnel, and release-race tests; synchronize contracts/specs without mutating config health semantics.
|
||||
- [ ] Run the focused, package, race, vet, and diff verification commands with fresh output.
|
||||
- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.
|
||||
|
||||
### [REFACTOR-1] Carry authoritative reception identity
|
||||
|
||||
**Problem:** `apps/edge/internal/transport/connection_handlers.go:15-54` captures the receiving client but invokes callbacks with only the wire message. A spoofed/stale event can therefore be processed without proving which registered connection delivered it.
|
||||
|
||||
**Solution:** Add an atomic registry lookup that returns a cloned current owner only when the supplied client still owns it. Change transport's internal lifecycle/tunnel callback contracts to include that authoritative node id and generation, drop callbacks from unregistered/stale clients, and wire the new service entry points from bootstrap. Keep observability fanout message-only.
|
||||
|
||||
Before (`apps/edge/internal/transport/connection_handlers.go:29`):
|
||||
|
||||
```go
|
||||
if lifecycle != nil {
|
||||
lifecycle(e)
|
||||
}
|
||||
```
|
||||
|
||||
After:
|
||||
|
||||
```go
|
||||
owner, ok := s.registry.CurrentOwnerForClient(client)
|
||||
if ok && lifecycle != nil {
|
||||
lifecycle(owner.NodeID, owner.ConnectionGeneration, e)
|
||||
}
|
||||
```
|
||||
|
||||
**Modified Files and Checklist:**
|
||||
|
||||
- [ ] `apps/edge/internal/node/registry.go`: add lock-safe current-owner-by-client lookup returning a clone.
|
||||
- [ ] `apps/edge/internal/node/registry_test.go`: prove current owner success and stale/unregistered client rejection across reconnect.
|
||||
- [ ] `apps/edge/internal/transport/server.go`: type reception-aware lifecycle/tunnel callbacks.
|
||||
- [ ] `apps/edge/internal/transport/connection_handlers.go`: resolve current owner at receipt and fail closed for stale clients before correctness callbacks.
|
||||
- [ ] `apps/edge/internal/transport/server_test.go`: assert authoritative node/generation and no callback from stale connection while observability separation remains intact.
|
||||
- [ ] `apps/edge/internal/bootstrap/runtime.go`: wire reception-aware service methods.
|
||||
|
||||
**Test Strategy:** Extend transport/registry fixtures with two clients for one node generation; assert only the live receiver reaches lifecycle/tunnel callbacks and wire metadata cannot substitute another owner.
|
||||
|
||||
**Verification:** `go test -count=1 ./apps/edge/internal/node ./apps/edge/internal/transport ./apps/edge/internal/bootstrap -run 'CurrentOwner|Reception|Lifecycle|Tunnel'` must PASS (Go treats unmatched package patterns as no tests, but every named new test must run in its owning package).
|
||||
|
||||
### [REFACTOR-2] Apply a lease-bound runtime health overlay
|
||||
|
||||
**Problem:** `apps/edge/internal/service/model_queue_types.go:173-186` cannot verify adapter/target, `providerResourceState` at lines 463-477 has no runtime observation state, and `model_queue_admission.go:75-113` plus `model_queue_snapshot.go:47-71` consult only config/connectivity. `Service.HandleRunLifecycleEvent` at `service.go:106-115` releases before classifying typed evidence.
|
||||
|
||||
**Solution:** Extend the immutable lease with dispatch adapter/target and maintain a separate `(node_id, connection_generation, provider_id)` overlay state under the queue lock. For a terminal owned by the receiving generation, compare provider/adapter/target and strictly increasing observation sequence; only `unavailable` sets unhealthy and a later same-generation, higher-sequence `available` clears it. Unknown/request-stalled leaves provider-wide projection unchanged. Attach Edge-local `provider_id`, normalized `provider_health`, and `recovery_eligible=true` to every confirmed, current, identity-bound `response_stalled` terminal, including `unknown`; this marker authorizes only ingress evaluation and never same-provider fallback. Missing stable provider identity, unconfirmed fence, stale owner/sequence, or binding mismatch remains terminal-only. Then release through the existing idempotent lease transition and pump. Apply the same path before routing tunnel ERROR frames.
|
||||
|
||||
Before (`apps/edge/internal/service/service.go:110`):
|
||||
|
||||
```go
|
||||
func (s *Service) HandleRunLifecycleEvent(event *iop.RunEvent) {
|
||||
if event == nil || s.queue == nil || !isTerminalRunEvent(event) { return }
|
||||
s.queue.releaseRun(event.GetRunId(), event.GetType())
|
||||
}
|
||||
```
|
||||
|
||||
After:
|
||||
|
||||
```go
|
||||
func (s *Service) HandleReceivedRunLifecycle(nodeID string, generation uint64, event *iop.RunEvent) {
|
||||
s.queue.applyTerminalEvidenceAndRelease(nodeID, generation, event)
|
||||
}
|
||||
```
|
||||
|
||||
**Modified Files and Checklist:**
|
||||
|
||||
- [ ] `apps/edge/internal/service/model_queue_types.go`: add immutable adapter/target binding and generation-scoped overlay sequence/health state separate from config.
|
||||
- [ ] `apps/edge/internal/service/model_queue_admission.go`: mint full bindings and reject runtime-unhealthy candidates under the existing queue lock.
|
||||
- [ ] `apps/edge/internal/service/model_queue_release.go`: atomically validate terminal evidence, transition overlay, annotate eligible failure, release once, and pump.
|
||||
- [ ] `apps/edge/internal/service/model_queue_snapshot.go`: project effective runtime unhealthy/recovery without changing catalog config.
|
||||
- [ ] `apps/edge/internal/service/service.go`: expose reception-aware normalized lifecycle handling while retaining the compatibility wrapper.
|
||||
- [ ] `apps/edge/internal/service/provider_tunnel.go`: validate/annotate terminal ERROR before request routing; duplicate stream cleanup remains a no-op release.
|
||||
- [ ] `apps/edge/internal/service/provider_health_overlay_test.go`: add the S04 table and normalized/tunnel release-race fixtures.
|
||||
- [ ] `agent-contract/inner/execution-runtime.md`: document lease binding, Edge-local eligibility annotation, and release ordering.
|
||||
- [ ] `agent-contract/inner/edge-node-runtime-wire.md`: document reception identity as out-of-band authority and stale evidence rejection.
|
||||
- [ ] `agent-contract/inner/edge-config-runtime-refresh.md`: distinguish immutable config health from runtime overlay.
|
||||
- [ ] `agent-spec/runtime/edge-node-execution.md`: reflect reception fencing and terminal handoff.
|
||||
- [ ] `agent-spec/runtime/provider-pool-config-refresh.md`: reflect effective admission/snapshot overlay behavior.
|
||||
|
||||
**Test Strategy:** Create table tests for missing provider id, wrong node/provider/adapter/target, stale generation, equal/lower sequence, unavailable transition, unknown/request-stalled overlay no-op, higher-sequence available recovery, and a new connection generation. Prove available, unavailable, and unknown confirmed bound stalls all receive the raw-free request-local handoff marker, while only unavailable/available mutate overlay state. Run normalized and tunnel terminal duplicates concurrently and assert one decrement, no negative count, and no newer lease release.
|
||||
|
||||
**Verification:** `go test -count=20 ./apps/edge/internal/service -run '^(TestProviderHealthOverlay|TestReceived.*Failure|Test.*ReleaseOnce)'` must PASS.
|
||||
|
||||
## Dependencies and Execution Order
|
||||
|
||||
1. `05+04_failure_wire` must first produce `agent-task/m-node-provider-execution-liveness-recovery/05+04_failure_wire/complete.log`; it is active/missing at plan creation.
|
||||
2. Implement REFACTOR-1 before REFACTOR-2. This subtask must PASS before `07+06_retry_candidate_policy` starts.
|
||||
|
||||
## Modified Files Summary
|
||||
|
||||
| File | Item |
|
||||
|------|------|
|
||||
| `apps/edge/internal/node/registry.go` | REFACTOR-1 |
|
||||
| `apps/edge/internal/node/registry_test.go` | REFACTOR-1 |
|
||||
| `apps/edge/internal/transport/server.go` | REFACTOR-1 |
|
||||
| `apps/edge/internal/transport/connection_handlers.go` | REFACTOR-1 |
|
||||
| `apps/edge/internal/transport/server_test.go` | REFACTOR-1 |
|
||||
| `apps/edge/internal/bootstrap/runtime.go` | REFACTOR-1 |
|
||||
| `apps/edge/internal/service/model_queue_types.go` | REFACTOR-2 |
|
||||
| `apps/edge/internal/service/model_queue_admission.go` | REFACTOR-2 |
|
||||
| `apps/edge/internal/service/model_queue_release.go` | REFACTOR-2 |
|
||||
| `apps/edge/internal/service/model_queue_snapshot.go` | REFACTOR-2 |
|
||||
| `apps/edge/internal/service/service.go` | REFACTOR-2 |
|
||||
| `apps/edge/internal/service/provider_tunnel.go` | REFACTOR-2 |
|
||||
| `apps/edge/internal/service/provider_health_overlay_test.go` | REFACTOR-2 |
|
||||
| `agent-contract/inner/execution-runtime.md` | REFACTOR-2 |
|
||||
| `agent-contract/inner/edge-node-runtime-wire.md` | REFACTOR-2 |
|
||||
| `agent-contract/inner/edge-config-runtime-refresh.md` | REFACTOR-2 |
|
||||
| `agent-spec/runtime/edge-node-execution.md` | REFACTOR-2 |
|
||||
| `agent-spec/runtime/provider-pool-config-refresh.md` | REFACTOR-2 |
|
||||
| `agent-task/m-node-provider-execution-liveness-recovery/06+05_health_overlay/CODE_REVIEW-cloud-G08.md` | REFACTOR-1, REFACTOR-2 |
|
||||
|
||||
## Final Verification
|
||||
|
||||
Fresh Go output is required; cached output is not acceptable.
|
||||
|
||||
1. `go test -count=1 ./apps/edge/internal/node ./apps/edge/internal/transport ./apps/edge/internal/bootstrap ./packages/go/streamgate ./apps/edge/internal/openai ./apps/edge/internal/service ./apps/edge/internal/controlplane` — PASS, including the repository Edge local profile packages.
|
||||
2. `go test -count=20 ./apps/edge/internal/service -run '^(TestProviderHealthOverlay|TestReceived.*Failure|Test.*ReleaseOnce)'` — PASS on every iteration.
|
||||
3. `go test -race -count=3 ./apps/edge/internal/node ./apps/edge/internal/transport ./apps/edge/internal/service` — PASS with no race report.
|
||||
4. `go vet ./apps/edge/internal/node ./apps/edge/internal/transport ./apps/edge/internal/bootstrap ./packages/go/streamgate ./apps/edge/internal/openai ./apps/edge/internal/service ./apps/edge/internal/controlplane` — no diagnostics.
|
||||
5. `git diff --check` — no whitespace errors.
|
||||
|
||||
After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`.
|
||||
|
|
@ -0,0 +1,201 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/07+06_retry_candidate_policy plan=1 tag=REFACTOR milestone-task=bounded-retry -->
|
||||
|
||||
# Code Review Reference - REFACTOR
|
||||
|
||||
> **[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-05
|
||||
task=m-node-provider-execution-liveness-recovery/07+06_retry_candidate_policy, plan=1, tag=REFACTOR
|
||||
|
||||
## Archive Evidence Snapshot
|
||||
|
||||
- Prior pair: `plan_local_G05_0.log` and `code_review_cloud_G05_0.log` in this task directory. It was unimplemented and has no official verdict, Required/Suggested/Nit finding, code change, or verification evidence.
|
||||
- Material self-review finding: its selection algorithm was sound, but verification stopped at package tests and did not cover the repository-required deterministic provider-pool full-cycle or live provider-pool preflight/scenario.
|
||||
- Replan carryover: preserve the request-local zero-value-compatible policy and initial/queued parity. Predecessor `06+05_health_overlay` remains active and must produce `complete.log` before implementation.
|
||||
|
||||
|
||||
## 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/07+06_retry_candidate_policy/`. If WARN/FAIL, fully write the next filesystem state required by the code-review skill.
|
||||
4. If PASS and task group is `m-node-provider-execution-liveness-recovery`, preserve the first-line `milestone-task` metadata in `complete.log` and report it for the runtime aggregation event. Roadmap state evaluation belongs to `sync-milestone-workstate`.
|
||||
5. Check applicable `Review-Only Checklist` items at the final `.log` location before reporting.
|
||||
|
||||
---
|
||||
|
||||
## Implementation Item Completion
|
||||
|
||||
| Item | Status |
|
||||
|------|---------|
|
||||
| REFACTOR-1: Prefer an alternate provider without inventing a retry loop | [ ] |
|
||||
|
||||
## Implementation Checklist
|
||||
|
||||
- [ ] REFACTOR-1 adds request-local avoided-provider preference plus explicit same-provider fallback permission to initial and queued provider-pool resolution, using runtime eligibility and preserving zero-value behavior.
|
||||
- [ ] Add focused available/unknown alternate, same-only available, same-only unavailable/unknown, and queued re-resolution tests; synchronize the execution contract/spec.
|
||||
- [ ] Run focused, package, race, vet, provider-only/local-capacity full-cycles, live provider-pool preflight/scenario, and diff verification commands with fresh output.
|
||||
- [ ] 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/07+06_retry_candidate_policy/` to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/07+06_retry_candidate_policy/` 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 zero-value dispatch requests preserve current selection and avoidance state is request-local and never persisted as health.
|
||||
- Confirm initial and queued re-resolution apply identical runtime-eligible alternate preference, and only the explicit available-derived flag permits same-provider fallback.
|
||||
- Confirm exactly one reservation/dispatch occurs and deterministic plus live provider-pool evidence covers the policy.
|
||||
|
||||
## Verification Results
|
||||
|
||||
> Implementing agent: run each command exactly as written and paste its actual stdout/stderr under `Output`. Record any replacement command and reason in `Deviations from Plan`.
|
||||
|
||||
### Verification 1
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go test -count=20 ./apps/edge/internal/service -run '^TestProviderRecoverySelection'
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 2
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go test -count=1 ./packages/go/streamgate ./apps/edge/internal/openai ./apps/edge/internal/service ./apps/edge/internal/controlplane
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 3
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go test -race -count=3 ./apps/edge/internal/service -run '^TestProviderRecoverySelection'
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 4
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go vet ./packages/go/streamgate ./apps/edge/internal/openai ./apps/edge/internal/service ./apps/edge/internal/controlplane
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 5
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
./scripts/e2e-smoke.sh
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 6
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
./scripts/e2e-provider-capacity-smoke.sh
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 7
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
bash scripts/e2e-long-context-admission-smoke.sh --preflight && bash scripts/e2e-long-context-admission-smoke.sh --scenario normal-10
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 8
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
git diff --check
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
---
|
||||
|
||||
> **[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,145 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/07+06_retry_candidate_policy plan=1 tag=REFACTOR milestone-task=bounded-retry -->
|
||||
|
||||
# Request-Local Recovery Candidate Preference
|
||||
|
||||
## For the Implementing Agent
|
||||
|
||||
Implement only this bounded policy after the predecessor PASS, run every verification command, and fill all implementation-owned sections of `CODE_REVIEW-cloud-G06.md` with actual notes and raw output. Keep active files in place and report ready for review; finalization belongs to the code-review skill. If blocked, record exact blocker evidence, commands/output, and resume conditions only. Do not ask the user, call user-input tools, create control-plane stop files, classify the next state, archive logs, or write `complete.log`.
|
||||
|
||||
## Background
|
||||
|
||||
StreamGate can already redispatch through the provider-pool surface, but that request has no way to prefer a provider other than the one that just stalled. S05 requires request-local avoidance on every candidate re-resolution, with same-provider fallback only when no runtime-available alternate exists and the stalled provider has exact probe-backed `available` evidence; this is selection policy, not a new retry counter.
|
||||
|
||||
## Archive Evidence Snapshot
|
||||
|
||||
- Prior pair: `plan_local_G05_0.log` and `code_review_cloud_G05_0.log` in this task directory. It was unimplemented and has no official verdict, Required/Suggested/Nit finding, code change, or verification evidence.
|
||||
- Material self-review finding: its selection algorithm was sound, but verification stopped at package tests and did not cover the repository-required deterministic provider-pool full-cycle or live provider-pool preflight/scenario.
|
||||
- Replan carryover: preserve the request-local zero-value-compatible policy and initial/queued parity. Predecessor `06+05_health_overlay` remains active and must produce `complete.log` before implementation.
|
||||
|
||||
## Analysis
|
||||
|
||||
### Files Read
|
||||
|
||||
- `apps/edge/internal/service/provider_pool.go`, `apps/edge/internal/service/provider_resolution.go`
|
||||
- `apps/edge/internal/service/model_queue_types.go`, `apps/edge/internal/service/model_queue_admission.go`, `apps/edge/internal/service/model_queue_release.go`
|
||||
- `apps/edge/internal/service/provider_pool_admission_test.go`, `apps/edge/internal/service/provider_scheduling_test.go`, `apps/edge/internal/service/model_queue_test_support_test.go`
|
||||
- `apps/edge/internal/openai/stream_gate_dispatcher.go`, `apps/edge/internal/openai/stream_gate_dispatcher_test.go`
|
||||
- `agent-contract/inner/execution-runtime.md`, `agent-spec/runtime/edge-node-execution.md`
|
||||
- `agent-test/local/edge-smoke.md`, `scripts/e2e-smoke.sh`, `scripts/e2e-provider-capacity-smoke.sh`, `scripts/e2e-long-context-admission-smoke.sh`
|
||||
|
||||
### SDD Criteria
|
||||
|
||||
- SDD: `agent-roadmap/sdd/operational-observability-provider-management/node-provider-execution-liveness-recovery/SDD.md`; status `[승인됨]`, and this pair's first-line id is `milestone-task=bounded-retry`.
|
||||
- Acceptance Scenario S05 and Evidence Map S05 require provider-pool failover with bounded dispatch count: exclude the stalled provider for the recovery cycle, but allow it when no alternate exists and that attempt carries exact probe-backed `available` evidence. Unknown health still permits a runtime-eligible alternate; it only forbids falling back to the stalled provider.
|
||||
- This checklist derives request-local avoided-provider and explicit fallback fields, one overlay-aware preference step reused for initial and queued re-resolution, and fixtures for alternate, unavailable/unknown, same-only, and re-resolution behavior.
|
||||
|
||||
### Verification Context
|
||||
|
||||
- Handoff baseline passed fresh at starting HEAD `56e7d78af3cda4a8d6a85af091ad26bce935f8b6`. Implementation waits for `06+05_health_overlay/complete.log`, which supplies runtime-health eligibility under the queue lock.
|
||||
- `ProviderPoolDispatchRequest` currently has capability predicates only. `SubmitProviderPool` duplicates filter application for initial resolution and its queued resolver closure at `provider_pool.go:121-174`.
|
||||
- Existing provider scheduling tests cover priority/capacity/refresh but not request-local avoided-provider preference. Deterministic local verification uses focused/race tests plus `e2e-provider-capacity-smoke.sh`.
|
||||
- External verification preflight was run from `/config/workspace/iop-s1` at HEAD `56e7d78af3cda4a8d6a85af091ad26bce935f8b6`: `bash scripts/e2e-long-context-admission-smoke.sh --preflight` validated `configs/edge.yaml` but returned rc=3 because the dev `/v1/models` and runner-local status endpoints were unreachable. The implementer must rerun from a source-synchronized authorized dev runner with the documented runtime identity/ports and then run an applicable scenario; inability is a verification blocker. Confidence is medium-high because initial and deferred resolution share mutable queue state.
|
||||
|
||||
### Test Coverage Gaps
|
||||
|
||||
- No test asserts alternate-provider preference after a failed attempt.
|
||||
- No test distinguishes same-only probe-available fallback from unavailable/unknown same-only terminal/no admission, or proves unknown health may still select an alternate.
|
||||
- No test asserts the policy survives queued candidate re-resolution.
|
||||
|
||||
### Symbol References
|
||||
|
||||
- No symbol is renamed or removed. `ProviderPoolDispatchRequest` gains two internal request-local fields; real construction sites are in OpenAI StreamGate runtime/dispatcher and test doubles, with both zero values preserving current behavior.
|
||||
|
||||
### Split Judgment
|
||||
|
||||
- Predecessor `06+05_health_overlay` is active with missing `complete.log`; implementation waits for it.
|
||||
- This compact packet's stable contract is: given `AvoidProviderID`, every pool admission prefers a runtime-eligible alternate; only `AllowAvoidedProviderFallback=true` may retain the still-eligible avoided provider when no alternate exists. `08+07_stall_recovery` derives that flag from exact `available` evidence, consumes this contract, and must wait for this packet's PASS.
|
||||
|
||||
### Scope Rationale
|
||||
|
||||
Do not parse stall failures, create StreamGate intents, consume recovery budget, mint run ids, or add health state. This packet exposes only the service-layer selection seam that 08 will populate.
|
||||
|
||||
### Final Routing
|
||||
|
||||
- `evaluation_mode=isolated-reassessment`; finalizer=`finalize-task-policy.sh pair`.
|
||||
- Build closure true, scores `(1,1,1,1,2)`, grade G06, route `local-fit` -> `PLAN-local-G06.md`.
|
||||
- Review closure true, scores `(1,1,1,1,2)`, grade G06, route `official-review` -> `CODE_REVIEW-cloud-G06.md` (`codex`, `gpt-5.6-sol`, `xhigh`).
|
||||
- `large_indivisible_context=false`; positive loop risks: `temporal_state`, `boundary_contract` (2). `review_rework_count=0`, `evidence_integrity_failure=false`; no capability gap.
|
||||
|
||||
## Implementation Checklist
|
||||
|
||||
- [ ] REFACTOR-1 adds request-local avoided-provider preference plus explicit same-provider fallback permission to initial and queued provider-pool resolution, using runtime eligibility and preserving zero-value behavior.
|
||||
- [ ] Add focused available/unknown alternate, same-only available, same-only unavailable/unknown, and queued re-resolution tests; synchronize the execution contract/spec.
|
||||
- [ ] Run focused, package, race, vet, provider-only/local-capacity full-cycles, live provider-pool preflight/scenario, and diff verification commands with fresh output.
|
||||
- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.
|
||||
|
||||
### [REFACTOR-1] Prefer an alternate provider without inventing a retry loop
|
||||
|
||||
**Problem:** `apps/edge/internal/service/provider_pool.go:88-99` has no recovery-cycle candidate hint, and its initial/closure filters at lines 121-174 cannot distinguish the failed provider. A retry may therefore immediately choose the same provider even when a healthy alternate exists.
|
||||
|
||||
**Solution:** Add `AvoidProviderID` and `AllowAvoidedProviderFallback` to the internal dispatch request. After operation and acceptance predicates, use a queue-owned helper over already runtime-eligible candidates: return all alternates whenever one exists; when none exists, retain the avoided provider only if the explicit fallback flag is true and the provider remains runtime eligible. An empty avoid id preserves the current candidate set. Apply the identical helper from the initial path and queued resolver closure. Do not infer fallback permission from current overlay state—only the stalled attempt's exact probe result can grant it—and do not reserve a slot, change provider priority, persist the hints, or count retries in service.
|
||||
|
||||
Before (`apps/edge/internal/service/provider_pool.go:92`):
|
||||
|
||||
```go
|
||||
type ProviderPoolDispatchRequest struct {
|
||||
Run SubmitRunRequest
|
||||
Tunnel SubmitProviderTunnelRequest
|
||||
AcceptCandidate ProviderPoolCandidatePredicate
|
||||
}
|
||||
```
|
||||
|
||||
After:
|
||||
|
||||
```go
|
||||
type ProviderPoolDispatchRequest struct {
|
||||
Run SubmitRunRequest
|
||||
Tunnel SubmitProviderTunnelRequest
|
||||
AcceptCandidate ProviderPoolCandidatePredicate
|
||||
AvoidProviderID string
|
||||
AllowAvoidedProviderFallback bool
|
||||
}
|
||||
```
|
||||
|
||||
**Modified Files and Checklist:**
|
||||
|
||||
- [ ] `apps/edge/internal/service/provider_pool.go`: define both zero-value-compatible hints and one shared filtering pipeline used by first resolution and re-resolution.
|
||||
- [ ] `apps/edge/internal/service/model_queue_admission.go`: add a lock-safe, non-reserving helper that applies alternate preference and the explicit same-provider fallback permission after normal runtime eligibility.
|
||||
- [ ] `apps/edge/internal/service/provider_recovery_selection_test.go`: cover available and unknown alternate preference, same-only explicit fallback, same-only unavailable/unknown rejection, zero-value behavior, and an overlay change before queued re-resolution.
|
||||
- [ ] `agent-contract/inner/execution-runtime.md`: document request-local avoidance and the no-counter/no-persistence boundary.
|
||||
- [ ] `agent-spec/runtime/edge-node-execution.md`: record provider-pool recovery candidate selection behavior.
|
||||
|
||||
**Test Strategy:** Write table-driven service tests with two providers on one/two nodes and explicit overlay states from the predecessor. Prove an unknown stalled-provider probe with a healthy alternate selects that alternate, while unknown or unavailable same-only requests reject admission; only the explicit available-derived flag permits same-only fallback. For queue re-resolution, hold capacity, enqueue with both hints, change overlay/capacity, release, and assert the admitted provider. Assert exactly one dispatch/reservation.
|
||||
|
||||
**Verification:** `go test -count=20 ./apps/edge/internal/service -run '^TestProviderRecoverySelection'` must PASS every iteration.
|
||||
|
||||
## Dependencies and Execution Order
|
||||
|
||||
1. `06+05_health_overlay` must produce `agent-task/m-node-provider-execution-liveness-recovery/06+05_health_overlay/complete.log`; it is active/missing at plan creation.
|
||||
2. This packet must produce `complete.log` before `08+07_stall_recovery` starts.
|
||||
|
||||
## Modified Files Summary
|
||||
|
||||
| File | Item |
|
||||
|------|------|
|
||||
| `apps/edge/internal/service/provider_pool.go` | REFACTOR-1 |
|
||||
| `apps/edge/internal/service/model_queue_admission.go` | REFACTOR-1 |
|
||||
| `apps/edge/internal/service/provider_recovery_selection_test.go` | REFACTOR-1 |
|
||||
| `agent-contract/inner/execution-runtime.md` | REFACTOR-1 |
|
||||
| `agent-spec/runtime/edge-node-execution.md` | REFACTOR-1 |
|
||||
| `agent-task/m-node-provider-execution-liveness-recovery/07+06_retry_candidate_policy/CODE_REVIEW-cloud-G06.md` | REFACTOR-1 |
|
||||
|
||||
## Final Verification
|
||||
|
||||
Fresh Go output is required; cached output is not acceptable.
|
||||
|
||||
1. `go test -count=20 ./apps/edge/internal/service -run '^TestProviderRecoverySelection'` — PASS every iteration.
|
||||
2. `go test -count=1 ./packages/go/streamgate ./apps/edge/internal/openai ./apps/edge/internal/service ./apps/edge/internal/controlplane` — PASS under the Edge local profile and confirms zero-value request compatibility.
|
||||
3. `go test -race -count=3 ./apps/edge/internal/service -run '^TestProviderRecoverySelection'` — PASS with no race report.
|
||||
4. `go vet ./packages/go/streamgate ./apps/edge/internal/openai ./apps/edge/internal/service ./apps/edge/internal/controlplane` — no diagnostics.
|
||||
5. `./scripts/e2e-smoke.sh` — PASS for provider-only queue/reconnect fencing with zero-value requests.
|
||||
6. `./scripts/e2e-provider-capacity-smoke.sh` — PASS for deterministic local provider-pool queue/release behavior.
|
||||
7. `bash scripts/e2e-long-context-admission-smoke.sh --preflight && bash scripts/e2e-long-context-admission-smoke.sh --scenario normal-10` — PASS on the authorized synchronized dev runner; if endpoint/runtime identity remains blocked, preserve exact output and do not claim completion.
|
||||
8. `git diff --check` — no whitespace errors.
|
||||
|
||||
After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`.
|
||||
|
|
@ -0,0 +1,161 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/07+06_retry_candidate_policy plan=0 tag=REFACTOR milestone-task=bounded-retry -->
|
||||
|
||||
# Code Review Reference - REFACTOR
|
||||
|
||||
> **[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-05
|
||||
task=m-node-provider-execution-liveness-recovery/07+06_retry_candidate_policy, plan=0, tag=REFACTOR
|
||||
|
||||
|
||||
|
||||
## 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-G05.md` → `code_review_cloud_G05_0.log` and `PLAN-local-G05.md` → `plan_local_G05_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/07+06_retry_candidate_policy/`. If WARN/FAIL, fully write the next filesystem state required by the code-review skill.
|
||||
4. If PASS and task group is `m-node-provider-execution-liveness-recovery`, preserve the first-line `milestone-task` metadata in `complete.log` and report it for the runtime aggregation event. Roadmap state evaluation belongs to `sync-milestone-workstate`.
|
||||
5. Check applicable `Review-Only Checklist` items at the final `.log` location before reporting.
|
||||
|
||||
---
|
||||
|
||||
## Implementation Item Completion
|
||||
|
||||
| Item | Status |
|
||||
|------|---------|
|
||||
| REFACTOR-1: Request-local recovery candidate preference | [ ] |
|
||||
|
||||
## Implementation Checklist
|
||||
|
||||
- [ ] REFACTOR-1 adds request-local avoided-provider preference plus explicit same-provider fallback permission to initial and queued provider-pool resolution, using runtime eligibility and preserving zero-value behavior.
|
||||
- [ ] Add focused available/unknown alternate, same-only available, same-only unavailable/unknown, and queued re-resolution tests; synchronize the execution contract/spec.
|
||||
- [ ] Run focused, package, race, vet, and diff verification commands with fresh output.
|
||||
- [ ] 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_G05_0.log`.
|
||||
- [ ] Archive active `PLAN-*-G??.md` to `plan_local_G05_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/07+06_retry_candidate_policy/` to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/07+06_retry_candidate_policy/` 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 `AvoidProviderID` and `AllowAvoidedProviderFallback` are request-local, zero-value compatible, non-persistent, and do not create a retry counter.
|
||||
- Confirm initial and queued re-resolution use the identical overlay-aware preference rule.
|
||||
- Confirm unknown health may select an alternate, while same-provider fallback occurs only when no runtime-eligible alternate exists, the explicit available-derived flag is true, and the same provider remains runtime eligible.
|
||||
|
||||
## Verification Results
|
||||
|
||||
> Implementing agent: run each command exactly as written and paste its actual stdout/stderr under `Output`. Record any replacement command and reason in `Deviations from Plan`.
|
||||
|
||||
### Verification 1
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go test -count=20 ./apps/edge/internal/service -run '^TestProviderRecoverySelection'
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 2
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go test -count=1 ./packages/go/streamgate ./apps/edge/internal/openai ./apps/edge/internal/service ./apps/edge/internal/controlplane
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 3
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go test -race -count=3 ./apps/edge/internal/service -run '^TestProviderRecoverySelection'
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 4
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go vet ./packages/go/streamgate ./apps/edge/internal/openai ./apps/edge/internal/service ./apps/edge/internal/controlplane
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 5
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
git diff --check
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
---
|
||||
|
||||
> **[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,135 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/07+06_retry_candidate_policy plan=0 tag=REFACTOR milestone-task=bounded-retry -->
|
||||
|
||||
# Request-Local Recovery Candidate Preference
|
||||
|
||||
## For the Implementing Agent
|
||||
|
||||
Implement only this bounded policy after the predecessor PASS, run every verification command, and fill all implementation-owned sections of `CODE_REVIEW-cloud-G05.md` with actual notes and raw output. Keep active files in place and report ready for review; finalization belongs to the code-review skill. If blocked, record exact blocker evidence, commands/output, and resume conditions only. Do not ask the user, call user-input tools, create control-plane stop files, classify the next state, archive logs, or write `complete.log`.
|
||||
|
||||
## Background
|
||||
|
||||
StreamGate can already redispatch through the provider-pool surface, but that request has no way to prefer a provider other than the one that just stalled. S05 requires request-local avoidance on every candidate re-resolution, with same-provider fallback only when no runtime-available alternate exists and the stalled provider has exact probe-backed `available` evidence; this is selection policy, not a new retry counter.
|
||||
|
||||
## Analysis
|
||||
|
||||
### Files Read
|
||||
|
||||
- `apps/edge/internal/service/provider_pool.go`, `apps/edge/internal/service/provider_resolution.go`
|
||||
- `apps/edge/internal/service/model_queue_types.go`, `apps/edge/internal/service/model_queue_admission.go`, `apps/edge/internal/service/model_queue_release.go`
|
||||
- `apps/edge/internal/service/provider_pool_admission_test.go`, `apps/edge/internal/service/provider_scheduling_test.go`, `apps/edge/internal/service/model_queue_test_support_test.go`
|
||||
- `apps/edge/internal/openai/stream_gate_dispatcher.go`, `apps/edge/internal/openai/stream_gate_dispatcher_test.go`
|
||||
- `agent-contract/inner/execution-runtime.md`, `agent-spec/runtime/edge-node-execution.md`
|
||||
- `agent-test/local/edge-smoke.md`
|
||||
|
||||
### SDD Criteria
|
||||
|
||||
- SDD: `agent-roadmap/sdd/operational-observability-provider-management/node-provider-execution-liveness-recovery/SDD.md`; status `[승인됨]`, and this pair's first-line id is `milestone-task=bounded-retry`.
|
||||
- Acceptance Scenario S05 and Evidence Map S05 require provider-pool failover with bounded dispatch count: exclude the stalled provider for the recovery cycle, but allow it when no alternate exists and that attempt carries exact probe-backed `available` evidence. Unknown health still permits a runtime-eligible alternate; it only forbids falling back to the stalled provider.
|
||||
- This checklist derives request-local avoided-provider and explicit fallback fields, one overlay-aware preference step reused for initial and queued re-resolution, and fixtures for alternate, unavailable/unknown, same-only, and re-resolution behavior.
|
||||
|
||||
### Verification Context
|
||||
|
||||
- Handoff baseline passed fresh at starting HEAD `56e7d78af3cda4a8d6a85af091ad26bce935f8b6`. Implementation waits for `06+05_health_overlay/complete.log`, which supplies runtime-health eligibility under the queue lock.
|
||||
- `ProviderPoolDispatchRequest` currently has capability predicates only. `SubmitProviderPool` duplicates filter application for initial resolution and its queued resolver closure at `provider_pool.go:121-174`.
|
||||
- No external runner is needed. Gap: existing provider scheduling tests cover priority/capacity/refresh but not request-local avoided-provider preference. Confidence is high because this packet changes only internal request state and deterministic queue selection.
|
||||
|
||||
### Test Coverage Gaps
|
||||
|
||||
- No test asserts alternate-provider preference after a failed attempt.
|
||||
- No test distinguishes same-only probe-available fallback from unavailable/unknown same-only terminal/no admission, or proves unknown health may still select an alternate.
|
||||
- No test asserts the policy survives queued candidate re-resolution.
|
||||
|
||||
### Symbol References
|
||||
|
||||
- No symbol is renamed or removed. `ProviderPoolDispatchRequest` gains two internal request-local fields; real construction sites are in OpenAI StreamGate runtime/dispatcher and test doubles, with both zero values preserving current behavior.
|
||||
|
||||
### Split Judgment
|
||||
|
||||
- Predecessor `06+05_health_overlay` is active with missing `complete.log`; implementation waits for it.
|
||||
- This compact packet's stable contract is: given `AvoidProviderID`, every pool admission prefers a runtime-eligible alternate; only `AllowAvoidedProviderFallback=true` may retain the still-eligible avoided provider when no alternate exists. `08+07_stall_recovery` derives that flag from exact `available` evidence, consumes this contract, and must wait for this packet's PASS.
|
||||
|
||||
### Scope Rationale
|
||||
|
||||
Do not parse stall failures, create StreamGate intents, consume recovery budget, mint run ids, or add health state. This packet exposes only the service-layer selection seam that 08 will populate.
|
||||
|
||||
### Final Routing
|
||||
|
||||
- `evaluation_mode=first-pass`; finalizer=`finalize-task-policy.sh pair`.
|
||||
- Build closure true, scores `(1,1,1,1,1)`, grade G05, route `local-fit` -> `PLAN-local-G05.md`.
|
||||
- Review closure true, scores `(1,1,1,1,1)`, grade G05, route `official-review` -> `CODE_REVIEW-cloud-G05.md` (`codex`, `gpt-5.6-sol`, `xhigh`).
|
||||
- `large_indivisible_context=false`; positive loop risks: `temporal_state`, `boundary_contract` (2). `review_rework_count=0`, `evidence_integrity_failure=false`; no capability gap.
|
||||
|
||||
## Implementation Checklist
|
||||
|
||||
- [ ] REFACTOR-1 adds request-local avoided-provider preference plus explicit same-provider fallback permission to initial and queued provider-pool resolution, using runtime eligibility and preserving zero-value behavior.
|
||||
- [ ] Add focused available/unknown alternate, same-only available, same-only unavailable/unknown, and queued re-resolution tests; synchronize the execution contract/spec.
|
||||
- [ ] Run focused, package, race, vet, and diff verification commands with fresh output.
|
||||
- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.
|
||||
|
||||
### [REFACTOR-1] Prefer an alternate provider without inventing a retry loop
|
||||
|
||||
**Problem:** `apps/edge/internal/service/provider_pool.go:88-99` has no recovery-cycle candidate hint, and its initial/closure filters at lines 121-174 cannot distinguish the failed provider. A retry may therefore immediately choose the same provider even when a healthy alternate exists.
|
||||
|
||||
**Solution:** Add `AvoidProviderID` and `AllowAvoidedProviderFallback` to the internal dispatch request. After operation and acceptance predicates, use a queue-owned helper over already runtime-eligible candidates: return all alternates whenever one exists; when none exists, retain the avoided provider only if the explicit fallback flag is true and the provider remains runtime eligible. An empty avoid id preserves the current candidate set. Apply the identical helper from the initial path and queued resolver closure. Do not infer fallback permission from current overlay state—only the stalled attempt's exact probe result can grant it—and do not reserve a slot, change provider priority, persist the hints, or count retries in service.
|
||||
|
||||
Before (`apps/edge/internal/service/provider_pool.go:92`):
|
||||
|
||||
```go
|
||||
type ProviderPoolDispatchRequest struct {
|
||||
Run SubmitRunRequest
|
||||
Tunnel SubmitProviderTunnelRequest
|
||||
AcceptCandidate ProviderPoolCandidatePredicate
|
||||
}
|
||||
```
|
||||
|
||||
After:
|
||||
|
||||
```go
|
||||
type ProviderPoolDispatchRequest struct {
|
||||
Run SubmitRunRequest
|
||||
Tunnel SubmitProviderTunnelRequest
|
||||
AcceptCandidate ProviderPoolCandidatePredicate
|
||||
AvoidProviderID string
|
||||
AllowAvoidedProviderFallback bool
|
||||
}
|
||||
```
|
||||
|
||||
**Modified Files and Checklist:**
|
||||
|
||||
- [ ] `apps/edge/internal/service/provider_pool.go`: define both zero-value-compatible hints and one shared filtering pipeline used by first resolution and re-resolution.
|
||||
- [ ] `apps/edge/internal/service/model_queue_admission.go`: add a lock-safe, non-reserving helper that applies alternate preference and the explicit same-provider fallback permission after normal runtime eligibility.
|
||||
- [ ] `apps/edge/internal/service/provider_recovery_selection_test.go`: cover available and unknown alternate preference, same-only explicit fallback, same-only unavailable/unknown rejection, zero-value behavior, and an overlay change before queued re-resolution.
|
||||
- [ ] `agent-contract/inner/execution-runtime.md`: document request-local avoidance and the no-counter/no-persistence boundary.
|
||||
- [ ] `agent-spec/runtime/edge-node-execution.md`: record provider-pool recovery candidate selection behavior.
|
||||
|
||||
**Test Strategy:** Write table-driven service tests with two providers on one/two nodes and explicit overlay states from the predecessor. Prove an unknown stalled-provider probe with a healthy alternate selects that alternate, while unknown or unavailable same-only requests reject admission; only the explicit available-derived flag permits same-only fallback. For queue re-resolution, hold capacity, enqueue with both hints, change overlay/capacity, release, and assert the admitted provider. Assert exactly one dispatch/reservation.
|
||||
|
||||
**Verification:** `go test -count=20 ./apps/edge/internal/service -run '^TestProviderRecoverySelection'` must PASS every iteration.
|
||||
|
||||
## Dependencies and Execution Order
|
||||
|
||||
1. `06+05_health_overlay` must produce `agent-task/m-node-provider-execution-liveness-recovery/06+05_health_overlay/complete.log`; it is active/missing at plan creation.
|
||||
2. This packet must produce `complete.log` before `08+07_stall_recovery` starts.
|
||||
|
||||
## Modified Files Summary
|
||||
|
||||
| File | Item |
|
||||
|------|------|
|
||||
| `apps/edge/internal/service/provider_pool.go` | REFACTOR-1 |
|
||||
| `apps/edge/internal/service/model_queue_admission.go` | REFACTOR-1 |
|
||||
| `apps/edge/internal/service/provider_recovery_selection_test.go` | REFACTOR-1 |
|
||||
| `agent-contract/inner/execution-runtime.md` | REFACTOR-1 |
|
||||
| `agent-spec/runtime/edge-node-execution.md` | REFACTOR-1 |
|
||||
| `agent-task/m-node-provider-execution-liveness-recovery/07+06_retry_candidate_policy/CODE_REVIEW-cloud-G05.md` | REFACTOR-1 |
|
||||
|
||||
## Final Verification
|
||||
|
||||
Fresh Go output is required; cached output is not acceptable.
|
||||
|
||||
1. `go test -count=20 ./apps/edge/internal/service -run '^TestProviderRecoverySelection'` — PASS every iteration.
|
||||
2. `go test -count=1 ./packages/go/streamgate ./apps/edge/internal/openai ./apps/edge/internal/service ./apps/edge/internal/controlplane` — PASS under the Edge local profile and confirms zero-value request compatibility.
|
||||
3. `go test -race -count=3 ./apps/edge/internal/service -run '^TestProviderRecoverySelection'` — PASS with no race report.
|
||||
4. `go vet ./packages/go/streamgate ./apps/edge/internal/openai ./apps/edge/internal/service ./apps/edge/internal/controlplane` — no diagnostics.
|
||||
5. `git diff --check` — no whitespace errors.
|
||||
|
||||
After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`.
|
||||
|
|
@ -0,0 +1,229 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/08+07_stall_recovery plan=1 tag=API milestone-task=bounded-retry -->
|
||||
|
||||
# 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-05
|
||||
task=m-node-provider-execution-liveness-recovery/08+07_stall_recovery, plan=1, tag=API
|
||||
|
||||
## Archive Evidence Snapshot
|
||||
|
||||
- Prior pair: `plan_cloud_G08_0.log` and `code_review_cloud_G08_0.log` in this task directory. It was unimplemented and has no official verdict, Required/Suggested/Nit finding, code change, or verification evidence.
|
||||
- Material self-review finding: the prior plan overloaded the configurable `provider_error` semantic filter even though `openai.stream_evidence_gate.filters[]` is optional; a gate-enabled request without that configured filter would have no stall recovery owner. It also omitted the required OpenAI/provider-pool full-cycle verification.
|
||||
- Replan carryover: keep Core budget/commit/cancel ownership and the S05 matrix, but register a dedicated request-local liveness filter whenever StreamGate is enabled, independent of configurable semantic filters and provider capabilities. Gate-disabled/unsupported surfaces remain the explicit no-owner terminal boundary.
|
||||
|
||||
|
||||
## 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/08+07_stall_recovery/`. If WARN/FAIL, fully write the next filesystem state required by the code-review skill.
|
||||
4. If PASS and task group is `m-node-provider-execution-liveness-recovery`, preserve the first-line `milestone-task` metadata in `complete.log` and report it for the runtime aggregation event. Roadmap state evaluation belongs to `sync-milestone-workstate`.
|
||||
5. Check applicable `Review-Only Checklist` items at the final `.log` location before reporting.
|
||||
|
||||
---
|
||||
|
||||
## Implementation Item Completion
|
||||
|
||||
| Item | Status |
|
||||
|------|---------|
|
||||
| API-1: Convert typed execution stalls into raw-free StreamGate events | [ ] |
|
||||
| API-2: Gate exact replay and hand off the failed provider | [ ] |
|
||||
| API-3: Prove bounded recovery across OpenAI variants | [ ] |
|
||||
|
||||
## Implementation Checklist
|
||||
|
||||
- [ ] API-1 preserves typed normalized/buffered/tunnel stalls as one raw-free StreamGate `response_stalled` provider error, retaining only sanitized fence/health/handoff tokens while generic failures keep existing terminal behavior.
|
||||
- [ ] API-2 installs one internal liveness recovery filter for every StreamGate-enabled request independent of configured semantic filters/capabilities; only confirmed Edge-eligible, uncommitted, side-effect-safe stalls produce ExactReplay, close the fenced old transport, and hand the failed provider/fallback evidence to admission.
|
||||
- [ ] API-3 adds Chat/Responses normalized/tunnel fixtures for available, unavailable, and unknown alternate recovery; available-only same-provider fallback; unavailable/unknown same-only terminal; no-owner, post-commit, unconfirmed, cancel/tool-side-effect, and shared-budget exhaustion; synchronize contracts/specs.
|
||||
- [ ] Run focused, package, race, vet, provider-only/OpenAI/local-capacity full-cycles, live provider-pool preflight/scenario, and diff verification with fresh output; assert new identities plus exactly one terminal/dispatch per allowed cycle.
|
||||
- [ ] 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/08+07_stall_recovery/` to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/08+07_stall_recovery/` 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 typed normalized/buffered/tunnel stalls map to one raw-free descriptor while generic failures retain current terminal behavior.
|
||||
- Confirm every StreamGate-enabled request gets exactly one private liveness filter independent of configured filters/capabilities, while gate-disabled/unsupported ingress remains no-owner terminal.
|
||||
- Confirm commit/cancel/side-effect/fence/shared-budget gates, confirmed-terminal close, failed-provider handoff, available-only fallback, new run identity, and single dispatch/terminal across Chat/Responses variants.
|
||||
- Confirm focused/race tests plus provider-only, fake-vLLM, local-capacity, and live provider-pool evidence satisfy the final commands.
|
||||
|
||||
## Verification Results
|
||||
|
||||
> Implementing agent: run each command exactly as written and paste its actual stdout/stderr under `Output`. Record any replacement command and reason in `Deviations from Plan`.
|
||||
|
||||
### Verification 1
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go test -count=20 ./apps/edge/internal/openai -run '^(TestOpenAIStallRecoveryFilter|TestOpenAIAttemptControllerConfirmedStall|TestOpenAIAttemptDispatcherStalledProvider)'
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 2
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go test -count=10 ./apps/edge/internal/openai -run '^TestOpenAIStallRecovery'
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 3
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go test -count=1 ./packages/go/streamgate ./apps/edge/internal/openai ./apps/edge/internal/service ./apps/edge/internal/controlplane
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 4
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go test -race -count=3 ./apps/edge/internal/service ./apps/edge/internal/openai
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 5
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go vet ./packages/go/streamgate ./apps/edge/internal/service ./apps/edge/internal/openai ./apps/edge/internal/controlplane
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 6
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
./scripts/e2e-smoke.sh
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 7
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
IOP_VLLM_MODE=fake ./scripts/e2e-openai-vllm.sh
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 8
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
./scripts/e2e-provider-capacity-smoke.sh
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 9
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
bash scripts/e2e-long-context-admission-smoke.sh --preflight && bash scripts/e2e-long-context-admission-smoke.sh --scenario normal-10
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 10
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
git diff --check
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
---
|
||||
|
||||
> **[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,211 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/08+07_stall_recovery plan=1 tag=API milestone-task=bounded-retry -->
|
||||
|
||||
# OpenAI Typed Stall Recovery Handoff
|
||||
|
||||
## For the Implementing Agent
|
||||
|
||||
Implement only the items below after all predecessors PASS, run every verification command, and fill all implementation-owned sections of `CODE_REVIEW-cloud-G08.md` with actual notes and raw output. Keep active files in place and report ready for review; finalization belongs to the code-review skill. If blocked, record exact blocker evidence, attempted commands/output, and resume conditions only. Do not ask the user, call user-input tools, create control-plane stop files, classify the next state, archive logs, or write `complete.log`.
|
||||
|
||||
## Background
|
||||
|
||||
The OpenAI host currently maps every normalized/tunnel terminal error to generic StreamGate provider errors, and the provider-error filter intentionally never constructs recovery. S05 requires a typed `response_stalled` handoff that remains terminal unless Edge confirmed the attempt fence and the request is uncommitted, uncanceled, side-effect-safe, and within the existing shared Core budget; eligible recovery must use a new run identity and prefer another provider.
|
||||
|
||||
## Archive Evidence Snapshot
|
||||
|
||||
- Prior pair: `plan_cloud_G08_0.log` and `code_review_cloud_G08_0.log` in this task directory. It was unimplemented and has no official verdict, Required/Suggested/Nit finding, code change, or verification evidence.
|
||||
- Material self-review finding: the prior plan overloaded the configurable `provider_error` semantic filter even though `openai.stream_evidence_gate.filters[]` is optional; a gate-enabled request without that configured filter would have no stall recovery owner. It also omitted the required OpenAI/provider-pool full-cycle verification.
|
||||
- Replan carryover: keep Core budget/commit/cancel ownership and the S05 matrix, but register a dedicated request-local liveness filter whenever StreamGate is enabled, independent of configurable semantic filters and provider capabilities. Gate-disabled/unsupported surfaces remain the explicit no-owner terminal boundary.
|
||||
|
||||
## Analysis
|
||||
|
||||
### Files Read
|
||||
|
||||
- `apps/edge/internal/openai/stream_gate_ingress.go`, `apps/edge/internal/openai/stream_gate_policy.go`, `apps/edge/internal/openai/stream_gate_filters.go`, `apps/edge/internal/openai/stream_gate_runtime.go`
|
||||
- `apps/edge/internal/openai/stream_gate_dispatcher.go`, `apps/edge/internal/openai/run_result.go`, `apps/edge/internal/openai/responses_stream_gate.go`, `apps/edge/internal/openai/chat_completion.go`
|
||||
- `apps/edge/internal/openai/stream_gate_filters_test.go`, `apps/edge/internal/openai/stream_gate_dispatcher_test.go`, `apps/edge/internal/openai/stream_gate_pipeline_test.go`, `apps/edge/internal/openai/stream_gate_vertical_slice_test.go`
|
||||
- `apps/edge/internal/openai/provider_test_support_test.go`, `apps/edge/internal/openai/server_test_support_test.go`
|
||||
- `packages/go/streamgate/runtime.go`, `packages/go/streamgate/recovery_coordinator.go`, `packages/go/streamgate/recovery_plan.go`, `packages/go/streamgate/commit_boundary.go`, `packages/go/streamgate/event.go`, `packages/go/streamgate/filter_registry.go`, `packages/go/streamgate/terminal.go`
|
||||
- `packages/go/config/config.go`, `agent-contract/inner/edge-config-runtime-refresh.md`, `agent-contract/inner/execution-runtime.md`, `agent-contract/outer/openai-compatible-api.md`
|
||||
- `agent-spec/runtime/stream-evidence-gate.md`, `agent-spec/input/openai-compatible-surface.md`, `agent-spec/runtime/edge-node-execution.md`
|
||||
- `agent-test/local/edge-smoke.md`, `agent-test/local/platform-common-smoke.md`, `scripts/e2e-smoke.sh`, `scripts/e2e-openai-vllm.sh`, `scripts/e2e-provider-capacity-smoke.sh`, `scripts/e2e-long-context-admission-smoke.sh`
|
||||
|
||||
### SDD Criteria
|
||||
|
||||
- SDD: `agent-roadmap/sdd/operational-observability-provider-management/node-provider-execution-liveness-recovery/SDD.md`; status `[승인됨]`, and this pair's first-line id is `milestone-task=bounded-retry`.
|
||||
- Acceptance Scenario S05 and Evidence Map S05 require healthy request stall, unhealthy failover, unknown probe, same-provider-only, no owner, post-commit, unconfirmed fence, caller cancel/tool-side-effect, and shared-budget fixtures with a new run identity and bounded dispatch/terminal count. Unknown health may recover through an alternate candidate but never grants same-provider fallback.
|
||||
- API-1 derives a raw-free typed event, API-2 implements exact-replay eligibility/fence teardown/provider handoff, and API-3 proves both Chat/Responses plus normalized/tunnel variants against those rows.
|
||||
|
||||
### Verification Context
|
||||
|
||||
- Handoff supplied starting HEAD `56e7d78af3cda4a8d6a85af091ad26bce935f8b6`; fresh baseline tests passed for execution, Node, Edge transport/service/OpenAI.
|
||||
- This plan waits for `07+06_retry_candidate_policy/complete.log`; transitively, typed wire and Edge eligibility/overlay contracts are also PASS before implementation.
|
||||
- Core already owns `transport_uncommitted`, caller-cancel propagation, abort-before-dispatch, new `AttemptBinding`, and one request-local total/per-strategy fault budget. No liveness counter or Core recovery coordinator is needed.
|
||||
- Existing OpenAI event sources discard typed failures (`stream_gate_runtime.go:128-188`, `402-478`, `responses_stream_gate.go:110-135`) and `collectRunResult` flattens them (`run_result.go:87-95`). Provider-error filtering is explicitly foundation-only (`stream_gate_filters.go:29-32`, `198-237`).
|
||||
- `openai.stream_evidence_gate.enabled` defaults false and `filters[]` is optional; configured filters alone therefore cannot own the S05 handoff. The host must add a private liveness registration only to enabled request runtimes, without adding a configured filter/capability requirement or changing generic `provider_error` behavior.
|
||||
- External provider-pool preflight was run from `/config/workspace/iop-s1` at HEAD `56e7d78af3cda4a8d6a85af091ad26bce935f8b6`: `bash scripts/e2e-long-context-admission-smoke.sh --preflight` validated `configs/edge.yaml` but returned rc=3 because the dev `/v1/models` and runner-local status endpoints were unreachable. The implementer must rerun on a source-synchronized authorized dev runner and run an applicable scenario; local fake-vLLM and capacity smokes remain mandatory deterministic evidence. Confidence is medium-high due to multi-variant runtime and terminal ordering.
|
||||
|
||||
### Test Coverage Gaps
|
||||
|
||||
- Typed normalized and tunnel stalls are not distinguished from generic provider errors.
|
||||
- Provider-error filter has only unmatched PASS coverage; no test proves that configured-filter absence still installs exactly one internal liveness owner or that gate-disabled requests remain no-owner terminal.
|
||||
- Recovery dispatcher does not hand the failed provider to pool admission or distinguish a confirmed Node terminal from a still-running attempt during abort.
|
||||
- Existing vertical slices prove shared-budget recovery generally, but not Chat/Responses stall variants, unknown/no-owner/post-commit/unconfirmed gates, or duplicate-terminal absence.
|
||||
|
||||
### Symbol References
|
||||
|
||||
- No symbol is renamed or removed. Constructor signatures for request-local dispatcher/filter wiring may gain internal state; update all call sites in `stream_gate_runtime.go`, `responses_stream_gate.go`, and their direct tests.
|
||||
|
||||
### Split Judgment
|
||||
|
||||
- Predecessor `07+06_retry_candidate_policy` is active with missing `complete.log`; implementation waits for it. That predecessor transitively requires `05+04_failure_wire` and `06+05_health_overlay` PASS.
|
||||
- This final packet is indivisible at the OpenAI host boundary: the same raw-free eligibility token must drive filter intent, confirmed-terminal teardown, failed-provider handoff, and endpoint terminal rendering. Partial wiring could either duplicate dispatch or authorize an unfenced replay.
|
||||
|
||||
### Scope Rationale
|
||||
|
||||
Do not add a Core/Node/Edge retry loop, new counter, schema, metric, non-OpenAI recovery owner, or retry to legacy surfaces. Do not expose raw provider messages/metadata. StreamGate Core behavior remains unchanged; this packet consumes its existing budget/commit/cancel/side-effect contracts.
|
||||
|
||||
### Final Routing
|
||||
|
||||
- `evaluation_mode=isolated-reassessment`; finalizer=`finalize-task-policy.sh pair`.
|
||||
- Build closure true, scores `(2,2,1,1,2)`, grade G08, base `local-fit`, escalated by `risk-boundary` -> `PLAN-cloud-G08.md`.
|
||||
- Review closure true, scores `(2,2,1,1,2)`, grade G08, route `official-review` -> `CODE_REVIEW-cloud-G08.md` (`codex`, `gpt-5.6-sol`, `xhigh`).
|
||||
- `large_indivisible_context=false`; positive loop risks: `temporal_state`, `concurrent_consistency`, `boundary_contract`, `variant_product` (4). `review_rework_count=0`, `evidence_integrity_failure=false`; no capability gap.
|
||||
|
||||
## Implementation Checklist
|
||||
|
||||
- [ ] API-1 preserves typed normalized/buffered/tunnel stalls as one raw-free StreamGate `response_stalled` provider error, retaining only sanitized fence/health/handoff tokens while generic failures keep existing terminal behavior.
|
||||
- [ ] API-2 installs one internal liveness recovery filter for every StreamGate-enabled request independent of configured semantic filters/capabilities; only confirmed Edge-eligible, uncommitted, side-effect-safe stalls produce ExactReplay, close the fenced old transport, and hand the failed provider/fallback evidence to admission.
|
||||
- [ ] API-3 adds Chat/Responses normalized/tunnel fixtures for available, unavailable, and unknown alternate recovery; available-only same-provider fallback; unavailable/unknown same-only terminal; no-owner, post-commit, unconfirmed, cancel/tool-side-effect, and shared-budget exhaustion; synchronize contracts/specs.
|
||||
- [ ] Run focused, package, race, vet, provider-only/OpenAI/local-capacity full-cycles, live provider-pool preflight/scenario, and diff verification with fresh output; assert new identities plus exactly one terminal/dispatch per allowed cycle.
|
||||
- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.
|
||||
|
||||
### [API-1] Convert typed execution stalls into raw-free StreamGate events
|
||||
|
||||
**Problem:** `apps/edge/internal/openai/stream_gate_runtime.go:182-184` and `473-478` emit generic `run_failed`/`provider_tunnel_error`, while `apps/edge/internal/openai/run_result.go:87-95` converts a terminal event into an untyped formatted error. Buffered Chat/Responses therefore cannot preserve the same failure semantics as live/tunnel paths.
|
||||
|
||||
**Solution:** Add an internal terminal error that defensively retains the protobuf failure while its `Error()` exposes only a stable code. Centralize conversion of typed execution failure to `ExternalDescriptor(code=response_stalled)` and bounded allowlisted fence, provider-health, provider-id, and Edge handoff tokens; never copy raw message or arbitrary metadata into StreamGate. Use it in live run, buffered collector, Responses, and tunnel ERROR sources. Nil/other typed failures retain existing generic terminal behavior. Mapping preserves `unknown` as a health token; it does not decide retry eligibility.
|
||||
|
||||
Before (`apps/edge/internal/openai/run_result.go:87`):
|
||||
|
||||
```go
|
||||
case "error", "cancelled":
|
||||
return "", "", "", nil, nil, false, fmt.Errorf("%s", msg)
|
||||
```
|
||||
|
||||
After:
|
||||
|
||||
```go
|
||||
case "error", "cancelled":
|
||||
return "", "", "", nil, nil, false, newOpenAIRunTerminalError(event)
|
||||
```
|
||||
|
||||
**Modified Files and Checklist:**
|
||||
|
||||
- [ ] `apps/edge/internal/openai/run_result.go`: retain cloned typed terminal failure behind a safe internal error.
|
||||
- [ ] `apps/edge/internal/openai/stream_gate_runtime.go`: centralize failure-to-event conversion and apply it to live run, buffered Chat, and tunnel sources.
|
||||
- [ ] `apps/edge/internal/openai/responses_stream_gate.go`: apply the same conversion to buffered normalized Responses attempts.
|
||||
|
||||
**Test Strategy:** API-3 covers present/absent typed failures across every source. Unit-level assertions inspect descriptor/cause tokens and prove raw messages, provider bodies, prompts, credentials, and arbitrary metadata are absent.
|
||||
|
||||
**Verification:** `go test -count=1 ./apps/edge/internal/openai -run '^(TestOpenAIStallEventMapping)'` must PASS.
|
||||
|
||||
### [API-2] Gate exact replay and hand off the failed provider
|
||||
|
||||
**Problem:** `apps/edge/internal/openai/stream_gate_filters.go:198-237` always passes provider errors, and `stream_gate_policy.go:288-318` registers that filter only when explicitly listed in `filters[]`; simply making it recover would silently leave gate-enabled requests without that config entry ownerless. `stream_gate_dispatcher.go:353-376` also treats CancelRun success as the only abort result, while recovery admission builders copy no failed-provider hint.
|
||||
|
||||
**Solution:** Store the current eligible stalled provider plus its allowlisted probe classification in request-local ingress state. Whenever StreamGate is enabled for Chat or Responses, register exactly one private `response_stalled` liveness filter through the existing extra-registration seam; do not require or mutate `filters[]`, the configurable `provider_error` foundation filter, selector policy, or provider capability admission. The private filter emits ExactReplay only when descriptor/cause proves a confirmed Edge handoff, `EvidenceBatch` is `transport_uncommitted`, there is no tool fragment/side-effect in current/pending/look-behind evidence, and a request snapshot ref exists. Generic provider errors PASS to their existing terminal behavior; gate-disabled/unsupported ingress has no recovery owner and stays terminal. `available`, `unavailable`, and `unknown` confirmed handoffs may request recovery because the pool can find an alternate. For an eligible terminal, close request-local transport/lease without sending another CancelRun or inferring a fence; other recovery reasons keep current cancel behavior. Consume the recorded provider once into `AvoidProviderID`, set fallback only for exact `available`, and clear/replace state per serialized cycle.
|
||||
|
||||
Before (`apps/edge/internal/openai/stream_gate_filters.go:222`):
|
||||
|
||||
```go
|
||||
case openAIOutputFilterProviderError:
|
||||
if batchHasProviderError(batch) {
|
||||
descriptor = "provider_error_observed_unmatched"
|
||||
}
|
||||
```
|
||||
|
||||
After:
|
||||
|
||||
```go
|
||||
case openAIOutputFilterProviderError:
|
||||
return f.evaluateProviderError(fctx, batch)
|
||||
```
|
||||
|
||||
**Modified Files and Checklist:**
|
||||
|
||||
- [ ] `apps/edge/internal/openai/stream_gate_ingress.go`: own concurrency-safe request-local eligible-stall state and construct one internal liveness registration only for enabled StreamGate requests.
|
||||
- [ ] `apps/edge/internal/openai/stream_gate_policy.go`: keep configured semantic filter/capability resolution unchanged and prove the internal registration is outside that admission policy.
|
||||
- [ ] `apps/edge/internal/openai/stream_gate_filters.go`: add the private liveness evaluator for descriptor/cause, commit state, request ref, and side effects; keep configurable generic `provider_error` foundation behavior unchanged.
|
||||
- [ ] `apps/edge/internal/openai/stream_gate_dispatcher.go`: use confirmed-terminal close semantics and pass request-local state through recovery controllers.
|
||||
- [ ] `apps/edge/internal/openai/stream_gate_runtime.go`: share state across Chat/tunnel builders/controllers and set `AvoidProviderID` plus the available-derived fallback flag on pool recovery only.
|
||||
- [ ] `apps/edge/internal/openai/responses_stream_gate.go`: share the identical state through Responses builders/controllers.
|
||||
- [ ] `apps/edge/internal/openai/stream_gate_filters_test.go`: add available/unavailable/unknown eligible, unconfirmed, post-commit, and tool-side-effect private-filter tests; preserve configured generic-provider-error unmatched PASS and prove no configured-filter capability coupling.
|
||||
- [ ] `apps/edge/internal/openai/stream_gate_dispatcher_test.go`: assert confirmed terminal closes without cancel, ordinary recovery still cancels, provider/available-fallback hints are consumed once, and controllers remain idempotent.
|
||||
|
||||
**Test Strategy:** Build batches with stable descriptor/cause tokens and each commit/side-effect boundary. Assert a gate-enabled request with empty configured `filters[]` owns exactly one liveness registration and can emit `RecoveryStrategyExactReplay`; explicitly configured `provider_error` neither duplicates that intent nor changes candidate capabilities. Gate-disabled, unconfirmed/unbound/post-commit/unsafe rows have no intent. Dispatcher spies assert zero extra CancelRun for an already confirmed terminal, one close, one avoided-provider handoff, and fallback only for available.
|
||||
|
||||
**Verification:** `go test -count=20 ./apps/edge/internal/openai -run '^(TestOpenAIStallRecoveryFilter|TestOpenAIAttemptControllerConfirmedStall|TestOpenAIAttemptDispatcherStalledProvider)'` must PASS every iteration.
|
||||
|
||||
### [API-3] Prove bounded recovery across OpenAI variants
|
||||
|
||||
**Problem:** Existing StreamGate vertical slices prove generic recovery and path switching, but none establish S05's typed liveness gates or same failure semantics across Chat/Responses and normalized/tunnel transports.
|
||||
|
||||
**Solution:** Add a focused scripted provider-pool matrix. Each recoverable fixture starts uncommitted with a confirmed eligible stall and returns a successful new attempt with a different run id. Available, unavailable, and unknown evidence all select an alternate when one exists; only available evidence permits a runtime-eligible same-provider fallback when no alternate exists. Unavailable/unknown same-only, no-owner, post-commit, unconfirmed, canceled, tool-bearing/side-effect, and exhausted shared budget remain one typed terminal with no duplicate provider dispatch. Exercise both streaming and buffered response release paths without widening public error data.
|
||||
|
||||
**Modified Files and Checklist:**
|
||||
|
||||
- [ ] `apps/edge/internal/openai/stream_gate_stall_recovery_test.go`: add normalized/tunnel and Chat/Responses S05 matrix with dispatch/cancel/close/terminal identity assertions.
|
||||
- [ ] `agent-contract/inner/execution-runtime.md`: document Edge eligibility -> OpenAI recovery handoff, confirmed-terminal close, and provider avoidance ownership.
|
||||
- [ ] `agent-contract/inner/edge-config-runtime-refresh.md`: document that the internal liveness owner follows `stream_evidence_gate.enabled`, not configured `filters[]` or provider capability admission, and that disabled ingress remains no-owner terminal.
|
||||
- [ ] `agent-contract/outer/openai-compatible-api.md`: document terminal versus transparent pre-commit recovery behavior without exposing internals/raw data.
|
||||
- [ ] `agent-spec/runtime/stream-evidence-gate.md`: record typed provider-error matcher, ExactReplay gates, and shared budget reuse.
|
||||
- [ ] `agent-spec/input/openai-compatible-surface.md`: record Chat/Responses variant behavior and no-owner boundary.
|
||||
- [ ] `agent-spec/runtime/edge-node-execution.md`: reflect final failure-handoff-to-retry integration and new attempt identity.
|
||||
|
||||
**Test Strategy:** Use existing scripted pool service and response sinks. Assert request count is initial+at-most-shared-budget, every recovery run id differs, `AvoidProviderID` equals the actual stalled provider, and `AllowAvoidedProviderFallback` is true only for the available same-only row. Cover unknown-with-alternate success and unknown-same-only terminal explicitly. Assert no leaked raw failure data, one old close, and one caller terminal. Include a two-fault fixture where another recovery strategy already consumes budget, proving no liveness-specific counter.
|
||||
|
||||
**Verification:** `go test -count=10 ./apps/edge/internal/openai -run '^TestOpenAIStallRecovery'` must PASS every iteration.
|
||||
|
||||
## Dependencies and Execution Order
|
||||
|
||||
1. `07+06_retry_candidate_policy` must produce `agent-task/m-node-provider-execution-liveness-recovery/07+06_retry_candidate_policy/complete.log`; it is active/missing at plan creation.
|
||||
2. Implement API-1, then API-2, then API-3. Do not enable recovery before the typed mapper and controller/provider handoff are both present.
|
||||
|
||||
## Modified Files Summary
|
||||
|
||||
| File | Item |
|
||||
|------|------|
|
||||
| `apps/edge/internal/openai/run_result.go` | API-1 |
|
||||
| `apps/edge/internal/openai/stream_gate_runtime.go` | API-1, API-2 |
|
||||
| `apps/edge/internal/openai/responses_stream_gate.go` | API-1, API-2 |
|
||||
| `apps/edge/internal/openai/stream_gate_ingress.go` | API-2 |
|
||||
| `apps/edge/internal/openai/stream_gate_policy.go` | API-2 |
|
||||
| `apps/edge/internal/openai/stream_gate_filters.go` | API-2 |
|
||||
| `apps/edge/internal/openai/stream_gate_dispatcher.go` | API-2 |
|
||||
| `apps/edge/internal/openai/stream_gate_filters_test.go` | API-2 |
|
||||
| `apps/edge/internal/openai/stream_gate_dispatcher_test.go` | API-2 |
|
||||
| `apps/edge/internal/openai/stream_gate_stall_recovery_test.go` | API-3 |
|
||||
| `agent-contract/inner/execution-runtime.md` | API-3 |
|
||||
| `agent-contract/inner/edge-config-runtime-refresh.md` | API-3 |
|
||||
| `agent-contract/outer/openai-compatible-api.md` | API-3 |
|
||||
| `agent-spec/runtime/stream-evidence-gate.md` | API-3 |
|
||||
| `agent-spec/input/openai-compatible-surface.md` | API-3 |
|
||||
| `agent-spec/runtime/edge-node-execution.md` | API-3 |
|
||||
| `agent-task/m-node-provider-execution-liveness-recovery/08+07_stall_recovery/CODE_REVIEW-cloud-G08.md` | API-1, API-2, API-3 |
|
||||
|
||||
## Final Verification
|
||||
|
||||
Fresh Go output is required; cached output is not acceptable.
|
||||
|
||||
1. `go test -count=20 ./apps/edge/internal/openai -run '^(TestOpenAIStallRecoveryFilter|TestOpenAIAttemptControllerConfirmedStall|TestOpenAIAttemptDispatcherStalledProvider)'` — PASS every iteration.
|
||||
2. `go test -count=10 ./apps/edge/internal/openai -run '^TestOpenAIStallRecovery'` — PASS every iteration.
|
||||
3. `go test -count=1 ./packages/go/streamgate ./apps/edge/internal/openai ./apps/edge/internal/service ./apps/edge/internal/controlplane` — PASS under the Edge local profile.
|
||||
4. `go test -race -count=3 ./apps/edge/internal/service ./apps/edge/internal/openai` — PASS with no race report.
|
||||
5. `go vet ./packages/go/streamgate ./apps/edge/internal/service ./apps/edge/internal/openai ./apps/edge/internal/controlplane` — no diagnostics.
|
||||
6. `./scripts/e2e-smoke.sh` — PASS for provider-only dispatch/tunnel/queue/reconnect fencing.
|
||||
7. `IOP_VLLM_MODE=fake ./scripts/e2e-openai-vllm.sh` — PASS for credential-free OpenAI Chat streaming/non-streaming Edge -> Node -> provider full-cycle.
|
||||
8. `./scripts/e2e-provider-capacity-smoke.sh` — PASS for deterministic local provider-pool queue and release behavior.
|
||||
9. `bash scripts/e2e-long-context-admission-smoke.sh --preflight && bash scripts/e2e-long-context-admission-smoke.sh --scenario normal-10` — PASS on the authorized synchronized dev runner; if endpoint/runtime identity remains blocked, record exact output and do not claim completion.
|
||||
10. `git diff --check` — no whitespace errors.
|
||||
|
||||
After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`.
|
||||
|
|
@ -0,0 +1,188 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/08+07_stall_recovery plan=0 tag=API milestone-task=bounded-retry -->
|
||||
|
||||
# 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-05
|
||||
task=m-node-provider-execution-liveness-recovery/08+07_stall_recovery, 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/08+07_stall_recovery/`. If WARN/FAIL, fully write the next filesystem state required by the code-review skill.
|
||||
4. If PASS and task group is `m-node-provider-execution-liveness-recovery`, preserve the first-line `milestone-task` metadata in `complete.log` and report it for the runtime aggregation event. Roadmap state evaluation belongs to `sync-milestone-workstate`.
|
||||
5. Check applicable `Review-Only Checklist` items at the final `.log` location before reporting.
|
||||
|
||||
---
|
||||
|
||||
## Implementation Item Completion
|
||||
|
||||
| Item | Status |
|
||||
|------|---------|
|
||||
| API-1: Typed stall to raw-free StreamGate event | [ ] |
|
||||
| API-2: ExactReplay eligibility and failed-provider handoff | [ ] |
|
||||
| API-3: OpenAI variant recovery matrix | [ ] |
|
||||
|
||||
## Implementation Checklist
|
||||
|
||||
- [ ] API-1 preserves typed normalized/buffered/tunnel stalls as one raw-free StreamGate `response_stalled` provider error, retaining only sanitized fence/health/handoff tokens while generic failures keep existing terminal behavior.
|
||||
- [ ] API-2 makes only confirmed Edge-eligible, uncommitted, side-effect-safe stalls produce ExactReplay, closes the fenced old transport without inferring fence from CancelRun, passes the actual failed provider to the next pool admission, and permits same-provider fallback only for exact `available` evidence.
|
||||
- [ ] API-3 adds Chat/Responses normalized/tunnel fixtures for available, unavailable, and unknown alternate recovery; available-only same-provider fallback; unavailable/unknown same-only terminal; no-owner, post-commit, unconfirmed, cancel/tool-side-effect, and shared-budget exhaustion; synchronize contracts/specs.
|
||||
- [ ] Run focused, package, race, vet, and diff verification with fresh output and assert new identities plus exactly one terminal/dispatch per allowed cycle.
|
||||
- [ ] 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/08+07_stall_recovery/` to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/08+07_stall_recovery/` 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 typed failure conversion is identical across live, buffered, Responses, and tunnel sources and never retains raw provider data.
|
||||
- Confirm all Edge-eligible confirmed uncommitted side-effect-safe stalls may create ExactReplay regardless of available/unavailable/unknown probe result, while all negative gates commit one typed terminal.
|
||||
- Confirm old transport closes once without inferring fence from CancelRun, failed-provider avoidance is handed off once, same-provider fallback is true only for available, shared budget is reused, and recovered attempts have new run identities.
|
||||
|
||||
## Verification Results
|
||||
|
||||
> Implementing agent: run each command exactly as written and paste its actual stdout/stderr under `Output`. Record any replacement command and reason in `Deviations from Plan`.
|
||||
|
||||
### Verification 1
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go test -count=1 ./apps/edge/internal/openai -run '^(TestOpenAIStallEventMapping)'
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 2
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go test -count=20 ./apps/edge/internal/openai -run '^(TestOpenAIProviderErrorFilterStall|TestOpenAIAttemptControllerConfirmedStall|TestOpenAIAttemptDispatcherStalledProvider)'
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 3
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go test -count=10 ./apps/edge/internal/openai -run '^TestOpenAIStallRecovery'
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 4
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go test -count=1 ./packages/go/streamgate ./apps/edge/internal/openai ./apps/edge/internal/service ./apps/edge/internal/controlplane
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 5
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go test -race -count=3 ./apps/edge/internal/service ./apps/edge/internal/openai
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 6
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
go vet ./packages/go/streamgate ./apps/edge/internal/service ./apps/edge/internal/openai ./apps/edge/internal/controlplane
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
### Verification 7
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
git diff --check
|
||||
```
|
||||
|
||||
Output:
|
||||
|
||||
_Paste actual stdout/stderr here._
|
||||
|
||||
---
|
||||
|
||||
> **[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,198 @@
|
|||
<!-- task=m-node-provider-execution-liveness-recovery/08+07_stall_recovery plan=0 tag=API milestone-task=bounded-retry -->
|
||||
|
||||
# OpenAI Typed Stall Recovery Handoff
|
||||
|
||||
## For the Implementing Agent
|
||||
|
||||
Implement only the items below after all predecessors PASS, run every verification command, and fill all implementation-owned sections of `CODE_REVIEW-cloud-G08.md` with actual notes and raw output. Keep active files in place and report ready for review; finalization belongs to the code-review skill. If blocked, record exact blocker evidence, attempted commands/output, and resume conditions only. Do not ask the user, call user-input tools, create control-plane stop files, classify the next state, archive logs, or write `complete.log`.
|
||||
|
||||
## Background
|
||||
|
||||
The OpenAI host currently maps every normalized/tunnel terminal error to generic StreamGate provider errors, and the provider-error filter intentionally never constructs recovery. S05 requires a typed `response_stalled` handoff that remains terminal unless Edge confirmed the attempt fence and the request is uncommitted, uncanceled, side-effect-safe, and within the existing shared Core budget; eligible recovery must use a new run identity and prefer another provider.
|
||||
|
||||
## Analysis
|
||||
|
||||
### Files Read
|
||||
|
||||
- `apps/edge/internal/openai/stream_gate_ingress.go`, `apps/edge/internal/openai/stream_gate_policy.go`, `apps/edge/internal/openai/stream_gate_filters.go`, `apps/edge/internal/openai/stream_gate_runtime.go`
|
||||
- `apps/edge/internal/openai/stream_gate_dispatcher.go`, `apps/edge/internal/openai/run_result.go`, `apps/edge/internal/openai/responses_stream_gate.go`, `apps/edge/internal/openai/chat_completion.go`
|
||||
- `apps/edge/internal/openai/stream_gate_filters_test.go`, `apps/edge/internal/openai/stream_gate_dispatcher_test.go`, `apps/edge/internal/openai/stream_gate_pipeline_test.go`, `apps/edge/internal/openai/stream_gate_vertical_slice_test.go`
|
||||
- `apps/edge/internal/openai/provider_test_support_test.go`, `apps/edge/internal/openai/server_test_support_test.go`
|
||||
- `packages/go/streamgate/runtime.go`, `packages/go/streamgate/recovery_coordinator.go`, `packages/go/streamgate/recovery_plan.go`, `packages/go/streamgate/commit_boundary.go`, `packages/go/streamgate/event.go`, `packages/go/streamgate/filter_registry.go`, `packages/go/streamgate/terminal.go`
|
||||
- `agent-contract/inner/execution-runtime.md`, `agent-contract/outer/openai-compatible-api.md`
|
||||
- `agent-spec/runtime/stream-evidence-gate.md`, `agent-spec/input/openai-compatible-surface.md`, `agent-spec/runtime/edge-node-execution.md`
|
||||
- `agent-test/local/edge-smoke.md`, `agent-test/local/platform-common-smoke.md`
|
||||
|
||||
### SDD Criteria
|
||||
|
||||
- SDD: `agent-roadmap/sdd/operational-observability-provider-management/node-provider-execution-liveness-recovery/SDD.md`; status `[승인됨]`, and this pair's first-line id is `milestone-task=bounded-retry`.
|
||||
- Acceptance Scenario S05 and Evidence Map S05 require healthy request stall, unhealthy failover, unknown probe, same-provider-only, no owner, post-commit, unconfirmed fence, caller cancel/tool-side-effect, and shared-budget fixtures with a new run identity and bounded dispatch/terminal count. Unknown health may recover through an alternate candidate but never grants same-provider fallback.
|
||||
- API-1 derives a raw-free typed event, API-2 implements exact-replay eligibility/fence teardown/provider handoff, and API-3 proves both Chat/Responses plus normalized/tunnel variants against those rows.
|
||||
|
||||
### Verification Context
|
||||
|
||||
- Handoff supplied starting HEAD `56e7d78af3cda4a8d6a85af091ad26bce935f8b6`; fresh baseline tests passed for execution, Node, Edge transport/service/OpenAI.
|
||||
- This plan waits for `07+06_retry_candidate_policy/complete.log`; transitively, typed wire and Edge eligibility/overlay contracts are also PASS before implementation.
|
||||
- Core already owns `transport_uncommitted`, caller-cancel propagation, abort-before-dispatch, new `AttemptBinding`, and one request-local total/per-strategy fault budget. No liveness counter or Core recovery coordinator is needed.
|
||||
- Existing OpenAI event sources discard typed failures (`stream_gate_runtime.go:128-188`, `402-478`, `responses_stream_gate.go:110-135`) and `collectRunResult` flattens them (`run_result.go:87-95`). Provider-error filtering is explicitly foundation-only (`stream_gate_filters.go:29-32`, `198-237`).
|
||||
- No external runner is required. Gaps are host mapping, provider-error intent, attempt-fence-aware close, and cross-endpoint fixtures. Confidence is medium-high due to multi-variant runtime and terminal ordering, so full package plus race verification is required.
|
||||
|
||||
### Test Coverage Gaps
|
||||
|
||||
- Typed normalized and tunnel stalls are not distinguished from generic provider errors.
|
||||
- Provider-error filter has only unmatched PASS coverage; it lacks eligible/fatal boundary cases.
|
||||
- Recovery dispatcher does not hand the failed provider to pool admission or distinguish a confirmed Node terminal from a still-running attempt during abort.
|
||||
- Existing vertical slices prove shared-budget recovery generally, but not Chat/Responses stall variants, unknown/no-owner/post-commit/unconfirmed gates, or duplicate-terminal absence.
|
||||
|
||||
### Symbol References
|
||||
|
||||
- No symbol is renamed or removed. Constructor signatures for request-local dispatcher/filter wiring may gain internal state; update all call sites in `stream_gate_runtime.go`, `responses_stream_gate.go`, and their direct tests.
|
||||
|
||||
### Split Judgment
|
||||
|
||||
- Predecessor `07+06_retry_candidate_policy` is active with missing `complete.log`; implementation waits for it. That predecessor transitively requires `05+04_failure_wire` and `06+05_health_overlay` PASS.
|
||||
- This final packet is indivisible at the OpenAI host boundary: the same raw-free eligibility token must drive filter intent, confirmed-terminal teardown, failed-provider handoff, and endpoint terminal rendering. Partial wiring could either duplicate dispatch or authorize an unfenced replay.
|
||||
|
||||
### Scope Rationale
|
||||
|
||||
Do not add a Core/Node/Edge retry loop, new counter, schema, metric, non-OpenAI recovery owner, or retry to legacy surfaces. Do not expose raw provider messages/metadata. StreamGate Core behavior remains unchanged; this packet consumes its existing budget/commit/cancel/side-effect contracts.
|
||||
|
||||
### Final Routing
|
||||
|
||||
- `evaluation_mode=first-pass`; finalizer=`finalize-task-policy.sh pair`.
|
||||
- Build closure true, scores `(2,2,1,1,2)`, grade G08, base `local-fit`, escalated by `risk-boundary` -> `PLAN-cloud-G08.md`.
|
||||
- Review closure true, scores `(2,2,1,1,2)`, grade G08, route `official-review` -> `CODE_REVIEW-cloud-G08.md` (`codex`, `gpt-5.6-sol`, `xhigh`).
|
||||
- `large_indivisible_context=false`; positive loop risks: `temporal_state`, `concurrent_consistency`, `boundary_contract`, `variant_product` (4). `review_rework_count=0`, `evidence_integrity_failure=false`; no capability gap.
|
||||
|
||||
## Implementation Checklist
|
||||
|
||||
- [ ] API-1 preserves typed normalized/buffered/tunnel stalls as one raw-free StreamGate `response_stalled` provider error, retaining only sanitized fence/health/handoff tokens while generic failures keep existing terminal behavior.
|
||||
- [ ] API-2 makes only confirmed Edge-eligible, uncommitted, side-effect-safe stalls produce ExactReplay, closes the fenced old transport without inferring fence from CancelRun, passes the actual failed provider to the next pool admission, and permits same-provider fallback only for exact `available` evidence.
|
||||
- [ ] API-3 adds Chat/Responses normalized/tunnel fixtures for available, unavailable, and unknown alternate recovery; available-only same-provider fallback; unavailable/unknown same-only terminal; no-owner, post-commit, unconfirmed, cancel/tool-side-effect, and shared-budget exhaustion; synchronize contracts/specs.
|
||||
- [ ] Run focused, package, race, vet, and diff verification with fresh output and assert new identities plus exactly one terminal/dispatch per allowed cycle.
|
||||
- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.
|
||||
|
||||
### [API-1] Convert typed execution stalls into raw-free StreamGate events
|
||||
|
||||
**Problem:** `apps/edge/internal/openai/stream_gate_runtime.go:182-184` and `473-478` emit generic `run_failed`/`provider_tunnel_error`, while `apps/edge/internal/openai/run_result.go:87-95` converts a terminal event into an untyped formatted error. Buffered Chat/Responses therefore cannot preserve the same failure semantics as live/tunnel paths.
|
||||
|
||||
**Solution:** Add an internal terminal error that defensively retains the protobuf failure while its `Error()` exposes only a stable code. Centralize conversion of typed execution failure to `ExternalDescriptor(code=response_stalled)` and bounded allowlisted fence, provider-health, provider-id, and Edge handoff tokens; never copy raw message or arbitrary metadata into StreamGate. Use it in live run, buffered collector, Responses, and tunnel ERROR sources. Nil/other typed failures retain existing generic terminal behavior. Mapping preserves `unknown` as a health token; it does not decide retry eligibility.
|
||||
|
||||
Before (`apps/edge/internal/openai/run_result.go:87`):
|
||||
|
||||
```go
|
||||
case "error", "cancelled":
|
||||
return "", "", "", nil, nil, false, fmt.Errorf("%s", msg)
|
||||
```
|
||||
|
||||
After:
|
||||
|
||||
```go
|
||||
case "error", "cancelled":
|
||||
return "", "", "", nil, nil, false, newOpenAIRunTerminalError(event)
|
||||
```
|
||||
|
||||
**Modified Files and Checklist:**
|
||||
|
||||
- [ ] `apps/edge/internal/openai/run_result.go`: retain cloned typed terminal failure behind a safe internal error.
|
||||
- [ ] `apps/edge/internal/openai/stream_gate_runtime.go`: centralize failure-to-event conversion and apply it to live run, buffered Chat, and tunnel sources.
|
||||
- [ ] `apps/edge/internal/openai/responses_stream_gate.go`: apply the same conversion to buffered normalized Responses attempts.
|
||||
|
||||
**Test Strategy:** API-3 covers present/absent typed failures across every source. Unit-level assertions inspect descriptor/cause tokens and prove raw messages, provider bodies, prompts, credentials, and arbitrary metadata are absent.
|
||||
|
||||
**Verification:** `go test -count=1 ./apps/edge/internal/openai -run '^(TestOpenAIStallEventMapping)'` must PASS.
|
||||
|
||||
### [API-2] Gate exact replay and hand off the failed provider
|
||||
|
||||
**Problem:** `apps/edge/internal/openai/stream_gate_filters.go:198-237` always passes provider errors, and `stream_gate_dispatcher.go:353-376` treats CancelRun success as the only abort result. Recovery admission builders at `stream_gate_runtime.go:815-825` and `1323-1336` also copy no failed-provider hint.
|
||||
|
||||
**Solution:** Store the current eligible stalled provider plus its allowlisted probe classification in a request-local ingress state shared by provider-error filters, attempt controllers, and recovery admission builders. The filter emits an ExactReplay violation only when descriptor/cause proves a confirmed Edge handoff, `EvidenceBatch` is `transport_uncommitted`, there is no tool fragment/side-effect in current/pending/look-behind evidence, and a request snapshot ref exists; otherwise return a fatal/pass decision that commits the typed terminal. `available`, `unavailable`, and `unknown` confirmed handoffs may all request recovery because provider-pool resolution can find an alternate. For an eligible terminal, `AbortAttempt` closes request-local transport/lease without sending another CancelRun and without inferring a fence; other recovery reasons keep existing cancel behavior. Consume the recorded provider once into `ProviderPoolDispatchRequest.AvoidProviderID`, and set `AllowAvoidedProviderFallback=true` only when the exact stalled-attempt classification is `available`; unknown/unavailable remain alternate-only. Clear/replace state per serialized recovery cycle.
|
||||
|
||||
Before (`apps/edge/internal/openai/stream_gate_filters.go:222`):
|
||||
|
||||
```go
|
||||
case openAIOutputFilterProviderError:
|
||||
if batchHasProviderError(batch) {
|
||||
descriptor = "provider_error_observed_unmatched"
|
||||
}
|
||||
```
|
||||
|
||||
After:
|
||||
|
||||
```go
|
||||
case openAIOutputFilterProviderError:
|
||||
return f.evaluateProviderError(fctx, batch)
|
||||
```
|
||||
|
||||
**Modified Files and Checklist:**
|
||||
|
||||
- [ ] `apps/edge/internal/openai/stream_gate_ingress.go`: own concurrency-safe, request-local eligible-stall provider/fence/health state and clear it on request close.
|
||||
- [ ] `apps/edge/internal/openai/stream_gate_policy.go`: bind the same state to the request-local provider-error filter without changing selector/capability policy.
|
||||
- [ ] `apps/edge/internal/openai/stream_gate_filters.go`: validate descriptor/cause, commit state, request ref, and tool/side-effect evidence; construct ExactReplay intent or fatal terminal decision with sanitized evidence.
|
||||
- [ ] `apps/edge/internal/openai/stream_gate_dispatcher.go`: use confirmed-terminal close semantics and pass request-local state through recovery controllers.
|
||||
- [ ] `apps/edge/internal/openai/stream_gate_runtime.go`: share state across Chat/tunnel builders/controllers and set `AvoidProviderID` plus the available-derived fallback flag on pool recovery only.
|
||||
- [ ] `apps/edge/internal/openai/responses_stream_gate.go`: share the identical state through Responses builders/controllers.
|
||||
- [ ] `apps/edge/internal/openai/stream_gate_filters_test.go`: replace foundation-only expectation with available/unavailable/unknown eligible, unconfirmed, post-commit, and tool-side-effect ExactReplay/fatal table tests; preserve unmatched generic PASS.
|
||||
- [ ] `apps/edge/internal/openai/stream_gate_dispatcher_test.go`: assert confirmed terminal closes without cancel, ordinary recovery still cancels, provider/available-fallback hints are consumed once, and controllers remain idempotent.
|
||||
|
||||
**Test Strategy:** Build provider-error batches with stable descriptor/cause tokens and each commit/side-effect boundary. Assert every confirmed bound available/unavailable/unknown pre-commit case returns `Violation` with `RecoveryStrategyExactReplay`, while unconfirmed/unbound/post-commit/unsafe cases have no intent. Dispatcher spies assert zero extra CancelRun for an already confirmed Node terminal, one transport close, one avoided-provider handoff, and a true fallback flag only for available.
|
||||
|
||||
**Verification:** `go test -count=20 ./apps/edge/internal/openai -run '^(TestOpenAIProviderErrorFilterStall|TestOpenAIAttemptControllerConfirmedStall|TestOpenAIAttemptDispatcherStalledProvider)'` must PASS every iteration.
|
||||
|
||||
### [API-3] Prove bounded recovery across OpenAI variants
|
||||
|
||||
**Problem:** Existing StreamGate vertical slices prove generic recovery and path switching, but none establish S05's typed liveness gates or same failure semantics across Chat/Responses and normalized/tunnel transports.
|
||||
|
||||
**Solution:** Add a focused scripted provider-pool matrix. Each recoverable fixture starts uncommitted with a confirmed eligible stall and returns a successful new attempt with a different run id. Available, unavailable, and unknown evidence all select an alternate when one exists; only available evidence permits a runtime-eligible same-provider fallback when no alternate exists. Unavailable/unknown same-only, no-owner, post-commit, unconfirmed, canceled, tool-bearing/side-effect, and exhausted shared budget remain one typed terminal with no duplicate provider dispatch. Exercise both streaming and buffered response release paths without widening public error data.
|
||||
|
||||
**Modified Files and Checklist:**
|
||||
|
||||
- [ ] `apps/edge/internal/openai/stream_gate_stall_recovery_test.go`: add normalized/tunnel and Chat/Responses S05 matrix with dispatch/cancel/close/terminal identity assertions.
|
||||
- [ ] `agent-contract/inner/execution-runtime.md`: document Edge eligibility -> OpenAI recovery handoff, confirmed-terminal close, and provider avoidance ownership.
|
||||
- [ ] `agent-contract/outer/openai-compatible-api.md`: document terminal versus transparent pre-commit recovery behavior without exposing internals/raw data.
|
||||
- [ ] `agent-spec/runtime/stream-evidence-gate.md`: record typed provider-error matcher, ExactReplay gates, and shared budget reuse.
|
||||
- [ ] `agent-spec/input/openai-compatible-surface.md`: record Chat/Responses variant behavior and no-owner boundary.
|
||||
- [ ] `agent-spec/runtime/edge-node-execution.md`: reflect final failure-handoff-to-retry integration and new attempt identity.
|
||||
|
||||
**Test Strategy:** Use existing scripted pool service and response sinks. Assert request count is initial+at-most-shared-budget, every recovery run id differs, `AvoidProviderID` equals the actual stalled provider, and `AllowAvoidedProviderFallback` is true only for the available same-only row. Cover unknown-with-alternate success and unknown-same-only terminal explicitly. Assert no leaked raw failure data, one old close, and one caller terminal. Include a two-fault fixture where another recovery strategy already consumes budget, proving no liveness-specific counter.
|
||||
|
||||
**Verification:** `go test -count=10 ./apps/edge/internal/openai -run '^TestOpenAIStallRecovery'` must PASS every iteration.
|
||||
|
||||
## Dependencies and Execution Order
|
||||
|
||||
1. `07+06_retry_candidate_policy` must produce `agent-task/m-node-provider-execution-liveness-recovery/07+06_retry_candidate_policy/complete.log`; it is active/missing at plan creation.
|
||||
2. Implement API-1, then API-2, then API-3. Do not enable recovery before the typed mapper and controller/provider handoff are both present.
|
||||
|
||||
## Modified Files Summary
|
||||
|
||||
| File | Item |
|
||||
|------|------|
|
||||
| `apps/edge/internal/openai/run_result.go` | API-1 |
|
||||
| `apps/edge/internal/openai/stream_gate_runtime.go` | API-1, API-2 |
|
||||
| `apps/edge/internal/openai/responses_stream_gate.go` | API-1, API-2 |
|
||||
| `apps/edge/internal/openai/stream_gate_ingress.go` | API-2 |
|
||||
| `apps/edge/internal/openai/stream_gate_policy.go` | API-2 |
|
||||
| `apps/edge/internal/openai/stream_gate_filters.go` | API-2 |
|
||||
| `apps/edge/internal/openai/stream_gate_dispatcher.go` | API-2 |
|
||||
| `apps/edge/internal/openai/stream_gate_filters_test.go` | API-2 |
|
||||
| `apps/edge/internal/openai/stream_gate_dispatcher_test.go` | API-2 |
|
||||
| `apps/edge/internal/openai/stream_gate_stall_recovery_test.go` | API-3 |
|
||||
| `agent-contract/inner/execution-runtime.md` | API-3 |
|
||||
| `agent-contract/outer/openai-compatible-api.md` | API-3 |
|
||||
| `agent-spec/runtime/stream-evidence-gate.md` | API-3 |
|
||||
| `agent-spec/input/openai-compatible-surface.md` | API-3 |
|
||||
| `agent-spec/runtime/edge-node-execution.md` | API-3 |
|
||||
| `agent-task/m-node-provider-execution-liveness-recovery/08+07_stall_recovery/CODE_REVIEW-cloud-G08.md` | API-1, API-2, API-3 |
|
||||
|
||||
## Final Verification
|
||||
|
||||
Fresh Go output is required; cached output is not acceptable.
|
||||
|
||||
1. `go test -count=20 ./apps/edge/internal/openai -run '^(TestOpenAIProviderErrorFilterStall|TestOpenAIAttemptControllerConfirmedStall|TestOpenAIAttemptDispatcherStalledProvider)'` — PASS every iteration.
|
||||
2. `go test -count=10 ./apps/edge/internal/openai -run '^TestOpenAIStallRecovery'` — PASS every iteration.
|
||||
3. `go test -count=1 ./packages/go/streamgate ./apps/edge/internal/openai ./apps/edge/internal/service ./apps/edge/internal/controlplane` — PASS under the Edge local profile.
|
||||
4. `go test -race -count=3 ./apps/edge/internal/service ./apps/edge/internal/openai` — PASS with no race report.
|
||||
5. `go vet ./packages/go/streamgate ./apps/edge/internal/service ./apps/edge/internal/openai ./apps/edge/internal/controlplane` — no diagnostics.
|
||||
6. `git diff --check` — no whitespace errors.
|
||||
|
||||
After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`.
|
||||
Loading…
Reference in a new issue