Attachments stage 2: server-side expiry sweep

.github/workflows/deploy.yml

@@ -69,7 +69,9 @@ jobs:
         if: vars.SUPABASE_PROJECT_REF != ''
         env:
           SUPABASE_ACCESS_TOKEN: ${{ secrets.SUPABASE_ACCESS_TOKEN }}
-        run: supabase functions deploy venice --project-ref ${{ vars.SUPABASE_PROJECT_REF }}
+        run: |
+          supabase functions deploy venice --project-ref ${{ vars.SUPABASE_PROJECT_REF }}
+          supabase functions deploy expire-attachments --project-ref ${{ vars.SUPABASE_PROJECT_REF }}
 
   build:
     needs: sync-supabase

docs/dev/attachments.md

@@ -4,10 +4,12 @@
 > private `attachments` Storage bucket (the `documents` pattern), not as
 > base64 in `message_attachments.data`. Liveness keys on `storage_path`,
 > reads go through signed URLs, and the legacy base64 was reclaimed
-> one-time (pre-bucket rows are treated as expired). Still pending:
-> (1) the **expiry sweep is not yet server-side** - the old browser
-> worker / `expire_old_attachments` RPC are inert, so bucket objects do
-> not yet expire; (2) the `data` column drop. See
+> one-time (pre-bucket rows are treated as expired). Expiry now runs
+> **server-side** (the standalone `expire-attachments` edge function +
+> hourly cron deletes bucket objects 30 days after a thread goes
+> dormant). Still pending: (1) retiring the now-INERT browser
+> `attachment_expiry` worker + `expire_old_attachments` RPC (cleanup);
+> (2) the `data` column drop. See
 > [`./in-progress/attachments-storage-migration.md`](./in-progress/attachments-storage-migration.md).
 > Sections below are mid-update; where they describe base64-in-`data`,
 > read it as "historical, now storage_path + bucket".

docs/dev/in-progress/attachments-storage-migration.md

@@ -174,10 +174,23 @@ row with `data` set but no `storage_path` just gets `data` nulled
   `data` column stays (write-never, read-never after the reclaim).
   The old browser expiry worker / `expire_old_attachments` RPC are
   left INERT - so bucket objects do not yet expire.
-- **Stage 2 - server-side expiry (TODO).** The pg_cron + edge-
-  function sweep described under Expiry above; retire the browser
-  `attachment_expiry` worker + the inert RPC. Until this lands,
-  uploaded objects accumulate in the bucket.
+- **Stage 2 - server-side expiry sweep (LANDED, server side).** The
+  standalone `expire-attachments` edge function + hourly pg_cron job,
+  with the `list_expirable_attachments` / `mark_attachments_expired`
+  service-role RPCs and the I/O-free `runExpiry` drain loop
+  (`_shared/expire-attachments.ts`, unit-tested). Deployed via its own
+  line in `deploy.yml`. Cron/Storage round-trip is unverified from the
+  cloud env - confirm after deploy that an object actually disappears
+  ~30 days after its thread goes dormant.
+- **Stage 2b - retire the browser worker (TODO, cleanup).** The
+  `attachment_expiry` UNIT inside the supervisor
+  (`src/lib/agents/supervisor/{loop,manager,worker}.ts` +
+  `src/lib/agents/attachment_expiry/`), `SupabaseService.expireOldAttachments`,
+  the `expire_old_attachments` RPC, and `tests/attachment-expiry-loop.test.ts`.
+  Left in place for now because it is INERT post-Stage-1 (the RPC nulls
+  `data where data is not null`, which matches no rows after the reclaim,
+  so it no-ops). Pure deletion; deferred only to avoid rushed supervisor
+  surgery.
 - **Collapse PR (follow-up)** drops the `data` column AND removes
   the reclaim UPDATE together. Splitting is required: a single
   apply can't both keep an UPDATE that references `data` and drop

docs/dev/in-progress/venice-edge-functions/text-parser.md

@@ -1,17 +1,81 @@
 # Text parser milestone
 
-*Skeleton - embeddings lessons folded in (step 8); target state
-still to define.* Part of the [Venice edge functions](./README.md)
-project.
+*Now bug-driven: text extraction is **broken from the browser**
+(see [Motivating bug](#motivating-bug-text-extraction-broken-from-the-browser)),
+so this milestone is the fix, not just a consolidation. Embeddings
+lessons folded in (step 8).* Part of the
+[Venice edge functions](./README.md) project.
 
 Wraps `POST /augment/text-parser` (`VeniceClient.extractText`)
 as a `/text-parser` route on the `venice` function. This is the
 endpoint behind the attachments flow (see
-[attachments](../../attachments.md)).
+[attachments](../../attachments.md)) AND the Library document
+upload (see [library](../../library.md)).
 
 The one with a file upload: `extractText` posts a `Blob` +
 filename as multipart, unlike the JSON-bodied endpoints.
 
+## Motivating bug: text extraction broken from the browser
+
+Observed 2026-05-30 during attachments-storage QA, on current
+`main`.
+
+**Symptom.** Uploading any non-image file (tried `.pdf`,
+`.txt`, `.md`, all <1 KB) fails at the composer: the attachment
+pill turns red and sending shows
+
+> "dishes.txt": Text extraction failed: Network error
+> contacting Venice: Failed to fetch
+
+That string is `extractText`'s catch block in
+`src/lib/venice.ts` wrapping a thrown `TypeError: Failed to
+fetch` as `VeniceError(..., 'network')`. "Failed to fetch" is a
+browser network/CORS-layer rejection - the request never
+completed in a CORS-readable way - NOT an HTTP error response
+(a 4xx/5xx would surface through `classifyError`, with a status).
+
+**Why it is CORS, not a bad request.**
+
+- The endpoint path is correct and current: Venice's docs list
+  `POST /api/v1/augment/text-parser`, multipart/form-data,
+  accepting PDF/DOCX/PPTX/XLSX/plain-text up to 25 MB - exactly
+  the files that failed. So it is not a wrong URL (would 404)
+  or a rejected format (would 400); both are HTTP errors, not
+  "Failed to fetch".
+- `app.venice` is a `VeniceClient` pointed at the default
+  `https://api.venice.ai/api/v1` (`new VeniceClient({ apiKey })`
+  in `src/lib/state.svelte.ts`), calling **directly from the
+  browser**.
+- Image paths work against the **same host and key**: inline
+  vision (`/chat/completions`) and `analyze_image` were verified
+  live in the same session, and `generate_image` hits
+  `/image/generate`. So the browser can reach Venice with CORS
+  for those endpoints - the key, origin, and general CORS are
+  fine.
+- Therefore the failure is **endpoint-specific**: Venice
+  CORS-enables its chat/image endpoints but evidently NOT
+  `/augment/text-parser`. (Their docs make no browser-safety
+  claim for it.) A 404 whose error response omits CORS headers
+  would also read as "Failed to fetch", so a quietly-moved or
+  gated route is a secondary possibility - but the path matches
+  the live docs.
+
+**This is pre-existing, not caused by the attachments-storage
+migration.** Extraction runs at the composer BEFORE any storage
+write; the storage migration is entirely downstream. It was
+simply never exercised live with a non-image file until now
+(image uploads skip extraction). Unconfirmed whether it ever
+worked from the browser or Venice tightened CORS later - the
+browser devtools Network entry for the failed request (CORS
+error vs a status code) would settle that, but does not change
+the fix.
+
+**The fix = this milestone.** Routing text extraction through
+the `venice` edge function makes the call **server-side**, where
+browser CORS does not apply and the project-global key already
+lives. Fixes both the chat-attachment path and the Library
+upload path in one move.
+
 ## Why this one is different
 
 - **Multipart upload.** The request carries file bytes, not
@@ -26,19 +90,63 @@ filename as multipart, unlike the JSON-bodied endpoints.
 
 ## Current state
 
-To document: `extractText` in `src/lib/venice.ts`, the
-attachments upload flow that calls it, and the size/expiration
-constraints from the attachments feature doc.
+`extractText(file: Blob, filename: string): Promise<string>` in
+`src/lib/venice.ts`:
+
+- Builds a `FormData` with `file` (the Blob + filename) and
+  `response_format: 'json'`.
+- `POST`s to `${baseUrl}/augment/text-parser` with ONLY an
+  `Authorization: Bearer <key>` header - deliberately not
+  `this.headers()`, because a JSON Content-Type would clobber
+  the multipart boundary the browser sets.
+- On a thrown fetch error wraps it as `VeniceError('Network
+  error contacting Venice: ...', 'network')` - the string the
+  user saw.
+- On `!res.ok` calls `classifyError`; on success reads `text`
+  off the JSON body (with a couple of fallback keys).
+
+**Two call sites**, both must end up routed server-side:
+
+1. **Chat attachments** - `src/screens/Chat.svelte`
+   `addAttachment(file)` calls `app.venice.extractText(file,
+   file.name)` for non-image files at compose time, before the
+   message is sent. This is the path the bug report hit.
+2. **Library uploads** - `src/lib/documents.ts` `ingestDocument`
+   calls `venice.extractText(file, file.name)` after the bucket
+   upload; a failure there marks the document `extraction_status
+   = 'failed'` (the doc is still stored, just not searchable).
+
+Size/expiration constraints to mind: attachments cap at
+`MAX_ATTACHMENT_BYTES` (10 MB); Library at
+`MAX_DOCUMENT_FILE_BYTES` (25 MB) - which is also Venice's
+text-parser limit. The edge-function request-size ceiling vs
+these caps is the open question below.
 
 ## Target state
 
-To define.
+A `/text-parser` route on the `venice` edge function that accepts
+the multipart upload (user JWT auth, `verify_jwt` on, shared key
+from `app_config` via service role - copy `/embed`'s model, not
+`/backfill`'s), forwards it to Venice server-side, and returns
+the parsed `{ text, ... }`. The browser stops calling Venice
+directly: `extractText` becomes a thin call to the function
+(e.g. a `SupabaseService.extractText` mirroring how browser
+embeds now go through the function), and both call sites above
+move onto it. Keep the `VeniceError` shape so the composer pill
+and the `ingestDocument` failure branch render unchanged.
 
 ## Open questions
 
 - Edge-function payload size limit vs the largest attachment we
-  accept - does anything need chunking or a direct-to-Venice
-  escape hatch for large files?
+  accept (10 MB attachments, 25 MB Library docs) - does anything
+  need chunking or a direct-to-Venice escape hatch for large
+  files? If a large file can't round-trip through the function,
+  the direct-from-browser path it would fall back to is the very
+  one that's CORS-broken - so the escape hatch may have to be a
+  signed-upload-to-storage-then-server-fetch shape rather than
+  browser-direct-to-Venice.
+- Confirm the CORS diagnosis from devtools (nice-to-have; the
+  server-side route fixes it regardless).
 
 ## Lessons from the embeddings milestone
 

supabase/functions/_shared/expire-attachments.ts

@@ -0,0 +1,77 @@
+// I/O-free orchestration for the attachment-expiry sweep, the server-side
+// replacement for the old browser attachment_expiry worker (Stage 2 of the
+// attachments-storage migration). All I/O is injected so this runs under
+// `deno test` with fakes - the edge function (expire-attachments/index.ts)
+// wires the real Supabase service-role client in.
+//
+// The sweep: pull a bounded batch of live attachments whose owning thread has
+// been dormant past the cutoff, delete their bucket objects, then mark the rows
+// expired (null storage_path + stamp expired_at). Repeat until a batch comes
+// back short (queue drained), the row cap is hit, or the time budget elapses;
+// the next cron tick resumes. Deletion and marking are idempotent, so no
+// per-row claim is needed - overlapping ticks at worst redo harmless work.
+
+export interface ExpireBatchRow {
+  id: string;
+  storagePath: string;
+}
+
+export interface ExpireDeps {
+  /** Next batch of expirable rows (live + dormant), at most `batchSize`. */
+  listBatch: (batchSize: number) => Promise<ExpireBatchRow[]>;
+  /** Delete these object keys from the attachments bucket. Idempotent. */
+  deleteObjects: (paths: string[]) => Promise<void>;
+  /** Null storage_path + stamp expired_at for these ids. Returns row count. */
+  markExpired: (ids: string[]) => Promise<number>;
+}
+
+export interface ExpireOpts {
+  batchSize: number;
+  maxRows: number;
+  timeBudgetMs: number;
+  now?: () => number;
+}
+
+export interface ExpireSummary {
+  /** Rows marked expired (objects deleted). */
+  expired: number;
+  /** Batches processed. */
+  batches: number;
+  /** True when stopped on the cap/budget rather than draining the queue. */
+  bounded: boolean;
+  durationMs: number;
+}
+
+export async function runExpiry(deps: ExpireDeps, opts: ExpireOpts): Promise<ExpireSummary> {
+  const now = opts.now ?? Date.now;
+  const start = now();
+  let expired = 0;
+  let batches = 0;
+  let bounded = false;
+
+  for (;;) {
+    if (expired >= opts.maxRows) {
+      bounded = true;
+      break;
+    }
+    if (now() - start >= opts.timeBudgetMs) {
+      bounded = true;
+      break;
+    }
+
+    const remaining = opts.maxRows - expired;
+    const batchSize = Math.min(opts.batchSize, remaining);
+    const rows = await deps.listBatch(batchSize);
+    if (rows.length === 0) break; // queue drained
+
+    await deps.deleteObjects(rows.map((r) => r.storagePath));
+    const marked = await deps.markExpired(rows.map((r) => r.id));
+    expired += marked;
+    batches += 1;
+
+    // A short batch means the eligible set is exhausted for now.
+    if (rows.length < batchSize) break;
+  }
+
+  return { expired, batches, bounded, durationMs: now() - start };
+}