From d827e33aae9676530c4024d20231460c77b07e3d Mon Sep 17 00:00:00 2001
From: SandipBajracharya <sndp106.sb@gmail.com>
Date: Mon, 11 May 2026 14:26:47 +0545
Subject: [PATCH 1/8] refactor(OUT-3544): drop dead null guards in IntuitAPI

The fetch.helper getFetcher/postFetcher always returned response.json()
(a Promise that resolves to a defined value or threw), so the 13
`if (!res) throw new APIError(...)` branches scattered across IntuitAPI
methods were unreachable. Removing them de-noises the file ahead of the
forthcoming throw-on-non-2xx behavior. Also corrects a stale comment in
resolveUniqueCustomerName whose math referenced the old retries=3 budget.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 src/utils/intuitAPI.ts | 104 ++---------------------------------------
 1 file changed, 4 insertions(+), 100 deletions(-)

diff --git a/src/utils/intuitAPI.ts b/src/utils/intuitAPI.ts
index e652b33a..d874ddca 100644
--- a/src/utils/intuitAPI.ts
+++ b/src/utils/intuitAPI.ts
@@ -121,12 +121,6 @@ export default class IntuitAPI {
     const url = `${intuitBaseUrl}/v3/company/${this.tokens.intuitRealmId}/query?query=${encodeURIComponent(query)}&minorversion=${intuitApiMinorVersion}`
     const res = await this.getFetchWithHeader(url)
 
-    if (!res)
-      throw new APIError(
-        httpStatus.BAD_REQUEST,
-        'IntuitAPI#customQuery | message = no response',
-      )
-
     if (res?.Fault) {
       CustomLogger.error({ obj: res.Fault?.Error, message: 'Error: ' })
       throw new APIError(
@@ -146,12 +140,6 @@ export default class IntuitAPI {
     const url = `${intuitBaseUrl}/v3/company/${this.tokens.intuitRealmId}/invoice?minorversion=${intuitApiMinorVersion}`
     const invoice = await this.postFetchWithHeaders(url, payload)
 
-    if (!invoice)
-      throw new APIError(
-        httpStatus.BAD_REQUEST,
-        'IntuitAPI#createInvoice | message = no response',
-      )
-
     if (invoice?.Fault) {
       CustomLogger.error({ obj: invoice.Fault?.Error, message: 'Error: ' })
       throw new APIError(
@@ -178,12 +166,6 @@ export default class IntuitAPI {
     const url = `${intuitBaseUrl}/v3/company/${this.tokens.intuitRealmId}/customer?minorversion=${intuitApiMinorVersion}`
     const customer = await this.postFetchWithHeaders(url, payload)
 
-    if (!customer)
-      throw new APIError(
-        httpStatus.BAD_REQUEST,
-        'IntuitAPI#createCustomer | message = no response',
-      )
-
     if (customer?.Fault) {
       CustomLogger.error({ obj: customer.Fault?.Error, message: 'Error: ' })
       throw new APIError(
@@ -208,12 +190,6 @@ export default class IntuitAPI {
     const url = `${intuitBaseUrl}/v3/company/${this.tokens.intuitRealmId}/item?minorversion=${intuitApiMinorVersion}`
     const item = await this.postFetchWithHeaders(url, payload)
 
-    if (!item)
-      throw new APIError(
-        httpStatus.BAD_REQUEST,
-        'IntuitAPI#createItem | message = no response',
-      )
-
     if (item?.Fault) {
       CustomLogger.error({ obj: item.Fault?.Error, message: 'Error: ' })
       throw new APIError(
@@ -346,9 +322,10 @@ export default class IntuitAPI {
    * Inactive records are intentionally excluded: QBO auto-suffixes their
    * DisplayName with " (deleted)" when deactivated, freeing the original name.
    *
-   * Intentionally NOT wrapped in wrapWithRetry — the inner customQuery calls
-   * already retry on 429; re-wrapping would amplify rate-limit bursts (worst
-   * case 4 × 3 × 4 = 48 requests) and make recovery worse.
+   * Intentionally NOT wrapped in wrapWithRetry — each of the three inner
+   * customQuery calls is already retried by withRetry (up to 5 attempts on
+   * 429/5xx/transient network). Re-wrapping would amplify rate-limit bursts
+   * (worst case 5 × 3 × 5 = 75 requests) and make recovery worse.
    *
    * Throws if every candidate is taken.
    */
@@ -471,12 +448,6 @@ export default class IntuitAPI {
     const url = `${intuitBaseUrl}/v3/company/${this.tokens.intuitRealmId}/invoice?minorversion=${intuitApiMinorVersion}`
     const invoice = await this.postFetchWithHeaders(url, payload)
 
-    if (!invoice)
-      throw new APIError(
-        httpStatus.BAD_REQUEST,
-        'IntuitAPI#InvoiceSparseUpdate | message = no response',
-      )
-
     if (invoice?.Fault) {
       CustomLogger.error({ obj: invoice.Fault?.Error, message: 'Error: ' })
       throw new APIError(
@@ -503,12 +474,6 @@ export default class IntuitAPI {
     const url = `${intuitBaseUrl}/v3/company/${this.tokens.intuitRealmId}/customer?minorversion=${intuitApiMinorVersion}`
     const customer = await this.postFetchWithHeaders(url, payload)
 
-    if (!customer)
-      throw new APIError(
-        httpStatus.BAD_REQUEST,
-        'IntuitAPI#customerSparseUpdate | message = no response',
-      )
-
     if (customer?.Fault) {
       CustomLogger.error({ obj: customer.Fault?.Error, message: 'Error: ' })
       throw new APIError(
@@ -535,12 +500,6 @@ export default class IntuitAPI {
     const url = `${intuitBaseUrl}/v3/company/${this.tokens.intuitRealmId}/item?minorversion=${intuitApiMinorVersion}`
     const item = await this.postFetchWithHeaders(url, payload)
 
-    if (!item)
-      throw new APIError(
-        httpStatus.BAD_REQUEST,
-        'IntuitAPI#itemFullUpdate | message = no response',
-      )
-
     if (item?.Fault) {
       CustomLogger.error({ obj: item.Fault?.Error, message: 'Error: ' })
       throw new APIError(
@@ -569,12 +528,6 @@ export default class IntuitAPI {
     const url = `${intuitBaseUrl}/v3/company/${this.tokens.intuitRealmId}/account?minorversion=${intuitApiMinorVersion}`
     const account = await this.postFetchWithHeaders(url, payload)
 
-    if (!account)
-      throw new APIError(
-        httpStatus.BAD_REQUEST,
-        'IntuitAPI#updateAccount | message = no response',
-      )
-
     if (account?.Fault) {
       CustomLogger.error({ obj: account.Fault?.Error, message: 'Error: ' })
       throw new APIError(
@@ -601,12 +554,6 @@ export default class IntuitAPI {
     const url = `${intuitBaseUrl}/v3/company/${this.tokens.intuitRealmId}/payment?minorversion=${intuitApiMinorVersion}`
     const payment = await this.postFetchWithHeaders(url, payload)
 
-    if (!payment)
-      throw new APIError(
-        httpStatus.BAD_REQUEST,
-        'IntuitAPI#createPayment | message = no response',
-      )
-
     if (payment?.Fault) {
       CustomLogger.error({ obj: payment.Fault?.Error, message: 'Error: ' })
       throw new APIError(
@@ -631,12 +578,6 @@ export default class IntuitAPI {
     const query = `select Id, SyncToken, DocNumber from Invoice where DocNumber = '${escapeForQBQuery(invoiceNumber)}' maxresults 1`
     const invoice = await this.customQuery(query)
 
-    if (!invoice)
-      throw new APIError(
-        httpStatus.BAD_REQUEST,
-        'IntuitAPI#getInvoice | message = no response',
-      )
-
     if (!invoice.Invoice) return null
 
     CustomLogger.info({
@@ -654,12 +595,6 @@ export default class IntuitAPI {
     const url = `${intuitBaseUrl}/v3/company/${this.tokens.intuitRealmId}/invoice?operation=void&minorversion=${intuitApiMinorVersion}`
     const invoice = await this.postFetchWithHeaders(url, payload)
 
-    if (!invoice)
-      throw new APIError(
-        httpStatus.BAD_REQUEST,
-        'IntuitAPI#voidInvoice | message = no response',
-      )
-
     if (invoice?.Fault) {
       CustomLogger.error({ obj: invoice.Fault?.Error, message: 'Error: ' })
       throw new APIError(
@@ -684,13 +619,6 @@ export default class IntuitAPI {
     const url = `${intuitBaseUrl}/v3/company/${this.tokens.intuitRealmId}/invoice?operation=delete&minorversion=${intuitApiMinorVersion}`
     const invoice = await this.postFetchWithHeaders(url, payload)
 
-    if (!invoice) {
-      throw new APIError(
-        httpStatus.BAD_REQUEST,
-        'IntuitAPI#deleteInvoice | No invoice deletion confirmation was received from Quickbooks API',
-      )
-    }
-
     if (invoice?.Fault) {
       CustomLogger.error({ obj: invoice.Fault?.Error, message: 'Error: ' })
       throw new APIError(
@@ -715,12 +643,6 @@ export default class IntuitAPI {
     const url = `${intuitBaseUrl}/v3/company/${this.tokens.intuitRealmId}/payment?operation=delete&minorversion=${intuitApiMinorVersion}`
     const payment = await this.postFetchWithHeaders(url, payload)
 
-    if (!payment)
-      throw new APIError(
-        httpStatus.BAD_REQUEST,
-        'IntuitAPI#deletePayment | message = no response',
-      )
-
     if (payment?.Fault) {
       CustomLogger.error({ obj: payment.Fault?.Error, message: 'Error: ' })
       throw new APIError(
@@ -788,12 +710,6 @@ export default class IntuitAPI {
     const url = `${intuitBaseUrl}/v3/company/${this.tokens.intuitRealmId}/account?minorversion=${intuitApiMinorVersion}`
     const account = await this.postFetchWithHeaders(url, payload)
 
-    if (!account)
-      throw new APIError(
-        httpStatus.BAD_REQUEST,
-        'IntuitAPI#createAccount | message = no response',
-      )
-
     if (account?.Fault) {
       CustomLogger.error({ obj: account.Fault?.Error, message: 'Error: ' })
       throw new APIError(
@@ -818,12 +734,6 @@ export default class IntuitAPI {
     const url = `${intuitBaseUrl}/v3/company/${this.tokens.intuitRealmId}/purchase?minorversion=${intuitApiMinorVersion}`
     const purchase = await this.postFetchWithHeaders(url, payload)
 
-    if (!purchase)
-      throw new APIError(
-        httpStatus.BAD_REQUEST,
-        'IntuitAPI#createPurchase | message = no response',
-      )
-
     if (purchase?.Fault) {
       CustomLogger.error({ obj: purchase.Fault?.Error, message: 'Error: ' })
       throw new APIError(
@@ -848,12 +758,6 @@ export default class IntuitAPI {
     const url = `${intuitBaseUrl}/v3/company/${this.tokens.intuitRealmId}/purchase?operation=delete&minorversion=${intuitApiMinorVersion}`
     const purchase = await this.postFetchWithHeaders(url, payload)
 
-    if (!purchase)
-      throw new APIError(
-        httpStatus.BAD_REQUEST,
-        'IntuitAPI#deletePurchase | message = no response',
-      )
-
     if (purchase?.Fault) {
       CustomLogger.error({ obj: purchase.Fault?.Error, message: 'Error: ' })
       throw new APIError(

From 53dae5f2c37b8deff0045d4f1b5c4056d5da663c Mon Sep 17 00:00:00 2001
From: SandipBajracharya <sndp106.sb@gmail.com>
Date: Mon, 11 May 2026 14:27:08 +0545
Subject: [PATCH 2/8] feat(OUT-3544): timeouts + HttpFetchError + broader retry
 classifier for external calls

Outbound calls to QBO and Copilot could previously hang for the full 300s
Vercel function budget on a stalled connection, returned only 429 to the
retry classifier, and lost the real upstream error detail by the time the
sync log was written. This change makes all three layers resilient:

- AbortSignal.timeout (default 30s, EXTERNAL_FETCH_TIMEOUT_MS, NaN-safe)
  applied in fetch.helper and CopilotAPI.manualFetch so hung connections
  release the function slot and surface a retryable TimeoutError.
- New HttpFetchError captures {status, statusText, url, body} on non-2xx
  responses; buildHttpFetchError extracts the upstream detail
  (Intuit Fault.Error[].{Message,Detail}, generic {message}/{error}) into
  error.message so qb_sync_logs records the real reason, not "HTTP 400".
- isRetryableError now handles 429/5xx, undici fetch-failed envelopes,
  AbortSignal TimeoutError/AbortError, and Node ECONNRESET/REFUSED/etc.,
  with cause.code checked before the generic 'fetch failed' fallback so
  ENOTFOUND short-circuits to non-retryable. Retry budget retuned to
  5 attempts / ~20.5s backoff, fitting under the webhook execution cap.
- getMessageAndCodeFromError and withErrorHandler both grow a branch for
  HttpFetchError so source attribution (intuit/copilot) and Sentry capture
  work consistently for transport failures.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 src/app/api/core/utils/withErrorHandler.ts |  10 +-
 src/app/api/core/utils/withRetry.ts        | 120 ++++++++--
 src/config/index.ts                        |  16 ++
 src/helper/fetch.helper.ts                 |  89 ++++++-
 src/utils/copilotAPI.ts                    |  11 +-
 src/utils/error.ts                         |  45 ++++
 test/unit/core/withRetry.test.ts           | 103 +++++++-
 test/unit/helper/fetch.helper.test.ts      | 261 +++++++++++++++++++++
 8 files changed, 634 insertions(+), 21 deletions(-)
 create mode 100644 test/unit/helper/fetch.helper.test.ts

diff --git a/src/app/api/core/utils/withErrorHandler.ts b/src/app/api/core/utils/withErrorHandler.ts
index dedbc87f..e6c9bf8a 100644
--- a/src/app/api/core/utils/withErrorHandler.ts
+++ b/src/app/api/core/utils/withErrorHandler.ts
@@ -13,7 +13,11 @@ import {
   isIntuitOAuthError,
 } from '@/app/api/core/exceptions/custom'
 import * as Sentry from '@sentry/nextjs'
-import { getMessageAndCodeFromError, RetryableError } from '@/utils/error'
+import {
+  getMessageAndCodeFromError,
+  HttpFetchError,
+  RetryableError,
+} from '@/utils/error'
 import { getCategory } from '@/utils/synclog'
 
 type RequestHandler = (req: NextRequest, params: any) => Promise<NextResponse>
@@ -72,6 +76,9 @@ export const withErrorHandler = (handler: RequestHandler): RequestHandler => {
       } else if (error instanceof RetryableError) {
         status = error.status
         message = error.message || message
+      } else if (error instanceof HttpFetchError) {
+        status = error.status
+        message = error.message || message
       } else if (isIntuitOAuthError(error)) {
         message = error.error
         status = httpStatus.BAD_REQUEST
@@ -91,6 +98,7 @@ export const withErrorHandler = (handler: RequestHandler): RequestHandler => {
       } else if (
         error instanceof APIError ||
         error instanceof CopilotApiError ||
+        error instanceof HttpFetchError ||
         isAxiosError(error) ||
         isIntuitOAuthError(error)
       ) {
diff --git a/src/app/api/core/utils/withRetry.ts b/src/app/api/core/utils/withRetry.ts
index 3a71b07a..1c996a51 100644
--- a/src/app/api/core/utils/withRetry.ts
+++ b/src/app/api/core/utils/withRetry.ts
@@ -1,8 +1,103 @@
-import { StatusableError } from '@/type/CopilotApiError'
 import pRetry, { FailedAttemptError } from 'p-retry'
 import * as Sentry from '@sentry/nextjs'
 import { RetryableError } from '@/utils/error'
 
+const RETRYABLE_HTTP_STATUSES: ReadonlySet<number> = new Set([
+  429, // rate limit
+  500, // internal server error (often transient at QBO)
+  502, // bad gateway (upstream proxy hiccup)
+  503, // service unavailable
+  504, // gateway timeout
+])
+
+const RETRYABLE_NETWORK_CODES: ReadonlySet<string> = new Set([
+  'ECONNRESET',
+  'ECONNREFUSED',
+  'ETIMEDOUT',
+  'UND_ERR_CONNECT_TIMEOUT',
+])
+
+const RETRYABLE_ERROR_NAMES: ReadonlySet<string> = new Set([
+  'TimeoutError', // AbortSignal.timeout() rejection
+  'AbortError', // generic AbortController rejection (treat as transient)
+])
+
+/**
+ * Centralized classifier for whether an error should trigger a retry.
+ * Exported so it can be unit-tested independent of pRetry's timer plumbing.
+ *
+ * The error shapes inspected here come from different layers; a single
+ * unified type doesn't exist, which is why the input is `unknown` and
+ * each field is checked defensively:
+ *
+ *   - `RetryableError` (ours, src/utils/error.ts) — explicit retry flag.
+ *   - `HttpFetchError` (ours) and Copilot SDK's `StatusableError` —
+ *     `status: number` set after a non-2xx response was received.
+ *   - undici (Node fetch) network failures — thrown as
+ *     `TypeError: fetch failed` with the underlying error on `.cause`
+ *     (e.g. `{ code: 'ECONNRESET' }`). No HTTP response was ever built,
+ *     so there is no status to inspect.
+ *   - `AbortSignal.timeout()` and `AbortController.abort()` reject with
+ *     a `DOMException` whose `name` is `'TimeoutError'` / `'AbortError'`.
+ *   - Top-level `error.code` is checked defensively for legacy Node
+ *     error paths; in current Node fetch the code lives under `.cause`.
+ *
+ * Retry-nesting note: several call sites wrap an inner retried call
+ * (e.g. `IntuitAPI.getInvoice → customQuery`, both `withRetry`-wrapped;
+ * `authenticate.ts` → `copilotClient.getTokenPayload`). With the broadened
+ * retry set, worst-case wait is `outer_attempts × inner_attempts × per_call_timeout`.
+ * Inner wrappers should throw `new RetryableError(status, msg, false)` when
+ * they exhaust their own attempts so outer wrappers do not re-amplify them.
+ */
+export const isRetryableError = (error: unknown): boolean => {
+  if (error instanceof RetryableError) return error.retry
+
+  if (typeof error !== 'object' || error === null) return false
+
+  const err = error as {
+    status?: unknown
+    code?: unknown
+    name?: unknown
+    message?: unknown
+    cause?: unknown
+  }
+
+  if (typeof err.status === 'number' && RETRYABLE_HTTP_STATUSES.has(err.status))
+    return true
+
+  if (typeof err.name === 'string' && RETRYABLE_ERROR_NAMES.has(err.name))
+    return true
+
+  if (typeof err.code === 'string' && RETRYABLE_NETWORK_CODES.has(err.code))
+    return true
+
+  // Check `cause` BEFORE the generic 'fetch failed' message match so undici
+  // envelopes with a known-permanent sub-code (e.g. ENOTFOUND for DNS, ENETUNREACH)
+  // short-circuit to non-retryable instead of being retried 5× pointlessly.
+  if (err.cause && typeof err.cause === 'object') {
+    const cause = err.cause as { code?: unknown; name?: unknown }
+    if (
+      typeof cause.code === 'string' &&
+      RETRYABLE_NETWORK_CODES.has(cause.code)
+    )
+      return true
+    if (typeof cause.name === 'string' && RETRYABLE_ERROR_NAMES.has(cause.name))
+      return true
+    // cause is present with a recognized shape but not in either retry set →
+    // treat as permanent (DNS failure, cert error, etc.).
+    if (typeof cause.code === 'string' || typeof cause.name === 'string')
+      return false
+  }
+
+  // Fallback: undici wraps low-level network errors as `TypeError: fetch failed`.
+  // Only reach this branch when no usable cause was attached — treat as a
+  // generic transient network blip.
+  if (typeof err.message === 'string' && err.message === 'fetch failed')
+    return true
+
+  return false
+}
+
 export const withRetry = async <T>(
   fn: (...args: any[]) => Promise<T>,
   args: any[],
@@ -36,26 +131,23 @@ export const withRetry = async <T>(
     },
 
     {
-      retries: 3,
+      // Tuned so that maxTimeout actually engages on the final attempt.
+      // With minTimeout=500, factor=4, retries=4 the back-off sequence is
+      // 500ms / 2000ms / 8000ms / 10000ms (capped) = ~20.5s of waiting
+      // across 5 total attempts. Combined with the 30s fetch timeout, a
+      // single op caps at ~170s — well within the 300s webhook budget
+      // and gives the upstream a meaningful recovery window.
+      retries: 4,
       minTimeout: 500,
-      maxTimeout: 2000,
-      factor: 2, // Exponential factor for timeout delay. Tweak this if issues still persist
+      maxTimeout: 10_000,
+      factor: 4,
       onFailedAttempt: (error: FailedAttemptError) => {
         console.warn(
           `CopilotAPI#withRetry | Attempt ${error.attemptNumber} failed. There are ${error.retriesLeft} retries left. Error:`,
           error,
         )
       },
-      shouldRetry: (error: any) => {
-        if (error instanceof RetryableError) {
-          return error.retry
-        }
-
-        // Typecasting because Copilot doesn't export an error class
-        const err = error as StatusableError
-        // Retry only if statusCode === 429
-        return err.status === 429
-      },
+      shouldRetry: (error: unknown) => isRetryableError(error),
     },
   )
 }
diff --git a/src/config/index.ts b/src/config/index.ts
index 66e6194d..465c5d4a 100644
--- a/src/config/index.ts
+++ b/src/config/index.ts
@@ -37,6 +37,22 @@ export const intuitBaseUrl =
 export const intuitApiMinorVersion =
   process.env.INTUIT_API_MINOR_VERSION || '75'
 
+// Default timeout (ms) for outbound fetches to external APIs (QuickBooks,
+// Copilot). Prevents requests from hanging indefinitely against a slow or
+// unresponsive upstream. Per-call overrides are supported by the fetch helpers.
+// Guarded against NaN / non-positive values so a malformed env var doesn't
+// crash `AbortSignal.timeout()` on every external call.
+const parsePositiveMs = (raw: string | undefined, fallback: number): number => {
+  const n = Number(raw)
+  return Number.isFinite(n) && n > 0 ? n : fallback
+}
+
+// Tune EXTERNAL_FETCH_TIMEOUT_MS down (floor ~15s, ceiling ~45s per withRetry budget) once Sentry shows real QBO/Copilot P99 latency.
+export const externalFetchTimeoutMs = parsePositiveMs(
+  process.env.EXTERNAL_FETCH_TIMEOUT_MS,
+  30_000,
+)
+
 // Supabase
 export const supabaseProjectUrl =
   process.env.NEXT_PUBLIC_SUPABASE_PROJECT_URL || ''
diff --git a/src/helper/fetch.helper.ts b/src/helper/fetch.helper.ts
index 07c92e96..851c5d40 100644
--- a/src/helper/fetch.helper.ts
+++ b/src/helper/fetch.helper.ts
@@ -1,21 +1,106 @@
+import { externalFetchTimeoutMs } from '@/config'
+import { HttpFetchError } from '@/utils/error'
+
+export type FetcherOptions = {
+  // number = use this timeout; null = disable timeout entirely (no AbortSignal
+  // attached); undefined = fall back to externalFetchTimeoutMs.
+  timeoutMs?: number | null
+}
+
+// Pulls a human-readable detail out of common upstream error shapes so the
+// thrown HttpFetchError surfaces *why* the call failed in `error.message`
+// (which is what gets written to qb_sync_logs). Without this, the message
+// degrades to a generic "HTTP 400 Bad Request from <url>" and the real
+// reason — e.g. Intuit's "Required param missing" — only lives in
+// `error.body`, which most downstream consumers don't inspect.
+const extractUpstreamDetail = (body: unknown): string | undefined => {
+  if (!body || typeof body !== 'object') return undefined
+  const b = body as Record<string, any>
+
+  // Intuit (QBO):
+  //   { Fault: { Error: [{ Message, Detail, code }], type: '...' } }
+  // Detail is usually the diagnostic; Message is a short label.
+  const intuitErr = b?.Fault?.Error?.[0]
+  if (intuitErr && typeof intuitErr === 'object') {
+    const parts = [intuitErr.Message, intuitErr.Detail]
+      .filter((p) => typeof p === 'string' && p.length > 0)
+      .join(' — ')
+    if (parts) return parts
+  }
+
+  // Copilot and many other JSON APIs:  { message: '...' }  or  { error: '...' }
+  if (typeof b.message === 'string' && b.message) return b.message
+  if (typeof b.error === 'string' && b.error) return b.error
+
+  return undefined
+}
+
+// Exported so other clients with their own fetch wrappers (e.g. CopilotAPI's
+// manualFetch) can produce consistently-shaped HttpFetchError instances
+// without duplicating the JSON/text body-parsing logic.
+export const buildHttpFetchError = async (
+  response: Response,
+  url: string,
+): Promise<HttpFetchError> => {
+  const rawBody = await response.text().catch(() => '')
+  let body: unknown = rawBody
+  if (rawBody) {
+    try {
+      body = JSON.parse(rawBody)
+    } catch {
+      // not JSON; keep raw text
+    }
+  }
+
+  // Prefer the upstream detail as the message when present — it's what
+  // qb_sync_logs.message and any user-facing surface actually want to show.
+  // The HTTP status lives on error.status / qb_sync_logs.code and the URL
+  // lives on error.url, so we don't lose them by omitting them here.
+  const detail = extractUpstreamDetail(body)
+  const message =
+    detail ?? `HTTP ${response.status} ${response.statusText || ''}`.trim()
+
+  return new HttpFetchError({
+    status: response.status,
+    statusText: response.statusText,
+    url,
+    body,
+    message,
+  })
+}
+
+const resolveSignal = (opts: FetcherOptions): AbortSignal | undefined => {
+  if (opts.timeoutMs === null) return undefined
+  return AbortSignal.timeout(opts.timeoutMs ?? externalFetchTimeoutMs)
+}
+
 export const postFetcher = async (
   url: string,
   headers: Record<string, string>,
   body: Record<string, any>,
+  opts: FetcherOptions = {},
 ) => {
   const response = await fetch(url, {
     method: 'POST',
     headers,
-    body: JSON.stringify(body), // body data type must match "Content-Type" header
+    body: JSON.stringify(body),
+    signal: resolveSignal(opts),
   })
 
+  if (!response.ok) throw await buildHttpFetchError(response, url)
   return response.json()
 }
 
 export const getFetcher = async (
   url: string,
   headers: Record<string, string>,
+  opts: FetcherOptions = {},
 ) => {
-  const response = await fetch(url, { headers })
+  const response = await fetch(url, {
+    headers,
+    signal: resolveSignal(opts),
+  })
+
+  if (!response.ok) throw await buildHttpFetchError(response, url)
   return response.json()
 }
diff --git a/src/utils/copilotAPI.ts b/src/utils/copilotAPI.ts
index 4d37b904..4f2cc6a4 100644
--- a/src/utils/copilotAPI.ts
+++ b/src/utils/copilotAPI.ts
@@ -1,5 +1,10 @@
 import { withRetry } from '@/app/api/core/utils/withRetry'
-import { copilotAPIKey as apiKey, appId } from '@/config'
+import {
+  copilotAPIKey as apiKey,
+  appId,
+  externalFetchTimeoutMs,
+} from '@/config'
+import { buildHttpFetchError } from '@/helper/fetch.helper'
 import {
   ClientRequest,
   ClientResponse,
@@ -82,7 +87,11 @@ export class CopilotAPI {
         'X-API-KEY': workspaceId ? `${workspaceId}/${apiKey}` : apiKey,
         accept: 'application/json',
       },
+      signal: AbortSignal.timeout(externalFetchTimeoutMs),
     })
+
+    if (!resp.ok) throw await buildHttpFetchError(resp, url.toString())
+
     return await resp.json()
   }
 
diff --git a/src/utils/error.ts b/src/utils/error.ts
index 08e6616f..a33352ad 100644
--- a/src/utils/error.ts
+++ b/src/utils/error.ts
@@ -63,6 +63,26 @@ export const getMessageAndCodeFromError = (
         ? refreshTokenExpireMessage
         : error.error
     return { message, code: httpStatus.BAD_REQUEST, source: 'intuit' }
+  } else if (error instanceof HttpFetchError) {
+    // Transport-layer failure (non-2xx response from QBO/Copilot). Surface
+    // the real upstream status so qb_sync_logs records 503 as 503 instead of
+    // bucketing every transport failure as a generic 500.
+    //
+    // Source is inferred from a substring match on the request URL. Expected
+    // hostnames at time of writing:
+    //   intuit:  quickbooks.api.intuit.com (prod) / sandbox-quickbooks.api.intuit.com
+    //   copilot: api.copilot.app (prod) / api.copilot-staging.app
+    // If either vendor migrates to a domain that omits these substrings (e.g.
+    // a future `api.assembly.com`), revisit this heuristic — `unknown` would
+    // mislabel qb_sync_logs.source and skew the reaper/retry buckets.
+    const source: 'intuit' | 'copilot' | 'unknown' = error.url.includes(
+      'intuit',
+    )
+      ? 'intuit'
+      : error.url.includes('copilot')
+        ? 'copilot'
+        : 'unknown'
+    return { message: error.message, code: error.status, source }
   } else if (error instanceof Error && error.message) {
     return { message: error.message, code, source: 'unknown' }
   } else if (isAxiosError(error)) {
@@ -85,3 +105,28 @@ export class RetryableError extends Error {
     this.status = status
   }
 }
+
+export class HttpFetchError extends Error {
+  readonly status: number
+  readonly statusText: string
+  readonly url: string
+  readonly body: unknown
+
+  constructor(args: {
+    status: number
+    statusText: string
+    url: string
+    body: unknown
+    message?: string
+  }) {
+    super(
+      args.message ??
+        `HTTP ${args.status} ${args.statusText || ''} from ${args.url}`.trim(),
+    )
+    this.name = 'HttpFetchError'
+    this.status = args.status
+    this.statusText = args.statusText
+    this.url = args.url
+    this.body = args.body
+  }
+}
diff --git a/test/unit/core/withRetry.test.ts b/test/unit/core/withRetry.test.ts
index 0a04f220..4ba9e6b8 100644
--- a/test/unit/core/withRetry.test.ts
+++ b/test/unit/core/withRetry.test.ts
@@ -1,11 +1,59 @@
 import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'
 import { RetryableError } from '@/utils/error'
-import { withRetry } from '@/app/api/core/utils/withRetry'
+import { withRetry, isRetryableError } from '@/app/api/core/utils/withRetry'
 
 vi.mock('@sentry/nextjs', () => ({
   withScope: vi.fn(),
 }))
 
+describe('isRetryableError', () => {
+  it('returns the retry flag from RetryableError', () => {
+    expect(isRetryableError(new RetryableError(500, 'x', true))).toBe(true)
+    expect(isRetryableError(new RetryableError(500, 'x', false))).toBe(false)
+  })
+
+  it('treats 429/500/502/503/504 as retryable, others as not', () => {
+    for (const status of [429, 500, 502, 503, 504]) {
+      expect(isRetryableError(Object.assign(new Error(), { status }))).toBe(
+        true,
+      )
+    }
+    for (const status of [400, 401, 404, 501]) {
+      expect(isRetryableError(Object.assign(new Error(), { status }))).toBe(
+        false,
+      )
+    }
+  })
+
+  it('treats undici-style fetch failure with network code on cause as retryable', () => {
+    const err = Object.assign(new TypeError('fetch failed'), {
+      cause: { code: 'ECONNRESET' },
+    })
+    expect(isRetryableError(err)).toBe(true)
+  })
+
+  it('treats AbortSignal.timeout TimeoutError as retryable', () => {
+    const err = Object.assign(new Error('timed out'), { name: 'TimeoutError' })
+    expect(isRetryableError(err)).toBe(true)
+  })
+
+  it('treats AbortError as retryable', () => {
+    const err = Object.assign(new Error('aborted'), { name: 'AbortError' })
+    expect(isRetryableError(err)).toBe(true)
+  })
+
+  it('treats top-level network code as retryable', () => {
+    const err = Object.assign(new Error('boom'), { code: 'ETIMEDOUT' })
+    expect(isRetryableError(err)).toBe(true)
+  })
+
+  it('returns false for non-error inputs', () => {
+    expect(isRetryableError(null)).toBe(false)
+    expect(isRetryableError(undefined)).toBe(false)
+    expect(isRetryableError('oops')).toBe(false)
+  })
+})
+
 describe('withRetry', () => {
   beforeEach(() => {
     vi.useFakeTimers()
@@ -70,6 +118,55 @@ describe('withRetry', () => {
     expect(fn).toHaveBeenCalledTimes(1)
   })
 
+  it.each([500, 502, 503, 504])(
+    'retries on transient %i and succeeds',
+    async (status) => {
+      const error = Object.assign(new Error(`transient ${status}`), { status })
+      const fn = vi
+        .fn()
+        .mockRejectedValueOnce(error)
+        .mockResolvedValueOnce('recovered')
+
+      const promise = withRetry(fn, [])
+      await vi.runAllTimersAsync()
+
+      expect(await promise).toBe('recovered')
+      expect(fn).toHaveBeenCalledTimes(2)
+    },
+  )
+
+  it('retries on undici fetch-failed wrapping ECONNRESET and succeeds', async () => {
+    const error = Object.assign(new TypeError('fetch failed'), {
+      cause: { code: 'ECONNRESET' },
+    })
+    const fn = vi
+      .fn()
+      .mockRejectedValueOnce(error)
+      .mockResolvedValueOnce('recovered')
+
+    const promise = withRetry(fn, [])
+    await vi.runAllTimersAsync()
+
+    expect(await promise).toBe('recovered')
+    expect(fn).toHaveBeenCalledTimes(2)
+  })
+
+  it('retries on AbortSignal.timeout TimeoutError and succeeds', async () => {
+    const error = Object.assign(new Error('timed out'), {
+      name: 'TimeoutError',
+    })
+    const fn = vi
+      .fn()
+      .mockRejectedValueOnce(error)
+      .mockResolvedValueOnce('recovered')
+
+    const promise = withRetry(fn, [])
+    await vi.runAllTimersAsync()
+
+    expect(await promise).toBe('recovered')
+    expect(fn).toHaveBeenCalledTimes(2)
+  })
+
   it('exhausts retries and throws on persistent 429', async () => {
     const error = Object.assign(new Error('rate limited'), { status: 429 })
     const fn = vi.fn().mockRejectedValue(error)
@@ -81,7 +178,7 @@ describe('withRetry', () => {
     const result = await promise
     expect(result).toBeInstanceOf(Error)
     expect((result as Error).message).toBe('rate limited')
-    // 1 initial + 3 retries = 4 total
-    expect(fn).toHaveBeenCalledTimes(4)
+    // 1 initial + 4 retries = 5 total
+    expect(fn).toHaveBeenCalledTimes(5)
   })
 })
diff --git a/test/unit/helper/fetch.helper.test.ts b/test/unit/helper/fetch.helper.test.ts
new file mode 100644
index 00000000..eba1d2ba
--- /dev/null
+++ b/test/unit/helper/fetch.helper.test.ts
@@ -0,0 +1,261 @@
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'
+import { getFetcher, postFetcher } from '@/helper/fetch.helper'
+import { HttpFetchError } from '@/utils/error'
+
+const okJson = (body: unknown) =>
+  new Response(JSON.stringify(body), {
+    status: 200,
+    headers: { 'content-type': 'application/json' },
+  })
+
+describe('fetch.helper', () => {
+  beforeEach(() => {
+    vi.stubGlobal('fetch', vi.fn())
+  })
+
+  afterEach(() => {
+    vi.unstubAllGlobals()
+    vi.restoreAllMocks()
+  })
+
+  describe('getFetcher', () => {
+    it('returns parsed JSON on a 2xx response', async () => {
+      ;(fetch as any).mockResolvedValueOnce(okJson({ ok: true, n: 1 }))
+
+      const result = await getFetcher('https://example.com/x', {
+        Authorization: 'Bearer t',
+      })
+
+      expect(result).toEqual({ ok: true, n: 1 })
+    })
+
+    it('passes an AbortSignal to fetch', async () => {
+      ;(fetch as any).mockResolvedValueOnce(okJson({}))
+
+      await getFetcher('https://example.com/x', {})
+
+      const init = (fetch as any).mock.calls[0][1] as RequestInit
+      expect(init.signal).toBeInstanceOf(AbortSignal)
+    })
+
+    it('throws HttpFetchError with parsed JSON body on non-2xx', async () => {
+      ;(fetch as any).mockResolvedValueOnce(
+        new Response(
+          JSON.stringify({ Fault: { Error: [{ Detail: 'nope' }] } }),
+          {
+            status: 503,
+            statusText: 'Service Unavailable',
+            headers: { 'content-type': 'application/json' },
+          },
+        ),
+      )
+
+      await expect(
+        getFetcher('https://example.com/x', {}),
+      ).rejects.toBeInstanceOf(HttpFetchError)
+    })
+
+    it('captures status, url, and parsed body on the thrown error', async () => {
+      ;(fetch as any).mockResolvedValueOnce(
+        new Response('{"err":"boom"}', {
+          status: 500,
+          statusText: 'Server Error',
+          headers: { 'content-type': 'application/json' },
+        }),
+      )
+
+      const url = 'https://example.com/y'
+      try {
+        await getFetcher(url, {})
+        throw new Error('expected throw')
+      } catch (e) {
+        expect(e).toBeInstanceOf(HttpFetchError)
+        const err = e as HttpFetchError
+        expect(err.status).toBe(500)
+        expect(err.statusText).toBe('Server Error')
+        expect(err.url).toBe(url)
+        expect(err.body).toEqual({ err: 'boom' })
+      }
+    })
+
+    it('surfaces Intuit Fault.Error detail into error.message', async () => {
+      ;(fetch as any).mockResolvedValueOnce(
+        new Response(
+          JSON.stringify({
+            Fault: {
+              Error: [
+                {
+                  Message: 'Required param missing',
+                  Detail: 'Name is required',
+                  code: '2020',
+                },
+              ],
+              type: 'ValidationFault',
+            },
+          }),
+          {
+            status: 400,
+            statusText: 'Bad Request',
+            headers: { 'content-type': 'application/json' },
+          },
+        ),
+      )
+
+      try {
+        await getFetcher('https://sandbox-quickbooks.api.intuit.com/v3/x', {})
+        throw new Error('expected throw')
+      } catch (e) {
+        const err = e as HttpFetchError
+        expect(err.message).toBe('Required param missing — Name is required')
+        // Status and URL are preserved on the error fields, not in the message.
+        expect(err.status).toBe(400)
+        expect(err.url).toBe('https://sandbox-quickbooks.api.intuit.com/v3/x')
+      }
+    })
+
+    it('surfaces Copilot { message } into error.message', async () => {
+      ;(fetch as any).mockResolvedValueOnce(
+        new Response(JSON.stringify({ message: 'workspace not found' }), {
+          status: 404,
+          statusText: 'Not Found',
+          headers: { 'content-type': 'application/json' },
+        }),
+      )
+
+      try {
+        await getFetcher('https://api.copilot.app/v1/workspaces/x', {})
+        throw new Error('expected throw')
+      } catch (e) {
+        const err = e as HttpFetchError
+        expect(err.message).toBe('workspace not found')
+      }
+    })
+
+    it('falls back to generic HTTP message when body has no recognizable detail', async () => {
+      ;(fetch as any).mockResolvedValueOnce(
+        new Response(JSON.stringify({ unrelated: 'shape' }), {
+          status: 502,
+          statusText: 'Bad Gateway',
+          headers: { 'content-type': 'application/json' },
+        }),
+      )
+
+      try {
+        await getFetcher('https://example.com/y', {})
+        throw new Error('expected throw')
+      } catch (e) {
+        const err = e as HttpFetchError
+        expect(err.message).toBe('HTTP 502 Bad Gateway')
+        expect(err.url).toBe('https://example.com/y')
+      }
+    })
+
+    it('falls back to raw text body when response is not JSON', async () => {
+      ;(fetch as any).mockResolvedValueOnce(
+        new Response('<html>oops</html>', {
+          status: 504,
+          statusText: 'Gateway Timeout',
+          headers: { 'content-type': 'text/html' },
+        }),
+      )
+
+      try {
+        await getFetcher('https://example.com/y', {})
+        throw new Error('expected throw')
+      } catch (e) {
+        const err = e as HttpFetchError
+        expect(err.status).toBe(504)
+        expect(err.body).toBe('<html>oops</html>')
+      }
+    })
+
+    it('aborts the request when the per-call timeoutMs elapses', async () => {
+      vi.useFakeTimers()
+      try {
+        // Real fetch honors the signal: reject when the signal aborts.
+        ;(fetch as any).mockImplementationOnce(
+          (_url: string, init: RequestInit) =>
+            new Promise((_resolve, reject) => {
+              const signal = init.signal as AbortSignal
+              const onAbort = () => {
+                const err = new Error('The operation was aborted')
+                ;(err as any).name = 'AbortError'
+                reject(err)
+              }
+              if (signal.aborted) onAbort()
+              else signal.addEventListener('abort', onAbort, { once: true })
+            }),
+        )
+
+        const pending = getFetcher(
+          'https://example.com/x',
+          {},
+          { timeoutMs: 50 },
+        )
+        // Attach a rejection handler synchronously so the unhandled-rejection
+        // path doesn't trip when the timer fires.
+        const settled = pending.catch((e) => e)
+
+        await vi.advanceTimersByTimeAsync(60)
+        const result = await settled
+        expect(result).toBeInstanceOf(Error)
+        expect((result as Error).name).toBe('AbortError')
+      } finally {
+        vi.useRealTimers()
+      }
+    })
+
+    it('does not abort before the configured timeout elapses', async () => {
+      vi.useFakeTimers()
+      try {
+        ;(fetch as any).mockImplementationOnce(
+          (_url: string, _init: RequestInit) =>
+            new Promise((resolve) =>
+              setTimeout(() => resolve(okJson({ late: true })), 10),
+            ),
+        )
+
+        const pending = getFetcher(
+          'https://example.com/x',
+          {},
+          { timeoutMs: 1000 },
+        )
+        await vi.advanceTimersByTimeAsync(15)
+        await expect(pending).resolves.toEqual({ late: true })
+      } finally {
+        vi.useRealTimers()
+      }
+    })
+  })
+
+  describe('postFetcher', () => {
+    it('serializes body and returns parsed JSON on a 2xx response', async () => {
+      ;(fetch as any).mockResolvedValueOnce(okJson({ ok: true }))
+
+      const result = await postFetcher(
+        'https://example.com/x',
+        { 'content-type': 'application/json' },
+        { foo: 'bar' },
+      )
+
+      expect(result).toEqual({ ok: true })
+      const init = (fetch as any).mock.calls[0][1] as RequestInit
+      expect(init.method).toBe('POST')
+      expect(init.body).toBe(JSON.stringify({ foo: 'bar' }))
+    })
+
+    it('throws HttpFetchError on non-2xx', async () => {
+      ;(fetch as any).mockResolvedValueOnce(
+        new Response('{"err":"x"}', {
+          status: 500,
+          statusText: 'Server Error',
+          headers: { 'content-type': 'application/json' },
+        }),
+      )
+
+      await expect(
+        postFetcher('https://example.com/x', {}, { a: 1 }),
+      ).rejects.toBeInstanceOf(HttpFetchError)
+    })
+  })
+})

From 59ab70b255c1baff9a9ece8d8df2255d210852c8 Mon Sep 17 00:00:00 2001
From: SandipBajracharya <sndp106.sb@gmail.com>
Date: Mon, 11 May 2026 14:27:23 +0545
Subject: [PATCH 3/8] refactor(OUT-3544): adapt hooks/SWR to fetch.helper
 throw-on-error contract
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

postFetcher/getFetcher now throw HttpFetchError on non-2xx instead of
returning the parsed body (which was always truthy), so the truthiness
checks in useSettings/useQuickbooks were already dead code. Switch them
to try/catch and pass { timeoutMs: null } so browser fetches against the
app's own routes are not subject to the external-API timeout — SWR
handles its own error-retry for GETs, and user-initiated POSTs surface
errors back to the form rather than auto-aborting.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 src/helper/swr.helper.ts  |  4 +++-
 src/hook/useQuickbooks.ts |  6 +++++-
 src/hook/useSettings.ts   | 36 ++++++++++++++++--------------------
 3 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/src/helper/swr.helper.ts b/src/helper/swr.helper.ts
index 7f7d83f3..4665ffae 100644
--- a/src/helper/swr.helper.ts
+++ b/src/helper/swr.helper.ts
@@ -1,7 +1,9 @@
 import { getFetcher } from '@/helper/fetch.helper'
 import useSWR, { SWRConfiguration } from 'swr'
 
-const fetcher = (url: string) => getFetcher(url, {})
+// In-app SWR fetches against this app's own routes — no client-side timeout.
+// SWR handles its own error-retry; aborts here would just produce false negatives.
+const fetcher = (url: string) => getFetcher(url, {}, { timeoutMs: null })
 
 export const useSwrHelper = (key: any, opts: SWRConfiguration = {}) =>
   useSWR(key, fetcher, {
diff --git a/src/hook/useQuickbooks.ts b/src/hook/useQuickbooks.ts
index f2673b46..99e516a2 100644
--- a/src/hook/useQuickbooks.ts
+++ b/src/hook/useQuickbooks.ts
@@ -289,7 +289,11 @@ export const useAppBridge = ({
       enable: false,
     }
     const url = `/api/quickbooks/token/change-enable-status?token=${token}`
-    await postFetcher(url, {}, payload)
+    try {
+      await postFetcher(url, {}, payload, { timeoutMs: null })
+    } catch (err) {
+      console.error('Error disconnecting QuickBooks account', err)
+    }
   }
 
   const downloadCsvAction = async () => {
diff --git a/src/hook/useSettings.ts b/src/hook/useSettings.ts
index 6ea735ef..f8b4251a 100644
--- a/src/hook/useSettings.ts
+++ b/src/hook/useSettings.ts
@@ -121,6 +121,7 @@ export const useProductMappingSettings = () => {
       `/api/quickbooks/product/map?token=${token}`,
       {},
       { mappingItems, changedItemReference },
+      { timeoutMs: null },
     )
   }
 
@@ -129,6 +130,7 @@ export const useProductMappingSettings = () => {
       `/api/quickbooks/setting?type=${SettingType.PRODUCT}&token=${token}`,
       {},
       { ...productSetting, type: SettingType.PRODUCT },
+      { timeoutMs: null },
     )
   }
 
@@ -138,23 +140,16 @@ export const useProductMappingSettings = () => {
       showProductConfirm: false,
     }))
     setSettingShowConfirm(false)
-    const [tableRes, settingRes] = await Promise.all([
-      tableMappingSubmit(),
-      settingSubmit(),
-    ])
-
-    if (tableRes && settingRes) {
+    try {
+      await Promise.all([tableMappingSubmit(), settingSubmit()])
       mutate(`/api/quickbooks/product/map?token=${token}`)
       mutate(
         `/api/quickbooks/setting?type=${SettingType.PRODUCT}&token=${token}`,
       )
       setChangedItemReference([])
-    } else {
+    } catch (err) {
       setSettingShowConfirm(true) // show the update settings button if error
-      console.error('Error submitting product settings', {
-        tableRes,
-        settingRes,
-      })
+      console.error('Error submitting product settings', err)
     }
   }
 
@@ -558,16 +553,17 @@ export const useInvoiceDetailSettings = () => {
 
   const submitInvoiceSettings = async () => {
     setShowButton(false)
-    const res = await postFetcher(
-      `/api/quickbooks/setting?type=${SettingType.INVOICE}&token=${token}`,
-      {},
-      { ...settingState, type: SettingType.INVOICE },
-    )
-    if (!res || res?.error) {
-      setShowButton(true) // show the update settings button if error
-      console.error('Error submitting Invoice settings', { res })
-    } else {
+    try {
+      await postFetcher(
+        `/api/quickbooks/setting?type=${SettingType.INVOICE}&token=${token}`,
+        {},
+        { ...settingState, type: SettingType.INVOICE },
+        { timeoutMs: null },
+      )
       mutate(`/api/quickbooks/setting?type=invoice&token=${token}`)
+    } catch (err) {
+      setShowButton(true) // show the update settings button if error
+      console.error('Error submitting Invoice settings', err)
     }
   }
 

From c8307cb92f9c5161bd0c8f7ff32ff0e750fca031 Mon Sep 17 00:00:00 2001
From: SandipBajracharya <sndp106.sb@gmail.com>
Date: Mon, 11 May 2026 15:01:57 +0545
Subject: [PATCH 4/8] fix(OUT-3544): unwrap IntuitAPI read methods to eliminate
 withRetry nesting
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Greptile P1: _getInvoice, _getSingleIncomeAccount, _getCompanyInfo and
their siblings (_getACustomer, _getAnItem, _getAllItems, _getAnAccount)
each call this.customQuery — the wrapWithRetry-wrapped version — from
inside their own wrapWithRetry-wrapped public exports. With the broader
retry classifier (5xx + timeouts), worst-case attempts compound to
5 outer × 5 inner = 25 calls × 30s timeout, far past the 300s webhook
maxDuration.

Drop the outer wrapWithRetry from the public read exports (now plain
.bind(this) like getCustomerByEmail already did) so retries happen only
at the customQuery network boundary. Writes (create*/update*/delete*/
void*/*SparseUpdate) stay wrapped — they hit the network directly via
postFetchWithHeaders and have no nesting concern.

_getCompanyInfo previously threw a RetryableError(NOT_FOUND, true) when
customQuery returned falsy, relying on the outer wrap to retry. With
the unwrap, that defensive retry path becomes ineffective. Replace it
with a terminal APIError — an empty CompanyInfo response is almost
always a permanent issue (wrong realmId, bad token) that retries do
not fix. Drops the unused RetryableError import as a side effect.

Add a wrap-convention comment block above the export list and a
nesting-hazard note in the withRetry JSDoc so future contributors do
not silently reintroduce the amplification by wrapping new read methods.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 src/app/api/core/utils/withRetry.ts | 13 ++++++------
 src/utils/intuitAPI.ts              | 33 +++++++++++++++--------------
 2 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/src/app/api/core/utils/withRetry.ts b/src/app/api/core/utils/withRetry.ts
index 1c996a51..61f8f88b 100644
--- a/src/app/api/core/utils/withRetry.ts
+++ b/src/app/api/core/utils/withRetry.ts
@@ -42,12 +42,13 @@ const RETRYABLE_ERROR_NAMES: ReadonlySet<string> = new Set([
  *   - Top-level `error.code` is checked defensively for legacy Node
  *     error paths; in current Node fetch the code lives under `.cause`.
  *
- * Retry-nesting note: several call sites wrap an inner retried call
- * (e.g. `IntuitAPI.getInvoice → customQuery`, both `withRetry`-wrapped;
- * `authenticate.ts` → `copilotClient.getTokenPayload`). With the broadened
- * retry set, worst-case wait is `outer_attempts × inner_attempts × per_call_timeout`.
- * Inner wrappers should throw `new RetryableError(status, msg, false)` when
- * they exhaust their own attempts so outer wrappers do not re-amplify them.
+ * Retry-nesting hazard: do not call a `withRetry`-wrapped function from
+ * inside another `withRetry`-wrapped function. With the broadened retry
+ * set, worst-case wait is `outer × inner × per_call_timeout`, which can
+ * blow past the 300s webhook execution budget. Inside `IntuitAPI._*`
+ * methods that are themselves wrapped at the public level (see exports
+ * at the bottom of `src/utils/intuitAPI.ts`), call the unwrapped `_*`
+ * counterparts directly (e.g. `this._customQuery`, not `this.customQuery`).
  */
 export const isRetryableError = (error: unknown): boolean => {
   if (error instanceof RetryableError) return error.retry
diff --git a/src/utils/intuitAPI.ts b/src/utils/intuitAPI.ts
index d874ddca..bd8af7cf 100644
--- a/src/utils/intuitAPI.ts
+++ b/src/utils/intuitAPI.ts
@@ -29,7 +29,6 @@ import {
   SingleIdAndTokenResponseSchema,
 } from '@/type/dto/intuitAPI.dto'
 import { escapeForQBQuery, getNameAsCustomer } from '@/utils/string'
-import { RetryableError } from '@/utils/error'
 import CustomLogger from '@/utils/logger'
 import httpStatus from 'http-status'
 
@@ -782,11 +781,7 @@ export default class IntuitAPI {
     const companyInfo = await this.customQuery(query)
 
     if (!companyInfo)
-      throw new RetryableError(
-        httpStatus.NOT_FOUND,
-        'No company info found',
-        true,
-      )
+      throw new APIError(httpStatus.NOT_FOUND, 'No company info found')
 
     const parsedCompanyInfo = CompanyInfoSchema.parse(companyInfo)
     return parsedCompanyInfo.CompanyInfo[0]
@@ -798,11 +793,17 @@ export default class IntuitAPI {
     return (...args: Args): Promise<R> => withRetry(fn.bind(this), args)
   }
 
+  // Wrap convention: `customQuery` and writes (create*/update*/delete*/void*/
+  // *SparseUpdate) are wrapped here so a single retry layer protects the
+  // network call. Read methods (get*) compose `customQuery` internally and are
+  // therefore left unwrapped — wrapping them would nest `withRetry` and
+  // amplify the worst-case attempts past the 300s webhook budget. See
+  // src/app/api/core/utils/withRetry.ts JSDoc for the underlying rule.
   customQuery = this.wrapWithRetry(this._customQuery)
   createInvoice = this.wrapWithRetry(this._createInvoice)
   createCustomer = this.wrapWithRetry(this._createCustomer)
   createItem = this.wrapWithRetry(this._createItem)
-  getSingleIncomeAccount = this.wrapWithRetry(this._getSingleIncomeAccount)
+  getSingleIncomeAccount = this._getSingleIncomeAccount.bind(this)
   getACustomer: {
     (
       displayName: string,
@@ -819,10 +820,10 @@ export default class IntuitAPI {
       id: string,
       includeInactive?: boolean,
     ): Promise<CustomerQueryResponseType>
-  } = this.wrapWithRetry(this._getACustomer) as any
-  // Intentionally NOT wrapped in wrapWithRetry — a transient 429 mid-walk would
-  // replay from page 1 and amplify rate-limit pressure. The inner customQuery
-  // calls already retry on 429 (same reasoning as resolveUniqueCustomerName).
+  } = this._getACustomer.bind(this) as any
+  // Additional rationale beyond the wrap convention: a transient 429 mid-walk
+  // would replay from page 1 and amplify rate-limit pressure (same reasoning
+  // as resolveUniqueCustomerName).
   getCustomerByEmail = this._getCustomerByEmail.bind(this)
   getAnItem: {
     (
@@ -840,13 +841,13 @@ export default class IntuitAPI {
       id: string,
       includeInactive?: boolean,
     ): Promise<ItemResponseType>
-  } = this.wrapWithRetry(this._getAnItem) as any
-  getAllItems = this.wrapWithRetry(this._getAllItems)
+  } = this._getAnItem.bind(this) as any
+  getAllItems = this._getAllItems.bind(this)
   invoiceSparseUpdate = this.wrapWithRetry(this._invoiceSparseUpdate)
   customerSparseUpdate = this.wrapWithRetry(this._customerSparseUpdate)
   itemFullUpdate = this.wrapWithRetry(this._itemFullUpdate)
   createPayment = this.wrapWithRetry(this._createPayment)
-  getInvoice = this.wrapWithRetry(this._getInvoice)
+  getInvoice = this._getInvoice.bind(this)
   voidInvoice = this.wrapWithRetry(this._voidInvoice)
   deleteInvoice = this.wrapWithRetry(this._deleteInvoice)
   getAnAccount: {
@@ -865,11 +866,11 @@ export default class IntuitAPI {
       id: string,
       includeInactive?: boolean,
     ): Promise<AccountResponseType>
-  } = this.wrapWithRetry(this._getAnAccount) as any
+  } = this._getAnAccount.bind(this) as any
   createAccount = this.wrapWithRetry(this._createAccount)
   updateAccount = this.wrapWithRetry(this._updateAccount)
   createPurchase = this.wrapWithRetry(this._createPurchase)
   deletePayment = this.wrapWithRetry(this._deletePayment)
   deletePurchase = this.wrapWithRetry(this._deletePurchase)
-  getCompanyInfo = this.wrapWithRetry(this._getCompanyInfo)
+  getCompanyInfo = this._getCompanyInfo.bind(this)
 }

From dd108c27b2f3a6d505dfdbbe5e2b1455c24907ba Mon Sep 17 00:00:00 2001
From: SandipBajracharya <sndp106.sb@gmail.com>
Date: Mon, 11 May 2026 15:05:30 +0545
Subject: [PATCH 5/8] chore(OUT-3544): tidy withRetry + fetch.helper test names
 and remove redundancy
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Rename "does not retry on non-429 status errors" to "does not retry on
  permanent 4xx responses" — the old name was misleading once the retry
  classifier was broadened to cover 5xx.
- Drop the "throws HttpFetchError with parsed JSON body on non-2xx" test:
  its only assertion is instanceof HttpFetchError, which is already
  covered by every downstream test that catches and inspects err.status /
  err.url / err.body / err.message.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 test/unit/core/withRetry.test.ts      |  2 +-
 test/unit/helper/fetch.helper.test.ts | 17 -----------------
 2 files changed, 1 insertion(+), 18 deletions(-)

diff --git a/test/unit/core/withRetry.test.ts b/test/unit/core/withRetry.test.ts
index 4ba9e6b8..acbfd3fa 100644
--- a/test/unit/core/withRetry.test.ts
+++ b/test/unit/core/withRetry.test.ts
@@ -110,7 +110,7 @@ describe('withRetry', () => {
     expect(fn).toHaveBeenCalledTimes(1)
   })
 
-  it('does not retry on non-429 status errors', async () => {
+  it('does not retry on permanent 4xx responses', async () => {
     const error = Object.assign(new Error('bad request'), { status: 400 })
     const fn = vi.fn().mockRejectedValue(error)
 
diff --git a/test/unit/helper/fetch.helper.test.ts b/test/unit/helper/fetch.helper.test.ts
index eba1d2ba..5e2ff625 100644
--- a/test/unit/helper/fetch.helper.test.ts
+++ b/test/unit/helper/fetch.helper.test.ts
@@ -38,23 +38,6 @@ describe('fetch.helper', () => {
       expect(init.signal).toBeInstanceOf(AbortSignal)
     })
 
-    it('throws HttpFetchError with parsed JSON body on non-2xx', async () => {
-      ;(fetch as any).mockResolvedValueOnce(
-        new Response(
-          JSON.stringify({ Fault: { Error: [{ Detail: 'nope' }] } }),
-          {
-            status: 503,
-            statusText: 'Service Unavailable',
-            headers: { 'content-type': 'application/json' },
-          },
-        ),
-      )
-
-      await expect(
-        getFetcher('https://example.com/x', {}),
-      ).rejects.toBeInstanceOf(HttpFetchError)
-    })
-
     it('captures status, url, and parsed body on the thrown error', async () => {
       ;(fetch as any).mockResolvedValueOnce(
         new Response('{"err":"boom"}', {

From 8e21c5629c1d2360726bffb2dd54cb82de0ec7f9 Mon Sep 17 00:00:00 2001
From: SandipBajracharya <sndp106.sb@gmail.com>
Date: Mon, 11 May 2026 15:10:46 +0545
Subject: [PATCH 6/8] chore(OUT-3544): trim verbose inline comments

Shorten the HttpFetchError source-detection block in error.ts and the
wrap-convention block in intuitAPI.ts to the actionable rule. The
extended hostname list and budget arithmetic lived in the prose; they
belong in PR descriptions, not inline.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 src/utils/error.ts     | 13 ++-----------
 src/utils/intuitAPI.ts |  9 +++------
 2 files changed, 5 insertions(+), 17 deletions(-)

diff --git a/src/utils/error.ts b/src/utils/error.ts
index a33352ad..82325174 100644
--- a/src/utils/error.ts
+++ b/src/utils/error.ts
@@ -64,17 +64,8 @@ export const getMessageAndCodeFromError = (
         : error.error
     return { message, code: httpStatus.BAD_REQUEST, source: 'intuit' }
   } else if (error instanceof HttpFetchError) {
-    // Transport-layer failure (non-2xx response from QBO/Copilot). Surface
-    // the real upstream status so qb_sync_logs records 503 as 503 instead of
-    // bucketing every transport failure as a generic 500.
-    //
-    // Source is inferred from a substring match on the request URL. Expected
-    // hostnames at time of writing:
-    //   intuit:  quickbooks.api.intuit.com (prod) / sandbox-quickbooks.api.intuit.com
-    //   copilot: api.copilot.app (prod) / api.copilot-staging.app
-    // If either vendor migrates to a domain that omits these substrings (e.g.
-    // a future `api.assembly.com`), revisit this heuristic — `unknown` would
-    // mislabel qb_sync_logs.source and skew the reaper/retry buckets.
+    // Surface real upstream status to qb_sync_logs. Source inferred by URL
+    // substring (intuit.com / copilot.app); revisit if either host migrates.
     const source: 'intuit' | 'copilot' | 'unknown' = error.url.includes(
       'intuit',
     )
diff --git a/src/utils/intuitAPI.ts b/src/utils/intuitAPI.ts
index bd8af7cf..41b1a9bd 100644
--- a/src/utils/intuitAPI.ts
+++ b/src/utils/intuitAPI.ts
@@ -793,12 +793,9 @@ export default class IntuitAPI {
     return (...args: Args): Promise<R> => withRetry(fn.bind(this), args)
   }
 
-  // Wrap convention: `customQuery` and writes (create*/update*/delete*/void*/
-  // *SparseUpdate) are wrapped here so a single retry layer protects the
-  // network call. Read methods (get*) compose `customQuery` internally and are
-  // therefore left unwrapped — wrapping them would nest `withRetry` and
-  // amplify the worst-case attempts past the 300s webhook budget. See
-  // src/app/api/core/utils/withRetry.ts JSDoc for the underlying rule.
+  // Wrap convention: writes + customQuery are wrapped here. Read methods
+  // (get*) compose customQuery and stay unwrapped to avoid nested withRetry
+  // (see withRetry.ts).
   customQuery = this.wrapWithRetry(this._customQuery)
   createInvoice = this.wrapWithRetry(this._createInvoice)
   createCustomer = this.wrapWithRetry(this._createCustomer)

From 5f3db267d90f5f55faa3fecca1626a957c9ddfe2 Mon Sep 17 00:00:00 2001
From: SandipBajracharya <sndp106.sb@gmail.com>
Date: Mon, 11 May 2026 15:13:49 +0545
Subject: [PATCH 7/8] =?UTF-8?q?fix(OUT-3544):=20drop=20AbortError=20from?=
 =?UTF-8?q?=20retryable=20set=20=E2=80=94=20only=20TimeoutError?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

AbortController.abort() throws AbortError and signals a deliberate
cancellation; retrying would defeat the intent. Node 18+ emits a
distinct TimeoutError for AbortSignal.timeout() rejections, which is
the only abort flavor we actually want to retry. Update the test
that previously asserted AbortError as retryable to assert the
opposite, and switch the fetch.helper timeout test to mock
TimeoutError (matching production behavior).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 src/app/api/core/utils/withRetry.ts   | 10 ++++++----
 test/unit/core/withRetry.test.ts      |  4 ++--
 test/unit/helper/fetch.helper.test.ts |  7 +++----
 3 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/src/app/api/core/utils/withRetry.ts b/src/app/api/core/utils/withRetry.ts
index 61f8f88b..4c526823 100644
--- a/src/app/api/core/utils/withRetry.ts
+++ b/src/app/api/core/utils/withRetry.ts
@@ -18,8 +18,9 @@ const RETRYABLE_NETWORK_CODES: ReadonlySet<string> = new Set([
 ])
 
 const RETRYABLE_ERROR_NAMES: ReadonlySet<string> = new Set([
-  'TimeoutError', // AbortSignal.timeout() rejection
-  'AbortError', // generic AbortController rejection (treat as transient)
+  'TimeoutError', // AbortSignal.timeout() rejection (Node 18+)
+  // 'AbortError' is intentionally excluded — it signals a deliberate
+  // AbortController.abort() and retrying would defeat the cancellation.
 ])
 
 /**
@@ -37,8 +38,9 @@ const RETRYABLE_ERROR_NAMES: ReadonlySet<string> = new Set([
  *     `TypeError: fetch failed` with the underlying error on `.cause`
  *     (e.g. `{ code: 'ECONNRESET' }`). No HTTP response was ever built,
  *     so there is no status to inspect.
- *   - `AbortSignal.timeout()` and `AbortController.abort()` reject with
- *     a `DOMException` whose `name` is `'TimeoutError'` / `'AbortError'`.
+ *   - `AbortSignal.timeout()` rejects with a `DOMException` whose
+ *     `name` is `'TimeoutError'` (retryable). `AbortController.abort()`
+ *     produces `'AbortError'` and is NOT retried (deliberate cancellation).
  *   - Top-level `error.code` is checked defensively for legacy Node
  *     error paths; in current Node fetch the code lives under `.cause`.
  *
diff --git a/test/unit/core/withRetry.test.ts b/test/unit/core/withRetry.test.ts
index acbfd3fa..d4234198 100644
--- a/test/unit/core/withRetry.test.ts
+++ b/test/unit/core/withRetry.test.ts
@@ -37,9 +37,9 @@ describe('isRetryableError', () => {
     expect(isRetryableError(err)).toBe(true)
   })
 
-  it('treats AbortError as retryable', () => {
+  it('does NOT retry AbortError — deliberate cancellation should propagate', () => {
     const err = Object.assign(new Error('aborted'), { name: 'AbortError' })
-    expect(isRetryableError(err)).toBe(true)
+    expect(isRetryableError(err)).toBe(false)
   })
 
   it('treats top-level network code as retryable', () => {
diff --git a/test/unit/helper/fetch.helper.test.ts b/test/unit/helper/fetch.helper.test.ts
index 5e2ff625..81595d75 100644
--- a/test/unit/helper/fetch.helper.test.ts
+++ b/test/unit/helper/fetch.helper.test.ts
@@ -156,13 +156,14 @@ describe('fetch.helper', () => {
       vi.useFakeTimers()
       try {
         // Real fetch honors the signal: reject when the signal aborts.
+        // AbortSignal.timeout() produces a TimeoutError (Node 18+).
         ;(fetch as any).mockImplementationOnce(
           (_url: string, init: RequestInit) =>
             new Promise((_resolve, reject) => {
               const signal = init.signal as AbortSignal
               const onAbort = () => {
                 const err = new Error('The operation was aborted')
-                ;(err as any).name = 'AbortError'
+                ;(err as any).name = 'TimeoutError'
                 reject(err)
               }
               if (signal.aborted) onAbort()
@@ -175,14 +176,12 @@ describe('fetch.helper', () => {
           {},
           { timeoutMs: 50 },
         )
-        // Attach a rejection handler synchronously so the unhandled-rejection
-        // path doesn't trip when the timer fires.
         const settled = pending.catch((e) => e)
 
         await vi.advanceTimersByTimeAsync(60)
         const result = await settled
         expect(result).toBeInstanceOf(Error)
-        expect((result as Error).name).toBe('AbortError')
+        expect((result as Error).name).toBe('TimeoutError')
       } finally {
         vi.useRealTimers()
       }

From 77df1c381515b7a71c70568b39dc34d889a6e163 Mon Sep 17 00:00:00 2001
From: SandipBajracharya <sndp106.sb@gmail.com>
Date: Mon, 11 May 2026 16:12:20 +0545
Subject: [PATCH 8/8] fix(OUT-3544): strict retry classifier for non-idempotent
 write paths
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The OUT-3544 broadening of the retry classifier (429 → 429 + 5xx + network)
applies uniformly to every wrapped call, including QBO write endpoints
(`createInvoice`, `createCustomer`, `createItem`, `createPayment`,
`createAccount`, `createPurchase`) which have no Intuit-side request-key
dedupe. A 5xx or network error after the upstream commits could cause
pRetry to replay the write and produce a duplicate financial record.

Adds an `idempotent` option to `withRetry` / `isRetryableError`. In strict
mode (`idempotent: false`), the classifier retries only on 429 and explicit
`RetryableError.retry === true`; 5xx, network codes, and AbortSignal
timeouts are all treated as possibly-after-commit and never replayed.

Inverts the convention in both QBO-facing wrappers:

- `IntuitAPI.wrapWithRetry` defaults to strict; `customQuery` (the only
  read in the wrapped set) opts back into broad retry. All 13 wrapped
  writes (create/update/void/delete) inherit strict by default, so any
  future write method added without options is automatically safe.
- `Intuit.wrapWithRetry` (OAuth) also defaults to strict. `createToken`
  and `refreshAccessToken` consume single-use credentials and would
  previously misdiagnose a 5xx-after-commit refresh as `invalid_grant`
  via `tokenRefresh.handleInvalidGrant`, throwing `QBReconnectRequiredError`
  for a healthy connection. Strict mode lets the transport error surface
  honestly.

Both wrappers merge `{ idempotent: false, ...options }` rather than
relying on a parameter default, so an explicitly-passed `{}` cannot
silently flip back to the global broad-retry default.

Out of scope (not addressed here):
- The same after-commit-with-dropped-response can still produce a
  duplicate when the 3-hour resync re-runs the write. Eliminating that
  requires QBO's `requestid` query parameter on POST endpoints — a
  separate ticket.
- `copilotAPI.ts` / `authenticate.ts` retain the global `idempotent: true`
  default. None of those calls are QBO writes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 src/app/api/core/utils/withRetry.ts |  83 ++++++++++++----------
 src/utils/intuit.ts                 |  10 ++-
 src/utils/intuitAPI.ts              |  15 ++--
 test/unit/core/withRetry.test.ts    | 102 ++++++++++++++++++++++++++++
 4 files changed, 167 insertions(+), 43 deletions(-)

diff --git a/src/app/api/core/utils/withRetry.ts b/src/app/api/core/utils/withRetry.ts
index 4c526823..65ce290b 100644
--- a/src/app/api/core/utils/withRetry.ts
+++ b/src/app/api/core/utils/withRetry.ts
@@ -2,12 +2,17 @@ import pRetry, { FailedAttemptError } from 'p-retry'
 import * as Sentry from '@sentry/nextjs'
 import { RetryableError } from '@/utils/error'
 
-const RETRYABLE_HTTP_STATUSES: ReadonlySet<number> = new Set([
-  429, // rate limit
-  500, // internal server error (often transient at QBO)
-  502, // bad gateway (upstream proxy hiccup)
-  503, // service unavailable
-  504, // gateway timeout
+// 429 means the server explicitly rejected the request without processing it,
+// so retrying is always safe regardless of idempotency.
+const ALWAYS_RETRY_STATUSES: ReadonlySet<number> = new Set([429])
+
+// 5xx and network/timeout errors straddle "did not commit" and "committed but
+// response dropped." Safe to replay on idempotent reads; on non-idempotent
+// writes lacking a server-side request-key primitive, a retry after the upstream
+// committed would duplicate the write — so the classifier returns false for
+// these in `idempotent: false` mode.
+const IDEMPOTENT_ONLY_RETRY_STATUSES: ReadonlySet<number> = new Set([
+  500, 502, 503, 504,
 ])
 
 const RETRYABLE_NETWORK_CODES: ReadonlySet<string> = new Set([
@@ -23,36 +28,34 @@ const RETRYABLE_ERROR_NAMES: ReadonlySet<string> = new Set([
   // AbortController.abort() and retrying would defeat the cancellation.
 ])
 
+export type RetryOptions = {
+  /**
+   * False for non-idempotent writes (QBO create/update/void/delete) whose
+   * upstream has no request-key dedupe. In strict mode the classifier
+   * retries only on 429 and explicit `RetryableError.retry === true`;
+   * 5xx, network errors, and AbortSignal timeouts all bubble. Defaults
+   * to true (safe-to-replay reads).
+   */
+  idempotent?: boolean
+}
+
 /**
- * Centralized classifier for whether an error should trigger a retry.
- * Exported so it can be unit-tested independent of pRetry's timer plumbing.
- *
- * The error shapes inspected here come from different layers; a single
- * unified type doesn't exist, which is why the input is `unknown` and
- * each field is checked defensively:
+ * Classifies whether an error should trigger a retry. 429 and explicit
+ * `RetryableError.retry === true` retry in both modes. Strict mode
+ * (`options.idempotent === false`) short-circuits past 429: 5xx, network
+ * codes, AbortSignal timeouts, and undici fetch-failed envelopes are all
+ * treated as possibly-after-commit and never replayed.
  *
- *   - `RetryableError` (ours, src/utils/error.ts) — explicit retry flag.
- *   - `HttpFetchError` (ours) and Copilot SDK's `StatusableError` —
- *     `status: number` set after a non-2xx response was received.
- *   - undici (Node fetch) network failures — thrown as
- *     `TypeError: fetch failed` with the underlying error on `.cause`
- *     (e.g. `{ code: 'ECONNRESET' }`). No HTTP response was ever built,
- *     so there is no status to inspect.
- *   - `AbortSignal.timeout()` rejects with a `DOMException` whose
- *     `name` is `'TimeoutError'` (retryable). `AbortController.abort()`
- *     produces `'AbortError'` and is NOT retried (deliberate cancellation).
- *   - Top-level `error.code` is checked defensively for legacy Node
- *     error paths; in current Node fetch the code lives under `.cause`.
- *
- * Retry-nesting hazard: do not call a `withRetry`-wrapped function from
- * inside another `withRetry`-wrapped function. With the broadened retry
- * set, worst-case wait is `outer × inner × per_call_timeout`, which can
- * blow past the 300s webhook execution budget. Inside `IntuitAPI._*`
- * methods that are themselves wrapped at the public level (see exports
- * at the bottom of `src/utils/intuitAPI.ts`), call the unwrapped `_*`
- * counterparts directly (e.g. `this._customQuery`, not `this.customQuery`).
+ * Retry-nesting hazard: don't call a wrapped function from inside another.
+ * Inside `IntuitAPI._*` methods call the unwrapped `_*` counterparts
+ * (e.g. `this._customQuery`, not `this.customQuery`).
  */
-export const isRetryableError = (error: unknown): boolean => {
+export const isRetryableError = (
+  error: unknown,
+  options: RetryOptions = {},
+): boolean => {
+  const { idempotent = true } = options
+
   if (error instanceof RetryableError) return error.retry
 
   if (typeof error !== 'object' || error === null) return false
@@ -65,7 +68,16 @@ export const isRetryableError = (error: unknown): boolean => {
     cause?: unknown
   }
 
-  if (typeof err.status === 'number' && RETRYABLE_HTTP_STATUSES.has(err.status))
+  if (typeof err.status === 'number' && ALWAYS_RETRY_STATUSES.has(err.status))
+    return true
+
+  // Strict mode: nothing past this point is post-commit-safe.
+  if (!idempotent) return false
+
+  if (
+    typeof err.status === 'number' &&
+    IDEMPOTENT_ONLY_RETRY_STATUSES.has(err.status)
+  )
     return true
 
   if (typeof err.name === 'string' && RETRYABLE_ERROR_NAMES.has(err.name))
@@ -104,6 +116,7 @@ export const isRetryableError = (error: unknown): boolean => {
 export const withRetry = async <T>(
   fn: (...args: any[]) => Promise<T>,
   args: any[],
+  options: RetryOptions = {},
 ): Promise<T> => {
   let isEventProcessorRegistered = false
 
@@ -150,7 +163,7 @@ export const withRetry = async <T>(
           error,
         )
       },
-      shouldRetry: (error: unknown) => isRetryableError(error),
+      shouldRetry: (error: unknown) => isRetryableError(error, options),
     },
   )
 }
diff --git a/src/utils/intuit.ts b/src/utils/intuit.ts
index 299510ed..6273985c 100644
--- a/src/utils/intuit.ts
+++ b/src/utils/intuit.ts
@@ -1,5 +1,5 @@
 import { IntuitOAuthError } from '@/app/api/core/exceptions/custom'
-import { withRetry } from '@/app/api/core/utils/withRetry'
+import { RetryOptions, withRetry } from '@/app/api/core/utils/withRetry'
 import {
   intuitClientId,
   intuitClientSecret,
@@ -79,10 +79,16 @@ export default class Intuit {
     return tokenInfo
   }
 
+  // `createToken` / `refreshAccessToken` consume single-use credentials with
+  // no Intuit-side dedupe; a post-commit retry sees `invalid_grant` and
+  // would be misdiagnosed as revocation by `tokenRefresh.handleInvalidGrant`.
+  // So the wrapper defaults to strict, same as `IntuitAPI.wrapWithRetry`.
   private wrapWithRetry<Args extends unknown[], R>(
     fn: (...args: Args) => Promise<R>,
+    options?: RetryOptions,
   ): (...args: Args) => Promise<R> {
-    return (...args: Args): Promise<R> => withRetry(fn.bind(this), args)
+    return (...args: Args): Promise<R> =>
+      withRetry(fn.bind(this), args, { idempotent: false, ...options })
   }
 
   authorizeUri = this.wrapWithRetry(this._authorizeUri)
diff --git a/src/utils/intuitAPI.ts b/src/utils/intuitAPI.ts
index 41b1a9bd..e00271d0 100644
--- a/src/utils/intuitAPI.ts
+++ b/src/utils/intuitAPI.ts
@@ -1,5 +1,5 @@
 import APIError from '@/app/api/core/exceptions/api'
-import { withRetry } from '@/app/api/core/utils/withRetry'
+import { RetryOptions, withRetry } from '@/app/api/core/utils/withRetry'
 import { intuitApiMinorVersion, intuitBaseUrl } from '@/config'
 import { QBPortalConnectionSelectSchemaType } from '@/db/schema/qbPortalConnections'
 import { getFetcher, postFetcher } from '@/helper/fetch.helper'
@@ -789,14 +789,17 @@ export default class IntuitAPI {
 
   private wrapWithRetry<Args extends unknown[], R>(
     fn: (...args: Args) => Promise<R>,
+    options?: RetryOptions,
   ): (...args: Args) => Promise<R> {
-    return (...args: Args): Promise<R> => withRetry(fn.bind(this), args)
+    return (...args: Args): Promise<R> =>
+      withRetry(fn.bind(this), args, { idempotent: false, ...options })
   }
 
-  // Wrap convention: writes + customQuery are wrapped here. Read methods
-  // (get*) compose customQuery and stay unwrapped to avoid nested withRetry
-  // (see withRetry.ts).
-  customQuery = this.wrapWithRetry(this._customQuery)
+  // Writes default to `idempotent: false` (no QBO request-key dedupe — a
+  // post-commit retry would duplicate). `customQuery` is the one read in
+  // this set and opts back into broad retry. `get*` stay unwrapped and
+  // inherit retry via `customQuery` (see withRetry.ts on nesting).
+  customQuery = this.wrapWithRetry(this._customQuery, { idempotent: true })
   createInvoice = this.wrapWithRetry(this._createInvoice)
   createCustomer = this.wrapWithRetry(this._createCustomer)
   createItem = this.wrapWithRetry(this._createItem)
diff --git a/test/unit/core/withRetry.test.ts b/test/unit/core/withRetry.test.ts
index d4234198..f28448e6 100644
--- a/test/unit/core/withRetry.test.ts
+++ b/test/unit/core/withRetry.test.ts
@@ -52,6 +52,59 @@ describe('isRetryableError', () => {
     expect(isRetryableError(undefined)).toBe(false)
     expect(isRetryableError('oops')).toBe(false)
   })
+
+  describe('idempotent: false (write-safety mode)', () => {
+    it('retries 429 — server explicitly rejected without committing', () => {
+      expect(
+        isRetryableError(Object.assign(new Error(), { status: 429 }), {
+          idempotent: false,
+        }),
+      ).toBe(true)
+    })
+
+    it('honors explicit RetryableError.retry regardless of idempotent option', () => {
+      expect(
+        isRetryableError(new RetryableError(500, 'x', true), {
+          idempotent: false,
+        }),
+      ).toBe(true)
+      expect(
+        isRetryableError(new RetryableError(500, 'x', false), {
+          idempotent: false,
+        }),
+      ).toBe(false)
+    })
+
+    it('does NOT retry 5xx — commit-then-dropped-response would duplicate', () => {
+      for (const status of [500, 502, 503, 504]) {
+        expect(
+          isRetryableError(Object.assign(new Error(), { status }), {
+            idempotent: false,
+          }),
+        ).toBe(false)
+      }
+    })
+
+    it('does NOT retry network errors — ECONNRESET/ETIMEDOUT can occur mid-response', () => {
+      const econnreset = Object.assign(new TypeError('fetch failed'), {
+        cause: { code: 'ECONNRESET' },
+      })
+      expect(isRetryableError(econnreset, { idempotent: false })).toBe(false)
+
+      const etimedout = Object.assign(new Error('boom'), { code: 'ETIMEDOUT' })
+      expect(isRetryableError(etimedout, { idempotent: false })).toBe(false)
+
+      const fetchFailed = new TypeError('fetch failed')
+      expect(isRetryableError(fetchFailed, { idempotent: false })).toBe(false)
+    })
+
+    it('does NOT retry AbortSignal.timeout — could fire after request committed', () => {
+      const timeout = Object.assign(new Error('timed out'), {
+        name: 'TimeoutError',
+      })
+      expect(isRetryableError(timeout, { idempotent: false })).toBe(false)
+    })
+  })
 })
 
 describe('withRetry', () => {
@@ -181,4 +234,53 @@ describe('withRetry', () => {
     // 1 initial + 4 retries = 5 total
     expect(fn).toHaveBeenCalledTimes(5)
   })
+
+  it('does not retry 5xx when idempotent: false', async () => {
+    const error = Object.assign(new Error('upstream 503'), { status: 503 })
+    const fn = vi.fn().mockRejectedValue(error)
+
+    await expect(withRetry(fn, [], { idempotent: false })).rejects.toThrow(
+      'upstream 503',
+    )
+    expect(fn).toHaveBeenCalledTimes(1)
+  })
+
+  it('does not retry network errors when idempotent: false', async () => {
+    const error = Object.assign(new TypeError('fetch failed'), {
+      cause: { code: 'ECONNRESET' },
+    })
+    const fn = vi.fn().mockRejectedValue(error)
+
+    await expect(withRetry(fn, [], { idempotent: false })).rejects.toThrow(
+      'fetch failed',
+    )
+    expect(fn).toHaveBeenCalledTimes(1)
+  })
+
+  it('still retries 429 when idempotent: false — server explicitly rejected', async () => {
+    const error = Object.assign(new Error('rate limited'), { status: 429 })
+    const fn = vi
+      .fn()
+      .mockRejectedValueOnce(error)
+      .mockResolvedValueOnce('recovered')
+
+    const promise = withRetry(fn, [], { idempotent: false })
+    await vi.runAllTimersAsync()
+
+    expect(await promise).toBe('recovered')
+    expect(fn).toHaveBeenCalledTimes(2)
+  })
+
+  it('still retries RetryableError(retry=true) when idempotent: false', async () => {
+    const fn = vi
+      .fn()
+      .mockRejectedValueOnce(new RetryableError(500, 'transient', true))
+      .mockResolvedValueOnce('ok')
+
+    const promise = withRetry(fn, [], { idempotent: false })
+    await vi.runAllTimersAsync()
+
+    expect(await promise).toBe('ok')
+    expect(fn).toHaveBeenCalledTimes(2)
+  })
 })