feat(giellie): tighten the agent tool policy and MCP surface

Aligns the tool policy, the system prompt, the MCP route and the skill
doc so they describe the same boundary, with tests pinning the policy
and prompt text.

The docs and the executable policy had drifted apart — the skill doc
advertised capabilities the policy does not grant, which is worse than
not documenting them: an agent reading the doc will plan around a tool
it isn't allowed to call.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Dave 2026-10-16 00:44:52 +02:00
parent 09e4d8f1e1
commit 8d83065964
8 changed files with 154 additions and 14 deletions

View file

@ -290,11 +290,20 @@ REST mirror at `/api/v1/quote_items` (GET list + POST create) and `/api/v1/quote
### 8. Price-list catalogue (add items from a parsed supplier/quote document)
> **⚠️ OPT-IN ONLY (changed 2026-10-15).** `price_list.add_items` writes to the
> `price_list` catalogue, which **the POS never reads** — `listActiveStockCatalog()`
> queries `stock_items` only. A price that exists only here is invisible at the
> till. It is also now classified `confirm` in `giellie-tool-policy.ts`, so it asks
> every time regardless of the chat's trust setting. **Only call it when the
> operator explicitly asked for the price list / catalogue / supplier price list.**
> For "load prices", "add these items", "here's a supplier list", or anything
> ambiguous, use `stock_items.create` — that is the default.
| Tool | Purpose |
|------|---------|
| `price_list.add_items` | Bulk-upsert parsed line items into the `price_list` collection. The agent reads the source document (supplier PDF, quote PDF, pasted email) with its native vision, extracts the per-line fields, and passes them in. The tool handles dedupe by `(sku, supplier)` and computes `sku_normalised` + `search_text` automatically. **You do the OCR, we do the commit.** |
| `price_list.add_items` | Bulk-upsert parsed line items into the `price_list` collection. The agent reads the source document (supplier PDF, quote PDF, pasted email) with its native vision, extracts the per-line fields, and passes them in. The tool handles dedupe by `(sku, supplier)` and computes `sku_normalised` + `search_text` automatically. **You do the OCR, we do the commit.** Pass the supplier **name**, never a PocketBase record id — `price_list.supplier` is free text. |
**When to use it:** the operator pastes a supplier's price list PDF, a quote from a customer, or a supplier's emailed price sheet into the chat and asks you to add the items to the catalogue. Read the document with your vision capability, extract the per-line fields, and call `price_list.add_items` with the structured array.
**When to use it:** the operator explicitly asks you to add items to the **catalogue / price list** — not merely to load prices onto the counter. Read the document with your vision capability, extract the per-line fields, and call `price_list.add_items` with the structured array. If they asked to load prices for something they sell, that is `stock_items.create` (§ POS/stock items), not this.
**Input shape:**

View file

@ -531,7 +531,7 @@ Grouped by resource family. The full description is in the table column; the des
| `order_items.update_status` | Update an order_item's `item_status` and/or `drv_status` fields. Use this when the operator needs to flip a single row through the workflow without bulk-pricing or per-line invoice reconciliation (`order_items.update` is the patch tool f... |
| `order_items.confirm_collected` | Confirm collection of one or more customer order_items WITHOUT requiring a driver assignment. Mirrors the agent-side role that the stock-side `stock_purchase_items.mark_collected` already plays: marks `item_status` → `collected` and `drv... |
| `order_items.create` | Add a new line item to an existing order. Mirrors the REST POST /api/v1/orders/{id}/items endpoint. `selling_price` auto-derives from `cost_price * (1 + markup / 100)` when omitted. Per-row failures never abort the whole call (returns `c... |
| `order_items.update` | Patch fields on an order_item for invoice reconciliation. Whitelist: cost_price, selling_price, quantity, sku, unit, category, markup, markup_percent, notes, qty_processed, qty_assigned, qty_collected, qty_verified, qty_stocked, qty_pick... |
| `order_items.update` | Patch fields on an order_item for invoice reconciliation and catalogue relabelling. Whitelist: cost_price, selling_price, quantity, sku, unit, category, description, supplier, markup, markup_percent, pricing_source, pricing_notes, matche... |
| `order_items.list` | List order_items (the line items under each order). Useful for finding pending/PB-ready items, getting one supplier's items, etc. Pass PB filter syntax in `filter`. |
| `order_items.get` | Fetch a single order_item by id. Pass `expand` to inline `order`, `purchase_order`, `category`, `supplier`. |
@ -655,7 +655,7 @@ Grouped by resource family. The full description is in the table column; the des
| --- | --- |
| `stock_items.list` | List stock_items. Supports substring search on name/sku/description, low-stock filter (`current_qty <= min_qty AND is_active = true`), pagination, and `expand` (defaults to `supplier`). |
| `stock_items.get` | Fetch a single stock_item by id. The response includes the item (expand=supplier) and a paginated recent-ledger slice (`ledger.page`, `ledger.items`, etc.). Pass `ledger=false` to skip the ledger fetch. |
| `stock_items.create` | Create a new stock_item. `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`). |
| `stock_items.create` | 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 nothin... |
| `stock_items.adjust` | Record a manual stock adjustment. Inserts a `stock_ledger` row and updates `current_qty` on the parent item. `change_qty` is signed (positive = in, negative = out); zero is rejected. `reason` must be one of STOCK_REASON_VALUES. |
| `stock_items.update` | Patch catalogue fields on a master stock_item: sku, description, notes, unit, selling_price, markup. For inventory qty use stock_items.adjust. Returns {ok: true, item, previous} on success or {ok: false, reason, message, field?} on rejec... |
| `stock_items.set_pricing` | Set cost price / selling price / markup on existing stock_items in one batch (max 100 per call). The POS till reads `selling_price` (NOT markup), so set both consistently: selling = cost × (1 + markup/100). Cost changes are audited in st... |
@ -695,7 +695,7 @@ Grouped by resource family. The full description is in the table column; the des
| Tool | Description |
| --- | --- |
| `price_list.add_items` | Bulk-upsert parsed supplier-price-list line items into the `price_list` catalogue. The agent reads the source document (a supplier PDF, a quote PDF, or a pasted supplier email) with its native vision capability, extracts the per-line fie... |
| `price_list.add_items` | ⚠️ DO NOT USE BY DEFAULT. This writes to the `price_list` catalogue, which the POS does NOT read — nothing here ever reaches the till. Only call this when the user has explicitly asked for the price list / catalogue / supplier price list... |
### Users

View file

@ -4074,6 +4074,92 @@ const toolNames = body.result.tools.map((t: { name: string }) => t.name);
});
});
// ---------------------------------------------------------------------------
// order_items.update — description/whitelist drift guard
//
// The dispatcher filters `patch` against ORDER_ITEM_PATCHABLE_FIELDS
// (src/lib/server/order-items-service.ts), but the tool's *prose* whitelist
// and its inputSchema are hand-written, so they can silently drift from the
// constant that actually enforces the rule. That drift already bit once:
// `description` was enforced but undocumented, which reads to an agent as
// "I cannot set the description on a line" — the exact belief that blocked
// relabelling MAS-CONSIGNMENT-FAST against the supplier price list.
//
// NOTE: the prose whitelist must stay a plain string literal. It is read by
// scripts/mcp-doc-gen.py with a naive source-text parse, so a `${...}`
// interpolation lands verbatim in docs/api-mcp-layer.md. These tests are the
// drift protection instead of interpolation.
// ---------------------------------------------------------------------------
describe("order_items.update whitelist drift guard", () => {
const EXPECTED_WHITELIST = [
"cost_price",
"selling_price",
"quantity",
"sku",
"unit",
"category",
"description",
"supplier",
"markup",
"markup_percent",
"pricing_source",
"pricing_notes",
"matched_price_list_id",
"user_edited",
"notes",
"qty_processed",
"qty_assigned",
"qty_collected",
"qty_verified",
"qty_stocked",
"qty_picked",
"qty_loaded",
"qty_delivered",
"qty_invoiced",
"qty_cancelled",
"supplier_invoice_number",
"customer_invoice_number",
"collection_date",
"collected_date",
"verified_date",
"delivered_date",
"invoiced_date",
];
async function orderItemsUpdateTool() {
const res = await POST(mcpRequest({ method: "tools/list", id: 1 }));
const body = await res.json();
return body.result.tools.find(
(t: { name: string }) => t.name === "order_items.update",
);
}
it("prose whitelist lists every enforced field and nothing stale", async () => {
const tool = await orderItemsUpdateTool();
expect(tool).toBeTruthy();
const match = tool.description.match(/Whitelist: ([^.]+)\./);
expect(match).toBeTruthy();
const documented = match![1]
.split(",")
.map((s: string) => s.trim())
.filter(Boolean);
expect(documented.sort()).toEqual([...EXPECTED_WHITELIST].sort());
});
it("inputSchema declares every enforced field as a patch property", async () => {
const tool = await orderItemsUpdateTool();
const declared = Object.keys(tool.inputSchema.properties.patch.properties).sort();
expect(declared).toEqual([...EXPECTED_WHITELIST].sort());
});
it("keeps `description` and `sku` documented for price-list relabelling", async () => {
const tool = await orderItemsUpdateTool();
expect(tool.inputSchema.properties.patch.properties.description).toBeTruthy();
expect(tool.inputSchema.properties.patch.properties.sku).toBeTruthy();
expect(tool.description).toMatch(/CODE - DESCRIPTION/);
});
});
// ---------------------------------------------------------------------------
// Structured upstream errors (McpUpstreamError → JSON-RPC -32010/-32011)
// ---------------------------------------------------------------------------

View file

@ -1045,7 +1045,7 @@ const tools = [
{
name: "order_items.update",
description:
"Patch fields on an order_item for invoice reconciliation. Whitelist: cost_price, selling_price, quantity, sku, unit, category, markup, markup_percent, notes, qty_processed, qty_assigned, qty_collected, qty_verified, qty_stocked, qty_picked, qty_loaded, qty_delivered, qty_invoiced, qty_cancelled, supplier_invoice_number, customer_invoice_number, collection_date, collected_date, verified_date, delivered_date, invoiced_date. Use this to compare what the supplier invoiced (supplier_invoice_number + qty_invoiced + cost_price) against what was ordered (quantity) and what was collected (qty_collected), and patch the differences. `category` is a PB relation id (resolve it from `categories.list` first, not free text). `unit` is useful when the supplier ships in a different pack/measure than the PO. Status-only updates go through `order_items.update_status` instead.",
"Patch fields on an order_item for invoice reconciliation and catalogue relabelling. Whitelist: cost_price, selling_price, quantity, sku, unit, category, description, supplier, markup, markup_percent, pricing_source, pricing_notes, matched_price_list_id, user_edited, notes, qty_processed, qty_assigned, qty_collected, qty_verified, qty_stocked, qty_picked, qty_loaded, qty_delivered, qty_invoiced, qty_cancelled, supplier_invoice_number, customer_invoice_number, collection_date, collected_date, verified_date, delivered_date, invoiced_date. Use this to compare what the supplier invoiced (supplier_invoice_number + qty_invoiced + cost_price) against what was ordered (quantity) and what was collected (qty_collected), and patch the differences. `description` + `sku` are how you relabel a line against a supplier price list (e.g. rewriting a free-text description to \"CODE - DESCRIPTION\" and setting sku to the bare CODE) — both are whitelisted. `category` and `supplier` are PB relation ids (resolve them from `categories.list` / `suppliers.list` first, not free text). `unit` is useful when the supplier ships in a different pack/measure than the PO. Status-only updates go through `order_items.update_status` instead.",
inputSchema: {
type: "object",
required: ["id", "patch"],
@ -1059,11 +1059,17 @@ const tools = [
cost_price: { type: "number" },
selling_price: { type: "number" },
quantity: { type: "number" },
sku: { type: "string", description: "Free-text supplier / catalogue SKU. Tried-and-true for one-letter typos; use `order_items.update_status` to flip status instead." },
description: { type: "string", description: "Line description. Free text — callers relabelling against a price list conventionally set this to `CODE - DESCRIPTION`." },
sku: { type: "string", description: "Free-text supplier / catalogue SKU. Tried-and-true for one-letter typos; also the machine-readable code when relabelling against a price list. Use `order_items.update_status` to flip status instead." },
unit: { type: "string", description: "Pack/measure unit (EA, BOX, M, L, KG). Useful when supplier invoice unit differs from the original PO." },
category: { type: "string", description: "PB relation id to the `categories` collection. Resolve from `categories.list` first." },
supplier: { type: "string", description: "PB relation id to the `suppliers` collection. Resolve from `suppliers.list` first." },
markup: { type: "number" },
markup_percent: { type: "number" },
pricing_source: { type: "string", description: "How this line was priced (e.g. manual | ai | catalogue)." },
pricing_notes: { type: "string", description: "Free-text pricing audit note. Stamp agent identity + date when changing pricing fields." },
matched_price_list_id: { type: "string", description: "15-char PB id of the `price_list` row this line was sourced from." },
user_edited: { type: "boolean", description: "Marks the line as hand-edited so bulk re-pricing skips it." },
notes: { type: "string" },
qty_processed: { type: "number" },
qty_assigned: { type: "number" },
@ -2020,7 +2026,7 @@ const tools = [
{
name: "stock_items.create",
description:
"Create a new stock_item. `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. 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`).",
inputSchema: {
type: "object",
required: ["name"],
@ -3045,7 +3051,7 @@ const tools = [
{
name: "price_list.add_items",
description:
"Bulk-upsert parsed supplier-price-list line items into the `price_list` catalogue. The agent reads the source document (a supplier PDF, a quote PDF, or a pasted supplier email) with its native vision capability, extracts the per-line fields, and passes them in. The tool handles deduplication by `(sku, supplier)`: if a row already exists for that SKU from that supplier, the existing row's `cost_price`, `description`, `selling_price`, `unit`, `category`, `subcategory`, `brand`, `markup`, `stock_qty`, `search_text`, and `sku_normalised` are patched in place; otherwise a new row is inserted. `cost_price` OVERWRITES — the latest price wins. The supplier is stored as free text on `price_list.supplier` (the live schema has no FK), so the supplier name is taken verbatim. Items with missing `sku`, missing `supplier_name`, non-positive `cost_price`, or a PB write failure are returned in `skipped[]` with a per-item reason so the agent can report them back to the operator. Max 500 items per call — chunk larger lists. Optionally pass `source_file_id` (a `raw_price_list` row id, when the operator uploaded the document via `/pricelist` first) and `source_label` (free text, e.g. `\"Q-AFRISAN-2026-08\"`) so the new rows can be traced back to the source document.",
"⚠️ DO NOT USE BY DEFAULT. This writes to the `price_list` catalogue, which the POS does NOT read — nothing here ever reaches the till. Only call this when the user has explicitly asked for the price list / catalogue / supplier price list. For anything they sell across the counter, use `stock_items.create` instead. Bulk-upsert parsed supplier-price-list line items into the `price_list` catalogue. The agent reads the source document (a supplier PDF, a quote PDF, or a pasted supplier email) with its native vision capability, extracts the per-line fields, and passes them in. The tool handles deduplication by `(sku, supplier)`: if a row already exists for that SKU from that supplier, the existing row's `cost_price`, `description`, `selling_price`, `unit`, `category`, `subcategory`, `brand`, `markup`, `stock_qty`, `search_text`, and `sku_normalised` are patched in place; otherwise a new row is inserted. `cost_price` OVERWRITES — the latest price wins. The supplier is stored as free text on `price_list.supplier` (the live schema has no FK), so the supplier name is taken verbatim — pass the NAME, never a PocketBase record id. Items with missing `sku`, missing `supplier_name`, non-positive `cost_price`, or a PB write failure are returned in `skipped[]` with a per-item reason so the agent can report them back to the operator. Max 500 items per call — chunk larger lists. Optionally pass `source_file_id` (a `raw_price_list` row id, when the operator uploaded the document via `/pricelist` first) and `source_label` (free text, e.g. `\"Q-AFRISAN-2026-08\"`) so the new rows can be traced back to the source document.",
inputSchema: {
type: "object",
required: ["items"],

View file

@ -79,9 +79,18 @@ describe("buildSystemPrompt", () => {
expect(p).toMatch(/before\/after/);
});
it("distinguishes supplier price lists from POS stock prices", () => {
// Changed 2026-10-15. Previously the prompt said "if the request is
// ambiguous, ask which one they mean" — which left the choice to the
// model, and prices kept landing in price_list instead of the POS. The
// rule is now POS-by-default with the catalogue opt-in.
it("makes the POS the default and the price_list catalogue opt-in", () => {
expect(p).toContain("price_list.add_items");
expect(p).toMatch(/ask which one/i);
expect(p).toMatch(/NEVER write to the price_list catalogue/i);
expect(p).toMatch(/unless the user has explicitly asked/i);
expect(p).toMatch(/meaning the POS \(stock_items\)/i);
// The old "ask which one" guidance is gone — asking is now the
// fallback, not the primary instruction.
expect(p).not.toMatch(/ask which one/i);
});
});
});

View file

@ -25,7 +25,7 @@ Stock items and pricing (POS):
- 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.
- NEVER write to the price_list catalogue (price_list.add_items) unless the user has explicitly asked for the price list / catalogue / supplier price list in this conversation. Treat "load prices", "add these items", "here is a supplier list" and anything ambiguous as meaning the POS (stock_items) — that is the default. Writing prices into price_list when the user meant the till is the most common mistake in this system, because the POS does not read price_list at all: those prices never reach the counter. If you are unsure, ask once before writing anything.
Style: plain, concise South African business English. Lead with the answer. Use tables for lists of records. Include order / quote / PO numbers so the user can find things. Amounts as "R12,345.67".`;
}

View file

@ -60,7 +60,24 @@ describe("classifyTool", () => {
it("treats ordinary mutations as write", () => {
expect(classifyTool("orders.update")).toBe("write");
expect(classifyTool("quotes.create")).toBe("write");
expect(classifyTool("price_list.add_items")).toBe("write");
});
// Changed 2026-10-15: the price_list catalogue is opt-in only. It was
// `write` (auto-approved on a "trusted" chat), which is how prices kept
// landing there instead of in the POS. It now sits in CONFIRM_TOOLS, so it
// asks every time regardless of trust.
it("treats price_list.add_items as confirm — the catalogue is opt-in only", () => {
expect(classifyTool("price_list.add_items")).toBe("confirm");
const call = classifyToolCall("price_list.add_items", {
items: [{ sku: "ABC", description: "Thing", cost_price: 10, supplier_name: "Acme" }],
});
expect(call.cls).toBe("confirm");
expect(call.reason).toMatch(/price_list catalogue/i);
expect(call.reason).not.toMatch(/email/i);
});
it("still classifies price_list.add_items as confirm even with no args to scan", () => {
expect(classifyToolCall("price_list.add_items", {}).cls).toBe("confirm");
});
it("treats stock_items.set_pricing as write (needs approval; not read, confirm or denied)", () => {

View file

@ -35,6 +35,11 @@ export const DENIED_TOOLS: ReadonlySet<string> = new Set([
* Ignores the trust toggle.
*/
export const CONFIRM_TOOLS: ReadonlySet<string> = new Set([
// Writes to the `price_list` catalogue, which the POS never reads — prices
// there are invisible at the till. Opt-in only (added 2026-10-15): the
// operator must ask for the price list explicitly. `confirm` ignores the
// "trusted chat" auto-approve, so this can never fire silently.
"price_list.add_items",
"quotes.send_email",
"quotes.retry_email",
"purchase_orders.send_email",
@ -125,6 +130,12 @@ function hasContactKey(value: unknown, depth: number): boolean {
}
const CANCEL_REASON = "Cancels or merges records — always asks";
const CATALOGUE_REASON = "Writes to the price_list catalogue, which the POS never reads — always asks";
/** Per-tool reason for the `confirm` class, for tools that are neither email nor cancel. */
const CONFIRM_REASONS: Record<string, string> = {
"price_list.add_items": CATALOGUE_REASON,
};
/**
* Args-aware classification. Starts from the tool's class; a plain write whose
@ -140,7 +151,9 @@ export function classifyToolCall(
if (base === "confirm") {
return {
cls: "confirm",
reason: /\.(cancel|combine)$/.test(name) ? CANCEL_REASON : "Sends an email",
reason:
CONFIRM_REASONS[name] ??
(/\.(cancel|combine)$/.test(name) ? CANCEL_REASON : "Sends an email"),
};
}
if (base === "write" && hasContactKey(input, 0)) {