docs(spec): P6 — VM process manager + session mux, density without Kubernetes

[pdettori]

Design-only. Adds the P6 spec and ADR-0034, and registers both. No code, no manifests.

Realizes the deployment-model slice that P5 §6 and ADR-0032 explicitly defer — ScaledJob → elastic pool, and #55's overload shift from pod-level to session-level — on a non-Kubernetes substrate, with two experiments as the deliverable: E8 (density/saturation on one VM) and E9 (deployment-tier comparison against Knative, model tier held constant).

The premise

Scale-to-zero and high density are substitutes, not complements. Once one process holds N sessions the deployment never idles at zero, so Knative's autoscaler stops earning the cold start it charges. That is not speculative: EXPERIMENTS.md:96 already attributes E6's p95 blowup under concurrency to "LLM latency + Knative cold-start … not the sandbox".

What tracing the tree at c12a97c found

Most of the stack is already substrate-neutral — the SandboxTransport gRPC path (whose Go worker dials out and needs no inbound route), Redis presence and leases, session state, the queue, leaf results, and a plain node:http surface with no Knative coupling in src. MU1 already put credentials behind CredentialStore. The volume-envelope PVC turns out to be documentation-era; results are in Redis.

Two existing measurements decided the design rather than decorating it:

• E6/E7's 2–8% sandbox duty (N ≈ 29–48 sessions per sandbox) ⇒ nothing below the worker line changes, and that ratio becomes the provisioning input.
• E2's constant 6-entry / ~900-byte rehydration ⇒ warm in-process session state is worth far less than intuition suggests, so stateless least-in-flight routing is the default and sticky affinity is demoted to a sweep variant the experiment prices.

One code-level blocker: select-sandbox.ts:87 lists pods via kubectl unconditionally, before the gRPC branch — so the presence path cannot run on a host without kubectl despite needing nothing from it. Fixed by an explicit SH_SANDBOX_DISCOVERY source selector, not by swallowing ENOENT, which would turn a broken kubectl on the cluster path into a silently empty pool.

The load-bearing decision

The supervisor hands off accepted sockets over IPC and never touches a response byte. A byte-proxying supervisor would put its own event loop inside the very ceiling E8 exists to find, so a knee could be the supervisor's with no way to tell. Hand-off removes that confound structurally — and settles the supervisor's language as Node, since there are no bytes on its hot path.

The honesty crux

Because routing is stateless, an idle session costs ~900 bytes, so "N concurrent sessions" could be inflated arbitrarily. The spec fixes the vocabulary before any driver exists: concurrent in-flight turns (what the knee applies to) versus sessions addressable (a Redis capacity statement, not a density claim).

Scope discipline

Round one claims density and scalability only. Cost accounting is round two (E9 compares capacity, not cost). Isolation-at-density is inherited from P5 and tested, but not claimed — which is what keeps Firecracker/gVisor/Kata out of this slice and in P4 (#57), where it already lives. Single-host, vertical only: no multi-VM placement, which is where the "we rebuilt Kubernetes, worse" risk sits.

The Kubernetes path is preserved and used, not merely kept — it supplies E9's comparison arm. startServer() is untouched (the only change is handler gaining an export), and two regression pins assert the cluster path is unchanged.

One cross-track ask

§3.6: P5's credential scrub should land as a shared function both entry points call rather than inline in server.ts, since the VM worker is a third entry point with the same multiplexing exposure. Coordinated with that track, not duplicated here. §3.10 states precisely what P5 gates — mixed-subject rungs and the isolation claim, not the work, which is buildable and measurable single-subject first.

Written to be plannable from cold

A fresh planning session should be able to start from the spec + ADR alone: §3.8 names the configuration surface, §3.9 states the supervisor↔worker IPC contract completely (including the bounded over-admission it accepts and why closing it would repeat §3.2's mistake), §3.10 gives ordered steps with a provable-by column, and §9 fixes the file layout, driver homes, and detectKnee's c === 1 baseline requirement instead of leaving them to be chosen.

Every line-number citation was verified against the tip rather than carried over from a stale base.

---

🤖 Generated with Claude Code

[pdettori]

Pushed d47ee13 in response to a review question — does the VM path still use a static pool and gRPC streams? Tracing the wiring to answer it turned up something that affects E9's validity, not just performance, so it's worth calling out rather than leaving in a commit message.

The finding. extension.ts:38 and :49-51 show opts.transport overriding both transport tiers, and run-leaf.ts:786 passes exactly that for a leased gRPC record. So the gRPC path has no persistent fast channel — every read/write/edit/ls/find is a full Exec RPC ending in a fresh bash -c, where the kubectl path serves the same op as one nonce-framed line on an already-running bash. Pre-existing on the remote path, not introduced here.

Why it matters for E9. Left to their defaults the two arms would not match: the Knative arm resolves pods and gets the fast channel, the VM arm runs gRPC and does not. E9 would then charge the deployment tier for a transport difference — biased against the VM, and the same class of confound E6 caught in itself. §5.3 now pins both arms to the relay + gRPC transport, which the cluster can already do (SH_REMOTE_SANDBOX=1 against the in-cluster relay, as relay-leaf-smoke.sh exercises). Configuration, not code. The invariant is now stated explicitly: E9 varies the deployment tier and nothing else, and any future arm inherits both the model-tier and tool-tier pins.

Why the fix isn't in this PR. persistent-exec.ts is kubectl-specific in only two places — the binary name at :84 and buildPersistentKubectlArgs at :12 — while framing, cap-at-source and fallback are transport-agnostic and already declare producer-side-cap. So a container-exec variant is a parameterized argv rather than a new protocol. But it lifts both substrates equally, so folding it in would improve both E9 arms at once while adding transport surface to a slice whose job is to measure the deployment tier. Filed as #245 on the ST track.

Also in this push:

• §3.1 records that the pool is self-registering, not statically configured — presence is written on a live Attach stream and removed on close, so a docker run joins the pool and a stop leaves it. Only the count is fixed; there's no autoscaler.
• §3.1a states the relay-stays trade with its real price, replacing my earlier and incomplete "not worth a fourth transport to save a loopback hop."
• §5.2 turns the metric list into a table mapping each metric to the tier it attributes a knee to, adding per-file-op latency and sandbox-container CPU. Without those two a sandbox-bound run reads as a harness density limit. Relevant prior: E7 validated mixed-ref converge correctness at 6 concurrent refs on one pod — P6's rungs go well past that, so whether E6's 2–8% duty holds is an open question.
• §2.3 now notes E6's duty numbers were measured on the kubectl path, so they are simultaneously the right provisioning input and something to re-check at this concurrency.

[cwiklik]

Docs-only: the P6 spec (618 lines) plus ADR-0034, both registered. I verified every file:line citation in the spec and the ADR against the tree at c12a97c through the GitHub API rather than a local checkout — git fetch fails on TLS interception in my environment, so a local tree would have been stale at 02109bc and would have made resolve-pod.ts look like a phantom citation. Flagging the method because it is load-bearing for what follows.

The citations hold up: detectKnee's c === 1 baseline throw (sharing.ts:14-15), handler's module-privacy at server.ts:495 and the startServer() boundary at :576-577, extension.ts:38 / :49-51's both-tier transport override, resolve-pod.ts:32's bare spawn('kubectl'), select-sandbox.ts:87's unconditional pod list, and the E6/E7 duty quotes are all as described. The design reasoning is unusually well-grounded in existing measurements, and the socket hand-off argument in §3.2 is the right call for the reason given.

One must-fix: stickyBySession (§3.4/§3.9) is specified to route from the request head, but the session id is read from the JSON body (server.ts:93-101), so the sweep variant cannot be implemented as written — and the natural fix puts a body parse on the supervisor's accept path, which is precisely the confound §3.2 exists to remove. Two suggestions: the "2–8% duty" range and the "29–48:1" ratio come from different experiments and are arithmetically incompatible at the point where §5.4 makes both drive stub calibration and sandbox provisioning; and S is named a session cap but must mean an in-flight-turn cap under stateless routing — the exact conflation §5.1 fixes the vocabulary to prevent. Plus two nits.

Author: pdettori (MEMBER — maintainer)
Areas reviewed: Docs (specs + ADRs), citation verification against c12a97c
Agent/IDE config (.claude/.vscode): none
Commits: 2 commits, all signed-off: yes (Assisted-By: used, not Co-Authored-By, per CLAUDE.md)
CI status: passing (12/12 green, including DCO, lint, CodeQL, Trivy)

[cwiklik]

must-fix — stickyBySession cannot be implemented from the request head, so the sweep variant as specified is not buildable.

Verified at c12a97c: sessionId is read from the JSON body — packages/knative-server/src/server.ts:93-101 does JSON.parse(body) then const { sessionId, prompt } = parsed. And /turn is matched on method plus exact URL only (:564, req.method === 'POST' && req.url === '/turn'), so there is no path segment or query string carrying it either. A supervisor that pre-reads only the request head therefore has nothing to key on; socket.unshift(head) faithfully restores a request whose session id the supervisor never saw. The SSE branch (:107-108) takes the same body, so it does not help.

The reason this is blocking rather than a wording fix: the obvious repair is to buffer and JSON-parse the body in the supervisor, which puts request bytes and a parse on the accept path — partially reintroducing the confound §3.2 removes structurally, and turning sticky's supervisor overhead into a term E8 must measure rather than something that is free. That changes the cost side of the very knob §3.4 says the experiment will price.

Two cheap ways out:

1. Have the sweep driver carry the id in the head — /turn?sid=<id> or a request header — read only by stickyBySession, leaving the body contract and the stateless default untouched. §3.9's head? row then describes a genuinely head-sized read.
2. State explicitly that sticky buffers and parses the body, and that its supervisor-side cost is a measured term in the E8 sweep rather than assumed negligible.

Either is fine; §3.4 and §3.9 need to say which, since §3.10's ordered steps currently treat sticky as a small variant on top of stateless routing.

[pdettori]

Fixed in a900d58 — sticky now keys on an X-SH-Session header, option 1, with the body contract untouched.

One addition your analysis surfaces: a query string is not just inelegant here, it would not route. :564 matches req.url === '/turn' by exact equality, so /turn?sid=<id> 404s before any handler sees it — option 1 via a path or query would have required touching that match, i.e. a real change to the Knative path this slice promises not to make. A header costs nothing: the supervisor already pre-reads headers in sticky mode, and X-SH-Subject (§3.6) establishes the precedent for a head-carried key.

§3.4 now carries three paragraphs: the constraint with your 93-101 / :564 citations, the rejected body-parse alternative with §3.2 as the reason, and the price stated — sticky is measurable but not adoptable as-is, since a production client would have to start sending the header. That last one is the part I would have been tempted to leave in a footnote; it belongs in the finding, because E8 prices sticky's benefit and the header is the precondition that benefit is contingent on.

Also: §3.9's head? row now says "headers only", §3.10 step 6 names sticky as the sweep arm rather than leaving it implicit, §7 adds a unit test for the mechanic (keys on the header, reads no body bytes, unshift leaves the worker's parse intact), and ADR-0034's routing paragraph records the constraint and its consequence.

[cwiklik]

suggestion — the "2–8% duty" range and the "N ≈ 29–48:1" ratio come from two different experiments and cannot both be true of the same measurement.

Verified at c12a97c:

• deploy/knative/EXPERIMENTS.md:96 (E6): the pinned sandbox is only "~6–8 % busy (duty 0.06–0.08)".
• :168 (E7): "busy only ~2–4 % of leaf wall-clock (duty 0.021 Kind / 0.035 OCP → N ≈ 29–48:1)".

So "2–8%" is the union of two experiments' ranges, while "29–48:1" derives only from E7's half: 1/0.035 ≈ 29 and 1/0.021 ≈ 48. At E6's 8% duty the ratio is roughly 12:1, not 29–48:1. (Incidentally "on both Kind and OCP" is E7's phrasing at :168 — E6 at :96 is the single-pinned-sandbox measurement.)

This matters because §5.4 makes both numbers drive the same two decisions — the stub's tool-call rate and how many sandbox containers a run provisions. Calibrating the stub at the top of the stated range while provisioning at 29–48:1 under-provisions the sandbox tier by about 2.4×, which produces exactly the sandbox-bound run that §5.2's two new saturation metrics exist to detect. Better not to build it into the calibration.

Suggest picking one basis and naming its experiment: E7's 2–4% / 29–48:1 for provisioning (they are the same measurement, so they stay consistent), with E6's 6–8% cited separately as the higher-duty case a heavier workload produces — and then say which one the E8 driver calibrates against. ADR-0034's Context paragraph carries the same pair, so both need the fix.

[pdettori]

Split in a900d58 — but landing on the opposite basis from the one you suggested, on the strength of a line neither of us cited.

EXPERIMENTS.md:65, inside E6's own section, reads: "This supersedes the earlier single-N figure (N ≈ 29–48:1), which used a trivial marker.txt leaf with no real converge." So the document itself retires 29–48:1 for real-converge work. Provisioning from E7's row would have had the spec provision from the figure the repo declares superseded.

The arithmetic supports the same direction. E7's 0.021/0.035 comes from a 13.9 s leaf wall (:121, :161) against E6's ~6–7 s: same fixed git plumbing, smaller share of a longer turn. So 29–48:1 is the light-leaf end and 12–24:1 the code-review end — and provisioning from the lower ratio is also the conservative direction against §3.1a's per-op process churn, which is the hazard §5.2's two new metrics exist to catch. Two reasons pointing the same way.

Your framing of the fix is what got applied, though: one basis, named, per decision. §2.3 now tables both with per-experiment cites (E6 real code review 0.06–0.08 → 12–17:1 OCP, ~20–24:1 Kind; E7 mixed-ref converge 0.021/0.035 → 29–48:1) plus the invariant N ≈ 1/duty, so a blended pair fails a visible check. §5.4 pins the E8 driver to the E6 row for both decisions — stub calibration and container count — names E7's row as a lighter sweep point taken whole, and asserts K ≥ ceil(W × S × duty) so mixing cannot happen by accident. ADR-0034's Context paragraph carries the same split.

Two things your note prompted me to write down rather than resolve: E6's 6–8% appears both as per-leaf duty (:94) and as the pinned sandbox's busy fraction at C_max (:96), which are not the same quantity even where the range coincides; and every run now records which basis it used. Both are in §2.3's closing paragraph — flagged rather than papered over, since E8 will re-measure it at concurrency the kubectl-path numbers were never taken at.

("on both Kind and OCP" was indeed E7's phrasing — gone from §2.3 along with the blended range.)

[cwiklik]

suggestion — S is named a session cap throughout, but under the stateless default it can only be an in-flight turn cap. This is the conflation §5.1 fixes the vocabulary to prevent, so it is worth applying that vocabulary here too.

SH_SESSIONS_PER_WORKER, "per-worker soft cap" (§3.8), "all workers at their session cap" (§3.5), and "in-flight session count" (§3.4/§3.9) all name a session quantity. But with leastInFlight stateless routing no worker owns a session between turns — the session lives in Redis and the next turn can land on any worker. So the only thing a worker can report, and the only thing S can bound, is concurrent in-flight turns. Either rename to SH_TURNS_PER_WORKER, or keep the name and define S explicitly as in-flight turns in §3.8.

Same fix sharpens §3.9's staleness argument, which currently asserts over-admission "by up to W" without giving the direction of the error. With optimistic increment on hand-off, the stale case that actually yields over-admission is a second turn arriving on a kept-alive socket: in-flight rises with no hand-off for the supervisor to count, so its estimate stays low until the next load message. The opposite lag — a turn finishing before its load lands — makes the estimate high and produces spurious 429s. That is under-admission, and it is arguably the worse failure for this experiment: it silently truncates E8's rungs by refusing offered concurrency the pool could have served, so the knee reads early with nothing in the data to distinguish it. Worth logging alongside the over-admission events §5.2 already records.

[pdettori]

Both parts fixed in a900d58, including the rename: SH_SESSIONS_PER_WORKER → SH_TURNS_PER_WORKER.

Rename rather than redefine, because the reasoning generalizes past the one variable — §3.1's diagram and component table, §3.3's "S concurrent sessions is S concurrent awaits", §3.4/§3.9's counts, §3.5's cap, §3.6's output-guard blast radius, §5.2, §5.7's claim template and ADR-0034 all said sessions where only turns are bounded. §3.8 now states it once, as §5.1's vocabulary applied to the configuration surface, and adds the sticky wrinkle you implicitly raise: under stickyBySession a worker does retain affinity for sessions it has served, but S still bounds admission — resident session count becomes a separate measured quantity, not a second meaning for S.

On staleness: your direction analysis is right and §3.9 now tables both cases with their sign. Over-admission from a second turn on a kept-alive socket (supervisor never sees that connection again, so no increment; estimate reads low) — bounded at ≈1 per worker and self-correcting on the next load. Under-admission from a turn finishing before its load lands (estimate reads high → spurious 429) — and this is now called out as the worse case for E8, in your words: it refuses concurrency the pool could have served, so the rung truncates and the knee reads early with nothing in the data to distinguish it from a real ceiling.

That last point changed the instrumentation, not just the prose. §5.2 gains a metric row — every 429 recorded with the supervisor's estimate at refusal against the next load per worker — so a refusal the pool could have absorbed is identifiable after the fact. Without it, an under-admitting run is indistinguishable from a genuine finding, which is exactly the class of error E6 caught in itself. I also dropped the bare "over-admit by up to W sessions" claim in favour of stating what actually bounds it (in-flight changes inside one round trip), since the old sentence asserted a bound without a mechanism.

[cwiklik]

nit — README.md:233 does not state this convention. Verified at c12a97c: README.md is 350 lines and contains no occurrence of docs/plans, "gitignored", or "ephemeral"; line 233 is about HOME/REDIS_URL and byte-identical bundles.

The convention itself is real and stated almost verbatim — just elsewhere. .gitignore:27-30:

# Implementation plans are local-only, ephemeral work artifacts (see docs/plans/README.md).
docs/plans/*
!docs/plans/README.md

Suggest citing .gitignore:27-30 (or docs/plans/README.md) instead. Flagging a one-line citation fix only because the PR body states every line-number citation was verified against the tip — the rest of them are accurate, including the non-obvious ones, so this is the lone exception rather than a pattern.

[pdettori]

Fixed in a900d58 — now cites .gitignore:27-30 and docs/plans/README.md.

Confirmed your read: README.md:233 is the --home/--redis-url paragraph about HOME/REDIS_URL and byte-identical bundles, and the file is 350 lines with no occurrence of docs/plans. The convention is verbatim at .gitignore:27-30, which is presumably where I read it and then mis-attributed.

[cwiklik]

nit — SH_SANDBOX_DISCOVERY puts a third sandbox knob under a different prefix from the two it sits beside.

The other four new variables follow the dominant env convention cleanly (SH_MODEL_*, SH_BUDGET_*, SH_RELAY_*). The awkward one is this: the existing sandbox namespace is KAGENTI_SANDBOX_* — KAGENTI_SANDBOX_POOL_SELECTOR (select-sandbox.ts:75) and KAGENTI_SANDBOX_CAP — and §3.8 lists both as inherited a couple of rows below this one, so the split is visible inside a single table.

Either prefix is defensible (SH_ for new harness-owned config, KAGENTI_ for continuity with the sandbox layer). The ask is only that it be a stated decision rather than an accident, since §7's default-semantics pin already establishes that this spec settles this kind of question — and SH_SANDBOX_DISCOVERY is the variable an operator reads next to KAGENTI_SANDBOX_CAP when a pool comes up empty.

[pdettori]

Stated as a decision in §3.8 as of a900d58, keeping SH_.

The line I drew: KAGENTI_SANDBOX_* addresses the sandbox layer's own objects — KAGENTI_SANDBOX_POOL_SELECTOR is a label selector (select-sandbox.ts:75), KAGENTI_SANDBOX_CAP is a lease cap (run-leaf.ts:391) — while SH_* is harness-owned config about which mechanism the harness uses. Discovery-source selection is the second kind.

What settles it beyond taste is the sibling: SH_REMOTE_SANDBOX is already SH_, it is the flag whose branch this seam gates, and §4.2's default is defined in terms of it (pods, or both when SH_REMOTE_SANDBOX=1). Splitting those two across prefixes would be worse than the split you flagged, and renaming SH_REMOTE_SANDBOX for symmetry is not worth breaking deployed config over. §3.8 says all of that, including the last clause, so the next reader gets the reasoning rather than re-deriving it.

Your framing of the ask — a stated decision rather than an accident — is what I applied; the operator reading SH_SANDBOX_DISCOVERY next to KAGENTI_SANDBOX_CAP on an empty pool now has a rule to hang them on.

[pdettori]

Pushed a900d58 — all five review items addressed, with detail in each thread. Two things worth surfacing here rather than leaving buried in a thread, because both changed a design input.

The must-fix repair had a second constraint the review did not have. Sticky now keys on an X-SH-Session header (option 1). Not just because option 2 puts a body parse on the accept path, but because the head-carried alternatives were narrower than they look: /turn is matched on exact URL equality (server.ts:564), so /turn?sid=<id> 404s before any handler runs. A query string would have required editing that match — a real change to the Knative path this slice promises to leave byte-identical. The header is free by comparison, and X-SH-Subject already sets the precedent. §3.4 also now states sticky's price: it is measurable but not adoptable without a client-contract addition, which belongs in the finding since E8 prices the benefit.

The duty split landed on the opposite basis from the suggestion, on the strength of EXPERIMENTS.md:65. That line, inside E6's own section, records that E6's real-converge finding supersedes "the earlier single-N figure (N ≈ 29–48:1), which used a trivial marker.txt leaf with no real converge." Provisioning from E7's row would have pinned P6 to the figure the repo declares retired. The arithmetic agrees: E7's 0.021/0.035 comes from a 13.9 s leaf wall against E6's ~6–7 s — same fixed git plumbing, smaller share of a longer turn — so 29–48:1 is the light-leaf end and 12–24:1 the code-review end, and the lower ratio is also the conservative direction against §3.1a's process churn. The shape of the fix is exactly as asked: §2.3 tables both bases with per-experiment cites and the invariant N ≈ 1/duty, §5.4 pins one row for both decisions and asserts K ≥ ceil(W × S × duty).

Also in this push: SH_SESSIONS_PER_WORKER → SH_TURNS_PER_WORKER (rename, not just a definition — the conflation ran through the diagram, six sections and the ADR); §3.9's staleness error now has a direction, with under-admission named as the worse case for E8 and a new §5.2 metric row recording every 429 against the next load so a truncated rung is identifiable rather than invisible; the docs/plans citation moved to .gitignore:27-30; and §3.8 states the SH_ vs KAGENTI_SANDBOX_* split as a rule (SH_ is harness-owned mechanism config, and SH_SANDBOX_DISCOVERY's sibling switch SH_REMOTE_SANDBOX is already SH_).

On the verification method flagged in the review: it caught the one citation that was wrong and correctly read the two numbers as incompatible. Worth noting that reviewing against the tip is what made :65 findable — a stale checkout would have reproduced the blended figure instead of retiring it.

[cwiklik]

Second pass, verifying a900d58 against main and the pinned fork rather than re-reading the argument. Both blocking items are properly fixed, and one of them is fixed better than I asked for.

On the duty split: the commit lands opposite my suggested basis and is right to. EXPERIMENTS.md:65 does say E6's real-converge finding supersedes "the earlier single-N figure (N ≈ 29–48:1), which used a trivial marker.txt leaf with no real converge" — so provisioning from E6's row is the authoritative reading and the conservative direction. My suggestion pointed at E7's pairing for internal consistency and would have under-provisioned. §2.3's two-row table with per-experiment cites is the right shape, and every derivation in it checks out arithmetically against the source rows.

On sticky: keying on X-SH-Session in the head is the correct resolution, and the rejected alternative is stated with the right reason. SH_TURNS_PER_WORKER is carried through §3.1/§3.3/§3.4/§3.5/§3.8/§3.9/§5.1/§5.2/§5.7 and the ADR, with the only surviving mention of the old name being the deliberate "it is why the variable is not SH_SESSIONS_PER_WORKER". §3.9's new staleness table is a genuine addition — naming under-admission as the dangerous direction because it truncates rungs and makes the knee read early is the same category of catch as E6's tier attribution.

Verified against main: server.ts:93-101 (JSON.parse(body) at :95, const { sessionId, prompt } = parsed at :101), :564's exact URL equality, .gitignore:27-30 plus the existence of docs/plans/README.md, select-sandbox.ts:75's selector, run-leaf.ts:391's cap, and all six duty/N figures against EXPERIMENTS.md:62-65, :76-80, :88-98, :121, :161, :168.

Requesting changes on one new must-fix, which is the previous finding one layer down rather than a regression: sticky's key is now per-request, but socket hand-off makes affinity per-connection, so a driver that pools connections across sessions turns the sticky arm into a measurement of its own connection reuse. §3.4 already names keep-alive reuse as real and §3.9 already relies on it — only sticky's dependence on it is unstated. One clause plus a run-record field plus one assertion in the test §7 already adds.

One suggestion (§5.4's default pairs the OCP duty with the two-cluster ratio, which fails §2.3's own N ≈ 1/duty test and the driver assertion three lines below it) and one nit (X-SH-Session vs MU1's "session token"), both inline.

Docs-only, all 12 checks green, three commits signed off, Assisted-By correct.

Prior review: #244 (review above) · P5 amendments now landed on docs/239-p5-spec-amendments include §3.2 step 3's shared scrub function, §4's non-CLI entry-point pin, and §5's parameterizable test entry point — the three forward references this spec consumes.

[cwiklik]

must-fix — the routing key is now per-request, but hand-off makes affinity per-connection. Sticky can still measure the wrong thing.

The header fix is right and it closes the previous finding. What it leaves open is one layer down: the supervisor inspects a connection once, at accept time, and then gives the socket away. After that it holds nothing — so every subsequent request on that same keep-alive connection reaches the worker the first request's X-SH-Session selected, and the supervisor cannot see, let alone re-route, a second session id arriving on it.

This is not hypothetical in this document. §3.4 argues against connection-level round-robin precisely because "a driver reuses keep-alive connections", and §3.9's staleness table already relies on the same fact for the over-admission row ("A second turn arrives on a kept-alive socket already handed off"). So the spec knows requests share connections; only sticky's dependence on that is unstated.

The consequence lands on the number, which is why I'm flagging it rather than filing it as a nit:

• If the E8 driver pools connections across sessions — the default in most HTTP clients, and undici/node:http agents do it unless told otherwise — stickyBySession degenerates toward whatever the pool does. Affinity hit-rate becomes a property of the driver's connection reuse, not of the policy.
• The arm then prices warmth at roughly zero and reads as "affinity isn't worth it", with nothing in the recorded data separating that from a genuine null result. §5.2's whole discipline is that a confound must be identifiable after the fact; this one currently isn't.
• It also quietly weakens §3.4's stated price. "A production client would have to start sending X-SH-Session" is necessary but not sufficient — it would also have to not multiplex sessions over one connection, which is a larger client-contract ask than the header alone.

Cheap to close, and it needs no design change:

1. One clause in §3.4: sticky affinity is connection-scoped, so the sweep arm requires one connection per session (a per-session agent, or Connection: close).
2. Record it per rung in §5.2 alongside the basis, so a sticky run that violated it is identifiable rather than invisible.
3. Extend §7's new unit test — it already asserts the header read and socket.unshift(head); asserting that a second request on the same socket is not re-routed pins the semantics as deliberate rather than accidental.

Alternatively state it as a limitation and drop the per-request framing: affinity is per connection, keyed by the first request's session id. Either resolution is fine; leaving it implicit is the thing that costs an experiment.

[pdettori]

Fixed in 9f87f11 — all three of your closures, plus the framing from your alternative, since they are not in tension.

You are right that this is the previous finding one layer down, and right that the spec already knew the fact twice over without connecting it to sticky. §3.4 now derives it rather than asserting it: affinity is connection-scoped because §3.2 gives the socket away, keyed by the first request on the connection, so a second id on that connection is neither seen nor re-routable. Stating it as a derivation matters — it makes clear there is no version of hand-off where sticky is per-request, so nobody later tries to "fix" it.

The three closures as landed:

1. §3.4 states the driver requirement — one connection per session for the sticky arm, a per-session agent or Connection: close — with your reason recorded, that http.globalAgent and undici pool by default.
2. §5.2 gains a run-record paragraph: connections-per-session per rung, which must be 1, sitting beside the duty basis. Both are conditions a rung can silently violate, so they belong together.
3. §7's unit test asserts a second request on the same socket is not re-routed even carrying a different id — pinned as deliberate, with the note that this is what stops a later reader from turning it into a body parse.

Your third bullet was the one I would not have caught: "a production client would have to start sending X-SH-Session-Id" was necessary but not sufficient. The price paragraph and ADR-0034 now name both preconditions — the header, and not multiplexing sessions over one connection. That is a materially larger client-contract ask than the header alone, and the finding is where it has to live.

Also X-SH-Session → X-SH-Session-Id throughout, per your nit.

[cwiklik]

suggestion — this default pairs the OCP duty with the two-cluster ratio, which is the blend §2.3 defines a test for.

§2.3 splits the E6 basis into two rows, correctly:

	Duty	Implied N
E6 / OCP	0.06–0.08	12–17:1
E6 / Kind	~0.04–0.05	~20–24:1

and then states the check: "The pairs are internally consistent because N ≈ 1/duty; a pair that fails that check has been blended from two rows."

This line says the two decisions come from "the same row of §2.3's table", then names duty = 0.06–0.08 (OCP) with 12–24:1 (OCP ∪ Kind). 1/0.06 = 16.7, not 24 — so the stated default is exactly the pair its own invariant rejects. The residual error is ~1.9×, smaller than the 2.4× the section was written to kill, but the same kind.

The sharper argument is that it contradicts the assertion three lines below. Provisioning at 24:1 gives K = ceil(W × S / 24), while K ≥ ceil(W × S × 0.08) = ceil(W × S / 12.5). A run configured from the default as written, at the top of its stated range, fails the driver assertion the same section defines — so the guard is right and the prose is what's wrong, which is the good direction to have the bug but still worth fixing.

Suggest naming the row rather than the experiment: default to E6/OCP — duty = 0.06–0.08, provision at 12–17:1 (also the conservative row, which is the direction §3.1a's process churn argues for), with E6/Kind's 0.04–0.05 / 20–24:1 available whole as a second point, exactly as E7's row already is.

The same union appears in three other places, all reading 6–8% alongside 12–24:1:

• line 179 (§3.1) — "the provisioning ratio of the duty basis §5.4 pins (E6's, so 12–24:1)"
• line 684 (§9 references) — "E6's 6–8% / 12–24:1"
• docs/adrs/0034-vm-process-manager-socket-handoff.md:27-28 — "duty at 6–8% … (N ≈ 12–24:1, cluster-dependent)"

The ADR's "cluster-dependent" is honest about why the range is wide, so it may be fine as a summary; the two spec sites feed decisions and are worth pinning to one row.

(For the record, the derivations all check out against main: OCP execMs 455–518 over wallMs 6551–7108 gives 0.061–0.079 and N 12.6–16.5 (EXPERIMENTS.md:88-94); Kind 277–290 over 5711–6648 gives 0.042–0.049 and N 19.7–24.0 (:76-80); E7 is 0.021/0.035 → 47.6/28.6 (:121, :161). And :65 does say what the commit message says it says — you were right to provision from E6 and I was wrong to suggest E7's pairing.)

[pdettori]

Fixed in 9f87f11, and the fix was to make the unit explicit rather than just swap the numbers: a basis is a row, not an experiment.

You're right on all three counts — the pairing fails §2.3's own invariant, it fails the driver assertion three lines below (24:1 → K = ceil(W × S / 24) against a guard demanding ceil(W × S / 12.5)), and the guard catching the prose is the better direction for that error to run. My §2.3 table had already split E6 into two rows; §5.4 then reached back up and named the experiment, re-blending what the table had separated. That is the same mistake one level up from the one the section was written to kill, which is why naming the granularity was the fix rather than editing the range.

§2.3 now has three peer rows with per-row cites and the exact per-row derivations you verified — E6/OCP 0.061–0.079 / 12.6–16.5 (:88-94), E6/Kind 0.042–0.051 / 19.7–24.0 (:76-80), E7 0.021/0.035 / 28.6–47.6 (:121, :161) — replacing the rounded ranges, since a section about exact pairing should not round. §5.4 defaults to E6/OCP with your reasoning (conservative row, the direction §3.1a's churn argues for) and keeps the other two as whole sweep points, and it now records the ~1.9× blend as the pinned-out failure so the next reader sees why the row is named.

All three other sites are pinned to the row: §3.1 (E6/OCP, so 12.6–16.5), §10's reference (all three rows enumerated), and the ADR Context — I took the ADR's precise route rather than keeping "cluster-dependent" as a summary, because it also states the rule (a basis is a row, not an experiment) and a summary that spans two rows would undercut the rule in the same paragraph.

[cwiklik]

nit — X-SH-Session collides semantically with MU1's "session token".

MU1 §3.5 claims Authorization: Bearer <session token> for caller auth, and P5 §3.2 reserves X-SH-Subject for whose work it is. Adding X-SH-Session for a Pi conversation id puts two different meanings of session in one header family — one a credential, one a resumable-state identifier — and the credential one is the one an implementer will meet first, since it's the header that gates the request.

X-SH-Session-Id costs one word now. It stops being free the moment the E8 driver, the sticky policy, and the client-contract ask in §3.4 all reference the shorter name.

[pdettori]

Renamed to X-SH-Session-Id in 9f87f11.

Verified the collision: MU1's header table (:249-252, in §3.5) claims Authorization: Bearer <session token> for may this caller use the harness against P5's X-SH-Subject for whose work this is — and MU1 §5.2 owns the token. Your point about which meaning arrives first is the operative one, so §3.4 records it: the credential is the header that gates the request, so it is the sense an implementer meets first, and one word now is cheaper than the E8 driver, the policy and §3.4's client-contract ask all carrying the shorter name.

Recorded rather than silently renamed, so the -Id does not get trimmed back later by someone who sees only redundancy in it.

[pdettori]

Pushed 9f87f11 — the new must-fix, the suggestion and the nit, with detail in each thread.

The connection-scoping catch was the right one to block on, and its most useful part was the third bullet. I had stated sticky's price as "a production client would have to start sending the header"; that is necessary but not sufficient, since it would also have to stop multiplexing sessions over one connection. §3.4 and ADR-0034 now name both preconditions. The mechanism itself is stated as a derivation from §3.2 rather than as a caveat — the supervisor inspects a connection once and then holds nothing, so there is no version of hand-off in which sticky is per-request — plus the driver requirement (one connection per session), a connections-per-session run-record field in §5.2, and the §7 assertion that a second request on the same socket is not re-routed.

The §5.4 blend was the same error one level up from the one §2.3 was written to kill, so the fix was to make the granularity explicit rather than edit a range: a basis is a row, not an experiment. §2.3 now carries three peer rows with per-row cites and the exact derivations — E6/OCP 0.061–0.079 / 12.6–16.5, E6/Kind 0.042–0.051 / 19.7–24.0, E7 0.021/0.035 / 28.6–47.6 — replacing the rounded ranges, since a section about exact pairing shouldn't round. §5.4 defaults to E6/OCP, and §3.1, §10's reference and the ADR Context are all pinned to that row rather than to the union. I did not keep the ADR's "cluster-dependent" phrasing: that paragraph now also states the row rule, and a summary spanning two rows would undercut the rule in the same breath.

Also X-SH-Session → X-SH-Session-Id, with the reason recorded in §3.4 so the -Id isn't trimmed back later by someone who sees only redundancy.

Noted on the P5 amendments landing on docs/239-p5-spec-amendments — that closes all three forward references §3.6 and §3.10 consume, so P6's only cross-track ask is now satisfied upstream of it.

[cwiklik]

Third pass. All three findings from 5155465832 are closed, and I re-derived the numbers against EXPERIMENTS.md on main rather than taking the new table's word for it.

must-fix — connection-scoped affinity. Closed, and §3.4 now derives it from §3.2 rather than asserting it (:284-291), which is better than what I asked for: stating it as a consequence of hand-off makes clear there is no version of this architecture where sticky is per-request, so nobody later "fixes" it into a body parse. All three closures landed — the driver requirement with the http.globalAgent/undici reason (:293-299), the §5.2 run-record field (:519-523), and §7's assertion that a second request on the same socket is not re-routed even with a different id (:625-628). The ADR now names both preconditions rather than just the header (:64).

suggestion — the duty basis. Making the unit explicit was a better fix than swapping numbers, and you were right to land on E6/OCP against my suggestion of E6-generally, on the strength of EXPERIMENTS.md:65. Every row checks out, and checks out in a way that makes the invariant self-enforcing: each (duty, N) pair is exactly 1/N from the document's own N column, so N ≈ 1/duty now holds by construction rather than by arithmetic that could drift.

• E6/OCP 0.061–0.079 / 12.6–16.5 — 1/16.5 = 0.0606, 1/12.6 = 0.0794; N values are the table's own 14.8 / 12.6 / 16.5. Cite :88-94 correct (rows :90-92, summary :94).
• E6/Kind 0.042–0.051 / 19.7–24.0 — 1/24.0 = 0.0417, 1/19.7 = 0.0508. Cite :76-80 correct.
• E7 0.021/0.035 / 28.6–47.6 — :121 gives 0.021 → N ≈ 48:1 over a 13.9 s leaf wall, :161 gives 0.035 → N ≈ 29:1, :135 gives the 28.6 explicitly.

The derived claims hold too: ~3.8× end to end is 47.6/12.6 = 3.78, and 1.9× across E6's two clusters alone is 24.0/12.6 = 1.90. The union is scrubbed from all four sites I flagged — §3.1 :179, §5.4, §10's reference :721, ADR Context. The surviving 0.06–0.08 / 12–24:1 at :579 is the passage naming the old error, which is where it belongs; the surviving 29–48:1 mentions are either the "superseded" framing or pre-existing docs this PR does not touch (0022, the E6 spec).

nit — the header name. Renamed at all five sites including the ADR (:64) and §7 (:625); the one surviving bare X-SH-Session at :274 is the sentence explaining why the bare name was rejected, which is the right place for it. Same discipline held on the earlier SH_TURNS_PER_WORKER rename — the lone SH_SESSIONS_PER_WORKER at :363 is the "why not this name" line.

---

Approving. One suggestion, which is the last mile of the first finding rather than a new direction: the driver requirement pins the sticky arm but not the default one, so the two arms now differ in driver configuration as well as in policy. I would not hold the PR for it — the spec is sound to build from, and it is a protocol detail the E8 driver has to settle regardless. Plus one nit on a wall-clock range that is itself blended across rows.

Also verified: CI 12/12 green on 9f87f11 (lint included, so prettier is satisfied), DCO passing with sign-off on all four commits, docs(spec): title prefix valid, ## Summary present, docs-only diff with no .claude/ or .vscode/ paths, and no secrets.

[cwiklik]

suggestion — this closes sticky's measurement gap, but it pins only one of the two arms, which reintroduces the cost asymmetry this section rejects the body-parse alternative for avoiding.

The derivation at :284 is not sticky-specific. leastInFlight also decides once, at accept, and the supervisor also holds nothing afterwards — so every later request on a kept-alive socket lands on the worker chosen for the first one, however loaded it has since become. Two things follow.

1. §3.4's opening argument now over-claims. :254 rejects connection-level round-robin because it "distributes badly the moment a driver reuses keep-alive connections," and :288 cites that sentence as the document already knowing the fact. But under hand-off that sentence describes the architecture, not round-robin — the default inherits it. Least-in-flight is still better, and for a reason worth stating: at each accept it sees true load, so a pool of C sockets created one at a time gets a roughly even static partition across W workers, where round-robin's is positional. What it does not have is immunity — it has a staleness that grows with reuse, and skews with turn-cost heterogeneity, because nothing rebalances a socket once given away. "Strictly better for the same cost" reads as immunity. One clause — least-in-flight is decided per connection too; what it buys is that each connection's initial placement is load-aware — would keep the comparison honest and stop the next reader concluding routing is per-request.

2. The arms differ in driver configuration, not only in policy. §5.2 records connections-per-session on sticky rungs only (:520). So the sticky arm runs one connection per session while the default arm pools — meaning sticky pays one accept + hand-off per session and holds an idle socket for each session's life, where the default pays roughly one per peak-concurrency slot and reuses across sessions. On a 200-session rung at peak concurrency 50, that is ~200 accepts and 200 open sockets against ~50 and ~50.

That cost lands on exactly the side this section says must stay free. :279 rejects the body parse because it turns "sticky's supervisor cost into a term E8 would have to measure rather than one that is free" — one connection per session is a smaller version of the same thing. It matters here specifically because §2.4 already predicts the benefit is small (~900-byte session log against a megabyte config bundle). A small benefit netted against a small unmatched cost can flip sign on the confound, which is the "reads as 'affinity isn't worth it' with nothing in the data separating that from a genuine null result" hazard arriving from the other direction.

Cheapest fix consistent with what landed: make connections-per-session a field on every rung rather than sticky rungs only, and state that both arms of a comparison must carry the same value — i.e. run the default arm one-connection-per-session too, so the only difference between arms is which worker gets chosen. That also gives the default arm's rungs the one driver property determining how much of its load balancing actually happened, which §5.2 currently cannot reconstruct after the fact: a default rung whose driver pooled heavily measured a static partition, not least-in-flight, and nothing in the record distinguishes the two.

Non-blocking, and I would not hold the PR for it — the spec is sound to build from as it stands, and this is a protocol detail the E8 driver has to settle anyway. Worth a sentence here so it is a decision rather than something discovered at rung time.

[pdettori]

Both parts taken in de7c0a9, despite the non-blocking framing — part 1 is a live over-claim in the document and part 2 would have cost the comparison, so neither is really a rung-time detail.

Part 1. You're right that I derived the property for sticky and then left the default's paragraph asserting immunity from it. Worse, :254's reason for rejecting round-robin — "distributes badly the moment a driver reuses keep-alive connections" — is, under hand-off, a description of the architecture rather than of round-robin, so I was citing the default's inherited constraint as evidence for the default. §3.4 now states the distinction your clause names: least-in-flight buys load-aware initial placement where round-robin's is positional (C sockets opened one at a time get an even partition reflecting real load, not arrival order), and explicitly does not buy immunity from reuse — staleness grows with reuse, skews with turn-cost heterogeneity, and nothing rebalances a socket once given away. "Strictly better for the same cost" is gone; "better default, not a per-request balancer" replaces it.

Part 2 is the one I'm glad you pushed on, because the asymmetry is the same shape as the thing §3.4 had just finished rejecting. Your accounting is the argument: ≈200 accepts and 200 held sockets against ≈50 and ≈50 is an unmatched cost on the arm §2.4 already predicts has a small benefit, and a small benefit netted against a small unmatched cost can flip sign — the identical confound to the pooled-driver one, arriving from the other side. So both arms of a comparison run the same connections-per-session, framed as §5.3's E9 invariant applied one tier up: vary one thing, pin the rest. That framing is what makes it a decision rather than a driver knob.

Your closing observation drove the §5.2 change more than the fairness argument did: a default rung whose driver pooled heavily measured a static load-aware partition, not least-in-flight in motion, and nothing in the record distinguished those. So the field is recorded on every rung with the two reasons stated separately — sticky's affinity hit-rate, and the default's how-much-balancing-actually-happened — plus the note that a mismatch between arms charges one policy for the other's connection budget. ADR-0034 carries the both-arms pin.

[cwiklik]

nit — this wall-clock range is itself blended across E6's two rows, in the sentence that establishes rows as the unit.

~6.1–7.1 s matches neither row: E6/Kind is 5711–6648 ms (:76-78) and E6/OCP is 6551–7108 ms (:90-92). 6.1 is Kind's L0 (6095) and 7.1 is OCP's L2 (7108), so the range spans the row boundary the paragraph is drawing.

Nothing downstream depends on it — it is a descriptive comparison against E7's leaf wall, not a (duty, N) pair, so no invariant fails. It is just one number away from being an instance of the thing the section exists to prevent. ~6.6–7.1 s (E6/OCP, the row the spec provisions from, which is also the sharper comparison since that is the basis §5.4 pins) or ~5.7–7.1 s (both E6 rows, said to be both) would each do.

Verified while I was in there: 13.9 s is E7/Kind (:121) and E7/OCP is ~14 s (:161), so the single figure is fine for the comparison being made.

[pdettori]

Fixed in de7c0a9 — now E6/OCP's ~6.6–7.1 s (6551–7108 ms), the row §5.4 pins.

Took the OCP option for the reason you give: it is the sharper comparison, since it is the basis the design provisions from, and it keeps the paragraph that establishes rows as the unit from containing a cross-row range. Confirmed 6.1 was Kind's L0 (6095) against 7.1 as OCP's L2 (7108) — a blend I introduced in the same edit that split the rows apart, which is a good illustration of why the invariant had to be made checkable rather than just stated.

[pdettori]

Pushed de7c0a9. Both post-approval items are addressed rather than deferred — the first is a live over-claim in §3.4, and the second would have biased the E8 comparison, so neither reduces to a rung-time detail.

§3.4's opening asserted immunity the architecture does not grant. leastInFlight decides once at accept too, so its stated reason for rejecting round-robin — that drivers reuse keep-alive connections — was really a description of hand-off, i.e. of a constraint the default inherits. §3.4 now says what least-in-flight actually buys (load-aware initial placement, where round-robin's is positional) and what it does not (immunity from reuse: staleness grows with reuse, skews with turn-cost heterogeneity, nothing rebalances a socket once given away). "Strictly better for the same cost" is gone.

The one-connection-per-session pin covered one arm, which is the asymmetry §3.4 had just rejected the body parse for creating. ≈200 accepts and 200 held sockets against ≈50 and ≈50 is an unmatched cost on the arm §2.4 already predicts has a small benefit — and a small benefit netted against a small unmatched cost can flip sign, the same confound from the other direction. Both arms now run the same connections-per-session, framed as §5.3's E9 invariant one tier up: vary one thing, pin the rest. §5.2 records the field on every rung, with each arm's reason stated separately — the review's sharpest point being that a pooled default rung measured a static load-aware partition rather than least-in-flight in motion, and nothing in the record told those apart. ADR-0034 carries the pin.

And the nit: ~6.1–7.1 s was Kind's L0 against OCP's L2 — a cross-row blend I introduced in the same edit that split the rows, now E6/OCP's ~6.6–7.1 s. A fair demonstration of why that invariant needed to be checkable rather than merely stated.

Thanks for the three passes — each one found the layer below the last, and the numbers in §2.3 are now self-enforcing because of it.