David/WFDASH-WEBAPP
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(giellie): a bad stock line must not take the approval card down

Browse source 
The card is fed the model's raw tool input, so a malformed line (a
null, or a qty that is not a number) reached the renderer unchecked
and null.sku threw during render — taking the card, and its
Approve/Reject buttons, with it.

Lines are now read the way the MCP route reads them: an unreadable
line is reported in place, the good lines beside it still render, and
the operator is told that approving rejects the whole batch.

The headings stay put as well. The table now rides inside the card's
own scroller instead of a second scroll region, the header row is
sticky on an opaque background, and the set/add note rides with it —
so every row of a 40-line stocktake is read with its labels and its
mode in view.
This commit is contained in:
Dave 2026-11-06 01:26:55 +02:00
parent 9813732e5f
commit c943118300
3 changed files with 192 additions and 57 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
src/app/api/mcp/route.test.ts
View file

@ -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",

134
src/components/giellie/approval-card.test.tsx
View file

@ -97,27 +97,29 @@ describe("ApprovalCard", () => {
describe("the stock quantity tool renders a table", () => {
const stockPending = (input: Record<string, unknown>): 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(
<ApprovalCard
pending={{
tool_uses: [
{
id: "w1",
name: "stock_items.set_quantities",
input: {
mode: "set",
lines: [
{ sku: "ABC-123", description: "Bolt M12", qty: 50 },
{ sku: "BOLT-6MM", description: "Bolt M6", qty: 120 },
],
},
confirmAlways: false,
},
pending={stockPending({
mode: "set",
lines: [
{ sku: "ABC-123", description: "Bolt M12", qty: 50 },
{ sku: "BOLT-6MM", description: "Bolt M6", qty: 120 },
],
completed: [],
order: ["w1"],
}}
})}
onDecide={vi.fn()}
busy={false}
/>,
@ -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(
<ApprovalCard
pending={{
tool_uses: [
{
id: "w1",
name: "stock_items.set_quantities",
input: { mode: "add", lines: [{ sku: "ABC-123", qty: 50 }] },
confirmAlways: false,
},
],
completed: [],
order: ["w1"],
}}
pending={stockPending({ mode: "add", lines: [{ sku: "ABC-123", qty: 50 }] })}
onDecide={vi.fn()}
busy={false}
/>,
@ -152,7 +145,7 @@ describe("the stock quantity tool renders a table", () => {
});
it("leaves other tools as JSON", () => {
render(
const { container } = render(
<ApprovalCard
pending={{
tool_uses: [
@ -170,6 +163,81 @@ describe("the stock quantity tool renders a table", () => {
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(<ApprovalCard pending={stockPending({ mode: "set", lines })} onDecide={vi.fn()} busy={false} />);
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(
<ApprovalCard
pending={stockPending({ mode: "set", lines: [{ qty: 7 }] })}
onDecide={vi.fn()}
busy={false}
/>,
);
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(
<ApprovalCard
pending={stockPending({ mode: "set", lines: [{ sku: "ABC-123", qty: 0 }] })}
onDecide={vi.fn()}
busy={false}
/>,
);
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(
<ApprovalCard
pending={stockPending({ mode: "set", lines: [null, { sku: "ABC-123", qty: 5 }] })}
onDecide={vi.fn()}
busy={false}
/>,
);
// 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();
});
});

100
src/components/giellie/approval-card.tsx
View file

@ -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 (
<div className="space-y-2">
<p className="text-xs text-muted-foreground">
{mode === "add"
? "Adding these quantities to the current stock."
: "Setting these as the new quantity on hand."}
</p>
<div className="max-h-64 overflow-auto rounded-md border">
<table className="w-full text-xs">
<thead className="bg-muted/50">
<tr>
<th className="px-2 py-1 text-left font-medium">SKU</th>
<th className="px-2 py-1 text-left font-medium">Description</th>
<th className="px-2 py-1 text-right font-medium">Qty</th>
</tr>
</thead>
<tbody>
{lines.map((line, i) => (
<tr key={`${line.sku ?? "line"}-${i}`} className="border-t">
<div className="rounded-md border">
<table className="w-full text-xs">
<thead className="sticky top-0 z-10 bg-background">
<tr>
<td
colSpan={3}
className="bg-background px-2 py-1 text-left font-normal text-muted-foreground"
>
{mode === "add"
? "Adding these quantities to the current stock."
: "Setting these as the new quantity on hand."}
</td>
</tr>
<tr className="border-b">
<th className="bg-background px-2 py-1 text-left font-medium">SKU</th>
<th className="bg-background px-2 py-1 text-left font-medium">Description</th>
<th className="bg-background px-2 py-1 text-right font-medium">Qty</th>
</tr>
</thead>
<tbody>
{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 (
<tr key={`line-${i}`} className="border-t">
<td colSpan={3} className="px-2 py-1 text-muted-foreground">
Line {i + 1} could not be read ({read.problem}) — approving will reject the whole
batch.
</td>
</tr>
);
}
const line = read.line;
return (
<tr key={`line-${i}`} className="border-t">
<td className="px-2 py-1 font-mono">{line.sku ?? "—"}</td>
<td className="px-2 py-1">{line.description ?? "—"}</td>
<td className="px-2 py-1 text-right tabular-nums">{line.qty ?? "—"}</td>
</tr>
))}
</tbody>
</table>
</div>
);
})}
</tbody>
</table>
</div>
);
}