diff --git a/src/servers/api/chat/chat.ts b/src/servers/api/chat/chat.ts index c64ac1d8..fce5fa11 100644 --- a/src/servers/api/chat/chat.ts +++ b/src/servers/api/chat/chat.ts @@ -1,4 +1,4 @@ -import { getPluginChatsDir, getBotsDir } from '@@/data-path'; +import { getPluginChatsDir, getBotsDir, resolveEmailAccountDir } from '@@/data-path'; import { tearDownBot, sweepOrphanWatches } from '../bots/teardown'; import type { Context } from 'hono'; import { emitToSession, sessionKeyForTranscript } from './websocket'; @@ -128,6 +128,14 @@ const cwdOf = (ctx: Context, who: { home: string; email: string }): string => { if (context === 'plugin' && contextId) return getPluginChatsDir(who.email, contextId); // Every bot shares one directory, so this needs no id — the context IS the address. if (context === 'bots') return getBotsDir(who.email); + // An email chat runs in the MAILBOX's directory, beside `emails.db`. `contextId` is the account picked on + // screen, and is optional: without one the resolver takes the most recently synced account. + // + // This branch was missing, and the failure was quiet in the way that costs the most time — the context + // fell through to `who.home`, so the picker above an email composer listed the home's conversations. A + // message sent into the mailbox then could not be found in the list directly above it. Same resolver as + // the socket, so the turn and the list cannot disagree about where the chat lives. + if (context === 'email') return resolveEmailAccountDir(who.email, contextId); return ctx.req.query('cwd')?.trim() || who.home; }; diff --git a/src/servers/api/chat/websocket.ts b/src/servers/api/chat/websocket.ts index 744ceb83..9c567cb0 100644 --- a/src/servers/api/chat/websocket.ts +++ b/src/servers/api/chat/websocket.ts @@ -17,9 +17,8 @@ import * as sidecar from '@@/sidecar-registry'; import { join } from 'path'; import { getOwnerHomeDir, - getEmailAccountsDir, getPluginChatsDir, - findEmailAccountName, + resolveEmailAccountDir, getCommanderDir, } from '../../../servers/data-path'; import { attachedFilePaths } from './attached-files'; @@ -197,24 +196,11 @@ export const resolveBaseCwd = (home: string, cwd?: string) => resolveCwd(home, c // `accountEmail` will come from the account selector (msg.contextId) later; for now default to the // owner's first enabled account. Falls back to the email_accounts root if there are no accounts. async function resolveEmailCwd(ownerEmail: string, accountEmail?: string): Promise { - // Found on disk, never queried. Email is a plugin and the platform may not import its queries; the - // directory layout is already the platform's, so reading it answers the question with no coupling and - // nothing to throw when the plugin is not installed. - // - // The named mailbox has to EXIST before it is joined. `accountEmail` reaches here from the client — it is - // the mailbox picker's `?account=` — and this function ends in `mkdirSync`, so an unrecognised value does - // not fail, it CREATES a directory named after whatever was sent and runs the turn in it. The list's - // unified view uses the sentinel `all`, which is the value most likely to arrive by accident; the panel - // drops it, and this is the half that does not depend on the panel remembering to. - // - // Falling back to the most recently synced account is the same answer the unified list is mostly showing. - const requested = accountEmail?.trim(); - const known = requested && existsSync(join(getEmailAccountsDir(ownerEmail), requested, 'emails.db')); - if (requested && !known) { - console.warn(`[chat] email: no mailbox named ${requested} — using the most recently synced one instead`); - } - const account = (known ? requested : null) ?? findEmailAccountName(ownerEmail); - const dir = account ? join(getEmailAccountsDir(ownerEmail), account) : getEmailAccountsDir(ownerEmail); + // `resolveEmailAccountDir` is shared with `GET /chat/sessions`, deliberately: the turn and the list of + // conversations must resolve the same directory, and they did not — the sessions route knew nothing about + // `email` and fell through to the owner's home, so the picker above this composer offered chats from + // somewhere else. See its own comment for why `accountEmail` is verified rather than sanitised. + const dir = resolveEmailAccountDir(ownerEmail, accountEmail); mkdirSync(dir, { recursive: true }); // The same brief a plugin's own chats get, written where an email chat actually runs. // diff --git a/src/servers/data-path.ts b/src/servers/data-path.ts index bdb8da0e..f5deedb6 100644 --- a/src/servers/data-path.ts +++ b/src/servers/data-path.ts @@ -371,6 +371,35 @@ export function findEmailAccountName(ownerEmail: string): string | null { } } +/** + * The directory an email chat belongs to — the mailbox's own, the one holding `emails.db`. + * + * ── Why this is a function and not two lines at each call site ── + * + * It IS an address, and it had two resolvers. The chat socket resolved it to run the turn; the sessions + * route resolved it to list the conversations. Only the socket knew about `email`, so the picker fell + * through to the owner's home and offered chats from somewhere else entirely — you could send a message + * into the mailbox and then not find it in the list above the composer. Two answers to "where does this + * chat live" is one too many. + * + * `requested` reaches here from the client (the mailbox picker's `?account=`) and is verified against disk + * before it is joined, never sanitised into shape. That matters because the socket's caller ends in + * `mkdirSync`: an unrecognised value would not fail, it would CREATE a directory named after whatever + * arrived. The list's unified view uses the sentinel `all`, which is the value most likely to show up by + * accident. + * + * Falls back to the most recently synced account, and to the accounts root when there is no account at + * all — a real state for a fresh install, and a directory that exists rather than one invented here. + * + * Pure: no mkdir, no writes. The caller that needs the directory to exist says so itself. + */ +export function resolveEmailAccountDir(ownerEmail: string, requested?: string): string { + const named = requested?.trim(); + const known = named && existsSync(join(getEmailAccountsDir(ownerEmail), named, 'emails.db')) ? named : null; + const account = known ?? findEmailAccountName(ownerEmail); + return account ? join(getEmailAccountsDir(ownerEmail), account) : getEmailAccountsDir(ownerEmail); +} + // Sanitises a display username or email into a bare, lowercase, shell-safe token. The name and the // 32-char Linux limit are the last trace of the per-container architecture, where this really did name // a Linux user inside the user's container. Nothing creates a Linux user now — the value is carried diff --git a/src/servers/email-account-dir.test.ts b/src/servers/email-account-dir.test.ts new file mode 100644 index 00000000..3335cd1e --- /dev/null +++ b/src/servers/email-account-dir.test.ts @@ -0,0 +1,55 @@ +import { describe, expect, test, beforeAll, afterAll } from 'bun:test'; +import { mkdirSync, rmSync, writeFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { getEmailAccountsDir, resolveEmailAccountDir } from './data-path'; + +/** + * Where an email chat runs. + * + * This is regression cover rather than API cover. The address had two resolvers — the chat socket's, for + * running the turn, and `GET /chat/sessions`'s, for listing the conversations — and only the socket knew + * about the `email` context. The list fell through to the owner's home, so the picker above an email + * composer offered conversations from somewhere else and a message sent into the mailbox could not be + * found in the list directly above it. One resolver now, and these are the three answers it owes. + */ + +const OWNER = 'resolve-email-dir-test@officer.dev'; +const ROOT = getEmailAccountsDir(OWNER); + +const account = (name: string, withDb = true) => { + const dir = join(ROOT, name); + mkdirSync(dir, { recursive: true }); + if (withDb) writeFileSync(join(dir, 'emails.db'), ''); + return dir; +}; + +describe('resolveEmailAccountDir', () => { + beforeAll(() => { + rmSync(ROOT, { recursive: true, force: true }); + mkdirSync(ROOT, { recursive: true }); + }); + + afterAll(() => rmSync(ROOT, { recursive: true, force: true })); + + test('a named mailbox that exists is the answer', () => { + const dir = account('someone@gmail.com'); + expect(resolveEmailAccountDir(OWNER, 'someone@gmail.com')).toBe(dir); + }); + + test("`all` is the list's sentinel, not an address — it never becomes a directory", () => { + // The caller ends in mkdirSync, so accepting this would CREATE a mailbox named `all` and run turns in + // it. The panel drops the sentinel; this is the half that does not depend on the panel remembering to. + expect(resolveEmailAccountDir(OWNER, 'all')).not.toBe(join(ROOT, 'all')); + expect(resolveEmailAccountDir(OWNER, 'all')).toBe(join(ROOT, 'someone@gmail.com')); + }); + + test('a mailbox with no database is not a mailbox', () => { + account('half-configured@gmail.com', false); + expect(resolveEmailAccountDir(OWNER, 'half-configured@gmail.com')).toBe(join(ROOT, 'someone@gmail.com')); + }); + + test('no account at all resolves to the accounts root, which exists', () => { + const empty = 'no-accounts-test@officer.dev'; + expect(resolveEmailAccountDir(empty)).toBe(getEmailAccountsDir(empty)); + }); +});