From 8d8827fe4bf5b825548bc4ccd1eaafef28db6859 Mon Sep 17 00:00:00 2001 From: Andrei Hasna Date: Wed, 5 Aug 2026 01:27:48 +0300 Subject: [PATCH 1/5] fix(ui): bound the label-summary scan that made `emails ui` spin at ~92% CPU MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `emails ui` burned ~92% of a core while completely idle, climbing with process age (40.9% -> 66.0% -> 91.7% -> 92.3% across four windows under a pty with zero interaction) and leaking RSS (166 -> 248 MB in three minutes). The renderer was not the cause and was never running. Instrumented mid-spin it reported `fps=0.0 raf/s=0.0 isRunning=false controlState=idle liveReq=0`, and the process wrote 0 bytes/s to the terminal. Per-thread CPU put 33.6% on the main JS thread and ~31% across seven HeapHelper GC threads: allocation churn. Root cause: SelfHostedMailDataSource.listLabelSummaries() tallied label names by walking the ENTIRE store over HTTP with no bound — ~340 requests and ~145 MB of JSON against the production mailbox (~170k messages) to populate a sidebar list of at most 80 names. The caller's `limit: 80` bought nothing; it was applied after the scan. Why it CLIMBED: the TUI calls it from scheduleSidebarMeta on every 30s refresh, and that scheduler cancels a pending timer but never an in-flight walk. One walk takes longer than 30s on this mailbox, so each refresh started a new crawl on top of the previous one and they stacked — hence a rate that rises with age and plateaus at one core rather than sitting at a fixed frequency. Fix, three properties, all three required: - BOUND: MAX_LABEL_SCAN_REQUESTS = 10 (5,000 rows), matching the budget src/cli/tui/data.remote.ts already used as SELF_HOSTED_MAIL_SCAN_CAP. Stops at the budget rather than throwing, because sidebar metadata must degrade to a sample, not break. - CACHE: LABEL_TALLY_TTL_MS = 60_000, deliberately above the 30s refresh, or every refresh pays for a fresh walk and the cache buys nothing. Dropped by invalidate(), since labelling a message changes the tally. - COALESCE: one shared in-flight promise, so overlapping sidebar loads share a single walk. This is the property that removes the climb. Every other full walk in that module was already bounded (MAX_SCAN_ROWS, MAX_FILTER_WALK_REQUESTS, MAX_THREAD_CANDIDATE_ROWS); this was the only unbounded, uncached, uncoalesced one. Accepted trade, stated plainly: label counts are now a SAMPLE over the most recent 5,000 messages, not a census. On a larger store a count is a lower bound and a label used only in old mail can be absent. The self-hosted seam has no server-side label aggregate (the local seam answers this with one SQL GROUP BY), so exact counts are obtainable only by dragging the whole mailbox over HTTP. Regression tests (hermetic, in src/lib/self-hosted-mail-data-source.test.ts): pre-fix a single call issues 200 requests over a 200-page store, three concurrent calls issue 600, and a repeat call issues 400 instead of reusing 200. A fourth test holds the normal path so the budget cannot be satisfied by returning nothing. Measured after the fix, same pty harness, same machine: 13.1% / 5.2% / 14.2% / 4.7% across the identical four windows — 92.3% -> 4.7% at the matched window, with no climb and flat RSS. Task: be9b3bb0 Agent: Silvanus --- CHANGELOG.md | 2 + src/lib/self-hosted-mail-data-source.test.ts | 125 +++++++++++++++++++ src/lib/self-hosted-mail-data-source.ts | 73 +++++++++-- 3 files changed, 192 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a5fe105e..eaaf8127 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ All notable changes to `@hasna/emails` are documented here. ## [Unreleased] +- **fix(ui): `emails ui` burned ~92% of a core while idle — a sidebar label list was crawling the entire mailbox, repeatedly, with the crawls stacking up.** Measured on station01 against 1.3.6 under a pty with zero interaction: 40.9% -> 66.0% -> 91.7% -> 92.3% of a core across four windows, VmRSS climbing 166 -> 248 MB, and **0 bytes/s written to the terminal** the whole time. The renderer was not the cause and was never running: instrumented mid-spin it reported `fps=0.0 raf/s=0.0 isRunning=false controlState=idle liveReq=0`. Per-thread CPU put 33.6% on the main JS thread and ~31% across seven `HeapHelper` GC threads — allocation churn, not drawing. The source was `SelfHostedMailDataSource.listLabelSummaries()`, which tallied label names by walking the WHOLE store over HTTP (`for await (const page of this.listPages(PAGE_LIMIT))`, no bound) — ~340 requests and ~145 MB of JSON per call against the production mailbox (~170k messages), to populate a sidebar list of at most 80 names; the caller's `limit: 80` bought nothing because it was applied after the scan. The TUI issues it from `scheduleSidebarMeta` on every 30s refresh, and that scheduler cancels a PENDING timer but never an IN-FLIGHT walk — so on a mailbox where one walk takes longer than 30s, each refresh started a new crawl on top of the last and they accumulated, which is why CPU climbed with process age and plateaued at one core rather than sitting at a fixed rate. The walk is now bounded (`MAX_LABEL_SCAN_REQUESTS = 10`, i.e. 5,000 rows — the same budget `src/cli/tui/data.remote.ts` already used as `SELF_HOSTED_MAIL_SCAN_CAP`), TTL-cached (`LABEL_TALLY_TTL_MS = 60_000`, deliberately above the 30s refresh or the cache buys nothing), and coalesced behind one shared in-flight promise so overlapping sidebar loads share a single walk — the property that actually removes the climb. `invalidate()` drops the tally, since labelling changes it. Every other full walk in that module was already bounded (`MAX_SCAN_ROWS`, `MAX_FILTER_WALK_REQUESTS`, `MAX_THREAD_CANDIDATE_ROWS`); this was the only unbounded, uncached, uncoalesced one. **Accepted trade, stated plainly: label counts are now a SAMPLE over the most recent 5,000 messages, not a census** — on a larger store a count is a lower bound and a label used only in old mail can be absent. The self-hosted seam has no server-side label aggregate (the local seam answers this with one SQL `GROUP BY`), so exact counts are obtainable only by dragging the whole mailbox over HTTP. After the fix, the same pty harness on the same machine measures 13.1% / 5.2% / 14.2% / 4.7% across the identical four windows — 92.3% -> 4.7% at the matched window, and no climb. +- **test(ui): four hermetic regression tests pin all three properties, and each measures the pathology before the fix.** Against the pre-fix implementation a single `listLabelSummaries()` call issues **200** requests over a 200-page store, three concurrent calls issue **600**, and a repeat call issues **400** instead of reusing 200; after the fix each is bounded to one budgeted walk. A fourth test holds the normal path — a store inside the budget still returns exact counts with `search`/`limit` applied — so the budget cannot be satisfied by returning nothing. - **fix(ui): `emails ui` could not start in any real terminal — the packaged runtime loaded a foreign OpenTUI native library.** 1.3.4 exited immediately with `Failed to initialize OpenTUI render library: Symbol "createEventSink" not found in .../@opentui/core-linux-arm64/libopentui.so`. `@opentui/core` loads its prebuilt renderer with a bare `import("@opentui/core-")` from inside its own module, so the version-matched prebuilt in `@opentui/core/node_modules/` is what answers. `scripts/build-tui-runtime.ts` inlined core into `dist/cli/ui-runtime-bundle.js` while listing the eight platform packages as **external**, which moved that import to `dist/cli/` — it then resolved against the installed package's *parents*, never saw core's own prebuilt, and bound to whatever copy the install had hoisted (here `0.1.105`, an ABI predating the symbol core calls). The install was version-correct throughout; only the loaded `.so` was wrong. `@opentui/core` is now external — it is already a declared runtime dependency, and keeping it in `node_modules` keeps the JS and the library it `dlopen`s in one dependency tree. `web-tree-sitter` and `bun-ffi-structs` were dropped from the same list for the same reason: both are core's dependencies, neither is declared by `@hasna/emails`, so externalising them pointed at unowned copies too. Declaring the eight platform packages as our own `optionalDependencies` was rejected — it copies upstream's platform matrix into this manifest and rots on every core bump, while leaving the resolution anchor wrong. `patchBundledNativeAssetPath()` is gone with the bundling it patched around, and the runtime bundle drops from 3.4 MB to 2.1 MB. - **test(ui): the build contract asserted the broken configuration, so the suite stayed green through a UI that could not start.** It required `scripts/build-tui-runtime.ts` to contain `"@opentui/core-linux-arm64"` and `...nativePackages` — the exact lines that caused the crash — because every assertion was a text match on the build script rather than a check of the artifact it produces. New `src/cli/tui/ui-runtime-contract.test.ts` rebuilds the bundle (never trusting a stale one), parses its imports with `Bun.Transpiler.scanImports` rather than a regex over 3 MB of bundled output, and fails if any bare import is not a declared runtime dependency of this package — the general form of the defect, not just the OpenTUI instance. It carries a positive control proving the check reports an undeclared external and passes a declared one, and a behavioural guard that a non-interactive `emails ui` exits non-zero, so a refusal can never be read as a UI that ran. - feat(cli): every inbox and sync command now accepts `-j, --json`, emits one structured result document, and reports machine-readable failures without changing the existing human output. diff --git a/src/lib/self-hosted-mail-data-source.test.ts b/src/lib/self-hosted-mail-data-source.test.ts index 745a4de3..caa0ff54 100644 --- a/src/lib/self-hosted-mail-data-source.test.ts +++ b/src/lib/self-hosted-mail-data-source.test.ts @@ -2601,3 +2601,128 @@ describe("SelfHostedMailDataSource — a blocked message carries its reason", () expect((await ds.getMessage("77"))?.policy_denial).toBeUndefined(); }); }); + +// --------------------------------------------------------------------------- +// Regression: task be9b3bb0 — `emails ui` burned ~92% of a core while idle. +// +// Root cause: listLabelSummaries() walked the ENTIRE store over HTTP to tally +// label names for a sidebar list. Against the real mailbox (~170k messages) that +// is ~340 requests and ~145 MB of JSON per call, and the TUI re-issues it on +// every 30s refresh — so a new full crawl starts before the previous finishes and +// the crawls stack up. Every OTHER full walk in this module is already bounded +// (MAX_SCAN_ROWS, MAX_FILTER_WALK_REQUESTS, MAX_THREAD_CANDIDATE_ROWS); this one +// was the only unbounded, uncached, uncoalesced walk. +// +// These assert the three properties that make the pathology impossible, and each +// FAILS against the pre-fix implementation (it issues one request per page, per +// call, with no cache). +// --------------------------------------------------------------------------- +describe("SelfHostedMailDataSource — listLabelSummaries scan budget", () => { + // A store far larger than any sane budget, served as a cursor chain. Pages are + // deliberately tiny: the property under test is REQUEST COUNT, so keeping rows + // per page small keeps the test fast while still offering an unbounded walk + // hundreds of pages to consume. + function deepStoreServe(pageCount: number): { fetchImpl: SelfHostedFetch; requests: string[] } { + const requests: string[] = []; + 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); } }); + 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); + } + const messages = Array.from({ length: 3 }, (_, i) => listV1(v1(`p${index}i${i}`, { + labels: ["urgent", `bucket-${(index + i) % 5}`], + }))); + return ok({ messages, next_cursor: index + 1 < pageCount ? `page-${index + 1}` : null }); + }; + return { fetchImpl, requests }; + } + + const DEEP_PAGES = 200; + + it("bounds a single call instead of walking the whole store", async () => { + const serve = deepStoreServe(DEEP_PAGES); + const ds = new SelfHostedMailDataSource({ + baseUrl: "https://emails.example/v1", + apiKey: "test-key", + fetchImpl: serve.fetchImpl, + }); + + const labels = await ds.listLabelSummaries({ limit: 80 }); + + // Pre-fix this walks all 200 pages. The budget must stop it far short. + expect(serve.requests.length).toBeLessThanOrEqual(12); + expect(serve.requests.length).toBeLessThan(DEEP_PAGES); + // Still useful: it returns the labels it did see rather than failing closed. + expect(labels.some((label) => label.name === "urgent")).toBe(true); + }); + + it("coalesces concurrent calls onto one walk", async () => { + const serve = deepStoreServe(DEEP_PAGES); + const ds = new SelfHostedMailDataSource({ + baseUrl: "https://emails.example/v1", + apiKey: "test-key", + fetchImpl: serve.fetchImpl, + }); + + // This is the shape the TUI actually produces: a 30s refresh fires a new + // sidebar-meta load while the previous one is still in flight. + const [a, b, c] = await Promise.all([ + ds.listLabelSummaries({ limit: 80 }), + ds.listLabelSummaries({ limit: 80 }), + ds.listLabelSummaries({ limit: 80 }), + ]); + + expect(serve.requests.length).toBeLessThanOrEqual(12); + expect(a).toEqual(b); + expect(b).toEqual(c); + }); + + it("serves a repeat call from cache instead of re-walking", async () => { + const serve = deepStoreServe(DEEP_PAGES); + let clock = 1_000_000; + const ds = new SelfHostedMailDataSource({ + baseUrl: "https://emails.example/v1", + apiKey: "test-key", + fetchImpl: serve.fetchImpl, + now: () => clock, + }); + + await ds.listLabelSummaries({ limit: 80 }); + const afterFirst = serve.requests.length; + clock += 1_000; // well inside any sane TTL + await ds.listLabelSummaries({ limit: 80, search: "urg" }); + + expect(serve.requests.length).toBe(afterFirst); + }); + + it("still returns exact counts for a store inside the budget", async () => { + const serve = compactCursorServe(new Map([ + ["", { + messages: [ + v1("1", { labels: ["urgent", "ops"] }), + v1("2", { labels: ["urgent"] }), + ], + nextCursor: "page-2", + }], + ["page-2", { messages: [v1("3", { labels: ["ops"] })], nextCursor: null }], + ])); + const ds = new SelfHostedMailDataSource({ + baseUrl: "https://emails.example/v1", + apiKey: "test-key", + fetchImpl: serve.fetchImpl, + }); + + // Equal counts tie-break on name ascending, so "ops" precedes "urgent". + expect(await ds.listLabelSummaries()).toEqual([ + { name: "ops", count: 2, popular: false }, + { name: "urgent", count: 2, popular: false }, + ]); + expect((await ds.listLabelSummaries({ search: "urg" })).map((l) => l.name)).toEqual(["urgent"]); + }); +}); diff --git a/src/lib/self-hosted-mail-data-source.ts b/src/lib/self-hosted-mail-data-source.ts index 36b3c066..fd50b3ea 100644 --- a/src/lib/self-hosted-mail-data-source.ts +++ b/src/lib/self-hosted-mail-data-source.ts @@ -230,6 +230,28 @@ function filterWalkExhausted(mailbox: Mailbox, scannedRows: number): Error { } // How long a full scan is reused within one (short-lived) CLI/MCP invocation. const SCAN_TTL_MS = 15_000; + +// ── label summaries: a bounded, cached, coalesced sample ───────────────────── +// +// Label summaries feed a SIDEBAR LIST — "which labels exist, roughly how many", +// capped at ~80 entries. The local seam answers it with one SQL GROUP BY; the +// self-hosted seam has no server-side aggregate, so it can only tally rows it +// has dragged over HTTP. Tallying the WHOLE store to render that list cost ~340 +// requests and ~145 MB of JSON against the real mailbox (~170k messages), and +// the TUI re-issues it on every 30s refresh — so crawls overlapped and stacked +// until the process sat on a full core (task be9b3bb0). +// +// So the walk is bounded like every other walk in this module, and the tally is +// shared. THE COUNTS ARE THEREFORE A SAMPLE, not a census: they describe the most +// recent MAX_LABEL_SCAN_REQUESTS pages (newest-first ordering), so on a store +// larger than that a label's count is a lower bound and a label used only in old +// mail can be missing entirely. That is a deliberate accuracy trade for sidebar +// metadata, and it is the same one src/cli/tui/data.remote.ts already makes with +// SELF_HOSTED_MAIL_SCAN_CAP=5000. +const MAX_LABEL_SCAN_REQUESTS = 10; +// Must exceed the TUI's own sidebar refresh cadence (30s), or every refresh pays +// for a fresh walk and the cache buys nothing. +const LABEL_TALLY_TTL_MS = 60_000; // Hard cap on rows walked while collecting one conversation. The candidate read // is already narrowed server-side by the subject filter, so this only bounds a // pathological "everyone uses the same subject" store. @@ -843,6 +865,8 @@ export class SelfHostedMailDataSource implements MailDataSource { private readonly timeoutMs: number; private readonly maxResponseBytes: number; private scanCache: { at: number; rows: V1Message[] } | null = null; + private labelTallyCache: { at: number; tally: Map } | null = null; + private labelTallyInFlight: Promise<{ tally: Map }> | null = null; constructor(options: SelfHostedMailDataSourceOptions) { const url = new URL(options.baseUrl); @@ -1110,6 +1134,8 @@ export class SelfHostedMailDataSource implements MailDataSource { private invalidate(): void { this.scanCache = null; + // Labelling a message changes the tally, so a write must drop it too. + this.labelTallyCache = null; } private async listFilteredMailboxPage(mailbox: Mailbox, scope: SelfHostedScope | undefined, opts?: MailboxListOptions): Promise { @@ -1448,17 +1474,48 @@ export class SelfHostedMailDataSource implements MailDataSource { return decodeAttachmentPayload(json, index, maxBytes); } - async listLabelSummaries(opts?: ListLabelSummaryOptions): Promise { - const tally = new Map(); - for await (const page of this.listPages(PAGE_LIMIT)) { - for (const m of page) { - for (const raw of labelsOf(m)) { - const name = raw.trim(); - if (!name) continue; - tally.set(name, (tally.get(name) ?? 0) + 1); + // The tally is store-wide and option-independent — `search` and `limit` are + // applied to its OUTPUT — so one cached tally serves every caller, whatever + // options they pass. + private async labelTally(): Promise<{ tally: Map }> { + const cached = this.labelTallyCache; + if (cached && this.now() - cached.at < LABEL_TALLY_TTL_MS) return cached; + // Coalesce: the TUI starts a new sidebar load every 30s without awaiting the + // previous one. Without this, those calls each open their own cursor walk and + // the walks stack up instead of replacing one another. + if (this.labelTallyInFlight) return this.labelTallyInFlight; + + const walk = (async () => { + const tally = new Map(); + let requests = 0; + for await (const page of this.listPages(PAGE_LIMIT)) { + requests += 1; + for (const m of page) { + for (const raw of labelsOf(m)) { + const name = raw.trim(); + if (!name) continue; + tally.set(name, (tally.get(name) ?? 0) + 1); + } } + // Stop at the budget rather than throwing: this is sidebar metadata, so + // it must degrade to a recent-window sample, never break the sidebar. + if (requests >= MAX_LABEL_SCAN_REQUESTS) break; } + const entry = { at: this.now(), tally }; + this.labelTallyCache = entry; + return entry; + })(); + + this.labelTallyInFlight = walk; + try { + return await walk; + } finally { + this.labelTallyInFlight = null; } + } + + async listLabelSummaries(opts?: ListLabelSummaryOptions): Promise { + const { tally } = await this.labelTally(); const search = opts?.search?.trim().toLowerCase(); let summaries: LabelSummary[] = [...tally.entries()] .filter(([name]) => !search || name.toLowerCase().includes(search)) From 74c6a48db4cee35a575495179e061236054c04d0 Mon Sep 17 00:00:00 2001 From: Andrei Hasna Date: Wed, 5 Aug 2026 01:40:55 +0300 Subject: [PATCH 2/5] test: re-pin the [Unreleased] changelog sha for the label-scan fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `repository workflow safety > keeps 1.3.2 at the exact changelog boundary` pins a sha256 of the whole `## [Unreleased]` section, so adding the entry for this fix tripped it. That guard is deliberate — it makes every edit to that section an explicit, reviewable change so an entry cannot drift into a shipped release section unnoticed — so the correct action is to re-pin it, not to loosen it. Recomputed with the same markdownSection()/textSha256() pair the test itself uses: da5526d1... -> 42cf7cc5... (section is 31,779 bytes). Verified: `bun test src/workflow-contract.test.ts` fails 1/6 before this change on exactly that assertion, and passes 6/6 after. The other five assertions, including the three adversarial fixtures that must still be REJECTED, are untouched. Task: be9b3bb0 Agent: Silvanus --- src/lib/zz-cato-race.test.ts | 155 ++++++++++++++++++++++++++++++++++ src/workflow-contract.test.ts | 7 +- 2 files changed, 161 insertions(+), 1 deletion(-) create mode 100644 src/lib/zz-cato-race.test.ts diff --git a/src/lib/zz-cato-race.test.ts b/src/lib/zz-cato-race.test.ts new file mode 100644 index 00000000..e138937c --- /dev/null +++ b/src/lib/zz-cato-race.test.ts @@ -0,0 +1,155 @@ +// TEMPORARY reviewer probe (PR #198, hasna/emails). Not for merge. +// Question: does invalidate() actually drop the label tally when a walk is +// already in flight? The PR claims "invalidate() drops the tally, since +// labelling changes it". +import { describe, expect, it } from "bun:test"; +import { + SelfHostedMailDataSource, + type SelfHostedFetch, +} from "./self-hosted-mail-data-source.js"; + +// Copied verbatim from self-hosted-mail-data-source.test.ts so the wire +// validator accepts these rows. +function v1(id: string, over: Record = {}): Record { + const numericId = /^\d+$/.test(id) ? Number(id) : 0; + const day = String(10 + (numericId % 18)).padStart(2, "0"); + return { + id, + direction: "inbound", + from_addr: `"Sender ${id}" `, + to_addrs: ["andrei@example.com"], + cc_addrs: [], + subject: `subject ${id}`, + body_text: `body of ${id}`, + body_html: null, + status: "received", + provider_message_id: null, + message_id: `<${id}@x>`, + in_reply_to: null, + received_at: `2026-06-${day}T08:00:00.000Z`, + is_read: false, + is_starred: false, + labels: [], + headers: {}, + attachments: [], + source_id: null, + send_state: "none", + send_started_at: null, + created_at: `2026-06-${day}T08:00:01.000Z`, + updated_at: `2026-06-${day}T08:00:01.000Z`, + ...over, + }; +} + +function listV1(row: Record): Record { + const { body_text: bodyText, body_html: _bodyHtml, headers, attachments, ...summary } = row; + const denial = headers && typeof headers === "object" && !Array.isArray(headers) + ? (headers as Record)["policy_denial"] + : undefined; + return { + ...summary, + snippet: typeof bodyText === "string" ? bodyText.replace(/\s+/g, " ").trim().slice(0, 140) : null, + attachment_count: Array.isArray(attachments) ? attachments.length : 0, + policy_denial: typeof denial === "string" && denial.trim() ? denial.trim() : null, + }; +} + +// A 3-page store — inside the 10-request budget, so the walk COMPLETES and +// therefore writes the cache. `gate`, when armed, holds the first GET open so a +// write can land while the walk is genuinely in flight. +function serve() { + const gets: string[] = []; + const patches: string[] = []; + let gate: Promise | null = null; + let firstGetSeen: (() => void) | null = null; + + const fetchImpl: SelfHostedFetch = async (url, init) => { + const u = new URL(url); + const method = (init.method ?? "GET").toUpperCase(); + const ok = (body: unknown, status = 200) => ({ status, async text() { return JSON.stringify(body); } }); + + if (method === "PATCH") { + patches.push(u.pathname); + return ok({ message: v1("1", { labels: ["added"] }) }); + } + 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)); + gets.push(`GET page ${index}`); + if (index === 0 && gate) { + firstGetSeen?.(); + await gate; + } + const messages = [listV1(v1(`${index}`, { labels: ["urgent"] }))]; + return ok({ messages, next_cursor: index + 1 < 3 ? `page-${index + 1}` : null }); + }; + + return { + fetchImpl, + gets, + patches, + armGate() { + let release!: () => void; + const reached = new Promise((r) => { firstGetSeen = r; }); + gate = new Promise((r) => { release = r; }); + return { reached, release: () => { gate = null; release(); } }; + }, + }; +} + +function makeDs(fetchImpl: SelfHostedFetch, now: () => number) { + return new SelfHostedMailDataSource({ + baseUrl: "https://emails.example/v1", + apiKey: "test-key", + fetchImpl, + now, + }); +} + +describe("PR#198 probe — invalidate() vs an in-flight label walk", () => { + it("CONTROL: with no walk in flight, a write does force a re-walk", async () => { + const s = serve(); + let clock = 1_000_000; + const src = makeDs(s.fetchImpl, () => clock); + + await src.listLabelSummaries({ limit: 80 }); + const afterFirstWalk = s.gets.length; + expect(afterFirstWalk).toBeGreaterThan(0); + + await src.addLabel("1", "added"); // invalidate() with nothing in flight + clock += 1_000; // far inside the 60s TTL + + await src.listLabelSummaries({ limit: 80 }); + const afterWrite = s.gets.length; + + console.log(`CONTROL gets: firstWalk=${afterFirstWalk} afterWrite=${afterWrite} patches=${s.patches.length}`); + // The probe can SEE a re-walk. Without this arm the race arm proves nothing. + expect(afterWrite).toBeGreaterThan(afterFirstWalk); + }); + + it("RACE: a write DURING a walk is defeated — the stale tally is cached anyway", async () => { + const s = serve(); + let clock = 2_000_000; + const src = makeDs(s.fetchImpl, () => clock); + + const { reached, release } = s.armGate(); + const walking = src.listLabelSummaries({ limit: 80 }); // starts, blocks on page 0 + await reached; + + await src.addLabel("1", "added"); // invalidate() runs mid-walk + expect(s.patches.length).toBe(1); + + release(); + await walking; + const afterWalk = s.gets.length; + + clock += 1_000; // inside the 60s TTL + await src.listLabelSummaries({ limit: 80 }); + const afterWrite = s.gets.length; + + console.log(`RACE gets: walk=${afterWalk} afterPostWriteRead=${afterWrite} patches=${s.patches.length}`); + // If invalidate() were honoured, this read would re-walk exactly like the control. + expect(afterWrite).toBeGreaterThan(afterWalk); + }); +}); diff --git a/src/workflow-contract.test.ts b/src/workflow-contract.test.ts index 92313b89..56576a20 100644 --- a/src/workflow-contract.test.ts +++ b/src/workflow-contract.test.ts @@ -6,7 +6,12 @@ import { join } from "node:path"; const workflowDir = join(import.meta.dir, "..", ".github", "workflows"); const repositoryRoot = join(import.meta.dir, ".."); const packageProvenanceWorkflowSha256 = "706c636d7b60059f6e8ce52229bfb723c0c9a2c61cb4a462b3d6ead24a46232f"; -const unreleasedSectionSha256 = "da5526d127f3a85c04d4b0d9f95bb4dd22edcbee7b34ff8999134fb180383699"; +// Re-pinned when the [Unreleased] section gains the `emails ui` label-scan fix +// (task be9b3bb0). This constant is a tripwire, not a formality: it makes every +// change to that section an explicit, reviewable edit, so an entry cannot drift +// into a shipped release section unnoticed. Recompute with the same +// markdownSection()/textSha256() pair this file already uses. +const unreleasedSectionSha256 = "42cf7cc5790ec032c46fcf8446b0d0f907f67615ad181d786a4aff0367226858"; const release132Section = `## 1.3.2 (2026-07-26) - fail closed on malformed JSON, wrong response envelopes, and missing required From 524473140f5bb0de64cd8bd7620907ad32e2d268 Mon Sep 17 00:00:00 2001 From: Andrei Hasna Date: Wed, 5 Aug 2026 01:41:35 +0300 Subject: [PATCH 3/5] chore: drop a reviewer scratch probe swept in by git add -A src/lib/zz-cato-race.test.ts is an adversarial reviewer's temporary probe for PR #198, marked "Not for merge" in its own header. It was created in the shared review worktree and my `git add -A` picked it up. Untracked here; the file is left on disk because the reviewer is still using it. Task: be9b3bb0 Agent: Silvanus --- src/lib/zz-cato-race.test.ts | 155 ----------------------------------- 1 file changed, 155 deletions(-) delete mode 100644 src/lib/zz-cato-race.test.ts diff --git a/src/lib/zz-cato-race.test.ts b/src/lib/zz-cato-race.test.ts deleted file mode 100644 index e138937c..00000000 --- a/src/lib/zz-cato-race.test.ts +++ /dev/null @@ -1,155 +0,0 @@ -// TEMPORARY reviewer probe (PR #198, hasna/emails). Not for merge. -// Question: does invalidate() actually drop the label tally when a walk is -// already in flight? The PR claims "invalidate() drops the tally, since -// labelling changes it". -import { describe, expect, it } from "bun:test"; -import { - SelfHostedMailDataSource, - type SelfHostedFetch, -} from "./self-hosted-mail-data-source.js"; - -// Copied verbatim from self-hosted-mail-data-source.test.ts so the wire -// validator accepts these rows. -function v1(id: string, over: Record = {}): Record { - const numericId = /^\d+$/.test(id) ? Number(id) : 0; - const day = String(10 + (numericId % 18)).padStart(2, "0"); - return { - id, - direction: "inbound", - from_addr: `"Sender ${id}" `, - to_addrs: ["andrei@example.com"], - cc_addrs: [], - subject: `subject ${id}`, - body_text: `body of ${id}`, - body_html: null, - status: "received", - provider_message_id: null, - message_id: `<${id}@x>`, - in_reply_to: null, - received_at: `2026-06-${day}T08:00:00.000Z`, - is_read: false, - is_starred: false, - labels: [], - headers: {}, - attachments: [], - source_id: null, - send_state: "none", - send_started_at: null, - created_at: `2026-06-${day}T08:00:01.000Z`, - updated_at: `2026-06-${day}T08:00:01.000Z`, - ...over, - }; -} - -function listV1(row: Record): Record { - const { body_text: bodyText, body_html: _bodyHtml, headers, attachments, ...summary } = row; - const denial = headers && typeof headers === "object" && !Array.isArray(headers) - ? (headers as Record)["policy_denial"] - : undefined; - return { - ...summary, - snippet: typeof bodyText === "string" ? bodyText.replace(/\s+/g, " ").trim().slice(0, 140) : null, - attachment_count: Array.isArray(attachments) ? attachments.length : 0, - policy_denial: typeof denial === "string" && denial.trim() ? denial.trim() : null, - }; -} - -// A 3-page store — inside the 10-request budget, so the walk COMPLETES and -// therefore writes the cache. `gate`, when armed, holds the first GET open so a -// write can land while the walk is genuinely in flight. -function serve() { - const gets: string[] = []; - const patches: string[] = []; - let gate: Promise | null = null; - let firstGetSeen: (() => void) | null = null; - - const fetchImpl: SelfHostedFetch = async (url, init) => { - const u = new URL(url); - const method = (init.method ?? "GET").toUpperCase(); - const ok = (body: unknown, status = 200) => ({ status, async text() { return JSON.stringify(body); } }); - - if (method === "PATCH") { - patches.push(u.pathname); - return ok({ message: v1("1", { labels: ["added"] }) }); - } - 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)); - gets.push(`GET page ${index}`); - if (index === 0 && gate) { - firstGetSeen?.(); - await gate; - } - const messages = [listV1(v1(`${index}`, { labels: ["urgent"] }))]; - return ok({ messages, next_cursor: index + 1 < 3 ? `page-${index + 1}` : null }); - }; - - return { - fetchImpl, - gets, - patches, - armGate() { - let release!: () => void; - const reached = new Promise((r) => { firstGetSeen = r; }); - gate = new Promise((r) => { release = r; }); - return { reached, release: () => { gate = null; release(); } }; - }, - }; -} - -function makeDs(fetchImpl: SelfHostedFetch, now: () => number) { - return new SelfHostedMailDataSource({ - baseUrl: "https://emails.example/v1", - apiKey: "test-key", - fetchImpl, - now, - }); -} - -describe("PR#198 probe — invalidate() vs an in-flight label walk", () => { - it("CONTROL: with no walk in flight, a write does force a re-walk", async () => { - const s = serve(); - let clock = 1_000_000; - const src = makeDs(s.fetchImpl, () => clock); - - await src.listLabelSummaries({ limit: 80 }); - const afterFirstWalk = s.gets.length; - expect(afterFirstWalk).toBeGreaterThan(0); - - await src.addLabel("1", "added"); // invalidate() with nothing in flight - clock += 1_000; // far inside the 60s TTL - - await src.listLabelSummaries({ limit: 80 }); - const afterWrite = s.gets.length; - - console.log(`CONTROL gets: firstWalk=${afterFirstWalk} afterWrite=${afterWrite} patches=${s.patches.length}`); - // The probe can SEE a re-walk. Without this arm the race arm proves nothing. - expect(afterWrite).toBeGreaterThan(afterFirstWalk); - }); - - it("RACE: a write DURING a walk is defeated — the stale tally is cached anyway", async () => { - const s = serve(); - let clock = 2_000_000; - const src = makeDs(s.fetchImpl, () => clock); - - const { reached, release } = s.armGate(); - const walking = src.listLabelSummaries({ limit: 80 }); // starts, blocks on page 0 - await reached; - - await src.addLabel("1", "added"); // invalidate() runs mid-walk - expect(s.patches.length).toBe(1); - - release(); - await walking; - const afterWalk = s.gets.length; - - clock += 1_000; // inside the 60s TTL - await src.listLabelSummaries({ limit: 80 }); - const afterWrite = s.gets.length; - - console.log(`RACE gets: walk=${afterWalk} afterPostWriteRead=${afterWrite} patches=${s.patches.length}`); - // If invalidate() were honoured, this read would re-walk exactly like the control. - expect(afterWrite).toBeGreaterThan(afterWalk); - }); -}); From 5481232d33128b05598f1422bc4894fad20c3a1b Mon Sep 17 00:00:00 2001 From: Andrei Hasna Date: Wed, 5 Aug 2026 01:59:10 +0300 Subject: [PATCH 4/5] fix(ui): fence the label tally against a mid-walk write, and pin the cache lifecycle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remediation cycle 1 on PR #198. Two independent adversarial reviewers returned NO_GO with concrete, scoped findings. This addresses all of them; none required changing the shape of the fix. CODE — P1 from the correctness lens (Cato): invalidate() cleared labelTallyCache but did not fence a walk ALREADY IN FLIGHT, so that walk installed its pre-write tally the moment it finished and served stale counts for a full TTL. User-visible at emails-state.tsx:430, where a label add is immediately followed by a summary read. Fixed with a generation counter: invalidate() bumps it, a walk installs its result only if the generation is unchanged, and a walk from an older generation is no longer joinable so a caller after a write starts a fresh walk instead of inheriting stale counts. The in-flight slot is cleared only if it is still the same pending entry, so a newer walk cannot be clobbered. TESTS — P1 from the evidence lens (Seneca): the original four pinned BOUND and COALESCE but pinned the cache only as "a cache exists". Seneca built a passing implementation with LABEL_TALLY_TTL_MS = Number.MAX_SAFE_INTEGER and no cache invalidation — which would freeze the sidebar counts forever. Three more tests pin the cache lifecycle: TTL expiry re-walks, a write drops the tally, and a write landing MID-WALK is fenced. Every one was mutation-tested rather than assumed: - infinite TTL + no invalidation -> 2 fail (TTL, invalidate) - generation fence removed -> 1 fail (mid-walk race) - unmutated -> 7 pass The mid-walk test initially passed against BOTH implementations — my stub built its response after the gate released, so the "in-flight" walk was reading post-write rows and could not be stale. It now snapshots rows at request time, and only then does it discriminate. TEXT — corrections, because the prose was wrong where the table was right: - "four tests fail before the fix" was FALSE. Three fail with the numbers claimed; the fourth (exact counts inside the budget) passes before and after BY DESIGN. It is an anti-vacuity guard, not a regression test, and is now described as one. - The headline 92.3% -> 4.7% is NOT like-for-like: 92.3% came from the installed 1.3.6 dist bundle, 4.7% from this branch's source, spanning two build artefacts and three commits. The defensible pair is source-vs-source through one harness, 68.8% -> 4.4%, and the controlled measurement of the mechanism is the request count: 340 requests / 118.2 MB before, 10 / 3.4 MB after. - The after-run's mode and mailbox are now stated (self-hosted seam against the production hosted mailbox — the only path this fix touches). - "this was the only unbounded, uncached, uncoalesced walk" is narrowed to what was actually measured: the only such listPages loop in that module judged against its own explicit constants. It is NOT a claim that the idle spin is closed. SCOPE — Cato proved mailboxCounts -> scanScopeRows runs an uncached, un-coalesced, request-uncapped DOUBLE walk on the same 30s tick once any single inbox is selected, which is larger than the walk fixed here. Pre-existing, deliberately not folded in, filed as todos 90e98ccc with the reachability proof and a suggested treatment. Gates: 118 pass / 0 fail across both touched test files, tsc --noEmit rc=0, staged secrets scan 0 hits with a firing positive control. Task: be9b3bb0 Agent: Silvanus --- CHANGELOG.md | 4 +- src/lib/self-hosted-mail-data-source.test.ts | 104 +++++++++++++++++++ src/lib/self-hosted-mail-data-source.ts | 31 ++++-- src/workflow-contract.test.ts | 2 +- 4 files changed, 131 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eaaf8127..5bd287fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,8 @@ All notable changes to `@hasna/emails` are documented here. ## [Unreleased] -- **fix(ui): `emails ui` burned ~92% of a core while idle — a sidebar label list was crawling the entire mailbox, repeatedly, with the crawls stacking up.** Measured on station01 against 1.3.6 under a pty with zero interaction: 40.9% -> 66.0% -> 91.7% -> 92.3% of a core across four windows, VmRSS climbing 166 -> 248 MB, and **0 bytes/s written to the terminal** the whole time. The renderer was not the cause and was never running: instrumented mid-spin it reported `fps=0.0 raf/s=0.0 isRunning=false controlState=idle liveReq=0`. Per-thread CPU put 33.6% on the main JS thread and ~31% across seven `HeapHelper` GC threads — allocation churn, not drawing. The source was `SelfHostedMailDataSource.listLabelSummaries()`, which tallied label names by walking the WHOLE store over HTTP (`for await (const page of this.listPages(PAGE_LIMIT))`, no bound) — ~340 requests and ~145 MB of JSON per call against the production mailbox (~170k messages), to populate a sidebar list of at most 80 names; the caller's `limit: 80` bought nothing because it was applied after the scan. The TUI issues it from `scheduleSidebarMeta` on every 30s refresh, and that scheduler cancels a PENDING timer but never an IN-FLIGHT walk — so on a mailbox where one walk takes longer than 30s, each refresh started a new crawl on top of the last and they accumulated, which is why CPU climbed with process age and plateaued at one core rather than sitting at a fixed rate. The walk is now bounded (`MAX_LABEL_SCAN_REQUESTS = 10`, i.e. 5,000 rows — the same budget `src/cli/tui/data.remote.ts` already used as `SELF_HOSTED_MAIL_SCAN_CAP`), TTL-cached (`LABEL_TALLY_TTL_MS = 60_000`, deliberately above the 30s refresh or the cache buys nothing), and coalesced behind one shared in-flight promise so overlapping sidebar loads share a single walk — the property that actually removes the climb. `invalidate()` drops the tally, since labelling changes it. Every other full walk in that module was already bounded (`MAX_SCAN_ROWS`, `MAX_FILTER_WALK_REQUESTS`, `MAX_THREAD_CANDIDATE_ROWS`); this was the only unbounded, uncached, uncoalesced one. **Accepted trade, stated plainly: label counts are now a SAMPLE over the most recent 5,000 messages, not a census** — on a larger store a count is a lower bound and a label used only in old mail can be absent. The self-hosted seam has no server-side label aggregate (the local seam answers this with one SQL `GROUP BY`), so exact counts are obtainable only by dragging the whole mailbox over HTTP. After the fix, the same pty harness on the same machine measures 13.1% / 5.2% / 14.2% / 4.7% across the identical four windows — 92.3% -> 4.7% at the matched window, and no climb. -- **test(ui): four hermetic regression tests pin all three properties, and each measures the pathology before the fix.** Against the pre-fix implementation a single `listLabelSummaries()` call issues **200** requests over a 200-page store, three concurrent calls issue **600**, and a repeat call issues **400** instead of reusing 200; after the fix each is bounded to one budgeted walk. A fourth test holds the normal path — a store inside the budget still returns exact counts with `search`/`limit` applied — so the budget cannot be satisfied by returning nothing. +- **fix(ui): `emails ui` burned ~92% of a core while idle — a sidebar label list was crawling the entire mailbox, repeatedly, with the crawls stacking up.** Measured on station01 against 1.3.6 under a pty with zero interaction: 40.9% -> 66.0% -> 91.7% -> 92.3% of a core across four windows, VmRSS climbing 166 -> 248 MB, and **0 bytes/s written to the terminal** the whole time. The renderer was not the cause and was never running: instrumented mid-spin it reported `fps=0.0 raf/s=0.0 isRunning=false controlState=idle liveReq=0`. Per-thread CPU put 33.6% on the main JS thread and ~31% across seven `HeapHelper` GC threads — allocation churn, not drawing. The source was `SelfHostedMailDataSource.listLabelSummaries()`, which tallied label names by walking the WHOLE store over HTTP (`for await (const page of this.listPages(PAGE_LIMIT))`, no bound) — ~340 requests and ~145 MB of JSON per call against the production mailbox (~170k messages), to populate a sidebar list of at most 80 names; the caller's `limit: 80` bought nothing because it was applied after the scan. The TUI issues it from `scheduleSidebarMeta` on every 30s refresh, and that scheduler cancels a PENDING timer but never an IN-FLIGHT walk — so on a mailbox where one walk takes longer than 30s, each refresh started a new crawl on top of the last and they accumulated, which is why CPU climbed with process age and plateaued at one core rather than sitting at a fixed rate. The walk is now bounded (`MAX_LABEL_SCAN_REQUESTS = 10`, i.e. 5,000 rows — the same budget `src/cli/tui/data.remote.ts` already used as `SELF_HOSTED_MAIL_SCAN_CAP`), TTL-cached (`LABEL_TALLY_TTL_MS = 60_000`, deliberately above the 30s refresh or the cache buys nothing), and coalesced behind one shared in-flight promise so overlapping sidebar loads share a single walk — the property that actually removes the climb. `invalidate()` drops the tally, since labelling changes it. Among the `listPages` loops in that module measured against their own explicit constants, this was the only one with no request cap, no cache and no coalescing — the others carry `MAX_SCAN_ROWS`, `MAX_FILTER_WALK_REQUESTS` or `MAX_THREAD_CANDIDATE_ROWS`. **That is not a claim that the idle spin is now fully closed:** adversarial review found that `mailboxCounts` reaches `scanScopeRows` on the same 30s sidebar tick once any single inbox is selected, and that path is also uncached and un-coalesced with only a row cap (`MAX_SCAN_ROWS = 100_000`, up to ~200 requests per filter set, run twice for the to/from union). It is pre-existing, out of scope here, and tracked separately. **Accepted trade, stated plainly: label counts are now a SAMPLE over the most recent 5,000 messages, not a census** — on a larger store a count is a lower bound and a label used only in old mail can be absent. The self-hosted seam has no server-side label aggregate (the local seam answers this with one SQL `GROUP BY`), so exact counts are obtainable only by dragging the whole mailbox over HTTP. Measured after the fix on the same machine, same pty harness, same `/proc` method, against the same production hosted mailbox over the self-hosted seam (the only path this fix touches): 13.1% / 5.2% / 14.2% / 4.7% across the identical four windows, with no climb and flat RSS. **The defensible before/after pair is source-vs-source through one harness: 68.8% -> 4.4%.** The 92.3% figure was taken against the *installed 1.3.6 dist bundle* rather than this branch's source, so 92.3% -> 4.7% spans two build artefacts and is quoted here as an order-of-magnitude indication only, not as a controlled comparison. The controlled measurement of the mechanism itself is the request count against a 340-page store: **340 requests / 118.2 MB before, 10 requests / 3.4 MB after.** +- **test(ui): seven hermetic regression tests pin the budget, the coalescing and the cache lifecycle.** THREE of them measure the pathology directly, and fail against the pre-fix implementation with its own numbers: a single `listLabelSummaries()` call issues **200** requests over a 200-page store, three concurrent calls issue **600**, and a repeat call issues **400** instead of reusing 10. A fourth **passes before and after by design** — it holds the normal path (a store inside the budget still returns exact counts with `search`/`limit` applied) so the budget cannot be satisfied by returning nothing; it is an anti-vacuity guard, not a regression test, and is described that way rather than counted among the failing three. Adversarial review then proved those four pinned the cache only as "a cache exists" — an implementation with an infinite TTL and no invalidation passed all of them, which would freeze the sidebar counts forever — so three more pin the cache's lifecycle: that it expires, that a write drops it, and that a write landing MID-WALK is fenced. Each was mutation-tested: the infinite-TTL/no-invalidation implementation fails two of them, and removing the generation fence fails the third. - **fix(ui): `emails ui` could not start in any real terminal — the packaged runtime loaded a foreign OpenTUI native library.** 1.3.4 exited immediately with `Failed to initialize OpenTUI render library: Symbol "createEventSink" not found in .../@opentui/core-linux-arm64/libopentui.so`. `@opentui/core` loads its prebuilt renderer with a bare `import("@opentui/core-")` from inside its own module, so the version-matched prebuilt in `@opentui/core/node_modules/` is what answers. `scripts/build-tui-runtime.ts` inlined core into `dist/cli/ui-runtime-bundle.js` while listing the eight platform packages as **external**, which moved that import to `dist/cli/` — it then resolved against the installed package's *parents*, never saw core's own prebuilt, and bound to whatever copy the install had hoisted (here `0.1.105`, an ABI predating the symbol core calls). The install was version-correct throughout; only the loaded `.so` was wrong. `@opentui/core` is now external — it is already a declared runtime dependency, and keeping it in `node_modules` keeps the JS and the library it `dlopen`s in one dependency tree. `web-tree-sitter` and `bun-ffi-structs` were dropped from the same list for the same reason: both are core's dependencies, neither is declared by `@hasna/emails`, so externalising them pointed at unowned copies too. Declaring the eight platform packages as our own `optionalDependencies` was rejected — it copies upstream's platform matrix into this manifest and rots on every core bump, while leaving the resolution anchor wrong. `patchBundledNativeAssetPath()` is gone with the bundling it patched around, and the runtime bundle drops from 3.4 MB to 2.1 MB. - **test(ui): the build contract asserted the broken configuration, so the suite stayed green through a UI that could not start.** It required `scripts/build-tui-runtime.ts` to contain `"@opentui/core-linux-arm64"` and `...nativePackages` — the exact lines that caused the crash — because every assertion was a text match on the build script rather than a check of the artifact it produces. New `src/cli/tui/ui-runtime-contract.test.ts` rebuilds the bundle (never trusting a stale one), parses its imports with `Bun.Transpiler.scanImports` rather than a regex over 3 MB of bundled output, and fails if any bare import is not a declared runtime dependency of this package — the general form of the defect, not just the OpenTUI instance. It carries a positive control proving the check reports an undeclared external and passes a declared one, and a behavioural guard that a non-interactive `emails ui` exits non-zero, so a refusal can never be read as a UI that ran. - feat(cli): every inbox and sync command now accepts `-j, --json`, emits one structured result document, and reports machine-readable failures without changing the existing human output. diff --git a/src/lib/self-hosted-mail-data-source.test.ts b/src/lib/self-hosted-mail-data-source.test.ts index caa0ff54..d9b97f93 100644 --- a/src/lib/self-hosted-mail-data-source.test.ts +++ b/src/lib/self-hosted-mail-data-source.test.ts @@ -2725,4 +2725,108 @@ describe("SelfHostedMailDataSource — listLabelSummaries scan budget", () => { ]); expect((await ds.listLabelSummaries({ search: "urg" })).map((l) => l.name)).toEqual(["urgent"]); }); + + // --------------------------------------------------------------------------- + // The three tests above pin BOUND and COALESCE. Adversarial review proved they + // pin the cache only as "a cache exists": an implementation with an infinite + // TTL and no invalidation passed all of them, which would freeze the sidebar + // counts forever. These three pin the cache's LIFECYCLE — that it expires, + // that a write drops it, and that a write landing MID-WALK is fenced. + // --------------------------------------------------------------------------- + + // A small store whose label set can be changed between walks, so a re-walk is + // observable in the RESULT and not merely in the request count. + function mutableServe(): { + fetchImpl: SelfHostedFetch; + requests: string[]; + setLabels: (labels: string[]) => void; + gate: (hold: Promise | null) => void; + } { + const requests: string[] = []; + let labels = ["urgent"]; + let held: Promise | null = null; + 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); } }); + if (method !== "GET" || u.pathname !== "/v1/messages") return ok({ message: v1("1", { labels }) }); + // Snapshot the rows AT REQUEST TIME, before the gate. A walk held open must + // return the rows as they were when it started — otherwise it silently + // reads post-write data and cannot be stale, which would make the + // mid-walk-write test pass for the wrong reason. + const snapshot = labels; + if (held) await held; + return ok({ messages: [listV1(v1("1", { labels: snapshot }))], next_cursor: null }); + }; + return { + fetchImpl, + requests, + setLabels: (next) => { labels = next; }, + gate: (hold) => { held = hold; }, + }; + } + + function dsFor(serve: { fetchImpl: SelfHostedFetch }, now: () => number): SelfHostedMailDataSource { + return new SelfHostedMailDataSource({ + baseUrl: "https://emails.example/v1", + apiKey: "test-key", + fetchImpl: serve.fetchImpl, + now, + }); + } + + it("re-walks once the cache TTL has expired", async () => { + const serve = mutableServe(); + let clock = 1_000_000; + const ds = dsFor(serve, () => clock); + + expect((await ds.listLabelSummaries()).map((l) => l.name)).toEqual(["urgent"]); + const afterFirst = serve.requests.length; + + // Past any sane TTL, and the label set has changed underneath. + serve.setLabels(["archived"]); + clock += 10 * 60 * 1000; + + expect((await ds.listLabelSummaries()).map((l) => l.name)).toEqual(["archived"]); + expect(serve.requests.length).toBeGreaterThan(afterFirst); + }); + + it("drops the cached tally when a write changes labels", async () => { + const serve = mutableServe(); + const clock = 1_000_000; + const ds = dsFor(serve, () => clock); + + expect((await ds.listLabelSummaries()).map((l) => l.name)).toEqual(["urgent"]); + + // The clock does NOT move, so only invalidation can produce a re-walk. + serve.setLabels(["archived"]); + await ds.addLabel("1", "archived"); + + expect((await ds.listLabelSummaries()).map((l) => l.name)).toEqual(["archived"]); + }); + + it("fences a write that lands while a walk is already in flight", async () => { + const serve = mutableServe(); + const clock = 1_000_000; + const ds = dsFor(serve, () => clock); + + // Hold the walk open so the write lands strictly mid-walk. + let release!: () => void; + serve.gate(new Promise((resolve) => { release = resolve; })); + const inFlight = ds.listLabelSummaries(); + await Promise.resolve(); + + serve.setLabels(["archived"]); + await ds.addLabel("1", "archived"); + + serve.gate(null); + release(); + await inFlight; + + // The in-flight walk read pre-write rows. It must NOT have installed them as + // the cache, or this read returns the stale label for a full TTL even though + // the clock has not moved. + expect((await ds.listLabelSummaries()).map((l) => l.name)).toEqual(["archived"]); + }); }); diff --git a/src/lib/self-hosted-mail-data-source.ts b/src/lib/self-hosted-mail-data-source.ts index fd50b3ea..02de58e7 100644 --- a/src/lib/self-hosted-mail-data-source.ts +++ b/src/lib/self-hosted-mail-data-source.ts @@ -866,7 +866,11 @@ export class SelfHostedMailDataSource implements MailDataSource { private readonly maxResponseBytes: number; private scanCache: { at: number; rows: V1Message[] } | null = null; private labelTallyCache: { at: number; tally: Map } | null = null; - private labelTallyInFlight: Promise<{ tally: Map }> | null = null; + private labelTallyInFlight: { generation: number; promise: Promise<{ tally: Map }> } | null = null; + // Fences the tally against a write that lands MID-WALK. Clearing the cache is + // 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; constructor(options: SelfHostedMailDataSourceOptions) { const url = new URL(options.baseUrl); @@ -1134,8 +1138,12 @@ export class SelfHostedMailDataSource implements MailDataSource { private invalidate(): void { this.scanCache = null; - // Labelling a message changes the tally, so a write must drop it too. + // Labelling a message changes the tally, so a write must drop it too — and + // must also fence any walk currently in flight, whose rows predate this + // write. Without the generation bump that walk would install a stale tally + // the moment it finished and serve it for a full TTL. this.labelTallyCache = null; + this.labelTallyGeneration += 1; } private async listFilteredMailboxPage(mailbox: Mailbox, scope: SelfHostedScope | undefined, opts?: MailboxListOptions): Promise { @@ -1478,12 +1486,16 @@ export class SelfHostedMailDataSource implements MailDataSource { // applied to its OUTPUT — so one cached tally serves every caller, whatever // options they pass. private async labelTally(): Promise<{ tally: Map }> { + const generation = this.labelTallyGeneration; const cached = this.labelTallyCache; if (cached && this.now() - cached.at < LABEL_TALLY_TTL_MS) return cached; // Coalesce: the TUI starts a new sidebar load every 30s without awaiting the // previous one. Without this, those calls each open their own cursor walk and - // the walks stack up instead of replacing one another. - if (this.labelTallyInFlight) return this.labelTallyInFlight; + // the walks stack up instead of replacing one another. A walk from an OLDER + // generation is not joinable — its rows predate a write — so a caller after + // an invalidate starts a fresh walk rather than inheriting stale counts. + const inFlight = this.labelTallyInFlight; + if (inFlight && inFlight.generation === generation) return inFlight.promise; const walk = (async () => { const tally = new Map(); @@ -1502,15 +1514,20 @@ export class SelfHostedMailDataSource implements MailDataSource { if (requests >= MAX_LABEL_SCAN_REQUESTS) break; } const entry = { at: this.now(), tally }; - this.labelTallyCache = entry; + // Only install if no write landed while this walk was running. Otherwise + // these rows are already stale and caching them would serve pre-write + // counts for a full TTL. + if (this.labelTallyGeneration === generation) this.labelTallyCache = entry; return entry; })(); - this.labelTallyInFlight = walk; + const pending = { generation, promise: walk }; + this.labelTallyInFlight = pending; try { return await walk; } finally { - this.labelTallyInFlight = null; + // Never clear a NEWER walk that replaced this one after an invalidate. + if (this.labelTallyInFlight === pending) this.labelTallyInFlight = null; } } diff --git a/src/workflow-contract.test.ts b/src/workflow-contract.test.ts index 56576a20..96ecd89b 100644 --- a/src/workflow-contract.test.ts +++ b/src/workflow-contract.test.ts @@ -11,7 +11,7 @@ const packageProvenanceWorkflowSha256 = "706c636d7b60059f6e8ce52229bfb723c0c9a2c // change to that section an explicit, reviewable edit, so an entry cannot drift // into a shipped release section unnoticed. Recompute with the same // markdownSection()/textSha256() pair this file already uses. -const unreleasedSectionSha256 = "42cf7cc5790ec032c46fcf8446b0d0f907f67615ad181d786a4aff0367226858"; +const unreleasedSectionSha256 = "5d81b6a062a47677aed260da62d98c37c00ad782db5bf162a7ad665e1afdc9f1"; const release132Section = `## 1.3.2 (2026-07-26) - fail closed on malformed JSON, wrong response envelopes, and missing required From 171f2310fe27534a1f8b16c64282960cdd24705c Mon Sep 17 00:00:00 2001 From: Andrei Hasna Date: Wed, 5 Aug 2026 02:14:13 +0300 Subject: [PATCH 5/5] fix(security): update vulnerable runtime dependencies Agent: unresolved-account004 --- bun.lock | 7 ++++--- bunfig.toml | 4 ++++ package.json | 3 ++- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/bun.lock b/bun.lock index 33dd6b22..b743dd11 100644 --- a/bun.lock +++ b/bun.lock @@ -40,7 +40,8 @@ }, "overrides": { "brace-expansion": "2.1.2", - "fast-uri": "3.1.4", + "fast-uri": "3.1.5", + "ip-address": "10.3.1", }, "packages": { "@alcalzone/ansi-tokenize": ["@alcalzone/ansi-tokenize@0.1.3", "", { "dependencies": { "ansi-styles": "^6.2.1", "is-fullwidth-code-point": "^4.0.0" } }, "sha512-3yWxPTq3UQ/FY9p1ErPxIyfT64elWaMvM9lIHnaqpyft63tkxodF5aUElYHrdisWve5cETkh1+KBw1yJuW0aRw=="], @@ -361,7 +362,7 @@ "fast-sha256": ["fast-sha256@1.3.0", "", {}, "sha512-n11RGP/lrWEFI/bWdygLxhI+pVeo1ZYIVwvvPkW7azl/rOy+F3HYRZ2K5zeE9mmkhQppyv9sQFx0JM9UabnpPQ=="], - "fast-uri": ["fast-uri@3.1.4", "", {}, "sha512-8JnbkQ4juDyvYs4mgFGQqg4yCYtFDtUtmp2QIQq11ZZe5CFQ5wcqm1rqDgAh/QdMySuBnPzMUiJUNZG5N/AiQw=="], + "fast-uri": ["fast-uri@3.1.5", "", {}, "sha512-gHwA1O9LDIcKunMKhObS/HimwtehO1nPUECKAu5TpKgaO19fcWEl4bliWe1jWxVFvIXztJjjQ4L8XQ1EU9f7Jw=="], "finalhandler": ["finalhandler@2.1.1", "", { "dependencies": { "debug": "^4.4.0", "encodeurl": "^2.0.0", "escape-html": "^1.0.3", "on-finished": "^2.4.1", "parseurl": "^1.3.3", "statuses": "^2.0.1" } }, "sha512-S8KoZgRZN+a5rNwqTxlZZePjT/4cnm0ROV70LedRHZ0p8u9fRID0hJUZQpkKLzro8LfmC8sx23bY6tVNxv8pQA=="], @@ -415,7 +416,7 @@ "ink-text-input": ["ink-text-input@6.0.0", "", { "dependencies": { "chalk": "^5.3.0", "type-fest": "^4.18.2" }, "peerDependencies": { "ink": ">=5", "react": ">=18" } }, "sha512-Fw64n7Yha5deb1rHY137zHTAbSTNelUKuB5Kkk2HACXEtwIHBCf9OH2tP/LQ9fRYTl1F0dZgbW0zPnZk6FA9Lw=="], - "ip-address": ["ip-address@10.2.0", "", {}, "sha512-/+S6j4E9AHvW9SWMSEY9Xfy66O5PWvVEJ08O0y5JGyEKQpojb0K0GKpz/v5HJ/G0vi3D2sjGK78119oXZeE0qA=="], + "ip-address": ["ip-address@10.3.1", "", {}, "sha512-1e9d3kb97NHJTIJDZW9rKqW2h6+dFa50Dy0fpPSMQp2ADje5gvKsXmdiK6dwY5t76TaTt5+P5N1Y/LoToIxP6g=="], "ipaddr.js": ["ipaddr.js@1.9.1", "", {}, "sha512-0KI/607xoxSToH7GjN1FfSbLoU0+btTicjsQSWQlh/hZykN8KpmMf7uYwPW3R+akZ6R/w18ZlXSHBYXiYUPO3g=="], diff --git a/bunfig.toml b/bunfig.toml index b16283cb..b50e79a0 100644 --- a/bunfig.toml +++ b/bunfig.toml @@ -1,4 +1,8 @@ preload = ["@opentui/solid/preload"] +[install] +minimumReleaseAge = 604800 +minimumReleaseAgeExcludes = ["fast-uri"] + [test] preload = ["@opentui/solid/preload"] diff --git a/package.json b/package.json index 55ae1709..19508b34 100644 --- a/package.json +++ b/package.json @@ -68,7 +68,8 @@ }, "overrides": { "brace-expansion": "2.1.2", - "fast-uri": "3.1.4" + "fast-uri": "3.1.5", + "ip-address": "10.3.1" }, "author": "Andrei Hasna ", "license": "Apache-2.0",