Skip to content

fix(qbo): send job-site ShipAddr on milestone invoices; fail on total drift - #431

Open
Clarion1631 wants to merge 1 commit into
mainfrom
fix/qbo-milestone-ship-addr
Open

fix(qbo): send job-site ShipAddr on milestone invoices; fail on total drift#431
Clarion1631 wants to merge 1 commit into
mainfrom
fix/qbo-milestone-ship-addr

Conversation

@Clarion1631

Copy link
Copy Markdown
Owner

Why

QBO Automated Sales Tax rates an invoice by its ShipAddr. The ProBuild→QBO milestone push never sent one, so QBO used the company address (Vancouver, 8.9%) and re-rated the Berg ADU job (Winlock, 8.0%): INV-00177-2 became $15,125 for a $15,000 milestone, leaving a phantom $125 balance. (That invoice was corrected by hand in QBO on 2026-08-29; this PR stops it recurring on the next non-Vancouver job.)

What

  • qbShipAddrFor() in wa-tax.ts: street + WA zip from the project location (reusing parseLocationText), else the client's structured address, else null → no ShipAddr (previous behavior). Partial or project-name-prefixed addresses are never sent, since a wrong jurisdiction is worse than the company default. WA zip range 98001–99403.
  • createQBMilestoneInvoice takes shipAddr and sends it.
  • Both push paths (pushMilestoneToQuickBooks, stageProgressBillingToQuickBooksCore) now require QBO's grand total to match the milestone within one cent; otherwise the QBO invoice is deleted and the push fails with a message pointing at the job-site address / tax rate. Previously a console.warn on the first path and no check on the second.

Verification

  • tsc --noEmit clean; npm run test:unit 566/566 (new tests/qb-ship-addr.test.ts, 7 cases).
  • Codex review, 2 rounds: round-1 blockers (free-form Line1, unsafe parser reuse, missing total invariant on the staging path) all addressed; round-2 items (tolerance, zip bounds) addressed. Not added: push-path serialization / QBO-mismatch integration tests (no QBO mock harness in the unit tier; tsc enforces the wiring).

Note

CLAUDE.md still says "lib/quickbooks.ts sends pre-tax lines and lets QBO compute its own tax" — that's stale (the push has sent TxnTaxDetail since the tax-line work); worth a one-line doc fix separately.

🤖 Generated with Claude Code

https://claude.ai/code/session_019zwfh13xTH7W9qeqRrd7Sm

…il on total drift

QBO Automated Sales Tax rates an invoice by its ShipAddr. The push never sent
one, so QBO fell back to the company address (Vancouver, 8.9%) and re-rated a
Winlock job (8.0%): INV-00177-2 came out $15,125 for a $15,000 milestone and
left a phantom $125 balance (fixed by hand in QBO 2026-08-29).

- wa-tax.ts: export parseLocationText; add qbShipAddrFor() — street + WA zip
  from the project location, else the client's structured address, else null
  (no ShipAddr, the old behavior). Partial/prefixed addresses are never sent.
- quickbooks.ts: createQBMilestoneInvoice accepts shipAddr and sends ShipAddr.
- quickbooks-payments.ts / progress-billing.ts: pass the address; the QBO
  grand total must match the milestone within one cent or the QBO invoice is
  deleted and the push fails (was a console.warn on the milestone path, and
  unchecked on the progress-billing path).
- tests/qb-ship-addr.test.ts: parser/fallback/zip-boundary cases.

Codex-reviewed, two rounds.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019zwfh13xTH7W9qeqRrd7Sm
@vercel

vercel Bot commented Aug 29, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
probuild Ready Ready Preview Aug 29, 2026 11:49pm

Request Review

@github-actions

Copy link
Copy Markdown
  1. The total invariant is bypassed for existing QBO links. quickbooks-payments.ts returns status?.total without validating it. billing-core.ts bypasses the push function for linked invoices and retains a five-cent threshold at line 894. Consequently, a 2–5¢ drift can still be emailed, while NaN can bypass every comparison. This contradicts the advertised one-cent, fail-closed invariant. Centralize the check, reject missing/non-finite totals, and apply it to both created and reused invoices before reporting success or sending. Intuit confirms TotalAmt includes taxes and is recalculated by QBO. Intuit Invoice documentation

  2. The supposedly conservative address validation still emits ambiguous project text. wa-tax.ts treats anything beginning with a digit as a street, so "2026 Berg ADU, 204 SW Kerron St, Winlock, WA 98596" sends Line1: "2026 Berg ADU" instead of falling back to the client. Line 37 also selects the last WA-shaped five-digit token, so a trailing job/unit identifier can replace the real ZIP. That directly violates the claim that project-name-prefixed or ambiguous addresses are never sent. Reject these ambiguous shapes and add regression tests for numeric project prefixes and trailing five-digit identifiers.

VERDICT: REQUEST_CHANGES

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.

1 participant