feat/auto-deploy #15
Merged
feat/auto-deploy #15
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Added new VSCode configuration files: extensions.json, settings.json, and tasks.json. - Updated .gitignore to include new VSCode settings files for better project management. - Configured tasks for starting frontend and backend services, along with worker services for streamlined development.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
lumpinif
changed the title
📝 WalkthroughWalkthroughAdds multi-mode auth (better-auth | jwt | none), playground device-scoped API key lifecycle (client + UI + server fallbacks), JWT minting tooling and middleware, auth-worker RPC/type surface and Next route for session-by-api-key, env/JSONC sync tooling, Upstash rate-limit refactor/flags, branding/runtime utilities, and many UI/tooling updates. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client (Browser/SDK)
participant NextAPI as Next API Route
participant AuthWorker as Auth Worker (RPC)
participant BetterAuthHTTP as Better‑Auth HTTP
Client->>NextAPI: POST /api/auth/getSessionWithAPIKey (apiKey in body/header/Bearer)
NextAPI->>AuthWorker: getSessionWithAPIKey(apiKey) [if binding present]
alt AuthWorker returns session
AuthWorker-->>NextAPI: Session
else missing or errored
NextAPI->>BetterAuthHTTP: GET /get-session with x-api-key / Authorization
BetterAuthHTTP-->>NextAPI: Session or error
end
NextAPI-->>Client: JSON response (session or error)
sequenceDiagram
participant Browser as Client
participant Worker as Hono Worker
participant Crypto as HS256 Verify
participant Context as Worker Context
Browser->>Worker: Request with Authorization: Bearer <token>
Worker->>Worker: if AUTH_MODE == 'jwt' then proceed else next
Worker->>Crypto: verify(token, JWT_SECRET, iss, aud)
Crypto-->>Worker: payload or throws
alt verified
Worker->>Context: build Session from payload
Context-->>Browser: continue (session attached)
else invalid
Worker-->>Browser: log warning and continue
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Comment |
There was a problem hiding this comment.
Pull request overview
This PR adds VSCode workspace configuration files to improve the development experience by providing pre-configured tasks and editor settings. The changes standardize the development environment for all contributors by configuring Biome as the default formatter and providing convenient task runners for common development workflows.
Changes:
- Added VSCode tasks for running development servers (frontend, backend, and combined)
- Configured Biome as the default formatter with auto-format on save and code actions
- Added VSCode extension recommendation for Biome
- Updated .gitignore to include VSCode configuration files while allowing local overrides
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| .vscode/tasks.json | Defines development tasks for starting the Next.js dashboard, Cloudflare workers, and type generation utilities |
| .vscode/settings.json | Configures Biome as the default formatter with organize imports and fix-on-save enabled |
| .vscode/extensions.json | Recommends the Biome VSCode extension for consistent tooling |
| .gitignore | Allows VSCode configuration files to be committed while preserving the ability to have local overrides via settings.local.json |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Added a new POST route to handle session retrieval using an API key. - The route checks for the API key in the request body or headers and returns an error if missing. - Utilizes the getAuth function to fetch the session based on the provided API key.
- Included the isNull function in the index export of the database module for enhanced query capabilities.
- Introduced a new type-only RPC surface for the auth worker service. - Defined methods for session retrieval and API key cache management without runtime code dependencies.
- Added support for multiple authentication modes: `better-auth`, `jwt`, and `none`. - Updated environment variable configuration to include JWT settings. - Implemented JWT authentication middleware for session management. - Enhanced README documentation to clarify authentication modes and JWT usage. - Refactored existing authentication logic to accommodate new modes.
- Introduced a new script for minting JWTs, allowing users to generate tokens with customizable claims. - Added a CLI command `jwt:mint` to the package.json for easy access to the minting functionality. - The script includes prompts for required inputs and options to write JWT secrets to environment variable files.
- Added functionality to prompt users for writing JWT secrets to environment variable files. - Introduced dividers for improved output formatting in the console. - Removed the old `maybeWriteEnvFiles` function, streamlining the process of updating environment variables directly after generating the JWT secret.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@apps/app/app/api/auth/getSessionWithAPIKey/route.ts`:
- Around line 5-25: The authorization header extraction currently uses
replace('Bearer ', '') which will accept non‑Bearer schemes; update the apiKey
parsing logic so you only accept a Bearer token: read
request.headers.get('authorization'), check that it's a string and
startsWith('Bearer '), then extract the token via substring or split and trim;
fall back to body?.apiKey and x-api-key only if the extracted token is a
non-empty string. Ensure the final apiKey variable is validated as a non-empty
string before calling headers.set('x-api-key', apiKey) and auth.api.getSession
so you never pass invalid header values downstream.
In `@apps/workers/v0/src/middlewares/api-key-auth.hono.ts`:
- Around line 60-68: The code calls c.env.BETTER_AUTH_URL.replace(...) without
checking for undefined which will throw if the env var is missing; update the
middleware (where baseUrl is computed) to first check that c.env.BETTER_AUTH_URL
is a non-empty string, log a clear warning via the existing logger if it's
missing, and only call .replace() when present; if missing, fall back to the
existing no-auth path (do not attempt to build the Request to
`${baseUrl}/getSessionWithAPIKey`) so the API-key auth branch gracefully skips
instead of throwing.
In `@scripts/jwt-mint.ts`:
- Around line 75-195: The promptInputs function writes JWT-related env updates
(updates, upsertEnvFile) before interactive prompts for issuer/audience, so
values entered by the user aren’t persisted; fix by moving the env update logic
(construction of updates, devVarsPath/prodVarsPath writes via upsertEnvFile and
writeDev/writeProd prompts) to after the interactive prompts for issuer and
audience (or update the updates map after reading issuer/audience) so that
prompted issuer/audience from the rl questions are included when updating
.dev.vars/.dev.vars.production; update references: promptInputs, updates,
devVarsPath, prodVarsPath, writeDev, writeProd, upsertEnvFile.
🧹 Nitpick comments (10)
apps/workers/v0/README.md (1)
189-191: Add language specifier to the fenced code block.The code block is missing a language identifier, which triggers a markdownlint warning and may affect syntax highlighting.
📝 Proposed fix
-``` +```text Authorization: Bearer <token></details> </blockquote></details> <details> <summary>apps/workers/v0/.dev.vars.example (1)</summary><blockquote> `7-12`: **Remove trailing whitespace on line 8.** Line 8 contains a space character rather than being a blank line, which may cause formatting inconsistencies. <details> <summary>📝 Proposed fix</summary> ```diff AUTH_MODE=better-auth - + JWT_SECRET=apps/workers/v0/src/services/analytics/activity-logger.service.ts (1)
47-60: Minor: Redundant null coalescing.
this.userId ?? nullis slightly redundant sincethis.userIdis already typed asstring | null. You could simplify touserId: this.userId, though the current form is harmless and makes the fallback explicit.📝 Optional simplification
const logData = { path, id: requestId, - userId: this.userId ?? null, + userId: this.userId, success,apps/workers/v0/src/middlewares/cookie-auth.hono.ts (1)
4-4: Consider metrics helpers for timing logs.The auth-mode guard looks good. Since duration is logged here, consider using
getMetrics()/formatDuration()for consistent performance tracking across the worker.As per coding guidelines: Use getMetrics() and formatDuration() utility functions from utils/metrics.ts for performance tracking.
Also applies to: 55-96
apps/workers/v0/src/middlewares/api-key-auth.hono.ts (1)
7-55: Auth-mode guard looks good; align timing with metrics helpers.Since this middleware logs durations, consider switching to
getMetrics()/formatDuration()for consistent performance tracking.As per coding guidelines: Use getMetrics() and formatDuration() utility functions from utils/metrics.ts for performance tracking.
scripts/jwt-mint.ts (2)
1-20: PreferinterfaceforInputs.
Inputsis an object shape and fits the interface guideline better.♻️ Suggested fix
-type Inputs = { +interface Inputs { secret?: string; sub?: string; email?: string; name?: string; issuer?: string; audience?: string; expiresInHours?: number; writeDevVars?: boolean; writeDevVarsProduction?: boolean; -}; +}As per coding guidelines: Use interfaces over types in TypeScript.
293-295: Avoidconsole.errorin repo scripts.Use a logging utility or write to stderr directly to align with the logging guideline.
♻️ Suggested fix
-run().catch((error) => { - console.error(`❌ Failed to mint JWT: ${error.message}`); - process.exit(1); -}); +run().catch((error) => { + process.stderr.write(`❌ Failed to mint JWT: ${error.message}\n`); + process.exit(1); +});As per coding guidelines: No console statements in production code - use custom logging utilities for development.
apps/workers/v0/src/middlewares/jwt-auth.hono.ts (2)
8-19: Preferinterfaceovertypefor object shapes.As per coding guidelines, use interfaces over types in TypeScript.
♻️ Suggested change
-type JwtPayload = { +interface JwtPayload { sub?: string; email?: string; name?: string; picture?: string; email_verified?: boolean; exp?: number; iat?: number; jti?: string; iss?: string; aud?: string | string[]; -}; +}
88-95: Simplify the redundant session existence check.The condition on lines 90-91 is redundant—if
c.get('session')is falsy, accessing.sessionor.useron it will fail. The first check alone is sufficient.♻️ Suggested simplification
- if ( - c.get('session') || - c.get('session')?.session || - c.get('session')?.user - ) { + if (c.get('session')) { logDebug('✅ Skipping [jwtAuthMiddleware] Session found'); return next(); }apps/workers/v0/scripts/worker-config-override.ts (1)
55-59: Static analysis ReDoS warning is low-risk in this context.The flagged regex constructs a pattern from
interfaceName, but since this parameter only receives hardcoded string literals ('Env'and'ProductionEnv'at lines 83-84), there's no user-controlled input vector. The risk is minimal for this build-time script.If you want to be extra defensive, you could escape the
interfaceNameor use a static pattern:♻️ Optional: Use literal patterns
-const ensureFields = (source: string, interfaceName: string) => { - const pattern = new RegExp( - `interface ${interfaceName} \\{([\\s\\S]*?)\\n\\t\\}`, - 'm', - ); +const ensureFields = ( + source: string, + interfaceName: 'Env' | 'ProductionEnv', +) => { + const patterns: Record<'Env' | 'ProductionEnv', RegExp> = { + Env: /interface Env \{([\s\S]*?)\n\t\}/m, + ProductionEnv: /interface ProductionEnv \{([\s\S]*?)\n\t\}/m, + }; + const pattern = patterns[interfaceName];
- Introduced a new `.dev.vars.example` file to provide a template for environment variables used in local development. - Included placeholders for authentication and database configuration, enhancing setup clarity for developers.
- Introduced a new authentication mode, `better-auth`, with corresponding environment variable support. - Updated navigation items to reflect the change from "Dashboard" to "Playground". - Enhanced various components and routes to conditionally render based on the selected authentication mode. - Improved session management and API key handling in the context of the new auth mode. - Updated README documentation to include details on the new authentication configuration.
…across components - Changed text and links in various components to reflect the transition from "Dashboard" to "Playground". - Updated loading messages and navigation items for consistency. - Enhanced layout configurations and descriptions to align with the new terminology.
- Introduced a new environment variable `ENABLE_ACTIVITY_LOGS` to control logging behavior, defaulting to true. - Updated README to include instructions for applying local D1 migrations. - Added a new script command `d1-db:migrate:local` for local database migrations. - Enhanced type definitions to include the new logging configuration.
- Removed the conditional check for the `AUTH_JWT_TOKEN` environment variable in the `getAuthMode` function. - Cleaned up the code to streamline the authentication mode retrieval process.
- Modified the workspace settings to specify the default formatter for TypeScript React files. - Updated task details and labels to replace "Dashboard" with "Playground" for consistency across the development environment.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.vscode/tasks.json:
- Around line 15-17: The tasks.json "group" object is missing the required
"kind" property; update the "group" object (the JSON object named group with
property isDefault) to include a "kind" field set to the appropriate value
(e.g., "build" or "test" depending on task intent) so the schema is satisfied
and the default grouping behavior applies; ensure you add the "kind" property
alongside isDefault in the same group object.
In `@apps/app/lib/auth-mode.ts`:
- Around line 28-40: When isJwtMode() is true and no JWT token is configured,
fail fast instead of returning headers without Authorization: inside
buildDeepcrawlHeaders check JWT_TOKEN (process.env.AUTH_JWT_TOKEN) and if it's
falsy throw a clear, descriptive Error (e.g., "JWT mode enabled but
AUTH_JWT_TOKEN is not set") so callers immediately see the misconfiguration;
keep existing behavior of returning nextHeaders when not in JWT mode or when the
token is present and still set the Authorization header via
nextHeaders.set('authorization', `Bearer ${JWT_TOKEN}`).
In `@apps/workers/v0/README.md`:
- Around line 225-227: Update the fenced code block in the README snippet that
currently shows "Authorization: Bearer <token>" so it includes a language
specifier (e.g., add "http", "text", or "bash" after the opening backticks) to
ensure consistent rendering; modify the triple-backtick opener for that block to
include the chosen language.
🧹 Nitpick comments (7)
apps/workers/v0/src/utils/tail-jobs/post-processing.ts (1)
56-71: Consider extractingresolveBoolto a shared utility.This boolean resolution helper appears useful for parsing environment variables across the codebase. If similar parsing is needed elsewhere (e.g., for other feature flags), consider moving it to a shared utils module.
♻️ Example extraction
Create a new file like
@/utils/env.ts:export const resolveBool = ( value: string | boolean | undefined, fallback: boolean, ): boolean => { if (value === undefined) { return fallback; } if (typeof value === 'boolean') { return value; } const normalized = value.trim().toLowerCase(); if (!normalized) { return fallback; } return ['1', 'true', 'yes', 'y', 'on'].includes(normalized); };apps/app/.env.example (1)
10-15: Consider documentingNEXT_PUBLIC_AUTH_MODEif clients need it.
getAuthMode()also checksNEXT_PUBLIC_AUTH_MODE; adding it here (or notingAUTH_MODEis server-only) avoids confusion.Optional doc addition
# Auth mode for dashboard server # Supported: better-auth | jwt | none AUTH_MODE=better-auth +# Optional: client-side override +NEXT_PUBLIC_AUTH_MODE= # Used when AUTH_MODE=jwt AUTH_JWT_TOKEN=scripts/jwt-mint.ts (2)
7-17: Use an interface forInputs.
This object shape should be declared with an interface per repo conventions.As per coding guidelines: `**/*.{ts,tsx}`: Use interfaces over types in TypeScript.♻️ Suggested change
-type Inputs = { +interface Inputs { secret?: string; sub?: string; email?: string; name?: string; issuer?: string; audience?: string; expiresInHours?: number; writeDevVars?: boolean; writeDevVarsProduction?: boolean; -}; +}
340-342: Avoidconsole.error; use stderr or a logger.
This script should follow the repo logging convention.As per coding guidelines: `**/*.{ts,tsx}`: No console statements in production code - use custom logging utilities for development.🔧 Suggested change
-run().catch((error) => { - console.error(`❌ Failed to mint JWT: ${error.message}`); - process.exit(1); -}); +run().catch((error) => { + process.stderr.write(`❌ Failed to mint JWT: ${error.message}\n`); + process.exit(1); +});apps/app/lib/navigation-config.ts (1)
42-44: Consider fixing the typo inNAVGATION_ITEMS.The constant name
NAVGATION_ITEMSappears to be missing a letter (should beNAVIGATION_ITEMS). While this is an existing naming issue rather than a new one, it may be worth correcting during this refactor sincegetNavigationItemsnow exposes this pattern more broadly.apps/app/app/(dashboard)/layout.tsx (1)
58-68: Consider distinguishing auth failures from missing sessions.The
.catch(() => { throw redirect('/login'); })pattern redirects to login for any error fromauthGetSession, including network failures or server errors. This could mask legitimate issues where the user is authenticated but the auth service is temporarily unavailable.If this is intentional (fail-safe to login), the current approach is acceptable. Otherwise, consider logging the error or showing an error state for non-auth failures.
♻️ Optional: Add error logging before redirect
const currentSession = isBetterAuth - ? await authGetSession().catch(() => { + ? await authGetSession().catch((error) => { // Auth failed - redirect to login + console.error('Auth session fetch failed:', error); throw redirect('/login'); }) : null;apps/app/components/user/user-dropdown.tsx (1)
82-95: Consider extracting props to an interface.Per coding guidelines, prefer interfaces over inline type annotations for component props. This improves readability and allows for extension.
♻️ Suggested refactor to use interface
+interface UserDropdownProps { + session: Session | null; + redirectLogout?: string; + navigationMode?: NavigationMode; + enableLayoutViewToggle?: boolean; + authMode?: AuthMode; + className?: string; +} + export function UserDropdown({ session, redirectLogout, navigationMode, enableLayoutViewToggle = true, authMode, className, -}: { - session: Session | null; - redirectLogout?: string; - navigationMode?: NavigationMode; - enableLayoutViewToggle?: boolean; - authMode?: AuthMode; - className?: string; -}) { +}: UserDropdownProps) {As per coding guidelines: "Use interfaces over types in TypeScript"
- Changed the default formatter for TypeScript React files to `fronterior.biome-monorepo` and disabled format on save. - Updated VSCode extension recommendations to reflect the new formatter. - Reorganized component aliases in `components.json` for clarity. - Added `@deepcrawl/ui` to the list of transpile packages in the Next.js configuration. - Introduced 'use no memo' directive in several components to optimize rendering. - Enhanced the `biome.jsonc` file to exclude additional directories from processing.
- Increased the default log limit from 5 to 10 in both the configuration and documentation files. - Added a margin-bottom class to the LogsDataGridCard component for improved layout.
…t management - Added a new package `@deepcrawl/runtime` for centralized environment variable management and runtime configuration helpers. - Implemented scripts to generate example environment files and synchronize local environment variables across applications. - Updated `.env.example` files for various applications to reflect new environment variable structure. - Enhanced the `auth-mode` handling by utilizing shared logic from the new runtime package. - Introduced new environment variables for better configuration and local development support.
…efactor auth middleware - Introduced a new internal document, `AUTH_MODES_AND_WORKERS.md`, detailing authentication modes and backends. - Removed deprecated API key validation route from the auth worker. - Updated middleware to enhance handling of authentication modes and better integration with the new documentation. - Refactored environment variable examples to clarify configuration for Better Auth integration. - Improved logging and error handling in authentication middleware for better debugging.
…h URL handling - Changed references in documentation and scripts from `.env.development.local` to `.env` for consistency in local development setup. - Added `NEXT_PUBLIC_BETTER_AUTH_URL` to environment variable examples for better clarity in browser bundle configurations. - Refactored `getAuthBaseURL` and validation functions to streamline URL handling and ensure proper validation of authentication configurations. - Updated scripts to reflect the new environment variable structure, enhancing the overall developer experience.
- Bumped versions of several dependencies in package.json files, including @biomejs/biome, shadcn, turbo, wrangler, and others. - Updated catalog dependencies in pnpm-workspace.yaml for improved compatibility and features. - Enhanced app-specific dependencies in apps/app/package.json and apps/workers/v0/package.json. - Made minor version updates to eslint-config and ui packages for better linting and UI component functionality.
…on endpoints - Replaced hardcoded rate limit settings with centralized configurations for API key validation and authentication endpoints. - Introduced a new `auth-rate-limit.ts` file to define rate limit rules and configurations. - Updated `auth.config.ts` to utilize the new rate limit settings, enhancing maintainability and clarity. - Removed deprecated rate limit constants from `constants.ts` to streamline the configuration process.
- Replaced parsing logic with safe parsing for options in 'getMarkdown', 'readUrl', and 'extractLinks' operations. - Enhanced error handling to return structured error responses with validation status and type. - Streamlined the integration of validated options into API calls, improving robustness and clarity.
… integrate into layout - Introduced a new `DeployAttributionBanner` component to display deployment attribution information. - Integrated the banner into the `RootLayout` for visibility across the application. - Refactored `DeepcrawlLogo` component to support additional props for better flexibility. - Updated various components to ensure proper layout and spacing adjustments in response to the new banner. - Enhanced `PageContainer` and other layout components for improved responsiveness and visual consistency.
- Changed dependency versions in package.json to use 'catalog:' for several packages, including @biomejs/biome, @changesets/*, @orpc/*, better-auth, dotenv-cli, husky, lint-staged, rimraf, shadcn, turbo, wrangler, and zod. - Updated pnpm-lock.yaml to reflect the new catalog references and ensure consistency across the workspace. - Adjusted pnpm-workspace.yaml to align with the new catalog specifications for improved dependency management.
- Introduced normalization functions for API keys to ensure consistent handling across different sources (body, headers). - Updated the `apiKeyAuthMiddleware` to prefer the canonical Authorization header for API key extraction. - Added logging for missing `BETTER_AUTH_URL` in both `apiKeyAuthMiddleware` and `cookieAuthMiddleware` to improve debugging. - Refactored JWT-related input handling in the JWT minting script for better clarity and user experience. - Fixed typos in variable names and improved type definitions for better code quality. - Added tests for better-auth URL resolution and trusted origins to ensure robustness.
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
apps/app/components/playground/playground-skeleton.tsx (1)
7-31:⚠️ Potential issue | 🟡 MinorUse single quotes for JSX string literals.
Current JSX attributes use double quotes in the modified lines.✅ Suggested update
- <PageTitle - className="mx-auto mt-28 mb-10 w-full text-center md:mt-[13svh]" - description="Make any website data AI-ready for agents" - desPos="bottom" - title="What would you like to see?" - titleClassName="mb-2" - titleSize="3xl" - > - <span className="text-muted-foreground"> + <PageTitle + className='mx-auto mt-28 mb-10 w-full text-center md:mt-[13svh]' + description='Make any website data AI-ready for agents' + desPos='bottom' + title='What would you like to see?' + titleClassName='mb-2' + titleSize='3xl' + > + <span className='text-muted-foreground'> API Playground for Deepcrawl </span> </PageTitle> @@ - <div className="container relative mx-auto flex size-full h-svh items-center justify-center"> + <div className='container relative mx-auto flex size-full h-svh items-center justify-center'> <div className="flex w-full flex-col items-center justify-center gap-4 max-sm:mb-16"> <LoadingSpinner size={25} /> - <DeepcrawlLogoText className="animate-pulse text-center text-lg sm:text-2xl"> + <DeepcrawlLogoText className='animate-pulse text-center text-lg sm:text-2xl'> Deepcrawl Playground & Dashboard is loading </DeepcrawlLogoText> </div> </div>As per coding guidelines “Use single quotes instead of double quotes for strings”.
packages/auth/src/configs/constants.ts (1)
87-87:⚠️ Potential issue | 🟡 MinorRename
EXpiresIntoexpiresInfor consistency with camelCase convention.The property has inconsistent casing (capital X) and needs to be renamed. Update the constant definition in
packages/auth/src/configs/constants.tsand all 8 usages across:
packages/auth/src/templates/magic-link.tsxpackages/auth/src/templates/organization-invitation.tsxpackages/auth/src/templates/password-reset.tsxpackages/auth/src/templates/email-verification.tsxpackages/auth/src/configs/auth.config.ts(4 usages)📝 Proposed fix
export const EMAIL_CONFIG = { /** * Time in seconds until the magic link expires. * `@default` (60 * 5) // 5 minutes * */ - EXpiresIn: { + expiresIn: { resetPassword: 3600, // 1 hour magicLink: 60 * 5, // 5 minutes emailVerification: 3600, // 1 hour invitation: 2 * 24 * 60 * 60, // By default, it's 48 hours (2 days) }, };packages/auth/src/configs/auth.config.ts (4)
322-348:⚠️ Potential issue | 🟡 MinorReplace console statements with the shared logger.
Lines 325, 346 use
console.warnandconsole.errorin production code paths.As per coding guidelines: "No console statements in production code - use custom logging utilities for development".
🛠️ Suggested fix
sendMagicLink: async ({ email, token }) => { if (!(emailEnabled && resend)) { - console.warn('⚠️ Magic link email not sent - Resend not configured'); + logWarn('⚠️ Magic link email not sent - Resend not configured'); return; } // ... } catch (error) { - console.error('❌ Failed to send magic link email:', error); + logWarn('❌ Failed to send magic link email:', error); }
378-383:⚠️ Potential issue | 🟡 MinorReplace console.error with the shared logger.
Line 381 uses
console.errorin production code.As per coding guidelines: "No console statements in production code - use custom logging utilities for development".
406-411:⚠️ Potential issue | 🟡 MinorReplace console.error with the shared logger.
Line 409 uses
console.errorin production code.As per coding guidelines: "No console statements in production code - use custom logging utilities for development".
413-441:⚠️ Potential issue | 🟡 MinorReplace console statements with the shared logger.
Lines 416, 438, 439 use
console.warnandconsole.errorin production code.As per coding guidelines: "No console statements in production code - use custom logging utilities for development".
🤖 Fix all issues with AI agents
In `@apps/app/components/deepcrawl-logo.tsx`:
- Around line 57-72: The fallback span branch is unreachable because href is
given a default of '/' in the destructuring; either remove the dead branch or
stop defaulting href so the span can be used: locate the deepcrawl-logo
component where props are destructured (the variables asChild/_asChild, href =
'/', children = brandName, ...linkProps) and either 1) delete the if (!href)
return <span...> block and always render <Link> with mergedClassName and
children, or 2) remove the href = '/' default so href can be undefined/empty and
the existing if (!href) branch (which renders a span with mergedClassName and
children) becomes reachable; update the prop types if you choose option 2 to
allow href to be optional.
In `@apps/app/components/deploy-attribution-banner.tsx`:
- Around line 269-329: Replace the awkward user-facing string "This app is maybe
deployed with" in the JSX of the Deploy Attribution Banner component with a
clearer phrasing (e.g. "This app may be deployed with" or "This app appears to
be deployed with") — edit the span containing that text in the
deploy-attribution-banner component (the JSX near the code element and Link
components) so the displayed message is grammatically correct; no other logic
(dismissBanner, TOP_OFFSET_CSS_VAR use, or button handlers) should be changed.
In `@apps/app/components/layout/header-navigation-layout.tsx`:
- Around line 12-17: Rename the boolean prop hideAuthEntries to
shouldHideAuthEntries in the HeaderNavigationLayoutProps interface and
everywhere inside the HeaderNavigationLayout component (including destructuring,
conditional checks, prop forwarding to children, and default values); update the
prop name in the component signature and any references to the old name
(hideAuthEntries) so all conditionals and JSX use shouldHideAuthEntries instead,
and run/adjust any local tests or prop usages inside this file to match the new
identifier.
In `@apps/app/hooks/playground/use-playground-operations.ts`:
- Around line 160-163: The default branch in the switch inside
use-playground-operations.ts returns only { error: string }, causing
inconsistent responses for handlePlaygroundClientErrorResponse; update the
default return to match the other validation failure shapes by returning the
full error object (include errorType and status) — e.g., set errorType to a
meaningful identifier like "unknown_operation" and status to a 4xx code (400)
and keep the toast.error call and returned error message consistent with other
branches so downstream logic receives a uniform structure.
In `@apps/workers/v0/package.json`:
- Around line 23-38: Add Cloudflare Workers-specific configuration and usage
changes for the listed packages: add a Browser Rendering binding in
wrangler.toml (browser = { binding = "...", type = "browser" }) and enable
node_compat/nodejs_compat as needed (for `@cloudflare/puppeteer` and transitive
deps like `@scalar/hono-api-reference`) and ensure compatibility_date is set;
change usage of `@cloudflare/puppeteer` so you launch with the binding
(puppeteer.launch(env.MYBROWSER)); import and instantiate Upstash Redis from the
Cloudflare entrypoint (use import from "@upstash/redis/cloudflare" and
Redis.fromEnv(env)); and enable nodejs_compat if bundler reports Node API issues
for `@scalar/hono-api-reference`. Ensure these configuration/usage updates are
applied alongside the package entries "@cloudflare/puppeteer", "@upstash/redis",
and "@scalar/hono-api-reference".
In `@apps/workers/v0/src/middlewares/jwt-auth.hono.ts`:
- Around line 93-97: The middleware currently checks const secret =
c.env.JWT_SECRET and calls logError('❌ JWT_SECRET is missing while
AUTH_MODE=jwt') then returns next(), which lets requests continue; change this
to fail fast by returning an explicit 401 response when JWT_SECRET is missing
(keep the logError message), e.g. respond with an unauthorized status and
message instead of invoking next(), so the jwt-auth middleware blocks requests
when AUTH_MODE=jwt and the secret is not configured.
In `@packages/auth/src/utils/logger.ts`:
- Around line 1-16: logWarn and logError currently always call console and
violate the "no console in production" rule; update both functions (logWarn and
logError) to short-circuit when process.env.NODE_ENV === 'production' (same
pattern used in logDebug) so they only call console.warn/console.error in
non-production environments, preserving existing signatures and behavior
otherwise.
In `@packages/eslint-config/package.json`:
- Around line 12-21: Update the undocumented `@next/eslint-plugin-next` version in
package.json: replace the "@next/eslint-plugin-next" dependency value (the entry
named "@next/eslint-plugin-next") with a documented patch release (preferably
"15.5.9" or "15.5.7"), then reinstall dependencies and update the lockfile so
the change is persisted; ensure any references to Next.js linting in docs or
scripts follow the Next 15.5 guidance about using the ESLint CLI
(eslint.config.mjs) if applicable.
In `@packages/runtime/src/env.ts`:
- Around line 42-267: The new exported API surface (ENV_VARS,
getEnvVarsForTarget, listEnvKeysForTarget) lacks unit tests; add tests that
import these symbols and assert: ENV_VARS contains expected entries (including
any newly added public vars), getEnvVarsForTarget(target) returns only entries
whose .targets include the given EnvTarget, and listEnvKeysForTarget(target)
returns the corresponding keys array; include edge cases (unknown target ->
empty array) and test a public-var like 'NEXT_PUBLIC_APP_URL' to cover the
public env path.
- Around line 20-27: Replace the type alias EnvVar with an interface declaration
to follow project coding guidelines; change "export type EnvVar = { ... }" to
"export interface EnvVar { ... }" preserving all members (key, group:
EnvVarGroup, targets: readonly EnvTarget[], optional description, example,
secret) and keep the export name the same so existing references to EnvVar
elsewhere (imports, function signatures, etc.) continue to work without changes.
In `@scripts/jwt-mint.ts`:
- Around line 25-73: The parseArgs function is mis-parsing when a flag that
requires a value is immediately followed by another flag (e.g., "--secret --sub
foo"); update parseArgs to check that the candidate value exists and does not
start with '--' before assigning (for flags '--secret', '--sub', '--email',
'--name', '--issuer', '--audience', '--expires-in'), only increment i when a
real value was consumed, and for missing values emit a clear error (throw or
console.error + process.exit(1)) so the script fails fast; also preserve the
Number(...) conversion for '--expires-in' when a valid numeric value is present.
🧹 Nitpick comments (10)
apps/app/components/api-keys/api-keys-page-client.tsx (1)
74-87: Error state is missingPageHeader, creating UI inconsistency.The skeleton state (lines 24-66) and success state (lines 90-123) both render
PageHeaderbeforePageContainer, but the error state only rendersPageContainer. This means users encountering an error won't see the "Your API Keys" title, making the error context less clear.Additionally, consider adding
role="alert"to the error message for better screen reader accessibility.♻️ Suggested fix for consistency and accessibility
if (error) { return ( + <> + <PageHeader + description="Manage your API keys for accessing Deepcrawl services." + title="Your API Keys" + /> <PageContainer> <Card className="bg-card"> <CardContent className="pt-6"> - <div className="py-8 text-center"> + <div className="py-8 text-center" role="alert"> <p className="text-destructive"> Failed to load API keys. Please try again. </p> </div> </CardContent> </Card> </PageContainer> + </> ); }apps/app/hooks/playground/use-playground-operations.ts (2)
175-181: Fragile string comparison for error detection.Using
error.message === 'Unauthorized'is brittle—if the error message changes or varies across different error sources, this check will silently fail. Consider using a typed error class or error code instead.♻️ Suggested approach
+// Define at module level or in a shared error types file +interface PlaygroundApiKeyError extends Error { + code?: 'UNAUTHORIZED' | 'KEY_NOT_FOUND' | 'KEY_CREATION_FAILED'; +} } catch (error) { // If we can't create/read the key, guide the user to the API Keys page. // Avoid noisy toasts when the user is not logged in. - if (!(error instanceof Error && error.message === 'Unauthorized')) { + const isUnauthorized = + error instanceof Error && + ((error as PlaygroundApiKeyError).code === 'UNAUTHORIZED' || + error.message === 'Unauthorized'); + if (!isUnauthorized) { showMissingPlaygroundApiKeyToast(); } }Alternatively, have
ensurePlaygroundApiKeythrow a custom error type that can be reliably identified.
79-92: Consider moving helper to hook level.
showMissingPlaygroundApiKeyToastis defined inside thetryblock but only needs access torouter. Moving it to the hook level (alongsideexecuteApiCall) would improve readability and make the try block more focused on the operation logic.packages/auth/src/utils/better-auth-url.ts (1)
23-50: Prefer arrow-function exports for consistency.This file already uses
constexports elsewhere; aligning these helpers keeps the style uniform.As per coding guidelines: "Use arrow functions over function expressions."♻️ Suggested update
-export function resolveBetterAuthApiBaseUrl(rawUrl: string): string { +export const resolveBetterAuthApiBaseUrl = (rawUrl: string): string => { const cleaned = rawUrl.trim(); if (!cleaned) { throw new Error('Better Auth base URL is required.'); } @@ -} +}; @@ -export function resolveBetterAuthOriginUrl(rawUrl: string): string { +export const resolveBetterAuthOriginUrl = (rawUrl: string): string => { const apiBase = resolveBetterAuthApiBaseUrl(rawUrl); return apiBase.slice(0, apiBase.length - API_AUTH_PATH.length); -} +};packages/auth/src/__tests__/trusted-origins.test.ts (1)
31-40: Consider adding a negative test forisDevelopment=false.The test verifies dev origins are included when
isDevelopment=true, but there's no explicit test confirming they are excluded whenisDevelopment=false(or when omitted, since the default isfalse).📝 Suggested additional test
test('resolveTrustedOrigins excludes local dev origins when isDevelopment=false', () => { const origins = resolveTrustedOrigins({ appURL: 'https://example.com', authURL: 'https://auth.example.com', isDevelopment: false, }); assert.ok(!origins.includes('http://localhost:3000')); assert.ok(!origins.includes('http://localhost:8787')); assert.ok(!origins.includes('http://localhost:8080')); });packages/runtime/src/api-rate-limit.ts (1)
1-6: Consider using an interface forApiRateLimitRule.Per coding guidelines, interfaces are preferred over types for object shapes.
ApiRateLimitTieris correctly a type (union), butApiRateLimitRulerepresents a pure object shape.♻️ Suggested change
export type ApiRateLimitTier = 'free' | 'pro'; -export type ApiRateLimitRule = Readonly<{ - limit: number; - window: string; -}>; +export interface ApiRateLimitRule { + readonly limit: number; + readonly window: string; +}As per coding guidelines: Use interfaces over types in TypeScript.
packages/runtime/src/auth-rate-limit.ts (1)
1-4: Consider using an interface forAuthenticationRateLimitRule.Similar to
ApiRateLimitRule, this is a pure object shape that could be expressed as an interface per coding guidelines.♻️ Suggested change
-export type AuthenticationRateLimitRule = Readonly<{ - window: number; - max: number; -}>; +export interface AuthenticationRateLimitRule { + readonly window: number; + readonly max: number; +}As per coding guidelines: Use interfaces over types in TypeScript.
apps/app/components/deploy-attribution-banner.tsx (1)
100-119: Consider removing or documenting the commented-out NODE_ENV check.The commented code at lines 101-103 suggests the banner might have been intended to hide in non-production environments. If this is intentional debug behavior, consider adding a brief comment explaining why it's disabled. If it's leftover code, remove it.
apps/app/lib/navigation-config.ts (1)
80-82: Export theNavigationItemsOptionsinterface for external consumers.The interface is used as a parameter type for the exported
getNavigationItemsfunction but isn't exported itself. Consumers who want to type their options object won't have access to this type.🔧 Suggested fix
-interface NavigationItemsOptions { +export interface NavigationItemsOptions { hideAuthEntries?: boolean; }apps/workers/v0/src/middlewares/jwt-auth.hono.ts (1)
58-78: Avoid type assertion; align return type withSessioninterface.The
as Sessioncast on line 78 may hide type mismatches between the constructed object and the actualSessiontype from@deepcrawl/auth/types. If theSessioninterface changes, this won't produce compile-time errors. Consider ensuring the object structure matchesSessionexactly or using a type-safe builder.
- Enhanced the JWT minting process by introducing new options for writing JWT settings to environment files. - Updated documentation to clarify the configuration of JWT-related variables, including `JWT_SECRET`, `JWT_ISSUER`, and `JWT_AUDIENCE`. - Removed `JWT_ISSUER` and `JWT_AUDIENCE` from the environment variable manifest as they are now considered non-secret. - Added a new task in VSCode for minting JWT tokens interactively. - Introduced tests for environment variable handling to ensure correctness and reliability.
… and functionality - Removed unnecessary prop from `HeaderNavigationLayout` and replaced it with a more descriptive `shouldHideAuthEntries`. - Simplified the `DeepcrawlLogo` component by removing redundant conditional rendering. - Corrected text in `DeployAttributionBanner` for grammatical accuracy. - Adjusted class names in `PageTitle` and `Sidebar` components for better CSS utility usage. - Enhanced error handling in `usePlaygroundOperations` to include additional error details. - Updated JWT middleware to return a proper unauthorized response when the secret is missing. - Improved logging functions to prevent output in production environment. - Made minor adjustments to tooltip and global styles for consistency.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@apps/app/components/page-elements.tsx`:
- Line 61: The title rendering uses two different Tailwind utilities — the
string-title branch uses 'wrap-break-word text-wrap font-semibold' while the
ReactElement-title branch still uses 'break-words'; update the ReactElement
branch to use the same v4 utility (replace 'break-words' with 'wrap-break-word'
and keep any accompanying classes like 'text-wrap' and 'font-semibold' for
parity) so both code paths render titles with identical Tailwind behavior;
search for the class 'break-words' in the title rendering logic and align it to
the 'wrap-break-word' variant used in the string branch.
In `@apps/workers/v0/src/middlewares/jwt-auth.hono.ts`:
- Around line 99-132: The middleware currently calls next() for missing or
invalid JWTs which allows unauthenticated access when AUTH_MODE=jwt; update the
logic in the jwt-auth middleware (around getBearerToken, verify,
resolveJwtSession, and the existing next() calls) to check c.env.AUTH_MODE ===
'jwt' and, when true, return an HTTP 401 response for (a) no token returned from
getBearerToken, (b) resolveJwtSession returning falsy (missing sub), and (c) any
verification error caught in the try/catch instead of calling next(); preserve
existing logging (logWarn/logDebug) but replace the fallthrough with a 401
response so only valid tokens set c.set('session') and proceed.
In `@packages/ui/src/components/ui/sidebar.tsx`:
- Line 275: Update the Tailwind important modifier and split the long class
string in the sidebar component: replace occurrences of "!duration-0" with the
v4 postfix "duration-0!" (e.g., change
"group-data-[dragging=true]_*:!duration-0" to
"group-data-[dragging=true]_*:duration-0!"), and break the combined class string
like "group-data-[dragging=true]:duration-0!
group-data-[dragging=true]_*:!duration-0" into two smaller string entries so no
single line exceeds the 80-character width (apply the same fixes for the other
occurrence with the same class pattern).
In `@packages/ui/src/components/ui/tooltip.tsx`:
- Line 55: The TooltipPrimitive.Arrow JSX element uses a double-quoted, overly
long className string; change it to use single quotes and split the class list
across multiple concatenated or template lines to respect the 80-character max
and Biome style. Locate TooltipPrimitive.Arrow in tooltip.tsx, replace
className="..." with className='...' and break the CSS utility tokens into
multiple string segments (or use an array join) so each source line stays within
80 chars, preserving all classes (bg-primary, fill-primary, dark:bg-accent,
dark:fill-accent, z-50, size-2.5, translate-y-[calc(-50%-2px)], rotate-45,
rounded-[2px]) and run the formatter.
🧹 Nitpick comments (2)
apps/app/components/deploy-attribution-banner.tsx (2)
21-25: Consider usinginterfaceinstead oftypefor object shapes.Per coding guidelines, prefer
interfaceovertypefor object type definitions.♻️ Suggested refactor
-export type DeployAttributionBannerState = { +export interface DeployAttributionBannerState { canRender: boolean; isDismissed: boolean; isRendered: boolean; -}; +}As per coding guidelines: "Use interfaces over types in TypeScript".
100-103: Remove or restore commented-out code.This commented-out NODE_ENV check appears to be leftover debug code. Consider either removing it entirely or implementing it properly if the behavior is needed.
- Introduced a new function to read environment file keys, ensuring necessary keys are present. - Updated the JWT minting process to re-add placeholder for `AUTH_JWT_TOKEN` if removed by the user. - Ensured non-secret JWT-related variables are initialized even if left blank by the user. - Improved handling of updates to environment files for better user experience and reliability.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@scripts/jwt-mint.ts`:
- Around line 26-109: The parseArgs function currently silently ignores unknown
CLI flags; update its default switch branch so that when key starts with '--' it
throws a descriptive Error (e.g. `Unknown flag: ${key}`) to fail fast and
surface typos, while leaving non-flag positional args unchanged; reference
parseArgs, Inputs and existing flag names like '--secret'/'--expires-in' when
making the change to ensure only true unknown flags trigger the error and you
don't accidentally skip the next argument.
- Changed `DeployAttributionBannerState` from a type to an interface for better extensibility. - Removed commented-out code in `computeCanRenderBanner` for cleaner code. - Adjusted class names in `PageTitle` and `Sidebar` components for improved CSS utility usage. - Enhanced CORS middleware to handle credentialed requests more robustly based on environment variables. - Updated JWT middleware to allow preflight requests and return proper unauthorized responses for missing tokens. - Improved tooltip component structure for better readability and maintainability.
- Added checks to skip empty keys and literal "--" delimiters in argument parsing. - Enhanced error handling to throw an error for unknown flags starting with "--".
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/workers/v0/src/middlewares/cors.hono.ts`:
- Around line 60-70: The guard preventing credentialed CORS is too strict: relax
the condition that builds trustedOrigins so it only requires appUrl and
betterAuthUrl (not apiUrl) before calling resolveTrustedOrigins; call
resolveTrustedOrigins with appURL: appUrl, authURL: betterAuthUrl, apiURL:
apiUrl (allowing apiUrl to be undefined per ResolveTrustedOriginsInput) and wrap
the result in a Set for trustedOrigins, otherwise fallback to new Set<string>().
In `@apps/workers/v0/src/middlewares/jwt-auth.hono.ts`:
- Around line 58-79: The returned object currently uses an unsafe "as Session"
cast while omitting nullable DB fields; update the session payload construction
in the function returning Session to explicitly include the nullable fields
impersonatedBy and activeOrganizationId set to null (and keep
ipAddress/userAgent null defaults), and remove the broad "as Session" type
assertion so TypeScript enforces the exact Session shape.
🧹 Nitpick comments (3)
apps/app/components/deploy-attribution-banner.tsx (3)
44-98: Consider using arrow functions per coding guidelines.The codebase guidelines prefer arrow functions over function declarations. While the current implementation is functionally correct and consistent within the file, converting to arrow functions would align with the project standards.
♻️ Example refactor for a few functions
-function notifyStoreListeners(): void { - for (const listener of storeListeners) { - listener(); - } -} +const notifyStoreListeners = (): void => { + for (const listener of storeListeners) { + listener(); + } +}; -function setStoreState(next: DeployAttributionBannerState): void { +const setStoreState = (next: DeployAttributionBannerState): void => { // ... body unchanged -} +};As per coding guidelines: "Use arrow functions over function expressions"
226-232: Consider simplifying nested ternary for readability.The logic is correct but the nested ternary can be harder to parse at a glance.
♻️ Proposed simplification
- const offsetPx = assumeRendered - ? state.isDismissed - ? 0 - : BANNER_HEIGHT_PX - : state.isRendered - ? BANNER_HEIGHT_PX - : 0; + const shouldOffset = assumeRendered ? !state.isDismissed : state.isRendered; + const offsetPx = shouldOffset ? BANNER_HEIGHT_PX : 0;
307-316: Redundant CSS variable setting in dismiss handler.The manual CSS variable update is redundant since
dismissBanner()updates the store state toisRendered: false, which triggers theuseEffecton line 240 to set the CSS variable to'0px'. The manual setting provides marginally faster visual feedback but duplicates logic.♻️ Proposed simplification
onClick={() => { dismissBanner(); - try { - document.documentElement.style.setProperty( - TOP_OFFSET_CSS_VAR, - '0px', - ); - } catch { - // Ignore - } }}
…n structure - Updated CORS middleware to remove unnecessary dependency on apiUrl for trusted origins. - Refactored JWT session handling to improve clarity and added null fields for impersonation and organization ID.
- Introduced a new GitHub Actions workflow to automatically sync the template branch with the main branch on push events. - Configured the workflow to run on an Ubuntu environment, ensuring the template remains a mirror of the main branch without manual commits. - Included steps for checking out the repository and configuring Git user details for the sync process.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/sync-template-main.yml:
- Around line 27-29: The current push step uses a blind force push ("git push
origin HEAD:refs/heads/template/main --force") which lets an older run overwrite
newer commits; change this to use "git push origin HEAD:refs/heads/template/main
--force-with-lease" and add job-level concurrency to the workflow (concurrency
key and cancel-in-progress: true) so stale runs are automatically cancelled;
update the step that contains the git push command and the workflow top-level to
include the concurrency block and replace "--force" with "--force-with-lease".
- Added concurrency settings to the sync workflow to manage simultaneous runs and prevent conflicts. - Updated the push command to use `--force-with-lease` for safer branch updates, ensuring that local changes are not overwritten unintentionally.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Enhancements
Documentation