<!-- task=m-node-provider-execution-liveness-recovery/07+06_reception_fence plan=4 tag=REFACTOR milestone-task=failure-handoff -->

# Code Review Reference - REFACTOR

> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is the mandatory final step of implementation.**
> The task is NOT complete until every implementation-owned section below is filled in.
> Complete the `Implementation Checklist`; the final checklist item is mandatory before saving.
> Fill implementation-owned sections, then stop with active files in place and report ready for review.
> If implementation is blocked, record the exact blocker, attempted commands/output, and resume condition only in implementation-owned evidence fields.
> Do not ask the user directly, present choices, call user-input tools, create control-plane stop files, or classify the next state.
> Finalization (`Code Review Result`, log rename, `complete.log`, archive moves, `Review-Only Checklist`) is review-agent-only, even after compaction/resume.
> Follow the ownership table at the bottom of this file for which sections you own.

## Overview

date=2026-08-05
task=m-node-provider-execution-liveness-recovery/07+06_reception_fence, plan=4, tag=REFACTOR

## Archive Evidence Snapshot

- Refined parent: `plan_cloud_G09_2.log` and `code_review_cloud_G09_2.log` in this directory; unimplemented, no verdict or implementation evidence.
- Fresh review split the stable reception producer from the queue-locked overlay/probe consumer. This child retains parent REFACTOR-1 only.
- Union preparation review archived the unimplemented plan=3 pair as `plan_local_G08_3.log` and `code_review_cloud_G08_3.log`; it had no verdict or implementation evidence. `scripts/e2e-smoke.sh` runs package tests only, so it is not the required transport/bootstrap multi-process cycle.

## For the Review Agent

> **[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_4.log` and `PLAN-local-G08.md` → `plan_local_G08_4.log`.
3. If PASS, write `complete.log` and move active task directory to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/07+06_reception_fence/`. If WARN/FAIL, fully write the next filesystem state required by the code-review skill.
4. If PASS, preserve the first-line `milestone-task` metadata in `complete.log` and report it for runtime aggregation. Roadmap state evaluation belongs to `sync-milestone-workstate`.
5. Check applicable `Review-Only Checklist` items at the final `.log` location before reporting.

---

## Implementation Item Completion

| Item | Status |
|------|---------|
| REFACTOR-1: Carry authoritative reception identity | [x] |

## Implementation Checklist

- [x] REFACTOR-1 derives authoritative node/generation from the receiving client for RunEvent and tunnel callbacks and drops stale/unregistered receivers before correctness callbacks.
- [x] Preserve message-only observability fanout and compatibility-delegate the new bootstrap callback shape until the dependent overlay consumer uses its authority values.
- [x] Add current/stale/unregistered two-client fixtures and run focused, package, race, vet, package smoke, actual Edge/Node reconnect diagnostic, and diff verification.
- [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_4.log`.
- [x] Archive active `PLAN-*-G??.md` to `plan_local_G08_4.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/07+06_reception_fence/` to `agent-task/archive/YYYY/MM/m-node-provider-execution-liveness-recovery/07+06_reception_fence/` and update this checklist at the final archive path.
- [ ] If PASS, preserve and report `milestone-task` metadata for runtime aggregation, without modifying roadmap or directly calling `update-roadmap`.
- [ ] If PASS for split work, remove empty active parent `agent-task/m-node-provider-execution-liveness-recovery/` or verify it was kept due to remaining siblings/files.
- [x] If WARN/FAIL, write the next filesystem state matching code-review verdict and do not write `complete.log`.

## Deviations from Plan

None.

## Key Design Decisions

- Added `Registry.CurrentOwnerForClient(client)` to atomically resolve a cloned `NodeEntry` under `r.mu.RLock()` for the given client connection.
- Transport listeners (`registerRunEventListener`, `registerTunnelFrameListener`) resolve the current owner at frame receipt time and pass authoritative `(nodeID, generation)` to internal correctness callbacks (`onRunLifecycle`, `onTunnelFrame`).
- Stale/unregistered clients fail closed and are dropped before reaching correctness callbacks.
- Observability fanout (`onRunEvent`, `onNodeEvent`) remains message-only.
- Bootstrap adapts the new `(nodeID, generation, event/frame)` callback contract to existing service methods until the consumer child consumes the authoritative node/generation directly.

## Reviewer Checkpoints

- Confirm current-owner lookup is atomic and returns a clone.
- Confirm stale/unregistered clients never reach correctness callbacks and payload node metadata cannot substitute authority.
- Confirm observability remains message-only, bootstrap compatibility does not consume queue/overlay semantics early, and the actual Edge/Node reconnect diagnostic dispatches successfully before and after re-registration.

## Verification Results

> Implementing agent: run each command exactly as written and paste its actual stdout/stderr under `Output`. Record any replacement command and reason in `Deviations from Plan`.

### Verification 1

Command:

```bash
go test -count=1 ./apps/edge/internal/node ./apps/edge/internal/transport ./apps/edge/internal/bootstrap -run 'CurrentOwner|Reception|Lifecycle|Tunnel'
```

Output:

```
ok  	iop/apps/edge/internal/node	0.093s
ok  	iop/apps/edge/internal/transport	0.133s
ok  	iop/apps/edge/internal/bootstrap	0.103s [no tests to run]
```

### Verification 2

Command:

```bash
go test -count=1 ./apps/edge/internal/node ./apps/edge/internal/transport ./apps/edge/internal/bootstrap
```

Output:

```
ok  	iop/apps/edge/internal/node	0.100s
ok  	iop/apps/edge/internal/transport	4.864s
ok  	iop/apps/edge/internal/bootstrap	0.512s
```

### Verification 3

Command:

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

Output:

```
ok  	iop/apps/edge/internal/node	1.071s
ok  	iop/apps/edge/internal/transport	15.635s
ok  	iop/apps/edge/internal/bootstrap	3.289s
```

### Verification 4

Command:

```bash
go vet ./apps/edge/internal/node ./apps/edge/internal/transport ./apps/edge/internal/bootstrap
```

Output:

```
(no output, exit code 0)
```

### Verification 5

Command:

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

Output:

```
[e2e] verifying provider-only Node command and cancellation boundary
ok  	iop/apps/node/internal/node	0.037s
[e2e] verifying Edge dispatch, provider tunnel, queue, and reconnect fencing
ok  	iop/apps/edge/internal/service	4.360s
ok  	iop/apps/edge/internal/transport	0.312s
[e2e] provider-only Edge-Node smoke PASSED
```

### Verification 6

Command:

```bash
IOP_DEV_RECONNECT_BIND_TIMEOUT=45 ./scripts/dev/edge-node-reconnect-diagnostic.sh
```

Output:

```
[edge] sent run_id=manual-1785909379340628842 node=node0 adapter=mock target=mock-stream session=diagnostic-correlation background=false
[node0-evt] start run_id=manual-1785909379340628842
[node0-msg] echo: Convert token IOP_E2E_PING_BASIC and reply only with converted token
[node0-evt] complete run_id=manual-1785909379340628842 detail="mock execution complete"
[diagnostic] Verifying payload sequence, terminal ordering, and command responses...
[diagnostic] Checking run 1 run_id=manual-1785909370640947463 token=IOP_E2E_HELLO_BASIC
[diagnostic] Checking run 2 run_id=manual-1785909371152750047 token=IOP_E2E_HELLO_FORMAL
[diagnostic] Checking run 3 run_id=manual-1785909379340628842 token=IOP_E2E_PING_BASIC
[diagnostic] PASS: 3 mock-provider runs verified — registration, ordered Node==Edge payloads, one terminal after the last payload, /nodes, /capabilities, /transport, reconnect, and absent session/status ownership commands.
[diagnostic] Cleaning up...
```

### Verification 7

Command:

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

Output:

```
(no output, exit code 0)
```

---

> **[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:**
  - Correctness: Fail — reception authority is ambiguous when one TCP client owns more than one node id.
  - Completeness: Fail — the authoritative client-to-owner invariant is not closed for every accepted registry state.
  - Test coverage: Fail — current/stale/unregistered coverage omits same-client multi-node registration.
  - API contract: Fail — registration does not preserve the singular connection-to-node ownership implied by the Edge-Node wire contract.
  - Code quality: Pass — scoped production changes are focused and free of debug or dead code.
  - Implementation deviation: Fail — the plan requires authoritative node/generation derivation, but the implemented lookup can select an arbitrary map entry.
  - Verification trust: Pass — submitted commands are present and fresh scoped tests/race/vet plus the reconnect diagnostic corroborated the exercised paths; the focused reproducer exposes a missing case rather than fabricated evidence.
  - Spec conformance: Fail — SDD S04 requires fail-closed reception binding, which an ambiguous client owner does not provide.
- **Findings:**
  - **Required R1** — `apps/edge/internal/node/registry.go:91`: `RegisterIfAbsent` rejects only a duplicate node id, so one non-nil `TcpClient` can own two different node ids. `CurrentOwnerForClient` then returns the first matching `byID` map entry at line 200, making the supposedly authoritative `(node_id, generation)` nondeterministic. A focused reproducer registered `node-a` and `node-b` to the same client and failed with `ambiguous client must fail closed, got arbitrary owner "node-a" generation 1`. Reject a client already bound to another node under the same registry lock, make reverse lookup fail closed if an ambiguous state exists, preserve the original owner/generation on rejection, and add deterministic regression coverage.
- **Routing Signals:** `review_rework_count=1`, `evidence_integrity_failure=false`
- **Next Step:** Archive this pair and materialize the routed `PLAN-local-G07.md` / `CODE_REVIEW-cloud-G07.md` follow-up for Required R1.
