diff --git a/agent-task/m-node-provider-execution-liveness-recovery/07+06_reception_fence/CODE_REVIEW-cloud-G08.md b/agent-task/m-node-provider-execution-liveness-recovery/07+06_reception_fence/CODE_REVIEW-cloud-G08.md index 9c291111..f92ec811 100644 --- a/agent-task/m-node-provider-execution-liveness-recovery/07+06_reception_fence/CODE_REVIEW-cloud-G08.md +++ b/agent-task/m-node-provider-execution-liveness-recovery/07+06_reception_fence/CODE_REVIEW-cloud-G08.md @@ -1,4 +1,4 @@ - + # Code Review Reference - REFACTOR @@ -14,12 +14,13 @@ ## Overview date=2026-08-05 -task=m-node-provider-execution-liveness-recovery/07+06_reception_fence, plan=3, tag=REFACTOR +task=m-node-provider-execution-liveness-recovery/07+06_reception_fence, plan=4, tag=REFACTOR ## Archive Evidence Snapshot - Refined parent: `plan_cloud_G09_2.log` and `code_review_cloud_G09_2.log` in this directory; unimplemented, no verdict or implementation evidence. -- This child retains parent REFACTOR-1 only; overlay/probe consumption moved to `08+07_health_overlay`. +- Fresh review split the stable reception producer from the queue-locked overlay/probe consumer. This child retains parent REFACTOR-1 only. +- Union preparation review archived the unimplemented plan=3 pair as `plan_local_G08_3.log` and `code_review_cloud_G08_3.log`; it had no verdict or implementation evidence. `scripts/e2e-smoke.sh` runs package tests only, so it is not the required transport/bootstrap multi-process cycle. ## For the Review Agent @@ -29,7 +30,7 @@ Compare implementation of each item against source files and verify that output Review completion means the following steps are finished: 1. Append verdict and `review_rework_count` / `evidence_integrity_failure` routing signals. -2. Archive `CODE_REVIEW-cloud-G08.md` → `code_review_cloud_G08_3.log` and `PLAN-local-G08.md` → `plan_local_G08_3.log`. +2. Archive `CODE_REVIEW-cloud-G08.md` → `code_review_cloud_G08_4.log` and `PLAN-local-G08.md` → `plan_local_G08_4.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_reception_fence/`. If WARN/FAIL, fully write the next filesystem state required by the code-review skill. 4. If PASS, preserve the first-line `milestone-task` metadata in `complete.log` and report it for runtime aggregation. Roadmap state evaluation belongs to `sync-milestone-workstate`. 5. Check applicable `Review-Only Checklist` items at the final `.log` location before reporting. @@ -46,7 +47,7 @@ Review completion means the following steps are finished: - [ ] REFACTOR-1 derives authoritative node/generation from the receiving client for RunEvent and tunnel callbacks and drops stale/unregistered receivers before correctness callbacks. - [ ] Preserve message-only observability fanout and compatibility-delegate the new bootstrap callback shape until the dependent overlay consumer uses its authority values. -- [ ] Add current/stale/unregistered two-client fixtures and run focused, package, race, vet, provider-only reconnect smoke, and diff verification. +- [ ] Add current/stale/unregistered two-client fixtures and run focused, package, race, vet, package smoke, actual Edge/Node reconnect diagnostic, and diff verification. - [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output. ## Review-Only Checklist @@ -56,8 +57,8 @@ Review completion means the following steps are finished: - [ ] Append one verdict of `PASS`, `WARN`, or `FAIL` and verified `review_rework_count`, `evidence_integrity_failure` to `Code Review Result`. - [ ] Verify that verdict, `Dimension Assessment`, and Required/Suggested/Nit classifications match. -- [ ] Archive active `CODE_REVIEW-*-G??.md` to `code_review_cloud_G08_3.log`. -- [ ] Archive active `PLAN-*-G??.md` to `plan_local_G08_3.log`. +- [ ] Archive active `CODE_REVIEW-*-G??.md` to `code_review_cloud_G08_4.log`. +- [ ] Archive active `PLAN-*-G??.md` to `plan_local_G08_4.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_reception_fence/` to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/07+06_reception_fence/` and update this checklist at the final archive path. @@ -77,7 +78,7 @@ _Record key design decisions here._ - Confirm current-owner lookup is atomic and returns a clone. - Confirm stale/unregistered clients never reach correctness callbacks and payload node metadata cannot substitute authority. -- Confirm observability remains message-only and bootstrap compatibility does not consume queue/overlay semantics early. +- Confirm observability remains message-only, bootstrap compatibility does not consume queue/overlay semantics early, and the actual Edge/Node reconnect diagnostic dispatches successfully before and after re-registration. ## Verification Results @@ -147,6 +148,18 @@ _Paste actual stdout/stderr here._ Command: +```bash +IOP_DEV_RECONNECT_BIND_TIMEOUT=45 ./scripts/dev/edge-node-reconnect-diagnostic.sh +``` + +Output: + +_Paste actual stdout/stderr here._ + +### Verification 7 + +Command: + ```bash git diff --check ``` diff --git a/agent-task/m-node-provider-execution-liveness-recovery/07+06_reception_fence/PLAN-local-G08.md b/agent-task/m-node-provider-execution-liveness-recovery/07+06_reception_fence/PLAN-local-G08.md index ca1415a0..1b46df5f 100644 --- a/agent-task/m-node-provider-execution-liveness-recovery/07+06_reception_fence/PLAN-local-G08.md +++ b/agent-task/m-node-provider-execution-liveness-recovery/07+06_reception_fence/PLAN-local-G08.md @@ -1,4 +1,4 @@ - + # Authoritative Reception Identity Fence @@ -14,6 +14,7 @@ A typed failure is not authoritative merely because its payload names a node. Ed - Refined parent: `plan_cloud_G09_2.log` and `code_review_cloud_G09_2.log` in this directory; unimplemented, no verdict or implementation evidence. - Fresh review split the stable reception producer from the queue-locked overlay/probe consumer. This child retains parent REFACTOR-1 only. +- Union preparation review archived the unimplemented plan=3 pair as `plan_local_G08_3.log` and `code_review_cloud_G08_3.log`; it had no verdict or implementation evidence. `scripts/e2e-smoke.sh` runs package tests only, so it is not the required transport/bootstrap multi-process cycle. ## Analysis @@ -33,7 +34,7 @@ A typed failure is not authoritative merely because its payload names a node. Ed ### Verification Context -- `06+05_failure_wire_mapping` must PASS first. Registry generations and compare-by-client primitives already exist; focused two-client reconnect tests, package/race/vet checks, and the provider-only reconnect smoke are local evidence. +- `06+05_failure_wire_mapping` must PASS first. Registry generations and compare-by-client primitives already exist. Focused two-client reconnect tests plus package/race/vet checks prove the callback fence; the repository diagnostic launches real Edge and Node processes, dispatches before and after reconnect, and is the required transport/bootstrap full-cycle. ### Test Coverage Gaps @@ -62,7 +63,7 @@ Do not inspect provider/adapter/target binding, mutate overlay health, release l - [ ] REFACTOR-1 derives authoritative node/generation from the receiving client for RunEvent and tunnel callbacks and drops stale/unregistered receivers before correctness callbacks. - [ ] Preserve message-only observability fanout and compatibility-delegate the new bootstrap callback shape until the dependent overlay consumer uses its authority values. -- [ ] Add current/stale/unregistered two-client fixtures and run focused, package, race, vet, provider-only reconnect smoke, and diff verification. +- [ ] Add current/stale/unregistered two-client fixtures and run focused, package, race, vet, package smoke, actual Edge/Node reconnect diagnostic, and diff verification. - [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output. ### [REFACTOR-1] Carry authoritative reception identity @@ -109,7 +110,8 @@ Fresh Go output is required. 2. `go test -count=1 ./apps/edge/internal/node ./apps/edge/internal/transport ./apps/edge/internal/bootstrap` — PASS. 3. `go test -race -count=3 ./apps/edge/internal/node ./apps/edge/internal/transport ./apps/edge/internal/bootstrap` — PASS with no race report. 4. `go vet ./apps/edge/internal/node ./apps/edge/internal/transport ./apps/edge/internal/bootstrap` — no diagnostics. -5. `./scripts/e2e-smoke.sh` — PASS for provider-only dispatch/tunnel/reconnect fencing. -6. `git diff --check` — no whitespace errors. +5. `./scripts/e2e-smoke.sh` — PASS for the repository package smoke. +6. `IOP_DEV_RECONNECT_BIND_TIMEOUT=45 ./scripts/dev/edge-node-reconnect-diagnostic.sh` — PASS with initial dispatch, Node restart/re-registration, and post-reconnect dispatch across actual Edge/Node processes. +7. `git diff --check` — no whitespace errors. After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`. diff --git a/agent-task/m-node-provider-execution-liveness-recovery/07+06_reception_fence/code_review_cloud_G08_3.log b/agent-task/m-node-provider-execution-liveness-recovery/07+06_reception_fence/code_review_cloud_G08_3.log new file mode 100644 index 00000000..9c291111 --- /dev/null +++ b/agent-task/m-node-provider-execution-liveness-recovery/07+06_reception_fence/code_review_cloud_G08_3.log @@ -0,0 +1,176 @@ + + +# 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_reception_fence, plan=3, tag=REFACTOR + +## Archive Evidence Snapshot + +- Refined parent: `plan_cloud_G09_2.log` and `code_review_cloud_G09_2.log` in this directory; unimplemented, no verdict or implementation evidence. +- This child retains parent REFACTOR-1 only; overlay/probe consumption moved to `08+07_health_overlay`. + +## 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_3.log` and `PLAN-local-G08.md` → `plan_local_G08_3.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_reception_fence/`. If WARN/FAIL, fully write the next filesystem state required by the code-review skill. +4. If PASS, preserve the first-line `milestone-task` metadata in `complete.log` and report it for runtime aggregation. Roadmap state evaluation belongs to `sync-milestone-workstate`. +5. Check applicable `Review-Only Checklist` items at the final `.log` location before reporting. + +--- + +## Implementation Item Completion + +| Item | Status | +|------|---------| +| REFACTOR-1: Carry authoritative reception identity | [ ] | + +## Implementation Checklist + +- [ ] REFACTOR-1 derives authoritative node/generation from the receiving client for RunEvent and tunnel callbacks and drops stale/unregistered receivers before correctness callbacks. +- [ ] Preserve message-only observability fanout and compatibility-delegate the new bootstrap callback shape until the dependent overlay consumer uses its authority values. +- [ ] Add current/stale/unregistered two-client fixtures and run focused, package, race, vet, provider-only reconnect smoke, and diff verification. +- [ ] 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_3.log`. +- [ ] Archive active `PLAN-*-G??.md` to `plan_local_G08_3.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_reception_fence/` to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/07+06_reception_fence/` and update this checklist at the final archive path. +- [ ] If PASS, 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 current-owner lookup is atomic and returns a clone. +- Confirm stale/unregistered clients never reach correctness callbacks and payload node metadata cannot substitute authority. +- Confirm observability remains message-only and bootstrap compatibility does not consume queue/overlay semantics early. + +## 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=1 ./apps/edge/internal/node ./apps/edge/internal/transport ./apps/edge/internal/bootstrap +``` + +Output: + +_Paste actual stdout/stderr here._ + +### Verification 3 + +Command: + +```bash +go test -race -count=3 ./apps/edge/internal/node ./apps/edge/internal/transport ./apps/edge/internal/bootstrap +``` + +Output: + +_Paste actual stdout/stderr here._ + +### Verification 4 + +Command: + +```bash +go vet ./apps/edge/internal/node ./apps/edge/internal/transport ./apps/edge/internal/bootstrap +``` + +Output: + +_Paste actual stdout/stderr here._ + +### Verification 5 + +Command: + +```bash +./scripts/e2e-smoke.sh +``` + +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 | diff --git a/agent-task/m-node-provider-execution-liveness-recovery/07+06_reception_fence/plan_local_G08_3.log b/agent-task/m-node-provider-execution-liveness-recovery/07+06_reception_fence/plan_local_G08_3.log new file mode 100644 index 00000000..ca1415a0 --- /dev/null +++ b/agent-task/m-node-provider-execution-liveness-recovery/07+06_reception_fence/plan_local_G08_3.log @@ -0,0 +1,115 @@ + + +# Authoritative Reception Identity Fence + +## For the Implementing Agent + +Implement only this reception-fence producer 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 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 + +A typed failure is not authoritative merely because its payload names a node. Edge transport must derive node id and connection generation from the actual receiving client and drop stale/unregistered owners before any correctness callback can consume the event. + +## Archive Evidence Snapshot + +- Refined parent: `plan_cloud_G09_2.log` and `code_review_cloud_G09_2.log` in this directory; unimplemented, no verdict or implementation evidence. +- Fresh review split the stable reception producer from the queue-locked overlay/probe consumer. This child retains parent REFACTOR-1 only. + +## 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` +- `agent-contract/inner/edge-node-runtime-wire.md`, `agent-spec/runtime/edge-node-execution.md` +- `agent-test/local/edge-smoke.md`, `scripts/e2e-smoke.sh` + +### SDD Criteria + +- SDD: `agent-roadmap/sdd/operational-observability-provider-management/node-provider-execution-liveness-recovery/SDD.md`; status `[승인됨]`; `milestone-task=failure-handoff`. +- S04/Evidence Map S04 requires connection-generation authority to be out-of-band and stale receivers to be rejected. This child supplies that authority token; `08+07_health_overlay` consumes it for binding/transition/release. + +### Verification Context + +- `06+05_failure_wire_mapping` must PASS first. Registry generations and compare-by-client primitives already exist; focused two-client reconnect tests, package/race/vet checks, and the provider-only reconnect smoke are local evidence. + +### Test Coverage Gaps + +- Current listener closures retain `*toki.TcpClient` but correctness callbacks receive only the wire message. No fixture proves a stale client cannot invoke lifecycle/tunnel callbacks after reconnect. + +### Symbol References + +- Transport callback types change internally. Call sites are bootstrap wiring and transport tests. Existing one-argument service handlers remain compatible until the dependent consumer installs reception-aware handlers. + +### Split Judgment + +- The stable child output is a registry-derived `(node_id, connection_generation)` callback contract with stale-owner drop. Bootstrap may adapt it to existing handlers so this producer independently compiles; the next child consumes the authoritative values and replaces that compatibility delegation. + +### Scope Rationale + +Do not inspect provider/adapter/target binding, mutate overlay health, release leases, apply probes, choose candidates, or own retry. + +### Final Routing + +- `evaluation_mode=isolated-reassessment`; 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`. + +## Implementation Checklist + +- [ ] REFACTOR-1 derives authoritative node/generation from the receiving client for RunEvent and tunnel callbacks and drops stale/unregistered receivers before correctness callbacks. +- [ ] Preserve message-only observability fanout and compatibility-delegate the new bootstrap callback shape until the dependent overlay consumer uses its authority values. +- [ ] Add current/stale/unregistered two-client fixtures and run focused, package, race, vet, provider-only reconnect smoke, and diff verification. +- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output. + +### [REFACTOR-1] Carry authoritative reception identity + +**Problem:** connection handlers capture the receiving client but invoke lifecycle/tunnel callbacks with only the wire payload, so a stale or spoofed identity can reach correctness handling without proving the current owner. + +**Solution:** Add an atomic registry lookup returning a cloned entry only when the supplied client is still current. Resolve it at receipt, pass node/generation to internal callbacks, and drop stale/unregistered clients before correctness callbacks. Keep observability fanout message-only. Adapt bootstrap to the new callback shape without consuming identity-dependent queue semantics yet. + +**Modified Files and Checklist:** + +- [ ] `apps/edge/internal/node/registry.go`: add lock-safe current-owner-by-client lookup. +- [ ] `apps/edge/internal/node/registry_test.go`: prove current success and stale/unregistered 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 and fail closed before correctness callbacks. +- [ ] `apps/edge/internal/transport/server_test.go`: assert authoritative node/generation, stale drop, and observability separation. +- [ ] `apps/edge/internal/bootstrap/runtime.go`: adapt service wiring to the reception-aware callback contract while retaining legacy behavior until the consumer child. + +**Test Strategy:** Use two clients for one node across reconnect. Only the current client may reach callbacks, payload metadata cannot substitute authority, and observability remains independently message-only. + +**Verification:** focused registry/transport/bootstrap tests must execute the new current/stale cases. + +## Dependencies and Execution Order + +1. `06+05_failure_wire_mapping` must produce `agent-task/m-node-provider-execution-liveness-recovery/06+05_failure_wire_mapping/complete.log`. +2. This child must PASS before `08+07_health_overlay` 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 | +| `agent-task/m-node-provider-execution-liveness-recovery/07+06_reception_fence/CODE_REVIEW-cloud-G08.md` | REFACTOR-1 | + +## Final Verification + +Fresh Go output is required. + +1. `go test -count=1 ./apps/edge/internal/node ./apps/edge/internal/transport ./apps/edge/internal/bootstrap -run 'CurrentOwner|Reception|Lifecycle|Tunnel'` — PASS and new named fixtures execute. +2. `go test -count=1 ./apps/edge/internal/node ./apps/edge/internal/transport ./apps/edge/internal/bootstrap` — PASS. +3. `go test -race -count=3 ./apps/edge/internal/node ./apps/edge/internal/transport ./apps/edge/internal/bootstrap` — PASS with no race report. +4. `go vet ./apps/edge/internal/node ./apps/edge/internal/transport ./apps/edge/internal/bootstrap` — no diagnostics. +5. `./scripts/e2e-smoke.sh` — PASS for provider-only dispatch/tunnel/reconnect fencing. +6. `git diff --check` — no whitespace errors. + +After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`. diff --git a/agent-task/m-node-provider-execution-liveness-recovery/08+07_health_overlay/CODE_REVIEW-cloud-G09.md b/agent-task/m-node-provider-execution-liveness-recovery/08+07_health_overlay/CODE_REVIEW-cloud-G09.md index 07fb94dc..ac7e5939 100644 --- a/agent-task/m-node-provider-execution-liveness-recovery/08+07_health_overlay/CODE_REVIEW-cloud-G09.md +++ b/agent-task/m-node-provider-execution-liveness-recovery/08+07_health_overlay/CODE_REVIEW-cloud-G09.md @@ -1,4 +1,4 @@ - + # Code Review Reference - REFACTOR @@ -14,7 +14,13 @@ ## Overview date=2026-08-05 -task=m-node-provider-execution-liveness-recovery/08+07_health_overlay, plan=0, tag=REFACTOR +task=m-node-provider-execution-liveness-recovery/08+07_health_overlay, plan=1, tag=REFACTOR + +## Archive Evidence Snapshot + +- Union preparation review archived the unimplemented plan=0 pair as `plan_cloud_G09_0.log` and `code_review_cloud_G09_0.log`; it had no verdict, implementation evidence, or verification output. +- Material ownership finding: reception/binding/fence confirmation must be an Edge handoff fact, not `recovery_eligible`. This replan uses `recovery_handoff=confirmed` only to prove the current binding and local fence; the OpenAI ingress recovery owner still decides commit, cancel, side effects, budget, candidates, and replay eligibility. +- Verification finding: this packet changes provider-pool eligibility and ProviderSnapshot projection, so the testing domain requires live long-context preflight plus the needed scenario as an auxiliary regression in addition to focused S04 evidence. ## For the Review Agent @@ -24,7 +30,7 @@ Compare implementation of each item against source files and verify that output Review completion means the following steps are finished: 1. Append verdict and `review_rework_count` / `evidence_integrity_failure` routing signals. -2. Archive `CODE_REVIEW-cloud-G09.md` → `code_review_cloud_G09_0.log` and `PLAN-cloud-G09.md` → `plan_cloud_G09_0.log`. +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/08+07_health_overlay/`. If WARN/FAIL, fully write the next filesystem state required by the code-review skill. 4. If PASS, preserve the first-line `milestone-task` metadata in `complete.log` and report it for runtime aggregation. Roadmap state evaluation belongs to `sync-milestone-workstate`. 5. Check applicable `Review-Only Checklist` items at the final `.log` location before reporting. @@ -40,10 +46,10 @@ Review completion means the following steps are finished: ## Implementation Checklist -- [ ] REFACTOR-1 validates reception plus immutable provider/adapter/target lease identity, sequence-fences runtime unhealthy/recovery transitions, gates admission/snapshots, annotates confirmed bound stalls for request-local handoff, and releases valid terminals exactly once. +- [ ] REFACTOR-1 validates reception plus immutable provider/adapter/target lease identity, sequence-fences runtime unhealthy/recovery transitions, gates admission/snapshots, annotates confirmed bound stalls with the non-approval token `recovery_handoff=confirmed`, and releases valid terminals exactly once. - [ ] REFACTOR-2 turns exact-target CAPABILITIES into fail-closed Session-sequenced health evidence and applies only unambiguous current-generation higher-sequence `available` to overlay recovery. - [ ] Add missing/ambiguous identity, stale/mismatch/sequence, normalized/tunnel release-race, and production-probe recovery fixtures; synchronize contracts/specs without mutating config health. -- [ ] Run focused, package, race, vet, provider-only/local-capacity full-cycle, and diff verification with fresh output. +- [ ] Run focused, package, race, vet, provider-only/local-capacity full-cycles, required live long-context preflight/`normal-10` auxiliary regression, and diff verification with fresh output. - [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output. ## Review-Only Checklist @@ -53,8 +59,8 @@ Review completion means the following steps are finished: - [ ] Append one verdict of `PASS`, `WARN`, or `FAIL` and verified `review_rework_count`, `evidence_integrity_failure` to `Code Review Result`. - [ ] Verify that verdict, `Dimension Assessment`, and Required/Suggested/Nit classifications match. -- [ ] Archive active `CODE_REVIEW-*-G??.md` to `code_review_cloud_G09_0.log`. -- [ ] Archive active `PLAN-*-G??.md` to `plan_cloud_G09_0.log`. +- [ ] 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/08+07_health_overlay/` to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/08+07_health_overlay/` and update this checklist at the final archive path. @@ -72,9 +78,10 @@ _Record key design decisions here._ ## Reviewer Checkpoints -- Confirm reception identity and full immutable lease binding fence every overlay transition and handoff annotation. +- Confirm reception identity and full immutable lease binding fence every overlay transition, and confirm `recovery_handoff=confirmed` is only an authority token while ingress retains full eligibility. - Confirm unavailable/available sequence semantics, config immutability, admission/snapshot projection, and exactly-once release under duplicates/races. - Confirm CAPABILITIES uses fail-closed `ProbeHealth` plus Session sequence and only exact current higher-sequence available recovers. +- Confirm long-context preflight/`normal-10` is treated as an auxiliary live eligibility/snapshot regression, with any unavailable runner captured as external-execution evidence rather than an S04 oracle. ## Verification Results @@ -156,6 +163,30 @@ _Paste actual stdout/stderr here._ Command: +```bash +./scripts/e2e-long-context-admission-smoke.sh --preflight +``` + +Output: + +_Paste actual stdout/stderr here._ + +### Verification 8 + +Command: + +```bash +./scripts/e2e-long-context-admission-smoke.sh --scenario normal-10 +``` + +Output: + +_Paste actual stdout/stderr here._ + +### Verification 9 + +Command: + ```bash git diff --check ``` diff --git a/agent-task/m-node-provider-execution-liveness-recovery/08+07_health_overlay/PLAN-cloud-G09.md b/agent-task/m-node-provider-execution-liveness-recovery/08+07_health_overlay/PLAN-cloud-G09.md index 576400db..e8b7e13b 100644 --- a/agent-task/m-node-provider-execution-liveness-recovery/08+07_health_overlay/PLAN-cloud-G09.md +++ b/agent-task/m-node-provider-execution-liveness-recovery/08+07_health_overlay/PLAN-cloud-G09.md @@ -1,4 +1,4 @@ - + # Lease-Bound Provider Health Overlay and Recovery Probe @@ -10,6 +10,12 @@ Implement only this overlay/probe consumer after the predecessor PASS, run every Authoritative reception identity must be compared with the immutable dispatch lease before typed stall evidence may affect provider-wide health. S04 also requires a separate generation/sequence-fenced runtime overlay, exactly-once terminal release, and a real later bounded exact-target probe that can recover an unavailable provider without mutating config health. +## Archive Evidence Snapshot + +- Union preparation review archived the unimplemented plan=0 pair as `plan_cloud_G09_0.log` and `code_review_cloud_G09_0.log`; it had no verdict, implementation evidence, or verification output. +- Material ownership finding: reception/binding/fence confirmation must be an Edge handoff fact, not `recovery_eligible`. This replan uses `recovery_handoff=confirmed` only to prove the current binding and local fence; the OpenAI ingress recovery owner still decides commit, cancel, side effects, budget, candidates, and replay eligibility. +- Verification finding: this packet changes provider-pool eligibility and ProviderSnapshot projection, so the testing domain requires live long-context preflight plus the needed scenario as an auxiliary regression in addition to focused S04 evidence. + ## Analysis ### Files Read @@ -32,7 +38,7 @@ Authoritative reception identity must be compared with the immutable dispatch le ### Verification Context - `07+06_reception_fence` supplies authoritative callback identity. Queue leases currently omit adapter/target and provider resources have no runtime sequence/health overlay. -- Node CAPABILITIES currently bypasses fail-closed `ProbeHealth` and Session observation sequence; Edge returns the response without applying it. Focused/race tests and repository-native provider/queue smokes are the complete local oracle; no external live scenario is required. +- Node CAPABILITIES currently bypasses fail-closed `ProbeHealth` and Session observation sequence; Edge returns the response without applying it. Focused/race tests and repository-native provider/queue smokes are the S04 semantic oracle. Because this packet changes provider-pool eligibility and ProviderSnapshot projection, the testing domain also requires live long-context preflight plus `normal-10` as an auxiliary regression. If an authorized live runner or credential is unavailable, record an `external-execution` verification blocker; it is not a product decision and does not weaken the focused oracle. - Related SDDs keep retry/commit ownership in StreamGate Core and do not change this service-layer overlay boundary. ### Test Coverage Gaps @@ -60,17 +66,17 @@ Do not create recovery intents, choose alternate providers, consume StreamGate b ## Implementation Checklist -- [ ] REFACTOR-1 validates reception plus immutable provider/adapter/target lease identity, sequence-fences runtime unhealthy/recovery transitions, gates admission/snapshots, annotates confirmed bound stalls for request-local handoff, and releases valid terminals exactly once. +- [ ] REFACTOR-1 validates reception plus immutable provider/adapter/target lease identity, sequence-fences runtime unhealthy/recovery transitions, gates admission/snapshots, annotates confirmed bound stalls with the non-approval token `recovery_handoff=confirmed`, and releases valid terminals exactly once. - [ ] REFACTOR-2 turns exact-target CAPABILITIES into fail-closed Session-sequenced health evidence and applies only unambiguous current-generation higher-sequence `available` to overlay recovery. - [ ] Add missing/ambiguous identity, stale/mismatch/sequence, normalized/tunnel release-race, and production-probe recovery fixtures; synchronize contracts/specs without mutating config health. -- [ ] Run focused, package, race, vet, provider-only/local-capacity full-cycle, and diff verification with fresh output. +- [ ] Run focused, package, race, vet, provider-only/local-capacity full-cycles, required live long-context preflight/`normal-10` auxiliary regression, and diff verification with fresh output. - [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output. ### [REFACTOR-1] Apply lease-bound runtime health and terminal handoff **Problem:** leases cannot verify adapter/target, provider state lacks runtime observation health, admission/snapshot consult config/connectivity only, and terminal release happens before typed evidence classification. -**Solution:** Extend immutable leases with adapter/target and keep `(node_id,generation,provider_id)` overlay state under the queue lock. Validate receiving generation plus full binding and increasing sequence. Only `unavailable` lowers; request-stalled/available and unknown do not. Annotate every confirmed current bound stall with Edge-local provider/health/`recovery_eligible=true`, then release/pump through the idempotent lease transition. Apply the same ordering to tunnel ERROR. +**Solution:** Extend immutable leases with adapter/target and keep `(node_id,generation,provider_id)` overlay state under the queue lock. Validate receiving generation plus full binding and increasing sequence. Only `unavailable` lowers; request-stalled/available and unknown do not. Annotate every confirmed current bound stall with Edge-local provider/health and `recovery_handoff=confirmed`, which proves only reception/binding/local-fence authority and never approves replay. Ingress remains the sole owner of full recovery eligibility. Then release/pump through the idempotent lease transition. Apply the same ordering to tunnel ERROR. **Modified Files and Checklist:** @@ -82,7 +88,7 @@ Do not create recovery intents, choose alternate providers, consume StreamGate b - [ ] `apps/edge/internal/service/service.go`: expose reception-aware normalized lifecycle handling with compatibility wrapper. - [ ] `apps/edge/internal/service/provider_tunnel.go`: validate/annotate tunnel terminal before routing and keep duplicate cleanup idempotent. - [ ] `apps/edge/internal/service/provider_health_overlay_test.go`: add the S04 table and normalized/tunnel release races. -- [ ] `agent-contract/inner/execution-runtime.md`, `agent-contract/inner/edge-node-runtime-wire.md`, `agent-contract/inner/edge-config-runtime-refresh.md`: document binding, annotation, release ordering, reception authority, and config/overlay separation. +- [ ] `agent-contract/inner/execution-runtime.md`, `agent-contract/inner/edge-node-runtime-wire.md`, `agent-contract/inner/edge-config-runtime-refresh.md`: document binding, the non-approval handoff token, ingress-owned eligibility, release ordering, reception authority, and config/overlay separation. - [ ] `agent-spec/runtime/edge-node-execution.md`, `agent-spec/runtime/provider-pool-config-refresh.md`: reflect terminal handoff and effective admission/snapshot behavior. **Test Strategy:** Cover missing provider id, wrong node/provider/adapter/target, stale generation, equal/lower sequence, unavailable, unknown/request-stalled no-op, new generation, and duplicate normalized/tunnel terminals. Assert one decrement and no newer lease release. @@ -143,8 +149,10 @@ Fresh Go output is required. 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 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 dispatch/tunnel/queue/reconnect. +5. `./scripts/e2e-smoke.sh` — PASS for the repository package smoke. 6. `./scripts/e2e-provider-capacity-smoke.sh` — PASS with zeroed final counters. -7. `git diff --check` — no whitespace errors. +7. `./scripts/e2e-long-context-admission-smoke.sh --preflight` — PASS on the authorized live dev provider pool; otherwise capture the exact external-execution blocker. +8. `./scripts/e2e-long-context-admission-smoke.sh --scenario normal-10` — PASS as an auxiliary live admission/snapshot regression; it is not the S04 semantic oracle. +9. `git diff --check` — no whitespace errors. After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`. diff --git a/agent-task/m-node-provider-execution-liveness-recovery/08+07_health_overlay/code_review_cloud_G09_0.log b/agent-task/m-node-provider-execution-liveness-recovery/08+07_health_overlay/code_review_cloud_G09_0.log new file mode 100644 index 00000000..07fb94dc --- /dev/null +++ b/agent-task/m-node-provider-execution-liveness-recovery/08+07_health_overlay/code_review_cloud_G09_0.log @@ -0,0 +1,185 @@ + + +# 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/08+07_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-G09.md` → `code_review_cloud_G09_0.log` and `PLAN-cloud-G09.md` → `plan_cloud_G09_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_health_overlay/`. If WARN/FAIL, fully write the next filesystem state required by the code-review skill. +4. If PASS, preserve the first-line `milestone-task` metadata in `complete.log` and report it for runtime aggregation. Roadmap state evaluation belongs to `sync-milestone-workstate`. +5. Check applicable `Review-Only Checklist` items at the final `.log` location before reporting. + +--- + +## Implementation Item Completion + +| Item | Status | +|------|---------| +| REFACTOR-1: Apply lease-bound runtime health and terminal handoff | [ ] | +| REFACTOR-2: Feed recovery from the bounded status probe | [ ] | + +## Implementation Checklist + +- [ ] REFACTOR-1 validates reception plus immutable provider/adapter/target lease identity, sequence-fences runtime unhealthy/recovery transitions, gates admission/snapshots, annotates confirmed bound stalls for request-local handoff, and releases valid terminals exactly once. +- [ ] REFACTOR-2 turns exact-target CAPABILITIES into fail-closed Session-sequenced health evidence and applies only unambiguous current-generation higher-sequence `available` to overlay recovery. +- [ ] Add missing/ambiguous identity, stale/mismatch/sequence, normalized/tunnel release-race, and production-probe recovery fixtures; synchronize contracts/specs without mutating config health. +- [ ] Run focused, package, race, vet, provider-only/local-capacity full-cycle, and diff verification 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_0.log`. +- [ ] Archive active `PLAN-*-G??.md` to `plan_cloud_G09_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_health_overlay/` to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/08+07_health_overlay/` and update this checklist at the final archive path. +- [ ] If PASS, 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 reception identity and full immutable lease binding fence every overlay transition and handoff annotation. +- Confirm unavailable/available sequence semantics, config immutability, admission/snapshot projection, and exactly-once release under duplicates/races. +- Confirm CAPABILITIES uses fail-closed `ProbeHealth` plus Session sequence and only exact current higher-sequence available recovers. + +## 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 +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 | diff --git a/agent-task/m-node-provider-execution-liveness-recovery/08+07_health_overlay/plan_cloud_G09_0.log b/agent-task/m-node-provider-execution-liveness-recovery/08+07_health_overlay/plan_cloud_G09_0.log new file mode 100644 index 00000000..576400db --- /dev/null +++ b/agent-task/m-node-provider-execution-liveness-recovery/08+07_health_overlay/plan_cloud_G09_0.log @@ -0,0 +1,150 @@ + + +# Lease-Bound Provider Health Overlay and Recovery Probe + +## For the Implementing Agent + +Implement only this overlay/probe consumer 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 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 + +Authoritative reception identity must be compared with the immutable dispatch lease before typed stall evidence may affect provider-wide health. S04 also requires a separate generation/sequence-fenced runtime overlay, exactly-once terminal release, and a real later bounded exact-target probe that can recover an unavailable provider without mutating config health. + +## Analysis + +### Files Read + +- `apps/edge/internal/bootstrap/runtime.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-roadmap/sdd/knowledge-tool-optimization-extension/openai-compatible-output-validation-filters/SDD.md`, `agent-roadmap/sdd/knowledge-tool-optimization-extension/iop-hot-path-one-shot-execution/SDD.md` +- `agent-test/local/edge-smoke.md`, `agent-test/local/node-smoke.md`, `scripts/e2e-smoke.sh`, `scripts/e2e-provider-capacity-smoke.sh` + +### SDD Criteria + +- SDD: `agent-roadmap/sdd/operational-observability-provider-management/node-provider-execution-liveness-recovery/SDD.md`; status `[승인됨]`; `milestone-task=failure-handoff`. +- S04/Evidence Map S04 requires missing identity, stale generation/sequence, or binding mismatch to be projection no-ops; only current bound `unavailable` lowers, and a later higher-sequence same-generation exact `available` probe recovers. Every valid terminal still releases once. These rules define both implementation items and verification tables. + +### Verification Context + +- `07+06_reception_fence` supplies authoritative callback identity. Queue leases currently omit adapter/target and provider resources have no runtime sequence/health overlay. +- Node CAPABILITIES currently bypasses fail-closed `ProbeHealth` and Session observation sequence; Edge returns the response without applying it. Focused/race tests and repository-native provider/queue smokes are the complete local oracle; no external live scenario is required. +- Related SDDs keep retry/commit ownership in StreamGate Core and do not change this service-layer overlay boundary. + +### Test Coverage Gaps + +- Existing queue tests do not cover adapter/target binding or health sequence transitions; snapshots use config/connectivity only. CAPABILITIES tests do not prove normalized sequenced evidence or Edge recovery. + +### Symbol References + +- Add reception-aware service siblings while retaining one-argument compatibility methods. The predecessor callback contract is consumed in bootstrap; no public symbol is removed. + +### Split Judgment + +- Overlay transition/release and the production recovery probe share the queue lock, provider binding, observation sequence, contracts, and integration oracle, so they remain one child. Candidate selection waits for the completed effective eligibility projection. + +### Scope Rationale + +Do not create recovery intents, choose alternate providers, consume StreamGate budget, add metrics, or mutate Node/config health. + +### 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`. + +## Implementation Checklist + +- [ ] REFACTOR-1 validates reception plus immutable provider/adapter/target lease identity, sequence-fences runtime unhealthy/recovery transitions, gates admission/snapshots, annotates confirmed bound stalls for request-local handoff, and releases valid terminals exactly once. +- [ ] REFACTOR-2 turns exact-target CAPABILITIES into fail-closed Session-sequenced health evidence and applies only unambiguous current-generation higher-sequence `available` to overlay recovery. +- [ ] Add missing/ambiguous identity, stale/mismatch/sequence, normalized/tunnel release-race, and production-probe recovery fixtures; synchronize contracts/specs without mutating config health. +- [ ] Run focused, package, race, vet, provider-only/local-capacity full-cycle, and diff verification with fresh output. +- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output. + +### [REFACTOR-1] Apply lease-bound runtime health and terminal handoff + +**Problem:** leases cannot verify adapter/target, provider state lacks runtime observation health, admission/snapshot consult config/connectivity only, and terminal release happens before typed evidence classification. + +**Solution:** Extend immutable leases with adapter/target and keep `(node_id,generation,provider_id)` overlay state under the queue lock. Validate receiving generation plus full binding and increasing sequence. Only `unavailable` lowers; request-stalled/available and unknown do not. Annotate every confirmed current bound stall with Edge-local provider/health/`recovery_eligible=true`, then release/pump through the idempotent lease transition. Apply the same ordering to tunnel ERROR. + +**Modified Files and Checklist:** + +- [ ] `apps/edge/internal/bootstrap/runtime.go`: consume predecessor node/generation callbacks through reception-aware service methods. +- [ ] `apps/edge/internal/service/model_queue_types.go`: add immutable binding and separate overlay state. +- [ ] `apps/edge/internal/service/model_queue_admission.go`: mint full bindings and reject runtime-unhealthy candidates. +- [ ] `apps/edge/internal/service/model_queue_release.go`: validate, transition, annotate, release once, and pump atomically. +- [ ] `apps/edge/internal/service/model_queue_snapshot.go`: project effective runtime health without changing config. +- [ ] `apps/edge/internal/service/service.go`: expose reception-aware normalized lifecycle handling with compatibility wrapper. +- [ ] `apps/edge/internal/service/provider_tunnel.go`: validate/annotate tunnel terminal before routing and keep duplicate cleanup idempotent. +- [ ] `apps/edge/internal/service/provider_health_overlay_test.go`: add the S04 table and normalized/tunnel release races. +- [ ] `agent-contract/inner/execution-runtime.md`, `agent-contract/inner/edge-node-runtime-wire.md`, `agent-contract/inner/edge-config-runtime-refresh.md`: document binding, annotation, release ordering, reception authority, and config/overlay separation. +- [ ] `agent-spec/runtime/edge-node-execution.md`, `agent-spec/runtime/provider-pool-config-refresh.md`: reflect terminal handoff and effective admission/snapshot behavior. + +**Test Strategy:** Cover missing provider id, wrong node/provider/adapter/target, stale generation, equal/lower sequence, unavailable, unknown/request-stalled no-op, new generation, and duplicate normalized/tunnel terminals. Assert one decrement and no newer lease release. + +**Verification:** overlay/release fixtures must PASS repeatedly. + +### [REFACTOR-2] Feed recovery from the bounded status probe + +**Problem:** Node CAPABILITIES calls the raw prober without normalized health/Session sequence, and Edge does not bind/apply the response. + +**Solution:** Reuse `ProbeHealth` for an exact target, allocate `health_observation_seq` from the same Session, and return only stable identity/normalized status. Edge retains sending node/generation and clears unavailable only for an unambiguous current mapping with strictly higher `available` sequence. Empty/malformed/ambiguous/stale/unknown/unavailable results are no-ops. + +**Modified Files and Checklist:** + +- [ ] `apps/node/internal/node/command_handler.go`: use `ProbeHealth`, Session sequence, and stable result keys. +- [ ] `apps/node/internal/node/command_test.go`: cover exact available, fail-closed unknown rows, and monotonic sequence. +- [ ] `apps/edge/internal/service/node_command.go`: retain authoritative dispatch identity and offer validated evidence to the queue. +- [ ] `apps/edge/internal/service/model_queue_release.go`: share the locked probe-evidence transition. +- [ ] `apps/edge/internal/service/provider_health_overlay_test.go`: drive real CAPABILITIES recovery and all rejection rows. +- [ ] `agent-contract/inner/execution-runtime.md`, `agent-contract/inner/edge-node-runtime-wire.md`, `agent-spec/runtime/edge-node-execution.md`: document probe evidence ownership and recovery fences. + +**Test Strategy:** Lower through a real unavailable terminal, recover only with a later exact current available CAPABILITIES response, and prove every stale/ambiguous/inconclusive response is a no-op. + +**Verification:** Node/Edge capability recovery fixtures must PASS repeatedly. + +## Dependencies and Execution Order + +1. `07+06_reception_fence` must produce `agent-task/m-node-provider-execution-liveness-recovery/07+06_reception_fence/complete.log`. +2. Implement REFACTOR-1 before REFACTOR-2. This child must PASS before `09+08_retry_candidate_policy` starts. + +## Modified Files Summary + +| File | Item | +|------|------| +| `apps/edge/internal/bootstrap/runtime.go` | REFACTOR-1 | +| `apps/edge/internal/service/model_queue_types.go` | REFACTOR-1 | +| `apps/edge/internal/service/model_queue_admission.go` | REFACTOR-1 | +| `apps/edge/internal/service/model_queue_release.go` | REFACTOR-1, REFACTOR-2 | +| `apps/edge/internal/service/model_queue_snapshot.go` | REFACTOR-1 | +| `apps/edge/internal/service/service.go` | REFACTOR-1 | +| `apps/edge/internal/service/provider_tunnel.go` | REFACTOR-1 | +| `apps/edge/internal/service/provider_health_overlay_test.go` | REFACTOR-1, REFACTOR-2 | +| `apps/node/internal/node/command_handler.go` | REFACTOR-2 | +| `apps/node/internal/node/command_test.go` | REFACTOR-2 | +| `apps/edge/internal/service/node_command.go` | REFACTOR-2 | +| `agent-contract/inner/execution-runtime.md` | REFACTOR-1, REFACTOR-2 | +| `agent-contract/inner/edge-node-runtime-wire.md` | REFACTOR-1, REFACTOR-2 | +| `agent-contract/inner/edge-config-runtime-refresh.md` | REFACTOR-1 | +| `agent-spec/runtime/edge-node-execution.md` | REFACTOR-1, REFACTOR-2 | +| `agent-spec/runtime/provider-pool-config-refresh.md` | REFACTOR-1 | +| `agent-task/m-node-provider-execution-liveness-recovery/08+07_health_overlay/CODE_REVIEW-cloud-G09.md` | REFACTOR-1, REFACTOR-2 | + +## Final Verification + +Fresh Go output is required. + +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. +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 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 dispatch/tunnel/queue/reconnect. +6. `./scripts/e2e-provider-capacity-smoke.sh` — PASS with zeroed final counters. +7. `git diff --check` — no whitespace errors. + +After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`. diff --git a/agent-task/m-node-provider-execution-liveness-recovery/09+08_retry_candidate_policy/CODE_REVIEW-cloud-G06.md b/agent-task/m-node-provider-execution-liveness-recovery/09+08_retry_candidate_policy/CODE_REVIEW-cloud-G06.md index f5b54506..944c3b60 100644 --- a/agent-task/m-node-provider-execution-liveness-recovery/09+08_retry_candidate_policy/CODE_REVIEW-cloud-G06.md +++ b/agent-task/m-node-provider-execution-liveness-recovery/09+08_retry_candidate_policy/CODE_REVIEW-cloud-G06.md @@ -1,4 +1,4 @@ - + # Code Review Reference - REFACTOR @@ -14,14 +14,13 @@ ## Overview date=2026-08-05 -task=m-node-provider-execution-liveness-recovery/09+08_retry_candidate_policy, plan=2, tag=REFACTOR +task=m-node-provider-execution-liveness-recovery/09+08_retry_candidate_policy, plan=3, tag=REFACTOR ## Archive Evidence Snapshot - Prior pair: `plan_local_G06_1.log` and `code_review_cloud_G06_1.log` in this task directory. It was unimplemented and has no official verdict, implementation evidence, code change, or verification output. -- Material fresh-review finding: the candidate policy and deterministic capacity oracle were sound, but `normal-10` exercises long-context admission rather than `AvoidProviderID`/fallback behavior and was incorrectly made a completion blocker. -- Replan carryover: preserve the request-local zero-value-compatible policy, initial/queued parity, focused/race and deterministic provider-pool verification, and predecessor overlay PASS gate. Reconfirm the latest related StreamGate SDD boundaries without assigning retry ownership to this service slice. - +- Material prior-review finding: the candidate policy and deterministic capacity oracle were sound, but `normal-10` does not prove `AvoidProviderID` or same-provider fallback semantics. +- Union preparation review archived the unimplemented plan=2 pair as `plan_local_G06_2.log` and `code_review_cloud_G06_2.log`; it had no verdict or implementation evidence. It corrected the consumer reference from 08 to `10+09_stall_recovery` and restored long-context preflight/`normal-10` only as the testing-domain-required auxiliary live admission regression, never as the policy oracle. ## For the Review Agent @@ -31,7 +30,7 @@ Compare implementation of each item against source files and verify that output Review completion means the following steps are finished: 1. Append verdict and `review_rework_count` / `evidence_integrity_failure` routing signals. -2. Archive `CODE_REVIEW-cloud-G06.md` → `code_review_cloud_G06_2.log` and `PLAN-local-G06.md` → `plan_local_G06_2.log`. +2. Archive `CODE_REVIEW-cloud-G06.md` → `code_review_cloud_G06_3.log` and `PLAN-local-G06.md` → `plan_local_G06_3.log`. 3. If PASS, write `complete.log` and move active task directory to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/09+08_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. @@ -48,7 +47,7 @@ Review completion means the following steps are finished: - [ ] 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, and diff verification commands with fresh output. +- [ ] Run focused, package, race, vet, provider-only/local-capacity full-cycles, required live long-context preflight/`normal-10` auxiliary regression, 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 @@ -58,8 +57,8 @@ Review completion means the following steps are finished: - [ ] Append one verdict of `PASS`, `WARN`, or `FAIL` and verified `review_rework_count`, `evidence_integrity_failure` to `Code Review Result`. - [ ] Verify that verdict, `Dimension Assessment`, and Required/Suggested/Nit classifications match. -- [ ] Archive active `CODE_REVIEW-*-G??.md` to `code_review_cloud_G06_2.log`. -- [ ] Archive active `PLAN-*-G??.md` to `plan_local_G06_2.log`. +- [ ] Archive active `CODE_REVIEW-*-G??.md` to `code_review_cloud_G06_3.log`. +- [ ] Archive active `PLAN-*-G??.md` to `plan_local_G06_3.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/09+08_retry_candidate_policy/` to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/09+08_retry_candidate_policy/` and update this checklist at the final archive path. @@ -80,6 +79,7 @@ _Record key design decisions here._ - 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 focused/race plus deterministic provider-pool evidence covers the policy. +- Confirm long-context preflight/`normal-10` is auxiliary admission regression evidence only and any unavailable runner is recorded as external-execution evidence. ## Verification Results @@ -161,6 +161,30 @@ _Paste actual stdout/stderr here._ Command: +```bash +./scripts/e2e-long-context-admission-smoke.sh --preflight +``` + +Output: + +_Paste actual stdout/stderr here._ + +### Verification 8 + +Command: + +```bash +./scripts/e2e-long-context-admission-smoke.sh --scenario normal-10 +``` + +Output: + +_Paste actual stdout/stderr here._ + +### Verification 9 + +Command: + ```bash git diff --check ``` diff --git a/agent-task/m-node-provider-execution-liveness-recovery/09+08_retry_candidate_policy/PLAN-local-G06.md b/agent-task/m-node-provider-execution-liveness-recovery/09+08_retry_candidate_policy/PLAN-local-G06.md index 24fed75b..be005c97 100644 --- a/agent-task/m-node-provider-execution-liveness-recovery/09+08_retry_candidate_policy/PLAN-local-G06.md +++ b/agent-task/m-node-provider-execution-liveness-recovery/09+08_retry_candidate_policy/PLAN-local-G06.md @@ -1,4 +1,4 @@ - + # Request-Local Recovery Candidate Preference @@ -13,8 +13,8 @@ StreamGate can already redispatch through the provider-pool surface, but that re ## Archive Evidence Snapshot - Prior pair: `plan_local_G06_1.log` and `code_review_cloud_G06_1.log` in this task directory. It was unimplemented and has no official verdict, implementation evidence, code change, or verification output. -- Material fresh-review finding: the candidate policy and deterministic capacity oracle were sound, but `normal-10` exercises long-context admission rather than `AvoidProviderID`/fallback behavior and was incorrectly made a completion blocker. -- Replan carryover: preserve the request-local zero-value-compatible policy, initial/queued parity, focused/race and deterministic provider-pool verification, and predecessor overlay PASS gate. Reconfirm the latest related StreamGate SDD boundaries without assigning retry ownership to this service slice. +- Material prior-review finding: the candidate policy and deterministic capacity oracle were sound, but `normal-10` does not prove `AvoidProviderID` or same-provider fallback semantics. +- Union preparation review archived the unimplemented plan=2 pair as `plan_local_G06_2.log` and `code_review_cloud_G06_2.log`; it had no verdict or implementation evidence. It corrected the consumer reference from 08 to `10+09_stall_recovery` and restored long-context preflight/`normal-10` only as the testing-domain-required auxiliary live admission regression, never as the policy oracle. ## Analysis @@ -39,7 +39,7 @@ StreamGate can already redispatch through the provider-pool surface, but that re - Handoff baseline passed fresh at starting HEAD `56e7d78af3cda4a8d6a85af091ad26bce935f8b6`. Implementation waits for `08+07_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`. -- No required verification leaves this checkout. Focused/race tests prove both initial and deferred resolution, and `e2e-provider-capacity-smoke.sh` supplies the repository-native queue/release full-cycle; the related SDDs confirm that this slice exposes policy only while StreamGate Core retains retry/budget ownership. Confidence is medium-high because initial and deferred resolution share mutable queue state. +- Focused/race tests prove both initial and deferred resolution, and `e2e-provider-capacity-smoke.sh` supplies the deterministic queue/release full-cycle. Because this packet changes provider-pool admission, the testing domain also requires live long-context preflight plus `normal-10` as an auxiliary admission regression; it does not prove avoidance/fallback semantics. If an authorized live runner or credential is unavailable, record an `external-execution` verification blocker. The related SDDs confirm that this slice exposes policy only while StreamGate Core retains retry/budget ownership. ### Test Coverage Gaps @@ -58,7 +58,7 @@ StreamGate can already redispatch through the provider-pool surface, but that re ### 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. +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 `10+09_stall_recovery` will populate. ### Final Routing @@ -71,7 +71,7 @@ Do not parse stall failures, create StreamGate intents, consume recovery budget, - [ ] 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, and diff verification commands with fresh output. +- [ ] Run focused, package, race, vet, provider-only/local-capacity full-cycles, required live long-context preflight/`normal-10` auxiliary regression, 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 @@ -140,6 +140,8 @@ Fresh Go output is required; cached output is not acceptable. 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. `git diff --check` — no whitespace errors. +7. `./scripts/e2e-long-context-admission-smoke.sh --preflight` — PASS on the authorized live dev provider pool; otherwise capture the exact external-execution blocker. +8. `./scripts/e2e-long-context-admission-smoke.sh --scenario normal-10` — PASS as an auxiliary live admission regression; it is not the avoidance/fallback semantic oracle. +9. `git diff --check` — no whitespace errors. After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`. diff --git a/agent-task/m-node-provider-execution-liveness-recovery/09+08_retry_candidate_policy/code_review_cloud_G06_2.log b/agent-task/m-node-provider-execution-liveness-recovery/09+08_retry_candidate_policy/code_review_cloud_G06_2.log new file mode 100644 index 00000000..f5b54506 --- /dev/null +++ b/agent-task/m-node-provider-execution-liveness-recovery/09+08_retry_candidate_policy/code_review_cloud_G06_2.log @@ -0,0 +1,189 @@ + + +# 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/09+08_retry_candidate_policy, plan=2, tag=REFACTOR + +## Archive Evidence Snapshot + +- Prior pair: `plan_local_G06_1.log` and `code_review_cloud_G06_1.log` in this task directory. It was unimplemented and has no official verdict, implementation evidence, code change, or verification output. +- Material fresh-review finding: the candidate policy and deterministic capacity oracle were sound, but `normal-10` exercises long-context admission rather than `AvoidProviderID`/fallback behavior and was incorrectly made a completion blocker. +- Replan carryover: preserve the request-local zero-value-compatible policy, initial/queued parity, focused/race and deterministic provider-pool verification, and predecessor overlay PASS gate. Reconfirm the latest related StreamGate SDD boundaries without assigning retry ownership to this service slice. + + +## 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_2.log` and `PLAN-local-G06.md` → `plan_local_G06_2.log`. +3. If PASS, write `complete.log` and move active task directory to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/09+08_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, 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_2.log`. +- [ ] Archive active `PLAN-*-G??.md` to `plan_local_G06_2.log`. +- [ ] Verify that the Agent-Ops managed block in `.gitignore` unignores `agent-task/**/*.md` and `agent-task/**/*.log` and ignores `agent-roadmap/current.md`. +- [ ] If PASS, write `complete.log` based on `agent-ops/skills/common/code-review/templates/complete-log-template.md` and leave no active `.md` files. +- [ ] If PASS, move active task directory `agent-task/m-node-provider-execution-liveness-recovery/09+08_retry_candidate_policy/` to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/09+08_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 focused/race plus deterministic 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 +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 | diff --git a/agent-task/m-node-provider-execution-liveness-recovery/09+08_retry_candidate_policy/plan_local_G06_2.log b/agent-task/m-node-provider-execution-liveness-recovery/09+08_retry_candidate_policy/plan_local_G06_2.log new file mode 100644 index 00000000..24fed75b --- /dev/null +++ b/agent-task/m-node-provider-execution-liveness-recovery/09+08_retry_candidate_policy/plan_local_G06_2.log @@ -0,0 +1,145 @@ + + +# 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_G06_1.log` and `code_review_cloud_G06_1.log` in this task directory. It was unimplemented and has no official verdict, implementation evidence, code change, or verification output. +- Material fresh-review finding: the candidate policy and deterministic capacity oracle were sound, but `normal-10` exercises long-context admission rather than `AvoidProviderID`/fallback behavior and was incorrectly made a completion blocker. +- Replan carryover: preserve the request-local zero-value-compatible policy, initial/queued parity, focused/race and deterministic provider-pool verification, and predecessor overlay PASS gate. Reconfirm the latest related StreamGate SDD boundaries without assigning retry ownership to this service slice. + +## 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-roadmap/sdd/knowledge-tool-optimization-extension/openai-compatible-output-validation-filters/SDD.md`, `agent-roadmap/sdd/knowledge-tool-optimization-extension/iop-hot-path-one-shot-execution/SDD.md` +- `agent-test/local/edge-smoke.md`, `scripts/e2e-smoke.sh`, `scripts/e2e-provider-capacity-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 `08+07_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`. +- No required verification leaves this checkout. Focused/race tests prove both initial and deferred resolution, and `e2e-provider-capacity-smoke.sh` supplies the repository-native queue/release full-cycle; the related SDDs confirm that this slice exposes policy only while StreamGate Core retains retry/budget ownership. 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 `08+07_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. `10+09_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, 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. `08+07_health_overlay` must produce `agent-task/m-node-provider-execution-liveness-recovery/08+07_health_overlay/complete.log`; it is active/missing at refinement. +2. This packet must produce `complete.log` before `10+09_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/09+08_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. `git diff --check` — no whitespace errors. + +After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`. diff --git a/agent-task/m-node-provider-execution-liveness-recovery/10+09_stall_recovery/CODE_REVIEW-cloud-G08.md b/agent-task/m-node-provider-execution-liveness-recovery/10+09_stall_recovery/CODE_REVIEW-cloud-G08.md index 09d44436..e735c3ae 100644 --- a/agent-task/m-node-provider-execution-liveness-recovery/10+09_stall_recovery/CODE_REVIEW-cloud-G08.md +++ b/agent-task/m-node-provider-execution-liveness-recovery/10+09_stall_recovery/CODE_REVIEW-cloud-G08.md @@ -1,4 +1,4 @@ - + # Code Review Reference - API @@ -14,14 +14,14 @@ ## Overview date=2026-08-05 -task=m-node-provider-execution-liveness-recovery/10+09_stall_recovery, plan=2, tag=API +task=m-node-provider-execution-liveness-recovery/10+09_stall_recovery, plan=3, tag=API ## Archive Evidence Snapshot - Prior pair: `plan_cloud_G08_1.log` and `code_review_cloud_G08_1.log` in this task directory. It was unimplemented and has no official verdict, implementation evidence, code change, or verification output. -- Material fresh-review finding: the internal liveness-owner design correctly preserves the latest output-filter contract, but the mandatory `normal-10` live scenario exercises long-context admission and does not prove typed stall mapping, recovery gating, provider avoidance, or bounded dispatch. -- Replan carryover: keep the raw-free mapper, private enabled-gate registration, Core budget/commit/cancel ownership, and S05 matrix; use focused/race plus fake-vLLM/provider-capacity repository-native full cycles as the completion oracle. Gate-disabled/unsupported surfaces remain the explicit no-owner terminal boundary. - +- Prior fresh-review finding: `normal-10` is a long-context admission smoke and does not prove typed stall mapping, recovery gating, provider avoidance, or bounded dispatch. +- Union preparation review archived the unimplemented plan=2 pair as `plan_cloud_G08_2.log` and `code_review_cloud_G08_2.log`; it had no verdict or implementation evidence. The material scope defect was treating default `stream_evidence_gate.enabled=false` OpenAI requests as ownerless even though the approved SDD assigns typed-stall recovery to the supported OpenAI-compatible host. +- Replan carryover: preserve the raw-free mapper, shared Core commit/cancel/side-effect/budget ownership, and S05 matrix. Install one internal liveness owner for every supported OpenAI Chat/Responses normalized or tunnel request regardless of semantic-gate enablement/configuration; only unsupported or non-OpenAI surfaces remain the no-owner typed-terminal boundary. ## For the Review Agent @@ -31,7 +31,7 @@ Compare implementation of each item against source files and verify that output Review completion means the following steps are finished: 1. Append verdict and `review_rework_count` / `evidence_integrity_failure` routing signals. -2. Archive `CODE_REVIEW-cloud-G08.md` → `code_review_cloud_G08_2.log` and `PLAN-cloud-G08.md` → `plan_cloud_G08_2.log`. +2. Archive `CODE_REVIEW-cloud-G08.md` → `code_review_cloud_G08_3.log` and `PLAN-cloud-G08.md` → `plan_cloud_G08_3.log`. 3. If PASS, write `complete.log` and move active task directory to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/10+09_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. @@ -48,9 +48,9 @@ Review completion means the following steps are finished: ## 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. +- [ ] API-1 preserves typed normalized/buffered/tunnel stalls as one raw-free StreamGate `response_stalled` provider error, retaining only sanitized fence/health and `recovery_handoff=confirmed` authority tokens while generic failures keep existing terminal behavior. +- [ ] API-2 installs exactly one internal liveness recovery owner for every supported OpenAI Chat/Responses normalized or tunnel request independent of `stream_evidence_gate.enabled` and configured semantic filters/capabilities; only confirmed handoff, uncommitted, uncanceled, side-effect-safe, budget-available stalls produce ExactReplay, close the fenced old transport, and hand failed-provider/fallback evidence to admission. +- [ ] API-3 adds semantic-gate-enabled/disabled Chat/Responses normalized/tunnel fixtures for available, unavailable, and unknown alternate recovery; available-only same-provider fallback; unavailable/unknown same-only terminal; unsupported/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, 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. @@ -61,8 +61,8 @@ Review completion means the following steps are finished: - [ ] Append one verdict of `PASS`, `WARN`, or `FAIL` and verified `review_rework_count`, `evidence_integrity_failure` to `Code Review Result`. - [ ] Verify that verdict, `Dimension Assessment`, and Required/Suggested/Nit classifications match. -- [ ] Archive active `CODE_REVIEW-*-G??.md` to `code_review_cloud_G08_2.log`. -- [ ] Archive active `PLAN-*-G??.md` to `plan_cloud_G08_2.log`. +- [ ] Archive active `CODE_REVIEW-*-G??.md` to `code_review_cloud_G08_3.log`. +- [ ] Archive active `PLAN-*-G??.md` to `plan_cloud_G08_3.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/10+09_stall_recovery/` to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/10+09_stall_recovery/` and update this checklist at the final archive path. @@ -80,8 +80,8 @@ _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 typed normalized/buffered/tunnel stalls map to one raw-free descriptor carrying only allowlisted fence/health plus `recovery_handoff=confirmed`, while generic failures retain current terminal behavior. +- Confirm every supported OpenAI Chat/Responses normalized or tunnel request gets exactly one private liveness owner regardless of semantic-gate enablement/configuration, normal gate-disabled output stays compatible, and only unsupported/non-OpenAI 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, and local-capacity repository-native evidence satisfy the final commands. diff --git a/agent-task/m-node-provider-execution-liveness-recovery/10+09_stall_recovery/PLAN-cloud-G08.md b/agent-task/m-node-provider-execution-liveness-recovery/10+09_stall_recovery/PLAN-cloud-G08.md index 45767f86..cc6acbd2 100644 --- a/agent-task/m-node-provider-execution-liveness-recovery/10+09_stall_recovery/PLAN-cloud-G08.md +++ b/agent-task/m-node-provider-execution-liveness-recovery/10+09_stall_recovery/PLAN-cloud-G08.md @@ -1,4 +1,4 @@ - + # OpenAI Typed Stall Recovery Handoff @@ -13,8 +13,9 @@ The OpenAI host currently maps every normalized/tunnel terminal error to generic ## Archive Evidence Snapshot - Prior pair: `plan_cloud_G08_1.log` and `code_review_cloud_G08_1.log` in this task directory. It was unimplemented and has no official verdict, implementation evidence, code change, or verification output. -- Material fresh-review finding: the internal liveness-owner design correctly preserves the latest output-filter contract, but the mandatory `normal-10` live scenario exercises long-context admission and does not prove typed stall mapping, recovery gating, provider avoidance, or bounded dispatch. -- Replan carryover: keep the raw-free mapper, private enabled-gate registration, Core budget/commit/cancel ownership, and S05 matrix; use focused/race plus fake-vLLM/provider-capacity repository-native full cycles as the completion oracle. Gate-disabled/unsupported surfaces remain the explicit no-owner terminal boundary. +- Prior fresh-review finding: `normal-10` is a long-context admission smoke and does not prove typed stall mapping, recovery gating, provider avoidance, or bounded dispatch. +- Union preparation review archived the unimplemented plan=2 pair as `plan_cloud_G08_2.log` and `code_review_cloud_G08_2.log`; it had no verdict or implementation evidence. The material scope defect was treating default `stream_evidence_gate.enabled=false` OpenAI requests as ownerless even though the approved SDD assigns typed-stall recovery to the supported OpenAI-compatible host. +- Replan carryover: preserve the raw-free mapper, shared Core commit/cancel/side-effect/budget ownership, and S05 matrix. Install one internal liveness owner for every supported OpenAI Chat/Responses normalized or tunnel request regardless of semantic-gate enablement/configuration; only unsupported or non-OpenAI surfaces remain the no-owner typed-terminal boundary. ## Analysis @@ -42,13 +43,13 @@ The OpenAI host currently maps every normalized/tunnel terminal error to generic - This plan waits for `09+08_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. +- `openai.stream_evidence_gate.enabled` defaults false, while the approved liveness SDD assigns typed-stall recovery to the supported OpenAI-compatible host without conditioning ownership on that flag. Separate semantic evidence-gate activation from liveness ownership: every supported Chat/Responses normalized or tunnel request gets exactly one internal liveness owner and the existing request-local commit/recovery coordinator; the flag and `filters[]` continue to control only configured semantic filters, evidence holding, and capability admission. Normal non-stall behavior with the semantic gate disabled must remain byte/ordering compatible. - No required verification leaves this checkout. Focused/race fixtures directly exercise S05, while fake-vLLM and provider-capacity scripts cover repository-native OpenAI and queue full cycles; the latest output-filter and Hot Path SDDs preserve Core retry/terminal ownership and do not add a second liveness loop. 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. +- Provider-error filter has only unmatched PASS coverage; no test proves that both semantic-gate-enabled and semantic-gate-disabled supported OpenAI requests install exactly one internal liveness owner, while unsupported/non-OpenAI surfaces remain ownerless 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. @@ -63,7 +64,7 @@ The OpenAI host currently maps every normalized/tunnel terminal error to generic ### 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. +Do not add a Core/Node/Edge retry loop, new counter, schema, metric, or non-OpenAI/unsupported recovery owner. 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 @@ -74,9 +75,9 @@ Do not add a Core/Node/Edge retry loop, new counter, schema, metric, non-OpenAI ## 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. +- [ ] API-1 preserves typed normalized/buffered/tunnel stalls as one raw-free StreamGate `response_stalled` provider error, retaining only sanitized fence/health and `recovery_handoff=confirmed` authority tokens while generic failures keep existing terminal behavior. +- [ ] API-2 installs exactly one internal liveness recovery owner for every supported OpenAI Chat/Responses normalized or tunnel request independent of `stream_evidence_gate.enabled` and configured semantic filters/capabilities; only confirmed handoff, uncommitted, uncanceled, side-effect-safe, budget-available stalls produce ExactReplay, close the fenced old transport, and hand failed-provider/fallback evidence to admission. +- [ ] API-3 adds semantic-gate-enabled/disabled Chat/Responses normalized/tunnel fixtures for available, unavailable, and unknown alternate recovery; available-only same-provider fallback; unavailable/unknown same-only terminal; unsupported/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, 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. @@ -84,7 +85,7 @@ Do not add a Core/Node/Edge retry loop, new counter, schema, metric, non-OpenAI **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. +**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 `recovery_handoff=confirmed` authority token; 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`): @@ -112,9 +113,9 @@ case "error", "cancelled": ### [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. +**Problem:** `apps/edge/internal/openai/stream_gate_filters.go:198-237` always passes provider errors, while runtime construction is skipped when `stream_evidence_gate.enabled=false`. Merely changing the configured `provider_error` filter would leave the default supported OpenAI path ownerless and would incorrectly couple liveness to semantic filter configuration. `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. +**Solution:** Store the current eligible stalled provider plus its allowlisted probe classification in request-local ingress state. For every supported OpenAI Chat/Responses normalized or tunnel request, construct the request-local StreamGate commit/recovery host and register exactly one private `response_stalled` liveness filter through the existing extra-registration seam, even when semantic evidence gating is disabled. Do not require or mutate `filters[]`, the configurable `provider_error` foundation filter, selector policy, or provider capability admission. With semantic gating disabled, normal non-stall events release without semantic holding and preserve legacy wire ordering; only typed stalls enter the liveness evaluator. 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; only unsupported/non-OpenAI 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`): @@ -134,8 +135,8 @@ case openAIOutputFilterProviderError: **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_ingress.go`: own concurrency-safe request-local eligible-stall state and construct exactly one internal liveness registration for every supported OpenAI request, independent of semantic-gate enablement. +- [ ] `apps/edge/internal/openai/stream_gate_policy.go`: separate always-on supported-path liveness ownership from `stream_evidence_gate.enabled`; 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. @@ -143,7 +144,7 @@ case openAIOutputFilterProviderError: - [ ] `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. +**Test Strategy:** Build batches with stable descriptor/cause tokens and each commit/side-effect boundary. Assert both semantic-gate-enabled and disabled supported requests own exactly one liveness registration and can emit `RecoveryStrategyExactReplay`; explicitly configured `provider_error` neither duplicates that intent nor changes candidate capabilities. Prove semantic-gate-disabled non-stall output remains legacy-compatible. Unsupported/non-OpenAI, 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. @@ -151,19 +152,19 @@ case openAIOutputFilterProviderError: **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. +**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, unsupported/no-owner, post-commit, unconfirmed, canceled, tool-bearing/side-effect, and exhausted shared budget remain one typed terminal with no duplicate provider dispatch. Run recoverable rows with semantic gating both enabled and disabled. 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/inner/edge-config-runtime-refresh.md`: document that supported-path internal liveness ownership is independent of `stream_evidence_gate.enabled`, configured `filters[]`, and provider capability admission; the flag continues to control semantic evidence-gate behavior. - [ ] `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. +**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. Run supported-path recovery with semantic gating enabled and disabled, and reserve no-owner terminal coverage for unsupported/non-OpenAI surfaces. 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. diff --git a/agent-task/m-node-provider-execution-liveness-recovery/10+09_stall_recovery/code_review_cloud_G08_2.log b/agent-task/m-node-provider-execution-liveness-recovery/10+09_stall_recovery/code_review_cloud_G08_2.log new file mode 100644 index 00000000..09d44436 --- /dev/null +++ b/agent-task/m-node-provider-execution-liveness-recovery/10+09_stall_recovery/code_review_cloud_G08_2.log @@ -0,0 +1,217 @@ + + +# 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/10+09_stall_recovery, plan=2, tag=API + +## Archive Evidence Snapshot + +- Prior pair: `plan_cloud_G08_1.log` and `code_review_cloud_G08_1.log` in this task directory. It was unimplemented and has no official verdict, implementation evidence, code change, or verification output. +- Material fresh-review finding: the internal liveness-owner design correctly preserves the latest output-filter contract, but the mandatory `normal-10` live scenario exercises long-context admission and does not prove typed stall mapping, recovery gating, provider avoidance, or bounded dispatch. +- Replan carryover: keep the raw-free mapper, private enabled-gate registration, Core budget/commit/cancel ownership, and S05 matrix; use focused/race plus fake-vLLM/provider-capacity repository-native full cycles as the completion oracle. 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_2.log` and `PLAN-cloud-G08.md` → `plan_cloud_G08_2.log`. +3. If PASS, write `complete.log` and move active task directory to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/10+09_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, 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_2.log`. +- [ ] Archive active `PLAN-*-G??.md` to `plan_cloud_G08_2.log`. +- [ ] Verify that the Agent-Ops managed block in `.gitignore` unignores `agent-task/**/*.md` and `agent-task/**/*.log` and ignores `agent-roadmap/current.md`. +- [ ] If PASS, write `complete.log` based on `agent-ops/skills/common/code-review/templates/complete-log-template.md` and leave no active `.md` files. +- [ ] If PASS, move active task directory `agent-task/m-node-provider-execution-liveness-recovery/10+09_stall_recovery/` to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/10+09_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, and local-capacity repository-native 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 +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 | diff --git a/agent-task/m-node-provider-execution-liveness-recovery/10+09_stall_recovery/plan_cloud_G08_2.log b/agent-task/m-node-provider-execution-liveness-recovery/10+09_stall_recovery/plan_cloud_G08_2.log new file mode 100644 index 00000000..45767f86 --- /dev/null +++ b/agent-task/m-node-provider-execution-liveness-recovery/10+09_stall_recovery/plan_cloud_G08_2.log @@ -0,0 +1,211 @@ + + +# 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_1.log` and `code_review_cloud_G08_1.log` in this task directory. It was unimplemented and has no official verdict, implementation evidence, code change, or verification output. +- Material fresh-review finding: the internal liveness-owner design correctly preserves the latest output-filter contract, but the mandatory `normal-10` live scenario exercises long-context admission and does not prove typed stall mapping, recovery gating, provider avoidance, or bounded dispatch. +- Replan carryover: keep the raw-free mapper, private enabled-gate registration, Core budget/commit/cancel ownership, and S05 matrix; use focused/race plus fake-vLLM/provider-capacity repository-native full cycles as the completion oracle. 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-roadmap/sdd/knowledge-tool-optimization-extension/openai-compatible-output-validation-filters/SDD.md`, `agent-roadmap/sdd/knowledge-tool-optimization-extension/iop-hot-path-one-shot-execution/SDD.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` + +### 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 `09+08_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. +- No required verification leaves this checkout. Focused/race fixtures directly exercise S05, while fake-vLLM and provider-capacity scripts cover repository-native OpenAI and queue full cycles; the latest output-filter and Hot Path SDDs preserve Core retry/terminal ownership and do not add a second liveness loop. 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 `09+08_retry_candidate_policy` is active with missing `complete.log`; implementation waits for it. That predecessor transitively requires `05+04_failure_wire_contract`, `06+05_failure_wire_mapping`, `07+06_reception_fence`, and `08+07_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, 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. `09+08_retry_candidate_policy` must produce `agent-task/m-node-provider-execution-liveness-recovery/09+08_retry_candidate_policy/complete.log`; it is active/missing at refinement. +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/10+09_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. `git diff --check` — no whitespace errors. + +After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`. diff --git a/agent-task/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/CODE_REVIEW-cloud-G05.md b/agent-task/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/CODE_REVIEW-cloud-G05.md new file mode 100644 index 00000000..cfabc9af --- /dev/null +++ b/agent-task/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/CODE_REVIEW-cloud-G05.md @@ -0,0 +1,160 @@ + + +# 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. +> Execute the plan's selected root cause, scope, files, and dependency decisions as written. Do not choose another owner, narrow/expand the write boundary, or replace a fix with another verification attempt. +> 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/11+06_node_liveness_observability, plan=4, tag=REFACTOR + +## Archive Evidence Snapshot + +- Prior pair: `agent-task/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/plan_local_G05_2.log` and `agent-task/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/code_review_cloud_G05_2.log`; it was an unimplemented plan=2 pair with no official verdict, implementation evidence, code change, or verification output. +- Replan finding: plan=2 correctly isolated Node code but assigned the shared contract/spec consolidation to no active child. Child 14 now owns those shared documents after producers 11, 12, and 13 pass. +- Union preparation review archived the unimplemented plan=3 pair as `plan_local_G05_3.log` and `code_review_cloud_G05_3.log`; it had no verdict or implementation evidence. Its write set overlaps `06+05_failure_wire_mapping` at `apps/node/internal/node/liveness_watchdog.go`, so the task path now encodes predecessor 06 instead of allowing unsafe parallel implementation. +- Carryover: preserve the two existing claimed-stall seams, S06 label/log boundary, process-global production collectors, isolated test registries, duplicate-construction coverage, repository-native two-process diagnostic, and source/test-only 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-G05.md` → `code_review_cloud_G05_4.log` and `PLAN-local-G05.md` → `plan_local_G05_4.log`. +3. If PASS, write `complete.log` and move active task directory to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/`. 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 | [ ] | +| REFACTOR-2 | [ ] | + +## Implementation Checklist + +- [ ] REFACTOR-1 emits exactly one Node response-stall counter observation, duration sample, and safe structured log from the normalized and tunnel stall-finalization seams using one process-global production collector set and only bounded execution-path, health, classification, and fence values. +- [ ] REFACTOR-2 proves request-stalled-but-provider-available and provider-unhealthy outcomes on deterministic normalized/tunnel fixtures, verifies exact metric families/labels and repeated Node construction, and proves request/session/raw prompt/response plus other high-cardinality values are absent from the dedicated log and metric labels. +- [ ] Run every focused, package, race, vet, two-process Edge/Node diagnostic, and diff command in Final Verification 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_4.log`. +- [ ] Archive active `PLAN-*-G??.md` to `plan_local_G05_4.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/11+06_node_liveness_observability/` to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/` and update this checklist at the final archive path. +- [ ] If PASS and task group is `m-node-provider-execution-liveness-recovery`, preserve and report `milestone-task` metadata for runtime aggregation, without modifying roadmap or directly calling `update-roadmap`. +- [ ] If PASS for split work, remove empty active parent `agent-task/m-node-provider-execution-liveness-recovery/` or verify it was kept due to remaining siblings/files. +- [ ] If WARN/FAIL, write the next filesystem state matching code-review verdict and do not write `complete.log`. + +## Deviations from Plan + +_Record any deviations from the plan and the rationale here._ + +## Key Design Decisions + +_Record key design decisions here._ + +## Reviewer Checkpoints + +- Verify predecessor 06 completed before implementation and both claimed-stall branches use the resulting final `liveness_watchdog.go` mapping seam. +- Verify both claimed-stall branches call one observer only after immutable fence/probe evidence exists and that terminal behavior is unchanged. +- Verify default collectors are registered once at package lifetime, every `Node` reuses them, and private-registerer tests cannot mutate or duplicate the default registry. +- Verify metric family names and label names/values are closed and contain no identifier fallback. +- Verify the dedicated log carries only bounded classifications plus numeric duration and that the test seeds and rejects high-cardinality/raw sentinels. +- Verify normalized and provider-tunnel fixtures cover available/request-stalled and unavailable/provider-unhealthy outcomes without sleeps. +- Verify this child changes only its declared Node source/test files; shared contracts/specs are reserved for dependency-ordered child 14. + +## Verification Results + +Fill each output block with actual stdout/stderr. If a command changes, record the replacement and reason in `Deviations from Plan`. + +### Verification 1 + +Command: `go test -count=20 ./apps/node/internal/node -run '^TestNodeLivenessObservability'` + +Expected: PASS every iteration and all four named path/health subtests execute. + +Output: + +### Verification 2 + +Command: `go test -count=1 ./packages/go/execution ./apps/node/...` + +Expected: PASS under the Node local profile. + +Output: + +### Verification 3 + +Command: `go test -race -count=3 ./apps/node/internal/node -run 'LivenessObservability|Watchdog|HealthEvidence'` + +Expected: PASS with no race report. + +Output: + +### Verification 4 + +Command: `go vet ./packages/go/execution ./apps/node/...` + +Expected: no diagnostics. + +Output: + +### Verification 5 + +Command: `IOP_DEV_RECONNECT_BIND_TIMEOUT=45 ./scripts/dev/edge-node-reconnect-diagnostic.sh` + +Expected: PASS using separate Edge/Node entrypoints for registration, two same-session messages, one post-reconnect message, Node-to-Edge payload equality, `/nodes`, `/capabilities`, `/transport`, reconnect, and exactly-once terminal ordering. + +Output: + +### Verification 6 + +Command: `git diff --check` + +Expected: no whitespace errors. + +Output: + +--- + +> **[IMPLEMENTING AGENT — BEFORE SAVING] Have you filled in every implementation-owned section?** +> If anything is blank, go back and fill it in before saving this file. +> Leave review-agent-only sections unchanged. + +## Section Ownership + +| Section | Owner | Note | +|---------|-------|------| +| Header comment, Overview, Review Agent Instructions | Fixed at stub creation | Implementing agent must not modify or execute these (archive, complete.log, and task-directory archive move are review-agent only) | +| Archive Evidence Snapshot | Fixed at stub creation from plan when present | Implementing agent uses it as default prior-loop context; read only the specific archive files cited there when more detail is required | +| Implementation Item Completion (item names) | Fixed at stub creation | Implementing agent checks `[ ]` → `[x]` only | +| Implementation Checklist (item text/order) | Fixed at stub creation from plan | Implementing agent checks `[ ]` → `[x]` only | +| Review-Only Checklist | Review agent only | Implementing agent must not modify or check this section | +| Deviations from Plan, Key Design Decisions | Implementing agent | Replace placeholder text with actual content | +| Reviewer Checkpoints | Fixed at stub creation | Pre-filled from plan | +| Verification Results (section headings + commands) | Fixed at stub creation | Implementing agent fills in command output only; command changes require a `Deviations from Plan` entry | +| Code Review Result | Review agent appends | Not included in stub | diff --git a/agent-task/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/PLAN-local-G05.md b/agent-task/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/PLAN-local-G05.md new file mode 100644 index 00000000..3bcf0624 --- /dev/null +++ b/agent-task/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/PLAN-local-G05.md @@ -0,0 +1,185 @@ + + +# Node Response-Stall Operational Evidence + +## For the Implementing Agent + +Implement only this Node liveness-observability slice after predecessor 06 has produced `complete.log`, 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, 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 Node already produces one fenced `response_stalled` terminal with joined health evidence for normalized and tunnel attempts, but operators cannot count or time those stalls without inspecting request-scoped events. This slice adds bounded metrics and a structured-log contract at the existing exactly-once stall finalization seam without changing execution, wire, or retry behavior. Shared execution contracts and the living Edge/Node spec are consolidated by the ordered sibling `14+11,12,13_observability_contracts` after all three operational-evidence producers pass. + +## Archive Evidence Snapshot + +- Prior pair: `agent-task/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/plan_local_G05_2.log` and `agent-task/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/code_review_cloud_G05_2.log`; it was an unimplemented plan=2 pair with no official verdict, implementation evidence, code change, or verification output. +- Replan finding: plan=2 correctly isolated Node code but assigned the shared contract/spec consolidation to no active child. Child 14 now owns those shared documents after producers 11, 12, and 13 pass. +- Union preparation review archived the unimplemented plan=3 pair as `plan_local_G05_3.log` and `code_review_cloud_G05_3.log`; it had no verdict or implementation evidence. Its write set overlaps `06+05_failure_wire_mapping` at `apps/node/internal/node/liveness_watchdog.go`, so the task path now encodes predecessor 06 instead of allowing unsafe parallel implementation. +- Carryover: preserve the two existing claimed-stall seams, S06 label/log boundary, process-global production collectors, isolated test registries, duplicate-construction coverage, repository-native two-process diagnostic, and source/test-only boundary. + +## Analysis + +### Files Read + +- `apps/node/internal/node/node.go` +- `apps/node/internal/node/liveness_watchdog.go` +- `apps/node/internal/node/liveness_health_evidence.go` +- `apps/node/internal/node/liveness_watchdog_test.go` +- `apps/node/internal/node/liveness_watchdog_lifecycle_test.go` +- `apps/node/internal/node/liveness_health_evidence_test.go` +- `apps/node/internal/node/provider_tunnel_liveness_test.go` +- `packages/go/observability/observability.go` +- `agent-contract/inner/execution-runtime.md` +- `agent-contract/inner/edge-node-runtime-wire.md` +- `agent-spec/runtime/edge-node-execution.md` +- `agent-roadmap/phase/operational-observability-provider-management/milestones/node-provider-execution-liveness-recovery.md` +- `agent-roadmap/sdd/operational-observability-provider-management/node-provider-execution-liveness-recovery/SDD.md` +- `agent-task/m-node-provider-execution-liveness-recovery/08+07_health_overlay/PLAN-cloud-G09.md` +- `agent-task/m-node-provider-execution-liveness-recovery/10+09_stall_recovery/PLAN-cloud-G08.md` +- `agent-task/m-node-provider-execution-liveness-recovery/12+08_health_overlay_observability/PLAN-cloud-G08.md` +- `agent-task/m-node-provider-execution-liveness-recovery/13+10_recovery_observability/PLAN-cloud-G08.md` +- `agent-test/local/node-smoke.md` +- `agent-ops/rules/project/domain/testing/rules.md` +- `agent-ops/skills/project/e2e-smoke/SKILL.md` +- `scripts/dev/edge.sh` +- `scripts/dev/node.sh` +- `scripts/dev/edge-node-reconnect-diagnostic.sh` + +### SDD Criteria + +- SDD: `agent-roadmap/sdd/operational-observability-provider-management/node-provider-execution-liveness-recovery/SDD.md`; status `[승인됨]`; first-line `milestone-task=ops-evidence`. +- Acceptance Scenario S06 and Evidence Map S06 require Node stall count/duration plus fence/probe result for deterministic normalized-run and tunnel stalls, with request/session/raw prompt/response and high-cardinality values absent from metric labels and the dedicated structured log. +- Those rows define REFACTOR-1's closed label vocabulary and REFACTOR-2's two-path health matrix and negative leakage assertions. + +### Verification Context + +- No handoff artifact was supplied. The requested pre-refine checkpoint is `729f458a42f2c0c05fcb5d1c84738b41b41cd7cf`, which matched HEAD during replanning; current production source was unchanged from the baseline used by the prior pair. +- The local Node profile supplies `go version && go env GOMOD`, `go test -count=1 ./packages/go/execution ./apps/node/...`, and `git diff --check`. Prior planning evidence recorded Go `1.26.2`, the repository module, executable diagnostic scripts, and a passing focused liveness baseline. This preparation stage did not rerun product tests. +- The current stall seams are `liveness_watchdog.go:213-224` and `liveness_watchdog.go:304-311`; both already follow a successful fence claim and produce exactly one terminal. `06+05_failure_wire_mapping` also writes this file, so its PASS is required before these line anchors and mapping semantics are implemented against the final predecessor source. Confidence is high because this child then observes the immutable `stallObservation` without adding lifecycle state. +- No external verification is required. Manual clocks and fake normalized/tunnel providers give deterministic local evidence. The repository diagnostic starts real Edge and Node entrypoints separately and verifies registration, ordered runs, reconnect, transport state, payload equality, and exactly-once terminal ordering. + +### Test Coverage Gaps + +- Existing watchdog tests verify terminal metadata and races but do not gather Prometheus series or capture a dedicated safe structured log. +- No test proves normalized and tunnel attempts use the same bounded labels for both `request_stalled`/available and `provider_unhealthy`/unavailable evidence. +- No test rejects run, attempt, request, session, adapter, target, prompt, response, or credential values from the new label/log surface. +- No test proves constructing multiple `Node` instances reuses one process-global production collector set instead of registering the same metric names repeatedly. + +### Symbol References + +- None. No existing symbol is renamed or removed; `Node` gains one internal observer initialized by `New` and replaceable only by same-package tests. + +### Split Judgment + +- This child is the cohesive Node producer: one immutable `stallObservation` maps to one counter, one duration histogram, and one dedicated log for both execution paths. Predecessor index 06 (`06+05_failure_wire_mapping`) is active and its `complete.log` is missing; the dependency is required because both packets write `liveness_watchdog.go`. +- `12+08_health_overlay_observability` and `13+10_recovery_observability` own independent Edge overlay and recovery evidence. New sibling `14+11,12,13_observability_contracts` depends on completed children 11, 12, and 13 and alone owns the shared execution contract, wire contract, and living Edge/Node spec. The split closes the Epic scope union while leaving every implementation write set disjoint. + +### Scope Rationale + +Do not change stall detection, timer reset, fence/probe ordering, wire metadata, retryability, Edge ingestion, provider overlay, recovery selection, dashboards, config, contracts, or specs. Do not add node/run/attempt/provider/session/adapter/target identifiers as metric labels or dedicated log fields. Shared contract/spec consolidation is explicitly owned by child 14 and must not be performed here. + +### Final Routing + +- `evaluation_mode=isolated-reassessment`; finalizer=`finalize-task-policy.sh pair`. +- Build closure true; scores `(1,1,2,0,1)`, grade G05, route `local-fit` -> `PLAN-local-G05.md`. +- Review closure true; scores `(1,1,2,0,1)`, grade G05, route `official-review` -> `CODE_REVIEW-cloud-G05.md` (`codex`, `gpt-5.6-sol`, `xhigh`). +- `large_indivisible_context=false`; positive loop risks: `concurrent_consistency`, `variant_product` (2). No recovery signal, capability gap, review rework, or evidence-integrity failure. + +## Implementation Checklist + +- [ ] REFACTOR-1 emits exactly one Node response-stall counter observation, duration sample, and safe structured log from the normalized and tunnel stall-finalization seams using one process-global production collector set and only bounded execution-path, health, classification, and fence values. +- [ ] REFACTOR-2 proves request-stalled-but-provider-available and provider-unhealthy outcomes on deterministic normalized/tunnel fixtures, verifies exact metric families/labels and repeated Node construction, and proves request/session/raw prompt/response plus other high-cardinality values are absent from the dedicated log and metric labels. +- [ ] Run every focused, package, race, vet, two-process Edge/Node diagnostic, and diff command in Final Verification with fresh output. +- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output. + +### [REFACTOR-1] Emit bounded Node stall metrics and logs + +**Problem:** `apps/node/internal/node/liveness_watchdog.go:213-224` and `apps/node/internal/node/liveness_watchdog.go:304-311` finalize typed stall evidence but expose it only through request-scoped terminals. Operators cannot count or time stalls by safe fence/probe axes. + +**Solution:** Add a test-injectable `nodeLivenessObserver`. Register one package-level production collector set exactly once with the default Prometheus registerer and reuse it from every `Node`; a constructor that accepts an explicit `prometheus.Registerer` creates isolated collectors only for tests. Never call `promauto.New*` or `MustRegister` from `Node.New` or per attempt. Emit `iop_node_response_stalls_total{execution_path,provider_health,liveness_classification,attempt_fence}` and `iop_node_response_stall_duration_seconds` with the identical four-label set. Normalize every label through closed allowlists (`normalized|provider_tunnel|unknown`, the three health/classification pairs, and `confirmed|unconfirmed|unknown`). Write `node_response_stall_observation` with only those labels and numeric `idle_duration_ms`. Install the reusable observer on `Node` and invoke it immediately after `stallObservationFrom` in each already-claimed stall branch; observer failure or disabled logging must never change terminal delivery. + +Before (`apps/node/internal/node/liveness_watchdog.go:213`): + +```go +obs := stallObservationFrom(result, time.Duration(spec.ResponseStallTimeoutMS)*time.Millisecond, seq) +sink.queueClaimedTerminal(stalledRuntimeEvent(spec, obs)) +``` + +After: + +```go +obs := stallObservationFrom(result, time.Duration(spec.ResponseStallTimeoutMS)*time.Millisecond, seq) +n.liveness.Observe("normalized", obs) +sink.queueClaimedTerminal(stalledRuntimeEvent(spec, obs)) +``` + +Apply the same call with `provider_tunnel` before `emitClaimedTerminal` at the tunnel seam. Use the existing Prometheus and zap dependencies; do not add an alternate metrics server. + +**Modified Files and Checklist:** + +- [ ] `apps/node/internal/node/node.go`: hold the internal observer and initialize its production collectors/logger without changing the public constructor signature. +- [ ] `apps/node/internal/node/liveness_watchdog.go`: invoke the observer once in each claimed normalized/tunnel stall path. +- [ ] `apps/node/internal/node/liveness_observability.go`: define collectors, closed normalization, safe log fields, and the test-injection constructor. + +**Test Strategy:** Write tests in REFACTOR-2; do not alter existing lifecycle fixtures except to reuse their manual clocks/providers. + +**Verification:** `go test -count=20 ./apps/node/internal/node -run '^TestNodeLivenessObservability'` must pass every iteration and report both paths. + +### [REFACTOR-2] Prove the bounded evidence matrix + +**Problem:** `apps/node/internal/node/liveness_health_evidence.go:43-70` intentionally includes run/attempt/adapter/target in terminal metadata, so copying that map into metrics or the dedicated log would violate S06 even though the wire terminal itself is valid. Existing tests do not guard this new boundary. + +**Solution:** Add a two-path table using the production watchdog seams and a private Prometheus registry/zap observer. Cover available/request-stalled and unavailable/provider-unhealthy with confirmed and unconfirmed fences where deterministic. Assert counter delta one, histogram count/duration, exact label names and allowlisted values, one dedicated log per claimed stall, and absence of sentinel high-cardinality/raw values from labels and encoded log fields. Construct multiple default `Node` values in one process and assert no duplicate-registration panic while a private registry remains isolated. Preserve the existing richer internal terminal metadata contract without editing shared contracts/specs from this child. + +Before (`apps/node/internal/node/liveness_health_evidence.go:56`): + +```go +metadata := map[string]string{ + "failure_code": string(runtime.FailureCodeResponseStalled), + "run_id": runID, + "attempt_id": runID, +``` + +After (observability projection, not terminal metadata replacement): + +```go +labels := normalizeNodeLivenessLabels(path, obs) +observer.stalls.WithLabelValues(labels...).Inc() +observer.duration.WithLabelValues(labels...).Observe(obs.idle.Seconds()) +``` + +**Modified Files and Checklist:** + +- [ ] `apps/node/internal/node/liveness_observability_test.go`: add deterministic normalized/tunnel metric, duration, exact-once, allowlist, and log-leakage cases. + +**Test Strategy:** Create `TestNodeLivenessObservability` subtests for `normalized/request-stalled`, `normalized/provider-unhealthy`, `provider_tunnel/request-stalled`, and `provider_tunnel/provider-unhealthy`, plus repeated-default-construction. Seed run/session/adapter/target/prompt/response/credential sentinels and inspect gathered DTO labels plus zap fields/message text for absence. + +**Verification:** the focused test above plus the Node package/race commands below must pass with no zero-match test run. + +## Dependencies and Execution Order + +1. Predecessor index 06, `06+05_failure_wire_mapping`, must produce `agent-task/m-node-provider-execution-liveness-recovery/06+05_failure_wire_mapping/complete.log` or exactly one matching same-task-group archived `complete.log`; it is active and missing at preparation. +2. This index-11 producer must PASS before `14+11,12,13_observability_contracts` starts. + +## Modified Files Summary + +| File | Item | +|------|------| +| `apps/node/internal/node/node.go` | REFACTOR-1 | +| `apps/node/internal/node/liveness_watchdog.go` | REFACTOR-1 | +| `apps/node/internal/node/liveness_observability.go` | REFACTOR-1 | +| `apps/node/internal/node/liveness_observability_test.go` | REFACTOR-2 | +| `agent-task/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/CODE_REVIEW-cloud-G05.md` | REFACTOR-1, REFACTOR-2 | + +## Final Verification + +Fresh Go output is required; cached output is not acceptable. + +1. `go test -count=20 ./apps/node/internal/node -run '^TestNodeLivenessObservability'` — PASS every iteration and all four named path/health subtests execute. +2. `go test -count=1 ./packages/go/execution ./apps/node/...` — PASS under the Node local profile. +3. `go test -race -count=3 ./apps/node/internal/node -run 'LivenessObservability|Watchdog|HealthEvidence'` — PASS with no race report. +4. `go vet ./packages/go/execution ./apps/node/...` — no diagnostics. +5. `IOP_DEV_RECONNECT_BIND_TIMEOUT=45 ./scripts/dev/edge-node-reconnect-diagnostic.sh` — PASS using separate `scripts/dev/edge.sh` and `scripts/dev/node.sh` processes; registration, the first two same-session messages, post-reconnect message, Node-to-Edge payload equality, `/nodes`, `/capabilities`, `/transport`, and exactly-once terminal ordering are all verified. +6. `git diff --check` — no whitespace errors. + +After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`. diff --git a/agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/code_review_cloud_G05_0.log b/agent-task/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/code_review_cloud_G05_0.log similarity index 100% rename from agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/code_review_cloud_G05_0.log rename to agent-task/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/code_review_cloud_G05_0.log diff --git a/agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/code_review_cloud_G05_1.log b/agent-task/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/code_review_cloud_G05_1.log similarity index 100% rename from agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/code_review_cloud_G05_1.log rename to agent-task/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/code_review_cloud_G05_1.log diff --git a/agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/code_review_cloud_G05_2.log b/agent-task/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/code_review_cloud_G05_2.log similarity index 100% rename from agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/code_review_cloud_G05_2.log rename to agent-task/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/code_review_cloud_G05_2.log diff --git a/agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/CODE_REVIEW-cloud-G05.md b/agent-task/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/code_review_cloud_G05_3.log similarity index 100% rename from agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/CODE_REVIEW-cloud-G05.md rename to agent-task/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/code_review_cloud_G05_3.log diff --git a/agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/plan_local_G05_0.log b/agent-task/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/plan_local_G05_0.log similarity index 100% rename from agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/plan_local_G05_0.log rename to agent-task/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/plan_local_G05_0.log diff --git a/agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/plan_local_G05_1.log b/agent-task/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/plan_local_G05_1.log similarity index 100% rename from agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/plan_local_G05_1.log rename to agent-task/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/plan_local_G05_1.log diff --git a/agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/plan_local_G05_2.log b/agent-task/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/plan_local_G05_2.log similarity index 100% rename from agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/plan_local_G05_2.log rename to agent-task/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/plan_local_G05_2.log diff --git a/agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/PLAN-local-G05.md b/agent-task/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/plan_local_G05_3.log similarity index 100% rename from agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/PLAN-local-G05.md rename to agent-task/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/plan_local_G05_3.log diff --git a/agent-task/m-node-provider-execution-liveness-recovery/14+11,12,13_observability_contracts/CODE_REVIEW-cloud-G05.md b/agent-task/m-node-provider-execution-liveness-recovery/14+11,12,13_observability_contracts/CODE_REVIEW-cloud-G05.md index 3747a744..1f8be6c8 100644 --- a/agent-task/m-node-provider-execution-liveness-recovery/14+11,12,13_observability_contracts/CODE_REVIEW-cloud-G05.md +++ b/agent-task/m-node-provider-execution-liveness-recovery/14+11,12,13_observability_contracts/CODE_REVIEW-cloud-G05.md @@ -19,7 +19,7 @@ task=m-node-provider-execution-liveness-recovery/14+11,12,13_observability_contr ## Archive Evidence Snapshot -- Replaced pair evidence: `agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/plan_local_G05_2.log` and `agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/code_review_cloud_G05_2.log`; the pair was unimplemented and had no official verdict or verification output. +- Replaced pair evidence: `agent-task/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/plan_local_G05_2.log` and `agent-task/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/code_review_cloud_G05_2.log`; the pair was unimplemented and had no official verdict or verification output. - Pre-refine intent at checkpoint `729f458a42f2c0c05fcb5d1c84738b41b41cd7cf`: the immediate prior pair set assigned `agent-contract/inner/execution-runtime.md`, `agent-contract/inner/edge-node-runtime-wire.md`, and `agent-spec/runtime/edge-node-execution.md` across children 11/12/13. Refinement removed overlap but did not create a replacement owner. - Carryover: preserve disjoint implementation write sets and document only reviewed behavior. Do not copy planned claims into current contracts/specs before dependencies pass, and do not reopen the overlay-specific or OpenAI-specific documents that remain owned by children 12 and 13. diff --git a/agent-task/m-node-provider-execution-liveness-recovery/14+11,12,13_observability_contracts/PLAN-local-G05.md b/agent-task/m-node-provider-execution-liveness-recovery/14+11,12,13_observability_contracts/PLAN-local-G05.md index bc114b61..ea078076 100644 --- a/agent-task/m-node-provider-execution-liveness-recovery/14+11,12,13_observability_contracts/PLAN-local-G05.md +++ b/agent-task/m-node-provider-execution-liveness-recovery/14+11,12,13_observability_contracts/PLAN-local-G05.md @@ -12,7 +12,7 @@ The three operational-evidence producers are intentionally independent: child 11 ## Archive Evidence Snapshot -- Replaced pair evidence: `agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/plan_local_G05_2.log` and `agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/code_review_cloud_G05_2.log`; the pair was unimplemented and had no official verdict or verification output. +- Replaced pair evidence: `agent-task/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/plan_local_G05_2.log` and `agent-task/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/code_review_cloud_G05_2.log`; the pair was unimplemented and had no official verdict or verification output. - Pre-refine intent at checkpoint `729f458a42f2c0c05fcb5d1c84738b41b41cd7cf`: the immediate prior pair set assigned `agent-contract/inner/execution-runtime.md`, `agent-contract/inner/edge-node-runtime-wire.md`, and `agent-spec/runtime/edge-node-execution.md` across children 11/12/13. Refinement removed overlap but did not create a replacement owner. - Carryover: preserve disjoint implementation write sets and document only reviewed behavior. Do not copy planned claims into current contracts/specs before dependencies pass, and do not reopen the overlay-specific or OpenAI-specific documents that remain owned by children 12 and 13. @@ -24,7 +24,7 @@ The three operational-evidence producers are intentionally independent: child 11 - `agent-roadmap/sdd/operational-observability-provider-management/node-provider-execution-liveness-recovery/SDD.md` - `agent-task/m-node-provider-execution-liveness-recovery/08+07_health_overlay/PLAN-cloud-G09.md` - `agent-task/m-node-provider-execution-liveness-recovery/10+09_stall_recovery/PLAN-cloud-G08.md` -- `agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/PLAN-local-G05.md` +- `agent-task/m-node-provider-execution-liveness-recovery/11+06_node_liveness_observability/PLAN-local-G05.md` - `agent-task/m-node-provider-execution-liveness-recovery/12+08_health_overlay_observability/PLAN-cloud-G08.md` - `agent-task/m-node-provider-execution-liveness-recovery/13+10_recovery_observability/PLAN-cloud-G08.md` - `agent-contract/index.md` @@ -53,7 +53,7 @@ The three operational-evidence producers are intentionally independent: child 11 ### Verification Context - No handoff artifact was supplied. The requested comparison checkpoint is `729f458a42f2c0c05fcb5d1c84738b41b41cd7cf`, which matched HEAD during preparation. No product source had changed relative to the plans being reviewed. -- This child is dependency-waiting at creation: predecessor indices 11 (`11_node_liveness_observability`), 12 (`12+08_health_overlay_observability`), and 13 (`13+10_recovery_observability`) are active and their `complete.log` files are missing. At implementation, check the active sibling first and then the same task group's matching archived sibling; exactly one candidate per index must exist. +- This child is dependency-waiting at creation: predecessor indices 11 (`11+06_node_liveness_observability`), 12 (`12+08_health_overlay_observability`), and 13 (`13+10_recovery_observability`) are active and their `complete.log` files are missing. At implementation, check the active sibling first and then the same task group's matching archived sibling; exactly one candidate per index must exist. - Once unblocked, completion evidence and current source—not planned symbol names alone—are authoritative. Verification reuses the focused/package tests required by the three producer children, then checks the exact documented metric families and a clean diff. - No external service is required. This is a documentation-only closure over locally reviewed implementation and repository-local tests. @@ -85,7 +85,7 @@ Update only the current behavior proven by all three dependency completion logs ## Dependencies and Execution Order -1. Predecessor index 11, `11_node_liveness_observability`, must produce one active or same-task-group archived `complete.log`; it is active and missing at plan creation. +1. Predecessor index 11, `11+06_node_liveness_observability`, must produce one active or same-task-group archived `complete.log`; it is active and missing at plan creation. 2. Predecessor index 12, `12+08_health_overlay_observability`, must produce one active or same-task-group archived `complete.log`; it is active and missing at plan creation. 3. Predecessor index 13, `13+10_recovery_observability`, must produce one active or same-task-group archived `complete.log`; it is active and missing at plan creation. 4. Implement REFACTOR-1 before REFACTOR-2 so the living spec cites the finalized shared contract language.