fix: resolve PO bug, add sku, improve webhooks

Add `sku` to the order_item.patch MCP tool whitelist and update its documentation
Fix stuck stock POs in buyer tasks po_sent tab by filtering out downstream statuses
Rewrite margin approval webhook to use fire-and-forget dispatch to avoid UI blocking, add server-side error logging
Add MCP feature requests and local setup documentation

.remember/remember.md

@@ -0,0 +1,15 @@
+# Handoff
+
+## State
+Wired up `wfdash-mcp` in `.mcp.json` pointing at `https://wfdash.apphub.co.za/api/mcp` with the user-provided `AGENT_API_KEY` (Bearer auth — matches `assertAgentAuth` in `src/lib/server/agent-auth.ts`). Verified `tools/list` returns 200 with the full catalog. `.mcp.json` is gitignored, so the key is safe.
+
+## Next
+1. User restarts Claude Code so the new MCP server actually loads (new `.mcp.json` entries only take effect at session start).
+2. User has "a couple of things" they want to try via MCP — none attempted yet. They prefer MCP over direct API calls because their Hermes agent keeps losing API keys.
+3. If any requested action isn't covered by existing tools in `src/app/api/mcp/route.ts`, register new tool handlers there.
+
+## Context
+- New MCP tools are added in `src/app/api/mcp/route.ts` (~3100 lines, JSON-RPC style, ~50+ tools already registered). `jsonRpcOk` helper centralises the response envelope.
+- Auth gate is `AGENT_API_KEY` env var compared against `Authorization: Bearer <key>` — exact string match, no hashing.
+- CORS is wide-open by design (per route comment) — Origin is not the gate, the key is.
+- User uses Hermes agents in n8n that hit this MCP — they previously had issues with API keys getting dropped, hence the MCP preference.

src/app/api/mcp/FEATURE-REQUESTS.md

@@ -0,0 +1,124 @@
+# MCP Feature Requests & Gaps
+
+Captures requests the existing WFDASH MCP tools can't fulfil, surfaced from real
+questions asked by agent operators / the agent itself. Each entry should be
+small enough to ship in a single PR.
+
+When one ships, move it to **Implemented** with a one-line commit/PR reference.
+
+## Entry template
+
+```
+### [SHORT TITLE]
+- **Surfaced:** YYYY-MM-DD — <quoted or paraphrased user question>
+- **Workaround:** <what we did in the meantime to answer the question>
+- **Severity:** now | nice-to-have
+- **Suggested tool:** <name + signature, e.g. `orders.stats({from, to})`>
+- **Schema needs:** <PB field additions / changes, or "none">
+- **Open questions:** <anything to clarify with ops before implementing>
+```
+
+## Open requests
+
+### Orders: filter by customer
+- **Surfaced:** 2026-08-06 — "last 3 orders for Seth"
+- **Workaround:** filter `order_items` by `order.customer = "<id>"`, dedupe to distinct orders
+- **Severity:** now — every "orders for customer X" question hits this
+- **Suggested tool:** add `customer` (id or name) + `from`/`to` filters to `orders.list`
+- **Schema needs:** none
+
+### Orders: date-range filter on `orders.list`
+- **Surfaced:** 2026-08-06 — "how many orders were placed yesterday"
+- **Workaround:** fall back to `order_items.list` with `created` filter, dedupe to distinct orders
+- **Severity:** now — every "yesterday / last week / this month" question hits this
+- **Suggested tool:** add `filter` (PB syntax) + `createdAfter` / `createdBefore` to `orders.list`
+- **Schema needs:** none
+
+### Orders: aggregate money value (count + selling total + cost total)
+- **Surfaced:** 2026-08-06 — "...give me the money value aswell"
+- **Workaround:** sum `selling_price × quantity` across items client-side
+- **Severity:** now — every count question also wants a value
+- **Suggested tool:** `orders.stats({from, to, groupBy?: 'day'|'customer'|'supplier'})` returning `{count, totalSell, totalCost}`
+- **Schema needs:** none
+
+### Order items: clarify "received" — no unambiguous field
+- **Surfaced:** 2026-08-06 — "how many orders were received yesterday"
+- **Workaround:** filtered by `qty_collected > 0 && updated` on target date — proxy, not exact
+- **Severity:** now — user had to disambiguate ("received" can mean received-from-supplier, collected-from-warehouse, or delivered-to-customer)
+- **Open questions:** ops must decide which definition to standardise on; current candidates are `drv_collected_at` (12 items/d yesterday) and `delivered_at` (0 items/d yesterday)
+- **Suggested tool:** once defined, add a `received_at` filter param to `order_items.list` and a `orders.received_between` helper
+- **Schema needs:** *probably* none if we pick an existing field; might need a new `received_at` if the chosen definition isn't already captured
+
+### Order items: detect items missing `selling_price`
+- **Surfaced:** 2026-08-06 — 4 of 42 items placed yesterday had `selling_price = 0` (still unpriced at end of day)
+- **Workaround:** filter `selling_price = 0 && created > X` and report
+- **Severity:** nice-to-have — these are unpriced orders that shouldn't ship without review
+- **Suggested tool:** `orders.unpriced_items({from, to, minAgeHours?: number})` returning `{orderId, itemId, description, createdAt, hoursUnpriced}`
+- **Schema needs:** none
+
+### Dashboard: single-call daily summary
+- **Surfaced:** 2026-08-06 — combining counts + values across multiple tools to answer one question
+- **Workaround:** 3–4 separate tool calls + client-side aggregation
+- **Severity:** nice-to-have
+- **Suggested tool:** `dashboard.daily_summary({date, includeReceived: bool})` returning a single payload with placed + received counts and totals
+- **Schema needs:** none
+
+### Order items: whitelist missing `sku` field
+- **Surfaced:** 2026-08-06 — "the items on PO-2026-08-006 add this sku for each item and save it" — every patch with `sku` rejected
+- **Workaround:** none — `sku` is a real PB field on `order_items`, but the route.ts whitelist (`ORDER_ITEM_PATCH_FIELDS`) omitted it. Locally fixed by adding `"sku"` to the array at `route.ts:145`; still pending deploy on `wfdash.apphub.co.za`
+- **Severity:** now — every supplier PO needs SKUs and the agent can't set them
+- **Suggested tool:** no new tool — just keep the whitelist in sync with the real PB schema. Better long-term: derive whitelist from the live PB schema on each call (or at boot) instead of a hardcoded array, so schema drift never silently blocks writes
+- **Schema needs:** none — PB field exists, the gap is MCP-only
+- **Open questions:** should the patch tool also reject unknown keys silently (current behaviour is a hard 4xx — fine), or warn-and-accept? User seems happy with hard rejects for now.
+
+### Orders: no create-from-scratch — only from a quote
+- **Surfaced:** 2026-08-06 — "can you create a order or load items as a order using mcp?"
+- **Workaround:** the only path is `quotes.*` → `orders.create_from_quote`. To create an order from scratch via MCP: (1) seed a quote via `quotes_create_from_email` or `quotes_create_from_template`, (2) add items with `quote_items_create` (or `bulk_update_quote_items`), (3) call `orders.create_from_quote`. There is NO `orders.create` and NO `order_items.create`.
+- **Severity:** nice-to-have — the quote-pipe works, but it's 3+ calls and forces the user to materialise a quote just to load an order
+- **Suggested tool:** `orders.create({customer, company, items: [{description, quantity, unit?, sku?, cost_price?, selling_price?}]})` returning the new order + items. (Pushes the same idea further: `order_items.create({order_id, ...})` if items need to be appended to an existing order.)
+- **Schema needs:** none — every field already exists on `orders` / `order_items`
+- **Open questions:** if we ship this, does `quotes.*` still matter as a destination, or do we collapse quote and order into "draft vs committed"? Currently the app forces the two-step dance; an opinionated answer here would also clean up the buyer flow.
+
+### Quote/order items: whitelist missing `category` field
+- **Surfaced:** 2026-08-06 — "can you see the category and ai insights? can you change the category"
+- **Workaround:** reads work fine (`category` is returned on every item + `expand=category` exposes the name/match_keywords). Writes are blocked — same shape as the `sku` whitelist gap: `QUOTE_ITEM_PATCH_FIELDS` and `ORDER_ITEM_PATCH_FIELDS` in `route.ts` don't list `category`, so any patch with it is rejected with `forbidden_field`. Until that whitelist gap is closed + deployed, the only way to recategorize is via PB admin UI. Audit found 2 items needing change: KYOCERA TIPS (Fasteners → Cutting Tools) and Bulldog Disc (Unknown → Fasteners).
+- **Severity:** now — recategorization is a real operator workflow and Hermes can't do it
+- **Suggested tool:** no new tool — fold into the same `route.ts` whitelist expansion that fixes `sku` (and consider regenerating the whitelists from the live PB schema so this stops happening). Optional companion tool: `categories.classify({description})` running the master-list keyword logic in `route.ts` so the AI agent doesn't have to memorise the 10 categories.
+- **Schema needs:** none
+- **Open questions:** none — same pattern as the `sku` entry above.
+
+### Stock purchases: no PDF → stock order ingestion
+- **Surfaced:** 2026-08-06 — "can you load a stock order from a pdf that i give you or items"
+- **Workaround:** `stock_purchases_create({supplier, items:[...]})` works for structured input but the operator has to type each line. No tool accepts a PDF or an email payload. Existing attachment uploads (`purchase_orders_add_attachment`, `quotes_list_attachments`) attach files *to existing records*; they don't parse them.
+- **Severity:** nice-to-have
+- **Suggested tool:** `stock_purchases.create_from_pdf({supplier, base64, content_type})` that parses supplier PO PDFs (using something like `pdf-parse` or `unpdf`) and emits a draft `stock_purchase` for review. Alternatively, `stock_purchases.create_from_email({from, subject, body, ...})` mirroring `quotes_create_from_email`.
+- **Schema needs:** none — the `stock_purchase` + `stock_purchase_item` collections already exist
+- **Open questions:** which PDF format(s) does the user want supported? Many SA suppliers send bespoke PO PDFs — a generic parser would need supplier-specific extraction rules. Probably needs an LLM-driven extraction step rather than layout heuristics. Worth scoping before implementation.
+
+### Drivers: no MCP visibility or assignment
+- **Surfaced:** 2026-08-06 — "can you see the driver stops and assign stops to drivers?"
+- **Workaround:** "driver" data lives only as three fields on `order_items` (`drv_id`, `drv_assigned_at`, `drv_collected_at`). There is no separate `drivers` collection, no MCP tool to list drivers, no tool to see today's stops for one driver, no tool to bulk-assign stops to a driver. Operators can call `orders.list` and manually click through. Effectively the driver flow is invisible to the agent.
+- **Severity:** now — driver dispatch is a daily workflow and currently bypasses the agent entirely
+- **Suggested tool:** `drivers.list()` returning `{id, name, currentLoad, maxStops}`. `drivers.assign_stops({drv_id, order_item_ids:[...]})` to bulk-assign. `drivers.stops({drv_id, date})` returning today's route. If `drivers` isn't a real PB collection, the simplest first cut is virtual (read drivers off `users` or `suppliers` where they probably already exist) — but that needs ops to confirm the canonical source.
+- **Schema needs:** possibly a new `drivers` collection if there isn't one already; otherwise a way to identify the driver relation (`drv_id` currently points to a freeform string, not a PB relation)
+- **Open questions:** where is the canonical drivers list today? `users`? `suppliers`? a CSV? "Assign" semantics — operator workflow or agent-controlled?
+
+### Suppliers: read/update works, geocoding does not
+- **Surfaced:** 2026-08-06 — "can you access suppliers and geocode them update the lat and long if needed ? and the address ?"
+- **Read:** ✅ `suppliers.list` / `suppliers.get` already return full records including `coordinates` (legacy "lat, lng" string) and `supplieraddress`
+- **Update:** ✅ `suppliers.update({id, patch:{supplieraddress, coordinates, ...}})` patches those fields today
+- **Geocode:** ❌ no MCP tool — there's no geocoding service exposed, so the agent can't *generate* the lat/lng from an address. Anyone wanting current coordinates has to call Google Maps / Nominatim externally and then patch the result via `suppliers.update`. `coordinates` is also still the legacy text "lat, lng" string on `suppliers` (it's pre-`geoPoint`) — even when the agent *has* lat/lng it has to know to format it as "lat,lng".
+- **Severity:** now — the user clearly wants the round-trip (read missing coords → geocode → write back) automated, not hand-driven
+- **Suggested tool:** `suppliers.geocode({id?, address?, limit?: 10})` calling a hosted geocoder (OpenStreetMap Nominatim is free and sufficient; Google would need a key). Returns `{lat, lng, formatted_address, confidence}` and optionally persists via `suppliers.update` if `id` is passed. Migrate `coordinates` to `geoPoint` while we're here so the round trip becomes a single field write.
+- **Schema needs:** migrate `suppliers.coordinates` from text to PB `geoPoint` (delete the field, recreate it as lat/lng pair). Backfill existing rows from the parsed "lat,lng" strings first.
+
+## Schema questions for ops
+
+These came up but need a human answer before code changes:
+
+- **"Received" semantics** — see `Order items: clarify "received"` above. Also, the `order_items_update` docstring references fields (`collection_date`, `collected_date`, `verified_date`, `delivered_date`) that don't exist in the current PB schema (only `drv_collected_at`, `delivered_at`, `assigned_at`, `drv_assigned_at` exist). Either the docstring is stale or those fields were planned and never created.
+- **Currency notes** — prices are stored as numbers but the `notes` field on items embeds "R" (ZAR) strings. Confirm the system is single-currency ZAR before adding any multi-currency stats.
+
+## Implemented
+
+_(move entries here with a PR / commit link when they ship)_

src/app/api/mcp/route.ts

@@ -143,6 +143,7 @@ const ORDER_ITEM_PATCH_FIELDS = [
   "cost_price",
   "selling_price",
   "quantity",
+  "sku",
   "markup",
   "markup_percent",
   "notes",
@@ -306,7 +307,7 @@ const tools = [
   {
     name: "order_items.update",
     description:
-      "Patch fields on an order_item for invoice reconciliation. Whitelist: cost_price, selling_price, quantity, markup, markup_percent, notes, qty_processed, qty_assigned, qty_collected, qty_verified, qty_stocked, qty_picked, qty_loaded, qty_delivered, qty_invoiced, qty_cancelled, supplier_invoice_number, customer_invoice_number, collection_date, collected_date, verified_date, delivered_date, invoiced_date. Use this to compare what the supplier invoiced (supplier_invoice_number + qty_invoiced + cost_price) against what was ordered (quantity) and what was collected (qty_collected), and patch the differences. Status-only updates go through `order_items.update_status` instead.",
+      "Patch fields on an order_item for invoice reconciliation. Whitelist: cost_price, selling_price, quantity, sku, markup, markup_percent, notes, qty_processed, qty_assigned, qty_collected, qty_verified, qty_stocked, qty_picked, qty_loaded, qty_delivered, qty_invoiced, qty_cancelled, supplier_invoice_number, customer_invoice_number, collection_date, collected_date, verified_date, delivered_date, invoiced_date. Use this to compare what the supplier invoiced (supplier_invoice_number + qty_invoiced + cost_price) against what was ordered (quantity) and what was collected (qty_collected), and patch the differences. Status-only updates go through `order_items.update_status` instead.",
     inputSchema: {
       type: "object",
       required: ["id", "patch"],

src/lib/server/buyer-tasks-service.test.ts

@@ -684,6 +684,82 @@ describe("listBuyerTasks", () => {
     expect(stock!.groupId).toMatch(/^stock::/);
   });
 
+  it("excludes stock items already in collection_pool from the po_sent group", async () => {
+    // The buyer marks stock lines ready via the buyer-tasks
+    // MarkReadyDialog, which transitions stock_purchase_items.status
+    // to "collection_pool". Mirrors the customer-order po_sent
+    // exclusion (item_status != "collection_pool"). Without this,
+    // every line stays visible on /buyer-tasks even after the
+    // supplier has confirmed pickup — operators reported this
+    // specific stock PO (PO26073109STK) as "stuck" because the card
+    // never moved off the po_sent tab.
+    const { admin, getFullList } = makeAdmin([], {
+      stockPurchases: [baseStockPurchase],
+      stockPurchaseItems: [
+        baseStockItem({ id: "stk-item-1", status: "collection_pool" }),
+        baseStockItem({ id: "stk-item-2", status: "" }), // still awaiting
+      ],
+    });
+    vi.mocked(getAdminPocketBase).mockResolvedValue(admin as never);
+
+    const groups = await listBuyerTasks("po_sent");
+
+    expect(groups).toHaveLength(1);
+    expect(groups[0].itemCount).toBe(1);
+    expect(groups[0].items[0].id).toBe("stk-item-2");
+
+    // The PB filter for stock_purchase_items must drop the
+    // collection_pool items on the wire so a stock PO with hundreds
+    // of lines doesn't ship every collection_pool row across the
+    // network just to filter them out client-side.
+    const stockItemCall = getFullList.mock.calls.find(
+      (c) => Array.isArray(c[0]?.filter) || typeof c[0]?.filter === "string",
+    );
+    expect(stockItemCall).toBeDefined();
+  });
+
+  it("excludes all other downstream stock item statuses (assigned_to_driver, collected, booked)", async () => {
+    // `stock_purchase_items.status` transitions past `collection_pool`
+    // through `assigned_to_driver` → `collected` → `booked`. None of
+    // these belong on the buyer's po_sent tab — once the driver owns
+    // the line, the buyer is done. When ALL items are downstream,
+    // the entire group is dropped (see the next test for the empty-
+    // group skip).
+    const { admin } = makeAdmin([], {
+      stockPurchases: [baseStockPurchase],
+      stockPurchaseItems: [
+        baseStockItem({ id: "stk-1", status: "assigned_to_driver" }),
+        baseStockItem({ id: "stk-2", status: "collected" }),
+        baseStockItem({ id: "stk-3", status: "booked" }),
+      ],
+    });
+    vi.mocked(getAdminPocketBase).mockResolvedValue(admin as never);
+
+    const groups = await listBuyerTasks("po_sent");
+
+    expect(groups).toHaveLength(0);
+  });
+
+  it("drops the entire stock group when every line is downstream", async () => {
+    // All lines already past collection_pool → the buyer has nothing
+    // left to do. The po_sent card must NOT keep surfacing this PO
+    // (this was the operator-reported "stuck" symptom on
+    // PO26073109STK).
+    const { admin } = makeAdmin([], {
+      stockPurchases: [baseStockPurchase],
+      stockPurchaseItems: [
+        baseStockItem({ id: "stk-1", status: "collection_pool" }),
+        baseStockItem({ id: "stk-2", status: "assigned_to_driver" }),
+        baseStockItem({ id: "stk-3", status: "collected" }),
+      ],
+    });
+    vi.mocked(getAdminPocketBase).mockResolvedValue(admin as never);
+
+    const groups = await listBuyerTasks("po_sent");
+
+    expect(groups).toHaveLength(0);
+  });
+
   it("truncates stock email_error to 200 chars (mirrors customer behaviour)", async () => {
     const longError = "[webhook] " + "x".repeat(300);
     const { admin } = makeAdmin([], {

src/lib/server/buyer-tasks-service.ts

@@ -95,6 +95,21 @@ export async function listBuyerTasks(tab: BuyerTaskTab): Promise<BuyerTaskGroup[
   // they don't have an unsent / draft-equivalent state in the
   // buyer-tasks framing (operator drafts stock POs directly from
   // /stock-orders/new, not from buyer-tasks).
+  //
+  // The stock-item filter excludes downstream statuses (collection_pool,
+  // assigned_to_driver, collected, booked) so once a supplier has
+  // confirmed pickup, the line leaves the buyer's po_sent tab. The
+  // mirror of the customer-order item_status exclusions a few lines
+  // up — without it, a fully-marked PO like PO26073109STK stays
+  // pinned on the list (operator reported this card "stuck" — the
+  // bug). Also: a stock PO with zero remaining items is dropped
+  // below so the parent doesn't surface an empty card.
+  const STOCK_ITEM_DOWNSTREAM = [
+    "collection_pool",
+    "assigned_to_driver",
+    "collected",
+    "booked",
+  ];
   let stockPurchases: Array<Record<string, unknown>> = [];
   let stockItemMap = new Map<string, Array<{ id: string; description: string; quantity: number; cost_price: number }>>();
   if (tab === "po_sent") {
@@ -103,11 +118,21 @@ export async function listBuyerTasks(tab: BuyerTaskTab): Promise<BuyerTaskGroup[
       expand: "supplier",
     });
     if (stockPurchases.length > 0) {
+      const parentFilter = stockPurchases.map((sp) => `purchase = "${sp.id}"`).join(" || ");
+      const statusExclusions = STOCK_ITEM_DOWNSTREAM.map((s) => `status != "${s}"`).join(" && ");
       const stockItems = await admin.collection("stock_purchase_items").getFullList({
-        filter: stockPurchases.map((sp) => `purchase = "${sp.id}"`).join(" || "),
+        filter: `(${parentFilter}) && ${statusExclusions}`,
       });
+      // Defence in depth: filter in JS too. The PB filter cuts the
+      // wire payload for real backends, but mocks and backends with
+      // listRule quirks don't always honour `fields=` / filter
+      // combinations the same way — the empty-group skip below is
+      // gated on `stockItems.length === 0`, so any item we leave in
+      // here still gets surfaced in the group.
+      const downstream = new Set<string>(STOCK_ITEM_DOWNSTREAM);
       for (const si of stockItems) {
-        const item = si as unknown as { id: string; purchase: string; description: string; quantity: number; cost_price: number };
+        const item = si as unknown as { id: string; purchase: string; description: string; quantity: number; cost_price: number; status?: string };
+        if (item.status && downstream.has(item.status)) continue;
         const list = stockItemMap.get(item.purchase) ?? [];
         list.push({
           id: item.id,
@@ -209,6 +234,12 @@ export async function listBuyerTasks(tab: BuyerTaskTab): Promise<BuyerTaskGroup[
     if (!supplierId) continue;
     const supplier = stock.expand?.supplier;
     const stockItems = stockItemMap.get(stock.id) ?? [];
+    // Skip stock POs whose lines have all moved downstream (every
+    // item is collection_pool/assigned_to_driver/collected/booked).
+    // The parent record still has status="sent", but the buyer has
+    // nothing left to do — surfacing an empty card would be the
+    // "stuck PO" symptom.
+    if (stockItems.length === 0) continue;
     const groupId = `stock::${stock.id}::${supplierId}`;
     const totalCost = stockItems.reduce(
       (sum, it) => sum + it.quantity * it.cost_price,

src/lib/server/order-item-margin-approval-service.test.ts

@@ -139,6 +139,12 @@ describe("order-item-margin-approval-service", () => {
     );
   });
   it("keeps the item blocked when the webhook fails", async () => {
+    // The webhook is now fire-and-forget — we don't propagate its
+    // status into the synchronous response (it would force the
+    // operator to wait for a notification roundtrip). The contract is:
+    // PB state is saved first, the item is blocked, the webhook is
+    // dispatched in the background, and server-side logs capture any
+    // failure. This test pins the "PB first" half of that contract.
     vi.mocked(fetch).mockResolvedValueOnce(new Response("bad gateway", { status: 502 }));
 
     const result = await requestOrderItemMarginApproval({
@@ -153,8 +159,46 @@ describe("order-item-margin-approval-service", () => {
       expect.objectContaining({ item_status: "approval_needed" }),
       { expand: "order,supplier" },
     );
-    expect(result.webhookFired).toBe(false);
-    expect(result.webhookError).toContain("HTTP 502");
+    expect(result.item).toEqual(
+      expect.objectContaining({ item_status: "approval_needed" }),
+    );
+    // Background dispatch always reports "fired" from the caller's
+    // perspective — the response shape is intentionally synchronous
+    // success so the UI toast can fire immediately.
+    expect(result.webhookFired).toBe(true);
+  });
+
+  it("does not block the response while the webhook is slow", async () => {
+    // Operators were paying the full 15s WEBHOOK_TIMEOUT_MS every time
+    // the n8n webhook lagged, because the service awaited the fetch
+    // before returning. PB state is already saved (item blocked) —
+    // the webhook is a best-effort notification, not a precondition
+    // for the toast. Dispatch it in the background and return
+    // immediately so the UI doesn't hang.
+    const SLOW = 5_000;
+    vi.mocked(fetch).mockImplementationOnce(
+      () => new Promise<Response>((resolve) => setTimeout(() => resolve(new Response(null, { status: 200 })), SLOW)),
+    );
+
+    const start = Date.now();
+    const result = await requestOrderItemMarginApproval({
+      itemId: "item-1",
+      source: "order_process",
+      requestedBy: "operator@example.com",
+      draft: { cost_price: 45, selling_price: 50 },
+    });
+    const elapsed = Date.now() - start;
+
+    // Far less than the 5s the fake webhook would take if we were
+    // awaiting it. We give some headroom for test jitter but the
+    // point is: this is not the 15s the operator used to see.
+    expect(elapsed).toBeLessThan(1_000);
+    expect(itemUpdate).toHaveBeenCalledWith(
+      "item-1",
+      expect.objectContaining({ item_status: "approval_needed" }),
+      { expand: "order,supplier" },
+    );
+    expect(result.webhookFired).toBe(true);
   });
 
   it("does not create an approval request at or above 22%", async () => {

src/lib/server/order-item-margin-approval-service.ts

@@ -272,34 +272,35 @@ export async function requestOrderItemMarginApproval(input: {
     };
   }
 
-  try {
-    const response = await fetch(webhookUrl, {
-      method: "POST",
-      headers: { "Content-Type": "application/json" },
-      body: JSON.stringify(webhookPayload),
-      signal: AbortSignal.timeout(WEBHOOK_TIMEOUT_MS),
+  // Fire-and-forget the webhook. PB state is already saved (item is
+  // blocked in `approval_needed`), so the operator's workflow is
+  // correct regardless of whether the notification gets through.
+  // Awaiting the fetch here used to make the operator wait the full
+  // `WEBHOOK_TIMEOUT_MS` (15s) every time n8n was slow — the toast
+  // shouldn't be gated on a notification roundtrip. Failures are
+  // logged server-side so ops still see them.
+  void fetch(webhookUrl, {
+    method: "POST",
+    headers: { "Content-Type": "application/json" },
+    body: JSON.stringify(webhookPayload),
+    signal: AbortSignal.timeout(WEBHOOK_TIMEOUT_MS),
+  })
+    .then((response) => {
+      if (!response.ok) {
+        console.warn(
+          "[margin-approval webhook] background dispatch failed: HTTP",
+          response.status,
+          { orderItemId: updated.id, source: input.source },
+        );
+      }
+    })
+    .catch((error) => {
+      console.warn(
+        "[margin-approval webhook] background dispatch failed:",
+        error instanceof Error ? error.message : error,
+        { orderItemId: updated.id, source: input.source },
+      );
     });
-    if (!response.ok) {
-      return {
-        item: updated,
-        markup,
-        threshold: ORDER_MARKUP_APPROVAL_THRESHOLD,
-        webhookFired: false,
-        webhookError: `Webhook returned HTTP ${response.status}`,
-        webhookPayload,
-      };
-    }
-  } catch (error) {
-    return {
-      item: updated,
-      markup,
-      threshold: ORDER_MARKUP_APPROVAL_THRESHOLD,
-      webhookFired: false,
-      webhookError:
-        error instanceof Error ? error.message : "Webhook request failed",
-      webhookPayload,
-    };
-  }
 
   return {
     item: updated,