carlos / platform Public

ACCEPT WITH FIXES

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.

Findings affecting the dormant branch

None found.

Findings latent until generations are enabled

1. Medium — retired generation pools can accumulate without bound. internal/edge/genproxy.go:145, also the selection-to-pool interval at line 233.

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.

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.

Fix before merge. The ledger’s description as a “bounded leak” is incorrect.

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.

Plan’s spec questions

1. Foreign layouts: confirm. Rev 17 §2.9 explicitly requires canonical sockets and hold.
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.
3. Cache while asleep: confirm. Retaining the last serving generation preserves cache-before-wake.
4. Request generation: confirm. Attribution to the selected connection’s generation handles first wakes correctly.
5. Version headers: confirm. Successful managed responses use the serving build; dial failures have no serving-build header.
6. Claims-on wakes: confirm as staged work. Canonical handles are acceptable here while dormant; generation-aware claim spawning remains a plan-3 prerequisite.
7. Version probes: confirm unchanged here. There are no production callers requiring migration on this branch. Future generation health checks must use handles.
8. Workers: confirm as staged work. Socketless generation identity remains later-plan work, not a completed guarantee of plan 1.

Ledger deferred-minor triage

| Item | Disposition | |---|---| | Task 1: global-switch tripwire | Defer. No demonstrated defect. | | Task 2: missing socket-sandbox guards, forwarding/refusal tests | Defer. These are coverage and test-portability gaps. | | Task 2: stale comments | Nonblocking. | | Task 3: duplicate guards | Nonblocking. | | 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. | | Task 3: claim-path and empty-socket coverage | Defer; claim integration needs coverage in plan 3. | | Task 4: advisory race and cleanup-helper coverage | Defer. The described advisory race produces diagnostic noise, not a false stray warning. | | Task 5: recreated retired pool | Must fix before merge: finding 1. | | Task 5: delayed retirement closes a replacement’s shared-path pool | Defer. It causes reconnection without interrupting active requests or misattributing responses. | | 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. | | 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. | | Task 6: hook test only counts registration | Defer. Production wiring directly registers genPools.Retire. | | Task 7: incomplete integration coverage | Defer. No flag-off tagging or unmanaged-route defect found. | | Task 7: stale cache entries occupy variant slots | Defer. Existing cache limits bound retention; generation checks prevent stale delivery. | | Task 8: status, exclusions and delivery-retirement coverage | Defer. No additional implementation defect found. |

Each controller “Ruling:”

1. Worktree location — confirm. It does not change the branch’s behavior or reviewability.
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.
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.
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.
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.
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.