fix(api): enforce grid occupancy, foreign keys, and atomic derivation writes (#74)

* refactor(api): take ContainerWriteInput only at container creation.

Callers pass quantity as an option instead of flattening to ContainerData before persist.

* refactor(api): assemble container reads through one view.

List, collection detail, GET /containers/:id, and export project from the same load. ADR 0009 records the decision.

* refactor(web): split derivations bulk-import into reducer, effects, and gateway.

The page and hook become a thin shell over a testable core.

* fix(api): enforce grid occupancy, foreign keys, and atomic derivation writes

Reconnects were leaving SQLite foreign keys off, and two containers could share a grid cell. Open every connection with FK on, unique-index occupied positions, and wrap derivation writes in a transaction that survives await.

Author: m-murphy

CONTEXT.md

@@ -124,9 +124,13 @@ A place in the storage hierarchy where collections are kept (e.g. freezer, shelf
 _Avoid_: Site (unless referring to an external facility), storage (too vague)
 
 **Container placement**:
-Where a **Container** sits in the physical storage hierarchy — its position within a **Collection**, the collection it belongs to, and the **Location** path where that collection is stored. For tubes and wells, the immediate **Collection** is a plate or box and may include a grid position. For **Paper** containers, the immediate **Collection** is a **Sheet** (within a box or bag). Distinct from specimen provenance (**Source**, study/control context) and from container identity metadata (barcodes, **Tags**).
+Where a **Container** sits in the physical storage hierarchy — its **grid position** within a **Collection**, the collection it belongs to, and the **Location** path where that collection is stored. For tubes and wells, the immediate **Collection** is a plate or box. For **Paper** containers, the immediate **Collection** is a **Sheet** (within a box or bag). Distinct from specimen provenance (**Source**, study/control context) and from container identity metadata (barcodes, **Tags**).
 _Avoid_: Enrichment (implementation term); conflating placement with specimen or source lookups; placing paper directly on a box or bag without a sheet
 
+**Grid position**:
+The well or slot of a tube or well **Container** in its plate or box (e.g. A01). Optional: a tube or well may belong to a **Collection** with no grid position (legacy rows). At most one Container occupies a given grid position in a Collection; a missing position occupies no cell. Not applicable to **Paper**.
+_Avoid_: Well (as the domain term); treating a missing position as occupying a slot; requiring every tube to have a position before it can exist in a Collection
+
 ### Export
 
 **Container export**:

packages/api/initial_schema.sql

@@ -206,6 +206,7 @@ CREATE TABLE IF NOT EXISTS micronix_tube (
   barcode TEXT NOT NULL UNIQUE,
   position TEXT
 );--> statement-breakpoint
+CREATE UNIQUE INDEX IF NOT EXISTS micronix_tube_collection_position_idx ON micronix_tube(collection_id, position) WHERE position IS NOT NULL;--> statement-breakpoint
 CREATE TABLE IF NOT EXISTS cryovial_box (
   id INTEGER PRIMARY KEY,
   location_id INTEGER NOT NULL REFERENCES location(id),
@@ -223,6 +224,7 @@ CREATE TABLE IF NOT EXISTS cryovial_tube (
   barcode TEXT,
   position TEXT
 );--> statement-breakpoint
+CREATE UNIQUE INDEX IF NOT EXISTS cryovial_tube_collection_position_idx ON cryovial_tube(collection_id, position) WHERE position IS NOT NULL;--> statement-breakpoint
 CREATE TABLE IF NOT EXISTS box (
   id INTEGER PRIMARY KEY,
   location_id INTEGER NOT NULL REFERENCES location(id),
@@ -266,6 +268,7 @@ CREATE TABLE IF NOT EXISTS static_well (
   collection_id INTEGER NOT NULL REFERENCES micronix_plate(id),
   position TEXT
 );--> statement-breakpoint
+CREATE UNIQUE INDEX IF NOT EXISTS static_well_collection_position_idx ON static_well(collection_id, position) WHERE position IS NOT NULL;--> statement-breakpoint
 CREATE TABLE IF NOT EXISTS strain (
   id INTEGER PRIMARY KEY,
   name TEXT NOT NULL UNIQUE,
@@ -279,7 +282,7 @@ CREATE TABLE IF NOT EXISTS storage_type (
 CREATE TABLE IF NOT EXISTS schema_version (
   version INTEGER NOT NULL
 );--> statement-breakpoint
-INSERT INTO schema_version (version) VALUES (3);--> statement-breakpoint
+INSERT INTO schema_version (version) VALUES (4);--> statement-breakpoint
 CREATE TABLE IF NOT EXISTS settings (
   key TEXT NOT NULL,
   user_id INTEGER REFERENCES users(id),

packages/api/src/db/README.md

@@ -6,7 +6,9 @@ See [ADR-0003: SQLite schema evolution](../../../docs/adr/0003-sqlite-schema-evo
 
 ## Opening a database
 
-Use `openOperationalDatabase(optionalPath?)` from the client module. It resolves the file path, enables WAL, runs schema evolution, and returns Drizzle + raw `bun:sqlite` handles. Importing the client module does **not** open a connection.
+Use `openOperationalDatabase(optionalPath?)` from the client module. It resolves the file path, enables WAL, turns on foreign keys, runs schema evolution, and returns Drizzle + raw `bun:sqlite` handles. Importing the client module does **not** open a connection.
+
+Multi-statement writes that `await` must use `withWriteTransaction` from `write-transaction.ts`. Drizzle's bun-sqlite `db.transaction(async () => …)` commits when the callback first yields; it does not await the returned Promise.
 
 Tests should use the same entry point (e.g. `openOperationalDatabase(':memory:')` via `setupTestDatabase()`).
 

packages/api/src/db/__tests__/client.test.ts

@@ -29,6 +29,9 @@ describe('Database Client', () => {
     const db = new Database(testDbPath)
     db.exec('CREATE TABLE study (id INTEGER PRIMARY KEY)')
     db.exec('CREATE TABLE settings (key TEXT, user_id INTEGER, value TEXT, PRIMARY KEY (key, user_id))')
+    db.exec(`CREATE TABLE micronix_tube (id INTEGER PRIMARY KEY, collection_id INTEGER NOT NULL, barcode TEXT, position TEXT)`)
+    db.exec(`CREATE TABLE cryovial_tube (id INTEGER PRIMARY KEY, collection_id INTEGER NOT NULL, barcode TEXT, position TEXT)`)
+    db.exec(`CREATE TABLE static_well (id INTEGER PRIMARY KEY, collection_id INTEGER NOT NULL, position TEXT)`)
     // Intentionally omit paper — migration 003 preflight creates legacy stub
     // Intentionally do NOT create error_logs
     const before = db.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name='error_logs'").get()

packages/api/src/db/__tests__/schema-evolution.test.ts

@@ -12,6 +12,26 @@ import {
 import { openOperationalDatabase } from '../open'
 import { SchemaMigrationError, runSqlMigrationStatements } from '../migration-runner'
 
+function createGridPositionTables(sqlite: Database): void {
+  sqlite.exec(`CREATE TABLE IF NOT EXISTS micronix_tube (
+    id INTEGER PRIMARY KEY,
+    collection_id INTEGER NOT NULL,
+    barcode TEXT,
+    position TEXT
+  )`)
+  sqlite.exec(`CREATE TABLE IF NOT EXISTS cryovial_tube (
+    id INTEGER PRIMARY KEY,
+    collection_id INTEGER NOT NULL,
+    barcode TEXT,
+    position TEXT
+  )`)
+  sqlite.exec(`CREATE TABLE IF NOT EXISTS static_well (
+    id INTEGER PRIMARY KEY,
+    collection_id INTEGER NOT NULL,
+    position TEXT
+  )`)
+}
+
 describe('schema evolution', () => {
   let testDbPath: string
 
@@ -53,6 +73,18 @@ describe('schema evolution', () => {
     sqlite.close()
   })
 
+  it('openOperationalDatabase enables foreign keys on empty and reconnect', () => {
+    const first = openOperationalDatabase(testDbPath)
+    const emptyFk = first.sqlite.prepare('PRAGMA foreign_keys').get() as { foreign_keys: number }
+    expect(emptyFk.foreign_keys).toBe(1)
+    first.sqlite.close()
+
+    const second = openOperationalDatabase(testDbPath)
+    const reconnectFk = second.sqlite.prepare('PRAGMA foreign_keys').get() as { foreign_keys: number }
+    expect(reconnectFk.foreign_keys).toBe(1)
+    second.sqlite.close()
+  })
+
   it('legacy unversioned database gains error_logs and schema_version 1 then upgrades to current', () => {
     const sqlite = new Database(testDbPath)
     sqlite.exec('CREATE TABLE study (id INTEGER PRIMARY KEY)')
@@ -67,6 +99,7 @@ describe('schema evolution', () => {
       barcode TEXT,
       position TEXT
     )`)
+    createGridPositionTables(sqlite)
     sqlite.close()
 
     const { sqlite: opened } = openOperationalDatabase(testDbPath)
@@ -91,6 +124,7 @@ describe('schema evolution', () => {
       barcode TEXT,
       position TEXT
     )`)
+    createGridPositionTables(sqlite)
     evolveOperationalSchema(sqlite)
     expect(getRecordedSchemaVersion(sqlite)).toBe(CURRENT_SCHEMA_VERSION)
     sqlite.close()
@@ -108,6 +142,7 @@ describe('schema evolution', () => {
       barcode TEXT,
       position TEXT
     )`)
+    createGridPositionTables(sqlite)
     evolveOperationalSchema(sqlite)
     expect(getRecordedSchemaVersion(sqlite)).toBe(CURRENT_SCHEMA_VERSION)
     const columns = sqlite
@@ -121,6 +156,7 @@ describe('schema evolution', () => {
     const sqlite = new Database(testDbPath)
     sqlite.exec('CREATE TABLE schema_version (version INTEGER NOT NULL)')
     sqlite.exec('INSERT INTO schema_version (version) VALUES (2)')
+    createGridPositionTables(sqlite)
     evolveOperationalSchema(sqlite)
     expect(getRecordedSchemaVersion(sqlite)).toBe(CURRENT_SCHEMA_VERSION)
     const paperTable = sqlite
@@ -155,6 +191,39 @@ describe('schema evolution', () => {
     sqlite.close()
   })
 
+  it('applies migration 004 unique indexes when database is at version 3', () => {
+    const sqlite = new Database(testDbPath)
+    sqlite.exec('CREATE TABLE schema_version (version INTEGER NOT NULL)')
+    sqlite.exec('INSERT INTO schema_version (version) VALUES (3)')
+    createGridPositionTables(sqlite)
+    evolveOperationalSchema(sqlite)
+    expect(getRecordedSchemaVersion(sqlite)).toBe(CURRENT_SCHEMA_VERSION)
+    const names = sqlite
+      .prepare(
+        `SELECT name FROM sqlite_master WHERE type='index' AND name LIKE '%collection_position%'`,
+      )
+      .all() as Array<{ name: string }>
+    expect(names.map((row) => row.name).sort()).toEqual([
+      'cryovial_tube_collection_position_idx',
+      'micronix_tube_collection_position_idx',
+      'static_well_collection_position_idx',
+    ])
+    sqlite.close()
+  })
+
+  it('migration 004 aborts when duplicate occupied grid positions exist', () => {
+    const sqlite = new Database(testDbPath)
+    sqlite.exec('CREATE TABLE schema_version (version INTEGER NOT NULL)')
+    sqlite.exec('INSERT INTO schema_version (version) VALUES (3)')
+    createGridPositionTables(sqlite)
+    sqlite.exec(`INSERT INTO micronix_tube (id, collection_id, barcode, position) VALUES (1, 1, 'A', 'A01')`)
+    sqlite.exec(`INSERT INTO micronix_tube (id, collection_id, barcode, position) VALUES (2, 1, 'B', 'A01')`)
+
+    expect(() => evolveOperationalSchema(sqlite)).toThrow(SchemaMigrationError)
+    expect(getRecordedSchemaVersion(sqlite)).toBe(3)
+    sqlite.close()
+  })
+
   it('fail-hard: invalid migration leaves schema_version unchanged', () => {
     const sqlite = new Database(testDbPath)
     sqlite.exec('CREATE TABLE schema_version (version INTEGER NOT NULL)')

packages/api/src/db/__tests__/write-transaction.test.ts

@@ -0,0 +1,56 @@
+import { describe, it, expect, beforeEach, afterEach } from 'vitest'
+import { setupTestDatabase, cleanupTestDatabase } from '../../__tests__/helpers/db-setup'
+import { createTestStudy } from '../../__tests__/helpers/factories'
+import { study } from '../schema'
+import { withWriteTransaction } from '../write-transaction'
+import type { Database } from '../client'
+
+describe('withWriteTransaction', () => {
+  let testDb: Database
+  let sqlite: Awaited<ReturnType<typeof setupTestDatabase>>['sqlite']
+
+  beforeEach(async () => {
+    const setup = await setupTestDatabase()
+    testDb = setup.db
+    sqlite = setup.sqlite
+  })
+
+  afterEach(() => {
+    if (sqlite) cleanupTestDatabase(sqlite)
+  })
+
+  it('rolls back awaited writes when fn throws', async () => {
+    await expect(
+      withWriteTransaction(testDb, async (db) => {
+        await createTestStudy(db, { title: 'Rollback Study', shortCode: 'RB1' })
+        throw new Error('boom')
+      }),
+    ).rejects.toThrow('boom')
+
+    const rows = await testDb.select().from(study)
+    expect(rows.find((row) => row.shortCode === 'RB1')).toBeUndefined()
+  })
+
+  it('commits awaited writes when fn returns', async () => {
+    await withWriteTransaction(testDb, async (db) => {
+      await createTestStudy(db, { title: 'Commit Study', shortCode: 'CM1' })
+    })
+
+    const rows = await testDb.select().from(study)
+    expect(rows.find((row) => row.shortCode === 'CM1')).toBeDefined()
+  })
+
+  it('does not nest when already in a transaction', async () => {
+    await expect(
+      withWriteTransaction(testDb, async (outer) => {
+        await withWriteTransaction(outer, async (inner) => {
+          await createTestStudy(inner, { title: 'Nested Study', shortCode: 'N1' })
+        })
+        throw new Error('outer fail')
+      }),
+    ).rejects.toThrow('outer fail')
+
+    const rows = await testDb.select().from(study)
+    expect(rows.find((row) => row.shortCode === 'N1')).toBeUndefined()
+  })
+})

packages/api/src/db/migrations/004_collection_grid_position_unique.sql

@@ -0,0 +1,22 @@
+-- Migration 004: at most one Container per grid position in a Collection.
+-- NULL positions are excluded (legacy unpositioned tubes occupy no cell).
+CREATE TEMP TABLE IF NOT EXISTS __migration_004_guard (id INTEGER PRIMARY KEY);--> statement-breakpoint
+CREATE TRIGGER __migration_004_guard_tr BEFORE INSERT ON __migration_004_guard
+BEGIN
+  SELECT RAISE(ABORT, 'Cannot add unique grid position index: duplicate (collection_id, position) rows exist.')
+  WHERE EXISTS (
+    SELECT 1 FROM micronix_tube WHERE position IS NOT NULL GROUP BY collection_id, position HAVING COUNT(*) > 1
+  )
+  OR EXISTS (
+    SELECT 1 FROM cryovial_tube WHERE position IS NOT NULL GROUP BY collection_id, position HAVING COUNT(*) > 1
+  )
+  OR EXISTS (
+    SELECT 1 FROM static_well WHERE position IS NOT NULL GROUP BY collection_id, position HAVING COUNT(*) > 1
+  );
+END;--> statement-breakpoint
+INSERT INTO __migration_004_guard DEFAULT VALUES;--> statement-breakpoint
+DROP TRIGGER __migration_004_guard_tr;--> statement-breakpoint
+DROP TABLE __migration_004_guard;--> statement-breakpoint
+CREATE UNIQUE INDEX IF NOT EXISTS micronix_tube_collection_position_idx ON micronix_tube(collection_id, position) WHERE position IS NOT NULL;--> statement-breakpoint
+CREATE UNIQUE INDEX IF NOT EXISTS cryovial_tube_collection_position_idx ON cryovial_tube(collection_id, position) WHERE position IS NOT NULL;--> statement-breakpoint
+CREATE UNIQUE INDEX IF NOT EXISTS static_well_collection_position_idx ON static_well(collection_id, position) WHERE position IS NOT NULL;

packages/api/src/db/open.ts

@@ -109,6 +109,7 @@ export function openOperationalDatabase(dbPath?: string): {
 
   const sqlite = new SQLiteDatabase(resolvedPath)
   sqlite.exec('PRAGMA journal_mode = WAL')
+  sqlite.exec('PRAGMA foreign_keys = ON')
 
   try {
     evolveOperationalSchema(sqlite)

packages/api/src/db/schema-evolution.ts

@@ -7,7 +7,7 @@ import { listNumberedMigrations, runSqlMigration } from './migration-runner'
 const PAPER_SUBLABEL_MIGRATION_VERSION = 3
 
 /** Canonical schema level; bump when adding numbered deltas under migrations/. */
-export const CURRENT_SCHEMA_VERSION = 3
+export const CURRENT_SCHEMA_VERSION = 4
 
 const SCHEMA_VERSION_TABLE = 'schema_version'
 const LEGACY_BASELINE_VERSION = 1

packages/api/src/db/schema.ts

@@ -1,4 +1,4 @@
-import { sqliteTable, text, integer, real, check, primaryKey, unique, index } from 'drizzle-orm/sqlite-core'
+import { sqliteTable, text, integer, real, check, primaryKey, unique, uniqueIndex, index } from 'drizzle-orm/sqlite-core'
 import { sql, type InferSelectModel } from 'drizzle-orm'
 
 // Type exports for all database tables
@@ -288,7 +288,11 @@ export const micronixTube = sqliteTable('micronix_tube', {
   collectionId: integer('collection_id').notNull().references(() => micronixPlate.id),
   barcode: text('barcode').notNull().unique(),
   position: text('position'),
-})
+}, (table) => ({
+  collectionPositionUniq: uniqueIndex('micronix_tube_collection_position_idx')
+    .on(table.collectionId, table.position)
+    .where(sql`${table.position} IS NOT NULL`),
+}))
 
 export const cryovialBox = sqliteTable('cryovial_box', {
   id: integer('id').primaryKey(),
@@ -308,7 +312,11 @@ export const cryovialTube = sqliteTable('cryovial_tube', {
   collectionId: integer('collection_id').notNull().references(() => cryovialBox.id),
   barcode: text('barcode'),
   position: text('position'),
-})
+}, (table) => ({
+  collectionPositionUniq: uniqueIndex('cryovial_tube_collection_position_idx')
+    .on(table.collectionId, table.position)
+    .where(sql`${table.position} IS NOT NULL`),
+}))
 
 export const box = sqliteTable('box', {
   id: integer('id').primaryKey(),
@@ -360,7 +368,11 @@ export const staticWell = sqliteTable('static_well', {
   id: integer('id').primaryKey().references(() => storageContainer.id),
   collectionId: integer('collection_id').notNull().references(() => micronixPlate.id),
   position: text('position'),
-})
+}, (table) => ({
+  collectionPositionUniq: uniqueIndex('static_well_collection_position_idx')
+    .on(table.collectionId, table.position)
+    .where(sql`${table.position} IS NOT NULL`),
+}))
 
 // Additional reference tables
 export const strain = sqliteTable('strain', {