WFDASH-WEBAPP/docs/audit/backend-data.md

Backend / API / Data Layer Audit

Scope: src/lib/pocketbase.ts, src/lib/pb-helpers.ts (legacy), src/lib/server/*-service.ts (current layer), src/app/api/v1/* route handlers, /api/mcp, and the route/service contract between them.

Date: 2026-08-28 Author: bob (audit only — no code changes shipped)

This audit is the data-layer companion to security-perf-prod.md (tom, 2026-08-28). It deliberately does not re-cover the auth/secrets surface — see that doc for the critical AGENT_API_KEY / PB admin / CORS issues. Here we focus on:

• legacy pb-helpers.ts vs current src/lib/server/*.service.ts drift
• PB filter / query foot-guns (N+1, getFirstListItem, IN-vs-OR)
• schema-missing fallback pattern and select-field pitfalls
• input validation, error envelope, and route-handler boilerplate
• multi-write consistency (no transactions, partial-success semantics)
• service-layer API inconsistencies (admin PB vs cookie PB)

---

0. Headline summary

Severity	Count	What it is
Critical	3	Legacy pb-helpers.ts used directly by 32 files; schema-field gaps invisible to the type system; pb-helpers.ts PO/quote/order number generators are racy and were deprecated but still callable
Major	12	No schema-validation library; 30+ getFirstListItem calls with inconsistent error handling; N+1 patterns in several services; getAdminPocketBase everywhere bypasses PB listRules; 7077-line MCP route file; schema-missing fallback duplicated 4× with subtle behavioural drift; cookie-auth PB singleton used by Server Components
Minor	10	Inconsistent error envelopes; ad-hoc PO/quote-status string-literal duplication; search hard-caps hardcoded; legacy client-side duplicate-detection post-filter; no structured logging; service-layer returns vary between throwing and returning partial-success

The legacy layer (pb-helpers.ts) is the single biggest source of drift. AGENTS.md says "do NOT extend pb-helpers.ts", and the service layer is the right place for new endpoints — but 32 callers still hit it directly, and the data shape they consume is subtly different from what the service layer emits. Migrating page-by-page is safe; the hazard is doing it inconsistently.

---

1. Critical findings

1.1 pb-helpers.ts PO/quote/order number generators are racy, still exported

File: src/lib/pb-helpers.ts:1755-1772 (generateQuoteNumber) File: src/lib/pb-helpers.ts:1773-1796 (generateOrderNumber) File: src/lib/pb-helpers.ts:1799-1864 (generatePONumber)

All three functions use the read-then-write pattern:

const existing = await pb.collection("orders").getList<Order>(1, 1, {
  filter: `order_number ~ "${prefix}"`,
  sort: "-order_number",
});
let seq = ... + 1;       // <- race window
return `ORD-${...}-${seq}`;

Two simultaneous calls both observe 007 and both write 008. The PB-side atomic counter collection (order_counters, quote_counters) already exists and is used by orders-service.ts:1302-1325 and quotes-service.ts:879-909. AGENTS.md explicitly deprecates these helpers, but they're still exported and reachable:

• generateQuoteNumber — @deprecated comment at line 1748, but nothing prevents a new caller from importing it.
• generateOrderNumber — no deprecation marker; no callers found via grep on the codebase, but still public.
• generatePONumber — inline comment at line 1745 calls it a "client-side mirror" of src/lib/server/po-number.ts. Both exist; the client-side one is racy.

Recommended fix: delete generateQuoteNumber, generateOrderNumber, and generatePONumber from pb-helpers.ts (or move to a legacy sub-export with a build-time ban) and add a CI lint rule that fails if any non-pb-helpers.ts file imports them. Server-side callers already use the atomic variants in orders-service.ts / quotes-service.ts / po-number.ts.

1.2 Legacy pb-helpers.ts is the data layer for 32 files, drift risk

File: src/lib/pb-helpers.ts:1-1994 File: 32 files import from @/lib/pb-helpers (verified 2026-08-28 via grep -rl 'from "@/lib/pb-helpers"' src/ | wc -l)

AGENTS.md says new code goes through src/lib/server/*.service.ts. The legacy helpers are still imported by Server Components, Client Components, and form components directly. The two layers read the same PB collections but emit subtly different shapes:

Helper	Service-layer equivalent
getCustomer / getAllCustomers (pb-helpers.ts:32-46)	customers-service.ts
getCompanies / getCompany / getAllCompanies (pb-helpers.ts:48-67)	not yet migrated
getSuppliers / getSupplier / getAllSuppliers (pb-helpers.ts:69-87)	not yet migrated
getQuotes (pb-helpers.ts:213-247)	quotes-service.ts
getOrders / getOrder / getOrdersWithItems (pb-helpers.ts:617-635, :729-790)	orders-service.ts
createOrderFromQuote (pb-helpers.ts:1231...)	orders-service.createOrderFromQuote
getDeliveryNotes (pb-helpers.ts:1412-1420)	delivery-notes-service.ts

The risk is real and visible: getQuotes in pb-helpers.ts:213 has the deliveryNoteOnly option for the Quotes page delivery-note tab, while quotes-service.ts re-implements the same query with a different filter order. A page that switches from the helper to the service changes the wire payload, which changes the page's render.

Recommended fix: per AGENTS.md, migrate page-by-page. The highest-risk migrations first (orders, quotes) because they emit shapes that already drift from the service. Add a console.warn to the legacy helpers that fires once per session if NODE_ENV !== 'test' and the call comes from a Server Component — the breadcrumb will surface un-migrated callers in the browser console.

1.3 pocketbase.ts browser singleton auth pattern doesn't translate to Server Components

File: src/lib/pocketbase.ts:9-45

let pb: PocketBase | null = null;
export function getPocketBase(): PocketBase {
  if (typeof window === "undefined") {
    // Server-side: create new instance each time
    return new PocketBase(POCKETBASE_URL);
  }
  // Client-side: reuse singleton — load auth from localStorage
  ...
}

Two problems:

1. The server-side path always creates an unauthenticated instance. Server Components that import getPocketBase make PB calls with the _superusers collection bypass or no auth at all, depending on PB rule configuration. getPocketBase itself can't impersonate the user — there's no cookie / header to read on the server. See dev-identity.ts for the project's solution (a cookie called wfdash_dev_email that holds the user's email as the dev placeholder).

2. The client-side singleton uses localStorage for the auth store (pocketbase.ts:23-30). This is fine for browser use, but it couples the data layer to the browser. Any attempt to share a helper between Server and Client Components requires a wrapper (the service layer does this via getAdminPocketBase()).

Recommended fix: make getPocketBase server-side UNSAFE (it should throw if typeof window === "undefined") and force all server-side callers through getAdminPocketBase. Document the rule in AGENTS.md. Today, server components using getCustomer(id) from pb-helpers.ts:32 are silently making unauthenticated PB calls — PB's per-collection listRule/viewRule triages this, but a future rule relaxation will silently expose data.

---

2. Major findings

2.1 No schema-validation library (no zod / yup / valibot / joi / ajv)

File: every src/app/api/v1/**/route.ts that calls req.json() (57 routes)

Input validation is entirely ad-hoc. Each route hand-rolls its required-field checks:

// customers/route.ts:67-69
if (!body?.customer) {
  return NextResponse.json({ error: "customer is required" }, { status: 400 });
}

Verification: grep -i 'zod\|yup\|valibot\|joi\|ajv' package.json returns 0 hits. The cost is real:

• No type safety for body shapes — TypeScript sees the body as any after await req.json(). The next line that reads body.customer will compile even if customer is missing.
• Inconsistent error messages — some routes say "item_description is required.", others say "customer is required", others say "Invalid JSON body". The MCP layer (mcp/route.ts) has a bespoke QUOTE_ITEM_PATCH_FIELDS allowlist (line 161-176), and the v1 layer has no equivalent on most endpoints.
• No coercion — string "123" and number 123 aren't normalised. Many service-layer functions wrap with Number(...) defensively (pb-helpers.ts:1255-1262), but a route handler that passes body.cost_price straight into a pb.update(...) will write a string into a number field.
• No max-size enforcement — the only size cap is the multipart file limit (25 MB on the mcp.add_attachment tool, not on JSON bodies). A 50 MB JSON body is parsed in full before the handler can 413.

Recommended fix: add zod to package.json (small, no deps, TS-native), write one parseBody(req, schema) helper that lives in src/lib/server/http-helpers.ts, and convert routes incrementally. The 7 highest-value routes first (write endpoints that mutate PB): customers, suppliers, companies, orders, order_items, quotes, quote_items. The MCP route already enforces its own per-tool schema via JSON Schema (not zod), so leave it alone.

2.2 getFirstListItem throws on miss — 30+ inconsistent handlers

File: 31 call sites in src/lib/server/ and src/app/api/v1/

PB's getFirstListItem(filter) throws a 404 if the filter matches zero rows. Half the call sites defensively catch, half don't. Examples:

• orders-service.ts:1404 — getFirstListItem(year=${year} && month=${month}) is inside a try, catches via fallback (line 1411).
• orders-service.ts:2574 — getFirstListItem<Order>(order_number = "${esc}") is inside its own try/catch, returns null on 404 (line 2580).
• management-service.ts:247 — getFirstListItem(company = "${escaped}") returns the result (or rejects promise); the caller at line 245 wraps it in .catch(() => null) — but the path between line 240 and 247 doesn't catch.
• management-service.ts:232-236 — two .catch(() => null) chains on users lookups, OK.

A non-defensive call site bubbles a 500 to the route handler, which returns a generic {error: message} with the PB 404 stack trace included. This is loud in logs but confusing to the operator — they typed a real order number and got "404 not found" wrapped as a 500-class error.

Recommended fix: wrap once at the service boundary with a firstOrNull<T>(collection, filter) helper. Every service should call firstOrNull and let the route handler decide whether "no rows matched" is a 404, 422, or a null field in a successful response. This is the same pattern findOrderByOrderNumber uses at orders-service.ts:2573-2588 — apply it everywhere.

2.3 getAdminPocketBase everywhere bypasses PB listRules / field-level perms

File: every service-layer file (43 import sites)

The admin PB instance has superuser rights — every listRule, viewRule, and modifyRule is bypassed. This means:

• Data classification doesn't apply. A future "sensitive customer" field added to customers with viewRule: "" (no public read) would still be readable through every v1 route, the MCP route, and the legacy helpers.
• Field-level visibility doesn't apply. Same shape: a salary_band field on users with a restrictive viewRule would be returned by every service that expands users.
• Audit trail is lost. Superuser actions log as _superusers in PB's audit log, not as the operator who actually made the call. The getServerIdentity(req) helper (dev-identity.ts:42-58) is used to record the operator on specific fields, but the PB audit row still says _superusers.

This is a known design decision, not a bug — the rationale (server scripts, n8n, AI agents that can't carry the user's JWT) is documented in management-service.ts:1-25. The risk is that the rationale becomes invisible as more routes get added.

Recommended fix: add a comment to pocketbase-admin.ts:62-72 warning that the admin client bypasses ALL PB rules, and require a JSDoc on every new service file that explains why admin is required. For browser-facing reads (customers/list/route.ts:30), keep using assertBrowserSession + the cookie PB (or migrate to the user-PB instance). The assertAgentAuth / assertBrowserSession split is correct as designed — the issue is when a third auth tier creeps in (e.g. an assertServiceAuth for cron jobs) and the proof of which tier a route uses gets buried in the route body.

2.4 N+1 patterns in service-layer loops

File: src/lib/server/orders-service.ts:2174 (per-item patch loop inside standalone order create) File: src/lib/server/orders-service.ts:2503-2506 (cancelOrder per-item item_status: "cancelled" update loop) File: src/lib/server/quotes-service.ts (multiple per-item update loops) File: src/lib/server/buyer-tasks-service.ts:115-167 (per-tab fetch × N tabs)

Examples:

// orders-service.ts:2174 (standalone order create flow)
for (let i = 0; i < itemsResult.created.length; i++) {
  const created = itemsResult.created[i];
  const original = input.items[i];
  if (!original) continue;
  const patch: Record<string, unknown> = {};
  if (original.sku !== undefined) patch.sku = original.sku;
  if (original.unit !== undefined) patch.unit = original.unit;
  if (original.supplier_id !== undefined) patch.supplier = original.supplier_id || null;
  if (original.notes !== undefined) patch.notes = original.notes;
  if (Object.keys(patch).length > 0) {
    try {
      await pb.collection(orderItemsCollection).update(created.id, patch);
    } catch (err) {
      // ... per-row failure recorded, loop continues
    }
  }
}

createOrderItems returns N rows; this loop fires N PB round-trips to patch the secondary fields (sku, unit, supplier_id, notes). At the WFDASH fleet scale (≤30 items per order) it's tolerable, but the helper comment immediately above the loop admits this. Better shape: pass the optional secondary fields into createOrderItems as a parallel array, so the helper writes them in the same create call. PB supports fields on create.

cancelOrder (orders-service.ts:2503-2506, the for (const item of toCancel) { await pb.collection(...).update(item.id, { item_status: "cancelled" }); } block) is the same shape: one PB write per cancelled item. The surrounding cancelOrdersBulk (orders-service.ts:2523-2535) is a separate loop at the order level — it iterates orderIds and calls cancelOrder per order; the per-item fan-out lives inside cancelOrder itself.

buyer-tasks-service.ts:115 opens with Promise.all-batched fetches for supplementary data, but the surrounding loops are per-item on the response side.

Recommended fix: before each loop, ask "can I do this with one PB call?" Specific wins:

• orders-service.ts:2174 — fold secondary fields into the createOrderItems call so we don't need a follow-up patch.
• cancelOrder — collect all eligible items and use PB's batch create endpoint (or a single update with a different verb). PB 0.22+ supports transactional batch via pb.send().
• buyer-tasks-service.ts — the per-tab fan-out is intentional and correct; the per-item post-processing inside each tab is the N+1 target.

2.5 No transactional writes across collections

File: every service that mutates >1 PB row in one operation

A representative example: pb-helpers.ts:1264-1287 (createOrderFromQuote) creates the orders row, then loops order_items creates. If the network drops between the first order_items.create and the last, the order exists with N items where N is whatever the network reached. The transaction equivalent would be PB's pb.send() with a chain of ops, or a manual compensating-rollback helper.

The current mitigation is "log loudly and let the operator clean up manually". This is fine at WFDASH fleet scale, but the pattern scales badly — and the OrderNumberCollisionError flow at orders-service.ts:2145-2198 shows the cost of catching a partial- failure state and rolling forward.

Recommended fix: two changes, in priority order:

1. Document the partial-success semantics in each multi-write service's JSDoc, with an explicit list of which steps are idempotent (safe to re-run) and which aren't. Today this knowledge lives in operator tribal memory.
2. For the highest-value multi-writes (order creation, PO creation, delivery note creation, quote split), add a try/catch that deletes / rolls-back the partial state on failure. The PB SDK's pb.send() batch endpoint supports this natively.

2.6 MCP route handler is a single 7077-line file

File: src/app/api/mcp/route.ts:1-7077

Everything — the CORS helper, JSON-RPC envelope, upstream-fetch helper, schema validation, 50+ tool definitions, 50+ tool handlers — lives in one file. Tool definitions alone occupy lines 270-1500 (~1230 lines of tools: [{ name: ..., inputSchema: ...}]). Tool handlers are interleaved with definitions rather than grouped, so changing one tool requires scanning hundreds of lines.

This is a known single-file pattern (the existing audit covers it), but the data-layer perspective adds:

• No way to mock individual tools for tests. The MCP route has tests at route.test.ts and po-agent-workflow.test.ts, but both test the route shape end-to-end. Service-layer splits would let each tool be tested in isolation (the underlying services already have unit tests).
• Tool input schemas are written by hand instead of using the same zod schema as the route handlers (see 2.1). The MCP layer could share schemas via zod-to-json-schema and the v1 routes.
• Adding a new tool is a 30-minute scroll-and-edit. A tools/<name>.ts per-tool structure would make git diff readable and reduce merge conflicts when multiple devs add tools.

Recommended fix: extract each tool's inputSchema and handler into src/lib/server/mcp-tools/<name>.ts. The tools array becomes Object.entries(MCP_TOOLS).map(...). JSON-RPC dispatch stays in route.ts. Risk: low — the file is a leaf in the import graph.

2.7 Schema-missing fallback pattern duplicated 4× with subtle drift

File: src/lib/pb-helpers.ts:128-185 (getQuotes) File: src/lib/pb-helpers.ts:204-228 (getQuoteAttachments) File: src/lib/pb-helpers.ts:230-258 (getQuoteStatusCounts) File: src/lib/pb-helpers.ts:188-202 (getTemplates / getFollowUpDueQuotes)

All four use the same is400(err) + warnSchemaMissing pattern:

function is400(err: unknown): boolean { ... }
let warnedSchemaMissing = false;
function warnSchemaMissing(what: string) {
  if (warnedSchemaMissing) return;  // <- only warns ONCE for the whole process
  warnedSchemaMissing = true;
  console.warn(`[pb-helpers] ... (${what}). Import pb_schema to enable the new filter.`);
}

Three issues:

1. warnedSchemaMissing is a module-level boolean, so the warn fires once per process. After the first fallback, the operator never sees another "field missing" warning even if a different collection is missing a different field. The "what" argument is silently dropped.
2. is400 only checks err.status === 400, but PB can also return 400 with a different error shape (the new PB versions embed a JSON data payload). The check misses anything that isn't a literal {status: 400} object.
3. The fallback filters may compile but return wrong results. getQuotes drops the is_template != true clause on fallback (line 175), so a quote marked as template silently appears in the New tab. getQuoteStatusCounts keeps the per-status queries but loses the templates-tab exclusion (line 263).

The 2026-07-22 PB schema import project (mentioned in AGENTS.md) is the path that resolves this; until then, the fallbacks should at minimum warn with the what argument.

Recommended fix: replace the module-level latch with a Set<string> that tracks warned fields, and surface the warning to a UI diagnostic (a banner on the operator dashboard, not a console.warn). If a route's fallback returns wrong data, that should surface in QA tests — write tests that assert "templates don't appear in the New tab when is_template is missing".

2.8 Service-layer error envelopes vary between routes

File: every src/app/api/v1/**/route.ts that returns JSON

Three observed shapes:

// Shape A — most routes
{ error: "message" }

// Shape B — quote pdf route
{ error: "PDF render failed", detail: "..." }

// Shape C — quote pdf validation
{ error: "items not ready", code: "items_not_ready", details: { itemIds: [...], issues: [...] } }

// Shape D — MCP route (JSON-RPC)
{ jsonrpc: "2.0", id: ..., error: { code: -32010, message: "...", data: {...} } }

The route handler picks whichever envelope the original author chose, with no consistent shape across endpoints. The MCP layer's JSON-RPC envelope is correct per spec but uses error codes that don't map to HTTP status (the agent has to learn the mapping from the hint strings in TOOL_HINTS at mcp/route.ts:271-450).

The v1 route handlers also return inconsistent HTTP statuses for similar failures:

• customers/route.ts:78 returns 400 with {error: "customer is required"}.
• customers/[id]/route.ts:48 returns 400 with {error: "JSON object body required"}.
• quotes/[id]/pdf/route.ts:165-168 returns 422 with a structured body.
• quote-items/route.ts (need to read) returns 500 on validation failures (no dedicated check).

Recommended fix: standardise on:

{ error: { code: "string_enum", message: "human-readable", details?: object } }

with a top-level helper err(code, message, status, details?). The MCP layer keeps its JSON-RPC envelope (it's a different protocol) but maps to the same codes internally.

2.9 Search hard-caps silently truncate results

File: src/lib/pb-helpers.ts:1066 (SEARCH_HARD_CAP = 500) File: src/lib/server/orders-service.ts:1075 (search caps in service layer, separate value) File: src/lib/pb-helpers.ts:431-453 (status counts — 22 parallel queries)

SEARCH_HARD_CAP = 500 caps searchOrdersWithItems at 500 results. Past 500, the result is silently truncated — no warning to the operator that there are more matches.

The status counts (getQuoteStatusCounts at pb-helpers.ts:230) fires 22 parallel getList(1, 1, ...) queries, one per PB status. With no quotes.status index on the live PB, each query is a full scan over ~8.5k quotes = ~2.6s wall time per the inline comment. The service-layer replacement (quotes-service.ts) collapses this to 1 scan.

Recommended fix: when a search hits the hard cap, return { items, truncated: true, totalApprox: "≥ 500" } so the UI can render "showing 500 of 500+, refine your query". Migrate the status counts through the service layer's collapsed-counts implementation.

2.10 pb-helpers.ts duplicate-detection post-filter is fine but verbose

File: src/lib/pb-helpers.ts:1889-1953 (findNameMatches, findCompanyDuplicates, etc.)

PB's ~ operator does case-insensitive substring match; for exact case-insensitive equality, the helper does:

const result = await pb.collection(collectionName).getList<T>(1, limit, {
  filter: `${field} ~ "${escapeDup(trimmed)}"`,
});
return result.items.filter((row) => normDup(row[field]) === target);

This works at WFDASH scale but has two limitations:

• (field) ~ "value" is still substring, so a search for "Acme" returns "Acme Limited" AND "New Acme Holdings" both. The post-filter narrows to exact, but the wire payload already included the unwanted rows.
• The helper calls pb.collection(...).getList rather than the service-layer wrapper, so the auth context drifts (cookie PB vs admin PB depending on caller).

Recommended fix: migrate findCompanyDuplicates / findSupplierDuplicates / findCustomerDuplicates into management-service.ts so they use admin PB (consistent with the CRUD paths), and use PB's = with a lower() equivalent. PB 0.26 doesn't have lower(), so the post-filter is unavoidable, but the narrowed PB filter should be the strict substring with ~, not a substring that includes everything.

2.11 Counter last_seq+ increments — concurrent-edge case

File: src/lib/server/orders-service.ts:1413-1426 (getOrCreateOrderCounter) File: src/lib/server/quotes-service.ts:930-944 (mirror for quotes)

try {
  return await pb.collection(ORDER_COUNTERS_COLLECTION)
    .getFirstListItem(`year=${year} && month=${month}`);
} catch {
  // First request for this (year, month) — backfill from existing orders,
  // then create. Without backfill the new code would start at ORD-...-001
  // and collide with the existing sequence.
  const backfill = await getMaxOrderSequenceInMonth(pb, year, month);
  try {
    return await pb.collection(ORDER_COUNTERS_COLLECTION).create({
      year, month, last_seq: backfill,
    });
  } catch {
    // Concurrent caller beat us to it — re-fetch.
    return await pb.collection(ORDER_COUNTERS_COLLECTION)
      .getFirstListItem(`year=${year} && month=${month}`);
  }
}

The third try (line 1421-1425) catches the concurrent-beat-us case but doesn't catch the underlying unique-constraint error inside the .catch. If the unique constraint fails AND the re-fetch also fails, the original error is swallowed.

More importantly: getMaxOrderSequenceInMonth does a getFullList({ filter: order_number ~ "..." }), which scans every order in the month. On a busy month (or for the orders of the month, not just the current one), this is the slow path of an otherwise O(1) counter increment.

Recommended fix: the third try should re-throw the original error if the re-fetch fails. The slow-path backfill can be a one- shot job run by ops, not a per-create step.

2.12 PO number generator getOrCreate lookup lacks fallback for race

File: src/lib/server/po-number.ts:1-... (mirror of pb-helpers.ts:1799-1864)

The server-side PO number generator lives at src/lib/server/po-number.ts (matching the deprecated client-side generatePONumber at pb-helpers.ts:1799-1864). Per the inline comment at pb-helpers.ts:1745-1752, they should mirror field-by-field.

Two issues:

1. The client-side helper at pb-helpers.ts:1799-1864 post-filters the existing POs on the suffix (line 1840-1848). The server-side helper (read separately) presumably uses a more correct index hit. Verify the formats actually match — a divergence here means previews in the email composer don't match the persisted PO number.

2. Both helpers look up "all POs in the (year, month)" via getList(1, 500) and filter client-side. For a busy month this pulls 500 records per PO generation. At the WFDASH fleet scale it's tolerable but wasteful.

Recommended fix: read both side-by-side and assert the formats match byte-for-byte. Add a comment in po-number.ts pinning the contract: "client-side preview at pb-helpers.ts:1799-1864 MUST produce the same string".

---

3. Minor findings

3.1 deliveryNoteOnly option on getQuotes is fragile

File: src/lib/pb-helpers.ts:213-247 (getQuotes options parameter)

The options: { deliveryNoteOnly?: boolean } shape is the only optional options object on the legacy helper. Adding a second boolean later would require a breaking change to the type. Use a discriminated options object instead — getQuotes(page, perPage, search, sort, statuses, options).

3.2 Cookie PB singleton persists auth across logout

File: src/lib/pocketbase.ts:23-46

When the user clicks logout, the operator's PB token is removed from localStorage (presumably — verify in the logout UI), but the singleton pb instance in pocketbase.ts:9 is module-scoped and not cleared. The next getPocketBase() returns the same instance with the stale auth.

Recommended fix: add an exported clearPocketBaseAuth() that resets the singleton and clears the localStorage entry. Call it from the logout handler.

3.3 assertBrowserSession is duplicated inline in quotes/items/rewrite/route.ts

File: src/app/api/v1/quotes/items/rewrite/route.ts:99-122 File: src/lib/server/browser-session.ts:1-30

The rewrite route has its own assertBrowserSession instead of importing the shared one. The two implementations are identical (fetch ${pbUrl}/api/collections/users/auth-refresh with a 5s timeout). Drift risk if one is updated and the other isn't.

Recommended fix: import from @/lib/server/browser-session.

3.4 isPocketBaseId regex is too lax (downgraded — minor)

File: src/lib/server/management-service.ts:253-257

function isPocketBaseId(s: string): boolean {
  // PocketBase ids are 15 alphanumeric chars by default. We don't try
  // to be stricter than that — false positives just fall through to
  // the name lookup which is harmless.
  return /^[a-z0-9]{15}$/i.test(s);
}

PB's default id format is 15 alphanumeric chars, but admins can configure custom id formats. The regex matches any 15-char alphanumeric string (e.g. "customerCustomer1234" — not a real PB id). However, the actual call sites (resolveCompanyRef / resolveUserRef at management-service.ts:206-237) wrap the getOne in try/catch and explicitly fall through to the name lookup on any failure (literal comment: // Fall through to name lookup. / // Fall through.). So a 15-char string that doesn't resolve as an id correctly triggers the name-lookup fallback, and a string that does resolve as an id returns the real record.

The remaining (narrow) risk is a false positive that happens to collide with an unrelated record's real 15-char PB id — i.e. the operator types a company name like "abc123def456ghi" and it silently shadows a different real company. Probability is low (15-char alphanumeric is a ~10^27 space at 15 chars), but not zero, and the symptom is silent mis-routing. This is minor, not major.

Severity: Minor (downgraded from a prior draft that incorrectly described the fallback as missing — the fallback IS present).

Recommended fix: tighten the regex when PB exposes a stable id shape (e.g. include the pre_ / custom-prefix configuration), or require a prefix hint on user-supplied input. For the time being, add a unit test that asserts both the fast-path and the fallback paths resolve to the same id when given an ambiguous 15-char input.

3.5 PB file proxy is publicly readable (per code comment)

File: src/app/api/v1/quotes/[id]/pdf/route.ts:694-697

// PB's `/api/files/...` endpoint is publicly readable for these
// records (no Authorization header required — the file collection
// rule allows it). Stream the bytes through so the iframe can
// render without exposing the storage URL directly.

The comment admits that PB's per-file rule allows anonymous reads. The PDF route streams the bytes back through Next so the iframe gets the data via the same-origin response, but anyone with the file token can hit ${PB}/api/files/quote_attachments/${id}/${name} directly. The security-perf-prod.md audit covers this.

Recommended fix (data-layer specific): add a PB rule that gates /api/files/quote_attachments/* on a valid auth token. Today, the application relies on the file token being unguessable — PB's token format is randomsuffixed so it's hard to enumerate, but the attachment-id IS in the URL.

3.6 Schema-mismatch silently degrades quote_attachments direction

File: src/lib/pb-helpers.ts:540-585 (getQuoteAttachments)

The direction field on quote_attachments was added in a 2026-06-23 migration. The fallback at pb-helpers.ts:570-583 returns the historical "all attachments" view when the field doesn't exist. This means after the migration, an operator who hasn't run it sees all attachments (including outbound and generated), which changes the "Original Email" card on the quote detail page. Document this in the migration guide.

3.7 csv cell encoding is naive

File: src/lib/pb-helpers.ts:1914-1929 (exportToCSV)

The CSV encoder only escapes commas and double quotes, not newlines. A description with \n (rare in the codebase, but possible from copy/paste from a Word doc) breaks the CSV row.

Recommended fix: also escape \n and \r (CRLF inside a quoted cell is allowed by RFC 4180, but only if the cell is quoted; the current code only quotes when commas or double quotes are present).

3.8 console.error count is 37 across 165+ routes

File: console.error and console.warn callsites across src/app/api/v1/**

A grep -c 'console.error\|console.warn\|console.log' src/app/api/v1/**/*.ts shows 37. Most routes either log nothing or log a single error per failure. With structured logging missing (per the existing audit's 2.8), debugging a customer-reported failure today requires PB logs + Next logs + guessing.

Recommended fix: introduce a logError(scope, error, context) helper that adds the route path, request id, and container id to a single JSON line. Wrap every catch block with it. Even without a logger backend, the JSON-line format makes production logs grep-able.

3.9 searchOrdersWithItems SEARCH_HARD_CAP is in legacy only

File: src/lib/pb-helpers.ts:1066 vs src/lib/server/orders-service.ts:1075

The legacy cap is 500; the service-layer cap is presumably different (verify). Different caps means a page that migrated to the service shows a different count than a page that hasn't.

3.10 pb-helpers.ts getOrderItemStatusTabCounts is N round-trips

File: src/lib/pb-helpers.ts:660-693

Six getFullList calls in parallel (tabs.map(...)). Each scans order_items filtered by item_status — at WFDASH scale this is fast, but each tab also returns the full order relation string, even though the function only dedupes to a count. Add fields: "order" to narrow the wire payload (this is the same fix the service layer applies).

---

4. Production-readiness checklist (data-layer specific)

• Schema validation: add zod, write parseBody(req, schema) helper, convert the 7 highest-risk write routes.
• Legacy deprecation: move generateQuoteNumber, generateOrderNumber, generatePONumber out of pb-helpers.ts exports; add a lint rule that bans non-legacy imports.
• Browser-singleton fix: getPocketBase throws on server-side. Service-layer is the only path.
• getFirstListItem abstraction: write firstOrNull<T>(collection, filter) and migrate the 31 call sites.
• Multi-write rollback: document partial-success semantics on the 4 multi-write services (orders, PO, DN, quote-split).
• MCP route refactor: extract each tool into lib/server/mcp-tools/<name>.ts.
• Search cap visibility: return truncated: true from any search that hits a hard cap.
• Schema-missing warnings: Set<string> per-field, surfaced to a UI diagnostic.
• Error envelope standardisation: { error: { code, message, details? } } shape across all v1 routes.
• isPocketBaseId tightening: verify both id and name lookups for ambiguous inputs.
• CSV export hardening: escape \n and \r.
• Logging: structured JSON logError(scope, error, context) helper.

5. What I deliberately did not flag

• Auth surface. Covered comprehensively in security-perf-prod.md §1.1-1.4, §2.1-2.2, §2.11. This audit stays data-focused.
• Frontend / UX issues. See frontend-ux.md.
• PB schema field disuse. See unused-pb-fields.md.
• Test coverage gaps. The service-layer files have *.test.ts companions for every business-critical module; the gaps are at the route-handler layer (most routes have no tests, e.g. agent/scan/route.ts). Coverage audit is a separate task.
• PDF rendering / @react-pdf / SVG data URIs. The PDF rendering foot-guns (raster corruption at byte 0x80) are covered in the PDF route JSDoc and don't affect the data layer.