feat(bigtable): add health checker for conns - #6

…e spec-driven review workflow

Self-contained HTML deck (no external deps, 23 KB) plus the Marp
markdown source, covering:

1. How we use SPECs on the Session subsystem — the 5 spec files,
   the edit → hook → agents → PASS/VIOLATION → commit workflow.
2. Example: SESSION_SPEC.md #6 (GOAWAY + LastRpcIdAdmitted) — full
   invariant with 7-step order-of-ops + Java parity + anti-pattern.
3. SESSION_COMPONENT_SPEC.md — 7-layer topology (Part A) + boundary
   rules highlight table (B1/B2/B3/B6/B8) + Go-only structural quirks.
4. Three prompts for three change shapes — simple fix, multi-file
   fix, new feature (spec-first flow for the last).
5. PostToolUse hook mechanics — trigger, matcher regex, exit-code
   semantics, batching behavior, guarantees.

Open the HTML in any browser (arrow keys / space / 1-5 to navigate,
F for fullscreen). The Marp version renders identically via Marp CLI
or the VS Code Marp extension if slide export is needed later.

…itted, formalize retry-signal provenance

CLIENT_SIDE_METRICS_SPEC.md
- New invariant #2: per-attempt latency labels on the session path (client_blocking_latencies from SentAt, server_latencies from Stats.BackendLatency, transport_type/*) MUST be sourced only from InvokeResult and stamped in exactly one place — sessionTable.stampAttempt. Corrects a prior wrong claim that InvokeResult.TransportLatency was not a client-side metric.
- New invariant #3: session-tracer-hosted OTel histograms (session.durations, session.open_latencies, session.uptime, transport_latencies) — registered once by InitializeSessionMetrics, emitted only by sessionTracer, session_name label is pool-scoped (bounded) not Session.LogName (unbounded).
- Scope line updated to reference session_tracer.go and session_pool.go recording sites; invariant count bumped to 3.

SESSION_SPEC.md
- Invariant #6: deprecate the LastRpcIdAdmitted retry oracle. Proto field is still sent by the server but the Go transport does not read it (git grep returns zero hits in internal/transport/). Section heading trimmed; deferred-work callout notes any reintroduction requires paired code+spec+test change with a Java-parity check.
- Invariant #9: new 'Retry-signal provenance' table splitting server-only inputs (RetryInfo, narrow always-retry code set, x-goog-cbt-cookie-routing-cookie) from client-only inputs (AttemptState tag, idempotency check, 3-attempt cap, deadline-fit, zero-value fail-closed). Absorbs several redundant bullets from the prior 'Additional invariants' list.
- Added 'How to verify' pointer to CLAUDE.md smoke-gate for consistency (2 of 5 specs updated; remaining specs to follow).

SESSION_CLIENT_SPEC.md
- Added 'How to verify' pointer.

SESSION_COMPONENT_SPEC.md
- New Part C row for session-tracer OTel histograms: sessionTracer sole writer, registered once by InitializeSessionMetrics, transport_latencies recorded only from session_pool.go:650 under the positive-delta gate, session_name must be pool-scoped not LogName.

…ant rows, dedicated reviewer slide

Slide 2 — invariant table:
- Trimmed SESSION_SPEC #6 example to just the Ready→Closing state transition (drop the in-flight-vRPC / 30s grace details that belonged in the full spec, not the deck cell).
- Rewrote SESSION_POOL_SPEC #2 to name both production picker strategies (LeastInFlight for fewest active vRPCs, LeastLatency using e2eEwma with Java parity) instead of only calling out the e2eEwma vs transportEwma distinction.
- Strengthened SESSION_COMPONENT_SPEC B2 example to explain layer direction + rationale + why it is compiler-enforced (Go import-cycle detector) with the grep as belt-and-braces, not the primary check.
- Removed the 'concrete failure mode' callout that duplicated the same GOAWAY example.

Slide 3 — Part C excerpt: dropped the In-flight vRPC slot row (already implied by the surrounding spec discussion, and the row was redundant given the SESSION_SPEC #2 coverage on slide 2).

Slide 4 — big-feature example replaced:
- Was: unified debugview/ package fold.
- Now: tcpz page (per-connection TCP_INFO for every conn-pool connection). Two-sentence prompt + why the specs made it survivable (B3 forced a fourth Handler arg for TCPStats, POOL #4 kept getsockopt syscalls outside pool locks, B10 kept the sub-1500 PMTU flag on the DTO producer). Six-commit iteration story preserved.

Slide 5 — new dedicated slide 'How the reviewer verifies specs and runs tests':
- Extracted the reviewer flow that used to sit at the bottom of slide 4.
- Adds report-format example table (PASS / VIOLATION / AMBIGUOUS with example rows).
- Adds the smoke-gate command: go test ./internal/transport/ -count=1 -short -timeout=90s.
- Mentions the two new spec-enforcing tests (TestInvoke_ConcurrentSecondFailsWithMultiplexViolation, TestHooks_FiredInSpecOrder) as concrete coverage examples.

Slide counter updated 4 → 5; nav shortcuts (1–5) still work.

Restructure per feedback: 9 slides covering one-shot prompting,
Session Client scale problem + 5-step spec fix, a worked SPEC
example (SESSION_SPEC #6), component restrictions (Part A/B/C),
three prompt-size slides (small/medium/large-tcpz), a dedicated
A/B experiment slide (SynchronizationContext POC), and a closing
Plan-Flush-Execute slide with the two known limitations
(race conditions, if/else drift).

- Slide 3: rewrite the GOAWAY spec (#6) in plain-language, one-sentence-per-step, no code references
- Slide 4: remove B8 (lock inversion) row from Part B boundary-rule table and adjust the trailing summary

v1 (854d216) ported Java-parity slotMu one-in-flight lifecycle:
ctx.Done leaves the slot claimed, drainSlot on the server response
frees it. But the pool's Invoke defer still called releaseSession +
signalFree unconditionally, so a session with an undrained slot could
land back in its AFE idle queue while the slot was still busy. v2
(e3fb88a) reconciled with a busy-skip loop in sessionList.Checkout.
Two mechanisms racing to describe "session is free"; the busy-skip
loop is the reconciliation cost.

v3 collapses them. The AFE idle queue contains only sessions with
empty slots, by construction. Only drainSlot success re-enqueues and
wakes; the pool's Invoke return path no longer touches the queue or
the wake channel. Checkout goes back to a plain dequeue.

Change list:

- session_pool.go Invoke defer — drops releaseSession(sh) + signalFree().
  Keeps DecOutstanding + recordVRpcOutcome (per-caller in-flight
  counter + per-AFE PeakEwma update; those piggyback on the caller's
  return, not on drain, because the caller is where invokeErr /
  latency / backendDur are known).

- session_vrpc.go — notifySlotDrained now fires from every request-
  path drainSlot success: normal handleVRPCResponse (was cancelled-
  branch-only), normal handleVRPCErrorResponse (same), and the Send-
  failure branch of Invoke. cancelActiveRPCs (session teardown) still
  does NOT fire the callback — OnSessionClosing/OnSessionClosed
  handle removal from routing structures.

- session_pool_lifecycle.go OnActive — onSlotDrained callback grows
  from "signalFree only" to "releaseSession(sh) + signalFree()".

- session_list.go Checkout — the v2 busy-skip loop deletes; plain
  dequeue restores. Docstring pins the new invariant ("dequeued
  handles are guaranteed idle-slot by construction — drainSlot is
  the sole re-enqueue site").

Ordering caveat (documented, accepted): the wake fires from the
response-handler goroutine BEFORE Invoke's defer runs, so a picker on
another goroutine may see pre-update EWMAs for one tick. Same window
v2 already ships on the cancelled-drain path; v3 generalizes it.

Complexity delta:

- 25 lines gone from Checkout (busy-skip loop + snapshotted-length
  reasoning + tail-rotation + ready-set gymnastics).
- 66 lines gone from session_list_test.go (v2's
  TestPool_CheckoutSkipsSessionWithUndrainedSlot — the invariant it
  pinned is now unreachable by construction).
- 3 net lines added in session_vrpc.go (notifySlotDrained calls in
  two normal branches + Send-failure branch).
- Dual-authority invariant collapses to a single owner: drainSlot.
- The v1/v2 window where Invoke's defer re-enqueues a busy session
  and the next Checkout has to skip it never happens.

Specs paired:

- SESSION_SPEC.md #2 — "Every request-path drainSlot success fires
  notifySlotDrained" (enumerates the five fire sites; exempts
  cancelActiveRPCs as a session-teardown path).
- SESSION_POOL_SPEC.md #2 — "wake-ups fire centrally from Invoke's
  defer" rewritten to "from Session.notifySlotDrained".
- SESSION_POOL_SPEC.md #6 — state-machine transition table grew a
  "Triggered by" column; InFlight→Idle row cites the drain callback.
- SESSION_COMPONENT_SPEC.md — new ownership row for the "Session→pool
  slot drained signal" naming Session.notifySlotDrained →
  SessionHandle.onSlotDrained as the sole path.

Green: transport + debugview -race -short (12.8s + 1.0s);
TestInvoke_ForceCloseRace_BoundedReturnUnderCtx ×200 -race (58.9s);
three new v3 tests pinning the drain-driven wake — normal
handleVRPCResponse deliver, normal handleVRPCErrorResponse deliver,
Send-failure Invoke branch — all PASS under -race. session-reviewer
and session-component-review both PASS with no violations. Pre-
existing top-level bigtable integration failures (TestIntegration_
SessionVRpc_* Diverter-flip timeouts) reproduce on the pre-v3
baseline — unrelated to this change.

Plan: ~/.claude/plans/session-slotmu-v3.md.

v1 (854d216) ported Java-parity slotMu one-in-flight lifecycle:
ctx.Done leaves the slot claimed, drainSlot on the server response
frees it. But the pool's Invoke defer still called releaseSession +
signalFree unconditionally, so a session with an undrained slot could
land back in its AFE idle queue while the slot was still busy. v2
(e3fb88a) reconciled with a busy-skip loop in sessionList.Checkout.
Two mechanisms racing to describe "session is free"; the busy-skip
loop is the reconciliation cost.

v3 collapses them. The AFE idle queue contains only sessions with
empty slots, by construction. Only drainSlot success re-enqueues and
wakes; the pool's Invoke return path no longer touches the queue or
the wake channel. Checkout goes back to a plain dequeue.

Change list:

- session_pool.go Invoke defer — drops releaseSession(sh) + signalFree().
  Keeps DecOutstanding + recordVRpcOutcome (per-caller in-flight
  counter + per-AFE PeakEwma update; those piggyback on the caller's
  return, not on drain, because the caller is where invokeErr /
  latency / backendDur are known).

- session_vrpc.go — notifySlotDrained now fires from every request-
  path drainSlot success: normal handleVRPCResponse (was cancelled-
  branch-only), normal handleVRPCErrorResponse (same), and the Send-
  failure branch of Invoke. cancelActiveRPCs (session teardown) still
  does NOT fire the callback — OnSessionClosing/OnSessionClosed
  handle removal from routing structures.

- session_pool_lifecycle.go OnActive — onSlotDrained callback grows
  from "signalFree only" to "releaseSession(sh) + signalFree()".

- session_list.go Checkout — the v2 busy-skip loop deletes; plain
  dequeue restores. Docstring pins the new invariant ("dequeued
  handles are guaranteed idle-slot by construction — drainSlot is
  the sole re-enqueue site").

Ordering caveat (documented, accepted): the wake fires from the
response-handler goroutine BEFORE Invoke's defer runs, so a picker on
another goroutine may see pre-update EWMAs for one tick. Same window
v2 already ships on the cancelled-drain path; v3 generalizes it.

Complexity delta:

- 25 lines gone from Checkout (busy-skip loop + snapshotted-length
  reasoning + tail-rotation + ready-set gymnastics).
- 66 lines gone from session_list_test.go (v2's
  TestPool_CheckoutSkipsSessionWithUndrainedSlot — the invariant it
  pinned is now unreachable by construction).
- 3 net lines added in session_vrpc.go (notifySlotDrained calls in
  two normal branches + Send-failure branch).
- Dual-authority invariant collapses to a single owner: drainSlot.
- The v1/v2 window where Invoke's defer re-enqueues a busy session
  and the next Checkout has to skip it never happens.

Specs paired:

- SESSION_SPEC.md #2 — "Every request-path drainSlot success fires
  notifySlotDrained" (enumerates the five fire sites; exempts
  cancelActiveRPCs as a session-teardown path).
- SESSION_POOL_SPEC.md #2 — "wake-ups fire centrally from Invoke's
  defer" rewritten to "from Session.notifySlotDrained".
- SESSION_POOL_SPEC.md #6 — state-machine transition table grew a
  "Triggered by" column; InFlight→Idle row cites the drain callback.
- SESSION_COMPONENT_SPEC.md — new ownership row for the "Session→pool
  slot drained signal" naming Session.notifySlotDrained →
  SessionHandle.onSlotDrained as the sole path.

Green: transport + debugview -race -short (12.8s + 1.0s);
TestInvoke_ForceCloseRace_BoundedReturnUnderCtx ×200 -race (58.9s);
three new v3 tests pinning the drain-driven wake — normal
handleVRPCResponse deliver, normal handleVRPCErrorResponse deliver,
Send-failure Invoke branch — all PASS under -race. session-reviewer
and session-component-review both PASS with no violations. Pre-
existing top-level bigtable integration failures (TestIntegration_
SessionVRpc_* Diverter-flip timeouts) reproduce on the pre-v3
baseline — unrelated to this change.

Plan: ~/.claude/plans/session-slotmu-v3.md.

…lers

handleVRPCResponse and handleVRPCErrorResponse were 90% identical —
same state gate, activeVRPC nil check, id-match guard, drainSlot,
counter bump, cancel-vs-deliver branch, notifySlotDrained, and Closing
signalQuiescent, differing only in the frame proto, nil-tag, counter,
and vrpcResult payload. Fold the shared skeleton into a private
routeVRPCFrame(rpcID, frameName, nilTag, counter, result) so the two
call sites can't drift on any of the Java-parity gating.

Log wording change: the wrongState assertion + drop debugf now use the
proto frame name uniformly ("VirtualRpcResponse" / "ErrorResponse")
instead of the split "vRPC response"/"vRPC error" (wrongState) vs
proto name (debugf) pair. No callers grep the text.

Behavioral parity verified by session-reviewer (SESSION_SPEC #2/#5/#6/
#10) and session-component-review (Part B B2/B7/B11 + Part C).

Deletes the bare Session.slotDrainedFn field and its setSlotDrainedCallback
setter / notifySlotDrained fire method in favor of a fifth field on the
existing SessionHooks struct — OnSlotDrained. The struct already carries
the four ordered lifecycle callbacks (OnStart / OnActive / OnClosing /
OnClose) wired at construction; the drain callback was the only reason
Session carried a second, parallel hook mechanism with its own nil-guard
and its own doc block.

After: one hook mechanism on Session. Every fire site still fires on the
same drain paths (Send-failure early drain, response-frame drain,
error-frame drain via routeVRPCFrame); cancelActiveRPCs still does NOT
fire it. The pool's installer folds into the same SessionHooks{...}
literal createSession was already building.

Also refreshes SESSION_SPEC #2, #10, SESSION_POOL_SPEC #2, #6, and
SESSION_COMPONENT_SPEC B7 + Part C to name the new identifiers instead
of slotDrainedFn / notifySlotDrained / setSlotDrainedCallback.

session-reviewer (4 behavioral specs) and session-component-review
(SESSION_COMPONENT_SPEC B1-B12 + Part C) returned PASS on the diff.

…construction

Java-parity port of SessionPoolImpl.java:424-448. Deletes the
Session→SessionHandle back-ref (Session.poolHandle) whose nil-ness
was abused as a "already handled" signal, and wires per-session
SessionHooks closures at createSession time that capture *SessionHandle
directly. The pool never walks back through Session to find the handle.

Shape changes:
- SessionHandle gains three atomic.Bool one-shot dedup flags:
  activated (defensive onActive re-entry gate) and
  closingRecorded / closeRecorded (Pool.Close Phase1↔Phase2 short-
  circuit; NOT a substitute for SESSION_SPEC #4 exactly-once).
  Loses the onSlotDrained field (moves onto Session).
- Session loses poolHandle atomic.Pointer[SessionHandle]. Gains
  slotDrainedFn func() with a lowercase setSlotDrainedCallback setter.
  notifySlotDrained now calls the field directly (no atomic Load, no
  pointer chase). Single-writer-at-construction: setter called from
  createSession before Session.Start spawns any goroutine.
- SessionPoolImpl.startingSessions retyped from map[*Session]bool to
  map[*SessionHandle]struct{}.
- SessionPoolImpl.createSession mints the handle before NewSession,
  builds hooks closures capturing sh, installs the slot-drained
  closure via setSlotDrainedCallback, backfills sh.session /
  sh.createdAt in two statements immediately after.
- SessionPoolImpl.{OnActive,OnClosing,OnClose} renamed to lowercase
  onActive/onClosing/onClose taking *SessionHandle. onActive CAS-
  gates on sh.activated. onClosing / onClose CAS-gate on the paired
  dedup flag; short-circuit when Pool.Close Phase-1 already ran the
  bookkeeping.
- Pool.Close Phase-1 flips both dedup flags on each snapshot handle
  in order: recordLifetime → recordSessionClose → flip flags →
  sl.OnSessionClosed. Replaces the prior sh.session.poolHandle.Store(nil)
  trick, expressing the dedup as an actual dedup flag instead of
  abusing a back-ref's nullability.

Test migration:
- injectActiveSession / injectActiveOnAfe rewired to the new closure-
  hooks shape. The six test sites that used to do s.poolHandle.Store(sh)
  by hand become no-ops (or move to s.setSlotDrainedCallback(...) for
  the drain-signal tests in session_vrpc_test.go).
- session_pool_lifecycle_test callers use lowercase p.onActive(sh) /
  p.onClosing(sh) / p.onClose(sh, err). The poolHandle assertions
  in TestOnClosing_DropsFromReadyCountAndRecordsLifetime and
  TestOnClosing_StartingSessionIsNoOp were replaced (the former
  drops the check outright; the latter now asserts sh.closingRecorded
  stays unset for a starting-only handle).

Spec updates (paired with code, both reviewers green):
- SESSION_SPEC.md #2 / #10: slot-drained wire re-described; poolHandle
  dropped from the atomic-state list; slotDrainedFn documented as
  single-writer-at-construction.
- SESSION_POOL_SPEC.md #6 transition table (NotRegistered→Idle and
  InFlight→Idle rows) point at Session.slotDrainedFn and lowercase
  SessionPoolImpl.onActive.
- SESSION_COMPONENT_SPEC.md B7 whitelist swaps poolHandle for
  slotDrainedFn; Part C ownership matrix updates the slot-drained
  row (install site now createSession, callback location now Session);
  new Part C row for the three atomic.Bool flags with explicit scope
  disclaimer.

Behavior shift worth flagging (not a spec violation):
- The "pool closed before session became active" race in onActive
  now runs onClosing's recordLifetime path once (createdAt is set
  at createSession, so time.Since is meaningful). Old back-ref
  path skipped it because poolHandle was never stored. Adds one
  lifetimes-ring entry per race case; no spec pins lifetimes-ring
  semantics for this race so treated as improvement, not regression.

Pre-existing race unchanged by this diff:
- TestHeartBeatLoop_ForceClosesOnMissedHeartbeat leaks a heartBeatLoop
  goroutine that races with TestSessionTracer_MetricsRoundTrip's
  global metric init. Reproduces on the base branch under -race
  -count>1; not caused by this refactor.

Deletes the bare Session.slotDrainedFn field and its setSlotDrainedCallback
setter / notifySlotDrained fire method in favor of a fifth field on the
existing SessionHooks struct — OnSlotDrained. The struct already carries
the four ordered lifecycle callbacks (OnStart / OnActive / OnClosing /
OnClose) wired at construction; the drain callback was the only reason
Session carried a second, parallel hook mechanism with its own nil-guard
and its own doc block.

After: one hook mechanism on Session. Every fire site still fires on the
same drain paths (Send-failure early drain, response-frame drain,
error-frame drain via routeVRPCFrame); cancelActiveRPCs still does NOT
fire it. The pool's installer folds into the same SessionHooks{...}
literal createSession was already building.

Also refreshes SESSION_SPEC #2, #10, SESSION_POOL_SPEC #2, #6, and
SESSION_COMPONENT_SPEC B7 + Part C to name the new identifiers instead
of slotDrainedFn / notifySlotDrained / setSlotDrainedCallback.

session-reviewer (4 behavioral specs) and session-component-review
(SESSION_COMPONENT_SPEC B1-B12 + Part C) returned PASS on the diff.

Fold all 6 items from igor's re-review of 150350b:

Semantic (1 change):
- New sweepStuckSessionsInterval = 30 * time.Second const, decoupled
  from waitServerCloseGrace. Worst-case detection is now grace +
  interval (~5m30s) instead of the old 2×grace (10 min). Sweep loop
  wakes at the interval cadence but only acts when a session has been
  in WSC past the grace.

Docs (5 fixes):
- waitServerCloseGrace docstring: justify why 5 min (typical drain vs
  hung server), drop stale "sweeper still runs at Tick's 1s cadence"
  phrase.
- Tick docstring in session_pool_scaling.go: drop "prunes stuck
  sessions" — sweep is on its own loop now.
- tickOnce docstring: drop redundant-sweep mention, keep the
  redundant-sampleActiveUptimes one.
- Close docstring in session_lifecycle.go: fix dangling reference to
  the non-existent startStuckSessionMonitor → startSweepStuckSessionsLoop.
- SESSION_SPEC.md invariant #6 bullet 3: (30s) → (5 min) to match the
  new waitServerCloseGrace value.