From 3675079a4cd43f146c23cad92b60625fc8b21ce1 Mon Sep 17 00:00:00 2001 From: whekin Date: Thu, 5 Mar 2026 21:13:38 +0400 Subject: [PATCH] fix(review): address CodeRabbit review feedback - Guard prepare script for Docker builds without .git - Add pre-push hook for heavier quality gates (typecheck/test/build) - Pin drizzle-orm version in scripts/package.json - Add E2E_SMOKE_ALLOW_WRITE opt-in guard via e2eEnv abstraction - Create @household/config env-e2e.ts using same t3-env + zod pattern - Make e2e teardown robust with optional chaining + allSettled - Fix markdown code block language identifier (MD040) - Fix CI integration docs to reflect actual workflow --- bun.lock | 3 +- docs/runbooks/e2e-tests.md | 12 +++--- lefthook.yml | 10 +++++ package.json | 2 +- packages/config/src/env-e2e.ts | 21 +++++++++++ packages/config/src/index.ts | 1 + scripts/e2e/billing-flow.ts | 67 +++++++++++++++++++--------------- scripts/package.json | 3 +- 8 files changed, 81 insertions(+), 38 deletions(-) create mode 100644 packages/config/src/env-e2e.ts diff --git a/bun.lock b/bun.lock index 12b03ee..ebf3887 100644 --- a/bun.lock +++ b/bun.lock @@ -72,8 +72,9 @@ "scripts": { "name": "@household/scripts", "devDependencies": { + "@household/config": "workspace:*", "@household/db": "workspace:*", - "drizzle-orm": "*", + "drizzle-orm": "^0.44.5", }, }, }, diff --git a/docs/runbooks/e2e-tests.md b/docs/runbooks/e2e-tests.md index 1c33b83..3db6f5a 100644 --- a/docs/runbooks/e2e-tests.md +++ b/docs/runbooks/e2e-tests.md @@ -14,6 +14,7 @@ smoke test for the billing pipeline. It exercises: - Bun 1.3+ installed - A running Supabase/Postgres database with the schema applied - `DATABASE_URL` set (via `.env` or environment) +- `E2E_SMOKE_ALLOW_WRITE=true` set explicitly (safety guard) ## Running locally @@ -26,7 +27,7 @@ cp .env.example .env bun run db:migrate # 3. Run the e2e smoke test -bun run test:e2e +E2E_SMOKE_ALLOW_WRITE=true bun run test:e2e ``` The test seeds its own data (household + 3 roommates), runs the full @@ -37,7 +38,7 @@ cleans up after itself. On success: -``` +```text E2E smoke passed: purchase ingestion, utility updates, and statements are deterministic ``` @@ -45,9 +46,10 @@ On failure the script exits with code 1 and prints the assertion error. ## CI integration -The e2e smoke test runs in CI as part of the quality matrix when the -`DATABASE_URL` secret is configured. Without the secret, the job is -skipped automatically. +Run the e2e smoke test with `bun run test:e2e` locally or in a dedicated +CI job. If you wire it into CI, gate it on `DATABASE_URL` and +`E2E_SMOKE_ALLOW_WRITE` to avoid false failures. The test is **not** +part of the standard CI quality matrix by default. ## Test data diff --git a/lefthook.yml b/lefthook.yml index a729917..3de18ee 100644 --- a/lefthook.yml +++ b/lefthook.yml @@ -9,3 +9,13 @@ pre-commit: lint: glob: '*.{ts,tsx,js,jsx}' run: bun run lint + +pre-push: + parallel: true + commands: + typecheck: + run: bun run typecheck + test: + run: bun run test + build: + run: bun run build diff --git a/package.json b/package.json index 74545fd..8b2025e 100644 --- a/package.json +++ b/package.json @@ -11,7 +11,7 @@ "build": "bun run --filter '*' build", "typecheck": "bun run --filter '*' typecheck", "test": "bun run --filter '*' test", - "prepare": "lefthook install", + "prepare": "[ -d .git ] && lefthook install || true", "lint": "oxlint .", "lint:fix": "oxlint --fix .", "format": "bunx oxfmt .", diff --git a/packages/config/src/env-e2e.ts b/packages/config/src/env-e2e.ts new file mode 100644 index 0000000..3dda18a --- /dev/null +++ b/packages/config/src/env-e2e.ts @@ -0,0 +1,21 @@ +import { createEnv } from '@t3-oss/env-core' +import { z } from 'zod' + +const server = { + DATABASE_URL: z.string().url(), + E2E_SMOKE_ALLOW_WRITE: z + .enum(['true', 'false']) + .default('false') + .transform((v) => v === 'true') +} + +export const e2eEnv = createEnv({ + server, + runtimeEnv: process.env, + emptyStringAsUndefined: true, + onValidationError: (issues) => { + console.error('Invalid e2e environment variables:') + console.error(JSON.stringify(issues, null, 2)) + throw new Error('E2E environment validation failed') + } +}) diff --git a/packages/config/src/index.ts b/packages/config/src/index.ts index 40238d6..4c860d9 100644 --- a/packages/config/src/index.ts +++ b/packages/config/src/index.ts @@ -1 +1,2 @@ export { env } from './env' +export { e2eEnv } from './env-e2e' diff --git a/scripts/e2e/billing-flow.ts b/scripts/e2e/billing-flow.ts index 1eafd58..ef36a40 100644 --- a/scripts/e2e/billing-flow.ts +++ b/scripts/e2e/billing-flow.ts @@ -3,6 +3,7 @@ import { randomUUID } from 'node:crypto' import { eq } from 'drizzle-orm' +import { e2eEnv } from '@household/config' import { createDbClient, schema } from '@household/db' import { createTelegramBot } from '../../apps/bot/src/bot' @@ -12,11 +13,12 @@ import { registerPurchaseTopicIngestion } from '../../apps/bot/src/purchase-topic-ingestion' -const databaseUrl = process.env.DATABASE_URL -if (!databaseUrl) { - throw new Error('DATABASE_URL is required for e2e smoke test') +if (!e2eEnv.E2E_SMOKE_ALLOW_WRITE) { + throw new Error('Set E2E_SMOKE_ALLOW_WRITE=true to run e2e smoke test') } +const databaseUrl: string = e2eEnv.DATABASE_URL + const chatId = '-100123456' const purchaseTopicId = 77 const commandChatIdNumber = -100123456 @@ -118,15 +120,9 @@ async function run(): Promise { carol: '900003' } - const coreClient = createDbClient(databaseUrl as string, { - max: 2, - prepare: false - }) - - const ingestionClient = createPurchaseMessageRepository(databaseUrl as string) - const financeService = createFinanceCommandsService(databaseUrl as string, { - householdId: ids.household - }) + let coreClient: ReturnType | undefined + let ingestionClient: ReturnType | undefined + let financeService: ReturnType | undefined const bot = createTelegramBot('000000:test-token') const replies: string[] = [] @@ -154,19 +150,29 @@ async function run(): Promise { return { ok: true, result: true } as any }) - registerPurchaseTopicIngestion( - bot, - { - householdId: ids.household, - householdChatId: chatId, - purchaseTopicId - }, - ingestionClient.repository - ) - - financeService.register(bot) - try { + coreClient = createDbClient(databaseUrl, { + max: 2, + prepare: false + }) + + ingestionClient = createPurchaseMessageRepository(databaseUrl) + financeService = createFinanceCommandsService(databaseUrl, { + householdId: ids.household + }) + + registerPurchaseTopicIngestion( + bot, + { + householdId: ids.household, + householdChatId: chatId, + purchaseTopicId + }, + ingestionClient.repository + ) + + financeService.register(bot) + await coreClient.db.insert(schema.households).values({ id: ids.household, name: 'E2E Smoke Household' @@ -303,12 +309,13 @@ async function run(): Promise { 'E2E smoke passed: purchase ingestion, utility updates, and statements are deterministic' ) } finally { - await coreClient.db.delete(schema.households).where(eq(schema.households.id, ids.household)) - - await Promise.all([ - coreClient.queryClient.end({ timeout: 5 }), - ingestionClient.close(), - financeService.close() + await Promise.allSettled([ + coreClient + ? coreClient.db.delete(schema.households).where(eq(schema.households.id, ids.household)) + : undefined, + coreClient?.queryClient.end({ timeout: 5 }), + ingestionClient?.close(), + financeService?.close() ]) } } diff --git a/scripts/package.json b/scripts/package.json index af74a54..3fd96a5 100644 --- a/scripts/package.json +++ b/scripts/package.json @@ -6,7 +6,8 @@ "typecheck": "tsgo --project tsconfig.json --noEmit" }, "devDependencies": { - "drizzle-orm": "*", + "drizzle-orm": "^0.44.5", + "@household/config": "workspace:*", "@household/db": "workspace:*" } }