From 21228e25bf38c63a0efb521f91afe88889a8ac96 Mon Sep 17 00:00:00 2001 From: Andrei Hasna Date: Wed, 5 Aug 2026 22:39:40 +0300 Subject: [PATCH 1/3] fix(ui): remember a scoped count walk that failed closed, so a broken scope stops re-walking every 30s MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #201/#202 bounded the scoped folder-count walk, which made ONE walk finite. It did not make the SEQUENCE of walks finite. Only a walk that COMPLETES reaches the cache write at the end of the walk body, so a scope that trips the bound was remembered nowhere: the TUI catches the throw into `lastError`, reschedules the sidebar 30s later, and pays the whole walk again. Measured by the regression added here, against the pre-fix code: Expected: <= 1 Received: 201 That is 201 further requests on every 30s refresh, indefinitely. It is the STEADY STATE on the real mailbox rather than an edge case. Measured on production (emails.hasna.xyz, shipped 1.3.9, no local patch): emails inbox status 174,493 total / 174,120 inbox emails inbox list --to andrei@hasna.com \ --offset 99000 --limit 1 rc=1, "scanned 99579 rows over 200 requests without completing" CONTROL, rare address: emails inbox list --to zz-no-such-mailbox-41529344@example.invalid --limit 1 rc=0, 0 rows, 1.88s The control is what makes 99,579 meaningful: had the serve ignored `?to=`, the rare address would have walked the whole store too. It returned in 1.88s against 78s, so `?to=` is honoured server-side and those rows are genuinely one address's mail, still arriving ~498 per page when the walk stopped 421 rows short of MAX_SCAN_ROWS. So the primary address cannot complete a scoped count, and this path always fails. The failure is remembered for 15 minutes rather than the 60s a success gets: it is structural — the store's size against a compile-time constant — so it cannot resolve on a count's timescale, and re-deriving it costs the MAXIMUM walk the bound allows rather than a typical one. It is fenced by the same write generation as the success cache and cleared by invalidate(), so a write that makes the scope countable again is reflected at once rather than after the window. Steady state goes from 201 requests/30s to 201 requests/15min, about 30x. WHAT THIS DOES NOT DO: it makes the broken state cheap, it does not un-break the counts. A single bounded walk is still 201 requests, so any 3-minute window containing a retry sees ~201. Removing the walk needs a server-side recipient filter on /v1/messages/counts, which already accepts ?domain= and no ?to=/?from=; filed as todos 3ae0181e. Refs: todos 41529344 Agent: agent-ceo --- src/lib/self-hosted-mail-data-source.test.ts | 104 +++++++++++++++++++ src/lib/self-hosted-mail-data-source.ts | 95 +++++++++++++---- 2 files changed, 177 insertions(+), 22 deletions(-) diff --git a/src/lib/self-hosted-mail-data-source.test.ts b/src/lib/self-hosted-mail-data-source.test.ts index 5ae132e4..3ac05efc 100644 --- a/src/lib/self-hosted-mail-data-source.test.ts +++ b/src/lib/self-hosted-mail-data-source.test.ts @@ -2919,7 +2919,111 @@ describe("SelfHostedMailDataSource — scoped mailboxCounts scan budget", () => // 110_000 synthetic rows: slow to build, so it needs more than the 5s default. }, 60_000); + // A serve whose page is BUILT ONCE and handed back for every cursor. The + // property under test is what a REPEAT call costs after the walk has already + // failed closed, so rebuilding 100k synthetic rows per walk would make the + // test's own cost dominate the thing it measures. Row ids repeat, which the + // dedupe set absorbs; `scannedRows` still accumulates page.length, and that is + // the bound these exercise. + function exhaustingServe(pageCount: number): { fetchImpl: SelfHostedFetch; requests: string[] } { + const requests: string[] = []; + const page = Array.from({ length: 500 }, (_, i) => listV1(v1(`row${i}`, { to_addrs: [SCOPED] }))); + const fetchImpl: SelfHostedFetch = async (url, init) => { + const u = new URL(url); + const method = (init.method ?? "GET").toUpperCase(); + requests.push(`${method} ${u.pathname}${u.search}`); + const ok = (body: unknown, status = 200) => ({ status, async text() { return JSON.stringify(body); } }); + const idMatch = u.pathname.match(/^\/v1\/messages\/([^/]+)$/); + if (idMatch && method === "PATCH") return ok({ message: v1(decodeURIComponent(idMatch[1]!)) }); + if (method !== "GET" || u.pathname !== "/v1/messages") return ok({ error: "not found" }, 404); + const cursor = u.searchParams.get("cursor") ?? ""; + const index = cursor === "" ? 0 : Number(cursor.slice("page-".length)); + if (!Number.isInteger(index) || index < 0 || index >= pageCount) { + return ok({ error: "cursor is not a valid pagination cursor" }, 400); + } + return ok({ messages: page, next_cursor: index + 1 < pageCount ? `page-${index + 1}` : null }); + }; + return { fetchImpl, requests }; + } + + it("does not re-walk a scope whose count walk already failed closed", async () => { + // THE RESIDUAL THE ROW BOUND LEFT BEHIND. The bound makes one walk finite; + // it does not make the SEQUENCE of walks finite. Only a SUCCEEDING walk + // reaches the `scopedCountsCache.set` at the end of the walk body, so a + // scope that fails closed is remembered nowhere: the TUI catches the throw + // into `lastError`, reschedules the sidebar 30s later, and pays the whole + // 200-request walk again — forever, for as long as the inbox stays selected. + // On the production mailbox (174_482 messages against MAX_SCAN_ROWS = + // 100_000) that is the steady state, not an edge case. + const serve = exhaustingServe(400); + let clock = 1_000_000; + const ds = new SelfHostedMailDataSource({ + baseUrl: "https://emails.example/v1", + apiKey: "test-key", + fetchImpl: serve.fetchImpl, + now: () => clock, + }); + + await expect(ds.mailboxCounts({ source: source(SCOPED) })).rejects.toThrow(/scoped folder counts/i); + const afterFirst = serve.requests.length; + // Control: the first walk really did walk, so a low delta below cannot be an + // artefact of the serve refusing to page at all. + expect(afterFirst).toBeGreaterThan(100); + + clock += 30_000; + await expect(ds.mailboxCounts({ source: source(SCOPED) })).rejects.toThrow(/scoped folder counts/i); + expect(serve.requests.length - afterFirst).toBeLessThanOrEqual(1); + }, 120_000); + + it("retries an exhausted scope once the failure has aged out", async () => { + // The other side of the same property: remembering the failure must not + // become a permanent lockout. A scope that fails while the server is + // mid-deploy, or before an operator narrows the read, has to become + // countable again without restarting the TUI. + const serve = exhaustingServe(400); + let clock = 1_000_000; + const ds = new SelfHostedMailDataSource({ + baseUrl: "https://emails.example/v1", + apiKey: "test-key", + fetchImpl: serve.fetchImpl, + now: () => clock, + }); + + await expect(ds.mailboxCounts({ source: source(SCOPED) })).rejects.toThrow(/scoped folder counts/i); + const afterFirst = serve.requests.length; + + // Past SCOPED_COUNT_FAILURE_TTL_MS (15 min). The window is deliberately much + // longer than the 60s success TTL — the failure is a property of the store's + // size against a compile-time constant, so it cannot resolve on a + // count's timescale — but it is a window, not a permanent lockout. + clock += 20 * 60_000; + await expect(ds.mailboxCounts({ source: source(SCOPED) })).rejects.toThrow(/scoped folder counts/i); + expect(serve.requests.length - afterFirst).toBeGreaterThan(100); + }, 120_000); + + it("re-counts a failed scope immediately after a write, instead of serving the remembered failure", async () => { + // A remembered failure must be fenced by the same write invalidation the + // remembered COUNTS are, or a write that fixes the scope (a clear, a move) + // keeps reporting the old failure for a full TTL. + const serve = exhaustingServe(400); + let clock = 1_000_000; + const ds = new SelfHostedMailDataSource({ + baseUrl: "https://emails.example/v1", + apiKey: "test-key", + fetchImpl: serve.fetchImpl, + now: () => clock, + }); + + await expect(ds.mailboxCounts({ source: source(SCOPED) })).rejects.toThrow(/scoped folder counts/i); + const afterFirst = serve.requests.length; + + await ds.setRead("row0", true); + await expect(ds.mailboxCounts({ source: source(SCOPED) })).rejects.toThrow(/scoped folder counts/i); + expect(serve.requests.length - afterFirst).toBeGreaterThan(100); + }, 120_000); + it("does NOT break scoped stores that complete today with many small pages", async () => { + // THE REGRESSION GUARD FOR THE BOUND ITSELF. An earlier revision capped this // THE REGRESSION GUARD FOR THE BOUND ITSELF. An earlier revision capped this // walk at 200 REQUESTS; adversarial review measured two stores that resolve // exactly on the pre-fix code and threw under that cap. Both sit far below diff --git a/src/lib/self-hosted-mail-data-source.ts b/src/lib/self-hosted-mail-data-source.ts index 5cdec0aa..390a5a6b 100644 --- a/src/lib/self-hosted-mail-data-source.ts +++ b/src/lib/self-hosted-mail-data-source.ts @@ -298,6 +298,33 @@ const MAX_SCOPED_COUNT_REQUESTS = 10_000; // Must exceed the TUI's 30s sidebar refresh, for the same reason as the tally. const SCOPED_COUNT_TTL_MS = 60_000; +// ── remembering a walk that failed closed ──────────────────────────────────── +// +// THE BOUND ABOVE MAKES ONE WALK FINITE. IT DOES NOT MAKE THE SEQUENCE OF WALKS +// FINITE. Only a walk that COMPLETES reaches the cache write at the end of the +// walk body, so a scope that trips the bound was remembered nowhere: the TUI +// catches the throw into `lastError`, reschedules the sidebar 30s later, and +// pays the whole walk again. Measured on the regression added with this change: +// the second call issued 201 further requests, and would have kept doing so for +// as long as the inbox stayed selected. +// +// That is the STEADY STATE on the real mailbox rather than an edge case — +// 174_482 messages against a MAX_SCAN_ROWS of 100_000 cannot complete, so the +// production configuration is exactly the one that never caches. +// +// The failure is STRUCTURAL: it is a property of the store's size against a +// compile-time constant, so it cannot resolve on the 60s timescale that suits a +// count. Re-deriving it also costs the MAXIMUM walk the bound allows (~200 +// requests) rather than a typical one, so it is the most expensive thing to +// recompute and the least likely to have changed. Hence a longer window than +// SCOPED_COUNT_TTL_MS. +// +// It is deliberately not permanent. A scope that failed during a serve deploy, +// or before an operator narrowed the read, has to become countable again +// without restarting the TUI — and a write invalidates it immediately, so a +// clear() or a move is reflected at once rather than after the window. +const SCOPED_COUNT_FAILURE_TTL_MS = 15 * 60_000; + // Both figures are the REAL ones. An earlier revision reported // `requests * PAGE_LIMIT`, which invented a row count — a 600-row store claimed // "scanned 100500 rows … holds more than 100000 messages" and so corroborated @@ -939,6 +966,9 @@ export class SelfHostedMailDataSource implements MailDataSource { private labelTallyGeneration = 0; private scopedCountsCache = new Map(); private scopedCountsInFlight = new Map }>(); + // A walk that failed CLOSED is a result too, and until it was remembered here + // it was the only outcome this class recomputed from scratch on every refresh. + private scopedCountsFailureCache = new Map(); // Its own fence, moving in lockstep with the tally's but kept separate so the // shipped label path is untouched by this change. private scopedCountsGeneration = 0; @@ -1219,6 +1249,10 @@ export class SelfHostedMailDataSource implements MailDataSource { // scoped counts must drop too — and any counting walk already in flight must // be fenced, since its pages predate this write. this.scopedCountsCache.clear(); + // A remembered failure must drop on the same write, or a clear() or a move + // that makes the scope countable again keeps reporting the old failure for + // the rest of its (deliberately long) window. + this.scopedCountsFailureCache.clear(); this.scopedCountsGeneration += 1; } @@ -1454,6 +1488,13 @@ export class SelfHostedMailDataSource implements MailDataSource { const generation = this.scopedCountsGeneration; const cached = this.scopedCountsCache.get(key); if (cached && this.now() - cached.at < SCOPED_COUNT_TTL_MS) return { ...cached.counts }; + // A remembered failure is re-thrown as the ORIGINAL error, not a fresh one + // wrapped in "(cached)": its text already names both causes and both + // remedies, and that advice is exactly as true the second time. Rewording it + // per-hit would drift the message that the sibling walks' errors are matched + // against, to tell the reader something they cannot act on differently. + const failed = this.scopedCountsFailureCache.get(key); + if (failed && this.now() - failed.at < SCOPED_COUNT_FAILURE_TTL_MS) throw failed.error; // Coalesce: the TUI starts a new sidebar load every 30s without awaiting the // previous one, so without this the walks stack instead of replacing one // another — the accumulation behind the climbing idle CPU. A walk from an @@ -1464,31 +1505,41 @@ export class SelfHostedMailDataSource implements MailDataSource { const walk = (async () => { const counts = emptyCounts(); const seen = new Set(); - for (const filters of scopeServerFilterSets(scope)) { - // PER FILTER SET, not across the union. An address scope reads the same - // store twice (?to= then ?from=) and today's bound counts DEDUPED - // matches, so a store of 60_000 rows read twice is 60_000 against that - // bound and would be 120_000 against a shared one — which would have - // thrown on a store that completes today. Each set gets the same - // MAX_SCAN_ROWS headroom the single-scan path already has. - let requests = 0; - let scannedRows = 0; - for await (const page of this.listPages(PAGE_LIMIT, filters)) { - requests += 1; - scannedRows += page.length; - if (scannedRows > MAX_SCAN_ROWS || requests > MAX_SCOPED_COUNT_REQUESTS) { - throw scopedCountWalkExhausted(scannedRows, requests); - } - for (const message of page) { - // An address scope is a union of two server reads, so the same - // message can arrive twice; count it once. - if (!scopeMatch(message, scope) || seen.has(message.id)) continue; - seen.add(message.id); - for (const folder of MAILBOXES) { - if (folderMatch(message, folder)) counts[folder] += 1; + try { + for (const filters of scopeServerFilterSets(scope)) { + // PER FILTER SET, not across the union. An address scope reads the same + // store twice (?to= then ?from=) and today's bound counts DEDUPED + // matches, so a store of 60_000 rows read twice is 60_000 against that + // bound and would be 120_000 against a shared one — which would have + // thrown on a store that completes today. Each set gets the same + // MAX_SCAN_ROWS headroom the single-scan path already has. + let requests = 0; + let scannedRows = 0; + for await (const page of this.listPages(PAGE_LIMIT, filters)) { + requests += 1; + scannedRows += page.length; + if (scannedRows > MAX_SCAN_ROWS || requests > MAX_SCOPED_COUNT_REQUESTS) { + throw scopedCountWalkExhausted(scannedRows, requests); + } + for (const message of page) { + // An address scope is a union of two server reads, so the same + // message can arrive twice; count it once. + if (!scopeMatch(message, scope) || seen.has(message.id)) continue; + seen.add(message.id); + for (const folder of MAILBOXES) { + if (folderMatch(message, folder)) counts[folder] += 1; + } } } } + } catch (error) { + // Remembered under the SAME generation fence the success path uses: a + // write that landed mid-walk may be exactly what makes this scope + // countable again, so a failure from before it must not be installed. + if (this.scopedCountsGeneration === generation) { + this.scopedCountsFailureCache.set(key, { at: this.now(), error }); + } + throw error; } // Only install if no write landed while this walk was running: those pages // are already stale, and caching them would serve pre-write counts for a From d3f2ecf83de61cb66dc3d1ce5314d043ddc38dbc Mon Sep 17 00:00:00 2001 From: Andrei Hasna Date: Wed, 5 Aug 2026 23:02:54 +0300 Subject: [PATCH 2/3] fix(ui): derive the scoped-count cache lifetime from what the walk cost, so an idle sidebar stops re-buying a six-figure scope every minute MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #201/#202 bounded the scoped folder-count walk and cached it for 60s. The previous commit on this branch remembered the walk that FAILS CLOSED. Neither touches the case the production mailbox actually takes, which is that the walk SUCCEEDS. MEASURED DIRECTLY AGAINST A REAL SERVE, on the shipped 1.3.9 client, calling mailboxCounts({ source: { address } }) — the exact function under test — with a request counter wrapped around fetch: refresh#1 t=+0s reqs=205 outcome=ok refresh#2 t=+61s reqs=0 outcome=ok ... 6-minute run, pre-change: 615 requests / 362s / 222 MB one full walk per minute, indefinitely outcome=ok is the finding: the walk COMPLETES at 205 requests. It does not trip MAX_SCAN_ROWS, because that bound is PER FILTER SET and each half of the to/from union stays under it. So the failure cache never engages on this mailbox, and the sustained cost is a SUCCESSFUL walk that the 60s TTL re-buys on the next refresh — about 205 req/min and 4.4 GB/hour for one idle sidebar. WHY THE SERVER CANNOT ANSWER THIS CHEAPLY, measured rather than assumed: GET /v1/messages/counts?to= returns the WHOLE-STORE counts, identical to unfiltered GET /v1/messages?limit=1&to= envelope keys are exactly messages,next_cursor — no total CONTROL for both: ?to=
on the LIST endpoint returns 0 rows, so the serve does honour the recipient filter on list reads. The filter works; there is no aggregate that uses it and no total to read, so an exact scoped count has to walk. THE CHANGE: the cache lifetime is derived from the walk's measured request cost against a stated budget for sidebar metadata (12 req/min), floored at today's 60s and capped at 15 minutes. 3 requests -> 60s, exactly as before 205 requests -> capped 15min A cheap scope is bit-for-bit unaffected. Every write still invalidates immediately and the message list still refreshes at 30s; only how long an EXPENSIVE tally is reused changes. 6-minute run, post-change: 205 requests at t=0, then 0. 1,624,283 count calls over 336s issued requests exactly once. WHAT THIS DOES NOT DO: it lowers the duty cycle, it does not remove the burst. Each refill is still one whole walk, so a 3-minute window containing a refill still sees ~205. Removing it needs a recipient filter on /v1/messages/counts — todos 3ae0181e. CORRECTION TO THE PREVIOUS COMMIT ON THIS BRANCH: it states that scoped counts throw for the primary address and that this path "always fails". Its evidence is `emails inbox list --to --offset 99000`, whose error text ("scanned 99579 rows over 200 requests without completing") is filterWalkExhausted at MAX_FILTER_WALK_REQUESTS = 200 — the FILTERED LIST path. The counts path is capped at MAX_SCOPED_COUNT_REQUESTS = 10_000 and cannot print 200. Both measurements are correct; they measured different functions. Exercising mailboxCounts directly returns outcome=ok. The failure cache it added is still worth keeping — a scope above the bound, or a serve that ignores the filter, does fail and did re-walk every 30s — but it is robustness, not the production cost fix. Tests: three for the cost-aware TTL (a cheap scope keeps 60s; an expensive scope holds across ten 30s refreshes and refreshes past the ceiling; a write still drops it immediately). Mutation-checked: forcing the budget term to 0 fails the backoff test. Refs: todos 41529344 Agent: agent-ceo --- src/lib/self-hosted-mail-data-source.test.ts | 79 ++++++++++++++++++++ src/lib/self-hosted-mail-data-source.ts | 56 +++++++++++++- 2 files changed, 132 insertions(+), 3 deletions(-) diff --git a/src/lib/self-hosted-mail-data-source.test.ts b/src/lib/self-hosted-mail-data-source.test.ts index 3ac05efc..7537c35b 100644 --- a/src/lib/self-hosted-mail-data-source.test.ts +++ b/src/lib/self-hosted-mail-data-source.test.ts @@ -3022,6 +3022,85 @@ describe("SelfHostedMailDataSource — scoped mailboxCounts scan budget", () => expect(serve.requests.length - afterFirst).toBeGreaterThan(100); }, 120_000); + it("keeps a CHEAP scope on the original 60s freshness", async () => { + // The budget must not tax the common case. A scope that answers in a + // handful of requests has to behave exactly as it did before the cost-aware + // TTL existed: stale after 60s, re-walked on the next refresh. + const serve = scopedDeepServe(3, { rowsPerPage: 2 }); + let clock = 1_000_000; + const ds = new SelfHostedMailDataSource({ + baseUrl: "https://emails.example/v1", + apiKey: "test-key", + fetchImpl: serve.fetchImpl, + now: () => clock, + }); + + await ds.mailboxCounts({ source: source(SCOPED) }); + const afterFirst = serve.requests.length; + + clock += 30_000; // inside 60s: still served from cache + await ds.mailboxCounts({ source: source(SCOPED) }); + expect(serve.requests.length).toBe(afterFirst); + + clock += 31_000; // past 60s: a cheap scope refreshes as it always did + await ds.mailboxCounts({ source: source(SCOPED) }); + expect(serve.requests.length).toBeGreaterThan(afterFirst); + }); + + it("backs an EXPENSIVE scope off past the 60s TTL, so an idle sidebar stops re-buying it", async () => { + // The production shape, measured: the walk SUCCEEDS at ~205 requests and the + // 60s TTL then re-buys it every minute — 205 req/min, ~4.4 GB/hour, for one + // idle sidebar. 205 requests against a 12/min budget earns ~17min, capped at + // SCOPED_COUNT_MAX_TTL_MS (15min). + const serve = scopedDeepServe(205, { rowsPerPage: 2 }); + let clock = 1_000_000; + const ds = new SelfHostedMailDataSource({ + baseUrl: "https://emails.example/v1", + apiKey: "test-key", + fetchImpl: serve.fetchImpl, + now: () => clock, + }); + + await ds.mailboxCounts({ source: source(SCOPED) }); + const afterFirst = serve.requests.length; + // Control: this scope really is expensive, so the assertions below are about + // the budget and not about a walk that never happened. + expect(afterFirst).toBeGreaterThan(200); + + // Five minutes of 30s refreshes: every one of these cost a full walk before. + for (let i = 0; i < 10; i += 1) { + clock += 30_000; + await ds.mailboxCounts({ source: source(SCOPED) }); + } + expect(serve.requests.length).toBe(afterFirst); + + // Past the ceiling it refreshes, so counts cannot be frozen indefinitely. + clock += 16 * 60_000; + await ds.mailboxCounts({ source: source(SCOPED) }); + expect(serve.requests.length).toBeGreaterThan(afterFirst); + }, 60_000); + + it("still drops an expensive scope's long-lived counts the moment a write lands", async () => { + // The longer TTL must not outrank invalidation, or a user action appears to + // do nothing to the sidebar for up to fifteen minutes. + const serve = scopedDeepServe(205, { rowsPerPage: 2 }); + let clock = 1_000_000; + const ds = new SelfHostedMailDataSource({ + baseUrl: "https://emails.example/v1", + apiKey: "test-key", + fetchImpl: serve.fetchImpl, + now: () => clock, + }); + + await ds.mailboxCounts({ source: source(SCOPED) }); + const afterFirst = serve.requests.length; + + await ds.setRead("p0i0", true); + clock += 1_000; + await ds.mailboxCounts({ source: source(SCOPED) }); + expect(serve.requests.length).toBeGreaterThan(afterFirst); + }, 60_000); + it("does NOT break scoped stores that complete today with many small pages", async () => { // THE REGRESSION GUARD FOR THE BOUND ITSELF. An earlier revision capped this // THE REGRESSION GUARD FOR THE BOUND ITSELF. An earlier revision capped this diff --git a/src/lib/self-hosted-mail-data-source.ts b/src/lib/self-hosted-mail-data-source.ts index 390a5a6b..e27d9e60 100644 --- a/src/lib/self-hosted-mail-data-source.ts +++ b/src/lib/self-hosted-mail-data-source.ts @@ -298,6 +298,48 @@ const MAX_SCOPED_COUNT_REQUESTS = 10_000; // Must exceed the TUI's 30s sidebar refresh, for the same reason as the tally. const SCOPED_COUNT_TTL_MS = 60_000; +// ── a SUCCESSFUL walk is the expensive case, and the 60s TTL re-buys it ────── +// +// MEASURED AGAINST A REAL SERVE, 2026-08-05, one address scope on a six-figure +// production mailbox (inbox ~174_000): the walk COMPLETES at 205 requests and +// ~74 MB. It does not trip MAX_SCAN_ROWS, because the serve DOES honour ?to= +// (control: ?to=
returns 0 rows) and the matched set +// is under the bound. So the bound never fires and the failure cache below +// never engages — the whole cost is a SUCCESSFUL walk, cached for 60s, re-run +// on the next refresh, forever. Six-minute live run: 615 requests, 222 MB, one +// full walk per minute. +// +// The 60s TTL was chosen to outlive the TUI's 30s refresh, which it does. What +// it does not do is bound the SUSTAINED cost: 205 requests per 60s is 205 +// req/min and ~4.4 GB/hour for one idle sidebar. +// +// So the cache lifetime is derived from what the walk actually COST, against a +// stated budget for sidebar metadata. A scope that answers in a handful of +// requests keeps today's 60s freshness exactly; a scope that costs hundreds +// backs off until it fits the budget, capped so staleness stays bounded. +// +// THIS IS NOT "raise the refresh interval". The refresh interval is untouched, +// the message list still refreshes at 30s, every write still invalidates +// immediately, and a cheap scope is bit-for-bit unaffected. What changes is +// only how long an EXPENSIVE tally is reused, which is the one term in +// requests-per-minute that a client can set without a server-side aggregate. +// +// THE BURST IS NOT REMOVED, ONLY ITS DUTY CYCLE. Each refill still costs one +// whole walk, because /v1/messages/counts takes no recipient filter (measured: +// ?to= returns the WHOLE-STORE counts, byte-identical to unfiltered) and +// the list envelope carries only `messages` and `next_cursor` — no total to +// read. Removing the burst needs a server-side filtered count; that is filed +// separately and is the actual fix. +const SCOPED_COUNT_REQUEST_BUDGET_PER_MIN = 12; +// Staleness ceiling. Counts are sidebar metadata and every write invalidates +// them, so this only bounds how long NEW INBOUND mail can go uncounted. +const SCOPED_COUNT_MAX_TTL_MS = 15 * 60_000; + +function scopedCountTtlMs(requests: number): number { + const budgeted = (requests / SCOPED_COUNT_REQUEST_BUDGET_PER_MIN) * 60_000; + return Math.min(SCOPED_COUNT_MAX_TTL_MS, Math.max(SCOPED_COUNT_TTL_MS, budgeted)); +} + // ── remembering a walk that failed closed ──────────────────────────────────── // // THE BOUND ABOVE MAKES ONE WALK FINITE. IT DOES NOT MAKE THE SEQUENCE OF WALKS @@ -964,7 +1006,8 @@ export class SelfHostedMailDataSource implements MailDataSource { // not enough on its own: a walk already in flight would still install its // now-stale result afterwards and serve it for a full TTL. private labelTallyGeneration = 0; - private scopedCountsCache = new Map(); + // `ttl` is per-entry because it is derived from what THAT scope's walk cost. + private scopedCountsCache = new Map(); private scopedCountsInFlight = new Map }>(); // A walk that failed CLOSED is a result too, and until it was remembered here // it was the only outcome this class recomputed from scratch on every refresh. @@ -1487,7 +1530,7 @@ export class SelfHostedMailDataSource implements MailDataSource { const key = scopedCountsKey(scope); const generation = this.scopedCountsGeneration; const cached = this.scopedCountsCache.get(key); - if (cached && this.now() - cached.at < SCOPED_COUNT_TTL_MS) return { ...cached.counts }; + if (cached && this.now() - cached.at < cached.ttl) return { ...cached.counts }; // A remembered failure is re-thrown as the ORIGINAL error, not a fresh one // wrapped in "(cached)": its text already names both causes and both // remedies, and that advice is exactly as true the second time. Rewording it @@ -1505,6 +1548,10 @@ export class SelfHostedMailDataSource implements MailDataSource { const walk = (async () => { const counts = emptyCounts(); const seen = new Set(); + // Across the WHOLE walk, including both halves of an address's to/from + // union: the budget is about what one refill costs the network, and the + // caller pays for both halves. + let walkRequests = 0; try { for (const filters of scopeServerFilterSets(scope)) { // PER FILTER SET, not across the union. An address scope reads the same @@ -1517,6 +1564,7 @@ export class SelfHostedMailDataSource implements MailDataSource { let scannedRows = 0; for await (const page of this.listPages(PAGE_LIMIT, filters)) { requests += 1; + walkRequests += 1; scannedRows += page.length; if (scannedRows > MAX_SCAN_ROWS || requests > MAX_SCOPED_COUNT_REQUESTS) { throw scopedCountWalkExhausted(scannedRows, requests); @@ -1544,7 +1592,9 @@ export class SelfHostedMailDataSource implements MailDataSource { // Only install if no write landed while this walk was running: those pages // are already stale, and caching them would serve pre-write counts for a // full TTL. - if (this.scopedCountsGeneration === generation) this.scopedCountsCache.set(key, { at: this.now(), counts }); + if (this.scopedCountsGeneration === generation) { + this.scopedCountsCache.set(key, { at: this.now(), ttl: scopedCountTtlMs(walkRequests), counts }); + } return counts; })(); From 7d5ad47516867b1c752fc8d88e41a3889fa44727 Mon Sep 17 00:00:00 2001 From: Andrei Hasna Date: Wed, 5 Aug 2026 23:12:48 +0300 Subject: [PATCH 3/3] fix(ui): remember only the walk's OWN bound, so one 503 cannot freeze a scope's counts for fifteen minutes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-review of the two commits before it on this branch. The failure cache catches every error the walk can raise and remembers it for SCOPED_COUNT_FAILURE_TTL_MS. Only one of those errors is a property of the store; the rest are events. scopedCountWalkExhausted the walk hit its own bound — structural, cannot resolve on a count's timescale, costs the maximum walk to re-derive. Worth remembering. 503 / socket reset / transient. Before this commit, one blip took a timeout / any listPages working scope and served the remembered error for error the whole window — strictly worse than the re-walk the cache exists to prevent, because the counts were fine and now they are missing. The discriminator is a non-enumerable symbol set by the error's own constructor, not the message text, so rewording the error cannot silently turn a structural failure back into a retried one. Regression: a serve that 503s, then recovers. The next refresh must go back to the network and return real counts rather than replay the remembered error. Mutation-checked — dropping the isScopedCountWalkExhausted() guard fails it. Refs: todos 41529344 Agent: agent-ceo --- src/lib/self-hosted-mail-data-source.test.ts | 35 ++++++++++++++++++++ src/lib/self-hosted-mail-data-source.ts | 23 +++++++++++-- 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/src/lib/self-hosted-mail-data-source.test.ts b/src/lib/self-hosted-mail-data-source.test.ts index 7537c35b..2435dc66 100644 --- a/src/lib/self-hosted-mail-data-source.test.ts +++ b/src/lib/self-hosted-mail-data-source.test.ts @@ -2975,6 +2975,41 @@ describe("SelfHostedMailDataSource — scoped mailboxCounts scan budget", () => expect(serve.requests.length - afterFirst).toBeLessThanOrEqual(1); }, 120_000); + it("does NOT remember a TRANSIENT failure — one 503 must not freeze a scope for the window", async () => { + // The failure cache exists for the walk's own bound, which is a property of + // the store. A 503, a reset socket or a timeout reaches the same catch and is + // an event: remembering it would take one blip and turn it into fifteen + // minutes of missing counts, which is worse than the re-walk being prevented. + let failing = true; + const requests: string[] = []; + const page = [listV1(v1("only", { to_addrs: [SCOPED] }))]; + const fetchImpl: SelfHostedFetch = async (url, init) => { + const u = new URL(url); + requests.push(`${(init.method ?? "GET").toUpperCase()} ${u.pathname}`); + if (failing) return { status: 503, async text() { return JSON.stringify({ error: "upstream unavailable" }); } }; + return { status: 200, async text() { return JSON.stringify({ messages: page, next_cursor: null }); } }; + }; + let clock = 1_000_000; + const ds = new SelfHostedMailDataSource({ + baseUrl: "https://emails.example/v1", + apiKey: "test-key", + fetchImpl, + now: () => clock, + }); + + await expect(ds.mailboxCounts({ source: source(SCOPED) })).rejects.toThrow(); + const afterFailure = requests.length; + expect(afterFailure).toBeGreaterThan(0); + + // The blip clears. The very next refresh must go back to the network rather + // than replay the remembered error. + failing = false; + clock += 30_000; + const counts = await ds.mailboxCounts({ source: source(SCOPED) }); + expect(requests.length).toBeGreaterThan(afterFailure); + expect(counts.inbox).toBe(1); + }); + it("retries an exhausted scope once the failure has aged out", async () => { // The other side of the same property: remembering the failure must not // become a permanent lockout. A scope that fails while the server is diff --git a/src/lib/self-hosted-mail-data-source.ts b/src/lib/self-hosted-mail-data-source.ts index e27d9e60..fd6bdd9c 100644 --- a/src/lib/self-hosted-mail-data-source.ts +++ b/src/lib/self-hosted-mail-data-source.ts @@ -371,14 +371,28 @@ const SCOPED_COUNT_FAILURE_TTL_MS = 15 * 60_000; // `requests * PAGE_LIMIT`, which invented a row count — a 600-row store claimed // "scanned 100500 rows … holds more than 100000 messages" and so corroborated // the wrong one of the two causes it offers. +// Marks the ONE failure that is a structural property of the store rather than +// an event: the walk hit its own bound. Everything else reaching the same catch +// — a 503, a socket reset, a timeout — is transient and must NOT be remembered, +// or one network blip freezes a scope's counts for the whole failure window. +// A property rather than the message text, so rewording the error cannot +// silently turn a structural failure back into a retried one. +const SCOPED_COUNT_EXHAUSTED = Symbol.for("emails.scopedCountWalkExhausted"); + +function isScopedCountWalkExhausted(error: unknown): boolean { + return typeof error === "object" && error !== null && SCOPED_COUNT_EXHAUSTED in error; +} + function scopedCountWalkExhausted(scannedRows: number, requests: number): Error { - return new Error( + const error = new Error( `self-hosted emails: scoped folder counts scanned ${scannedRows} rows over ` + `${requests} requests without completing. Either this server ignored the ` + "GET /v1/messages ?to=/?from= recipient filters, or this one address holds more " + `than ${MAX_SCAN_ROWS} messages — upgrade the emails-serve deployment, or scope the ` + "read to a domain instead of an address.", ); + Object.defineProperty(error, SCOPED_COUNT_EXHAUSTED, { value: true, enumerable: false }); + return error; } // The cache key IS the scope, so two scopes can never share an entry. Both @@ -1581,10 +1595,15 @@ export class SelfHostedMailDataSource implements MailDataSource { } } } catch (error) { + // ONLY the walk's own bound is remembered. A 503, a reset socket or a + // timeout lands here too and is transient: caching it would freeze this + // scope's counts for the whole window over one blip, which is strictly + // worse than the re-walk this cache exists to prevent. + // // Remembered under the SAME generation fence the success path uses: a // write that landed mid-walk may be exactly what makes this scope // countable again, so a failure from before it must not be installed. - if (this.scopedCountsGeneration === generation) { + if (isScopedCountWalkExhausted(error) && this.scopedCountsGeneration === generation) { this.scopedCountsFailureCache.set(key, { at: this.now(), error }); } throw error;