From c60e3d658342ceea2822dfae416e432ca44bb63e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Sat, 22 Aug 2026 00:45:57 +0000 Subject: [PATCH] every route can name its mailbox, and the list can span them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Multiple accounts were already supported everywhere except where it counted. email_accounts is keyed (user_id, email), each account syncs into its own emails.db, and IDLE watchers run per account — so three accounts would all sync happily and only one could ever be read. The assumption lived in two lines: store.ts openUserEmailDb() accounts.find(a => a.enabled) ?? accounts[0] routes.ts getSmtpTransport() accounts.find(a => a.enabled) ?? accounts[0] The first had thirteen callers — every read route. The second decided who sends. Now `?account=` names one mailbox and `?account=all` spans them. Absent stays the old behaviour, first enabled, because the mobile app and the agent's email_db tool both call these routes and know nothing about accounts; making it required would have broken both silently. ONE DATABASE PER ACCOUNT, and the unified view is a MERGE rather than a join. Not a limitation — emails_fts is a per-database FTS5 virtual table and MATCH cannot be aimed across an ATTACH boundary through a view, so a unified search has to be a merge of local searches. The upside is that every SQL string in store.ts is reused verbatim, searchEmails included, so the operator parser and the OR branching are not reimplemented for the unified case. The merge is a distributed top-K and it is exact, not approximate: each mailbox is asked for offset+limit rows, because a row ranked third locally can belong fortieth globally and anything less leaves a hole in a later page. Threads are collapsed per mailbox before merging, deliberately. A conversation does not continue across two accounts, so local collapse is both cheaper and more correct than any cross-database window function. Four details worth keeping: - Routes addressed by an id search the scope for the mailbox HOLDING it. A unified list returns ids from every account, so opening the first would 404 on mail plainly on screen. The predicate is `id OR thread_id` because these routes take both and the id spaces overlap by design. - sync-status across mailboxes reports the OLDEST stamp. "Synced as of" is a claim about everything on screen and stops being true at the laggard; the freshest would call a unified inbox current while one account had not been reached for a day. - Sending honours the named account and REFUSES an id that is not the caller's, rather than falling back to the default. Replying from the wrong account leaks which addresses belong to one person and breaks the thread for the recipient. - queryAcross has no `descending` option. It had one for twenty minutes; a test caught that it flipped the merge while every caller's SQL still said ORDER BY date DESC, so asking for the oldest mail returned the oldest of the newest page. The requirement — mailboxes hand back newest-first — is now written down and pinned by a test that demonstrates the silent wrongness rather than asserting the fix. UI is next: the picker for switch-or-all. Co-Authored-By: Claude Opus 5 --- sidecar/mail-scope.test.ts | 58 +++++++++++++ sidecar/mail-scope.ts | 138 +++++++++++++++++++++++++++++ sidecar/routes.ts | 174 ++++++++++++++++++++++++++++--------- sidecar/store.ts | 27 +++++- 4 files changed, 352 insertions(+), 45 deletions(-) create mode 100644 sidecar/mail-scope.test.ts create mode 100644 sidecar/mail-scope.ts diff --git a/sidecar/mail-scope.test.ts b/sidecar/mail-scope.test.ts new file mode 100644 index 0000000..e73e8a5 --- /dev/null +++ b/sidecar/mail-scope.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, it } from 'bun:test'; +import { queryAcross } from './mail-scope'; + +// The merge is the part that can be silently wrong: a page with a hole in it looks like missing mail rather +// than like a bug. These pin the property that makes it exact — each mailbox is asked for offset+limit. +const mailbox = (dates: string[]) => + ({ + account: { id: dates.length, email: `a${dates.length}@x` }, + db: {} as never, + }) as never; + +describe('queryAcross', () => { + const A = ['2026-08-10', '2026-08-08', '2026-08-02']; + const B = ['2026-08-09', '2026-08-07', '2026-08-01']; + + const opened = [ + { account: { id: 1, email: 'a@x' }, db: 'A' }, + { account: { id: 2, email: 'b@x' }, db: 'B' }, + ] as never as Parameters[0]; + + const run = (db: unknown, take: number) => + ((db as unknown as string) === 'A' ? A : B).slice(0, take).map((date) => ({ date })); + + it('interleaves newest-first across mailboxes', () => { + const rows = queryAcross(opened, run as never, { sortBy: (r) => String(r.date), limit: 4, offset: 0 }); + expect(rows.map((r) => r.date)).toEqual(['2026-08-10', '2026-08-09', '2026-08-08', '2026-08-07']); + }); + + it('labels every row with the mailbox it came from', () => { + const rows = queryAcross(opened, run as never, { sortBy: (r) => String(r.date), limit: 2, offset: 0 }); + expect(rows.map((r) => r.accountEmail)).toEqual(['a@x', 'b@x']); + }); + + // The reason each mailbox is asked for offset+limit rather than limit: page 2 must not skip a row that + // ranked low locally and high globally. + it('pages without a hole', () => { + const p1 = queryAcross(opened, run as never, { sortBy: (r) => String(r.date), limit: 2, offset: 0 }); + const p2 = queryAcross(opened, run as never, { sortBy: (r) => String(r.date), limit: 2, offset: 2 }); + const p3 = queryAcross(opened, run as never, { sortBy: (r) => String(r.date), limit: 2, offset: 4 }); + expect([...p1, ...p2, ...p3].map((r) => r.date)).toEqual([...A, ...B].sort().reverse()); + }); + + // The contract, pinned: local order must match merge order. A mailbox handing back rows in the wrong + // direction breaks top-K silently, which is why the `descending` option was removed rather than fixed — + // it flipped the merge while every caller's SQL still said ORDER BY date DESC. + it('trusts each mailbox to be newest-first', () => { + const backwards = (db: unknown, take: number) => + ((db as unknown as string) === 'A' ? [...A].reverse() : [...B].reverse()) + .slice(0, take) + .map((date) => ({ date })); + const rows = queryAcross(opened, backwards as never, { sortBy: (r) => String(r.date), limit: 2, offset: 0 }); + // The true two newest are 08-10 and 08-09. Fed oldest-first mailboxes, the helper sorts correctly + // within what it was handed and returns neither — no error, no gap, just quietly the wrong mail. That + // is the failure the written contract exists to prevent, and it is why the `descending` option went. + expect(rows.map((r) => r.date)).toEqual(['2026-08-08', '2026-08-07']); + expect(rows.map((r) => r.date)).not.toContain('2026-08-10'); + }); +}); diff --git a/sidecar/mail-scope.ts b/sidecar/mail-scope.ts new file mode 100644 index 0000000..c3e0496 --- /dev/null +++ b/sidecar/mail-scope.ts @@ -0,0 +1,138 @@ +import type { Database } from 'bun:sqlite'; +import type { EmailAccount } from '../db/types'; +import { getEmailAccounts } from '../db/queries'; +import { openEmailDb } from './store'; + +/* + Which mailboxes a request is about. + + Every read route used to answer this the same way, in a line that appeared twice and was copied a third + time into the web list: + + const account = accounts.find((a) => a.enabled) ?? accounts[0]; + + So an account could be added, could sync, could fill its own database — and could never be read. Thirteen + routes looked at the first one and nothing said so. + + ── One database per account, and no ATTACH ── + + Each account keeps its own `emails.db`. A unified view is therefore a MERGE, not a join, and that is a + choice rather than a limitation: `emails_fts` is a per-database FTS5 virtual table, and `MATCH` cannot be + aimed through a view over an ATTACHed sibling. Merging keeps every SQL string in `store.ts` exactly as it + is — the same query, run per account, combined here. + + The merge is a distributed top-K, which is EXACT rather than approximate: to serve rows + [offset, offset+limit) sorted by date across N mailboxes, ask each for its first (offset+limit) rows and + take the window from the combined sort. Nothing in a mailbox's tail can outrank a row it has already been + outranked by locally. +*/ + +export type MailScope = { + /** The accounts in play. One for a normal request, all enabled ones for `?account=all`. */ + accounts: EmailAccount[]; + /** True when this is the unified view — routes that cannot merge use it to refuse clearly. */ + unified: boolean; +}; + +/** + * Resolve the `account` query parameter. + * + * Absent means "the first enabled one", which is what every existing client sends and what the mobile app + * and the agent's `email_db` tool still assume. Making it required would have broken both silently, so the + * old behaviour is the default and the parameter is how you opt into knowing better. + * + * A numeric id must belong to the caller. `getEmailAccounts(userId)` is already scoped to them, so the id + * is checked against that list rather than fetched directly — an account id from another user resolves to + * nothing rather than to somebody else's mail. + */ +export async function resolveMailScope(userId: number, param: string | undefined): Promise { + const accounts = await getEmailAccounts(userId); + const enabled = accounts.filter((a) => a.enabled); + + if (param === 'all') return { accounts: enabled.length ? enabled : accounts, unified: true }; + + if (param) { + const id = Number(param); + const one = Number.isFinite(id) ? accounts.find((a) => a.id === id) : undefined; + return { accounts: one ? [one] : [], unified: false }; + } + + const first = enabled[0] ?? accounts[0]; + return { accounts: first ? [first] : [], unified: false }; +} + +/** Open every mailbox in the scope, tagged so a merged row can say where it came from. */ +export function openScope(ownerEmail: string, scope: MailScope): Array<{ account: EmailAccount; db: Database }> { + return scope.accounts.map((account) => ({ account, db: openEmailDb(ownerEmail, account.email) })); +} + +type AcrossOptions = { + /** The value each row is ordered by. Dates are ISO strings here, so string compare is chronological. */ + sortBy: (row: T) => string | number; + limit: number; + offset: number; +}; + +/** + * Run one query across several mailboxes and return the correct window of the combined result. + * + * Each mailbox is asked for `offset + limit` rows, not `limit`: a row at position 3 in one mailbox can + * belong at position 40 of the merge, so anything less risks a hole in a later page. That is also the cost + * — page 20 of a unified inbox reads 20 pages from every account. Acceptable for the page depths anybody + * scrolls to by hand, and the reason a unified view should not be the place anyone paginates deeply. + * + * **The per-mailbox query must already be ordered newest-first**, the same direction as the merge. That is + * the whole basis of a top-K merge: taking the first K locally is only safe if local order agrees with + * global order. This briefly had a `descending: false` option, which was wrong in a way a test caught — it + * flipped the merge while the callers' SQL still said `ORDER BY date DESC`, so asking for the oldest mail + * returned the oldest of the newest page. There is no caller for ascending, and an option that is only + * correct if you also remember to flip the SQL is worse than no option. + */ +export function queryAcross>( + opened: Array<{ account: EmailAccount; db: Database }>, + run: (db: Database, take: number) => T[], + options: AcrossOptions, +): Array { + const take = options.offset + options.limit; + const rows = opened.flatMap(({ account, db }) => + run(db, take).map((row) => ({ ...row, accountId: account.id, accountEmail: account.email })), + ); + rows.sort((a, b) => { + const av = options.sortBy(a); + const bv = options.sortBy(b); + return av === bv ? 0 : av > bv ? -1 : 1; + }); + return rows.slice(options.offset, options.offset + options.limit); +} + +/** + * Open the one mailbox holding a message or a thread, closing the rest. + * + * Matches `id` OR `thread_id`, because these routes are addressed by both and the two id spaces overlap by + * design: a thread id is `sha1(Message-Id)` of the root message, so it usually IS a message id — but not + * for mail threaded by the subject fallback, where it is not. One predicate covers both without the caller + * having to know which kind of id it is holding. + * + * Asks each mailbox in turn and stops at the first hit. A collision across accounts would need the same + * message in both, in which case either copy answers the question correctly. + */ +export async function openHolding( + ownerEmail: string, + userId: number, + accountParam: string | undefined, + id: string, +): Promise { + const scope = await resolveMailScope(userId, accountParam); + const opened = openScope(ownerEmail, scope); + let held: Database | null = null; + for (const { db } of opened) { + if (held) { + db.close(); + continue; + } + const hit = db.query('SELECT 1 FROM emails WHERE id = ? OR thread_id = ? LIMIT 1').get(id, id); + if (hit) held = db; + else db.close(); + } + return held; +} diff --git a/sidecar/routes.ts b/sidecar/routes.ts index b7c3110..fc23484 100644 --- a/sidecar/routes.ts +++ b/sidecar/routes.ts @@ -8,18 +8,39 @@ import { getEmailAttachmentCacheDir } from '@@/data-path'; import { getEmailAccounts } from '../db/queries'; import { openEmailDb, openUserEmailDb, rowToSummary, getSyncMeta, searchEmails } from './store'; import { accountsRouter } from './accounts'; +import { openHolding, openScope, queryAcross, resolveMailScope } from './mail-scope'; export const emailRouter = createRouter(); emailRouter.route('/accounts', accountsRouter); // ── Sending (SMTP) — sends as the connected account using its app password ── +/** + * The mailbox a message goes out from. + * + * `accountParam` is the account id the composer names. It matters most for a REPLY: mail arrives on one + * account, and answering it from another is not a cosmetic slip — it leaks which addresses belong to the + * same person, and the thread breaks for the recipient. `emails.email_account` records where each message + * landed, so the composer can name the right one and this can honour it. + * + * `all` is not a sender. It falls through to the default, because "send from every account" is not a thing + * anybody means — and a unified compose with no account chosen has to pick one rather than refuse. + */ async function getSmtpTransport( userId: number, + accountParam?: string, ): Promise<{ transport: ReturnType; from: string }> { const accounts = await getEmailAccounts(userId); - const acct = accounts.find((a) => a.enabled) ?? accounts[0]; + const asked = + accountParam && accountParam !== 'all' && Number.isFinite(Number(accountParam)) + ? accounts.find((a) => a.id === Number(accountParam)) + : undefined; + const acct = asked ?? accounts.find((a) => a.enabled) ?? accounts[0]; if (!acct) throw errors.BAD_REQUEST('No email account configured'); + if (accountParam && accountParam !== 'all' && !asked) { + // Naming an account that is not yours must not quietly send as you. Refusing is the only safe answer. + throw errors.BAD_REQUEST('That mail account does not exist on this login'); + } const pass = (acct.credentials as Record | null)?.password as string | undefined; if (!pass) throw errors.BAD_REQUEST('This account has no SMTP password — sending needs an app-password account'); // Gmail: smtp.gmail.com:465 (SSL). Derive from the IMAP host for other providers. @@ -60,7 +81,11 @@ emailRouter.post('/send', async (ctx) => { const html = str(form.html).trim(); const inReplyTo = str(form.inReplyTo).trim(); - const { transport, from } = await getSmtpTransport(user.id); + // The composer names the account; a reply names the one the original arrived on. + const { transport, from } = await getSmtpTransport( + user.id, + typeof form.account === 'string' ? form.account : ctx.req.query('account'), + ); await transport.sendMail({ from, to, @@ -79,7 +104,7 @@ emailRouter.post('/send', async (ctx) => { emailRouter.get('/contacts', async (ctx) => { const user = ctx.get('user'); const like = `%${(ctx.req.query('q') ?? '').trim().toLowerCase()}%`; - const db = await openUserEmailDb(user.email, user.id); + const db = await openUserEmailDb(user.email, user.id, ctx.req.query('account')); if (!db) return ctx.json([]); try { const rows = db @@ -161,13 +186,32 @@ emailRouter.get('/search', async (ctx) => { const offset = (page - 1) * limit; if (!q) return ctx.json({ messages: [], total: 0 }); - const db = await openUserEmailDb(user.email, user.id); - if (!db) return ctx.json({ messages: [], total: 0 }); + const scope = await resolveMailScope(user.id, ctx.req.query('account')); + const opened = openScope(user.email, scope); + if (!opened.length) return ctx.json({ messages: [], total: 0 }); try { - const { rows, total } = searchEmails(db, q, limit, offset); - return ctx.json({ messages: rows.map(rowToSummary), total }); + // `searchEmails` is reused verbatim, per mailbox. It has to be: `emails_fts` is a per-database FTS5 + // virtual table and `MATCH` cannot be aimed across an ATTACH boundary, so a unified search is a merge + // of local searches rather than one query. The upside is that the operator parser, the OR branching + // and every FTS detail stay in one place and are not reimplemented for the unified case. + let total = 0; + const merged = queryAcross( + opened, + (db, take) => { + const { rows, total: t } = searchEmails(db, q, take, 0); + total += t; + return rows as unknown as Array>; + }, + { sortBy: (row) => String(row.date ?? ''), limit, offset }, + ); + const messages = merged.map((row) => ({ + ...rowToSummary(row), + account: row.accountEmail, + accountId: row.accountId, + })); + return ctx.json({ messages, total }); } finally { - db.close(); + for (const { db } of opened) db.close(); } }); @@ -190,31 +234,56 @@ emailRouter.get('/messages', async (ctx) => { const { where: folderWhere, params: folderParams } = folderFilter(folder); - const db = await openUserEmailDb(user.email, user.id); - if (!db) return ctx.json({ messages: [], total: 0 }); + const scope = await resolveMailScope(user.id, ctx.req.query('account')); + const opened = openScope(user.email, scope); + if (!opened.length) return ctx.json({ messages: [], total: 0 }); try { // One row per conversation: the latest message in each thread within this folder, plus the // thread's message count and how many are unread. COALESCE guards any un-backfilled rows. - const rows = db - .query( - `SELECT * FROM ( - SELECT e.*, - COUNT(*) OVER (PARTITION BY COALESCE(thread_id, id)) AS thread_count, - SUM(CASE WHEN read = 0 THEN 1 ELSE 0 END) OVER (PARTITION BY COALESCE(thread_id, id)) AS thread_unread, - ROW_NUMBER() OVER (PARTITION BY COALESCE(thread_id, id) ORDER BY date DESC, id DESC) AS rn - FROM emails e - WHERE ${folderWhere} - ) WHERE rn = 1 - ORDER BY date DESC LIMIT ? OFFSET ?`, - ) - .all(...folderParams, limit, offset) as Record[]; - const countRow = db - .query(`SELECT COUNT(DISTINCT COALESCE(thread_id, id)) as total FROM emails WHERE ${folderWhere}`) - .get(...folderParams) as { total: number }; - const messages = rows.map(rowToSummary); - return ctx.json({ messages, total: countRow.total }); + // + // Unchanged from the single-account version, and run per mailbox rather than rewritten to span them. + // Threading is per account by definition — a conversation does not continue across two mailboxes — so + // collapsing threads locally and merging the collapsed rows is both cheaper and more correct than any + // cross-database window function would be. + const listOne = (db: (typeof opened)[number]['db'], take: number) => + db + .query( + `SELECT * FROM ( + SELECT e.*, + COUNT(*) OVER (PARTITION BY COALESCE(thread_id, id)) AS thread_count, + SUM(CASE WHEN read = 0 THEN 1 ELSE 0 END) OVER (PARTITION BY COALESCE(thread_id, id)) AS thread_unread, + ROW_NUMBER() OVER (PARTITION BY COALESCE(thread_id, id) ORDER BY date DESC, id DESC) AS rn + FROM emails e + WHERE ${folderWhere} + ) WHERE rn = 1 + ORDER BY date DESC LIMIT ?`, + ) + .all(...folderParams, take) as Record[]; + + const countOne = (db: (typeof opened)[number]['db']) => + ( + db + .query(`SELECT COUNT(DISTINCT COALESCE(thread_id, id)) as total FROM emails WHERE ${folderWhere}`) + .get(...folderParams) as { total: number } + ).total; + + const merged = queryAcross(opened, listOne, { + sortBy: (row) => String(row.date ?? ''), + limit, + offset, + }); + const total = opened.reduce((sum, entry) => sum + countOne(entry.db), 0); + + // `account` rides on every row so a unified list can label who it arrived as, and so a reply can go + // out from the right mailbox without a second lookup. + const messages = merged.map((row) => ({ + ...rowToSummary(row), + account: row.accountEmail, + accountId: row.accountId, + })); + return ctx.json({ messages, total }); } finally { - db.close(); + for (const { db } of opened) db.close(); } }); @@ -248,7 +317,10 @@ emailRouter.get('/messages/:id', async (ctx) => { const user = ctx.get('user'); const id = ctx.req.param('id'); - const db = await openUserEmailDb(user.email, user.id); + // Which mailbox holds it. Addressed by id, so the account cannot be assumed: a unified list returns ids + // from every mailbox, and opening the first would 404 on mail that is plainly on screen. + const holder = await openHolding(user.email, user.id, ctx.req.query('account'), id); + const db = holder; if (!db) return ctx.text('Not found', 404); try { const row = db.query('SELECT * FROM emails WHERE id = ? AND deleted = 0').get(id) as Record | null; @@ -264,7 +336,10 @@ emailRouter.get('/thread/:id', async (ctx) => { const user = ctx.get('user'); const id = ctx.req.param('id'); - const db = await openUserEmailDb(user.email, user.id); + // Which mailbox holds it. Addressed by id, so the account cannot be assumed: a unified list returns ids + // from every mailbox, and opening the first would 404 on mail that is plainly on screen. + const holder = await openHolding(user.email, user.id, ctx.req.query('account'), id); + const db = holder; if (!db) return ctx.text('Not found', 404); try { const head = db.query('SELECT thread_id, subject FROM emails WHERE id = ? AND deleted = 0').get(id) as { @@ -289,7 +364,10 @@ emailRouter.patch('/thread/:id/read', async (ctx) => { const user = ctx.get('user'); const id = ctx.req.param('id'); - const db = await openUserEmailDb(user.email, user.id); + // Which mailbox holds it. Addressed by id, so the account cannot be assumed: a unified list returns ids + // from every mailbox, and opening the first would 404 on mail that is plainly on screen. + const holder = await openHolding(user.email, user.id, ctx.req.query('account'), id); + const db = holder; if (!db) return ctx.text('Not found', 404); try { const head = db.query('SELECT thread_id FROM emails WHERE id = ?').get(id) as { thread_id: string | null } | null; @@ -307,7 +385,10 @@ emailRouter.post('/messages/:id/attachments/:index/extract', async (ctx) => { const id = ctx.req.param('id'); const index = Number(ctx.req.param('index')); - const db = await openUserEmailDb(user.email, user.id); + // Which mailbox holds it. Addressed by id, so the account cannot be assumed: a unified list returns ids + // from every mailbox, and opening the first would 404 on mail that is plainly on screen. + const holder = await openHolding(user.email, user.id, ctx.req.query('account'), id); + const db = holder; if (!db) return ctx.text('Attachment not found', 404); try { const row = db.query('SELECT filename, content FROM attachments WHERE email_id = ? AND idx = ?').get(id, index) as { @@ -339,7 +420,10 @@ emailRouter.patch('/messages/:id/read', async (ctx) => { const user = ctx.get('user'); const id = ctx.req.param('id'); - const db = await openUserEmailDb(user.email, user.id); + // Which mailbox holds it. Addressed by id, so the account cannot be assumed: a unified list returns ids + // from every mailbox, and opening the first would 404 on mail that is plainly on screen. + const holder = await openHolding(user.email, user.id, ctx.req.query('account'), id); + const db = holder; if (!db) return ctx.json({ ok: true }); try { db.run('UPDATE emails SET read = 1 WHERE id = ? AND read = 0', [id]); @@ -353,7 +437,10 @@ emailRouter.delete('/messages/:id', async (ctx) => { const user = ctx.get('user'); const id = ctx.req.param('id'); - const db = await openUserEmailDb(user.email, user.id); + // Which mailbox holds it. Addressed by id, so the account cannot be assumed: a unified list returns ids + // from every mailbox, and opening the first would 404 on mail that is plainly on screen. + const holder = await openHolding(user.email, user.id, ctx.req.query('account'), id); + const db = holder; if (!db) return ctx.text('Not found', 404); try { const result = db.run('UPDATE emails SET deleted = 1 WHERE id = ? AND deleted = 0', [id]); @@ -367,13 +454,18 @@ emailRouter.delete('/messages/:id', async (ctx) => { emailRouter.get('/sync-status', async (ctx) => { const user = ctx.get('user'); - const db = await openUserEmailDb(user.email, user.id); - if (!db) return ctx.json({ lastSyncAt: null }); + const scope = await resolveMailScope(user.id, ctx.req.query('account')); + const opened = openScope(user.email, scope); + if (!opened.length) return ctx.json({ lastSyncAt: null }); try { - const lastSyncAt = getSyncMeta(db, 'last_sync_at'); + // Across mailboxes the honest answer is the OLDEST, not the newest. "Synced as of" is a claim about + // everything on screen, and it stops being true at the laggard — reporting the freshest would say the + // unified inbox is current while one account has not been reached for a day. + const stamps = opened.map(({ db }) => getSyncMeta(db, 'last_sync_at')).filter((v): v is string => !!v); + const lastSyncAt = stamps.length === opened.length && stamps.length ? stamps.sort()[0] : null; return ctx.json({ lastSyncAt }); } finally { - db.close(); + for (const { db } of opened) db.close(); } }); @@ -383,7 +475,7 @@ emailRouter.get('/stats', async (ctx) => { const { where: folderWhere, params: folderParams } = folderFilter(folder); - const db = await openUserEmailDb(user.email, user.id); + const db = await openUserEmailDb(user.email, user.id, ctx.req.query('account')); if (!db) return ctx.json({ total: 0, byDomain: [], bySender: [] }); try { const total = ( @@ -409,7 +501,7 @@ emailRouter.get('/stats', async (ctx) => { emailRouter.get('/labels', async (ctx) => { const user = ctx.get('user'); - const db = await openUserEmailDb(user.email, user.id); + const db = await openUserEmailDb(user.email, user.id, ctx.req.query('account')); if (!db) return ctx.json({ labels: [] }); try { const rows = db.query('SELECT labels FROM emails WHERE deleted = 0 AND labels IS NOT NULL').all() as Array<{ diff --git a/sidecar/store.ts b/sidecar/store.ts index 817b238..ffc84b7 100644 --- a/sidecar/store.ts +++ b/sidecar/store.ts @@ -160,12 +160,31 @@ export function openEmailDb(ownerEmail: string, accountEmail: string): Database } /** - * Open the email DB for a user's configured account. Emails are stored per account - * (email_accounts//emails.db); for now we use the user's first account. Returns null if the - * user has no email account configured yet. + * Open ONE mailbox — the account asked for, or the first enabled one when nothing is asked. + * + * `accountParam` is the `?account=` query value: an account id, `all`, or absent. This used to take no + * parameter at all and always answer with the first enabled account, which is how a second mail account + * could be added, sync a full database, and never be readable — thirteen routes looked at the first one + * and nothing said so. + * + * `all` resolves to the first account HERE rather than throwing, because the routes that can genuinely + * span mailboxes do not come through this function: they use `openScope`/`queryAcross` in `mail-scope.ts`. + * A route still using this one is a route whose answer is per mailbox — a contact list, a sync status — + * and giving it the first account is the same answer it gave yesterday. */ -export async function openUserEmailDb(ownerEmail: string, userId: number): Promise { +export async function openUserEmailDb( + ownerEmail: string, + userId: number, + accountParam?: string, +): Promise { const accounts = await getEmailAccounts(userId); + if (accountParam && accountParam !== 'all') { + const id = Number(accountParam); + // Scoped to this user by `getEmailAccounts`, so an id belonging to somebody else finds nothing rather + // than opening their mail. + const asked = Number.isFinite(id) ? accounts.find((a) => a.id === id) : undefined; + return asked ? openEmailDb(ownerEmail, asked.email) : null; + } const account = accounts.find((a) => a.enabled) ?? accounts[0]; return account ? openEmailDb(ownerEmail, account.email) : null; }