-
Notifications
You must be signed in to change notification settings - Fork 484
External db sync clickhouse support #1054
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: external-db-sync
Are you sure you want to change the base?
Conversation
<!-- Make sure you've read the CONTRIBUTING.md guidelines: https://github.com/stack-auth/stack-auth/blob/dev/CONTRIBUTING.md --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added environment variable handling and logging interception for improved script execution visibility. * **Bug Fixes** * Enhanced error handling with structured error responses and logs. * **Chores** * Updated dependencies: arktype and react-dom. * Improved temporary work directory management and cleanup process. * Optimized script execution model for better performance. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
<!-- Make sure you've read the CONTRIBUTING.md guidelines: https://github.com/stack-auth/stack-auth/blob/dev/CONTRIBUTING.md --> # New - quick-links component: Better UI that is more aligned with current styling, as well as centered on page for better experience. <img width="812" height="342" alt="image" src="https://github.com/user-attachments/assets/3de105af-eff0-4ddd-92b4-20c4a388fb03" /> # Update - Updates sidebar with slight visualization to separate Overview, and FAQ links. <img width="268" height="471" alt="image" src="https://github.com/user-attachments/assets/7372f557-f4c5-42d8-9dea-8c3b1b0e5514" /> # fixes - Re-add removed mcp-setup.mdx into /content so its available again. - update meta.json to reflect re-added mc-setup.mdx - centered app-icons on overview page <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a comprehensive MCP setup guide covering major IDEs and AI clients. * Introduced QuickLinks navigation component for enhanced docs navigation. * **Documentation** * Refreshed overview with QuickLinks-based layout and preserved link targets. * Added ready-to-copy installation/config snippets and multi-client setup steps. * **Style** * Updated docs sidebar styling and grouping for clearer “Overview”/FAQ separation. * Centered app cards layout and adjusted header nav activation logic. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
<!-- Make sure you've read the CONTRIBUTING.md guidelines: https://github.com/stack-auth/stack-auth/blob/dev/CONTRIBUTING.md --> <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Adds a new Cmd+K spotlight command palette with AI docs chat and app navigation/install previews (dev-only trigger), plus a mock AI API, design guide consolidation, and minor layout/UX tweaks. > > - **Dashboard UX**: > - **Command Palette (Cmd+K)**: Implement `CmdKSearch` with nested results, visual previews, and keyboard navigation; `CmdKTrigger` added to header (development only). > - **App Discovery**: Show installed apps and uninstalled apps with preview, screenshots, tags, and one-click enable via `getUninstalledAppIds`. > - **AI Docs Chat**: Add `AIChatPreview` with streaming UI; integrates with the palette. > - **Mock AI API**: New `POST /api/ai-search` streaming endpoint using `ai` mock model and system prompt. > - **Sidebar/Layout**: Integrate palette into `sidebar-layout.tsx`; small header/spacing tweaks; minor `PacificaCard` class fix; `StackCompanion` drawer visibility/animation refinements. > - **Dev UX**: New dismissible `DevelopmentPortDisplay` and imported into root layout. > - **Dependencies**: Add `ai`, `@ai-sdk/openai`, and `react-markdown` to dashboard. > - **Docs**: Replace `DESIGN_GUIDE.md` with comprehensive `DESIGN-GUIDE.md`; update `AGENTS.md` with env var prefix rule. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 8640d15. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * AI-powered command palette/spotlight with rich previews, app discovery, and streaming AI responses. * Docs-backed AI search and an in-app conversational AI preview for contextual queries. * Dismissible development port banner visible in dev builds. * **Bug Fixes / UX** * Header layout and drawer/handle behavior improved for smoother interactions. * **Documentation** * Dashboard design guide consolidated and replaced with a single comprehensive design-system guide. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Konstantin Wohlwend <[email protected]>
<!-- Make sure you've read the CONTRIBUTING.md guidelines: https://github.com/stack-auth/stack-auth/blob/dev/CONTRIBUTING.md --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Interactive screenshot gallery with clickable thumbnails and left/right navigation * Full-screen screenshot preview modal with keyboard support (arrow keys, Escape) * App tiles are now fully clickable to open app details * **Content** * Enriched app store descriptions and expanded screenshot sets for multiple apps * **Style** * Updated dialog/layout styling for improved visual hierarchy <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai --> <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Adds rich app store descriptions and screenshots across apps, plus a full-screen screenshot preview with keyboard navigation and minor dialog layout tweaks. > > - **UI/UX** > - **Interactive Preview**: Full-screen screenshot preview modal with arrow/Escape key support in `components/app-store-entry.tsx`. > - **Layout**: Makes `DialogContent` in `.../@modal/(.)apps/[appId]/page-client.tsx` a flex column container. > - **Content & Data** > - **App Descriptions**: Populates `storeDescription` for multiple apps in `lib/apps-frontend.tsx`. > - **Screenshots**: > - Adds `getScreenshots` helper to generate image paths. > - Expands `screenshots` for numerous apps (e.g., `authentication`, `teams`, `rbac`, `payments`, `emails`, `data-vault`, `webhooks`, `vercel`). > - Updates `AppFrontend.screenshots` type to `(string | StaticImageData)[]`. > - Removes fallback copy; always renders `storeDescription` in `AppStoreEntry`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit b5e3b8a. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Konsti Wohlwend <[email protected]>
<!-- Make sure you've read the CONTRIBUTING.md guidelines: https://github.com/stack-auth/stack-auth/blob/dev/CONTRIBUTING.md --> # 🔒 Hidden Internal Endpoints from Public API Docs Internal endpoints were incorrectly appearing in public API documentation. Added hidden: true metadata to: - /internal/init-script-callback - Telegram notification callback - /internal/send-sign-in-invitation - Team invitation endpoint (public equivalent: /team-invitations/send-code) - /internal/projects/current - Internal project CRUD (adminRead, adminUpdate, adminDelete) - /internal/emails - Internal emails CRUD (adminList) # 🏷️ Fixed Endpoint Categorization (moved from "Others") Several endpoints were appearing in the "Others" category instead of their proper sections: - /emails/send-email -> Added tags: ["Emails"] - Notification preferences CRUD -> Added complete docs with tags: ["Emails"] - OAuth providers CRUD -> Changed tag from "OAuth Providers" → "Oauth" to match naming convention; added missing clientUpdate docs # 📝 API Documentation Improvements - Fixed duplicate API Key titles: Updated handlers to use dynamic summary (Create ${type} API key) instead of hardcoded "Create API key" for both user and team keys - Fixed empty Request section: API playground now only renders the Request panel when there are actual parameters or request body fields to display # 🎨 UserButton Component Fixes ### Component (packages/template/src/components/user-button.tsx): - Fixed hover effect centering (removed w-12 constraint, added p-1.5 padding) - Fixed showUserInfo not displaying (changed flex-grow w-0 → min-w-0) - Updated text styling with proper font sizes and theme-aware colors - Added hover:transition-none for snappy UX ### Documentation Demo (docs/src/components/stack-auth/stack-user-button-demo.tsx): - Added not-prose class to fix avatar not rendering in MDX context <img width="482" height="190" alt="image" src="https://github.com/user-attachments/assets/c7c847c8-3ca5-4a40-bb33-f89949b6dbad" /> | Old | New | |-----|-----| | <img src="https://github.com/user-attachments/assets/b0f2afe4-1499-49c0-946a-618a29876479" width="180" /> | <img src="https://github.com/user-attachments/assets/a358499c-f7e6-42c0-a0a0-2e6ad21728d2" width="180" /> | | Old | New | |-----|-----| | <img width="176" height="73" alt="image" src="https://github.com/user-attachments/assets/1e17d77e-e4df-484d-adf4-19fcdaa0b471" /> | <img width="198" height="76" alt="image" src="https://github.com/user-attachments/assets/61d95ca8-61e5-48db-8fd8-75335751622f" /> |
|
Skipped: This PR changes more files than the configured file change limit: ( |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Older cmux preview screenshots (latest comment is below)Preview ScreenshotsOpen Workspace (1 hr expiry) · Open Dev Browser (1 hr expiry) · Open Diff Heatmap Captured 5 screenshots for commit The Background services section heading The new ClickHouse app card on port 8133 added to the dev launchpad The ClickHouse entry (8133: ClickHouse) in the background services list Full page view of the Stack Auth Dev Launchpad showing all app cards including the new ClickHouse and MCPJam Inspector entries The new MCPJam Inspector app card on port 8126 added to the dev launchpad Generated by cmux preview system |
| const client = createClickhouseClient("external", body.timeout_ms); | ||
| const queryId = randomUUID(); | ||
| const resultSet = await Result.fromPromise(client.query({ | ||
| query: body.query, | ||
| query_id: queryId, | ||
| query_params: body.params, | ||
| clickhouse_settings: { | ||
| SQL_project_id: auth.tenancy.project.id, | ||
| SQL_branch_id: auth.tenancy.branchId, | ||
| }, | ||
| format: "JSONEachRow", | ||
| })); | ||
|
|
||
| if (resultSet.status === "error") { | ||
| const message = resultSet.error instanceof Error ? resultSet.error.message : null; | ||
| if (message === "Timeout error.") { | ||
| throw new KnownErrors.AnalyticsQueryTimeout(body.timeout_ms); | ||
| } | ||
| throw new KnownErrors.AnalyticsQueryError(message ?? "Unknown error"); | ||
| } | ||
|
|
||
| const rows = await resultSet.data.json<Record<string, unknown>[]>(); | ||
| const stats = await getQueryTimingStats(client, queryId); | ||
|
|
||
| return { | ||
| statusCode: 200, | ||
| bodyType: "json", | ||
| body: { | ||
| result: rows, | ||
| stats: { | ||
| cpu_time: stats.cpu_time_ms, | ||
| wall_clock_time: stats.wall_clock_time_ms, | ||
| }, | ||
| }, | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ClickHouse client created for executing analytics queries is never closed, causing a resource leak. Every API call leaves an open connection to ClickHouse.
View Details
📝 Patch Details
diff --git a/apps/backend/src/app/api/latest/analytics/query/route.tsx b/apps/backend/src/app/api/latest/analytics/query/route.tsx
index ce18dba8..eb22bbba 100644
--- a/apps/backend/src/app/api/latest/analytics/query/route.tsx
+++ b/apps/backend/src/app/api/latest/analytics/query/route.tsx
@@ -36,40 +36,44 @@ export const POST = createSmartRouteHandler({
}),
async handler({ body, auth }) {
const client = createClickhouseClient("external", body.timeout_ms);
- const queryId = randomUUID();
- const resultSet = await Result.fromPromise(client.query({
- query: body.query,
- query_id: queryId,
- query_params: body.params,
- clickhouse_settings: {
- SQL_project_id: auth.tenancy.project.id,
- SQL_branch_id: auth.tenancy.branchId,
- },
- format: "JSONEachRow",
- }));
+ try {
+ const queryId = randomUUID();
+ const resultSet = await Result.fromPromise(client.query({
+ query: body.query,
+ query_id: queryId,
+ query_params: body.params,
+ clickhouse_settings: {
+ SQL_project_id: auth.tenancy.project.id,
+ SQL_branch_id: auth.tenancy.branchId,
+ },
+ format: "JSONEachRow",
+ }));
- if (resultSet.status === "error") {
- const message = resultSet.error instanceof Error ? resultSet.error.message : null;
- if (message === "Timeout error.") {
- throw new KnownErrors.AnalyticsQueryTimeout(body.timeout_ms);
+ if (resultSet.status === "error") {
+ const message = resultSet.error instanceof Error ? resultSet.error.message : null;
+ if (message === "Timeout error.") {
+ throw new KnownErrors.AnalyticsQueryTimeout(body.timeout_ms);
+ }
+ throw new KnownErrors.AnalyticsQueryError(message ?? "Unknown error");
}
- throw new KnownErrors.AnalyticsQueryError(message ?? "Unknown error");
- }
- const rows = await resultSet.data.json<Record<string, unknown>[]>();
- const stats = await getQueryTimingStats(client, queryId);
+ const rows = await resultSet.data.json<Record<string, unknown>[]>();
+ const stats = await getQueryTimingStats(client, queryId);
- return {
- statusCode: 200,
- bodyType: "json",
- body: {
- result: rows,
- stats: {
- cpu_time: stats.cpu_time_ms,
- wall_clock_time: stats.wall_clock_time_ms,
+ return {
+ statusCode: 200,
+ bodyType: "json",
+ body: {
+ result: rows,
+ stats: {
+ cpu_time: stats.cpu_time_ms,
+ wall_clock_time: stats.wall_clock_time_ms,
+ },
},
- },
- };
+ };
+ } finally {
+ await client.close();
+ }
},
});
Analysis
ClickHouse client connection leak in analytics query handler
What fails: POST /api/latest/analytics/query handler in apps/backend/src/app/api/latest/analytics/query/route.tsx creates a ClickHouse client connection that is never closed, leaving open TCP connections to accumulate.
How to reproduce:
# Make repeated API calls to the analytics query endpoint
for i in {1..100}; do
curl -X POST http://localhost:8102/api/latest/analytics/query \
-H "Content-Type: application/json" \
-H "Authorization: Bearer $TOKEN" \
-d '{
"query": "SELECT 1",
"params": {},
"timeout_ms": 1000
}'
done
# Observe via netstat or connection monitoring:
netstat -an | grep ESTABLISHED | grep clickhouse | wc -l
# Without fix: connection count continuously increases
# With fix: connections are properly released back to the poolResult: Each API call leaves behind an unclosed ClickHouse client connection. The client maintains a pool of connections (default max 10 per host), and without explicit closure via client.close(), connections are never returned to the pool. Over time under moderate load, the connection pool exhausts and the service fails with connection limit errors.
Expected: According to ClickHouse JS client documentation, the close() method "closes all the open connections and releases resources." The same pattern is used correctly in apps/backend/src/lib/events.tsx (lines 196-217) with a try-finally block ensuring cleanup on all code paths.
Fix: Wrapped the handler logic in a try-finally block that calls await client.close() to ensure the connection is released on all code paths (success, timeout error, and generic error).
Preview Screenshots⏳ Preview screenshots are being captured... Workspace and dev browser links will appear here once the preview environment is ready. Generated by cmux preview system |
| cpu_time_ms: number, | ||
| wall_clock_time_ms: number, | ||
| }>(); | ||
| return stats.data[0]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| return stats.data[0]; | |
| const result = stats.data[0]; | |
| if (!result) { | |
| throw new Error('Failed to retrieve query timing statistics'); | |
| } | |
| return result; |
The getQueryTimingStats function returns stats.data[0] without checking if the data array is empty, which could cause a null reference error when the caller tries to access properties like cpu_time_ms.
View Details
Analysis
Missing null check in getQueryTimingStats allows undefined reference error
What fails: The getQueryTimingStats function in apps/backend/src/lib/clickhouse.tsx returns stats.data[0] without verifying the array is not empty. When ClickHouse returns no rows, this returns undefined, which causes a runtime crash in the caller when accessing stats.cpu_time_ms and stats.wall_clock_time_ms.
How to reproduce: When the ClickHouse system.query_log hasn't flushed or the query entry doesn't exist yet (e.g., due to timing issues), the SELECT query returns 0 rows. According to ClickHouse's JSON format specification, the data array is empty in this case, making data[0] undefined.
The error occurs in apps/backend/src/app/api/latest/analytics/query/route.tsx (lines 68-69) when the route handler tries to access:
cpu_time: stats.cpu_time_ms,
wall_clock_time: stats.wall_clock_time_ms,
Result: Runtime error: "Cannot read properties of undefined (reading 'cpu_time_ms')"
Expected: The function should check if stats.data[0] exists before returning it, and throw a descriptive error if no query timing data is available.
Fix applied: Added a null/undefined check after retrieving stats.data[0] to throw an error with context rather than allowing undefined to propagate to the caller.
No description provided.