Harden the retry classifier and pin per-attempt deadlines (closes #80)

A POST or PUT is replayed only when every errno in the cause chain is a
connect errno and the walk reached the end of the chain, and
postJSON/putJSON no longer follow redirects, so a redirect is an
ApiError that is not retried. getRetryOptions() returns a copy. New
tests pin every errno the classifier names, the cause-chain depth
limit, a chain deeper than the limit, a two-error cycle, and a fresh
deadline per attempt for every retrying entry point. The README
endpoint list is now the one place naming the requests the replay rule
covers; code comments point to it.

Model: opus-5-5

src/api/client.ts

@@ -146,8 +146,9 @@ export class ApiClient {
     // The policy this client was configured with, so that a caller wrapping a
     // whole operation in its own `withRetry` — the download layer — runs under
     // the same settings rather than under the library defaults.
+    // A copy, so the caller cannot change this client's settings through it.
     getRetryOptions(): ResolvedRetryOptions {
-        return this.retry;
+        return { ...this.retry };
     }
 
     private headers(extra?: Record<string, string>): Record<string, string> {
@@ -228,15 +229,15 @@ export class ApiClient {
 
     async postJSON<T>(path: string, body: unknown): Promise<T> {
         const url = `${this.apiOrigin}${path}`;
-        // Idempotency: this reaches `/users/srp/create-session`,
-        // `/users/two-factor/verify` and `/users/ott`, all of which change
-        // server state — verifying a second factor consumes one of a small
-        // number of attempts. So a POST is replayed only on a failure that
-        // establishes no TCP connection to the server ever existed: DNS
-        // produced no address, or the peer refused the connection. A 5xx, a
-        // mid-flight reset, a routing errno (which Linux also delivers on an
-        // established socket) and a timeout are all left to the caller,
-        // because each of them can occur after the server has already acted.
+        // Not idempotent: a POST is replayed only when `isSafeToReplay`
+        // says no request byte can have reached the server. The endpoints
+        // this covers are listed in the README under "Endpoints used".
+        //
+        // Redirects are not followed. The origin has already received the
+        // request when it answers with one, so a connection refused by the
+        // redirect target would look replay-safe when it is not. The API has
+        // no legitimate redirect, so one surfaces as an `ApiError` with its
+        // 3xx status, which is not retried.
         return withRetry(
             async () => {
                 const resp = await this._fetch(url, {
@@ -245,6 +246,7 @@ export class ApiClient {
                         "Content-Type": "application/json",
                     }),
                     body: JSON.stringify(body),
+                    redirect: "manual",
                     signal: AbortSignal.timeout(this.requestTimeoutMs),
                 });
                 await this.throwIfError(resp);
@@ -303,9 +305,7 @@ export class ApiClient {
 
     async putJSON<T>(path: string, body: unknown): Promise<T> {
         const url = `${this.apiOrigin}${path}`;
-        // Same idempotency rule as `postJSON`, for the same reason: this
-        // reaches `/files/thumbnail`, which registers an uploaded thumbnail
-        // against a file.
+        // Same replay and redirect rules as `postJSON`, for the same reasons.
         return withRetry(
             async () => {
                 const resp = await this._fetch(url, {
@@ -314,6 +314,7 @@ export class ApiClient {
                         "Content-Type": "application/json",
                     }),
                     body: JSON.stringify(body),
+                    redirect: "manual",
                     signal: AbortSignal.timeout(this.requestTimeoutMs),
                 });
                 await this.throwIfError(resp);

src/retry.ts

@@ -88,18 +88,21 @@ const MAX_CAUSE_DEPTH = 8;
 // errno on the error it throws — it hangs the underlying socket error off
 // `cause`, sometimes more than one level down — so a classifier that only read
 // the top-level error would see a bare `Error` and call every dropped
-// connection permanent.
-const causeCodes = (err: unknown): string[] => {
+// connection permanent. `complete` is false when the walk stopped at the
+// depth limit with more of the chain still below it.
+const causeCodes = (err: unknown): { codes: string[]; complete: boolean } => {
     const codes: string[] = [];
     let current: unknown = err;
     for (let depth = 0; depth < MAX_CAUSE_DEPTH; depth++) {
-        if (current === null || typeof current !== "object") break;
+        if (current === null || typeof current !== "object") {
+            return { codes, complete: true };
+        }
         const { code, cause } = current as { code?: unknown; cause?: unknown };
         if (typeof code === "string") codes.push(code);
-        if (cause === current) break;
+        if (cause === current) return { codes, complete: true };
         current = cause;
     }
-    return codes;
+    return { codes, complete: current === null || typeof current !== "object" };
 };
 
 const isAbort = (err: unknown): boolean => {
@@ -145,15 +148,15 @@ export const isRetryable = (err: unknown): boolean => {
     // have succeeded; the cost of the imprecision is bounded by the attempt
     // count.
     if (err instanceof TypeError) return true;
-    return causeCodes(err).some((code) => TRANSPORT_CODES.has(code));
+    return causeCodes(err).codes.some((code) => TRANSPORT_CODES.has(code));
 };
 
 // Could the first attempt already have taken effect on the server?
 //
 // `isRetryable` is the wrong question for a request that changes state.
-// quak's non-idempotent calls are `/users/srp/create-session`,
-// `/users/two-factor/verify` — which consumes one of a small number of 2FA
-// attempts — and `/files/thumbnail`. They are replayed only on the failures in
+// `postJSON` and `putJSON` use this for every `POST` and `PUT` listed in the
+// README under "Endpoints used"; verifying a second factor, for one, consumes
+// one of a small number of attempts. They are replayed only on the failures in
 // `CONNECT_CODES`, which establish that no TCP connection to the server ever
 // existed: there was no address to connect to, or the peer refused the
 // connection outright. A request byte cannot have been transmitted, so the
@@ -162,9 +165,19 @@ export const isRetryable = (err: unknown): boolean => {
 // Everything else is ambiguous. A 5xx proves the server did process the
 // request. A reset or a broken pipe can arrive after it was fully sent and
 // acted on. A routing errno can be delivered on an established socket. A
-// deadline says nothing at all about the server's state.
-export const isSafeToReplay = (err: unknown): boolean =>
-    isRetryable(err) && causeCodes(err).some((code) => CONNECT_CODES.has(code));
+// deadline says nothing at all about the server's state. So every errno in the
+// cause chain must be a connect errno: one other errno anywhere in the chain
+// is doubt, and doubt is not replayed. A chain longer than the walk is doubt
+// too: the links below the limit were never read.
+export const isSafeToReplay = (err: unknown): boolean => {
+    const { codes, complete } = causeCodes(err);
+    return (
+        isRetryable(err) &&
+        complete &&
+        codes.length > 0 &&
+        codes.every((code) => CONNECT_CODES.has(code))
+    );
+};
 
 export interface WithRetryOptions extends RetryOptions {
     isRetryable?: (err: unknown) => boolean;