| 1 | **ACCEPT WITH FIXES** |
| 2 | |
| 3 | Reviewed `4f26f15c..c8dbe3de`. No flag-off traffic regression found for supported routes with canonical real sockets. Production never calls `activator.SetGenerations`; the required cross-plan interfaces are present. |
| 4 | |
| 5 | **Findings affecting the dormant branch** |
| 6 | |
| 7 | None found. |
| 8 | |
| 9 | **Findings latent until generations are enabled** |
| 10 | |
| 11 | 1. **Medium — retired generation pools can accumulate without bound.** |
| 12 | [internal/edge/genproxy.go:145](../../../internal/edge/genproxy.go:145), also the selection-to-pool interval at line 233. |
| 13 | |
| 14 | Sequence: a request samples generation N → execution pauses → the activator switches generations and `Retire` deletes N’s pool → the request resumes and `pool` recreates N’s entry → `dialGen` rejects the obsolete generation and the request retries successfully. Nothing removes the recreated entry. Repeating this across generations retains arbitrarily many transports in both edge and delivery pool maps. `IdleConnTimeout` does not remove map entries. |
| 15 | |
| 16 | **Smallest fix:** while holding `GenPools.mu`, revalidate the sampled generation against `ServingGen` before finding or creating its pool. If it changed or disappeared, return to generation selection. Because retirement takes the same pool lock, it then either removes an already-published pool or precedes a validation that refuses publication. Add a deterministic repeated-retirement test asserting pool count remains bounded. |
| 17 | |
| 18 | **Fix before merge.** The ledger’s description as a “bounded leak” is incorrect. |
| 19 | |
| 20 | `git diff --check` and formatting checks on changed Go files passed. I did not execute builds or tests because they write files; runtime results remain those reported in the ledger. |
| 21 | |
| 22 | **Plan’s spec questions** |
| 23 | |
| 24 | 1. **Foreign layouts: confirm.** Rev 17 §2.9 explicitly requires canonical sockets and `hold`. |
| 25 | 2. **Canonical links: confirm.** Publishing/removing links and skipping the advisory’s checked dial are consistent. Flag-off removal of a leftover symlink is the intentional rollback exception; regular files and real sockets remain untouched. |
| 26 | 3. **Cache while asleep: confirm.** Retaining the last serving generation preserves cache-before-wake. |
| 27 | 4. **Request generation: confirm.** Attribution to the selected connection’s generation handles first wakes correctly. |
| 28 | 5. **Version headers: confirm.** Successful managed responses use the serving build; dial failures have no serving-build header. |
| 29 | 6. **Claims-on wakes: confirm as staged work.** Canonical handles are acceptable here while dormant; generation-aware claim spawning remains a plan-3 prerequisite. |
| 30 | 7. **Version probes: confirm unchanged here.** There are no production callers requiring migration on this branch. Future generation health checks must use handles. |
| 31 | 8. **Workers: confirm as staged work.** Socketless generation identity remains later-plan work, not a completed guarantee of plan 1. |
| 32 | |
| 33 | **Ledger deferred-minor triage** |
| 34 | |
| 35 | | Item | Disposition | |
| 36 | |---|---| |
| 37 | | Task 1: global-switch tripwire | Defer. No demonstrated defect. | |
| 38 | | Task 2: missing socket-sandbox guards, forwarding/refusal tests | Defer. These are coverage and test-portability gaps. | |
| 39 | | Task 2: stale comments | Nonblocking. | |
| 40 | | Task 3: duplicate guards | Nonblocking. | |
| 41 | | Task 3: unconditional `noteCurrent` store | Confirm current implementation. Calls are serialized at publication; generation allocation order is not itself publication order. Later switches must maintain that serialization. | |
| 42 | | Task 3: claim-path and empty-socket coverage | Defer; claim integration needs coverage in plan 3. | |
| 43 | | Task 4: advisory race and cleanup-helper coverage | Defer. The described advisory race produces diagnostic noise, not a false stray warning. | |
| 44 | | Task 5: recreated retired pool | **Must fix before merge: finding 1.** | |
| 45 | | Task 5: delayed retirement closes a replacement’s shared-path pool | Defer. It causes reconnection without interrupting active requests or misattributing responses. | |
| 46 | | Task 5: in-flight connection retained for 90 seconds after retirement | Reject as a general defect. Go’s `CloseIdleConnections` sets `closeIdle`, closing connections when they subsequently become idle. | |
| 47 | | Task 5: dead branch, body mutation, retry/timeout coverage, unused assignments | Nonblocking. The proxy supplies its outgoing request; the listed coverage gaps do not establish another defect. | |
| 48 | | Task 6: hook test only counts registration | Defer. Production wiring directly registers `genPools.Retire`. | |
| 49 | | Task 7: incomplete integration coverage | Defer. No flag-off tagging or unmanaged-route defect found. | |
| 50 | | Task 7: stale cache entries occupy variant slots | Defer. Existing cache limits bound retention; generation checks prevent stale delivery. | |
| 51 | | Task 8: status, exclusions and delivery-retirement coverage | Defer. No additional implementation defect found. | |
| 52 | |
| 53 | **Each controller “Ruling:”** |
| 54 | |
| 55 | 1. **Worktree location — confirm.** It does not change the branch’s behavior or reviewability. |
| 56 | 2. **Go cache and socket-test scratch — partially overrule.** An existing on-disk cache fallback is reasonable when the default is unavailable. `/tmp/k` conflicts with the supplied machine rules; future scratch must use the permitted on-disk location. The consequence is not merely build speed. |
| 57 | 3. **Model allocation — partially overrule.** Mechanical implementation assignments fit the repository policy. Sonnet reviews alone do not satisfy its production-path review requirement; this explicitly requested whole-branch Astra review provides the final assessment. |
| 58 | 4. **Explicit listener closure in the Task-5 fixture — confirm.** Closing the listener synchronously addresses the `Serve` registration race and prevents its later close from unlinking a replacement socket. |
| 59 | 5. **Controller performs Task-9 verification — confirm.** Delegation adds no necessary independence to inventory and gate execution. This ruling does not establish that the complete gate has passed; the supplied ledger does not record its completion. |
| 60 | 6. **`LaunchEnvironment` remains canonical — confirm.** It describes launch composition rather than dialing a process. More decisively, `moves.LayoutVars` classifies `CARLOS_CONTROL_SOCKET` as a presence-only comparison: different generation paths do not produce the hypothesized value-mismatch failure. |
amadan