feat: add vitest test infrastructure and configuration (STARTUP-139)#50
feat: add vitest test infrastructure and configuration (STARTUP-139)#50ian wants to merge 4 commits into
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.
🔴 Keywords handler uses process.env instead of Workers env bindings — always returns mock data
The fetchKeywords function at workers/pro/src/keywords.ts:69-70 reads API credentials via process.env.DATAFORSEO_LOGIN and process.env.DATAFORSEO_KEY. In Cloudflare Workers, environment bindings (secrets/vars) are accessed via c.env, not process.env — even with nodejs_compat enabled. Compare with the SEO handler (workers/pro/src/seo.ts:62-64) which correctly uses env.DATA_FOR_SEO_API_LOGIN. Additionally, the variable names are wrong: the Env interface and wrangler.toml define DATA_FOR_SEO_API_LOGIN/DATA_FOR_SEO_API_KEY, not DATAFORSEO_LOGIN/DATAFORSEO_KEY. This means the keywords endpoint will never use real DataForSEO data and will always fall back to mock/random data in production.
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: |
8c67476
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f55ab8c8.startupkit-975.pages.dev |
| Branch Preview URL: | https://startup-139-startupkit-pro.startupkit-975.pages.dev |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
- Fix auth route mismatch: mount authRouter at both /auth and /api/auth - Fix CLI auth URLs to use base URL without /api suffix - Add missing await on response.json() in pro dashboard - Fix credits /balance to return total = balance + used - Fix MCP credits tool double-counting (available = balance, not balance - used) - Fix Porkbun provider to use domain check API for real availability - Fix TOCTOU race condition in deductCredits using D1 batch for atomic deduction
| const result = await db.batch([ | ||
| db | ||
| .prepare( | ||
| `UPDATE users SET | ||
| credits = CASE WHEN credits >= ? THEN credits - ? ELSE 0 END, | ||
| bonus_credits = CASE WHEN credits >= ? THEN bonus_credits ELSE bonus_credits - (? - credits) END | ||
| WHERE id = ? AND (credits + bonus_credits) >= ?`, | ||
| ) | ||
| .bind(amount, amount, amount, amount, userId, amount), | ||
| db | ||
| .prepare( | ||
| `INSERT INTO credit_transactions (id, user_id, amount, type, tool, description) | ||
| VALUES (?, ?, ?, 'debit', ?, ?)`, | ||
| ) | ||
| .bind(transactionId, userId, -amount, tool, description || null), | ||
| ]); |
There was a problem hiding this comment.
🔴 Phantom debit transactions inserted on insufficient credits
The deductCredits function uses db.batch() which executes both the UPDATE and INSERT atomically. When the user has insufficient credits, the UPDATE's WHERE clause (credits + bonus_credits) >= ? doesn't match any rows (0 changes), but the INSERT into credit_transactions still succeeds unconditionally within the same batch. The code then checks rowsAffected === 0 and returns success: false, but the debit transaction record has already been persisted. Over time, this accumulates phantom debit records that corrupt the transaction history and could skew the /credits/history endpoint at workers/pro/src/credits.ts:45-69.
Prompt for agents
In workers/pro/src/lib/credits.ts, the deductCredits function uses db.batch() to execute an UPDATE and INSERT together. The problem is that when the UPDATE matches 0 rows (insufficient credits), the INSERT still runs, creating a phantom debit transaction.
The fix should ensure the INSERT only happens when the UPDATE succeeds. Two approaches:
1. Check credits before the batch: Query the user's credits first, and only execute the batch if they have enough. This still has a small race window but is simpler.
2. Separate the operations: First execute the UPDATE alone, check rowsAffected, and only INSERT the transaction record if the UPDATE succeeded.
Approach 2 is preferred:
- Execute the UPDATE with the WHERE clause
- Check result[0].meta.changes
- Only if changes > 0, then INSERT the credit_transaction record
- This ensures transaction records are only created for actual deductions
Was this helpful? React with 👍 or 👎 to provide feedback.
| app.route("/auth", authRouter); | ||
| app.route("/api/auth", authRouter); | ||
| app.use("/api/*", authMiddleware); |
There was a problem hiding this comment.
🔴 CLI whoami always fails: /auth/me route lacks auth middleware
The worker mounts auth routes at both /auth and /api/auth (workers/pro/src/index.ts:31-32), but the auth middleware only applies to /api/* (workers/pro/src/index.ts:33). The /auth/me endpoint at workers/pro/src/auth.ts:121 expects c.get("user") to be populated by the middleware, but since no middleware runs on /auth/*, user is always null and it returns 401.
The CLI's whoami function at packages/pro/src/lib/auth.ts:56-57 strips /api from the base URL and calls ${baseUrl}/auth/me — this hits the unprotected /auth/me route which always returns 401. The error handler then calls config.clear(), wiping the user's stored credentials. This means any whoami call will fail and log the user out.
Prompt for agents
The problem is a mismatch between the CLI's URL construction and the server's middleware configuration.
In workers/pro/src/index.ts, auth routes are mounted at /auth (unprotected) and /api/auth (protected by authMiddleware). The /auth/me endpoint needs the user to be set by auth middleware, but /auth/* routes don't go through it.
The CLI (packages/pro/src/lib/auth.ts) strips /api from the base URL for auth endpoints, meaning it calls /auth/me instead of /api/auth/me.
Fix options:
1. Change the CLI's whoami function to NOT strip /api, so it calls /api/auth/me which goes through auth middleware. But this would also affect login/register which need to be unprotected.
2. Better approach: In the worker, add auth middleware specifically for /auth/me. For example, split the auth router into public routes (register, login, logout) and protected routes (me), and only apply middleware to the protected ones.
3. Alternatively, have the /auth/me handler manually parse the Authorization header and look up the user, bypassing the middleware pattern.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary