Add commerce store and dating foundation#86
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughHigh-level: adds two advisory /autoplan pre-tool hooks and wires them into hook settings; refactors MCP admin authorization to use user isAdmin; introduces dating and commerce domains (DB migrations, schema, Zod validators, server APIs, client UI, checkout/fulfillment flow with CustomCat), expands managed-data syncs, adds tests and docs, and migrates grant task naming plus IAM admin flagging. ChangesAutoplan Enforcement Hooks
Database schema, migrations, and Zod validators
Managed-data syncs and seed data
MCP consent and admin authorization refactor
Commerce / Shirt / Store checkout, fulfillment, and integrations
Dating domain: server, APIs, client, pages
Miscellaneous: packages, scripts, docs, small fixes
Estimated code review effort 🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
9e62a07 to
e474445
Compare
There was a problem hiding this comment.
Pull request overview
Adds a new FAQ entry to the /love landing page advising users to avoid dating-app “shadowban” behaviors (copy/paste messaging, mass-liking) so the campaign distribution strategy remains effective.
Changes:
- Added a new FAQ item to the
/lovepage content inpage.tsx. - Updated the corresponding logged-out markdown snapshot to match the rendered content.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/web/src/app/love/page.tsx | Adds a new FAQ question/answer warning against copy-paste messaging and mass-liking to avoid reduced visibility. |
| packages/web/src/app/love/page.logged-out.md | Updates the logged-out snapshot content to include the new FAQ entry. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Code reviewTwo issues found. 1. Wishonia voice violation + hardcoded numbersFile: Run-on sentence. After the opener, all advice collapses into a single 107-word colon-delimited sentence. CLAUDE.md requires short, punchy declaratives: "Write like Kurt Vonnegut. Plain declaratives." and "Short sentences — punchy. Declarative." The adjacent FAQ answers are each 1–3 short sentences. Break the tips into separate declarative sentences. Hardcoded numeric ranges. 2. Missing
|
Code reviewTwo issues found. Checked for bugs (none) and CLAUDE.md compliance. 1. Wishonia voice violations —
|
PR review packetStart here
Review checklist
Changed files considered
Updated automatically when this PR's preview or visual review reruns. |
…hooks MCP server: hasAdminTaskWriteAccess now returns isAdmin alone (option B). Admin role IS the gate; TASKS_ADMIN scope is documentation/UX, not a security gate. Reversible to multi-admin posture by tightening one line in mcp-server.ts:198. Mike's User row now gets isAdmin: true via managed-iam-organization sync, unblocking the ic2ewd-grant updateTask Forbidden bug after deploy. DEFAULT_SCOPES renamed to DEFAULT_CONSENT_SCOPES with comment explaining what the constant actually controls (consent-UI preselection, filtered by allowedMcpScopesForUser). 189 lines of mcp-server.test.ts coverage including explicit regression tests for the ic2ewd-grant update + delete failure modes. /love source: removed "Hey Google, set a timer for one minute" leftover from prior dictation pass. Autoplan hooks: enforce-feature-preexistence-check warns when /autoplan references feature nouns that already exist as routes/branches/commits. enforce-cba-table-on-plan-files warns when plan files lack a structured Cost-Benefit Matrix. Both fix structural gaps in the 2026-05-19 autoplan failure that proposed a feature already shipped. TODO.md sweep: audited against actual code state, 207 lines of shipped items removed (login spam fix, wishonia smirk-to-happy avatar, email 12px-to-14px, grandma headshot crop, direct-reports-to-employees, humanity-manager-status panel, forward-to-better-fit mailto, dynamic approved-org + court/HvG sitemaps). Added dating registry deferred section + autoplan process rules. todo-touched: login-spam, wishonia-avatar, email-12px, grandma-headshot, direct-reports-rename, humanity-manager-status, forward-to-better-fit, dynamic-sitemaps qa-passed: option-B admin MCP simplification + ic2ewd-grant regression tests, copy:preview snapshots, mcp-server tests (123 passed), tsc clean, managed-iam-organization tests passed, /love smoke clean, dev-log scan clean Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Finishes a prior naming decision that didn't propagate to every surface. Live source/test scope now has zero 'icewad' / 'ICEWAD' hits; only ignored generated paths (.next/cache, packages/web/output/tmp-*) and historical plan files retain the old name. Renamed seed task keys: icewad-grant-* → ic2ewd-grant-*, icewad:grant:* → ic2ewd:grant:*, plus methodology key and interest tag. Production data risk surfaced by audit: 10 old icewad:grant:* rows exist (7 not soft-deleted). Audit found 0 active comments, 0 task communications, 0 sent/received communications, 0 email logs on the old rows. After deploy + managed-data sync, new ic2ewd-grant-* rows will be created; old rows remain orphaned but carry no user state. Files: packages/db/src/__tests__/seed.integration.test.ts (+3/-3), packages/db/src/managed-data/managed-seed-data.ts (+8/-8), packages/web/scripts/soft-delete-funding-tasks.ts (+4/-4), packages/web/src/lib/__tests__/mcp-server.test.ts (+2/-2 — comment refs updated to ic2ewd-grant for consistency with the rename). Verification: pnpm copy:preview passed, web tests + db tests + tsc clean. todo-skipped: rename completed a prior intent not tracked in TODO.md. qa-passed: source rename complete, focused tests + typecheck clean, no live-source icewad hits remain Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/web/src/lib/mcp-server.ts (1)
6728-6756:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
blockedTaskIdsstill rejects admin task management.This block now exempts admins from the private-dependency visibility check, but
forbiddenBlockedTaskIdsstill errors on any target task not created byuserId. Admins therefore still cannot create a new prerequisite that blocks an existing public or other-owned task.Allow the same admin override here
const forbiddenBlockedTaskIds = dependencyTasks .filter( (task) => blockedTaskIds.includes(task.id) && - task.createdByUserId !== userId, + task.createdByUserId !== userId && + !hasAdminTaskWriteAccess(scopes, isAdmin), ) .map((task) => task.id);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/lib/mcp-server.ts` around lines 6728 - 6756, The forbiddenBlockedTaskIds check incorrectly blocks admins; update the filter that builds forbiddenBlockedTaskIds (which iterates dependencyTasks and uses blockedTaskIds and task.createdByUserId !== userId) to also exempt admins by checking hasAdminTaskWriteAccess(scopes, isAdmin) — i.e., only mark a blocked task as forbidden if task.createdByUserId !== userId AND the caller does not have admin task write access (use hasAdminTaskWriteAccess(scopes, isAdmin)) so admins can create prerequisites that block other users' tasks.
🧹 Nitpick comments (4)
packages/web/src/app/developers/page.logged-out.md (2)
7-7: ⚡ Quick winCanonical metadata appears unset; verify source metadata export.
Line 7 shows
Canonical: [missing]. If this reflects actual route metadata, it can create avoidable SEO ambiguity for/developers.Please verify
packages/web/src/app/developers/page.tsx(or shared metadata generator) defines a canonical URL for this route.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/app/developers/page.logged-out.md` at line 7, The page's metadata is missing a canonical URL; open the route's metadata export in packages/web/src/app/developers/page.tsx (look for export const metadata or export async function generateMetadata) and add a canonical link (e.g., metadata.openGraph.url or metadata.alternates.canonical) pointing to the absolute URL for /developers; if you use a shared helper (e.g., getCanonicalUrl or buildMetadata), update that generator to return a canonical property using your SITE_URL/appConfig so the canonical is always populated for this route.
57-57: ⚡ Quick winRemove internal mechanism wording from user-facing copy.
“delivery envelopes stay internal” reads like implementation detail and distracts from the user action.
Suggested rewrite
-- The agent posts task comments for status updates, questions, and next steps. Comment posting handles comment notifications; delivery envelopes stay internal. +- The agent posts task comments for status updates, questions, and next steps. Comment posting handles notifications automatically.As per coding guidelines, “Do not leak implementation or planning terms into user copy.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/app/developers/page.logged-out.md` at line 57, The user-facing sentence in packages/web/src/app/developers/page.logged-out.md contains an implementation detail—specifically the phrase "delivery envelopes stay internal"—which should be removed; update the sentence that currently reads "The agent posts task comments for status updates, questions, and next steps. Comment posting handles comment notifications; delivery envelopes stay internal." to a concise user-focused line (e.g., keep that the agent posts comments and notifications are handled) that omits any internal mechanism wording and ensures clarity for users rather than developers.packages/db/src/__tests__/seed.integration.test.ts (1)
157-157: ⚡ Quick winAdd a regression check that legacy
icewad:grant:tasks are absent.This test currently proves new keys exist, but not that old keys are gone. A negative assertion will catch duplicate-seed regressions.
✅ Suggested assertion
expect(grantTasks).toHaveLength(foundationSlugs.length); @@ expect(grantTasks[0]?.description).toContain("Suggested grant: $1."); + + const legacyGrantTaskCount = await prisma.task.count({ + where: { deletedAt: null, taskKey: { startsWith: "icewad:grant:" } }, + }); + expect(legacyGrantTaskCount).toBe(0);Also applies to: 161-161, 186-186
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/db/src/__tests__/seed.integration.test.ts` at line 157, Add a negative regression check to ensure legacy "icewad:grant:" task keys are not present: when verifying grant tasks (reference the grantTaskKeyPrefix variable and the test assertions around it), add an assertion that no keys start with or equal "icewad:grant:" (e.g., use the test framework's negative containment/assertion method) so the test fails if old legacy keys remain; apply the same negative check to the other similar spots noted (the checks around lines referencing grantTaskKeyPrefix at the other two locations).packages/web/src/lib/mcp-server.ts (1)
3770-3774: ⚡ Quick winAlign the MCP descriptions with the new admin contract.
updateTask/deleteTasknow allow admins to operate on public and non-owned tasks, but the schema text in this file still says “Update a private task...” and “Delete one of your own tasks...”. Schema-guided MCP clients will keep avoiding those paths unless the tool descriptions are updated too. While you are here, thecreateTask.visibilitydocs should also state that the defaultPUBLICbehavior for organization-assigned tasks is admin-only.Also applies to: 7902-7935, 8185-8194
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/lib/mcp-server.ts` around lines 3770 - 3774, The schema documentation strings for createTask.visibility, updateTask and deleteTask are out of date: update the createTask.visibility description to explicitly state that the default PUBLIC behavior for tasks assigned to an organization is allowed only for admin callers, and update the updateTask and deleteTask descriptions to say that admins may operate on PUBLIC and non-owned tasks (rather than “a private task” / “one of your own tasks”); locate the doc strings attached to the createTask visibility option and the updateTask/deleteTask operation descriptions (search for "create a task. Visibility defaults to", "Update a private task", and "Delete one of your own tasks") and revise their text accordingly, and mirror the same wording changes in the other duplicated schema-description locations where those strings appear.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/hooks/enforce-feature-preexistence-check-on-autoplan.mjs:
- Around line 237-245: searchRecentCommits currently runs safeExec(`git log
main..HEAD ...`) per candidate and hardcodes "main", which can timeout and
misses repos not using main; instead, run a single git log once, detect the
correct base branch dynamically (e.g., resolve origin/HEAD or use git merge-base
to find a common ancestor) and cache that output for reuse. Refactor by
extracting a collector (e.g., collectRecentCommits or precomputeRecentCommits)
that calls safeExec a single time (no hardcoded "main"), stores the raw log, and
change searchRecentCommits to accept or query that cached log and then filter by
candidate; update callers that previously invoked searchRecentCommits repeatedly
so they use the precomputed log.
In `@packages/db/src/managed-data/managed-seed-data.ts`:
- Around line 1347-1350: The seeded constants IC2EWD_CAMPAIGN_KEY_STEM,
IC2EWD_GRANT_TASK_ID_PREFIX, IC2EWD_GRANT_TASK_KEY_PREFIX and
IC2EWD_GRANT_METHODOLOGY_KEY introduce new deterministic IDs/keys and can create
duplicate grant task rows if legacy "icewad" rows remain; before inserting the
new seeded rows in managed-seed-data.ts, detect any existing rows that use the
old stems (e.g. "icewad" prefix) and either migrate them to the new values
(update id/taskKey to the new IC2EWD_* values) or perform an idempotent upsert
that updates existing rows instead of inserting duplicates, and apply the same
fix for the other renamed constants around the 1414–1416 region so seeding is
migration-safe.
In `@packages/web/src/app/developers/page.logged-out.md`:
- Around line 87-88: The markdown contains an incomplete setup step: the line
"MCP Server URL for step 2:" is missing the actual URL or placeholder before the
next heading ("#### HEADS-UP: DEEP RESEARCH MODE"); update the content in
packages/web/src/app/developers/page.logged-out.md to include the correct MCP
Server URL (or a clear placeholder like "<MCP_SERVER_URL>") immediately after
"MCP Server URL for step 2:" so the ChatGPT setup step is actionable, and ensure
the value is shown before any follow-up instruction or heading.
In `@packages/web/src/lib/mcp-server.ts`:
- Around line 7910-7924: The admin fallback that sets existingTask from
adminVisibleTask returns only a partial record, which lets resolveTaskEconomics
fall back to defaults and causes attachDirectTaskImpactEstimate to overwrite
true economics; update the admin fallback (the prisma.task.findFirst that
assigns adminVisibleTask and the existingTask assignment) to either: 1) select
and load the full set of economics-related fields used by
resolveTaskEconomics/attachDirectTaskImpactEstimate (e.g., cash_cost, hours,
p_success, EV/probability/time-to-impact inputs), or 2) if those fields are not
present, fail closed by leaving existingTask undefined so the admin path does
not write new estimates; ensure you modify the query and the assignment logic
around existingTask/adminVisibleTask so complete economics inputs are available
before allowing admin-only updates.
---
Outside diff comments:
In `@packages/web/src/lib/mcp-server.ts`:
- Around line 6728-6756: The forbiddenBlockedTaskIds check incorrectly blocks
admins; update the filter that builds forbiddenBlockedTaskIds (which iterates
dependencyTasks and uses blockedTaskIds and task.createdByUserId !== userId) to
also exempt admins by checking hasAdminTaskWriteAccess(scopes, isAdmin) — i.e.,
only mark a blocked task as forbidden if task.createdByUserId !== userId AND the
caller does not have admin task write access (use
hasAdminTaskWriteAccess(scopes, isAdmin)) so admins can create prerequisites
that block other users' tasks.
---
Nitpick comments:
In `@packages/db/src/__tests__/seed.integration.test.ts`:
- Line 157: Add a negative regression check to ensure legacy "icewad:grant:"
task keys are not present: when verifying grant tasks (reference the
grantTaskKeyPrefix variable and the test assertions around it), add an assertion
that no keys start with or equal "icewad:grant:" (e.g., use the test framework's
negative containment/assertion method) so the test fails if old legacy keys
remain; apply the same negative check to the other similar spots noted (the
checks around lines referencing grantTaskKeyPrefix at the other two locations).
In `@packages/web/src/app/developers/page.logged-out.md`:
- Line 7: The page's metadata is missing a canonical URL; open the route's
metadata export in packages/web/src/app/developers/page.tsx (look for export
const metadata or export async function generateMetadata) and add a canonical
link (e.g., metadata.openGraph.url or metadata.alternates.canonical) pointing to
the absolute URL for /developers; if you use a shared helper (e.g.,
getCanonicalUrl or buildMetadata), update that generator to return a canonical
property using your SITE_URL/appConfig so the canonical is always populated for
this route.
- Line 57: The user-facing sentence in
packages/web/src/app/developers/page.logged-out.md contains an implementation
detail—specifically the phrase "delivery envelopes stay internal"—which should
be removed; update the sentence that currently reads "The agent posts task
comments for status updates, questions, and next steps. Comment posting handles
comment notifications; delivery envelopes stay internal." to a concise
user-focused line (e.g., keep that the agent posts comments and notifications
are handled) that omits any internal mechanism wording and ensures clarity for
users rather than developers.
In `@packages/web/src/lib/mcp-server.ts`:
- Around line 3770-3774: The schema documentation strings for
createTask.visibility, updateTask and deleteTask are out of date: update the
createTask.visibility description to explicitly state that the default PUBLIC
behavior for tasks assigned to an organization is allowed only for admin
callers, and update the updateTask and deleteTask descriptions to say that
admins may operate on PUBLIC and non-owned tasks (rather than “a private task” /
“one of your own tasks”); locate the doc strings attached to the createTask
visibility option and the updateTask/deleteTask operation descriptions (search
for "create a task. Visibility defaults to", "Update a private task", and
"Delete one of your own tasks") and revise their text accordingly, and mirror
the same wording changes in the other duplicated schema-description locations
where those strings appear.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a75f65ee-1234-4f9f-bfc0-9a8a2a0f20cc
📒 Files selected for processing (16)
.claude/hooks/enforce-cba-table-on-plan-files.mjs.claude/hooks/enforce-feature-preexistence-check-on-autoplan.mjs.claude/settings.jsonTODO.mdpackages/db/src/__tests__/seed.integration.test.tspackages/db/src/managed-data/managed-iam-organization.test.tspackages/db/src/managed-data/managed-iam-organization.tspackages/db/src/managed-data/managed-seed-data.tspackages/web/scripts/soft-delete-funding-tasks.tspackages/web/src/app/developers/page.logged-out.mdpackages/web/src/app/love/page.logged-out.mdpackages/web/src/app/love/page.tsxpackages/web/src/app/mcp/authorize/page.tsxpackages/web/src/lib/__tests__/mcp-server.test.tspackages/web/src/lib/mcp-scopes.tspackages/web/src/lib/mcp-server.ts
qa-passed: prisma validate, db zod tests, focused checkout/webhook/fulfillment/dating tests, typecheck:fast, full web production build, and local screenshot review passed. todo-touched: documented checkout launch gates and dating foundation decisions.
qa-passed: db build, web typecheck:fast, managed-data dry-run, and seed.integration targeted test command (skipped without local DATABASE_URL). todo-skipped: review fix only; no TODO state changed.
There was a problem hiding this comment.
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/web/src/lib/mcp-server.ts (2)
3767-3771:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSync the
visibilitydocs with the new admin-only default.The top-level
createTaskdescription is updated, but thevisibilityfield description below still says organization-assigned tasks default toPUBLICunconditionally. MCP clients that surface field-level schema docs will still infer the wrong behavior.📝 Suggested doc fix
visibility: { type: "string", enum: Object.values(TaskVisibility), description: - "Optional visibility override. Defaults to PUBLIC for organization-assigned tasks and PRIVATE otherwise.", + "Optional visibility override. Defaults to PRIVATE; admin callers creating organization-assigned tasks default to PUBLIC.", },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/lib/mcp-server.ts` around lines 3767 - 3771, Update the field-level documentation for the visibility property to match the new top-level createTask behavior: state that organization-assigned tasks only default to PUBLIC for admin callers when assigneeOrganizationId is set, non-admin callers requesting PUBLIC will be rejected, and callers can override by passing visibility='PRIVATE' or 'PUBLIC'; modify the visibility doc near the createTask schema/field definition in packages/web/src/lib/mcp-server.ts (the visibility field description that currently claims organization-assigned tasks unconditionally default to PUBLIC) so it reflects the admin-only default and rejection rule.
6704-6785:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep
TASKS_ADMINas the consent gate for admin task writes.These paths now treat any admin with
TASKS_PERSONALas a full admin writer/deleter. That means a personal-only OAuth grant can create PUBLIC tasks here and, via the same helper, update or delete non-owned/PUBLIC tasks below. This collapses the consent boundary on mixed-scope tools and weakens least-privilege for admin tokens.🔐 Minimal fix
function hasAdminTaskWriteAccess( scopes: McpScope[] | undefined, isAdmin: boolean, ) { - void scopes; - return isAdmin; + return isAdmin && scopes?.includes(McpScope.TASKS_ADMIN) === true; }Also applies to: 7902-7960, 8210-8217
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/lib/mcp-server.ts` around lines 6704 - 6785, The code currently treats any admin with TASKS_PERSONAL as a full admin writer by relying on hasAdminTaskWriteAccess(scopes, isAdmin); change the gate to require the TASKS_ADMIN consent explicitly (or update hasAdminTaskWriteAccess to only return true when TASKS_ADMIN is present) so only scopes that include "TASKS_ADMIN" allow creating PUBLIC tasks, updating/deleting non-owned/PUBLIC tasks, and bypassing blockedTaskIds checks; update the checks around resolveCreateTaskIsPublic, the isAdminTaskWriter use, and the blocked/forbidden checks (the uses of hasAdminTaskWriteAccess(scopes, isAdmin) and isAdminTaskWriter) to instead verify the presence of TASKS_ADMIN in scopes (or call the revised helper) so TASKS_PERSONAL does not grant full admin write/delete rights.
🟠 Major comments (26)
packages/db/prisma/migrations/20260520043000_add_dating_foundation/migration.sql-178-210 (1)
178-210:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd DB checks to prevent self-target rows in pair/interaction tables.
Self-like/self-match/self-block rows should be rejected by constraints instead of relying on application code.
Suggested migration change
CREATE TABLE "DatingMatchScore" ( @@ - CONSTRAINT "DatingMatchScore_pkey" PRIMARY KEY ("id") + CONSTRAINT "DatingMatchScore_no_self_chk" CHECK ("profileAId" <> "profileBId"), + CONSTRAINT "DatingMatchScore_pkey" PRIMARY KEY ("id") ); @@ CREATE TABLE "DatingInteraction" ( @@ - CONSTRAINT "DatingInteraction_pkey" PRIMARY KEY ("id") + CONSTRAINT "DatingInteraction_no_self_chk" CHECK ("fromProfileId" <> "toProfileId"), + CONSTRAINT "DatingInteraction_pkey" PRIMARY KEY ("id") ); @@ CREATE TABLE "DatingMatch" ( @@ - CONSTRAINT "DatingMatch_pkey" PRIMARY KEY ("id") + CONSTRAINT "DatingMatch_no_self_chk" CHECK ("profileAId" <> "profileBId"), + CONSTRAINT "DatingMatch_pkey" PRIMARY KEY ("id") ); @@ CREATE TABLE "DatingBlock" ( @@ - CONSTRAINT "DatingBlock_pkey" PRIMARY KEY ("id") + CONSTRAINT "DatingBlock_no_self_chk" CHECK ("blockerProfileId" <> "blockedProfileId"), + CONSTRAINT "DatingBlock_pkey" PRIMARY KEY ("id") );Also applies to: 213-226, 284-294
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/db/prisma/migrations/20260520043000_add_dating_foundation/migration.sql` around lines 178 - 210, Add DB-level CHECK constraints to reject self-target rows: for the DatingMatchScore table add a constraint ensuring "profileAId" <> "profileBId" (so matches where both profile IDs are identical are rejected), and for the DatingInteraction table add a constraint ensuring "fromProfileId" <> "toProfileId" (so interactions to the same profile are rejected). Update the migration that creates DatingMatchScore and DatingInteraction to include these CHECK constraints (referencing the tables and columns by name) and apply the same pattern to any other pair/interaction tables referenced elsewhere in the migration.packages/db/prisma/migrations/20260520043000_add_dating_foundation/migration.sql-416-443 (1)
416-443:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake pair uniqueness symmetric for match and score records.
Current unique indexes allow both
(A,B)and(B,A), which can duplicate the same logical pair.Suggested migration change
-CREATE UNIQUE INDEX "DatingMatchScore_profileAId_profileBId_key" ON "DatingMatchScore"("profileAId", "profileBId"); +CREATE UNIQUE INDEX "DatingMatchScore_profilePair_key" + ON "DatingMatchScore"(LEAST("profileAId", "profileBId"), GREATEST("profileAId", "profileBId")); @@ -CREATE UNIQUE INDEX "DatingMatch_profileAId_profileBId_key" ON "DatingMatch"("profileAId", "profileBId"); +CREATE UNIQUE INDEX "DatingMatch_profilePair_key" + ON "DatingMatch"(LEAST("profileAId", "profileBId"), GREATEST("profileAId", "profileBId"));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/db/prisma/migrations/20260520043000_add_dating_foundation/migration.sql` around lines 416 - 443, The unique indexes on DatingMatch ("DatingMatch_profileAId_profileBId_key") and DatingMatchScore ("DatingMatchScore_profileAId_profileBId_key") are asymmetric and allow both (A,B) and (B,A); change the migration to enforce pair uniqueness symmetrically by dropping those two unique indexes and replacing them with a single unique constraint on the ordered pair (e.g. create UNIQUE INDEX using expressions LEAST("profileAId","profileBId") and GREATEST("profileAId","profileBId") for both DatingMatch and DatingMatchScore, or add generated columns minProfileId/maxProfileId computed from profileAId/profileBId and create UNIQUE INDEX on (minProfileId, maxProfileId)); update index names accordingly (referencing DatingMatch_profileAId_profileBId_key and DatingMatchScore_profileAId_profileBId_key) so the same logical pair cannot be inserted in swapped order.packages/db/prisma/migrations/20260520030000_add_commerce_ledger/migration.sql-149-167 (1)
149-167:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEnforce positive
CommerceOrderItem.quantityat the database layer.Without a CHECK, zero/negative quantities can be persisted and break order totals and fulfillment logic.
Suggested migration change
CREATE TABLE "CommerceOrderItem" ( @@ - "quantity" INTEGER NOT NULL DEFAULT 1, + "quantity" INTEGER NOT NULL DEFAULT 1, @@ - CONSTRAINT "CommerceOrderItem_pkey" PRIMARY KEY ("id") + CONSTRAINT "CommerceOrderItem_quantity_positive_chk" CHECK ("quantity" > 0), + CONSTRAINT "CommerceOrderItem_pkey" PRIMARY KEY ("id") );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/db/prisma/migrations/20260520030000_add_commerce_ledger/migration.sql` around lines 149 - 167, Add a DB-level CHECK to prevent zero/negative quantities for CommerceOrderItem: update the CREATE TABLE for CommerceOrderItem to include a CHECK constraint on "quantity" (e.g., CONSTRAINT "CommerceOrderItem_quantity_positive" CHECK ("quantity" > 0)) alongside the other column constraints so inserts/updates cannot persist quantity <= 0; keep the existing DEFAULT 1 and ensure the new constraint is declared before the closing parenthesis of the CREATE TABLE statement.packages/db/prisma/schema.prisma-7502-7528 (1)
7502-7528:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftEnforce canonical pair uniqueness in the schema.
DatingMatchScoreandDatingMatchcurrently rely on writer ordering forprofileAIdandprofileBId. If one path writesA/Band another writesB/A, you can persist duplicate rows for the same pair and split matching state across both records. This should be enforced with a DB-level canonical pair key or equivalent constraint, ideally alongside a self-pair guard.Also applies to: 7561-7583
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/db/prisma/schema.prisma` around lines 7502 - 7528, The schema allows duplicate pair rows when writers flip A/B; for both DatingMatchScore and DatingMatch, add a DB-level canonical pair enforcement and self-pair guard: create a migration that (1) normalizes existing rows by merging/deduplicating pairs into the canonical ordering (ensure for each pair you keep the record where profileAId < profileBId or move data into that canonical row), (2) adds a check constraint like "profileAId <> profileBId AND profileAId < profileBId" to prevent self-pairs and enforce ordering at the DB level, and (3) ensure the existing @@unique([profileAId, profileBId]) index remains (or create a unique index on the canonical columns) for both DatingMatchScore (model DatingMatchScore, fields profileAId/profileBId) and DatingMatch (model DatingMatch, fields profileAId/profileBId) so future A/B flips cannot create duplicates.packages/db/prisma/schema.prisma-8078-8116 (1)
8078-8116:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd a DB-level idempotency key for entitlement grants.
Checkout, webhook, and fulfillment retries are normal.
CommerceEntitlementcurrently has no unique natural key, so a retried handler can grant the same entitlement more than once for a single purchase. Add a deterministicgrantKeyor similar unique field anchored to the source purchase + recipient before this ships.Possible schema shape
model CommerceEntitlement { id String `@id` `@default`(cuid()) + grantKey String `@unique` orderId String? orderItemId String? offerId String?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/db/prisma/schema.prisma` around lines 8078 - 8116, Add a deterministic, DB-enforced idempotency key to CommerceEntitlement by adding a new field (e.g., grantKey String) and enforcing uniqueness (either grantKey `@unique` or a composite @@unique if you prefer deterministic components). Ensure the grantKey is derived deterministically from the source purchase and recipient (for example: orderId or orderItemId + entitlementType + subjectUserId/subjectOrganizationId) so retries resolve to the same key, and add an index on grantKey to optimize lookups; also update any entitlement creation code to populate this field before inserting.packages/db/src/managed-data/managed-dating-catalog.ts-73-132 (1)
73-132:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftMake prompt/question sync atomic with
prisma.$transaction.A failure in the second loop can leave partially applied managed catalog data. Please wrap the full write sequence in one transaction.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/db/src/managed-data/managed-dating-catalog.ts` around lines 73 - 132, The syncManagedDatingCatalog function currently performs multiple upserts across MANAGED_DATING_PROMPTS and MANAGED_DATING_QUESTIONS individually, which can leave partial writes if one fails; wrap the entire sequence of prisma.datingPrompt.upsert and prisma.datingQuestion.upsert calls in a single Prisma transaction (using prisma.$transaction or the transaction callback form) so all upserts succeed or none are applied, ensure you collect/await all upsert promises inside that transaction and return the result only after the transaction completes, and keep existing identifiers (promptId, questionId, DatingQuestionStatus) unchanged.packages/db/src/managed-data/managed-commerce-catalog.ts-161-408 (1)
161-408:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftWrap catalog upserts in a single transaction.
This sync can persist partial state if one later upsert fails. Please execute the full offer/variant/mapping write path inside one
prisma.$transaction(...)so runs are atomic.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/db/src/managed-data/managed-commerce-catalog.ts` around lines 161 - 408, The syncManagedCommerceCatalog function currently performs multiple independent prisma.upsert calls (e.g., prisma.commerceOffer.upsert, prisma.commerceOfferVariant.upsert, prisma.commerceFulfillmentMapping.upsert) which can leave partial state; wrap the entire sequence of DB writes inside a single prisma.$transaction(...) call so they run atomically, keeping the preliminary dry-run early return (if !options.apply) intact, and ensure the function returns the same result only after the transaction completes successfully.packages/web/src/app/api/dating/date-plans/route.ts-38-45 (1)
38-45:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not return raw internal error messages as 400 responses.
This exposes server internals and marks unexpected failures as client errors. Return a generic 500 for unclassified
Errors (and log the original error server-side).Suggested fix
- if (error instanceof Error) { - return NextResponse.json({ error: error.message }, { status: 400 }); - } - console.error("[dating] Failed to propose date:", error); - return NextResponse.json( - { error: "Failed to propose dating date." }, - { status: 500 }, - ); + console.error("[dating] Failed to propose date:", error); + return NextResponse.json( + { error: "Failed to propose dating date." }, + { status: 500 }, + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/app/api/dating/date-plans/route.ts` around lines 38 - 45, The handler currently returns internal Error.message as a 400 response; change the error handling in the route (the block that checks "if (error instanceof Error)") to log the original error server-side (console.error or processLogger) and return a generic 500 NextResponse.json({ error: "Internal server error" }, { status: 500 }) instead of exposing error.message; ensure any other unclassified errors still follow the same pattern (log then return 500) so client-facing responses never include raw internal error details.packages/web/src/app/api/dating/questions/[questionId]/answer/route.ts-41-43 (1)
41-43:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid returning raw exception messages from answer submission.
This leaks internal backend details to clients. Return a stable generic error response and keep specifics in server logs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/app/api/dating/questions/`[questionId]/answer/route.ts around lines 41 - 43, The current catch branch in the answer submission route (where it checks "if (error instanceof Error)" and returns NextResponse.json({ error: error.message }, { status: 400 })) must not expose raw exception messages to clients; instead, log the full error server-side (use your logger or console.error) and return a stable generic JSON error to the client (e.g. { error: "Unable to submit answer" } with status 400 or 500 as appropriate) from the function handling the request in route.ts, keeping error.message only in logs and not in the response.packages/web/src/app/api/dating/questions/[questionId]/answer/route.ts-10-11 (1)
10-11:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReplace
z.any()with concrete/guarded boundary types.Using
z.any()here removes meaningful request validation at a real HTTP boundary. Define explicit schema types (orz.unknown()+ refinements) so malformed payloads fail early and predictably.As per coding guidelines: “TypeScript strict mode ON… No
any—use proper types orunknownwith guards.”Also applies to: 23-25
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/app/api/dating/questions/`[questionId]/answer/route.ts around lines 10 - 11, The schema in route.ts uses z.any() for acceptableValues and answerValues (also elsewhere lines 23-25) which bypasses HTTP-boundary validation; replace z.any() with concrete Zod types or z.unknown() plus refinements/transformers that validate the expected shapes (e.g., string, number, boolean, array, or object with specific keys) and add .optional()/.nullable() where appropriate; update the schema definitions referenced by acceptableValues and answerValues (and the other occurrences noted) to perform strong runtime checks and return early validation errors so malformed payloads fail before business logic runs.packages/web/src/app/api/dating/profile/photos/route.ts-29-31 (1)
29-31:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse client-safe error responses for photo upload failures.
Sending raw
error.messageto clients can leak internals. Keep response text generic and log detailed errors server-side.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/app/api/dating/profile/photos/route.ts` around lines 29 - 31, The current error branch in the photo upload route returns error.message to clients (the block checking "if (error instanceof Error)" and calling NextResponse.json), which may leak internals; change it to send a generic client-safe message like "Photo upload failed" with the same status (e.g., 400) and move the detailed error information to server-side logs by logging the full error via your server logger (e.g., processLogger.error or console.error) before returning the generic response. Ensure you update the "if (error instanceof Error)" branch only and keep the HTTP status handling unchanged.packages/web/src/app/api/dating/interactions/route.ts-31-33 (1)
31-33:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid echoing raw server errors in API responses.
This branch returns
error.messagedirectly to clients, which can leak internal details and make responses unstable across backend refactors. Return a generic message and log full details server-side instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/app/api/dating/interactions/route.ts` around lines 31 - 33, The error handling branch in route.ts currently returns error.message to clients; instead, log the full error server-side (e.g., console.error or your app logger) inside the error instanceof Error block and return a generic response body (e.g., { error: "Internal server error" }) with an appropriate HTTP status (500) so internal details are not leaked. Make the change in the error instanceof Error branch in the exported route handler.packages/web/src/app/api/dating/messages/route.ts-29-31 (1)
29-31:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not expose internal error messages from message send failures.
Returning
error.messageto callers can disclose internal logic/state. Prefer a fixed client-safe message and server-side logging.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/app/api/dating/messages/route.ts` around lines 29 - 31, The handler in route.ts currently returns internal error messages by using error.message when error instanceof Error; change this to return a fixed, client-safe message (e.g., "Failed to send message") via NextResponse.json instead of the raw error string, and record the real error server-side using the server logger or console.error for diagnostics; update the error branch that uses NextResponse.json to log the error and return the generic message while leaving other control flow unchanged.packages/web/src/app/api/dating/reports/route.ts-32-34 (1)
32-34:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t expose backend exception text in safety-report responses.
Returning
error.messagedirectly can reveal internals. Use a generic client message and log details on the server.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/app/api/dating/reports/route.ts` around lines 32 - 34, The handler in route.ts currently returns error.message when catching exceptions (the branch using "if (error instanceof Error)" and NextResponse.json), which can leak internals; change it to log the full error server-side (e.g., console.error or your app logger) and return a generic client-facing JSON like { error: "Unable to process request" } with the same 400 status; keep the error instanceof Error check to decide how to log details but do not include error.message in the response.packages/web/src/app/api/stripe/create-checkout/route.ts-211-266 (1)
211-266:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWrap pre-Stripe order creation in the same guarded error path.
Order creation/item extraction can throw before entering your current session-creation
try/catch, causing an uncaught route failure and inconsistent API response behavior. Move those DB/precondition steps into a guarded block that returns a controlled JSON error.Suggested shape
- const commerceOrder = await prisma.commerceOrder.create({ ... }); - const commerceOrderItem = commerceOrder.items[0]; - if (!commerceOrderItem) { - throw new Error("Created shirt commerce order without an item."); - } - metadata.commerce_order_id = commerceOrder.id; - metadata.commerce_order_item_id = commerceOrderItem.id; + let commerceOrder; + try { + commerceOrder = await prisma.commerceOrder.create({ ... }); + const commerceOrderItem = commerceOrder.items[0]; + if (!commerceOrderItem) { + return NextResponse.json({ error: "Failed to prepare order." }, { status: 500 }); + } + metadata.commerce_order_id = commerceOrder.id; + metadata.commerce_order_item_id = commerceOrderItem.id; + } catch (error) { + log.error("Failed to prepare commerce order", error); + return NextResponse.json({ error: "Failed to prepare checkout." }, { status: 500 }); + }Also applies to: 407-455
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/app/api/stripe/create-checkout/route.ts` around lines 211 - 266, The DB pre-checks and order creation (prisma.commerceOrder.create) and subsequent commerceOrder.items[0] extraction must be moved into a guarded try/catch so any error is caught and you return a controlled JSON error response instead of letting the route fail; wrap the block that creates commerceOrder and reads commerceOrderItem in a try block, catch errors and return a consistent NextResponse.json({ error: "...", details: ... }, { status: 500 }) (or your existing error shape) and ensure you bail out before creating the Stripe session; apply the same pattern to the similar pre-Stripe DB logic around the other block mentioned (the 407-455 area) so all pre-session DB work uses the same guarded error path.packages/web/src/app/love/dating/dating-client.tsx-389-391 (1)
389-391:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDo not expose user emails in public dating cards.
Line 390 falls back to
candidate.user.email, which leaks PII and bypasses the required public identity formatting path.Use the user-display helper flow (or a neutral fallback label) and stop passing raw email to this UI surface unless explicitly required for a private/admin context.
As per coding guidelines:
Person owns every public-facing identity field... Display reads go through getUserDisplayName/Handle/Avatar/Href/Label from@/lib/user-display``.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/app/love/dating/dating-client.tsx` around lines 389 - 391, The heading is currently leaking raw emails by falling back to candidate.user.email; instead route all public-facing identity reads through the user-display helpers. Replace the fallback and any direct use of candidate.user.email in the dating card with getUserDisplayName / getUserHandle / getUserLabel (from the user-display helper) and/or a neutral fallback like "Member" so that candidate.user.person is the primary source and raw emails are never rendered in this UI; update the code paths that produce the name (the h2 text rendering around candidate.user.person?.displayName) to call the appropriate helper and pass the person/user object to it.packages/web/src/app/love/dating/matches/page.tsx-14-18 (1)
14-18:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not render raw server error messages to users.
This path propagates
error.messagedirectly into UI. If upstream throws DB/provider/internal errors, you leak internals to end users.Return a generic user-safe message from
loadMatchesand log the original error server-side.Also applies to: 49-51
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/app/love/dating/matches/page.tsx` around lines 14 - 18, The catch blocks in loadMatches currently return error.instanceof Error ? error.message, which leaks internal error details to the UI; change both catch handlers (the one around the main fetch and the one at lines referenced as 49-51) to return a generic user-safe message like "Could not load matches." while logging the original error server-side using your server logger (e.g., call processLogger.error or console.error with the full error and context) so the UI never receives raw error.message but you still retain the original error for debugging.packages/web/src/app/developers/page.tsx-17-31 (1)
17-31: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winUse route metadata helper instead of hardcoded metadata in page files.
This page defines
metadatainline, but page metadata inapp/**/page.tsxshould come fromgetRouteMetadata()for consistency and centralized route SEO behavior.As per coding guidelines:
packages/web/src/app/**/{page,layout}.{ts,tsx}: Use getRouteMetadata() for page metadata instead of hardcoding page titles.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/app/developers/page.tsx` around lines 17 - 31, The page currently exports a hardcoded metadata object named "metadata"; replace this with a call to the shared helper getRouteMetadata() so pages use centralized SEO defaults. Update the export to obtain metadata via getRouteMetadata({ title: "Developers | Optimitron", description: "Connect AI agents to the live Optimitron task graph so they can take the highest-value action to optimize Earth.", alternates: { canonical: "/developers" }, openGraph: { title: "Developers | Optimitron", description: "...", url: "/developers" } }) (or pass only title/slug if the helper composes the rest) and remove the inline metadata object; ensure the symbol getRouteMetadata is imported where metadata is currently declared and that the exported binding remains named "metadata".packages/web/src/app/api/stripe/webhook/route.ts-143-167 (1)
143-167:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftMake fulfillment creation idempotent under concurrent webhook retries.
Line 143 + Line 152 use a check-then-create pattern that is race-prone. Two concurrent deliveries can both miss
findFirstand create duplicate manual fulfillments.Suggested direction
- const existingFulfillment = await prisma.commerceFulfillment.findFirst({ ... }); - if (existingFulfillment) return; - - await prisma.commerceFulfillment.create({ ... }); + await prisma.$transaction(async (tx) => { + // Prefer enforcing a DB unique constraint (e.g. provider + externalOrderId + deletedAt null strategy) + // then use upsert/insert-on-conflict semantics for true idempotency. + const existingFulfillment = await tx.commerceFulfillment.findFirst({ ... }); + if (existingFulfillment) return; + await tx.commerceFulfillment.create({ ... }); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/app/api/stripe/webhook/route.ts` around lines 143 - 167, The current check-then-create using prisma.commerceFulfillment.findFirst and prisma.commerceFulfillment.create is race-prone; make fulfillment creation idempotent by relying on a uniqueness constraint and an atomic operation: add (or ensure) a unique DB constraint/index on the fulfillment keys (e.g., externalOrderId + provider or externalOrderId + provider + orderItemId), then replace the findFirst/create pattern with an atomic upsert (or a plain create wrapped in a transaction that catches unique-constraint violations) on prisma.commerceFulfillment so concurrent webhooks cannot insert duplicates; if using Prisma, handle Prisma.PrismaClientKnownRequestError (P2002) to treat duplicate insert attempts as no-ops.packages/web/src/app/store/store-client.tsx-58-64 (1)
58-64:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate contribution amount before checkout request.
Line [58] accepts invalid numeric states (e.g., empty input =>
NaN) and still posts to checkout. Add finite/range validation againstminAmountCents/maxAmountCentsbeforefetch.Suggested fix
async function handleCheckout() { setError(null); setState("loading"); try { + const parsedAmount = Number.parseFloat(amount); + const minAmount = minAmountCents / 100; + const maxAmount = maxAmountCents ? maxAmountCents / 100 : null; + if ( + !Number.isFinite(parsedAmount) || + parsedAmount < minAmount || + (maxAmount !== null && parsedAmount > maxAmount) + ) { + throw new Error("Enter a valid contribution amount."); + } + const response = await fetch("/api/stripe/create-checkout", { body: JSON.stringify({ - custom_amount: Number.parseFloat(amount), + custom_amount: parsedAmount, offer_key: offerKey, order_type: "store_offer",Also applies to: 111-119
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/app/store/store-client.tsx` around lines 58 - 64, Validate the contribution amount before sending the checkout POST: parse the input into a numeric value and ensure Number.isFinite(amount) and amount (or amountInCents if your code uses cents) falls between minAmountCents and maxAmountCents; if the value is invalid, short-circuit the checkout call (do not call fetch) and surface an error/return early. Update the sections that build the payload (the object using custom_amount, offer_key, order_type, sourceReferrer, sourceUrl, variant_key) and the other occurrence around lines 111-119 to perform this validation, converting to cents consistently and only passing a valid custom_amount to the backend. Ensure you use the existing minAmountCents and maxAmountCents variables and treat empty/NaN inputs as invalid.packages/web/src/app/shirt/shirt-client.tsx-169-171 (1)
169-171:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate custom amount before calling checkout API.
Line [169] can send
NaN(or invalid values) when the custom field is empty/invalid. Reject client-side before POST so users get immediate feedback and you avoid bad checkout requests.Suggested fix
async function handleSubmit() { setError(null); setIsSubmitting(true); try { + const parsedCustomAmount = + tier === "custom" ? Number.parseFloat(customAmount) : undefined; + if ( + tier === "custom" && + (!Number.isFinite(parsedCustomAmount) || parsedCustomAmount < 25) + ) { + throw new Error("Enter a valid custom total of at least $25."); + } + const response = await fetch("/api/stripe/create-checkout", { body: JSON.stringify({ - custom_amount: - tier === "custom" ? Number.parseFloat(customAmount) : undefined, + custom_amount: parsedCustomAmount, donation_tier: tier, handle, order_type: "shirt",Also applies to: 242-252
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/app/shirt/shirt-client.tsx` around lines 169 - 171, The code currently constructs custom_amount using Number.parseFloat(customAmount) (when tier === "custom") which can produce NaN or invalid values; before calling the checkout POST (where donation_tier and custom_amount are sent) validate customAmount client-side: parse it to a number, ensure Number.isFinite(value) and value > 0 (and enforce any app max if needed), and if invalid set a user-visible error state and abort the request. Update the validation logic where custom_amount is built (references: custom_amount, customAmount, tier, donation_tier) and apply the same checks to the other POST location mentioned (lines ~242-252) so no NaN/invalid values are sent to the API.packages/web/src/lib/customcat.server.ts-61-67 (1)
61-67:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not return payloads containing
api_key.
payloadcurrently includes the CustomCat API token and is returned to callers. That creates a direct secret-leak path through logs/telemetry/response passthrough.🔐 Suggested fix
+function redactCustomCatPayload(payload: Record<string, unknown>) { + const { api_key: _apiKey, ...safePayload } = payload; + return safePayload; +} ... - const result = parseOrderResult(parsedResponse, payload); + const result = parseOrderResult(parsedResponse, redactCustomCatPayload(payload)); ... - payload, + payload: redactCustomCatPayload(payload),Also applies to: 114-116, 298-320, 436-442, 476-490, 502-516
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/lib/customcat.server.ts` around lines 61 - 67, The CustomCatOrderResult.payload currently contains the CustomCat API token and must not be returned or logged; update code that constructs or returns CustomCatOrderResult (symbol: CustomCatOrderResult and any functions that build/return it) to sanitize the payload by cloning it and deleting the "api_key" (and any other secret keys) before assigning to the payload property or before logging/returning; ensure the type remains Record<string, unknown> but the returned object never includes api_key (apply the same sanitization at all other payload build sites referenced in the comment).packages/web/src/lib/env.ts-115-115 (1)
115-115: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winStandardize the boolean toggle to one string format.
SHIRT_COMMERCE_ENABLEDaccepts four different string values ("0","1","false","true") for what appears to be a boolean toggle. This creates ambiguity and forces consuming code to check multiple conditions. All four strings are truthy in JavaScript (non-empty strings), soif (env.SHIRT_COMMERCE_ENABLED)would be true for"0"and"false".Pick one convention: either
z.enum(["0", "1"])(matchesCUSTOMCAT_SANDBOXbelow and is common for env vars) orz.enum(["false", "true"]), or use a transform to normalize to boolean.♻️ Recommended standardization
- SHIRT_COMMERCE_ENABLED: z.enum(["0", "1", "false", "true"]).optional(), + SHIRT_COMMERCE_ENABLED: z.enum(["0", "1"]).optional(),Then consuming code can use:
env.SHIRT_COMMERCE_ENABLED === "1".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/lib/env.ts` at line 115, SHIRT_COMMERCE_ENABLED currently allows ambiguous values ("0","1","false","true"); change its schema to a single consistent convention (e.g., match CUSTOMCAT_SANDBOX and use z.enum(["0","1"]) or switch to z.enum(["false","true"]) or apply a z.preprocess/z.transform to convert to a boolean). Update the declaration of SHIRT_COMMERCE_ENABLED in the env schema so consuming code can reliably check a single form (for example check env.SHIRT_COMMERCE_ENABLED === "1" if using "0"/"1" or treat it as a boolean if transformed). Ensure the symbol name SHIRT_COMMERCE_ENABLED is the one modified and adjust any tests or usage sites that rely on the previous multi-format values.packages/web/src/lib/shirt-fulfillment.server.ts-177-179 (1)
177-179:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle both
order_typeandorderTypebefore skipping fulfillment.The guard only checks
order_type, so sessions tagged withorderTypewill be skipped and never fulfilled.Proposed fix
- if (session.metadata?.order_type !== "shirt") { + const orderType = session.metadata?.order_type ?? session.metadata?.orderType; + if (orderType !== "shirt") { return { skipped: true as const }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/lib/shirt-fulfillment.server.ts` around lines 177 - 179, The guard in the shirt fulfillment flow currently checks only session.metadata?.order_type and returns skipped; update the condition used in the fulfillment entry point (the code that contains session.metadata checks in shirt-fulfillment.server.ts) to also consider session.metadata?.orderType (or normalize metadata first, e.g., const orderType = metadata?.order_type ?? metadata?.orderType) so that both snake_case and camelCase keys are handled before returning { skipped: true }. Ensure the same normalized variable is used for subsequent logic that assumes the order type.packages/web/src/lib/shirt-fulfillment.server.ts-109-124 (1)
109-124:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid ambiguous
findFirstlookup across different order identifiers.Using
ORwithfindFirstcan select the wrong order ifcommerce_order_idandstripeCheckoutSessionIdpoint to different rows, which risks fulfilling against the wrong order.Proposed fix
async function findCommerceOrder(session: Stripe.Checkout.Session) { - const predicates: Prisma.CommerceOrderWhereInput[] = [ - { stripeCheckoutSessionId: session.id }, - ]; - if (session.metadata?.commerce_order_id) { - predicates.unshift({ id: session.metadata.commerce_order_id }); - } - - return prisma.commerceOrder.findFirst({ - include: { items: true }, - where: { - OR: predicates, - deletedAt: null, - }, - }); + const byMetadataId = session.metadata?.commerce_order_id + ? await prisma.commerceOrder.findFirst({ + include: { items: true }, + where: { id: session.metadata.commerce_order_id, deletedAt: null }, + }) + : null; + + if (byMetadataId) return byMetadataId; + + return prisma.commerceOrder.findFirst({ + include: { items: true }, + where: { stripeCheckoutSessionId: session.id, deletedAt: null }, + }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/lib/shirt-fulfillment.server.ts` around lines 109 - 124, The current findCommerceOrder uses OR with prisma.commerceOrder.findFirst which can return a mismatched row; change it to perform a deterministic lookup: if session.metadata?.commerce_order_id exists, first fetch by id (use prisma.commerceOrder.findUnique or findFirst with where: {id: ... , deletedAt: null}) and verify its stripeCheckoutSessionId equals session.id (if not, surface/log/throw an error); otherwise, if no commerce_order_id, fetch by stripeCheckoutSessionId only (where: { stripeCheckoutSessionId: session.id, deletedAt: null }); update the function findCommerceOrder to use these explicit queries instead of OR to avoid ambiguous matches.packages/web/src/lib/store-commerce.server.ts-23-25 (1)
23-25:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
offerKeycamelCase requests are rejected even though alias support is intended.
offer_keyis required whileofferKeyis optional, so a camelCase-only payload fails before parsing fallback runs.Proposed fix
-const storeOfferCheckoutSchema = z.object({ +const storeOfferCheckoutSchema = z + .object({ amount: z.number().finite().positive().optional(), custom_amount: z.number().finite().positive().optional(), customAmount: z.number().finite().positive().optional(), - offer_key: z.string().trim().min(1).max(120), - offerKey: z.string().trim().min(1).max(120).optional(), + offer_key: z.string().trim().min(1).max(120).optional(), + offerKey: z.string().trim().min(1).max(120).optional(), order_type: z.literal(STORE_ORDER_TYPE).optional(), orderType: z.literal(STORE_ORDER_TYPE).optional(), sourceReferrer: z.string().optional(), sourceUrl: z.string().optional(), variant_key: z.string().trim().min(1).max(120).optional(), variantKey: z.string().trim().min(1).max(120).optional(), -}); +}) + .refine((data) => Boolean(data.offer_key ?? data.offerKey), { + message: "offer_key is required", + path: ["offer_key"], + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/lib/store-commerce.server.ts` around lines 23 - 25, The schema currently requires offer_key but only optionally accepts offerKey, causing payloads that provide only camelCase to fail; update the schema so both offer_key and offerKey are accepted as optional and normalize them into a single canonical value during parsing (e.g., make offer_key: z.string().trim().min(1).max(120).optional() and keep offerKey: z.string().trim().min(1).max(120).optional(), then in the surrounding Zod object use a preprocess or superRefine step to set the canonical value from offerKey when offer_key is missing); reference the existing symbols offer_key and offerKey in your changes so the parser accepts either form and falls back correctly.
🟡 Minor comments (7)
packages/web/docs/customcat-integration.md-17-17 (1)
17-17:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix the sentence fragment for clarity.
“Cannot be re-displayed; if lost, generate a new one.” should include an explicit subject (e.g., “The token cannot be re-displayed…”).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/docs/customcat-integration.md` at line 17, Update the fragment sentence describing the API token so it has an explicit subject: replace "Cannot be re-displayed; if lost, generate a new one." with a full sentence such as "The token cannot be re-displayed; if it is lost, generate a new one." Locate the sentence beginning "Single UUID-format token per API Store. Generated once in CustomCat dashboard → API → "Generate API Token"." and change the fragment "Cannot be re-displayed; if lost, generate a new one." to the suggested complete sentence (or similar wording) so the paragraph reads clearly.packages/web/src/app/store/[offerKey]/page.tsx-20-23 (1)
20-23:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHandle
0cents explicitly in price formatting.Line [21] uses a falsy check, so
0returnsnull. That mislabels valid zero-cost amounts as missing.Suggested fix
function formatDollars(cents: number | null | undefined) { - if (!cents) return null; + if (cents == null) return null; return `$${Math.round(cents / 100).toLocaleString("en-US")}`; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/app/store/`[offerKey]/page.tsx around lines 20 - 23, The formatDollars function treats 0 as falsy and returns null for zero-priced items; change the falsy check to an explicit null/undefined check (e.g., cents == null) so only null/undefined returns null and 0 is formatted as "$0". Update the guard in formatDollars to only short-circuit when cents is null or undefined and keep the existing rounding/formatting logic for numeric values.packages/web/src/app/store/page.tsx-12-15 (1)
12-15:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPrice formatter incorrectly treats
0as missing.Line [13] returns
nullfor0, so free items won’t render as$0.Suggested fix
function formatDollars(cents: number | null | undefined) { - if (!cents) return null; + if (cents == null) return null; return `$${Math.round(cents / 100).toLocaleString("en-US")}`; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/app/store/page.tsx` around lines 12 - 15, The price formatter formatDollars currently treats 0 as falsy and returns null for free items; change the null check to only catch null/undefined (e.g., use cents == null or cents === null || cents === undefined) so 0 is formatted as "$0" while still returning null for missing values, leaving the rest of the function (Math.round/cents/100 and toLocaleString) unchanged.packages/web/src/app/store/store-client.tsx-46-48 (1)
46-48:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDon’t use truthy checks for numeric variant prices.
Line [46] skips updating amount when
unitAmountCentsis0. Use!= nullinstead so zero-valued variants are handled correctly.Suggested fix
- if (nextVariant?.unitAmountCents) { + if (nextVariant?.unitAmountCents != null) { setAmount(String(nextVariant.unitAmountCents / 100)); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/app/store/store-client.tsx` around lines 46 - 48, The current truthy check nextVariant?.unitAmountCents skips zero-priced variants; update the conditional in the component/function that calls setAmount so it tests for presence with != null (e.g., nextVariant?.unitAmountCents != null) instead of a truthy check, then call setAmount(String(nextVariant.unitAmountCents / 100)) to ensure 0 values are handled correctly.packages/web/src/app/store/page.tsx-29-44 (1)
29-44:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a fallback branch for unknown offer kinds.
If a new/invalid kind arrives,
getOfferKindLabelcurrently returnsundefinedand the UI label goes blank.Suggested fix
function getOfferKindLabel(kind: CommerceOfferKind) { switch (kind) { case CommerceOfferKind.PHYSICAL_GOOD: return "Thing"; @@ case CommerceOfferKind.DONATION: return "Donation"; + default: + return "Offer"; } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/app/store/page.tsx` around lines 29 - 44, The getOfferKindLabel function can return undefined for unknown CommerceOfferKind values, causing blank UI; add a fallback/default branch in getOfferKindLabel that handles unexpected kinds (e.g., return a safe label like "Unknown" or the raw kind string) so the UI always shows a meaningful label; update the switch over CommerceOfferKind to include a default case that returns this fallback label (optionally log or assert the unexpected kind for debugging).packages/web/src/lib/customcat.server.ts-420-423 (1)
420-423:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winValidate shipping-quote
catalogSkuthe same way as order submission.This path currently forwards unvalidated SKU values; reusing
parseCustomCatIntegerStringavoids bad upstream input reaching CustomCat.🧩 Suggested fix
items: input.items.map((item) => ({ - catalog_sku: String(item.catalogSku), + catalog_sku: parseCustomCatIntegerString( + item.catalogSku, + "CustomCat shipping quote catalog_sku", + ), quantity: item.quantity ?? 1, })),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/lib/customcat.server.ts` around lines 420 - 423, The items mapping forwards unvalidated catalogSku values; update the mapping in the CustomCat order/quote builder to run each item.catalogSku through parseCustomCatIntegerString (the same validator used for order submission), ensure the result is used (converted to String if needed) for catalog_sku, and handle invalid/undefined parse results consistently (e.g., skip item or throw/return an error) so invalid SKU input is not forwarded to CustomCat.packages/web/src/lib/dating.server.ts-467-473 (1)
467-473:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReject empty message bodies after trimming.
Whitespace-only inputs currently persist as empty messages.
✉️ Suggested fix
+ const body = input.body.trim().slice(0, 4000); + if (!body) { + throw new Error("Message body is required."); + } + const message = await prisma.datingMessage.create({ data: { - body: input.body.trim().slice(0, 4000), + body, conversationId: conversation.id, senderProfileId: profile.id, }, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/lib/dating.server.ts` around lines 467 - 473, Before calling prisma.datingMessage.create, validate input.body after trimming and reject if empty: compute trimmed = input.body.trim(), if trimmed.length === 0 throw or return a validation error (so no whitespace-only messages are created) and otherwise use trimmed.slice(0, 4000) for the body passed to prisma.datingMessage.create; reference input.body and prisma.datingMessage.create (and existing conversation.id / profile.id) so the check sits immediately above the create call.
🧹 Nitpick comments (2)
packages/web/src/app/shirt/page.tsx (1)
214-215: ⚡ Quick winExtract env-flag parsing instead of inline magic literals.
Line [214] and Line [215] duplicate string checks (
"1","true"). Move this to a shared boolean parser (or typed env accessor) and reuse it to avoid drift across pages.As per coding guidelines,
**/*.{ts,tsx}should use enums instead of magic strings in TypeScript code.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/app/shirt/page.tsx` around lines 214 - 215, Replace the inline literal comparisons for serverEnv.SHIRT_COMMERCE_ENABLED used to compute shirtCommerceEnabled with a reusable typed boolean parser (e.g., a function parseEnvBoolean or getEnvFlag) and an enum of accepted truthy values; update the code to call that helper (pass serverEnv.SHIRT_COMMERCE_ENABLED into parseEnvBoolean) and replace any other occurrences of direct string checks so all pages use the shared parser instead of repeating `"1"`/`"true"` magic strings.packages/web/src/lib/__tests__/shirt-fulfillment.server.test.ts (1)
76-103: ⚡ Quick winReplace
as anysession casts with a minimal typed fixture.These casts hide shape drift in the webhook/session contract. Prefer a small local interface (or
unknown+ guard) for the test payloads.As per coding guidelines:
TypeScript strict mode ON... No any—use proper types or unknown with guards.Also applies to: 161-165, 178-206
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/lib/__tests__/shirt-fulfillment.server.test.ts` around lines 76 - 103, Replace the broad "as any" cast on the test Stripe session payload passed to fulfillShirtCheckoutSession with a small, explicit fixture type (or use unknown plus a minimal runtime type guard) that models only the fields your test uses (e.g., id, customer_details.email/name/phone, metadata.donation_amount_cents/handle/order_type/shirt_color/shirt_size/userId, shipping_details.address.*, shipping_details.name). Update the test to construct the payload as that typed fixture (or assert via the guard) and pass it into fulfillShirtCheckoutSession so TypeScript catches contract drift instead of hiding it with any; apply the same change to the other occurrences noted in the file.
qa-passed: targeted shirt-back-image test, web typecheck:fast, and git diff --check passed. todo-skipped: CI stability fix only.
qa-passed: site-variant-ui targeted test, web typecheck:fast, and git diff --check passed. todo-skipped: CI-only test contract fix.
Summary
/storeroutes plus/shirt, Stripe checkout creation, CustomCat shirt fulfillment helpers, and webhook completion for shirt and generic store offers./love/datingpages.Deployment notes
pnpm db:sync:managed-data -- --apply, then deploys the prebuilt artifact.Reviewed pages
Local desktop/mobile screenshots were captured and inspected for the edited surfaces. Preview pages to review once the deployment is available:
/store?logout=1/store/flyer-run-sponsorship?logout=1/shirt?logout=1/love?logout=1/love/dating?logout=1/love/dating?login=demo/love/dating/profile?login=demo/love/dating/questions?login=demo/love/dating/discover?login=demo/love/dating/matches?login=demo/developers?logout=1Verification
git diff --cached --checkpnpm --filter @optimitron/db run prisma:validatepnpm --filter @optimitron/db test -- src/__tests__/zod-validators.test.tspnpm --filter @optimitron/web test -- src/app/api/stripe/create-checkout/route.test.ts src/app/api/stripe/webhook/route.test.ts src/lib/__tests__/customcat.server.test.ts src/lib/__tests__/shirt-fulfillment.server.test.ts src/lib/__tests__/shirt-commerce.server.test.ts src/lib/__tests__/shirt-back-image.server.test.ts src/lib/__tests__/dating.server.test.tspnpm --filter @optimitron/web run typecheck:fastpnpm --filter @optimitron/web run buildpnpm db:sync:managed-data --dry-runSummary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores