Clickhouse sync fixing by BilalG1 · Pull Request #1198 · stack-auth/stack-auth

BilalG1 · 2026-02-14T00:04:39Z

Summary by CodeRabbit

Bug Fixes
- Enhanced sign-up rejection error messages to provide clearer feedback when administrator sign-up rules prevent registration.
Chores
- Infrastructure improvements to strengthen system reliability and readiness verification during application initialization.

vercel · 2026-02-14T00:04:45Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
stack-backend	Ready	Preview, Comment	Feb 16, 2026 7:37pm
stack-dashboard	Ready	Preview, Comment	Feb 16, 2026 7:37pm
stack-demo	Ready	Preview, Comment	Feb 16, 2026 7:37pm
stack-docs	Ready	Preview, Comment	Feb 16, 2026 7:37pm

coderabbitai · 2026-02-14T00:04:47Z

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

This PR introduces ClickHouse readiness checks across CI/CD workflows and the startup sequence, refactors ClickHouse migration SQL to use direct field access with explicit type casting, removes deduplicationId from Upstash request payloads, adds diagnostic logging to external database sync logic, changes metadata column types from JSON to String in the ClickHouse schema, and updates error message text in sign-up rule test snapshots.

Changes

Cohort / File(s)	Summary
GitHub Workflow ClickHouse Readiness `.github/workflows/db-migration-backwards-compatibility.yaml`, `.github/workflows/e2e-api-tests.yaml`, `.github/workflows/e2e-custom-base-port-api-tests.yaml`, `package.json`	Added "Wait on ClickHouse" steps that ping ClickHouse health endpoints after service dependency waits and before database initialization. Introduced new npm script to wait for ClickHouse readiness in startup sequence.
ClickHouse Migration SQL `apps/backend/scripts/clickhouse-migrations.ts`	Refactored TOKEN_REFRESH_EVENT_ROW_FORMAT_MUTATION_SQL to replace JSONExtractString utilities with direct typed field access and explicit null handling for refresh_token_id, is_anonymous, and ip_info nested fields. Updated WHERE clause condition from JSONHas check to nullable String comparison.
Upstash Payload Updates `apps/backend/src/app/api/latest/internal/external-db-sync/poller/route.ts`, `apps/backend/src/lib/external-db-sync-queue.ts`	Removed deduplicationId field from request payloads sent to Upstash while preserving flowControl and overall enqueue behavior.
External DB Sync Diagnostics `apps/backend/src/lib/external-db-sync.ts`	Added console.log diagnostic output when source fetch returns multiple rows in both syncPostgresMapping and syncClickhouseMapping operations.
Database Schema & Tests `packages/stack-shared/src/config/db-sync-mappings.ts`, `apps/e2e/tests/backend/endpoints/api/v1/auth/sign-up-rules.test.ts`	Changed ClickHouse analytics_internal.users table metadata columns (client_metadata, client_read_only_metadata, server_metadata) from JSON to String type. Updated sign-up rejection error message in test snapshots to reflect administrator sign-up rule rejection.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

clickhouse user sync #1159 — Modifies ClickHouse migration script and database sync mappings for user table metadata handling
fix clickhouse flaky tests #1196 — Changes metadata columns from JSON to String in ClickHouse schema and adjusts migration SQL
[Refactor] Improve CI Run Times by Reducing Test Flakiness and Speeding up Test Suite #1166 — Updates request payload handling in the external-db-sync poller route

Suggested reviewers

N2D4

Poem

🐰 ClickHouse awaits with patient ping,
Workflows dance in rhythm spring,
Metadata sheds its JSON chains,
Direct fields flow through migration veins,
A synchronized hop toward readiness! ✨

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch clickhouse-sync-fixing

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

apps/backend/src/app/api/latest/internal/external-db-sync/poller/route.ts

greptile-apps · 2026-02-16T19:40:14Z

Greptile Summary

This PR fixes several ClickHouse sync issues and improves CI reliability:

ClickHouse readiness checks: Adds wait-on steps for ClickHouse in all 3 CI workflows and the local start-deps script, ensuring ClickHouse is ready before db:init runs. This addresses flaky CI failures from race conditions.
ClickHouse migration SQL fix: Rewrites the $token-refresh event migration from the error-prone JSONExtractString(toJSONString(data), ...) pattern to the modern data.field::Type accessor syntax, with proper null handling via isNull/ifNull.
Schema type fix: Changes ClickHouse metadata columns (client_metadata, client_read_only_metadata, server_metadata) from JSON to String in both the migration and the sync mapping schema, matching the JSON.stringify() serialization already done in pushRowsToClickhouse.
Deduplication cleanup: Removes deduplicationId from the sync queue and poller, since deduplication is already handled by the deduplicationKey column with ON CONFLICT ... DO NOTHING.
Diagnostic logging: Adds console.log when more than 1 row is returned from source DB fetch during sync.
Test update: Updates sign-up rules test snapshot to match improved error message wording.

Confidence Score: 4/5

This PR is safe to merge — changes are well-scoped fixes for ClickHouse sync reliability with no risky architectural changes.
The changes are focused on fixing known ClickHouse sync issues: CI race conditions, migration SQL compatibility, schema type mismatches, and redundant deduplication. All changes are internally consistent. Minor points: the sign-up-rule-trigger migration was not updated to the same modern SQL pattern as the token-refresh migration (inconsistency, not a bug), and the new diagnostic logging uses console.log instead of the codebase's captureError pattern.
Pay attention to apps/backend/scripts/clickhouse-migrations.ts — the sign-up-rule-trigger migration still uses the older SQL pattern that was fixed for token-refresh.

Important Files Changed

Filename	Overview
apps/backend/scripts/clickhouse-migrations.ts	Token-refresh migration SQL updated from old JSONExtractString(toJSONString(data),...) to modern data.field::Type accessor syntax, fixing issues with null handling. The sign-up-rule-trigger migration was NOT updated with the same pattern, which is inconsistent but not a blocking issue.
apps/backend/src/lib/external-db-sync.ts	Added diagnostic console.log when more than 1 row returned from source DB fetch for both Postgres and ClickHouse sync paths. The logging uses console.log instead of the codebase's captureError pattern, which is a minor style inconsistency.
apps/backend/src/lib/external-db-sync-queue.ts	Removed deduplicationId from the outgoing request JSON. Deduplication is already handled by the deduplicationKey column and ON CONFLICT clause, so this removal is safe.
apps/backend/src/app/api/latest/internal/external-db-sync/poller/route.ts	Removed deduplicationId extraction and forwarding from the poller buildUpstashRequest. Consistent with the removal from the queue enqueue side. The processRequest function (direct sync) was already not using deduplicationId.
packages/stack-shared/src/config/db-sync-mappings.ts	Changed ClickHouse metadata columns (client_metadata, client_read_only_metadata, server_metadata) from JSON to String type. Consistent with pushRowsToClickhouse which JSON.stringifys these values, and consistent with the migration file's USERS_TABLE_BASE_SQL.
.github/workflows/e2e-api-tests.yaml	Added ClickHouse readiness check (wait-on http://localhost:8136/ping) before database initialization in CI workflow.
.github/workflows/db-migration-backwards-compatibility.yaml	Added ClickHouse readiness check before database initialization in the backwards-compatibility CI workflow.
.github/workflows/e2e-custom-base-port-api-tests.yaml	Added ClickHouse readiness check using custom port prefix (6736) before database initialization.
package.json	Added wait-until-clickhouse-is-ready script and integrated it into start-deps:no-delay to ensure ClickHouse is ready before db:init runs.
apps/e2e/tests/backend/endpoints/api/v1/auth/sign-up-rules.test.ts	Updated test snapshot to reflect the new sign-up rejection error message. Simple test maintenance change.

Sequence Diagram

sequenceDiagram
    participant CI as CI Workflow
    participant Docker as Docker Compose
    participant CH as ClickHouse
    participant PG as PostgreSQL
    participant Init as db:init

    CI->>Docker: deps-compose up
    Docker->>PG: Start PostgreSQL
    Docker->>CH: Start ClickHouse
    CI->>PG: wait-until-postgres-is-ready
    PG-->>CI: Ready
    CI->>CH: wait-on http://localhost:PORT/ping
    CH-->>CI: Ready (NEW)
    CI->>Init: pnpm run db:init
    Init->>PG: Run Prisma migrations
    Init->>CH: Run ClickHouse migrations
    Note over CH: Token-refresh migration<br/>uses new data.field::Type syntax
    Note over CH: Users table uses<br/>String instead of JSON<br/>for metadata columns

_{Last reviewed commit: 8d1a710}

greptile-apps

_{10 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-16T19:40:21Z

apps/backend/src/lib/external-db-sync.ts

+    if (rows.length > 1) {
+      console.log("db-sync-postgres: more than 1 row returned from source db fetch", { tenancyId, numRows: rows.length });
+    }


Consider using captureError instead of console.log

The rest of this file uses captureError for reporting unexpected conditions (lines 731, 735, 775, 779, 793). Using console.log here means this diagnostic signal won't appear in your error tracking/alerting system. If this is expected in normal operation (e.g., batch queries returning multiple rows), then a console.log is fine as a temporary debug aid. But if "more than 1 row" indicates an unexpected condition worth monitoring, captureError would be more consistent with the existing patterns in this file.

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

greptile-apps · 2026-02-16T19:40:22Z

Additional Comments (1)

apps/backend/scripts/clickhouse-migrations.ts
Inconsistent migration SQL pattern

TOKEN_REFRESH_EVENT_ROW_FORMAT_MUTATION_SQL was updated in this PR to use the modern data.field::Type accessor syntax, but SIGN_UP_RULE_TRIGGER_EVENT_ROW_FORMAT_MUTATION_SQL still uses the older JSONExtractString(toJSONString(data), 'field') pattern.

Looking at git history, the token-refresh migration was updated from the old pattern to the new one (commit 64d6b60) to fix issues, but this migration was left untouched — likely an oversight since it was added around the same time. Both mutations are structurally analogous (ALTER TABLE ... UPDATE data ... WHERE event_type = ... AND has_legacy_field). Consider updating this mutation to the newer pattern for consistency and to avoid the same issues that required fixing the token-refresh migration.

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

BilalG1 added 2 commits February 13, 2026 16:00

fix migration, add log

26dc88a

fix db sync tests

2c38d22

github-actions bot assigned BilalG1 Feb 14, 2026

BilalG1 added 5 commits February 13, 2026 16:17

fix lint

3f7c74f

fix test attempt

de5fb50

fix ch migration

64d6b60

fix

7c53cf2

Merge branch 'dev' into clickhouse-sync-fixing

b94f75d

BilalG1 commented Feb 16, 2026

View reviewed changes

apps/backend/src/app/api/latest/internal/external-db-sync/poller/route.ts Show resolved Hide resolved

BilalG1 added 2 commits February 16, 2026 09:40

fix test

b4b23d7

attempt test fix

63d8828

N2D4 approved these changes Feb 16, 2026

View reviewed changes

revert test timeout increase

8d1a710

BilalG1 marked this pull request as ready for review February 16, 2026 19:30

BilalG1 merged commit 907a983 into dev Feb 16, 2026
18 of 25 checks passed

BilalG1 deleted the clickhouse-sync-fixing branch February 16, 2026 19:30

greptile-apps bot reviewed Feb 16, 2026

View reviewed changes

N2D4 pushed a commit that referenced this pull request Feb 17, 2026

Clickhouse sync fixing (#1198)

98470d6

coderabbitai bot mentioned this pull request Mar 19, 2026

clickhouse sync contact channels, teams #1256

Closed

This was referenced Apr 6, 2026

various new clickhouse syncs #1267

Open

clickhouse new syncs and verify-data #1304

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clickhouse sync fixing#1198

Clickhouse sync fixing#1198
BilalG1 merged 10 commits intodevfrom
clickhouse-sync-fixing

BilalG1 commented Feb 14, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

vercel bot commented Feb 14, 2026 •

edited

Loading

Uh oh!

coderabbitai bot commented Feb 14, 2026 •

edited

Loading

Review failed

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Uh oh!

Uh oh!

Uh oh!

greptile-apps bot commented Feb 16, 2026

Uh oh!

greptile-apps bot left a comment

Uh oh!

greptile-apps bot Feb 16, 2026

Uh oh!

greptile-apps bot commented Feb 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

BilalG1 commented Feb 14, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

vercel bot commented Feb 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai bot commented Feb 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review failed

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Uh oh!

Uh oh!

Uh oh!

greptile-apps bot commented Feb 16, 2026

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Sequence Diagram

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot commented Feb 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

BilalG1 commented Feb 14, 2026 •

edited by coderabbitai bot

Loading

vercel bot commented Feb 14, 2026 •

edited

Loading

coderabbitai bot commented Feb 14, 2026 •

edited

Loading