diff --git a/src/app/api/mcp/route.test.ts b/src/app/api/mcp/route.test.ts index 076b9c7..e83384b 100644 --- a/src/app/api/mcp/route.test.ts +++ b/src/app/api/mcp/route.test.ts @@ -2207,17 +2207,25 @@ const toolNames = body.result.tools.map((t: { name: string }) => t.name); // ─── stock_items.create pricing fields ─────────────────────────────── describe("stock_items.create pricing", () => { + const CREATE_ACTOR = { id: "user123456789ab", name: "Dave" }; const callCreate = (args: Record) => POST( mcpRequest({ method: "tools/call", id: 210, - params: { name: "stock_items.create", arguments: args }, + params: { + name: "stock_items.create", + // The opening-balance ledger row is stamped with the + // acting user, so every create carries one. + arguments: { actor: CREATE_ACTOR, ...args }, + }, }), ); beforeEach(() => { create.mockImplementation(async (data: Record) => ({ id: "newitem1234567x", ...data })); + getOne.mockResolvedValue({ id: "newitem1234567x", current_qty: 0 }); + update.mockResolvedValue({ id: "newitem1234567x", current_qty: 5 }); }); it("advertises optional cost_price / selling_price / markup (number, min 0)", async () => { @@ -2287,6 +2295,74 @@ const toolNames = body.result.tools.map((t: { name: string }) => t.name); expect(ledger?.[0]).toMatchObject({ change_qty: 5, reason: "adjustment" }); }); + it("stamps the opening-balance ledger row with the acting user", async () => { + await callCreate({ name: "Widget", current_qty: 5, cost_price: 10 }); + const ledger = create.mock.calls.find((c) => c[0]?.stock_item === "newitem1234567x"); + expect(ledger?.[0]).toMatchObject({ + change_qty: 5, + reason: "adjustment", + created_by: CREATE_ACTOR.id, + notes: "Opening balance at stock item creation (via MCP).", + }); + }); + + it("writes the opening-balance ledger row before the quantity", async () => { + await callCreate({ name: "Widget", current_qty: 5, cost_price: 10 }); + const ledgerIndex = create.mock.calls.findIndex( + (c) => c[0]?.stock_item === "newitem1234567x", + ); + expect(ledgerIndex).toBeGreaterThanOrEqual(0); + // The item is created with no quantity; the opening balance is + // booked through the audited write path. + expect(create.mock.calls[0][0]).toMatchObject({ current_qty: 0 }); + expect(update).toHaveBeenCalledWith("newitem1234567x", { current_qty: 5 }); + expect(create.mock.invocationCallOrder[ledgerIndex]).toBeLessThan( + update.mock.invocationCallOrder[0], + ); + }); + + it("reverses the opening-balance ledger row when the quantity write fails", async () => { + update.mockRejectedValue(new Error("PB write failed")); + const res = await callCreate({ name: "Widget", current_qty: 5 }); + const body = await res.json(); + expect(body.error.message).toMatch(/opening balance/i); + const ledgerRows = create.mock.calls + .map((c) => c[0]) + .filter((row) => row?.stock_item === "newitem1234567x"); + expect(ledgerRows).toHaveLength(2); + expect(ledgerRows[0]).toMatchObject({ change_qty: 5, created_by: CREATE_ACTOR.id }); + expect(ledgerRows[1]).toMatchObject({ change_qty: -5, created_by: CREATE_ACTOR.id }); + }); + + it("refuses a create without an acting user", async () => { + const res = await POST( + mcpRequest({ + method: "tools/call", + id: 210, + params: { name: "stock_items.create", arguments: { name: "Widget" } }, + }), + ); + const body = await res.json(); + expect(body.error.message).toMatch(/actor\.id is required/i); + expect(create).not.toHaveBeenCalled(); + }); + + it("refuses an actor id that is not a user id", async () => { + const res = await POST( + mcpRequest({ + method: "tools/call", + id: 210, + params: { + name: "stock_items.create", + arguments: { name: "Widget", actor: { id: "not-a-user-id" } }, + }, + }), + ); + const body = await res.json(); + expect(body.error.message).toMatch(/15-character user id/i); + expect(create).not.toHaveBeenCalled(); + }); + it.each([ ["cost_price", -1], ["cost_price", Number.NaN], diff --git a/src/app/api/mcp/route.ts b/src/app/api/mcp/route.ts index ab6a314..66abee2 100644 --- a/src/app/api/mcp/route.ts +++ b/src/app/api/mcp/route.ts @@ -2126,7 +2126,7 @@ const tools = [ { name: "stock_items.create", description: - "Create a new stock_item (a POS/till item). This is the DEFAULT place to put a product and its prices — use it unless the user explicitly asked for the price list / catalogue. The till reads `selling_price` from this collection and nothing else; a price that exists only in `price_list` is invisible at the counter. Pass `supplier` as the supplier's PocketBase record id (resolve it with `suppliers.list`); it is a relation, not free text. `name` is required. When `current_qty > 0` an opening-balance ledger row is written so the running total and the ledger sum stay in sync (reason=`adjustment`, reference_type=`manual`).", + "Create a new stock_item (a POS/till item). This is the DEFAULT place to put a product and its prices — use it unless the user explicitly asked for the price list / catalogue. The till reads `selling_price` from this collection and nothing else; a price that exists only in `price_list` is invisible at the counter. Pass `supplier` as the supplier's PocketBase record id (resolve it with `suppliers.list`); it is a relation, not free text. `name` is required, and `actor.id` is required so the opening balance is attributed. When `current_qty > 0` an opening-balance ledger row is written so the running total and the ledger sum stay in sync (reason=`adjustment`, reference_type=`manual`); that row is stamped with the acting user, is written BEFORE the quantity, and is reversed if the quantity write fails.", inputSchema: { type: "object", required: ["name"], @@ -2158,6 +2158,15 @@ const tools = [ description: "Optional markup as a percentage on cost (100 = 100%, i.e. double the cost). Must be >= 0. The POS till prices from selling_price, not markup — set both. If only cost_price is given the app default markup (80%) and selling price are applied.", }, + actor: { + type: "object", + description: + "Who is making the change (recorded on the opening-balance stock_ledger row). `id` is required and must be the 15-char id of a WFDASH user (see users.lookup); `name` defaults to the id.", + properties: { + id: { type: "string" }, + name: { type: "string" }, + }, + }, }, }, }, @@ -5782,6 +5791,10 @@ async function handleMcpPost(req: NextRequest) { if (!args.name || typeof args.name !== "string" || !args.name.trim()) { throw new Error("name is required for stock_items.create."); } + // The opening-balance ledger row is stamped with the acting + // user, so a call without one is refused rather than written + // unattributed. + const actor = requireStockActor(args, "stock_items.create"); const openingQty = Number(args.current_qty ?? 0); const minQty = Number(args.min_qty ?? 0); const pricing: Record = {}; @@ -5793,13 +5806,20 @@ async function handleMcpPost(req: NextRequest) { } pricing[field] = v; } + // A POSITIVE opening balance is booked through the audited + // write path below: the ledger row is written first and the + // quantity second, and the row is reversed if the quantity + // write fails. The item is created with 0 for now. A zero or + // negative opening balance is written on the create itself, + // exactly as before. + const bookedThroughLedger = openingQty > 0; const createPayload: Record = { sku: typeof args.sku === "string" ? args.sku.trim() : "", name: args.name.trim(), description: typeof args.description === "string" ? args.description : "", unit: typeof args.unit === "string" ? args.unit : "", location: typeof args.location === "string" ? args.location : "", - current_qty: openingQty, + current_qty: bookedThroughLedger ? 0 : openingQty, min_qty: minQty, target_qty: Number(args.target_qty ?? 0), supplier: typeof args.supplier === "string" ? args.supplier : "", @@ -5812,20 +5832,31 @@ async function handleMcpPost(req: NextRequest) { // Never overwrites values the caller supplied. applyDefaultMarkupToPatch(createPayload); const item = await pb.collection("stock_items").create(createPayload); - if (openingQty > 0) { - try { - await pb.collection("stock_ledger").create({ - stock_item: item.id, - change_qty: openingQty, - reason: "adjustment", - reference_type: "manual", - reference_id: "", - notes: "Opening balance at stock item creation (via MCP).", - created_by: "", - }); - } catch (ledgerErr) { - console.error("[mcp:stock_items.create] opening-balance ledger failed", ledgerErr); + if (bookedThroughLedger) { + // Audit BEFORE cache: the opening-balance ledger row is + // written first, the quantity second. The write path + // reverses the ledger row if the quantity write fails, so + // SUM(change_qty) still rebuilds current_qty and a retry is + // safe. + const result = await updateStockItemsQuantities({ + pb: pb as unknown as StockItemsPb, + updates: [{ id: String(item.id), currentQty: openingQty }], + actor, + reason: "adjustment", + referenceType: "manual", + referenceId: "", + notes: "Opening balance at stock item creation (via MCP).", + noteContext: "at stock item creation (via MCP)", + }); + const row = result.results[0]; + if (!row || !row.ok) { + const message = + row && !row.ok ? row.message : "the opening balance was not applied"; + throw new Error( + `stock_items was created with id ${item.id}, but its opening balance of ${openingQty} was not applied: ${message} The item is on the catalogue with quantity 0.`, + ); } + return jsonRpcOk(body.id, row.item); } return jsonRpcOk(body.id, item); } diff --git a/src/lib/server/stock-quantities-service.ts b/src/lib/server/stock-quantities-service.ts index c35aad5..e83245e 100644 --- a/src/lib/server/stock-quantities-service.ts +++ b/src/lib/server/stock-quantities-service.ts @@ -11,13 +11,14 @@ import type { StockItem } from "@/lib/types"; * trail stays honest. `min_qty` and `target_qty` are simple * patch-as-you-go fields — no ledger row. * - * Three callers share it: the /stock page (which keeps a + * Four callers share it: the /stock page (which keeps a * non-negative floor — the operator types a physical count there), the * Giellie agent (which may write an absolute count below zero, because - * the till oversells on purpose), and the MCP `stock_items.adjust` - * tool (a signed delta turned into an absolute target, carrying the - * caller's own `reason`). The agent and the adjust tool opt in with - * `allowNegativeQty`; the page never does. + * the till oversells on purpose), the MCP `stock_items.adjust` tool (a + * signed delta turned into an absolute target, carrying the caller's + * own `reason`), and the MCP `stock_items.create` tool (the opening + * balance on a newly created item). The agent and the adjust tool opt + * in with `allowNegativeQty`; the page and the create path never do. * * Why a separate service instead of folding into * `updateStockItemsPricing`?