fix: address review — empty-string timing fields, IPv6 comment, suspicious-batch logs

5 items + 2 coverage gaps from the latest cr round.

### Concerns

1. **Empty-string handling on timing fields.** `update({ extend_by: "" })`
   and `mintLink({ expires_in: "" })` previously round-tripped to the API
   as `{"extend_by": ""}`, hitting either the mutual-exclusion check
   (because `"" !== undefined`) or the server's duration parser. Both
   write surfaces now reject empty-string timing fields with a structured
   `ValidationError` — `description: ""` keeps its documented
   clear-the-field semantic. Three new tests: rejection on each surface
   plus a positive test locking in `description: ""` round-trips.

2. **Conflicting IPv6 hostname comment.** Tightened the outer comment
   that said `URL.hostname` strips IPv6 brackets — the inner comment
   correctly notes Node returns hosts WITH brackets, and the code
   handles both forms. Outer comment now points at the inner block
   instead of contradicting it.

4. **`request_id` shadowing debug log.** Added a both-fields-present
   debug log in `batchCreate` mirroring `mapQurlsField`'s pattern.
   If the server ever puts `request_id` on both `data` and `meta`,
   operators see the divergence rather than learning about it via
   support-ticket archaeology.

5. **Empty BatchCreateOutput debug log.** Added a debug log in
   `validateBatchCreateResponse` for the `succeeded: 0, failed: 0,
   results: []` case — shape-valid but suspicious (the SDK definitely
   sent items). Keeps the response valid; surfaces the anomaly.

### Coverage

- **list-side `mapQurlsField` mapping.** Test asserts list items
  containing wire-format `qurls` map to `access_tokens` consistently.
  Locks in the defensive `data.map(mapQurlsField)` in `list()`.

### No action

- **#3 RFC 3339 client-side parse for `expires_at`.** Reviewer marked
  optional ("if you ever want a tiny sanity check"). Keeping the
  duration-parser stance: server is authoritative.
- **#6 `requireValidTags` empty-array semantics.** Reviewer noted as
  intentional and well-supported by existing test.
- **#7 `Number.isFinite` for `maxRetries` null-coverage.** Reviewer
  confirmed the `?? DEFAULT_MAX_RETRIES` chain already handles null.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

Author: justin-layerv

src/client.test.ts

@@ -439,6 +439,49 @@ describe("QURLClient", () => {
     expect(result.has_more).toBe(false);
   });
 
+  it("list items with qurls field map to access_tokens (defensive)", async () => {
+    // The list response is documented as a flat list of QURL resources
+    // with no nested tokens, but client.list() defensively threads each
+    // item through mapQurlsField in case the API ever surfaces nested
+    // tokens on list items (e.g. embedded summaries). Locks in that
+    // the rename happens consistently on the list path too.
+    const fetch = mockFetch({
+      status: 200,
+      body: {
+        data: [
+          {
+            resource_id: "r_with_tokens",
+            target_url: "https://example.com",
+            status: "active",
+            created_at: "2026-03-10T10:00:00Z",
+            qurls: [
+              {
+                qurl_id: "at_xyz",
+                status: "active",
+                one_time_use: false,
+                max_sessions: 5,
+                session_duration: 3600,
+                use_count: 0,
+                created_at: "2026-03-10T10:00:00Z",
+                expires_at: "2026-04-10T10:00:00Z",
+              },
+            ],
+          },
+        ],
+        meta: { has_more: false },
+      },
+    });
+
+    const client = createClient(fetch);
+    const result = await client.list();
+
+    expect(result.qurls).toHaveLength(1);
+    expect(result.qurls[0].access_tokens).toHaveLength(1);
+    expect(result.qurls[0].access_tokens![0].qurl_id).toBe("at_xyz");
+    // Wire-format key stripped from each item.
+    expect((result.qurls[0] as unknown as { qurls?: unknown }).qurls).toBeUndefined();
+  });
+
   it("list rejects limit: 0 below the spec's minimum", async () => {
     // Per OpenAPI (`GET /v1/qurls` → `limit: minimum: 1, maximum: 100`),
     // `limit` must be in [1, 100]. Client-side validation catches this
@@ -1168,6 +1211,69 @@ describe("QURLClient", () => {
     expect(fetch).not.toHaveBeenCalled();
   });
 
+  it("update rejects empty-string timing fields (extend_by / expires_at)", async () => {
+    // Empty string is always a caller mistake on timing fields —
+    // typically uninitialized form state. `description: ""` has
+    // documented clear-the-field semantics (see UpdateInput JSDoc)
+    // and is preserved; the timing fields don't, so they get a
+    // structured ValidationError instead of round-tripping garbage.
+    const fetch = mockFetch({ status: 200, body: { data: {} } });
+    const client = createClient(fetch);
+
+    for (const key of ["extend_by", "expires_at"] as const) {
+      const error = await client
+        .update("r_abc", { [key]: "" } as unknown as Parameters<typeof client.update>[1])
+        .catch((e: unknown) => e as ValidationError);
+      expect(error).toBeInstanceOf(ValidationError);
+      const detail = (error as ValidationError).detail;
+      expect(detail).toContain(key);
+      expect(detail).toContain("must not be an empty string");
+    }
+    expect(fetch).not.toHaveBeenCalled();
+  });
+
+  it("update preserves description: '' (documented clear-the-field semantic)", async () => {
+    // Mirror image of the timing-field rejection above: `description: ""`
+    // is a documented way to clear the resource's description and must
+    // round-trip to the API as-is. Locks in that the empty-string
+    // rejection is timing-field-only.
+    const fetch = mockFetch({
+      status: 200,
+      body: {
+        data: {
+          resource_id: "r_abc",
+          target_url: "https://example.com",
+          status: "active",
+          created_at: "2026-03-10T10:00:00Z",
+        },
+      },
+    });
+    const client = createClient(fetch);
+    await client.update("r_abc", { description: "" });
+
+    expect(fetch).toHaveBeenCalledTimes(1);
+    const body = JSON.parse((fetch as ReturnType<typeof vi.fn>).mock.calls[0][1].body as string);
+    expect(body.description).toBe("");
+  });
+
+  it("mintLink rejects empty-string timing fields (expires_in / expires_at)", async () => {
+    // Symmetric with update() — empty string on timing fields is
+    // always a caller mistake, never a meaningful value.
+    const fetch = mockFetch({ status: 200, body: { data: {} } });
+    const client = createClient(fetch);
+
+    for (const key of ["expires_in", "expires_at"] as const) {
+      const error = await client
+        .mintLink("r_abc", { [key]: "" } as unknown as Parameters<typeof client.mintLink>[1])
+        .catch((e: unknown) => e as ValidationError);
+      expect(error).toBeInstanceOf(ValidationError);
+      const detail = (error as ValidationError).detail;
+      expect(detail).toContain(key);
+      expect(detail).toContain("must not be an empty string");
+    }
+    expect(fetch).not.toHaveBeenCalled();
+  });
+
   it("update treats `{ extend_by: undefined }` as empty (no stealth empty-input)", async () => {
     // A caller who threads an `undefined` through their own types
     // should still hit the empty-input guard — the some()-based check

src/client.ts

@@ -448,9 +448,9 @@ export class QURLClient {
         `baseUrl: must be a valid URL (got ${JSON.stringify(rawBaseUrl).slice(0, 60)})`,
       );
     }
-    // `URL.hostname` strips IPv6 brackets, so the canonical loopback
-    // forms compared here are bracket-free. Reject any non-http/https
-    // scheme outright (ftp://, file://, javascript:, ...).
+    // Reject any non-http/https scheme outright (ftp://, file://,
+    // javascript:, ...). The loopback exemption logic below handles
+    // IPv6 bracket form on `URL.hostname` — see the inner comment.
     if (parsedBaseUrl.protocol !== "http:" && parsedBaseUrl.protocol !== "https:") {
       throw clientValidationError(
         `baseUrl: must use http:// or https:// scheme (got ${JSON.stringify(rawBaseUrl).slice(0, 60)})`,
@@ -635,6 +635,18 @@ export class QURLClient {
       }
     }
 
+    // Suspicious-but-shape-valid: empty results AND zero counts on a
+    // batch the SDK definitely sent items for. Doesn't fail the guard
+    // (the contract permits it) — surface via debug so operators see
+    // it if it ever shows up in the wild.
+    if (result.results.length === 0) {
+      this.log("batchCreate: response has empty results and zero counts (suspicious)", {
+        succeeded: result.succeeded,
+        failed: result.failed,
+        request_id: requestId,
+      });
+    }
+
     return result;
   }
 
@@ -769,6 +781,18 @@ export class QURLClient {
     // field. The data-side variant would be a wire-format duplication
     // we don't want to silently honor.
     const requestId = envelope.meta?.request_id;
+    // Both-fields-present log: mirrors mapQurlsField's pattern. If
+    // the server ever puts request_id on both `data` and `meta`, the
+    // spread silently lets `meta` win — fine, but operators should
+    // see the divergence rather than discover it via support-ticket
+    // archaeology.
+    const dataRequestId = (result as { request_id?: unknown }).request_id;
+    if (dataRequestId !== undefined && requestId !== undefined && dataRequestId !== requestId) {
+      this.log("batchCreate: response carries request_id on BOTH data and meta; keeping meta", {
+        meta_request_id: requestId,
+        data_request_id: String(dataRequestId).slice(0, 80),
+      });
+    }
     return requestId !== undefined ? { ...result, request_id: requestId } : result;
   }
 
@@ -969,6 +993,18 @@ export class QURLClient {
       }
     }
 
+    // Empty-string check on timing fields. `description: ""` has
+    // documented clear semantics (see UpdateInput JSDoc) and stays
+    // as-is; the timing fields don't — `extend_by: ""` would either
+    // trip the mutual-exclusion check below or hit the server as a
+    // malformed duration. Reject up front with a structured error
+    // matching the rest of the validators.
+    for (const key of ["extend_by", "expires_at"] as const) {
+      if (normalized[key] === "") {
+        throw clientValidationError(`${key}: must not be an empty string`);
+      }
+    }
+
     if (normalized.extend_by !== undefined && normalized.expires_at !== undefined) {
       throw clientValidationError(
         "update: `extend_by` and `expires_at` are mutually exclusive — provide at most one",
@@ -1017,6 +1053,16 @@ export class QURLClient {
       }
       normalized = stripped as MintInput;
     }
+    // Empty-string check on timing fields. Symmetric with update();
+    // the timing fields have no "clear" semantic, so an empty string
+    // is always a caller mistake (e.g. uninitialized form state).
+    if (normalized !== undefined) {
+      for (const key of ["expires_in", "expires_at"] as const) {
+        if (normalized[key] === "") {
+          throw clientValidationError(`${key}: must not be an empty string`);
+        }
+      }
+    }
     if (normalized?.expires_in !== undefined && normalized.expires_at !== undefined) {
       throw clientValidationError(
         "mintLink: `expires_in` and `expires_at` are mutually exclusive — provide at most one",