=> { ... },
};
}
```
In `mcp/index.ts`, register like `imageHandlers` (no userId dependency):
```typescript
// In createMcpServer():
const catalogHandlers = registerCatalogTools(db);
for (const def of catalogToolDefinitions) {
const handler = catalogHandlers[def.name as keyof typeof catalogHandlers];
server.tool(def.name, def.description, def.inputSchema, handler);
}
```
### Pattern 4: Schema Migration for Columns + Unique Constraint
Adding columns to an existing table and a unique constraint in Drizzle:
```typescript
// In src/db/schema.ts — globalItems table
export const globalItems = pgTable(
"global_items",
{
id: serial("id").primaryKey(),
brand: text("brand").notNull(),
model: text("model").notNull(),
category: text("category"),
weightGrams: doublePrecision("weight_grams"),
priceCents: integer("price_cents"),
imageUrl: text("image_url"),
description: text("description"),
// NEW attribution columns:
sourceUrl: text("source_url"),
imageCredit: text("image_credit"),
imageSourceUrl: text("image_source_url"),
createdAt: timestamp("created_at").defaultNow().notNull(),
},
(table) => [unique().on(table.brand, table.model)], // NEW unique constraint
);
```
Run `bun run db:generate` to generate the migration SQL, then `bun run db:push` to apply. The generated SQL will contain `ALTER TABLE "global_items" ADD COLUMN ...` statements and a `CREATE UNIQUE INDEX` or `CONSTRAINT` statement.
**Important:** The existing `seedGlobalItems` function in `src/db/seed-global-items.ts` seeds with plain `db.insert()`. After the unique constraint lands, a second seed call would fail for duplicate `(brand, model)` pairs. The seed function is already idempotent by checking `existing.length > 0`, so no changes needed there.
### Pattern 5: Attribution Display (Client)
The `$globalItemId.tsx` component renders the image in a `div` below which we add attribution. The `useGlobalItem` hook interface needs updating to include new fields, then the component can conditionally render them:
```tsx
{/* After the image div, before the header */}
{(item.imageCredit || item.imageSourceUrl) && (
{item.imageCredit && Photo: {item.imageCredit}}
{item.imageSourceUrl && (
Source
)}
)}
```
### Pattern 6: Tag Handling in Upsert (Claude's Discretion)
**Recommendation: create-if-not-exists.** The existing `tags` table has a unique constraint on `name`. Use `onConflictDoUpdate` (or `onConflictDoNothing`) on the tags table to get the tag ID, then upsert the `globalItemTags` junction. This lets agents pass tag names without pre-populating the tags table.
```typescript
async function syncGlobalItemTags(tx: Tx, globalItemId: number, tagNames: string[]) {
// Delete existing tags for this item
await tx.delete(globalItemTags).where(eq(globalItemTags.globalItemId, globalItemId));
for (const name of tagNames) {
// Upsert tag (create if not exists)
const [tag] = await tx
.insert(tags)
.values({ name })
.onConflictDoUpdate({ target: tags.name, set: { name } })
.returning({ id: tags.id });
await tx.insert(globalItemTags).values({ globalItemId, tagId: tag.id });
}
}
```
### Anti-Patterns to Avoid
- **Do NOT modify `mcp/index.ts` `createMcpServer` signature** to remove `userId` just because catalog tools don't need it. Pass `db` only to `registerCatalogTools` — accept the `userId` parameter in `createMcpServer` and simply not forward it to catalog tools.
- **Do NOT validate tags at the Zod level as enum values.** Tags are open-ended strings; only length/format validation is appropriate.
- **Do NOT return partial success for bulk upsert.** D-07 mandates all-or-nothing. Zod schema validation at the route level catches structural errors before any DB work begins.
- **Do NOT add rate limiting to `POST /api/global-items` or `POST /api/global-items/bulk`.** These are authenticated-write endpoints — the auth middleware already gates them. The rate limiting added in Phase 24 only applies to public GET endpoints.
## Don't Hand-Roll
| Problem | Don't Build | Use Instead | Why |
|---------|-------------|-------------|-----|
| Upsert on conflict | Manual SELECT then INSERT/UPDATE | `onConflictDoUpdate()` | Drizzle handles atomicity; already used in settings.ts and auth.service.ts |
| All-or-nothing bulk write | Try/catch with manual rollback | `db.transaction(async (tx) => { ... })` | Drizzle handles rollback on throw; used in setup.service.ts |
| Tag create-if-not-exists | Check exists, insert if not | `onConflictDoUpdate` on tags.name | Same conflict mechanism |
| Request body validation | Manual type checking in handler | `zValidator("json", schema)` from `@hono/zod-validator` | All POST/PUT routes use this; Hono returns 400 automatically |
| Auth on POST endpoints | Custom auth check in handler | Existing `requireAuth` middleware in `src/server/index.ts` | Auth middleware already gates all non-GET `/api/global-items*` after Phase 24 |
**Key insight:** The auth middleware in `src/server/index.ts` (lines 150-170) already exempts only GET requests on `/api/global-items` from auth. POST requests on that path fall through to `requireAuth` automatically — no changes to `index.ts` needed.
## Common Pitfalls
### Pitfall 1: Unique Constraint Not Applied to Existing Data
**What goes wrong:** If the database already has duplicate `(brand, model)` pairs (e.g., from early seeding), adding a unique constraint via migration will fail with a duplicate key error.
**Why it happens:** The migration tries to add `UNIQUE(brand, model)` to a table that already has conflicting rows.
**How to avoid:** Check for existing duplicates before generating the migration. In development with test data, truncate `global_items` or resolve duplicates first. The test database is reset between tests (`TRUNCATE ... RESTART IDENTITY CASCADE`) so tests are not affected.
**Warning signs:** `drizzle-kit push` fails with "could not create unique index" or similar.
### Pitfall 2: Client `GlobalItem` Interface Missing New Fields
**What goes wrong:** `useGlobalItems.ts` defines a local `GlobalItem` interface. After the migration adds columns, the API returns new fields but the TypeScript interface doesn't include them, so the component can't render them.
**Why it happens:** The interface at the top of `useGlobalItems.ts` (lines 3-14) is manually maintained — it's not generated from Drizzle schema types.
**How to avoid:** Update the `GlobalItem` interface in `useGlobalItems.ts` to add `sourceUrl: string | null`, `imageCredit: string | null`, `imageSourceUrl: string | null`. The `GlobalItemWithOwnerCount` extends it and gets the fields for free.
**Warning signs:** TypeScript error `Property 'imageCredit' does not exist on type 'GlobalItemWithOwnerCount'` in `$globalItemId.tsx`.
### Pitfall 3: Bulk Endpoint Registered Before Auth Middleware Applies
**What goes wrong:** `/api/global-items/bulk` is a POST endpoint. The auth middleware in `src/server/index.ts` exempts GET on `/api/global-items` (line 165-167). If the route registration order matters, the bulk POST could be accidentally exempt.
**Why it happens:** The middleware skip check uses `c.req.path.startsWith("/api/global-items") && c.req.method === "GET"` — it's already method-gated, so POST requests are not exempt. No issue in practice.
**How to avoid:** No special handling needed. The middleware skip condition already checks `c.req.method === "GET"`. Verify this by reading `src/server/index.ts` lines 165-167 before implementing.
**Warning signs:** POST to `/api/global-items/bulk` returns 200 without `X-API-Key` header.
### Pitfall 4: MCP Tool for Bulk Returns Success on Partial Failure
**What goes wrong:** If the service function for bulk upsert silently skips failed items instead of throwing, the MCP tool returns `{ created: X, updated: Y }` even though some items were not persisted.
**Why it happens:** Swallowing errors inside a loop without re-throwing.
**How to avoid:** Zod validation happens at the route level (for HTTP) and at the MCP tool handler level (parse inputSchema). The service function should assume pre-validated data and let Drizzle throw naturally inside the transaction. The transaction auto-rolls-back on any thrown error.
**Warning signs:** Bulk upsert returns counts that don't add up to the input array length, with no error.
### Pitfall 5: `onConflictDoUpdate` Target Requires Constraint, Not Arbitrary Columns
**What goes wrong:** Calling `.onConflictDoUpdate({ target: [globalItems.brand, globalItems.model], ... })` without a unique constraint on those columns causes a Postgres error at runtime.
**Why it happens:** PostgreSQL's `ON CONFLICT (col1, col2) DO UPDATE` requires an existing unique index or constraint. Drizzle passes through to Postgres directly.
**How to avoid:** The unique constraint must be added to the schema and migration applied BEFORE any upsert code runs. Always run `bun run db:push` before testing the new service functions.
**Warning signs:** `ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification`.
## Code Examples
Verified patterns from the codebase:
### Drizzle Upsert (multi-column target)
```typescript
// Source: src/server/routes/settings.ts lines 33-37
await database
.insert(settings)
.values({ userId, key, value: body.value })
.onConflictDoUpdate({
target: [settings.userId, settings.key],
set: { value: body.value },
});
```
### Drizzle Transaction
```typescript
// Source: src/server/services/setup.service.ts line 164
return await db.transaction(async (tx) => {
// ... multiple tx.insert / tx.select calls
// Any thrown error rolls back automatically
});
```
### Hono Route with zValidator
```typescript
// Source: src/server/routes/setups.ts line 36
app.post("/", zValidator("json", createSetupSchema), async (c) => {
const db = c.get("db");
const userId = c.get("userId")!;
const data = c.req.valid("json");
// ...
});
```
### MCP Tool Definition + Handler (no-userId pattern)
```typescript
// Source: src/server/mcp/tools/images.ts — full file
export const imageToolDefinitions = [
{
name: "upload_image_from_url",
description: "...",
inputSchema: { url: z.string().describe("...") },
},
];
export function registerImageTools() { // no db, no userId
return {
upload_image_from_url: async (args: { url: string }): Promise => {
try {
const result = await fetchImageFromUrl(args.url);
return textResult(result);
} catch (err) {
return errorResult((err as Error).message);
}
},
};
}
```
### Schema with Unique Constraint (table-level)
```typescript
// Source: src/db/schema.ts line 26-38 (categories table — same pattern)
export const categories = pgTable(
"categories",
{
id: serial("id").primaryKey(),
// ...
},
(table) => [unique().on(table.userId, table.name)],
);
```
## State of the Art
| Old Approach | Current Approach | When Changed | Impact |
|--------------|------------------|--------------|--------|
| Separate `item_global_links` junction table | `globalItemId` FK directly on `items` | Phase migration 0002 | Simpler joins; one less table |
| No unique constraint on globalItems | `unique().on(brand, model)` | This phase | Prevents duplicate catalog entries |
| Read-only global item API | Read + write (upsert) API | This phase | Enables agent-powered seeding |
## Open Questions
1. **Tag replacement vs merge on upsert**
- What we know: D-09 includes `tags?: string[]` as optional field
- What's unclear: When tags are omitted on an upsert of an existing item, should existing tags be left untouched, or cleared?
- Recommendation: Leave existing tags untouched when `tags` field is absent in the payload. Only sync (replace) tags when `tags` is explicitly provided (even if empty array = clear all tags). This is the least-surprising behavior for agents that send partial updates.
2. **`imageUrl` field on globalItems vs `imageFilename`**
- What we know: `globalItems.imageUrl` stores an absolute URL (unlike `items.imageFilename` which stores a filename for S3). The bulk upsert input accepts `imageUrl`.
- What's unclear: Should agents use `upload_image_from_url` first, then pass the returned filename as `imageUrl`? Or pass the original URL directly?
- Recommendation: Accept both — agents can pass any URL to `imageUrl`. When the agent wants to mirror the image to S3, they call `upload_image_from_url` first and use the result. The `imageSourceUrl` attribution field is separate and intended to record the original source regardless of where the image is now stored.
## Environment Availability
Step 2.6: SKIPPED (no external dependencies identified — this phase is purely code and schema changes within the existing stack; PostgreSQL/PGlite is already operational)
## Validation Architecture
### Test Framework
| Property | Value |
|----------|-------|
| Framework | Bun test runner (built-in) |
| Config file | none — `bun test` discovers `tests/**/*.test.ts` |
| Quick run command | `bun test tests/services/global-item.service.test.ts tests/routes/global-items.test.ts tests/mcp/tools.test.ts` |
| Full suite command | `bun test` |
### Phase Requirements → Test Map
| Req ID | Behavior | Test Type | Automated Command | File Exists? |
|--------|----------|-----------|-------------------|-------------|
| CATL-01 | Attribution columns present and returnable | integration | `bun test tests/services/global-item.service.test.ts` | ✅ (extend) |
| CATL-02 | Duplicate (brand, model) upserts rather than errors | integration | `bun test tests/services/global-item.service.test.ts` | ✅ (extend) |
| CATL-03 | Attribution rendered in detail page | manual | Visual check in browser | — |
| CATL-04 | `POST /api/global-items/bulk` accepts array | integration | `bun test tests/routes/global-items.test.ts` | ✅ (extend) |
| CATL-05 | Bulk upsert updates on conflict, returns created/updated counts | integration | `bun test tests/routes/global-items.test.ts` | ✅ (extend) |
| SEED-01 | `upsert_catalog_item` MCP tool writes to globalItems | integration | `bun test tests/mcp/tools.test.ts` | ✅ (extend) |
| SEED-02 | `bulk_upsert_catalog` MCP tool persists all items | integration | `bun test tests/mcp/tools.test.ts` | ✅ (extend) |
| SEED-03 | Attribution fields available as MCP tool parameters | unit | `bun test tests/mcp/tools.test.ts` | ✅ (extend) |
### Sampling Rate
- **Per task commit:** `bun test tests/services/global-item.service.test.ts tests/routes/global-items.test.ts tests/mcp/tools.test.ts`
- **Per wave merge:** `bun test`
- **Phase gate:** Full suite green before `/gsd:verify-work`
### Wave 0 Gaps
None — existing test infrastructure covers all phase requirements. The three test files already exist and test the relevant modules; new test cases are added to existing `describe` blocks.
## Project Constraints (from CLAUDE.md)
- Use `bun run db:generate` + `bun run db:push` for all schema changes (not raw SQL)
- Prices stored as cents (`priceCents: integer`), weights as grams (`doublePrecision`)
- Services take `(db, ...)` as first argument — no HTTP awareness
- Hono routes delegate to services; use `@hono/zod-validator` for all request validation
- Shared Zod schemas live in `src/shared/schemas.ts`
- MCP tools: definitions array + register function pattern per domain
- Route tree is auto-generated — never edit `routeTree.gen.ts`
- Always reuse existing components; check `src/client/components/` before creating new UI elements
- `@/*` maps to `./src/*`
- Tabs, double quotes, organized imports (Biome lint)
## Sources
### Primary (HIGH confidence)
- Direct codebase inspection: `src/db/schema.ts` — current globalItems table definition
- Direct codebase inspection: `src/server/services/global-item.service.ts` — existing read patterns
- Direct codebase inspection: `src/server/routes/global-items.ts` — existing route handlers
- Direct codebase inspection: `src/server/mcp/index.ts` — tool registration loop pattern
- Direct codebase inspection: `src/server/mcp/tools/items.ts` — tool definition + handler pattern
- Direct codebase inspection: `src/server/mcp/tools/images.ts` — no-userId factory pattern
- Direct codebase inspection: `src/server/routes/settings.ts` — multi-column `onConflictDoUpdate` pattern
- Direct codebase inspection: `src/server/services/auth.service.ts` — single-column `onConflictDoUpdate` pattern
- Direct codebase inspection: `src/server/services/setup.service.ts` — `db.transaction()` pattern
- Direct codebase inspection: `src/server/index.ts` — auth middleware skip logic for global-items GET
- Direct codebase inspection: `tests/helpers/db.ts` — PGlite test setup with migrations
- Direct codebase inspection: `tests/services/global-item.service.test.ts` — existing test structure
- Direct codebase inspection: `tests/routes/global-items.test.ts` — existing route test structure
- Direct codebase inspection: `tests/mcp/tools.test.ts` — MCP tool test structure
### Secondary (MEDIUM confidence)
- drizzle-orm 0.45.1 installed version confirmed via `bun pm ls` — upsert API stable since 0.28
## Metadata
**Confidence breakdown:**
- Standard stack: HIGH — all libraries confirmed installed; APIs confirmed in use
- Architecture: HIGH — all patterns directly observed in codebase, not assumed
- Pitfalls: HIGH — derived from reading the actual middleware skip logic and schema constraints
**Research date:** 2026-04-10
**Valid until:** 2026-05-10 (stable stack, no fast-moving dependencies)