From 594af9368b9408daa5e49dd847ac83f1a03be6a7 Mon Sep 17 00:00:00 2001 From: Jan Date: Wed, 10 Jun 2026 16:22:01 +0000 Subject: [PATCH 1/5] Implement product lifecycle supplier planning --- .beads/issues.jsonl | 60 ++++--- .../analytics/forecast/forecast-client.tsx | 31 +++- .../inventory-stats-client.tsx | 2 +- .../product-profitability-client.tsx | 3 +- .../sales-stats/sales-stats-client.tsx | 2 +- app/(dashboard)/inventory/[id]/page.tsx | 27 ++- .../inventory/inventory-header.tsx | 5 +- app/(dashboard)/inventory/page.tsx | 13 +- app/(dashboard)/inventory/product-filters.tsx | 25 ++- app/(dashboard)/purchase-orders/[id]/page.tsx | 2 +- app/(dashboard)/purchase-orders/page.tsx | 2 +- app/(dashboard)/purchase-orders/po-form.tsx | 18 ++ app/(dashboard)/sales/page.tsx | 4 +- app/(dashboard)/supplier/products/page.tsx | 6 +- app/actions/forecasting.ts | 73 +++++--- app/actions/import.ts | 44 ++++- app/actions/products.ts | 68 ++++++- app/actions/purchase-orders.ts | 37 ++-- .../cron/product-lifecycle-archive/route.ts | 20 +++ app/api/export/analytics/route.ts | 4 +- app/api/export/products/route.ts | 8 +- components/inventory/product-columns.ts | 2 + components/inventory/product-form.tsx | 43 ++++- components/inventory/product-table.tsx | 17 +- components/inventory/variant-generator.tsx | 6 +- docs/plan.md | 28 +-- lib/connectors/woocommerce/sync/stock-sync.ts | 4 +- .../inventory/product-lifecycle-archive.ts | 170 ++++++++++++++++++ lib/domain/inventory/replenishment-reports.ts | 23 ++- lib/domain/purchasing/preferred-supplier.ts | 53 ++++++ lib/products/lifecycle.ts | 31 ++-- lib/security/route-auth-policy.ts | 4 + lib/shopping.ts | 2 +- .../migration.sql | 72 ++++++++ prisma/schema.prisma | 11 +- .../product-lifecycle-archive.test.ts | 47 +++++ .../inventory/replenishment-reports.test.ts | 5 + .../purchasing/preferred-supplier.test.ts | 88 +++++++++ tests/products-lifecycle.test.ts | 49 +++++ 39 files changed, 976 insertions(+), 133 deletions(-) create mode 100644 app/api/cron/product-lifecycle-archive/route.ts create mode 100644 lib/domain/inventory/product-lifecycle-archive.ts create mode 100644 lib/domain/purchasing/preferred-supplier.ts create mode 100644 prisma/migrations/20260610152000_product_lifecycle_supplier_reorder/migration.sql create mode 100644 tests/domain/inventory/product-lifecycle-archive.test.ts create mode 100644 tests/domain/purchasing/preferred-supplier.test.ts create mode 100644 tests/products-lifecycle.test.ts diff --git a/.beads/issues.jsonl b/.beads/issues.jsonl index 2b2409df..ed16c491 100644 --- a/.beads/issues.jsonl +++ b/.beads/issues.jsonl @@ -1,44 +1,59 @@ -{"_type":"issue","id":"onetwo3d-ims-q6w","title":"Periodic AR/AP FX revaluation journals","description":"Open foreign-currency AR/AP needs reversible month-end revaluation. The realised settlement fix now books payment-date FX variances, but open receivables/payables still need an on-demand or cron-driven revaluation job that: computes outstanding foreign balances at the current rate, posts unrealised FX to configured AR/AP control and unrealised FX accounts, reverses prior revaluation before posting the next one, and is idempotent per valuation date.","acceptance_criteria":"Running the revaluation job twice for the same valuation date is idempotent; running it for a later date reverses the prior open-balance revaluation before posting the new one; open SalesOrder and PurchaseInvoice balances exclude settled amounts; base-currency documents are skipped; Xero/QuickBooks manual journal payloads are balanced and covered by tests.","notes":"Implemented periodic open AR/AP FX revaluation. Added UNREALISED_FX_JOURNAL sync type and connector settings for unrealised FX gain/loss accounts. Added /api/cron/accounting-fx-revaluation?date=YYYY-MM-DD, which skips duplicate valuation dates, reverses unreversed prior revaluation journals before posting a later date, excludes settled/base-currency documents, and queues balanced Xero/QuickBooks manual journal payloads via AccountingSyncLog idempotency keys.","status":"closed","priority":0,"issue_type":"bug","assignee":"Jan","owner":"info@onetwo3d.co.uk","created_at":"2026-04-25T12:58:23Z","created_by":"Jan","updated_at":"2026-04-25T13:12:19Z","started_at":"2026-04-25T13:04:19Z","closed_at":"2026-04-25T13:12:19Z","close_reason":"Fixed by adding idempotent cron-driven AR/AP FX revaluation with prior-valuation reversals, open-balance filtering, and balanced unrealised FX journal queueing.","labels":["accounting","fx"],"dependencies":[{"issue_id":"onetwo3d-ims-q6w","depends_on_id":"onetwo3d-ims-ahu","type":"discovered-from","created_at":"2026-04-25T12:58:22Z","created_by":"OneTwo3D IMS","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-q6w","title":"Periodic AR/AP FX revaluation journals","description":"Open foreign-currency AR/AP needs reversible month-end revaluation. The realised settlement fix now books payment-date FX variances, but open receivables/payables still need an on-demand or cron-driven revaluation job that: computes outstanding foreign balances at the current rate, posts unrealised FX to configured AR/AP control and unrealised FX accounts, reverses prior revaluation before posting the next one, and is idempotent per valuation date.","acceptance_criteria":"Running the revaluation job twice for the same valuation date is idempotent; running it for a later date reverses the prior open-balance revaluation before posting the new one; open SalesOrder and PurchaseInvoice balances exclude settled amounts; base-currency documents are skipped; Xero/QuickBooks manual journal payloads are balanced and covered by tests.","notes":"Implemented periodic open AR/AP FX revaluation. Added UNREALISED_FX_JOURNAL sync type and connector settings for unrealised FX gain/loss accounts. Added /api/cron/accounting-fx-revaluation?date=YYYY-MM-DD, which skips duplicate valuation dates, reverses unreversed prior revaluation journals before posting a later date, excludes settled/base-currency documents, and queues balanced Xero/QuickBooks manual journal payloads via AccountingSyncLog idempotency keys.","status":"closed","priority":0,"issue_type":"bug","assignee":"Jan","owner":"info@onetwo3d.co.uk","created_at":"2026-04-25T12:58:23Z","created_by":"Jan","updated_at":"2026-04-25T13:12:19Z","started_at":"2026-04-25T13:04:19Z","closed_at":"2026-04-25T13:12:19Z","close_reason":"Fixed by adding idempotent cron-driven AR/AP FX revaluation with prior-valuation reversals, open-balance filtering, and balanced unrealised FX journal queueing.","labels":["accounting","fx"],"dependencies":[{"issue_id":"onetwo3d-ims-q6w","depends_on_id":"onetwo3d-ims-ahu","type":"discovered-from","created_at":"2026-04-25T12:58:22Z","created_by":"Jan","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-e99","title":"Bill can be paid before goods received; bill qty can exceed received qty","description":"markBillPaid does not check that the corresponding goods have been received. createInvoice only validates qtyBilled \u003c= poQty, not qtyBilled \u003c= qtyReceived. Combined, you can pay for stock that has not arrived.\n\nRefs:\n- app/actions/purchase-orders.ts:2620-2720 (markBillPaid: no RECEIVED gate)\n- app/actions/purchase-orders.ts:2520-2535 (createInvoice: validates against PO qty, not received qty)\n\nVERIFY before fixing: this may be intentional if the business supports supplier prepayments / deposits. If not, gate both actions on qtyReceived \u003e= qtyBilled.","status":"closed","priority":0,"issue_type":"bug","assignee":"OneTwo3D IMS","owner":"dev@onetwo3d.com","created_at":"2026-04-25T09:55:52Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T10:31:50Z","started_at":"2026-04-25T10:31:50Z","closed_at":"2026-04-25T10:31:50Z","close_reason":"Closing as needs product input. Adding a hard 'qtyReceived gates qtyBilled and markBillPaid' rule would force users into one workflow. Some businesses legitimately pay supplier deposits / prepayments before goods arrive. The codebase has no 'deposit' / 'prepay' concept (grep returns 0), so it currently relies on operator discipline. Decide product-side: (a) gate strictly, (b) allow with a confirmation modal, or (c) add explicit 'deposit/prepay' workflow. None of these are pure code fixes.","labels":["code-review","p2p","state-machine","verify"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-hyz","title":"Per-line VAT vs order-level discount conversion produces wrong base-currency net","description":"Discount net-extraction divides by a single order-level vatPct, but VAT can vary per line. Mixed-rate carts produce inverted or incorrect discounts that flow into Xero GL splits.\n\nRefs:\n- lib/sales-currency.ts:22-27, :35 (no rounding rule; uses single vatPct)\n- app/actions/purchase-orders.ts:940-944 (same pattern on the buy side)\n\nFix direction: distribute the discount per line proportional to line value, then extract VAT per-line. Add explicit Math.round at the boundary to avoid sub-penny drift downstream.","notes":"Verified: claim holds up. lib/sales-currency.ts:18-41 uses single order-level vatPct to extract net from VAT-inclusive discount. Mixed-rate orders produce wrong line/order discount-base for dashboard, profitability, and stats reports.\n\nSchema: SalesOrderLine has taxRateId → TaxRate (with rateValue percent). Fix shape:\n1. Change normalizeLineDiscountBase signature to accept lineTaxRatePercent, fall back to order vatPct if undefined.\n2. For VAT-inclusive orders, use the line's own rate to extract net.\n3. For order-level discount: distribute proportional to line-gross × lineTaxRate, then sum nets.\n4. Update 6 call sites: lib/connectors/xero/daily-sync.ts:60, lib/connectors/quickbooks/daily-sync.ts:60, app/actions/dashboard.ts:233-234, app/actions/product-profitability.ts:154+162, app/actions/sales-stats.ts:83+103.\n\nRequires finance team validation that the new distribution method matches their reporting expectations. Likely affects historical aggregate queries — back-fill considerations.","status":"open","priority":0,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-04-25T09:55:52Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T10:31:53Z","labels":["code-review","fx","vat"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-wim","title":"Inconsistent rounding precision (4dp / 6dp / 2dp) across money paths","description":"Same monetary value is rounded to different precisions depending on which file touches it. Cumulative drift across thousands of lines.\n\nRefs:\n- app/actions/purchase-orders.ts:280-283 (4dp foreign total)\n- app/actions/purchase-orders.ts:946-950 (6dp unit cost)\n- lib/connectors/xero/daily-sync.ts:50 (2dp via round2())\n\nFix direction: introduce a single MONEY_PRECISION constant per currency and a helper (roundMoney(amount, currency)). All boundary conversions go through it.","status":"closed","priority":0,"issue_type":"bug","assignee":"OneTwo3D IMS","owner":"dev@onetwo3d.com","created_at":"2026-04-25T09:55:52Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T10:31:52Z","started_at":"2026-04-25T10:31:51Z","closed_at":"2026-04-25T10:31:52Z","close_reason":"Closing as known limitation, no concrete loss observed. Mixed precision (4dp foreign total, 6dp unit cost, 2dp daily-sync) is intentional: 6dp unit cost preserves accuracy for small-margin products, 4dp at line/order level is what Xero accepts, 2dp at daily-batch matches GL precision. The cumulative drift the agent worried about (~0.0001 GBP per line) only matters at scale where rounding policy should be unified. Open a focused refactor task with a roundMoney(amount, currency) helper if drift becomes measurable in reconciliation; until then, no bug.","labels":["code-review","fx","money-math"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-3i0","title":"COGS recognition deferred to nightly Xero batch — null cost-layer snapshots until Group B","description":"ShipmentLine.costLayerSnapshot is only populated by the daily Xero Group B batch. Until that runs, interim P\u0026L and margin reports are wrong. A failed batch leaves COGS unrecognized indefinitely. Manual SHIPPED transitions (outside the allocation flow) crash the next batch run.\n\nRefs:\n- app/actions/allocation.ts:1089 (confirmAllocations creates ShipmentLine without snapshot)\n- lib/connectors/xero/daily-sync.ts:124-126 (Group B throws if snapshot is null)\n- lib/connectors/xero/daily-sync.ts:950 (snapshot only populated here, hours later)\n\nFix direction: take the snapshot at the SHIPPED transition, not in the batch. Make the batch idempotent against pre-existing snapshots.","status":"closed","priority":0,"issue_type":"bug","assignee":"OneTwo3D IMS","owner":"dev@onetwo3d.com","created_at":"2026-04-25T09:55:51Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T10:31:49Z","started_at":"2026-04-25T10:31:49Z","closed_at":"2026-04-25T10:31:49Z","close_reason":"Verified: not a bug. costLayerSnapshot IS populated on ShipmentLine at the SHIPPED transition (allocation.ts:1341-1350) — the agent confused snapshot *consumption* by the daily batch with snapshot *creation*. Snapshots are written synchronously during transitionShipmentStatus (allocation.ts:1277), inside the same tx as the FIFO consumption + StockLevel decrement. The daily-sync.ts:124-126 throw is a defensive guard that fires only if a shipment was somehow marked SHIPPED outside the standard path — and there is no such path in the codebase (grep confirms allocation.ts is the only writer of status:SHIPPED on Shipment).","labels":["code-review","cogs","o2c","sync-xero"],"dependency_count":0,"dependent_count":0,"comment_count":0} -{"_type":"issue","id":"onetwo3d-ims-ahu","title":"No realised FX gain/loss on payment, no AR/AP revaluation","description":"Pay a EUR bill weeks later at a different rate — settlement variance is silently dropped. Sales invoices in non-base currency are never revalued. No grep hits for 'realised', 'fx gain', 'fx loss' anywhere in the codebase.\n\nImpact: P\u0026L missing FX gains/losses for every multi-currency settlement; AR/AP balances drift away from underlying foreign value over time; month-end reconciliation to Xero will fail.\n\nRefs:\n- app/actions/sales.ts:2690-2800 (payment matching is foreign-amount-only)\n- app/actions/purchase-orders.ts:2620-2720 (markBillPaid)\n\nFix direction: at payment, compute (foreign × rate-now) - (foreign × rate-on-invoice), book to FX gain/loss account. Add a periodic revaluation job for open AR/AP.","notes":"Verified: claim holds up. Grep for 'realised', 'fx gain', 'fx loss' returns 0 hits in the codebase; payment matching at sales.ts:2690-2800 and markBillPaid at purchase-orders.ts:2620-2720 reconcile by foreign-currency amount only, never compute the GBP settlement variance.\n\nTwo distinct features needed:\n1. Realised FX on payment: when a foreign bill/invoice is settled at a different rate than booked, post the difference to a P\u0026L FX gain/loss account.\n2. AR/AP revaluation: at month-end (or on demand), revalue open foreign-currency AR and AP to current rate, post unrealised FX gain/loss to a B/S contra account, reverse on the next revaluation.\n\nBoth depend on the FX policy decision in onetwo3d-ims-a8u — the booking rate must be stable to compute the variance. Schedule together.\nRe-checked 2026-04-25 during a COGS/Tax/FX workflow analysis. The implementation appears to have landed since this issue was filed:\n\n- lib/accounting-fx.ts:32-105 computes realised FX gain/loss on payment settlement (threshold 0.01 base; AP/AR control vs FX gain/loss leg).\n- lib/accounting-fx-revaluation.ts + /api/cron/accounting-fx-revaluation handle daily unrealised FX revaluation of open AR/AP, with prior-day reversal.\n- lib/connectors/xero/settings.ts exposes xero_realised_fx_gain_loss_account / xero_unrealised_fx_gain_loss_account and the xero_sync_realised_fx_journal / xero_sync_unrealised_fx_journal toggles.\n\nThe original 'grep returns 0 hits' verification is no longer accurate. Recommend either closing this with a link to the implementing commit or narrowing scope to whatever (if anything) is still missing — e.g. payment matching at sales.ts:2690-2800 / purchase-orders.ts:2620-2720 should be re-verified to confirm it now invokes the realised FX path.","status":"open","priority":0,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-04-25T09:55:51Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T20:48:07Z","labels":["accounting","code-review","fx"],"dependencies":[{"issue_id":"onetwo3d-ims-ahu","depends_on_id":"onetwo3d-ims-a8u","type":"blocks","created_at":"2026-04-25T12:38:45Z","created_by":"OneTwo3D IMS","metadata":"{}"}],"dependency_count":1,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-ahu","title":"No realised FX gain/loss on payment, no AR/AP revaluation","description":"Pay a EUR bill weeks later at a different rate — settlement variance is silently dropped. Sales invoices in non-base currency are never revalued. No grep hits for 'realised', 'fx gain', 'fx loss' anywhere in the codebase.\n\nImpact: P\u0026L missing FX gains/losses for every multi-currency settlement; AR/AP balances drift away from underlying foreign value over time; month-end reconciliation to Xero will fail.\n\nRefs:\n- app/actions/sales.ts:2690-2800 (payment matching is foreign-amount-only)\n- app/actions/purchase-orders.ts:2620-2720 (markBillPaid)\n\nFix direction: at payment, compute (foreign × rate-now) - (foreign × rate-on-invoice), book to FX gain/loss account. Add a periodic revaluation job for open AR/AP.","notes":"Verified: claim holds up. Grep for 'realised', 'fx gain', 'fx loss' returns 0 hits in the codebase; payment matching at sales.ts:2690-2800 and markBillPaid at purchase-orders.ts:2620-2720 reconcile by foreign-currency amount only, never compute the GBP settlement variance.\n\nTwo distinct features needed:\n1. Realised FX on payment: when a foreign bill/invoice is settled at a different rate than booked, post the difference to a P\u0026L FX gain/loss account.\n2. AR/AP revaluation: at month-end (or on demand), revalue open foreign-currency AR and AP to current rate, post unrealised FX gain/loss to a B/S contra account, reverse on the next revaluation.\n\nBoth depend on the FX policy decision in onetwo3d-ims-a8u — the booking rate must be stable to compute the variance. Schedule together.\nRe-checked 2026-04-25 during a COGS/Tax/FX workflow analysis. The implementation appears to have landed since this issue was filed:\n\n- lib/accounting-fx.ts:32-105 computes realised FX gain/loss on payment settlement (threshold 0.01 base; AP/AR control vs FX gain/loss leg).\n- lib/accounting-fx-revaluation.ts + /api/cron/accounting-fx-revaluation handle daily unrealised FX revaluation of open AR/AP, with prior-day reversal.\n- lib/connectors/xero/settings.ts exposes xero_realised_fx_gain_loss_account / xero_unrealised_fx_gain_loss_account and the xero_sync_realised_fx_journal / xero_sync_unrealised_fx_journal toggles.\n\nThe original 'grep returns 0 hits' verification is no longer accurate. Recommend either closing this with a link to the implementing commit or narrowing scope to whatever (if anything) is still missing — e.g. payment matching at sales.ts:2690-2800 / purchase-orders.ts:2620-2720 should be re-verified to confirm it now invokes the realised FX path.","status":"open","priority":0,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-04-25T09:55:51Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T20:48:07Z","labels":["accounting","code-review","fx"],"dependencies":[{"issue_id":"onetwo3d-ims-ahu","depends_on_id":"onetwo3d-ims-a8u","type":"blocks","created_at":"2026-04-25T12:38:45Z","created_by":"Jan","metadata":"{}"}],"dependency_count":1,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-a8u","title":"FIFO cost layer not reconciled with supplier invoice (FX/landed cost)","description":"Cost layers are created with the PO line's base-cost at receipt, but the bill can arrive later with a different FX rate or unit cost. There is no reconciliation, so the FIFO layer (and therefore COGS) is permanently in a different base currency than AP.\n\nRefs:\n- app/actions/purchase-orders.ts:1827-1837 (layer create on receipt uses unitCostBase from PO line)\n- app/actions/purchase-orders.ts:2398-2401 (invoice FX rate looked up at invoice date, not PO date)\n- app/actions/purchase-orders.ts:3360-3370 (recalculateDirectLandedCosts uses primary PO unitCostBase, ignores invoice FX)\n- app/actions/purchase-orders.ts:2157-2158 (returns reverse PO-line cost, not billed cost)\n\nImpact: AP/inventory mismatch in base currency; balance sheet does not tie out for foreign-currency suppliers; landed-cost allocations are skewed when invoice FX != PO FX.\n\nDecision needed: pick a single FX policy (PO-rate, receipt-rate, or invoice-rate for the layer; book the FX delta as gain/loss) and retrofit. See related issues for AR/AP revaluation and realised FX on payment.","notes":"Verified: claim holds up. Cost layer cost basis (purchase-orders.ts:1827-1837 receipt path) uses the PO line's unitCostBase (computed at PO creation time from the PO's fxRateToBase). Invoice creation (purchase-orders.ts:2398-2401) re-resolves FX rate at invoice date. The two are never reconciled, so AP and inventory base-currency totals diverge for foreign-currency suppliers when PO date and invoice date differ.\n\nDecision needed (finance team):\n- Policy A: lock FX at PO issuance (current FIFO behaviour). Invoice rate unused for valuation; FX delta booked at invoice as gain/loss.\n- Policy B: lock FX at receipt (current FIFO behaviour matches if PO rate == receipt-day rate); invoice rate booked separately to AP, FX delta at receipt vs invoice booked as gain/loss.\n- Policy C: lock FX at invoice; FIFO layer gets retroactively corrected when bill arrives (most accurate, hardest to implement — requires re-walking COGS for shipments that consumed the layer between receipt and invoice).\n\nRecommend Policy A for simplicity unless finance team is doing accrual revaluation. Pair with onetwo3d-ims-ahu (realised FX gain/loss) for a complete fix.","status":"open","priority":0,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-04-25T09:55:50Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T10:31:53Z","labels":["code-review","cogs","fx","p2p"],"dependency_count":0,"dependent_count":1,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-6w6","title":"Implement CI invariant gate","description":"Finish docs/plan.md group 20 by adding an invariant-check preflight command to CI, failing on critical findings, documenting remediation, and adding focused regression coverage.","status":"closed","priority":1,"issue_type":"task","assignee":"Jan","owner":"info@onetwo3d.co.uk","created_at":"2026-06-10T13:49:04Z","created_by":"Jan","updated_at":"2026-06-10T14:08:28Z","started_at":"2026-06-10T13:49:14Z","closed_at":"2026-06-10T14:08:28Z","close_reason":"Implemented invariant preflight command, CI production-readiness job, remediation documentation, and focused regression tests; added group 21 product lifecycle plan item as requested.","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-ecp","title":"Guest checkouts (externalCustomerId=0) bypass the customer-link check on the shopping invoice PDF route (PR #154)","description":"PR #154's `findShoppingInvoicePdfOrder` (app/api/shopping/[connector]/invoice-pdf/route.ts) verifies the customer link only when externalCustomerId is set AND non-zero:\n\n```ts\nif (request.externalCustomerId \u0026\u0026 request.externalCustomerId !== '0' \u0026\u0026 order.customerId) {\n const customerLink = await db.shoppingCustomerLink.findFirst({\n where: { connector, externalCustomerId, customerId: order.customerId },\n select: { id: true },\n })\n if (!customerLink) return null\n}\nreturn { orderId, invoiceNumber, invoicePdfPath }\n```\n\nThe WP plugin's payload includes `externalCustomerId: (string) $order-\u003eget_customer_id()`. For **guest checkouts**, WooCommerce sets `customer_id = 0`. So the WP plugin sends `externalCustomerId: '0'` in the HMAC payload.\n\nThe IMS route's check `request.externalCustomerId !== '0'` SKIPS the customer-link verification entirely for guest checkout payloads. After the skip, the function returns the order's PDF metadata unconditionally — anyone with a valid HMAC signature for that order ID can fetch the guest-checkout invoice.\n\n**Bypass:**\n\n1. Attacker controls a WordPress site with the OTI helper plugin and a copy of the shared HMAC secret (e.g., a partner store, a compromised plugin install, or a leaked secret from any of the per-tenant deployments using the same secret).\n2. Attacker enumerates / guesses guest order IDs (often sequential, even when 'random' the search space is bounded).\n3. Attacker constructs HMAC-signed payload: `{connector: 'woocommerce', externalOrderId: '\u003cguess\u003e', externalCustomerId: '0', issuedAt, expiresAt, nonce}`.\n4. POST to /api/shopping/woocommerce/invoice-pdf with valid X-OTI-Signature.\n5. IMS verifies signature (✓), parses payload (✓), connector matches (✓), TTL (✓), then looks up order. If the order exists AND was a guest checkout (customer_id = 0), the customer-link skip fires and the PDF is returned.\n\nThe check that GATES this exists for member orders (`customerLink` lookup). Guest orders are silently exempted.\n\n**Why this matters:** Invoice PDFs contain customer name, billing address, line items with prices, payment method — PII. Guest checkout users have NO authenticated channel; the customer-link verification is the only authorization for member orders. Skipping it for guests means ZERO authorization on guest invoices beyond 'know the order ID + have the HMAC secret'.\n\n**Why the check exists at all:** the original intent was to prevent logged-in customer A from accessing customer B's invoice. For guest orders there's no customer to scope against, but the right answer isn't to skip the check — it's to require some other proof of order ownership (email match, order key, etc.).","acceptance_criteria":"- For guest checkouts (externalCustomerId == '0' or missing), require a SECOND proof-of-ownership token from the WP plugin (e.g., WooCommerce order key, billing email hash, recent order session marker)\n- Alternative: refuse guest-checkout invoice PDF requests via this route entirely; route guests to a different flow (e.g., email-only delivery)\n- Add a test fixture for the guest case: assert that externalCustomerId='0' WITHOUT proof of ownership returns 404\n- Audit the WP plugin's `oti_can_download_invoice_pdf`: it already requires wp_verify_nonce for the WP-local request, but the IMS-side check is what stops cross-site abuse","notes":"Filed during PR #154 review. The WP plugin's nonce check prevents same-site cross-user abuse — but a cross-site attacker with the HMAC secret bypasses WP entirely and calls IMS directly. Highest priority of the PR's findings because it's a real, currently-exploitable bypass.","status":"open","priority":1,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-10T11:40:17Z","created_by":"Jan","updated_at":"2026-06-10T11:40:17Z","labels":["code-review","pr-154","security","shopping"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-v6z","title":"Connection-test fingerprint includes raw secrets — DB-read attacker can brute-force passwords (PR #152)","description":"PR #152's `buildIntegrationConnectionFingerprint` is called with raw secrets baked into the input:\n\n```ts\n// app/actions/company.ts - SMTP\nbuildIntegrationConnectionFingerprint({\n host, port, user,\n pass: storedPassword, // \u003c-- plaintext password\n secure, fromName, fromEmail, replyTo,\n})\n\n// app/actions/wc-sync.ts - WooCommerce \nbuildWcConnectionFingerprint({\n url, key,\n secret: input.secret.trim(), // \u003c-- consumer secret\n})\n\n// app/actions/mintsoft-sync.ts - Mintsoft\nbuildMintsoftConnectionFingerprint({\n baseUrl, username,\n password: input.password.trim(), // \u003c-- plaintext password\n orderLookupConnector,\n})\n```\n\nThe resulting SHA256 hex is then **stored in the `setting` table** under `{id}_connection_test_fingerprint`.\n\n**Why this matters:**\n\nA user / attacker with READ access to the `setting` table (admin auditor, partial DB dump, backup compromise) cannot directly reverse the SHA256. But they CAN brute-force the secret offline:\n\n1. Read `wc_url` (plaintext), `wc_consumer_key` (plaintext), `woocommerce_connection_test_fingerprint` (hash).\n2. The hash is `SHA256(JSON({url, key, secret}))` — all inputs except `secret` are visible.\n3. Iterate candidate secrets (offline, fast — billions per second on commodity GPUs), compute the hash, compare.\n4. For a 32-character random WooCommerce consumer secret, this is computationally infeasible.\n5. For an SMTP password chosen by a human (8-16 chars, common patterns), this is feasible with consumer hardware in hours/days.\n\nSame applies to Mintsoft passwords.\n\nThis is the same attack class as 'stop using fast hashes for password storage' — SHA256 is the wrong primitive for hashing user-chosen secrets.\n\n**Mitigations:**\n\n- **Don't include the secret in the fingerprint at all** — fingerprint changes when other fields change. The fingerprint's purpose is to detect 'settings have changed'; including the secret is one way to detect a rotated secret, but you could ALSO detect rotation by storing a stable secret-version identifier (e.g., a UUID generated on secret-write) and including that in the fingerprint instead of the secret itself.\n- **OR hash the secret separately with HMAC + server-side key** — `hash = SHA256(JSON({url, key, HMAC_serverKey(secret)}))`. DB-read attacker cannot precompute candidates without the server key (which lives in env, not DB).\n- **OR use a slow KDF** (Argon2id, bcrypt, scrypt) for the secret portion — makes offline brute-force impractical.","acceptance_criteria":"- Choose between: (a) drop secret from fingerprint input + use secret-version UUID, (b) HMAC with server key, (c) Argon2id on secret portion\n- Update buildIntegrationConnectionFingerprint to apply the chosen pattern across SMTP, WC, Mintsoft (and any future caller)\n- Add a test asserting the same secret reproduces the same fingerprint, AND a fingerprint comparison cannot be brute-forced (effectively: assert HMAC key is needed, OR slow-KDF is applied)\n- Consider a migration to re-fingerprint existing connection-test records once the new scheme lands (otherwise existing records' status flips to 'fingerprint mismatch' and operators must retest)","notes":"Filed during PR #152 review. Highest priority of the findings — secret leakage from fingerprint, even if difficult, is a tangible security risk. Mintsoft and SMTP passwords are the highest concern because they're often human-chosen.","status":"open","priority":1,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-10T09:48:57Z","created_by":"Jan","updated_at":"2026-06-10T09:48:57Z","labels":["code-review","integrations","pr-152","security"],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-6la","title":"Implement cron rate limits and Xero batch cap","description":"Implement docs/plan.md suggested group 15: P6.1 cron endpoint rate limiting and P7.6 Xero accounting daily batch size cap with tests and plan status updates.","status":"closed","priority":1,"issue_type":"task","assignee":"Jan","owner":"info@onetwo3d.co.uk","created_at":"2026-06-09T17:57:33Z","created_by":"Jan","updated_at":"2026-06-09T18:07:11Z","started_at":"2026-06-09T17:57:42Z","closed_at":"2026-06-09T18:07:11Z","close_reason":"Implemented group 15: cron routes now enforce authenticated per-job rate limiting with schedule-compatible quotas; Xero daily batch groups process bounded limit+1 windows with deterministic split-batch references and hasMore metadata; docs/env/plan updated; focused tests, npm run validate, and npm run validate:db pass.","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-8vp","title":"Backup manifest critical-tables list is 4 tables; FIFO/COGS/accounting tables missing (PR #148)","description":"PR #148's `lib/backup-manifest.ts` defines critical tables for backup-integrity validation:\n\n```ts\nexport const CRITICAL_BACKUP_TABLES = [\n 'users',\n 'products',\n 'sales_orders',\n 'purchase_orders',\n] as const\n```\n\nThe IMS schema has 59+ Prisma models. The 4-table critical list catches the most user-visible tables but MISSES the tables whose absence would silently destroy financial state:\n\n- **`cost_layers`** — FIFO costing depends entirely on these rows. A restore with empty cost_layers but populated sales/purchases means COGS becomes uncomputable; refunds posts wrong amounts; analytics show zero margin.\n- **`cogs_entries`** — audit trail for consumed FIFO layers. Required for refund reversal correctness.\n- **`stock_levels`** — denormalized stock-on-hand. Recoverable from movements but expensive.\n- **`stock_movements`** — every inventory event. Without these, FIFO is unreplayable.\n- **`accounting_sync_logs`** — Xero outbox. Restoring without these double-posts every prior journal on next sync.\n- **`activity_logs`** — audit / compliance trail.\n- **`payments`** — AR/AP settlement records.\n- **`invoices`** / **`shipments`** / **`order_allocations`**\n\n**Why this matters:** A restore from a backup that's missing `cost_layers` passes manifest validation today, but produces an inventory system with no FIFO history. The PR's stated purpose ('critical-table backup manifests') is satisfied superficially but the choice of 4 tables doesn't match what 'critical' means for IMS-class data.\n\nThe 4 chosen tables (`users`, `products`, `sales_orders`, `purchase_orders`) are PRESENCE-of-business-data signals. Missing tables that break accounting/inventory are different signals — schema integrity vs business-data presence.","acceptance_criteria":"- Expand CRITICAL_BACKUP_TABLES to include the FIFO/COGS/accounting-critical tables listed above\n- Add an integration test that creates a backup, deletes all rows from cost_layers (or similar), and asserts the manifest validation rejects on restore\n- Document in the manifest's JSDoc what 'critical' means — is it 'should not be empty' or 'must exist in schema'?\n- Consider dynamic critical-table list: compute from Prisma metadata (tables with FK refs, or @@map'd to known financial models)","notes":"Filed during PR #148 review. The 4-table list reads as an MVP starting point; production-readiness requires the financial-state tables to be in scope.","status":"open","priority":1,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-09T15:58:21Z","created_by":"Jan","updated_at":"2026-06-09T15:58:21Z","labels":["accounting","backup","code-review","fifo","pr-148"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-fpr","title":"Backup manifest validation only fires on stored-backup restores, not uploaded restores (PR #148)","description":"PR #148 adds manifest validation in `createBackupRestorePostHandler` (`app/api/backup/restore/route.ts`) but only on the STORED-backup branch:\n\n```ts\n// Stored-backup path (line ~605):\nmanifest = await resolvedDeps.validateBackupManifest(restorePath)\nif (manifest.backupFilename !== path.basename(restorePath)) {\n return NextResponse.json({ error: 'Backup manifest does not match the selected backup.' }, { status: 400 })\n}\n\n// Upload path: no manifest validation. The uploaded file goes straight to runRestore.\n```\n\nAn admin uploading a hand-crafted SQL file via the multipart upload bypasses:\n- The critical-table presence check (`validateCriticalBackupTables`)\n- The schema version check\n- The row-count integrity signal\n\nThis is the same admin trust model as the rest of the system, but the PR's claim is that the manifest improves restore safety — for uploaded backups (the most common 'restore from external source' workflow), it provides zero new protection.\n\n**Why this matters:** An admin who accidentally uploads a partial or corrupt SQL dump (e.g., a dev-env dump missing several tables, or a dump from a different IMS instance with different schema) restores it with no integrity check. The manifest's stated purpose — 'critical-table backup manifests for generated/restored backups' — is half-implemented.\n\nThe upload path also can't easily check the manifest because manifests are sidecar files that live next to local backups. For uploaded restores, the manifest would need to be uploaded alongside (multi-file form) or embedded in the SQL header.","acceptance_criteria":"- Decide policy:\n - **Option A (preferred)**: require manifest upload alongside SQL file in the multipart form for uploaded restores\n - **Option B**: embed manifest as a SQL comment block at the start of the dump (`-- BACKUP MANIFEST: {json}`); read and validate during upload\n - **Option C**: skip manifest for uploads but require operator to explicitly check 'I acknowledge no integrity validation' UI flag\n- Update tests to assert the chosen behavior for uploaded restores\n- Update docs/plan.md P7.7 'Backup manifest' to reflect the chosen scope (uploaded vs stored)","notes":"Filed during PR #148 review. The PR addresses the stored-backup path well but the upload path is where most operator-driven restore-from-remote workflows go. The security claim is stronger than the implementation provides.","status":"open","priority":1,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-09T15:58:01Z","created_by":"Jan","updated_at":"2026-06-09T15:58:01Z","labels":["backup","code-review","pr-148","security"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-2it","title":"Invoice PDF Content-Disposition filename now uses random nonce instead of invoice id (PR #146)","description":"PR #146 changes the invoice PDF download filename from the invoice id to the token's nonce:\n\n```diff\n- 'Content-Disposition': `inline; filename=\"invoice-${safeInvoiceFilenameId(id)}.pdf\"`,\n+ 'Content-Disposition': `inline; filename=\"invoice-${safeInvoiceFilenameId(verification.payload.nonce)}.pdf\"`,\n```\n\n(`app/api/invoices/[id]/route.ts` around line 165 of the diff)\n\nThe nonce is `randomBytes(16).toString('base64url')` — a 22-character random string like `aB3xY7zP9_4kQrmTuVwXyZ`.\n\nUser impact: instead of downloading `invoice-INV-2025-001.pdf`, the user gets `invoice-aB3xY7zP9_4kQrmTuVwXyZ.pdf`. Files in the downloads folder have random-looking names that don't match the invoice they represent.\n\n**Why this matters:**\n1. **UX regression** — users can't identify invoice files at a glance; finance teams downloading dozens of invoices can't filter/sort by invoice number.\n2. **No security benefit** — the URL path `/api/invoices/{id}?token=...` already exposes the invoice id; the user clicked a link knowing what invoice they wanted. Hiding the id in the filename adds zero security.\n3. **Likely accidental** — the change isn't mentioned in the PR description's bullet points. The other PDF changes (token binding, TTL) are explicit; this filename change reads as collateral damage from the same edit.","acceptance_criteria":"- Revert the Content-Disposition filename to use `safeInvoiceFilenameId(id)` (the invoice id), not the nonce\n- Add a test that asserts the Content-Disposition header includes the invoice id, not the nonce\n- If there was a deliberate reason to use the nonce (e.g., preventing concurrent downloads from clashing), document it explicitly in a code comment AND update the PR body","notes":"Filed during PR #146 review. Highest priority of the findings because it's a likely unintentional UX regression that ships out of an otherwise good security PR.","status":"open","priority":1,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-08T23:07:42Z","created_by":"Jan","updated_at":"2026-06-08T23:07:42Z","labels":["code-review","pr-146","security","ux"],"dependency_count":0,"dependent_count":0,"comment_count":0} -{"_type":"issue","id":"onetwo3d-ims-xin","title":"WC tax fallback block-policy allows fallback to zero-rate even when destination should be taxed (PR #144)","description":"PR #144's `shouldBlockWcTaxRateFallback()` (lib/connectors/woocommerce/sync/order-import.ts) blocks WooCommerce imports only when the fallback tax rate value is \u003e 0:\n\n```ts\nexport function shouldBlockWcTaxRateFallback(lines: WcTaxRateFallbackLine[]): boolean {\n return lines.some((line) =\u003e line.taxRateValue \u003e 0)\n}\n```\n\nThe original bd issue `onetwo3d-ims-xx4` (now closed) was about tax-rate fallback being silent on WC import. The fix correctly blocks imports when the fallback uses a non-zero default rate. But the inverse case is still silent:\n\n**Scenario:** A WC order ships to Germany (DE), 19% standard VAT. The DE/STANDARD tax rate exists in IMS but the WC tax rate is unmapped. The IMS resolver falls back through (country+category → country+STANDARD → global+category → order default). If the order default is a 0% rate (e.g., for a tenant that has a misconfigured zero-rated default), the resolver returns rate 0 — and `shouldBlockWcTaxRateFallback` does NOT block because `taxRateValue === 0`.\n\nResult: a 19% DE order is imported with 0% tax. Understated VAT, silent.\n\n**Why this matters:** This is the same class of bug as the original `xx4`: silent tax misconfiguration produces wrong VAT. The fix only addresses the over-rated direction.","acceptance_criteria":"- Add a second blocking condition: when the destination country has any active non-zero tax rate configured for the product category and the resolver fell back to a different (zero or otherwise non-matching) rate, block the import\n- Alternatively: require that the resolver's matched rate ID actually matches the destination jurisdiction (countryCode == destCountry) — block on any fallback regardless of rate value\n- Add a focused WC import test for the zero-rated fallback case where the destination should have been non-zero\n- Update the Sales Settings fallback widget to also surface zero-rate fallback events","notes":"Identified during PR #144 review. The PR addressed bd onetwo3d-ims-xx4 in the over-rated direction but the under-rated direction (silent zero-rate import for a non-zero-rated jurisdiction) is the more dangerous failure mode because under-reported VAT is harder to detect than over-reported VAT.","status":"open","priority":1,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-07T23:36:43Z","created_by":"Jan","updated_at":"2026-06-07T23:36:43Z","labels":["code-review","connectors","pr-144","vat"],"dependency_count":0,"dependent_count":0,"comment_count":0} -{"_type":"issue","id":"onetwo3d-ims-xjj","title":"Tax snapshot refresh has a read-then-write race against status transitions (PR #144)","description":"PR #144 adds `refreshMutableDocumentTaxSnapshotsForRate()` (lib/tax/document-tax-snapshot-refresh.ts) which is called transactionally from `updateTaxRate()` when a TaxRate is edited. The function:\n\n1. `findMany` for SalesOrderLines whose `order.status IN [DRAFT, PENDING_PAYMENT]`.\n2. Loops through each line and calls `salesOrderLine.update()` plus header `salesOrder.update()` to apply the new rate.\n\nThe findMany result is materialized at read time. If a SalesOrder transitions from DRAFT → PROCESSING (or PENDING_PAYMENT → PROCESSING) between the findMany and the update, the function silently rewrites tax fields on an order that should now be immutable.\n\nThe transaction surrounding it doesn't take a SELECT FOR UPDATE on the orders, so concurrent writers can sneak in.\n\nSame gap exists in `refreshMutablePurchaseOrderTaxSnapshots` (status set `[DRAFT, RFQ_SENT, QUOTE_RECEIVED]`).\n\n**Why this matters:** Tax rate edits typically happen during operator setup, but a user shipping an order at the same moment as another user editing the tax rate can cause the shipped order's persisted tax to silently change. Hard to detect post-hoc.","acceptance_criteria":"- Either (a) add SELECT FOR UPDATE on the SalesOrder/PurchaseOrder rows before re-checking status and applying updates, or (b) re-check the order status inside the update by using a conditional update (`update where: { id, status: { in: MUTABLE_STATUSES } }`) and counting affected rows\n- Add a concurrency test that exercises the race: start the tax refresh transaction, transition the order to PROCESSING from another connection, assert no update lands on the now-immutable order\n- Document the chosen safety mechanism in lib/tax/document-tax-snapshot-refresh.ts","notes":"Identified during PR #144 review. The risk is bounded by how rarely operators edit a TaxRate concurrently with order processing, but the failure mode is silent — tax fields are denormalized onto orders/lines, so a stale-write here changes accounting numbers without an audit trail beyond the tax_rate_snapshot_refresh activity log entry (which doesn't include which lines were touched).","status":"open","priority":1,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-07T23:36:24Z","created_by":"Jan","updated_at":"2026-06-07T23:36:24Z","labels":["code-review","concurrency","pr-144","tax"],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-6p5","title":"Tighten account balance freshness plan group","description":"Verified docs/plan.md suggested groups 1 and 2 are already implemented, marked their plan items complete, and implemented group 12/P5.3 by requiring a previous-day opening GL balance snapshot by default.","status":"closed","priority":1,"issue_type":"task","assignee":"Jan","owner":"info@onetwo3d.co.uk","created_at":"2026-06-08T21:52:04Z","created_by":"Jan","updated_at":"2026-06-08T21:57:22Z","started_at":"2026-06-08T21:52:11Z","closed_at":"2026-06-08T21:57:22Z","close_reason":"Implemented P5.3 account-balance freshness and updated docs/plan.md completion status for verified completed groups.","dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-xin","title":"WC tax fallback block-policy allows fallback to zero-rate even when destination should be taxed (PR #144)","description":"PR #144's `shouldBlockWcTaxRateFallback()` (lib/connectors/woocommerce/sync/order-import.ts) blocks WooCommerce imports only when the fallback tax rate value is \u003e 0:\n\n```ts\nexport function shouldBlockWcTaxRateFallback(lines: WcTaxRateFallbackLine[]): boolean {\n return lines.some((line) =\u003e line.taxRateValue \u003e 0)\n}\n```\n\nThe original bd issue `onetwo3d-ims-xx4` (now closed) was about tax-rate fallback being silent on WC import. The fix correctly blocks imports when the fallback uses a non-zero default rate. But the inverse case is still silent:\n\n**Scenario:** A WC order ships to Germany (DE), 19% standard VAT. The DE/STANDARD tax rate exists in IMS but the WC tax rate is unmapped. The IMS resolver falls back through (country+category → country+STANDARD → global+category → order default). If the order default is a 0% rate (e.g., for a tenant that has a misconfigured zero-rated default), the resolver returns rate 0 — and `shouldBlockWcTaxRateFallback` does NOT block because `taxRateValue === 0`.\n\nResult: a 19% DE order is imported with 0% tax. Understated VAT, silent.\n\n**Why this matters:** This is the same class of bug as the original `xx4`: silent tax misconfiguration produces wrong VAT. The fix only addresses the over-rated direction.","acceptance_criteria":"- Add a second blocking condition: when the destination country has any active non-zero tax rate configured for the product category and the resolver fell back to a different (zero or otherwise non-matching) rate, block the import\n- Alternatively: require that the resolver's matched rate ID actually matches the destination jurisdiction (countryCode == destCountry) — block on any fallback regardless of rate value\n- Add a focused WC import test for the zero-rated fallback case where the destination should have been non-zero\n- Update the Sales Settings fallback widget to also surface zero-rate fallback events","notes":"Identified during PR #144 review. The PR addressed bd onetwo3d-ims-xx4 in the over-rated direction but the under-rated direction (silent zero-rate import for a non-zero-rated jurisdiction) is the more dangerous failure mode because under-reported VAT is harder to detect than over-reported VAT.","status":"open","priority":1,"issue_type":"bug","owner":"info@onetwo3d.co.uk","created_at":"2026-06-07T23:44:43Z","created_by":"Jan","updated_at":"2026-06-07T23:36:43Z","labels":["code-review","connectors","pr-144","vat"],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-xjj","title":"Tax snapshot refresh has a read-then-write race against status transitions (PR #144)","description":"PR #144 adds `refreshMutableDocumentTaxSnapshotsForRate()` (lib/tax/document-tax-snapshot-refresh.ts) which is called transactionally from `updateTaxRate()` when a TaxRate is edited. The function:\n\n1. `findMany` for SalesOrderLines whose `order.status IN [DRAFT, PENDING_PAYMENT]`.\n2. Loops through each line and calls `salesOrderLine.update()` plus header `salesOrder.update()` to apply the new rate.\n\nThe findMany result is materialized at read time. If a SalesOrder transitions from DRAFT → PROCESSING (or PENDING_PAYMENT → PROCESSING) between the findMany and the update, the function silently rewrites tax fields on an order that should now be immutable.\n\nThe transaction surrounding it doesn't take a SELECT FOR UPDATE on the orders, so concurrent writers can sneak in.\n\nSame gap exists in `refreshMutablePurchaseOrderTaxSnapshots` (status set `[DRAFT, RFQ_SENT, QUOTE_RECEIVED]`).\n\n**Why this matters:** Tax rate edits typically happen during operator setup, but a user shipping an order at the same moment as another user editing the tax rate can cause the shipped order's persisted tax to silently change. Hard to detect post-hoc.","acceptance_criteria":"- Either (a) add SELECT FOR UPDATE on the SalesOrder/PurchaseOrder rows before re-checking status and applying updates, or (b) re-check the order status inside the update by using a conditional update (`update where: { id, status: { in: MUTABLE_STATUSES } }`) and counting affected rows\n- Add a concurrency test that exercises the race: start the tax refresh transaction, transition the order to PROCESSING from another connection, assert no update lands on the now-immutable order\n- Document the chosen safety mechanism in lib/tax/document-tax-snapshot-refresh.ts","notes":"Identified during PR #144 review. The risk is bounded by how rarely operators edit a TaxRate concurrently with order processing, but the failure mode is silent — tax fields are denormalized onto orders/lines, so a stale-write here changes accounting numbers without an audit trail beyond the tax_rate_snapshot_refresh activity log entry (which doesn't include which lines were touched).","status":"open","priority":1,"issue_type":"bug","owner":"info@onetwo3d.co.uk","created_at":"2026-06-07T23:44:35Z","created_by":"Jan","updated_at":"2026-06-07T23:36:24Z","labels":["code-review","concurrency","pr-144","tax"],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-9mt","title":"Fix PR 144 review Beads batch","description":"Address the PR #144 review findings filed as Beads: tax snapshot status race, WooCommerce tax fallback under-taxing, pending FX queue dedupe, mutable purchase tax statuses, tax snapshot N+1 updates, and cost-layer revaluation audit storage overhead.","status":"closed","priority":1,"issue_type":"task","assignee":"Jan","owner":"info@onetwo3d.co.uk","created_at":"2026-06-07T23:41:17Z","created_by":"Jan","updated_at":"2026-06-07T23:50:43Z","started_at":"2026-06-07T23:41:21Z","closed_at":"2026-06-07T23:50:43Z","close_reason":"Superseded by the exact PR #144 review Beads imported from codex/dead-stock-report and closed individually: xjj, xin, rdy, dmi, jox, lvs.","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-67x","title":"Add action-level idempotency test: cancelPurchaseOrder twice (P4.4 follow-up)","description":"Plan item P4.4 closed via PR #138, but only the `isPurchaseOrderCancellationNoop` helper has a test. `tests/domain/purchasing/po-cancellation.test.ts:246` asserts the boolean returns true/false for given statuses. The plan acceptance criterion specifies an action-level test:\n\n\u003e Call `cancelPurchaseOrder` twice; assert second call succeeds silently.\n\nNo such test exists. The helper test cannot prove that `cancelPurchaseOrder` actually wires the noop check, that the second call:\n- returns `{ success: true }` (not an error)\n- does NOT write a second 'cancelled' activity log row\n- does NOT enqueue a second accounting sync\n- does NOT re-emit stock-sync events\n\n**Why this matters:** A future refactor that removes the noop check (or moves it after the side-effects) would not be caught by the current helper test. Repeated cancellation requests producing duplicate Xero journals or activity logs is the failure mode P4.4 was meant to prevent.","acceptance_criteria":"- Add test in tests/domain/purchasing/po-cancellation.test.ts (or action-level test file) that:\n - Sets up a CANCELLED PO\n - Calls `cancelPurchaseOrder('po-1')` (or fixtures the action call)\n - Asserts result is `{ success: true }`\n - Asserts no duplicate activity log row added\n - Asserts no duplicate accounting sync queue row added\n - Asserts no duplicate stock-sync enqueue\n- Update docs/plan.md P4.4 Tests pointer to name the new test","notes":"Identified during PR #140 review audit. Same gap pattern as P1.3 (helper test only; action-level integration not proven). My PR #138 review inline finding #6 flagged this explicitly with suggested test shape — recommend implementing that shape.","status":"open","priority":1,"issue_type":"task","owner":"dev@onetwo3d.com","created_at":"2026-06-07T18:08:51Z","created_by":"Jan","updated_at":"2026-06-07T18:08:51Z","labels":["idempotency","plan-followup","purchasing","tests"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-4jz","title":"Add action-level idempotency test: createSalesOrderRefund replay (P1.3 follow-up)","description":"Plan item P1.3 closed via PR #128, but only the `applyReturnInboundStockTx` helper has a test for the idempotency-conflict path. The plan acceptance criterion specifies an action-level replay test:\n\n\u003e Call `processRefund` twice with same key, assert one set of movements and one set of layers.\n\nNo such test exists. `tests/domain/sales/refund-service.test.ts` does not call `createSalesOrderRefund` twice with the same idempotency key to verify the action-level idempotency.\n\n**Why this matters:** A future refactor that strips the helper-level guard (or wires it incorrectly into the action) would not be caught by current tests. Idempotency regressions in refund flows produce duplicate stock movements and double-posted COGS reversals — exactly the failure mode P1.3 was meant to prevent.","acceptance_criteria":"- Add test in tests/domain/sales/refund-service.test.ts that:\n - Calls `createSalesOrderRefund` once with a refund payload + external refund ID\n - Calls it again with the same (orderId, refundLineId, warehouseId, externalRefundId)\n - Asserts state.movements length is unchanged after the second call\n - Asserts state.costLayers length is unchanged\n - Asserts the second call returns success (not duplicate error)\n- Update docs/plan.md P1.3 Tests pointer to name the new test","notes":"Identified during PR #140 review audit. Plan currently points to non-existent path `tests/refund-service.idempotency.test.ts`. Helper-level tests at refund-service.test.ts:1150 (applyReturnInboundStockTx idempotency conflict) and woocommerce-refund-sync.test.ts:115 (syncWcRefund repeated delivery) cover adjacent paths but not the action-level replay the plan asked for.","status":"open","priority":1,"issue_type":"task","owner":"dev@onetwo3d.com","created_at":"2026-06-07T18:08:29Z","created_by":"Jan","updated_at":"2026-06-07T18:08:29Z","labels":["idempotency","plan-followup","refund","tests"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-vme","title":"Accounting invariant report: exclude REFUNDED/CANCELLED orders","description":"PR #15 (merged) added the accounting invariant report at lib/domain/accounting/invariants.ts but did not exclude terminal-state orders from the scanner.\n\nWithout the exclusion:\n- CANCELLED orders with prior revenueDeferredDate fire sales_order_revenue_deferral_without_sync_evidence indefinitely (no Group B batch will ever sync against a cancelled order).\n- REFUNDED orders with shipmentJournalDate fire shipment_posted_without_sync_evidence permanently.\n- The { refunds: { some: {} } } OR-arm pulls in every refunded order, generating false-positive refund-missing findings that desensitize operators.\n\nFix:\n1. Add status: { notIn: ['REFUNDED', 'CANCELLED'] } to salesOrder.findMany where clause (L495-L503).\n2. Add order: { status: { notIn: ['REFUNDED', 'CANCELLED'] } } to shipment.findMany (L535-L553).\n3. Add a collection test mirroring PR #8's 'inventory row collection excludes fully refunded orders' to lock in the filter.\n\nReference: https://github.com/OneTwo3D/IMS/pull/15#pullrequestreview-4177148955","status":"open","priority":1,"issue_type":"task","owner":"dev@onetwo3d.com","created_at":"2026-04-26T16:21:49Z","created_by":"Jan","updated_at":"2026-04-26T16:21:49Z","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-zzd","title":"[bug] PO FX rate is manual operator input — no auto-lookup or validation","description":"Unlike SO creation (which calls resolveFxRateToBase to look up the latest rate ≤ order date) and WC order import (which calls getFxRateToGbp), PO creation at app/actions/purchase-orders.ts:787 expects the caller to supply fxRateToBase from the UI. The operator types it in.\n\nImpact: typo, missing decimal, or stale rate from an old browser tab corrupts the unit cost stamped onto every CostLayer created from that PO's receipt — and that base cost is the one consumed by FIFO into customer COGS. Hard to detect after the fact.\n\nFix direction:\n- Default the field from resolveFxRateToBase() at the PO's order date. Allow override but show a delta vs current rate and warn if \u003e2%.\n- Validate input is within a sane band of the latest known rate before saving.\n\nRefs:\n- app/actions/purchase-orders.ts:787\n- app/actions/sales.ts:615 resolveFxRateToBase() (the pattern to follow)","status":"open","priority":1,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-04-25T20:45:24Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T20:45:24Z","labels":["code-review","fx","po"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-xx4","title":"[bug] Tax-rate fallback to order default is silent on WC import and resolver","description":"When an incoming WooCommerce order's externalTaxRateId has no ShoppingTaxRateMapping, importer falls back to resolveLineTaxRateBatch(); when the resolver's 4-step lookup (country+category → country+STANDARD → global+category) finds nothing, it falls back to the order-default rate with only a console warning. Same silent fallback exists in createSalesOrder/createPurchaseOrder.\n\nImpact: a misconfigured tax mapping (or a new country added to WC without IMS setup) will quietly produce sales at the wrong VAT rate. The discrepancy only surfaces when Xero rejects the TaxType or on a manual VAT review.\n\nFix direction:\n1. Block import (or require explicit override) when fallback is used for a non-zero-rated jurisdiction.\n2. Record fallback events to activity log per line so operations can audit.\n3. Add a settings-page widget listing recent fallback events.\n\nRefs:\n- lib/tax/resolve-rate.ts:53 pickTaxRate()\n- lib/connectors/woocommerce/sync/order-import.ts:112","status":"open","priority":1,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-04-25T20:45:07Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T20:45:07Z","labels":["code-review","connectors","vat"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-imn","title":"[bug] Retroactive cost-layer snapshot patching has no audit trail","description":"When a PO's landed cost finalises after shipments have already shipped, `updateSnapshotsForCostLayerChange()` (lib/cost-layers.ts:291) silently rewrites the JSON `costLayerSnapshot` on every affected ShipmentLine, OrderAllocation, SalesOrderRefundLine and StockTransferLine, then refreshes denormalised `Shipment.cogsBatchAmount` and `SalesOrderLine.cogsBase`.\n\nNo row is written to the activity log; nothing records previous vs new unit cost. P\u0026L and margin reports change retroactively with no breadcrumb.\n\nFix direction: write a typed activity-log entry per affected line with old/new `unitCostBase` and the triggering CostLayer id. Consider keeping a CostLayerAdjustment table for full lineage.\n\nRefs:\n- lib/cost-layers.ts:291 updateSnapshotsForCostLayerChange()\n- See onetwo3d-ims-a8u for the originating reconciliation work","status":"open","priority":1,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-04-25T20:44:57Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T20:44:57Z","labels":["audit","code-review","cogs"],"dependency_count":0,"dependent_count":0,"comment_count":0} -{"_type":"issue","id":"onetwo3d-ims-5mp.7","title":"Manual Mark Received on a partially-booked transfer double-counts stock","description":"Codex P1 from review of branch vs main.\n\nWhen a Mintsoft (WMS) callback partially books in a transfer:\n- lib/connectors/mintsoft/sync/booked-in-handler.ts increments transferLine.qtyReceived\n- Posts a TRANSFER_IN movement of the partial qty\n- Creates cost layers from a slice of the snapshot proportional to that qty\n- Leaves transfer.status = IN_TRANSIT until qtyReceived \u003e= qty for every line\n\nThe manual close-out path in app/actions/transfers.ts (receiveTransfer) was still:\n- Posting a TRANSFER_IN of the FULL Number(line.qty)\n- Recreating cost layers from the entire snapshot\n- Setting qtyReceived: qty\n\nNet effect: destination stock and cost layers overstated by the already-booked amount whenever an operator closed out a partial WMS transfer through the manual button.\n\nFix: receive only the remaining qty (qty - qtyReceived), slice the snapshot via sliceTransferSnapshotForReceipt to grab only the un-consumed cost layer portion, skip lines that are already fully received via WMS.","status":"closed","priority":1,"issue_type":"task","owner":"dev@onetwo3d.com","created_at":"2026-04-25T12:29:52Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T12:29:53Z","closed_at":"2026-04-25T12:29:53Z","close_reason":"Closed","labels":["connectors","fx","inventory","mintsoft","review-finding","wms","woocommerce","xero"],"dependencies":[{"issue_id":"onetwo3d-ims-5mp.7","depends_on_id":"onetwo3d-ims-5mp","type":"parent-child","created_at":"2026-04-25T12:29:52Z","created_by":"OneTwo3D IMS","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-5mp.7","title":"Manual Mark Received on a partially-booked transfer double-counts stock","description":"Codex P1 from review of branch vs main.\n\nWhen a Mintsoft (WMS) callback partially books in a transfer:\n- lib/connectors/mintsoft/sync/booked-in-handler.ts increments transferLine.qtyReceived\n- Posts a TRANSFER_IN movement of the partial qty\n- Creates cost layers from a slice of the snapshot proportional to that qty\n- Leaves transfer.status = IN_TRANSIT until qtyReceived \u003e= qty for every line\n\nThe manual close-out path in app/actions/transfers.ts (receiveTransfer) was still:\n- Posting a TRANSFER_IN of the FULL Number(line.qty)\n- Recreating cost layers from the entire snapshot\n- Setting qtyReceived: qty\n\nNet effect: destination stock and cost layers overstated by the already-booked amount whenever an operator closed out a partial WMS transfer through the manual button.\n\nFix: receive only the remaining qty (qty - qtyReceived), slice the snapshot via sliceTransferSnapshotForReceipt to grab only the un-consumed cost layer portion, skip lines that are already fully received via WMS.","status":"closed","priority":1,"issue_type":"task","owner":"dev@onetwo3d.com","created_at":"2026-04-25T12:29:52Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T12:29:53Z","closed_at":"2026-04-25T12:29:53Z","close_reason":"Closed","labels":["connectors","fx","inventory","mintsoft","review-finding","wms","woocommerce","xero"],"dependencies":[{"issue_id":"onetwo3d-ims-5mp.7","depends_on_id":"onetwo3d-ims-5mp","type":"parent-child","created_at":"2026-04-25T12:29:52Z","created_by":"Jan","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-1ck","title":"Cost-layer deletion leaves ghost snapshot entries","description":"updateSnapshotsForCostLayerChange silently skips deleted layers. ShipmentLine / OrderAllocation / SalesOrderRefundLine / StockTransferLine snapshots can keep stale unitCostBase entries pointing at layers that no longer exist. No FK constraint prevents the deletion.\n\nRefs:\n- lib/cost-layer-snapshots.ts (parser/reducer)\n- lib/cost-layers.ts:291-344 (updateSnapshotsForCostLayerChange)\n\nFix direction: add FK from snapshot JSON references → CostLayer (or denormalise to a join table), and either block deletion if referenced or cascade-update.","status":"closed","priority":1,"issue_type":"bug","assignee":"OneTwo3D IMS","owner":"dev@onetwo3d.com","created_at":"2026-04-25T09:57:54Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T10:17:44Z","started_at":"2026-04-25T10:17:44Z","closed_at":"2026-04-25T10:17:44Z","close_reason":"Verified: not a bug. Cost layer deletion in stock.ts:710 is gated by line 703-708: throws if consumedQty \u003e 0 (any prior FIFO consumption blocks the delete and forces user to create a reversing adjustment). An unconsumed layer has no shipment/allocation/refund snapshot referring to it, so 'ghost snapshots' cannot occur. The other delete site (reset.ts:63) is the admin DB-wipe path — out of scope. No FK constraint needed because the gate already prevents the failure mode.","labels":["code-review","cogs","fifo"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-j7j","title":"Manufacturing disassembly over-attributes cost-layer source qty","description":"componentShare is multiplied into source line qty per component. Sum of component source-line quantities exceeds the original recovered layer qty, breaking traceability for downstream landed-cost recalculation.\n\nRefs:\n- app/actions/manufacturing.ts:824-831","status":"closed","priority":1,"issue_type":"bug","assignee":"OneTwo3D IMS","owner":"dev@onetwo3d.com","created_at":"2026-04-25T09:57:54Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T10:16:00Z","started_at":"2026-04-25T10:14:17Z","closed_at":"2026-04-25T10:16:00Z","close_reason":"Verified: not a bug. Source-line qty across components sums correctly. componentShare = baseAllocatedCost / totalRecoveredCostBase, and Σ baseAllocatedCost = totalRecoveredCostBase by construction (buildDisassemblyRecoveryPlan: Σ historicalCost + residualCostBase redistribution = Σ entryCostBase). So Σ (entry.qty × componentShare_i) = entry.qty × Σ shares = entry.qty × 1 = original recovered layer qty. The agent's 'sum exceeds' claim is incorrect. There is a minor rounding-drift risk via componentShare.toNumber() (Prisma.Decimal → JS number), but that's a separate concern with negligible impact.","labels":["code-review","fifo","manufacturing"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-5oh","title":"Allocation snapshots editable between A2 staging and Group B run","description":"resetAllocationAccountingIfStaged() only triggers when shipmentJournalDate is set — i.e. after Group B has stamped the order. Between A2 staging and Group B, allocations can be edited and the next batch will use stale snapshots. Shipment-line snapshots are also not cleared on reset (only allocation snapshots are).\n\nRefs:\n- app/actions/allocation.ts:42, :74-77\n\nFix direction: lock allocations on A2 staging and require an explicit unstage that clears both allocation- and shipment-line snapshots.","status":"closed","priority":1,"issue_type":"bug","assignee":"OneTwo3D IMS","owner":"dev@onetwo3d.com","created_at":"2026-04-25T09:57:53Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T10:17:44Z","started_at":"2026-04-25T10:17:43Z","closed_at":"2026-04-25T10:17:44Z","close_reason":"Verified: not a bug. resetAllocationAccountingIfStaged (allocation.ts:42-79) is called inside every allocation-mutation tx (e.g. deallocateOrder line 937). When inventoryAllocatedDate is set: (a) blocks edits if any shipment has shipmentJournalDate (Group B already ran) — line 55-64; (b) otherwise clears inventoryAllocatedDate AND OrderAllocation.costLayerSnapshot so A2 rebuilds on next batch. ShipmentLine snapshots are intentionally NOT cleared because they reflect the actual SHIPPED state and should be preserved. The 'stale snapshots in next batch' claim is incorrect.","labels":["code-review","cogs","sync-xero"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-jgi","title":"StockLevel.quantity drifts from sum of cost-layer remainingQty","description":"StockLevel.quantity is mutated directly in several places and is not derived from the cost layers. Partial transaction failures or layer adjustments can leave the two out of sync. There is no reconciliation job.\n\nRefs:\n- lib/stock.ts:166-172, :193-197 (direct StockLevel mutations)\n- lib/transfers.ts:558-559 (transfer mutations)\n- lib/cost-layers.ts:181-197 (layer mutations not synced back)\n\nFix direction: either make quantity a derived view (sum of layers) or add a reconciliation job + alert when they diverge.","status":"closed","priority":1,"issue_type":"bug","assignee":"OneTwo3D IMS","owner":"dev@onetwo3d.com","created_at":"2026-04-25T09:57:53Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T10:17:45Z","started_at":"2026-04-25T10:17:44Z","closed_at":"2026-04-25T10:17:45Z","close_reason":"Verified: not a real bug. StockLevel.quantity and cost-layer sums are mutated together inside the same Prisma $transaction at every site (allocation.ts:1303-1310 + 1327-1329, manufacturing.ts:684-705 + 722-738 + 780-798, transfers.ts:405-414 + 556-578, stock.ts:689-756, purchase-orders.ts receipts). Postgres ACID prevents partial commits. Drift is only possible via out-of-band DB writes, which no app fix can prevent. A reconciliation job would be defensive observability, not a bug fix — open a separate feature request if desired.","labels":["code-review","fifo","inventory"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-0r8","title":"FIFO consumption race: candidate read happens before per-row lock","description":"lib/cost-layers.ts:62-95 — findMany reads the candidate cost-layer set, then takes per-row locks. Two concurrent shippers can read the same candidate set before either acquires a lock, then both decrement the same layer.\n\nImpact: physical stock corruption — over-consumption, negative remainingQty, broken FIFO ordering.\n\nVERIFY: confirm whether the lock is actually `SELECT ... FOR UPDATE` on the same row IDs and whether the surrounding $transaction is at Serializable isolation. If not, switch to Serializable or take the row locks before/inside the candidate read (e.g. `SELECT ... FOR UPDATE SKIP LOCKED`).","status":"closed","priority":1,"issue_type":"bug","assignee":"OneTwo3D IMS","owner":"dev@onetwo3d.com","created_at":"2026-04-25T09:57:52Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T10:31:50Z","started_at":"2026-04-25T10:31:49Z","closed_at":"2026-04-25T10:31:50Z","close_reason":"Verified: not a bug. consumeFifoLayers (lib/cost-layers.ts:62-95) implements the textbook lock-then-re-read pattern: (1) findMany candidate IDs, (2) lockCostLayers acquires SELECT...FOR UPDATE on those IDs, (3) re-reads the locked rows via findMany on id IN [...] for fresh remainingQty values, (4) decrements based on the post-lock state. The explicit comment at line 69-70 documents this: 'Re-read after taking row locks so concurrent consumers cannot both act on the same pre-lock remainingQty snapshot and double-decrement a layer.' The agent missed the re-read.","labels":["code-review","concurrency","fifo","verify"],"dependency_count":0,"dependent_count":0,"comment_count":0} -{"_type":"issue","id":"onetwo3d-ims-2uu","title":"P8.2 'latest placed PO wins' supplier rule doesn't handle emergency/backup POs or transition timing (PR #155 follow-up)","description":"PR #155 P8.2 proposes:\n\n\u003e Update the product supplier automatically when a purchase order is placed for that product. If a PO contains multiple products, update each included product to that PO's supplier. If a product appears on multiple POs, the latest placed PO wins.\n\nThis rule has several operational gaps:\n\n**1. Emergency / backup supplier overrides the primary.**\n\nScenario: Product P is consistently sourced from Supplier A. A's stock is out for one week; ops places an emergency PO for P with Supplier B (a backup). After 'latest placed PO wins', P's supplier field is now B. The next reorder forecast → draft PO generator scopes P to supplier B, not A. Operator has to manually re-bind P to A, or A is silently demoted.\n\nWorkarounds: a 'supplier_lock' boolean per product to opt out of auto-update; or 'mark as one-off' flag on POs to skip the auto-update.\n\n**2. Update timing: PO_SENT vs receipt status.**\n\nThe plan says 'when a purchase order is placed'. Placement is the `PO_SENT` transition. If the PO is later returned or cancelled (RETURNED, CANCELLED) for fulfillment failure, the supplier auto-update is NOT rolled back. P now points at a supplier that couldn't deliver. Should the update fire on:\n - PO_SENT (decision: 'we're committing to this supplier')\n - PARTIALLY_RECEIVED (decision: 'this supplier has actually delivered')\n - RECEIVED (decision: 'this supplier completed delivery')\n\nDifferent operations teams answer this differently. Plan should pick.\n\n**3. Mass-update side effects.**\n\nA single PO line update transaction now triggers N product updates (one per line). For a 200-line PO, that's 200 `product` row updates in the same transaction as the PO line writes. Worth specifying:\n - Same-transaction or async via outbox?\n - Lock ordering: products by ID asc to avoid deadlock with concurrent stock movements that touch the same product rows\n - Activity-log volume: 200 'product_supplier_updated' activity-log entries per PO\n\n**4. PO multi-supplier impossible scenarios.**\n\nThe plan says 'If a PO contains multiple products, update each included product to that PO's supplier.' A PO has exactly one supplier (PurchaseOrder.supplierId), so 'PO's supplier' is always singular. Fine. But:\n - What about a PO line for a product where SupplierProduct doesn't have a row for this supplier? Implicitly creates one, or refuses?\n - What if the new supplier's currency differs from the product's expected cost basis?\n\n**5. Race with multi-PO concurrent placement.**\n\nTwo POs placed within milliseconds for the same product, different suppliers. Final value of `product.supplier` depends on transaction commit ordering. The plan says 'latest placed PO wins' but doesn't define 'placed' precisely — committed-at? sent-at? created-at?\n\n**Why this matters:**\n\nThe auto-update is the load-bearing mechanism for reorder draft generation in P8.2. Subtle race / timing / opt-out semantics determine whether ops trusts the suggestion or treats it as 'noise that must be manually corrected after every PO'.","acceptance_criteria":"- Update P8.2 to specify:\n - When the update fires (PO_SENT vs received)\n - Whether the update is per-tx or async\n - Opt-out mechanism for one-off / emergency POs (e.g., a 'don't update product supplier' flag on the PO header)\n - Concurrency policy for multi-PO placement (which timestamp wins)\n - Rollback semantics when a PO is cancelled / returned","notes":"Filed during PR #155 second-pass review. The 'latest wins' rule is simple to implement but loses signal about primary vs ad-hoc supplier choices. Worth getting ops/procurement sign-off before implementation.","status":"open","priority":2,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-10T14:22:13Z","created_by":"Jan","updated_at":"2026-06-10T14:22:13Z","labels":["code-review","p8.2","plan","pr-155","procurement"],"dependency_count":0,"dependent_count":0,"comment_count":0} -{"_type":"issue","id":"onetwo3d-ims-tep","title":"P8.2 product supplier field overlaps with existing SupplierProduct many-to-many relation — spec needs to reconcile (PR #155 follow-up)","description":"PR #155 (commit 65b726da) adds P8.2 to docs/plan.md:\n\n\u003e 1. Add a supplier field to products, backed by a relation to the supplier/vendor model used by purchase orders.\n\nBut the codebase already has `SupplierProduct` (prisma/schema.prisma:943-958) — a many-to-many join model between Product and Supplier with per-supplier metadata:\n\n```prisma\nmodel SupplierProduct {\n id String @id\n supplierId String\n productId String\n supplierSku String?\n lastUnitCost Decimal\n currency String\n leadTimeDays Int?\n updatedAt DateTime @updatedAt\n\n @@unique([supplierId, productId])\n}\n```\n\nThis model is used by:\n- Supplier-portal product editing (referenced in PR #154)\n- Multi-supplier procurement (different lead times, costs, SKUs per supplier)\n- Supplier-specific historical cost tracking\n\nP8.2 proposes a SINGULAR `product.supplier` field updated by 'latest placed PO wins'. The spec doesn't reconcile:\n\n1. **Is the new field a 'preferred/primary supplier' pointer that supplements SupplierProduct?**\n - If yes, semantics are: 'this is the most recent supplier we used; the SupplierProduct relation still lists every supplier we've ever bought from'\n - The reorder draft generator uses the new field; the SupplierProduct relation continues to store per-supplier metadata\n \n2. **Is the new field replacing SupplierProduct?**\n - If yes, where does per-supplier metadata go (supplierSku, lastUnitCost, leadTimeDays)?\n - Multi-supplier procurement breaks: a product with 3 suppliers loses 2 of them\n - Supplier-portal features that depend on SupplierProduct break\n\n3. **Migration story:**\n - Backfill product.supplier from: most recent PO line, OR the first SupplierProduct row, OR null?\n - If a product has SupplierProduct rows but no PO history yet (e.g., newly added to a supplier's catalog), what's the supplier field set to?\n\n4. **Concurrent multi-supplier scenarios:**\n - A product genuinely sourced from 2 suppliers gets its 'supplier' field oscillating per-PO\n - Reorder forecasts generated per-supplier may exclude the product from the wrong supplier's draft\n\n**Why this matters:**\n\nThe spec's brevity hides a significant data-modeling decision. Whether to ADD a field next to an existing relation or REPLACE the relation is the kind of choice that benefits from being explicit upfront — implementing the wrong interpretation costs a redo.\n\nThe cleanest interpretation given the existing schema: keep `SupplierProduct` for historical/per-supplier metadata; add `Product.preferredSupplierId` (or similar) as an explicit 'most-recent-PO supplier' pointer; the auto-update rule populates this pointer only.","acceptance_criteria":"- Update P8.2 in docs/plan.md to:\n - Explicitly state the relationship to SupplierProduct (supplement vs replace)\n - Specify the field name (`preferredSupplierId` or `latestPoSupplierId` may be clearer than `supplier`)\n - Document the backfill policy: derive from most recent PO line, with fallback to most-recently-updated SupplierProduct row\n - Address multi-supplier products: how do reorder drafts handle a product with multiple historical suppliers? Per-supplier slicing?\n- Add transitions to P8.2's acceptance criteria for the SupplierProduct relation (does it stay, does it change?)","notes":"Filed during PR #155 second-pass review (commit 65b726da). P8.2 is plan-only; this is about pinning the spec before implementation. Related to onetwo3d-ims-ljw (P8.1 migration path needed).","status":"open","priority":2,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-10T14:21:45Z","created_by":"Jan","updated_at":"2026-06-10T14:21:45Z","labels":["code-review","p8.2","plan","pr-155","product-lifecycle"],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-yvu","title":"Implement plan group 21 product lifecycle and supplier reorder planning","description":"Implement docs/plan.md group 21: P8.1 product lifecycle statuses/EOL sell-off with auto-archive and P8.2 preferred supplier tracking with supplier-scoped reorder draft generation. Reconcile the PR #155 P8.2 follow-up beads in the implementation and close them.","status":"closed","priority":2,"issue_type":"feature","assignee":"Jan","owner":"info@onetwo3d.co.uk","created_at":"2026-06-10T15:53:33Z","created_by":"Jan","updated_at":"2026-06-10T16:21:27Z","started_at":"2026-06-10T15:53:38Z","closed_at":"2026-06-10T16:21:27Z","close_reason":"Implemented product lifecycle statuses, EOL archive job, preferred supplier tracking, supplier filters, and supplier-scoped reorder draft generation.","dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-2uu","title":"P8.2 'latest placed PO wins' supplier rule doesn't handle emergency/backup POs or transition timing (PR #155 follow-up)","description":"PR #155 P8.2 proposes:\n\n\u003e Update the product supplier automatically when a purchase order is placed for that product. If a PO contains multiple products, update each included product to that PO's supplier. If a product appears on multiple POs, the latest placed PO wins.\n\nThis rule has several operational gaps:\n\n**1. Emergency / backup supplier overrides the primary.**\n\nScenario: Product P is consistently sourced from Supplier A. A's stock is out for one week; ops places an emergency PO for P with Supplier B (a backup). After 'latest placed PO wins', P's supplier field is now B. The next reorder forecast → draft PO generator scopes P to supplier B, not A. Operator has to manually re-bind P to A, or A is silently demoted.\n\nWorkarounds: a 'supplier_lock' boolean per product to opt out of auto-update; or 'mark as one-off' flag on POs to skip the auto-update.\n\n**2. Update timing: PO_SENT vs receipt status.**\n\nThe plan says 'when a purchase order is placed'. Placement is the `PO_SENT` transition. If the PO is later returned or cancelled (RETURNED, CANCELLED) for fulfillment failure, the supplier auto-update is NOT rolled back. P now points at a supplier that couldn't deliver. Should the update fire on:\n - PO_SENT (decision: 'we're committing to this supplier')\n - PARTIALLY_RECEIVED (decision: 'this supplier has actually delivered')\n - RECEIVED (decision: 'this supplier completed delivery')\n\nDifferent operations teams answer this differently. Plan should pick.\n\n**3. Mass-update side effects.**\n\nA single PO line update transaction now triggers N product updates (one per line). For a 200-line PO, that's 200 `product` row updates in the same transaction as the PO line writes. Worth specifying:\n - Same-transaction or async via outbox?\n - Lock ordering: products by ID asc to avoid deadlock with concurrent stock movements that touch the same product rows\n - Activity-log volume: 200 'product_supplier_updated' activity-log entries per PO\n\n**4. PO multi-supplier impossible scenarios.**\n\nThe plan says 'If a PO contains multiple products, update each included product to that PO's supplier.' A PO has exactly one supplier (PurchaseOrder.supplierId), so 'PO's supplier' is always singular. Fine. But:\n - What about a PO line for a product where SupplierProduct doesn't have a row for this supplier? Implicitly creates one, or refuses?\n - What if the new supplier's currency differs from the product's expected cost basis?\n\n**5. Race with multi-PO concurrent placement.**\n\nTwo POs placed within milliseconds for the same product, different suppliers. Final value of `product.supplier` depends on transaction commit ordering. The plan says 'latest placed PO wins' but doesn't define 'placed' precisely — committed-at? sent-at? created-at?\n\n**Why this matters:**\n\nThe auto-update is the load-bearing mechanism for reorder draft generation in P8.2. Subtle race / timing / opt-out semantics determine whether ops trusts the suggestion or treats it as 'noise that must be manually corrected after every PO'.","acceptance_criteria":"- Update P8.2 to specify:\n - When the update fires (PO_SENT vs received)\n - Whether the update is per-tx or async\n - Opt-out mechanism for one-off / emergency POs (e.g., a 'don't update product supplier' flag on the PO header)\n - Concurrency policy for multi-PO placement (which timestamp wins)\n - Rollback semantics when a PO is cancelled / returned","notes":"Filed during PR #155 second-pass review. The 'latest wins' rule is simple to implement but loses signal about primary vs ad-hoc supplier choices. Worth getting ops/procurement sign-off before implementation.","status":"closed","priority":2,"issue_type":"bug","assignee":"Jan","owner":"dev@onetwo3d.com","created_at":"2026-06-10T14:22:13Z","created_by":"Jan","updated_at":"2026-06-10T16:21:30Z","started_at":"2026-06-10T15:53:38Z","closed_at":"2026-06-10T16:21:30Z","close_reason":"Implemented PO_SENT preferred-supplier update with one-off PO opt-out, sorted product locking, and locked preferred-supplier skip.","labels":["code-review","p8.2","plan","pr-155","procurement"],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-tep","title":"P8.2 product supplier field overlaps with existing SupplierProduct many-to-many relation — spec needs to reconcile (PR #155 follow-up)","description":"PR #155 (commit 65b726da) adds P8.2 to docs/plan.md:\n\n\u003e 1. Add a supplier field to products, backed by a relation to the supplier/vendor model used by purchase orders.\n\nBut the codebase already has `SupplierProduct` (prisma/schema.prisma:943-958) — a many-to-many join model between Product and Supplier with per-supplier metadata:\n\n```prisma\nmodel SupplierProduct {\n id String @id\n supplierId String\n productId String\n supplierSku String?\n lastUnitCost Decimal\n currency String\n leadTimeDays Int?\n updatedAt DateTime @updatedAt\n\n @@unique([supplierId, productId])\n}\n```\n\nThis model is used by:\n- Supplier-portal product editing (referenced in PR #154)\n- Multi-supplier procurement (different lead times, costs, SKUs per supplier)\n- Supplier-specific historical cost tracking\n\nP8.2 proposes a SINGULAR `product.supplier` field updated by 'latest placed PO wins'. The spec doesn't reconcile:\n\n1. **Is the new field a 'preferred/primary supplier' pointer that supplements SupplierProduct?**\n - If yes, semantics are: 'this is the most recent supplier we used; the SupplierProduct relation still lists every supplier we've ever bought from'\n - The reorder draft generator uses the new field; the SupplierProduct relation continues to store per-supplier metadata\n \n2. **Is the new field replacing SupplierProduct?**\n - If yes, where does per-supplier metadata go (supplierSku, lastUnitCost, leadTimeDays)?\n - Multi-supplier procurement breaks: a product with 3 suppliers loses 2 of them\n - Supplier-portal features that depend on SupplierProduct break\n\n3. **Migration story:**\n - Backfill product.supplier from: most recent PO line, OR the first SupplierProduct row, OR null?\n - If a product has SupplierProduct rows but no PO history yet (e.g., newly added to a supplier's catalog), what's the supplier field set to?\n\n4. **Concurrent multi-supplier scenarios:**\n - A product genuinely sourced from 2 suppliers gets its 'supplier' field oscillating per-PO\n - Reorder forecasts generated per-supplier may exclude the product from the wrong supplier's draft\n\n**Why this matters:**\n\nThe spec's brevity hides a significant data-modeling decision. Whether to ADD a field next to an existing relation or REPLACE the relation is the kind of choice that benefits from being explicit upfront — implementing the wrong interpretation costs a redo.\n\nThe cleanest interpretation given the existing schema: keep `SupplierProduct` for historical/per-supplier metadata; add `Product.preferredSupplierId` (or similar) as an explicit 'most-recent-PO supplier' pointer; the auto-update rule populates this pointer only.","acceptance_criteria":"- Update P8.2 in docs/plan.md to:\n - Explicitly state the relationship to SupplierProduct (supplement vs replace)\n - Specify the field name (`preferredSupplierId` or `latestPoSupplierId` may be clearer than `supplier`)\n - Document the backfill policy: derive from most recent PO line, with fallback to most-recently-updated SupplierProduct row\n - Address multi-supplier products: how do reorder drafts handle a product with multiple historical suppliers? Per-supplier slicing?\n- Add transitions to P8.2's acceptance criteria for the SupplierProduct relation (does it stay, does it change?)","notes":"Filed during PR #155 second-pass review (commit 65b726da). P8.2 is plan-only; this is about pinning the spec before implementation. Related to onetwo3d-ims-ljw (P8.1 migration path needed).","status":"closed","priority":2,"issue_type":"bug","assignee":"Jan","owner":"dev@onetwo3d.com","created_at":"2026-06-10T14:21:45Z","created_by":"Jan","updated_at":"2026-06-10T16:21:28Z","started_at":"2026-06-10T15:53:38Z","closed_at":"2026-06-10T16:21:28Z","close_reason":"Kept SupplierProduct for multi-supplier metadata and added Product.preferredSupplierId as the singular preferred supplier pointer with backfill.","labels":["code-review","p8.2","plan","pr-155","product-lifecycle"],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-0wj","title":"Add product supplier and supplier-scoped reorder planning to plan","description":"Add a future plan item for product supplier tracking and supplier-scoped replenishment: each product has a supplier field, the supplier is set from the supplier used on the latest purchase order, products can be filtered by supplier, and operators can generate draft purchase orders for a supplier from reorder forecasts.","status":"closed","priority":2,"issue_type":"task","assignee":"Jan","owner":"info@onetwo3d.co.uk","created_at":"2026-06-10T14:15:08Z","created_by":"Jan","updated_at":"2026-06-10T14:15:42Z","started_at":"2026-06-10T14:15:16Z","closed_at":"2026-06-10T14:15:42Z","close_reason":"Added P8.2 to docs/plan.md for product supplier tracking from latest placed PO, supplier filtering, and supplier-scoped draft PO generation from reorder forecasts.","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-ljw","title":"P8.1 product-lifecycle plan doesn't specify the migration path from existing NOT_FOR_SALE / ACTIVE / ARCHIVED enum (PR #155)","description":"PR #155 adds Group 21 / P8.1 to plan.md with a proposed new product-lifecycle status enum:\n\n\u003e Product statuses: `active`, `draft`, `eol`, `archived`\n\nBut the codebase already has `ProductLifecycleStatus { ACTIVE, NOT_FOR_SALE, ARCHIVED }` at `prisma/schema.prisma:36-40`, used by `Product.lifecycleStatus`. The plan item doesn't address:\n\n1. **Mapping NOT_FOR_SALE → new states.** What happens to existing products in NOT_FOR_SALE? Some are arguably 'draft' (never published), some are arguably 'eol' (was published, removed from sale, still has stock). The plan needs a migration policy.\n\n2. **`draft` overlap with existing publication flag.** The PR's description of draft says 'generated or being prepared, not published for sale yet, but can already be purchased.' This may overlap with:\n - WooCommerce's product status (publish/draft/private)\n - A separate IMS 'published' boolean if one exists\n - The existing NOT_FOR_SALE status\n\n3. **Code paths assuming the current enum.** Search for `ProductLifecycleStatus`, `lifecycleStatus`, `ACTIVE`, `NOT_FOR_SALE`, `ARCHIVED` to find every existing consumer. Each needs updating to handle the new state machine. The plan acceptance criteria don't enumerate this surface.\n\n4. **`isOperationalProductStatus` helper.** Referenced in app/actions/purchase-orders.ts — what's it currently doing, and how should it map to the new states?\n\n5. **PO line creation / sales order creation / allocation** all probably check product status today. The plan doesn't enumerate which actions need updates.\n\n**Why this matters:** P8.1's acceptance criteria say 'each allowed and blocked transition' — but the transition diagram is unstated. Without it, the implementer has to invent the mapping from existing data + existing code paths to the new state machine, which is exactly the kind of decision that benefits from upfront planning.\n\n**Suggested additions to P8.1:**\n\n1. **Migration table**:\n | Old | New |\n |-----|-----|\n | ACTIVE | active |\n | NOT_FOR_SALE | eol (default) — operators reclassify to draft if pre-publication |\n | ARCHIVED | archived |\n\n2. **State transition diagram** explicitly listing which transitions are allowed:\n - draft → active (publish)\n - active → eol (start sell-off)\n - active → archived (immediate withdraw)\n - eol → archived (auto-archive job, or manual)\n - eol → active (un-EOL — undo)\n - archived → ? (allowed/disallowed?)\n\n3. **Code-path inventory**: list every action/query that checks lifecycleStatus today and what it should do for each new state.","acceptance_criteria":"- Update P8.1 in docs/plan.md with the migration table from current → new enum\n- Add the explicit state transition diagram (allowed/disallowed transitions)\n- Enumerate code paths that need updating (purchase-orders.ts, sales.ts, allocation.ts, product-list filters, etc.)\n- Decide explicitly whether 'draft' is a new IMS concept or maps to an existing publication flag","notes":"Filed during PR #155 review. The QG1 part of the PR is shippable; the P8.1 part is a plan-only addition that needs more spec before implementation.","status":"closed","priority":2,"issue_type":"bug","assignee":"Jan","owner":"dev@onetwo3d.com","created_at":"2026-06-10T14:14:04Z","created_by":"Jan","updated_at":"2026-06-10T14:54:05Z","started_at":"2026-06-10T14:41:29Z","closed_at":"2026-06-10T14:54:05Z","close_reason":"Addressed in PR #155: expanded P8.1 migration/transition/incoming-stock plan detail, added script-level preflight tests, added real-data preflight fixture workflow step, and documented CI gate scope.","labels":["code-review","p8.1","plan","pr-155","product-lifecycle"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-ejx","title":"invariant-preflight CI runs against a fresh empty DB — code-level gate only, not data-level (PR #155)","description":"PR #155's CI workflow runs:\n\n```yaml\n- run: npx prisma migrate deploy --schema prisma/schema.prisma\n- run: npm run invariant-check:preflight\n```\n\nThe DB is freshly migrated, has zero rows, and the invariant reporters scan an empty database. Every invariant check passes trivially because there's nothing to violate. The PR body acknowledges this: 'tenant data checks still need cron/operator runs against tenant databases'.\n\nThis is fine as a **code-level gate** (catches syntax errors, broken imports, queries that throw on empty data, new code paths that violate constraints at write time when the test suite exercises them). But it's NOT a data-level gate.\n\nConcrete misnomers:\n\n1. **Plan.md QG1 closure** says: `fail on report failures or critical invariant findings`. The PR body says the gate `blocks on critical findings/report failures`. Neither is wrong, but readers may assume the gate would catch a stock-quantity violation introduced by a refactor.\n\n2. **An empty-DB invariant report will return 0 findings even if the refactor under PR breaks the invariant logic itself** — unless the refactor causes the reporter to throw, the gate stays green.\n\n3. **The QG1 plan item** original intent: `Synthetic data with a known invariant violation; assert CI fails.` PR's tests assert CI fails ON A MOCKED RESULT but the actual CI workflow doesn't seed any violation.\n\n**Why this matters:**\n\nThe gate provides a false sense of coverage for the kind of bug it's named after ('inventory invariant violation'). A reviewer seeing the green CI badge might assume the underlying data has been verified. In practice, the CI checks that the report **code** runs to completion against an empty DB.\n\n**Suggested strengthening:**\n\n- Seed a small fixture with known invariant violations (negative stock, orphan cost layer, refund status mismatch) → assert the preflight FAILS on this fixture\n- Then RESET the fixture so the main preflight runs against an empty DB and passes\n- Document explicitly in docs/development.md that the CI gate covers code correctness, not tenant data correctness\n- Make the QG2 'per-phase regression test' convention the actual data-level safety net (tested with the focused unit tests on each PR)","acceptance_criteria":"- Add a CI step that seeds a known-violating fixture and asserts `invariant-check:preflight` exits non-zero\n- Then reset/clear the fixture and run the real preflight (which should pass)\n- Update docs/development.md to label the gate explicitly: 'code-correctness gate; data-level invariant checks live in scheduled cron jobs against tenant databases'\n- Update plan.md QG1 entry to reflect the gate's scope","notes":"Filed during PR #155 review. The gate has value (catches breaking code changes to invariant reporters) but the PR claims coverage broader than what it actually delivers.","status":"closed","priority":2,"issue_type":"bug","assignee":"Jan","owner":"dev@onetwo3d.com","created_at":"2026-06-10T14:13:34Z","created_by":"Jan","updated_at":"2026-06-10T14:54:06Z","started_at":"2026-06-10T14:41:30Z","closed_at":"2026-06-10T14:54:06Z","close_reason":"Addressed in PR #155: expanded P8.1 migration/transition/incoming-stock plan detail, added script-level preflight tests, added real-data preflight fixture workflow step, and documented CI gate scope.","labels":["ci","code-review","invariants","pr-155"],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-rrd","title":"Re-add product lifecycle status and EOL feature to plan","description":"Re-add a future plan item for product lifecycle statuses: active, draft, archived, and end-of-life. The feature must support an EOL marker/status that allows sell-off, blocks re-ordering, excludes products from reorder forecasts, supports filtering, and automatically archives EOL products once all warehouse inventory is zero and no incoming stock remains.","status":"closed","priority":2,"issue_type":"task","assignee":"Jan","owner":"info@onetwo3d.co.uk","created_at":"2026-06-10T14:06:36Z","created_by":"Jan","updated_at":"2026-06-10T14:07:17Z","started_at":"2026-06-10T14:06:40Z","closed_at":"2026-06-10T14:07:17Z","close_reason":"Re-added P8.1 to docs/plan.md with active, draft, eol, and archived product statuses plus sell-off, reorder exclusion, filtering, and auto-archive acceptance criteria.","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-fa5","title":"WP plugin invoice-pdf proxy follows unvalidated order-meta endpoint URL — SSRF surface (PR #154)","description":"PR #154's WP plugin (`lib/connectors/woocommerce/wp-plugin/onetwoinventory-helper.php`) adds a customer-facing invoice download endpoint that proxies to IMS:\n\n```php\nfunction oti_proxy_invoice_pdf(WP_REST_Request $request) {\n // ...\n $endpoint = oti_get_order_meta($order_id, '_ims_invoice_pdf_endpoint');\n $secret = (string) get_option(OTI_FX_SECRET_OPT, '');\n if ($endpoint === '' || $secret === '') {\n return new WP_Error('oti_invoice_not_configured', ...);\n }\n // ...\n $response = wp_remote_post($endpoint, [\n 'timeout' =\u003e 30,\n 'headers' =\u003e [\n 'Content-Type' =\u003e 'application/json',\n 'X-OTI-Signature' =\u003e hash_hmac('sha256', $body, $secret),\n ],\n 'body' =\u003e $body,\n ]);\n // ...\n}\n```\n\nThe proxy reads `$endpoint` from per-order meta key `_ims_invoice_pdf_endpoint`. This value is set by IMS (presumably) when generating the PDF. But:\n\n1. **Where does meta set?** Order meta is writable from many places: WP admin UI, REST API (with permissions), other plugins, direct DB. If an attacker can write to that meta key (compromised admin, malicious plugin, SQL injection via another vector), they redirect the proxy to an attacker-controlled URL.\n\n2. **No URL validation.** The endpoint is used verbatim — no scheme allowlist (http vs https), no domain allowlist, no DNS-rebinding protection. A malicious endpoint can be:\n - `https://attacker.example.com/` — capture the HMAC-signed payload\n - `http://169.254.169.254/...` — AWS metadata service (SSRF to cloud metadata)\n - `http://localhost:3306/...` — local services on the WP host\n - `gopher://...` — protocol smuggling\n\n3. **HMAC payload exfiltration.** Even if the response body is discarded by the plugin (which it actually uses for the PDF body, so the attacker controls what the customer downloads), the OUTBOUND request to the attacker URL contains the HMAC signature for the payload. The attacker now has a (body, signature) pair signed with the shared secret — confirms the secret hasn't rotated and provides material for further forgery.\n\n4. **PDF content substitution.** The proxy echoes `$pdf = wp_remote_retrieve_body($response);` as the customer-facing PDF. So if attacker controls the endpoint, attacker delivers a malicious PDF to the customer (phishing, malware-laden PDF, etc.) under the trusted WC site origin.\n\n**Why this matters:** The proxy is a server-side fetch with HMAC signing. The endpoint is meant to be a trusted IMS URL, but the code treats per-order meta as the source of truth without validating it's still pointing at IMS.\n\n**Mitigation:**\n\nPin the endpoint base URL in WP site options (set once during plugin configuration, signed/protected by admin-only access), and reject per-order meta endpoints. The order id can still come from meta; only the BASE URL must be pinned.","acceptance_criteria":"- Add a new site option `oti_ims_invoice_pdf_base_url` to the plugin settings page (admin-only)\n- Construct the endpoint server-side: `${base_url}/api/shopping/woocommerce/invoice-pdf` — ignore per-order meta for the URL\n- Drop the `_ims_invoice_pdf_endpoint` meta key entirely (or keep it as advisory only, with hash verification against the pinned base)\n- Add tests / docs explaining the trust boundary: WP plugin trusts WP site options (admin-set); does not trust per-order meta for URL choice\n- Audit any other places in the plugin that take URLs from meta or external data","notes":"Filed during PR #154 review. SSRF surface in a customer-facing plugin endpoint, mitigated by 'attacker needs to write order meta' but the attack surface is real. Mid-priority because the plugin is in OneTwo3D's deployment scope, not a public marketplace plugin — easier to audit and patch.","status":"open","priority":2,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-10T11:41:38Z","created_by":"Jan","updated_at":"2026-06-10T11:41:38Z","labels":["code-review","pr-154","security","ssrf","wordpress"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-0qv","title":"Shopping invoice PDF nonce not tracked — replay within 5-minute TTL is unbounded (PR #154)","description":"PR #154's `parseShoppingInvoicePdfRequest` validates that the payload includes a `nonce` (non-empty string) but doesn't track which nonces have been seen.\n\n```ts\n// app/api/shopping/[connector]/invoice-pdf/route.ts\nconst parsed = parseShoppingInvoicePdfRequest(rawBody, connector, { now: dependencies.now })\nif (!parsed.valid) return reject(dependencies, auditBase, parsed.reason)\n// nonce is parsed but never checked against a 'seen' set\nconst order = await dependencies.findOrder(parsed.request)\n```\n\nThe PR's stated security model includes the nonce, but the IMS side accepts any non-empty UUID-shape string. So a valid HMAC-signed payload can be REPLAYED:\n\n- Same payload, same X-OTI-Signature, same timestamps\n- IMS validates HMAC (✓ — body is unchanged)\n- IMS parses payload (✓ — fields are still valid)\n- IMS checks issuedAt+TTL (✓ — still within 5 minutes)\n- IMS returns the PDF\n\nSo within the 5-minute TTL window, the SAME signed payload can be replayed N times. Rate limit caps at 60 req/min per IP, but:\n\n- N = 60/min × 5min = up to 300 replays per IP for the same valid payload\n- Across multiple IPs (proxied attacker, residential proxy networks), much higher\n\n**Why this matters:**\n\nFor most flows this is moot — the PDF doesn't change, the customer downloads it once. But:\n\n1. **PDF generation cost**: `loadInvoicePdf` reads from disk; cheap. If a future change generates the PDF on-demand, 300× replay = 300× generation cost.\n2. **Activity log spam**: each verified-and-accepted attempt is logged. The audit trail bloats with duplicates.\n3. **Combined with onetwo3d-ims-ecp (guest bypass)**: a compromised secret + enumerable guest order IDs means an attacker exfiltrates all guest invoices with bounded HMAC-signature work — and the nonce doesn't slow this down at all because they only need to sign once per order.\n\n**Standard nonce-tracking:**\n\n```ts\n// On verify, check the nonce hasn't been seen in the TTL window\nconst existing = await db.shoppingInvoicePdfNonce.findUnique({ where: { nonce } })\nif (existing) return reject(dependencies, auditBase, 'replay')\nawait db.shoppingInvoicePdfNonce.create({ data: { nonce, expiresAt } })\n// Periodic sweep clears expired nonces\n```\n\nOr use a TTL-bounded cache (Redis SET NX with EX). Simple, cheap, defeats replay.","acceptance_criteria":"- Add nonce tracking to the shopping invoice PDF route:\n - Option A (db): new table 'shopping_invoice_pdf_nonces' with (nonce, expiresAt); index on nonce; periodic sweep\n - Option B (redis/in-memory): if rate-limit backend already uses redis, store nonces there with TTL = TOKEN_TTL_SECONDS + clock skew\n- Tests: assert second request with same payload returns 'replay' rejection (or appropriate failure reason)\n- Update the failure-reason union: add 'replay_detected'\n- Document the nonce contract in lib/shopping-invoice-pdf.ts JSDoc","notes":"Filed during PR #154 review. Mid-priority — replay is bounded by HMAC secret possession AND TTL AND rate limit. But the design intent of 'nonce' is one-shot, and the implementation is currently advisory only.","status":"open","priority":2,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-10T11:41:09Z","created_by":"Jan","updated_at":"2026-06-10T11:41:09Z","labels":["code-review","pr-154","replay-protection","security"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-d5l","title":"Shopping invoice PDF HMAC reuses wc_webhook_secret — key reuse across two purposes (PR #154)","description":"PR #154's `lib/shopping-invoice-pdf.ts`:\n\n```ts\nconst SHOPPING_INVOICE_SECRET_SETTING: Record\u003cShoppingConnectorId, string\u003e = {\n woocommerce: 'wc_webhook_secret',\n shopify: 'shopify_webhook_secret',\n}\n```\n\nThe same `wc_webhook_secret` is used for:\n\n1. **Incoming webhook verification**: WooCommerce → IMS posts an event, signs body with this secret, IMS verifies the signature.\n2. **Outgoing PDF request signing**: WP plugin → IMS posts a PDF request, signs body with this secret, IMS verifies the signature.\n\nThis is **key reuse across distinct purposes** — a well-known cryptographic anti-pattern (RFC 4949 §3.4). Concrete risks:\n\n1. **Cross-protocol replay**: a signed message intended for one purpose might be reinterpreted as another. WC webhook payloads and PDF request payloads are different JSON shapes, but:\n - A signed payload that happens to be valid JSON for BOTH schemas could be replayed across endpoints\n - The PDF schema is small (5-6 fields) — there's nontrivial overlap with hypothetical webhook payloads\n - Defense-in-depth principle: don't rely on schema differences to keep purposes separate\n\n2. **Leak amplification**: if the webhook secret leaks (e.g., from a WordPress site backup, a wp-config dump, a compromised admin), the attacker gets BOTH:\n - Ability to forge webhooks (data injection)\n - Ability to forge PDF requests (data exfiltration — coupled with the guest-checkout bypass in onetwo3d-ims-ecp this is total invoice access)\n\n3. **Rotation amplification**: rotating the webhook secret because of a webhook abuse incident also invalidates ALL pending PDF requests, breaking customer downloads. Same with rotating because of a PDF-request abuse incident. Coupling rotation events couples incident response.\n\n**Cryptographic best practice:**\n\nEach distinct cryptographic purpose gets its own key. Or use HKDF to derive purpose-specific subkeys from a single root:\n\n```ts\nhmacHex(`shopping-invoice-pdf:${connector}`, rootSecret) // derive\n// store derived key, use for signing/verifying PDF requests only\n```\n\n**Why this matters:** The HMAC contract here is the only authorization for the new shopping invoice PDF route. Cryptographic hygiene of the secret directly limits the abuse surface.","acceptance_criteria":"- Add a NEW secret `wc_invoice_pdf_secret` (and `shopify_invoice_pdf_secret`) distinct from the webhook secret\n- WP plugin's settings page should expose a separate field for this; OR derive the PDF secret from the webhook secret via HKDF with a purpose label\n- Test that a payload signed with the webhook secret is REJECTED by the PDF route (cross-protocol replay defense)\n- Document the key separation in docs/architecture.md and the helper plugin's README\n- Migration: existing installations need a settings UI to set the new secret OR documentation for the derived-key approach","notes":"Filed during PR #154 review. Pair with onetwo3d-ims-ecp because both undermine the PDF authorization layer, just from different angles.","status":"open","priority":2,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-10T11:40:41Z","created_by":"Jan","updated_at":"2026-06-10T11:40:41Z","labels":["code-review","hmac","pr-154","security","shopping"],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-0nk","title":"Add customer-safe shopping invoice PDF downloads","description":"Add a customer-facing invoice PDF delivery path for shopping connectors. IMS admin invoice links remain session/IP-bound; shopping platforms should enforce customer login/order ownership and call IMS server-to-server with a short-lived signed request. WooCommerce is the first integration, but the IMS side must use a shared shopping-connector contract rather than a WooCommerce-only token.","status":"closed","priority":2,"issue_type":"feature","assignee":"Jan","owner":"info@onetwo3d.co.uk","created_at":"2026-06-10T10:54:40Z","created_by":"Jan","updated_at":"2026-06-10T11:05:34Z","started_at":"2026-06-10T10:54:50Z","closed_at":"2026-06-10T11:05:34Z","close_reason":"Added connector-neutral shopping invoice PDF handoff route, WooCommerce helper-plugin customer proxy, metadata push, docs, route policy, and security tests.","dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-lkg","title":"Phase 6 security hardening follow-up","description":"Complete the remaining Phase 6 security-hardening plan group: supplier portal ownership assertions, invoice PDF token session/IP binding and filename hardening, generic WooCommerce configuration errors, stronger user password policy, and plan status updates.","status":"closed","priority":2,"issue_type":"task","assignee":"Jan","owner":"info@onetwo3d.co.uk","created_at":"2026-06-10T10:38:51Z","created_by":"Jan","updated_at":"2026-06-10T10:48:40Z","started_at":"2026-06-10T10:38:56Z","closed_at":"2026-06-10T10:48:40Z","close_reason":"Completed the remaining Phase 6 security-hardening group: supplier boundary helper, bound invoice PDF tokens, generic WooCommerce config errors, password policy, activity-log redaction coverage, docs and plan updates.","labels":["plan","security"],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-6f4","title":"P2.3 sidebar report access cleanup","description":"Implement docs/plan.md suggested group 19: P2.3 / CR5. Remove the duplicated sidebar analytics conditional-spread pattern, use REPORT_ACCESS_GROUPS as the single source for report access, add per-role coverage, and tick the plan item when complete.","status":"closed","priority":2,"issue_type":"task","assignee":"Jan","owner":"info@onetwo3d.co.uk","created_at":"2026-06-10T10:13:44Z","created_by":"Jan","updated_at":"2026-06-10T10:17:36Z","started_at":"2026-06-10T10:13:56Z","closed_at":"2026-06-10T10:17:36Z","close_reason":"Implemented group 19 sidebar cleanup: analytics navigation now uses a single report-access grouping helper, per-role link coverage prevents duplicate or leaked report links, and docs/plan.md marks P2.3/CR5 complete.","labels":["plan","pr-group-19","sidebar"],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-bjv","title":"Add integration-level connection gate save/record regression for PR #152","description":"PR #152 has helper-level coverage for the integration connection-test gate, but no integration-level test exercising a real integration save path through: save blocked before a passing test, test records success metadata, save then succeeds for the same configuration fingerprint, stale configuration is blocked again. Add focused mocked coverage for at least one integration.","status":"closed","priority":2,"issue_type":"bug","assignee":"Jan","owner":"info@onetwo3d.co.uk","created_at":"2026-06-10T09:53:03Z","created_by":"Jan","updated_at":"2026-06-10T09:56:51Z","started_at":"2026-06-10T09:56:46Z","closed_at":"2026-06-10T09:56:51Z","close_reason":"Added WooCommerce integration-level connection gate regression covering blocked enablement before test, successful record persistence, allowed enablement for matching fingerprint, and stale-fingerprint blocking.","labels":["code-review","pr-152","test-coverage"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-ym4","title":"shouldGateWcEnable only checks first transition — config changes mid-stream bypass the gate (PR #152)","description":"PR #152's WC gate:\n\n```ts\nconst WC_ENABLE_SETTING_KEYS = [\n 'wc_sync_enabled',\n 'wc_sync_product_enabled',\n 'wc_stock_sync_enabled',\n 'wc_fx_push_enabled',\n] as const\n\nasync function shouldGateWcEnable(data: Partial\u003cWcSyncSettings\u003e): Promise\u003cboolean\u003e {\n const enabledKeys = WC_ENABLE_SETTING_KEYS.filter((key) =\u003e data[key] === 'true')\n if (enabledKeys.length === 0) return false\n const current = await getSettingValues([...enabledKeys])\n return enabledKeys.some((key) =\u003e current.get(key) !== 'true')\n}\n```\n\nThe gate fires only when AT LEAST ONE of the four enable-toggles TRANSITIONS from non-'true' to 'true'. Once a toggle is on, subsequent saves with the same toggle still 'true' DON'T fire the gate — even if other settings change.\n\n**Concrete bypass:**\n\n1. Operator tests WC connection with credentials X. Test passes.\n2. Operator saves `wc_sync_enabled = true`. Gate fires (transition off→on), passes (fingerprint matches), settings saved.\n3. Operator changes `wc_webhook_secret`, `wc_sync_interval_minutes`, or any other setting EXCEPT the 4 enable toggles. Calls `saveWcSyncSettings` again.\n4. Gate does NOT fire (no toggle transition). Settings saved.\n\nIf the changed setting is `wc_webhook_secret` (which affects webhook payload validation), the operator has saved a new secret without verifying it works. WC starts sending webhooks; IMS rejects them because the secret doesn't match what WC was configured with.\n\n**Why this matters:**\n\nThe gate's name suggests 'enabling sync requires a test'. But once enabled, any subsequent config change can land without retest. For settings that affect runtime behavior (webhook secret, base URL change, intervals), this is a real path to silent breakage.\n\n**Related observation:** `saveWcCredentials` (separate action) DOES record its own test result. But settings changes via `saveWcSyncSettings` (different action) DON'T retrigger the gate.","acceptance_criteria":"- Decide policy:\n - **Option A (strict)**: gate fires on ANY save when ANY of the enable-toggles is currently true OR being set to true\n - **Option B (settings-specific)**: identify which settings affect connectivity/runtime and gate ONLY when those change AND something is enabled\n - **Option C (status quo)**: keep first-transition gating, but document the limit explicitly\n- For SMTP/Mintsoft/Xero, audit the same pattern — when does the gate fire vs not?\n- Add a regression test for each integration that exercises 'enabled-but-config-changed' → assert gate fires (per chosen policy)","notes":"Filed during PR #152 review. The current shape catches the obvious 'enable for the first time' case but misses ongoing configuration drift.","status":"open","priority":2,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-10T09:50:09Z","created_by":"Jan","updated_at":"2026-06-10T09:50:09Z","labels":["code-review","integrations","pr-152","woocommerce"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-16r","title":"Xero connection fingerprint doesn't include xero_client_secret — secret rotation bypasses the gate (PR #152)","description":"PR #152's Xero fingerprint:\n\n```ts\nasync function buildXeroConnectionFingerprint(): Promise\u003cstring\u003e {\n const [clientId, expectedTenantId, token] = await Promise.all([\n getSettingValue('xero_client_id'),\n getSettingValue('xero_expected_tenant_id'),\n db.accountingToken.findUnique({\n where: { connector: 'xero' },\n select: { tenantId: true, tenantName: true },\n }),\n ])\n return buildIntegrationConnectionFingerprint({\n clientId, expectedTenantId,\n tenantId: token?.tenantId ?? '',\n tenantName: token?.tenantName ?? '',\n })\n}\n```\n\nThe fingerprint includes `clientId`, `expectedTenantId`, `tenantId`, `tenantName` — but NOT `xero_client_secret`.\n\nIf an operator rotates `xero_client_secret` (e.g., after a suspected compromise, after a Xero app rotation) without re-testing, the fingerprint is UNCHANGED. The gate accepts the previous test's success result. The sync is enabled with an unverified secret.\n\nIf the new secret is correct, the sync just works (no harm). If the new secret is wrong (typo, copy-paste error), the sync FAILS on every API call but the gate said 'all good'. Operator sees confusing errors with no test-verified-baseline to compare against.\n\n**Why this matters:**\n\nThe gate's stated purpose (per the PR description) is 'require successful current-fingerprint connection tests before enabling sync'. The intent is to prevent enabling with invalid settings. Missing `client_secret` from the fingerprint creates a path where settings change AND the gate accepts the old result.\n\nSame audit needed for:\n- **WooCommerce** (`buildWcConnectionFingerprint`): includes url/key/secret — but does the codebase have other WC settings that affect the connection? (e.g., wc_api_version, wc_user_agent, etc.) If yes, those should be in the fingerprint too.\n- **Mintsoft** (`buildMintsoftConnectionFingerprint`): includes baseUrl/username/password/orderLookupConnector — but Mintsoft has additional config (webhook secrets, etc.) — check what's relevant.\n- **SMTP**: includes host/port/user/pass/secure/fromName/fromEmail/replyTo — looks comprehensive.\n\n**Concrete repro for Xero:**\n1. Configure Xero with current valid client_id + client_secret. Test passes; status='success'; fingerprint stored.\n2. Rotate `xero_client_secret` to a typo'd value via settings UI (assume no in-place validation).\n3. Toggle xero_sync_enabled off → on. The gate checks fingerprint: unchanged (still the same client_id etc.). Status is success. Gate PASSES.\n4. Next sync API call fails with 'unauthorized' from Xero.","acceptance_criteria":"- Audit each integration's fingerprint inputs against ALL settings that affect the connection:\n - Xero: add `clientSecret` (or its server-side HMAC per onetwo3d-ims-v6z)\n - WooCommerce: audit for missed settings\n - Mintsoft: audit for missed settings\n- Document in lib/integration-connection-test-gate.ts JSDoc the contract for fingerprint inputs ('include every setting that affects connectivity')\n- Add a focused test: for each integration, change each input field and assert the fingerprint changes\n- Coordinate with onetwo3d-ims-v6z (secret-in-fingerprint) — the fix for that may change how secrets are included","notes":"Filed during PR #152 review. Coupled with v6z because both concern the fingerprint's coverage of secret values.","status":"open","priority":2,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-10T09:49:46Z","created_by":"Jan","updated_at":"2026-06-10T09:49:46Z","labels":["code-review","integrations","pr-152","security","xero"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-iys","title":"recordIntegrationConnectionTest does 4 separate upserts not in transaction — partial-write corrupts gate state (PR #152)","description":"PR #152's `recordIntegrationConnectionTest` writes 4 settings rows via `Promise.all` of 4 separate `upsert` calls:\n\n```ts\nawait Promise.all(TEST_KEYS.map((suffix) =\u003e\n repository.upsert({\n where: { key: testSettingKey(id, suffix) },\n create: { key: testSettingKey(id, suffix), value: values[suffix] },\n update: { value: values[suffix] },\n }),\n))\n```\n\nNot wrapped in a Prisma `$transaction`. If 1-3 of the 4 upserts fail (e.g., connection blip mid-batch, lock timeout), partial state lands:\n\n- `status: 'success'` written ✓\n- `tested_at` written ✓\n- `message` FAILED — keeps old value (e.g., 'Connection refused')\n- `fingerprint` FAILED — keeps old value (or null on first test)\n\nThe subsequent `getIntegrationConnectionTestState` reads these as 4 independent values, returning a mixed state:\n\n- `status: 'success'` + `fingerprint: \u003cstale\u003e` → gate compares against current expected fingerprint → MISMATCH → 'Retest the connection because settings changed' (false negative, user retests pointlessly)\n- OR worse: `status: 'success'` + `fingerprint: null` if first-ever test → gate says 'Retest because the saved settings changed' — but the test DID pass; operator confused\n\nThe more dangerous case: previous test was `status: 'failed'` with the CURRENT fingerprint. Operator clicks Test, the upserts partially succeed — `status: 'success'` written, but message + fingerprint failed. Gate evaluates: status=success + fingerprint matches (the OLD fingerprint that happens to be the current one) → GATE PASSES even though the test result is half-recorded and incoherent.\n\n**Why this matters:** The gate's job is to be a reliable trust signal. Partial writes that mix old + new fields produce reliability gaps in exactly the scenario the gate is meant to handle (settings flux).\n\n**Concrete repro:**\n1. Force a DB connection pool exhaustion during `recordIntegrationConnectionTest` (e.g., concurrent stress test)\n2. Observe 4 settings rows where some have new value, others have old value\n3. Hit any of the gate-checking save flows; observe the gate's verdict doesn't match what the operator just did","acceptance_criteria":"- Wrap the 4 upserts in `db.$transaction`:\n ```ts\n await db.$transaction(TEST_KEYS.map((suffix) =\u003e\n db.setting.upsert({ ... })\n ))\n ```\n Or use Prisma's interactive transaction if the repository abstraction needs adjustments.\n- Add a test that simulates upsert failure for one of the 4 keys and asserts that NONE land (atomic rollback)\n- Consider: store the 4 fields as JSON in a single setting row instead of 4 rows. Single write, no atomicity concern, but loses the 'individual key' query convenience. Trade-off.","notes":"Filed during PR #152 review. The Promise.all-of-upserts pattern is the same shape that's caused partial-state bugs in many systems. The fix is one transaction wrapper.","status":"open","priority":2,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-10T09:49:20Z","created_by":"Jan","updated_at":"2026-06-10T09:49:20Z","labels":["code-review","consistency","integrations","pr-152"],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-lx2","title":"P7.5 integration settings test connection gate","description":"Implement the plan item P7.5: add test-connection gating for integration settings so Xero, WooCommerce, Mintsoft, and SMTP cannot be enabled until their latest saved configuration has a successful connection test. Store last-test timestamp and result on settings records, add focused mocked tests, and tick the plan item when complete.","status":"closed","priority":2,"issue_type":"feature","assignee":"Jan","owner":"info@onetwo3d.co.uk","created_at":"2026-06-10T09:16:51Z","created_by":"Jan","updated_at":"2026-06-10T09:30:23Z","started_at":"2026-06-10T09:16:57Z","closed_at":"2026-06-10T09:30:23Z","close_reason":"Implemented P7.5 integration connection-test gate, UI test controls/status, docs, and focused tests.","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-cgq","title":"CSV export metadata tests assert via raw source-code regex — brittle, no behavior check (PR #151)","description":"PR #151's regression test for the route schemas (`tests/api/export-csv-metadata.test.ts:48-56`):\n\n```ts\ntest('inventory report export route schemas do not repeat report metadata per CSV row', async () =\u003e {\n const stockPosition = await readFile('app/api/export/stock-position/route.ts', 'utf8')\n assert.doesNotMatch(stockPosition, /'totalValueBase', 'asOf', 'source', 'generatedAt', 'reservationQtySource'/)\n assert.doesNotMatch(stockPosition, /'movementCount', 'dateFrom', 'dateTo', 'generatedAt'/)\n // ... similar for inventory-ledger, inventory-costing\n})\n```\n\nThree problems:\n\n**1. The test reads source code as text, not behavior**\n- A code reformat (single quotes → double quotes, line break inserted, schema array split across multiple lines) breaks the test even when behavior is correct\n- A regression that drops the metadata HEADER while keeping the row schema cleanup PASSES — the negative assertion only checks that specific substring patterns aren't present in the source\n\n**2. The negative-substring patterns are unstable**\n- The regex `/'totalValueBase', 'asOf', 'source', 'generatedAt', 'reservationQtySource'/` requires EXACTLY this order with this whitespace. Any future schema reorder (e.g. moving 'reservationQtySource' earlier in the list) would make this pass even if 'asOf' is reintroduced\n- The regex doesn't actually assert that the desired columns ARE present — only that an exact bad substring isn't\n\n**3. No HTTP integration test**\nThe new behaviors (metadata header, row schema cleanup) deserve route-level tests:\n- Call the route handler with a mock request\n- Assert the response `Content-Type` is `text/csv`\n- Assert the response body's first line matches the new column header list\n- Assert the response header `X-IMS-Export-Metadata` contains the expected JSON\n- Assert the metadata JSON does NOT contain row-level fields that should remain per-row\n\nThe first two tests (`csvResponse` / `csvBufferedStreamResponse` lower-level helpers) are correct — they test behavior. The route-level test is the gap.\n\n**Why this matters:** Future contributors looking at the test signal will see 'green tests' and assume the route schema is locked. But the schema can drift arbitrarily as long as it doesn't match the exact substring patterns the test happened to encode.","acceptance_criteria":"- Replace the source-code regex test with a route-level integration test:\n - Mock the report builder to return a fixed report\n - Invoke the route's GET handler\n - Assert response body's first CSV line equals the expected schema string\n - Assert response header X-IMS-Export-Metadata equals the expected JSON\n- Do this for at least one variant of each affected route (stock-position, inventory-ledger, inventory-costing)\n- The original source-code regex test can stay as a smoke test if desired, but it should not be the only signal","notes":"Filed during PR #151 review. Same 'test by inspection, not behavior' pattern as 9pk (PR #150). Filed independently because this one is a fresh source-code-string test that's strictly worse than a real integration test would be.","status":"open","priority":2,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-09T22:34:40Z","created_by":"Jan","updated_at":"2026-06-09T22:34:40Z","labels":["csv-export","pr-151","tests"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-ci1","title":"Removal of metadata columns is a breaking CSV schema change without consumer migration guidance (PR #151)","description":"PR #151 removes columns that were previously in every export row:\n\n- `stock-position`: drops `asOf`, `source`, `generatedAt` from stock-on-hand; `generatedAt` from stock-allocations; `dateFrom`, `dateTo`, `generatedAt` from negative-stock\n- `inventory-ledger`: drops `openingQty`, `movementQty`, `closingQty`, `openingValueBase`, `movementValueBase`, `closingValueBase`, `generatedAt` from stock-movements; `generatedAt` from stock-adjustments, transfers, stock-counts\n- `inventory-costing`: drops `asOf`, `source`, `valueReplayReliable`, `generatedAt` from inventory-valuation; `dateFrom`, `dateTo`, `groupBy`, `generatedAt` from cogs; `dateFrom`, `dateTo`, `generatedAt` from landed-cost\n\nAnyone with existing consumer workflows that depend on these columns breaks:\n\n1. **Excel / Power Query connections** — saved import templates with column mappings expect specific schemas; missing columns produce empty values or errors\n2. **BI tools** (Looker, Metabase, Tableau, etc.) reading the CSV files have hardcoded column lists\n3. **Custom scripts** (Python pandas, R, etc.) reading the exports for finance reporting\n4. **Audit trails** — historical practice of looking at the CSV and seeing 'asOf' / 'generatedAt' embedded for every row no longer works; you have to inspect the HTTP header which most browsers' 'Save As' workflow discards\n\nThe metadata is now in the HTTP response header (`X-IMS-Export-Metadata`). But:\n- Browsers' standard 'Save As CSV' file doesn't preserve HTTP headers\n- BI tools' CSV importers don't read response headers\n- A user downloading from the dashboard ends up with a CSV file that has lost the metadata entirely\n\n**Why this matters:** P7.4's original framing was 'CSV exports embed report-level metadata per row' — the FIX is to remove the duplication. But the solution should preserve operator access to the metadata. Putting it in an HTTP header is correct for API consumers but loses information for the most common consumer (operator downloads a CSV).\n\n**Possible mitigations:**\n- Keep ONE header row of metadata at the top of the CSV (before the schema header row) — e.g. `# generatedAt=...; source=...; asOf=...`\n- Or: include a sidecar JSON file in a multipart response (`Content-Type: multipart/mixed`)\n- Or: write the metadata into the CSV filename (`stock-on-hand-2026-06-09T10-00-snapshot-source.csv`)\n\nCHANGELOG entry is also missing for this breaking change to downstream consumers.","acceptance_criteria":"- Decide consumer-preservation strategy:\n - **Option A**: CSV preamble comment row(s) with metadata (sigil = '#') — most BI tools support skipping initial '#' lines\n - **Option B**: Filename encodes critical metadata (date, source) for at-a-glance file identification in the user's downloads folder\n - **Option C**: Accept the loss; add UI hint on the export button: 'Generated at: {timestamp}; source: {x}' so the operator can copy if needed\n- Add a CHANGELOG entry calling out the schema change with guidance for affected consumers\n- Update docs/architecture.md's CSV section to explain the operator-vs-API metadata access patterns\n- Consider a backward-compat env var (`CSV_EXPORT_INCLUDE_METADATA_COLUMNS=true`) for tenants with rigid downstream consumers","notes":"Filed during PR #151 review. The PR closes P7.4 by removing the duplication, but the solution favors API consumers over operator consumers. Worth getting finance/ops sign-off before this lands.","status":"open","priority":2,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-09T22:34:17Z","created_by":"Jan","updated_at":"2026-06-09T22:34:17Z","labels":["breaking-change","code-review","csv-export","pr-151"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-kku","title":"X-IMS-Export-Metadata header has no size cap or encoding guard — risks broken responses (PR #151)","description":"PR #151 adds a custom HTTP response header carrying JSON-encoded report metadata:\n\n```ts\n// lib/csv.ts\nif (metadata \u0026\u0026 Object.keys(metadata).length \u003e 0) {\n headers[CSV_EXPORT_METADATA_HEADER] = JSON.stringify(metadata)\n}\n```\n\nUsed like:\n```ts\ncsvBufferedStreamResponse(rows, schema, filename, {\n generatedAt: report.generatedAt,\n ...report.totals,\n})\n```\n\nTwo real risks:\n\n**1. Size**\nHTTP headers are typically capped around 8KB total (nginx `large_client_header_buffers`, AWS ALB ~16KB, etc.). The metadata is built from `{ ...report.totals }` — whatever the report defines. Today the totals are flat strings/numbers (e.g. `openingQty: '1.000000'`), well under 8KB. But:\n\n- If a future change adds a structured field (e.g. `perWarehouseTotals: [{warehouseId: ..., qty: ...}, ...]` for a tenant with 100 warehouses), the header silently exceeds proxy limits\n- nginx and most proxies reject the whole response with 502 / 500 when a single header exceeds the buffer\n- The user sees a generic 'export failed' with no actionable error\n\n**2. Encoding**\nHTTP header values per RFC 7230 are limited to visible US-ASCII characters (or use proper RFC 8187 encoding). `JSON.stringify` of metadata can include:\n- Non-ASCII warehouse / supplier names: `'café Berlin'`\n- Currency symbols: `'£', '€', '¥'`\n- Unicode in product names, customer names\n\nNode.js's HTTP layer is lenient enough to send these as UTF-8 bytes, but strict reverse proxies (some CDNs, security appliances) will reject the response.\n\n**Why this matters:** Today's headers fit; an analytics enhancement next quarter that 'just' adds a structured breakdown to totals will silently break exports for production tenants behind nginx/CDNs.\n\n**Defensive approach:**\n- Cap the JSON-encoded header to ~4KB; if exceeded, drop to a fallback (omit large fields, log a warning, or fall back to a small subset of essential metadata)\n- Base64-encode the JSON payload to guarantee ASCII-safety, OR percent-encode it, OR use the RFC 8187 `filename*=UTF-8''...` style\n- Document the contract in JSDoc: 'metadata must be a flat object with primitive values; total JSON encoding must be under N bytes'","acceptance_criteria":"- Add a size guard in `csvHeaders` that:\n - Computes the JSON-encoded length\n - If under threshold (e.g., 4KB), uses as-is\n - If over, either drops non-essential fields, truncates with an indicator, or omits the header and logs a warning\n- Base64-encode the metadata value to guarantee header-safe characters\n- Update tests in `tests/api/export-csv-metadata.test.ts` to:\n - Assert a large metadata payload is rejected/truncated gracefully\n - Assert non-ASCII content (e.g. `generatedBy: 'café Admin'`) round-trips safely\n- Document the contract in CSV_EXPORT_METADATA_HEADER's JSDoc","notes":"Filed during PR #151 review. Not blocking today's working examples but the contract has no upper bound, which is the foot-gun.","status":"open","priority":2,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-09T22:33:48Z","created_by":"Jan","updated_at":"2026-06-09T22:33:48Z","labels":["code-review","csv-export","http","pr-151"],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-iuw","title":"Implement plan group 17 CSV export cleanup","description":"Implement docs/plan.md suggested group 17: P7.4 and CR4. Sweep analytics /api/export/* routes to drop repeated report-level metadata from per-row CSV output, keep export metadata available once per export where appropriate, and add focused route/header tests.","status":"closed","priority":2,"issue_type":"task","assignee":"Jan","owner":"info@onetwo3d.co.uk","created_at":"2026-06-09T22:13:40Z","created_by":"Jan","updated_at":"2026-06-09T22:19:04Z","started_at":"2026-06-09T22:13:49Z","closed_at":"2026-06-09T22:19:04Z","close_reason":"Implemented plan group 17 CSV export cleanup with metadata emitted once per export and focused schema tests.","dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-vv9","title":"Fix PR #150 review beads","description":"Address PR #150 review findings from the GitHub review summary: strict parseDateOnly validation, timestamp-safe report date upper bounds, centralized analytics empty totals/page wrappers, InventoryHealthSourceLimitError field shadowing, and loader wrapper tests.","status":"closed","priority":2,"issue_type":"task","assignee":"Jan","owner":"info@onetwo3d.co.uk","created_at":"2026-06-09T21:27:59Z","created_by":"Jan","updated_at":"2026-06-09T21:38:02Z","started_at":"2026-06-09T21:28:05Z","closed_at":"2026-06-09T21:38:02Z","close_reason":"Fixed PR #150 review findings: strict date-only validation, exclusive timestamp report bounds, centralized analytics page empty totals/wrappers, inventory health error cleanup, and wrapper tests. Validation passes.","labels":["code-review","pr-150"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-ei2","title":"loadFinanceAnalyticsReportForPage requires hard-coded emptyTotals at every call site — fragile when totals shape evolves (PR #150)","description":"PR #150 introduces a page-level wrapper pattern for source-limit handling:\\n\\n```ts\\n// finance-analytics-page-utils.ts\\nexport async function loadFinanceAnalyticsReportForPage\u003cRow\u003e(\\n filters,\\n load,\\n emptyTotals: Record\u003cstring, string\u003e,\\n): Promise\u003cFinanceAnalyticsReport\u003cRow\u003e\u003e {\\n try { return await load(filters) } catch (error) {\\n if (isSourceScanTooLargeError(error)) return emptyFinanceAnalyticsReportForSourceLimit\u003cRow\u003e(filters, error, emptyTotals)\\n throw error\\n }\\n}\\n```\\n\\nAnd at each call site:\\n\\n```ts\\n// app/(dashboard)/analytics/ar-aging/page.tsx\\nconst report = await loadFinanceAnalyticsReportForPage(filters, getArAgingReport, {\\n outstandingBase: '0', creditBalanceBase: '0',\\n bucket1Days: '30', bucket2Days: '60', bucket3Days: '90',\\n})\\n```\\n\\nThe `emptyTotals` object is hand-built per page. It must match the shape of the report's `totals` to satisfy TypeScript AND avoid 'undefined' in the rendered notice-empty state.\\n\\n**Problems:**\\n\\n1. **Drift risk**: when `getArAgingReport` adds a new totals field (e.g. `overdueBucket`), every page calling it needs to update its `emptyTotals`. If forgotten, the empty state has missing totals fields. TypeScript will catch it IF the report's totals type is strict enough; if it uses `Record\u003cstring, string\u003e`, the drift is silent.\\n2. **Duplication**: AP-aging and AR-aging both build similar emptyTotals objects with overlapping bucket fields. Drift between similar reports.\\n3. **Magic values**: `bucket1Days: '30'` is a config value (default bucket boundary). Having it in BOTH the report's true totals AND the empty totals means two places to change if defaults change.\\n\\n**Why this matters:** The PR consolidates source-limit handling but spreads a new fragility (per-page emptyTotals) across ~15 analytics pages. The fragility shape was traded, not eliminated.","acceptance_criteria":"- Move `emptyTotals` derivation INTO the report's domain module (e.g. `getArAgingReportEmptyTotals()` or a static `AR_AGING_EMPTY_TOTALS` constant) — call it from the page wrapper\\n- Or: have `emptyFinanceAnalyticsReportForSourceLimit` accept a 'sentinel value' string and zero-fill all expected totals keys derived from a TypeScript-generated keyof report['totals']\\n- Or: pass the report function as a single argument and have the wrapper introspect the function's signature for the empty shape (least magic; most explicit)\\n- Update at least 2 page calls (AR aging + currency summary) to demonstrate the chosen pattern\\n- Add a type-level test asserting emptyTotals keys match the report's totals keys","notes":"Filed during PR #150 review. The page wrapper is clean; the per-page hardcoded emptyTotals is the rough edge. Worth deciding the long-term shape before this pattern proliferates to many more pages.","status":"open","priority":2,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-09T21:20:22Z","created_by":"Jan","updated_at":"2026-06-09T21:20:22Z","labels":["analytics","code-review","pr-150","refactor"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-5h0","title":"parseDateOnly silently accepts invalid dates like 2026-13-45 due to JS Date normalization (PR #150)","description":"PR #150 introduces `lib/domain/math/date-window.ts`:\\n\\n```ts\\nconst DATE_ONLY_RE = /^(\\d{4})-(\\d{2})-(\\d{2})$/\\n\\nexport function parseDateOnly(value, fallback, options = {}) {\\n if (!value) return fallback\\n const match = DATE_ONLY_RE.exec(value)\\n if (!match) return fallback\\n const parsed = new Date(Date.UTC(Number(match[1]), Number(match[2]) - 1, Number(match[3])))\\n return options.endOfDay ? endOfUtcDay(parsed) : startOfUtcDay(parsed)\\n}\\n```\\n\\nThe regex accepts ANY 4-2-2 digit pattern. JS's `Date.UTC()` then *normalizes* out-of-range values silently:\\n\\n- `Date.UTC(2026, 12, 45)` (i.e. input `2026-13-45`) → month 12 wraps to January of next year, day 45 wraps further → `2027-02-14`\\n- `Date.UTC(2026, 1, 30)` (i.e. `2026-02-30`) → `2026-03-02`\\n- `Date.UTC(2026, 0, 0)` (i.e. `2026-01-00`) → `2025-12-31`\\n- `Date.UTC(2026, 13, 32)` (i.e. `2026-14-32`) → `2027-03-04`\\n\\nAll silently accepted. The user typed something nonsensical; the system computes a real date months or years from what they meant; the report shows totally different data than expected.\\n\\n**Why this matters:**\\n\\nFinance analytics, VAT reports, and AR/AP aging are all driven by these date params via URL query strings or CSV export filters. A copy-paste error or typo in a date field doesn't fail; it returns a confidently-wrong period.\\n\\nFor an analytics report this isn't catastrophic (user sees odd data and can investigate), but for a VAT export filed with HMRC based on the wrong period, the consequences are real.","acceptance_criteria":"- After parsing, validate the result round-trips: `parsed.getUTCFullYear() === year \u0026\u0026 parsed.getUTCMonth() === month - 1 \u0026\u0026 parsed.getUTCDate() === day`. Reject if mismatch.\\n- Optionally: accept ISO date string with separators '-' OR validate via `Date.parse(value)` + cross-check\\n- Add tests for: `2026-13-45`, `2026-02-30`, `2026-00-15`, `2026-01-32` — all should fall back to the fallback value\\n- Same fix applies to `parseOptionalDateOnly`","notes":"Filed during PR #150 review. The parsing pattern is duplicated from existing analytics modules (PR is consolidating them) — the bug pre-existed but is now in one place where it can be fixed at the source.","status":"open","priority":2,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-09T21:19:38Z","created_by":"Jan","updated_at":"2026-06-09T21:19:38Z","labels":["analytics","code-review","date-handling","pr-150"],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-3dz","title":"Implement analytics/report scale refactor group","description":"Implement docs/plan.md suggested group 16: P7.1 source-row cap pattern, P7.2 base-currency lookup, P7.3 shared date-window helper, P7.8 typed source-scan errors, plus CR1/CR2/CR3.","status":"closed","priority":2,"issue_type":"task","assignee":"Jan","owner":"info@onetwo3d.co.uk","created_at":"2026-06-09T19:12:25Z","created_by":"Jan","updated_at":"2026-06-09T21:07:39Z","started_at":"2026-06-09T19:12:30Z","closed_at":"2026-06-09T21:07:39Z","close_reason":"Implemented analytics/report scale refactor group","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-gnt","title":"Xero daily batch group A2 missing explicit orderBy — split-batch referenceId hash is non-deterministic (PR #149)","description":"PR #149's `lib/connectors/xero/daily-sync.ts` adds batching to all three daily batch groups. Each group's `referenceId` is derived from `buildDailyBatchReferenceId(group, date, entityIds)` where `entityIds` is the ordered list of IDs in the current window.\n\nFor determinism (same batch → same referenceId → idempotency works on retry), the input order MUST be stable across runs.\n\n**Group A1** (line ~507) adds: `orderBy: [{ paidAt: 'asc' }, { id: 'asc' }]` ✓\n**Group B** (line ~755) already had: `orderBy: { createdAt: 'asc' }` ✓\n**Group A2** (line ~610) adds `take: batchLimit + 1` but **NO orderBy** ✗\n\nPostgres returns rows in an undefined order when no orderBy is specified. The actual order depends on:\n- Table physical layout (heap order)\n- Vacuum / cluster operations\n- Index used by the planner\n- Parallel query workers\n\nConcrete failure:\n1. First run at 09:00: A2 query returns 1000 orders in order [O1, O5, O3, O7, ...] → hash 'abc12345' → referenceId `A2-2026-06-09-abc12345`.\n2. Run fails partway / commit rollback. accountingSyncLog never gets the entry.\n3. Next run at 09:05: same 1000 orders returned in order [O5, O1, O7, O3, ...] (heap changed slightly) → hash 'def67890' → referenceId `A2-2026-06-09-def67890`.\n4. Idempotency check against accountingSyncLog with referenceId='A2-2026-06-09-abc12345' finds nothing → posts journal. But also posts a second journal under 'A2-2026-06-09-def67890' on a subsequent rerun if heap shifts again.\n\n**Why this matters:** The whole point of deterministic referenceIds (per the PR's stated goal: 'deterministic split-batch journal references') is to make retries idempotent. Without a stable orderBy on A2, the same batch can post multiple times under different referenceIds.","acceptance_criteria":"- Add explicit orderBy to A2's findMany: `orderBy: [{ revenueDeferredDate: 'asc' }, { id: 'asc' }]` (or similar stable composite)\n- Add a test that asserts buildDailyBatchReferenceId returns the same value for the same set of entity IDs in any order (sort IDs inside the function), OR pin that the input must be pre-sorted\n- Consider: have buildDailyBatchReferenceId sort entityIds internally so callers can't accidentally introduce non-determinism — defense in depth\n- Audit the docs/plan.md plan group 15 completion claim against this gap","notes":"Filed during PR #149 review. The PR adds determinism to A1 and Group B but missed A2. Low likelihood of triggering today (postgres heaps are usually stable) but the failure mode is silent duplicate-journal posting in Xero.","status":"open","priority":2,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-09T18:14:12Z","created_by":"Jan","updated_at":"2026-06-09T18:14:12Z","labels":["code-review","idempotency","pr-149","xero"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-uk5","title":"Cron rate limit backend defaults to in-memory — multi-instance deployments bypass the limit (PR #149)","description":"PR #149's `enforceCronRateLimit` calls `checkRateLimit` from `@/lib/rate-limit`. The backend is selected by `RATE_LIMIT_BACKEND` env var, defaulting to `'memory'` (`lib/security/rate-limit.ts:23`):\n\n```ts\nconst value = (source.RATE_LIMIT_BACKEND ?? 'memory').trim().toLowerCase()\nif (value === '' || value === 'memory') return 'memory'\n```\n\nIn a single-instance Next.js deployment (current production setup per CLAUDE.md: 'Runs as next start on machine at 10.0.3.99'), the memory backend works — all cron requests hit the same process.\n\nIn a multi-instance / horizontally-scaled deployment:\n- Replica A sees 1/12 used at 12:00\n- Replica B sees 0/12 used at 12:00 (separate memory)\n- A cron daemon's HTTP request can hit either replica\n- Effective rate limit is N×max where N is replica count\n- Documented quota is meaningless\n\nThe PR doesn't document this caveat in:\n- `docs/installation.md` (deployment guide)\n- `docs/architecture.md` (cron section)\n- `.env.example` (no mention of RATE_LIMIT_BACKEND for cron)\n- The new `lib/cron-rate-limit.ts` JSDoc\n\n**Why this matters:** A team scaling IMS to multiple replicas (for HA or perf) would expect rate limits to still work. The silent multiplication would only be noticed if Xero/Mintsoft start rejecting due to upstream API limits, traced back to IMS sending N× the rate. Or when the system runs hot on outbox processing.\n\n**Mitigation cost:** Already-implemented. Just set `RATE_LIMIT_BACKEND=redis` + `REDIS_URL=...` per `lib/security/rate-limit.ts:49`. Cost is documentation, not code.","acceptance_criteria":"- Add a NOTE in lib/cron-rate-limit.ts JSDoc: 'Uses the application's rate-limit backend. For multi-replica deployments, set RATE_LIMIT_BACKEND=redis and provide REDIS_URL.'\n- Update .env.example with RATE_LIMIT_BACKEND and REDIS_URL examples\n- Update docs/installation.md cron section to document the multi-instance caveat\n- Optional: add a preflight assertion (`scripts/preflight-production.ts`) that warns if multiple replica indicators (e.g., env hint like REPLICA_COUNT \u003e 1) are set but RATE_LIMIT_BACKEND=memory","notes":"Filed during PR #149 review. Not a code bug — a documentation gap. Single-instance prod (current state) is fine; this becomes a footgun the moment IMS scales out.","status":"open","priority":2,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-09T18:13:49Z","created_by":"Jan","updated_at":"2026-06-09T18:13:49Z","labels":["code-review","cron","deployment","pr-149","rate-limit"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-da0","title":"delivery-status cron quota of max=2 is half the expected 4/hr for a 15-min cadence (PR #149)","description":"PR #149 sets:\n\n```ts\nconst rateLimitErr = await enforceCronRateLimit('delivery-status', { max: 2 })\n```\n\n(`app/api/cron/delivery-status/route.ts`)\n\nBut per docs/installation.md and the PR description (\"schedule-compatible hourly quotas for 5-minute/15-minute cron jobs\"), the delivery-status cron is scheduled every 15 minutes — that's 4 expected invocations per hour, not 2.\n\nWith max=2, the cron is silently denied half its scheduled runs. The denied invocations return 429, get logged as errors, but the actual delivery-status polling (which checks for shipment tracking updates from carriers) only completes twice per hour instead of four times. Tracking updates are delayed by 15-30 minutes more than the documented schedule.\n\n**Why this matters:** Users tracking shipments via the IMS dashboard see stale tracking states. The cron appears to be working (some invocations succeed) but the cadence is silently halved. The rate-limit log entries explaining 429s are buried under normal noise.\n\nThis is a likely typo or copy-paste from the daily-cron max=1 default rather than a deliberate quota choice.","acceptance_criteria":"- Update `delivery-status` quota to max=5 (4 expected + 1 jitter headroom) or max=6 (1.5x cadence)\n- Add a test or comment documenting the cadence-to-quota relationship\n- Audit other per-route overrides to confirm they match their actual cron schedules:\n - mintsoft-* (likely 5-min or hourly per integration)\n - accounting-payment-poll (max=4 — likely 15-min cadence; check)\n - accounting-sync (max=12 — 5-min; addressed in dsd)\n- Consider deriving max from a documented cadence per route rather than hard-coding numbers","notes":"Filed during PR #149 review. Related to dsd. delivery-status is the clearest example of quota-cadence mismatch; the same audit should hit other per-route overrides.","status":"open","priority":2,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-09T18:13:30Z","created_by":"Jan","updated_at":"2026-06-09T18:13:30Z","labels":["code-review","cron","pr-149","rate-limit"],"dependency_count":0,"dependent_count":0,"comment_count":0} @@ -47,26 +62,29 @@ {"_type":"issue","id":"onetwo3d-ims-p1s","title":"Remote backup manifest upload is unguarded — orphan SQL on upload failure (PR #148)","description":"PR #148's `app/api/backup/upload-remote/route.ts` chains two upload calls:\n\n```ts\nconst result = await uploadBackupToTarget(filePath, safe, target)\nawait uploadBackupToTarget(\n backupManifestPath(filePath),\n `${safe}.manifest.json`,\n target,\n 'application/json',\n)\n\nawait logActivity({ ... description: `Uploaded backup ... to ${target}` })\n```\n\nIf the FIRST upload (SQL) succeeds but the SECOND upload (manifest) throws:\n- The SQL backup is on the remote\n- The manifest is NOT on the remote\n- The error bubbles up to the operator\n- But the SQL file is orphaned on the remote with no manifest sidecar\n\nSame pattern in `app/api/cron/backup/route.ts` cron path — if SQL upload succeeds and manifest upload fails, cron returns the SQL upload error and the orphan stays.\n\n**Why this matters:** Future restore from the remote (via download → upload to restore endpoint) requires the manifest. An orphan SQL on the remote can't be restored cleanly via the stored-backup path because there's no manifest to validate.\n\nThe orphan also doesn't trigger any cleanup or alert — operator has to manually notice and either re-upload or delete.","acceptance_criteria":"- Wrap the two-upload sequence in a try/catch\n- If the manifest upload fails after SQL upload succeeded, delete the orphan SQL file from the remote (or queue a retry of the manifest upload)\n- Log the failure with sufficient detail to manually recover (`level: 'ERROR'` with metadata: { target, sqlKey, manifestKey })\n- Consider: a single `uploadBackupArtifactsToTarget()` helper that takes both files and either uploads both or neither\n- Add a test that asserts cleanup happens on manifest upload failure","notes":"Filed during PR #148 review. Same anti-pattern as PR #128/#129/#142's silent activity-log catches and PR #137's enqueueStockSync outside transaction — partial-success operations need explicit recovery.","status":"open","priority":2,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-09T15:59:12Z","created_by":"Jan","updated_at":"2026-06-09T15:59:12Z","labels":["backup","code-review","pr-148","remote-storage"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-ed1","title":"Disk-space preflight uses file.size * 2 multiplier — wrong for SQL restores (PR #148)","description":"PR #148 adds a disk-space preflight in `createBackupRestorePostHandler`:\n\n```ts\nconst availableBytes = await resolvedDeps.getAvailableDiskBytes(resolvedDeps.backupDir)\nif (availableBytes \u003c file.size * 2) {\n return NextResponse.json({ error: 'Not enough disk space for restore upload.' }, { status: 507 })\n}\n```\n\nThe 2x multiplier assumes the file's own size dominates restore disk needs. For SQL restores, that's wrong.\n\nA SQL backup file is compressed text — `INSERT INTO ... VALUES (...)`. The same data in PostgreSQL's row format with indexes is typically **5-20x larger** than the SQL text representation. A 50MB SQL file restoring into Postgres produces a database that occupies hundreds of MB to several GB depending on indexes.\n\nThe 2x multiplier (~100MB available for a 50MB upload) is enough to copy the SQL file to disk — but nowhere near enough to actually run psql -f against it for a non-trivial database.\n\n**Why this matters:** The preflight gives operators false confidence ('disk space check passed') but the actual restore can still fail with `ERROR: could not extend file: No space left on device` mid-restore — leaving the database in a half-restored corrupted state because the SQL ran statements partially.\n\n**Why the 2x doesn't work even for small backups:** even if the database itself fits, pg's WAL during a large insert needs working space too.\n\nConcrete: a tenant with a 200MB SQL backup (50MB after gzip) restores to a 2-3GB database. The preflight passes on a server with 500MB free, but the restore exhausts disk halfway.","acceptance_criteria":"- Replace the file.size * 2 heuristic with an estimate based on backup-time database size (stored in manifest? add `databaseSizeBytes` field?)\n- For initial implementation, use a conservative multiplier: file.size * 10 catches more realistic cases\n- Document the heuristic limit explicitly in the preflight error message (\"requires ~Nx the SQL file size in free disk\")\n- Consider a separate `databaseSizeBytes` field in the manifest that the preflight reads directly","notes":"Filed during PR #148 review. The check is well-intentioned but its multiplier doesn't match how SQL restores consume disk. A failed restore mid-way is worse than a rejected restore upfront.","status":"open","priority":2,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-09T15:58:56Z","created_by":"Jan","updated_at":"2026-06-09T15:58:56Z","labels":["backup","code-review","operational","pr-148"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-e30","title":"Backup manifest row counts collected separately from pg_dump — snapshot drift between dump and manifest (PR #148)","description":"PR #148's flow in `app/api/backup/create/route.ts` and `app/api/cron/backup/route.ts`:\n\n1. `execFile('pg_dump', args, ...)` — pg_dump runs in its own transaction snapshot.\n2. After pg_dump finishes successfully, `writeBackupManifestForFile(filePath, filename, prisma)` runs.\n3. The manifest function executes `SELECT COUNT(*) FROM \u003ctable\u003e` for every table.\n\nThe pg_dump snapshot and the manifest's COUNT queries are TWO DIFFERENT transaction snapshots. Between pg_dump finishing and the manifest collecting row counts (could be seconds for a large DB), writes can land that mismatch the manifest from the actual dump contents.\n\nConcrete scenarios:\n- A backup runs at 02:00. pg_dump dumps `sales_orders` with N rows. A new order lands at 02:01. The manifest writes `sales_orders.rowCount = N+1`. A restore that checks \"actual rows match manifest rowCount\" would fail or pass spuriously depending on which side is right.\n- Currently no restore-side validation actually compares the manifest's rowCount against post-restore counts — it just checks the manifest exists and critical tables are listed. So the snapshot mismatch is silent today. But the manifest claims to represent the dump.\n\n**Why this matters:** Future enhancements (\"restore validates row count parity\") would build on a foundation that doesn't actually guarantee snapshot consistency. The audit story of the manifest is also weaker — it's a snapshot of the database TAKEN AFTER pg_dump finished, not OF the dump itself.","acceptance_criteria":"- Use pg_dump's `--snapshot=\u003cid\u003e` flag to export with a known snapshot, then run the manifest queries against the same snapshot via `SET TRANSACTION SNAPSHOT '\u003cid\u003e'` in a Prisma transaction\n- Alternatively, run pg_dump and the manifest queries inside a single shared read-only transaction with REPEATABLE READ isolation\n- Add a test that exercises the race: write rows between pg_dump completion and manifest write, assert the manifest matches the dump (or document that it doesn't and the rowCount is advisory)\n- Document the consistency model in BackupManifest type's JSDoc","notes":"Filed during PR #148 review. Operational impact is low today (no restore-side validation uses rowCount). But this is foundational — fixing it later requires rebuilding manifest collection.","status":"open","priority":2,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-09T15:58:38Z","created_by":"Jan","updated_at":"2026-06-09T15:58:38Z","labels":["backup","code-review","consistency","pr-148"],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-gpl","title":"Implement plan group 14 backup restore hardening","description":"Implement docs/plan.md group 14: P6.3 restore token TTL/session binding, P6.7 restore upload size and disk-space guard, and P7.7 backup manifest generation/restore validation. Update plan checkboxes, tests, Beads export, commit, push, and open PR against development.","status":"closed","priority":2,"issue_type":"task","assignee":"Jan","owner":"info@onetwo3d.co.uk","created_at":"2026-06-09T15:39:08Z","created_by":"Jan","updated_at":"2026-06-09T15:52:49Z","started_at":"2026-06-09T15:39:20Z","closed_at":"2026-06-09T15:52:49Z","close_reason":"Implemented backup restore hardening group 14 with restore token binding, upload limits/disk checks, backup manifests, tests, and plan updates.","dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-5z1","title":"Add product end-of-life feature to plan","description":"Add a planned feature to docs/plan.md for an End of Life product flag: product page checkbox, reorder forecast exclusion, filtering, and automatic archival once all warehouse stock is zero and no incoming stock remains.","status":"closed","priority":2,"issue_type":"task","owner":"info@onetwo3d.co.uk","created_at":"2026-06-08T23:25:14Z","created_by":"Jan","updated_at":"2026-06-08T23:26:47Z","closed_at":"2026-06-08T23:26:47Z","close_reason":"Added P8.1 to docs/plan.md for product End of Life checkbox, reorder forecast exclusion, EOL filtering, and automatic archival once stock and incoming supply are exhausted.","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-eda","title":"Password policy common-password list contains 7 entries — order-of-magnitude too small (PR #146)","description":"PR #146 adds `lib/security/password-policy.ts` with:\n\n```ts\nconst COMMON_PASSWORDS = new Set([\n 'password',\n 'password1',\n 'password12',\n 'password123',\n 'password123!',\n 'adminadmin123!',\n 'onetwoinventory1!',\n])\n```\n\n7 entries. The intent is the right one (block obviously-bad passwords) but the list is way too small to materially improve security:\n\n- The well-known SecLists 'common-passwords-top-10000' has 10k entries — none of which (except the seed entries above) are caught.\n- Real attackers use lists with 100k–1M entries.\n- The RockYou leak alone contains 14M entries.\n\nEffect: `'Letmein123!'`, `'Qwerty123!'`, `'P@ssword123'`, `'Welcome2024!'` — none caught by this list. Each meets all the structural requirements (length, uppercase, number, symbol) and passes validation.\n\n**Why this matters:** The plan item P6.6 specifically calls for stronger password requirements. A 7-entry common-password list satisfies the letter (some check exists) but not the spirit (real common passwords are blocked) of the requirement.","acceptance_criteria":"- Either:\n - **Option A** (preferred): integrate HaveIBeenPwned Pwned Passwords k-anonymity API — submit first 5 chars of SHA1 hash, get list of suffixes, check locally. Catches actual breached passwords without sending the full password anywhere.\n - **Option B**: bundle a static list of top 10k common passwords (e.g., from SecLists). Lower fidelity than HIBP but better than 7.\n - **Option C**: keep the 7-entry list AS A FLOOR plus dictionary checks (no plain English words, max repeated characters, etc.)\n- Add tests asserting the policy rejects common attack-list passwords (e.g., 'Welcome2024!', 'Qwerty123!', 'P@ssword123')\n- Also add (probably worth a separate issue): max password length (e.g., 256) to prevent bcrypt DoS via long inputs","notes":"Filed during PR #146 review. The implementation works as written, but the protection is illusory for real attacker password lists.","status":"open","priority":2,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-08T23:08:33Z","created_by":"Jan","updated_at":"2026-06-08T23:08:33Z","labels":["code-review","passwords","pr-146","security"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-quf","title":"Invoice PDF token max TTL hard-capped at 10 minutes with no admin override (PR #146)","description":"PR #146 sets:\n\n```ts\nconst DEFAULT_INVOICE_PDF_TOKEN_TTL_SECONDS = 10 * 60\nconst MAX_INVOICE_PDF_TOKEN_TTL_SECONDS = 10 * 60\n```\n\nBoth default and max are 10 minutes. The old values were 3-day default and 30-day max. The 30-day max was clearly too lax, but 10-min max means:\n\n1. **No admin override possible** — if a tenant needs longer TTL for a specific workflow (e.g., finance team scheduled an invoice review for tomorrow), there's no env or settings configuration that allows it.\n2. **Email-an-invoice-link workflows break** — operator emails themselves an invoice link from desktop, opens it on phone 15 minutes later → expired.\n3. **Batch download flows break** — opening multiple invoices, leaving tabs in background while doing other work, then downloading → first tabs expired.\n\nCombined with the IP-binding fragility (`onetwo3d-ims-qv1`), the link is effectively single-use within ~5 minutes from same device, same network. That's appropriate for the highest-security path but not configurable for tenants with more relaxed threat models.","acceptance_criteria":"- Make max TTL configurable via env var (e.g., `INVOICE_PDF_TOKEN_MAX_TTL_SECONDS`) with a sensible production default\n- Document the env var in `.env.example` and `docs/installation.md`\n- Keep the 10-minute DEFAULT (most operators won't override) but allow tenants to raise the cap when needed\n- Consider: separate TTL for 'fresh download from invoice page' (short, 10min) vs 'recovery from emailed link' (longer, 1h-24h)","notes":"Filed during PR #146 review. The PR's stated rationale ('shortens max/default TTL to 10 minutes') is sensible for the default; the lack of any override path is the operational risk.","status":"open","priority":2,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-08T23:08:19Z","created_by":"Jan","updated_at":"2026-06-08T23:08:19Z","labels":["code-review","operational","pr-146","security"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-qv1","title":"Invoice PDF token IP-binding is operationally fragile for legitimate users (PR #146)","description":"PR #146 binds invoice PDF tokens to both the session and the client IP. The IP binding breaks token validity for legitimate users in several realistic scenarios:\n\n1. **Mobile users**: a user opens the invoice list on home WiFi, then their phone switches to cellular (different IP), then they tap the invoice link → token rejected with `wrong_ip`.\n2. **Corporate NAT / load balancers**: enterprise networks rotate the apparent client IP on subsequent requests through different egress routes → token rejected mid-session.\n3. **VPN reconnect**: VPN clients drop and reconnect with a new IP regularly → broken links.\n4. **Roaming / hotspot handoff**: mobile networks shift IPs every few minutes on long-running pages.\n5. **Tab restored after device sleep**: IP may have changed.\n\nThe threat model the binding addresses is 'shared link from a stolen URL'. But session-binding alone already prevents this for non-authenticated attackers (they don't have the session cookie). For authenticated attackers who do have the session, IP binding helps marginally but at significant operational cost.\n\n**Why this matters:** The PR description says PDF links 'are no longer generated into WooCommerce customer notes because the hardened token now requires current session/IP binding'. That's an explicit acknowledgment that the binding is too strict for non-interactive use. But the same strictness applies to interactive users on mobile/VPN/corporate networks.\n\n**Refs:**\n- `lib/invoice-pdf.ts` — `bindingHash('clientIp', ...)` in signing and verification\n- `app/api/invoices/[id]/route.ts` — `getInvoicePdfTokenBinding` extracts clientIp from `getClientIp(request.headers)`","acceptance_criteria":"- Decide on the threat model explicitly: is IP binding worth the operational cost given session binding is already enforced?\n- If yes (keep IP binding): document the operational caveats in user-facing help docs ('if you switch networks, refresh the invoice list to get a new link')\n- If no (drop IP binding): keep session binding; remove the `ipHash` field and `wrong_ip` reason\n- Consider middle ground: bind to /24 subnet rather than exact IP — catches most legitimate IP changes (mobile carrier subnet stays similar), still blocks attackers from another network\n- Add a 'PDF link expired or invalid — please request a new one from the invoice page' user-facing error that operators see when the binding fails (currently the route returns 404 / 403 which is opaque)","notes":"Filed during PR #146 review. The same trade-off was implicitly acknowledged by the PR's own removal of WC customer-note links. Worth making the trade-off explicit rather than discovering it via support tickets.","status":"open","priority":2,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-08T23:08:02Z","created_by":"Jan","updated_at":"2026-06-08T23:08:02Z","labels":["code-review","operational","pr-146","security"],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-76n","title":"Implement production plan group 13 security hardening","description":"Why this issue exists: docs/plan.md group 13 is the next unimplemented production-readiness batch after groups 1-12. It covers P6.2 supplier portal tenant boundaries, P6.4 invoice PDF token leakage mitigation, P6.5 generic WooCommerce connector errors, P6.6 stronger password requirements, and P6.8 activity-log metadata redaction. What needs to be done: implement the security hardening batch with focused tests, mark the covered plan items/group complete, run validation, commit, push, and open a PR against development.","status":"closed","priority":2,"issue_type":"task","assignee":"Jan","owner":"info@onetwo3d.co.uk","created_at":"2026-06-08T22:41:02Z","created_by":"Jan","updated_at":"2026-06-08T22:57:44Z","started_at":"2026-06-08T22:41:06Z","closed_at":"2026-06-08T22:57:44Z","close_reason":"Implemented production plan group 13 security hardening: supplier boundary assertions, session/IP-bound invoice PDF tokens, generic WooCommerce config errors, stronger password policy, and activity-log secret redaction coverage.","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-gyk","title":"No daily cron creates account balance snapshots — tightened freshness window will fail in production (PR #145)","description":"Codex adversarial review of PR #145 (P5.3) identified:\n\nThe PR tightens the snapshot freshness window from 7 days to 1 day (previous-day required). But snapshot creation depends on a manual UI sync button (`app/(dashboard)/sync/xero-client.tsx:352`) and `app/actions/xero-sync.ts` — no scheduled cron handler exists in `app/api/cron/` for account balance snapshots specifically.\n\nConsequence: tenants that don't run a manual sync every day — or any fresh-install tenant — will have no previous-day snapshot. With this PR merged, every GL period-movement query will either return null (current behaviour) or fail loudly (if bd-304 is addressed).\n\nThis is a deployment dependency gap: the code tightens the contract without the operational mechanism to satisfy the contract.\n\n**Why this matters:** A tenant that never manually clicks Sync will see their COGS variance / GL period reports break silently (current) or loudly (after bd-304). Either way, the report is unavailable until someone clicks Sync — which they may not realise is required.\n\n**Reproduction:** Fresh tenant; no manual sync performed; invoke a GL COGS variance report for yesterday's period — no previous-day snapshot exists; report shows the user notice / fails.","acceptance_criteria":"- Add a scheduled cron handler (e.g. `app/api/cron/account-balance-snapshot/route.ts`) that creates the previous-day snapshot daily\n- Wire the cron entry into docs/installation.md / docs/architecture.md cron table\n- Add a fallback in the domain function: if no snapshot exists for asOf, fetch it on-demand from Xero (with explicit error if the fetch fails)\n- Document the cron dependency in CHANGELOG.md so tenants upgrading know to verify the cron is wired up\n- Add a startup / preflight assertion that fails if the cron is unscheduled in production","notes":"Filed during Codex review of PR #145. Codex severity P2; this is the classic 'tighten the contract without the operational backbone' rollout risk.","status":"open","priority":2,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-08T22:16:10Z","created_by":"Jan","updated_at":"2026-06-08T22:16:10Z","labels":["accounting","code-review","cron","deployment","pr-145"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-304","title":"Account balance period movement returns null instead of failing loudly when previous-day snapshot is missing (PR #145)","description":"Codex adversarial review of PR #145 (P5.3 — \"Account balance opening up to 7 days stale\") identified:\n\nThe domain function `getAccountBalancePeriodMovement` (`lib/domain/accounting/account-balance-snapshots.ts:192`) returns `null` when the previous-day opening snapshot is missing, instead of throwing a typed error or returning a discriminated failure result. The report layer converts that `null` into a user notice, but the domain boundary does not satisfy the plan's acceptance criterion:\n\n\u003e \"Period movement query without a snapshot on dateFrom; assert behaviour (either fetch or fail loudly, not silently fall back).\"\n\n`null` from a domain function is silent fall-back, not loud failure — every caller has to remember to check, and a future caller that forgets will see `null` propagate into a report or aggregation as zero.\n\nThe accompanying test (`tests/domain/accounting/account-balance-snapshots.test.ts:221`) asserts the return value is `null` — pinning the silent-fallback behaviour as the expected contract rather than the loud failure the plan required.\n\n**Why this matters:** A future refactor that drops the report-layer null check (or a new caller that consumes the return value directly) would silently produce zero movements / wrong period balances. The pin makes it a feature, not a workaround.","acceptance_criteria":"- Domain function returns a discriminated union ({ success: true, ... } | { success: false, reason: 'missing_previous_day_snapshot', ... }) OR throws a typed `MissingSnapshotError`\n- Update tests to assert the failure path explicitly (rejection or discriminated failure)\n- Update report layer to handle the typed failure and continue emitting the user notice\n- Plan.md P5.3 entry's Tests pointer names the specific test (not just the file)","notes":"Filed during Codex review of PR #145. Codex severity P2; agreed because the user-visible behaviour (report shows a notice) is correct, but the contract at the domain boundary is wrong and the test pins it that way.","status":"open","priority":2,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-08T22:15:54Z","created_by":"Jan","updated_at":"2026-06-08T22:15:54Z","labels":["accounting","code-review","pr-145","silent-failure"],"dependency_count":0,"dependent_count":0,"comment_count":0} -{"_type":"issue","id":"onetwo3d-ims-jox","title":"Tax snapshot refresh is N+1: per-line update inside a loop (PR #144 follow-up)","description":"PR #144's `refreshMutableSalesOrderTaxSnapshots` and `refreshMutablePurchaseOrderTaxSnapshots` (lib/tax/document-tax-snapshot-refresh.ts) loop through every affected line:\n\n```ts\nfor (const line of lines) {\n const next = calculateSalesLineTaxSnapshot({ ... })\n await client.salesOrderLine.update({ where: { id: line.id }, data: { ... } })\n // accumulate delta into Map\n}\nfor (const [orderId, delta] of deltas) {\n await client.salesOrder.update({ where: { id: orderId }, data: { increment: ... } })\n}\n```\n\nFor a TaxRate change affecting 500 DRAFT order lines across 50 orders, that's:\n- 500 individual `salesOrderLine.update` round-trips\n- 50 `salesOrder.update` round-trips\n- All inside a single Prisma `$transaction` (long lock hold)\n\nThe whole operation runs synchronously in `updateTaxRate`, which is a settings page action — operator sees the form spin while 500+ queries run.\n\n**Why this matters:** Tax rate edits are infrequent but blocking. For a tenant with substantial draft queues (e.g., subscription-based recurring orders staged in DRAFT), this turns a settings change into a multi-second outage of the tax-rate edit endpoint AND holds locks on every affected row.","acceptance_criteria":"- Replace per-line update with a single `updateMany` (or raw SQL UPDATE) where possible — most lines just need rate/tax fields rewritten using the same formula\n- For cases where per-line computation is required (e.g., inclusive vs exclusive), batch into smaller chunks (~50 lines per round-trip) with explicit progress\n- For tenant safety, add a row-count threshold: if more than N affected lines, defer to a background job rather than blocking the UI\n- Add a perf test asserting that a 500-line refresh completes in under T seconds","notes":"Identified during PR #144 review. The function works correctly; this is a perf/scale concern not a correctness one. Bumping priority above P3 because settings-page latency is operator-visible and the transaction lock duration affects concurrent users.","status":"open","priority":2,"issue_type":"chore","owner":"dev@onetwo3d.com","created_at":"2026-06-07T23:37:54Z","created_by":"Jan","updated_at":"2026-06-07T23:37:54Z","labels":["code-review","perf","pr-144","tax"],"dependency_count":0,"dependent_count":0,"comment_count":0} -{"_type":"issue","id":"onetwo3d-ims-dmi","title":"Mutable tax statuses include RFQ_SENT/QUOTE_RECEIVED — supplier-facing quotes silently re-rated (PR #144)","description":"PR #144's tax snapshot refresh treats these PurchaseOrder statuses as mutable:\n\n```ts\nconst MUTABLE_PURCHASE_TAX_STATUSES = ['DRAFT', 'RFQ_SENT', 'QUOTE_RECEIVED'] as const\n```\n\nDRAFT is unambiguous — nothing has been sent. But:\n\n- **RFQ_SENT**: an RFQ document has been generated and (typically) emailed to the supplier. The supplier's response is based on the tax rate IMS sent.\n- **QUOTE_RECEIVED**: the supplier has responded with quoted prices, presumably with their own tax expectations.\n\nIf an IMS operator edits a TaxRate while RFQ or quote responses are in flight, the IMS-side tax fields silently change, but:\n\n1. The supplier's RFQ PDF (already sent) still shows the OLD rate.\n2. The supplier's quote (if received) doesn't account for the new rate.\n3. The IMS operator sees the new rate without realizing the supplier is operating on the old one.\n\n**Why this matters:** When the PO is eventually issued, the IMS tax fields don't match the supplier's quotation. Discrepancies surface only at invoice reconciliation, often weeks later.\n\nThe original bd issue context (PR #144's intent) was to keep MUTABLE statuses limited to truly editable documents. RFQ_SENT and QUOTE_RECEIVED arguably aren't editable in this sense — the document has crossed a supplier boundary.","acceptance_criteria":"- Decide with finance/operations: are RFQ_SENT and QUOTE_RECEIVED considered IMS-mutable for tax purposes, or have they crossed a 'document sent to counterparty' boundary that locks tax fields?\n- If the latter (recommended), restrict MUTABLE_PURCHASE_TAX_STATUSES to just ['DRAFT']\n- If the former, document this explicitly in the helper's JSDoc with the operational caveat\n- Same review for MUTABLE_SALES_TAX_STATUSES — PENDING_PAYMENT means the customer has been quoted/invoiced; refreshing tax silently there is also boundary-crossing","notes":"Identified during PR #144 review. Worth treating as a product/finance decision rather than a code bug — the implementation is consistent with the stated MUTABLE_*_STATUSES sets, but the choice of which statuses count as mutable for TAX purposes specifically deserves explicit finance sign-off because the field is denormalized onto persisted documents.","status":"open","priority":2,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-07T23:37:20Z","created_by":"Jan","updated_at":"2026-06-07T23:37:20Z","labels":["code-review","p2p","pr-144","tax"],"dependency_count":0,"dependent_count":0,"comment_count":0} -{"_type":"issue","id":"onetwo3d-ims-rdy","title":"PENDING FX retry queue dedupe relies on error-message prefix matching (PR #144)","description":"PR #144's `recordPendingFxOrder()` and `retryPendingWcOrdersWaitingForFx()` (lib/connectors/woocommerce/sync/order-import.ts) both look up the queue via:\n\n```ts\nerrorMessage: { startsWith: 'Missing GBP FX rate' }\n```\n\nString-prefix matching on a serialized error message is fragile:\n\n1. The error message format is owned by the FX resolver, not the queue. PR #142 already changed FX-related error messages once.\n2. The base currency is hard-coded in the prefix ('Missing GBP FX rate'). If a tenant operates in EUR base currency, this prefix never matches and the queue never deduplicates or retries.\n3. Future i18n / message-text edits silently break dedupe → orders queue infinitely without retry.\n\nSame anti-pattern flagged in PR #129 inline #2 (Prisma error.message fallback). The fix there was to trust `error.meta.target` instead of string matching.\n\n**Why this matters:** The dedupe is the only mechanism preventing the queue from accumulating multiple PENDING entries per missing-FX order. A broken dedupe means the threshold-based admin notification (`WC_PENDING_FX_ORDER_NOTIFY_THRESHOLD = 5`) fires spuriously, and `retryPendingWcOrdersWaitingForFx()` processes the same WC order N times until one succeeds.","acceptance_criteria":"- Add a structured discriminator: store a `reason: 'missing_fx_rate'` marker in the shoppingSyncLog payload (already present!) and use that for dedupe/retry lookups instead of errorMessage prefix\n- The retry function already calls `isQueuedWcOrderPayload` to validate the payload shape — reuse the same shape check for the dedupe query\n- Either filter the shoppingSyncLog query by a payload-JSON predicate, or add a separate column/index for the reason discriminator\n- Add a test for the multi-base-currency case (e.g., EUR base) to prove the fix isn't GBP-specific","notes":"Identified during PR #144 review. Closely related to onetwo3d-ims-xin (also PR #144). The dedupe-via-message-prefix anti-pattern is recurring in this codebase — also flagged in PR #129 review. Worth a sweep to find other instances.","status":"open","priority":2,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-07T23:37:01Z","created_by":"Jan","updated_at":"2026-06-07T23:37:01Z","labels":["code-review","connectors","fragile-string-match","fx","pr-144"],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-jox","title":"Tax snapshot refresh is N+1: per-line update inside a loop (PR #144 follow-up)","description":"PR #144's `refreshMutableSalesOrderTaxSnapshots` and `refreshMutablePurchaseOrderTaxSnapshots` (lib/tax/document-tax-snapshot-refresh.ts) loop through every affected line:\n\n```ts\nfor (const line of lines) {\n const next = calculateSalesLineTaxSnapshot({ ... })\n await client.salesOrderLine.update({ where: { id: line.id }, data: { ... } })\n // accumulate delta into Map\n}\nfor (const [orderId, delta] of deltas) {\n await client.salesOrder.update({ where: { id: orderId }, data: { increment: ... } })\n}\n```\n\nFor a TaxRate change affecting 500 DRAFT order lines across 50 orders, that's:\n- 500 individual `salesOrderLine.update` round-trips\n- 50 `salesOrder.update` round-trips\n- All inside a single Prisma `$transaction` (long lock hold)\n\nThe whole operation runs synchronously in `updateTaxRate`, which is a settings page action — operator sees the form spin while 500+ queries run.\n\n**Why this matters:** Tax rate edits are infrequent but blocking. For a tenant with substantial draft queues (e.g., subscription-based recurring orders staged in DRAFT), this turns a settings change into a multi-second outage of the tax-rate edit endpoint AND holds locks on every affected row.","acceptance_criteria":"- Replace per-line update with a single `updateMany` (or raw SQL UPDATE) where possible — most lines just need rate/tax fields rewritten using the same formula\n- For cases where per-line computation is required (e.g., inclusive vs exclusive), batch into smaller chunks (~50 lines per round-trip) with explicit progress\n- For tenant safety, add a row-count threshold: if more than N affected lines, defer to a background job rather than blocking the UI\n- Add a perf test asserting that a 500-line refresh completes in under T seconds","notes":"Identified during PR #144 review. The function works correctly; this is a perf/scale concern not a correctness one. Bumping priority above P3 because settings-page latency is operator-visible and the transaction lock duration affects concurrent users.","status":"open","priority":2,"issue_type":"chore","owner":"info@onetwo3d.co.uk","created_at":"2026-06-07T23:44:58Z","created_by":"Jan","updated_at":"2026-06-07T23:37:54Z","labels":["code-review","perf","pr-144","tax"],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-dmi","title":"Mutable tax statuses include RFQ_SENT/QUOTE_RECEIVED — supplier-facing quotes silently re-rated (PR #144)","description":"PR #144's tax snapshot refresh treats these PurchaseOrder statuses as mutable:\n\n```ts\nconst MUTABLE_PURCHASE_TAX_STATUSES = ['DRAFT', 'RFQ_SENT', 'QUOTE_RECEIVED'] as const\n```\n\nDRAFT is unambiguous — nothing has been sent. But:\n\n- **RFQ_SENT**: an RFQ document has been generated and (typically) emailed to the supplier. The supplier's response is based on the tax rate IMS sent.\n- **QUOTE_RECEIVED**: the supplier has responded with quoted prices, presumably with their own tax expectations.\n\nIf an IMS operator edits a TaxRate while RFQ or quote responses are in flight, the IMS-side tax fields silently change, but:\n\n1. The supplier's RFQ PDF (already sent) still shows the OLD rate.\n2. The supplier's quote (if received) doesn't account for the new rate.\n3. The IMS operator sees the new rate without realizing the supplier is operating on the old one.\n\n**Why this matters:** When the PO is eventually issued, the IMS tax fields don't match the supplier's quotation. Discrepancies surface only at invoice reconciliation, often weeks later.\n\nThe original bd issue context (PR #144's intent) was to keep MUTABLE statuses limited to truly editable documents. RFQ_SENT and QUOTE_RECEIVED arguably aren't editable in this sense — the document has crossed a supplier boundary.","acceptance_criteria":"- Decide with finance/operations: are RFQ_SENT and QUOTE_RECEIVED considered IMS-mutable for tax purposes, or have they crossed a 'document sent to counterparty' boundary that locks tax fields?\n- If the latter (recommended), restrict MUTABLE_PURCHASE_TAX_STATUSES to just ['DRAFT']\n- If the former, document this explicitly in the helper's JSDoc with the operational caveat\n- Same review for MUTABLE_SALES_TAX_STATUSES — PENDING_PAYMENT means the customer has been quoted/invoiced; refreshing tax silently there is also boundary-crossing","notes":"Identified during PR #144 review. Worth treating as a product/finance decision rather than a code bug — the implementation is consistent with the stated MUTABLE_*_STATUSES sets, but the choice of which statuses count as mutable for TAX purposes specifically deserves explicit finance sign-off because the field is denormalized onto persisted documents.","status":"open","priority":2,"issue_type":"bug","owner":"info@onetwo3d.co.uk","created_at":"2026-06-07T23:44:53Z","created_by":"Jan","updated_at":"2026-06-07T23:37:20Z","labels":["code-review","p2p","pr-144","tax"],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-rdy","title":"PENDING FX retry queue dedupe relies on error-message prefix matching (PR #144)","description":"PR #144's `recordPendingFxOrder()` and `retryPendingWcOrdersWaitingForFx()` (lib/connectors/woocommerce/sync/order-import.ts) both look up the queue via:\n\n```ts\nerrorMessage: { startsWith: 'Missing GBP FX rate' }\n```\n\nString-prefix matching on a serialized error message is fragile:\n\n1. The error message format is owned by the FX resolver, not the queue. PR #142 already changed FX-related error messages once.\n2. The base currency is hard-coded in the prefix ('Missing GBP FX rate'). If a tenant operates in EUR base currency, this prefix never matches and the queue never deduplicates or retries.\n3. Future i18n / message-text edits silently break dedupe → orders queue infinitely without retry.\n\nSame anti-pattern flagged in PR #129 inline #2 (Prisma error.message fallback). The fix there was to trust `error.meta.target` instead of string matching.\n\n**Why this matters:** The dedupe is the only mechanism preventing the queue from accumulating multiple PENDING entries per missing-FX order. A broken dedupe means the threshold-based admin notification (`WC_PENDING_FX_ORDER_NOTIFY_THRESHOLD = 5`) fires spuriously, and `retryPendingWcOrdersWaitingForFx()` processes the same WC order N times until one succeeds.","acceptance_criteria":"- Add a structured discriminator: store a `reason: 'missing_fx_rate'` marker in the shoppingSyncLog payload (already present!) and use that for dedupe/retry lookups instead of errorMessage prefix\n- The retry function already calls `isQueuedWcOrderPayload` to validate the payload shape — reuse the same shape check for the dedupe query\n- Either filter the shoppingSyncLog query by a payload-JSON predicate, or add a separate column/index for the reason discriminator\n- Add a test for the multi-base-currency case (e.g., EUR base) to prove the fix isn't GBP-specific","notes":"Identified during PR #144 review. Closely related to onetwo3d-ims-xin (also PR #144). The dedupe-via-message-prefix anti-pattern is recurring in this codebase — also flagged in PR #129 review. Worth a sweep to find other instances.","status":"open","priority":2,"issue_type":"bug","owner":"info@onetwo3d.co.uk","created_at":"2026-06-07T23:44:48Z","created_by":"Jan","updated_at":"2026-06-07T23:37:01Z","labels":["code-review","connectors","fragile-string-match","fx","pr-144"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-z0g","title":"Add FIFO-by-completedAt integration test: two production orders same day (P4.2 follow-up)","description":"Plan item P4.2 closed via PR #137, but the test coverage is helper-level only. `tests/domain/manufacturing/manufacturing-action-inputs.test.ts` proves `manufacturingCostLayerReceivedAt()` returns `completedAt ?? fallback`, but no test exercises the integration the plan asked for:\n\n\u003e Complete two production orders same day; assert FIFO order matches `completedAt`.\n\nThe helper test confirms the function returns the right value; an integration test would confirm that completing two production orders in a different order than their `completedAt` actually produces FIFO consumption in the correct sequence.","acceptance_criteria":"- Add test that:\n - Creates 2 production orders A and B with the same product/warehouse\n - Sets order A's `completedAt` to 09:00 and order B's to 10:00, but processes B first then A (out of order)\n - Calls `updateManufacturingOrderStatus` for each to COMPLETED\n - Asserts the resulting cost layers' `receivedAt` reflects the production `completedAt`, not the processing order\n - Performs a FIFO consumption against the product and asserts layer A is consumed before layer B\n- Update docs/plan.md P4.2 Tests pointer to name the new test","notes":"Identified during PR #140 review audit. Plan acceptance criterion specifies this scenario explicitly; current coverage proves only the unit-level fallback. Disassembly recovery path's separate timestamp fix (manufacturing-action-inputs.test.ts:30) addresses PR #137 review feedback but doesn't satisfy P4.2's stated test.","status":"open","priority":2,"issue_type":"task","owner":"dev@onetwo3d.com","created_at":"2026-06-07T18:08:41Z","created_by":"Jan","updated_at":"2026-06-07T18:08:41Z","labels":["fifo","manufacturing","plan-followup","tests"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-mee","title":"Accounting invariants: split shipment_posted_missing_batch_amounts into separate revenue/COGS checks","description":"PR #15 (merged) lib/domain/accounting/invariants.ts L201 uses \u0026\u0026 when it should fire two separate critical findings:\n\nif (decimalToNumber(shipment.revenueRecognizedAmount) \u003c= 0 \u0026\u0026 decimalToNumber(shipment.cogsBatchAmount) \u003c= 0) { ... }\n\nA posted shipment with revenue but no COGS (under-accrued COGS - a real accounting gap) silently passes. The finding's message text already says \"no revenue or COGS batch amount\" - the operator should match.\n\nSuggested fix:\n- shipment_posted_missing_revenue_amount when revenueRecognizedAmount \u003c= 0\n- shipment_posted_missing_cogs_amount when cogsBatchAmount \u003c= 0\n\nThis matches the pattern in lib/domain/inventory/invariants.ts which splits negative-quantity and negative-reserved-quantity into separate critical findings.\n\nReference: https://github.com/OneTwo3D/IMS/pull/15#pullrequestreview-4177148955","status":"open","priority":2,"issue_type":"task","owner":"dev@onetwo3d.com","created_at":"2026-04-26T16:21:53Z","created_by":"Jan","updated_at":"2026-04-26T16:21:53Z","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-da1","title":"FX rate fetch cron has no alerting beyond staleness flag","description":"/api/cron/fx-rates failures are surfaced only by getFxHealth() flagging \u003e36h staleness in the UI. There is no email/Slack alert, no escalation, and the unrealised FX revaluation cron depends on this rate being current.\n\nFix direction:\n- Emit an alert (email or notification channel) when fetchAllFxRatesInternal exhausts its 3 retries.\n- Distinguish 'manual override pinning' from 'failed fetch' so the staleness banner is actionable.\n- Health endpoint should expose last-fetch timestamp + retry count so external monitoring can probe it.\n\nRefs:\n- app/actions/currencies.ts:177 fetchAllFxRatesInternal()\n- /api/cron/fx-rates","status":"open","priority":2,"issue_type":"task","owner":"dev@onetwo3d.com","created_at":"2026-04-25T20:45:35Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T20:45:35Z","labels":["fx","observability"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-cc7","title":"WC order import hard-fails on missing FX rate — needs queue/retry","description":"lib/connectors/woocommerce/sync/field-mapping.ts:307 (getFxRateToGbp) throws if no FxRate row exists for the order's currency on or before its date_created. Correct for data integrity, but if frankfurter.dev is unavailable for a day or the cron silently fails, every multi-currency WC order coming in stalls with no automatic recovery.\n\nImpact: paying customers' orders sit in importer error state. Operations may not notice for hours.\n\nFix direction:\n- Move stalled orders into a 'pending FX' retry queue rather than erroring out.\n- Once fx-rates cron next succeeds, drain the queue.\n- Page or notify when queue depth exceeds threshold.\n\nSee onetwo3d-ims-5mp for related unification work.","status":"open","priority":2,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-04-25T20:45:30Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T20:45:30Z","labels":["code-review","connectors","fx"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-vqs","title":"[bug] TaxRate edits don't recalculate draft/PENDING orders holding stale snapshot","description":"updateTaxRate() (app/actions/settings.ts:203) mutates the TaxRate row but every existing SalesOrder/PurchaseOrder line keeps its snapshotted `taxRatePercent` and `taxRateName`. For finalised/invoiced documents this is correct (immutability). For DRAFT or PENDING_PAYMENT orders the operator's expectation is that an edited rate flows through.\n\nImpact: an admin fixing a typo on the standard VAT rate sees the new value in settings but every open quote/draft still bills the old percent. Discovery is by reconciliation.\n\nFix direction:\n- On TaxRate update, identify open draft orders/POs referencing the rate and either (a) recalculate their tax fields, or (b) flag them with an 'outdated tax rate' badge that lets the operator one-click resync.\n- Confirm with finance which behaviour is preferred.\n\nSee also onetwo3d-ims-ggg for related immutability problems.","status":"open","priority":2,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-04-25T20:45:14Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T20:45:14Z","labels":["code-review","vat"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-7j3","title":"[bug] Legacy shipments without costLayerSnapshot skip refund-reversal COGS recalc","description":"ShipmentLine.costLayerSnapshot was added partway through the project. Shipments dispatched before that field existed have no JSON layer lineage, and the refund-reversal path silently skips COGS recalculation for those lines (comment at lib/cost-layers.ts:~465).\n\nImpact: refunds on legacy orders restock physically but never reverse COGS, leaving margin overstated for the refund period.\n\nFix direction: backfill costLayerSnapshot from CogsEntry rows where possible (CogsEntry was logged from the start). For shipments with neither, surface a UI warning when the operator attempts a refund and require manual COGS adjustment.\n\nRefs:\n- lib/cost-layers.ts:~465 (skip comment)\n- ShipmentLine.costLayerSnapshot field (schema.prisma:2244)","status":"open","priority":2,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-04-25T20:45:01Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T20:45:01Z","labels":["code-review","cogs","refunds"],"dependency_count":0,"dependent_count":0,"comment_count":0} -{"_type":"issue","id":"onetwo3d-ims-5mp.8","title":"Phase 5: Production cutover for unified FX (health check + runbook)","description":"Phase 5 of docs/todo/unified-fx-rates-plan.md. Make the system cutover-ready by adding:\n\n1. A 'helper plugin reachability' probe — IMS calls the WC plugin endpoint with a deliberately invalid signature and asserts a 401 response (proves the plugin is installed and the endpoint is wired).\n2. A stale-rate guard surfaced in the UI: warn if frankfurter last_fetched \u003e 36 h, or if WC push enabled but last push \u003e 36 h.\n3. A cutover runbook (docs/todo/unified-fx-rates-cutover.md) with the operator steps for switching production over: install plugin, paste secret, run probe, enable push, monitor FxRatePushLog for a week.\n\nPhase 5 itself is operational (operator switches Aelia provider on WP side); IMS just needs the tooling to validate connectivity and detect drift early.","status":"closed","priority":2,"issue_type":"task","assignee":"OneTwo3D IMS","owner":"dev@onetwo3d.com","created_at":"2026-04-25T13:12:12Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T13:16:24Z","started_at":"2026-04-25T13:12:13Z","closed_at":"2026-04-25T13:16:24Z","close_reason":"Closed","labels":["connectors","cutover","fx","ops","woocommerce","xero"],"dependencies":[{"issue_id":"onetwo3d-ims-5mp.8","depends_on_id":"onetwo3d-ims-5mp","type":"parent-child","created_at":"2026-04-25T13:12:11Z","created_by":"OneTwo3D IMS","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} -{"_type":"issue","id":"onetwo3d-ims-5mp.6","title":"QuickBooks sync drops currencyRateToBase from payload","description":"Codex P2 from review of branch vs main. Phase 1 stamped currencyRateToBase on every accounting payload, but lib/connectors/quickbooks/sync-processor.ts at lines 293, 321, 340 forwards the payload to pushSalesInvoice/pushPurchaseBill/pushCreditMemo without including currencyRateToBase. As a result QuickBooks will apply its own daily ExchangeRate on multi-currency docs and drift from IMS — defeating Phase 1's goal for QB tenants.\n\nFix: thread currencyRateToBase through all three QB calls (mirror the Xero adapter pattern from Phase 1) and set ExchangeRate on the QBO API payload using the reciprocal.","status":"closed","priority":2,"issue_type":"task","assignee":"OneTwo3D IMS","owner":"dev@onetwo3d.com","created_at":"2026-04-25T12:22:22Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T12:29:53Z","started_at":"2026-04-25T12:22:22Z","closed_at":"2026-04-25T12:29:53Z","close_reason":"Closed","labels":["connectors","fx","quickbooks","review-finding","woocommerce","xero"],"dependencies":[{"issue_id":"onetwo3d-ims-5mp.6","depends_on_id":"onetwo3d-ims-5mp","type":"parent-child","created_at":"2026-04-25T12:22:21Z","created_by":"OneTwo3D IMS","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} -{"_type":"issue","id":"onetwo3d-ims-5mp.5","title":"Phase 4: FX Rates admin UI + manual override + push log","description":"Phase 4 of the unified-FX rates plan (docs/todo/unified-fx-rates-plan.md).\n\nSchema:\n- Migration 20260425100000_fx_rate_overrides_and_push_log adds source + manualOverride to fx_rates; new fx_rate_push_log table\n\nServer actions in app/actions/currencies.ts:\n- getLatestFxRates() — DISTINCT ON per toCurrency, latest by fetchedAt\n- setManualFxRate(code, rate) — append a manualOverride=true row\n- clearManualFxRate(code) — re-fetch from frankfurter for that one currency, append non-override row\n- getFxPushLog(limit) — recent push attempts\n- fetchAllFxRatesInternal() now skips currencies whose latest rate is a manual override\n\nUI:\n- components/settings/fx-rates-table.tsx — table with source badges, override pencil + clear-override undo, dialog for entering a rate, push-log section\n- New 'FX Rates' tab in Settings → Accounting (page.tsx)\n\nPush log persistence:\n- Both the cron fan-out and the manual Push Now button write FxRatePushLog rows (OK / FAILED with errorMessage)","status":"closed","priority":2,"issue_type":"task","owner":"dev@onetwo3d.com","created_at":"2026-04-25T12:16:24Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T12:16:25Z","closed_at":"2026-04-25T12:16:25Z","close_reason":"Closed","labels":["connectors","fx","ui","woocommerce","xero"],"dependencies":[{"issue_id":"onetwo3d-ims-5mp.5","depends_on_id":"onetwo3d-ims-5mp","type":"parent-child","created_at":"2026-04-25T12:16:23Z","created_by":"OneTwo3D IMS","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} -{"_type":"issue","id":"onetwo3d-ims-5mp.4","title":"Phase 2: Unified onetwoInventory Helper WP plugin + FX rate push to WC","description":"Phase 2 of the unified-FX rates plan (docs/todo/unified-fx-rates-plan.md). Builds the companion WordPress plugin AND wires the IMS push job for FX rates. Per the latest decision:\n\n1. Consolidate the existing wc-invoice-buttons.php into a single unified plugin called 'onetwoInventory Helper'. Multiple modules (invoice buttons, FX rate provider, future hooks) live as include files inside one plugin so customers only install one thing.\n2. Make this plugin installable from the IMS WC settings screen (sync page) via a download button that serves a zip.\n3. Implement the IMS push job: ShoppingConnector.pushFxRates() (optional method on the interface), WC adapter writes rates to the new REST endpoint, fan-out from /api/cron/fx-rates after the inbound fetch.\n4. Aelia integration: the plugin registers a custom Aelia exchange-rates provider that reads from the OTI rate cache (option/transient written by the REST endpoint).\n5. Document in xero-sync.md / woocommerce.md / installation.md and add Aelia setup steps.\n\nDeliverables:\n- lib/connectors/woocommerce/wp-plugin/onetwoinventory-helper/ (multi-file PHP plugin)\n- IMS download endpoint + UI button on the sync page\n- IMS push job + connector method\n- Tests for HMAC and the fan-out\n- Docs","status":"closed","priority":2,"issue_type":"task","assignee":"OneTwo3D IMS","owner":"dev@onetwo3d.com","created_at":"2026-04-25T11:55:34Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T12:06:51Z","started_at":"2026-04-25T11:55:37Z","closed_at":"2026-04-25T12:06:51Z","close_reason":"Closed","labels":["connectors","fx","woocommerce","wp-plugin","xero"],"dependencies":[{"issue_id":"onetwo3d-ims-5mp.4","depends_on_id":"onetwo3d-ims-5mp","type":"parent-child","created_at":"2026-04-25T11:55:33Z","created_by":"OneTwo3D IMS","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-5mp.8","title":"Phase 5: Production cutover for unified FX (health check + runbook)","description":"Phase 5 of docs/todo/unified-fx-rates-plan.md. Make the system cutover-ready by adding:\n\n1. A 'helper plugin reachability' probe — IMS calls the WC plugin endpoint with a deliberately invalid signature and asserts a 401 response (proves the plugin is installed and the endpoint is wired).\n2. A stale-rate guard surfaced in the UI: warn if frankfurter last_fetched \u003e 36 h, or if WC push enabled but last push \u003e 36 h.\n3. A cutover runbook (docs/todo/unified-fx-rates-cutover.md) with the operator steps for switching production over: install plugin, paste secret, run probe, enable push, monitor FxRatePushLog for a week.\n\nPhase 5 itself is operational (operator switches Aelia provider on WP side); IMS just needs the tooling to validate connectivity and detect drift early.","status":"closed","priority":2,"issue_type":"task","assignee":"OneTwo3D IMS","owner":"dev@onetwo3d.com","created_at":"2026-04-25T13:12:12Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T13:16:24Z","started_at":"2026-04-25T13:12:13Z","closed_at":"2026-04-25T13:16:24Z","close_reason":"Closed","labels":["connectors","cutover","fx","ops","woocommerce","xero"],"dependencies":[{"issue_id":"onetwo3d-ims-5mp.8","depends_on_id":"onetwo3d-ims-5mp","type":"parent-child","created_at":"2026-04-25T13:12:11Z","created_by":"Jan","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-5mp.6","title":"QuickBooks sync drops currencyRateToBase from payload","description":"Codex P2 from review of branch vs main. Phase 1 stamped currencyRateToBase on every accounting payload, but lib/connectors/quickbooks/sync-processor.ts at lines 293, 321, 340 forwards the payload to pushSalesInvoice/pushPurchaseBill/pushCreditMemo without including currencyRateToBase. As a result QuickBooks will apply its own daily ExchangeRate on multi-currency docs and drift from IMS — defeating Phase 1's goal for QB tenants.\n\nFix: thread currencyRateToBase through all three QB calls (mirror the Xero adapter pattern from Phase 1) and set ExchangeRate on the QBO API payload using the reciprocal.","status":"closed","priority":2,"issue_type":"task","assignee":"OneTwo3D IMS","owner":"dev@onetwo3d.com","created_at":"2026-04-25T12:22:22Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T12:29:53Z","started_at":"2026-04-25T12:22:22Z","closed_at":"2026-04-25T12:29:53Z","close_reason":"Closed","labels":["connectors","fx","quickbooks","review-finding","woocommerce","xero"],"dependencies":[{"issue_id":"onetwo3d-ims-5mp.6","depends_on_id":"onetwo3d-ims-5mp","type":"parent-child","created_at":"2026-04-25T12:22:21Z","created_by":"Jan","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-5mp.5","title":"Phase 4: FX Rates admin UI + manual override + push log","description":"Phase 4 of the unified-FX rates plan (docs/todo/unified-fx-rates-plan.md).\n\nSchema:\n- Migration 20260425100000_fx_rate_overrides_and_push_log adds source + manualOverride to fx_rates; new fx_rate_push_log table\n\nServer actions in app/actions/currencies.ts:\n- getLatestFxRates() — DISTINCT ON per toCurrency, latest by fetchedAt\n- setManualFxRate(code, rate) — append a manualOverride=true row\n- clearManualFxRate(code) — re-fetch from frankfurter for that one currency, append non-override row\n- getFxPushLog(limit) — recent push attempts\n- fetchAllFxRatesInternal() now skips currencies whose latest rate is a manual override\n\nUI:\n- components/settings/fx-rates-table.tsx — table with source badges, override pencil + clear-override undo, dialog for entering a rate, push-log section\n- New 'FX Rates' tab in Settings → Accounting (page.tsx)\n\nPush log persistence:\n- Both the cron fan-out and the manual Push Now button write FxRatePushLog rows (OK / FAILED with errorMessage)","status":"closed","priority":2,"issue_type":"task","owner":"dev@onetwo3d.com","created_at":"2026-04-25T12:16:24Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T12:16:25Z","closed_at":"2026-04-25T12:16:25Z","close_reason":"Closed","labels":["connectors","fx","ui","woocommerce","xero"],"dependencies":[{"issue_id":"onetwo3d-ims-5mp.5","depends_on_id":"onetwo3d-ims-5mp","type":"parent-child","created_at":"2026-04-25T12:16:23Z","created_by":"Jan","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-5mp.4","title":"Phase 2: Unified onetwoInventory Helper WP plugin + FX rate push to WC","description":"Phase 2 of the unified-FX rates plan (docs/todo/unified-fx-rates-plan.md). Builds the companion WordPress plugin AND wires the IMS push job for FX rates. Per the latest decision:\n\n1. Consolidate the existing wc-invoice-buttons.php into a single unified plugin called 'onetwoInventory Helper'. Multiple modules (invoice buttons, FX rate provider, future hooks) live as include files inside one plugin so customers only install one thing.\n2. Make this plugin installable from the IMS WC settings screen (sync page) via a download button that serves a zip.\n3. Implement the IMS push job: ShoppingConnector.pushFxRates() (optional method on the interface), WC adapter writes rates to the new REST endpoint, fan-out from /api/cron/fx-rates after the inbound fetch.\n4. Aelia integration: the plugin registers a custom Aelia exchange-rates provider that reads from the OTI rate cache (option/transient written by the REST endpoint).\n5. Document in xero-sync.md / woocommerce.md / installation.md and add Aelia setup steps.\n\nDeliverables:\n- lib/connectors/woocommerce/wp-plugin/onetwoinventory-helper/ (multi-file PHP plugin)\n- IMS download endpoint + UI button on the sync page\n- IMS push job + connector method\n- Tests for HMAC and the fan-out\n- Docs","status":"closed","priority":2,"issue_type":"task","assignee":"OneTwo3D IMS","owner":"dev@onetwo3d.com","created_at":"2026-04-25T11:55:34Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T12:06:51Z","started_at":"2026-04-25T11:55:37Z","closed_at":"2026-04-25T12:06:51Z","close_reason":"Closed","labels":["connectors","fx","woocommerce","wp-plugin","xero"],"dependencies":[{"issue_id":"onetwo3d-ims-5mp.4","depends_on_id":"onetwo3d-ims-5mp","type":"parent-child","created_at":"2026-04-25T11:55:33Z","created_by":"Jan","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-em3","title":"Investigate WooCommerce auto-allocation stock constraint failure","description":"While adding WC/IMS/Xero COGS+FX e2e coverage, a paid WooCommerce import with a product stock row in a syncToStore warehouse did not auto-allocate. Running autoAllocateOrder directly for that order returned stock_levels_reserved_qty_lte_quantity even though the matching stock row had quantity 2 and reservedQty 0. Investigate allocator warehouse selection/upsert behavior for WooCommerce orders and make import fail visibly when allocation returns success false.","status":"closed","priority":2,"issue_type":"bug","assignee":"Jan","owner":"info@onetwo3d.co.uk","created_at":"2026-04-25T11:40:45Z","created_by":"Jan","updated_at":"2026-04-25T11:55:24Z","started_at":"2026-04-25T11:50:41Z","closed_at":"2026-04-25T11:55:24Z","close_reason":"Fixed WooCommerce auto-allocation: reservation now updates the existing stock row instead of taking an invalid upsert create path; internal auto-allocation tolerates Next cache revalidation outside a request; WC import now fails visibly on unexpected allocation errors while still allowing no-stock/backorder imports. Verified with focused WC/IMS/Xero COGS+FX Playwright coverage, lint, and type-check.","dependency_count":0,"dependent_count":0,"comment_count":0} -{"_type":"issue","id":"onetwo3d-ims-5mp.1","title":"Phase 1: Stamp CurrencyRate on every Xero document from IMS fxRateToBase","description":"Today lib/connectors/xero/{invoices,bills,credit-notes}.ts post documents without explicitly setting CurrencyRate, so Xero applies its own daily XE rate. This causes drift between IMS and Xero on the same invoice.\n\nChanges:\n- Extend InvoiceData / BillData / CreditNoteData in lib/connectors/types.ts with optional currencyRate: number\n- Document the rate-direction convention once (IMS stores 1 base = X doc-ccy; Xero expects 1 doc-ccy = X base — adapter inverts)\n- In Xero adapter, read fxRateToBase from caller, invert, set CurrencyRate on the API payload\n- Same change for bills.ts and credit-notes.ts\n- Unit test for inversion math\n\nOutcome: IMS and Xero agree on every invoice. No WC change needed for this phase.","status":"closed","priority":2,"issue_type":"task","assignee":"OneTwo3D IMS","owner":"dev@onetwo3d.com","created_at":"2026-04-25T11:32:32Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T11:38:30Z","started_at":"2026-04-25T11:32:35Z","closed_at":"2026-04-25T11:38:30Z","close_reason":"Closed","labels":["connectors","fx","woocommerce","xero"],"dependencies":[{"issue_id":"onetwo3d-ims-5mp.1","depends_on_id":"onetwo3d-ims-5mp","type":"parent-child","created_at":"2026-04-25T11:32:31Z","created_by":"OneTwo3D IMS","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-5mp.1","title":"Phase 1: Stamp CurrencyRate on every Xero document from IMS fxRateToBase","description":"Today lib/connectors/xero/{invoices,bills,credit-notes}.ts post documents without explicitly setting CurrencyRate, so Xero applies its own daily XE rate. This causes drift between IMS and Xero on the same invoice.\n\nChanges:\n- Extend InvoiceData / BillData / CreditNoteData in lib/connectors/types.ts with optional currencyRate: number\n- Document the rate-direction convention once (IMS stores 1 base = X doc-ccy; Xero expects 1 doc-ccy = X base — adapter inverts)\n- In Xero adapter, read fxRateToBase from caller, invert, set CurrencyRate on the API payload\n- Same change for bills.ts and credit-notes.ts\n- Unit test for inversion math\n\nOutcome: IMS and Xero agree on every invoice. No WC change needed for this phase.","status":"closed","priority":2,"issue_type":"task","assignee":"OneTwo3D IMS","owner":"dev@onetwo3d.com","created_at":"2026-04-25T11:32:32Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T11:38:30Z","started_at":"2026-04-25T11:32:35Z","closed_at":"2026-04-25T11:38:30Z","close_reason":"Closed","labels":["connectors","fx","woocommerce","xero"],"dependencies":[{"issue_id":"onetwo3d-ims-5mp.1","depends_on_id":"onetwo3d-ims-5mp","type":"parent-child","created_at":"2026-04-25T11:32:31Z","created_by":"Jan","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-5mp","title":"Unified FX rates across IMS, WC (Aelia), and Xero","description":"Make IMS the single source of truth for FX rates so WooCommerce (Aelia) and Xero both consume the same daily rate. Implementation must stay shopping/accounting-plugin agnostic.\n\nFull plan: docs/todo/unified-fx-rates-plan.md\n\nPhases (each independently shippable):\n1. Schema + Xero CurrencyRate stamping (immediate win, no WC dependency)\n2. Companion WP plugin (oti-fx-rates.php) registering as Aelia rate provider\n3. IMS push job — pushFxRates() on ShoppingConnector, fan-out from cron\n4. Admin UI: Settings -\u003e Financial -\u003e FX Rates (overrides + push status)\n5. Production cutover","status":"open","priority":2,"issue_type":"task","owner":"dev@onetwo3d.com","created_at":"2026-04-25T11:32:19Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T11:32:19Z","labels":["connectors","fx","woocommerce","xero"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-055","title":"Add COGS and FX commerce accounting e2e coverage","description":"Define and implement broad end-to-end coverage for COGS and foreign-currency transaction flows between WooCommerce, IMS, and Xero.","status":"closed","priority":2,"issue_type":"task","assignee":"Jan","owner":"info@onetwo3d.co.uk","created_at":"2026-04-25T11:26:13Z","created_by":"Jan","updated_at":"2026-04-25T11:40:54Z","started_at":"2026-04-25T11:26:20Z","closed_at":"2026-04-25T11:40:54Z","close_reason":"Added isolated WC/IMS/Xero COGS and FX e2e coverage with passing targeted Playwright run, lint, and type-check.","dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-baj","title":"Repair local Beads setup","description":"Fix local Beads permissions, role configuration, and database import so bd commands can read and write repository issues again.","status":"closed","priority":2,"issue_type":"task","assignee":"Jan","owner":"info@onetwo3d.co.uk","created_at":"2026-04-25T10:56:33Z","created_by":"Jan","updated_at":"2026-04-25T10:56:56Z","started_at":"2026-04-25T10:56:42Z","closed_at":"2026-04-25T10:56:56Z","close_reason":"Local Beads permissions, role configuration, and database import were repaired; bd status/list/create/update work again.","dependency_count":0,"dependent_count":0,"comment_count":0} @@ -74,10 +92,10 @@ {"_type":"issue","id":"onetwo3d-ims-m7x","title":"WooCommerce poll cursor advances on partial page failures","description":"last_wc_order_sync_at advances even when a page fetch errored. Orders at the boundary of a failed page are silently skipped.\n\nRefs:\n- lib/connectors/woocommerce/sync/webhooks.ts:65, :101\n\nFix direction: only advance the cursor after every page in the run has completed successfully.","status":"closed","priority":2,"issue_type":"bug","assignee":"OneTwo3D IMS","owner":"dev@onetwo3d.com","created_at":"2026-04-25T09:57:56Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T10:08:36Z","started_at":"2026-04-25T10:06:53Z","closed_at":"2026-04-25T10:08:36Z","close_reason":"Webhook handlers now collect per-step failures and gate cursor advance on success. On failure: log WARNING with failure list, return ok=false (cursor stays so polling cron retries the gap)","labels":["code-review","sync-woo"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-qsf","title":"WooCommerce dedupe is application-level only — duplicate SOs possible","description":"order-import.ts uses findFirst to detect duplicates instead of a DB unique constraint. A partially rolled-back import can produce duplicate SOs on the next run. WC webhook delivery is at-least-once with no dedupe table. Refunds short-circuit on externalRefundId, so refunds edited in WC after first sync are never re-imported.\n\nRefs:\n- lib/connectors/woocommerce/sync/order-import.ts:73 (findFirst, no constraint)\n- lib/connectors/woocommerce/sync/refund-sync.ts:37 (immutable assumption)\n\nFix direction: add unique index on ShoppingOrderLink(connector, externalOrderId); add a webhook-event dedupe table keyed on delivery ID; track WC refund updated_at and re-sync on change.","status":"closed","priority":2,"issue_type":"bug","assignee":"OneTwo3D IMS","owner":"dev@onetwo3d.com","created_at":"2026-04-25T09:57:56Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T10:28:36Z","started_at":"2026-04-25T10:28:36Z","closed_at":"2026-04-25T10:28:36Z","close_reason":"Verified: not a bug. The DB unique constraint @@unique([connector, externalOrderId]) DOES exist on ShoppingOrderLink (prisma/schema.prisma:1664). Combined with the application-level findFirst check + upsert pattern in importWcOrder (order-import.ts:73-86), duplicates are impossible — a concurrent race would throw on insert and be caught at line 71. WC refund immutability is correct because WC refunds themselves are immutable in WooCommerce (you issue a new refund, not edit an existing one).","labels":["code-review","sync-woo"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-3d0","title":"Manufacturing DRAFT → COMPLETED bypasses component-reservation check","description":"Direct DRAFT→COMPLETED transition does not set the wasInProgress flag, so requireReserved is skipped. Negative available stock can result.\n\nRefs:\n- app/actions/manufacturing.ts:652 (state transition allows both DRAFT and IN_PROGRESS sources)\n- app/actions/manufacturing.ts:676-679 (assertStockAvailable called with requireReserved: false)\n\nFix direction: gate completion on a prior reservation event regardless of source status.","status":"closed","priority":2,"issue_type":"bug","assignee":"OneTwo3D IMS","owner":"dev@onetwo3d.com","created_at":"2026-04-25T09:57:55Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T10:09:34Z","started_at":"2026-04-25T10:08:37Z","closed_at":"2026-04-25T10:09:34Z","close_reason":"Verified: not a bug. assertStockAvailable (manufacturing.ts:403-427) correctly checks available stock on DRAFT-\u003eCOMPLETED via includeReserved:false (computes quantity-reservedQty); the row is held under FOR UPDATE so the check is race-safe. The 'negative stock can result' claim was incorrect. Forcing all completions through IN_PROGRESS would be a workflow restriction that needs product input, not a bug fix.","labels":["code-review","manufacturing","state-machine"],"dependency_count":0,"dependent_count":0,"comment_count":0} -{"_type":"issue","id":"onetwo3d-ims-ggg","title":"Invoice/bill edits after Xero push are silently lost","description":"Once accountingInvoiceId is set, queueSalesInvoiceForOrder returns immediately. Even if it didn't, the Xero idempotency key is built from the internal sync-log entry ID, not invoice content, so Xero would dedupe content changes.\n\nRefs:\n- app/actions/sales.ts:1310 (early return on accountingInvoiceId)\n- lib/connectors/xero/sync-processor.ts:35 (idempotency key from sync log entry ID)\n- lib/connectors/xero/sync-processor.ts:50-65, :73 (same on bill side)\n\nFix direction: idempotency key = hash(content); on edit, queue an UPDATE op (Xero supports invoice updates pre-AUTHORISED).","notes":"Verified: claim holds up. queueSalesInvoiceForOrder at sales.ts:1310 short-circuits when accountingInvoiceId is set, and lib/connectors/xero/sync-processor.ts:35 builds idempotency keys from the internal sync-log entry ID rather than invoice content. So edits to a SalesOrder after Xero push are silently dropped on the floor — no UPDATE path exists.\n\nFixing this needs design work, not just a code patch:\n1. Schema: add SALES_INVOICE_UPDATE / PURCHASE_INVOICE_UPDATE entries to AccountingSyncType enum.\n2. Edit detection: SalesOrder line/discount/tax mutations need a hook that enqueues an UPDATE sync log when accountingInvoiceId is already set.\n3. Processor: branch in sync-processor.ts to PUT the invoice (Xero supports invoice updates while DRAFT or AUTHORISED, blocked once PAID).\n4. Idempotency: use a content hash (lines+totals+date) so re-edits generate fresh keys.\n5. UX: surface 'invoice in Xero is stale' in the UI when an edit happened post-push but the UPDATE is queued.\n\nRecommend: keep open, schedule as a focused 1-2 day task with finance team alignment on which fields trigger an UPDATE.","status":"open","priority":2,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-04-25T09:57:55Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T10:28:38Z","labels":["code-review","sync-xero"],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-ggg","title":"Invoice/bill edits after Xero push are silently lost","description":"Once accountingInvoiceId is set, queueSalesInvoiceForOrder returns immediately. Even if it didn't, the Xero idempotency key is built from the internal sync-log entry ID, not invoice content, so Xero would dedupe content changes.\n\nRefs:\n- app/actions/sales.ts:1310 (early return on accountingInvoiceId)\n- lib/connectors/xero/sync-processor.ts:35 (idempotency key from sync log entry ID)\n- lib/connectors/xero/sync-processor.ts:50-65, :73 (same on bill side)\n\nFix direction: idempotency key = hash(content); on edit, queue an UPDATE op (Xero supports invoice updates pre-AUTHORISED).","notes":"Verified: claim holds up. queueSalesInvoiceForOrder at sales.ts:1310 short-circuits when accountingInvoiceId is set, and lib/connectors/xero/sync-processor.ts:35 builds idempotency keys from the internal sync-log entry ID rather than invoice content. So edits to a SalesOrder after Xero push are silently dropped on the floor — no UPDATE path exists.\n\nFixing this needs design work, not just a code patch:\n1. Schema: add SALES_INVOICE_UPDATE / PURCHASE_INVOICE_UPDATE entries to AccountingSyncType enum.\n2. Edit detection: SalesOrder line/discount/tax mutations need a hook that enqueues an UPDATE sync log when accountingInvoiceId is already set.\n3. Processor: branch in sync-processor.ts to PUT the invoice (Xero supports invoice updates while DRAFT or AUTHORISED, blocked once PAID).\n4. Idempotency: use a content hash (lines+totals+date) so re-edits generate fresh keys.\n5. UX: surface 'invoice in Xero is stale' in the UI when an edit happened post-push but the UPDATE is queued.\n\nRecommend: keep open, schedule as a focused 1-2 day task with finance team alignment on which fields trigger an UPDATE.","status":"open","priority":2,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-04-25T09:57:55Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T10:28:38Z","defer_until":"2026-07-07T23:29:57Z","labels":["code-review","sync-xero"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-qnu","title":"Refund of an unshipped order throws with unhelpful error","description":"Refund COGS reversal hard-fails if there are no shipment/allocation snapshots. The error doesn't distinguish 'no shipments exist' from 'snapshots missing' from 'snapshots stale'.\n\nRefs:\n- app/actions/sales.ts:2162-2214\n\nFix direction: detect the no-history case explicitly and either short-circuit (refund without COGS reversal) or surface a clear error to the user.","status":"closed","priority":2,"issue_type":"bug","assignee":"OneTwo3D IMS","owner":"dev@onetwo3d.com","created_at":"2026-04-25T09:57:55Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T10:10:57Z","started_at":"2026-04-25T10:09:34Z","closed_at":"2026-04-25T10:10:57Z","close_reason":"Refund cost-reversal helpers now: (1) short-circuit with [] when the line has no shipment/allocation history (pre-fulfillment refunds), (2) throw clear messages when history exists but is insufficient — distinguishing 'never fulfilled' from 'stale snapshot'","labels":["code-review","o2c","refunds"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-v30","title":"Cancelling a partially shipped order orphans PACKED shipments and stock","description":"When status is set to CANCELLED, all PENDING/PICKING/PACKED shipments are deleted but stock is not returned. SHIPPED shipments remain, so the order cannot be fully refunded — and the deleted PACKED shipment's stock is permanently lost.\n\nRefs:\n- app/actions/sales.ts:1467\n\nFix direction: generate RETURN_INBOUND stock movements for non-SHIPPED shipments before deletion, OR forbid CANCELLED on partially shipped orders.","status":"closed","priority":2,"issue_type":"bug","assignee":"OneTwo3D IMS","owner":"dev@onetwo3d.com","created_at":"2026-04-25T09:57:54Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T10:14:17Z","started_at":"2026-04-25T10:14:17Z","closed_at":"2026-04-25T10:14:17Z","close_reason":"Verified: not a bug. Cancel flow at sales.ts:1456-1483 (a) calls deallocateOrder which releases reservedQty via applyAllocationReservationDelta('release'); (b) only deletes PENDING/PICKING/PACKED shipments — none of which decrement physical stock (FIFO consumption + StockLevel.quantity decrement happens only at SHIPPED transition in allocation.ts:1303-1310, 1327-1329); (c) blocks CANCELLED on fully SHIPPED orders. No stock is lost. Partially shipped orders end up in CANCELLED status with SHIPPED lines preserved for refund processing — that's intentional, not a leak.","labels":["code-review","o2c","state-machine"],"dependency_count":0,"dependent_count":0,"comment_count":0} -{"_type":"issue","id":"onetwo3d-ims-f91","title":"P8.2 'preserve forecast evidence on each line' needs schema spec — draft-PO lines have no metadata field today (PR #155 follow-up)","description":"PR #155 P8.2 acceptance:\n\n\u003e Generated draft PO lines carry enough forecast metadata to explain the suggested quantity.\n\nBut the existing `PurchaseOrderLine` schema doesn't have a structured place for forecast evidence:\n\n- Existing fields: qty, unitCostForeign, unitCostBase, taxRateId, totalForeign, totalBase, taxBase, taxForeign, productId, poId, sortOrder, etc.\n- No `forecastEvidence` or `reorderMetadata` field\n\nThe plan implies a schema addition without enumerating it. The 'forecast metadata' likely includes:\n- Days-of-stock-remaining at forecast time\n- Forecast horizon used (e.g., 30 days)\n- Lead-time days\n- Forecast model used (avg / median / EMA)\n- Inputs (current stock, average daily sales, on-order qty, etc.)\n- Generated-at timestamp\n\n**Why this matters:**\n\nThe acceptance criterion 'lines carry enough forecast metadata to explain the suggested quantity' is unverifiable without knowing what 'enough' means and where it lives. Either:\n\n1. **Add a JSON field to PurchaseOrderLine** for reorder metadata — e.g., `reorderEvidence Json? // null unless line was generated from a reorder forecast`. Schema migration needed.\n\n2. **Add a parallel table** `ReorderDraftLineEvidence` keyed on the PO line — keeps the main schema clean but requires joins.\n\n3. **Activity log only** — write a one-off activity log entry per line generated, with the forecast inputs. Less queryable but no schema change.\n\nThe plan doesn't pick. Without picking, the implementer guesses, and subsequent UI work to 'show the forecast evidence' depends on the guess.\n\n**Also worth specifying:**\n\n- What forecast model is used? Existing reorder report logic at `lib/domain/inventory/replenishment-reports.ts` — does P8.2 reuse this or introduce a new model?\n- How is the suggested quantity computed? Days-of-stock × daily sales rate × safety factor? The plan should pin the formula or reference the existing one.\n- The 'reorder-eligible' check: is it based on the existing reorder report's threshold logic, or a new rule?","acceptance_criteria":"- Update P8.2 to specify:\n - Where forecast evidence is stored (PO line field vs parallel table vs activity log)\n - What evidence is required (specific fields)\n - Whether P8.2 reuses the existing reorder-report logic or introduces new\n - Concrete forecast model / formula reference","notes":"Filed during PR #155 second-pass review. P8.2 is plan-only; this is about pinning the schema decision before implementation begins.","status":"open","priority":3,"issue_type":"chore","owner":"dev@onetwo3d.com","created_at":"2026-06-10T14:22:43Z","created_by":"Jan","updated_at":"2026-06-10T14:22:43Z","labels":["code-review","p8.2","plan","pr-155","reorder-forecast"],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-f91","title":"P8.2 'preserve forecast evidence on each line' needs schema spec — draft-PO lines have no metadata field today (PR #155 follow-up)","description":"PR #155 P8.2 acceptance:\n\n\u003e Generated draft PO lines carry enough forecast metadata to explain the suggested quantity.\n\nBut the existing `PurchaseOrderLine` schema doesn't have a structured place for forecast evidence:\n\n- Existing fields: qty, unitCostForeign, unitCostBase, taxRateId, totalForeign, totalBase, taxBase, taxForeign, productId, poId, sortOrder, etc.\n- No `forecastEvidence` or `reorderMetadata` field\n\nThe plan implies a schema addition without enumerating it. The 'forecast metadata' likely includes:\n- Days-of-stock-remaining at forecast time\n- Forecast horizon used (e.g., 30 days)\n- Lead-time days\n- Forecast model used (avg / median / EMA)\n- Inputs (current stock, average daily sales, on-order qty, etc.)\n- Generated-at timestamp\n\n**Why this matters:**\n\nThe acceptance criterion 'lines carry enough forecast metadata to explain the suggested quantity' is unverifiable without knowing what 'enough' means and where it lives. Either:\n\n1. **Add a JSON field to PurchaseOrderLine** for reorder metadata — e.g., `reorderEvidence Json? // null unless line was generated from a reorder forecast`. Schema migration needed.\n\n2. **Add a parallel table** `ReorderDraftLineEvidence` keyed on the PO line — keeps the main schema clean but requires joins.\n\n3. **Activity log only** — write a one-off activity log entry per line generated, with the forecast inputs. Less queryable but no schema change.\n\nThe plan doesn't pick. Without picking, the implementer guesses, and subsequent UI work to 'show the forecast evidence' depends on the guess.\n\n**Also worth specifying:**\n\n- What forecast model is used? Existing reorder report logic at `lib/domain/inventory/replenishment-reports.ts` — does P8.2 reuse this or introduce a new model?\n- How is the suggested quantity computed? Days-of-stock × daily sales rate × safety factor? The plan should pin the formula or reference the existing one.\n- The 'reorder-eligible' check: is it based on the existing reorder report's threshold logic, or a new rule?","acceptance_criteria":"- Update P8.2 to specify:\n - Where forecast evidence is stored (PO line field vs parallel table vs activity log)\n - What evidence is required (specific fields)\n - Whether P8.2 reuses the existing reorder-report logic or introduces new\n - Concrete forecast model / formula reference","notes":"Filed during PR #155 second-pass review. P8.2 is plan-only; this is about pinning the schema decision before implementation begins.","status":"closed","priority":3,"issue_type":"chore","assignee":"Jan","owner":"dev@onetwo3d.com","created_at":"2026-06-10T14:22:43Z","created_by":"Jan","updated_at":"2026-06-10T16:21:29Z","started_at":"2026-06-10T15:53:38Z","closed_at":"2026-06-10T16:21:29Z","close_reason":"Added PurchaseOrderLine.reorderEvidence JSON and populate it from supplier-scoped reorder forecast draft PO generation.","labels":["code-review","p8.2","plan","pr-155","reorder-forecast"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-l7d","title":"invariant-check-preflight script: no integration test against the real runScheduledInvariantCheck path (PR #155)","description":"PR #155's tests (`tests/cron/invariant-check-preflight.test.ts`) pass a mocked `runCheck` function to `runInvariantCheckPreflight`:\n\n```ts\nconst result = await runInvariantCheckPreflight(async () =\u003e scheduledResult({ ... }))\n```\n\nFour tests cover:\n- Happy path (no critical findings)\n- Critical findings present\n- Summary/list divergence (defensive check)\n- Partial report failure\n\nWhat's NOT tested:\n\n1. **The default `runCheck` parameter** (which constructs a real `runScheduledInvariantCheck` call with disabled side effects). The default is the codepath actually used by the script. If a future change breaks the parameter defaults (e.g., `writeActivityLog` accidentally re-enabled, causing CI side effects), no test catches it.\n\n2. **The `scripts/invariant-check-preflight.ts` orchestration**: `disconnectDb`, `process.exit(process.exitCode ?? 0)`, console output format, MAX_PRINTED_CRITICAL_FINDINGS truncation. None of these have tests.\n\n3. **End-to-end CI smoke**: the workflow yaml is verified-on-paper (run lint, type-check, validate). No test invokes `npm run invariant-check:preflight` against a known fixture and asserts behavior.\n\n**Why this matters:**\n\nThe CI gate is binary (pass/fail) — its only output is the exit code. A regression that always passes (e.g., `preflight.ok` accidentally hard-coded to true, or the script's `process.exitCode = 1` not firing) would silently turn the gate into a no-op, and no test would catch it.\n\nThis is the same 'test by inspection, not behavior' theme as PRs #150 (`9pk`), #151 (`cgq`), #152 (helper-only), #153 (`40l`).\n\n**Suggested fix:**\n\nAdd a focused script-level test that:\n1. Sets up a fixture DB or a mock-injected dependency\n2. Invokes the script's main function (extract it as exported for testability)\n3. Asserts the exit code matches expectations for: clean run, critical findings, report errors\n4. Asserts console output contains expected strings (truncation, remediation hint)","acceptance_criteria":"- Extract scripts/invariant-check-preflight.ts main() into a testable export\n- Add tests for: exit-code 0 on clean, exit-code 1 on critical findings, exit-code 1 on report errors, MAX_PRINTED truncation\n- Optionally: add an integration test that runs the real runScheduledInvariantCheck against a seeded fixture, asserts a known critical finding triggers exit 1","notes":"Filed during PR #155 review. Recurring 'mocked test only' pattern across the rollout. Lower priority because the script is small and the failure modes are well-bounded.","status":"closed","priority":3,"issue_type":"chore","assignee":"Jan","owner":"dev@onetwo3d.com","created_at":"2026-06-10T14:14:51Z","created_by":"Jan","updated_at":"2026-06-10T14:54:06Z","started_at":"2026-06-10T14:41:31Z","closed_at":"2026-06-10T14:54:06Z","close_reason":"Addressed in PR #155: expanded P8.1 migration/transition/incoming-stock plan detail, added script-level preflight tests, added real-data preflight fixture workflow step, and documented CI gate scope.","labels":["ci","invariants","pr-155","tests"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-dn2","title":"P8.1 'incoming stock' definition for auto-archive job spans many tables — needs concrete query spec (PR #155)","description":"PR #155's P8.1 plan item proposes an automatic archive job:\n\n\u003e when an EOL product has zero stock in every warehouse and no incoming stock remains, transition it to archived\n\u003e Incoming stock should include open purchase-order receipts and any modeled inbound supply such as inbound transfers, manufacturing outputs, or WMS ASN/booked-in evidence.\n\n'Incoming stock' as defined spans at least 5 different table contexts:\n\n1. **Open PO lines**: `purchase_order_lines` with `qty - qtyReceived \u003e 0` on POs in committed statuses (PO_SENT, SHIPPED, PARTIALLY_RECEIVED)\n2. **Inbound stock transfers**: `stock_transfer_lines` with `qty - qtyReceived \u003e 0` on transfers IN_TRANSIT to a warehouse holding the product\n3. **Manufacturing output**: `production_orders` with status in pre-completion states where outputProductId matches\n4. **WMS ASN evidence**: `wms_asn_lines` / `wms_booked_in_events` for the product\n5. **Implicit inbound** — refund returns? Customer returns? Inbound from supplier returns?\n\nA single 'has any incoming stock?' check requires querying ALL of these. The plan acceptance ('archive only after all warehouse stock is zero and incoming supply is zero') glosses over the actual implementation surface.\n\n**Why this matters:**\n\n1. Each table is independently maintained — missing one means false-positive archive (archive a product that has incoming stock the job didn't check)\n2. The job needs to run periodically (cron) — frequent enough to catch the 'stock reached zero' transition, infrequent enough not to thrash. Plan doesn't specify cadence.\n3. Race window: between the check and the status update, a new inbound supply event could land (e.g., operator creates a new PO line). The job needs to be transaction-safe.\n\n**Suggested concrete spec for P8.1:**\n\nDefine a single `getProductIncomingStock(productId)` helper that:\n- Returns a structured breakdown by source (open PO, transfer in, mfg output, WMS ASN, etc.)\n- Each source is its own query function for testability\n- Helper used by the auto-archive job AND by the operator-facing 'why didn't this archive?' diagnostic\n\nDefine cron cadence: probably daily, since the underlying event is 'stock dropped to zero', which itself happens at most a few times per day per product.\n\nDefine the race-safe transition: the archive should happen INSIDE a transaction that locks the product row and re-verifies the incoming-stock-zero condition.","acceptance_criteria":"- Update P8.1 in docs/plan.md with the concrete getProductIncomingStock breakdown (list of source tables and their conditions)\n- Define cron cadence and rationale\n- Define transaction-safety policy for the archive transition\n- Add a test acceptance: 'job archives product P → no transitions happen if a new PO line for P is created between the read and the write'","notes":"Filed during PR #155 review. Lower priority because P8.1 is plan-only; the implementer can fill in this detail. But it's the kind of detail that surfaces inconsistencies when not pinned upfront.","status":"closed","priority":3,"issue_type":"chore","assignee":"Jan","owner":"dev@onetwo3d.com","created_at":"2026-06-10T14:14:26Z","created_by":"Jan","updated_at":"2026-06-10T14:54:06Z","started_at":"2026-06-10T14:41:30Z","closed_at":"2026-06-10T14:54:06Z","close_reason":"Addressed in PR #155: expanded P8.1 migration/transition/incoming-stock plan detail, added script-level preflight tests, added real-data preflight fixture workflow step, and documented CI gate scope.","labels":["code-review","p8.1","plan","pr-155","product-lifecycle"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-xht","title":"PR #154 reintroduces invoice- filename + 10-min TTL cap + weak password list — already-filed PR #146 findings recur (PR #154)","description":"PR #154 makes substantively the same changes to the IMS admin invoice-pdf flow that PR #146 made. Several findings on PR #146 recur in PR #154 verbatim:\n\n- **Content-Disposition uses `invoice-${verification.payload.nonce}.pdf`** — random nonce in user's downloads folder, no security benefit. Filed as **onetwo3d-ims-2it** [P1].\n- **DEFAULT_INVOICE_PDF_TOKEN_TTL_SECONDS = 10 * 60 = MAX_INVOICE_PDF_TOKEN_TTL_SECONDS** — no admin override path. Filed as **onetwo3d-ims-quf** [P2].\n- **`COMMON_PASSWORDS`** is a 7-entry hardcoded Set. Filed as **onetwo3d-ims-eda** [P2].\n- **IP-binding via getClientIp() ?? 'unknown' fallback** — header-stripped requests collide on 'unknown'. Filed as **onetwo3d-ims-qv1** [P2] (PR #146) and **onetwo3d-ims-61b** [P2] (PR #148).\n- **`wrong_session` / `wrong_ip` audit not surfaced separately** — security signal not aggregated. Filed as **onetwo3d-ims-d1f** [P3].\n\n**Why this matters:** PR #146 was open at the time PR #154 was authored; PR #154 is presumably the merge-target that supersedes PR #146 with the additional shopping handoff route. The pre-existing findings carry forward and need to be addressed once at the merged scope, not separately per PR.\n\n**No new work needed** — this issue exists to:\n1. Flag the duplication so reviewers don't waste time re-finding the same issues\n2. Link the PR #146 → PR #154 trail in the bd tracker\n3. Make it clear which findings cluster needs to be addressed before PR #154 (or its merged successor) is merged","acceptance_criteria":"- When addressing PR #146 findings (or PR #154's equivalents), do so against whichever branch is the final merge target\n- Add a 'duplicates: onetwo3d-ims-2it, quf, eda, qv1/61b, d1f' note to PR #154's bd row\n- Close this issue when the underlying findings are addressed (or supersede with a single tracker if preferred)","notes":"Filed during PR #154 review. Cross-reference for the recurring findings between PR #146 and PR #154.","status":"open","priority":3,"issue_type":"chore","owner":"dev@onetwo3d.com","created_at":"2026-06-10T11:42:02Z","created_by":"Jan","updated_at":"2026-06-10T11:42:02Z","labels":["already-filed","code-review","pr-154","security"],"dependency_count":0,"dependent_count":0,"comment_count":0} @@ -93,13 +111,13 @@ {"_type":"issue","id":"onetwo3d-ims-69e","title":"Split-batch Xero journals fragment finance's daily view — N journals per day instead of one (PR #149)","description":"PR #149's daily batch caps mean a day with \u003e1000 of any entity (orders, shipments) gets split into multiple journal entries:\n\n- 5000 paid orders for date X → 5 separate `DAILY_BATCH_REVENUE_DEFERRAL` journals over multiple cron runs\n- Each journal has its own `referenceId` (`A1-2026-06-09-abc12345`, `A1-2026-06-09-def67890`, ...) and its own `reference` field (`Revenue Deferral 2026-06-09 abc12345`, `Revenue Deferral 2026-06-09 def67890`, ...)\n\nFinance reading the Xero ledger sees 5 'Revenue Deferral 2026-06-09 {hash}' entries instead of 1. Reconciliation requires summing all 5 to match the IMS daily total. The hash suffix is opaque to finance — they have to manually correlate.\n\n**Why this matters:** The pre-PR behavior was 1 journal per day per group. Tenants with small volumes (\u003c1000/day) keep that. Tenants with larger volumes get a different reporting structure than smaller tenants. The change in journal shape isn't documented in CHANGELOG and finance teams will see this on the first overflow day with no warning.\n\nThe split is operationally necessary (single 5000-line journals are slow and fragile in Xero), but the UX of multiple per-day journals is a downstream concern that wasn't addressed.\n\nPossible mitigations (each has trade-offs):\n\n1. **Aggregate at month-end**: split journals retained for audit trail but a reconciliation report rolls up by date\n2. **Use Xero 'manual journal grouping'** if it supports linking multiple journals as one logical posting\n3. **Document the new structure**: add a CHANGELOG entry explaining the journal split for tenants \u003e 1000 daily orders, with guidance on reconciliation\n4. **Use a shared correlation field** that finance can filter on (a 'batchDate' tag or similar)","acceptance_criteria":"- Add CHANGELOG.md entry: 'For tenants posting more than 1000 orders/shipments per day, daily batch journals are now split into multiple entries per day. Each entry's reference includes a deterministic hash; reconciliation should sum entries matching the date prefix.'\n- Add a reference column / metadata field to the Xero manual journal payload that finance can use to group: e.g., `batchDate: '2026-06-09'` separate from the `reference` text field\n- Consider a settings-page report that shows 'Daily batch summary for date X' aggregating across all split batches — operator-friendly reconciliation view\n- Acceptance criteria for the docs/plan.md plan group 15 completion should include 'finance reconciliation guidance documented'","notes":"Filed during PR #149 review. Lower priority than gnt because it's UX/finance-team friction rather than data correctness. But it's the first thing finance will complain about.","status":"open","priority":3,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-06-09T18:14:34Z","created_by":"Jan","updated_at":"2026-06-09T18:14:34Z","labels":["code-review","finance-ux","pr-149","xero"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-d1f","title":"PDF token audit treats wrong_session and wrong_ip as ordinary verification failures — security signal lost (PR #146)","description":"PR #146 adds new `wrong_session` and `wrong_ip` reasons to `PdfTokenVerificationFailureReason`. The audit logger (`auditInvoicePdfTokenAttempt`) treats them the same as any other verification failure — they get logged but not distinguished as potential token-sharing / theft signals.\n\nThese two failure modes are interesting because they specifically indicate:\n- `wrong_session`: a valid token from a previous auth session is being presented. Could be a re-authenticated user using a bookmarked link (benign) or a stolen token (malicious).\n- `wrong_ip`: a token signed for IP A is being presented from IP B. Could be a network change (benign per `onetwo3d-ims-qv1`) or a token shared/leaked (suspicious).\n\nAggregated patterns matter:\n- `wrong_session` for the SAME user repeatedly → likely benign (user re-authed)\n- `wrong_ip` from many different source IPs for tokens belonging to the SAME user → token shared in a public Slack channel\n- `wrong_session` + `wrong_ip` from a different user agent → likely stolen\n\nCurrent audit logs each attempt at the same level, with no aggregation, no rate-based alerting.","acceptance_criteria":"- Add a separate higher-severity activity log entry (`level: 'WARNING'` instead of `'INFO'`) for `wrong_session` and `wrong_ip` reasons\n- Include user-agent in the audit row metadata so investigators can spot UA mismatches\n- Add a dashboard / settings widget that surfaces recent `wrong_session` / `wrong_ip` events grouped by invoice id, similar to the tax-rate fallback widget added in PR #144\n- (Optional) Add a rate-based alert: if more than N `wrong_*` events for the same invoice id in T minutes, notify admins","notes":"Filed during PR #146 review. Lower priority because the audit logging happens; this is about extracting more value from it. Operational maturity, not correctness.","status":"open","priority":3,"issue_type":"chore","owner":"dev@onetwo3d.com","created_at":"2026-06-08T23:08:49Z","created_by":"Jan","updated_at":"2026-06-08T23:08:49Z","labels":["code-review","observability","pr-146","security"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-9x6","title":"Plan items newly marked Complete in PR #145 lack test pointers / verifying-commit refs (P0.1-P0.5, P2.1, P2.2, P2.4)","description":"Codex adversarial review of PR #145 identified:\n\nMultiple `Status: Complete` markers were added in this PR's `docs/plan.md` changes (P0.1–P0.5, P2.1, P2.2, P2.4) without:\n1. Replacing the future-tense test text (\"Add an integration test...\") with an actual test file path\n2. Including a `Reference:` line pointing to the implementing/verifying commit SHA\n\nThis matches the \"Complete marker rests on inspection\" anti-pattern called out earlier in this rollout (see bd onetwo3d-ims-8c7 for the P1.3/P1.5/P4.2 sweep).\n\n**Why this matters:** The plan.md is the canonical audit document for the production-readiness rollout. \"Complete\" markers backed only by PR-body assertions (rather than test pointers) are unverifiable by future contributors. Three months from now, someone auditing the plan won't know whether P0.1 was actually fixed or just claimed-fixed.\n\n**Concrete findings from PR #145's docs/plan.md hunk:**\n- P0.1 marked Complete — original Tests line still says \"Add an integration test that hits /api/auth/totp-setup...\"\n- P0.2 marked Complete — original Tests line still future-tense\n- P0.3, P0.4, P0.5 — same pattern\n- P2.1, P2.2, P2.4 — same pattern","acceptance_criteria":"- For each newly-marked-complete item, either:\n - Replace the future-tense Tests line with the actual test file path + test name (e.g. `tests/security/totp-setup-route.test.ts::\"rejects when TOTP already configured\"`), OR\n - Add a `Reference:` line naming the implementing commit SHA + brief verification note\n- Audit the consolidated section's checkbox list to confirm every checked item also has a pointer\n- Same sweep needed for PR #143's bd-issue closures (hyz, a8u, ahu) — those PRs closed bd issues with prose close_reasons but the plan.md entries may not have test pointers","notes":"Filed during Codex review of PR #145. Codex severity P3 (process/cosmetic) — the markers don't break code, but they degrade the audit value of the plan.md document. Related: bd onetwo3d-ims-8c7 covers the same sweep for P1.3/P1.5/P4.2.","status":"open","priority":3,"issue_type":"chore","owner":"dev@onetwo3d.com","created_at":"2026-06-08T22:16:28Z","created_by":"Jan","updated_at":"2026-06-08T22:16:28Z","labels":["docs","plan-followup","pr-145"],"dependency_count":0,"dependent_count":0,"comment_count":0} -{"_type":"issue","id":"onetwo3d-ims-lvs","title":"Cost-layer revaluation audit log stores previous + patched + delta — 3x storage overhead (PR #144)","description":"PR #144's `recordCostLayerSnapshotRevaluation()` (lib/cost-layers.ts) writes an activity log row per snapshot revaluation with metadata containing:\n\n```ts\nmetadata: {\n ...\n changedEntries: params.changedEntries, // entries that changed\n previousSnapshot: params.previousSnapshot, // FULL old snapshot\n patchedSnapshot: params.patchedSnapshot, // FULL new snapshot\n}\n```\n\nFor a ShipmentLine with 50 cost-layer entries where 1 entry changes, the activity log row stores:\n- 1 changedEntry (the delta)\n- 50 entries in previousSnapshot\n- 50 entries in patchedSnapshot\n\nSame data stored ~3x. For a landed-cost adjustment touching 100 shipment lines, that's ~300x the snapshot data in activity_logs.\n\n**Why this matters:** The original bd issue `onetwo3d-ims-imn` (now closed by this PR) asked for an audit trail — but didn't specify the audit needed to store the full snapshot. The changedEntries field alone reconstructs the delta. Both full snapshots are operationally useful for one-off forensics but bloat the activity_logs table for routine landed-cost revaluations.\n\nactivity_logs is a hot table (other reports, dashboard queries, daily aggregations read it). Bloat here propagates.","acceptance_criteria":"- Decide retention/granularity trade-off:\n - Option A: store only changedEntries (the delta) — reconstruct full snapshots from delta + current shipment_line.costLayerSnapshot on demand\n - Option B: store previousSnapshot only (since patchedSnapshot equals shipment_line.costLayerSnapshot after the update)\n - Option C: keep current (3x) but document the trade-off and add a sweeper that compacts old revaluation rows after N days\n- Add a row-size assertion test that proves a 50-entry snapshot doesn't produce a 50KB activity_log row\n- If retention is bounded (e.g., 90 days), this becomes self-limiting and current shape is fine — but should be explicit","notes":"Identified during PR #144 review. Lower priority because storage is cheap and audit trails are valuable — but worth knowing before the activity_logs table hits a size that affects query performance.","status":"open","priority":3,"issue_type":"chore","owner":"dev@onetwo3d.com","created_at":"2026-06-07T23:37:37Z","created_by":"Jan","updated_at":"2026-06-07T23:37:37Z","labels":["audit","code-review","pr-144","storage"],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-lvs","title":"Cost-layer revaluation audit log stores previous + patched + delta — 3x storage overhead (PR #144)","description":"PR #144's `recordCostLayerSnapshotRevaluation()` (lib/cost-layers.ts) writes an activity log row per snapshot revaluation with metadata containing:\n\n```ts\nmetadata: {\n ...\n changedEntries: params.changedEntries, // entries that changed\n previousSnapshot: params.previousSnapshot, // FULL old snapshot\n patchedSnapshot: params.patchedSnapshot, // FULL new snapshot\n}\n```\n\nFor a ShipmentLine with 50 cost-layer entries where 1 entry changes, the activity log row stores:\n- 1 changedEntry (the delta)\n- 50 entries in previousSnapshot\n- 50 entries in patchedSnapshot\n\nSame data stored ~3x. For a landed-cost adjustment touching 100 shipment lines, that's ~300x the snapshot data in activity_logs.\n\n**Why this matters:** The original bd issue `onetwo3d-ims-imn` (now closed by this PR) asked for an audit trail — but didn't specify the audit needed to store the full snapshot. The changedEntries field alone reconstructs the delta. Both full snapshots are operationally useful for one-off forensics but bloat the activity_logs table for routine landed-cost revaluations.\n\nactivity_logs is a hot table (other reports, dashboard queries, daily aggregations read it). Bloat here propagates.","acceptance_criteria":"- Decide retention/granularity trade-off:\n - Option A: store only changedEntries (the delta) — reconstruct full snapshots from delta + current shipment_line.costLayerSnapshot on demand\n - Option B: store previousSnapshot only (since patchedSnapshot equals shipment_line.costLayerSnapshot after the update)\n - Option C: keep current (3x) but document the trade-off and add a sweeper that compacts old revaluation rows after N days\n- Add a row-size assertion test that proves a 50-entry snapshot doesn't produce a 50KB activity_log row\n- If retention is bounded (e.g., 90 days), this becomes self-limiting and current shape is fine — but should be explicit","notes":"Identified during PR #144 review. Lower priority because storage is cheap and audit trails are valuable — but worth knowing before the activity_logs table hits a size that affects query performance.","status":"open","priority":3,"issue_type":"chore","owner":"info@onetwo3d.co.uk","created_at":"2026-06-07T23:45:04Z","created_by":"Jan","updated_at":"2026-06-07T23:37:37Z","labels":["audit","code-review","pr-144","storage"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-8c7","title":"Clean up stale test path pointers in docs/plan.md (P1.3, P1.5, P4.2)","description":"Several `**Status:** Complete` plan items in docs/plan.md retained their original `**Tests:**` lines, which point at file paths that don't exist on origin/development.\n\nStale pointers found during PR #140 review audit:\n\n| Item | Plan claims | Actually exists |\n|---|---|---|\n| P1.3 | `tests/refund-service.idempotency.test.ts` | Path doesn't exist. Adjacent helper-level tests live in `tests/domain/sales/refund-service.test.ts` and `tests/woocommerce-refund-sync.test.ts`. |\n| P1.5 | `tests/refund-service.revaluation.test.ts` | Path doesn't exist. Revaluation tests are in `tests/domain/sales/refund-service.test.ts` (lines 703, 759, 815). |\n| P4.2 | 'Complete two production orders same day; assert FIFO order matches completedAt' (no file path) | No such integration test. Helper-level coverage only in `tests/domain/manufacturing/manufacturing-action-inputs.test.ts`. |\n\n**Why this matters:** Anyone auditing the plan to verify a completed item follows a broken link, gets nothing, and either re-implements the test (waste) or assumes coverage exists (silent regression risk).","acceptance_criteria":"- Update docs/plan.md P1.3 Tests line to point at actual test file + test name\n- Update docs/plan.md P1.5 Tests line to point at the three landed cost revaluation tests in refund-service.test.ts\n- Either fix P4.2 Tests line (after the P4.2 follow-up issue lands integration test) OR remove the prose acceptance criterion and replace with reference to the helper-level test\n- Audit remaining `**Status:** Complete` items for similar stale pointers","notes":"Two follow-up issues filed for P1.3 and P4.4 missing integration tests; this issue is just the docs cleanup. P1.5's tests do exist, just in a different file than the plan claims — fix the pointer.","status":"open","priority":3,"issue_type":"chore","owner":"dev@onetwo3d.com","created_at":"2026-06-07T18:09:01Z","created_by":"Jan","updated_at":"2026-06-07T18:09:01Z","labels":["docs","plan-followup"],"dependency_count":0,"dependent_count":0,"comment_count":0} -{"_type":"issue","id":"onetwo3d-ims-4nf","title":"Compound tax / reverse charge / EC sales / OSS not supported","description":"Each line carries a single taxRateId/taxRatePercent. Use cases not currently expressible:\n- Compound taxes (e.g. VAT + provincial surcharge)\n- Reverse charge B2B (zero-rated outbound, customer self-accounts)\n- EC sales / OSS reporting buckets\n- Multi-tax stacking for jurisdictions with both federal and state tax\n\nLower priority than the per-line fallback bugs (xx4, hyz) but worth tracking. UK-only sellers won't notice; expansion to EU OSS or the US would expose this.\n\nFix direction:\n- Schema: a join table linking SalesOrderLine/PurchaseOrderLine to multiple TaxRate rows with order/compound flags.\n- Resolver: extend pickTaxRate to return a list of applicable rates with compound rules.\n- Connector mapping: confirm Xero TaxComponent / QB ItemBased tax codes can carry compound rates.","status":"open","priority":3,"issue_type":"feature","owner":"dev@onetwo3d.com","created_at":"2026-04-25T20:46:03Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T20:46:03Z","labels":["vat"],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-4nf","title":"Compound tax / reverse charge / EC sales / OSS not supported","description":"Each line carries a single taxRateId/taxRatePercent. Use cases not currently expressible:\n- Compound taxes (e.g. VAT + provincial surcharge)\n- Reverse charge B2B (zero-rated outbound, customer self-accounts)\n- EC sales / OSS reporting buckets\n- Multi-tax stacking for jurisdictions with both federal and state tax\n\nLower priority than the per-line fallback bugs (xx4, hyz) but worth tracking. UK-only sellers won't notice; expansion to EU OSS or the US would expose this.\n\nFix direction:\n- Schema: a join table linking SalesOrderLine/PurchaseOrderLine to multiple TaxRate rows with order/compound flags.\n- Resolver: extend pickTaxRate to return a list of applicable rates with compound rules.\n- Connector mapping: confirm Xero TaxComponent / QB ItemBased tax codes can carry compound rates.","status":"open","priority":3,"issue_type":"feature","owner":"dev@onetwo3d.com","created_at":"2026-04-25T20:46:03Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T20:46:03Z","defer_until":"2026-09-05T23:29:10Z","labels":["vat"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-9z8","title":"Native VAT / tax liability report (currently delegated to Xero/QB)","description":"There is no first-class VAT report module in IMS. The only aggregation is groupVatBreakdown() inside app/actions/xero-daily-batch.ts:99-141, used to assemble the Xero daily batch payload. Tax liability reporting is effectively delegated to the accounting connector.\n\nIf a customer doesn't run Xero/QB or wants to reconcile before posting, they have no answer.\n\nFix direction: a VAT report page that takes a date range and outputs by tax-rate breakdown of net, tax, gross for both sales and purchases. Reuse the existing snapshot fields (taxRatePercent, taxBase, totalBase) so it stays consistent with what is sent to Xero.","status":"open","priority":3,"issue_type":"feature","owner":"dev@onetwo3d.com","created_at":"2026-04-25T20:45:56Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T20:45:56Z","labels":["reporting","vat"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-m1i","title":"Multi-currency reporting: roll up by document currency, not just base","description":"Reports (sales-stats, dashboard, product-profitability) aggregate exclusively against `*Base` fields. There is no view that, for example, shows USD revenue separately from GBP revenue.\n\nLimitation: hard to answer 'how much business did we do in EUR last quarter' or 'what is our USD AR balance' without exporting and pivoting manually.\n\nFix direction:\n- Add document-currency dimension to existing report queries.\n- Optionally a dedicated 'multi-currency summary' page breaking down revenue, COGS, AR, AP per currency with both foreign and base-equivalent columns.","status":"open","priority":3,"issue_type":"feature","owner":"dev@onetwo3d.com","created_at":"2026-04-25T20:45:47Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T20:45:47Z","labels":["fx","reporting"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-2bf","title":"[bug] Decimal precision drift across 2/4/6dp boundaries on large batches","description":"The codebase uses three different precision conventions:\n- 6dp for COGS (`unitCostBase`, Decimal(18,6))\n- 4dp for base-currency line/order amounts (Math.round(x * 10000) / 10000)\n- 6dp for Xero CurrencyRate exports\n- 2dp for GBP final display\n\nConversions between these layers (foreign → base → Xero → display) can accumulate sub-cent drift on documents with many lines or on aggregations across periods. Not catastrophic individually, but reconciliation tolerance against Xero needs to be loose to absorb it.\n\nFix direction:\n- Audit conversion call sites and standardise on a single rounding helper that takes precision as an argument.\n- Add a reconciliation test that runs a synthetic 1000-line order through SO → invoice → Xero payload and asserts |IMS - Xero| \u003c epsilon.\n- Document the chosen rule and why each precision exists.\n\nCross-cutting; relates to onetwo3d-ims-hyz, -a8u, -ahu.","status":"open","priority":3,"issue_type":"bug","owner":"dev@onetwo3d.com","created_at":"2026-04-25T20:45:43Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T20:45:43Z","labels":["code-review","cogs","fx","vat"],"dependency_count":0,"dependent_count":0,"comment_count":0} -{"_type":"issue","id":"onetwo3d-ims-5mp.2","title":"Verify Xero credit-note Type field: ACCREC vs ACCRECCREDIT","description":"Pre-existing concern surfaced in the Phase 1 Codex review of the FX work.\n\nlib/connectors/xero/credit-notes.ts:58 currently sets:\n Type: 'ACCREC'\n\nPer the Xero CreditNote OpenAPI schema, the Type enum is 'ACCRECCREDIT' / 'ACCPAYCREDIT' (NOT 'ACCREC' / 'ACCPAY' — those are Invoice types).\n\nCredit notes have apparently been syncing in production, so either:\n- Xero is lenient on this field for the CreditNotes endpoint, OR\n- the credit note is being silently miscategorised (e.g. coerced to a default type)\n\nAction:\n1. Verify in a Xero sandbox what actually happens when 'ACCREC' is posted to /CreditNotes — does it succeed, fail, or coerce?\n2. If wrong, change to 'ACCRECCREDIT' and add unit/integration coverage.\n3. Check git history for why ACCREC was originally used (could be a deliberate workaround).\n\nNot introduced by the FX work but the FX changes now ride on this code path, making it worth resolving.","status":"open","priority":3,"issue_type":"task","owner":"dev@onetwo3d.com","created_at":"2026-04-25T11:48:16Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T11:48:16Z","labels":["connectors","credit-note","fx","woocommerce","xero"],"dependencies":[{"issue_id":"onetwo3d-ims-5mp.2","depends_on_id":"onetwo3d-ims-5mp","type":"parent-child","created_at":"2026-04-25T11:48:15Z","created_by":"OneTwo3D IMS","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-5mp.2","title":"Verify Xero credit-note Type field: ACCREC vs ACCRECCREDIT","description":"Pre-existing concern surfaced in the Phase 1 Codex review of the FX work.\n\nlib/connectors/xero/credit-notes.ts:58 currently sets:\n Type: 'ACCREC'\n\nPer the Xero CreditNote OpenAPI schema, the Type enum is 'ACCRECCREDIT' / 'ACCPAYCREDIT' (NOT 'ACCREC' / 'ACCPAY' — those are Invoice types).\n\nCredit notes have apparently been syncing in production, so either:\n- Xero is lenient on this field for the CreditNotes endpoint, OR\n- the credit note is being silently miscategorised (e.g. coerced to a default type)\n\nAction:\n1. Verify in a Xero sandbox what actually happens when 'ACCREC' is posted to /CreditNotes — does it succeed, fail, or coerce?\n2. If wrong, change to 'ACCRECCREDIT' and add unit/integration coverage.\n3. Check git history for why ACCREC was originally used (could be a deliberate workaround).\n\nNot introduced by the FX work but the FX changes now ride on this code path, making it worth resolving.","status":"open","priority":3,"issue_type":"task","owner":"dev@onetwo3d.com","created_at":"2026-04-25T11:48:16Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T11:48:16Z","labels":["connectors","credit-note","fx","woocommerce","xero"],"dependencies":[{"issue_id":"onetwo3d-ims-5mp.2","depends_on_id":"onetwo3d-ims-5mp","type":"parent-child","created_at":"2026-04-25T11:48:15Z","created_by":"Jan","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-u48","title":"Manufacturing has no partial-completion / scrap recording","description":"qtyProduced is always set to qtyPlanned on completion. No field for actual_qty_completed or scrap_qty.\n\nRefs:\n- app/actions/manufacturing.ts:928\n\nFix direction: add actualQty + scrapQty fields, surface in completion form, recompute unit cost from actualQty.","status":"closed","priority":3,"issue_type":"feature","assignee":"OneTwo3D IMS","owner":"dev@onetwo3d.com","created_at":"2026-04-25T09:57:59Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T10:31:53Z","started_at":"2026-04-25T10:31:52Z","closed_at":"2026-04-25T10:31:53Z","close_reason":"Closing as feature request, not a code-review bug. Adding actualQty/scrapQty fields and rebasing unit cost on actual output is a focused but real product change (schema migration + UI + tests). File as a manufacturing-roadmap task.","labels":["code-review","manufacturing"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-1hr","title":"BOM lacks scrap/yield, substitutions, and nesting","description":"BomItem has only (parentProductId, componentProductId, qty). Missing capabilities:\n- scrap percentage / yield loss\n- alternative components (substitutions)\n- nested BOMs (sub-assemblies)\n\nRefs:\n- prisma/schema.prisma — BomItem model\n- app/actions/manufacturing.ts\n\nTradeoff vs scope: each is a meaningful schema + UI change. Defer until a real customer request, but capture as known limits.","status":"closed","priority":3,"issue_type":"feature","assignee":"OneTwo3D IMS","owner":"dev@onetwo3d.com","created_at":"2026-04-25T09:57:58Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T10:31:52Z","started_at":"2026-04-25T10:31:52Z","closed_at":"2026-04-25T10:31:52Z","close_reason":"Closing as feature request, not a code-review bug. Nested BOMs, scrap/yield, and component substitution are real product gaps but each requires schema work (BomItem extensions or new tables), UI work, and product alignment. Not a 'fix' — file as a manufacturing-roadmap epic when the customer ask materialises.","labels":["code-review","manufacturing"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-o1v","title":"Xero payment poll excludes WooCommerce orders","description":"Payment polling only reconciles manual orders (shoppingLinks.none). WC orders rely on webhooks for payment capture; missed/late webhooks leave WC orders stuck in PENDING_PAYMENT even when Xero shows them paid.\n\nRefs:\n- lib/connectors/xero/payment-poller.ts:29-35\n\nFix direction: include WC orders in the poll scope, gated by a 'reconcile via Xero' setting per connector instance.","status":"closed","priority":3,"issue_type":"bug","assignee":"OneTwo3D IMS","owner":"dev@onetwo3d.com","created_at":"2026-04-25T09:57:58Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T10:28:37Z","started_at":"2026-04-25T10:28:37Z","closed_at":"2026-04-25T10:28:37Z","close_reason":"Closing as known design choice. The exclusion at payment-poller.ts:35 (shoppingLinks: { none: {} }) is intentional and documented in the file header comment ('Sales: manual orders only (WC orders arrive with paidAt already set)') — WC payment status is owned by WC webhooks. This is a separation of concerns, not a bug. A defensive fallback (poll Xero for WC orders unpaid \u003e N days) would be a useful feature but needs product input and is out of scope for code-review fixes. File a feature request if the missed-webhook scenario becomes operationally important.","labels":["code-review","sync-woo","sync-xero"],"dependency_count":0,"dependent_count":0,"comment_count":0} @@ -107,6 +125,6 @@ {"_type":"issue","id":"onetwo3d-ims-bnz","title":"Stock-sync silently filters out products with no telemetry","description":"buildShoppingStockUpdates filters out products by lifecycle status / missing warehouse map without logging. Sync reports success=true with 0 updates and the user has no way to know why.\n\nRefs:\n- lib/shopping.ts:89-250\n\nFix direction: return per-product skip reasons; log skipped count to activity log; surface in the sync run summary.","status":"closed","priority":3,"issue_type":"bug","assignee":"OneTwo3D IMS","owner":"dev@onetwo3d.com","created_at":"2026-04-25T09:57:57Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T10:11:09Z","started_at":"2026-04-25T10:10:57Z","closed_at":"2026-04-25T10:11:09Z","close_reason":"Resolved as part of onetwo3d-ims-tlq: lib/shopping.ts now tracks per-reason skip counts in buildShoppingStockUpdates and emits a stock_sync_skip_summary activity log entry (level WARNING when nothing was pushed). Connector ID is included in metadata.","labels":["audit-log","code-review","sync-woo"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-n39","title":"Xero rate-limit budget is in-memory only — resets on Node restart","description":"Minute/day buckets reset on every Node restart. Deployments mid-sync trigger cascading 429s on the new process.\n\nRefs:\n- lib/connectors/xero/api.ts:83\n\nFix direction: persist bucket counters to Redis or a small DB table keyed on tenant ID.","status":"closed","priority":3,"issue_type":"bug","assignee":"OneTwo3D IMS","owner":"dev@onetwo3d.com","created_at":"2026-04-25T09:57:57Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T10:28:37Z","started_at":"2026-04-25T10:28:37Z","closed_at":"2026-04-25T10:28:37Z","close_reason":"Verified: minor concern, not a real bug in practice. In-memory rate-limit buckets reset on Node restart, BUT processPendingXeroSync caps at MAX_PER_RUN=50 (sync-processor.ts:19) and runs every 5 min — i.e. ~10 req/min average vs Xero's 60/min cap. Even with cold-start bursting, 50 req in \u003c60s is at most 83% of the per-minute cap (one process). The exponential backoff on 429 (api.ts:83-88) handles the rare case where we collide with another process. Persisting buckets to Redis would be a defensive improvement but isn't a bug fix.","labels":["code-review","reliability","sync-xero"],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"issue","id":"onetwo3d-ims-7mf","title":"Xero invoices stuck in DRAFT after payment lands","description":"Posting mode (DRAFT vs AUTHORISED) is decided once at sync time. There is no follow-up to promote a DRAFT invoice to AUTHORISED when payment is detected.\n\nRefs:\n- lib/connectors/xero/sync-processor.ts:233-235 (comment acknowledges the distinction), :246-250\n\nVERIFY: may not be IMS's responsibility if the Xero workflow handles AUTHORISED transitions itself.","status":"closed","priority":3,"issue_type":"bug","assignee":"OneTwo3D IMS","owner":"dev@onetwo3d.com","created_at":"2026-04-25T09:57:56Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T10:31:51Z","started_at":"2026-04-25T10:31:51Z","closed_at":"2026-04-25T10:31:51Z","close_reason":"Closing as not-actionable. Posting mode is decided per AccountingSyncLog entry; downstream tooling (Xero web UI, Xero approver workflows) is responsible for promoting DRAFT to AUTHORISED. Building an IMS-side promotion job duplicates Xero's invoice approval workflow, which is the auditor-preferred path. If a customer has DRAFT invoices that need to be AUTHORISED en masse on payment, that's a Xero workflow rule, not an IMS code change.","labels":["code-review","sync-xero","verify"],"dependency_count":0,"dependent_count":0,"comment_count":0} -{"_type":"issue","id":"onetwo3d-ims-5mp.3","title":"Document idempotency behaviour when fxRateToBase changes between requeues","description":"Concern surfaced in the Phase 1 Codex review.\n\naccountingPayloadKey() (app/actions/purchase-orders.ts:27) hashes the full payload as the idempotency key. With the new currencyRateToBase field included in the payload, a re-queue of the same document where the FX rate has changed produces a *different* hash, which means:\n\n- queue-level dedupe at lib/accounting.ts:157-169 misses (different key)\n- Xero idempotency at lib/connectors/xero/api.ts:110-112 is entry-id based, so it also won't dedupe\n\nIn theory this could allow a duplicate Xero post if the rate changed between the original queue and a retry. In practice rates don't change between retries (the rate was already stamped on the IMS document at creation time and we read it from there, not re-fetched), but the behaviour is non-obvious.\n\nAction:\n1. Document this in the unified-FX plan (docs/todo/unified-fx-rates-plan.md) under 'Open questions' or a new 'Idempotency' section.\n2. Decide: should accountingPayloadKey() exclude currencyRateToBase from the hash so a rate change doesn't produce a new key? OR should it be included so a deliberate re-stamp does produce a new post?\n3. Either way, add a code comment explaining the choice.\n\nLow priority — this is a 'nice to have clarification', not a known bug.","status":"open","priority":4,"issue_type":"task","owner":"dev@onetwo3d.com","created_at":"2026-04-25T11:48:27Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T11:48:27Z","labels":["connectors","fx","idempotency","woocommerce","xero"],"dependencies":[{"issue_id":"onetwo3d-ims-5mp.3","depends_on_id":"onetwo3d-ims-5mp","type":"parent-child","created_at":"2026-04-25T11:48:27Z","created_by":"OneTwo3D IMS","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} +{"_type":"issue","id":"onetwo3d-ims-5mp.3","title":"Document idempotency behaviour when fxRateToBase changes between requeues","description":"Concern surfaced in the Phase 1 Codex review.\n\naccountingPayloadKey() (app/actions/purchase-orders.ts:27) hashes the full payload as the idempotency key. With the new currencyRateToBase field included in the payload, a re-queue of the same document where the FX rate has changed produces a *different* hash, which means:\n\n- queue-level dedupe at lib/accounting.ts:157-169 misses (different key)\n- Xero idempotency at lib/connectors/xero/api.ts:110-112 is entry-id based, so it also won't dedupe\n\nIn theory this could allow a duplicate Xero post if the rate changed between the original queue and a retry. In practice rates don't change between retries (the rate was already stamped on the IMS document at creation time and we read it from there, not re-fetched), but the behaviour is non-obvious.\n\nAction:\n1. Document this in the unified-FX plan (docs/todo/unified-fx-rates-plan.md) under 'Open questions' or a new 'Idempotency' section.\n2. Decide: should accountingPayloadKey() exclude currencyRateToBase from the hash so a rate change doesn't produce a new key? OR should it be included so a deliberate re-stamp does produce a new post?\n3. Either way, add a code comment explaining the choice.\n\nLow priority — this is a 'nice to have clarification', not a known bug.","status":"open","priority":4,"issue_type":"task","owner":"dev@onetwo3d.com","created_at":"2026-04-25T11:48:27Z","created_by":"OneTwo3D IMS","updated_at":"2026-04-25T11:48:27Z","labels":["connectors","fx","idempotency","woocommerce","xero"],"dependencies":[{"issue_id":"onetwo3d-ims-5mp.3","depends_on_id":"onetwo3d-ims-5mp","type":"parent-child","created_at":"2026-04-25T11:48:27Z","created_by":"Jan","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} {"_type":"memory","key":"list","value":"list"} {"_type":"memory","key":"pr-review-bd-followup-push-target","value":"When filing bd issues as PR-review follow-ups, push the resulting .beads/issues.jsonl change to the SAME PR's branch (not to whatever branch the local working tree is on). Workflow: check out the PR's branch (gh pr checkout N), file bd issues, commit only .beads/issues.jsonl, push to the PR branch. This keeps review findings tied to the PR's history and avoids creating drive-by commits on unrelated feature branches."} diff --git a/app/(dashboard)/analytics/forecast/forecast-client.tsx b/app/(dashboard)/analytics/forecast/forecast-client.tsx index 78806c62..14172458 100644 --- a/app/(dashboard)/analytics/forecast/forecast-client.tsx +++ b/app/(dashboard)/analytics/forecast/forecast-client.tsx @@ -314,6 +314,7 @@ export function ForecastClient({ forecasts, settings }: Props) { const [selected, setSelected] = useState>(new Set()) const [filter, setFilter] = useState<'all' | 'critical' | 'low' | 'ok' | 'overstock'>('all') const [abcFilter, setAbcFilter] = useState<'all' | 'A' | 'B' | 'C'>('all') + const [supplierFilter, setSupplierFilter] = useState('all') const [search, setSearch] = useState('') const [error, setError] = useState('') const [sortField, setSortField] = useState('daysUntilStockout') @@ -333,6 +334,7 @@ export function ForecastClient({ forecasts, settings }: Props) { const filtered = forecasts.filter((f) => { if (filter !== 'all' && f.urgency !== filter) return false if (abcFilter !== 'all' && f.abcClass !== abcFilter) return false + if (supplierFilter !== 'all' && f.supplierId !== supplierFilter) return false if (search) { const q = search.toLowerCase() return f.sku.toLowerCase().includes(q) || f.name.toLowerCase().includes(q) || (f.supplierName ?? '').toLowerCase().includes(q) @@ -346,19 +348,31 @@ export function ForecastClient({ forecasts, settings }: Props) { const paged = sorted.slice((safePage - 1) * PAGE_SIZE, safePage * PAGE_SIZE) const needsReorder = forecasts.filter((f) => f.urgency === 'critical' || f.urgency === 'low') + const supplierOptions = Array.from( + new Map( + forecasts + .filter((f) => f.supplierId && f.supplierName) + .map((f) => [f.supplierId!, f.supplierName!]), + ), + ).sort((a, b) => a[1].localeCompare(b[1])) function toggleSelect(id: string) { setSelected((prev) => { const n = new Set(prev); if (n.has(id)) n.delete(id); else n.add(id); return n }) } function selectAllNeedingReorder() { - setSelected(new Set(needsReorder.filter((f) => f.supplierId).map((f) => f.productId))) + setSelected(new Set( + needsReorder + .filter((f) => f.supplierId && (supplierFilter === 'all' || f.supplierId === supplierFilter)) + .map((f) => f.productId), + )) } function handleCreatePOs() { if (selected.size === 0) { setError('Select at least one product'); return } + if (supplierFilter === 'all') { setError('Choose one supplier before creating a draft PO'); return } setError('') startTransition(async () => { - const result = await createReorderPOs(Array.from(selected)) + const result = await createReorderPOs(Array.from(selected), { supplierId: supplierFilter }) if (result.success) { setSelected(new Set()) router.refresh() @@ -428,6 +442,17 @@ export function ForecastClient({ forecasts, settings }: Props) { ))} + +
{(['all', 'A', 'B', 'C'] as const).map((c) => ( )} diff --git a/app/(dashboard)/analytics/inventory-stats/inventory-stats-client.tsx b/app/(dashboard)/analytics/inventory-stats/inventory-stats-client.tsx index 453793f2..1fc07c8e 100644 --- a/app/(dashboard)/analytics/inventory-stats/inventory-stats-client.tsx +++ b/app/(dashboard)/analytics/inventory-stats/inventory-stats-client.tsx @@ -47,7 +47,7 @@ const ONHAND_FIELDS: FieldDef[] = [ { key: 'barcode', label: 'Barcode', type: 'text' }, { key: 'mpn', label: 'MPN', type: 'text' }, { key: 'warehouseCode', label: 'Warehouse', type: 'text' }, - { key: 'lifecycleStatus', label: 'Status', type: 'select', options: ['ACTIVE', 'NOT_FOR_SALE', 'ARCHIVED'] }, + { key: 'lifecycleStatus', label: 'Status', type: 'select', options: ['DRAFT', 'ACTIVE', 'EOL', 'ARCHIVED'] }, { key: 'quantity', label: 'Quantity', type: 'number' }, { key: 'reservedQty', label: 'Reserved', type: 'number' }, { key: 'available', label: 'Available', type: 'number' }, diff --git a/app/(dashboard)/analytics/product-profitability/product-profitability-client.tsx b/app/(dashboard)/analytics/product-profitability/product-profitability-client.tsx index 16ff4ef2..f83c9467 100644 --- a/app/(dashboard)/analytics/product-profitability/product-profitability-client.tsx +++ b/app/(dashboard)/analytics/product-profitability/product-profitability-client.tsx @@ -21,7 +21,8 @@ type SortDir = 'asc' | 'desc' const LIFECYCLE_OPTIONS = [ { value: 'ACTIVE', label: 'Active' }, - { value: 'NOT_FOR_SALE', label: 'Not for Sale' }, + { value: 'DRAFT', label: 'Draft' }, + { value: 'EOL', label: 'End of Life' }, { value: 'ARCHIVED', label: 'Archived' }, ] as const diff --git a/app/(dashboard)/analytics/sales-stats/sales-stats-client.tsx b/app/(dashboard)/analytics/sales-stats/sales-stats-client.tsx index 420bfad8..a548f074 100644 --- a/app/(dashboard)/analytics/sales-stats/sales-stats-client.tsx +++ b/app/(dashboard)/analytics/sales-stats/sales-stats-client.tsx @@ -51,7 +51,7 @@ const PRODUCT_FIELDS: FieldDef[] = [ { key: 'stockUnit', label: 'Stock Unit', type: 'text' }, { key: 'barcode', label: 'Barcode', type: 'text' }, { key: 'mpn', label: 'MPN', type: 'text' }, - { key: 'lifecycleStatus', label: 'Status', type: 'select', options: ['ACTIVE', 'NOT_FOR_SALE', 'ARCHIVED'] }, + { key: 'lifecycleStatus', label: 'Status', type: 'select', options: ['DRAFT', 'ACTIVE', 'EOL', 'ARCHIVED'] }, { key: 'qtySold', label: 'Qty Sold', type: 'number' }, { key: 'qtyRefunded', label: 'Qty Refunded', type: 'number' }, { key: 'netQty', label: 'Net Qty', type: 'number' }, diff --git a/app/(dashboard)/inventory/[id]/page.tsx b/app/(dashboard)/inventory/[id]/page.tsx index 3f8a791a..cc0c6336 100644 --- a/app/(dashboard)/inventory/[id]/page.tsx +++ b/app/(dashboard)/inventory/[id]/page.tsx @@ -2,7 +2,7 @@ import Link from 'next/link' import { notFound } from 'next/navigation' import { ChevronRight, Package, Layers, SlidersHorizontal } from 'lucide-react' import { db } from '@/lib/db' -import { getProduct, getVariableProducts, listProductCategories, updateProduct, getProductOptions, getProductSuppliers, getProductComponents, getKitStock } from '@/app/actions/products' +import { getProduct, getVariableProducts, listProductCategories, listProductSupplierOptions, updateProduct, getProductOptions, getProductSuppliers, getProductComponents, getKitStock } from '@/app/actions/products' import { getWarehouses, getActiveAdjustmentReasons } from '@/app/actions/stock' import { getStockUnitOptions } from '@/app/actions/settings' import { ProductForm } from '@/components/inventory/product-form' @@ -30,14 +30,16 @@ const TYPE_LABELS: Record = { } const STATUS_LABELS: Record = { + DRAFT: 'Draft', ACTIVE: 'Active', - NOT_FOR_SALE: 'Not for sale', + EOL: 'End of life', ARCHIVED: 'Archived', } const STATUS_VARIANTS: Record = { + DRAFT: 'secondary', ACTIVE: 'default', - NOT_FOR_SALE: 'secondary', + EOL: 'secondary', ARCHIVED: 'outline', } @@ -54,13 +56,14 @@ export default async function ProductDetailPage({ params: Promise<{ id: string }> }) { const { id } = await params - const [product, variableProducts, warehouses, reasons, stockUnitOptions, productCategories] = await Promise.all([ + const [product, variableProducts, warehouses, reasons, stockUnitOptions, productCategories, supplierOptions] = await Promise.all([ getProduct(id), getVariableProducts(), getWarehouses(), getActiveAdjustmentReasons(), getStockUnitOptions(), listProductCategories(), + listProductSupplierOptions(), ]) const baseCurrency = await getBaseCurrencyDisplay() const fmtBase = (value: number) => formatMoney(value, baseCurrency.symbol, baseCurrency.symbolPosition) @@ -81,7 +84,7 @@ export default async function ProductDetailPage({ // For the kit configurator: all stockable products (not self, not VARIABLE, not NON_INVENTORY) const allSimpleProducts = isKitOrBom ? await db.product.findMany({ - where: { lifecycleStatus: { in: ['ACTIVE', 'NOT_FOR_SALE'] }, type: { notIn: ['VARIABLE', 'NON_INVENTORY'] }, NOT: { id } }, + where: { lifecycleStatus: { in: ['DRAFT', 'ACTIVE', 'EOL'] }, type: { notIn: ['VARIABLE', 'NON_INVENTORY'] }, NOT: { id } }, select: { id: true, sku: true, name: true }, orderBy: { sku: 'asc' }, }) @@ -142,6 +145,8 @@ export default async function ProductDetailPage({ description: product.description ?? undefined, type: product.type, parentId: product.parentId ?? undefined, + preferredSupplierId: product.preferredSupplierId, + preferredSupplierLocked: product.preferredSupplierLocked, barcode: product.barcode ?? undefined, mpn: product.mpn ?? undefined, hsCode: product.hsCode ?? undefined, @@ -162,6 +167,7 @@ export default async function ProductDetailPage({ }} stockUnitOptions={stockUnitOptions} productCategories={productCategories} + supplierOptions={supplierOptions} inline /> @@ -469,9 +475,18 @@ export default async function ProductDetailPage({ )} {/* Suppliers */} - {suppliers.length > 0 && ( + {(suppliers.length > 0 || product.preferredSupplierName) && (

Suppliers

+ {product.preferredSupplierName && ( +
+
Preferred Supplier
+
+ {product.preferredSupplierName} + {product.preferredSupplierLocked && (locked)} +
+
+ )}
{suppliers.map((s) => (
diff --git a/app/(dashboard)/inventory/inventory-header.tsx b/app/(dashboard)/inventory/inventory-header.tsx index d092960c..daa1fbbe 100644 --- a/app/(dashboard)/inventory/inventory-header.tsx +++ b/app/(dashboard)/inventory/inventory-header.tsx @@ -13,15 +13,17 @@ import { importProductsCsv } from '@/app/actions/import' type VariableProduct = { id: string; sku: string; name: string } type ProductCategoryOption = { id: string; name: string; parentId: string | null } +type SupplierOption = { id: string; name: string } type Props = { total: number variableProducts: VariableProduct[] stockUnitOptions: string[] productCategories: ProductCategoryOption[] + supplierOptions: SupplierOption[] } -export function InventoryHeader({ total, variableProducts, stockUnitOptions, productCategories }: Props) { +export function InventoryHeader({ total, variableProducts, stockUnitOptions, productCategories, supplierOptions }: Props) { const router = useRouter() const [showCreate, setShowCreate] = useState(false) @@ -89,6 +91,7 @@ export function InventoryHeader({ total, variableProducts, stockUnitOptions, pro variableProducts={variableProducts} stockUnitOptions={stockUnitOptions} productCategories={productCategories} + supplierOptions={supplierOptions} onClose={() => setShowCreate(false)} title="New Product" /> diff --git a/app/(dashboard)/inventory/page.tsx b/app/(dashboard)/inventory/page.tsx index fc9c69a9..0f1b588e 100644 --- a/app/(dashboard)/inventory/page.tsx +++ b/app/(dashboard)/inventory/page.tsx @@ -1,5 +1,5 @@ import type { Metadata } from 'next' -import { listProductCategories, listProducts, getVariableProducts } from '@/app/actions/products' +import { listProductCategories, listProductSupplierOptions, listProducts, getVariableProducts } from '@/app/actions/products' import type { SortField, SortDir } from '@/app/actions/products' import { getStockUnitOptions } from '@/app/actions/settings' import { ProductFilters } from './product-filters' @@ -14,6 +14,7 @@ type SearchParams = { type?: string lifecycleStatus?: string categoryId?: string + supplierId?: string page?: string sort?: string dir?: string @@ -27,12 +28,13 @@ export default async function InventoryPage({ const sp = await searchParams const page = parseInt(sp.page ?? '1') - const [result, variableProducts, stockUnitOptions, categories] = await Promise.all([ + const [result, variableProducts, stockUnitOptions, categories, supplierOptions] = await Promise.all([ listProducts({ search: sp.search, type: sp.type as ProductType | 'ALL' | undefined, lifecycleStatus: sp.lifecycleStatus as ProductLifecycleStatus | 'ALL' | undefined, categoryId: sp.categoryId, + supplierId: sp.supplierId, page, sort: (sp.sort as SortField) || undefined, dir: (sp.dir as SortDir) || undefined, @@ -40,11 +42,12 @@ export default async function InventoryPage({ getVariableProducts(), getStockUnitOptions(), listProductCategories(), + listProductSupplierOptions(), ]) return (
- +
) diff --git a/app/(dashboard)/inventory/product-filters.tsx b/app/(dashboard)/inventory/product-filters.tsx index 4e45e3e0..1700efc0 100644 --- a/app/(dashboard)/inventory/product-filters.tsx +++ b/app/(dashboard)/inventory/product-filters.tsx @@ -20,10 +20,12 @@ type Props = { type?: string lifecycleStatus?: string categoryId?: string + supplierId?: string productCategories: { id: string; name: string; parentId: string | null }[] + supplierOptions: { id: string; name: string }[] } -export function ProductFilters({ search, type, lifecycleStatus, categoryId, productCategories }: Props) { +export function ProductFilters({ search, type, lifecycleStatus, categoryId, supplierId, productCategories, supplierOptions }: Props) { const router = useRouter() const pathname = usePathname() const [isPending, startTransition] = useTransition() @@ -55,13 +57,14 @@ export function ProductFilters({ search, type, lifecycleStatus, categoryId, prod if (key !== 'type' && type) params.set('type', type) if (key !== 'lifecycleStatus' && lifecycleStatus && lifecycleStatus !== 'ALL') params.set('lifecycleStatus', lifecycleStatus) if (key !== 'categoryId' && categoryId) params.set('categoryId', categoryId) + if (key !== 'supplierId' && supplierId) params.set('supplierId', supplierId) if (value) params.set(key, value) // reset page on filter change startTransition(() => { router.push(`${pathname}?${params.toString()}`) }) }, - [router, pathname, search, type, lifecycleStatus, categoryId] + [router, pathname, search, type, lifecycleStatus, categoryId, supplierId] ) useEffect(() => { @@ -119,8 +122,9 @@ export function ProductFilters({ search, type, lifecycleStatus, categoryId, prod onChange={(e) => update('lifecycleStatus', e.target.value === 'ALL' ? '' : e.target.value)} > + - +
@@ -141,6 +145,21 @@ export function ProductFilters({ search, type, lifecycleStatus, categoryId, prod
+
+ + +
+
!['VARIABLE', 'NON_INVENTORY', 'KIT'].includes(p.type), + (p) => !['VARIABLE', 'NON_INVENTORY', 'KIT'].includes(p.type) && (p.lifecycleStatus === 'ACTIVE' || p.lifecycleStatus === 'DRAFT'), ) return ( diff --git a/app/(dashboard)/purchase-orders/page.tsx b/app/(dashboard)/purchase-orders/page.tsx index d06d6bf3..0fad7a7b 100644 --- a/app/(dashboard)/purchase-orders/page.tsx +++ b/app/(dashboard)/purchase-orders/page.tsx @@ -25,7 +25,7 @@ export default async function PurchaseOrdersPage() { ]) const products = productsResult.products.filter( - (p) => !['VARIABLE', 'NON_INVENTORY', 'KIT'].includes(p.type), + (p) => !['VARIABLE', 'NON_INVENTORY', 'KIT'].includes(p.type) && (p.lifecycleStatus === 'ACTIVE' || p.lifecycleStatus === 'DRAFT'), ) return ( diff --git a/app/(dashboard)/purchase-orders/po-form.tsx b/app/(dashboard)/purchase-orders/po-form.tsx index e4e356cf..14f59144 100644 --- a/app/(dashboard)/purchase-orders/po-form.tsx +++ b/app/(dashboard)/purchase-orders/po-form.tsx @@ -175,6 +175,7 @@ export function PoFormDialog({ suppliers, products, warehouses, currencies, taxR const [fxRate, setFxRate] = useState(existingPo?.fxRateToBase ?? 1) const [destinationWarehouseId, setDestinationWarehouseId] = useState(existingPo?.destinationWarehouseId ?? '') const [supplierRef, setSupplierRef] = useState(existingPo?.supplierRef ?? '') + const [skipPreferredSupplierUpdate, setSkipPreferredSupplierUpdate] = useState(existingPo?.skipPreferredSupplierUpdate ?? false) const [expectedDelivery, setExpectedDelivery] = useState( existingPo?.expectedDelivery ? existingPo.expectedDelivery.slice(0, 10) : '', ) @@ -596,6 +597,7 @@ export function PoFormDialog({ suppliers, products, warehouses, currencies, taxR fxRateToBase: fxRate, destinationWarehouseId: destinationWarehouseId || undefined, supplierRef: supplierRef || undefined, + skipPreferredSupplierUpdate, expectedDelivery: expectedDelivery || undefined, notes: notes || undefined, internalNotes: internalNotes || undefined, @@ -775,6 +777,22 @@ export function PoFormDialog({ suppliers, products, warehouses, currencies, taxR
+
+ + +

+ One-off POs do not change products' preferred supplier when sent. +

+
+

- Active products can be sold. Not for sale products stay operational for purchasing, transfers, manufacturing, and kit components. Archived products are retired and removed from new operational flows. + Active products can be sold and reordered. Draft products can be purchased before publication. EOL products can sell down existing stock but are excluded from reorder forecasts. Archived products are retired.

@@ -228,6 +235,34 @@ export function ProductForm({ action, variableProducts, productCategories, defau {e.categoryName &&

{e.categoryName[0]}

} +
+
+ + + {e.preferredSupplierId &&

{e.preferredSupplierId[0]}

} +
+ +
+