fix: capture reliability — job/write lane split, network-idle precondition, and realistic budgets

.env.example

@@ -20,9 +20,12 @@ DATA_DIR=./data
 # Linux when unset.
 # CHROME_PATH=/usr/bin/chromium
 # Budget in milliseconds for ONE capture job. Capture and the LLM read share a job, so
-# this covers both. The default suits a CLI provider; a slow local model may need more.
-# Too low and an item fails as "timed out" with its page already captured.
-# CAPTURE_TIMEOUT_MS=180000
+# this covers both: page navigation (120s) + network settle (30s) + the agent's own
+# 300s ceiling all have to fit inside it. Deliberately generous — board-oss is
+# single-tenant, so this is a stop-a-wedged-job ceiling, not a latency target, and a
+# typical add finishes in ~40-50s. Too low and an item fails as "timed out" with its
+# page already captured.
+# CAPTURE_TIMEOUT_MS=600000
 
 # --- LLM provider (optional; unset = no-AI, enrichment disabled) ---
 # CLI agent id for the subprocess provider (claude / codex / cursor-agent).

README.md

@@ -35,7 +35,7 @@ the full annotated list). Empty/whitespace values are treated as unset.
 | `DATA_DIR` | `./data` | Persistent data root (SQLite DB + screenshots). |
 | `CHROME_PATH` | autodetect | System Chromium/Chrome binary; autodetected on Linux when unset. |
 | `LLM_AGENT` / `LLM_MODEL` / `LLM_BASE_URL` / `LLM_API_KEY` | unset | LLM provider. **Unset = no-AI** (enrichment disabled). With `LLM_AGENT=claude` and no `LLM_MODEL`, Board asks the CLI for **Sonnet** rather than inheriting your interactive default. |
-| `CAPTURE_TIMEOUT_MS` | `180000` | Budget for one capture job — page capture **and** the AI read share it. A CLI agent routinely takes 90–100s; raise it for slow local models. |
+| `CAPTURE_TIMEOUT_MS` | `600000` | Budget for one capture job — page capture **and** the AI read share it. Deliberately generous: it's a ceiling to stop a wedged job, not a latency target, and a typical add finishes in ~40–50s. Lower it only if you'd rather fail fast than wait. |
 | `BOARD_API_TOKEN` | unset | Bearer token for the `/api/v1` capture API. **Unset = the v1 API is off** (fail-closed). See [Integrations](docs/integrations.md). |
 
 ## Security & the reverse-proxy model

docs/bmad/architecture.md

@@ -40,7 +40,7 @@ note: "Decisions were reached by multi-round party consensus; this formalizes th
 - **AD3 SQLite/Drizzle** — WAL + JSON + FTS5; screenshots files-on-disk, path in DB.
 - **AD4 Capture in-process, concurrency 1** — launch→screenshot→kill; the offloadable sidecar *service* is deferred but its **contract is designed in v1** (token-authed, idempotent).
 - **AD5 `LLMProvider` seam, two transports** — `HttpProvider` (API key / open model) + `CliProvider` (coding-agent subprocess); default install zero-coding-CLI.
-- **AD6 Async job model** — in-process single-writer worker queue + `status` column + SSE; no external broker. The queue **is** the SQLite single-writer guard.
+- **AD6 Async job model** — in-process worker queue + `status` column + SSE; no external broker. **Two lanes:** jobs serialize at concurrency 1 (memory bound), SQLite writes serialize separately (write-interleaving bound). *Revised: the job lane was originally the SQLite single-writer guard too, which made every interactive write wait on a running capture's network I/O.*
 - **AD7 Reverse-proxy-only auth** — localhost bind default.
 - **AD9 Schema-as-data** — board behavior is a stored `board_descriptor` on a closed field-type set; enrichment + rendering are dynamic.
 - **AD10 Agentic composer** — v1 launch feature, built after the seeded boards; meta-schema + validate-and-repair.
@@ -87,7 +87,7 @@ interface CaptureAdapter { fetch(source: string, ctx): Promise<{ fields: Record<
 - **Composer meta-schema:** the JSON-schema *for a descriptor*; the composer emits a descriptor validated against it (validate-and-repair; closed types; field cap; reserved-key rejection).
 
 ### 4.5 Job model & status (AD6)
-- `JobQueue` = a single async worker draining jobs serially; capture + enrichment jobs run here (this is also the SQLite single-writer).
+- `JobQueue` = a single async worker draining jobs serially; capture + enrichment jobs run here. It is **not** the SQLite single-writer: writes have their own lane, because a job holds its slot across Chrome + LLM round-trips (up to `CAPTURE_TIMEOUT_MS`) while every SQLite write is sub-millisecond. Sharing one lane meant `POST /api/collections/:cid/items` blocked for the length of the running capture. Writes issued from inside a job go through the write lane like any other.
 - `item.status`: `pending → processing → done → error` (`error_reason` persisted). SSE endpoint streams transitions; refetch/poll fallback.
 
 ## 5. Data model

src/capture/adapter.ts

@@ -1,7 +1,7 @@
 import { eq } from 'drizzle-orm';
 
 import { boards, items, type NewAsset } from '../db/schema.js';
-import { writeItemDirect } from '../db/queue.js';
+import { enqueueWrite, writeItemDirect } from '../db/queue.js';
 import { statusHub } from '../sse.js';
 import { createUrlScreenshotAdapter } from './url-screenshot.js';
 import { createUrlReadableAdapter } from './url-readable.js';
@@ -148,8 +148,6 @@ export async function runCaptureForItem(
     registerTeardown: args.registerTeardown,
   });
 
-  const item = handle.db.select().from(items).where(eq(items.id, args.itemId)).get();
-
   // Captured keys that are SYSTEM COLUMNS (e.g. `title`) belong on the column, not in
   // the `item.fields` JSON bag (the descriptor contract: title/notes/favorite are
   // system columns). Lift those out; merge the rest into fields. Without this, a
@@ -160,7 +158,6 @@ export async function runCaptureForItem(
     if (SYSTEM_COLUMNS.has(k)) systemUpdates[k] = v;
     else capturedFields[k] = v;
   }
-  const mergedFields = { ...((item?.fields as Record<string, unknown>) ?? {}), ...capturedFields };
   const assetRows: NewAsset[] = result.assets.map((a, i) => ({
     id: `${args.itemId}-${a.kind}-${i}`,
     itemId: args.itemId,
@@ -171,14 +168,27 @@ export async function runCaptureForItem(
     hash: a.hash ?? null,
   }));
 
-  // Capture runs INSIDE the worker job (slot held) → use the DIRECT write (calling
-  // the enqueueing writeItem here would deadlock). Replaces the item's assets
-  // (delete-then-insert) → idempotent re-capture.
-  writeItemDirect(
-    handle,
-    { ...item, ...systemUpdates, id: args.itemId, boardId: args.boardId, fields: mergedFields },
-    assetRows,
-  );
+  // Read the row and write it back in ONE op on the write lane, with no await between
+  // them — `writeItemDirect` takes the FULL desired row, so a merge built from a stale
+  // snapshot would revert whatever landed in between. Enqueued rather than called
+  // directly: jobs and writes are separate lanes now, so a write from inside a job no
+  // longer self-deadlocks. Replaces the item's assets (delete-then-insert) → idempotent
+  // re-capture.
+  const prevTitle = await enqueueWrite(() => {
+    const item = handle.db.select().from(items).where(eq(items.id, args.itemId)).get();
+    writeItemDirect(
+      handle,
+      {
+        ...item,
+        ...systemUpdates,
+        id: args.itemId,
+        boardId: args.boardId,
+        fields: { ...((item?.fields as Record<string, unknown>) ?? {}), ...capturedFields },
+      },
+      assetRows,
+    );
+    return item?.title ?? undefined;
+  });
 
   // Progressive reveal: the row now holds the page (title + image) but the AI read is
   // still outstanding, and the item's DB status stays `processing` throughout. Announce
@@ -189,7 +199,7 @@ export async function runCaptureForItem(
     itemId: args.itemId,
     boardId: args.boardId,
     status: 'captured',
-    title: (systemUpdates as { title?: string }).title ?? item?.title ?? undefined,
+    title: (systemUpdates as { title?: string }).title ?? prevTitle,
     screenshot: shot?.path || undefined,
   });
 }

src/capture/url-readable.ts

@@ -33,7 +33,9 @@ const IMAGE_EXT: Record<string, string> = {
   'image/webp': 'webp',
 };
 const MAX_IMAGE_BYTES = 12 * 1024 * 1024; // a hero image over ~12MB is almost certainly wrong
-const IMAGE_TIMEOUT_MS = 8000;
+// A hero image can be several MB; 8s dropped it on any slow link, and the card then
+// fell back to its no-image placeholder with no sign anything had gone wrong.
+const IMAGE_TIMEOUT_MS = 30_000;
 
 interface Deps {
   fetchImpl?: typeof fetch;

src/capture/url-screenshot.test.ts

@@ -59,6 +59,42 @@ describe('url-screenshot adapter (Story 6.2)', () => {
     assert.equal(fb.isClosed(), true, 'browser must be closed on the error path');
   });
 
+  // Pages with a persistent connection (analytics socket, chat widget, video preload)
+  // never reach `networkidle2`, and a slow connection pushes an ordinary page past the
+  // 30s nav budget — either way navigation threw and the item died as "Timed out."
+  // Reproduced against twenty.com and stigg.io under CDP throttling: goto rejected at
+  // 30755ms with `TimeoutError: Navigation timeout of 30000 ms exceeded`, which
+  // cleanErrorReason maps to 'timed out'. A quiet network is a nice-to-have for a
+  // screenshot, not a preconditio — capture must degrade to "shot slightly early".
+  it('still captures a page whose network never goes quiet', async () => {
+    let idleAttempted = false;
+    let closed = false;
+    const page = {
+      setViewport: async () => {},
+      // Mirrors puppeteer: waiting for a quiet network on such a page rejects.
+      goto: async (_url: string, opts: { waitUntil: string }) => {
+        if (opts.waitUntil === 'networkidle2') throw new Error('Navigation timeout of 30000 ms exceeded');
+      },
+      waitForNetworkIdle: async () => {
+        idleAttempted = true;
+        throw new Error('Timed out waiting for the network to be idle');
+      },
+      screenshot: async () => Buffer.from('PNGDATA'),
+      evaluate: async (fn: (...a: unknown[]) => unknown) =>
+        fn.toString().includes('document.title') ? { title: 'Chatty', text: 'body text' } : undefined,
+    };
+    const browser: CaptureBrowser = { newPage: async () => page as never, close: async () => { closed = true; } };
+
+    const adapter = createUrlScreenshotAdapter({ launch: async () => browser, sleep: async () => {} });
+    const out = await adapter.fetch('https://chatty.example', { itemId: 'shot3', boardId: 'b', screenshotsDir: dir });
+
+    assert.equal(idleAttempted, true, 'should still TRY to let the network settle');
+    assert.equal(out.fields.title, 'Chatty', 'capture succeeds despite the noisy network');
+    assert.equal(out.assets.length, 1, 'a screenshot is still produced');
+    assert.ok(existsSync(join(dir, 'shot3.png')), 'image written');
+    assert.equal(closed, true, 'browser closed');
+  });
+
   // AC 1 — registered for ingest_mode url-screenshot
   it('declares ingest_mode = url-screenshot', () => {
     const adapter = createUrlScreenshotAdapter();

src/capture/url-screenshot.ts

@@ -15,12 +15,33 @@ import type { AssetSpec, CaptureAdapter, CaptureCtx, CaptureResult, CaptureSourc
 // `error`. Teardown (`close()`) is guaranteed in `finally` and on abort (timeout).
 
 const VIEWPORT = { width: 1440, height: 900, deviceScaleFactor: 1.5 };
-const GOTO_TIMEOUT_MS = 30_000;
+// Budget for reaching `domcontentloaded` — a REAL failure if it expires (no document,
+// nothing to shoot). 30s was inherited from when this timeout ALSO had to absorb the
+// wait for a quiet network; the settle is separate and best-effort now, so the two
+// budgets are sized independently.
+//
+// These are deliberately generous. board-oss is single-tenant and self-hosted: nobody
+// is queued behind you competing for a shared worker, so a capture that takes two
+// minutes and SUCCEEDS beats one that fails fast at 30s and leaves you re-adding the
+// link by hand. A heavy page on a slow link needs well over 30s just to parse — under
+// CDP throttling twenty.com missed the old budget at 30.9s and the item died with
+// "Timed out." Timeouts here exist to stop a genuinely wedged capture, not to enforce
+// a latency SLO.
+const GOTO_TIMEOUT_MS = 120_000;
+// How long to let the network go quiet AFTER the document is ready. Best-effort: when
+// it expires we shoot anyway. See the note on the goto strategy below.
+const SETTLE_TIMEOUT_MS = 30_000;
 
 // Minimal puppeteer-ish surfaces so the launcher is injectable (real Browser fits).
 export interface CapturePage {
   setViewport(vp: { width: number; height: number; deviceScaleFactor: number }): Promise<unknown>;
   goto(url: string, opts: { waitUntil: string; timeout: number }): Promise<unknown>;
+  /**
+   * Optional ONLY so hand-written test fakes stay valid. Real captures always take the
+   * settle path: puppeteer's Page has had this since v5 and we pin ^24. Don't read the
+   * `?.` at the call site as "this might not run in production" — it always does.
+   */
+  waitForNetworkIdle?(opts: { idleTime: number; timeout: number }): Promise<unknown>;
   screenshot(opts: { clip: { x: number; y: number; width: number; height: number } }): Promise<Buffer | Uint8Array>;
   evaluate<T>(fn: (...args: unknown[]) => T): Promise<T>;
 }
@@ -84,7 +105,18 @@ export function createUrlScreenshotAdapter(deps: Deps = {}): CaptureAdapter {
         const browser = await launchP;
         const page = await browser.newPage();
         await page.setViewport(VIEWPORT);
-        await page.goto(url, { waitUntil: 'networkidle2', timeout: GOTO_TIMEOUT_MS });
+        // Navigate on `domcontentloaded`, then let the network settle SEPARATELY and
+        // best-effort. `waitUntil: 'networkidle2'` made a quiet network a PRECONDITION
+        // of the capture: any page holding a connection open — analytics beacon, chat
+        // widget, video preload, websocket — never satisfies it, and goto rejected at
+        // 30s with a TimeoutError that cleanErrorReason renders on the card as
+        // "Timed out." (reproduced against twenty.com and stigg.io under CDP
+        // throttling: 30755ms, no item). A slow connection does the same to an
+        // ordinary page. Screenshot quality is worth waiting for; it is not worth
+        // losing the item over, so a settle that expires just means we shoot a beat
+        // early. renderPageText (browser.ts) already navigates this way.
+        await page.goto(url, { waitUntil: 'domcontentloaded', timeout: GOTO_TIMEOUT_MS });
+        await page.waitForNetworkIdle?.({ idleTime: 500, timeout: SETTLE_TIMEOUT_MS }).catch(() => {});
         await sleep(1000);
         await dismissOverlays(page);
         await sleep(400);

src/capture/url-snapshot.ts

@@ -23,7 +23,11 @@ import { assertCapturableUrl } from './net-guard.js';
 // than spawning) is verified by inspection/manual run (no real Chrome in the suite).
 
 const DEFAULT_MAX_BYTES = 8 * 1024 * 1024; // 8MB per-snapshot cap (footprint guardrail)
-const SNAPSHOT_TIMEOUT_MS = 45_000;
+// SingleFile inlines every asset on the page, so this is strictly slower than the
+// screenshot capture it mirrors — 45s under-served a heavy page and, because a failed
+// snapshot is swallowed (AC4), it just silently produced no archive. Sized like the
+// other capture budgets: a ceiling for a wedged job on a single-tenant box, not an SLO.
+const SNAPSHOT_TIMEOUT_MS = 180_000;
 
 export interface SnapshotBrowser extends TeardownBrowser {
   /** puppeteer Browser.wsEndpoint() — the CDP endpoint SingleFile connects to. */

src/config.test.ts

@@ -171,12 +171,17 @@ describe('snapshotsDir (Story 16.1)', () => {
 // the original fixed 60s an ordinary Claude-CLI enrichment could exhaust the budget
 // and land the item in `error: timed out` with a captured page already in hand.
 describe('capture timeout budget', () => {
+  // The default must EXCEED the sum of the parts it contains, or it silently truncates
+  // them: navigation (120s) + network settle (30s) + the CLI agent's own 300s ceiling.
   it('defaults to a budget that fits capture plus an LLM read', () => {
-    assert.equal(loadConfig({}).captureTimeoutMs, 180_000);
+    assert.ok(
+      loadConfig({}).captureTimeoutMs >= 120_000 + 30_000 + 300_000,
+      'default must leave room for navigation + settle + the agent wall-clock',
+    );
   });
 
   it('is overridable for slow local models', () => {
-    assert.equal(loadConfig({ CAPTURE_TIMEOUT_MS: '600000' }).captureTimeoutMs, 600_000);
+    assert.equal(loadConfig({ CAPTURE_TIMEOUT_MS: '900000' }).captureTimeoutMs, 900_000);
   });
 
   it('rejects a non-numeric or zero value', () => {

src/config.ts

@@ -39,8 +39,15 @@ export interface Config {
    * Budget for ONE capture job, which covers headless capture AND the LLM read
    * together (they share a job so the item holds a single `processing` state). The
    * old fixed 60s was tight enough that an ordinary CLI-provider enrichment could
-   * exhaust it and fail an item whose page had already been captured. Raise it for
-   * slow local models via CAPTURE_TIMEOUT_MS.
+   * exhaust it and fail an item whose page had already been captured. Tune via
+   * CAPTURE_TIMEOUT_MS.
+   *
+   * It must comfortably EXCEED the sum of its parts, or it silently truncates them:
+   * navigation (120s) + network settle (30s) + fixed waits (~1.5s) + the CLI agent's
+   * own 300s wall-clock. Generous by design — board-oss is single-tenant, so nothing
+   * is queued behind you competing for the worker, and this ceiling exists to stop a
+   * genuinely wedged job rather than to enforce a latency SLO. A typical add finishes
+   * in ~40-50s and never approaches it.
    */
   captureTimeoutMs: number;
   provider: ProviderConfig;
@@ -160,7 +167,7 @@ export function loadConfig(env: NodeJS.ProcessEnv): Config {
     screenshotsDir: path.join(dataDir, 'screenshots'),
     snapshotsDir: path.join(dataDir, 'snapshots'),
     chromePath: clean(env.CHROME_PATH) ?? null,
-    captureTimeoutMs: parsePositiveMs(env.CAPTURE_TIMEOUT_MS, 180_000),
+    captureTimeoutMs: parsePositiveMs(env.CAPTURE_TIMEOUT_MS, 600_000),
     provider,
     // Enabled when a transport is configured (agent OR base-URL/key). A model name
     // alone does not enable AI.