Correct misleading OIDC docs and mislabeled test

Address review findings where documentation and one test described
behavior the code does not implement. Comments/test-name only; no
runtime behavior changes.

- QwpWebSocketSender: three comments claimed a token-provider failure is
  retried within the reconnect budget and then terminates the sender. The
  actual behavior is the opposite - the foreground/SYNC connect fails fast
  with the provider's exception, while the running background drainer
  retries indefinitely (never budget-bounded, never terminal) per
  store-and-forward Invariant B. Rewrite all three to match.

- OidcDeviceAuth.getToken javadoc said the store per-identity lock wait is
  "a few seconds at most, then proceeds without the lock." That bound is
  the cross-process file lock; the in-process lock guarding two same-JVM
  instances of one identity is not time-bounded and can wait out the
  peer's whole refresh. Clarify the distinction.

- AbstractLineHttpSender: reword the drain comment - the per-flush budget
  bounds each recv() read, not the whole body cumulatively (fine here
  because the ILP server is trusted).

- FileTokenStoreTest: rename
  testConcurrentStealContentionTwoWayPreservesMutualExclusion to
  testSameProcessContendersSerializeAndBothStealStaleLock. Its
  overlaps==0/maxInside==1 assertions are guaranteed by the in-process
  PROCESS_LOCKS lock, not the file-lock capture-verify the old name
  claimed, so the cross-process exclusion is masked in a single JVM.
  Re-comment to document that the cross-process property is
  inspection-verified.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: glasstiger

core/src/main/java/io/questdb/client/cutlass/auth/OidcDeviceAuth.java

@@ -495,8 +495,12 @@ public String getAuthorizationHeaderValue() {
      * {@link Builder#httpTimeoutMillis(int)} and still failing fast the moment an interactive sign-in or
      * {@link #close()} begins meanwhile. It is not, otherwise, instantaneous - when the cached
      * token has expired it makes one synchronous refresh round-trip to the token endpoint (and, with a
-     * coordinating {@link TokenStore}, may first wait briefly to acquire the store's per-identity lock - a few
-     * seconds at most for {@link FileTokenStore}, then it proceeds without the lock - before that round-trip).
+     * coordinating {@link TokenStore}, may first wait to acquire the store's per-identity lock before that
+     * round-trip). For {@link FileTokenStore} the CROSS-process file lock is bounded to a few seconds and then
+     * proceeds without it; but the IN-process lock that serializes two instances sharing one identity in the
+     * same JVM (an ILP {@code Sender} and a {@code QwpQueryClient}, say) is not time-bounded, so such a
+     * concurrent caller instead waits out the peer's whole refresh - itself bounded only by the OS connect
+     * stall described next, not by a few seconds.
      * The send, response wait and body parse of that round-trip are each bounded by
      * {@link Builder#httpTimeoutMillis(int)} (30s by default); the connection phase that precedes them - DNS
      * resolution, the TCP connect and the TLS handshake - is bounded by the OS, not by httpTimeoutMillis, so an

core/src/main/java/io/questdb/client/cutlass/line/http/AbstractLineHttpSender.java

@@ -715,9 +715,12 @@ private void flush0(boolean closing) {
                 response.await(remainingMillis);
                 DirectUtf8Sequence statusCode = response.getStatusCode();
                 if (isSuccessResponse(statusCode)) {
-                    // bound the body drain by the whole per-flush budget (base + throughput extension), NOT the
-                    // raw request_timeout: recv() otherwise inherits defaultTimeout, so a tuned-low request_timeout
-                    // paired with request_min_throughput would abort a large, still-progressing chunked body
+                    // pass the whole per-flush budget (base + throughput extension) as EACH recv() read's
+                    // timeout, NOT the raw request_timeout: recv() otherwise inherits defaultTimeout, so a
+                    // tuned-low request_timeout paired with request_min_throughput would abort a large,
+                    // still-progressing chunked body. This bounds each read, not the whole body cumulatively -
+                    // fine here because the ILP server is trusted (unlike OidcDeviceAuth.parseBody, which also
+                    // caps total bytes and wall-clock time against an untrusted identity provider).
                     consumeChunkedResponse(response, actualTimeoutMillis); // if any
                     if (keepAliveDisabled(response)) {
                         // Server has HTTP keep-alive disabled, and it's closing this TCP connection.

core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java

@@ -145,8 +145,10 @@ public class QwpWebSocketSender implements Sender {
     // fixed token or Basic credential; for an httpTokenProvider it pulls a freshly refreshed token,
     // so the initial connect and every reconnect re-handshake carry the current token. May be null
     // when no auth is configured. Evaluated once per (re)connect round in buildAndConnect, before the
-    // endpoint walk (not once per endpoint); a throwing provider propagates to the connectWithRetry
-    // reconnect wrapper, which retries it within the reconnect budget and surfaces the provider's message.
+    // endpoint walk (not once per endpoint); a throwing provider is wrapped as
+    // QwpCredentialUnavailableException: the foreground/SYNC initial connect fails fast with the provider's
+    // own exception, while the running background drainer treats it as a transient outage and retries it
+    // indefinitely (never bounded by the reconnect budget, never terminal) per store-and-forward Invariant B.
     private final Supplier<String> authorizationHeaderSupplier;
     private final int autoFlushBytes;
     private final long autoFlushIntervalNanos;
@@ -2811,20 +2813,23 @@ private WebSocketClient connectWalk(ReconnectSupplier ctx) {
         // javadoc documents - NOT once per endpoint: a token-provider failure is cluster-wide (a failed silent
         // refresh, or not signed in), not a per-endpoint transport fault, so re-querying it per endpoint would
         // hammer the token endpoint with the same dead credential and mislabel the failure as "all endpoints
-        // unreachable". A throw here propagates to the connectWithRetry reconnect wrapper, which retries it
-        // within the reconnect budget like any other connect failure and surfaces the provider's own message,
-        // so a transient failed refresh recovers and only a persistent one terminates the sender (this
-        // transport's documented reconnect model). Mirrors QwpQueryClient, which likewise resolves the
-        // credential once before its endpoint walk.
+        // unreachable". A throw here is wrapped as QwpCredentialUnavailableException (below): the
+        // foreground/SYNC initial connect unwraps it and fails fast with the provider's own message, while the
+        // running background drainer treats it as a transient outage and retries it indefinitely with capped
+        // backoff (never bounded by the reconnect budget, never terminal), so even a persistent credential
+        // outage keeps the buffered rows in store-and-forward rather than terminating the sender (Invariant B).
+        // Mirrors QwpQueryClient, which likewise resolves the credential once before its endpoint walk.
         final String authHeader;
         try {
             authHeader = authorizationHeaderSupplier == null ? null : authorizationHeaderSupplier.get();
         } catch (RuntimeException e) {
-            // Tag the failure CLASS before it reaches a retry loop: a credential we cannot acquire is not a
-            // transport outage, so the background reconnect loop must not retry it forever under Invariant B
-            // -- it bounds this class by the reconnect budget and then terminates with the provider's message.
-            // A foreground connect unwraps this and rethrows the provider's own exception, so build() still
-            // surfaces the provider's error directly rather than an internal wrapper.
+            // Tag the failure CLASS so each context applies the right policy: a credential we cannot acquire is
+            // not a transport outage. A foreground/SYNC connect unwraps this and rethrows the provider's own
+            // exception, so build() surfaces the provider's error directly rather than an internal wrapper. The
+            // running background drainer, by contrast, treats it as a transient outage and retries it
+            // indefinitely under Invariant B -- never bounding it by the reconnect budget, never latching a
+            // terminal -- so a recoverable credential outage never drops a producer store-and-forward promised
+            // to keep alive.
             throw new QwpCredentialUnavailableException(e);
         }
         while (true) {

core/src/test/java/io/questdb/client/test/cutlass/auth/FileTokenStoreTest.java

@@ -206,8 +206,8 @@ public void testConcurrentStealContentionDegradesCleanly() throws Exception {
             // which two holders can briefly run concurrently. That residual needs three or more contenders and
             // degrades only to one extra token refresh (a re-prompt on a rotating-refresh-token IdP), never a
             // torn or forged credential, since the Layer-1 atomic-rename write is independent of the lock.
-            // testConcurrentStealContentionTwoWayPreservesMutualExclusion pins the exclusion the atomic capture
-            // does guarantee, deterministically, with two contenders.
+            // testSameProcessContendersSerializeAndBothStealStaleLock covers the two-contender same-JVM case
+            // (where PROCESS_LOCKS, not the file-lock capture, provides the exclusion it asserts).
             final int threads = 4;
             AtomicInteger ran = new AtomicInteger();
             TokenStore.CriticalSection section = () -> {
@@ -235,7 +235,7 @@ public void testConcurrentStealContentionDegradesCleanly() throws Exception {
     }
 
     @Test
-    public void testConcurrentStealContentionTwoWayPreservesMutualExclusion() throws Exception {
+    public void testSameProcessContendersSerializeAndBothStealStaleLock() throws Exception {
         assertMemoryLeak(() -> {
             Path dir = storeDir();
             Files.createDirectories(dir);
@@ -245,15 +245,16 @@ public void testConcurrentStealContentionTwoWayPreservesMutualExclusion() throws
             Files.write(lock, "crashed-holder-stamp".getBytes(StandardCharsets.UTF_8));
             Files.setLastModifiedTime(lock, FileTime.fromMillis(System.currentTimeMillis() - 600_000));
 
-            // TWO processes race to steal the one abandoned lock. With exactly two contenders the steal is
-            // deterministically mutually exclusive: the atomic capture (rename) lets exactly one steal the
-            // abandoned lock, and once a winner holds a freshly-stamped lock the loser reads that live stamp,
-            // judges it not stale (the 60s window far exceeds each ~100ms hold) and waits rather than stealing
-            // it; the empty-lock grace likewise stops the loser stealing the winner's lock in its brief
-            // create->stamp gap. The three-actor residual stealIfStale documents - a peer recreating the lock
-            // while a second captures it AND a third claims the freed path - structurally cannot arise with two
-            // threads, so exclusion holds exactly here (the N-way best-effort path is
-            // testConcurrentStealContentionDegradesCleanly).
+            // Two threads of the SAME JVM contend for the one abandoned lock. SCOPE NOTE: the mutual exclusion
+            // asserted below (overlaps==0, maxInside==1) is provided by the in-process PROCESS_LOCKS
+            // ReentrantLock, which inLock() takes on key.hash() BEFORE any file-lock logic - so it would hold
+            // even if the file-lock steal were broken. What this test genuinely proves is that two same-process
+            // contenders each steal the stale lock and run their critical section (ran==2), serialized, without
+            // leaving an orphaned capture temp (assertNoCaptureTempFiles). The CROSS-process capture-verify in
+            // stealIfStale - that among separate OS PROCESSES exactly one steal wins - is masked by PROCESS_LOCKS
+            // here and cannot be exercised in a single JVM; it is verified by inspection, and a two-holder
+            // outcome is a documented best-effort residual anyway. The N-way degrade path is
+            // testConcurrentStealContentionDegradesCleanly.
             final int threads = 2;
             AtomicInteger inside = new AtomicInteger();
             AtomicInteger maxInside = new AtomicInteger();
@@ -286,7 +287,7 @@ public void testConcurrentStealContentionTwoWayPreservesMutualExclusion() throws
             }
 
             Assert.assertEquals("every contender must run its critical section", threads, ran.get());
-            Assert.assertEquals("the steal must never admit two holders at once", 0, overlaps.get());
+            Assert.assertEquals("same-process contenders must never overlap (PROCESS_LOCKS serializes them)", 0, overlaps.get());
             Assert.assertEquals("at most one holder at a time", 1, maxInside.get());
             assertNoCaptureTempFiles(dir, key);
         });