feat: add vitest test infrastructure and configuration (STARTUP-139)#50
feat: add vitest test infrastructure and configuration (STARTUP-139)#50
Conversation
- Pro dashboard with credit tracking and tool catalog - MCP server for AI agent integration - CLI tool for pro features - Cloudflare Workers backend for API integrations - AI agent skills documentation - Domain provider integrations (Porkbun, Namecheap) - Credit-based usage model with multiple plans Issues: STARTUP-139
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| const apiUser = process.env.DATAFORSEO_LOGIN; | ||
| const apiKey = process.env.DATAFORSEO_KEY; |
There was a problem hiding this comment.
🔴 process.env doesn't access Wrangler secrets in Cloudflare Workers — DataForSEO API never used
In workers/pro/src/keywords.ts:50-51 and workers/pro/src/seo.ts:52-53, DataForSEO credentials are accessed via process.env.DATAFORSEO_LOGIN and process.env.DATAFORSEO_KEY. However, in Cloudflare Workers, Wrangler secrets and bindings are only accessible through the env parameter passed to the handler (i.e., c.env), not through process.env — even with the nodejs_compat flag. This means apiUser and apiKey will always be undefined, causing these functions to always fall back to mock data, making the real DataForSEO integration unreachable.
Was this helpful? React with 👍 or 👎 to provide feedback.
- Add UNIQUE constraint on (user_id, provider) for domain_provider_settings - Fix ON CONFLICT SQL in domains.ts to use (user_id, provider) - Use c.env for DataForSEO credentials instead of process.env - Fix credits /deduct endpoint to properly deduct from bonus_credits - Fix iOS installCount mapping (trackContentRating is age rating)
Deploying startupkit with
|
| Latest commit: |
3905d3f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://82f59d78.startupkit-975.pages.dev |
| Branch Preview URL: | https://startup-139-startupkit-pro.startupkit-975.pages.dev |
| } | ||
|
|
||
| export async function login(apiKey: string): Promise<User> { | ||
| const response = await axios.post(`${config.apiBaseUrl}/auth/login`, { apiKey }); |
There was a problem hiding this comment.
🔴 CLI auth endpoints hit wrong URL path — blocked by auth middleware
The CLI package builds auth URLs by prepending apiBaseUrl (default: https://pro.startupkit.com/api) to /auth/login, /auth/logout, and /auth/me, resulting in paths like /api/auth/login. However, the worker mounts auth routes at /auth (workers/pro/src/index.ts:26), not /api/auth. The /api/* prefix has the authMiddleware applied (workers/pro/src/index.ts:27), which requires a valid Bearer token. This means the login endpoint—which is supposed to be called before the user has a token—will be rejected with 401 by the auth middleware. Additionally, no route handler exists at /api/auth/*, so even if the middleware passed, the request would 404.
Prompt for agents
The CLI's apiBaseUrl defaults to 'https://pro.startupkit.com/api', so auth calls go to /api/auth/login, /api/auth/logout, /api/auth/me. But in the worker (workers/pro/src/index.ts), auth routes are mounted at '/auth' (line 26), not '/api/auth'. The /api/* prefix is protected by authMiddleware (line 27).
Two possible fixes:
1. In the worker, move auth routes under /api/auth and exempt login/register/logout from the auth middleware, OR
2. In the CLI's auth.ts, don't use apiBaseUrl for auth paths. Instead use a separate base URL without the /api suffix (e.g. config.apiBaseUrl.replace(/\/api$/, '') + '/auth/login').
Option 1 is cleaner: mount authRouter at '/api/auth' and apply the authMiddleware selectively (only to endpoints that need it like /me), or check the token manually in the auth router for /me.
Was this helpful? React with 👍 or 👎 to provide feedback.
| next: { revalidate: 0 } | ||
| }) | ||
| if (response.ok) { | ||
| return response.json() |
There was a problem hiding this comment.
🔴 Missing await on response.json() causes JSON errors to bypass try/catch
In getCreditsBalance() and getUsageHistory(), return response.json() is used without await. In an async function, return promise does not await the promise inside the function body, so if response.json() rejects (e.g., malformed JSON body), the rejection bypasses the catch block and propagates to the caller as an unhandled error. This would cause Next.js server-side rendering to fail instead of gracefully returning the null/[] fallback values.
| return response.json() | |
| return await response.json() |
Was this helpful? React with 👍 or 👎 to provide feedback.
| next: { revalidate: 0 } | ||
| }) | ||
| if (response.ok) { | ||
| return response.json() |
There was a problem hiding this comment.
🔴 Missing await on response.json() in getUsageHistory
Same issue as in getCreditsBalance(): return response.json() without await means JSON parse errors escape the try/catch block, causing the function to throw instead of returning the [] fallback.
| return response.json() | |
| return await response.json() |
Was this helpful? React with 👍 or 👎 to provide feedback.
| return c.json({ | ||
| balance: totalCredits, | ||
| used: usage?.used || 0, | ||
| total: totalCredits, | ||
| }); |
There was a problem hiding this comment.
🔴 Credits balance endpoint returns identical balance and total, making 'total' meaningless
The /balance endpoint at workers/pro/src/credits.ts:30-34 returns balance: totalCredits and total: totalCredits — the same value. Since user.credits and user.bonusCredits are decremented when tools are used (workers/pro/src/lib/credits.ts:37-40), totalCredits already represents the remaining balance, not the original allocation. The frontend at apps/home/src/app/pro/page.tsx:126 displays "of {total} total" which will always show the same number as the available balance. The total should represent the original allocation (remaining + used this month).
| return c.json({ | |
| balance: totalCredits, | |
| used: usage?.used || 0, | |
| total: totalCredits, | |
| }); | |
| return c.json({ | |
| balance: totalCredits, | |
| used: usage?.used || 0, | |
| total: totalCredits + (usage?.used || 0), | |
| }); |
Was this helpful? React with 👍 or 👎 to provide feedback.
| balance: data.balance, | ||
| used: data.used, | ||
| total: data.total, | ||
| available: data.balance - data.used, |
There was a problem hiding this comment.
🔴 MCP credits tool double-counts usage in available calculation
The creditsHandler computes available: data.balance - data.used. Since data.balance already represents the remaining credits after deductions (credits are decremented in the DB on each tool use), subtracting data.used double-counts the consumption. For example, if a user started with 20 credits and used 5, balance would be 15 and used would be 5, making available = 15 - 5 = 10 — but the user actually has 15 credits available.
| available: data.balance - data.used, | |
| available: data.balance, |
Was this helpful? React with 👍 or 👎 to provide feedback.
|
|
||
| return { | ||
| name: `${options.name}${ext}`, | ||
| available: Math.random() > 0.5, |
There was a problem hiding this comment.
🔴 Porkbun provider returns random availability instead of checking actual domain status
The Porkbun provider's search method fetches pricing data from the /pricing API endpoint (which does not return availability information), then sets available: Math.random() > 0.5 at line 55 even on a successful API response. Users who configure Porkbun as their domain provider and pay credits for domain searches will receive fabricated availability results. The Namecheap provider correctly checks availability via the namecheap.domains.check command, so this inconsistency is clearly a bug, not a placeholder.
Prompt for agents
The Porkbun provider at workers/pro/src/providers/porkbun.ts fetches the /pricing endpoint which only returns pricing data, not domain availability. The 'available' field is set to Math.random() > 0.5 on line 55, even for successful API responses.
To fix this, the search method should call the Porkbun domain check API (POST https://porkbun.com/api/json/v3/domain/checkAvailability/{domain}) for each domain, which returns actual availability status. The pricing data can still be used for the price fields. Each domain+extension combination needs a separate availability check request.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const user = await db.prepare('SELECT credits, bonus_credits FROM users WHERE id = ?').bind(userId).first<{ | ||
| credits: number; | ||
| bonus_credits: number; | ||
| }>(); | ||
|
|
||
| if (!user) { | ||
| return { success: false, creditsRemaining: 0 }; | ||
| } | ||
|
|
||
| const totalCredits = user.credits + user.bonus_credits; | ||
|
|
||
| if (totalCredits < amount) { | ||
| return { success: false, creditsRemaining: totalCredits }; | ||
| } | ||
|
|
||
| const transactionId = crypto.randomUUID(); | ||
| await db.prepare( | ||
| `INSERT INTO credit_transactions (id, user_id, amount, type, tool, description) | ||
| VALUES (?, ?, ?, 'debit', ?, ?)` | ||
| ).bind(transactionId, userId, -amount, tool, description || null).run(); | ||
|
|
||
| if (user.credits >= amount) { | ||
| await db.prepare('UPDATE users SET credits = credits - ? WHERE id = ?').bind(amount, userId).run(); | ||
| } else { | ||
| const fromBonus = amount - user.credits; | ||
| await db.prepare('UPDATE users SET credits = 0, bonus_credits = bonus_credits - ? WHERE id = ?').bind(fromBonus, userId).run(); | ||
| } | ||
|
|
||
| const updated = await db.prepare('SELECT credits, bonus_credits FROM users WHERE id = ?').bind(userId).first<{ | ||
| credits: number; | ||
| bonus_credits: number; | ||
| }>(); | ||
|
|
||
| return { | ||
| success: true, | ||
| creditsRemaining: (updated?.credits || 0) + (updated?.bonus_credits || 0), | ||
| }; |
There was a problem hiding this comment.
🔴 TOCTOU race condition in credit deduction allows overdraft
The deductCredits function in workers/pro/src/lib/credits.ts:15-51 performs a SELECT to check available credits, then separate INSERT/UPDATE statements to deduct them — all without a transaction. Two concurrent requests can both pass the balance check at line 26 (totalCredits < amount) before either has deducted credits, leading to both deducting and resulting in a negative balance. This is particularly concerning since every tool handler (trends, seo, keywords, domains, apps, research, chat) calls this function.
Prompt for agents
The deductCredits function in workers/pro/src/lib/credits.ts has a TOCTOU race condition: it reads the balance (line 15), checks it (line 26), then deducts (lines 31-41) in separate non-transactional statements.
Fix by wrapping the operations in a D1 batch or using a single atomic UPDATE that includes the balance check. For example, use an UPDATE with a WHERE clause that checks sufficient balance:
UPDATE users SET credits = credits - ? WHERE id = ? AND (credits + bonus_credits) >= ?
Then check the rows affected - if 0, insufficient credits. This makes the check-and-deduct atomic. The bonus credit split logic would need adjustment to work atomically as well.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
1 participant
Summary