From 7f477ca94996a906e0572a7f21e6c7e79f8d40c2 Mon Sep 17 00:00:00 2001 From: SJCoetz Date: Thu, 6 Aug 2026 13:13:57 +0200 Subject: [PATCH] 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 | 15 +++ src/app/api/mcp/FEATURE-REQUESTS.md | 124 ++++++++++++++++++ src/app/api/mcp/route.ts | 3 +- src/lib/server/buyer-tasks-service.test.ts | 76 +++++++++++ src/lib/server/buyer-tasks-service.ts | 35 ++++- ...order-item-margin-approval-service.test.ts | 48 ++++++- .../order-item-margin-approval-service.ts | 55 ++++---- 7 files changed, 324 insertions(+), 32 deletions(-) create mode 100644 .remember/remember.md create mode 100644 src/app/api/mcp/FEATURE-REQUESTS.md diff --git a/.remember/remember.md b/.remember/remember.md new file mode 100644 index 0000000..8d5d635 --- /dev/null +++ b/.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 ` — 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. \ No newline at end of file diff --git a/src/app/api/mcp/FEATURE-REQUESTS.md b/src/app/api/mcp/FEATURE-REQUESTS.md new file mode 100644 index 0000000..895cdce --- /dev/null +++ b/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 — +- **Workaround:** +- **Severity:** now | nice-to-have +- **Suggested tool:** +- **Schema needs:** +- **Open questions:** +``` + +## Open requests + +### Orders: filter by customer +- **Surfaced:** 2026-08-06 — "last 3 orders for Seth" +- **Workaround:** filter `order_items` by `order.customer = ""`, 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)_ diff --git a/src/app/api/mcp/route.ts b/src/app/api/mcp/route.ts index 31b732a..7c68cf5 100644 --- a/src/app/api/mcp/route.ts +++ b/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"], diff --git a/src/lib/server/buyer-tasks-service.test.ts b/src/lib/server/buyer-tasks-service.test.ts index 59c3d4b..400f923 100644 --- a/src/lib/server/buyer-tasks-service.test.ts +++ b/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([], { diff --git a/src/lib/server/buyer-tasks-service.ts b/src/lib/server/buyer-tasks-service.ts index 9a4fb91..87bfdac 100644 --- a/src/lib/server/buyer-tasks-service.ts +++ b/src/lib/server/buyer-tasks-service.ts @@ -95,6 +95,21 @@ export async function listBuyerTasks(tab: BuyerTaskTab): Promise> = []; let stockItemMap = new Map>(); if (tab === "po_sent") { @@ -103,11 +118,21 @@ export async function listBuyerTasks(tab: BuyerTaskTab): Promise 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(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 sum + it.quantity * it.cost_price, diff --git a/src/lib/server/order-item-margin-approval-service.test.ts b/src/lib/server/order-item-margin-approval-service.test.ts index 0b720ef..63bdd05 100644 --- a/src/lib/server/order-item-margin-approval-service.test.ts +++ b/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((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 () => { diff --git a/src/lib/server/order-item-margin-approval-service.ts b/src/lib/server/order-item-margin-approval-service.ts index 85c8f2f..94567d0 100644 --- a/src/lib/server/order-item-margin-approval-service.ts +++ b/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,