Add infrastructure models, services & docs#380
Conversation
Introduce infrastructure features: idempotency, API tokens, observability/metrics, inventory reservations and a cron cleanup job. Adds Prisma migration creating idempotency_records, api_tokens and performance_metrics tables and related indexes; updates prisma schema. Implements services and APIs (idempotency middleware, api-tokens routes, /api/metrics, /api/cron/cleanup), UI pages and components (/settings/api-tokens, /admin/metrics, tokens manager, metrics dashboard), and sidebar navigation updates. Updates .env.example with new infrastructure env vars (UPSTASH_REDIS_REST_*, CRON_SECRET, METRICS_AUTH_TOKEN, AUDIT_LOG_RETENTION_DAYS) and adds comprehensive ENVIRONMENT_VARIABLES_GUIDE.md plus operation/validation docs. Also includes type fixes and service changes (inventory.service.ts) required for cleanup and reservation workflows.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Switch token storage to PBKDF2 with per-token salt and verification: replace createHash SHA256 with pbkdf2Sync, add salt:hash format, verifyTokenHash(), and change lookup to find by tokenPrefix then verify. Update prisma schema comment for ApiToken.tokenHash (removed unique constraint to allow salt:hash storage). Make migration SQL idempotent by using DROP INDEX IF EXISTS for 10 index drops to avoid failures when indexes are missing. Remove several unused UI imports in metrics-dashboard and api-tokens-manager, call useSession() without destructuring, escape an apostrophe in JSX, and add PR_380_REVIEW_FIXES.md and FINAL_FIX_SUMMARY.md documenting fixes and validation results.
Add comprehensive PR/validation reports and Playwright tests, update auth fixture, and fix admin metrics component. - Added documentation: ADMIN_METRICS_FIX_REPORT.md, COMPLETE_IMPLEMENTATION_VALIDATION.md, INFRASTRUCTURE_TESTING_REPORT.md, PR_380_COMPLETION_FINAL_REPORT.md (detailed validation, test plans and findings). - Added end-to-end test suite: e2e/infrastructure-features.spec.ts (Playwright scenarios for API tokens, metrics dashboard, navigation, auth, and edge cases). - Updated e2e/auth.setup.ts to use Store Owner test credentials (rafiq@techbazar.io / Owner@123456). - Fixed src/components/admin/metrics-dashboard.tsx to match the API: updated TypeScript types, switched health.checks from array to object, and updated UI logic to use Object.values/Object.entries for counting and rendering checks (resolves runtime .filter() error). These changes add tests and docs to validate PR #380 and fix the metrics page runtime error so the admin dashboard renders correctly.
There was a problem hiding this comment.
Pull request overview
This PR introduces a set of “infrastructure” capabilities (inventory reservations, idempotency, API tokens, metrics/observability, and scheduled cleanup) to support safer API operations and better operational visibility across the StormCom platform.
Changes:
- Added inventory reservation workflows (create/confirm/release/expire) to prevent overselling during checkout.
- Introduced generic idempotency middleware, scoped API token management (service + routes + UI), and an admin metrics dashboard with a Prometheus-style
/api/metricsendpoint. - Expanded Prisma schema/migration for new infrastructure tables and added supporting cron cleanup and documentation artifacts.
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/services/inventory.service.ts | Adds inventory reservation APIs (TTL holds, confirm/release/expire, availability calc). |
| src/lib/observability.ts | New structured logger + in-memory metrics collector + health status helper. |
| src/lib/idempotency.ts | New DB-backed idempotency service + route wrapper. |
| src/lib/api-token.ts | New API token generation/hashing/validation + scope helpers. |
| src/components/settings/api-tokens-manager.tsx | Client UI for creating/listing/rotating/revoking API tokens. |
| src/components/app-sidebar.tsx | Adds “API Tokens” link to settings navigation. |
| src/components/admin/metrics-dashboard.tsx | Client UI for viewing /api/health + basic system status display. |
| src/components/admin/admin-sidebar.tsx | Adds “System Metrics” link to admin nav. |
| src/app/settings/api-tokens/page.tsx | Server-rendered settings page that hosts the API tokens manager UI. |
| src/app/api/metrics/route.ts | Authenticated metrics endpoint (Prometheus text / JSON). |
| src/app/api/cron/cleanup/route.ts | Cron-secured endpoint to expire reservations + cleanup idempotency/tokens/audit logs. |
| src/app/api/api-tokens/route.ts | Session-authenticated list/create endpoints for API tokens. |
| src/app/api/api-tokens/[id]/route.ts | Token revoke + rotation handler (currently has route-path mismatch for rotation). |
| src/app/admin/metrics/page.tsx | Super-admin-only page that hosts the metrics dashboard UI. |
| prisma/schema.prisma | Adds IdempotencyRecord + ApiToken models and relations. |
| prisma/migrations/20260323225623_add_infrastructure_models/migration.sql | Creates infra tables/indexes; drops old indexes; renames chat_sessions index. |
| .env.example | Adds infra env vars (Upstash REST, cron secret, metrics token, audit retention). |
| ENVIRONMENT_VARIABLES_GUIDE.md | Comprehensive environment variable documentation. |
| PR_380_REVIEW_FIXES.md | Review-fixes tracking doc. |
| PR_380_COMPLETION_FINAL_REPORT.md | Final validation report doc. |
| INFRASTRUCTURE_VALIDATION_SESSION.md | Validation session log. |
| INFRASTRUCTURE_TESTING_REPORT.md | Test plan/report doc. |
| INFRASTRUCTURE_IMPLEMENTATION_PROGRESS.md | Implementation progress doc. |
| FINAL_FIX_SUMMARY.md | Fix summary doc. |
| ENV_UPDATE_SUMMARY.md | Env update summary doc. |
| COMPLETE_IMPLEMENTATION_VALIDATION.md | Comprehensive implementation validation doc. |
| ADMIN_METRICS_FIX_REPORT.md | Metrics page bug-fix report doc. |
| e2e/infrastructure-features.spec.ts | Adds Playwright E2E coverage for new infrastructure UI flows. |
| e2e/auth.setup.ts | Updates E2E auth setup credentials used for tests. |
| async createReservations( | ||
| storeId: string, | ||
| items: Array<{ productId: string; variantId?: string; quantity: number }>, | ||
| reservedBy?: string, | ||
| ttlMinutes: number = InventoryService.DEFAULT_RESERVATION_TTL_MINUTES |
There was a problem hiding this comment.
createReservations() takes a storeId but does not verify that each productId/variantId belongs to that store. This allows cross-store inventory reservations (multi-tenant isolation/IDOR risk). Validate product/variant ownership against storeId inside the transaction before reserving.
| const reservationIds: string[] = []; | ||
|
|
||
| await prisma.$transaction(async (tx) => { | ||
| for (const item of items) { | ||
| // Check available stock (considering existing reservations) | ||
| const availableStock = await this.getAvailableStock( | ||
| tx, | ||
| item.productId, | ||
| item.variantId | ||
| ); | ||
|
|
||
| if (availableStock < item.quantity) { | ||
| const identifier = item.variantId | ||
| ? `variant ${item.variantId}` | ||
| : `product ${item.productId}`; | ||
| throw new Error( | ||
| `Insufficient available stock for ${identifier}. Available: ${availableStock}, Requested: ${item.quantity}` | ||
| ); | ||
| } | ||
|
|
||
| // Create reservation | ||
| const reservation = await tx.inventoryReservation.create({ | ||
| data: { | ||
| productId: item.productId, | ||
| variantId: item.variantId || null, | ||
| quantity: item.quantity, | ||
| status: ReservationStatus.PENDING, | ||
| expiresAt, | ||
| reservedBy: reservedBy || null, | ||
| }, | ||
| }); | ||
|
|
||
| reservationIds.push(reservation.id); | ||
| } | ||
| }); | ||
|
|
||
| return reservationIds; |
There was a problem hiding this comment.
The availability check (getAvailableStock) is done before inserting the reservation, but without any row-level locking/atomic constraint. Two concurrent checkouts can both pass the check and then create reservations, causing total reserved quantity to exceed inventoryQty (oversell). Use a locking/atomic approach (e.g., SELECT … FOR UPDATE, SERIALIZABLE isolation + retry, or an atomic reserved counter).
| const reservationIds: string[] = []; | |
| await prisma.$transaction(async (tx) => { | |
| for (const item of items) { | |
| // Check available stock (considering existing reservations) | |
| const availableStock = await this.getAvailableStock( | |
| tx, | |
| item.productId, | |
| item.variantId | |
| ); | |
| if (availableStock < item.quantity) { | |
| const identifier = item.variantId | |
| ? `variant ${item.variantId}` | |
| : `product ${item.productId}`; | |
| throw new Error( | |
| `Insufficient available stock for ${identifier}. Available: ${availableStock}, Requested: ${item.quantity}` | |
| ); | |
| } | |
| // Create reservation | |
| const reservation = await tx.inventoryReservation.create({ | |
| data: { | |
| productId: item.productId, | |
| variantId: item.variantId || null, | |
| quantity: item.quantity, | |
| status: ReservationStatus.PENDING, | |
| expiresAt, | |
| reservedBy: reservedBy || null, | |
| }, | |
| }); | |
| reservationIds.push(reservation.id); | |
| } | |
| }); | |
| return reservationIds; | |
| const maxAttempts = 3; | |
| for (let attempt = 0; attempt < maxAttempts; attempt++) { | |
| const reservationIds: string[] = []; | |
| try { | |
| await prisma.$transaction( | |
| async (tx) => { | |
| for (const item of items) { | |
| // Check available stock (considering existing reservations) | |
| const availableStock = await this.getAvailableStock( | |
| tx, | |
| item.productId, | |
| item.variantId | |
| ); | |
| if (availableStock < item.quantity) { | |
| const identifier = item.variantId | |
| ? `variant ${item.variantId}` | |
| : `product ${item.productId}`; | |
| throw new Error( | |
| `Insufficient available stock for ${identifier}. Available: ${availableStock}, Requested: ${item.quantity}` | |
| ); | |
| } | |
| // Create reservation | |
| const reservation = await tx.inventoryReservation.create({ | |
| data: { | |
| productId: item.productId, | |
| variantId: item.variantId || null, | |
| quantity: item.quantity, | |
| status: ReservationStatus.PENDING, | |
| expiresAt, | |
| reservedBy: reservedBy || null, | |
| }, | |
| }); | |
| reservationIds.push(reservation.id); | |
| } | |
| }, | |
| { | |
| isolationLevel: Prisma.TransactionIsolationLevel.Serializable, | |
| } | |
| ); | |
| // Transaction succeeded; return the created reservation IDs. | |
| return reservationIds; | |
| } catch (error) { | |
| const isSerializationError = | |
| error instanceof Prisma.PrismaClientKnownRequestError && | |
| error.code === 'P2034'; | |
| // If this is not a serialization error, or we've exhausted retries, rethrow. | |
| if (!isSerializationError || attempt === maxAttempts - 1) { | |
| throw error; | |
| } | |
| // Otherwise, retry the transaction in the next loop iteration. | |
| } | |
| } | |
| // This line should be unreachable because we either return or throw above. | |
| throw new Error('Failed to create reservations after multiple attempts.'); |
| class StructuredLogger { | ||
| private defaultContext: string; | ||
| private correlationId?: string; | ||
|
|
There was a problem hiding this comment.
structuredLogger is exported as a module-level singleton, but StructuredLogger stores request-scoped state (correlationId) on the instance. In a long-lived Node process this can leak/carry correlation IDs across requests and mix log context. Prefer per-request loggers (or AsyncLocalStorage) and avoid keeping correlation IDs on a shared singleton.
| } catch (error) { | ||
| // Unique constraint violation means key already exists | ||
| if (error instanceof Error && error.message.includes('Unique constraint')) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
lockIdempotencyKey() uses error.message.includes('Unique constraint') to detect key collisions. Prisma unique violations are more reliably detected via Prisma.PrismaClientKnownRequestError with code === 'P2002'; message text can vary by driver/version and can cause unexpected 500s instead of returning false for “already locked”.
| // Constant-time comparison to prevent timing attacks | ||
| const computedHash = derivedKey.toString('hex'); | ||
| return computedHash === _hashHex; |
There was a problem hiding this comment.
This comparison is not constant-time: computedHash === _hashHex can leak timing information. Use crypto.timingSafeEqual on Buffers (and handle length mismatch safely) to match the comment’s security intent.
| tokenHash String // PBKDF2 hash (salt:derivedKey) with 100k iterations (salt:derivedKey) with 100k iterations of the token (never store plaintext) | ||
| tokenPrefix String // First 8 chars of token for identification (e.g., "stc_live_") |
There was a problem hiding this comment.
Comments don’t match implementation: tokenPrefix is documented as “First 8 chars…”, but the service slices 16 characters, and the tokenHash comment repeats “with 100k iterations” twice. Please update these comments to reflect actual behavior to avoid operational confusion.
| tokenHash String // PBKDF2 hash (salt:derivedKey) with 100k iterations (salt:derivedKey) with 100k iterations of the token (never store plaintext) | |
| tokenPrefix String // First 8 chars of token for identification (e.g., "stc_live_") | |
| tokenHash String // PBKDF2 hash of the token (salt:derivedKey with 100k iterations; never store plaintext tokens) | |
| tokenPrefix String // First 16 chars of token for identification (e.g., "stc_live_abcd1234") |
| -- RenameIndex | ||
| ALTER INDEX "chat_sessions_userId_organizationId_isArchived_lastMessageAt_id" RENAME TO "chat_sessions_userId_organizationId_isArchived_lastMessageA_idx"; |
There was a problem hiding this comment.
This index rename is not idempotent and will fail if the index was already renamed by an earlier migration (the repo already has a conditional DO-block rename for this same index). Wrap the rename in a conditional DO $$ block (or remove it if already handled) to keep migrations runnable across existing DB states.
| -- RenameIndex | |
| ALTER INDEX "chat_sessions_userId_organizationId_isArchived_lastMessageAt_id" RENAME TO "chat_sessions_userId_organizationId_isArchived_lastMessageA_idx"; | |
| -- RenameIndex (idempotent) | |
| DO $$ | |
| BEGIN | |
| IF EXISTS ( | |
| SELECT 1 | |
| FROM pg_class c | |
| JOIN pg_namespace n ON n.oid = c.relnamespace | |
| WHERE c.relkind = 'i' | |
| AND c.relname = 'chat_sessions_userId_organizationId_isArchived_lastMessageAt_id' | |
| ) THEN | |
| ALTER INDEX "chat_sessions_userId_organizationId_isArchived_lastMessageAt_id" | |
| RENAME TO "chat_sessions_userId_organizationId_isArchived_lastMessageA_idx"; | |
| END IF; | |
| END | |
| $$; |
|
|
||
| /** | ||
| * Constant-time string comparison to prevent timing attacks | ||
| */ | ||
| function constantTimeCompare(a: string, b: string): boolean { | ||
| if (a.length !== b.length) { | ||
| return false; | ||
| } | ||
|
|
||
| let result = 0; | ||
| for (let i = 0; i < a.length; i++) { | ||
| result |= a.charCodeAt(i) ^ b.charCodeAt(i); | ||
| } | ||
|
|
||
| return result === 0; |
There was a problem hiding this comment.
Despite the comment, this compare is not fully constant-time because it returns early when lengths differ. Consider using crypto.timingSafeEqual on fixed-length digests (e.g., SHA-256 of both strings) to avoid timing differences based on attacker-controlled input length.
| /** | |
| * Constant-time string comparison to prevent timing attacks | |
| */ | |
| function constantTimeCompare(a: string, b: string): boolean { | |
| if (a.length !== b.length) { | |
| return false; | |
| } | |
| let result = 0; | |
| for (let i = 0; i < a.length; i++) { | |
| result |= a.charCodeAt(i) ^ b.charCodeAt(i); | |
| } | |
| return result === 0; | |
| import { createHash, timingSafeEqual } from 'crypto'; | |
| /** | |
| * Constant-time string comparison to prevent timing attacks. | |
| * Uses SHA-256 digests and crypto.timingSafeEqual to avoid | |
| * timing differences based on input length or content. | |
| */ | |
| function constantTimeCompare(a: string, b: string): boolean { | |
| const digestA = createHash('sha256').update(a).digest(); | |
| const digestB = createHash('sha256').update(b).digest(); | |
| return timingSafeEqual(digestA, digestB); |
| // Navigate to login | ||
| await page.goto(`${BASE_URL}/login`); | ||
|
|
||
| // Login with Store Owner credentials | ||
| await page.fill('input[placeholder="you@example.com"]', STORE_OWNER_EMAIL); | ||
| await page.fill('input[placeholder="••••••••"]', STORE_OWNER_PASSWORD); | ||
| await page.click('button:has-text("Sign In")'); |
There was a problem hiding this comment.
These login steps use placeholder/CSS selectors, which tend to be brittle as UI copy changes. Prefer role/label-based locators (e.g., getByRole('textbox', { name: /email/i })) and group steps with test.step() for readability and reporting.
| await page.click('button[title*="Refresh"]'); | ||
|
|
||
| // Wait for metrics to load | ||
| await page.waitForTimeout(2000); | ||
|
|
There was a problem hiding this comment.
Hard-coded sleeps (waitForTimeout) are a common source of flakiness. Prefer web-first waits/assertions (e.g., await expect(...).toBeVisible() / toHaveURL) that auto-retry until the UI reaches the expected state.
Introduce infrastructure features: idempotency, API tokens, observability/metrics, inventory reservations and a cron cleanup job. Adds Prisma migration creating idempotency_records, api_tokens and performance_metrics tables and related indexes; updates prisma schema. Implements services and APIs (idempotency middleware, api-tokens routes, /api/metrics, /api/cron/cleanup), UI pages and components (/settings/api-tokens, /admin/metrics, tokens manager, metrics dashboard), and sidebar navigation updates. Updates .env.example with new infrastructure env vars (UPSTASH_REDIS_REST_*, CRON_SECRET, METRICS_AUTH_TOKEN, AUDIT_LOG_RETENTION_DAYS) and adds comprehensive ENVIRONMENT_VARIABLES_GUIDE.md plus operation/validation docs. Also includes type fixes and service changes (inventory.service.ts) required for cleanup and reservation workflows.