(SP: 3) [Backend/DevOps] Make shop janitor production-runnable; harden inventory idempotency, refunds, and concurrency invariants#118
Conversation
…ERNAL_JANITOR_SECRET, 503 disabled, clamp) and validate production run
…able (auth, DB rate limit, sweep policy + no-payment orphan cleanup)
…nal restock-stale endpoint + local runner script + scheduled workflow stub
…K_RESERVING_TIMEOUT) with exactly-once stock release + tests
… to prevent double-reserve and enforce inventory invariants
…d normalize orphan inventory to released
… state invariants
…dedupe events + restock exactly-once
…ucts API + add contract tests
liudmylasovetovs
requested review from
AM1007 and
ViktorSvertoka
as code owners
✅ Deploy Preview for develop-devlovers ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThis PR introduces a comprehensive order management system overhaul, adding periodic janitor-based restock automation, Stripe webhook refund handling with idempotency tracking, SALE badge pricing validation requiring originalPriceMinor, and order refund UI. Changes span GitHub Actions workflows, authentication, database schema, API endpoints, form validation, and extensive test coverage for concurrency and restock scenarios. Changes
Sequence Diagram(s)sequenceDiagram
participant GHA as GitHub Actions
participant Janitor as Janitor Endpoint
participant JobDB as Job State DB
participant OrderSvc as Order Service
participant OrdDB as Orders DB
participant InvDB as Inventory DB
GHA->>Janitor: POST /api/shop/internal/orders/restock-stale<br/>(with janitor secret, time budget)
Janitor->>JobDB: acquireJobSlot(jobName, minIntervalSeconds)
alt Rate Limit Active
JobDB-->>Janitor: false (rate limited)
Janitor-->>GHA: 429 Retry-After
else Slot Available
JobDB-->>Janitor: true, update nextAllowedAt
Janitor->>OrderSvc: restockStuckReserving(budgetMs)
OrderSvc->>OrdDB: find orders with reserving timeout
OrderSvc->>InvDB: release reserves for timeout orders
OrderSvc-->>Janitor: processedCount
Janitor->>OrderSvc: restockStalePending(budgetMs)
OrderSvc->>OrdDB: find orders with pending timeout
OrderSvc->>InvDB: update inventory status
OrderSvc-->>Janitor: processedCount
Janitor->>OrderSvc: restockStaleNoPayment(budgetMs)
OrderSvc->>OrdDB: find orders without payment timeout
OrderSvc->>InvDB: release reserves
OrderSvc-->>Janitor: processedCount
Janitor-->>GHA: 200 with aggregated counts
end
sequenceDiagram
participant Stripe as Stripe Platform
participant Webhook as Webhook Endpoint
participant EventDB as Stripe Events DB
participant OrdDB as Orders DB
participant OrderSvc as Order Service
Stripe->>Webhook: POST /api/shop/webhooks/stripe<br/>(charge.refunded or<br/>charge.refund.updated)
Webhook->>EventDB: check if event.id already processedAt
alt Event Already Processed
EventDB-->>Webhook: processedAt exists
Webhook-->>Stripe: 200 (idempotent ack)
else First Time
EventDB-->>Webhook: processedAt null
Webhook->>Webhook: extract paymentIntentId<br/>(from charge, refund, or PI)
Webhook->>OrdDB: resolve orderId from metadata<br/>or fallback via paymentIntentId
alt Order Not Found
Webhook->>EventDB: mark processedAt (ack)
Webhook-->>Stripe: 200 (ack to avoid replay)
else Order Found
Webhook->>Webhook: determine refund type<br/>(full vs. partial)
alt Full Refund
Webhook->>OrderSvc: refundOrder(orderId)
OrderSvc->>OrdDB: update paymentStatus→refunded<br/>update status→CANCELED
OrderSvc->>InvDB: restore stock
OrderSvc-->>Webhook: OrderSummaryWithMinor
else Partial Refund
Webhook->>OrdDB: log PARTIAL_REFUND_IGNORED
Webhook->>OrdDB: update metadata with decision
end
Webhook->>EventDB: mark processedAt (ack)
Webhook-->>Stripe: 200 (ack)
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/lib/services/orders.ts (1)
1111-1130: Non-English comment should be translated.Line 1111 contains a Russian comment: "иначе sweep будет подбирать снова". For consistency and maintainability, comments should be in English.
Suggested fix
- // Stripe (or any non-none provider): stale orphan must become terminal, иначе sweep будет подбирать снова. + // Stripe (or any non-none provider): stale orphan must become terminal, otherwise the sweep will keep picking it up.
🤖 Fix all issues with AI agents
In @frontend/app/[locale]/shop/admin/orders/[id]/RefundButton.tsx:
- Around line 16-40: The onRefund function currently doesn't handle fetch
rejections; wrap the fetch call in a try/catch so network errors are caught,
call setError with the caught error's message (or a fallback like 'Network
error') and return early, and then proceed with the existing response.json
parsing and res.ok handling; keep the existing error extraction from json and
the startTransition(router.refresh()) on success so behavior is preserved.
In @frontend/app/[locale]/shop/admin/products/_components/product-form.tsx:
- Around line 324-338: In the SALE_ORIGINAL_REQUIRED branch of the submit
handler, call setIsSubmitting(false) before returning so the form doesn't remain
stuck; specifically, just before the early return in the block that checks if
(data.code === 'SALE_ORIGINAL_REQUIRED'), invoke setIsSubmitting(false) (keeping
the existing setOriginalPriceErrors and setError calls) so submission state is
cleared regardless of this error.
In @frontend/app/api/shop/admin/products/[id]/route.ts:
- Around line 66-71: getSaleViolationFromFormData currently swallows JSON.parse
errors and returns null, making malformed prices indistinguishable from "no
violations"; change the catch around JSON.parse(pricesRaw) so it does not return
null silently but instead surfaces a clear parse failure (either by throwing a
descriptive error or returning an explicit validation error object) so callers
like parseAdminProductForm and findSaleRuleViolation can distinguish "invalid
JSON" from "no sale violations"; reference the getSaleViolationFromFormData
function, the JSON.parse(pricesRaw) call, and the use of findSaleRuleViolation
when implementing this behavior.
In @frontend/app/api/shop/webhooks/stripe/route.ts:
- Around line 606-624: The full-refund detection for 'charge.refund.updated' is
incorrect because it compares a single refund.amount to order.totalAmountMinor;
instead compute cumulative refunded for the associated charge and compare that
to the original charge amount. In the 'charge.refund.updated' branch (where
refund is available), locate the related charge object (from refund.charge or by
retrieving the charge) and determine the cumulative refunded amount (use
charge.amount_refunded if present, or sum existing charge.refunds.data amounts
plus the current refund.amount) and set isFullRefund when cumulativeRefunded ===
charge.amount (or === order.totalAmountMinor if you prefer using order totals).
Update the code that sets isFullRefund in the charge.refund.updated case to use
this cumulative comparison rather than just refund.amount.
In @frontend/lib/auth/internal-janitor.ts:
- Around line 5-10: The timingSafeEqual function leaks secret length via the
early length check; modify timingSafeEqual to avoid early return by allocating
two buffers of equal length (max of aBuf.length and bBuf.length), copy a and b
into those fixed-size buffers (padding the remainder with zeros), then call
crypto.timingSafeEqual on the padded buffers (still handling non-buffer inputs
as before) so the comparison time does not vary with input length.
In @frontend/lib/tests/restock-stuck-reserving-sweep.test.ts:
- Around line 167-178: The finally block currently throws cleanup errors which
will overwrite any prior test assertion failures; capture the original error
from the test body (e.g., declare a let originalError variable and assign it in
the try/catch around the test assertions), then in the finally call
cleanupTestRows({ orderId, productId }) inside a try/catch: if cleanup fails,
log the cleanup error (console.error with orderId/productId) and if
originalError exists rethrow originalError, otherwise throw the cleanup
error—this preserves the real test failure while still surfacing cleanup
problems.
In @frontend/project-structure.txt:
- Line 407: The project now contains a tmp directory that must be ignored by
git; open the .gitignore file and add an entry for the tmp directory (e.g., tmp/
or /tmp) so temporary artifacts aren’t committed, then if tmp was already
tracked remove it from the index (git rm --cached -r tmp) and commit the
.gitignore change; reference the tmp directory and the .gitignore file when
making the update.
In @frontend/scripts/shop-janitor-restock-stale.mjs:
- Line 17: The timeout parsing uses Number(process.env.JANITOR_TIMEOUT_MS) which
can yield NaN and make setTimeout fire immediately; validate and normalize it by
parsing and checking that the result is a finite positive integer: read
process.env.JANITOR_TIMEOUT_MS, parse to a number (e.g., parseInt/Number), check
isFinite(value) and value > 0, and if invalid fall back to the default (25000)
or a configured minimum; update the const timeoutMs declaration and any use of
timeoutMs (e.g., the setTimeout call) to use the validated/normalized value.
In @frontend/tmp/replay-charge-refunded.json:
- Line 1: The JSON file begins with a BOM character that can break parsers and
contains the placeholder string "<PUT_EVT_ID_FROM_DB_HERE>" that must be
populated before replay; remove the leading BOM from
frontend/tmp/replay-charge-refunded.json, reformat the JSON for readability, and
replace or document the need to replace the "<PUT_EVT_ID_FROM_DB_HERE>"
placeholder (add a short note in the replay script or README explaining how to
obtain and insert the actual event ID from the DB).
In @frontend/tmp/replay-stripe-webhook.js:
- Line 1: The file has a UTF-8 BOM at the start (before the "const fs =
require('fs');" statement); remove the BOM so the file begins with the "const"
token, re-save the file as UTF-8 without BOM (or run a tool/IDE option to strip
BOM), and verify Node.js can load the file and linters/CI no longer flag the
leading invisible character.
- Around line 12-14: The template literals for the signed payload and header are
malformed: build the signed payload as the timestamp, a dot, then the JSON
string payload (e.g., use payload = JSON.stringify({id: "evt_test"}) and signed
= `${ts}.${payload}`), compute sig with crypto.createHmac('sha256',
secret).update(signed, 'utf8').digest('hex'), and set the header string to
`t=${ts},v1=${sig}` (use the existing variables signed, sig, secret, ts, and
payload).
🧹 Nitpick comments (17)
frontend/.env.example (1)
40-40: Add trailing newline for POSIX compliance.Most text editors and POSIX standards expect files to end with a newline character.
📝 Proposed fix
JANITOR_URL= +frontend/app/api/shop/admin/products/[id]/route.ts (2)
27-56: Consider stronger typing forfindSaleRuleViolationinput.Using
anyfor the input parameter reduces type safety. Consider defining a minimal interface for the expected shape.♻️ Suggested typing improvement
+type SaleValidationInput = { + badge?: string | null; + prices?: Array<{ + currency?: string | null; + priceMinor?: number | null; + originalPriceMinor?: number | null; + }>; +}; + -function findSaleRuleViolation(input: any): SaleRuleViolation | null { +function findSaleRuleViolation(input: SaleValidationInput | null | undefined): SaleRuleViolation | null {
195-204: Casting toanyto access error properties is fragile.If
InvalidPayloadErroris expected to havecode,field, anddetailsproperties, consider extending the error class or using a type guard instead of casting.♻️ Suggested improvement
if (error instanceof InvalidPayloadError) { - const anyErr = error as any; + const errWithDetails = error as InvalidPayloadError & { + code?: string; + field?: string; + details?: unknown; + }; return NextResponse.json( { error: error.message || 'Invalid product data', - code: anyErr.code, - field: anyErr.field, - details: anyErr.details, + code: errWithDetails.code, + field: errWithDetails.field, + details: errWithDetails.details, }, { status: 400 } ); }frontend/app/[locale]/shop/admin/orders/[id]/page.tsx (2)
21-27: Redundant type assertion inorderCurrency.The ternary already produces
'UAH'or'USD'string literals, making theas CurrencyCodecast unnecessary ifCurrencyCodeis'UAH' | 'USD'.♻️ Minor simplification
function orderCurrency( order: { currency?: string | null } | null, locale: string ): CurrencyCode { const c = order?.currency ?? resolveCurrencyFromLocale(locale); - return (c === 'UAH' ? 'UAH' : 'USD') as CurrencyCode; + return c === 'UAH' ? 'UAH' : 'USD'; }
43-46: Inconsistent indentation incanRefundassignment.The condition body appears to have inconsistent indentation (continuation lines not indented relative to the assignment).
♻️ Formatting fix
const canRefund = - order.paymentProvider === 'stripe' && - order.paymentStatus === 'paid' && - !!order.paymentIntentId; + order.paymentProvider === 'stripe' && + order.paymentStatus === 'paid' && + !!order.paymentIntentId;frontend/drizzle/0013_add_internal_job_state.sql (1)
1-6: Schema design looks solid for rate-limiting use case.The table structure appropriately supports job state tracking and rate limiting:
job_nameas PRIMARY KEY ensures singleton semanticsnext_allowed_atNOT NULL enforces rate limit contractupdated_atwith default tracks mutation timestampsFor very small tables (few job types), the current design is sufficient. If the table grows or
next_allowed_atis queried frequently in hot paths, consider adding an index.📊 Optional: Add index if query patterns warrant
If rate limit checks become a hot path or the table grows beyond a few dozen rows:
CREATE INDEX "internal_job_state_next_allowed_at_idx" ON "internal_job_state" ("next_allowed_at");Monitor query performance before adding, as sequential scans are fast for small tables.
.github/workflows/shop-janitor-restock-stale.yml (1)
33-33: Translate comment to English for consistency.The comment "тут secrets дозволені" (Ukrainian for "secrets are allowed here") should be in English for maintainability and consistency with the rest of the codebase.
Proposed fix
- # Step-level guard: тут secrets дозволені + # Step-level guard: secrets are accessible herefrontend/lib/tests/restock-order-only-once.test.ts (2)
146-153: Empty catch blocks silently swallow cleanup errors.The cleanup logic uses empty
catch {}blocks which will silently ignore any cleanup failures. This can lead to test pollution if rows are not properly deleted. Consider logging the error or using a more robust cleanup strategy like the one inrestock-stuck-reserving-sweep.test.ts.Proposed improvement
} finally { try { await db.delete(orders).where(eq(orders.id, orderId)); - } catch {} + } catch (e) { + console.error('[test cleanup failed] orders', { orderId, error: e }); + } try { await db.delete(products).where(eq(products.id, productId)); - } catch {} + } catch (e) { + console.error('[test cleanup failed] products', { productId, error: e }); + } }The same pattern appears in the other two tests (lines 249-255 and 387-393).
258-394: Add explicit timeout to the third test case.Tests 1 and 2 have explicit
30_000timeouts, but the third test ("duplicate refund restock") lacks one. For consistency and to prevent unexpected timeout failures, add the timeout parameter.Proposed fix
- }); + }, 30_000); }, 30000);frontend/lib/tests/restock-stuck-reserving-sweep.test.ts (1)
11-23: Consider extracting shared test helpers to avoid duplication.The
readRowsandcountMoveKeyfunctions are duplicated inrestock-order-only-once.test.ts. Consider extracting these to a shared test utilities file.frontend/lib/tests/stripe-webhook-paid-status-repair.test.ts (1)
65-114: Consider adding test cleanup to prevent data pollution.The tests insert orders and stripe events but don't clean up after themselves. While this may work with isolated test databases, it could cause issues in shared test environments or if test order changes. Consider adding an
afterEachhook to remove test data.💡 Suggested cleanup pattern
+afterEach(async () => { + // Clean up test data + await db.delete(stripeEvents).where(eq(stripeEvents.eventId, eventId)); + await db.delete(orders).where(eq(orders.id, orderId)); +});Alternatively, track inserted IDs and clean them in the
afterEachhook similar to the pattern used instripe-webhook-refund-full.test.ts.frontend/lib/tests/stripe-webhook-refund-full.test.ts (2)
63-70: Consider using English comments for consistency.The comment on lines 64-65 is in Ukrainian. For codebase consistency and maintainability by international contributors, consider translating to English.
💬 Suggested translation
async function cleanupInserted(ins: Inserted) { - // stripeEvents вставляються навіть коли orderId null (metadata missing), - // тому чистимо по paymentIntentId + // stripeEvents are inserted even when orderId is null (metadata missing), + // so we clean up by paymentIntentId await db
269-270: Minor: Missing blank line before test case.Add a blank line between line 269 and 270 for consistent formatting with other test cases.
🔧 Format fix
expect(restockOrder).toHaveBeenCalledTimes(0); }); + it('retry after 500 must reprocess same event.id until processedAt is set (restock not lost)', async () => {frontend/lib/tests/checkout-concurrency-stock1.test.ts (2)
62-77: Consider translating Ukrainian comments to English.Comments on lines 63 and 66 are in Ukrainian. For consistency with the rest of the codebase, consider using English.
💬 Suggested translation
beforeAll(() => { - // зберегти оригінальні env значення тільки для stripe-ключів + // save original env values only for stripe keys for (const k of stripeKeys) originalEnv[k] = process.env[k]; - // тест має бути незалежним від Stripe — гасимо stripe env + // test should be independent of Stripe — disable stripe env for (const k of stripeKeys) delete process.env[k]; });
215-240: Overly defensive field lookups can be simplified.The
pick()andnormalizeMoveKind()patterns with multiple fallback field names are more defensive than necessary given the known Drizzle schema. TheinventoryMovestable clearly definestypeandquantitycolumns.🔧 Simplified assertions
const moves = await db .select() .from(inventoryMoves) - .where(eq((inventoryMoves as any).productId, productId)); + .where(eq(inventoryMoves.productId, productId)); - const reserveMoves = (moves as any[]).filter(m => { - const kind = normalizeMoveKind( - pick(m, ['kind', 'type', 'moveType', 'action', 'op']) - ); - return kind === 'reserve' || kind === 'reserved'; - }); + const reserveMoves = moves.filter(m => m.type === 'reserve'); - const reservedUnits = reserveMoves.reduce((sum, m) => { - const q = pick(m, [ - 'quantity', - 'qty', - 'units', - 'delta', - 'deltaQty', - 'deltaQuantity', - ]); - return sum + Math.abs(toNum(q)); - }, 0); + const reservedUnits = reserveMoves.reduce((sum, m) => sum + m.quantity, 0);frontend/app/api/shop/internal/orders/restock-stale/route.ts (1)
424-427: Consider translating Ukrainian comment to English.💬 Suggested translation
} catch (e) { - // не ковтай: але без твого логера — мінімально + // don't swallow: but without your logger — minimal logging console.error('restock-stale failed', { runId, error: e });frontend/lib/services/orders.ts (1)
1333-1468: LGTM! Handles edge case of stuck reservations.The
restockStuckReservingOrdersfunction properly targets orders that crashed mid-reservation (inventoryStatusin['reserving', 'release_pending']). The use ofCOALESCEforfailureCode/failureMessage(lines 1436-1437) correctly preserves any existing error context while providing a default.Consider extracting shared constants.
The parameter bounds (
MIN_OLDER_MIN,MAX_BATCH, etc.) are duplicated acrossrestockStalePendingOrders,restockStuckReservingOrders, andrestockStaleNoPaymentOrders. Consider extracting to module-level constants for easier maintenance.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (40)
.github/workflows/shop-janitor-restock-stale.ymlfrontend/.env.examplefrontend/app/[locale]/shop/admin/orders/[id]/RefundButton.tsxfrontend/app/[locale]/shop/admin/orders/[id]/page.tsxfrontend/app/[locale]/shop/admin/products/_components/product-form.tsxfrontend/app/api/shop/admin/products/[id]/route.tsfrontend/app/api/shop/admin/products/route.tsfrontend/app/api/shop/internal/orders/restock-stale/route.tsfrontend/app/api/shop/webhooks/stripe/route.tsfrontend/components/header/UnifiedHeader.tsxfrontend/db/queries/shop/products.tsfrontend/db/schema/shop.tsfrontend/drizzle/0013_add_internal_job_state.sqlfrontend/drizzle/0014_add-stripe-events-processed-at.sqlfrontend/drizzle/0015_warm_dexter_bennett.sqlfrontend/drizzle/meta/0013_snapshot.jsonfrontend/drizzle/meta/0014_snapshot.jsonfrontend/drizzle/meta/_journal.jsonfrontend/lib/admin/parseAdminProductForm.tsfrontend/lib/auth/internal-janitor.tsfrontend/lib/services/orders.tsfrontend/lib/services/products.tsfrontend/lib/tests/admin-product-sale-contract.test.tsfrontend/lib/tests/checkout-concurrency-stock1.test.tsfrontend/lib/tests/checkout-no-payments.test.tsfrontend/lib/tests/order-items-snapshot-immutable.test.tsfrontend/lib/tests/product-sale-invariant.test.tsfrontend/lib/tests/restock-order-only-once.test.tsfrontend/lib/tests/restock-stale-stripe-orphan.test.tsfrontend/lib/tests/restock-stuck-reserving-sweep.test.tsfrontend/lib/tests/stripe-webhook-mismatch.test.tsfrontend/lib/tests/stripe-webhook-paid-status-repair.test.tsfrontend/lib/tests/stripe-webhook-psp-fields.test.tsfrontend/lib/tests/stripe-webhook-refund-full.test.tsfrontend/lib/types/shop.tsfrontend/lib/validation/shop.tsfrontend/project-structure.txtfrontend/scripts/shop-janitor-restock-stale.mjsfrontend/tmp/replay-charge-refunded.jsonfrontend/tmp/replay-stripe-webhook.js
💤 Files with no reviewable changes (1)
- frontend/drizzle/meta/_journal.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-30T16:01:02.523Z
Learnt from: kryvosheyin
Repo: DevLoversTeam/devlovers.net PR: 88
File: frontend/drizzle/schema.ts:153-153
Timestamp: 2025-12-30T16:01:02.523Z
Learning: The file frontend/drizzle/schema.ts is a legacy file not used by Drizzle or the runtime. The canonical schemas live in frontend/db/schema/* as configured in drizzle-config.ts, and that's what migrations and runtime use.
Applied to files:
frontend/drizzle/meta/0014_snapshot.json
🧬 Code graph analysis (17)
frontend/app/[locale]/shop/admin/orders/[id]/RefundButton.tsx (1)
frontend/scripts/shop-janitor-restock-stale.mjs (1)
res(23-33)
frontend/lib/tests/checkout-concurrency-stock1.test.ts (2)
frontend/db/index.ts (1)
db(39-39)frontend/db/schema/shop.ts (1)
inventoryMoves(334-363)
frontend/app/api/shop/admin/products/route.ts (1)
frontend/drizzle/schema.ts (1)
currency(7-7)
frontend/lib/services/products.ts (3)
frontend/lib/services/errors.ts (1)
InvalidPayloadError(35-41)frontend/db/schema/shop.ts (1)
productPrices(278-332)frontend/lib/shop/currency.ts (1)
CurrencyCode(2-2)
frontend/lib/tests/stripe-webhook-refund-full.test.ts (4)
frontend/tmp/replay-stripe-webhook.js (1)
crypto(2-2)frontend/db/schema/shop.ts (2)
orders(107-198)stripeEvents(262-276)frontend/lib/psp/stripe.ts (1)
verifyWebhookSignature(97-122)frontend/app/api/shop/webhooks/stripe/route.ts (1)
POST(115-707)
frontend/app/api/shop/admin/products/[id]/route.ts (1)
frontend/drizzle/schema.ts (1)
currency(7-7)
frontend/lib/types/shop.ts (2)
frontend/lib/validation/shop.ts (1)
OrderSummary(432-432)frontend/db/queries/shop/orders.ts (1)
OrderSummary(33-33)
frontend/app/[locale]/shop/admin/products/_components/product-form.tsx (3)
frontend/lib/validation/shop.ts (2)
CurrencyCode(32-32)ProductAdminInput(433-433)frontend/lib/shop/currency.ts (1)
CurrencyCode(2-2)frontend/drizzle/schema.ts (1)
currency(7-7)
frontend/lib/tests/restock-stuck-reserving-sweep.test.ts (4)
frontend/db/index.ts (1)
db(39-39)frontend/lib/shop/money.ts (1)
toDbMoney(58-60)frontend/lib/services/inventory.ts (1)
applyReserveMove(49-116)frontend/lib/services/orders.ts (1)
restockStuckReservingOrders(1334-1468)
frontend/app/api/shop/webhooks/stripe/route.ts (3)
frontend/db/index.ts (1)
db(39-39)frontend/db/schema/shop.ts (2)
stripeEvents(262-276)orders(107-198)frontend/lib/services/orders.ts (2)
restockOrder(1039-1197)restockOrder(1640-1640)
frontend/lib/tests/product-sale-invariant.test.ts (1)
frontend/lib/shop/money.ts (1)
toDbMoney(58-60)
frontend/lib/tests/admin-product-sale-contract.test.ts (2)
frontend/app/api/shop/admin/products/route.ts (1)
POST(68-193)frontend/app/api/shop/admin/products/[id]/route.ts (1)
PATCH(115-257)
frontend/lib/tests/restock-order-only-once.test.ts (4)
frontend/db/schema/shop.ts (2)
products(56-105)orders(107-198)frontend/lib/shop/money.ts (1)
toDbMoney(58-60)frontend/lib/services/inventory.ts (1)
applyReserveMove(49-116)frontend/lib/services/orders.ts (2)
restockOrder(1039-1197)restockOrder(1640-1640)
frontend/lib/services/orders.ts (3)
frontend/lib/types/shop.ts (2)
OrderSummaryWithMinor(32-35)PaymentStatus(44-44)frontend/db/schema/shop.ts (1)
orders(107-198)frontend/lib/shop/payments.ts (1)
PaymentStatus(9-9)
frontend/lib/auth/internal-janitor.ts (1)
frontend/tmp/replay-stripe-webhook.js (1)
crypto(2-2)
frontend/lib/admin/parseAdminProductForm.ts (1)
frontend/drizzle/schema.ts (1)
currency(7-7)
frontend/app/[locale]/shop/admin/orders/[id]/page.tsx (4)
frontend/app/[locale]/layout.tsx (1)
dynamic(17-17)frontend/lib/shop/money.ts (1)
fromDbMoney(51-56)frontend/lib/shop/currency.ts (3)
CurrencyCode(2-2)resolveCurrencyFromLocale(23-28)formatMoney(115-128)frontend/app/[locale]/shop/admin/orders/[id]/RefundButton.tsx (1)
RefundButton(11-59)
🪛 Biome (2.1.2)
frontend/lib/tests/restock-stuck-reserving-sweep.test.ts
[error] 177-178: Unsafe usage of 'throw'.
'throw' in 'finally' overwrites the control flow statements inside 'try' and 'catch'.
(lint/correctness/noUnsafeFinally)
frontend/tmp/replay-stripe-webhook.js
[error] 12-12: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 12-12: Expected a statement but instead found '.'.
Expected a statement here.
(parse)
[error] 12-12: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 14-14: Expected an expression, or an assignment but instead found '='.
Expected an expression, or an assignment here.
(parse)
🪛 dotenv-linter (4.0.0)
frontend/.env.example
[warning] 39-39: [UnorderedKey] The INTERNAL_JANITOR_MIN_INTERVAL_SECONDS key should go before the INTERNAL_JANITOR_SECRET key
(UnorderedKey)
[warning] 40-40: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
🪛 Gitleaks (8.30.0)
frontend/.env.example
[high] 38-39: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (72)
frontend/components/header/UnifiedHeader.tsx (1)
74-84: LGTM! Clean conditional rendering with proper accessibility.The Home link implementation is solid—proper aria-label for screen readers, placeholder div prevents layout shift between variants, and
flex-shrink-0ensures the icon doesn't collapse under flex constraints. The positioning in the right action cluster is logical alongside other utility buttons.frontend/.env.example (1)
38-40: LGTM! New janitor environment variables are well-structured.The additions support the internal janitor authentication and rate limiting as described in the PR objectives.
Note: The Gitleaks warning about
INTERNAL_JANITOR_SECRETis a false positive—this is an example file with placeholder values, not actual secrets.frontend/lib/tests/order-items-snapshot-immutable.test.ts (1)
213-213: LGTM! Timeout increase aligns with integration test requirements.The 30-second timeout is appropriate for end-to-end tests that involve database operations, product mutations, and snapshot validation.
frontend/lib/tests/checkout-no-payments.test.ts (2)
446-477: LGTM! Test data updated to reflect new sweep policy thresholds.The changes correctly align with the PR's updated janitor sweep behavior:
- Orphan created 11 minutes ago (exceeds the 10-minute threshold)
inventoryStatus: 'reserving'accurately represents a stuck-in-progress stateolderThanMinutes: 10matches the configured thresholdThese adjustments ensure the test validates the production sweep behavior.
505-505: LGTM! Timeout appropriate for integration test scope.The 20-second timeout accommodates the orphan cleanup path's database operations and restock logic.
frontend/lib/validation/shop.ts (2)
246-257: LGTM! SALE badge validation enforces pricing contract.The validation correctly implements the requirement that SALE products must include
originalPriceMinorfor all currencies. This ensures the UI can display the discount comparison.The error messages are clear and the validation is positioned after duplicate currency and USD requirement checks, maintaining a logical validation order.
297-307: LGTM! Update schema mirrors create schema validation.The SALE badge validation in
productAdminUpdateSchemacorrectly mirrors the create schema logic, ensuring consistent behavior across both operations.frontend/lib/admin/parseAdminProductForm.ts (3)
29-36: LGTM! Enhanced type handling inparseBooleanField.Adding support for boolean type values (line 35) improves robustness when FormData contains actual boolean values instead of just strings.
165-169: LGTM! More specific error message improves debugging.The new error message
'Invalid prices payload type'clearly indicates the issue when the prices field is not a string, making debugging easier compared to a generic error.
15-23: LGTM! Formatting improvements enhance readability.The multiline function signatures and reformatted objects improve code readability without changing behavior. The style is consistent throughout the file.
Also applies to: 25-37, 39-46, 98-105, 148-157, 181-191, 197-199, 265-268, 296-305
frontend/lib/tests/product-sale-invariant.test.ts (4)
1-17: LGTM! Clean test setup with Cloudinary mocks.The mock setup properly isolates external dependencies and provides deterministic test data.
19-31: LGTM! Proper test isolation with cleanup.The unique slug helper and afterEach cleanup ensure tests remain isolated without polluting the database.
33-50: LGTM! Test correctly validates SALE invariant on create.The test properly asserts that creating a SALE product without originalPriceMinor is rejected with the expected error message.
52-116: LGTM! Comprehensive test for SALE invariant on update.The test properly seeds a valid SALE product, attempts an invalid update (removing originalPriceMinor), and verifies both rejection and database integrity. The post-failure verification query is a nice defensive check.
frontend/app/[locale]/shop/admin/orders/[id]/RefundButton.tsx (1)
42-58: LGTM! Well-structured UI with proper loading and error states.The button correctly handles disabled states, provides loading feedback, and displays inline errors effectively.
frontend/lib/services/products.ts (3)
338-356: LGTM! SALE invariant helper is well-structured.The helper correctly validates that SALE badges require originalPriceMinor for each currency and that it exceeds priceMinor. The defensive check at Line 350-354 provides additional safety, even though
validatePriceRowsalready validates this constraint (lines 329-333).
378-382: LGTM! SALE validation properly integrated into createProduct.The validation sequence correctly enforces SALE invariants before any database operations, ensuring data integrity at the service boundary.
495-532: LGTM! Comprehensive SALE validation for update operations.The update flow correctly computes the final badge state and merges existing + incoming prices to validate the complete post-update state. This ensures SALE invariants are preserved across partial updates (PATCH operations).
frontend/app/api/shop/admin/products/route.ts (4)
15-49: LGTM! Well-structured SALE validation helpers.The
SaleRuleViolationtype andfindSaleRuleViolationhelper provide clear, structured error information for API responses, enabling client-side field-specific error handling.
51-66: LGTM! Pre-parse validation provides early feedback.The helper extracts and validates SALE pricing rules before full form parsing, enabling faster error responses for invalid SALE data.
84-125: LGTM! Consistent SALE validation with defense in depth.The dual validation approach (pre-parse and post-parse) ensures SALE pricing rules are enforced early while maintaining robustness. Error responses provide structured details for client-side field mapping.
142-152: LGTM! Enhanced error handling with richer context.The improved InvalidPayloadError handling extracts additional error context (code, field, details) for more informative API responses, supporting better client-side error handling.
frontend/lib/tests/admin-product-sale-contract.test.ts (4)
1-58: LGTM! Comprehensive mock setup for route isolation.The mock setup properly isolates API route behavior by mocking authentication, form parsing, and product services. Helper functions provide clean test data construction.
60-67: LGTM! Proper test isolation with beforeEach.The beforeEach hook correctly resets all mocks to ensure tests remain independent.
69-108: LGTM! Comprehensive contract test for POST with SALE validation.The test properly validates that the POST endpoint rejects SALE products missing originalPriceMinor, returns structured error information, and prevents the invalid product from being created.
110-157: LGTM! Comprehensive contract test for PATCH with SALE validation.The test properly validates that the PATCH endpoint rejects SALE products where originalPriceMinor is not greater than priceMinor, returns structured error information, and prevents the invalid update.
frontend/app/api/shop/admin/products/[id]/route.ts (1)
133-176: Dual SALE validation (pre-parse and post-parse) appears intentional but adds complexity.The code validates SALE rules twice: once from raw FormData (lines 134-150) and again from parsed data (lines 160-176). If
parseAdminProductFormtransforms the data significantly, this defense-in-depth approach is reasonable. Otherwise, consider consolidating to reduce duplication.frontend/app/[locale]/shop/admin/orders/[id]/page.tsx (1)
59-66: Refund eligibility and UI integration look correct.The
canRefundcheck appropriately gates the refund action to Stripe orders that are paid and have a payment intent ID. TheRefundButtoncomponent receives proper props with disabled state based on eligibility.frontend/app/[locale]/shop/admin/products/_components/product-form.tsx (2)
206-212: Good UX: Clearing per-currency errors when the user edits the field.This provides immediate feedback that the user's correction is being acknowledged.
700-707: Good defensive behavior: clearing SALE errors when badge changes.When the user switches away from the SALE badge, clearing
originalPriceErrorsprevents stale validation messages from persisting.frontend/project-structure.txt (1)
344-362: Comprehensive test coverage additions for critical paths.The new test files cover:
- Admin API killswitch
- Checkout concurrency (stock=1 scenario)
- Order item immutability
- Restock idempotency and sweep claim logic
- Stripe webhook contract, mismatch, PSP fields, paid-status repair, and refunds
This aligns well with the PR's goal of hardening inventory idempotency and concurrency invariants.
frontend/db/queries/shop/products.ts (2)
218-219: Safe handling of count result type.The bigint check ensures compatibility with different database drivers that may return count as bigint or number.
144-152: Sale detection logic change: now based onoriginalPriceMinorpresence.The sale category filter now checks
originalPriceMinor IS NOT NULLinstead of checking the badge. This aligns with the new SALE validation invariant that requiresoriginalPriceMinor > priceMinorfor SALE badge products.However, note that badge display has fallback logic: products with an explicit
badge = 'SALE'will still display the SALE badge even withoutoriginalPriceMinor(via the direct badge check inderiveBadge). Legacy data with SALE badge but missingoriginalPriceMinorwon't appear in the sale category filter query, though they'll still display the badge elsewhere. New products cannot have this mismatch due to validation enforcement.frontend/lib/tests/stripe-webhook-psp-fields.test.ts (1)
56-263: Well-structured integration test for PSP field persistence and idempotency.The test:
- Seeds realistic test data (product, price, order, items)
- Verifies PSP fields are written on
payment_intent.succeeded- Confirms idempotency by replaying the same event
- Properly cleans up in a
finallyblockThe 30-second timeout is appropriate for this database-heavy integration test.
frontend/lib/tests/stripe-webhook-mismatch.test.ts (1)
55-220: Critical security test: verifying mismatch detection prevents incorrect payment acceptance.This test validates that when Stripe reports a different amount than expected, the order is NOT marked as paid and the mismatch details are recorded in
pspStatusReasonandpspMetadata. This is essential for preventing revenue loss or fraud scenarios.The test correctly:
- Creates a real order flow
- Simulates a webhook with
amount + 1mismatch- Asserts
paymentStatus !== 'paid'- Verifies audit trail in metadata
frontend/lib/tests/restock-stale-stripe-orphan.test.ts (1)
11-11: LGTM! Test description and assertion are consistent.The updated test correctly reflects the new behavior where stale orphan orders (no inventory reservations) are marked with
inventoryStatus: 'released'instead of'failed'. This semantic distinction makes sense since there's no inventory to fail—it was never reserved.The description update ("and releases") accurately documents this behavior change.
Also applies to: 72-72
frontend/scripts/shop-janitor-restock-stale.mjs (2)
23-33: LGTM! Dual authentication headers and safe request configuration.The fetch configuration correctly:
- Sends both header variants (
x-internal-janitor-secretandBearer) as documented- Uses
'{}'body to avoid NextRequest.json() parse errors- Implements timeout via AbortController signal
- Sets appropriate content-type header
39-41: Correctly treats rate-limit response as success for schedulers.Treating HTTP 429 as success (exit code 0) is appropriate for scheduled jobs that may overlap. This prevents alerting on expected rate-limiting and aligns with the PR's DB-backed rate limiter design.
frontend/drizzle/0014_add-stripe-events-processed-at.sql (1)
1-1: Migration and idempotency logic alignment confirmed.The nullable
processed_atcolumn is correctly used in the webhook handler:
- New events inserted with
processedAt: null(unprocessed state)- After successful processing,
processedAtis set tonew Date()- Idempotency check at line 258 (
if (existing?.processedAt)) correctly distinguishes between NULL (unprocessed, allowing reprocessing) and timestamp values (already processed, skipped)The migration safely aligns with the webhook logic.
frontend/lib/types/shop.ts (1)
32-42: LGTM! Clean type extension for canonical minor-unit money model.The new
OrderSummaryWithMinortype and updatedCheckoutResultproperly introduce the canonicaltotalAmountMinorfield while maintaining backward compatibility via the legacytotalCents. The comment on line 40 clearly documents the rationale..github/workflows/shop-janitor-restock-stale.yml (1)
1-53: LGTM! Well-structured workflow with proper safeguards.The workflow correctly:
- Uses
cancel-in-progress: falseto ensure jobs complete- Has a 3-minute timeout that accommodates the 25s janitor timeout
- Gracefully skips when secrets are missing (emits warning instead of failing)
- Uses minimal
contents: readpermissionsfrontend/db/schema/shop.ts (3)
365-372: Consider adding$onUpdatetoupdatedAtfor consistency.Other tables in this schema (e.g.,
products,orders,productPrices) use$onUpdate(() => new Date())on theirupdatedAtfields. TheinternalJobStatetable lacks this, which meansupdatedAtwill only be set on insert unless explicitly updated in queries.This may be intentional if you want explicit control over updates, but verify this is the desired behavior.
272-272: LGTM!processedAtfield enables idempotent webhook handling.The new
processedAttimestamp onstripeEventssupports exactly-once processing semantics for webhook deduplication.
378-378: LGTM! Type export aligns with new schema.frontend/lib/tests/restock-order-only-once.test.ts (2)
25-154: LGTM! Comprehensive idempotency test for failed order restock.The test thoroughly validates:
- Reserve move decrements stock correctly
- First restock restores stock and sets
stockRestored/restockedAt- Second restock is a no-op (stock unchanged, timestamps preserved, single move key)
156-256: LGTM! Good concurrent restock test usingPromise.all.The test correctly verifies that two concurrent
restockOrdercalls result in exactly one stock restoration and one inventory move record, validating the concurrency guards.frontend/lib/tests/restock-stuck-reserving-sweep.test.ts (2)
25-38: Good cleanup helper that respects FK constraints.The
cleanupTestRowsfunction properly deletes child records (inventory_moves,order_items) before parent records, avoiding FK constraint violations. This is better than the cleanup pattern in the other test file.
40-166: LGTM! Thorough test for stuck-reserving sweep with exactly-once semantics.The test comprehensively validates:
- Stuck order is picked up by sweep and transitioned to terminal state
failureCodeis set toSTUCK_RESERVING_TIMEOUT- Stock is restored exactly once
- Second sweep is a no-op (0 processed, same
restockedAt)frontend/lib/auth/internal-janitor.ts (1)
12-43: LGTM! Auth flow is well-structured with appropriate HTTP status codes.The function correctly:
- Returns 503 when janitor is disabled (missing secret)
- Returns 401 for missing credentials
- Returns 403 for invalid credentials
- Returns
nullto signal success (allowing request to proceed)The fallback header
x-internal-secretprovides flexibility for callers.frontend/drizzle/meta/0013_snapshot.json (1)
1-2220: Auto-generated Drizzle snapshot file.This is a Drizzle ORM metadata snapshot that reflects schema state. The notable change is the
order_items_order_product_uqunique constraint which now covers only(order_id, product_id)without variant columns. Ensure this aligns with the intended business logic where a product can only appear once per order regardless of size/color selection.frontend/lib/tests/stripe-webhook-paid-status-repair.test.ts (1)
28-63: Well-structured webhook test helper with proper mocking.The
callWebhookhelper correctly usesvi.resetModules()beforevi.doMock()to ensure fresh module state for each call, which is essential for testing idempotency scenarios. The mock returns a syntheticpayment_intent.succeededevent with the necessary metadata.frontend/lib/tests/stripe-webhook-refund-full.test.ts (1)
72-168: Comprehensive refund webhook test with proper deduplication verification.This test thoroughly validates the full refund flow via
charge.refunded: PI fallback resolution, terminal status transition toCANCELED, stock restoration, and deduplication by eventId. The assertions correctly verify thatrestockOrderis called exactly once across duplicate webhook deliveries.frontend/lib/tests/checkout-concurrency-stock1.test.ts (1)
149-168: Well-implemented concurrent execution pattern.The gate pattern using a Promise with externally-resolved
release()function ensures both checkout requests start simultaneously, providing a proper race condition test. This is the correct approach for testing concurrency invariants.frontend/drizzle/meta/0014_snapshot.json (2)
1182-1219: Newinternal_job_statetable for cross-instance job coordination.This table provides the foundation for the DB-backed rate limiter used by the janitor endpoint. The schema correctly uses
job_nameas the primary key for efficient upsert operations with atomic conflict handling.
2166-2171:processed_atcolumn enables idempotent webhook handling.The nullable
processed_attimestamp onstripe_eventsallows distinguishing between: (1) never seen, (2) seen but failed (processedAt=NULL), and (3) successfully processed. This three-state model supports proper retry semantics.frontend/app/api/shop/internal/orders/restock-stale/route.ts (3)
223-260: Well-implemented DB-backed rate limiter with atomic semantics.The
acquireJobSlotfunction correctly uses PostgreSQL'sINSERT...ON CONFLICT DO UPDATE WHEREpattern for atomic cross-instance coordination. The two-query approach (attempt insert/update, then select if rate-limited) is the right pattern sinceON CONFLICT DO UPDATE WHEREdoesn't return rows when the WHERE condition fails.
199-208: Test environment bypass for rate limiter is appropriate.Returning
0forminIntervalSecondsin test environment prevents test flakes from persistent DB state affecting test isolation. This is a pragmatic approach for test reliability.
373-407: Sequential sweep execution with time budgets.The three sweep categories (stuckReserving, stalePending, orphanNoPayment) execute sequentially with decreasing time budgets. This ensures the endpoint respects
maxRuntimeMsacross all operations. The aggregation inprocessedByCategoryprovides good observability.frontend/app/api/shop/webhooks/stripe/route.ts (4)
229-235: Centralizedack()helper improves maintainability.The
ack()function ensuresprocessedAtis always set before returning success, preventing duplicate processing on retries. This is a clean pattern for idempotent webhook handling.
268-294: PI fallback order resolution handles missing metadata gracefully.When
metadata.orderIdis missing (e.g., refunds triggered externally), the fallback queries orders bypaymentIntentId. Limiting to 2 results correctly detects ambiguous cases (multiple orders for same PI) and logs appropriately before acknowledging.
440-468: Repair path handles paid status inconsistencies.The repair update at lines 443-468 fixes edge cases where
paymentStatus='paid'butstatus!='PAID'(e.g., from a previous partial failure or race condition). The WHERE conditions ensure this only applies when safe (not restocked, not released).
664-692: Full refund flow correctly transitions to terminal state.The full refund handling sets
paymentStatus='refunded'andstatus='CANCELED', then callsrestockOrderwithreason='refunded'. The WHERE clause withne(orders.paymentStatus, 'refunded')prevents duplicate processing.frontend/lib/services/orders.ts (10)
107-115: LGTM! Type migration toOrderSummaryWithMinoris correct.The
requireTotalCentshelper now properly validates the canonicaltotalAmountMinorfield, ensuring it's a finite number before use. This aligns with the minor-unit money model.
211-305: LGTM! Robust parsing with legacy fallback.The function properly handles the migration path by falling back to legacy money fields when minor-unit fields are null/undefined, while validating data integrity with
requireMinorandreadLegacyMoneyCentsOrThrow. The consistency check forpaymentProvider=nonewithpaymentIntentIdset is a good invariant guard.
307-309: LGTM! Return type properly updated.The
reconcileNoPaymentOrderfunction correctly returnsOrderSummaryWithMinorand maintains proper inventory reconciliation logic with appropriate error handling and rollback semantics.
453-456: LGTM!Return type correctly updated to
OrderSummaryWithMinor | null.
610-610: LGTM!Parameter type correctly updated to
OrderSummaryWithMinorfor idempotency validation.
930-942: LGTM!Return types for
getOrderSummaryandsetOrderPaymentIntentcorrectly updated toOrderSummaryWithMinor.
1199-1246: LGTM! Good production hardening with parameter bounds.The parameter validation with min/max clamping (e.g.,
MIN_OLDER_MIN=10,MAX_BATCH=100,MAX_TIME_BUDGET_MS=25_000) prevents misconfiguration and abuse. The time budget mechanism ensures sweeps don't run indefinitely.
1264-1287: LGTM! Proper separation of payment provider sweeps.Filtering by
paymentProvider='stripe'ensures this sweep only handles Stripe orders, while no-payment orders are handled byrestockStaleNoPaymentOrders. The conditional age cutoff (!hasExplicitIds) allows targeted processing in tests.
1470-1590: LGTM! Proper handling of no-payment order sweeps.The function correctly targets
paymentProvider='none'orders with incomplete inventory states (['none', 'reserving', 'release_pending']). This separation from Stripe sweeps ensures each payment workflow has its own cleanup logic aligned with its specific constraints (e.g., DB CHECK forcingpayment_statusforprovider='none').
1592-1638: LGTM! Refund flow correctly updated.Return type properly changed to
OrderSummaryWithMinor. The refund logic correctly validates the order state and callsrestockOrderwithreason: 'refunded'to ensure proper inventory restoration.
| async function onRefund() { | ||
| setError(null); | ||
|
|
||
| const res = await fetch(`/api/shop/admin/orders/${orderId}/refund`, { | ||
| method: 'POST', | ||
| credentials: 'same-origin', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| }); | ||
|
|
||
| let json: any = null; | ||
| try { | ||
| json = await res.json(); | ||
| } catch { | ||
| // ignore | ||
| } | ||
|
|
||
| if (!res.ok) { | ||
| setError(json?.error ?? json?.code ?? `HTTP_${res.status}`); | ||
| return; | ||
| } | ||
|
|
||
| startTransition(() => { | ||
| router.refresh(); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Add error handling for network failures.
The onRefund function does not catch network errors (e.g., fetch rejection due to network issues). If the fetch fails, the error will be unhandled and not displayed to the user.
🛡️ Proposed fix
async function onRefund() {
setError(null);
- const res = await fetch(`/api/shop/admin/orders/${orderId}/refund`, {
- method: 'POST',
- credentials: 'same-origin',
- headers: { 'Content-Type': 'application/json' },
- });
+ let res: Response;
+ try {
+ res = await fetch(`/api/shop/admin/orders/${orderId}/refund`, {
+ method: 'POST',
+ credentials: 'same-origin',
+ headers: { 'Content-Type': 'application/json' },
+ });
+ } catch (err) {
+ setError('NETWORK_ERROR');
+ return;
+ }
let json: any = null;
try {
json = await res.json();
} catch {
// ignore
}
if (!res.ok) {
setError(json?.error ?? json?.code ?? `HTTP_${res.status}`);
return;
}
startTransition(() => {
router.refresh();
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async function onRefund() { | |
| setError(null); | |
| const res = await fetch(`/api/shop/admin/orders/${orderId}/refund`, { | |
| method: 'POST', | |
| credentials: 'same-origin', | |
| headers: { 'Content-Type': 'application/json' }, | |
| }); | |
| let json: any = null; | |
| try { | |
| json = await res.json(); | |
| } catch { | |
| // ignore | |
| } | |
| if (!res.ok) { | |
| setError(json?.error ?? json?.code ?? `HTTP_${res.status}`); | |
| return; | |
| } | |
| startTransition(() => { | |
| router.refresh(); | |
| }); | |
| } | |
| async function onRefund() { | |
| setError(null); | |
| let res: Response; | |
| try { | |
| res = await fetch(`/api/shop/admin/orders/${orderId}/refund`, { | |
| method: 'POST', | |
| credentials: 'same-origin', | |
| headers: { 'Content-Type': 'application/json' }, | |
| }); | |
| } catch (err) { | |
| setError('NETWORK_ERROR'); | |
| return; | |
| } | |
| let json: any = null; | |
| try { | |
| json = await res.json(); | |
| } catch { | |
| // ignore | |
| } | |
| if (!res.ok) { | |
| setError(json?.error ?? json?.code ?? `HTTP_${res.status}`); | |
| return; | |
| } | |
| startTransition(() => { | |
| router.refresh(); | |
| }); | |
| } |
🤖 Prompt for AI Agents
In @frontend/app/[locale]/shop/admin/orders/[id]/RefundButton.tsx around lines
16 - 40, The onRefund function currently doesn't handle fetch rejections; wrap
the fetch call in a try/catch so network errors are caught, call setError with
the caught error's message (or a fallback like 'Network error') and return
early, and then proceed with the existing response.json parsing and res.ok
handling; keep the existing error extraction from json and the
startTransition(router.refresh()) on success so behavior is preserved.
| if (data.code === 'SALE_ORIGINAL_REQUIRED') { | ||
| const details = data.details as SaleRuleDetails | undefined; | ||
| const currency = details?.currency; | ||
| const msg = | ||
| details?.rule === 'greater_than_price' | ||
| ? SALE_GREATER_MSG | ||
| : SALE_REQUIRED_MSG; | ||
|
|
||
| if (currency === 'USD' || currency === 'UAH') { | ||
| setOriginalPriceErrors(prev => ({ ...prev, [currency]: msg })); | ||
| } | ||
|
|
||
| setError(data.error ?? msg); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Missing setIsSubmitting(false) before early return on SALE error.
When a SALE_ORIGINAL_REQUIRED error is returned from the API, the function returns early without resetting isSubmitting, leaving the form in a stuck "submitting" state.
🐛 Proposed fix
if (data.code === 'SALE_ORIGINAL_REQUIRED') {
const details = data.details as SaleRuleDetails | undefined;
const currency = details?.currency;
const msg =
details?.rule === 'greater_than_price'
? SALE_GREATER_MSG
: SALE_REQUIRED_MSG;
if (currency === 'USD' || currency === 'UAH') {
setOriginalPriceErrors(prev => ({ ...prev, [currency]: msg }));
}
setError(data.error ?? msg);
+ setIsSubmitting(false);
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (data.code === 'SALE_ORIGINAL_REQUIRED') { | |
| const details = data.details as SaleRuleDetails | undefined; | |
| const currency = details?.currency; | |
| const msg = | |
| details?.rule === 'greater_than_price' | |
| ? SALE_GREATER_MSG | |
| : SALE_REQUIRED_MSG; | |
| if (currency === 'USD' || currency === 'UAH') { | |
| setOriginalPriceErrors(prev => ({ ...prev, [currency]: msg })); | |
| } | |
| setError(data.error ?? msg); | |
| return; | |
| } | |
| if (data.code === 'SALE_ORIGINAL_REQUIRED') { | |
| const details = data.details as SaleRuleDetails | undefined; | |
| const currency = details?.currency; | |
| const msg = | |
| details?.rule === 'greater_than_price' | |
| ? SALE_GREATER_MSG | |
| : SALE_REQUIRED_MSG; | |
| if (currency === 'USD' || currency === 'UAH') { | |
| setOriginalPriceErrors(prev => ({ ...prev, [currency]: msg })); | |
| } | |
| setError(data.error ?? msg); | |
| setIsSubmitting(false); | |
| return; | |
| } |
🤖 Prompt for AI Agents
In @frontend/app/[locale]/shop/admin/products/_components/product-form.tsx
around lines 324 - 338, In the SALE_ORIGINAL_REQUIRED branch of the submit
handler, call setIsSubmitting(false) before returning so the form doesn't remain
stuck; specifically, just before the early return in the block that checks if
(data.code === 'SALE_ORIGINAL_REQUIRED'), invoke setIsSubmitting(false) (keeping
the existing setOriginalPriceErrors and setError calls) so submission state is
cleared regardless of this error.
| try { | ||
| const prices = JSON.parse(pricesRaw); | ||
| return findSaleRuleViolation({ badge, prices }); | ||
| } catch { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd frontend && find . -name "route.ts" -path "*/admin/products/\[id\]/*" | head -20Repository: DevLoversTeam/devlovers.net
Length of output: 165
🏁 Script executed:
cd frontend && find . -type f -name "*.ts" | xargs grep -l "parseAdminProductForm" | head -10Repository: DevLoversTeam/devlovers.net
Length of output: 238
🏁 Script executed:
cd frontend && cat -n app/api/shop/admin/products/\[id\]/route.ts | head -100Repository: DevLoversTeam/devlovers.net
Length of output: 3775
🏁 Script executed:
cd frontend && cat -n app/api/shop/admin/products/\[id\]/route.ts | sed -n '100,200p'Repository: DevLoversTeam/devlovers.net
Length of output: 3819
🏁 Script executed:
cd frontend && cat -n lib/admin/parseAdminProductForm.tsRepository: DevLoversTeam/devlovers.net
Length of output: 12364
Silent failure on JSON parse error creates ambiguous validation state.
When JSON.parse fails in getSaleViolationFromFormData, returning null makes the early SALE validation pass silently without distinguishing between "no violations found" and "couldn't validate because JSON is malformed." While the malformed JSON will eventually be caught by parseAdminProductForm with a generic "Invalid prices JSON" error, the early SALE check should explicitly handle and report JSON parse errors rather than treating them as absence of violations.
🤖 Prompt for AI Agents
In @frontend/app/api/shop/admin/products/[id]/route.ts around lines 66 - 71,
getSaleViolationFromFormData currently swallows JSON.parse errors and returns
null, making malformed prices indistinguishable from "no violations"; change the
catch around JSON.parse(pricesRaw) so it does not return null silently but
instead surfaces a clear parse failure (either by throwing a descriptive error
or returning an explicit validation error object) so callers like
parseAdminProductForm and findSaleRuleViolation can distinguish "invalid JSON"
from "no sale violations"; reference the getSaleViolationFromFormData function,
the JSON.parse(pricesRaw) call, and the use of findSaleRuleViolation when
implementing this behavior.
|
|
||
| if (eventType === 'charge.refunded' && charge) { | ||
| const amt = | ||
| typeof (charge as any).amount === 'number' | ||
| ? (charge as any).amount | ||
| : null; | ||
| const refunded = | ||
| typeof (charge as any).amount_refunded === 'number' | ||
| ? (charge as any).amount_refunded | ||
| : null; | ||
| isFullRefund = amt != null && refunded != null && refunded === amt; | ||
| } else if (eventType === 'charge.refund.updated' && refund) { | ||
| const refundAmt = | ||
| typeof (refund as any).amount === 'number' | ||
| ? (refund as any).amount | ||
| : null; | ||
| isFullRefund = | ||
| refundAmt != null && refundAmt === order.totalAmountMinor; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Stripe charge.refund.updated event payload structure documentation
💡 Result:
Short answer: charge.refund.updated (and related refund events) are delivered as a Stripe Event JSON whose data.object is a Refund object. See Stripe’s webhook/Event docs for the Event structure, the Refund object reference for fields inside data.object, and the changelog noting refund.updated behavior.
Sources:
[1] Stripe — Receive webhook events / Event object (webhooks).
[2] Stripe API reference — Refund object (fields & example payload).
[3] Stripe changelog — Adds refund.created / refund.updated / refund.failed (2024-10-28).
🌐 Web query:
Stripe Refund object amount field documentation API reference
💡 Result:
The Refund object’s amount is an integer in the currency’s smallest unit (e.g., cents for USD) representing how much was refunded. It’s a positive integer and can only be up to the remaining unrefunded amount of the payment. [1][2]
Sources:
[1] Stripe API reference — Refund object.
[2] Stripe API reference — Report a refund (amount field details).
Full refund detection logic is inconsistent across event types.
For charge.refunded, full refund detection correctly compares the cumulative amount_refunded against the charge amount. However, for charge.refund.updated, comparing a single refund's amount directly to order.totalAmountMinor fails to account for multiple partial refunds and incorrectly marks subsequent refunds as incomplete even when they complete a full refund sequence.
The charge.refund.updated handler should either:
- Track cumulative refund amounts across multiple refund events, or
- Compare against the charge's remaining unrefunded amount (original charge amount minus previously refunded total)
🤖 Prompt for AI Agents
In @frontend/app/api/shop/webhooks/stripe/route.ts around lines 606 - 624, The
full-refund detection for 'charge.refund.updated' is incorrect because it
compares a single refund.amount to order.totalAmountMinor; instead compute
cumulative refunded for the associated charge and compare that to the original
charge amount. In the 'charge.refund.updated' branch (where refund is
available), locate the related charge object (from refund.charge or by
retrieving the charge) and determine the cumulative refunded amount (use
charge.amount_refunded if present, or sum existing charge.refunds.data amounts
plus the current refund.amount) and set isFullRefund when cumulativeRefunded ===
charge.amount (or === order.totalAmountMinor if you prefer using order totals).
Update the code that sets isFullRefund in the charge.refund.updated case to use
this cumulative comparison rather than just refund.amount.
| function timingSafeEqual(a: string, b: string) { | ||
| const aBuf = Buffer.from(a); | ||
| const bBuf = Buffer.from(b); | ||
| if (aBuf.length !== bBuf.length) return false; | ||
| return crypto.timingSafeEqual(aBuf, bBuf); | ||
| } |
There was a problem hiding this comment.
Timing leak in length comparison.
The early return false on length mismatch (line 8) reveals information about the secret's length through timing differences. An attacker can determine the exact length of the secret by measuring response times.
To maintain constant-time behavior regardless of input length, pad both buffers to the same size before comparison.
Proposed fix
function timingSafeEqual(a: string, b: string) {
const aBuf = Buffer.from(a);
const bBuf = Buffer.from(b);
- if (aBuf.length !== bBuf.length) return false;
- return crypto.timingSafeEqual(aBuf, bBuf);
+ // Pad to same length to avoid timing leak on length difference
+ const maxLen = Math.max(aBuf.length, bBuf.length);
+ if (maxLen === 0) return true;
+ const aPadded = Buffer.alloc(maxLen);
+ const bPadded = Buffer.alloc(maxLen);
+ aBuf.copy(aPadded);
+ bBuf.copy(bPadded);
+ // Compare padded buffers AND check original lengths match
+ return crypto.timingSafeEqual(aPadded, bPadded) && aBuf.length === bBuf.length;
}🤖 Prompt for AI Agents
In @frontend/lib/auth/internal-janitor.ts around lines 5 - 10, The
timingSafeEqual function leaks secret length via the early length check; modify
timingSafeEqual to avoid early return by allocating two buffers of equal length
(max of aBuf.length and bBuf.length), copy a and b into those fixed-size buffers
(padding the remainder with zeros), then call crypto.timingSafeEqual on the
padded buffers (still handling non-buffer inputs as before) so the comparison
time does not vary with input length.
| 📁 scripts | ||
| 📄 guard-non-preview.ts | ||
| 📄 shop-janitor-restock-stale.mjs | ||
| 📁 tmp |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if tmp is in .gitignore
grep -E '^\s*/?tmp/?$|^\s*/tmp\s*$' .gitignore || echo "tmp NOT found in .gitignore"Repository: DevLoversTeam/devlovers.net
Length of output: 98
🏁 Script executed:
#!/bin/bash
# Check the actual .gitignore content for any tmp-related patterns
echo "=== Checking .gitignore for tmp-related patterns ==="
grep -i 'tmp' .gitignore || echo "No tmp patterns found in .gitignore"
echo ""
echo "=== Full .gitignore content ==="
cat .gitignore | head -50
echo ""
echo "=== Checking if frontend/project-structure.txt exists ==="
if [ -f "frontend/project-structure.txt" ]; then
echo "File exists"
echo "=== Lines around line 407 ==="
sed -n '405,410p' "frontend/project-structure.txt"
else
echo "File not found"
fiRepository: DevLoversTeam/devlovers.net
Length of output: 994
Add tmp directory to .gitignore.
The tmp directory has been added to the project structure but is not currently ignored by git. Temporary directories should be excluded from version control to prevent committing temporary artifacts.
🤖 Prompt for AI Agents
In @frontend/project-structure.txt at line 407, The project now contains a tmp
directory that must be ignored by git; open the .gitignore file and add an entry
for the tmp directory (e.g., tmp/ or /tmp) so temporary artifacts aren’t
committed, then if tmp was already tracked remove it from the index (git rm
--cached -r tmp) and commit the .gitignore change; reference the tmp directory
and the .gitignore file when making the update.
| process.exit(1); | ||
| } | ||
|
|
||
| const timeoutMs = Number(process.env.JANITOR_TIMEOUT_MS ?? '25000'); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Validate timeout to prevent NaN from invalid input.
Number() returns NaN for invalid inputs, which would cause setTimeout to fire immediately. Add validation to ensure a positive timeout value.
🛡️ Proposed validation
-const timeoutMs = Number(process.env.JANITOR_TIMEOUT_MS ?? '25000');
+const timeoutMs = Number(process.env.JANITOR_TIMEOUT_MS ?? '25000');
+if (isNaN(timeoutMs) || timeoutMs <= 0) {
+ console.error('[janitor] Invalid JANITOR_TIMEOUT_MS');
+ process.exit(1);
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const timeoutMs = Number(process.env.JANITOR_TIMEOUT_MS ?? '25000'); | |
| const timeoutMs = Number(process.env.JANITOR_TIMEOUT_MS ?? '25000'); | |
| if (isNaN(timeoutMs) || timeoutMs <= 0) { | |
| console.error('[janitor] Invalid JANITOR_TIMEOUT_MS'); | |
| process.exit(1); | |
| } |
🤖 Prompt for AI Agents
In @frontend/scripts/shop-janitor-restock-stale.mjs at line 17, The timeout
parsing uses Number(process.env.JANITOR_TIMEOUT_MS) which can yield NaN and make
setTimeout fire immediately; validate and normalize it by parsing and checking
that the result is a finite positive integer: read
process.env.JANITOR_TIMEOUT_MS, parse to a number (e.g., parseInt/Number), check
isFinite(value) and value > 0, and if invalid fall back to the default (25000)
or a configured minimum; update the const timeoutMs declaration and any use of
timeoutMs (e.g., the setTimeout call) to use the validated/normalized value.
| @@ -0,0 +1 @@ | |||
| {"data":{"object":{"id":"ch_3Sn5WRAzdq4b0hR02JIDJhDx","payment_intent":"pi_3Sn5WRAzdq4b0hR027sBGFiq","amount":5900,"refunds":{"data":[{"amount":5900,"status":"succeeded","id":"re_3Sn5WRAzdq4b0hR02gbWWFvP","reason":null}]},"metadata":{},"amount_refunded":5900,"status":"succeeded"}},"id":"\u003cPUT_EVT_ID_FROM_DB_HERE\u003e","type":"charge.refunded"} | |||
There was a problem hiding this comment.
Remove BOM character and document placeholder replacement.
The file contains:
- A BOM (byte order mark) character at the start that can cause JSON parsing issues in some tools
- A placeholder
<PUT_EVT_ID_FROM_DB_HERE>requiring manual replacement before use
Consider removing the BOM and adding a comment in the replay script or README documenting how to populate the event ID.
🧹 Suggested cleanup
Remove the BOM character and consider formatting the JSON for readability:
{
"data": {
"object": {
"id": "ch_3Sn5WRAzdq4b0hR02JIDJhDx",
"payment_intent": "pi_3Sn5WRAzdq4b0hR027sBGFiq",
"amount": 5900,
"refunds": {
"data": [
{
"amount": 5900,
"status": "succeeded",
"id": "re_3Sn5WRAzdq4b0hR02gbWWFvP",
"reason": null
}
]
},
"metadata": {},
"amount_refunded": 5900,
"status": "succeeded"
}
},
"id": "<PUT_EVT_ID_FROM_DB_HERE>",
"type": "charge.refunded"
}Add documentation explaining the manual step to replace <PUT_EVT_ID_FROM_DB_HERE> with an actual event ID from the database.
🤖 Prompt for AI Agents
In @frontend/tmp/replay-charge-refunded.json at line 1, The JSON file begins
with a BOM character that can break parsers and contains the placeholder string
"<PUT_EVT_ID_FROM_DB_HERE>" that must be populated before replay; remove the
leading BOM from frontend/tmp/replay-charge-refunded.json, reformat the JSON for
readability, and replace or document the need to replace the
"<PUT_EVT_ID_FROM_DB_HERE>" placeholder (add a short note in the replay script
or README explaining how to obtain and insert the actual event ID from the DB).
| @@ -0,0 +1,23 @@ | |||
| const fs = require('fs'); | |||
There was a problem hiding this comment.
Remove UTF-8 BOM character.
Line 1 contains a UTF-8 BOM () before the const keyword. This can cause issues with Node.js execution or when the file is read by other tools.
Proposed fix
-const fs = require('fs');
+const fs = require('fs');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const fs = require('fs'); | |
| const fs = require('fs'); |
🤖 Prompt for AI Agents
In @frontend/tmp/replay-stripe-webhook.js at line 1, The file has a UTF-8 BOM at
the start (before the "const fs = require('fs');" statement); remove the BOM so
the file begins with the "const" token, re-save the file as UTF-8 without BOM
(or run a tool/IDE option to strip BOM), and verify Node.js can load the file
and linters/CI no longer flag the leading invisible character.
| const signed = ${ts}.{"id":"evt_test"}; | ||
| const sig = crypto.createHmac('sha256', secret).update(signed, 'utf8').digest('hex'); | ||
| const header = =,v1=; |
There was a problem hiding this comment.
Critical syntax errors in template literal construction.
Lines 12 and 14 have broken template literal syntax that will cause parse errors (confirmed by static analysis). The Stripe signature header requires a specific format.
Proposed fix
-const signed = ${ts}.{"id":"evt_test"};
-const sig = crypto.createHmac('sha256', secret).update(signed, 'utf8').digest('hex');
-const header = =,v1=;
+const signed = `${ts}.${payload}`;
+const sig = crypto.createHmac('sha256', secret).update(signed, 'utf8').digest('hex');
+const header = `t=${ts},v1=${sig}`;The signed payload for Stripe webhooks should be {timestamp}.{payload} and the header format should be t={timestamp},v1={signature}.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (2.1.2)
[error] 12-12: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 12-12: Expected a statement but instead found '.'.
Expected a statement here.
(parse)
[error] 12-12: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 14-14: Expected an expression, or an assignment but instead found '='.
Expected an expression, or an assignment here.
(parse)
🤖 Prompt for AI Agents
In @frontend/tmp/replay-stripe-webhook.js around lines 12 - 14, The template
literals for the signed payload and header are malformed: build the signed
payload as the timestamp, a dot, then the JSON string payload (e.g., use payload
= JSON.stringify({id: "evt_test"}) and signed = `${ts}.${payload}`), compute sig
with crypto.createHmac('sha256', secret).update(signed, 'utf8').digest('hex'),
and set the header string to `t=${ts},v1=${sig}` (use the existing variables
signed, sig, secret, ts, and payload).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f3ec7f21b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const signed = ${ts}.{"id":"evt_test"}; | ||
| const sig = crypto.createHmac('sha256', secret).update(signed, 'utf8').digest('hex'); | ||
| const header = =,v1=; |
There was a problem hiding this comment.
The new replay helper is syntactically invalid: lines 12–14 contain unquoted template placeholders (e.g., const signed = ${ts}...) and an incomplete header assignment. As written, this file cannot be executed and will fail parsing for any tool that scans JS sources (e.g., npm run lint uses ESLint over the repo and the config does not ignore tmp/). This will break lint/CI and makes the replay script unusable until the placeholders are converted into real string/template literals.
Useful? React with 👍 / 👎.
(SP: 3) [Backend/DevOps] Make shop janitor production-runnable; harden inventory idempotency, refunds, and concurrency invariants
2 participants
Description
This PR makes the shop “janitor” runnable in production and hardens the order/inventory lifecycle against real-world async and concurrency failures (abandoned checkouts, webhook retries, stuck reserving states). It adds secure internal execution, cross-instance rate limiting, bounded runtime/batch policies, and idempotent cleanup paths that restore stock exactly once and mark orders terminal to prevent reprocessing loops.
Related Issue
Changes
Implemented production-safe internal janitor endpoint
POST /api/shop/internal/orders/restock-stale:INTERNAL_JANITOR_SECRET(fail-closed; constant-time compare).batchSize,olderThanMinutes(legacy + structured policy), andmaxRuntimeMs.Retry-AfterandCache-Control: no-store.stalePending,orphanNoPayment, andstuckReserving(handled by service).Hardened orders/inventory service for exactly-once behavior:
restockOrder(...)finalize-once semantics (no double stock increment; stable markers/timestamps).STUCK_RESERVING_TIMEOUTAdded scheduler runner and GitHub Actions workflow:
{}.429 RATE_LIMITEDas success (expected no-op for schedulers).Strengthened Stripe refund + webhook reliability:
stripe_eventstable (no duplicate side effects).refundedand restore stock exactly once (full refund scope).Added concurrency coverage:
stock=1-> exactly one success; no negative stock; no duplicate reserves.Enforced SALE rule contract for admin product mutations:
badge=SALE, requiresoriginalPriceMinorandoriginalPriceMinor > priceMinor.SALE_ORIGINAL_REQUIRED) for UI to render field-level feedback.Database Changes (if applicable)
Notes
internal_job_statetable for DB-backed rate limiting (cross-instance).How Has This Been Tested?
Commands / evidence
npx vitest run(all suites green)npm run build(Next build successful)POST /api/shop/internal/orders/restock-stalereturns 200; repeated calls return 429 withRetry-After(expected), runner exits0.Screenshots (if applicable)
N/A (backend + workflows).
(If needed: attach UI validation screenshot for SALE error rendering.)
Checklist
Before submitting
Reviewers
Operational note (for reviewers/lead)
To activate the scheduled janitor runs:
Configure GitHub Actions secrets:
JANITOR_URLINTERNAL_JANITOR_SECRETEnsure production runtime env includes:
INTERNAL_JANITOR_SECRET=<prod>INTERNAL_JANITOR_MIN_INTERVAL_SECONDS=300(or 600)Until secrets are set, the workflow will skip safely via the guard step.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.