fix(giellie): attribute the stock_items.create opening balance

Creating a stock item with `current_qty > 0` wrote the opening-balance
`stock_ledger` row with `created_by: ""`, so the row landed in the audit
trail unattributed — and it was written after the quantity, so a failure
in between left the two out of step.

- The create now requires `actor.id` (the same 15-char check and error
  style as `stock_items.set_quantities`) and stamps the opening-balance
  row with it.
- The opening balance is booked through the audited write path: the item
  is created with no quantity, the ledger row is written first, and the
  quantity second. If the quantity write fails, the service reverses
  the ledger row, so `SUM(change_qty)` still rebuilds `current_qty` and a
  retry is safe.
- A zero or negative opening balance is written on the create itself,
  exactly as before.
This commit is contained in:
Dave 2026-11-06 08:02:36 +02:00
parent ab16404003
commit 7031499bf0
3 changed files with 129 additions and 21 deletions

View file

@ -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<string, unknown>) =>
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<string, unknown>) => ({ 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],

View file

@ -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<string, number> = {};
@ -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<string, unknown> = {
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);
}

View file

@ -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`?