diff --git a/CHANGELOG.md b/CHANGELOG.md index 169e6bf..e36d3e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,24 @@ beeperbox follows [Semantic Versioning 2.0.0](https://semver.org/) with one conc Published tags on GHCR: `:X.Y.Z` (exact, immutable), `:X.Y` (rolling within a minor), `:X` (rolling within a major — always `:0` today), `:latest` (newest release tag, rebuilt weekly to pick up upstream Beeper AppImage drift), `:edge` (every push to `master`, may break). +## [Unreleased] + +Account-sync resilience + observability — filed by [multis](https://github.com/hamr0/multis) after a WhatsApp account added via noVNC didn't surface, and a plain `docker restart` didn't recover it (it self-corrected only later). MINOR-bound when released: `list_accounts` gains a new `status` schema field (new runtime behavior per the versioning policy). + +### Fixed + +- **The account map no longer wedges after a runtime account change.** `getAccountMap()` (the `accountID → network` map every chat verb uses for network labels) cached its result for the **whole process lifetime** with no TTL and no invalidation — so an account added at runtime (WhatsApp via noVNC) rendered `network:"unknown"` in `list_inbox` / `list_unread` / `read_chat` / `get_chat` / `search_messages` / `poll_messages` until the MCP **process** restarted. Worse, if the map was first populated during the backend's post-restart **sync window** (when `/v1/accounts` briefly returns empty), it froze that empty map for the whole run — so a plain `docker restart` re-poisoned it from a still-syncing backend and the wedge survived (recovering only on a luckier restart or a full `down`/`up`, exactly the reported symptom). The cache is now bounded by a TTL (`BEEPERBOX_ACCOUNT_CACHE_TTL_MS`, default 60 s) **and an empty result is never cached** — it is served for the one call that saw it but re-read on the next, so the map self-heals the instant Beeper finishes syncing. A runtime account-add is now reflected within the TTL with no restart. + +### Added + +- **`list_accounts` surfaces the backend connection `status`** per account (`"connected"`, `"connecting"`, …; new schema field). A still-syncing bridge is now distinguishable from a real account, instead of a transient reading as "gone". +- **Zero-account observability.** When `/v1/accounts` returns an empty list, both `list_accounts` and the internal account-map refresh log a clear stderr line ("Beeper may still be syncing — retry shortly") instead of silently relaying an ambiguous `0`. A recurrence is now self-diagnosing. +- **`BEEPERBOX_ACCOUNT_CACHE_TTL_MS`** (default `60000`) — TTL on the account-map cache; `0` disables caching (always read `/v1/accounts` live). + +### Documentation + +- `beeperbox.context.md` and `docs/PRD.md` document the new `status` field, the sync-window/retry guidance, and the `BEEPERBOX_ACCOUNT_CACHE_TTL_MS` tunable. + ## [0.8.1] — 2026-06-17 `[PATCH]` Documentation only. PATCH per the versioning policy — the MCP tool surface, raw/HTTP API, `Chat`/`Message` schemas, and default ports are bit-identical to v0.8.0, and no client-code edits are required. The headline is that the READMEs are now current with v0.6 → v0.8: the root README gained a **The MCP** map of all 12 verbs (including the previously undocumented `poll_messages` watch primitive and `download_asset` attachment reach), and the npm-page README was reshaped onto the shared `bare`-ecosystem skeleton. This is the first release whose npm tarball carries the reshaped lite-mode README — npm versions are immutable, so the doc refresh ships under a new version by necessity. diff --git a/beeperbox.context.md b/beeperbox.context.md index 950b679..d03adcb 100644 --- a/beeperbox.context.md +++ b/beeperbox.context.md @@ -122,8 +122,8 @@ All tools are called via JSON-RPC 2.0 `tools/call` with `{name, arguments}`. Req Discover which messaging platforms are connected. **Arguments:** none. -**Returns:** array of `{account_id, network, network_label, user: {id, display_name}}`. -**Use at session start** to see which networks are reachable before making branching decisions. +**Returns:** array of `{account_id, network, network_label, status, user: {id, display_name}}`. `status` is the bridge's backend connection state (`"connected"`, `"connecting"`, …) — use it to tell a still-syncing account from a real one rather than treating a transient as "gone". +**Use at session start** to see which networks are reachable before making branching decisions. A `0`/short result right after a container restart or a noVNC account-add usually means Beeper is still syncing — retry shortly rather than concluding "no accounts" (beeperbox logs this to stderr and never freezes a stale account map). ### `list_inbox` diff --git a/docs/PRD.md b/docs/PRD.md index 00d6e5d..4943c3b 100644 --- a/docs/PRD.md +++ b/docs/PRD.md @@ -111,6 +111,7 @@ All three publish to `127.0.0.1` by design. Remote access is a deliberate opt-in | `BEEPERBOX_PREFLIGHT` / `_PREFLIGHT_TIMEOUT_MS` | `1` (on) / `5000` | Startup preflight: one bounded `/v1/accounts` probe on boot logs a clear reachable-and-token-accepted / unreachable-or-rejected verdict to stderr (matters most in lite mode, which has no Docker `HEALTHCHECK`). Best-effort and non-fatal — an unset token or down API never blocks boot. Set `BEEPERBOX_PREFLIGHT=0` to disable. | | `BEEPERBOX_RESOLVE_RETRIES` / `_DELAY_MS` / `_TIMEOUT_MS` | `4` / `250` / `3000` | Echo-guard id resolution after a send: attempts to resolve `pendingMessageID` → final bridge id, delay between attempts, and per-attempt fetch timeout. Worst-case added send latency is bounded at `retries × (timeout + delay)`. Set retries `0` to disable resolution (falls back to text matching). | | `BEEPERBOX_SUPERVISE` / `_SUPERVISE_INTERVAL` / `_SUPERVISE_API_GRACE` | `1` / `10` / `6` | Backend supervision: enable (`0` = old forward-signal-and-wait), seconds between checks, and consecutive API-down checks (after the API has been up once) before recycling beepertexts. Non-numeric values fall back to the default. | +| `BEEPERBOX_ACCOUNT_CACHE_TTL_MS` | `60000` | TTL on the in-memory accountID → network map that chat normalizers use. Bounds how long a runtime account change (e.g. WhatsApp added via noVNC) can lag before chat verbs show its network label — without this the map was cached for the whole process life and an account-add stayed `network:"unknown"` until a restart. An **empty** `/v1/accounts` (the backend mid-sync after a restart/add) is never cached regardless, so the map self-heals the moment Beeper finishes syncing. Set `0` to disable caching (always read `/v1/accounts` live). | | `VNC_PASSWORD` | unset | When set, the noVNC/x11vnc session requires a password (RFB security type *VNC auth*). | | `BEEPER_VERSION` / `BEEPER_SHA256` | unset (build args) | Pin & hash-verify an exact Beeper AppImage for a reproducible build; unset ⇒ rolling auto-update. | diff --git a/mcp/server.js b/mcp/server.js index 3e6f984..569f225 100644 --- a/mcp/server.js +++ b/mcp/server.js @@ -95,6 +95,21 @@ const ASSET_TIMEOUT_MS = envIntNonNeg('BEEPERBOX_ASSET_TIMEOUT_MS', 30000); // neither, so this is its boot sanity check. Opt out with BEEPERBOX_PREFLIGHT=0. const PREFLIGHT_TIMEOUT_MS = envIntNonNeg('BEEPERBOX_PREFLIGHT_TIMEOUT_MS', 5000); +// The accountID -> network map is cached so chat normalizers don't re-fetch +// /v1/accounts on every call. But an UNBOUNDED cache is a resilience bug: an +// account added at runtime (e.g. WhatsApp via noVNC) would render network +// "unknown" in every chat verb until the MCP *process* restarts, and an empty/ +// partial map captured during the backend's post-restart sync window would +// freeze for the whole process life — so a plain `docker restart` re-poisons it +// from a still-syncing backend and the account list stays wrong (the exact wedge +// multis reported). Bound it with a TTL, and never cache an EMPTY result. Set to +// 0 to disable caching (always read live). +const ACCOUNT_CACHE_TTL_MS = envIntNonNeg('BEEPERBOX_ACCOUNT_CACHE_TTL_MS', 60000); + +// Injectable clock — overridable in tests so the TTL is verifiable without a +// real sleep. Production reads the wall clock. +let nowFn = () => Date.now(); + // Real attachment src_urls come in two shapes: remote Matrix content // (mxc:// / localmxc://) and, once Beeper caches the file locally, // file:///root/.config/BeeperTexts/media/... — verified against a live account. @@ -293,17 +308,59 @@ async function getNoteToSelfChatID() { throw rpcError(-32002, 'note-to-self chat not found in top 100 chats — open Beeper Desktop and verify a "Note to self" chat exists'); } +// Unwrap /v1/accounts into a bare array — Beeper returns a bare array today, but +// tolerate a {items:[...]} envelope. One place so list_accounts, getAccountMap, +// and preflight agree. +function accountList(accounts) { + return Array.isArray(accounts) ? accounts : (accounts?.items || []); +} + +// accountCache: { map, at } | null. Bounded by ACCOUNT_CACHE_TTL_MS and NEVER +// populated from an empty result — see ACCOUNT_CACHE_TTL_MS above for why the old +// unbounded cache wedged after a runtime account-add / post-restart sync window. async function getAccountMap() { - if (accountCache) return accountCache; - const accounts = await beeperFetch('/v1/accounts'); - accountCache = {}; - for (const a of (Array.isArray(accounts) ? accounts : (accounts.items || []))) { - accountCache[a.accountID] = { - network: networkSlug(a.network), - network_label: a.network, - }; + const now = nowFn(); + const cacheValid = + accountCache && + ACCOUNT_CACHE_TTL_MS > 0 && + (now - accountCache.at) < ACCOUNT_CACHE_TTL_MS; + if (cacheValid) return accountCache.map; + + const list = accountList(await beeperFetch('/v1/accounts')); + const map = {}; + for (const a of list) { + map[a.accountID] = { network: networkSlug(a.network), network_label: a.network }; + } + // An empty /v1/accounts is almost always the backend mid-sync (just after a + // restart or an account-add), not a real "no accounts" state. Caching it would + // freeze every chat verb's network labels until the *process* restarts. Serve + // it for THIS call (the verb still returns, just with "unknown" labels) but + // leave the cache empty so the very next call re-reads and self-heals the + // moment the backend populates. Log it so a recurrence is diagnosable, not silent. + if (list.length === 0) { + process.stderr.write('[beeperbox-mcp] /v1/accounts returned 0 accounts — Beeper may still be syncing; not caching, will re-read next call\n'); + accountCache = null; + return map; } - return accountCache; + accountCache = { map, at: now }; + return map; +} + +// Map one raw /v1/accounts entry into the list_accounts shape. `status` is the +// backend's per-account connection state ("connected" / "connecting" / …) — +// surfaced (not dropped) so a caller can tell a still-syncing bridge from a real +// one instead of reading a transient as "gone". +function normalizeAccount(a) { + return { + account_id: a.accountID, + network: networkSlug(a.network), + network_label: a.network, + status: a.status || null, + user: { + id: a.user?.id || null, + display_name: a.user?.fullName || a.user?.displayText || a.user?.username || null, + }, + }; } // ─── chat normalizer ────────────────────────────────────────────── @@ -673,7 +730,7 @@ function applyEchoTags(messages, now) { const TOOLS = [ { name: 'list_accounts', - description: 'List all messaging accounts (networks) connected to this Beeper account. Each account corresponds to one platform — WhatsApp, Telegram, Discord, etc. Use this to see which platforms are reachable before calling other tools, or to discover what kinds of chats exist. Returns network slug (machine-readable, e.g. "whatsapp"), network label (human, e.g. "WhatsApp"), the underlying account ID, and the user\'s display name on that platform.', + description: 'List all messaging accounts (networks) connected to this Beeper account. Each account corresponds to one platform — WhatsApp, Telegram, Discord, etc. Use this to see which platforms are reachable before calling other tools, or to discover what kinds of chats exist. Returns network slug (machine-readable, e.g. "whatsapp"), network label (human, e.g. "WhatsApp"), the underlying account ID, the bridge connection `status` ("connected", "connecting", etc. — lets you tell a still-syncing account from a real one rather than reading a transient as "gone"), and the user\'s display name on that platform.', inputSchema: { type: 'object', properties: {}, @@ -827,17 +884,15 @@ const TOOLS = [ async function callTool(name, args) { switch (name) { case 'list_accounts': { - const accounts = await beeperFetch('/v1/accounts'); - const list = Array.isArray(accounts) ? accounts : (accounts.items || []); - return list.map((a) => ({ - account_id: a.accountID, - network: networkSlug(a.network), - network_label: a.network, - user: { - id: a.user?.id || null, - display_name: a.user?.fullName || a.user?.displayText || a.user?.username || null, - }, - })); + const list = accountList(await beeperFetch('/v1/accounts')); + // A live read, so a 0 here is the backend's current truth — but that is far + // more often "Beeper still syncing after a restart/account-add" than a real + // empty account set. Log it so the operator can tell the difference instead + // of silently relaying an ambiguous empty list. + if (list.length === 0) { + process.stderr.write('[beeperbox-mcp] list_accounts: backend /v1/accounts returned 0 accounts — Beeper may still be syncing (retry shortly)\n'); + } + return list.map(normalizeAccount); } case 'get_chat': { @@ -1310,6 +1365,7 @@ module.exports = { selectDelivery, textHash, normalizeAttachments, + normalizeAccount, assertServableSrcUrl, matchSentMessage, recordSent, @@ -1317,4 +1373,8 @@ module.exports = { loadLedger, // test hook: drop the in-memory ledger so a test can re-load from a fresh path _resetLedger: () => { ledger = null; ledgerPersistWarned = false; }, + // account-map cache surface (resilience): the map + its TTL/empty-cache rules. + getAccountMap, + _resetAccountCache: () => { accountCache = null; }, + _setNow: (fn) => { nowFn = fn || (() => Date.now()); }, }; diff --git a/mcp/server.test.js b/mcp/server.test.js index 387f8ad..9e24142 100644 --- a/mcp/server.test.js +++ b/mcp/server.test.js @@ -12,6 +12,10 @@ const os = require('node:os'); const path = require('node:path'); const fs = require('node:fs'); +// beeperFetch refuses to run without a token (read at module load), and the +// account-map tests below drive it through a stubbed global.fetch. +process.env.BEEPER_TOKEN = process.env.BEEPER_TOKEN || 'test-token'; + const S = require('./server.js'); // ── cursor encode/decode ────────────────────────────────────────── @@ -495,3 +499,77 @@ test('recordSent persists and loadLedger restores across a "restart"', () => { S._resetLedger(); } }); + +// ── account-map cache resilience ────────────────────────────────── +// The multis "added WhatsApp via noVNC → it didn't show, and a plain docker +// restart didn't fix it, it recovered later" wedge. Root cause: getAccountMap +// cached the accountID->network map for the whole process life with no TTL and +// happily froze an EMPTY map captured during the backend's post-restart sync +// window. These prove the bound + never-cache-empty behavior; each is written +// to FAIL against the old unbounded cache. +const REAL_FETCH = global.fetch; + +function stubAccounts(seq) { + let i = 0, calls = 0; + global.fetch = async () => { + calls++; + const body = seq[Math.min(i++, seq.length - 1)]; + return { ok: true, status: 200, text: async () => JSON.stringify(body) }; + }; + return () => calls; +} +function acct(id, network, status = 'connected') { + return { accountID: id, network, status, user: { fullName: id } }; +} + +test('getAccountMap: an empty /v1/accounts is NOT cached (sync-window self-heals next call)', async () => { + S._resetAccountCache(); + S._setNow(() => 1000); + const calls = stubAccounts([ [], [acct('matrix', 'Beeper')] ]); + try { + const first = await S.getAccountMap(); // backend mid-sync returns [] + assert.deepEqual(first, {}); // serve empty for THIS call + const second = await S.getAccountMap(); // must re-read, not freeze [] + assert.ok(second.matrix, 'second call picks up the now-synced account'); + assert.equal(calls(), 2, 'empty result forced a re-read instead of freezing'); + } finally { S._setNow(null); global.fetch = REAL_FETCH; } +}); + +test('getAccountMap: a populated map IS cached within the TTL (one fetch for rapid calls)', async () => { + S._resetAccountCache(); + S._setNow(() => 5000); + const calls = stubAccounts([ [acct('matrix', 'Beeper')] ]); + try { + await S.getAccountMap(); + await S.getAccountMap(); + assert.equal(calls(), 1, 'second call within TTL served from cache'); + } finally { S._setNow(null); global.fetch = REAL_FETCH; } +}); + +test('getAccountMap: a runtime-added account becomes visible after the TTL, no process restart', async () => { + // The headline regression: old code froze the first map forever, so a WhatsApp + // account added via noVNC stayed network:"unknown" until the process restarted. + S._resetAccountCache(); + let t = 10000; + S._setNow(() => t); + const calls = stubAccounts([ + [acct('matrix', 'Beeper')], + [acct('matrix', 'Beeper'), acct('local-whatsapp_x', 'WhatsApp')], + ]); + try { + const before = await S.getAccountMap(); + assert.ok(!before['local-whatsapp_x'], 'whatsapp not present yet'); + t += 60001; // advance past the 60s TTL — NO restart + const after = await S.getAccountMap(); + assert.ok(after['local-whatsapp_x'], 'whatsapp visible after TTL without a restart'); + assert.equal(after['local-whatsapp_x'].network, 'whatsapp'); + assert.equal(calls(), 2); + } finally { S._setNow(null); global.fetch = REAL_FETCH; } +}); + +test('normalizeAccount surfaces the backend connection status (observability)', () => { + const out = S.normalizeAccount(acct('telegram', 'Telegram', 'connecting')); + assert.equal(out.status, 'connecting', 'status must be relayed so connecting != connected is visible'); + assert.equal(out.network, 'telegram'); + assert.equal(out.account_id, 'telegram'); +});