From 1aca3f4cf537399f9e32e30a5ddde094be6ab49a Mon Sep 17 00:00:00 2001 From: toki Date: Wed, 5 Aug 2026 14:02:50 +0900 Subject: [PATCH] =?UTF-8?q?chore(epic):=20liveness-operations=20=EC=A4=80?= =?UTF-8?q?=EB=B9=84=20=EA=B2=B0=EA=B3=BC=EB=A5=BC=20=EA=B2=80=EC=A6=9D?= =?UTF-8?q?=ED=95=9C=EB=8B=A4?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../CODE_REVIEW-cloud-G05.md | 18 +- .../PLAN-local-G05.md | 41 ++-- .../code_review_cloud_G05_2.log | 158 ++++++++++++++++ .../plan_local_G05_2.log | 178 ++++++++++++++++++ .../CODE_REVIEW-cloud-G05.md | 158 ++++++++++++++++ .../PLAN-local-G05.md | 176 +++++++++++++++++ 6 files changed, 700 insertions(+), 29 deletions(-) create mode 100644 agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/code_review_cloud_G05_2.log create mode 100644 agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/plan_local_G05_2.log create mode 100644 agent-task/m-node-provider-execution-liveness-recovery/14+11,12,13_observability_contracts/CODE_REVIEW-cloud-G05.md create mode 100644 agent-task/m-node-provider-execution-liveness-recovery/14+11,12,13_observability_contracts/PLAN-local-G05.md 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_node_liveness_observability/CODE_REVIEW-cloud-G05.md index eee25175..16437555 100644 --- 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_node_liveness_observability/CODE_REVIEW-cloud-G05.md @@ -1,4 +1,4 @@ - + # Code Review Reference - REFACTOR @@ -15,13 +15,13 @@ ## Overview date=2026-08-05 -task=m-node-provider-execution-liveness-recovery/11_node_liveness_observability, plan=2, tag=REFACTOR +task=m-node-provider-execution-liveness-recovery/11_node_liveness_observability, plan=3, tag=REFACTOR ## Archive Evidence Snapshot -- Prior pair: `agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/plan_local_G05_1.log` and `agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/code_review_cloud_G05_1.log`; it was an unimplemented plan=1 pair with no official verdict, implementation evidence, code change, or verification output. -- Replan finding: the plan's contract/spec write set overlapped independently runnable predecessor and sibling work (`08+07_health_overlay`, `09+08_retry_candidate_policy`, and observability siblings 12/13), so the child could create avoidable merge conflicts despite owning only Node-local evidence. -- Carryover: preserve the two existing claimed-stall seams, S06 label/log boundary, process-global production collectors, isolated test registries, duplicate-construction coverage, and the repository-native two-process diagnostic; keep this child source/test-only and leave living-contract consolidation to ordered dependent work or Milestone closure. +- Prior pair: `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`; 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. The Epic child-scope union therefore omitted `agent-contract/inner/execution-runtime.md`, `agent-contract/inner/edge-node-runtime-wire.md`, and `agent-spec/runtime/edge-node-execution.md`, although the SDD Source of Truth and the pre-refine pair set required those updates. +- Carryover: preserve the two existing claimed-stall seams, S06 label/log boundary, process-global production collectors, isolated test registries, duplicate-construction coverage, and repository-native two-process diagnostic. Keep this child source/test-only; the new dependency-ordered child 14 owns the shared documents without overlapping this child or independently runnable siblings 12/13. ## 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-G05.md` → `code_review_cloud_G05_2.log` and `PLAN-local-G05.md` → `plan_local_G05_2.log`. +2. Archive `CODE_REVIEW-cloud-G05.md` → `code_review_cloud_G05_3.log` and `PLAN-local-G05.md` → `plan_local_G05_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/11_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. @@ -59,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_G05_2.log`. -- [ ] Archive active `PLAN-*-G??.md` to `plan_local_G05_2.log`. +- [ ] Archive active `CODE_REVIEW-*-G??.md` to `code_review_cloud_G05_3.log`. +- [ ] Archive active `PLAN-*-G??.md` to `plan_local_G05_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/11_node_liveness_observability/` to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/` and update this checklist at the final archive path. @@ -83,7 +83,7 @@ _Record key design decisions here._ - 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 independent child changes only its declared Node source/test files and does not reopen shared contracts/specs owned by concurrent siblings or Milestone consolidation. +- Verify this child changes only its declared Node source/test files; shared contracts/specs are reserved for dependency-ordered child 14. ## Verification Results 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_node_liveness_observability/PLAN-local-G05.md index d70dee65..6fa7a2d7 100644 --- 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_node_liveness_observability/PLAN-local-G05.md @@ -1,4 +1,4 @@ - + # Node Response-Stall Operational Evidence @@ -8,13 +8,13 @@ Implement only this Node liveness-observability slice, run every verification co ## 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 a bounded metric and structured-log contract at the existing exactly-once stall finalization seam without changing execution, wire, or retry behavior. +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_node_liveness_observability/plan_local_G05_1.log` and `agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/code_review_cloud_G05_1.log`; it was an unimplemented plan=1 pair with no official verdict, implementation evidence, code change, or verification output. -- Replan finding: the plan's contract/spec write set overlapped independently runnable predecessor and sibling work (`08+07_health_overlay`, `09+08_retry_candidate_policy`, and observability siblings 12/13), so the child could create avoidable merge conflicts despite owning only Node-local evidence. -- Carryover: preserve the two existing claimed-stall seams, S06 label/log boundary, process-global production collectors, isolated test registries, duplicate-construction coverage, and the repository-native two-process diagnostic; keep this child source/test-only and leave living-contract consolidation to ordered dependent work or Milestone closure. +- Prior pair: `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`; 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. The Epic child-scope union therefore omitted `agent-contract/inner/execution-runtime.md`, `agent-contract/inner/edge-node-runtime-wire.md`, and `agent-spec/runtime/edge-node-execution.md`, although the SDD Source of Truth and the pre-refine pair set required those updates. +- Carryover: preserve the two existing claimed-stall seams, S06 label/log boundary, process-global production collectors, isolated test registries, duplicate-construction coverage, and repository-native two-process diagnostic. Keep this child source/test-only; the new dependency-ordered child 14 owns the shared documents without overlapping this child or independently runnable siblings 12/13. ## Analysis @@ -27,15 +27,16 @@ The Node already produces one fenced `response_stalled` terminal with joined hea - `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` -- `apps/edge/internal/openai/usage_metrics.go` -- `apps/edge/internal/openai/provider_observation.go` -- `apps/edge/internal/openai/provider_observability_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` @@ -51,10 +52,10 @@ The Node already produces one fenced `response_stalled` terminal with joined hea ### Verification Context -- No handoff artifact was supplied; the user supplied starting HEAD `0e594dfa3723431d2f8d83863a677d0c3d9b60be`, which matched the checkout during planning. -- The local Node profile supplied `go version && go env GOMOD`, `go test -count=1 ./packages/go/execution ./apps/node/...`, and `git diff --check`. Read-only preflight returned `go version go1.26.2 linux/arm64`, module `/config/workspace/iop-s1/go.mod`, and executable `scripts/dev/edge.sh`, `scripts/dev/node.sh`, and `scripts/dev/edge-node-reconnect-diagnostic.sh`. Planning baseline `go test -count=1 ./apps/node/internal/node -run 'Liveness|Watchdog|HealthEvidence|ProviderTunnelLiveness'` passed. -- 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. Confidence is high because the change can observe the immutable `stallObservation` without adding lifecycle state. -- No external verification is required. The repository's manual clocks and fake normalized/tunnel providers provide deterministic local evidence. The testing rule additionally requires the real Edge/Node entrypoints; `scripts/dev/edge-node-reconnect-diagnostic.sh` creates temporary mock configs, starts `scripts/dev/edge.sh` and `scripts/dev/node.sh` separately, proves registration, three ordered runs including two in one session, `/nodes`, `/capabilities`, `/transport`, reconnect, Node-to-Edge payload equality, and exactly-once terminal ordering. +- 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. Confidence is high because the change 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 @@ -65,16 +66,16 @@ The Node already produces one fenced `response_stalled` terminal with joined hea ### Symbol References -- None. No existing symbol is renamed or removed; `Node` gains one internal observer field initialized by `New` and replaceable only by same-package tests. +- 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 stable Node producer: one immutable `stallObservation` is mapped to one counter, one duration histogram, and one dedicated log for both execution paths. It has no active predecessor because the watchdog/health-evidence producers it consumes are already present at the supplied HEAD. -- `12+08_health_overlay_observability` and `13+10_recovery_observability` own Edge overlay and recovery evidence and do not share Node source/test files. This child also relinquishes shared contract/spec writes so independently runnable siblings cannot collide there. +- 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. It has no active predecessor because the watchdog and health-evidence producers it consumes are already present at the checkpoint. +- `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. Contract/spec consolidation is intentionally outside this independently runnable child to keep sibling write boundaries disjoint. +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 @@ -111,7 +112,7 @@ n.liveness.Observe("normalized", obs) sink.queueClaimedTerminal(stalledRuntimeEvent(spec, obs)) ``` -Apply the same call with `provider_tunnel` before `emitClaimedTerminal` at line 309. The new file imports `github.com/prometheus/client_golang/prometheus`, `github.com/prometheus/client_golang/prometheus/promauto`, and `go.uber.org/zap`; do not add an alternate metrics server. +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:** @@ -127,7 +128,7 @@ Apply the same call with `provider_tunnel` before `emitClaimedTerminal` at line **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 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-card/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 independent child. +**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`): @@ -150,7 +151,7 @@ observer.duration.WithLabelValues(labels...).Observe(obs.idle.Seconds()) - [ ] `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 a repeated-default-construction row. Seed run/session/adapter/target/prompt/response/credential sentinels and inspect gathered DTO labels plus zap fields/message text for absence. +**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. @@ -172,7 +173,7 @@ Fresh Go output is required; cached output is not acceptable. 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. This is the required repository-native full-cycle diagnostic, not an auxiliary smoke substitute. +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_2.log b/agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/code_review_cloud_G05_2.log new file mode 100644 index 00000000..eee25175 --- /dev/null +++ b/agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/code_review_cloud_G05_2.log @@ -0,0 +1,158 @@ + + +# 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_node_liveness_observability, plan=2, tag=REFACTOR + +## Archive Evidence Snapshot + +- Prior pair: `agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/plan_local_G05_1.log` and `agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/code_review_cloud_G05_1.log`; it was an unimplemented plan=1 pair with no official verdict, implementation evidence, code change, or verification output. +- Replan finding: the plan's contract/spec write set overlapped independently runnable predecessor and sibling work (`08+07_health_overlay`, `09+08_retry_candidate_policy`, and observability siblings 12/13), so the child could create avoidable merge conflicts despite owning only Node-local evidence. +- Carryover: preserve the two existing claimed-stall seams, S06 label/log boundary, process-global production collectors, isolated test registries, duplicate-construction coverage, and the repository-native two-process diagnostic; keep this child source/test-only and leave living-contract consolidation to ordered dependent work or Milestone closure. + +## 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_2.log` and `PLAN-local-G05.md` → `plan_local_G05_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/11_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_2.log`. +- [ ] Archive active `PLAN-*-G??.md` to `plan_local_G05_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/11_node_liveness_observability/` to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/11_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 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 independent child changes only its declared Node source/test files and does not reopen shared contracts/specs owned by concurrent siblings or Milestone consolidation. + +## 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_node_liveness_observability/plan_local_G05_2.log b/agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/plan_local_G05_2.log new file mode 100644 index 00000000..d70dee65 --- /dev/null +++ b/agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/plan_local_G05_2.log @@ -0,0 +1,178 @@ + + +# Node Response-Stall Operational Evidence + +## For the Implementing Agent + +Implement only this Node liveness-observability slice, 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 a bounded metric and structured-log contract at the existing exactly-once stall finalization seam without changing execution, wire, or retry behavior. + +## Archive Evidence Snapshot + +- Prior pair: `agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/plan_local_G05_1.log` and `agent-task/m-node-provider-execution-liveness-recovery/11_node_liveness_observability/code_review_cloud_G05_1.log`; it was an unimplemented plan=1 pair with no official verdict, implementation evidence, code change, or verification output. +- Replan finding: the plan's contract/spec write set overlapped independently runnable predecessor and sibling work (`08+07_health_overlay`, `09+08_retry_candidate_policy`, and observability siblings 12/13), so the child could create avoidable merge conflicts despite owning only Node-local evidence. +- Carryover: preserve the two existing claimed-stall seams, S06 label/log boundary, process-global production collectors, isolated test registries, duplicate-construction coverage, and the repository-native two-process diagnostic; keep this child source/test-only and leave living-contract consolidation to ordered dependent work or Milestone closure. + +## 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` +- `apps/edge/internal/openai/usage_metrics.go` +- `apps/edge/internal/openai/provider_observation.go` +- `apps/edge/internal/openai/provider_observability_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-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 user supplied starting HEAD `0e594dfa3723431d2f8d83863a677d0c3d9b60be`, which matched the checkout during planning. +- The local Node profile supplied `go version && go env GOMOD`, `go test -count=1 ./packages/go/execution ./apps/node/...`, and `git diff --check`. Read-only preflight returned `go version go1.26.2 linux/arm64`, module `/config/workspace/iop-s1/go.mod`, and executable `scripts/dev/edge.sh`, `scripts/dev/node.sh`, and `scripts/dev/edge-node-reconnect-diagnostic.sh`. Planning baseline `go test -count=1 ./apps/node/internal/node -run 'Liveness|Watchdog|HealthEvidence|ProviderTunnelLiveness'` passed. +- 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. Confidence is high because the change can observe the immutable `stallObservation` without adding lifecycle state. +- No external verification is required. The repository's manual clocks and fake normalized/tunnel providers provide deterministic local evidence. The testing rule additionally requires the real Edge/Node entrypoints; `scripts/dev/edge-node-reconnect-diagnostic.sh` creates temporary mock configs, starts `scripts/dev/edge.sh` and `scripts/dev/node.sh` separately, proves registration, three ordered runs including two in one session, `/nodes`, `/capabilities`, `/transport`, reconnect, Node-to-Edge 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 field initialized by `New` and replaceable only by same-package tests. + +### Split Judgment + +- This child is the stable Node producer: one immutable `stallObservation` is mapped to one counter, one duration histogram, and one dedicated log for both execution paths. It has no active predecessor because the watchdog/health-evidence producers it consumes are already present at the supplied HEAD. +- `12+08_health_overlay_observability` and `13+10_recovery_observability` own Edge overlay and recovery evidence and do not share Node source/test files. This child also relinquishes shared contract/spec writes so independently runnable siblings cannot collide there. + +### 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. Contract/spec consolidation is intentionally outside this independently runnable child to keep sibling write boundaries disjoint. + +### 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 line 309. The new file imports `github.com/prometheus/client_golang/prometheus`, `github.com/prometheus/client_golang/prometheus/promauto`, and `go.uber.org/zap`; 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 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-card/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 independent 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 a repeated-default-construction row. 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. + +## 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_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. This is the required repository-native full-cycle diagnostic, not an auxiliary smoke substitute. +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/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 new file mode 100644 index 00000000..3747a744 --- /dev/null +++ b/agent-task/m-node-provider-execution-liveness-recovery/14+11,12,13_observability_contracts/CODE_REVIEW-cloud-G05.md @@ -0,0 +1,158 @@ + + +# 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/14+11,12,13_observability_contracts, plan=0, tag=REFACTOR + +## 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. +- 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. + +## For the Review Agent + +> **[REVIEW AGENT ONLY]** The finalization steps below are review-agent only. Implementing agents must not execute this section. + +Compare implementation of each item against source files and verify that output in `Verification Results` matches code. +Review completion means the following steps are finished: + +1. Append verdict and `review_rework_count` / `evidence_integrity_failure` routing signals. +2. Archive `CODE_REVIEW-cloud-G05.md` → `code_review_cloud_G05_0.log` and `PLAN-local-G05.md` → `plan_local_G05_0.log`. +3. If PASS, write `complete.log` and move active task directory to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/14+11,12,13_observability_contracts/`. 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 synchronizes the shared execution and wire contracts with the reviewed Node stall, provider-health overlay, and recovery operational evidence, including owner, bounded label/log vocabulary, and the unchanged-wire boundary. +- [ ] REFACTOR-2 synchronizes the living Edge/Node execution spec with the exact reviewed source symbols, behavior, tests, and deterministic S06 verification while removing superseded future-work claims only where implementation now exists. +- [ ] Confirm all dependency gates, run every focused/package/document/diff command in Final Verification with fresh output, and verify the three-document write set is exact. +- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output. + +## Review-Only Checklist + +> **[REVIEW AGENT ONLY]** This checklist is used only by the review agent. +> Implementing agents must not modify or check this section. + +- [ ] Append one verdict of `PASS`, `WARN`, or `FAIL` and verified `review_rework_count`, `evidence_integrity_failure` to `Code Review Result`. +- [ ] Verify that verdict, `Dimension Assessment`, and Required/Suggested/Nit classifications match. +- [ ] Archive active `CODE_REVIEW-*-G??.md` to `code_review_cloud_G05_0.log`. +- [ ] Archive active `PLAN-*-G??.md` to `plan_local_G05_0.log`. +- [ ] Verify that the Agent-Ops managed block in `.gitignore` unignores `agent-task/**/*.md` and `agent-task/**/*.log` and ignores `agent-roadmap/current.md`. +- [ ] If PASS, write `complete.log` based on `agent-ops/skills/common/code-review/templates/complete-log-template.md` and leave no active `.md` files. +- [ ] If PASS, move active task directory `agent-task/m-node-provider-execution-liveness-recovery/14+11,12,13_observability_contracts/` to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/14+11,12,13_observability_contracts/` 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 all three declared dependency `complete.log` files exist, record PASS, and correspond to children 11, 12, and 13 before any shared document was edited. +- Verify every documented metric/event name, owner, bounded value, and exact-once/timing statement matches reviewed source and dependency completion evidence rather than the superseded plans. +- Verify the wire document explicitly states that operational projections do not add a frame, field, ordering rule, or retry semantic. +- Verify the execution contract distinguishes prohibited metric/general-log fields from valid request-scoped typed terminal metadata. +- Verify the living spec cites existing source symbols and non-zero-match deterministic tests for the Node, overlay, and recovery evidence matrix. +- Verify the diff changes only the three declared shared documents and this review stub; child 12/13 documents, code, tests, roadmap, SDD, rules, and skills remain untouched. + +## 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: `bash -O nullglob -c 'for index in 11 12 13; do matches=(agent-task/m-node-provider-execution-liveness-recovery/${index}_*/complete.log agent-task/m-node-provider-execution-liveness-recovery/${index}+*/complete.log agent-task/archive/*/*/m-node-provider-execution-liveness-recovery/${index}_*/complete.log agent-task/archive/*/*/m-node-provider-execution-liveness-recovery/${index}+*/complete.log); ((${#matches[@]} == 1)) || exit 1; done'` + +Expected: PASS only when exactly one active or same-task-group archived completion exists for each predecessor index. + +Output: + +### Verification 2 + +Command: `go test -count=1 ./packages/go/execution ./apps/node/internal/node ./apps/edge/internal/service ./packages/go/streamgate ./apps/edge/internal/openai` + +Expected: PASS for all affected runtime packages. + +Output: + +### Verification 3 + +Command: `go test -count=1 ./apps/node/internal/node -run '^TestNodeLivenessObservability' && go test -count=1 ./apps/edge/internal/service -run '^TestProviderHealthObservability' && go test -count=1 ./apps/edge/internal/openai -run '^(TestOpenAILivenessObservationSink|TestOpenAILivenessRecoveryObservability)$'` + +Expected: PASS with matching tests executed for all three producer surfaces; source-backed selector substitutions are recorded in Deviations from Plan if reviewed children use different exact names. + +Output: + +### Verification 4 + +Command: `rg --sort path -n 'iop_node_response_stalls_total|iop_edge_provider_health_evidence_total|iop_edge_liveness_recovery_eligibility_total|node_response_stall_observation|edge_provider_health_observation|edge_liveness_recovery_observation' agent-contract/inner/execution-runtime.md agent-contract/inner/edge-node-runtime-wire.md agent-spec/runtime/edge-node-execution.md` + +Expected: output contains the exact reviewed metric/event names and no speculative name. + +Output: + +### Verification 5 + +Command: `git diff -- agent-contract/inner/execution-runtime.md agent-contract/inner/edge-node-runtime-wire.md agent-spec/runtime/edge-node-execution.md agent-task/m-node-provider-execution-liveness-recovery/14+11,12,13_observability_contracts/CODE_REVIEW-cloud-G05.md` + +Expected: only declared contract/spec and implementation-evidence edits appear. + +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/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 new file mode 100644 index 00000000..bc114b61 --- /dev/null +++ b/agent-task/m-node-provider-execution-liveness-recovery/14+11,12,13_observability_contracts/PLAN-local-G05.md @@ -0,0 +1,176 @@ + + +# Liveness Operational Evidence Contract Closure + +## For the Implementing Agent + +Start only after all three dependency `complete.log` files exist. Re-read their exact completion evidence and the implemented source, update only the three declared shared documents, 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 three operational-evidence producers are intentionally independent: child 11 owns Node response-stall evidence, child 12 owns Edge provider-health overlay evidence, and child 13 owns OpenAI recovery evidence. The checkpoint refinement removed their overlapping shared-document writes but left those writes with no active owner. This dependency-ordered closure child restores the pre-refine intent by synchronizing the execution contract, wire-boundary contract, and living Edge/Node implementation spec only after all producers have passed review. + +## 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. +- 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. + +## Analysis + +### Files Read + +- `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/11_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` +- `agent-contract/inner/execution-runtime.md` +- `agent-contract/inner/edge-node-runtime-wire.md` +- `agent-spec/index.md` +- `agent-spec/runtime/edge-node-execution.md` +- `apps/node/internal/node/node.go` +- `apps/node/internal/node/liveness_watchdog.go` +- `apps/node/internal/node/liveness_health_evidence.go` +- `apps/edge/internal/service/bootstrap.go` +- `apps/edge/internal/service/service.go` +- `apps/edge/internal/openai/server.go` +- `apps/edge/internal/openai/provider_observation.go` +- `packages/go/observability/observability.go` +- `agent-test/local/node-smoke.md` +- `agent-test/local/edge-smoke.md` +- `agent-test/local/platform-common-smoke.md` + +### SDD Criteria + +- SDD: `agent-roadmap/sdd/operational-observability-provider-management/node-provider-execution-liveness-recovery/SDD.md`; status `[승인됨]`; first-line `milestone-task=ops-evidence`. +- Acceptance Scenario S06 and Evidence Map S06 require the living Source of Truth to describe Node stall count/duration/fence/probe evidence, Edge recovery-owner commit/eligibility/result evidence, provider unhealthy/fresh recovery overlay visibility, deterministic verification, and the no-high-cardinality/no-raw-payload boundary. +- The SDD names `execution-runtime.md`, `edge-node-runtime-wire.md`, and `edge-node-execution.md` as shared source-of-truth surfaces. REFACTOR-1 owns the two contracts; REFACTOR-2 owns the living implementation spec. + +### 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. +- 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. + +### Documentation Gaps + +- `agent-contract/inner/execution-runtime.md` describes liveness terminals and planned overlay/recovery behavior but does not yet define the bounded operational metric/log projections, their owners, or the raw/high-cardinality exclusion boundary. +- `agent-contract/inner/edge-node-runtime-wire.md` does not make explicit that these operational projections are local observations derived from established execution/health evidence and do not widen the Node↔Edge wire schema. +- `agent-spec/runtime/edge-node-execution.md` still treats portions of overlay and recovery observability as future work and lacks reviewed source/test evidence for the complete S06 matrix. + +### Symbol References + +- None are fixed at planning time. The implementing agent must use the exact reviewed symbols and test names recorded by children 11, 12, and 13, avoiding speculative documentation if implementation differs from their plans. + +### Split Judgment + +- This is the smallest stable closure unit: three documents describe one cross-component operational-evidence contract after three producers pass. Splitting each document would duplicate dependency reads and could create inconsistent metric ownership language. +- The child has no production-code writes and depends explicitly on predecessor 11 (missing active completion), predecessor 12 (missing active completion), and predecessor 13 (missing active completion). It does not overlap child 12's `edge-config-runtime-refresh.md`/`provider-pool.md` documents or child 13's OpenAI/streamgate documents. No further split is warranted. + +### Scope Rationale + +Update only the current behavior proven by all three dependency completion logs and implemented source. Do not change Go code or tests, wire/config schemas, metric exporters, retry policy, provider selection, dashboards, roadmap/SDD state, child-owned overlay/OpenAI contracts/specs, or any central rule/skill. Do not add request, session, run, attempt, provider, adapter, target, raw prompt/response, credential, or other unbounded identifiers to documented metric labels or general structured logs. + +### Final Routing + +- `evaluation_mode=isolated-reassessment`; finalizer=`finalize-task-policy.sh pair`. +- Build closure true; scores `(2,0,1,1,1)`, grade G05, route `local-fit` -> `PLAN-local-G05.md`. +- Review closure true; scores `(2,0,1,1,1)`, grade G05, route `official-review` -> `CODE_REVIEW-cloud-G05.md` (`codex`, `gpt-5.6-sol`, `xhigh`). +- `large_indivisible_context=false`; positive loop risks: `boundary_contract`, `variant_product` (2). No recovery signal, capability gap, review rework, or evidence-integrity failure. + +## 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. +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. + +If any predecessor has zero or multiple matching completion candidates, do not modify the shared documents. Record the exact missing or ambiguous dependency candidates in the review stub and stop as blocked. + +## Implementation Checklist + +- [ ] REFACTOR-1 synchronizes the shared execution and wire contracts with the reviewed Node stall, provider-health overlay, and recovery operational evidence, including owner, bounded label/log vocabulary, and the unchanged-wire boundary. +- [ ] REFACTOR-2 synchronizes the living Edge/Node execution spec with the exact reviewed source symbols, behavior, tests, and deterministic S06 verification while removing superseded future-work claims only where implementation now exists. +- [ ] Confirm all dependency gates, run every focused/package/document/diff command in Final Verification with fresh output, and verify the three-document write set is exact. +- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output. + +### [REFACTOR-1] Consolidate operational-evidence contracts + +**Problem:** The shared execution contract already defines liveness evidence and the wire contract defines terminal transport, but neither provides a complete current contract for the operational metric/log projections implemented by children 11-13. Leaving consolidation implicit would make ownership, bounded labels, and the no-wire-widening boundary unverifiable. + +**Solution:** After all dependencies pass, read their completion evidence and implemented sources. Update `execution-runtime.md` with the exact metric family names, observation owners, event names, closed label/status values, timing semantics, exactly-once seams, and prohibited raw/high-cardinality fields for Node stalls, provider health transitions/snapshots, recovery eligibility/owner commit/results. State how fresh health recovery appears in the existing provider snapshot overlay. Update `edge-node-runtime-wire.md` only to clarify that local metrics/logs project established terminal, health, and recovery decisions and introduce no new Node↔Edge frame, field, ordering, or retry semantic. Preserve richer request-scoped identifiers where the existing wire contract already requires them; the observability exclusion applies to metric labels and general logs, not to removal of valid typed terminal metadata. + +Before (`agent-contract/inner/execution-runtime.md:55` and `agent-contract/inner/edge-node-runtime-wire.md:47`): + +```text +Probe completion is evidence only; Edge overlay and recovery remain owned by later slices. +The wire defines the typed response-stall terminal but no local operational projection boundary. +``` + +After: + +```text +Reviewed Node and Edge owners expose bounded operational projections from existing evidence; no operational projection widens the wire protocol. +``` + +**Modified Files and Checklist:** + +- [ ] `agent-contract/inner/execution-runtime.md`: document the reviewed operational evidence, owners, bounded fields, timing/exact-once semantics, overlay reflection, and leakage boundary. +- [ ] `agent-contract/inner/edge-node-runtime-wire.md`: document the local-projection/no-wire-widening boundary without inventing a frame or schema change. + +**Test Strategy:** No new product test is added in this documentation-only child. Re-run the producer-focused and package tests in Final Verification, and compare documented names and values with the dependency completion evidence and current source. + +**Verification:** the dependency gate and Verifications 2-4 below must pass before the document diff is accepted. + +### [REFACTOR-2] Synchronize the living Edge/Node execution spec + +**Problem:** `agent-spec/runtime/edge-node-execution.md` is the matching current implementation spec, but it cannot truthfully describe the full S06 operational surface until children 11-13 pass. The refined pair set otherwise leaves the current spec stale after implementation. + +**Solution:** Replace only superseded future-state wording with reviewed current behavior. Record exact source ownership and source/test evidence for Node stall observations, provider health transition/snapshot observations, and OpenAI recovery observations. Describe their relation to established fence/probe, overlay, eligibility, owner-commit, and result semantics. Retain future-work statements for anything not proven by the completion logs. Document the deterministic test matrix and prohibited-field assertions without duplicating child-specific contract detail owned by the provider-pool, configuration, streamgate, or OpenAI specs. + +Before (`agent-spec/runtime/edge-node-execution.md:147`): + +```text +Edge reception-generation binding, stale-observation validation, Edge health overlay, Node retry, recovery, and candidate selection remain future work. +``` + +After: + +```text +The current spec maps reviewed Node and Edge observability producers to S06 behavior and deterministic tests. +``` + +**Modified Files and Checklist:** + +- [ ] `agent-spec/runtime/edge-node-execution.md`: synchronize current behavior, exact reviewed source/test evidence, bounded-data guarantees, and remaining future work. + +**Test Strategy:** No new product test. Verify the spec only cites symbols/tests that exist after dependency completion and that focused test selectors execute matching tests rather than zero tests. + +**Verification:** Verifications 2-5 below must pass and the final diff must contain no unrelated spec edits. + +## Modified Files Summary + +| File | Item | +|------|------| +| `agent-contract/inner/execution-runtime.md` | REFACTOR-1 | +| `agent-contract/inner/edge-node-runtime-wire.md` | REFACTOR-1 | +| `agent-spec/runtime/edge-node-execution.md` | REFACTOR-2 | +| `agent-task/m-node-provider-execution-liveness-recovery/14+11,12,13_observability_contracts/CODE_REVIEW-cloud-G05.md` | REFACTOR-1, REFACTOR-2 | + +## Final Verification + +Fresh output is required; cached output is not acceptable. + +1. `bash -O nullglob -c 'for index in 11 12 13; do matches=(agent-task/m-node-provider-execution-liveness-recovery/${index}_*/complete.log agent-task/m-node-provider-execution-liveness-recovery/${index}+*/complete.log agent-task/archive/*/*/m-node-provider-execution-liveness-recovery/${index}_*/complete.log agent-task/archive/*/*/m-node-provider-execution-liveness-recovery/${index}+*/complete.log); ((${#matches[@]} == 1)) || exit 1; done'` — PASS only when exactly one active or same-task-group archived completion exists for each predecessor index. +2. `go test -count=1 ./packages/go/execution ./apps/node/internal/node ./apps/edge/internal/service ./packages/go/streamgate ./apps/edge/internal/openai` — PASS for all affected runtime packages. +3. `go test -count=1 ./apps/node/internal/node -run '^TestNodeLivenessObservability' && go test -count=1 ./apps/edge/internal/service -run '^TestProviderHealthObservability' && go test -count=1 ./apps/edge/internal/openai -run '^(TestOpenAILivenessObservationSink|TestOpenAILivenessRecoveryObservability)$'` — PASS with matching tests executed for all three producer surfaces; if reviewed children use different exact names, record the source-backed selector substitutions in Deviations from Plan. +4. `rg --sort path -n 'iop_node_response_stalls_total|iop_edge_provider_health_evidence_total|iop_edge_liveness_recovery_eligibility_total|node_response_stall_observation|edge_provider_health_observation|edge_liveness_recovery_observation' agent-contract/inner/execution-runtime.md agent-contract/inner/edge-node-runtime-wire.md agent-spec/runtime/edge-node-execution.md` — output contains the exact reviewed metric/event names and no speculative name. +5. `git diff -- agent-contract/inner/execution-runtime.md agent-contract/inner/edge-node-runtime-wire.md agent-spec/runtime/edge-node-execution.md agent-task/m-node-provider-execution-liveness-recovery/14+11,12,13_observability_contracts/CODE_REVIEW-cloud-G05.md` — only declared contract/spec and implementation-evidence edits appear. +6. `git diff --check` — no whitespace errors. + +After completing all documentation changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`.