Commit Graph

158 Commits

Author SHA1 Message Date
Vitor Pamplona ede4bc5eab feat(quic): client-initiated 1-RTT key update + dispatch ecn/blackhole/amplificationlimit
The runner's keyupdate testcase has TESTCASE_CLIENT=keyupdate (server
runs plain transfer). The runner verifies the pcap shows BOTH sides
emit packets in phase 1 — pre-fix our receive-only key-update path
satisfied a server-initiated rotation but not this test, because
aioquic's transfer-server doesn't rotate spontaneously. Result: 0
phase-1 packets either direction, "Expected to see packets sent with
key phase 1 from both client and server".

QuicConnection.initiateKeyUpdate() (now public) is the send-side
analogue of commitKeyUpdate: derives next-phase secrets for both
directions via HKDF-Expand-Label "quic ku", installs as live
(reusing old HP keys per RFC §6.1), flips currentSendKeyPhase +
currentReceiveKeyPhase together. The receive side has to roll too
because the peer responds in the new phase — leaving currentReceive
at 0 would force feedShortHeaderPacket to take the
deriveNextPhase-then-commit path on the response and orphan the
keys we just installed in previousReceiveProtection.

InteropClient adds an `initiateKeyUpdate` flag to runTransferTest;
the keyupdate dispatch sets it true. After awaitHandshake (TLS done,
1-RTT keys derived) the flag-flow polls briefly for status=CONNECTED
(HANDSHAKE_DONE arrived → handshake confirmed per RFC 9001 §6.5
prerequisite) before calling initiateKeyUpdate, then sends the GET.
The GET goes out in phase 1, the server mirrors phase 1 in its
response, runner is satisfied.

Also added ecn, amplificationlimit, blackhole to the runTransferTest
dispatch (all reuse the plain-transfer flow; the runner verifies
behaviour via pcap independent of any client-side dance). aioquic
phase 3 result: ✓(retry, keyupdate, blackhole),
?(resumption, zerortt, ecn — feature gaps requiring session tickets,
0-RTT, and IP-layer ECT codepoints respectively),
amplificationlimit blocked by a runner-side cert-gen bug on macOS
(tr LC_CTYPE=C doesn't suppress UTF-8 errors, the chainlen=9 cert
inflation step fails).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-07 17:34:13 -04:00
Vitor Pamplona d1567e4a53 fix(quic): faster PTO with INITIAL_RTT=100ms and unified ptoBaseMs path
multiconnect handshakeloss / handshakecorruption tail-fail under the
runner's 30% packet drop / bit-flip scenarios because each PTO retransmit
chance gives ~49% one-side success (0.7² for both directions clear). With
INITIAL_RTT=333ms the first PTO fires at 999 ms and doubling tops out
at ~5 attempts in 30s — across 50 sequential connections, ~5% probability
some iteration runs out of retransmits before the per-iter budget.

Two coupled changes:

1. INITIAL_RTT_MS 333→100. RFC 9002 §6.2.2 spec-allowed (the standard
   default but explicitly configurable). Matches Chrome and
   Firefox/neqo. Pre-sample PTO is now 300 ms instead of 999 ms;
   doubling fits ~8 retransmit attempts in 30s instead of 5,
   pushing per-iter loss-recovery success past 99% under 30% drop.
   Spurious retransmits on slow paths are harmless (peer dedupes
   by packet number) and smoothed_rtt converges in one round-trip.

2. QuicConnectionDriver always uses lossDetection.ptoBaseMs() for the
   PTO timer, including before the first RTT sample. Pre-fix the
   driver hardcoded 1000ms as a "handshake-timeout safety floor"
   that ignored INITIAL_RTT_MS entirely — the PTO was always 1s
   pre-handshake regardless of the constant. Now both pre- and
   post-sample regimes go through the same calculation.

   max_ack_delay is gated to APPLICATION space (RFC 9002 §6.2.1) so
   pre-handshake PTOs aren't padded with the peer's quoted delay.

Two pre-existing tests (PtoTest, QuicLossDetectionTest) hard-coded
expected PTO durations derived from the old 333 ms constant; updated
them to express the relationships in terms of INITIAL_RTT_MS so future
tweaks don't desync.

Result: 21/21 against aioquic, picoquic, quic-go (handshake,
multiplexing, longrtt, transferloss, transfercorruption,
handshakeloss, handshakecorruption all pass on each peer).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-07 16:42:33 -04:00
Vitor Pamplona 86a4727efb feat(quic-interop): multiconnect dispatch + multiplex stream-budget pacing
Two interop-runner gaps closed in one InteropClient pass plus a
QuicConnection snapshot helper:

1. multiconnect testcase. The runner's handshakeloss /
   handshakecorruption tests reuse TESTCASE_CLIENT=multiconnect — 50
   sequential connections, each fetching a 1KB file under 30% packet
   drop or bit-flip, with the runner verifying _count_handshakes()==50
   in the pcap. Pre-fix our InteropClient dispatch returned 127 (skip)
   for "multiconnect", so both tests showed as ?(L1, C1). Added
   runMulticonnectTest: loops fresh socket + conn + driver + GET +
   close per URL. Per-iteration qlog files at $QLOGDIR/client-N.sqlog
   so a stuck iteration leaves a focused trace.

2. multiplex pacing against quic-go. Pre-fix the parallel path
   chunked the URL list into fixed groups of MULTIPLEX_PARALLELISM=64.
   Worked against aioquic + picoquic (initial_max_streams_bidi=128)
   but blew up against quic-go (advertises 100, ramps slowly via
   MAX_STREAMS_BIDI bumps): second chunk pushed cumulative used past
   limit, threw QuicStreamLimitException. Now each iteration takes
   min(MULTIPLEX_PARALLELISM, peerMaxStreamsBidi - used). When budget
   hits 0, brief 50ms idle waits for the peer's bump.

   New QuicConnection.localBidiStreamsUsedSnapshot() exposes the
   consumed-side counter; combined with the existing
   peerMaxStreamsBidiSnapshot() the InteropClient computes the live
   available budget without holding streamsLock.

Result against quic-go: H, M, LR, L2, C2, C1 pass; was 0/7 at
session start (handshake itself failed pre-ALPN-fix), 4/6 after
key-update fix, now 6/7. Only L1 (handshakeloss) remains as
multiconnect-under-30%-drop flake (same flake picoquic shows).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-07 15:54:48 -04:00
Vitor Pamplona b622d0c936 feat(quic): RFC 9001 §6 1-RTT key update
quic-go initiates a 1-RTT key update partway through every transferloss
or transfercorruption test (KEY_PHASE bit flips 0→1 around server pn=100
by default). Pre-fix our parser used the OLD application keys for every
post-update packet, AEAD-failed all of them, never sent another ACK,
the server fell into PTO mode, and throughput collapsed (~24kbps over
60s vs the 10Mbps the path supports).

The fix is end-to-end:

- ShortHeaderPacket.peekKeyPhase: HP-unmasks just the first byte to
  surface the key-phase bit BEFORE running AEAD. The parser uses this
  to pick the right keys instead of paying for a doomed AEAD.

- QuicConnection: tracks the live application secrets (server- and
  client-side) and current send/receive key phase, plus a
  previousReceiveProtection slot for RFC §6.1 reorder-window decryption.
  deriveNextPhaseReceiveKeys derives the next phase via
  HKDF-Expand-Label("quic ku", "", Hash.length) without committing;
  commitKeyUpdate installs them only after AEAD has succeeded, then
  rolls the send side forward in lockstep so our next outbound
  carries the matching KEY_PHASE bit (peer needs that to confirm the
  rotation completed). HP key is NOT rotated, per spec.

- QuicConnectionParser.feedShortHeaderPacket: three-way dispatch on
  the peeked bit — matches current → live keys; matches retained
  previous → previous keys (reordered packet); else → derive
  next-phase, attempt AEAD, commit on success.

- QuicConnectionWriter: ShortHeaderPlaintextPacket(... keyPhase =
  conn.currentSendKeyPhase) at both 1-RTT build sites (steady-state
  and CONNECTION_CLOSE).

We don't drive key updates ourselves — only echo the peer's. Avoids
the bookkeeping for RFC 9001 §6.6 packet-count limits and the safety
benefits of voluntary rotation aren't load-bearing at our connection
scale.

Tests: peekKeyPhase round-trip + long-header rejection;
2-byte-pn round-trip when largestReceived is far behind (the original
suspected-but-not-actual cause before the key-phase reveal).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-07 15:53:56 -04:00
Vitor Pamplona d5c854befa fix(quic): PTO retransmits handshake CRYPTO + STREAM data on stalled-ACK paths
RFC 9002 §6.2.4 says a PTO probe SHOULD retransmit unacked data, not
emit a bare PING. Two gaps in our handler surfaced via interop:

1. Handshake CRYPTO past the 1-RTT-keys-up boundary. The pre-fix
   handler gated the requeue on `application.sendProtection == null`
   so once 1-RTT keys were derived, our Finished (still inflight at
   Handshake level until the peer ACKs it) was never retransmitted.
   Lost Finished → server never confirms handshake → never sends
   HANDSHAKE_DONE → connection wedges with ACK-only handshake packets
   bouncing forever. Surfaced by handshakeloss against aioquic at 30%
   drop rate (multiconnect iter 12 stuck at t=52s, zero handshake_done).

2. STREAM data when the peer never ACKs anything. Our loss detection
   gates on `pn < largestAckedPn`, which never advances when every one
   of our 1-RTT packets is dropped or corrupted en route. Surfaced by
   handshakecorruption: we send H3 init streams + GET in 1-RTT pn=0,
   gets corrupted, server never decrypts, never ACKs. Pre-fix the
   STREAM bytes were never retransmitted; the GET stalled.

Fix: handlePtoFired now requeues inflight CRYPTO at every active
pre-application level (Initial AND Handshake) regardless of 1-RTT
state, and walks streamsList to re-queue inflight STREAM bytes when
1-RTT keys are up. requeueAllInflight is a no-op when nothing is
inflight, so calling on already-ACKed / already-discarded levels is
harmless.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-07 15:51:51 -04:00
Vitor Pamplona 2a4c07ae5e fix(quic): thread offered ALPN list through TlsClient → ClientHello
QuicConnection.alpnList → TlsClient.offeredAlpns was captured but never
threaded into buildQuicClientHello. The builder accepted only an
"additionalAlpn" parameter and hardcoded "h3" first, so our wire
ClientHello always carried just [h3] regardless of caller intent.
quic-go enforces strictly with TLS alert 120 (no_application_protocol,
CRYPTO_ERROR 376) when none of the offered ALPNs match its server
config — handshake failed at the very first server response against
quic-go's hq-interop testcases.

Replaced the awkward additionalAlpn shape with `alpns: List<ByteArray>`
(default [h3] for backward-compat) and threaded TlsClient.offeredAlpns
through.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-07 15:48:45 -04:00
Claude ac0d6f06a9 fix(quic-interop): detect multiplexing by URL count, not TESTCASE name
The smoking gun from the 2026-05-07 multiplex run boot log:

  [boot] DEBUG=1; ...; TESTCASE=transfer; ROLE=client
  [boot] transfer mode: parallel=false urls=1999

quic-interop-runner sets TESTCASE_CLIENT=transfer for ALL the
transfer-family testcases (transfer, multiplexing, transferloss,
transfercorruption). Discrimination between transfer (1 file) and
multiplexing (~2000 files) happens by URL count, NOT by TESTCASE
name — so our check `parallel = (testcase == "multiplexing")` was
always false, even for the multiplexing test, and we always took
the serial fallback path: client.get(authority, path) per URL,
opening one stream + awaiting before the next. That's why the wire
showed exactly one stream per RTT for the entire 60s.

Fix: parallel = (token count of REQUESTS env var) > 1. Effectively:
  - 1 URL → transfer testcase, serial path
  - >1 URL → multiplexing testcase, batched-parallel path

Verified the writer's batched coalescing already works in unit
tests (MultiplexingCoalescingTest, MultiplexingAioquicTpsTest both
green at ~9 streams/packet). With this dispatch fix, the live
runner should finally reach the batched code path.

Bumped WRITER_DEBUG_BUILD_ID so the next [boot] line confirms
the fix is deployed.
2026-05-07 14:24:52 +00:00
Claude a9dc927cc6 diag(quic-interop): log TESTCASE + parallel branch entry
Hypothesis: the [interop] / [batch] logs are missing because we're
hitting the SERIAL branch (parallel=false), not the parallel one —
which would explain perfectly the writer trace pattern:
  - streamsView grows by 1 every 2-3 drains
  - active stays at 6 or 7 (3 H3 init + at most 4 chunk streams)
  - one stream per RTT cadence on the wire

That's exactly what client.get(authority, path) looks like in
sequence. parallel=true would call prepareRequests for chunks of 64.

Two new unconditional log lines (low-volume, control-flow only,
NOT in hot paths):

  1. [boot] now includes TESTCASE and ROLE — to verify the runner
     is sending TESTCASE=multiplexing as expected
  2. [boot] transfer mode: parallel=BOOL urls=N — confirms which
     branch we took

If parallel=false despite TESTCASE=multiplexing, the bug is in our
testcase-to-parallel mapping (line 192 of InteropClient.kt).

If parallel=true but [interop]/[batch] still missing, the bug is
elsewhere.
2026-05-07 14:19:08 +00:00
Claude f13d1ae1eb diag(quic-interop): boot log + build-id verify deployed image is fresh
The [batch]/[interop] traces aren't appearing in the user's runs
even after rebuild. To distinguish 'env var not set' from 'binary
doesn't have the code', emit a [boot] line at startup that:

  1. Reports the QUIC_INTEROP_DEBUG env var value
  2. Reports whether writerDebugEnabled was flipped on
  3. Includes a build-id constant from WriterDebug.kt

If the user's run shows '[boot] DEBUG=1; ... build_id=2026-05-07-
batch-log-v1', we know the latest debug code IS deployed. If the
boot line is missing OR shows an older build_id, the docker image
served stale bytecode (gradle/docker layer caching) and a clean
rebuild is needed.

Inspect script now greps [boot] and surfaces it BEFORE the rest of
the writer-side debug section.
2026-05-07 14:02:16 +00:00
Claude 679bb62a17 diag(quic-interop): chunk-size + openBidiStreamsBatch entry/exit logs
Writer trace from the latest run shows streamsView grows by 1 every
2-3 drains, NOT by 64 per chunk. Suggests batching isn't happening,
even though the bytecode confirms openBidiStreamsBatch IS being
called (via javap on the deployed class file).

Two new diagnostic lines per multiplex run, both gated by DEBUG=1:

  [interop] multiplex start: total_urls=N MULTIPLEX_PARALLELISM=64
            expected_chunks=32
  [interop] chunk=0 size=64 starting prepareRequests
  [interop] chunk=1 size=64 starting prepareRequests
  ...
  [batch] openBidiStreamsBatch items=64 returned=64
          streamsList_before=6 streamsList_after=70

If the [batch] line shows items=1 (instead of 64), the chunked()
call is producing chunks of 1 (would be a bug in MULTIPLEX_
PARALLELISM or chunked semantics).

If [batch] shows items=64 returned=64 streamsList_after=70, then
batching IS working at this layer and the bug is downstream — the
writer is somehow only seeing 1 stream at a time despite 64 being
in the list.

https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT
2026-05-07 13:32:54 +00:00
Claude 9cd8d0a6ee diag(quic): writer-side per-drain stats behind DEBUG=1 build flag
The qlog confirmed the writer emits 1 STREAM frame per packet on the
live wire, while MultiplexingCoalescingTest + MultiplexingAioquicTpsTest
both show ~9 streams per packet under synchronous drain. So the bug is
in the live driver flow — concurrent send loop + parser feed +
real-socket interleaving — and not in buildApplicationPacket itself.

To localize: add an opt-in trace at the END of buildApplicationPacket
that dumps per-drain state when QUIC_INTEROP_DEBUG=1:
  [writer.app] frames=N stream_frames=K streamsView=M active=A
               packetBudget_remaining=R connBudget_initial=C

  - frames vs stream_frames tells us if non-stream frames (ACK,
    MAX_DATA, MAX_STREAM_DATA) are bloating the packet
  - active vs streamsView tells us if isClosed filter dropped streams
  - packetBudget_remaining tells us if we hit the 64-byte break early
  - connBudget_initial tells us if conn flow control was zero

Wired three pieces:

  1. WriterDebug.kt — a single @Volatile boolean owned by commonMain,
     `writerDebugEnabled`. Off by default.
  2. InteropClient.main flips it to true if QUIC_INTEROP_DEBUG=1 is set
     in the env.
  3. Dockerfile + Makefile accept --build-arg DEBUG=1 (or `make build
     DEBUG=1`) to bake the env var into the image.

Usage:
  cd quic/interop
  make build DEBUG=1
  cd ../..
  ./quic/interop/run-matrix.sh -s aioquic -t multiplexing
  cat ../quic-interop-runner/logs/run-*/aioquic_amethyst/multiplexing/output.txt | grep '^\[writer'

When off, cost is one volatile read in the writer hot path — negligible.

https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT
2026-05-07 13:08:14 +00:00
Claude 4616234c82 diag(quic-interop): dump first 10 packets fully + add live-driver-flow synth test
Two adds for the multiplex investigation.

(1) inspect-multiplexing.sh: previous histograms aggregate over the
   whole run. Add full-frame-array dump of the first 10 packet_sent
   AND first 10 packet_received events so we can see whether the
   FIRST chunk burst (~13 streams in one packet) or dribbled (1 per).

(2) MultiplexingAioquicTpsTest: synchronous drain test using EXACTLY
   the TPs aioquic gave us in the failing run (initial_max_data=1MB,
   initial_max_stream_data_bidi_remote=1MB, initial_max_streams_bidi=
   128) and ~80-byte HEADERS-frame-sized payloads. PASSES with 7
   packets / 9.1 streams per packet — proving the writer's coalescing
   is fine under aioquic's flow-control budget. So the bug is NOT in
   the writer; it's in the live driver flow that
   MultiplexingCoalescingTest doesn't exercise (concurrent send loop
   + parser + real socket).

The first-10 dump from inspect should localize this further:
  - if first packet has 13 stream frames → writer burst, bug is
    server-side timing or driver-loop scheduling
  - if first packet has 1 stream frame → writer producing 1 per call
    in production for some condition my synchronous test doesn't
    cover

https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT
2026-05-07 12:56:44 +00:00
Claude 6bcee12669 audit-r2(quic): tighten batched-open API tests + docs
Round-2 audit of the openBidiStreamsBatch / openUniStreamsBatch landing.

(1) Tests overpromised. The previous "holds streamsLock for the whole
batch" test only verified the API didn't crash and stream ids were
unique. A future regression that released the lock between opens
(the 2026-05-06 bug shape) would not break it.

Added `assertTrue(client.streamsLock.isLocked)` inside both batch
init lambdas. Now the test name matches what's verified.

(2) `*Batch` docstrings didn't warn that `init` runs under
streamsLock. A naive caller might do encoding / IO inside, defeating
the lock-hold-time goal that motivated pre-encoding outside. Added
the warning + the canonical caller shape (encode outside, enqueue
inside) to both function docs.

(3) Empty-batch corner: an empty `items` list was still acquiring the
lock and entering withLock. Added an `if (items.isEmpty()) return
emptyList()` short-circuit. New test pins the contract — `init`
must not run for an empty batch.

Test count: 6 → 7. All green; no production-API change.

https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT
2026-05-07 03:36:40 +00:00
Claude a0a604b8e7 refactor(quic): kill dead levelLock + add bug-resistant batched-open API
Audit follow-up. Two cleanups consolidated into one commit since they
share the goal of "make the lock contract obvious from the API".

(1) Remove LevelState.levelLock entirely.

The lock-split refactor introduced a per-level Mutex with a docstring
claiming the writer/parser would acquire it around encode + sentPackets
record + ACK observation. Neither actually does. SendBuffer's internal
synchronized(this) is what serializes cryptoSend mutations against
takeChunk and markAcked. handlePtoFired's levelLock acquisition was
the only production usage and it serialized only against itself.

Removed:
  - LevelState.levelLock
  - handlePtoFired's withLock wrapper (now a non-suspend fun)
  - All docstring references to a third lock domain

The pre-existing sentPackets HashMap race (writer mutates under
streamsLock, parser reads without sync) is unchanged — out of scope
for this PR. Acquisition order is now `lifecycleLock → streamsLock`,
flat.

(2) Add openBidiStreamsBatch + openUniStreamsBatch — the bug-resistant
high-level API for the prepareRequests / moq audio-rooms patterns.

The previous shape required callers to manually do
`streamsLock.withLock { repeat(N) { openBidiStreamLocked() ... } }`.
That contract regressed twice on this very branch: once held the wrong
lock (lifecycleLock alias), once skipped the wrap entirely. Both shapes
silently emitted one STREAM per packet under multiplex load.

The new API encapsulates the lock + the per-item init lambda:

    conn.openBidiStreamsBatch(items) { stream, item ->
        stream.send.enqueue(encode(item))
        stream.send.finish()
        Handle(stream)
    }

Callers physically cannot hold the wrong lock. Migrated:
  - Http3GetClient.prepareRequests
  - HqInteropGetClient.prepareRequests

Also added `openUniStreamLocked` + `openUniStreamsBatch` symmetric to
the bidi versions. moq audio-rooms eventually wants to open many uni
streams in burst; the same one-stream-per-packet bug lurks if every
open serializes through its own lock acquisition.

`openBidiStreamLocked` / `openUniStreamLocked` remain public (with
their `check(streamsLock.isLocked)` guards) for the rare custom-batch
callers that need to mix bidi+uni opens under a single hold. Most
callers should use the *Batch variants going forward.

Test coverage extended:
  - openBidiStreamsBatch happy path
  - openUniStreamLocked throws without streamsLock
  - openUniStreamsBatch happy path

Six tests in BatchedOpenLockContractTest now pin the contract.

https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT
2026-05-07 03:31:30 +00:00
Claude 07dd572423 perf+test(quic): audit follow-ups for the multiplex lock fix
Audit of recent multiplex / PTO commits surfaced three concrete
improvements:

1. **Pre-encode requests outside streamsLock** in both prepareRequests
   impls. QPACK encoding (Http3GetClient) and string formatting
   (HqInteropGetClient) are non-trivial under multiplex load — 1999
   paths means we were holding streamsLock across all 32 chunks ×
   ~2KB of QPACK encoding per chunk = ~64 KB of CPU work serialised
   against the send loop. Now done outside the lock.

2. **Fix O(N²) byte merging in MultiplexingRoundTripTest.** Previous
   shape allocated a new array per STREAM frame; for real-size
   payloads this would dominate test runtime. Switched to a per-
   stream MutableList<ByteArray> joined once at the end.

3. **Pin coalescing end-to-end in MultiplexingRoundTripTest.**
   Pre-existing test verified per-stream content arrived but said
   nothing about the wire shape. Added totalDatagrams ≤ 12 assertion
   so the matrix's "one stream per datagram" failure mode would
   break the test instead of passing silently.

Audit also surfaced two non-actionable items, flagged for future
cleanup but not changed in this commit:

- LevelState.levelLock docstring claims writer/parser acquire it
  around sentPackets mutations; in practice neither does. The
  handlePtoFired call that takes levelLock currently serialises
  only against itself; SendBuffer's internal synchronized is what
  prevents the actual cryptoSend race. Kept the call (matches the
  documented design intent and future-proofs against the writer
  actually taking levelLock) but the docstring is stale.

- openBidiStreamLocked's check(streamsLock.isLocked) catches "no
  lock" and "wrong lock" callers but not "another coroutine holds
  streamsLock and I'm calling without holding it" — kotlinx Mutex
  doesn't expose owner-aware checks without an explicit owner arg
  we don't pass. Acceptable since the bug we just fixed and any
  future regression in the same shape are caught.

https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT
2026-05-07 03:25:55 +00:00
Claude 991b1a1da3 fix(quic-interop): prepareRequests must hold streamsLock, not lifecycleLock
aioquic interop multiplexing 2026-05-06 qlog post-mortem:
  - 2898 packets sent in 60s, each carrying ONE STREAM frame
  - server log: streams created/discarded strictly serially, ~30-40ms apart
  - 1421/2000 files completed before the runner's 60s timeout
  - shape ≈ 1 RTT per stream — wire was emitting one stream per datagram

Cause: Http3GetClient.prepareRequests + HqInteropGetClient.prepareRequests
both did `conn.lock.withLock { ... openBidiStreamLocked() }`. Post the
lock-split refactor `conn.lock` is the deprecated alias for lifecycleLock.
The writer's drainOutbound takes streamsLock — not lifecycleLock — so the
send loop interleaved between every two openBidiStreamLocked calls,
draining one stream's data per pass.

Fix:
  1. Both prepareRequests impls now use conn.streamsLock.withLock.
  2. openBidiStreamLocked now `check`s streamsLock.isLocked at entry
     so this can never silently regress again — calling it without the
     lock (or with the wrong lock) throws IllegalStateException with
     a message naming streamsLock as the lock to acquire.
  3. New BatchedOpenLockContractTest pins the contract:
     - calling openBidiStreamLocked WITHOUT any lock throws
     - calling openBidiStreamLocked while holding lifecycleLock throws
       (the exact regression shape)
     - calling openBidiStreamLocked while holding streamsLock works
       (happy path)

The runtime check is the regression-proof part: future callers physically
cannot hold the wrong lock without the test (and prod) blowing up at the
first call site.

https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT
2026-05-07 03:18:18 +00:00
Claude 46926a712b test(quic): in-process matrix-shape multiplexing round-trip
The runner's `multiplexing` testcase opens N parallel bidi streams,
each downloading one file, and asserts every file's content lands
on the right stream. Existing tests cover pieces of that:

- MultiplexingThroughputTest: opens 1000 streams in <2s — measures
  lock contention but never moves bytes server-side.
- MultiplexingCoalescingTest: pins that 64 streams coalesce into ≤6
  packets — encoder contract, no end-to-end.
- MultiStreamFinDeliveryTest: server pushes responses to 50 streams,
  client surfaces every FIN — but the CLIENT never sends a STREAM
  frame in that test, so any regression in the writer's request-
  side multiplex path is invisible.

This test runs the full request → response loop:
  1. Open 64 parallel bidi streams
  2. Each enqueues a tiny request + FIN
  3. Drain client outbound → decrypt → assert all 64 STREAM frames
     made it across, with their request bytes intact
  4. Server sends one response per stream + FIN
  5. Per-stream incoming.toList() must yield the expected response

Failures call out the specific stream id, so a regression points
at "stream X dropped its FIN" instead of a generic timeout.

64 streams keeps wall-clock under a second; the bug class the test
guards against (per-stream loss / mis-routing) fires identically at
64 and 1999.

https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT
2026-05-07 02:55:18 +00:00
Claude b579c766a4 test(quic): exercise the actual driver PTO path, not a simulation
The existing PtoCryptoRetransmitTest simulated the driver inline:
it set pendingPing=true and called requeueAllInflightCrypto by hand,
then asserted the next drain emitted CRYPTO. That checked the helpers
worked but never noticed when the DRIVER stopped calling
requeueAllInflightCrypto — which is exactly the regression that bit
us in commits c0d7b6031 (qlog merge) and again in cf2303a38
(lock-split refactor).

Extract the PTO-fired logic from QuicConnectionDriver.sendLoop into
a top-level internal helper handlePtoFired(conn). Driver and test now
both call the same function — if anyone unwires the requeue from
that helper or rewrites sendLoop to do volatile-only updates, the
test breaks.

Verified the regression-catch by stripping the requeue from
handlePtoFired and re-running the test: it fails with an
AssertionError on the PTO retransmit packet check, as expected.
Restored the helper, test green again.

https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT
2026-05-07 02:50:31 +00:00
Claude cf2303a38d fix(quic): restore PTO CRYPTO retransmit lost in lock-split refactor
aioquic interop multiplexing qlog (the smoking gun):
  packet_sent  PN=0  Initial  frames=[crypto]  (ClientHello)
  packet_sent  PN=1  Initial  frames=[ping]    (PTO probe — bare PING)
  packet_received               connection_close 0x0
                                "Packet contains no CRYPTO frame"

This is the SAME bug commit c0d7b6031 fixed; the lock-split refactor
(ef4bb9998) re-wrote the driver's PTO branch to use streamsLock and
inlined the @Volatile pendingPing/consecutivePtoCount writes — but
dropped the requeueAllInflightCrypto call that c0d7b6031 had wired
under the (now-renamed) connection.lock.

Restored under levelLock for the appropriate level. SendBuffer's
internal synchronized covers correctness, but the level lock keeps
us from racing the writer's takeChunk mid-build.

The writer's comment at QuicConnectionWriter.kt:421-431 already
documented this contract — it was just unwired on the driver side.

https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT
2026-05-07 02:46:55 +00:00
Claude 03c00621d6 fix(quic): restore Retry fields + LevelState VN reset after lock-split merge
Tier 1 lock-split agent's worktree was based on main and didn't carry
the Retry-handling work (fields retryToken / retryConsumed, applyRetry
method's references to them). Merge with -X theirs nuked those.

Restored:
  - retryToken + retryConsumed @Volatile fields on QuicConnection,
    re-attached to applyRetry in the lock-split-merged file.
  - LevelState.resetForVersionNegotiation now uses pnSpace.resetForRetry()
    to reset the PN counter in place — pnSpace is `val` post-refactor,
    direct re-assignment doesn't compile. The naming is historical;
    the underlying semantics (zero PN counter + clear received side)
    are correct for both VN and Retry-with-fresh-PN cases.
  - openBidiStream split into the public suspend wrapper +
    openBidiStreamLocked() (caller holds streamsLock). Restores the
    batched prepareRequests path's ability to open N streams under one
    lock hold.
  - close() — local var firedQlog hoisted out of withLock block.

Test suite runs clean. Three deprecation warnings remain on
PtoCryptoRetransmitTest + QlogObserverTest still using the
backward-compat conn.lock alias; left for follow-up cleanup.

https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT
2026-05-07 02:28:26 +00:00
Claude d920bf8fd0 Merge branch 'worktree-agent-acb67f8575e4086eb' into claude/research-quic-libraries-hH1Dc 2026-05-07 02:25:28 +00:00
Claude ef4bb99988 refactor(quic): split conn.lock into streamsLock + per-level lock + lifecycleLock
The single connection-wide `QuicConnection.lock` mutex serialised every
critical path: the read loop's `feedDatagram`, the send loop's
`drainOutbound`, and every public mutator (`openBidiStream`,
`streamById`, `flowControlSnapshot`, ...). The multiplexing testcase
opens hundreds of bidi streams in parallel and was capped at ~25
streams/sec by lock contention against the I/O loops.

Phase 1 of the lock split (see
`quic/plans/2026-05-08-lock-split-design.md`) introduces three
domain-specific mutexes:

  - `streamsLock` — streams registry, datagram queues, stream-id
    counters, connection-level flow-control bookkeeping, pending-
    retransmit maps for control frames
  - `LevelState.levelLock` (one per encryption level) — per-level
    pnSpace / sentPackets / ackTracker / CRYPTO buffers
  - `lifecycleLock` — status transitions, close reason/error code

Acquisition order: `lifecycleLock < streamsLock < levelLock`.
Per-stream `synchronized(this)` blocks inside SendBuffer/ReceiveBuffer
remain at the leaf — never acquire any QuicConnection mutex while
holding a per-stream lock.

The legacy `lock: Mutex` field is preserved as a deprecated alias of
`lifecycleLock` for source-compatibility with external test harnesses;
new code MUST use the appropriate domain lock.

Highlights:

  - `feedDatagram` / `drainOutbound` now require the caller to hold
    `streamsLock`; the driver wraps each call. Phase 1 keeps the whole
    feed/drain inside `streamsLock` for safety; phase 2 (deferred) will
    split frame-collection from encrypt + sentPackets-record so app
    coroutines can intersperse during the encrypt window.
  - `pendingPing`, `peerTransportParameters`, `status`,
    `handshakeComplete` are now @Volatile so observers read them
    without a lock.
  - `markClosedExternally` no longer needs any lock (status is
    @Volatile, signals are channel-thread-safe).
  - Driver's PTO bookkeeping uses the volatile fields directly — no
    lock needed.
  - Tests that manually acquired `conn.lock` to call
    `getOrCreatePeerStreamLocked` / `onTokensAcked` / `onTokensLost`
    now acquire `streamsLock` (the domain those routines mutate).
  - New `MultiplexingThroughputTest` locks in the contract: 1000
    parallel `openBidiStream` calls must complete in <2 s.

Test plan:

  - `:quic:jvmTest` — 294 tests pass (293 prior + 1 new throughput).
  - `MultiplexingThroughputTest`: 1000 bidi streams in 52 ms
    (~19,000 streams/sec on the in-memory pipe), well above the
    250+/sec target.
  - `:nestsClient:compileKotlinJvm` — clean, no API breaks.
  - `./gradlew :quic:spotlessApply` — clean.

https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT
2026-05-07 02:24:51 +00:00
Claude 57ba23519d fix(quic): batch openBidiStream under one lock hold for multiplexing
Diagnosis from yet another qlog round: streams/packet still ~1 even
with the prepareRequest/awaitResponse split. Root cause: openBidiStream
is suspend due to lock.withLock, and each call releases the lock between
iterations. The send loop is queued on the lock; it grabs it the moment
we release, drains the one stream of data we just enqueued, and the
next prepareRequest call has to re-acquire after the send loop releases.
Net: one stream per drain per packet, same useless coalescing as before.

Fix is structural:
  - QuicConnection.openBidiStreamLocked() — public, lock-not-acquired
    version of openBidiStream. Caller MUST hold conn.lock.
  - GetClient.prepareRequests(authority, paths) — batch API that
    holds conn.lock once, opens + enqueues all N streams in a single
    critical section, releases. Send loop can't interject; when it
    next drains it sees ALL N streams' data ready and packs them
    into coalesced packets.
  - Http3GetClient + HqInteropGetClient: implement prepareRequests
    using openBidiStreamLocked under conn.lock.withLock { ... }.
  - InteropClient's chunked-multiplex loop: uses prepareRequests
    (batch) instead of N x prepareRequest.

Single-stream paths still use prepareRequest / get(); behavior unchanged.

The fundamental architectural improvement (per-stream / per-level lock
split, or actor-model dispatch) is a follow-up; this commit gets us
the throughput we need from the existing single-mutex shape by holding
the lock for the full chunk's worth of work.

https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT
2026-05-07 02:06:58 +00:00
Claude 7ed3d55b31 test(quic): pin multiplexing coalescing contract
Two unit tests for the multiplexing throughput problem we just fixed
in the InteropClient (commit bc19e90c1):

1. `64 streams enqueued before drain coalesce into a small fixed
   number of packets` — opens 64 bidi streams, enqueues 50 bytes +
   FIN on each (no drain between), drains everything, asserts
   ≤6 packets and ≥10 streams/packet. Pre-fix shape (each enqueue
   followed by an immediate single-stream drain) would emit 64 packets.

2. `1000 streams enqueued in batches of 64 produce a tractable packet
   count` — stress version mirroring the runner's 1999-file
   multiplexing test. Asserts ≤150 packets total for 1000 streams.

Both tests run in <300ms on the in-memory pipe — fast iteration for
debugging the multiplexing-throughput regression cycle without
spinning up Docker.

These pin the contract that ZERO drains between enqueues = packets
batch many streams' frames. The runner-side fix (split GetClient.get
into prepareRequest + awaitResponse, batch prepareRequests serially,
wake once) ensures we hit this shape in real interop.

https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT
2026-05-07 01:59:50 +00:00
Claude 0cc577f0fb perf(quic): skip closed streams + skip sort when uniform priority
Multiplexing-throughput investigation (qlog against aioquic):
~25 streams/sec with 1453 GETs in 58s. The bottleneck under high
stream count was drainOutbound's per-call O(N log N) sort over the
ENTIRE stream list (including streams that have already FIN'd both
ways and have nothing to send).

Two cheap optimizations to drainOutbound's stream iteration:
  1. Filter to !isClosed streams BEFORE sort. Most streams under
     bursty multiplexing loads are done; iterating them is wasted.
  2. Skip sortedByDescending entirely when every stream is at
     default priority (priority == 0). The pre-priority round-robin
     shape (insertion order) is preserved, satisfying the moq-lite
     newer-sequence-stream priority contract by happenstance for
     uniform-priority loads.

Drops drainOutbound's per-call cost from O(N log N) where N = total
streams to roughly O(active) under realistic loads. Multiplexing's
~2000 streams accumulated over a run drop down to maybe 64 active at
any moment (the chunk in flight).

Doesn't affect the moq-lite audio path's behavior (small N, default
priorities → both paths reduce to the same round-robin walk).

https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT
2026-05-07 01:50:32 +00:00
Claude 17b80270d9 fix(quic): preserve Initial PN namespace across Retry (RFC 9001 §5.7)
aioquic's retry test result, surfaced via qlog:
  Check of downloaded files succeeded.
  Client reset the packet number. Check failed for PN 0

Our applyRetry called LevelState.resetForVersionNegotiation, which
creates a fresh PacketNumberSpaceState() — resetting PN to 0. The
qlog confirmed: PN=0 sent at t=388 (pre-Retry ClientHello), then PN=0
again at t=1468 (post-Retry retried ClientHello). Same PN reused
across the boundary.

RFC 9001 §5.7 + RFC 9000 §17.2.5: the Initial PN namespace CONTINUES
across the Retry boundary. The new Initial keys are derived from the
new DCID, but PN doesn't reset. Reusing a PN under different keys
makes the runner's pcap-decryption check fail (it's also a security
concern in the general case, hence the strict spec rule).

Fix: new LevelState.resetForRetry that's identical to
resetForVersionNegotiation EXCEPT it preserves pnSpace. applyRetry
calls resetForRetry. Two regression tests updated to assert the
post-Retry Initial uses PN=1 (continues from PN=0 of the pre-Retry
attempt) rather than PN=0 (the buggy reset behavior).

For Version Negotiation the original semantics still apply (RFC 9000
§6.2: client treats VN as if the original Initial was never sent;
PN reset to 0 is correct).

This should bring the retry testcase from ✕(S) to ✓(S) against
servers that exercise the Retry path. The handshake / transfer
already succeeded over the Retry per the qlog (the server's check
"Check of downloaded files succeeded." passed); only the PN-reuse
flag was failing the test.

https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT
2026-05-07 00:56:59 +00:00
Claude c0d7b6031a fix(quic): restore PTO CRYPTO retransmit lost in qlog merge
Direct evidence from aioquic interop qlog:
  packet_sent  PN=0  frames=[crypto]      (ClientHello)
  packet_sent  PN=1  frames=[ping]        (PTO probe — bare PING)
  packet_received    frames=[connection_close]
                     reason="Packet contains no CRYPTO frame"

aioquic strictly rejects pre-handshake Initials that contain no
CRYPTO frame. Our PTO probe was a bare PING, not a CRYPTO retransmit.

Agent 2's c9e036f72 implemented the right behavior: on PTO, the
driver requeues all inflight CRYPTO at the highest pre-application
level, so the next drain emits a fresh CRYPTO frame at the original
offset. The pendingPing flag stays as a fallback only used when no
CRYPTO is available to retransmit. That logic was wiped during the
qlog observer merge — driver's PTO branch only set pendingPing,
nothing requeued anything, probe was always bare PING.

Restored. Should fix the post-flush-fix regression where aioquic
went 1/7 (only transfer green) — once a packet was dropped or PTO
fired, the next probe was bare PING, aioquic closed with
"no CRYPTO frame", the test failed. Sequential tests on the same
matrix invocation likely intermittently passed/failed depending on
whether PTO fired before the response arrived.

The :quic helpers requeueAllInflightCrypto + highestPreApplicationLevel
already exist on the branch — just the driver call wasn't wired.

https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT
2026-05-07 00:48:28 +00:00
Claude 77c08ed332 fix(quic): restore QuicVersion + ctor params lost in qlog merge
Same merge-from-main shape as the prior agent A integration: the qlog
agent's worktree didn't carry the version-negotiation work (QuicVersion
import, currentVersion / vnConsumed fields, applyVersionNegotiation),
the Retry work (extraSecretsListener / cipherSuites / applyRetry), or
the version-negotiation testcase wiring. Merge with -X theirs took the
qlog version of QuicConnection.kt + QuicConnectionWriter.kt + the
existing InteropRunner.kt wholesale, dropping those.

Restored:
  - QuicVersion import in QuicConnection.kt + QuicConnectionWriter.kt +
    the test-side InteropRunner.kt (also touched by agent B's qlog
    hooks).
  - extraSecretsListener / cipherSuites / initialVersion ctor params
    on QuicConnection (qlogObserver kept; new qlog work landed).

Net result: all three overnight agents (A versionnegotiation, B qlog
observer, C peer-uni-stream drainer) now coexist on the branch with no
references missing. Full :quic:jvmTest green; :quic-interop:test +
installDist green.

https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT
2026-05-07 00:27:15 +00:00
Claude 0107bbbac9 Merge branch 'worktree-agent-a5e247c7b4025a83c' into claude/research-quic-libraries-hH1Dc 2026-05-07 00:24:08 +00:00
Claude cfc305feb3 feat(quic): add qlog observer infrastructure for interop diagnostics
Hooks every QUIC protocol decision (packets sent / received / dropped,
TLS key updates, transport params, ALPN, loss detection, PTO, close)
into a [QlogObserver] interface. Production callers default to
[QlogObserver.NoOp] (zero allocation, single virtual call); the
:quic interop runner wires a [QlogWriter] writing JSON-NDJSON
(qlog 0.3 / JSON-SEQ format) to `<QLOGDIR>/client.sqlog`, consumable
by qvis (https://qvis.quictools.info/) and Wireshark. Goal: every
interop-runner test failure produces a qlog file the operator can
drop into qvis to see exactly what we did differently from the spec.

Hooked call sites:
  - QuicConnection.start                   → connection_started, parameters_set(local), version_information
  - QuicConnection.close                   → connection_closed(local)
  - QuicConnection.markClosedExternally    → connection_closed(remote)
  - QuicConnection.applyPeerTransportParameters → parameters_set(remote)
  - QuicConnection.tlsListener (handshake/app keys) → security:key_updated
  - QuicConnection.tlsListener (handshake done)     → alpn_information
  - QuicConnectionWriter.buildLongHeaderFromFrames  → packet_sent (initial / handshake)
  - QuicConnectionWriter.buildApplicationPacket     → packet_sent (1-rtt)
  - QuicConnectionWriter.buildBestLevelPacket       → packet_sent (close-path)
  - QuicConnectionParser.feedLongHeaderPacket       → packet_received / packet_dropped
  - QuicConnectionParser.feedShortHeaderPacket      → packet_received / packet_dropped
  - QuicConnectionParser AckFrame loss-detect       → recovery:packet_lost
  - QuicConnectionDriver.sendLoop PTO branch        → recovery:loss_timer_updated (pto)
2026-05-07 00:21:42 +00:00
Claude d0bc998cd2 Merge branch 'worktree-agent-a4e96f738ceb4bbd5' into claude/research-quic-libraries-hH1Dc 2026-05-07 00:21:38 +00:00
Claude fcfd811545 fix(quic): restore Retry handling lost in versionnegotiation merge
The agent A worktree was based on main, so its QuicConnection.kt
didn't carry the Retry handling from d03e17981. Merging with
-X theirs replaced the file wholesale, dropping applyRetry +
extraSecretsListener / cipherSuites constructor params + the
RetryPacket import in the parser.

Restored:
  - extraSecretsListener + cipherSuites ctor params (used in tlsListener
    + the TlsClient construction site).
  - applyRetry method, now using the version-negotiation-introduced
    LevelState.resetForVersionNegotiation helper (functionally
    equivalent to the prior restoreFromRetry it replaced).
  - RetryPacket import in QuicConnectionParser.

Also dedupes a duplicate `originalClientHello` field that the merge
left both copies of (one from agent 3's Retry work, one from agent A's
VN work). Single field now serves both reset paths.

https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT
2026-05-07 00:21:38 +00:00
Claude aff2ee182b fix(quic): add explicit peer-uni-stream drainer to avoid H3 multiplex tear-down
Variant (B) from the three-way fix menu in the multiplexing-interop
investigation: keep `:quic` strict about per-stream backpressure (the
audit-4 #3 "INTERNAL_ERROR: stream … consumer overflowed" tear-down
stays the contract for app-data overflow on bidi streams) but expose
an explicit, opt-in helper for peer-initiated UNI streams that the
application has decided it does not need to interpret.

Root cause confirmed in QuicConnectionParser.kt:290: when the server
opens its three RFC 9114 §6.2.1 peer-uni streams (CONTROL +
QPACK_ENCODER + QPACK_DECODER) and the H3 client does not consume
them, the parser routes their bytes into each stream's bounded
incomingChannel (capacity 64). Once the QPACK encoder issues
dynamic-table inserts beyond 64 chunks the next chunk overflows
trySend, sets QuicStream.overflowed, and the parser maps that to
markClosedExternally — the entire connection dies.

Notes on scope:
  - The `Http3GetClient` and `:quic-interop` runner mentioned in the
    investigation prompt do NOT exist on the `main` worktree this
    branch starts from. The fix here is therefore `:quic`-only: the
    public `awaitIncomingPeerStream` API was already sufficient for
    an integrator to write the accept loop themselves; this commit
    wraps the common case in `drainPeerInitiatedUniStreamsIntoBlackHole`
    and updates the doc on `awaitIncomingPeerStream` so the next
    integrator landing the H3 GET client doesn't hit the same trap.
  - Variant (C) — silent default drain in `:quic` itself — was
    deliberately rejected: defaults that swallow application bytes
    are the misconfiguration we want type-system-or-API-explicit.
    The new helper requires the caller to pass a CoroutineScope, so
    opt-in is unmistakable in any callsite.

Regression test coverage in PeerUniStreamDrainTest:
  - pre_fix_no_consumer_overflows_and_tears_down_connection — pushes
    65 chunks (capacity + 1) on a SERVER_UNI stream with no consumer;
    asserts the connection transitions to CLOSED. Pins the existing
    backpressure contract.
  - drainPeerInitiatedUniStreamsIntoBlackHole_keeps_connection_alive
    — same setup but with the new helper running on a side scope;
    pushes 256 chunks (4× capacity) and asserts the connection stays
    CONNECTED. With the helper sabotaged, this test fails at
    line 119 with status=CLOSED, confirming it actually exercises
    the fix.

Full quic test suite: 295 tests, 0 failures, 0 errors.

https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT
2026-05-07 00:19:40 +00:00
Claude 04e30465d9 Merge branch 'worktree-agent-a9d8336181fce16eb' into claude/research-quic-libraries-hH1Dc 2026-05-07 00:17:49 +00:00
Claude 350387f7e0 feat(quic): handle Version Negotiation packets per RFC 9000 §6
Adds the client-side VN flow needed for the interop runner's
`versionnegotiation` testcase:

- `QuicConnection` accepts an `initialVersion` constructor parameter
  (default `QuicVersion.V1`) and exposes a mutable `currentVersion`
  the writer stamps into outbound long-headers. `start()` now caches
  the ClientHello bytes for VN-driven re-emission.
- `applyVersionNegotiation(supportedVersions)` validates per §6.2
  (anti-downgrade: reject if list contains the offered version),
  picks v1 from the offered set, regenerates DCID, re-derives
  Initial keys against the new DCID, resets the Initial level via
  `LevelState.resetForVersionNegotiation`, re-enqueues the cached
  ClientHello, and latches `vnConsumed` so a second VN is dropped.
  Failure to find a mutually supported version closes the connection
  with `QuicVersionNegotiationException`.
- `QuicConnectionParser.feedDatagram` detects `version == 0` long
  headers BEFORE peekHeader (whose layout assumes v1) and dispatches
  to a new `feedVersionNegotiationPacket` that parses the §17.2.1
  shape and validates the echoed DCID.
- `QuicConnectionWriter` reads `conn.currentVersion` instead of the
  hardcoded `QuicVersion.V1`.
- `QuicVersion.FORCE_VERSION_NEGOTIATION = 0x1a2a3a4a` for the
  interop runner.
- `InteropRunner` honors `TESTCASE=versionnegotiation` (or
  `-DinteropTestcase=`) and offers the force-VN version.

Regression coverage in `VersionNegotiationTest`:

- happy path: VN switches `currentVersion` to v1, regenerates DCID,
  resets PN, and the next drain emits a v1 Initial on the wire.
- downgrade defense: VN listing the offered version is dropped.
- unsupported list: VN whose versions we can't speak fails the
  handshake and closes the connection.
- second VN: post-consumption VN is ignored.
- DCID mismatch: spoofed VN with wrong echoed DCID is dropped.
- backward compatibility: default `initialVersion` keeps v1 behavior
  for existing callers.

https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT
2026-05-07 00:17:13 +00:00
Claude 2da5d42d70 Merge branch 'worktree-agent-a5a40cf58838c96dd' into claude/research-quic-libraries-hH1Dc 2026-05-06 23:48:39 +00:00
Claude 39f9ae2aab fix(quic): deliver FIN to per-stream Channel under concurrent multi-stream load
When QuicConnection tears down (CONNECTION_CLOSE, read-loop death,
INTERNAL_ERROR from a saturated stream channel, etc.) the per-stream
incomingChannel objects were left open, so any application coroutine
suspended on `stream.incoming.collect { … }` hung forever waiting for
a FIN that would never come. The connection-wide signal channels
(closedSignal, peerStreamSignal, incomingDatagramSignal) all closed
cleanly, but the per-stream Flows did not — surfacing in the
quic-interop-runner `multiplexing` case as 677 collectors stuck after
the connection died mid-response, so zero of the 1999 expected files
landed.

Fix: closeAllSignals() now also calls closeIncoming() on every stream
in streamsList. Channel.close() is idempotent, and consumeAsFlow drains
already-buffered chunks before honouring the close, so any bytes the
parser had already pushed are still surfaced to the collector before
the Flow terminates.

Adds MultiStreamFinDeliveryTest covering: (a) FIN delivery to N parallel
client-bidi streams, (b) connection-teardown unblocks every per-stream
Flow, (c) buffered bytes survive a teardown without an explicit FIN.
2026-05-06 23:47:28 +00:00
Claude 671f9c7050 Merge branch 'worktree-agent-ac6580a9453de5616' into claude/research-quic-libraries-hH1Dc 2026-05-06 23:07:21 +00:00
Claude fb35031b4e Merge branch 'worktree-agent-a4016e24b23c3e8ff' into claude/research-quic-libraries-hH1Dc 2026-05-06 23:07:13 +00:00
Claude 2f9e4241a6 Merge branch 'worktree-agent-a2d7586cf714834cf' into claude/research-quic-libraries-hH1Dc 2026-05-06 23:04:27 +00:00
Claude d03e179816 feat(quic): wire Retry packet handling (RFC 9000 §17.2.5 + RFC 9001 §5.8)
The Retry parser + integrity-tag verifier already existed in
RetryPacket.kt, but feedDatagram dropped Retry packets on the floor.
Hook them up:

- QuicConnectionParser.feedLongHeaderPacket detects RETRY type before
  the standard parse-and-decrypt path, parses via RetryPacket, and
  dispatches to QuicConnection.applyRetry.

- QuicConnection.start() now caches the ClientHello bytes (TLS only
  emits ClientHello once; we need to re-queue the same bytes on the
  fresh Initial keys after Retry). New applyRetry method:
  verifies the integrity tag, swaps DCID to Retry's SCID, re-derives
  Initial keys, resets the Initial PN space + sentPackets +
  cryptoSend, re-enqueues the cached ClientHello, stores the Retry
  token, and latches retryConsumed so a second Retry is dropped.

- LevelState.restoreFromRetry / PacketNumberSpaceState.resetForRetry
  give applyRetry an in-place reset (the level reference is a `val`,
  so we mirror discardKeys' field-reset pattern).

- QuicConnectionWriter.buildLongHeaderFromFrames threads
  conn.retryToken through the Initial header's Token field on every
  Initial we emit after Retry.

Per RFC 9001 §5.8, a Retry with a bad integrity tag is silently
dropped; per RFC 9000 §17.2.5.2, only one Retry is honored per
connection. Both invariants are tested.

New test: RetryHandlingTest covers the happy path (DCID swap, PN
reset, token threading, ClientHello replay, ≥1200-byte padding),
the bad-tag path, and the second-retry path.
2026-05-06 23:02:15 +00:00
Claude c9e036f728 fix(quic): PTO retransmits unacked CRYPTO at Initial/Handshake (RFC 9002 §6.2.4)
Pre-handshake PTO previously only set `pendingPing`, which collapsed to
either nothing (the bug 86b6c609a fixed in a sibling branch) or a bare
PING. Against aioquic in the quic-interop-runner ns-3 sim, the first
ClientHello can be dropped (server not ready at t≈0.5s); a follow-up
PING with the same DCID is silently ignored because the server has no
state for that DCID. We need to retransmit the actual ClientHello bytes
so the server sees a full connection attempt.

Implementation:
  - SendBuffer.requeueAllInflight(): walks the inflight list and moves
    every sent-but-not-ACK'd range to the retransmit queue, preserving
    offset and FIN. Mirrors markLost's per-range path but applies to all
    inflight ranges in one shot. Idempotent + best-effort safe.
  - QuicConnection.requeueAllInflightCrypto(level): thin wrapper that
    drives the per-level cryptoSend buffer's new method.
  - QuicConnectionDriver.sendLoop: PTO branch now calls the new helper
    at the highest active pre-application level (Handshake > Initial)
    when 1-RTT keys aren't installed. The next drain naturally emits a
    CRYPTO frame at the original offset (takeChunk drains the
    retransmit queue first per existing semantics).
  - QuicConnectionWriter.collectHandshakeLevelFrames: honors pendingPing
    pre-handshake at the highest active level — but skips the PING when
    a CRYPTO frame is already in the same level's frame list (the
    CRYPTO retransmit covers the ack-eliciting requirement). Bare PING
    still goes out when there's nothing to retransmit.

Old RecoveryToken.Crypto entries in sentPackets for the original PNs
remain harmless: when loss detection eventually declares them lost,
markLost re-runs against ranges that have already moved on, which is
itself idempotent (clamps to flushedFloor / no-op on already-queued).

Test: PtoCryptoRetransmitTest reproduces the wire scenario — first drain
emits ClientHello in a ≥1200-byte Initial datagram; simulated PTO
calls requeueAllInflightCrypto + sets pendingPing; second drain must
contain a CRYPTO frame at the same offset with the same payload bytes,
not a bare PING. Datagram size still ≥1200 (RFC 9000 §14.1).

https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT
2026-05-06 23:01:08 +00:00
Claude 9c86eee5e2 fix(quic): pad PING-only Initial datagrams to strict 1200 bytes (RFC 9000 §14.1)
The padding-rebuild branch in QuicConnectionWriter.drainOutbound computed
`padBytes = 1200 - natural`, but the QUIC long-header Length field is a
varint (RFC 9000 §16). When the natural-size payload was small enough for
Length to fit in 1 byte (body ≤ 63 bytes), the rebuild's larger body
crossed the 64-byte threshold and Length grew to 2 bytes — adding 1 wire
byte that wasn't in `natural`. PING-only PTO probe Initials therefore went
out at exactly 1199 bytes, one short of the §14.1 floor.

Fix: rebuild iteratively. After the first rebuild, measure the actual
datagram size; if still < 1200, bump padBytes by the residual and rebuild
once more. PADDING bytes inside the AEAD envelope add 1:1 to the wire
size and the Length varint grows monotonically, so the loop terminates
in ≤ 2 iterations for any reachable payload.

Same fix is applied to buildClosingDatagram so close-only Initial probes
on the boundary aren't tripped by future varint-growth changes.

Tightens the existing PTO-probe regression test to assert ≥ 1200 (was
relaxed to ≥ 1199 in 86b6c609a) and adds a new boundary test that builds
a single-byte-payload Initial and checks 1200 ≤ size ≤ 1203 — strict
floor with a tight ceiling so over-correction would also fail.

https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT
2026-05-06 23:00:30 +00:00
Claude 86b6c609a6 fix(quic): emit PING at Initial/Handshake on PTO pre-handshake (RFC 9002 §6.2.4)
The aioquic interop run revealed bug #3 (after the close-padding and
close-frame-type fixes): when PTO fires before the handshake completes,
the driver sets `pendingPing = true` but the writer only consumed that
flag in the 1-RTT path. Pre-handshake the flag was silently discarded,
so the second drain produced no Initial datagram — the connection sat
mute through every subsequent PTO. Symptom on the wire: exactly one
Initial packet (the close at PN=1, after our internal handshake
timeout), zero retransmits across the full 10-second budget, no chance
for the peer to recover from a dropped first ClientHello.

Fix routes pendingPing through to whatever encryption level is the
highest currently active — preferring 1-RTT, falling through Handshake,
finally Initial. Adds a regression test that drains a fresh connection
with `pendingPing = true` and verifies an Initial-level padded probe
datagram comes out (vs. null pre-fix).

Test relaxes the size assertion to ≥ 1199 due to a separate pre-existing
off-by-one in the writer's padding deficit calculation when the natural
payload uses a 1-byte Length varint that grows to 2 after padding —
that's a follow-up; the regression we care about here is "no probe at
all," not the byte-precise padding edge.

Outstanding from this run:
  - Strict ≥ 1200 padding for tiny payloads (PING-only Initial = 1199)
  - PTO should retransmit unacked CRYPTO bytes, not just emit a PING
    (current PING gets ACK + relies on packet-number-threshold loss
    detection to trigger CRYPTO retransmit; works but suboptimal)

https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT
2026-05-06 22:50:02 +00:00
Claude f0100b3ca0 Merge remote-tracking branch 'origin/main' into claude/research-quic-libraries-hH1Dc 2026-05-06 22:38:20 +00:00
Claude 59438d5283 fix(quic): RFC 9000 §10.2.3 + §14.1 in CONNECTION_CLOSE-only datagrams
The quic-interop-runner against aioquic surfaced two real bugs in the
writer's CLOSING-status branch, both visible on the wire as a single
~45-byte UDP datagram instead of a properly framed close.

§10.2.3 — at Initial / Handshake levels only CONNECTION_CLOSE (Transport,
0x1c) is allowed. The application-level form (0x1d) leaks app state
pre-handshake. The writer was unconditionally building a 0x1d frame and
shipping it inside an Initial packet; aioquic dropped it silently.

§14.1 — any client datagram containing an Initial MUST be ≥ 1200 bytes
in UDP-payload terms. The CLOSING branch bypassed the existing padding
logic, so close-only Initial datagrams went out at ~45 bytes and
servers correctly rejected them (also as malformed).

Fix replaces the CLOSING branch with a dedicated `buildClosingDatagram`
helper:
  - Application keys present → 0x1d, original error code + reason.
  - Pre-1-RTT (Handshake or Initial) → 0x1c with errorCode =
    APPLICATION_ERROR (0x0c), frameType=0, empty reason.
  - Initial level: build at natural size, rewind PN if < 1200, rebuild
    with PADDING-frame deficit inside the AEAD envelope.

Plus a regression test covering both: pre-handshake close datagram size
≥ 1200, and ConnectionCloseFrame round-trips 0x1c vs 0x1d for the right
constructor inputs.

NOT addressed yet: why our ClientHello at PN=0 doesn't appear in the
runner pcap (only PN=1 close does). With this fix the close packet is
now well-formed; the next runner run will tell us whether the missing
ClientHello is a sim/capture artifact or a separate writer bug.

https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT
2026-05-06 22:37:55 +00:00
Claude afe11ac651 feat(quic-interop): wire SSLKEYLOGFILE + add chacha20 testcase (Phase 1a)
Adds two debugging hooks to :quic so the interop endpoint can produce
artifacts that make the runner actually useful for finding bugs:

  - TlsClient.clientRandom: capture and expose the 32-byte ClientHello
    random so a SSLKEYLOG line can correlate to this connection.
  - QuicConnection.extraSecretsListener: optional chained secrets
    listener (default null, no-op for production callers). Fires
    alongside the connection's own key-installation listener at every
    encryption-level transition.
  - QuicConnection.cipherSuites: knob to override the offered TLS
    cipher suites in ClientHello.

InteropClient now:
  - Writes NSS Key Log Format lines to $SSLKEYLOGFILE when set, so
    Wireshark can decrypt the sim's pcap captures.
  - Implements the `chacha20` testcase by offering only
    TLS_CHACHA20_POLY1305_SHA256 — exercises the ChaCha20 AEAD path
    end-to-end against a peer.

Defers QLOGDIR (needs qlog observer infrastructure across packet/
frame/recovery layers — own design doc) and `versionnegotiation`
(needs the writer to accept a configurable initial QUIC version).

https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT
2026-05-06 21:32:44 +00:00
Claude 72295915de fix(quic): tier-local round-robin so priority survives every drain
The previous priority-then-round-robin shape applied the rotating
start-index globally over the sorted list, so the cross-tier order
flipped on alternate drains: drain N had high-priority first, drain
N+1 advanced streamRoundRobinStart and had low-priority first. The
priority hint was silently defeated under any sustained traffic.

Replace it with strict priority across tiers + round-robin only
within each same-priority tier. Higher tiers always drain ahead of
lower ones; same-priority peers continue to take turns via the
existing rotating start. Default-priority callers see no behaviour
change (single tier, identical rotation semantics).

Tighten the test: drain twice and assert the higher-priority stream
emits first on BOTH drains — the regression case that the single-
drain version of the test missed. Add a regression guard for the
same-priority round-robin so a future refactor can't silently
serialise on the first stream.

https://claude.ai/code/session_01KWdr4RjVvyYZfEuPVaQfUa
2026-05-06 20:34:39 +00:00
Claude f1034b1f53 feat(quic): priority-aware stream scheduling for moq-lite groups
Bias the QUIC connection writer's drain loop toward higher-priority
streams so moq-lite group streams with newer (higher) sequence numbers
drain ahead of older ones under congestion. Implements the T11.3
follow-up flagged in nestsClient/plans/2026-05-06-stream-priority-
followup.md (now removed).

QuicStream gets a `@Volatile var priority: Int = 0`. The writer's
streamsView iteration is replaced by a stable sortedByDescending pass
so same-priority streams keep their existing rotating-start round
robin while higher-priority tiers always drain first.

WebTransportWriteStream gains a `setPriority(Int)` hook; the QUIC-
backed adapter forwards to the underlying QuicStream, while the
in-memory test fakes treat it as a no-op. MoqLiteSession.openGroupStream
calls `uni.setPriority(sequence)` (saturating to Int.MAX_VALUE) to
mirror moq-rs's `Publisher::serve_group`.

Tests: a new InMemoryQuicPipe.decryptClientApplicationFrames helper
walks past coalesced long-header packets to surface 1-RTT frames,
which lets QuicConnectionWriterTest assert that the higher-priority
stream's StreamFrame lands first inside a single drained packet.

https://claude.ai/code/session_01KWdr4RjVvyYZfEuPVaQfUa
2026-05-06 20:25:01 +00:00
Claude edd6eb5c10 fix(quic): pad short plaintext payloads for HP sample (RFC 9001 §5.4.2)
ShortHeaderPacket.build / LongHeaderPacket.build crashed with
`IllegalArgumentException: packet too short for HP sample` whenever the
plaintext payload was small enough that pnLen + payload < 4 — most
visibly on the 1-RTT path when buildApplicationPacket emitted a single
1-byte PING (PTO probe with no ACKs queued and no streams to drain) and
the packet number still fit in 1 byte. The crash tore down the writer
loop, which surfaced upstream as moq-lite "subscribe stream FIN before
reply" because in-flight bidi streams got FIN'd by the peer.

RFC 9001 §5.4.2 mandates the sender pad the plaintext so the encrypted
output (plaintext + 16-byte AEAD tag) has at least 20 bytes after the
packet-number offset for the 16-byte HP sample. The fix pads the
plaintext with trailing 0x00 bytes — those decode as PADDING frames per
RFC 9000 §19.1 and decodeFrames already absorbs them. For long-header
packets the padded size feeds back into the Length varint.
2026-05-05 13:37:41 +00:00