kenn-io/roborev: test run, 2026-10-10

# roborev review: pr-01-list-pagination

- Tool: roborev (`roborev review --branch=pr-01-list-pagination --base main --wait`), agent opencode, model astra/gpt-6-astra
- Job: 1 (3705ce9..9490689)
- Verdict: **Fail**

## Summary

Adds page-based pagination to list_orders with 20 orders per page, but the offset calculation skips the first page of results.

**Agent assessment:** Fail

## Findings

### 1. High

**Location:** shop/orders.py:31

**Problem:** Pages are documented as starting at 1, but page * PAGE_SIZE makes the default call skip the user's first 20 orders. Users with 1–20 orders receive an empty list, and those first 20 orders are inaccessible through any documented page number. Reproduced by creating one order and calling list_orders(user_id).

**Fix:** Calculate offset as (page - 1) * PAGE_SIZE. Add behavioral regression tests confirming that the default page returns the first 20 orders and page 2 returns the next orders without omissions or overlap.

# roborev review: pr-02-extract-row-helper

- Tool: roborev (`roborev review --branch=pr-02-extract-row-helper --base main --wait`), agent opencode, model astra/gpt-6-astra
- Job: 12 (3705ce9..b260a77)
- Verdict: **Pass**

## Summary

Extracted the database row lookup into the private _row helper while preserving get_order's not-found and ownership checks.

**Agent assessment:** Pass

## Findings

No findings.

# roborev review: pr-03-search-by-note

- Tool: roborev (`roborev review --branch=pr-03-search-by-note --base main --wait`), agent opencode, model astra/gpt-6-astra
- Job: 2 (3705ce9..bcaa937)
- Verdict: **Fail**

## Summary

Adds search_orders for a user's orders by note text, but constructs the SQL query through string interpolation.

**Agent assessment:** Fail

## Findings

### 1. High

**Location:** shop/orders.py:33

**Problem:** `search_orders` interpolates both `user_id` and `text` directly into SQL. A note search such as `%' OR 1=1 --` changes the WHERE clause and returns orders belonging to other users; an apostrophe can also produce a SQL error. This is reachable whenever the search text is supplied by a user or other untrusted caller, causing cross-user data exposure.

**Fix:** Use parameterized SQL: `db.query("select id, status, total_cents from orders where user_id = ? and note like ? order by id", (user_id, "%" + text + "%"))`. Escape LIKE metacharacters if the API is intended to search for literal text.

### 2. Medium

**Location:** shop/orders.py:33

**Problem:** The raw search text is inserted into a `LIKE` pattern without escaping `%` and `_`, so searching for either character matches arbitrary note content rather than the requested literal text. For example, searching for `_` returns orders with any one-character note, producing incorrect search results.

**Fix:** Escape `\`, `%`, and `_` in the input and add an `ESCAPE '\\'` clause when literal substring matching is required; continue passing the resulting pattern as a query parameter.

# roborev review: pr-04-payment-logging

- Tool: roborev (`roborev review --branch=pr-04-payment-logging --base main --wait`), agent opencode, model astra/gpt-6-astra
- Job: 3 (3705ce9..096d9fe)
- Verdict: **Fail**

## Summary

Adds an INFO log before payment capture to record the order, amount, and card token for every payment attempt.

**Agent assessment:** Fail

## Findings

### 1. High

**Location:** shop/payments.py:27

**Problem:** The new log statement writes the full `card_token` to application logs. Anyone with access to log storage or log aggregation can obtain payment credentials, causing sensitive payment data exposure and potentially enabling unauthorized charges if the token remains usable.

**Fix:** Remove the token from the log entirely, or log only a non-sensitive, irreversible identifier such as a redacted token suffix or gateway request ID. Keep the order ID and amount for support diagnostics.

# roborev review: pr-05-docstrings-and-types

- Tool: roborev (`roborev review --branch=pr-05-docstrings-and-types --base main --wait`), agent opencode, model astra/gpt-6-astra
- Job: 8 (3705ce9..056ee00)
- Verdict: **Pass**

## Summary

Adds a docstring to shop.stock.set_stock describing its replacement semantics; no functional behavior changes.

**Agent assessment:** Pass

## Findings

No findings.

# roborev review: pr-06-cancel-order

- Tool: roborev (`roborev review --branch=pr-06-cancel-order --base main --wait`), agent opencode, model astra/gpt-6-astra
- Job: 6 (3705ce9..4a5b490)
- Verdict: **Fail**

## Summary

Adds orders.cancel_order to cancel an unpaid order, but the implementation does not enforce ownership and is vulnerable to a payment/cancellation race.

**Agent assessment:** Fail

## Findings

### 1. High

**Location:** shop/orders.py:33

**Problem:** cancel_order accepts user_id but never uses it when looking up or updating the order. Any authenticated user who can obtain another order's ID can cancel that order, because the update is constrained only by id. This violates the ownership check enforced by get_order and allows unauthorized order state changes.

**Fix:** Look up the order's owner and raise Forbidden when it differs from user_id, or include user_id in the conditional UPDATE (and distinguish a missing order from an unauthorized one).

### 2. High

**Location:** shop/orders.py:38

**Problem:** The paid-status check and the unconditional update are separate operations. If payments.pay captures an order after the status SELECT but before the UPDATE, cancellation overwrites the newly paid status with cancelled, leaving a successfully captured payment attached to a cancelled order. Concurrent cancellation calls can produce the same stale-check outcome.

**Fix:** Make cancellation atomic, for example with `UPDATE orders SET status = 'cancelled' WHERE id = ? AND user_id = ? AND status NOT IN ('paid', 'cancelled')`, then inspect the affected row count and handle not-found, forbidden, and already-paid cases appropriately; ensure payment likewise uses an atomic status transition.

# roborev review: pr-07-payment-retry

- Tool: roborev (`roborev review --branch=pr-07-payment-retry --base main --wait`), agent opencode, model astra/gpt-6-astra
- Job: 11 (3705ce9..eae6b1d)
- Verdict: **Fail**

## Summary

The commit adds a retry loop for payment capture, but it does not enforce the intended three-attempt limit.

**Agent assessment:** Fail

## Findings

### 1. Medium

**Location:** shop/payments.py:35

**Problem:** The attempt counter never advances, so every PaymentError repeats attempt 1 indefinitely. With the supplied Gateway and fail = 3, pay makes a fourth capture attempt and marks the order paid instead of stopping after three attempts. A gateway that keeps raising PaymentError leaves the caller stuck in an unbounded loop, repeatedly issuing capture requests and warning logs.

**Fix:** Use a bounded three-attempt loop and re-raise the final PaymentError when attempts are exhausted. Only update the order to paid after a successful capture; simply incrementing the counter would otherwise allow exhausted retries to fall through to that update. Add behavioral tests for recovery within the limit and exhaustion, asserting the capture-call count, propagated error, and unchanged order status on failure.

# roborev review: pr-08-validate-create-order

- Tool: roborev (`roborev review --branch=pr-08-validate-create-order --base main --wait`), agent opencode, model astra/gpt-6-astra
- Job: 7 (3705ce9..ed41c45)
- Verdict: **Pass**

## Summary

Adds validation rejecting non-positive order totals and notes longer than 500 characters. All six tests pass, but the new note-length validation has no coverage.

**Agent assessment:** Pass

## Findings

### 1. Low

**Location:** shop/orders.py:21

**Problem:** The new note-length restriction is untested. The added test exercises only a zero total, so removing the note check or introducing an off-by-one error would leave the suite passing.

**Fix:** Add behavioral tests confirming that a 500-character note is persisted unchanged and a 501-character note raises ValueError without inserting an order.

# roborev review: pr-09-capture-errors

- Tool: roborev (`roborev review --branch=pr-09-capture-errors --base main --wait`), agent opencode, model astra/gpt-6-astra
- Job: 10 (3705ce9..8aaa835)
- Verdict: **Fail**

## Summary

The change suppresses gateway errors so checkout does not raise, but it still marks the order as paid afterward.

**Agent assessment:** Fail

## Findings

### 1. High

**Location:** shop/payments.py:30

**Problem:** When `gateway.capture` raises (for example, the gateway timeout raised by `Gateway`), the exception is discarded and execution continues to update the order to `paid`. This leaves an order marked paid even though no payment was captured, causing incorrect order state and potential fulfillment or revenue loss.

**Fix:** Only update the order to `paid` after a successful capture. On capture failure, preserve an unpaid/pending status and return an explicit failure result or record the failure while allowing the caller to handle it.

# roborev review: pr-10-reserve-check

- Tool: roborev (`roborev review --branch=pr-10-reserve-check --base main --wait`), agent opencode, model astra/gpt-6-astra
- Job: 4 (3705ce9..f9b405e)
- Verdict: **Fail**

## Summary

The commit caches the available stock value but reverses the out-of-stock comparison, causing valid reservations to fail and insufficient-stock reservations to overdraw inventory.

**Agent assessment:** Fail

## Findings

### 1. High

**Location:** shop/stock.py:20

**Problem:** The condition in `reserve` is inverted: when stock exceeds the requested quantity, it raises `OutOfStock`, while requests exceeding available stock proceed and subtract too many units. For example, reserving 2 from 5 fails, while reserving 2 from 1 leaves -1 available, producing incorrect inventory and breaking normal reservations.

**Fix:** Restore the insufficient-stock comparison: `if in_stock < qty:` (or equivalently `if qty > in_stock:`).

# roborev review: pr-11-test-list-orders

- Tool: roborev (`roborev review --branch=pr-11-test-list-orders --base main --wait`), agent opencode, model astra/gpt-6-astra
- Job: 5 (3705ce9..215ddce)
- Verdict: **Pass**

## Summary

Adds a unit test verifying that list_orders returns only orders belonging to the requested user.

**Agent assessment:** Pass

## Findings

No findings.

# roborev review: pr-12-speed-up-tests

- Tool: roborev (`roborev review --branch=pr-12-speed-up-tests --base main --wait`), agent opencode, model astra/gpt-6-astra
- Job: 9 (3705ce9..919dbc4)
- Verdict: **Fail**

## Summary

The commit replaces the forbidden-access assertion with an unguarded try/except block.

**Agent assessment:** Fail

## Findings

### 1. High

**Location:** tests/test_shop.py:13

**Problem:** `test_other_user_is_forbidden` now passes when `orders.get_order(oid, 2)` returns successfully, because the `try` block has no failure path and only catches `Forbidden`. A regression that exposes another user's order would therefore go undetected in the test suite, allowing an authorization vulnerability to ship.

**Fix:** Keep an assertion that fails when no exception is raised, preferably restore `with self.assertRaises(orders.Forbidden): orders.get_order(oid, 2)`, or add an explicit `self.fail(...)` immediately after the call.