Skip to content

Commit 838cb4e

Browse files
chore(OUT-4005): add test typecheck script + CI job, fix stale test types
Adds yarn typecheck:test (tsc over test/tsconfig.json) and a CI job so type errors in tests gate PRs. Fixes the pre-existing type errors it surfaces (missing bankAccountRef fixtures, stale UnitPrice input, string status/id) and loads the ambient shims in test/tsconfig. Also adds CLAUDE.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 65f07cc commit 838cb4e

9 files changed

Lines changed: 186 additions & 6 deletions

File tree

.github/workflows/test.yml

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,27 @@ name: CI
33
on: pull_request
44

55
jobs:
6+
typecheck:
7+
name: Typecheck tests
8+
runs-on: ubuntu-latest
9+
10+
steps:
11+
- name: Check out Git repository
12+
uses: actions/checkout@v4
13+
14+
- name: Set up Node.js
15+
uses: actions/setup-node@v4
16+
with:
17+
node-version: 22.14.0
18+
cache: yarn
19+
cache-dependency-path: './yarn.lock'
20+
21+
- name: Install dependencies
22+
run: yarn install
23+
24+
- name: Typecheck test files
25+
run: yarn typecheck:test
26+
627
run-tests:
728
name: Run tests
829
runs-on: ubuntu-latest

CLAUDE.md

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
# CLAUDE.md
2+
3+
This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.
4+
5+
## What this app is
6+
7+
A multi-tenant Next.js (App Router) service that synchronizes Copilot / Assembly workspaces with QuickBooks Online (QBO). It runs on Vercel, persists state in Postgres (Supabase in prod, Drizzle ORM throughout), and reacts to Copilot webhooks (`invoice.created/updated/paid/voided/deleted`, `product.updated`, `price.created`, `payment.succeeded`) by mirroring those entities into the corresponding QBO realm.
8+
9+
A "portal" is one Copilot/Assembly workspace bonded to one QuickBooks realm. Almost every table is keyed by `portalId`; almost every service derives `this.user.workspaceId` from the request token and scopes everything to that portal.
10+
11+
## Common commands
12+
13+
Package manager is **Yarn 4 (Berry)**, Node **22.14.0** (`.nvmrc`).
14+
15+
```bash
16+
yarn install # install
17+
yarn dev # Next dev (Turbopack)
18+
yarn build # next build (CI uses build.sh which also runs drizzle-kit migrate)
19+
yarn lint:check # ESLint over src/ and test/
20+
yarn prettier:check # Prettier check
21+
yarn lint:fix # ESLint --fix
22+
yarn prettier:fix # Prettier write
23+
24+
# Tests (Vitest, two projects defined in vitest.config.ts)
25+
yarn test # both: unit then integration (groupOrder enforces this)
26+
yarn test:watch # watch
27+
yarn test:coverage # v8 coverage
28+
npx vitest run --project unit # only unit
29+
npx vitest run --project integration # only integration
30+
npx vitest run test/integration/quickbooks/priceCreated/happyPath.test.ts # single file
31+
npx vitest run -t 'happy path' # by test-name pattern
32+
33+
# Trigger.dev (background task runtime)
34+
yarn trigger:dev # local dev worker
35+
yarn trigger:deploy # deploy tasks
36+
37+
# DB migrations (Drizzle Kit, schema lives at src/db/schema/)
38+
npx drizzle-kit generate # create new migration from schema changes
39+
npx drizzle-kit migrate # apply pending migrations to DATABASE_URL
40+
41+
# One-off operational scripts (tsx, see src/cmd/*)
42+
yarn cmd:rename-qb-accounts
43+
yarn cmd:backfill-product-info
44+
yarn cmd:sync-missed-invoices
45+
yarn cmd:sync-missed-products
46+
```
47+
48+
Husky `pre-commit` runs `lint-staged` (eslint --fix + prettier --write on `src/**/*.{ts,tsx}`). CI (`.github/workflows/test.yml`) runs `yarn test` on PRs; `.github/workflows/lint.yml` runs lint+prettier on every push. CI assumes the testcontainers Postgres image is available (Docker is preinstalled on `ubuntu-latest`).
49+
50+
## Architecture
51+
52+
### Request → handler shape
53+
54+
Every API route follows the same skeleton:
55+
56+
```
57+
src/app/api/<area>/<feature>/
58+
route.ts # exports { POST/GET } = withErrorHandler(controllerFn); sets maxDuration
59+
<feature>.controller.ts # auth + Sentry scope + parse + delegate to service
60+
<feature>.service.ts # extends BaseService; orchestrates DB + external APIs
61+
```
62+
63+
Controllers call `authenticate(req)` (`src/app/api/core/utils/authenticate.ts`), which reads `?token=…`, asks Copilot to decrypt it, and returns a `User` (`src/app/api/core/models/User.model.ts`). `User` carries `workspaceId` (= portalId), role, and the lazily-attached `qbConnection` (service-item / client-fee refs).
64+
65+
`withErrorHandler` (`src/app/api/core/utils/withErrorHandler.ts`) is the **only** error path. It maps `ZodError` / `APIError` / `CopilotApiError` / `RetryableError` / Intuit OAuth + Axios errors to HTTP responses and forwards categorized exceptions to Sentry. Don't add try/catch in route handlers — throw and let this wrapper format.
66+
67+
### BaseService and the DB singleton
68+
69+
Services extend `BaseService` (`src/app/api/core/services/base.service.ts`), which holds:
70+
71+
- `this.db` — the **module-level Drizzle singleton** from `src/db/index.ts` (`DBClient.getInstance()`); `casing: 'snake_case'`.
72+
- `this.user` — the authenticated `User` for the request.
73+
- `setTransaction(tx)` / `unsetTransaction()` — swap `this.db` for a transaction handle inside a `db.transaction(...)` callback, then restore.
74+
75+
**Pitfall (known, see `memory/project_unsetTransaction_bug.md`):** `unsetTransaction()` is sometimes called inside the transaction callback or skipped on error paths — across `BaseService` subclasses this leaves the singleton pointed at a closed tx. When introducing or modifying transactional code, audit that `setTransaction` / `unsetTransaction` are paired in `try/finally` and that nested service calls share the tx handle.
76+
77+
The DB singleton is also why test helpers (`test/helpers/seed.ts`, `test/helpers/testDb.ts`) import `@/db` directly — see `docs/why-test-helpers-use-the-app-db-singleton.md`. Don't introduce a separate test-only Drizzle client; tests must read what the app writes.
78+
79+
### Webhook flow (the central path)
80+
81+
`POST /api/quickbooks/webhook``WebhookService.handleWebhookEvent` (`src/app/api/quickbooks/webhook/webhook.service.ts`) is a switch on `payload.eventType` that dispatches to `InvoiceService` / `ProductService` / `PaymentService`. A few things to know before changing it:
82+
83+
1. **Idempotency is enforced via `qb_sync_logs` claim rows.** `SyncLogService.claimWebhookEvent({ copilotId, entityType, eventType, … })` returns `{ claimed: false }` if a row already exists; handlers exit early. Any new webhook handler must call `claimWebhookEvent` before doing real work or duplicate processing will leak into QBO.
84+
2. **`qb_sync_logs.quickbooks_id` is polymorphic.** Its meaning depends on `(entityType, eventType)` — for `INVOICE/PAID` it stores the QBO **Payment** ID, not the Invoice ID. See `memory/project_qb_sync_logs_semantics.md`.
85+
3. **Pre-claim sleeps for ordering.** `INVOICE_UPDATED` / `INVOICE_VOIDED` / `PAYMENT_SUCCEEDED` sleep before `claimWebhookEvent` so a companion event (e.g., `INVOICE_CREATED`) can claim first. The `delayMs` lives in the handler, not the caller — keep it that way; moving the sleep after the claim re-opens the race.
86+
4. **Setting flags gate handlers.** `PRICE_CREATED` / `PRODUCT_UPDATED` no-op when `createNewProductFlag` is false; `PAYMENT_SUCCEEDED` no-ops when `absorbedFeeFlag` is false or there's no platform-paid fee. Read `qb_settings` via `SettingService` rather than passing flags around.
87+
5. **There's a known TOCTOU race on `claimWebhookEvent`** — accepted, parked, will be addressed with an advisory lock + dedupe job, not a rewrite. See `memory/project_qb_sync_logs_toctou_parked.md`.
88+
89+
### Token refresh
90+
91+
QBO access tokens expire in ~1h, refresh tokens in ~100 days. `src/utils/intuitAPI.ts` sends authenticated requests; `src/utils/tokenRefresh.ts` (`getValidQbTokens`) refreshes when stale. The `vercel.json` cron `/api/quickbooks/refresh-tokens` runs daily at 06:00 UTC to keep refresh tokens warm. There's a known silent-401 bug — expired tokens cause `null` returns from `getFetchWithHeader/postFetchWithHeaders`; the planned fix is auto-refresh inside those helpers (design at `docs/intuit-api-token-refresh.md`, summary in `memory/project_intuit_api_token_refresh.md`).
92+
93+
### Background work
94+
95+
- **Vercel crons** (`vercel.json`):
96+
- `/api/quickbooks/cron` every 12h — kicks off `processResyncForFailedRecords` (Trigger.dev task) to retry failed sync logs. Auth via `Bearer ${CRON_SECRET}`.
97+
- `/api/quickbooks/refresh-tokens` daily 06:00 UTC.
98+
- **Trigger.dev** tasks live in `src/trigger/` (config at `trigger.config.ts`, runtime: node, default 3 retries, `maxDuration: 3600s`). Sentry source maps are uploaded only when `VERCEL_ENV === 'production'`.
99+
100+
## Multi-tenancy invariant
101+
102+
Every `WHERE` clause that touches a portal-scoped table needs `portalId = this.user.workspaceId`. Forgetting this leaks one tenant's data into another. The unique indexes on `qb_sync_logs` and `qb_invoice_sync` (see migrations 20260427100328 / 20260427055352) enforce some of this at the DB level, but most of it is service-layer discipline.
103+
104+
## Database & schema
105+
106+
- Drizzle schemas in `src/db/schema/*` registered in `src/db/schema/index.ts`. Relations in `relation.ts`.
107+
- Migrations in `src/db/migrations/` (prefix `supabase`, generated by drizzle-kit). The `init.sql` (20250701) defines all enums; subsequent files alter.
108+
- Custom column helpers in `src/db/helper/column.helper.ts` (`timestamps`) and enum bridge in `drizzle.helper.ts` (`enumToPgEnum`).
109+
- `qb_payments` table exists but is currently unused (reserved for future) — no rows in prod. See `memory/project_qb_payments_unused.md`.
110+
- Type-safe Zod schemas come from `drizzle-zod` (`createInsertSchema` / `createSelectSchema`); reuse those rather than hand-rolling Zod for DB rows.
111+
112+
## Testing
113+
114+
- Two Vitest **projects** in `vitest.config.ts``unit` (mock-heavy, isolated) and `integration` (real Postgres via testcontainers). Run order is enforced via `sequence.groupOrder` (unit=0, integration=1).
115+
- Integration project is configured **`pool: 'forks'` + `fileParallelism: false` + `isolate: false`** so all integration tests share one Postgres container _and_ one app DB connection. Don't change these without reading `docs/vitest-gotchas.md` and `docs/why-test-helpers-use-the-app-db-singleton.md`.
116+
- `.env.test` is loaded by `test/integration/globalSetup.ts` with `override: true` so a developer's local `.env` can't leak into tests. `DATABASE_URL` is intentionally **not** in `.env.test` — globalSetup sets it from the container's URI before any worker imports `src/config`.
117+
- Module mocks for integration are in `test/integration/setup.ts``@/utils/copilotAPI`, `@/utils/intuitAPI`, and `@sentry/nextjs` must be mocked with **explicit factories** (and Intuit/Copilot mock implementations must use `function`, not `=>`, because the code does `new IntuitAPI(...)`). See `docs/vitest-gotchas.md` items 1–3.
118+
- Test helpers in `test/helpers/`: `seed.ts` (`seedHealthyPortal`, `TEST_PORTAL_ID`, etc.), `webhook.ts` (`postWebhook` via `next-test-api-route-handler`), `testDb.ts` (`truncateAllTestTables`).
119+
- Test-data philosophy in `docs/test-data-dos-and-donts.md`: static fixtures for the thing under test, factories with explicit overrides for single-dimension variants, **no faker** in fixtures or assertions.
120+
121+
## Path aliases
122+
123+
```
124+
@/* → src/*
125+
@test/* → test/*
126+
```
127+
128+
Configured in `tsconfig.json` and propagated to Vitest via `vite-tsconfig-paths` (per-project in `vitest.config.ts`).
129+
130+
## Style notes
131+
132+
- Prettier: single quotes, no semis, trailing comma all (`.prettierrc`).
133+
- ESLint: `next/core-web-vitals` + TypeScript; `prefer-const` and `no-var` are errors; unused-var underscore prefix is exempt; `@typescript-eslint/no-explicit-any` is disabled (the codebase uses `any` deliberately at framework boundaries).
134+
- Tailwind v4 + `copilot-design-system`. UI surface is small (settings dashboard + OAuth callback) — most work happens in the API/service layer.
135+
- The `docs/` folder is **gitignored** (per `.gitignore`) and used for local decision notes — design docs, post-mortems, comparison tables. Save non-trivial tradeoff discussions there rather than in code comments or commit messages.
136+
137+
## Things to read before non-trivial changes
138+
139+
- `docs/testcontainers-vs-local-supabase.md` — why integration tests use testcontainers, not the local Supabase stack.
140+
- `docs/why-test-helpers-use-the-app-db-singleton.md` — why test helpers import `@/db` and what would break if you opened a separate client.
141+
- `docs/vitest-gotchas.md` — the five real traps already hit in this project.
142+
- `docs/test-data-dos-and-donts.md` — the test-data rules.
143+
- `docs/intuit-api-token-refresh.md` — design for the silent-401 fix.
144+
145+
## What this repo doesn't have
146+
147+
- No design system / shared component library — UI is a thin dashboard, mostly settings forms.
148+
- No GraphQL, no tRPC — plain Next.js Route Handlers + service classes.
149+
- No DI container — `BaseService` reads `db` from a module singleton; tests work _with_ that constraint, not around it.
150+
- No existing CLAUDE.md until this one.
151+
152+
## Engineering notes
153+
154+
- After a successful implementation, the changes will be reviewed by the team lead and greptileAI in github.
155+
- Do not use let unless absolutely necessary. Use const instead.
156+
- Always keep the comments short, on point and easy to understand with easy wordings. This is must.
157+
- Follow DRY, KISS, SOLID, YAGNI principles.

package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@
2626
"cmd:sync-missed-products": "tsx src/cmd/syncMissedProducts/index.ts",
2727
"test": "vitest run",
2828
"test:watch": "vitest",
29-
"test:coverage": "vitest run --coverage"
29+
"test:coverage": "vitest run --coverage",
30+
"typecheck:test": "tsc --noEmit -p test/tsconfig.json"
3031
},
3132
"dependencies": {
3233
"@sentry/nextjs": "^9.13.0",

test/integration/quickbooks/invoiceCreated/lazyItemCreation.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ describe('POST /api/quickbooks/webhook — invoice.created (lazy item creation f
4444
.where(
4545
eq(
4646
QBProductSync.productId,
47-
invoiceCreatedPayload.data.lineItems[0].productId,
47+
invoiceCreatedPayload.data.lineItems[0].productId!,
4848
),
4949
)
5050
expect(rows).toHaveLength(1)

test/integration/quickbooks/invoicePaid/frozenIntentRouting.test.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ describe('POST /api/quickbooks/webhook — invoice.paid routes off the frozen in
2121
await seedQBInvoiceSync({
2222
customerId: customer.id,
2323
isBatchedDeposit: true,
24-
status: 'open',
2524
})
2625
await seedInvoiceCreatedLog()
2726

@@ -41,7 +40,6 @@ describe('POST /api/quickbooks/webhook — invoice.paid routes off the frozen in
4140
await seedQBInvoiceSync({
4241
customerId: customer.id,
4342
isBatchedDeposit: false,
44-
status: 'open',
4543
})
4644
await seedInvoiceCreatedLog()
4745

test/tsconfig.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
{
22
"extends": "../tsconfig.json",
3-
"include": ["**/*.ts"],
3+
// intuit-oauth ambient shim the root config loads; needed for src/config.
4+
"include": ["../src/type/intuit.d.ts", "**/*.ts"],
45
"exclude": ["node_modules"]
56
}

test/unit/utils/intuitAPI.accounts.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ const baseTokens: IntuitAPITokensType = {
3232
assetAccountRef: 'asset',
3333
serviceItemRef: 'service',
3434
clientFeeRef: 'client-fee',
35+
bankAccountRef: 'bank',
3536
}
3637

3738
type Row = {

test/unit/utils/intuitAPI.responses.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ const baseTokens: IntuitAPITokensType = {
3535
assetAccountRef: 'asset',
3636
serviceItemRef: 'service',
3737
clientFeeRef: 'client-fee',
38+
bankAccountRef: 'bank',
3839
}
3940

4041
function makeApi() {
@@ -361,7 +362,6 @@ describe('IntuitAPI POST-based writes', () => {
361362
const api = makeApi()
362363
const result = await api.createItem({
363364
Name: 'Widget',
364-
UnitPrice: 25,
365365
Type: 'Service' as never,
366366
Taxable: false,
367367
})

test/unit/utils/intuitAPI.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ const baseTokens: IntuitAPITokensType = {
5252
assetAccountRef: 'asset',
5353
serviceItemRef: 'service',
5454
clientFeeRef: 'client-fee',
55+
bankAccountRef: 'bank',
5556
}
5657

5758
// Builds a customer row in the shape QBO returns inside `QueryResponse.Customer`.

0 commit comments

Comments
 (0)