<!-- task=m-node-provider-execution-liveness-recovery/08+07_health_overlay plan=2 tag=REVIEW_REFACTOR milestone-task=failure-handoff -->

# Code Review Reference - REVIEW_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/08+07_health_overlay, plan=2, tag=REVIEW_REFACTOR

## Archive Evidence Snapshot

- The plan=1 pair is archived as `agent-task/m-node-provider-execution-liveness-recovery/08+07_health_overlay/plan_cloud_G09_1.log` and `agent-task/m-node-provider-execution-liveness-recovery/08+07_health_overlay/code_review_cloud_G09_1.log` with verdict `FAIL`, Required=1, Suggested=0, Nit=0.
- Required R1 affects `apps/edge/internal/service/model_queue_release.go` and `apps/edge/internal/service/provider_health_overlay_test.go`: probe recovery must resolve exactly one current catalog provider and retain a per-provider high-water mark even while effective health is available.
- Reviewer reproduction proved both failures: one unavailable overlay was recovered despite a second healthy catalog provider with the same adapter/target, and an available sequence 2 was discarded before a delayed unavailable sequence 1 made the provider unavailable.
- Fresh focused/package/vet/provider smokes passed. The exact race suite contradicted the recorded PASS by timing out once in `TestEdgeServerRegistrationFailureReasons`; its immediate targeted race rerun passed, so fresh whole-command evidence is required. The authorized live long-context provider and Edge status endpoints remain unreachable; do not repeat those unchanged external commands in this repository-fix packet.
- Roadmap carryover remains `milestone-task=failure-handoff`, SDD S04. Existing contracts/specs already state exact unambiguous higher-sequence recovery and require no semantic rewrite for R1.

## 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-G07.md` → `plan_cloud_G07_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/08+07_health_overlay/`. If WARN/FAIL, fully write the next filesystem state required by the code-review skill.
4. If PASS and task group is `m-<milestone-slug>`, 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 |
|------|---------|
| REVIEW_REFACTOR-1: Enforce exact catalog identity and monotonic probe ordering | [x] |

## Implementation Checklist

- [x] REVIEW_REFACTOR-1 resolves an available probe to exactly one current catalog provider, records its same-generation high-water mark even when already available, and prevents ambiguous or lower-sequence state changes.
- [x] Add deterministic regressions for unavailable-plus-healthy catalog ambiguity and available-sequence-2-before-unavailable-sequence-1 ordering while retaining existing recovery cases.
- [x] Run focused, package, race, vet, provider smoke, local-capacity smoke, and diff verification with fresh output; do not retry the unchanged blocked live endpoints.
- [x] 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.

- [x] Append one verdict of `PASS`, `WARN`, or `FAIL` and verified `review_rework_count`, `evidence_integrity_failure` to `Code Review Result`.
- [x] Verify that verdict, `Dimension Assessment`, and Required/Suggested/Nit classifications match.
- [x] Archive active `CODE_REVIEW-*-G??.md` to `code_review_cloud_G08_2.log`.
- [x] Archive active `PLAN-*-G??.md` to `plan_cloud_G07_2.log`.
- [x] 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 and task group is `m-<milestone-slug>`, 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.
- [x] If WARN/FAIL, write the next filesystem state matching code-review verdict and do not write `complete.log`.

## Deviations from Plan

None. The unchanged external provider and Edge-status endpoints were not retried, as directed by the plan.

## Key Design Decisions

- CAPABILITIES recovery resolves adapter/target against the current NodeStore provider catalog under the queue lock and fails closed unless exactly one non-empty provider id matches.
- A strictly newer exact available observation creates or updates the generation-scoped overlay even when the provider is already available. Only an unavailable-to-available transition pumps the queue and reports recovery.
- The added regressions cover a healthy catalog sibling that makes recovery ambiguous and a sequence-2 available observation that prevents a delayed sequence-1 unavailable terminal from lowering effective health.

## Reviewer Checkpoints

- Confirm adapter/target resolves against every current configured provider on the authoritative Node record, not only runtime-unavailable overlays, and fails closed for zero or multiple matches.
- Confirm a fresh exact available observation stores the uniquely resolved provider's sequence even when no unavailable overlay exists, while the function reports/pumps only an actual recovery.
- Confirm a delayed lower/equal-sequence terminal cannot reverse the newer available observation and reconnect generation fencing still removes superseded overlays.
- Confirm config health, Node wire evidence, ingress recovery ownership, and unrelated admission/snapshot code remain unchanged.
- Confirm the exact whole race command has fresh trusted output; carry the archived external live blocker without retrying unchanged inaccessible endpoints.

## 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=50 ./apps/edge/internal/service -run '^(TestProviderHealthOverlayCapabilitiesRecoveryRejectsCatalogAmbiguity|TestProviderHealthOverlayCapabilitiesRecoveryPreservesAvailableHighWater|TestProviderHealthOverlayCapabilitiesRecoveryRejectsInconclusiveEvidence|TestProviderHealthOverlayCapabilitiesRecovery)$'
```

Output:

```text
ok  \tiop/apps/edge/internal/service\t0.025s
```

### 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:

```text
ok  \tiop/apps/edge/internal/service\t0.169s
ok  \tiop/apps/node/internal/node\t0.042s
ok  \tiop/apps/edge/internal/service\t0.025s
```

### Verification 3

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:

```text
ok  \tiop/packages/go/execution\t0.034s
ok  \tiop/apps/node/cmd/node\t0.129s
ok  \tiop/apps/node/internal/adapters\t0.111s
?   \tiop/apps/node/internal/adapters/mock\t[no test files]
ok  \tiop/apps/node/internal/adapters/ollama\t0.054s
ok  \tiop/apps/node/internal/adapters/openai_compat\t0.190s
ok  \tiop/apps/node/internal/adapters/vllm\t0.171s
ok  \tiop/apps/node/internal/bootstrap\t1.556s
ok  \tiop/apps/node/internal/node\t1.132s
ok  \tiop/apps/node/internal/router\t0.524s
ok  \tiop/apps/node/internal/store\t0.065s
ok  \tiop/apps/node/internal/transport\t5.793s
ok  \tiop/apps/edge/internal/node\t0.047s
ok  \tiop/apps/edge/internal/transport\t4.793s
ok  \tiop/apps/edge/internal/bootstrap\t0.601s
ok  \tiop/packages/go/streamgate\t0.966s
ok  \tiop/apps/edge/internal/openai\t7.475s
ok  \tiop/apps/edge/internal/service\t6.115s
ok  \tiop/apps/edge/internal/controlplane\t6.635s
```

### Verification 4

Command:

```bash
go test -race -count=3 ./apps/node/internal/node ./apps/edge/internal/node ./apps/edge/internal/transport ./apps/edge/internal/service
```

Output:

```text
ok  \tiop/apps/node/internal/node\t4.883s
ok  \tiop/apps/edge/internal/node\t1.068s
ok  \tiop/apps/edge/internal/transport\t20.503s
ok  \tiop/apps/edge/internal/service\t18.634s
```

### Verification 5

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:

```text
No stdout/stderr; command exited 0 with no diagnostics.
```

### Verification 6

Command:

```bash
./scripts/e2e-smoke.sh
```

Output:

```text
[e2e] verifying provider-only Node command and cancellation boundary
ok  \tiop/apps/node/internal/node\t0.030s
[e2e] verifying Edge dispatch, provider tunnel, queue, and reconnect fencing
ok  \tiop/apps/edge/internal/service\t4.331s
ok  \tiop/apps/edge/internal/transport\t0.239s
[e2e] provider-only Edge-Node smoke PASSED
```

### Verification 7

Command:

```bash
(cd scripts && sed -e 's|^SCRIPT_DIR=.*|SCRIPT_DIR="$(pwd)"|' -e 's|TMP_DIR="$(mktemp -d /tmp/iop-provider-capacity-smoke.XXXXXX)"|TMP_DIR="$(mktemp -d "$REPO_ROOT/.tmp/iop-provider-capacity-smoke.XXXXXX")"|' e2e-provider-capacity-smoke.sh | bash)
```

Output:

```text
[provider-capacity-smoke] building loopback binaries
[provider-capacity-smoke] metrics=disabled bounded_probe_timeout_sec=2
[provider-capacity-smoke] offline_snapshot_rejected=true
[provider-capacity-smoke] aliases=ornith:35b,ornith-fast queue_observed=true
[provider-capacity-smoke] backend={"calls":2,"active":0,"peak":1}
[provider-capacity-smoke] final_provider=[{"node_id":"ornith-node","connected":true,"providers":[{"id":"ornith-provider","status":"available","health":"available","capacity":1,"in_flight":0,"queued":0,"long_context_capacity":1,"long_in_flight":0,"long_queued":0}]}]
[provider-capacity-smoke] PASS evidence=/config/workspace/iop-s1/.tmp/iop-provider-capacity-smoke.wtwyVk
```

### Verification 8

Command:

```bash
git diff --check
```

Output:

```text
No output; command exited 0 with no whitespace errors.
```

---

> **[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 |

## Code Review Result

### Overall Verdict

FAIL

### Dimension Assessment

| Dimension | Result | Evidence |
|-----------|--------|----------|
| Correctness | FAIL | A higher-sequence available probe for one target can clear an unavailable overlay lowered by a different target on the same provider. |
| Completeness | FAIL | The current-catalog uniqueness and high-water fixes are present, but the SDD S04 same-provider/adapter/target recovery fence is incomplete. |
| Test Coverage | FAIL | The new regressions cover catalog ambiguity and available-before-terminal ordering, but not cross-target recovery on one multi-target provider. |
| API Contract | FAIL | `applyProviderProbeEvidence` violates the documented requirement that recovery use the same provider/adapter/target binding that lowered health. |
| Code Quality | PASS | The catalog resolver and high-water transition are localized and otherwise clear. |
| Implementation Deviation | PASS | The implementation follows the direct-fix file boundary and the recorded external-endpoint exclusion. |
| Verification Trust | PASS | Fresh focused, package, race, vet, provider smoke, capacity smoke, and diff commands matched the recorded passing results. |
| Spec Conformance | FAIL | SDD S04 and the runtime contracts require exact same-target recovery and stale-sequence no-op behavior. |

### Findings

- **Required R2 — Preserve the lowered target binding during probe recovery** (`apps/edge/internal/service/model_queue_release.go:249`, `apps/edge/internal/service/provider_health_overlay_test.go:372`). After resolving adapter/target to one current catalog provider, `applyProviderProbeEvidence` treats any newer available observation for that provider as recovery and overwrites the overlay binding. For a provider serving targets A and B, unavailable evidence for B at sequence 1 is therefore cleared by available evidence for A at sequence 2. This contradicts SDD S04 and the execution/wire contracts, which require the same provider/adapter/target binding. Retain the unavailable overlay binding on a cross-target available observation, advance the provider high-water mark without reporting recovery, and recover only when a later available observation matches the binding that lowered health. Add a deterministic multi-target regression covering cross-target no-recovery, sequence advancement, and subsequent exact-target recovery.

### Routing Signals

- `review_rework_count=2`
- `evidence_integrity_failure=false`

### Next Step

Prepare one follow-up packet that directly fixes R2 and reruns the focused multi-target ordering regression plus the repository verification suite. Preserve the archived external live blocker without retrying unchanged inaccessible endpoints.
