[Suggestion] This test uses a plain object { status: 409, message: 'Version conflict' } instead of an Error instance. getErrorMessage returns "[object Object]" for plain objects, so isTransientConflict never sees the actual message — the test passes purely by coincidence. With a real Error instance, the "conflict" substring in "Version conflict" would match, causing the test to fail and exposing that the 'conflict' keyword in isTransientConflict is overly broad.

[Suggestion] isTransientCapacityError now covers 408 and retryable network errors (ECONNRESET, ETIMEDOUT, etc.) in addition to 429/529. This function gates persistent retry mode — when QWEN_CODE_UNATTENDED_RETRY=true, qualifying errors are retried indefinitely for up to 6 hours. However, 408 can be a permanent condition (e.g. proxy timeout shorter than API latency), and network codes like ENOTFOUND, EHOSTUNREACH, ECONNREFUSED (in RETRYABLE_NETWORK_CODES) can indicate permanent config errors (wrong endpoint, firewall block). In CI/unattended mode, a simple endpoint typo would hang the process for hours instead of failing fast.

Consider scoping isTransientCapacityError back to 429/529 only (consistent with its name), or giving network/timeout errors a separate, shorter persistent cap.

— deepseek-v4-pro via Qwen Code /review

[Suggestion] The shouldRetryOnError callback now delegates entirely to classifyError(error).retryable for non-400 errors, replacing the previous explicit allowlist (status === 429, 5xx) with a return false fallback. While functionally equivalent today, this loses defense-in-depth: any future addition of a retryable category to classifyError (e.g. a new status code or message pattern) will automatically and silently apply to Gemini Chat streaming requests, bypassing Gemini Chat's context-specific judgment (which previously required explicit opt-in for each error type).

Consider adding an allowlist assertion after classifyError, or documenting which error categories are explicitly accepted for Gemini streaming retries.

— deepseek-v4-pro via Qwen Code /review

[Critical] Multiple expect(promise).rejects.toThrow(...) calls were changed to await expect(promise).rejects.toThrow(...), causing a deadlock under fake timers — the promise will not settle until vi.runAllTimersAsync() is called.

Affected locations (10+ in total):

• Line 103: should throw an error if all attempts fail
• Line 126: should default to 7 maxAttempts if no options are provided
• Line 146: should default to 7 maxAttempts if options.maxAttempts is undefined
• Line 197: should use default shouldRetry if not provided, retrying on 429
• Lines 266/281: should handle jitter correctly
• Lines 347/370/446: Qwen OAuth retry tests
• Line 661: should NOT retry indefinitely for 500 errors in persistent mode
• Line 754: should abort persistent retry when signal is aborted
• Lines 776/823: persistent mode fast-fail tests
• Line 1337: should exhaust retries on persistent 408 (new)
• Lines 1383/1478: should exhaust retries on persistent network error (new)

Root cause: When an error is retryable, retryWithBackoff internally calls delay(), creating an unfired setTimeout. await expect(...).rejects.toThrow(...) blocks until the promise rejects, but the rejection requires vi.runAllTimersAsync() to have been called first.

Revert all 10+ occurrences to the correct pattern without await, and restore the // eslint-disable-next-line vitest/valid-expect comments.

[Suggestion] The explicit if (status === 400) return false; check before delegating to classifyError is redundant — classifyError already classifies 400 as non-retryable. It creates an asymmetry: 400 is double-checked while 401/403/404/422 are only handled by classifyError. If a maintainer sees the explicit 400 check and removes the 400 handling from classifyError, callers of defaultShouldRetry would lose 400 protection.

Consider removing this explicit check and fully delegating to classifyError. Alternatively, move the message checks (isSchemaDepthError/isInvalidArgumentError) into classifyError as well, making the callback a clean delegation.

[Suggestion] ENOTFOUND (DNS resolution failure) is included in RETRYABLE_NETWORK_CODES, but DNS failures are typically permanent (hostname typo, DNS misconfiguration) rather than transient. This causes users to wait ~2 minutes before seeing a failure when there's a configuration error. ECONNREFUSED is also debatable — if the service isn't running at all, retrying is purely wasted time.

Consider removing ENOTFOUND from retryable codes, or add a comment explaining why DNS failures should be retried.