From 7e6cfc85af1b0959833c461d57d272d238b0431a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Wed, 26 Aug 2026 18:47:37 +0000 Subject: [PATCH] one resolver for where an email chat lives MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit the picker above an email composer listed conversations from the owner's home — a chat started somewhere else entirely, offered as if it belonged to the mailbox. the address had two resolvers. the chat socket knew `context: 'email'` and ran the turn in the account's directory; `GET /chat/sessions` did not, and fell through to `who.home`. so a message went into the mailbox and could not be found in the list directly above where it was typed. two answers to "where does this chat live" is one too many, and the one that was missing failed quietly. `resolveEmailAccountDir` in data-path.ts is now the only answer, and both call it. it verifies a client-named mailbox against disk rather than sanitising it into shape, which matters because the socket's caller ends in mkdirSync — an unrecognised value would not fail, it would CREATE a directory named after whatever arrived and run turns in it. `all`, the list's own sentinel for the unified view, is exactly the value most likely to arrive by accident. pure: no mkdir, no writes. the caller that needs the directory says so itself. tested, because a missing branch is what this was: the named mailbox wins, `all` never becomes a directory, a mailbox with no emails.db is not a mailbox, and no account at all resolves to the accounts root rather than to something invented. Co-Authored-By: Claude Opus 5 Officer-Session: 656f5c85-e49a-4f1f-b9f3-042a04b4d043 --- src/servers/api/chat/chat.ts | 10 ++++- src/servers/api/chat/websocket.ts | 26 +++---------- src/servers/data-path.ts | 29 ++++++++++++++ src/servers/email-account-dir.test.ts | 55 +++++++++++++++++++++++++++ 4 files changed, 99 insertions(+), 21 deletions(-) create mode 100644 src/servers/email-account-dir.test.ts 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)); + }); +});