yuris/garage

Fork 0

management: createStockAdjustment has no schema; the modal previews a change it silently cannot perform #69

New issue

Closed

opened 2026-08-28 02:18:40 +00:00 by yuris · 0 comments

yuris commented

2026-08-28 02:18:40 +00:00

Owner

Found by Author59 while fixing #59/#60 on PR #68, filed in its corrected form after the author retracted its own first version. The original report claimed a stock/ledger divergence; that was a measurement error and is withdrawn. See PR #68 comment 3231 for the full retraction. This issue is the narrower claim that survived measurement.

What was retracted, so nobody re-derives it

The first report said adjustStock does stock + delta in SQL and therefore silently rounds a fractional delta — measured as stock 12 + delta 1.5 landing on 14 while stock_movements.quantity recorded 1.5.

That measurement used a SQL numeric literal:

update inventory set stock = stock + 1.5   -- int4 + numeric -> numeric, assignment rounds to 14

Drizzle does not do that. It sends a bound parameter, and an int4 parameter cannot carry 1.5:

set stock = stock + $1  (1.5)   ERROR  invalid input syntax for type integer: "1.5"
set buy_price = $1      (1.5)   ERROR  invalid input syntax for type integer: "1.5"
set buy_price = $1      (-1)    OK     buy_price = -1

There is no rounding and no ledger divergence. Literal-vs-bound-parameter is the entire difference, and it is worth recording because the wrong version is more alarming and would have been easy to file.

The two real defects

1. `createStockAdjustment` has no schema at all

It is the only inventory write path with no zod validation whatsoever. Every other path was bounded by PR #68 (sell_price, buy_price, stock, weight_grams). This one is protected today only by the browser's implicit step="1", which arrives free with min="0" on <input type="number">.

That protection is accidental, not designed, and it evaporates under any of:

step="0.01" or any explicit non-integer step
noValidate on the form
a direct server-action call, or any non-browser client

The int4 column is the only thing left, and it produces a bare 500 (invalid input syntax for type integer) rather than a readable rejection — the same shape #60 closed everywhere else.

Measured through the real modal: typing 1.500 in Jumlah makes the modal's own preview offer

Perubahan: 12 → 13.5 (+1.5)

with save enabled. Clicking save hits native stepMismatch and the submit is blocked. No server call, no mutation, no error shown — stock stayed 12 with 0 movements.

So the operator is shown a specific result, allowed to commit to it, and then silently gets nothing. That is worse than a visible rejection: the preview actively asserts the value is understood.

Why this is not a duplicate of #59 or #60

#59 is the Berat (gram) widget on the two inventory forms. Fixed in PR #68.
#60 covers buy_price and stock on the REST and MCP write paths, and explicitly treats the adjustment path as stock's legitimate owner — which is why PR #68 correctly did not widen into it.
This is the fourth instance of the numeric-input class, and the only remaining write path with no server-side schema.

Scope

Add a zod schema to createStockAdjustment bounding delta as an integer within int4, rejecting readably and naming the field, leaving the row untouched on rejection — matching what PR #68 established for the other four paths. Reuse the existing constant rather than declaring a second int4 literal.
Make the modal's preview and its submit agree. Either the preview refuses the value it cannot perform, or the field refuses the input before a preview is offered. A disabled save with a stated reason beats an enabled save that silently no-ops.
Apply #56's settled ruling: refuse, do not reinterpret. 1.500 is 1500 to an Indonesian typist and 1,5 is genuinely decimal, so any rule rescuing one input misreads the other.

Explicitly not this issue

The missing API error handler — thrown errors flatten to bare 500s across every endpoint, including making an unauthenticated REST write answer 500 instead of 401. Pre-existing, out of scope for #60, and filed separately.

Verification

Drive Jumlah in the real modal with 1.500, 1,500, abc, empty, -1 and a 10-digit value. Read inventory.stock, stock_movements and inventory_lots back from Postgres each time.
Call createStockAdjustment directly, bypassing the browser, with a fractional and a 10-digit delta. Assert a readable 4xx and an untouched row — this is the path the accidental step="1" does not cover.
Confirm the preview can never display a change the submit will not perform.
Confirm the FIFO ledger and movement history stay consistent with whatever is allowed.

Acceptance

createStockAdjustment validates its input server-side and rejects readably without mutating the row; no adjustment input relies on a browser default for safety; the modal never previews an outcome it cannot perform; a direct non-browser call cannot produce a bare 500.

Added scope: the negative resulting stock invariant

Routed here by #71 (its "Explicitly not this issue" section says the negative-resulting-stock invariant "belongs in adjustStock under #69"), and flagged by Reviewer68r5 during round 5 of PR #68 — because this issue's own Scope and Acceptance did not cover it, so closing #69 on its stated criteria would have left the gap untracked while update_inventory_item's tool description points here.

The gap, measured

AdjustmentDeltaSchema (PR #68) bounds the delta. It does not bound the result. A negative delta is legitimate — it is a removal — so a schema cannot be where this is fixed:

create_stock_adjustment delta=-99 on stock 12  ->  {"ok": true}, stock = -87, movement quantity -99

apps/management/lib/catalog.ts:80-81 derives row.stock > 0 ? "ready" : row.allowIndent ? "indent" : "habis", so a negative count reaches buyers as one of exactly two states — there is no third:

`allow_indent`	public `availability`	what a buyer sees
`false`	`habis`	sold out — wrong, but harmless
`true`	`indent`	orderable — the item stays on sale against a negative count

The indent branch is the harmful one and is the easy one to miss. apps/store/app/products/[slug]/add-to-cart.tsx:38-40 disables the cart button for habis only; indent falls through to the full cart flow and writes indent: true onto the line. apps/store/lib/facets.ts:49-50 states the same rule.

update_inventory_item's description says the negative count is one "that FIFO then drives further down". Measured, that is conditional, not unconditional:

order qty 3, 10 lot-units remaining  ->  ok,      stock -97 -> -100
order qty 1,  0 lot-units remaining  ->  isError INSUFFICIENT_STOCK, stock stays -97

apps/management/lib/orders/service.ts:53-59 sizes its guard from sum(inventory_lots.qty_remaining) and never reads inventory.stock, while adjustStock moves inventory.stock and never touches a lot.

So the two numbers are independent, and that is the deeper problem: inventory.stock and the lot ledger can disagree without either being wrong on its own terms.

Scope for this part

Reject a write that would leave inventory.stock negative, at the service layer in adjustStock, so every caller inherits it — REST, MCP, the dashboard action, and the bot node from #71. A schema cannot do this: it sees the delta, not the current row.
Decide deliberately whether a negative resulting stock is ever legitimate. If it is, the derived-availability rule must state what it means publicly, because today it silently means either habis or orderable.
While in adjustStock: decide whether it should also move the lot ledger, or whether inventory.stock diverging from sum(qty_remaining) is intended. Note the opening balance has never had a lot or an IN movement on any path including the seed — recorded on PR #68 — so the ledger cannot currently reconstruct stock at all.

Acceptance for this part

No caller of adjustStock can commit a negative inventory.stock; the rejection is readable and leaves the row unchanged; and if a negative is ever permitted, the public availability it produces is a stated decision rather than a side effect of allowIndent.

Found by `Author59` while fixing #59/#60 on PR #68, **filed in its corrected form after the author retracted its own first version.** The original report claimed a stock/ledger divergence; that was a measurement error and is withdrawn. See PR #68 comment 3231 for the full retraction. This issue is the narrower claim that survived measurement. ## What was retracted, so nobody re-derives it The first report said `adjustStock` does `stock + delta` in SQL and therefore **silently rounds** a fractional delta — measured as stock 12 + delta 1.5 landing on 14 while `stock_movements.quantity` recorded 1.5. That measurement used a **SQL numeric literal**: ```sql update inventory set stock = stock + 1.5 -- int4 + numeric -> numeric, assignment rounds to 14 ``` Drizzle does not do that. It sends a **bound parameter**, and an int4 parameter cannot carry `1.5`: ``` set stock = stock + $1 (1.5) ERROR invalid input syntax for type integer: "1.5" set buy_price = $1 (1.5) ERROR invalid input syntax for type integer: "1.5" set buy_price = $1 (-1) OK buy_price = -1 ``` **There is no rounding and no ledger divergence.** Literal-vs-bound-parameter is the entire difference, and it is worth recording because the wrong version is more alarming and would have been easy to file. ## The two real defects ### 1. `createStockAdjustment` has no schema at all It is the **only inventory write path with no zod validation whatsoever**. Every other path was bounded by PR #68 (`sell_price`, `buy_price`, `stock`, `weight_grams`). This one is protected today **only by the browser's implicit `step="1"`**, which arrives free with `min="0"` on `<input type="number">`. That protection is accidental, not designed, and it evaporates under any of: - `step="0.01"` or any explicit non-integer step - `noValidate` on the form - a direct server-action call, or any non-browser client The int4 column is the only thing left, and it produces a **bare 500** (`invalid input syntax for type integer`) rather than a readable rejection — the same shape #60 closed everywhere else. ### 2. The modal displays an outcome it cannot perform, then does nothing Measured through the real modal: typing `1.500` in `Jumlah` makes the modal's **own preview** offer > Perubahan: 12 → 13.5 (+1.5) with **save enabled**. Clicking save hits native `stepMismatch` and the submit is blocked. **No server call, no mutation, no error shown** — stock stayed 12 with 0 movements. So the operator is shown a specific result, allowed to commit to it, and then silently gets nothing. That is worse than a visible rejection: the preview actively asserts the value is understood. ## Why this is not a duplicate of #59 or #60 - **#59** is the `Berat (gram)` widget on the two inventory forms. Fixed in PR #68. - **#60** covers `buy_price` and `stock` on the REST and MCP write paths, and explicitly treats the **adjustment path as stock's legitimate owner** — which is why PR #68 correctly did not widen into it. - This is the **fourth** instance of the numeric-input class, and the only remaining write path with no server-side schema. ## Scope - Add a zod schema to `createStockAdjustment` bounding `delta` as an integer within int4, rejecting readably and naming the field, leaving the row untouched on rejection — matching what PR #68 established for the other four paths. Reuse the existing constant rather than declaring a second int4 literal. - Make the modal's preview and its submit agree. Either the preview refuses the value it cannot perform, or the field refuses the input before a preview is offered. **A disabled save with a stated reason beats an enabled save that silently no-ops.** - Apply #56's settled ruling: **refuse, do not reinterpret.** `1.500` is 1500 to an Indonesian typist and `1,5` is genuinely decimal, so any rule rescuing one input misreads the other. ## Explicitly not this issue The missing API error handler — thrown errors flatten to bare 500s across every endpoint, including making an unauthenticated REST write answer 500 instead of 401. Pre-existing, out of scope for #60, and filed separately. ## Verification - Drive `Jumlah` in the real modal with `1.500`, `1,500`, `abc`, empty, `-1` and a 10-digit value. Read `inventory.stock`, `stock_movements` and `inventory_lots` back from Postgres each time. - Call `createStockAdjustment` **directly**, bypassing the browser, with a fractional and a 10-digit delta. Assert a readable 4xx and an untouched row — this is the path the accidental `step="1"` does not cover. - Confirm the preview can never display a change the submit will not perform. - Confirm the FIFO ledger and movement history stay consistent with whatever is allowed. ## Acceptance `createStockAdjustment` validates its input server-side and rejects readably without mutating the row; no adjustment input relies on a browser default for safety; the modal never previews an outcome it cannot perform; a direct non-browser call cannot produce a bare 500. --- ## Added scope: the negative *resulting* stock invariant Routed here by **#71** (its *"Explicitly not this issue"* section says the negative-resulting-stock invariant *"belongs in `adjustStock` under #69"*), and flagged by `Reviewer68r5` during round 5 of PR #68 — because **this issue's own Scope and Acceptance did not cover it**, so closing #69 on its stated criteria would have left the gap untracked while `update_inventory_item`'s tool description points here. ### The gap, measured `AdjustmentDeltaSchema` (PR #68) bounds the *delta*. It does not bound the *result*. A negative delta is legitimate — it is a removal — so a schema cannot be where this is fixed: ``` create_stock_adjustment delta=-99 on stock 12 -> {"ok": true}, stock = -87, movement quantity -99 ``` `apps/management/lib/catalog.ts:80-81` derives `row.stock > 0 ? "ready" : row.allowIndent ? "indent" : "habis"`, so a negative count reaches buyers as one of exactly two states — there is no third: | `allow_indent` | public `availability` | what a buyer sees | |---|---|---| | `false` | `habis` | sold out — wrong, but harmless | | `true` | `indent` | **orderable** — the item stays on sale against a negative count | **The `indent` branch is the harmful one and is the easy one to miss.** `apps/store/app/products/[slug]/add-to-cart.tsx:38-40` disables the cart button for `habis` **only**; `indent` falls through to the full cart flow and writes `indent: true` onto the line. `apps/store/lib/facets.ts:49-50` states the same rule. ### One related claim corrected, so it is not inherited `update_inventory_item`'s description says the negative count is one *"that FIFO then drives further down"*. Measured, that is **conditional**, not unconditional: ``` order qty 3, 10 lot-units remaining -> ok, stock -97 -> -100 order qty 1, 0 lot-units remaining -> isError INSUFFICIENT_STOCK, stock stays -97 ``` `apps/management/lib/orders/service.ts:53-59` sizes its guard from `sum(inventory_lots.qty_remaining)` and **never reads `inventory.stock`**, while `adjustStock` moves `inventory.stock` and never touches a lot. So the two numbers are independent, and that is the deeper problem: **`inventory.stock` and the lot ledger can disagree without either being wrong on its own terms.** ### Scope for this part - Reject a write that would leave `inventory.stock` negative, **at the service layer in `adjustStock`**, so every caller inherits it — REST, MCP, the dashboard action, and the bot node from **#71**. A schema cannot do this: it sees the delta, not the current row. - Decide deliberately whether a negative resulting stock is ever legitimate. If it is, the derived-availability rule must state what it means publicly, because today it silently means either `habis` or **orderable**. - While in `adjustStock`: decide whether it should also move the lot ledger, or whether `inventory.stock` diverging from `sum(qty_remaining)` is intended. Note the opening balance has **never** had a lot or an `IN` movement on any path including the seed — recorded on PR #68 — so the ledger cannot currently reconstruct `stock` at all. ### Acceptance for this part No caller of `adjustStock` can commit a negative `inventory.stock`; the rejection is readable and leaves the row unchanged; and if a negative is ever permitted, the public `availability` it produces is a stated decision rather than a side effect of `allowIndent`.

yuris referenced this issue

2026-08-28 02:36:45 +00:00

data: no published item has category, brand, fitment, oemNumbers or weightGrams — three of four facets are empty in production #61

yuris referenced this issue

2026-08-28 02:49:24 +00:00

fix(management): refuse unparseable numeric input in both inventory forms; bound buy_price, stock and weight_grams #68

yuris referenced this issue

2026-08-28 02:50:01 +00:00

fix(management): refuse unparseable numeric input in both inventory forms; bound buy_price, stock and weight_grams #68

yuris referenced this issue from a commit

2026-08-28 06:58:52 +00:00

fix(management): bound the stock-adjustment delta, the tenth write path