Skip to content

OUT-3528: notify IUs when QBO sync fails with manual-fix errors - #238

Merged
SandipBajracharya merged 14 commits into
masterfrom
OUT-3528
May 20, 2026
Merged

OUT-3528: notify IUs when QBO sync fails with manual-fix errors#238
SandipBajracharya merged 14 commits into
masterfrom
OUT-3528

Conversation

@SandipBajracharya

@SandipBajracharya SandipBajracharya commented Apr 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

When QBO returns an error caused by a manual change in QuickBooks that our app can't auto-resolve (e.g. 6140 duplicate doc number, 6210 closed accounting period, 6540 deposited transaction locked), surface an in-product + email notification to the workspace's IUs via the existing Copilot notification path. Until now these failures landed silently in qb_sync_logs with no IU-facing surfacing.

  • 11 manual-fix QBO codes routed to tailored notification copy: 6140, 6240, 6210, 6540, 610, 2500, 6190, 6000, 5010, 620, 2390.
  • Single dispatch hook at the sync log write site — fires on the row's transition into FAILED (PENDING/SUCCESS → FAILED). Retries don't re-page; partial updates that don't touch status short-circuit.
  • 5010 on products / customers is suppressed (auto-recovers via updateProductSyncToken / updateCustomerSyncToken on the next 3-hour cron tick). 5010 on invoices remains user-actionable since the invoice flow uses cached qbSyncToken directly with no equivalent refresh.
  • Sync history CSV now exposes the error_code column so IUs can filter past failures by QBO code.

Notification copy

A supervisor-facing recap of every body string is in docs/OUT-3528-notification-recap.md (local-only — docs/ is gitignored). Awaiting product / supervisor sign-off on copy before merge.

How it works

  1. Schema — new nullable qb_sync_logs.error_code varchar(50). Zero-downtime migration.
  2. PlumbinggetMessageAndCodeFromError returns the QBO Fault.Error.code (e.g. 6140) instead of the HTTP status when the error originates from QBO. Each FAILED-path call site (webhook, payment, sync resync) threads the code into createQBSyncLog / updateQBSyncLog.
  3. RegistryUserActionableErrorCodes: Record<string, NotificationActions> in src/constant/intuitErrorCode.ts.
  4. CopyNotificationCopy registry in src/app/api/notification/notification.helper.ts holds title + in-product body + email subject/header/body per action. IU_RECIPIENT_ACTIONS is derived from the registry keys.
  5. DispatchSyncErrorNotifier is fired from SyncLogService.scheduleFailureNotification via afterIfAvailable so the request scope (and any enclosing transaction) commits first; notifier failures are captured to Sentry and never undo the sync log write.

Commits

  1. feat(OUT-3528): add error_code column to qb_sync_logs
  2. feat(OUT-3528): plumb QBO error codes into sync log writes
  3. feat(OUT-3528): IU notifications for user-actionable QBO errors
  4. feat(OUT-3528): dispatch sync-failure notifications via SyncErrorNotifier
  5. test(OUT-3528): unit coverage for notifier and helper

Test plan

  • yarn tsc --noEmit clean
  • yarn lint:check no new errors / warnings on touched files
  • yarn vitest run --project unit — 94/94 passing
  • Integration tests (vitest run --project integration) — needs Docker for testcontainers/postgresql, deferred to CI / a Docker-capable env
  • Manual sandbox verification — trigger a 6140 in QBO sandbox by setting "Custom transaction numbers" preference and pushing two invoices with the same DocNumber; confirm Copilot bell + email arrive once and don't repeat on retry

Reviewer notes

  • Bodies for 5010 and 620 intentionally avoid prescribing specific fixes (e.g. "undo the void") because QBO's stale-object / link-failed errors don't tell us what changed in QuickBooks — only that our cached state is incompatible. The copy lists possibilities and asks the IU to verify the current state.
  • 6240 branches on entityType: items live in a separate QBO namespace from Customer/Vendor/Employee, so the item-flow copy doesn't claim cross-namespace collision.
  • AUTH_RECONNECT flow is pre-existing and untouched; only integrated into the new derived IU_RECIPIENT_ACTIONS set.

🤖 Generated with Claude Code

@linear-code

linear-code Bot commented Apr 28, 2026

Copy link
Copy Markdown

@vercel

vercel Bot commented Apr 28, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
quickbooks-sync Building Building May 19, 2026 0:07am
quickbooks-sync (dev) Ready Ready Preview, Comment May 19, 2026 0:07am

Request Review

@SandipBajracharya SandipBajracharya changed the title feat(OUT-3528): notify IUs when QBO sync fails with manual-fix errors OUT-3528: notify IUs when QBO sync fails with manual-fix errors Apr 28, 2026
@SandipBajracharya
SandipBajracharya marked this pull request as ready for review April 29, 2026 04:41
@SandipBajracharya

Copy link
Copy Markdown
Collaborator Author

@greptileai

@greptile-apps

greptile-apps Bot commented Apr 29, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds IU-facing in-product and email notifications for 11 user-actionable QBO sync failure codes (5010, 6190, 6210, 6240, 6430, 6540, 610, 620, 2390, 2500, 6000), routing them through the existing Copilot notification path via a new SyncErrorNotifier class fired from SyncLogService on every PENDING/SUCCESS → FAILED transition.

  • Schema + plumbing: A new nullable qb_sync_logs.error_code varchar(50) column captures the raw QBO fault code at each FAILED write site (webhook, payment, sync resync); the column is also exposed in the sync-history CSV export.
  • Registry + copy: UserActionableErrorCodes maps QBO codes to NotificationActions; NotificationCopy holds per-action title/body builders for both in-product and email channels; suppression logic prevents 5010 on PRODUCT entity (auto-recovers via updateProductSyncToken) from paging IUs.
  • Dispatch guard: afterIfAvailable defers notification until the request scope commits; errors in the notifier are swallowed and forwarded to Sentry, never rolling back the sync log write.

Confidence Score: 4/5

Safe to merge with the outstanding notification flood risk for workspace-level errors already flagged in a previous review cycle.

The core dispatch path is correct and well-tested. Pre-existing TOCTOU race and 6190 per-entity flood remain open but were not introduced here. The only new finding is a data-quality gap in error_code for non-Intuit failures.

syncLog.service.ts (TOCTOU race) and syncErrorNotifier.ts (6190 flood) carry pre-existing flagged concerns not resolved in this iteration.

Important Files Changed

Filename Overview
src/app/api/quickbooks/syncLog/syncErrorNotifier.ts New file — getActionForErrorCode registry lookup, getEntityKey identifier picker, SyncErrorNotifier.notify with 5010/PRODUCT suppression; logic is correct for the three existing EntityTypes.
src/app/api/quickbooks/syncLog/syncLog.service.ts Adds scheduleFailureNotification hook at createQBSyncLog and updateQBSyncLog; transition guard (priorStatus != FAILED) is correct; TOCTOU race on the findFirst+update pair is a pre-existing flagged concern.
src/app/api/notification/notification.helper.ts Refactored from map-returning functions to single-action lookup; buildEntityReference guards against empty segments; all 11 NotificationActions covered in NotificationCopy.
src/utils/error.ts New HttpFetchError branch extracts QBO Fault.Error[0].code when source=intuit; source field populated correctly; call sites store errorCode without a source guard, so HTTP statuses are stored for non-Intuit failures.
src/constant/intuitErrorCode.ts Adds 9 new QBO error codes and the UserActionableErrorCodes registry; 6140 correctly omitted (auto-retry handles it); 6430 added but not mentioned in PR description (minor doc inconsistency).
src/app/api/notification/notification.service.ts IU_RECIPIENT_ACTIONS derived from NotificationCopy keys ensures compile-time exhaustiveness; context threaded through sendNotificationToIU/createBulkNotification; Sentry capture added for dispatch failures.
src/app/api/quickbooks/webhook/webhook.service.ts Adds errorCode field at 4 FAILED-path write sites; code extracted without source guard, so non-Intuit HTTP statuses will be stored in error_code.
src/app/api/quickbooks/payment/payment.service.ts Adds errorCode field to FAILED-path sync log write; same source-guard absence as webhook.service.ts.
src/app/api/quickbooks/sync/sync.service.ts Adds errorCode field to FAILED-path sync log update; same source-guard absence as webhook.service.ts.
src/db/migrations/20260519083948_add_error_code_in_qb_sync_logs.sql Zero-downtime migration: adds nullable varchar(50) error_code column with no default and no backfill required.
src/db/schema/qbSyncLogs.ts Adds errorCode varchar(50) nullable column; schema comment clearly documents intent as QBO-only; exposed correctly in prepareSyncLogsForDownload CSV.
src/app/api/core/types/notification.ts Adds 10 new NotificationActions enum values and NotificationContext interface; all values covered in NotificationCopy.

Sequence Diagram

sequenceDiagram
    participant W as Webhook/Payment/Sync Handler
    participant SLS as SyncLogService
    participant AIA as afterIfAvailable
    participant SEN as SyncErrorNotifier
    participant NS as NotificationService
    participant CP as CopilotAPI

    W->>W: "getMessageAndCodeFromError(e) → {code, source}"
    W->>SLS: "updateQBSyncLog({status:FAILED, errorCode:'6210', ...})"
    SLS->>SLS: findFirst(conditions) → priorStatus
    SLS->>SLS: db.update().set(payload).returning() → log
    SLS->>AIA: scheduleFailureNotification(log) [deferred]
    Note over SLS,AIA: request scope + TX commit first
    AIA->>SEN: notify(log)
    SEN->>SEN: getActionForErrorCode('6210') → QB_CLOSED_PERIOD
    SEN->>SEN: "entityType===PRODUCT and action===QB_STALE_OBJECT? skip"
    SEN->>SEN: buildContext(log)
    SEN->>SEN: "getPortalConnection(workspaceId) → {intiatedBy}"
    SEN->>NS: sendNotificationToIU(senderId, QB_CLOSED_PERIOD, ctx)
    NS->>NS: IU_RECIPIENT_ACTIONS.has(action) → getInternalUsers()
    NS->>CP: getInternalUsers() → IU list
    loop for each IU
        NS->>CP: createNotification(inProduct, email)
    end
Loading

Reviews (4): Last reviewed commit: "fix(OUT-3528): narrow 5010 suppression t..." | Re-trigger Greptile

Comment thread src/utils/error.ts
Comment thread src/app/api/notification/notification.helper.ts
Comment thread src/app/api/notification/notification.service.ts
@priosshrsth

Copy link
Copy Markdown
Collaborator

@greptileai Review the PR again.

@priosshrsth priosshrsth left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Comment thread src/app/api/quickbooks/syncLog/syncErrorNotifier.ts
SandipBajracharya and others added 10 commits May 19, 2026 16:25
Persists the QBO Fault.Error.code on each sync log row so failures can
be routed to a notification action and filtered in the sync history
CSV by error type. Nullable varchar(50) — zero-downtime migration.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
getMessageAndCodeFromError now returns the Intuit Fault.Error.code
(e.g. 6140) instead of the HTTP status when the error originates
from QBO; renames IntuitErrorType.Code → code to match the actual
payload casing. Each FAILED-path createQBSyncLog / updateQBSyncLog
call site (webhook, payment, sync resync) now passes the resulting
errorCode so it lands in qb_sync_logs.error_code.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Maps 11 manual-fix QBO error codes (6140, 6240, 6210, 6540, 610,
2500, 6190, 6000, 5010, 620, 2390) to NotificationActions and
provides per-action in-product + email copy via a single
NotificationCopy registry. Bodies interpolate "(during X, ref Y)"
from log entityType/eventType + identifier so IUs can pinpoint the
failing record. Two appended advisories (MANUAL_EDIT_NOTE for
recoverable failures, FINAL_FAILURE_NOTE for 5010 invoice).

5010 + 6240 branch on entityType (separate QBO namespaces);
6000/6190 keep cause-honest framing without manual-edit advisory.
IU_RECIPIENT_ACTIONS in NotificationService is derived from the
registry keys so a new action can't be added without an opt-in.
Sentry capture added for dispatch failures.

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

SyncLogService schedules an IU notification on every transition
INTO LogStatus.FAILED (PENDING/SUCCESS → FAILED). Retries that keep
a row FAILED do not re-page; partial updates that don't set status
short-circuit before any DB lookup. Dispatch runs in afterIfAvailable
so the request scope (and any enclosing transaction) commits first;
notifier failures are captured to Sentry and never undo the sync log
write.

5010 stale-object on products and customers is suppressed because
those flows pre-fetch SyncToken via update{Product,Customer}SyncToken
and recover automatically on the next 3-hour cron tick. The invoice
flow uses the cached qbSyncToken directly with no equivalent refresh,
so 5010 on invoices remains user-actionable and surfaces.

Sync history CSV now exposes the error_code column so IUs can
filter past failures by QBO code.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
notification.helper.test: AUTH_RECONNECT regression guard, per-action
non-empty body assertions, 6240 entityType branching (item-only copy
must not leak Customer/Vendor/Employee), 5010 invoice "failure is
final" copy.

syncErrorNotifier.test: registry lookups (known + unknown codes,
empty/null), entity-key fallback ladder, status-not-FAILED skip,
unknown-code skip, 5010 suppression on product/customer/payment
(parameterized) vs dispatch on invoice, getPortalConnection-null
senderId fallback to ''.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both unit test files had hand-maintained action lists that could
silently miss new entries. Replaces them with derivations:
- syncErrorNotifier.test: it.each over Object.entries(UserActionableErrorCodes)
- notification.helper.test: it.each over Object.values(NotificationActions)
  (no manual filter — AUTH_RECONNECT is asserted explicitly elsewhere
  and runs through the smoke loop too)

A new code in the registry or a new enum value now picks up smoke
coverage automatically.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Greptile flagged that buildEntityReference unconditionally interpolated
both action and id even when describeAction returned '' (entityType or
eventType missing), producing "(during , ref INV-001)". Today's
qb_sync_logs schema prevents this on the real path (entityType +
eventType are NOT NULL), but the helper is callable from anywhere
and a future caller passing partial context would surface broken copy.

Conditional segment-building drops a missing dimension instead of
emitting an empty placeholder; if both are absent the parenthetical
is omitted entirely. Three regression tests added.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Copilot payment.succeeded webhook triggers a Fee (Purchase entity)
creation in QuickBooks — calling it "payment completion" in IU
notifications is misleading. Special-case the (payment, succeeded)
pair in describeAction so the reference clause reads "during invoice
fees creation, ref PUR-…" instead.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
QBO 6430 ("Invalid account type used") fires when a transaction
references an account whose AccountType doesn't match what QuickBooks
expects — e.g. a Purchase line citing a non-Expense account, or an
Item citing a non-Income account. Most plausible cause for us is an
account configured under settings (expense/income) being changed to
a different type in QuickBooks, or the wrong account picked at
setup.

Body explains the cause and points the IU at both remediation paths
(fix the account type in QuickBooks or change the selection in this
app's settings). Auto-coverage from the parameterized helper +
notifier tests picks up the new entry without manual additions.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
SandipBajracharya and others added 3 commits May 19, 2026 17:08
postFetcher throws HttpFetchError on any non-2xx, short-circuiting
before intuitAPI's assertNotQBFault can run. The HttpFetchError branch
of getMessageAndCodeFromError was returning HTTP status (usually 400)
as `code`, which then landed in qb_sync_logs.error_code and bypassed
the UserActionableErrorCodes registry (keyed by QBO codes like 5010).

Surface the Fault.Error[0].code via QBFaultSchema.safeParse on
error.body when source is intuit; fall back to error.status when no
Fault is present (timeouts, 5xx, etc.).

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

6140 Duplicate Document Number is now auto-recovered by the
suffix-retry in InvoiceService#webhookInvoiceCreated (OUT-3754), so
surfacing an IU notification for it is noise. Remove the
QB_DUPLICATE_DOC_NUMBER action and its copy entry. Pathological exit
paths (21-char limit, MAX_SUFFIX_ATTEMPTS exhaustion) still throw 6140
but go silent — acceptable per product call.

While here, extend QBOErrorCodes to cover every code in
UserActionableErrorCodes (12 entries, numeric order) and switch the
registry to computed property names so the source-of-truth lives in
one place and a typo'd code can't slip through. AccountErrorCodes is
unaffected.

Tests: helper smoke loops + notifier dispatch fixture now use
QB_CLOSED_PERIOD (6210) instead of the removed action.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two IU-facing strings in the QB_CLOSED_PERIOD entry used the lowercase
"Quickbooks"; surrounding copy uses "QuickBooks" everywhere else.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@SandipBajracharya
SandipBajracharya changed the base branch from preview to master May 19, 2026 11:33
@SandipBajracharya

Copy link
Copy Markdown
Collaborator Author

@greptileai

Greptile flagged that `entityType !== INVOICE` silently swallowed 5010
on PAYMENT (and any future non-invoice entity) without an auto-recovery
path. PRODUCT is the only entityType with an actual SyncToken refresh
(updateProductSyncToken); flip the guard to an explicit allow-list so
INVOICE and PAYMENT 5010s both surface to IUs.

Tests: suppression now asserts only on product; dispatch asserts on
both invoice and payment via it.each.

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

Copy link
Copy Markdown
Collaborator Author

@greptileai

@SandipBajracharya
SandipBajracharya merged commit eb44b4a into master May 20, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants