diff --git a/services/api/src/db/__tests__/migrations.test.ts b/services/api/src/db/__tests__/migrations.test.ts index df0541d..0acff0b 100644 --- a/services/api/src/db/__tests__/migrations.test.ts +++ b/services/api/src/db/__tests__/migrations.test.ts @@ -1,6 +1,6 @@ /** * Migration tests — apply runMigrations() to a fresh in-memory SQLite handle - * and verify schema, idempotency, and the consent_given → user_consents backfill. + * and verify schema shape and idempotency. */ import { describe, it, expect } from 'vitest'; import Database from 'better-sqlite3'; @@ -13,7 +13,7 @@ function freshDb() { } describe('runMigrations — fresh DB', () => { - it('creates the ADR-0014 tables and adds tone / tip_kinds_json on users', () => { + it('creates the ADR-0014 tables, adds tone/tip_kinds_json, and drops legacy consent columns', () => { const sqlite = freshDb(); runMigrations(sqlite); @@ -26,6 +26,38 @@ describe('runMigrations — fresh DB', () => { const colNames = userCols.map((c) => c.name); expect(colNames).toContain('tone'); expect(colNames).toContain('tip_kinds_json'); + // ADR-0014 step 8: legacy columns must be absent on a fresh DB + expect(colNames).not.toContain('consent_given'); + expect(colNames).not.toContain('consent_at'); + }); + + it('drops consent columns from an existing DB that still had them', () => { + const sqlite = freshDb(); + sqlite.pragma('foreign_keys = ON'); + // Simulate a pre-step-8 DB: create table with legacy columns and seed a user + sqlite.exec(` + CREATE TABLE users ( + id TEXT PRIMARY KEY, + email TEXT NOT NULL UNIQUE, + role TEXT NOT NULL DEFAULT 'user', + consent_given INTEGER NOT NULL DEFAULT 0, + consent_at TEXT, + created_at TEXT NOT NULL + ); + INSERT INTO users (id, email, role, consent_given, consent_at, created_at) + VALUES ('u1', 'u@test.com', 'user', 1, '2026-04-01T00:00:00Z', '2026-03-01T00:00:00Z'); + `); + runMigrations(sqlite); + + const colNames = (sqlite.prepare(`PRAGMA table_info(users)`).all() as { name: string }[]).map((c) => c.name); + expect(colNames).not.toContain('consent_given'); + expect(colNames).not.toContain('consent_at'); + + // Backfill should have migrated the consent row before dropping + const consent = sqlite + .prepare(`SELECT consent_key FROM user_consents WHERE user_id = 'u1'`) + .get() as { consent_key: string } | undefined; + expect(consent?.consent_key).toBe('data:core'); }); it('declares the expected composite primary keys', () => { @@ -53,71 +85,3 @@ describe('runMigrations — idempotency', () => { }); }); -describe('runMigrations — consent backfill', () => { - it('backfills users with consent_given=1 into user_consents (data:core)', () => { - const sqlite = freshDb(); - runMigrations(sqlite); - - sqlite.prepare( - `INSERT INTO users (id, email, role, consent_given, consent_at, created_at) - VALUES (?, ?, 'user', 1, ?, ?)`, - ).run('u1', 'u1@test.com', '2026-04-01T00:00:00Z', '2026-03-01T00:00:00Z'); - sqlite.prepare( - `INSERT INTO users (id, email, role, consent_given, consent_at, created_at) - VALUES (?, ?, 'user', 0, NULL, ?)`, - ).run('u2', 'u2@test.com', '2026-03-02T00:00:00Z'); - - // Re-run migrations to trigger the backfill (the first call ran before users existed). - runMigrations(sqlite); - - const rows = sqlite - .prepare(`SELECT user_id, consent_key, granted_at, revoked_at FROM user_consents`) - .all() as { user_id: string; consent_key: string; granted_at: string; revoked_at: string | null }[]; - expect(rows).toEqual([ - { user_id: 'u1', consent_key: 'data:core', granted_at: '2026-04-01T00:00:00Z', revoked_at: null }, - ]); - }); - - it('falls back to created_at when consent_at is null', () => { - const sqlite = freshDb(); - runMigrations(sqlite); - - sqlite.prepare( - `INSERT INTO users (id, email, role, consent_given, consent_at, created_at) - VALUES (?, ?, 'user', 1, NULL, ?)`, - ).run('u3', 'u3@test.com', '2026-02-15T00:00:00Z'); - - runMigrations(sqlite); - - const granted = sqlite - .prepare(`SELECT granted_at FROM user_consents WHERE user_id = 'u3'`) - .get() as { granted_at: string }; - expect(granted.granted_at).toBe('2026-02-15T00:00:00Z'); - }); - - it('does not overwrite an existing user_consents row on subsequent runs', () => { - const sqlite = freshDb(); - runMigrations(sqlite); - - sqlite.prepare( - `INSERT INTO users (id, email, role, consent_given, consent_at, created_at) - VALUES (?, ?, 'user', 1, ?, ?)`, - ).run('u4', 'u4@test.com', '2026-04-01T00:00:00Z', '2026-03-01T00:00:00Z'); - - runMigrations(sqlite); - - // Simulate user revoking core consent later via the new code path. - sqlite.prepare( - `UPDATE user_consents SET revoked_at = ? WHERE user_id = 'u4' AND consent_key = 'data:core'`, - ).run('2026-04-15T00:00:00Z'); - - // Re-running migrations must not resurrect the consent (i.e. must not overwrite revoked_at). - runMigrations(sqlite); - - const row = sqlite - .prepare(`SELECT granted_at, revoked_at FROM user_consents WHERE user_id = 'u4' AND consent_key = 'data:core'`) - .get() as { granted_at: string; revoked_at: string | null }; - expect(row.revoked_at).toBe('2026-04-15T00:00:00Z'); - expect(row.granted_at).toBe('2026-04-01T00:00:00Z'); - }); -}); diff --git a/services/api/src/db/migrations.ts b/services/api/src/db/migrations.ts index 874c087..538b898 100644 --- a/services/api/src/db/migrations.ts +++ b/services/api/src/db/migrations.ts @@ -15,8 +15,6 @@ export function runMigrations(handle: BetterSqlite3Database) { image TEXT, google_id TEXT UNIQUE, role TEXT NOT NULL DEFAULT 'user', - consent_given INTEGER NOT NULL DEFAULT 0, - consent_at TEXT, created_at TEXT NOT NULL, deleted_at TEXT ); @@ -199,16 +197,25 @@ export function runMigrations(handle: BetterSqlite3Database) { try { handle.exec(stmt); } catch { /* column already exists */ } } - // Backfill: ADR-0014 collapses users.consent_given into user_consents - // (consent_key='data:core'). Idempotent — INSERT OR IGNORE on the - // composite PK skips users already migrated. Stays in place until the - // column is dropped (PR 6 of the migration plan). - handle.exec(` - INSERT OR IGNORE INTO user_consents (user_id, consent_key, granted_at) - SELECT id, 'data:core', COALESCE(consent_at, created_at) - FROM users - WHERE consent_given = 1 - `); + // Backfill (ADR-0014 step 2): migrate consent_given=1 rows into user_consents. + // Wrapped in try/catch — silently skips on new DBs where consent_given never existed. + try { + handle.exec(` + INSERT OR IGNORE INTO user_consents (user_id, consent_key, granted_at) + SELECT id, 'data:core', COALESCE(consent_at, created_at) + FROM users + WHERE consent_given = 1 + `); + } catch { /* column already dropped — nothing to backfill */ } + + // Drop legacy consent columns (ADR-0014 step 8). Runs after the backfill above. + // Silently skips if already dropped (column not found error) or never existed (new DB). + for (const stmt of [ + `ALTER TABLE users DROP COLUMN consent_given`, + `ALTER TABLE users DROP COLUMN consent_at`, + ]) { + try { handle.exec(stmt); } catch { /* already dropped or never existed */ } + } // Seed first admin from env (ADMIN_SEED_EMAIL). const seedEmail = process.env.ADMIN_SEED_EMAIL; diff --git a/services/api/src/db/schema.ts b/services/api/src/db/schema.ts index 486ff5f..c9fe035 100644 --- a/services/api/src/db/schema.ts +++ b/services/api/src/db/schema.ts @@ -7,10 +7,6 @@ export const users = sqliteTable('users', { image: text('image'), googleId: text('google_id').unique(), role: text('role').notNull().default('user'), // 'user' | 'admin' - // Legacy single-bit consent. Superseded by user_consents (consent_key='data:core'). - // Kept for one release per ADR-0014 migration plan; reads consult both, writes go to user_consents only. - consentGiven: integer('consent_given', { mode: 'boolean' }).notNull().default(false), - consentAt: text('consent_at'), // Stable globals (ADR-0014). Per-agent prefs land in user_preferences instead. tone: text('tone'), // 'direct' | 'gentle' | 'motivational' tipKindsJson: text('tip_kinds_json'), // JSON array of allowed tip kinds; null = all diff --git a/services/api/src/profile/__tests__/builder.test.ts b/services/api/src/profile/__tests__/builder.test.ts index 8403f2b..0233b92 100644 --- a/services/api/src/profile/__tests__/builder.test.ts +++ b/services/api/src/profile/__tests__/builder.test.ts @@ -24,8 +24,8 @@ const SHORT_AGO = new Date(Date.now() - 30_000).toISOString(); beforeAll(async () => { await testDb.insert(users).values([ - { id: 'pf-user-1', email: 'pf1@test.com', role: 'user', consentGiven: true, consentAt: NOW, createdAt: NOW }, - { id: 'pf-user-empty', email: 'pfempty@test.com', role: 'user', consentGiven: true, consentAt: NOW, createdAt: NOW }, + { id: 'pf-user-1', email: 'pf1@test.com', role: 'user', createdAt: NOW }, + { id: 'pf-user-empty', email: 'pfempty@test.com', role: 'user', createdAt: NOW }, ]); }); diff --git a/services/api/src/profile/__tests__/eligibility.test.ts b/services/api/src/profile/__tests__/eligibility.test.ts index 67f9960..34fd74a 100644 --- a/services/api/src/profile/__tests__/eligibility.test.ts +++ b/services/api/src/profile/__tests__/eligibility.test.ts @@ -74,17 +74,6 @@ describe('getEligibleAgentIds', () => { expect(ids.has('agent-b')).toBe(false); }); - it('respects legacy consentGiven bit as data:core', async () => { - mockFetchRegistry.mockResolvedValue({ agents: [AGENT_A] }); - // no consent rows, but legacy bit set - await testDb.update(users).set({ consentGiven: true }); - - const ids = await getEligibleAgentIds('u1'); - expect(ids.has('agent-a')).toBe(true); - - await testDb.update(users).set({ consentGiven: false }); - }); - it('silences agents whose silenced_in_contexts intersects active contexts', async () => { mockFetchRegistry.mockResolvedValue({ agents: [AGENT_A, AGENT_C] }); // ensure data:core granted diff --git a/services/api/src/profile/__tests__/subscriber.test.ts b/services/api/src/profile/__tests__/subscriber.test.ts index d03bf06..8ea1931 100644 --- a/services/api/src/profile/__tests__/subscriber.test.ts +++ b/services/api/src/profile/__tests__/subscriber.test.ts @@ -23,8 +23,8 @@ const STALE_BASE = { beforeAll(async () => { await testDb.insert(users).values([ - { id: 'sub-user-1', email: 'sub1@test.com', role: 'user', consentGiven: true, consentAt: NOW, createdAt: NOW }, - { id: 'sub-user-2', email: 'sub2@test.com', role: 'user', consentGiven: true, consentAt: NOW, createdAt: NOW }, + { id: 'sub-user-1', email: 'sub1@test.com', role: 'user', createdAt: NOW }, + { id: 'sub-user-2', email: 'sub2@test.com', role: 'user', createdAt: NOW }, ]); }); diff --git a/services/api/src/profile/eligibility.ts b/services/api/src/profile/eligibility.ts index 7f7bd4c..df6b536 100644 --- a/services/api/src/profile/eligibility.ts +++ b/services/api/src/profile/eligibility.ts @@ -11,7 +11,7 @@ * consent checks. */ import { db } from '../db/index.js'; -import { users, userConsents, userPreferences, userContexts } from '../db/schema.js'; +import { userConsents, userPreferences, userContexts } from '../db/schema.js'; import { eq, and, isNull } from 'drizzle-orm'; import { fetchRegistry } from '../routes/agent-registry.js'; @@ -34,7 +34,7 @@ export async function getEligibleAgentIds(userId: string): Promise> return new Set(); } - const [consentRows, prefRows, contextRows, userRow] = await Promise.all([ + const [consentRows, prefRows, contextRows] = await Promise.all([ db .select({ consentKey: userConsents.consentKey }) .from(userConsents) @@ -47,19 +47,10 @@ export async function getEligibleAgentIds(userId: string): Promise> .select({ name: userContexts.name, active: userContexts.active }) .from(userContexts) .where(and(eq(userContexts.userId, userId), eq(userContexts.active, true))), - db - .select({ consentGiven: users.consentGiven }) - .from(users) - .where(eq(users.id, userId)) - .limit(1), ]); // Active consents (granted + not revoked) const activeConsents = new Set(consentRows.map((r) => r.consentKey)); - // Legacy fallback: consentGiven bit counts as data:core - if (!activeConsents.has('data:core') && userRow[0]?.consentGiven) { - activeConsents.add('data:core'); - } // Active context names const activeContextNames = new Set(contextRows.map((r) => r.name)); diff --git a/services/api/src/routes/__tests__/admin.test.ts b/services/api/src/routes/__tests__/admin.test.ts index 413097c..adb3e3f 100644 --- a/services/api/src/routes/__tests__/admin.test.ts +++ b/services/api/src/routes/__tests__/admin.test.ts @@ -38,9 +38,9 @@ const DAY_AGO = new Date(Date.now() - 23 * 60 * 60 * 1000).toISOString(); beforeAll(async () => { await testDb.insert(users).values([ - { id: 'admin-1', email: 'admin@test.com', role: 'admin', consentGiven: true, consentAt: NOW, createdAt: NOW }, - { id: 'user-1', email: 'alice@test.com', role: 'user', consentGiven: true, consentAt: NOW, createdAt: NOW }, - { id: 'user-2', email: 'bob@test.com', role: 'user', consentGiven: false, createdAt: NOW }, + { id: 'admin-1', email: 'admin@test.com', role: 'admin', createdAt: NOW }, + { id: 'user-1', email: 'alice@test.com', role: 'user', createdAt: NOW }, + { id: 'user-2', email: 'bob@test.com', role: 'user', createdAt: NOW }, ]); await testDb.insert(integrationTokens).values([ { id: 'tok-1', userId: 'user-1', provider: 'todoist', accessToken: 'secret', connectedAt: NOW }, diff --git a/services/api/src/routes/__tests__/profile.test.ts b/services/api/src/routes/__tests__/profile.test.ts index 58dff8a..0df8227 100644 --- a/services/api/src/routes/__tests__/profile.test.ts +++ b/services/api/src/routes/__tests__/profile.test.ts @@ -72,7 +72,6 @@ beforeAll(async () => { name: 'Alice', image: null, role: 'user', - consentGiven: false, tone: 'direct', tipKindsJson: JSON.stringify(['task', 'advice']), createdAt: NOW, @@ -90,13 +89,6 @@ describe('GET /api/profile', () => { expect(body.contexts).toEqual([]); }); - it('surfaces legacy consentGiven as data:core when no consent row exists', async () => { - await testDb.update(users).set({ consentGiven: true, consentAt: NOW }); - const res = await c('GET', '/api/profile'); - expect((res.body as any).consents['data:core']).toMatchObject({ revokedAt: null }); - await testDb.update(users).set({ consentGiven: false }); - }); - it('includes prefs grouped by scope', async () => { await testDb.insert(userPreferences).values([ { userId: 'user-1', scope: 'orchestrator', key: 'quietHours', valueJson: '"22:00-07:00"', source: 'user', updatedAt: NOW }, diff --git a/services/api/src/routes/__tests__/recommender.test.ts b/services/api/src/routes/__tests__/recommender.test.ts index 84eed95..27d44e2 100644 --- a/services/api/src/routes/__tests__/recommender.test.ts +++ b/services/api/src/routes/__tests__/recommender.test.ts @@ -58,7 +58,7 @@ describe('POST /recommend integration', () => { beforeAll(async () => { await testDb.insert(users).values({ id: 'user-1', email: 'u@test.com', role: 'user', - consentGiven: true, createdAt: new Date().toISOString(), + createdAt: new Date().toISOString(), }); await testDb.insert(integrationTokens).values({ id: 'tok-1', userId: 'user-1', provider: 'todoist', diff --git a/services/api/src/routes/admin.ts b/services/api/src/routes/admin.ts index 77ed599..ae16549 100644 --- a/services/api/src/routes/admin.ts +++ b/services/api/src/routes/admin.ts @@ -98,7 +98,6 @@ router.get('/users', async (req: AuthenticatedRequest, res: Response) => { name: users.name, image: users.image, role: users.role, - consentGiven: users.consentGiven, createdAt: users.createdAt, deletedAt: users.deletedAt, }) @@ -161,8 +160,6 @@ router.get('/users/:id', async (req: AuthenticatedRequest, res: Response) => { name: user.name, image: user.image, role: user.role, - consentGiven: user.consentGiven, - consentAt: user.consentAt, createdAt: user.createdAt, deletedAt: user.deletedAt, }, diff --git a/services/api/src/routes/auth.ts b/services/api/src/routes/auth.ts index 47b5b23..ab9abf7 100644 --- a/services/api/src/routes/auth.ts +++ b/services/api/src/routes/auth.ts @@ -2,7 +2,7 @@ import { type Router as ExpressRouter, Router, Request, Response } from 'express import * as client from 'openid-client'; import { nanoid } from 'nanoid'; import { db } from '../db/index.js'; -import { users, sessions } from '../db/schema.js'; +import { users, sessions, userConsents } from '../db/schema.js'; import { eq } from 'drizzle-orm'; import { config } from '../config.js'; import { logger } from '../logger.js'; @@ -104,7 +104,8 @@ router.get('/callback', async (req: Request, res: Response) => { if (!user) { const id = nanoid(); - await db.insert(users).values({ id, email, name, image, googleId, createdAt: now, consentGiven: true, consentAt: now }); + await db.insert(users).values({ id, email, name, image, googleId, createdAt: now }); + await db.insert(userConsents).values({ userId: id, consentKey: 'data:core', grantedAt: now }); [user] = await db.select().from(users).where(eq(users.id, id)).limit(1); } diff --git a/services/api/src/routes/profile.ts b/services/api/src/routes/profile.ts index 912658b..66d859f 100644 --- a/services/api/src/routes/profile.ts +++ b/services/api/src/routes/profile.ts @@ -48,15 +48,10 @@ router.get('/', requireAuth as any, async (req: AuthenticatedRequest, res: Respo } // Consents: include both active and revoked (callers can filter on revokedAt) - // Also fold in the legacy consentGiven bit if no user_consents row exists yet. const consentMap: Record = {}; for (const c of consents) { consentMap[c.consentKey] = { grantedAt: c.grantedAt, revokedAt: c.revokedAt ?? null }; } - // Legacy fallback: if data:core is missing and the old bit is set, synthesise it. - if (!consentMap['data:core'] && user.consentGiven) { - consentMap['data:core'] = { grantedAt: user.consentAt ?? user.createdAt, revokedAt: null }; - } res.json({ user: { diff --git a/services/api/src/routes/user.ts b/services/api/src/routes/user.ts index cda9d7f..7e1657c 100644 --- a/services/api/src/routes/user.ts +++ b/services/api/src/routes/user.ts @@ -1,7 +1,7 @@ import { type Router as ExpressRouter, Router, Response } from 'express'; import { db } from '../db/index.js'; -import { users, integrationTokens, tipFeedback, tipViews, sessions } from '../db/schema.js'; -import { eq } from 'drizzle-orm'; +import { users, integrationTokens, tipFeedback, tipViews, sessions, userConsents } from '../db/schema.js'; +import { eq, and, isNull } from 'drizzle-orm'; import { requireAuth, AuthenticatedRequest } from '../middleware/session.js'; const router: ExpressRouter = Router(); @@ -20,16 +20,19 @@ router.get('/me', requireAuth, async (req: AuthenticatedRequest, res: Response) image: user.image, role: user.role, createdAt: user.createdAt, - consentGiven: user.consentGiven, }); }); -/** POST /api/user/consent — record consent */ +/** POST /api/user/consent — record data:core consent */ router.post('/consent', requireAuth, async (req: AuthenticatedRequest, res: Response) => { + const now = new Date().toISOString(); await db - .update(users) - .set({ consentGiven: true, consentAt: new Date().toISOString() }) - .where(eq(users.id, req.userId!)); + .insert(userConsents) + .values({ userId: req.userId!, consentKey: 'data:core', grantedAt: now }) + .onConflictDoUpdate({ + target: [userConsents.userId, userConsents.consentKey], + set: { grantedAt: now, revokedAt: null }, + }); res.json({ ok: true }); }); diff --git a/services/api/src/test/db.ts b/services/api/src/test/db.ts index d57f552..59b0b22 100644 --- a/services/api/src/test/db.ts +++ b/services/api/src/test/db.ts @@ -20,8 +20,6 @@ export function makeTestDb(): DrizzleDb & { rawSqlite: BetterSqlite3Database } { image TEXT, google_id TEXT UNIQUE, role TEXT NOT NULL DEFAULT 'user', - consent_given INTEGER NOT NULL DEFAULT 0, - consent_at TEXT, tone TEXT, tip_kinds_json TEXT, created_at TEXT NOT NULL,