alibaba/open-code-review: test run, 2026-10-10

# pr-01-list-pagination

Tool: open-code-review (ocr) v1.12.13, `ocr review --from main --to pr-01-list-pagination`, model gpt-6-astra
Status: complete — files reviewed: 1, failed: 0

## 1. [high] [bug] `shop/orders.py:31-33`

`page` is documented as 1-based, but this computes page 1's offset as 20, so the first page skips the first 20 orders and page 2 starts at 40. Subtract one before multiplying (and consider validating that `page >= 1`).

Existing code:
```
    offset = page * PAGE_SIZE
    return db.query("select id, status, total_cents from orders where user_id = ? order by id limit ? offset ?",
                    (user_id, PAGE_SIZE, offset))
```

Suggested code:
```
    offset = (page - 1) * PAGE_SIZE
    return db.query("select id, status, total_cents from orders where user_id = ? order by id limit ? offset ?",
                    (user_id, PAGE_SIZE, offset))
```

# pr-02-extract-row-helper

Tool: open-code-review (ocr) v1.12.13, `ocr review --from main --to pr-02-extract-row-helper`, model gpt-6-astra
Status: complete — files reviewed: 1, failed: 0

No findings.

# pr-03-search-by-note

Tool: open-code-review (ocr) v1.12.13, `ocr review --from main --to pr-03-search-by-note`, model gpt-6-astra
Status: complete — files reviewed: 1, failed: 0

## 1. [high] [security] `shop/orders.py:31-32`

This interpolates both `user_id` and the caller-provided `text` directly into SQL. A note such as `%' OR 1=1 --` can bypass the user filter and expose other users' orders (and other SQL syntax can cause execution errors or worse). Use placeholders for both values and pass the wildcard pattern as a parameter, e.g. `where user_id = ? and note like ?`, `(user_id, f"%{text}%")`.

Existing code:
```
    sql = f"select id, status, total_cents from orders where user_id = {user_id} and note like '%{text}%' order by id"
    return db.query(sql)
```

# pr-04-payment-logging

Tool: open-code-review (ocr) v1.12.13, `ocr review --from main --to pr-04-payment-logging`, model gpt-6-astra
Status: complete — files reviewed: 1, failed: 0

## 1. [high] [security] `shop/payments.py:27`

This logs the full `card_token`, which is a payment credential and may allow replay or otherwise expose sensitive customer data in application logs. Do not log the token; if support needs correlation, log a non-sensitive request/payment ID or a carefully masked/irreversible representation instead.

Existing code:
```
    log.info("capturing order %s amount %s with card token %s", order_id, amount_cents, card_token)
```

# pr-05-docstrings-and-types

Tool: open-code-review (ocr) v1.12.13, `ocr review --from main --to pr-05-docstrings-and-types`, model gpt-6-astra
Status: complete — files reviewed: 1, failed: 0

No findings.

# pr-06-cancel-order

Tool: open-code-review (ocr) v1.12.13, `ocr review --from main --to pr-06-cancel-order`, model gpt-6-astra
Status: complete — files reviewed: 1, failed: 0

## 1. [high] [security] `shop/orders.py:31-36`

The `user_id` argument is never used, so any caller who knows an order ID can cancel another user's unpaid order. This should enforce ownership (for example, load via `get_order(order_id, user_id)` or include `user_id` in the lookup/update) before changing the status.

Existing code:
```
    rows = db.query("select status from orders where id = ?", (order_id,))
    if not rows:
        raise NotFound(order_id)
    if rows[0][0] == "paid":
        raise ValueError("paid orders cannot be cancelled")
    db.execute("update orders set status = 'cancelled' where id = ?", (order_id,))
```

# pr-07-payment-retry

Tool: open-code-review (ocr) v1.12.13, `ocr review --from main --to pr-07-payment-retry`, model gpt-6-astra
Status: complete — files reviewed: 1, failed: 0

## 1. [high] [bug] `shop/payments.py:35-36`

`attempt` is never incremented, so any `PaymentError` causes an infinite retry loop and the order is never marked paid. Increment it after a failed attempt (or use a bounded `for` loop) so capture is attempted at most three times; after the final failure, re-raise/return instead of executing the paid update.

Existing code:
```
        except PaymentError:
            log.warning("capture failed for order %s, attempt %s", order_id, attempt)
```

# pr-08-validate-create-order

Tool: open-code-review (ocr) v1.12.13, `ocr review --from main --to pr-08-validate-create-order`, model gpt-6-astra
Status: complete — files reviewed: 1, failed: 0

No findings.

# pr-09-capture-errors

Tool: open-code-review (ocr) v1.12.13, `ocr review --from main --to pr-09-capture-errors`, model gpt-6-astra
Status: complete — files reviewed: 1, failed: 0

## 1. [high] [bug] `shop/payments.py:27-31`

A capture failure is swallowed, but execution continues to mark the order as `paid` and logs successful payment. This can leave orders paid without a gateway charge (and silently loses the failure); handle the error by recording a non-paid/failed or retryable state and logging it, while allowing checkout to continue.

Existing code:
```
    try:
        gateway.capture(order_id, amount_cents, card_token)
    except Exception:
        pass
    db.execute("update orders set status = 'paid' where id = ?", (order_id,))
```

# pr-10-reserve-check

Tool: open-code-review (ocr) v1.12.13, `ocr review --from main --to pr-10-reserve-check`, model gpt-6-astra
Status: complete — files reviewed: 1, failed: 0

## 1. [high] [bug] `shop/stock.py:19-21`

The comparison is inverted: this raises `OutOfStock` when inventory exceeds the requested quantity, while allowing reservations that exceed available stock. Please retain the insufficient-stock check (`in_stock < qty`) so valid reservations succeed and over-reservations are rejected.

Existing code:
```
    in_stock = available(sku)
    if in_stock > qty:
        raise OutOfStock(sku)
```

Suggested code:
```
    in_stock = available(sku)
    if in_stock < qty:
        raise OutOfStock(sku)
```

# pr-11-test-list-orders

Tool: open-code-review (ocr) v1.12.13, `ocr review --from main --to pr-11-test-list-orders`, model gpt-6-astra
Status: skipped — "Review skipped: no items were selected."

No findings.

Note: the only changed file in this branch, `tests/test_shop.py`, is excluded from review by ocr's built-in default path rules (`ocr review --preview` reports "Excluded from review: [M] tests/test_shop.py (default_path)"). So the tool never sent this diff to the model. "No findings" here means it was not reviewed, not that it was reviewed and found clean.

# pr-12-speed-up-tests

Tool: open-code-review (ocr) v1.12.13, `ocr review --from main --to pr-12-speed-up-tests`, model gpt-6-astra
Status: skipped — "Review skipped: no items were selected."

No findings.

Note: the only changed file in this branch, `tests/test_shop.py`, is excluded from review by ocr's built-in default path rules (`ocr review --preview` reports "Excluded from review: [M] tests/test_shop.py (default_path)"). So the tool never sent this diff to the model. "No findings" here means it was not reviewed, not that it was reviewed and found clean.