bot: findInventoryByPrefix picks an arbitrary row, so a purchase can overwrite the wrong item's buy_price #81

New issue

Closed

opened 2026-09-02 16:50:30 +00:00 by yuris · 1 comment

yuris commented

2026-09-02 16:50:30 +00:00

Owner

Found by Reviewer78r1 during review of PR #78 and deliberately not fixed there — it is a different defect from #71's numeric bounds and wants its own change.

What it does

findInventoryByPrefix (apps/management/lib/inventory/queries.ts:54) resolves a raw item name to an inventory row with a case-insensitive substring match on a leading 8-character prefix, limit 1, and no ORDER BY.

So the row it returns is an arbitrary one of however many match. The bot then books a purchase against it.

Measured against a real database:

existing row  : "Oli Mesin 10W-40 PROBE"      <- the ONLY row present
bot receives  : "Oli Mesin 20W-50 BEDA"      <- a different product
prefix used   : "Oli Mesi"  ->  ilike '%Oli Mesi%'  limit 1
resolved to   : "Oli Mesin 10W-40 PROBE"     <- the wrong item
effect        : qty added to the wrong row, its buy_price OVERWRITTEN,
                and "Oli Mesin 20W-50 BEDA" is never created at all

No ambiguity is required. One matching row is enough: the incoming name is a
different product, and a loose substring match silently redirects it onto an
existing row. Because the resolver "succeeds", the new product is never created,
so that one row accumulates two products' stock as well as the wrong cost.

(The multi-match non-determinism described further down is real but is a second,
independent problem, and it was not what was measured here.)

Why it matters more than the case that was fixed

inventory.buy_price is the cost column every COGS, gross-profit and net-profit figure in the monthly report derives from. Overwriting it with another product's price silently biases every margin computed afterwards, and nothing surfaces — the operator gets a success reply.

That is the same cost-column poisoning #71 was filed about, and this route to it is more likely than the OCR magnitude error #71 addressed: it needs no misread at all, just two products sharing eight leading characters. Oil grades, filter variants and brake-pad sizes all share long prefixes by nature, so the collision is the normal case for this catalogue rather than an unlucky one.

Two separable problems

The measured defect is #2; #1 compounds it but is not needed to reach it.

Ambiguity is resolved silently. limit 1 with no ordering picks a row by whatever the plan happens to emit. Even with perfect data this is not reproducible across a vacuum or an index change.
A substring match is too loose for a write. ilike '%first-8-chars%' was chosen for a chat/OCR resolver where a near match beats no match — reasonable for reading, but this result is used to pick which row a purchase mutates.

What would fix it

The bot should refuse to guess when the prefix matches more than one row, and ask which item is meant — it already has a rejection path that says so plainly, describeBotRejection, and the whole-record refusal decision in lib/bot/nodes/rejection.ts argues exactly this case: the operator is holding the receipt and can answer in seconds, while a wrong cost basis is unrecoverable because nobody knows to look.

Concretely: count the matches instead of taking the first, and on more than one return a rejection naming the candidates. Also add a deterministic ORDER BY so the single-match path cannot drift.

Deliberately not proposing a longer prefix or an exact match: both change which chat messages resolve at all, which is a product decision about the bot's tolerance, not a bug fix. Worth deciding separately from closing the silent-wrong-row hole.

Not in scope for #78

#78 bounds the numbers that reach inventory.stock and inventory.buy_price and makes the services reject unreadable ones. This defect writes a valid number to the wrong row, so every bound in #78 passes. Filed separately rather than widened into that PR.

Found by `Reviewer78r1` during review of PR #78 and deliberately **not** fixed there — it is a different defect from #71's numeric bounds and wants its own change. ## What it does `findInventoryByPrefix` (`apps/management/lib/inventory/queries.ts:54`) resolves a raw item name to an inventory row with a case-insensitive substring match on a **leading 8-character prefix**, `limit 1`, and **no `ORDER BY`**. So the row it returns is an arbitrary one of however many match. The bot then books a purchase against it. Measured against a real database: ``` existing row : "Oli Mesin 10W-40 PROBE" <- the ONLY row present bot receives : "Oli Mesin 20W-50 BEDA" <- a different product prefix used : "Oli Mesi" -> ilike '%Oli Mesi%' limit 1 resolved to : "Oli Mesin 10W-40 PROBE" <- the wrong item effect : qty added to the wrong row, its buy_price OVERWRITTEN, and "Oli Mesin 20W-50 BEDA" is never created at all ``` **No ambiguity is required.** One matching row is enough: the incoming name is a different product, and a loose substring match silently redirects it onto an existing row. Because the resolver "succeeds", the new product is never created, so that one row accumulates two products' stock as well as the wrong cost. (The multi-match non-determinism described further down is real but is a second, independent problem, and it was not what was measured here.) ## Why it matters more than the case that was fixed `inventory.buy_price` is the cost column every COGS, gross-profit and net-profit figure in the monthly report derives from. Overwriting it with another product's price silently biases every margin computed afterwards, and nothing surfaces — the operator gets a success reply. That is the same cost-column poisoning #71 was filed about, and this route to it is **more likely** than the OCR magnitude error #71 addressed: it needs no misread at all, just two products sharing eight leading characters. Oil grades, filter variants and brake-pad sizes all share long prefixes by nature, so the collision is the normal case for this catalogue rather than an unlucky one. ## Two separable problems The measured defect is #2; #1 compounds it but is not needed to reach it. 1. **Ambiguity is resolved silently.** `limit 1` with no ordering picks a row by whatever the plan happens to emit. Even with perfect data this is not reproducible across a vacuum or an index change. 2. **A substring match is too loose for a write.** `ilike '%first-8-chars%'` was chosen for a chat/OCR resolver where a near match beats no match — reasonable for *reading*, but this result is used to pick which row a purchase mutates. ## What would fix it The bot should refuse to guess when the prefix matches more than one row, and ask which item is meant — it already has a rejection path that says so plainly, `describeBotRejection`, and the whole-record refusal decision in `lib/bot/nodes/rejection.ts` argues exactly this case: the operator is holding the receipt and can answer in seconds, while a wrong cost basis is unrecoverable because nobody knows to look. Concretely: count the matches instead of taking the first, and on more than one return a rejection naming the candidates. Also add a deterministic `ORDER BY` so the single-match path cannot drift. Deliberately not proposing a longer prefix or an exact match: both change which chat messages resolve at all, which is a product decision about the bot's tolerance, not a bug fix. Worth deciding separately from closing the silent-wrong-row hole. ## Not in scope for #78 #78 bounds the numbers that reach `inventory.stock` and `inventory.buy_price` and makes the services reject unreadable ones. This defect writes a **valid** number to the **wrong row**, so every bound in #78 passes. Filed separately rather than widened into that PR.

yuris commented

2026-09-02 16:59:26 +00:00

Author

Owner

Correction to the measured evidence in the description, from the reviewer it is attributed to.

The fixture block reads as though both rows existed and the bot was handed the name of one of them. That is not what I measured. In my run only "Oli Mesin 10W-40 PROBE" was in inventory; "Oli Mesin 20W-50 BEDA" was the incoming name, and it resolved onto the existing 10W-40 row. So the measured case is a genuinely new product being silently merged into an older near-name row, not an ambiguous pick between two existing rows:

existing row  : "Oli Mesin 10W-40 PROBE"          (the only match in inventory)
bot receives  : "Oli Mesin 20W-50 BEDA"           (a product not yet in inventory)
prefix used   : "Oli Mesi"  ->  ilike '%Oli Mesi%'  limit 1
findInventoryByPrefix -> HIT id=70 "Oli Mesin 10W-40 PROBE"
inventoryNameConflict -> none
=> bot resolves instead of creating: qty booked onto the 10W-40 row and its buy_price overwritten with the 20W-50 price

Two consequences worth keeping in the issue, because they differ from the two-existing-rows framing:

No ambiguity is required. A single matching row is enough. The limit 1-without-ORDER BY non-determinism is a second, independent problem that needs two or more matches; the wrong-row write needs only one.
It also silently prevents the new item from ever being created, so the catalogue never gains the 20W-50 entry and the 10W-40 row accumulates both products' stock. That is not visible in the two-existing-rows version.

The multi-match non-determinism described under "Two separable problems / 1" is real and I have not disproved it — limit 1 with no ordering genuinely picks by whatever the plan emits — I just did not measure that variant, so it should not sit inside the measured block.

Everything else in the description matches what I found, including the reasoning that this writes a valid number to the wrong row and therefore passes every bound PR #78 adds, which is why it belongs here rather than there.

Correction to the measured evidence in the description, from the reviewer it is attributed to. The fixture block reads as though **both** rows existed and the bot was handed the name of one of them. That is not what I measured. In my run only `"Oli Mesin 10W-40 PROBE"` was in `inventory`; `"Oli Mesin 20W-50 BEDA"` was the **incoming** name, and it resolved onto the existing 10W-40 row. So the measured case is a *genuinely new product being silently merged into an older near-name row*, not an ambiguous pick between two existing rows: ``` existing row : "Oli Mesin 10W-40 PROBE" (the only match in inventory) bot receives : "Oli Mesin 20W-50 BEDA" (a product not yet in inventory) prefix used : "Oli Mesi" -> ilike '%Oli Mesi%' limit 1 findInventoryByPrefix -> HIT id=70 "Oli Mesin 10W-40 PROBE" inventoryNameConflict -> none => bot resolves instead of creating: qty booked onto the 10W-40 row and its buy_price overwritten with the 20W-50 price ``` Two consequences worth keeping in the issue, because they differ from the two-existing-rows framing: - **No ambiguity is required.** A single matching row is enough. The `limit 1`-without-`ORDER BY` non-determinism is a *second*, independent problem that needs two or more matches; the wrong-row write needs only one. - **It also silently prevents the new item from ever being created**, so the catalogue never gains the 20W-50 entry and the 10W-40 row accumulates both products' stock. That is not visible in the two-existing-rows version. The multi-match non-determinism described under "Two separable problems / 1" is real and I have not disproved it — `limit 1` with no ordering genuinely picks by whatever the plan emits — I just did not measure that variant, so it should not sit inside the measured block. Everything else in the description matches what I found, including the reasoning that this writes a valid number to the wrong row and therefore passes every bound PR #78 adds, which is why it belongs here rather than there.

yuris referenced this issue from a commit

2026-09-02 17:21:44 +00:00

docs(management): narrow the one overclaiming lock clause, record the deploy invariant

yuris referenced this issue from a commit

2026-09-28 20:49:44 +00:00

fix(inventory): make prefix search deterministic and refuse ambiguous matches (#81)

yuris referenced this issue from a pull request that will close it,

2026-09-28 20:49:49 +00:00

fix(inventory): make prefix search deterministic and refuse ambiguous matches #119

yuris closed this issue

2026-09-28 20:50:09 +00:00

yuris referenced this issue from a commit