fix(14): revise plans based on checker feedback
This commit is contained in:
@@ -89,7 +89,7 @@ bun add postgres @electric-sql/pglite
|
||||
bun remove better-sqlite3 @types/better-sqlite3
|
||||
```
|
||||
|
||||
**Step 2: Rewrite `src/db/schema.ts`** — Clean rewrite per D-01. Replace all `sqliteTable` with `pgTable`, all imports from `drizzle-orm/sqlite-core` with `drizzle-orm/pg-core`.
|
||||
**Step 2: Rewrite `src/db/schema.ts`** -- Clean rewrite per D-01. Replace all `sqliteTable` with `pgTable`, all imports from `drizzle-orm/sqlite-core` with `drizzle-orm/pg-core`.
|
||||
|
||||
Column type mapping (apply to ALL 12 tables):
|
||||
- `integer("id").primaryKey({ autoIncrement: true })` -> `serial("id").primaryKey()`
|
||||
@@ -157,7 +157,7 @@ export async function seedDefaults() {
|
||||
}
|
||||
```
|
||||
|
||||
**Step 6: Update `src/shared/types.ts`** — No changes needed to the file content itself. The types infer from schema which still exports the same table names. Verify the file still compiles after schema change.
|
||||
**Step 6: Update `src/shared/types.ts`** -- No changes needed to the file content itself. The types infer from schema which still exports the same table names. Verify the file still compiles after schema change.
|
||||
|
||||
**Step 7: Update `drizzle.config.ts`** per D-02:
|
||||
```typescript
|
||||
@@ -174,7 +174,7 @@ export default defineConfig({
|
||||
```
|
||||
</action>
|
||||
<verify>
|
||||
<automated>grep -q "pgTable" src/db/schema.ts && grep -q "drizzle-orm/pg-core" src/db/schema.ts && grep -q "postgres-js" src/db/index.ts && grep -q "postgresql" drizzle.config.ts && grep -q "async function seedDefaults" src/db/seed.ts && echo "PASS" || echo "FAIL"</automated>
|
||||
<automated>grep -q "pgTable" src/db/schema.ts && grep -q "drizzle-orm/pg-core" src/db/schema.ts && grep -q "postgres-js" src/db/index.ts && grep -q "postgresql" drizzle.config.ts && grep -q "async function seedDefaults" src/db/seed.ts && bun run lint 2>&1 | tail -3 && echo "PASS" || echo "FAIL"</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- src/db/schema.ts contains `import { boolean, doublePrecision, integer, pgTable, serial, text, timestamp } from "drizzle-orm/pg-core"`
|
||||
@@ -226,7 +226,7 @@ Key changes from current:
|
||||
- `migrate` from `drizzle-orm/pglite/migrator` instead of `drizzle-orm/bun-sqlite/migrator`
|
||||
- Function is now `async` (returns Promise)
|
||||
- No `Database` import from `bun:sqlite`
|
||||
- No `":memory:"` — PGlite creates an in-memory Postgres instance by default
|
||||
- No `":memory:"` -- PGlite creates an in-memory Postgres instance by default
|
||||
- Migration folder changed to `./drizzle-pg`
|
||||
- `db.insert(...).values(...).run()` becomes `await db.insert(...).values(...)`
|
||||
|
||||
@@ -237,15 +237,15 @@ bunx drizzle-kit generate
|
||||
|
||||
This reads the updated `drizzle.config.ts` (dialect: "postgresql", schema: src/db/schema.ts) and generates SQL migration files in `drizzle-pg/`.
|
||||
|
||||
**Step 3: Verify migration was generated:**
|
||||
**Step 3: Verify migration was generated and is complete:**
|
||||
```bash
|
||||
ls drizzle-pg/
|
||||
cat drizzle-pg/*.sql
|
||||
```
|
||||
|
||||
Confirm the SQL contains `CREATE TABLE` statements for all 12 tables with correct Postgres types (serial, text, timestamp, boolean, double precision, etc.).
|
||||
Confirm the SQL contains `CREATE TABLE` statements for all 12 tables with correct Postgres types (serial, text, timestamp, boolean, double precision, etc.). Count the CREATE TABLE statements -- there must be at least 12 (categories, items, threads, thread_candidates, setups, setup_items, settings, users, sessions, api_keys, oauth_clients, oauth_codes, oauth_tokens).
|
||||
|
||||
**Step 4: Quick smoke test — verify PGlite test helper works:**
|
||||
**Step 4: Quick smoke test -- verify PGlite test helper works:**
|
||||
```bash
|
||||
bun -e "
|
||||
import { createTestDb } from './tests/helpers/db.ts';
|
||||
@@ -258,7 +258,7 @@ console.log('PGlite test helper works!');
|
||||
```
|
||||
</action>
|
||||
<verify>
|
||||
<automated>ls drizzle-pg/*.sql && grep -q "drizzle-orm/pglite" tests/helpers/db.ts && grep -q "async function createTestDb" tests/helpers/db.ts && echo "PASS" || echo "FAIL"</automated>
|
||||
<automated>ls drizzle-pg/*.sql && grep -c "CREATE TABLE" drizzle-pg/*.sql | tail -1 | grep -qE "^drizzle-pg/.*:1[2-9]$|^drizzle-pg/.*:[2-9][0-9]$" || { echo "WARNING: verify CREATE TABLE count manually"; }; grep -q "drizzle-orm/pglite" tests/helpers/db.ts && grep -q "async function createTestDb" tests/helpers/db.ts && bun run lint 2>&1 | tail -3 && echo "PASS" || echo "FAIL"</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- tests/helpers/db.ts contains `import { drizzle } from "drizzle-orm/pglite"`
|
||||
@@ -267,10 +267,11 @@ console.log('PGlite test helper works!');
|
||||
- tests/helpers/db.ts contains `migrationsFolder: "./drizzle-pg"`
|
||||
- tests/helpers/db.ts does NOT contain `bun:sqlite` or `drizzle-orm/bun-sqlite` or `.run()`
|
||||
- drizzle-pg/ directory exists with at least one .sql migration file
|
||||
- Migration SQL contains CREATE TABLE for categories, items, threads, thread_candidates, setups, setup_items, settings, users, sessions, api_keys, oauth_clients, oauth_codes, oauth_tokens
|
||||
- Migration SQL contains CREATE TABLE for all 12+ tables (categories, items, threads, thread_candidates, setups, setup_items, settings, users, sessions, api_keys, oauth_clients, oauth_codes, oauth_tokens)
|
||||
- `grep -c "CREATE TABLE" drizzle-pg/*.sql` shows at least 12 CREATE TABLE statements
|
||||
- PGlite smoke test (bun -e script above) exits 0
|
||||
</acceptance_criteria>
|
||||
<done>Test helper returns async PGlite Drizzle instance. Initial PostgreSQL migration generated in drizzle-pg/. Smoke test confirms PGlite can apply migrations and seed data.</done>
|
||||
<done>Test helper returns async PGlite Drizzle instance. Initial PostgreSQL migration generated in drizzle-pg/ with all 12+ CREATE TABLE statements. Smoke test confirms PGlite can apply migrations and seed data.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
@@ -279,11 +280,13 @@ console.log('PGlite test helper works!');
|
||||
- `grep -r "sqliteTable\|bun:sqlite\|drizzle-orm/sqlite-core\|drizzle-orm/bun-sqlite" src/db/ drizzle.config.ts tests/helpers/db.ts` returns NO matches
|
||||
- `grep -c "pgTable" src/db/schema.ts` returns 12+ (one per table, possibly more from import)
|
||||
- `ls drizzle-pg/*.sql` shows at least one migration file
|
||||
- `grep -c "CREATE TABLE" drizzle-pg/*.sql` shows at least 12 tables
|
||||
- PGlite smoke test exits 0
|
||||
- `bun run lint` passes
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
All database foundation files rewritten for PostgreSQL. Schema uses pg-core types. DB connection uses postgres.js. Test helper uses PGlite. Initial migration generated. No SQLite references remain in these files.
|
||||
All database foundation files rewritten for PostgreSQL. Schema uses pg-core types. DB connection uses postgres.js. Test helper uses PGlite. Initial migration generated with all 12+ tables. No SQLite references remain in these files. Lint passes.
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
|
||||
@@ -90,7 +90,7 @@ Conversion rules (apply to ALL service files):
|
||||
<action>
|
||||
Convert each service file following the async conversion rules. Read each file fully before modifying.
|
||||
|
||||
**item.service.ts** — 5 exported functions (getAllItems, getItemById, createItem, updateItem, duplicateItem, deleteItem):
|
||||
**item.service.ts** -- 5 exported functions (getAllItems, getItemById, createItem, updateItem, duplicateItem, deleteItem):
|
||||
- `getAllItems`: `async`, remove `.all()`, `return await db.select()...`
|
||||
- `getItemById`: `async`, replace `.get() ?? null` with `const [row] = await db.select()...; return row ?? null`
|
||||
- `createItem`: `async`, replace `.returning().get()` with `const [row] = await db.insert()...returning(); return row`
|
||||
@@ -98,29 +98,29 @@ Convert each service file following the async conversion rules. Read each file f
|
||||
- `duplicateItem`: `async`, same pattern as createItem
|
||||
- `deleteItem`: `async`, existence check `const [item] = await db.select()...`, delete `await db.delete()...`
|
||||
|
||||
**category.service.ts** — Has a transaction in `deleteCategory` (moves items to Uncategorized then deletes):
|
||||
**category.service.ts** -- Has a transaction in `deleteCategory` (moves items to Uncategorized then deletes):
|
||||
- All functions: `async`
|
||||
- Transaction: `await db.transaction(async (tx) => { await tx.update()...; await tx.delete()...; })`
|
||||
- All `.all()` -> remove, `.get()` -> destructure, `.run()` -> remove
|
||||
|
||||
**thread.service.ts** — Has transactions in `resolveThread` and `unresolveThread`:
|
||||
**thread.service.ts** -- Has transactions in `resolveThread` and `unresolveThread`:
|
||||
- All functions: `async`
|
||||
- `resolveThread` transaction: `await db.transaction(async (tx) => { ... })` with all inner operations awaited
|
||||
- `unresolveThread` transaction: same pattern
|
||||
- `.all()` -> remove, `.get()` -> destructure, `.run()` -> remove
|
||||
- `.returning().get()` -> `const [row] = await ...returning()`
|
||||
|
||||
**setup.service.ts** — Has a transaction in `updateSetupItems` (delete all + re-insert):
|
||||
**setup.service.ts** -- Has a transaction in `updateSetupItems` (delete all + re-insert):
|
||||
- All functions: `async`
|
||||
- Transaction: `await db.transaction(async (tx) => { await tx.delete()...; for (const item of items) { await tx.insert()...; } })`
|
||||
- `.all()` -> remove, `.get()` -> destructure, `.run()` -> remove
|
||||
|
||||
**totals.service.ts** — Read-only aggregate queries:
|
||||
**totals.service.ts** -- Read-only aggregate queries:
|
||||
- All functions: `async`
|
||||
- Remove `.all()`, `.get()` -> destructure
|
||||
</action>
|
||||
<verify>
|
||||
<automated>! grep -n "\.all()\|\.get()\|\.run()" src/server/services/item.service.ts src/server/services/category.service.ts src/server/services/thread.service.ts src/server/services/setup.service.ts src/server/services/totals.service.ts && grep -c "async function" src/server/services/item.service.ts | grep -q "[3-9]" && echo "PASS" || echo "FAIL"</automated>
|
||||
<automated>! grep -n "\.all()\|\.get()\|\.run()" src/server/services/item.service.ts src/server/services/category.service.ts src/server/services/thread.service.ts src/server/services/setup.service.ts src/server/services/totals.service.ts && grep -c "async function" src/server/services/item.service.ts | grep -q "[3-9]" && bun run lint 2>&1 | tail -3 && echo "PASS" || echo "FAIL"</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- item.service.ts: every exported function starts with `export async function`
|
||||
@@ -135,30 +135,30 @@ Convert each service file following the async conversion rules. Read each file f
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 2: Convert auth/oauth/csv/image services and update server index</name>
|
||||
<name>Task 2: Convert auth/oauth/csv/image services, update server index, and run PGlite smoke test</name>
|
||||
<files>src/server/services/auth.service.ts, src/server/services/oauth.service.ts, src/server/services/csv.service.ts, src/server/services/image.service.ts, src/server/index.ts</files>
|
||||
<read_first>src/server/services/auth.service.ts, src/server/services/oauth.service.ts, src/server/services/csv.service.ts, src/server/services/image.service.ts, src/server/index.ts</read_first>
|
||||
<action>
|
||||
**auth.service.ts** — User and session management:
|
||||
**auth.service.ts** -- User and session management:
|
||||
- All functions: `async`
|
||||
- Remove `.all()`, `.get()` -> destructure, `.run()` -> remove
|
||||
- `.returning().get()` -> `const [row] = await ...returning()`
|
||||
- Pay attention to boolean checks on `oauthCodes.used` — the column is now native `boolean` (true/false), not integer (0/1). If any code checks `=== 0` or `=== 1` for the `used` field, change to `=== false` or `=== true`.
|
||||
- Pay attention to boolean checks on `oauthCodes.used` -- the column is now native `boolean` (true/false), not integer (0/1). If any code checks `=== 0` or `=== 1` for the `used` field, change to `=== false` or `=== true`.
|
||||
|
||||
**oauth.service.ts** — OAuth client, code, token management:
|
||||
**oauth.service.ts** -- OAuth client, code, token management:
|
||||
- All functions: `async`
|
||||
- Same conversion patterns
|
||||
- IMPORTANT: The `used` column on `oauthCodes` is now `boolean` type. Any `.set({ used: 1 })` must become `.set({ used: true })`. Any `.where(eq(oauthCodes.used, 0))` must become `.where(eq(oauthCodes.used, false))`.
|
||||
|
||||
**csv.service.ts** — CSV export:
|
||||
**csv.service.ts** -- CSV export:
|
||||
- All functions: `async`
|
||||
- This is read-only, straightforward `.all()` removal
|
||||
|
||||
**image.service.ts** — Image handling:
|
||||
**image.service.ts** -- Image handling:
|
||||
- All functions: `async`
|
||||
- Same conversion patterns. May have fewer DB calls than other services.
|
||||
|
||||
**src/server/index.ts** — Server startup:
|
||||
**src/server/index.ts** -- Server startup:
|
||||
- Change `seedDefaults()` to `await seedDefaults()` at the top level
|
||||
- Since the file is a module (ESM), top-level await is supported. Wrap the seed call:
|
||||
```typescript
|
||||
@@ -167,22 +167,39 @@ Convert each service file following the async conversion rules. Read each file f
|
||||
```
|
||||
- If the file structure does not support top-level await cleanly (e.g., exports are synchronous), wrap in an async IIFE or move the await before the export.
|
||||
- The `seedDefaults` import already points to the async version from Plan 01.
|
||||
|
||||
**After all conversions, run a PGlite smoke test to verify at least one service works end-to-end:**
|
||||
```bash
|
||||
bun -e "
|
||||
import { createTestDb } from './tests/helpers/db.ts';
|
||||
import * as schema from './src/db/schema.ts';
|
||||
const db = await createTestDb();
|
||||
// Test a basic item service operation
|
||||
const { createItem } = await import('./src/server/services/item.service.ts');
|
||||
const [cat] = await db.select().from(schema.categories);
|
||||
const item = await createItem(db as any, { name: 'Smoke Test', categoryId: cat.id, quantity: 1 });
|
||||
if (!item || !item.id) { console.error('FAIL: createItem returned no result'); process.exit(1); }
|
||||
console.log('Service smoke test PASSED: item created with id', item.id);
|
||||
"
|
||||
```
|
||||
This validates that the async conversion is actually functional, not just structurally correct.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>! grep -n "\.all()\|\.get()\|\.run()" src/server/services/auth.service.ts src/server/services/oauth.service.ts src/server/services/csv.service.ts src/server/services/image.service.ts && grep -q "await seedDefaults" src/server/index.ts && echo "PASS" || echo "FAIL"</automated>
|
||||
<automated>! grep -n "\.all()\|\.get()\|\.run()" src/server/services/auth.service.ts src/server/services/oauth.service.ts src/server/services/csv.service.ts src/server/services/image.service.ts && grep -q "await seedDefaults" src/server/index.ts && bun run lint 2>&1 | tail -3 && echo "PASS" || echo "FAIL"</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- auth.service.ts: every exported function is `async`
|
||||
- auth.service.ts: does NOT contain `.all()`, `.get()`, or `.run()`
|
||||
- oauth.service.ts: every exported function is `async`
|
||||
- oauth.service.ts: does NOT contain `.set({ used: 1 })` — uses `.set({ used: true })` instead
|
||||
- oauth.service.ts: does NOT contain `eq(oauthCodes.used, 0)` — uses `eq(oauthCodes.used, false)` instead
|
||||
- oauth.service.ts: does NOT contain `.set({ used: 1 })` -- uses `.set({ used: true })` instead
|
||||
- oauth.service.ts: does NOT contain `eq(oauthCodes.used, 0)` -- uses `eq(oauthCodes.used, false)` instead
|
||||
- csv.service.ts: every exported function is `async`, no `.all()` calls
|
||||
- image.service.ts: every exported function is `async`
|
||||
- src/server/index.ts: contains `await seedDefaults()`
|
||||
- No file in this set contains `.all()`, `.get()`, or `.run()` calls on db objects
|
||||
- PGlite smoke test creating an item via service function exits 0
|
||||
</acceptance_criteria>
|
||||
<done>Auth, OAuth, CSV, and image services fully async. OAuth boolean conversion complete. Server startup awaits async seed.</done>
|
||||
<done>Auth, OAuth, CSV, and image services fully async. OAuth boolean conversion complete. Server startup awaits async seed. PGlite smoke test confirms services work against async DB.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
@@ -191,10 +208,12 @@ Convert each service file following the async conversion rules. Read each file f
|
||||
- `grep -rn "\.all()\|\.get()\|\.run()" src/server/services/` returns NO matches (except possibly string literals in error messages)
|
||||
- `grep -c "async function" src/server/services/*.ts` shows every service has async functions
|
||||
- `grep "await seedDefaults" src/server/index.ts` returns a match
|
||||
- `bun run lint` passes
|
||||
- PGlite smoke test exits 0
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
All 9 service files use async/await for every database operation. No SQLite-only methods (.all, .get, .run) remain. Transactions use async callbacks. OAuth boolean conversion complete. Server index awaits async seed.
|
||||
All 9 service files use async/await for every database operation. No SQLite-only methods (.all, .get, .run) remain. Transactions use async callbacks. OAuth boolean conversion complete. Server index awaits async seed. PGlite smoke test validates at least one service works end-to-end. Lint passes.
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
|
||||
@@ -14,6 +14,7 @@ files_modified:
|
||||
- src/server/routes/images.ts
|
||||
- src/server/routes/settings.ts
|
||||
- src/server/routes/totals.ts
|
||||
- src/server/middleware/auth.ts
|
||||
autonomous: true
|
||||
requirements: [DB-01, DB-02]
|
||||
must_haves:
|
||||
@@ -21,6 +22,7 @@ must_haves:
|
||||
- "Every route handler awaits service function calls"
|
||||
- "All route handlers that call services are async"
|
||||
- "No route returns a Promise object instead of resolved data"
|
||||
- "Auth middleware awaits all DB queries for session and API key validation"
|
||||
artifacts:
|
||||
- path: "src/server/routes/items.ts"
|
||||
provides: "Async item route handlers"
|
||||
@@ -28,18 +30,25 @@ must_haves:
|
||||
- path: "src/server/routes/settings.ts"
|
||||
provides: "Async settings handlers with direct DB calls"
|
||||
contains: "await"
|
||||
- path: "src/server/middleware/auth.ts"
|
||||
provides: "Async auth middleware with awaited DB lookups"
|
||||
contains: "await"
|
||||
key_links:
|
||||
- from: "src/server/routes/*.ts"
|
||||
to: "src/server/services/*.ts"
|
||||
via: "await service function calls"
|
||||
pattern: "await .*(get|create|update|delete)"
|
||||
- from: "src/server/middleware/auth.ts"
|
||||
to: "src/db/schema.ts"
|
||||
via: "session and API key DB queries"
|
||||
pattern: "await.*db\\.select"
|
||||
---
|
||||
|
||||
<objective>
|
||||
Convert all 9 route handler files to properly await async service calls. Route handlers that call service functions must be async and await the results.
|
||||
Convert all 9 route handler files and the auth middleware to properly await async service calls and DB operations. Route handlers that call service functions must be async and await the results.
|
||||
|
||||
Purpose: With services now async (Plan 03), route handlers must await them. Missing awaits would return Promise objects as JSON responses instead of actual data.
|
||||
Output: All route files properly await service calls.
|
||||
Purpose: With services now async (Plan 03), route handlers must await them. Missing awaits would return Promise objects as JSON responses instead of actual data. The auth middleware queries sessions and API keys on every request -- these direct DB calls must also be async.
|
||||
Output: All route files and auth middleware properly await service/DB calls.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@@ -56,6 +65,7 @@ Output: All route files properly await service calls.
|
||||
|
||||
@src/server/routes/items.ts
|
||||
@src/server/routes/settings.ts
|
||||
@src/server/middleware/auth.ts
|
||||
</context>
|
||||
|
||||
<interfaces>
|
||||
@@ -63,12 +73,18 @@ Output: All route files properly await service calls.
|
||||
<!-- Services are imported and called: const items = await getAllItems(db) -->
|
||||
<!-- Settings route accesses DB directly (no service layer): await db.select().from(settings) -->
|
||||
<!-- Some handlers may already be async (for body parsing). Add await to service calls. -->
|
||||
<!-- Auth middleware queries sessions table and apiKeys table directly on every authenticated request -->
|
||||
|
||||
Conversion rules for routes:
|
||||
- Handler callback must be `async (c) => { ... }`
|
||||
- Every service call: `const result = serviceFunction(db, ...)` -> `const result = await serviceFunction(db, ...)`
|
||||
- Settings route has direct DB calls: add `await` and remove `.all()/.get()/.run()`
|
||||
- OAuth routes may have direct DB calls for token validation
|
||||
|
||||
Conversion rules for auth middleware:
|
||||
- Middleware function must be async
|
||||
- Session lookup: `db.select()...where(eq(sessions.id, ...))` -> add `await`, remove `.get()`, use destructuring
|
||||
- API key lookup: same pattern
|
||||
</interfaces>
|
||||
|
||||
<tasks>
|
||||
@@ -84,15 +100,15 @@ For each route file, read the full file first. Then:
|
||||
2. Add `await` before every service function call
|
||||
3. If any handler has direct DB calls (`.select()`, `.insert()`, etc.), apply the same rules as services: remove `.all()/.get()/.run()`, use destructuring for single rows
|
||||
|
||||
**items.ts** — Handlers call: `getAllItems(db)`, `getItemById(db, id)`, `createItem(db, data)`, `updateItem(db, id, data)`, `duplicateItem(db, id)`, `deleteItem(db, id)`. Add `await` before each.
|
||||
**items.ts** -- Handlers call: `getAllItems(db)`, `getItemById(db, id)`, `createItem(db, data)`, `updateItem(db, id, data)`, `duplicateItem(db, id)`, `deleteItem(db, id)`. Add `await` before each.
|
||||
|
||||
**categories.ts** — Handlers call: `getAllCategories(db)`, `createCategory(db, data)`, `updateCategory(db, id, data)`, `deleteCategory(db, id)`. Add `await` before each.
|
||||
**categories.ts** -- Handlers call: `getAllCategories(db)`, `createCategory(db, data)`, `updateCategory(db, id, data)`, `deleteCategory(db, id)`. Add `await` before each.
|
||||
|
||||
**threads.ts** — Handlers call: `getAllThreads(db)`, `getThreadById(db, id)`, `createThread(db, data)`, `updateThread(db, id, data)`, `deleteThread(db, id)`, `resolveThread(db, id, data)`, `unresolveThread(db, id)`, `addCandidate(db, data)`, `updateCandidate(db, id, data)`, `removeCandidate(db, id)`, `reorderCandidates(db, data)`. Add `await` before each.
|
||||
**threads.ts** -- Handlers call: `getAllThreads(db)`, `getThreadById(db, id)`, `createThread(db, data)`, `updateThread(db, id, data)`, `deleteThread(db, id)`, `resolveThread(db, id, data)`, `unresolveThread(db, id)`, `addCandidate(db, data)`, `updateCandidate(db, id, data)`, `removeCandidate(db, id)`, `reorderCandidates(db, data)`. Add `await` before each.
|
||||
|
||||
**setups.ts** — Handlers call: `getAllSetups(db)`, `getSetupById(db, id)`, `createSetup(db, data)`, `updateSetup(db, id, data)`, `deleteSetup(db, id)`, `updateSetupItems(db, id, data)`, `updateClassification(...)`. Add `await` before each.
|
||||
**setups.ts** -- Handlers call: `getAllSetups(db)`, `getSetupById(db, id)`, `createSetup(db, data)`, `updateSetup(db, id, data)`, `deleteSetup(db, id)`, `updateSetupItems(db, id, data)`, `updateClassification(...)`. Add `await` before each.
|
||||
|
||||
**totals.ts** — Handlers call totals service functions. Add `await` before each.
|
||||
**totals.ts** -- Handlers call totals service functions. Add `await` before each.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>! grep -n "= getAllItems\|= getItemById\|= createItem\|= getAllCategories\|= getAllThreads\|= getAllSetups" src/server/routes/items.ts src/server/routes/categories.ts src/server/routes/threads.ts src/server/routes/setups.ts 2>/dev/null | grep -v "await" && echo "PASS" || echo "FAIL"</automated>
|
||||
@@ -109,28 +125,46 @@ For each route file, read the full file first. Then:
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 2: Convert auth, OAuth, settings, and images route handlers to async</name>
|
||||
<files>src/server/routes/auth.ts, src/server/routes/oauth.ts, src/server/routes/settings.ts, src/server/routes/images.ts</files>
|
||||
<name>Task 2: Convert auth, OAuth, settings, images routes and auth middleware to async</name>
|
||||
<files>src/server/routes/auth.ts, src/server/routes/oauth.ts, src/server/routes/settings.ts, src/server/routes/images.ts, src/server/middleware/auth.ts</files>
|
||||
<read_first>src/server/routes/auth.ts, src/server/routes/oauth.ts, src/server/routes/settings.ts, src/server/routes/images.ts, src/server/middleware/auth.ts</read_first>
|
||||
<action>
|
||||
**auth.ts** — Handlers call auth service functions. Add `await` before each service call.
|
||||
**auth.ts** -- Handlers call auth service functions. Add `await` before each service call.
|
||||
|
||||
**oauth.ts** — Handlers call OAuth service functions. Add `await` before each service call. Also check for any direct DB queries in OAuth routes and apply async conversion.
|
||||
**oauth.ts** -- Handlers call OAuth service functions. Add `await` before each service call. Also check for any direct DB queries in OAuth routes and apply async conversion.
|
||||
|
||||
**settings.ts** — This route likely accesses the database DIRECTLY (no service layer) using `db.select().from(settings)` etc. Apply full async conversion:
|
||||
- Remove `.all()` — `const rows = await db.select().from(settings)`
|
||||
- Remove `.get()` — `const [row] = await db.select().from(settings).where(...)`
|
||||
- Remove `.run()` — `await db.insert(settings).values(...)`
|
||||
**settings.ts** -- This route likely accesses the database DIRECTLY (no service layer) using `db.select().from(settings)` etc. Apply full async conversion:
|
||||
- Remove `.all()` -- `const rows = await db.select().from(settings)`
|
||||
- Remove `.get()` -- `const [row] = await db.select().from(settings).where(...)`
|
||||
- Remove `.run()` -- `await db.insert(settings).values(...)`
|
||||
|
||||
**images.ts** — May call image service functions. Add `await` before each service call.
|
||||
**images.ts** -- May call image service functions. Add `await` before each service call.
|
||||
|
||||
**Also check `src/server/middleware/auth.ts`** — The auth middleware queries sessions and API keys. If it has direct DB calls, convert them:
|
||||
**src/server/middleware/auth.ts** -- The auth middleware queries sessions and API keys on every authenticated request. These are direct DB calls that must become async:
|
||||
- Make the middleware function async (if not already)
|
||||
- Add `await` before DB queries
|
||||
- Remove `.get()` -> use destructuring
|
||||
- Add `await` before all DB queries (session lookup, API key lookup)
|
||||
- Remove `.get()` -> use destructuring: `const [session] = await db.select()...`
|
||||
- Remove `.all()` if present
|
||||
- This is critical -- the auth middleware runs on every POST/PUT/DELETE request, so missing awaits here would break ALL write operations
|
||||
|
||||
**After all conversions, run a PGlite smoke test to verify routes work end-to-end:**
|
||||
```bash
|
||||
bun -e "
|
||||
import { createTestDb } from './tests/helpers/db.ts';
|
||||
import * as schema from './src/db/schema.ts';
|
||||
const db = await createTestDb();
|
||||
// Verify auth middleware can be imported without errors
|
||||
const authMod = await import('./src/server/middleware/auth.ts');
|
||||
console.log('Auth middleware imports OK');
|
||||
// Verify settings route pattern works
|
||||
const rows = await db.select().from(schema.settings);
|
||||
console.log('Direct DB query works, settings count:', rows.length);
|
||||
console.log('Route smoke test PASSED');
|
||||
"
|
||||
```
|
||||
</action>
|
||||
<verify>
|
||||
<automated>! grep -n "\.all()\|\.get()\|\.run()" src/server/routes/settings.ts src/server/routes/auth.ts src/server/routes/oauth.ts src/server/routes/images.ts 2>/dev/null && echo "PASS" || echo "FAIL"</automated>
|
||||
<automated>! grep -n "\.all()\|\.get()\|\.run()" src/server/routes/settings.ts src/server/routes/auth.ts src/server/routes/oauth.ts src/server/routes/images.ts src/server/middleware/auth.ts 2>/dev/null && bun run lint 2>&1 | tail -3 && echo "PASS" || echo "FAIL"</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- auth.ts: every service call is preceded by `await`
|
||||
@@ -138,20 +172,24 @@ For each route file, read the full file first. Then:
|
||||
- settings.ts: does NOT contain `.all()`, `.get()`, or `.run()`
|
||||
- settings.ts: contains `await db.select()` and `await db.insert()`
|
||||
- images.ts: every service call is preceded by `await`
|
||||
- If auth middleware has DB calls, they are all awaited with no `.get()` or `.all()`
|
||||
- src/server/middleware/auth.ts: does NOT contain `.get()` or `.all()` on DB calls
|
||||
- src/server/middleware/auth.ts: contains `await` before all DB select queries
|
||||
- All files pass lint
|
||||
</acceptance_criteria>
|
||||
<done>Auth, OAuth, settings, and images routes properly await all DB operations. Auth middleware updated if needed.</done>
|
||||
<done>Auth, OAuth, settings, and images routes properly await all DB operations. Auth middleware fully converted to async DB operations. Lint passes.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<verification>
|
||||
- `grep -rn "\.all()\|\.get()\|\.run()" src/server/routes/` returns NO matches
|
||||
- `grep -rn "\.all()\|\.get()\|\.run()" src/server/routes/ src/server/middleware/auth.ts` returns NO matches
|
||||
- Every route handler that calls a service function uses `await`
|
||||
- Auth middleware awaits all DB queries
|
||||
- `bun run lint` passes
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
All 9 route files await async service calls. Settings route uses async direct DB calls. No route handler will return a Promise object instead of resolved data.
|
||||
All 9 route files and auth middleware await async service/DB calls. Settings route uses async direct DB calls. Auth middleware properly awaits session and API key lookups. No route handler will return a Promise object instead of resolved data. Lint passes.
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
|
||||
@@ -88,8 +88,8 @@ import * as schema from "../src/db/schema.ts";
|
||||
```
|
||||
|
||||
**Environment variables:**
|
||||
- `SQLITE_PATH` — path to SQLite database file (default: `"gearbox.db"`)
|
||||
- `DATABASE_URL` — PostgreSQL connection string (required)
|
||||
- `SQLITE_PATH` -- path to SQLite database file (default: `"gearbox.db"`)
|
||||
- `DATABASE_URL` -- PostgreSQL connection string (required)
|
||||
|
||||
**Structure:**
|
||||
1. Open SQLite database read-only
|
||||
@@ -113,19 +113,19 @@ function intToBool(val: number | null): boolean {
|
||||
|
||||
**Migration order and transform functions for each table:**
|
||||
|
||||
1. **categories** — `id` (serial), `name`, `icon`, `createdAt` (unixToDate)
|
||||
2. **users** — `id` (serial), `username`, `passwordHash`, `createdAt` (unixToDate)
|
||||
3. **settings** — `key`, `value` (no transforms needed, text PK)
|
||||
4. **items** — `id` (serial), `name`, `weightGrams`, `priceCents`, `categoryId`, `notes`, `productUrl`, `imageFilename`, `imageSourceUrl`, `quantity`, `createdAt` (unixToDate), `updatedAt` (unixToDate)
|
||||
5. **threads** — `id` (serial), `name`, `status`, `resolvedCandidateId`, `categoryId`, `createdAt` (unixToDate), `updatedAt` (unixToDate)
|
||||
6. **sessions** — `id` (text PK), `userId`, `expiresAt` (unixToDate)
|
||||
7. **apiKeys** — `id` (serial), `name`, `keyHash`, `keyPrefix`, `createdAt` (unixToDate)
|
||||
8. **oauthClients** — `id` (serial), `clientId`, `clientName`, `redirectUris`, `createdAt` (unixToDate)
|
||||
9. **threadCandidates** — `id` (serial), all fields, `createdAt`/`updatedAt` (unixToDate), `sortOrder` (keep as number)
|
||||
10. **setups** — `id` (serial), `name`, `createdAt`/`updatedAt` (unixToDate)
|
||||
11. **oauthCodes** — `id` (serial), all fields, `expiresAt` (unixToDate), `used` (intToBool)
|
||||
12. **oauthTokens** — `id` (serial), all fields, `expiresAt`/`refreshExpiresAt`/`createdAt` (unixToDate)
|
||||
13. **setupItems** — `id` (serial), `setupId`, `itemId`, `classification`
|
||||
1. **categories** -- `id` (serial), `name`, `icon`, `createdAt` (unixToDate)
|
||||
2. **users** -- `id` (serial), `username`, `passwordHash`, `createdAt` (unixToDate)
|
||||
3. **settings** -- `key`, `value` (no transforms needed, text PK)
|
||||
4. **items** -- `id` (serial), `name`, `weightGrams`, `priceCents`, `categoryId`, `notes`, `productUrl`, `imageFilename`, `imageSourceUrl`, `quantity`, `createdAt` (unixToDate), `updatedAt` (unixToDate)
|
||||
5. **threads** -- `id` (serial), `name`, `status`, `resolvedCandidateId`, `categoryId`, `createdAt` (unixToDate), `updatedAt` (unixToDate)
|
||||
6. **sessions** -- `id` (text PK), `userId`, `expiresAt` (unixToDate)
|
||||
7. **apiKeys** -- `id` (serial), `name`, `keyHash`, `keyPrefix`, `createdAt` (unixToDate)
|
||||
8. **oauthClients** -- `id` (serial), `clientId`, `clientName`, `redirectUris`, `createdAt` (unixToDate)
|
||||
9. **threadCandidates** -- `id` (serial), all fields, `createdAt`/`updatedAt` (unixToDate), `sortOrder` (keep as number)
|
||||
10. **setups** -- `id` (serial), `name`, `createdAt`/`updatedAt` (unixToDate)
|
||||
11. **oauthCodes** -- `id` (serial), all fields, `expiresAt` (unixToDate), `used` (intToBool)
|
||||
12. **oauthTokens** -- `id` (serial), all fields, `expiresAt`/`refreshExpiresAt`/`createdAt` (unixToDate)
|
||||
13. **setupItems** -- `id` (serial), `setupId`, `itemId`, `classification`
|
||||
|
||||
**For each table, use this pattern:**
|
||||
```typescript
|
||||
@@ -197,7 +197,7 @@ main().catch((err) => {
|
||||
```
|
||||
</action>
|
||||
<verify>
|
||||
<automated>test -f scripts/migrate-sqlite-to-postgres.ts && grep -q "bun:sqlite" scripts/migrate-sqlite-to-postgres.ts && grep -q "postgres" scripts/migrate-sqlite-to-postgres.ts && grep -q "setval" scripts/migrate-sqlite-to-postgres.ts && grep -q "unixToDate\|unix.*Date\|\\* 1000" scripts/migrate-sqlite-to-postgres.ts && echo "PASS" || echo "FAIL"</automated>
|
||||
<automated>test -f scripts/migrate-sqlite-to-postgres.ts && grep -q "bun:sqlite" scripts/migrate-sqlite-to-postgres.ts && grep -q "postgres" scripts/migrate-sqlite-to-postgres.ts && grep -q "setval" scripts/migrate-sqlite-to-postgres.ts && grep -q "unixToDate\|unix.*Date\|\\* 1000" scripts/migrate-sqlite-to-postgres.ts && bun run lint 2>&1 | tail -3 && echo "PASS" || echo "FAIL"</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- scripts/migrate-sqlite-to-postgres.ts exists
|
||||
@@ -221,10 +221,11 @@ main().catch((err) => {
|
||||
- `bun run scripts/migrate-sqlite-to-postgres.ts --help` or similar does not crash on syntax errors (will fail on missing DATABASE_URL, which is expected)
|
||||
- Script contains all 13 table migrations
|
||||
- Script resets sequences for all tables with serial IDs
|
||||
- `bun run lint` passes
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
One-time migration script exists, handles all type conversions (timestamps, booleans), preserves IDs, resets sequences. Can be run with `DATABASE_URL=... SQLITE_PATH=... bun run scripts/migrate-sqlite-to-postgres.ts`.
|
||||
One-time migration script exists, handles all type conversions (timestamps, booleans), preserves IDs, resets sequences. Can be run with `DATABASE_URL=... SQLITE_PATH=... bun run scripts/migrate-sqlite-to-postgres.ts`. Lint passes.
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
|
||||
@@ -156,7 +156,7 @@ If TypeScript complains about passing PGlite db to service functions that expect
|
||||
let db: any; // Use any to accommodate PGlite/postgres-js type difference
|
||||
```
|
||||
|
||||
**6. OAuth tests — boolean conversion:**
|
||||
**6. OAuth tests -- boolean conversion:**
|
||||
If any OAuth test checks `used === 0` or `used === 1`, change to `used === false` or `used === true`.
|
||||
|
||||
After converting each file, run it individually:
|
||||
@@ -166,7 +166,7 @@ bun test tests/services/item.service.test.ts
|
||||
Fix any issues before moving to the next file.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>bun test tests/services/ 2>&1 | tail -5</automated>
|
||||
<automated>bun test tests/services/ 2>&1; [ $? -eq 0 ] && echo "PASS" || echo "FAIL"</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- Every service test file has `beforeEach(async () => { db = await createTestDb(); })`
|
||||
@@ -181,7 +181,7 @@ Fix any issues before moving to the next file.
|
||||
<task type="auto">
|
||||
<name>Task 2: Convert all route tests + MCP test to async, run full suite</name>
|
||||
<files>tests/routes/items.test.ts, tests/routes/categories.test.ts, tests/routes/threads.test.ts, tests/routes/setups.test.ts, tests/routes/auth.test.ts, tests/routes/oauth.test.ts, tests/routes/images.test.ts, tests/routes/params.test.ts, tests/mcp/tools.test.ts</files>
|
||||
<read_first>tests/routes/items.test.ts, tests/routes/auth.test.ts, tests/mcp/tools.test.ts</read_first>
|
||||
<read_first>tests/routes/items.test.ts, tests/routes/categories.test.ts, tests/routes/threads.test.ts, tests/routes/setups.test.ts, tests/routes/auth.test.ts, tests/routes/oauth.test.ts, tests/routes/images.test.ts, tests/routes/params.test.ts, tests/mcp/tools.test.ts</read_first>
|
||||
<action>
|
||||
Route tests typically create a test app with a test database injected. The pattern is usually:
|
||||
|
||||
@@ -207,7 +207,7 @@ async function createTestApp() {
|
||||
|
||||
1. Make the test app factory `async` and `await createTestDb()`
|
||||
2. Make `beforeEach` async if it calls the factory
|
||||
3. Route tests use `app.request()` which returns a Promise — these should already be awaited. Verify each test awaits the response.
|
||||
3. Route tests use `app.request()` which returns a Promise -- these should already be awaited. Verify each test awaits the response.
|
||||
4. If any test does direct DB calls for setup/assertions, apply same async conversion as service tests
|
||||
5. Make all test callbacks async
|
||||
|
||||
@@ -232,7 +232,7 @@ grep -rn "bun:sqlite\|drizzle-orm/bun-sqlite\|\.all()\|\.get()\|\.run()" tests/
|
||||
Should return NO matches (except possibly string literals in test descriptions).
|
||||
</action>
|
||||
<verify>
|
||||
<automated>bun test tests/ 2>&1 | tail -10</automated>
|
||||
<automated>bun test tests/ 2>&1; [ $? -eq 0 ] && echo "PASS" || echo "FAIL"</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- Every route test file has async `createTestApp` or async `beforeEach` with `await createTestDb()`
|
||||
@@ -247,9 +247,9 @@ Should return NO matches (except possibly string literals in test descriptions).
|
||||
</tasks>
|
||||
|
||||
<verification>
|
||||
- `bun test tests/` — ALL tests pass (exit code 0)
|
||||
- `grep -rn "bun:sqlite\|drizzle-orm/bun-sqlite" tests/` — NO matches
|
||||
- `grep -rn "\.all()\b" tests/ | grep -v "describe\|it(" ` — NO matches on DB calls (may appear in test descriptions)
|
||||
- `bun test tests/` -- ALL tests pass (exit code 0)
|
||||
- `grep -rn "bun:sqlite\|drizzle-orm/bun-sqlite" tests/` -- NO matches
|
||||
- `grep -rn "\.all()\b" tests/ | grep -v "describe\|it(" ` -- NO matches on DB calls (may appear in test descriptions)
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
|
||||
Reference in New Issue
Block a user