fix(mcp): validate set_pricing actor and row ids; tighten pricing prompt

actor.id must be a 15-char user id (stock_ledger.created_by is a relation to
users; a non-user id left the cost patched but unaudited). Each updates[i].id
is validated, actor.name defaults to the actor id, and the Giellie prompt now
says to pass both selling_price and markup on create and to read the item
before a partial price change. Adds tests for null / numeric-string prices
and the explicit markup:0 default-markup behaviour.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

docs/api-mcp-layer.md

@@ -324,7 +324,7 @@ Anything else returns `reason: "forbidden_field"`. For inventory qty use
 `stock_items.adjust`. Returns `{ok: true, item, previous}` on success or
 `{ok: false, reason, message, field?}` on rejection.
 
-`stock_items.set_pricing` (required: `updates`; `actor.id` is required at runtime):
+`stock_items.set_pricing` (required: `updates`; `actor.id` is required at runtime and must be a 15-char user id — it is written to `stock_ledger.created_by`, a relation to `users`; `actor.name` defaults to the id):
 
 ```json
 {

src/app/api/mcp/route.test.ts

@@ -1995,10 +1995,70 @@ const toolNames = body.result.tools.map((t: { name: string }) => t.name);
       expect(JSON.parse(body.result.content[0].text)).toEqual(serviceResult);
     });
 
-    it("defaults actor.name to an empty string", async () => {
+    it.each([
+      ["missing", { id: actor.id }],
+      ["empty", { id: actor.id, name: "" }],
+      ["non-string", { id: actor.id, name: 5 }],
+    ])("defaults a %s actor.name to the actor id (no bare-space ledger note)", async (_l, a) => {
       updateStockItemsPricing.mockResolvedValueOnce({ results: [], summary: { total: 1, succeeded: 0, failed: 0 } });
-      await call({ updates: [{ id: ID1, cost_price: 1 }], actor: { id: actor.id } });
-      expect(updateStockItemsPricing.mock.calls[0][0].actor).toEqual({ id: actor.id, name: "" });
+      await call({ updates: [{ id: ID1, cost_price: 1 }], actor: a });
+      expect(updateStockItemsPricing.mock.calls[0][0].actor).toEqual({ id: actor.id, name: actor.id });
+    });
+
+    it.each([
+      ["hermes", "hermes"],
+      ["14-char", "abcde12345fghi"],
+      ["16-char", "abcde12345fghijk"],
+      ["uppercase", "ABCDE12345FGHIJ"],
+      ["path-shaped", "../../abcde12345"],
+    ])("rejects a non-user-id actor.id (%s) and never calls the service", async (_l, id) => {
+      const res = await call({ updates: [{ id: ID1, cost_price: 1 }], actor: { id, name: "X" } });
+      const body = await res.json();
+      expect(body.error.message).toBe(
+        "actor.id must be a valid 15-character user id (it is recorded in the cost audit trail).",
+      );
+      expect(updateStockItemsPricing).not.toHaveBeenCalled();
+    });
+
+    it("passes a valid 15-char actor.id through", async () => {
+      updateStockItemsPricing.mockResolvedValueOnce({ results: [], summary: { total: 1, succeeded: 0, failed: 0 } });
+      const res = await call({ updates: [{ id: ID1, cost_price: 1 }], actor: { id: "a1b2c3d4e5f6g7h", name: "Dave" } });
+      expect(res.status).toBe(200);
+      expect(updateStockItemsPricing.mock.calls[0][0].actor.id).toBe("a1b2c3d4e5f6g7h");
+    });
+
+    it.each([
+      ["missing", {}],
+      ["short", { id: "abc" }],
+      ["uppercase", { id: "ABCDE12345FGHIJ" }],
+      ["path-shaped", { id: "../x" }],
+      ["non-string", { id: 123456789012345 }],
+    ])("rejects an update whose id is %s, naming updates[i].id", async (_l, row) => {
+      const res = await call({ updates: [{ id: ID1, cost_price: 1 }, { ...row, cost_price: 1 }], actor });
+      const body = await res.json();
+      expect(body.error.message).toMatch(/updates\[1\]\.id must be a valid 15-character stock_item id/);
+      expect(updateStockItemsPricing).not.toHaveBeenCalled();
+    });
+
+    it.each([
+      ["null", null],
+      ["a numeric string", "100"],
+    ])("reports %s price values as per-row validation_error from the real service", async (_l, value) => {
+      const actual = await vi.importActual<typeof import("@/lib/server/stock-pricing-service")>(
+        "@/lib/server/stock-pricing-service",
+      );
+      updateStockItemsPricing.mockImplementationOnce((input: Parameters<typeof actual.updateStockItemsPricing>[0]) =>
+        actual.updateStockItemsPricing(input),
+      );
+      const res = await call({ updates: [{ id: ID1, cost_price: value }], actor });
+      expect(res.status).toBe(200);
+      const body = await res.json();
+      expect(body.error).toBeUndefined();
+      const data = JSON.parse(body.result.content[0].text);
+      expect(data.summary).toEqual({ total: 1, succeeded: 0, failed: 1 });
+      expect(data.results[0]).toMatchObject({ id: ID1, ok: false, reason: "validation_error" });
+      expect(data.results[0].message).toContain("costPrice");
+      expect(update).not.toHaveBeenCalled();
     });
 
     it.each([
@@ -2140,6 +2200,13 @@ const toolNames = body.result.tools.map((t: { name: string }) => t.name);
       expect(data.selling_price).toBeUndefined();
     });
 
+    it("documents current behaviour: an explicit markup of 0 with a positive cost is treated as absent, so the 80% default is applied", async () => {
+      await callCreate({ name: "Widget", cost_price: 100, markup: 0 });
+      const data = create.mock.calls[0][0];
+      expect(data.markup).toBe(80);
+      expect(data.selling_price).toBe(180);
+    });
+
     it("respects a caller-supplied selling price (no default override)", async () => {
       await callCreate({ name: "Widget", cost_price: 100, selling_price: 125 });
       const data = create.mock.calls[0][0];

src/app/api/mcp/route.ts

@@ -2123,7 +2123,7 @@ const tools = [
         },
         actor: {
           type: "object",
-          description: "Who is making the change (recorded in the cost audit trail). `id` is required.",
+          description: "Who is making the change (recorded in the cost audit trail). `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" },
@@ -6301,6 +6301,7 @@ async function handleMcpPost(req: NextRequest) {
 
       if (toolName === "stock_items.set_pricing") {
         const MAX_PRICING_UPDATES = 100;
+        const PB_ID_RE = /^[a-z0-9]{15}$/;
         const ALLOWED_PRICING_KEYS = ["id", "cost_price", "selling_price", "markup"];
         if (!Array.isArray(args.updates) || args.updates.length === 0) {
           throw new Error("updates must be a non-empty array for stock_items.set_pricing.");
@@ -6316,6 +6317,13 @@ async function handleMcpPost(req: NextRequest) {
             "actor.id is required for stock_items.set_pricing (it is recorded in the cost audit trail).",
           );
         }
+        // stock_ledger.created_by is a relation to `users`: a non-user id would
+        // make the ledger write fail AFTER the cost was already patched.
+        if (!PB_ID_RE.test(actorRaw.id)) {
+          throw new Error(
+            "actor.id must be a valid 15-character user id (it is recorded in the cost audit trail).",
+          );
+        }
         const pricingUpdates = (args.updates as unknown[]).map((raw, i) => {
           if (!raw || typeof raw !== "object" || Array.isArray(raw)) {
             throw new Error(`updates[${i}] must be an object.`);
@@ -6328,8 +6336,11 @@ async function handleMcpPost(req: NextRequest) {
               );
             }
           }
+          if (typeof row.id !== "string" || !PB_ID_RE.test(row.id)) {
+            throw new Error(`updates[${i}].id must be a valid 15-character stock_item id.`);
+          }
           const out: { id: string; costPrice?: number; sellingPrice?: number; markup?: number } = {
-            id: row.id as string,
+            id: row.id,
           };
           if (row.cost_price !== undefined) out.costPrice = row.cost_price as number;
           if (row.selling_price !== undefined) out.sellingPrice = row.selling_price as number;
@@ -6344,7 +6355,10 @@ async function handleMcpPost(req: NextRequest) {
           updates: pricingUpdates,
           actor: {
             id: actorRaw.id,
-            name: typeof actorRaw.name === "string" ? actorRaw.name : "",
+            name:
+              typeof actorRaw.name === "string" && actorRaw.name.trim()
+                ? actorRaw.name
+                : actorRaw.id,
           },
         });
         return jsonRpcOk(body.id, result);

src/lib/server/giellie-system-prompt.test.ts

@@ -67,6 +67,18 @@ describe("buildSystemPrompt", () => {
       expect(p).toMatch(/stock_items\.update cannot change cost/i);
     });
 
+    it("tells the model to pass both selling_price and markup on create, omitting rather than passing 0", () => {
+      expect(p).toMatch(/always pass BOTH selling_price and markup/);
+      expect(p).toMatch(/silently applies an 80% markup/);
+      expect(p).toMatch(/omit a field rather than passing 0/);
+    });
+
+    it("tells the model to read the item first when changing only markup or only cost", () => {
+      expect(p).toMatch(/only markup or only cost/);
+      expect(p).toContain("stock_items.get");
+      expect(p).toMatch(/before\/after/);
+    });
+
     it("distinguishes supplier price lists from POS stock prices", () => {
       expect(p).toContain("price_list.add_items");
       expect(p).toMatch(/ask which one/i);

src/lib/server/giellie-system-prompt.ts

@@ -22,6 +22,8 @@ How to work:
 Stock items and pricing (POS):
 - Before adding a stock item (or items from a list, photo or spreadsheet), search for an existing match first (stock_items.list with search by SKU and by name) so you do not create duplicates. Collect: name, SKU (if any), unit, qty on hand / min / target, supplier, and pricing: cost price, plus markup % or a selling price. If the cost or the markup / selling price is missing, ASK for it. You may suggest the app's default markup of 80% as an option, but never apply a markup the user did not give or confirm.
 - The POS till reads selling_price, not markup, so always set both consistently. selling_price = cost × (1 + markup/100), rounded to 2 decimals. If the user gives cost and selling price instead, markup = (selling ÷ cost − 1) × 100, to 1 decimal. Show the working for each price.
+- When calling stock_items.create always pass BOTH selling_price and markup explicitly (the backend silently applies an 80% markup if they are omitted), and omit a field rather than passing 0.
+- When you change only markup or only cost on an existing item, read the item first (stock_items.get) so you can recompute the other figures and show the before/after.
 - Set prices on new items with stock_items.create (cost_price / selling_price / markup) and on existing items with stock_items.set_pricing (one batched call for a whole list, max 100 per call). Never use stock_items.adjust or purchase orders to "record" a price, and stock_items.update cannot change cost.
 - Supplier price lists (price_list.add_items, the price_list catalogue) are different from POS stock prices (stock_items). If the request is ambiguous, ask which one they mean.