diff --git a/src/app/api/mcp/route.test.ts b/src/app/api/mcp/route.test.ts index 34c9317..6798db0 100644 --- a/src/app/api/mcp/route.test.ts +++ b/src/app/api/mcp/route.test.ts @@ -4372,6 +4372,21 @@ describe("MCP tool stock_items.set_quantities", () => { expect(tool!.inputSchema.properties.actor).toBeTruthy(); }); + it("documents that a row reported as applied on a dry run only means it would be applied", async () => { + const res = await GET( + new NextRequest("http://localhost/api/mcp", { + headers: { authorization: `Bearer ${process.env.AGENT_API_KEY}` }, + }), + ); + const body = (await res.json()) as { tools: Array<{ name: string; description: string }> }; + const tool = body.tools.find((t) => t.name === "stock_items.set_quantities"); + expect(tool).toBeTruthy(); + // On a dry run nothing is written, so "applied" means "would be + // applied". A reader of the description alone must not get the old + // ambiguity. + expect(tool!.description).toMatch(/dry run.*would be applied/i); + }); + it("forwards the batch to the engine: mode, dry_run, actor and truncated qty", async () => { const res = await call({ mode: "set", diff --git a/src/components/giellie/approval-card.test.tsx b/src/components/giellie/approval-card.test.tsx index 8946233..d10c21d 100644 --- a/src/components/giellie/approval-card.test.tsx +++ b/src/components/giellie/approval-card.test.tsx @@ -97,27 +97,29 @@ describe("ApprovalCard", () => { describe("the stock quantity tool renders a table", () => { + const stockPending = (input: Record): PendingAction => ({ + tool_uses: [ + { + id: "w1", + name: "stock_items.set_quantities", + input, + confirmAlways: false, + }, + ], + completed: [], + order: ["w1"], + }); + it("lists the sku, description and qty", () => { - render( + const { container } = render( , @@ -126,24 +128,15 @@ describe("the stock quantity tool renders a table", () => { expect(screen.getByText("Bolt M12")).toBeTruthy(); expect(screen.getByText("50")).toBeTruthy(); expect(screen.getByText(/new quantity on hand/i)).toBeTruthy(); - expect(document.body.textContent).not.toMatch(/"lines"/); + // The card renders a table — scoped to this card, not the whole page. + expect(screen.getByRole("table")).toBeTruthy(); + expect(container.textContent).not.toMatch(/"lines"/); }); it("says when the quantities are being added instead of set", () => { render( , @@ -152,7 +145,7 @@ describe("the stock quantity tool renders a table", () => { }); it("leaves other tools as JSON", () => { - render( + const { container } = render( { busy={false} />, ); - expect(document.body.textContent).toMatch(/"status"/); + expect(container.textContent).toMatch(/"status"/); + // No table for another tool … + expect(screen.queryByRole("table")).toBeNull(); + // … and the JSON keeps the card's own styling. + const pre = container.querySelector("pre"); + expect(pre).toBeTruthy(); + expect(pre!.className).toContain("max-h-48"); + expect(pre!.className).toContain("overflow-auto"); + expect(pre!.className).toContain("whitespace-pre-wrap"); + expect(pre!.className).toContain("break-words"); + expect(pre!.className).toContain("text-muted-foreground"); + }); + + it("keeps the headings in view for a 40-line batch, in the card's own scroller", () => { + const lines = Array.from({ length: 40 }, (_, i) => ({ + sku: `SKU-${i}`, + description: `Item ${i}`, + qty: i, + })); + render(); + const table = screen.getByRole("table"); + expect(table.querySelectorAll("tbody tr")).toHaveLength(40); + // The headings are pinned, so no row of the batch is read without them. + const head = table.querySelector("thead"); + expect(head!.className).toContain("sticky"); + expect(head!.className).toContain("bg-background"); + // The card's list is the only scroller — no second scroll region. + const list = screen.getByTestId("approval-list"); + expect(list.className).toContain("overflow-y-auto"); + expect(table.closest(".overflow-y-auto")).toBe(list); + }); + + it("shows a dash for a line with no sku or description", () => { + render( + , + ); + const cells = screen.getByRole("table").querySelectorAll("tbody td"); + expect(cells).toHaveLength(3); + expect(cells[0].textContent).toBe("—"); + expect(cells[1].textContent).toBe("—"); + expect(cells[2].textContent).toBe("7"); + }); + + it("shows a zero quantity as zero, not as a blank", () => { + render( + , + ); + const cells = screen.getByRole("table").querySelectorAll("tbody td"); + expect(cells[2].textContent).toBe("0"); + }); + + it("survives a malformed line and names it, without hiding the good lines", () => { + render( + , + ); + // The card did not fall over: the buttons are still on screen, so the + // operator is never stranded by a line they cannot see. + expect(screen.getByRole("button", { name: /approve/i })).toBeTruthy(); + expect(screen.getByRole("button", { name: /reject/i })).toBeTruthy(); + // The bad line is reported, and it does not read as a partial write. + expect(screen.getByText(/line 1 could not be read/i)).toBeTruthy(); + expect(screen.getByText(/reject the whole batch/i)).toBeTruthy(); + // The good line next to it still renders. + expect(screen.getByText("ABC-123")).toBeTruthy(); }); }); diff --git a/src/components/giellie/approval-card.tsx b/src/components/giellie/approval-card.tsx index 300afce..a62c965 100644 --- a/src/components/giellie/approval-card.tsx +++ b/src/components/giellie/approval-card.tsx @@ -7,6 +7,36 @@ import type { PendingAction } from "@/lib/giellie-types"; type StockQuantityLine = { sku?: string; description?: string; qty?: number }; +/** + * Read one pasted line defensively. The card is fed the model's raw + * tool input — nothing validates it before it reaches here — and the + * Approve/Reject buttons live in this card, so one malformed line must + * never take the card down. The rules mirror the MCP route's own + * validation, so a line the card calls unreadable is a line the route + * would reject. + */ +function readLine(raw: unknown): { line: StockQuantityLine } | { problem: string } { + if (!raw || typeof raw !== "object" || Array.isArray(raw)) { + return { problem: "not a line object" }; + } + const l = raw as { sku?: unknown; description?: unknown; qty?: unknown }; + const asText = (value: unknown, field: string): { value?: string; problem?: string } => { + if (value === undefined || value === null) return { value: undefined }; + if (typeof value === "string") return { value }; + // A spreadsheet paste sends a numeric SKU; the route reads it as text too. + if (typeof value === "number" && Number.isFinite(value)) return { value: String(value) }; + return { problem: `${field} must be text` }; + }; + const sku = asText(l.sku, "sku"); + if (sku.problem) return { problem: sku.problem }; + const description = asText(l.description, "description"); + if (description.problem) return { problem: description.problem }; + if (typeof l.qty !== "number" || !Number.isFinite(l.qty)) { + return { problem: "qty must be a number" }; + } + return { line: { sku: sku.value, description: description.value, qty: l.qty } }; +} + /** * The stock quantity tool is the one tool whose input is a list the * operator has to read line by line. JSON is not reviewable at that @@ -16,35 +46,57 @@ export function renderToolInput(toolName: string, input: unknown): ReactNode { if (toolName === "stock_items.set_quantities" && input && typeof input === "object") { const payload = input as { mode?: unknown; lines?: unknown }; const mode = typeof payload.mode === "string" ? payload.mode : ""; - const lines = Array.isArray(payload.lines) ? (payload.lines as StockQuantityLine[]) : []; - if (lines.length > 0) { + const rawLines: unknown[] = Array.isArray(payload.lines) ? payload.lines : []; + if (rawLines.length > 0) { + // One scroller only: the card's own list scrolls (that is what + // its pinned header was built for), so the table rides inside it + // and its headings stay put for every row of a 40-line batch. return ( -
-

- {mode === "add" - ? "Adding these quantities to the current stock." - : "Setting these as the new quantity on hand."} -

-
- - - - - - - - - - {lines.map((line, i) => ( - +
+
SKUDescriptionQty
+ + + + + + + + + + + + {rawLines.map((raw, i) => { + const read = readLine(raw); + if ("problem" in read) { + // Reported, never dropped: one bad line must not hide + // the rest, and it must not read as a partial write. + return ( + + + + ); + } + const line = read.line; + return ( + - ))} - -
+ {mode === "add" + ? "Adding these quantities to the current stock." + : "Setting these as the new quantity on hand."} +
SKUDescriptionQty
+ Line {i + 1} could not be read ({read.problem}) — approving will reject the whole + batch. +
{line.sku ?? "—"} {line.description ?? "—"} {line.qty ?? "—"}
-
+ ); + })} + +
); }